Better error when can't auto create index (#22488)

nik9000 · web-flow · commit 1fe74a6b4b23 · 2017-01-18T15:18:32.000-05:00
Changes the error message when `action.auto_create_index` or `index.mapper.dynamic` forbids automatic creation of an index from `no such index` to one of: * `no such index and [action.auto_create_index] is [false]` * `no such index and [index.mapper.dynamic] is [false]` * `no such index and [action.auto_create_index] contains [-<pattern>] which forbids automatic creation of the index` * `no such index and [action.auto_create_index] ([all patterns]) doesn't match` This should make it more clear *why* there is `no such index`. Closes #22435
diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml
@@ -168,7 +168,6 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]search[/\\]TransportMultiSearchAction.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]suggest[/\\]SuggestResponse.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]support[/\\]ActionFilter.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]support[/\\]AutoCreateIndex.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]support[/\\]DelegatingActionListener.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]support[/\\]IndicesOptions.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]support[/\\]ToXContentToBytes.java" checks="LineLength" />
diff --git a/core/src/main/java/org/elasticsearch/action/support/AutoCreateIndex.java b/core/src/main/java/org/elasticsearch/action/support/AutoCreateIndex.java
@@ -29,6 +29,7 @@
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.mapper.MapperService;
 
 import java.util.ArrayList;
