honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Return 404 when deleting a non existing data stream (#62224) 

Backport of #62059 to 7.x branch.

Return a 404 http status code when attempting to delete a non existing data stream.
However only return a 404 when targeting a data stream without any wildcards.

Closes #62022
This commit is contained in:
Martijn van Groningen 2020-09-11 15:36:05 +02:00 committed by GitHub
parent b118697368
commit 1bb094a27b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 105 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/DeleteDataStreamAction.java
View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.core.action;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.ActionType;
import org.elasticsearch.action.IndicesRequest;
@ -13,6 +14,7 @@ import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.action.support.master.MasterNodeRequest;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.util.CollectionUtils;
import java.io.IOException;
@ -34,8 +36,17 @@ public class DeleteDataStreamAction extends ActionType<AcknowledgedResponse> {
private String[] names;
// Security intercepts requests and rewrites names if wildcards are used to expand to concrete resources
// that a user has privileges for.
// This keeps track whether wildcards were originally specified in names,
// So that in the case no matches ds are found, that either an
// empty response can be returned in case wildcards were used or
// 404 status code returned in case no wildcard were used.
private final boolean wildcardExpressionsOriginallySpecified;
public Request(String[] names) {
this.names = Objects.requireNonNull(names);
this.wildcardExpressionsOriginallySpecified = Arrays.stream(names).anyMatch(Regex::isSimpleMatchPattern);
}
public String[] getNames() {
@ -54,12 +65,16 @@ public class DeleteDataStreamAction extends ActionType<AcknowledgedResponse> {
public Request(StreamInput in) throws IOException {
super(in);
this.names = in.readStringArray();
this.wildcardExpressionsOriginallySpecified = in.getVersion().onOrAfter(Version.V_7_10_0) && in.readBoolean();
}
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeStringArray(names);
if (out.getVersion().onOrAfter(Version.V_7_10_0)) {
out.writeBoolean(wildcardExpressionsOriginallySpecified);
}
}
@Override
@ -67,12 +82,15 @@ public class DeleteDataStreamAction extends ActionType<AcknowledgedResponse> {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Request request = (Request) o;
return Arrays.equals(names, request.names);
return wildcardExpressionsOriginallySpecified == request.wildcardExpressionsOriginallySpecified &&
Arrays.equals(names, request.names);
}
@Override
public int hashCode() {
return Arrays.hashCode(names);
int result = Objects.hash(wildcardExpressionsOriginallySpecified);
result = 31 * result + Arrays.hashCode(names);
return result;
}
@Override
@ -97,6 +115,10 @@ public class DeleteDataStreamAction extends ActionType<AcknowledgedResponse> {
this.names = indices;
return this;
}
public boolean isWildcardExpressionsOriginallySpecified() {
return wildcardExpressionsOriginallySpecified;
}
}
}

25
x-pack/plugin/data-streams/src/main/java/org/elasticsearch/xpack/datastreams/action/DeleteDataStreamTransportAction.java
View File

@ -8,6 +8,7 @@ package org.elasticsearch.xpack.datastreams.action;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
@ -25,7 +26,6 @@ import org.elasticsearch.common.Priority;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.Index;
import org.elasticsearch.snapshots.SnapshotInProgressException;
@ -35,6 +35,7 @@ import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.action.DeleteDataStreamAction;
import java.io.IOException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
@ -97,7 +98,7 @@ public class DeleteDataStreamTransportAction extends TransportMasterNodeAction<D
@Override
public ClusterState execute(ClusterState currentState) {
return removeDataStream(deleteIndexService, currentState, request);
return removeDataStream(deleteIndexService, indexNameExpressionResolver, currentState, request);
}
@Override
@ -110,19 +111,21 @@ public class DeleteDataStreamTransportAction extends TransportMasterNodeAction<D
static ClusterState removeDataStream(
MetadataDeleteIndexService deleteIndexService,
IndexNameExpressionResolver indexNameExpressionResolver,
ClusterState currentState,
DeleteDataStreamAction.Request request
) {
Set<String> dataStreams = new HashSet<>();
Set<String> snapshottingDataStreams = new HashSet<>();
for (String name : request.getNames()) {
for (String dataStreamName : currentState.metadata().dataStreams().keySet()) {
if (Regex.simpleMatch(name, dataStreamName)) {
dataStreams.add(dataStreamName);
}
}
Set<String> dataStreams = new HashSet<>(
indexNameExpressionResolver.dataStreamNames(currentState, request.indicesOptions(), request.getNames())
);
Set<String> snapshottingDataStreams = SnapshotsService.snapshottingDataStreams(currentState, dataStreams);
snapshottingDataStreams.addAll(SnapshotsService.snapshottingDataStreams(currentState, dataStreams));
if (dataStreams.isEmpty()) {
if (request.isWildcardExpressionsOriginallySpecified()) {
return currentState;
} else {
throw new ResourceNotFoundException("data streams " + Arrays.toString(request.getNames()) + " not found");
}
}
if (snapshottingDataStreams.isEmpty() == false) {

27
x-pack/plugin/data-streams/src/test/java/org/elasticsearch/xpack/datastreams/action/DeleteDataStreamTransportActionTests.java
View File

@ -6,9 +6,11 @@
package org.elasticsearch.xpack.datastreams.action;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.SnapshotsInProgress;
import org.elasticsearch.cluster.metadata.DataStream;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService;
import org.elasticsearch.common.Strings;
@ -29,19 +31,22 @@ import java.util.Set;
import static org.elasticsearch.cluster.DataStreamTestHelper.getClusterStateWithDataStreams;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.sameInstance;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
public class DeleteDataStreamTransportActionTests extends ESTestCase {
private final IndexNameExpressionResolver iner = new IndexNameExpressionResolver();
public void testDeleteDataStream() {
final String dataStreamName = "my-data-stream";
final List<String> otherIndices = randomSubsetOf(Arrays.asList("foo", "bar", "baz"));
ClusterState cs = getClusterStateWithDataStreams(Collections.singletonList(new Tuple<>(dataStreamName, 2)), otherIndices);
DeleteDataStreamAction.Request req = new DeleteDataStreamAction.Request(new String[] { dataStreamName });
ClusterState newState = DeleteDataStreamTransportAction.removeDataStream(getMetadataDeleteIndexService(), cs, req);
ClusterState newState = DeleteDataStreamTransportAction.removeDataStream(getMetadataDeleteIndexService(), iner, cs, req);
assertThat(newState.metadata().dataStreams().size(), equalTo(0));
assertThat(newState.metadata().indices().size(), equalTo(otherIndices.size()));
for (String indexName : otherIndices) {
@ -62,7 +67,7 @@ public class DeleteDataStreamTransportActionTests extends ESTestCase {
);
DeleteDataStreamAction.Request req = new DeleteDataStreamAction.Request(new String[] { "ba*", "eggplant" });
ClusterState newState = DeleteDataStreamTransportAction.removeDataStream(getMetadataDeleteIndexService(), cs, req);
ClusterState newState = DeleteDataStreamTransportAction.removeDataStream(getMetadataDeleteIndexService(), iner, cs, req);
assertThat(newState.metadata().dataStreams().size(), equalTo(1));
DataStream remainingDataStream = newState.metadata().dataStreams().get(dataStreamNames[0]);
assertNotNull(remainingDataStream);
@ -89,7 +94,7 @@ public class DeleteDataStreamTransportActionTests extends ESTestCase {
DeleteDataStreamAction.Request req = new DeleteDataStreamAction.Request(new String[] { dataStreamName });
SnapshotInProgressException e = expectThrows(
SnapshotInProgressException.class,
() -> DeleteDataStreamTransportAction.removeDataStream(getMetadataDeleteIndexService(), snapshotCs, req)
() -> DeleteDataStreamTransportAction.removeDataStream(getMetadataDeleteIndexService(), iner, snapshotCs, req)
);
assertThat(
@ -130,8 +135,20 @@ public class DeleteDataStreamTransportActionTests extends ESTestCase {
),
Collections.emptyList()
);
DeleteDataStreamAction.Request req = new DeleteDataStreamAction.Request(new String[] { dataStreamName });
ClusterState newState = DeleteDataStreamTransportAction.removeDataStream(getMetadataDeleteIndexService(), cs, req);
expectThrows(
ResourceNotFoundException.class,
() -> DeleteDataStreamTransportAction.removeDataStream(
getMetadataDeleteIndexService(),
iner,
cs,
new DeleteDataStreamAction.Request(new String[] { dataStreamName })
)
);
DeleteDataStreamAction.Request req = new DeleteDataStreamAction.Request(new String[] { dataStreamName + "*" });
ClusterState newState = DeleteDataStreamTransportAction.removeDataStream(getMetadataDeleteIndexService(), iner, cs, req);
assertThat(newState, sameInstance(cs));
assertThat(newState.metadata().dataStreams().size(), equalTo(cs.metadata().dataStreams().size()));
assertThat(
newState.metadata().dataStreams().keySet(),

45
x-pack/plugin/src/test/resources/rest-api-spec/test/data_stream/10_basic.yml
View File

@ -250,6 +250,51 @@ setup:
indices.get:
index: ".ds-simple-data-stream1-000001"
---
"Delete data stream missing behaviour":
- skip:
version: " - 7.8.99"
reason: "data streams only supported in 7.9+"
- do:
indices.create_data_stream:
name: simple-data-stream1
- is_true: acknowledged
- do:
indices.create_data_stream:
name: simple-data-stream2
- is_true: acknowledged
- do:
indices.create:
index: simple-data-streamz
- do:
indices.delete_data_stream:
name: simple-data-stream1
- is_true: acknowledged
- do:
indices.delete_data_stream:
name: simple-data-stream2
- is_true: acknowledged
- do:
indices.delete_data_stream:
name: simple-data-stream*
- is_true: acknowledged
- do:
catch: missing
indices.delete_data_stream:
name: simple-data-stream1
- do:
catch: missing
indices.delete_data_stream:
name: simple-data-stream2
---
"append-only writes to backing indices prohobited":
- skip: