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

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

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

@@ -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_8_0_0) && in.readBoolean();
         }
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
             super.writeTo(out);
             out.writeStringArray(names);
+            if (out.getVersion().onOrAfter(Version.V_8_0_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;
+        }
     }
 
 }

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

@@ -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;
@@ -36,6 +36,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;
 
@@ -99,7 +100,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
@@ -112,19 +113,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) {

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

@@ -6,10 +6,12 @@
 
 package org.elasticsearch.xpack.datastreams.action;
 
+import org.elasticsearch.ResourceNotFoundException;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.DataStreamTestHelper;
 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;
@@ -28,19 +30,22 @@ import java.util.Set;
 
 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(List.of("foo", "bar", "baz"));
 
         ClusterState cs = DataStreamTestHelper.getClusterStateWithDataStreams(List.of(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) {
@@ -61,7 +66,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);
@@ -88,7 +93,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(
@@ -129,8 +134,20 @@ public class DeleteDataStreamTransportActionTests extends ESTestCase {
             ),
             List.of()
         );
-        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(),

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

@@ -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: