Initial data stream commit #53666
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| { | ||
| "cluster.create_data_stream":{ | ||
| "documentation":{ | ||
| "url":"https://www.elastic.co/guide/en/elasticsearch/reference/master/data-streams.html", | ||
| "description":"Creates or updates a data stream" | ||
| }, | ||
| "stability":"experimental", | ||
| "url":{ | ||
| "paths":[ | ||
| { | ||
| "path":"/_data_stream/{name}", | ||
| "methods":[ | ||
| "PUT" | ||
| ], | ||
| "parts":{ | ||
| "name":{ | ||
| "type":"string", | ||
| "description":"The name of the data stream" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "params":{ | ||
| }, | ||
| "body":{ | ||
| "description":"The data stream definition", | ||
| "required":true | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| { | ||
| "cluster.delete_data_stream":{ | ||
|
There was a problem hiding this comment.
|
||
| "documentation":{ | ||
| "url":"https://www.elastic.co/guide/en/elasticsearch/reference/master/data-streams.html", | ||
| "description":"Deletes a data stream." | ||
| }, | ||
| "stability":"experimental", | ||
| "url":{ | ||
| "paths":[ | ||
| { | ||
| "path":"/_data_stream/{name}", | ||
| "methods":[ | ||
| "DELETE" | ||
| ], | ||
| "parts":{ | ||
| "name":{ | ||
| "type":"string", | ||
| "description":"The name of the data stream" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "params":{} | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| { | ||
| "cluster.get_data_streams":{ | ||
|
|
||
| "documentation":{ | ||
| "url":"https://www.elastic.co/guide/en/elasticsearch/reference/master/data-streams.html", | ||
| "description":"Returns data streams." | ||
| }, | ||
| "stability":"experimental", | ||
| "url":{ | ||
| "paths":[ | ||
| { | ||
| "path":"/_data_streams", | ||
| "methods":[ | ||
| "GET" | ||
| ] | ||
| }, | ||
| { | ||
| "path":"/_data_streams/{name}", | ||
| "methods":[ | ||
| "GET" | ||
| ], | ||
| "parts":{ | ||
| "name":{ | ||
| "type":"list", | ||
| "description":"The comma separated names of data streams" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "params":{ | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| --- | ||
| "Test stubs": | ||
| - skip: | ||
| version: " - 7.99.99" | ||
| reason: not backported yet | ||
|
|
||
| - do: | ||
| cluster.create_data_stream: | ||
| name: data-stream2 | ||
| body: | ||
| timestamp_field_name: "@timestamp" | ||
|
There was a problem hiding this comment. I think most places where we ask for a field, we use just "field" and not "field_name". I did not search all usages, but I think this should be just |
||
| - is_true: acknowledged | ||
|
|
||
| - do: | ||
| cluster.get_data_streams: {} | ||
| - match: { 0.name: my_data_stream1 } | ||
| - match: { 0.timestamp_field_name: '@timestamp' } | ||
| - match: { 0.indices: ['my_data_stream1-000000'] } | ||
| - match: { 1.name: my_data_stream2 } | ||
| - match: { 1.timestamp_field_name: '@timestamp' } | ||
| - match: { 1.indices: [] } | ||
|
|
||
| - do: | ||
| cluster.delete_data_stream: | ||
| name: data-stream2 | ||
| - is_true: acknowledged | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package org.elasticsearch.action.admin.cluster.datastream; | ||
|
There was a problem hiding this comment. I think we should move this outside the We might also want to follow the same substructure by adding |
||
|
|
||
| import org.elasticsearch.action.ActionListener; | ||
| import org.elasticsearch.action.ActionRequestValidationException; | ||
| import org.elasticsearch.action.ActionType; | ||
| import org.elasticsearch.action.support.ActionFilters; | ||
| import org.elasticsearch.action.support.master.AcknowledgedResponse; | ||
| import org.elasticsearch.action.support.master.MasterNodeRequest; | ||
| import org.elasticsearch.action.support.master.TransportMasterNodeAction; | ||
| import org.elasticsearch.cluster.ClusterState; | ||
| import org.elasticsearch.cluster.block.ClusterBlockException; | ||
| import org.elasticsearch.cluster.block.ClusterBlockLevel; | ||
| import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; | ||
| import org.elasticsearch.cluster.service.ClusterService; | ||
| import org.elasticsearch.common.inject.Inject; | ||
| import org.elasticsearch.common.io.stream.StreamInput; | ||
| import org.elasticsearch.common.io.stream.StreamOutput; | ||
| import org.elasticsearch.tasks.Task; | ||
| import org.elasticsearch.threadpool.ThreadPool; | ||
| import org.elasticsearch.transport.TransportService; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Objects; | ||
|
|
||
| public class CreateDataStreamAction extends ActionType<AcknowledgedResponse> { | ||
|
|
||
| public static final CreateDataStreamAction INSTANCE = new CreateDataStreamAction(); | ||
| public static final String NAME = "cluster:admin/data_stream/create"; | ||
|
There was a problem hiding this comment. I am in doubt if we should not add a specific prefix here? No need to look at this now, we can pick that up later. |
||
|
|
||
| private CreateDataStreamAction() { | ||
| super(NAME, AcknowledgedResponse::new); | ||
| } | ||
|
|
||
| public static class Request extends MasterNodeRequest<Request> { | ||
|
|
||
| private final String name; | ||
| private String timestampFieldName; | ||
|
|
||
| public Request(String name) { | ||
| this.name = name; | ||
| } | ||
|
|
||
| public void setTimestampFieldName(String timestampFieldName) { | ||
| this.timestampFieldName = timestampFieldName; | ||
| } | ||
|
|
||
| @Override | ||
| public ActionRequestValidationException validate() { | ||
| return null; | ||
| } | ||
|
|
||
| public Request(StreamInput in) throws IOException { | ||
| super(in); | ||
| this.name = in.readString(); | ||
| this.timestampFieldName = in.readString(); | ||
| } | ||
|
There was a problem hiding this comment. Do we need to read the optional There was a problem hiding this comment. For now this api will only create new empty indices. |
||
|
|
||
| @Override | ||
| public void writeTo(StreamOutput out) throws IOException { | ||
| super.writeTo(out); | ||
| out.writeString(name); | ||
| out.writeString(timestampFieldName); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
| Request request = (Request) o; | ||
| return name.equals(request.name) && | ||
| timestampFieldName.equals(request.timestampFieldName); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(name, timestampFieldName); | ||
| } | ||
| } | ||
|
|
||
| public static class TransportAction extends TransportMasterNodeAction<Request, AcknowledgedResponse> { | ||
|
|
||
| @Inject | ||
| public TransportAction(TransportService transportService, ClusterService clusterService, ThreadPool threadPool, | ||
| ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) { | ||
| super(NAME, transportService, clusterService, threadPool, actionFilters, Request::new, indexNameExpressionResolver); | ||
| } | ||
|
|
||
| @Override | ||
| protected String executor() { | ||
| return ThreadPool.Names.SAME; | ||
| } | ||
|
|
||
| @Override | ||
| protected AcknowledgedResponse read(StreamInput in) throws IOException { | ||
| return new AcknowledgedResponse(in); | ||
| } | ||
|
|
||
| @Override | ||
| protected void masterOperation(Task task, Request request, ClusterState state, | ||
| ActionListener<AcknowledgedResponse> listener) throws Exception { | ||
| listener.onResponse(new AcknowledgedResponse(true)); | ||
| } | ||
|
|
||
| @Override | ||
| protected ClusterBlockException checkBlock(Request request, ClusterState state) { | ||
| return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE); | ||
| } | ||
| } | ||
|
|
||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 wonder if this name should be:
data_streams.createto be consistent withindices.create?Other
clusteroperations seems to be mostly about the cluster state and cluster settings.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 chatting with Henning, I also came to the conclusion that data stream apis should be concidere indices based operations. (Ideally we should have a data_stream notion, but we need to think more about this and how this would work in security). So I will revert the commit in this pr that changes data stream crud apis back to indices based apis.
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.
the component template apis are cluster operations, I suspect those may need to be changed also, can you explain the reasoning and how it will relate to security?
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.
Just like indices based apis, data stream apis targets a namespace. So someone may be allowed to create a data stream for logs-* but not for events-*.
Ideally there should be a data stream privilege that handles this correctly, because data streams aren't indices and a data stream may have indices that don't share the data stream name as common prefix. But for now let's stick with
indices:based action names and group the apis in the indices client, until we a better there is a better understanding how security and data streams should integrate.I think component templates should remain cluster based apis. The reason is that these resources don't apply to a namespace. The index template (based on the specified pattern) that use component templates should be an index based action/operation, since they apply the an index namespace and soon also to a data stream namespace.
However currently index templates are treated as cluster privilege (see ClusterPrivilegeResolver line 196 in master), even though the action names start with
indices:prefix. With data streams we should properly fix this.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.
Okay, I originally had component template APIs using
indices:to match the existing template stuff, but I did end up changing it because that requires the request to provide the indices that it is going to apply to, and component templates don't apply to indices. +1 to properly fix the template exception with data streams.