@@ -63,18 +64,20 @@ public boolean needToCheck() {
 
     /**
      * Should the index be auto created?
+     * @throws IndexNotFoundException if the the index doesn't exist and shouldn't be auto created
      */
     public boolean shouldAutoCreate(String index, ClusterState state) {
+        if (resolver.hasIndexOrAlias(index, state)) {
+            return false;
+        }
         // One volatile read, so that all checks are done against the same instance:
         final AutoCreate autoCreate = this.autoCreate;
         if (autoCreate.autoCreateIndex == false) {
-            return false;
+            throw new IndexNotFoundException("no such index and [" + AUTO_CREATE_INDEX_SETTING.getKey() + "] is [false]", index);
         }
         if (dynamicMappingDisabled) {
-            return false;
-        }
-        if (resolver.hasIndexOrAlias(index, state)) {
-            return false;
+            throw new IndexNotFoundException("no such index and [" + MapperService.INDEX_MAPPER_DYNAMIC_SETTING.getKey() + "] is [false]",
+                    index);
         }
         // matches not set, default value of "true"
         if (autoCreate.expressions.isEmpty()) {
@@ -84,10 +87,15 @@ public boolean shouldAutoCreate(String index, ClusterState state) {
             String indexExpression = expression.v1();
             boolean include = expression.v2();
             if (Regex.simpleMatch(indexExpression, index)) {
-                return include;
+                if (include) {
+                    return true;
+                }
+                throw new IndexNotFoundException("no such index and [" + AUTO_CREATE_INDEX_SETTING.getKey() + "] contains [-"
+                        + indexExpression + "] which forbids automatic creation of the index", index);
             }
         }
-        return false;
+        throw new IndexNotFoundException("no such index and [" + AUTO_CREATE_INDEX_SETTING.getKey() + "] ([" + autoCreate
+                + "]) doesn't match", index);
     }
 
     AutoCreate getAutoCreate() {
@@ -101,6 +109,7 @@ void setAutoCreate(AutoCreate autoCreate) {
     static class AutoCreate {
         private final boolean autoCreateIndex;
         private final List<Tuple<String, Boolean>> expressions;
+        private final String string;
 
         private AutoCreate(String value) {
             boolean autoCreateIndex;
@@ -112,18 +121,21 @@ private AutoCreate(String value) {
                     String[] patterns = Strings.commaDelimitedListToStringArray(value);
                     for (String pattern : patterns) {
                         if (pattern == null || pattern.trim().length() == 0) {
-                            throw new IllegalArgumentException("Can't parse [" + value + "] for setting [action.auto_create_index] must be either [true, false, or a comma separated list of index patterns]");
+                            throw new IllegalArgumentException("Can't parse [" + value + "] for setting [action.auto_create_index] must "
+                                    + "be either [true, false, or a comma separated list of index patterns]");
                         }
                         pattern = pattern.trim();
                         Tuple<String, Boolean> expression;
                         if (pattern.startsWith("-")) {
                             if (pattern.length() == 1) {
-                                throw new IllegalArgumentException("Can't parse [" + value + "] for setting [action.auto_create_index] must contain an index name after [-]");
+                                throw new IllegalArgumentException("Can't parse [" + value + "] for setting [action.auto_create_index] "
+                                        + "must contain an index name after [-]");
                             }
                             expression = new Tuple<>(pattern.substring(1), false);
                         } else if(pattern.startsWith("+")) {
                             if (pattern.length() == 1) {
-                                throw new IllegalArgumentException("Can't parse [" + value + "] for setting [action.auto_create_index] must contain an index name after [+]");
+                                throw new IllegalArgumentException("Can't parse [" + value + "] for setting [action.auto_create_index] "
+                                        + "must contain an index name after [+]");
                             }
                             expression = new Tuple<>(pattern.substring(1), true);
                         } else {
@@ -139,6 +151,7 @@ private AutoCreate(String value) {
             }
             this.expressions = expressions;
             this.autoCreateIndex = autoCreateIndex;
+            this.string = value;
         }
 
         boolean isAutoCreateIndex() {
@@ -148,5 +161,10 @@ boolean isAutoCreateIndex() {
         List<Tuple<String, Boolean>> getExpressions() {
             return expressions;
         }
+
+        @Override
+        public String toString() {
+            return string;
+        }
     }
 }
diff --git a/core/src/main/java/org/elasticsearch/index/IndexNotFoundException.java b/core/src/main/java/org/elasticsearch/index/IndexNotFoundException.java
@@ -24,9 +24,16 @@
 import java.io.IOException;
 
 public final class IndexNotFoundException extends ResourceNotFoundException {
+    /**
+     * Construct with a custom message.
+     */
+    public IndexNotFoundException(String message, String index) {
+        super(message);
+        setIndex(index);
+    }
 
     public IndexNotFoundException(String index) {
-        this(index, null);
+        this(index, (Throwable) null);
     }
 
     public IndexNotFoundException(String index, Throwable cause) {
diff --git a/core/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java b/core/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java
@@ -28,6 +28,7 @@
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.test.ESTestCase;
 
@@ -83,7 +84,9 @@ public void testHandleSpaces() { // see #21449
     public void testAutoCreationDisabled() {
         Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), false).build();
         AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings);
-        assertThat(autoCreateIndex.shouldAutoCreate(randomAsciiOfLengthBetween(1, 10), buildClusterState()), equalTo(false));
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () ->
+            autoCreateIndex.shouldAutoCreate(randomAsciiOfLengthBetween(1, 10), buildClusterState()));
+        assertEquals("no such index and [action.auto_create_index] is [false]", e.getMessage());
     }
 
     public void testAutoCreationEnabled() {
@@ -110,7 +113,9 @@ public void testDynamicMappingDisabled() {
                 randomAsciiOfLengthBetween(1, 10)))
             .put(MapperService.INDEX_MAPPER_DYNAMIC_SETTING.getKey(), false).build();
         AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings);
-        assertThat(autoCreateIndex.shouldAutoCreate(randomAsciiOfLengthBetween(1, 10), buildClusterState()), equalTo(false));
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () ->
+            autoCreateIndex.shouldAutoCreate(randomAsciiOfLengthBetween(1, 10), buildClusterState()));
+        assertEquals("no such index and [index.mapper.dynamic] is [false]", e.getMessage());
     }
 
     public void testAutoCreationPatternEnabled() {
@@ -119,36 +124,37 @@ public void testAutoCreationPatternEnabled() {
         AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings);
         ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build();
         assertThat(autoCreateIndex.shouldAutoCreate("index" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(true));
-        assertThat(autoCreateIndex.shouldAutoCreate("does_not_match" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
+        expectNotMatch(clusterState, autoCreateIndex, "does_not_match" + randomAsciiOfLengthBetween(1, 5));
     }
 
     public void testAutoCreationPatternDisabled() {
         Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), "-index*").build();
         AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings);
         ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build();
-        assertThat(autoCreateIndex.shouldAutoCreate("index" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
-        //default is false when patterns are specified
-        assertThat(autoCreateIndex.shouldAutoCreate("does_not_match" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
+        expectForbidden(clusterState, autoCreateIndex, "index" + randomAsciiOfLengthBetween(1, 5), "-index*");
+        /* When patterns are specified, even if the are all negative, the default is can't create. So a pure negative pattern is the same
+         * as false, really. */
+        expectNotMatch(clusterState, autoCreateIndex, "does_not_match" + randomAsciiOfLengthBetween(1, 5));
     }
 
     public void testAutoCreationMultiplePatternsWithWildcards() {
         Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(),
                 randomFrom("+test*,-index*", "test*,-index*")).build();
         AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings);
         ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build();
-        assertThat(autoCreateIndex.shouldAutoCreate("index" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
+        expectForbidden(clusterState, autoCreateIndex, "index" + randomAsciiOfLengthBetween(1, 5), "-index*");
         assertThat(autoCreateIndex.shouldAutoCreate("test" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(true));
-        assertThat(autoCreateIndex.shouldAutoCreate("does_not_match" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
+        expectNotMatch(clusterState, autoCreateIndex, "does_not_match" + randomAsciiOfLengthBetween(1, 5));
     }
 
     public void testAutoCreationMultiplePatternsNoWildcards() {
         Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), "+test1,-index1").build();
         AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings);
         ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build();
         assertThat(autoCreateIndex.shouldAutoCreate("test1", clusterState), equalTo(true));
-        assertThat(autoCreateIndex.shouldAutoCreate("index" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
-        assertThat(autoCreateIndex.shouldAutoCreate("test" + randomAsciiOfLengthBetween(2, 5), clusterState), equalTo(false));
-        assertThat(autoCreateIndex.shouldAutoCreate("does_not_match" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
+        expectNotMatch(clusterState, autoCreateIndex, "index" + randomAsciiOfLengthBetween(1, 5));
+        expectNotMatch(clusterState, autoCreateIndex, "test" + randomAsciiOfLengthBetween(2, 5));
+        expectNotMatch(clusterState, autoCreateIndex, "does_not_match" + randomAsciiOfLengthBetween(1, 5));
     }
 
     public void testAutoCreationMultipleIndexNames() {
@@ -157,7 +163,7 @@ public void testAutoCreationMultipleIndexNames() {
         ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build();
         assertThat(autoCreateIndex.shouldAutoCreate("test1", clusterState), equalTo(true));
         assertThat(autoCreateIndex.shouldAutoCreate("test2", clusterState), equalTo(true));
-        assertThat(autoCreateIndex.shouldAutoCreate("does_not_match" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
+        expectNotMatch(clusterState, autoCreateIndex, "does_not_match" + randomAsciiOfLengthBetween(1, 5));
     }
 
     public void testAutoCreationConflictingPatternsFirstWins() {
@@ -166,8 +172,8 @@ public void testAutoCreationConflictingPatternsFirstWins() {
         AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings);
         ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build();
         assertThat(autoCreateIndex.shouldAutoCreate("test1", clusterState), equalTo(true));
-        assertThat(autoCreateIndex.shouldAutoCreate("test2", clusterState), equalTo(false));
-        assertThat(autoCreateIndex.shouldAutoCreate("does_not_match" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false));
+        expectForbidden(clusterState, autoCreateIndex, "test2", "-test2");
+        expectNotMatch(clusterState, autoCreateIndex, "does_not_match" + randomAsciiOfLengthBetween(1, 5));
     }
 
     public void testUpdate() {
@@ -208,4 +214,18 @@ private AutoCreateIndex newAutoCreateIndex(Settings settings) {
         return new AutoCreateIndex(settings, new ClusterSettings(settings,
                 ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), new IndexNameExpressionResolver(settings));
     }
+
+    private void expectNotMatch(ClusterState clusterState, AutoCreateIndex autoCreateIndex, String index) {
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () ->
+            autoCreateIndex.shouldAutoCreate(index, clusterState));
+        assertEquals("no such index and [action.auto_create_index] ([" + autoCreateIndex.getAutoCreate() + "]) doesn't match",
+                e.getMessage());
+    }
+
+    private void expectForbidden(ClusterState clusterState, AutoCreateIndex autoCreateIndex, String index, String forbiddingPattern) {
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () ->
+            autoCreateIndex.shouldAutoCreate(index, clusterState));
+        assertEquals("no such index and [action.auto_create_index] contains [" + forbiddingPattern
+                + "] which forbids automatic creation of the index", e.getMessage());
+    }
 }
diff --git a/core/src/test/java/org/elasticsearch/cluster/NoMasterNodeIT.java b/core/src/test/java/org/elasticsearch/cluster/NoMasterNodeIT.java
@@ -42,6 +42,7 @@
 
 import java.util.Collections;
 
+import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertExists;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows;
@@ -59,13 +60,9 @@ protected Settings nodeSettings(int nodeOrdinal) {
     }
 
     public void testNoMasterActions() throws Exception {
-        // note, sometimes, we want to check with the fact that an index gets created, sometimes not...
-        boolean autoCreateIndex = randomBoolean();
-        logger.info("auto_create_index set to {}", autoCreateIndex);
-
         Settings settings = Settings.builder()
                 .put("discovery.type", "zen")
-                .put("action.auto_create_index", autoCreateIndex)
+                .put("action.auto_create_index", true)
                 .put("discovery.zen.minimum_master_nodes", 2)
                 .put(ZenDiscovery.PING_TIMEOUT_SETTING.getKey(), "200ms")
                 .put("discovery.initial_state_timeout", "500ms")
@@ -126,7 +123,7 @@ public void run() {
                         .setScript(new Script(
                             ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, "test script", Collections.emptyMap())).setTimeout(timeout));
 
-        checkUpdateAction(autoCreateIndex, timeout,
+        checkUpdateAction(true, timeout,
                 client().prepareUpdate("no_index", "type1", "1")
                         .setScript(new Script(
                             ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, "test script", Collections.emptyMap())).setTimeout(timeout));
@@ -135,7 +132,7 @@ public void run() {
         checkWriteAction(false, timeout,
                 client().prepareIndex("test", "type1", "1").setSource(XContentFactory.jsonBuilder().startObject().endObject()).setTimeout(timeout));
 
-        checkWriteAction(autoCreateIndex, timeout,
+        checkWriteAction(true, timeout,
                 client().prepareIndex("no_index", "type1", "1").setSource(XContentFactory.jsonBuilder().startObject().endObject()).setTimeout(timeout));
 
         BulkRequestBuilder bulkRequestBuilder = client().prepareBulk();
@@ -149,15 +146,8 @@ public void run() {
         bulkRequestBuilder = client().prepareBulk();
         bulkRequestBuilder.add(client().prepareIndex("no_index", "type1", "1").setSource(XContentFactory.jsonBuilder().startObject().endObject()));
         bulkRequestBuilder.add(client().prepareIndex("no_index", "type1", "2").setSource(XContentFactory.jsonBuilder().startObject().endObject()));
-        if (autoCreateIndex) {
-            // we expect the bulk to fail because it will try to go to the master. Use small timeout and detect it has passed
-            timeout = new TimeValue(200);
-        } else {
-            // the request should fail very quickly - use a large timeout and make sure it didn't pass...
-            timeout = new TimeValue(5000);
-        }
         bulkRequestBuilder.setTimeout(timeout);
-        checkWriteAction(autoCreateIndex, timeout, bulkRequestBuilder);
+        checkWriteAction(true, timeValueSeconds(5), bulkRequestBuilder);
 
         internalCluster().startNode(settings);
         client().admin().cluster().prepareHealth().setWaitForGreenStatus().setWaitForNodes("2").execute().actionGet();
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingDisabledTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingDisabledTests.java
diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/25_no_auto_create.yaml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/25_no_auto_create.yaml
