[Rollup] Disallow index patterns that match rollup indices

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfig.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.ObjectParser;
 import org.elasticsearch.common.xcontent.ToXContent;
@@ -173,7 +174,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
         builder.endObject();
         return builder;
     }
-    
+
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeString(id);
@@ -336,6 +337,17 @@ public RollupJobConfig build() {
             if (indexPattern == null || indexPattern.isEmpty()) {
                 throw new IllegalArgumentException("An index pattern is mandatory.");
             }
+            if (Regex.isMatchAllPattern(indexPattern)) {
+                throw new IllegalArgumentException("Index pattern must not match all indices (as it would match it's own rollup index");
+            }
+            if (Regex.isSimpleMatchPattern(indexPattern)) {
+                if (Regex.simpleMatch(indexPattern, rollupIndex)) {
+                    throw new IllegalArgumentException("Index pattern would match rollup index name which is not allowed.");
+                }
+            }
+            if (indexPattern.equals(rollupIndex)) {
+                throw new IllegalArgumentException("Rollup index may not be the same as the index pattern.");
+            }
             if (rollupIndex == null || rollupIndex.isEmpty()) {
                 throw new IllegalArgumentException("A rollup index name is mandatory.");
             }

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java

@@ -21,6 +21,7 @@
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest;
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
 import org.elasticsearch.action.support.ActionFilters;
+import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.action.support.master.TransportMasterNodeAction;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.ClusterState;
@@ -44,10 +45,12 @@
 import org.elasticsearch.xpack.core.XPackField;
 import org.elasticsearch.xpack.core.rollup.RollupField;
 import org.elasticsearch.xpack.core.rollup.action.PutRollupJobAction;
+import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps;
 import org.elasticsearch.xpack.core.rollup.job.RollupJob;
 import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig;
 import org.elasticsearch.xpack.rollup.Rollup;
 
+import java.util.Arrays;
 import java.util.Map;
 import java.util.Objects;
 import java.util.stream.Collectors;
@@ -91,9 +94,16 @@ protected void masterOperation(PutRollupJobAction.Request request, ClusterState
             return;
         }
 
+        // Check to see if the index pattern matches any existing rollup indices
+        ActionRequestValidationException validationException = validateIndexPattern(request, clusterState);
+        if (validationException != null) {
+            listener.onFailure(validationException);
+            return;
+        }
+
         FieldCapabilitiesRequest fieldCapsRequest = new FieldCapabilitiesRequest()
-                .indices(request.getConfig().getIndexPattern())
-                .fields(request.getConfig().getAllFields().toArray(new String[0]));
+            .indices(request.getConfig().getIndexPattern())
+            .fields(request.getConfig().getAllFields().toArray(new String[0]));
 
         client.fieldCaps(fieldCapsRequest, new ActionListener<FieldCapabilitiesResponse>() {
             @Override
@@ -115,6 +125,26 @@ public void onFailure(Exception e) {
         });
     }
 
+    /**
+     * Ensure that the index pattern doesn't match any pre-existing rollup indices.  We don't need to check
+     * if the pattern would match it's own rollup index because the request already validated that.
+     */
+    private static ActionRequestValidationException validateIndexPattern(PutRollupJobAction.Request request, ClusterState clusterState) {
+        IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(Settings.EMPTY);
+        String[] indices = resolver.concreteIndexNames(clusterState, IndicesOptions.strictExpandOpenAndForbidClosed(),
+            request.getConfig().getIndexPattern());
+
+        ActionRequestValidationException validationException = new ActionRequestValidationException();
+
+        Arrays.stream(indices)
+            .forEach(index -> TransportGetRollupCapsAction.findRollupIndexCaps(index, clusterState.getMetaData().index(index))
+                .ifPresent(c -> validationException
+                    .addValidationError("Index pattern [" + request.getConfig().getIndexPattern() + "] matches existing rollup index ["
+                        + index + "] from jobs " + c.getJobCaps().stream().map(RollupJobCaps::getJobID).collect(Collectors.toList()))));
+
+        return validationException.validationErrors().size() > 0 ? validationException : null;
+    }
+
     private static RollupJob createRollupJob(RollupJobConfig config, ThreadPool threadPool) {
         // ensure we only filter for the allowed headers
         Map<String, String> filteredHeaders = threadPool.getThreadContext().getHeaders().entrySet().stream()

x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java

@@ -122,6 +122,37 @@ public void testEmptyIndexPattern() {
         assertThat(e.getMessage(), equalTo("An index pattern is mandatory."));
     }
 
+    public void testMatchAllIndexPattern() {
+        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
+        job.setIndexPattern("*");
+        Exception e = expectThrows(IllegalArgumentException.class, job::build);
+        assertThat(e.getMessage(), equalTo("Index pattern must not match all indices (as it would match it's own rollup index"));
+    }
+
+    public void testMatchOwnRollupPatternPrefix() {
+        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
+        job.setIndexPattern("foo-*");
+        job.setRollupIndex("foo-rollup");
+        Exception e = expectThrows(IllegalArgumentException.class, job::build);
+        assertThat(e.getMessage(), equalTo("Index pattern would match rollup index name which is not allowed."));
+    }
+
+    public void testMatchOwnRollupPatternSuffix() {
+        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
+        job.setIndexPattern("*-rollup");
+        job.setRollupIndex("foo-rollup");
+        Exception e = expectThrows(IllegalArgumentException.class, job::build);
+        assertThat(e.getMessage(), equalTo("Index pattern would match rollup index name which is not allowed."));
+    }
+
+    public void testIndexPatternIdenticalToRollup() {
+        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
+        job.setIndexPattern("foo");
+        job.setRollupIndex("foo");
+        Exception e = expectThrows(IllegalArgumentException.class, job::build);
+        assertThat(e.getMessage(), equalTo("Rollup index may not be the same as the index pattern."));
+    }
+
     public void testEmptyRollupIndex() {
         RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         job.setRollupIndex("");

x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml

@@ -159,5 +159,58 @@ setup:
             ]
           }
 
+---
+"Index pattern matches existing rollup index":
+
+  - do:
+      headers:
+        Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser
+      xpack.rollup.put_job:
+        id: foo
+        body:  >
+          {
+            "index_pattern": "foo",
+            "rollup_index": "foo_rollup",
+            "cron": "*/30 * * * * ?",
+            "page_size" :10,
+            "groups" : {
+              "date_histogram": {
+                "field": "the_field",
+                "interval": "1h"
+              }
+            },
+            "metrics": [
+              {
+                "field": "value_field",
+                "metrics": ["min", "max", "sum"]
+              }
+            ]
+          }
+  - is_true: acknowledged
 
+  - do:
+       catch: /Index pattern \[foo\*\] matches existing rollup index \[foo_rollup\] from jobs \[foo\]/
+       headers:
+         Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser
+       xpack.rollup.put_job:
+         id: foo2
+         body:  >
+           {
+             "index_pattern": "foo*",
+             "rollup_index": "some_index",
+             "cron": "*/30 * * * * ?",
+             "page_size" :10,
+             "groups" : {
+               "date_histogram": {
+                 "field": "the_field",
+                 "interval": "1h"
+               }
+             },
+             "metrics": [
+               {
+                 "field": "value_field",
+                 "metrics": ["min", "max", "sum"]
+               }
+             ]
+           }