-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Create an Ingest API #5213
Create an Ingest API #5213
Conversation
What are pros and cons of plugin vs core? If there are no big cons to putting this in a plugin, that seems like the best way to go (smaller core is nice). |
@kjbekkelund yup I agree. The last couple days I've been learning more about Hapi and the current state of the plugin system and that's the direction I'm going to move in. Should have some updated code today. |
cc28c1c
to
266ff03
Compare
@rashidkpc what level of validation do you think we should do on the request body JSON, if any? For instance should we parse the "fields" field and make sure it looks like it's formatted correctly? From a security standpoint, it looks like the /elasticsearch endpoint allows the user to send pretty much anything to the .kibana index. This new API forces the .kibana index and index-pattern type for all requests since it only deals with index patterns, but can you think of any potentially malicious payloads we should try to detect and reject? I also have some questions related to ES index template creation in the description of this PR that I'd appreciate if you could take a look at. Also forgot, I was curious if there was a reason why the "fields" field is stored as a string instead of a regular json array? |
b574171
to
398a916
Compare
I can't think of anything particularly malicious you could add to a mapping. Fields is stored as a string to keep elasticsearch from parsing it into real elasticsearch fields, causing the cluster state to be much larger than it would need to be. This is probably not a good move, we probably could've just turned off indexing of that field in the mapping. Not sure if that works on object level fields. We could try to validate that each item in the fields object at least has a name and a type. Those are really the only things Elasticsearch needs for a successful mapping. We're going to hit an odd issue here though, and thats that we denormalized |
@rashidkpc we'd really only be creating templates for new index patterns, right? Could we deprecate and hide the |
We certainly could, though we'd need to update a lot of code that looks for 'number' specifically. |
04239de
to
09b207b
Compare
968fd50
to
7e73b3f
Compare
@rashidkpc I think the API is pretty much functionally complete. I still have a few things I'd like to do (testing, investigate security/shield integration, refactor some common code into modules), but I'd love it if you could take a look when you get a chance next week and provide any feedback you might have. Thanks! |
const isWildcard = _.contains(indexPattern.title, '*') || (indexPattern.title.match(/\[.*]/) !== null); | ||
const mappings = _.omit(_.mapValues(_.indexBy(req.payload.fields, 'name'), (value) => { | ||
return value.mapping; | ||
}), _.isUndefined); |
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.
For readability I usually prefer something like this when I use a lot of lodash methods:
_(req.payload.fields)
.indexBy('name')
.mapValues(value => value.mapping)
.omit(_.isUndefined)
.value();
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.
Didn't know about that syntax. Mind blown!
@rashidkpc Updated. I think I covered all of the functionality as it currently stands. Let me know if you have any questions or concerns. My goal was to have the API combine all the info from the .kibana index, index templates, and indices in a way that's mostly seamless to the client, without duplicating data in any of those three locations. |
f070657
to
bb9aee0
Compare
@rashidkpc @spalger Now that the tests are complete and passing I've slapped the official review label on this guy. The only outstanding items from the last batch of feedback are:
I updated the PR description with some thorough documentation as well so please give that a read as you review. |
@Bargs can you update the pull description to fit the new routes? |
return 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.
Can you break these routes out into their own files, under an index_pattern directory? Then just include them in this file?
@spalger Just pushed some updates, I think I covered all of your feedback, please take another look! |
LGTM |
@spalger Whoooooo! @rashidkpc one final look for you as well? |
Implementation looks good, can you squash the commits? |
Is the Postman collection up to date? When I run the GET request against my local Kibana 5, it complains about a missing |
@kmoe the Postman examples should be up to date, it was working for another team member before. However it sounds like you're getting an old version, because there's no longer a GET endpoint. You might try deleting the postman collection and re-importing it, I've seen the sync act pretty flaky in the past. |
How to use this API? Why is a kbn-version Header required? And why is there a browser version check for an API?
|
@megastef This change only exists in master, which is version |
I'm using "master". I wonder more about the error messages ""Browser client is out of date, please refresh the page" - there was no complain about the version I used in the header (only when not present). And the API does not allow to specify the Kibana-Index (which is configured in the kibana.yml - static/gloabal config), while an API should give the freedom to be used for more general things. |
The "browser out of date" error you're getting actually is the error message from the version mismatch. When the error message was created, there were no APIs to access, so you'd only ever see it if you were using Kibana from your browser. We should update the error message. |
I created issue #6021 for updating the error message. |
Also note that this API is primarily meant for Kibana's own internal use
|
For us to solve this, we first needed to discover that the version of kibana installed on all nodes handling web requests for kibana:5601 were not running the same version. |
If you would prefer a method for loading dashboards and indexpatterns from the filesystem (for use with Chef, Puppet, Salt, etc) please vouch for it here: #2310 |
This feature was removed from 5.0. It does not currently have a target release because it will most likely require some changes.
Closes #5199
Documentation
If you like Postman, import this collection with example requests to play around with.
Endpoints
POST /api/kibana/ingest
Create a new index pattern and index template. Template mappings are given sensible defaults based on their type. Fields of type
number
will be created as adouble
in elasticsearch. Strings will be mapped as multi fields with an analyzed version and a non-analyzed version (called{fieldName}.raw
). Fields with a.
in their name will be mapped as object type fields with mappings included for each of their subfields.Note: You can only create a template if your pattern does not match existing indices.
Payload Schema
Each payload is a JSON object.
The object has the following properties:
id
: String (Required) This will be the ID of the index pattern that gets created.title
: String (Required)time_field_name
: String (optional, only needed for time based index patterns)interval_name
: String (optional)fields
: Array (required)field_format_map
: Object (optional, map of non-default formatters to user for fields)not_expandable
: Boolean (optional)An object in the
fields
array must have the following attributes:name
: String (required)type
: String (required)Valid values for
type
include:Errors
Example
Request:
Response:
DELETE /api/kibana/ingest/{id}
Deletes the index pattern with the given id and its associated index template.
Errors
Example
Request:
Response:
Done
Need to handle errors that the elasticsearch client might throw at me
https://www.elastic.co/guide/en/elasticsearch/client/javascript-api/current/errors.html
Create a Joi schema
Primarily going to rely on functional tests for API testing, do unit testing where appropriate.
Should accept API tokens for auth once Kibana API Tokens #5267 is merged.
Until then, how should we secure this? It's a bit different than most ways we access elasticsearch right now because it accesses the .kibana index, but it will be initiated from the frontend. Should look at how the current pattern creation screen does it with a shielded cluster.