-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Add logstash system index APIs #53350
Add logstash system index APIs #53350
Conversation
Pinging @elastic/es-core-infra (:Core/Infra/Core) |
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.
Looks good overall, left a few comments about tweaks I think we should make.
...gin/logstash/src/main/java/org/elasticsearch/xpack/logstash/action/DeletePipelineAction.java
Outdated
Show resolved
Hide resolved
...n/logstash/src/main/java/org/elasticsearch/xpack/logstash/action/DeletePipelineResponse.java
Show resolved
Hide resolved
...gstash/src/main/java/org/elasticsearch/xpack/logstash/action/TransportGetPipelineAction.java
Outdated
Show resolved
Hide resolved
...gstash/src/main/java/org/elasticsearch/xpack/logstash/action/TransportGetPipelineAction.java
Outdated
Show resolved
Hide resolved
...gstash/src/main/java/org/elasticsearch/xpack/logstash/action/TransportGetPipelineAction.java
Outdated
Show resolved
Hide resolved
mGetResponse -> listener.onResponse( | ||
new GetPipelineResponse( | ||
Arrays.stream(mGetResponse.getResponses()) | ||
.filter(itemResponse -> itemResponse.isFailed() == false) |
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 not sure what to do instead, but if some of the GETs fail, do we really just want to silently swallow that? Should we at least log something here?
...gstash/src/main/java/org/elasticsearch/xpack/logstash/action/TransportPutPipelineAction.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/elasticsearch/xpack/logstash/PipelineRequestResponseSerializationTests.java
Outdated
Show resolved
Hide resolved
@elasticmachine Please run elasticsearch-ci/2 |
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.
Left a few very minor comments, but otherwise LGTM. No need for another round of review after addressing my comments here.
...ugin/logstash/src/main/java/org/elasticsearch/xpack/logstash/action/GetPipelineResponse.java
Show resolved
Hide resolved
x-pack/plugin/src/test/java/org/elasticsearch/xpack/test/rest/LogstashSystemIndexIT.java
Outdated
Show resolved
Hide resolved
...gstash/src/test/java/org/elasticsearch/xpack/logstash/action/DeletePipelineRequestTests.java
Show resolved
Hide resolved
We want Logstash indices to be system indices, but the logstash service will still need to be able to manage its indices. This PR adds special system index APIs to the logstash plugin so that logstash can manage its pipelines without direct access to the underlying indices. * Add logstash module with dedicated logstash APIs * merge with x-pack plugin * add system index access allowance * Break out serialization tests into distinct classes * Log failures for partial multiget failure * Move LogstashSystemIndexIT to javaRestTest task Co-authored-by: William Brafford <william.brafford@elastic.co>
We want Logstash indices to be system indices, but the logstash service will still need to be able to manage its indices. This PR adds special system index APIs to the logstash plugin so that logstash can manage its pipelines without direct access to the underlying indices. * Add logstash module with dedicated logstash APIs * merge with x-pack plugin * add system index access allowance * Break out serialization tests into distinct classes * Log failures for partial multiget failure * Move LogstashSystemIndexIT to javaRestTest task Co-authored-by: William Brafford <william.brafford@elastic.co> Co-authored-by: Jay Modi <jaymode@users.noreply.github.com>
This change updates the logstash pipeline management plugin to use pipeline management APIs in Elasticsearch rather than directly accessing the .logstash index. In Elasticsearch 8.0, direct access to system indices will no longer be allowed when using standard APIs. Given this change, a new set of APIs has been created specifically for the management of Logstash pipelines and this change makes use of the APIs. Relates elastic/elasticsearch#53350
This commit adds REST apis for logstash and its system index,
.logstash
, which is used to store pipelines for central management. The module adds new endpoints that enable CRUD operationson the pipelines stored in the index.