-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Maps] Add endpoint to server for creating empty index & index pattern #94028
[Maps] Add endpoint to server for creating empty index & index pattern #94028
Conversation
…-and-points-back-end
Pinging @elastic/kibana-gis (Team:Geo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the exposed endpoint is to low level and would be better organized around the use case. For example, instead of having the endpoint be CREATE_INDEX_API_PATH
and return ImportResponse
, how about having a CREATE_DRAWING_LAYER
and return success or failure boolean.
There are still some details to be decided around the flow of the front end and how the back end will ultimately be used. At this point, this endpoint is built to be a placeholder with no unnecessary handling such as cluster settings or import pipeline details. We can confidently say we will need an index name and we will need mappings so these have been included. We'll plan to iterate on the API as we get further into it, test our use cases and get a better idea for what we need. This was similar to alerting where we built a best effort back end, followed by the front end which informed further changes to the back end. Consider this a very early phase in a slightly bigger effort. |
Following some offline discussion, this initial PR has been simplified to an endpoint with the following qualities:
The portion focused on adding data will be a separate PR following this one. The new endpoint introduced in this PR accepts |
…-and-points-back-end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx tested. works!
For testing, Kibana has a dedicated test-suite api_integration
, specifically for this kind of middle-ware:
I'd add a file create_index
and some tests there.
}, | ||
}; | ||
|
||
export async function createIndexSource( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: createIndexAndIndexPattern
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with this name, just seems long. Would you also be okay with posting to the endpoint api/maps/indexAndIndexPattern
? Trying to keep them consistent. I went with createIndexSource
and api/maps/indexSource
for brevity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some offline conversation, changed refs to doc source
const indexPatternsService = await dataPlugin.indexPatterns.indexPatternsServiceFactory( | ||
context.core.savedObjects.client, | ||
context.core.elasticsearch.client.asCurrentUser | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should do a privilige-check here to see if current-user can create index and index-pattern. If not, we can shortcut and return meaningful human-readable error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that we need to verify user permissions. I previously chatted with @nreese about this and we agreed we should probably leverage the same solution that file upload ends up using to resolve #41928. This might be in the form of a dedicated endpoint to check permissions, not sure. Regardless I think we probably want to handle it separately from this PR since it's not straightforward yet how best to do this.
…-and-points-back-end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one comment wrt tests, otherwise lgtm thx!
expect(resp.body.success).to.be(true); | ||
}); | ||
|
||
it('should fail to create index and pattern with invalid index', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these two test-stubs are dependent so I would not isolate them. (ie. disabling the first, will cause the other to succeed). I would move them together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chatted about this offline. I've added a test case confirming requests fail if a user attempts to clobber an existing index. We'll likely catch this ahead of time on the UI side as well similar to how we do with GeoJSON Upload
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
elastic#94028) Co-authored-by: Kibana Machine <[email protected]>
#94028) (#94880) Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
This PR adds a simple endpoint in the Maps app for creating an index and index pattern:
This functionality exists behind a feature flag. To enable, add the following to your
kibana.yml
:xpack.maps.enableDrawingFeature: true
Testing is doable if you're comfortable with using cUrl commands or using a client such as Insomnia. Running the PR locally, the following test cUrl command can be used after editing the
--url
arg and the--header 'Cookie:
arg.You will need to use your own cookie for this to work. To obtain one:
import
requests used to upload the fileCopy as cUrl
the network request and pull out the cookie portion for use in the above cUrl commandNotes: