Browse Source

Always fail if attempting to create an alias to backing index. (#68227)

This commit removes the ignore failure logic if an alias points
to a backing index. This was added so that clusters upgrading
from 7.9 and 7.10 to a newer version wouldn't immediately fail.

Relates to #67886
Martijn van Groningen 4 years ago
parent
commit
7a694e780b

+ 1 - 1
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java

@@ -329,7 +329,7 @@ public class MetadataRolloverService {
 
         final String matchedV2Template = findV2Template(metadata, rolloverIndexName, isHidden == null ? false : isHidden);
         if (matchedV2Template != null) {
-            List<Map<String, AliasMetadata>> aliases = MetadataIndexTemplateService.resolveAliases(metadata, matchedV2Template, false);
+            List<Map<String, AliasMetadata>> aliases = MetadataIndexTemplateService.resolveAliases(metadata, matchedV2Template);
             for (Map<String, AliasMetadata> aliasConfig : aliases) {
                 if (aliasConfig.containsKey(rolloverRequestAlias)) {
                     throw new IllegalArgumentException(String.format(Locale.ROOT,

+ 1 - 1
server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java

@@ -161,7 +161,7 @@ public class TransportSimulateIndexTemplateAction
         Settings settings = resolveSettings(simulatedState.metadata(), matchingTemplate);
 
         List<Map<String, AliasMetadata>> resolvedAliases = MetadataIndexTemplateService.resolveAliases(simulatedState.metadata(),
-            matchingTemplate, true);
+            matchingTemplate);
 
         // create the index with dummy settings in the cluster state so we can parse and validate the aliases
         Settings dummySettings = Settings.builder()

+ 1 - 9
server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java

@@ -43,7 +43,6 @@ import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.VersionedNamedWriteable;
-import org.elasticsearch.common.logging.HeaderWarning;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
@@ -1470,14 +1469,7 @@ public class Metadata implements Iterable<IndexMetadata>, Diffable<Metadata>, To
                     .map(IndexAbstraction::getName)
                     .collect(Collectors.toList());
                 if (conflictingAliases.isEmpty() == false) {
-                    // After backporting throw an IllegalStateException instead of logging a warning:
-                    // (in 7.x there might be aliases that refer to backing indices of a data stream and
-                    // throwing an exception here would avoid the cluster from functioning)
-                    String warning = "aliases " + conflictingAliases + " cannot refer to backing indices of data streams";
-                    // log as debug, this method is executed each time a new cluster state is created and could result
-                    // in many logs:
-                    logger.debug(warning);
-                    HeaderWarning.addWarning(warning);
+                    throw new IllegalStateException("aliases " + conflictingAliases + " cannot refer to backing indices of data streams");
                 }
             }
         }

+ 1 - 1
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java

@@ -517,7 +517,7 @@ public class MetadataCreateIndexService {
 
         return applyCreateIndexWithTemporaryService(currentState, request, silent, null, tmpImd, mappings,
             indexService -> resolveAndValidateAliases(request.index(), request.aliases(),
-                MetadataIndexTemplateService.resolveAliases(currentState.metadata(), templateName, false), currentState.metadata(),
+                MetadataIndexTemplateService.resolveAliases(currentState.metadata(), templateName), currentState.metadata(),
                 // the context is only used for validation so it's fine to pass fake values for the
                 // shard id and the current timestamp
                 aliasValidator, xContentRegistry, indexService.newSearchExecutionContext(0, 0, null, () -> 0L, null, emptyMap())),

+ 3 - 22
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java

@@ -1043,21 +1043,8 @@ public class MetadataIndexTemplateService {
 
     /**
      * Resolve the given v2 template into an ordered list of aliases
-     *
-     * @param failIfTemplateHasDataStream Whether to skip validating if a template has a data stream definition and an alias definition.
-     *                                    This validation is needed so that no template gets created that creates datastream and also
-     *                                    a an alias pointing to the backing indices of a data stream. Unfortunately this validation
-     *                                    was missing in versions prior to 7.11, which mean that there are cluster states out there,
-     *                                    that have this malformed templates. This method is used when rolling over a data stream
-     *                                    or creating new data streams. In order for these clusters to avoid failing these operations
-     *                                    immediately after an upgrade the failure should be optional. So that there is time to change
-     *                                    these templates. The logic that adds/updates index and component templates shouldn't skip this
-     *                                    validation.
      */
-    public static List<Map<String, AliasMetadata>> resolveAliases(final Metadata metadata,
-                                                                  final String templateName,
-                                                                  // TODO: remove in master after backport to 7.x:
-                                                                  final boolean failIfTemplateHasDataStream) {
+    public static List<Map<String, AliasMetadata>> resolveAliases(final Metadata metadata, final String templateName) {
         final ComposableIndexTemplate template = metadata.templatesV2().get(templateName);
         assert template != null : "attempted to resolve aliases for a template [" + templateName +
             "] that did not exist in the cluster state";
@@ -1081,13 +1068,7 @@ public class MetadataIndexTemplateService {
         // A template that creates data streams can't also create aliases.
         // (otherwise we end up with aliases pointing to backing indices of data streams)
         if (aliases.size() > 0 && template.getDataStreamTemplate() != null) {
-            if (failIfTemplateHasDataStream) {
-                throw new IllegalArgumentException("template [" + templateName + "] has alias and data stream definitions");
-            } else {
-                String warning = "template [" + templateName + "] has alias and data stream definitions";
-                logger.warn(warning);
-                HeaderWarning.addWarning(warning);
-            }
+            throw new IllegalArgumentException("template [" + templateName + "] has alias and data stream definitions");
         }
 
         // Aliases are applied in order, but subsequent alias configuration from the same name is
@@ -1141,7 +1122,7 @@ public class MetadataIndexTemplateService {
             tempIndexService -> {
                 // Validate aliases
                 MetadataCreateIndexService.resolveAndValidateAliases(temporaryIndexName, Collections.emptySet(),
-                    MetadataIndexTemplateService.resolveAliases(stateWithIndex.metadata(), templateName, true), stateWithIndex.metadata(),
+                    MetadataIndexTemplateService.resolveAliases(stateWithIndex.metadata(), templateName), stateWithIndex.metadata(),
                     new AliasValidator(),
                     // the context is only used for validation so it's fine to pass fake values for the
                     // shard id and the current timestamp

+ 5 - 12
server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java

@@ -699,19 +699,12 @@ public class MetadataRolloverServiceTests extends ESTestCase {
             CreateIndexRequest createIndexRequest = new CreateIndexRequest("_na_");
 
             // Ensure that a warning header is emitted
-            MetadataRolloverService.RolloverResult rolloverResult =
-                rolloverService.rolloverClusterState(clusterState, dataStream.getName(), null, createIndexRequest, metConditions,
-                    randomBoolean(), false);
-            assertWarnings(
-                "aliases [my-alias] cannot refer to backing indices of data streams",
-                "template [template] has alias and data stream definitions"
+            Exception e = expectThrows(
+                IllegalArgumentException.class,
+                () -> rolloverService.rolloverClusterState(clusterState, dataStream.getName(), null, createIndexRequest, metConditions,
+                    randomBoolean(), false)
             );
-
-            // Just checking that the rollover was successful:
-            String sourceIndexName = DataStream.getDefaultBackingIndexName(dataStream.getName(), dataStream.getGeneration());
-            String newIndexName = DataStream.getDefaultBackingIndexName(dataStream.getName(), dataStream.getGeneration() + 1);
-            assertEquals(sourceIndexName, rolloverResult.sourceIndexName);
-            assertEquals(newIndexName, rolloverResult.rolloverIndexName);
+            assertThat(e.getMessage(), equalTo("template [template] has alias and data stream definitions"));
         } finally {
             testThreadPool.shutdown();
         }

+ 3 - 9
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java

@@ -1115,7 +1115,7 @@ public class MetadataIndexTemplateServiceTests extends ESSingleNodeTestCase {
         state = service.addIndexTemplateV2(state, true, "my-template", it);
 
         List<Map<String, AliasMetadata>> resolvedAliases =
-            MetadataIndexTemplateService.resolveAliases(state.metadata(), "my-template", true);
+            MetadataIndexTemplateService.resolveAliases(state.metadata(), "my-template");
 
         // These should be order of precedence, so the index template (a3), then ct_high (a1), then ct_low (a2)
         assertThat(resolvedAliases, equalTo(List.of(a3, a1, a2)));
@@ -1132,11 +1132,8 @@ public class MetadataIndexTemplateServiceTests extends ESSingleNodeTestCase {
             .metadata(Metadata.builder().put("1", it).build())
             .build();
         Exception e =
-            expectThrows(IllegalArgumentException.class, () -> MetadataIndexTemplateService.resolveAliases(state1.metadata(), "1", true));
+            expectThrows(IllegalArgumentException.class, () -> MetadataIndexTemplateService.resolveAliases(state1.metadata(), "1"));
         assertThat(e.getMessage(), equalTo("template [1] has alias and data stream definitions"));
-        // Ignoring validation
-        assertThat(MetadataIndexTemplateService.resolveAliases(state1.metadata(), "1", false), equalTo(List.of(a1)));
-        assertWarnings("template [1] has alias and data stream definitions");
 
         // index template can't have data streams and a component template with an aliases
         ComponentTemplate componentTemplate = new ComponentTemplate(new Template(null, null, a1), null, null);
@@ -1145,11 +1142,8 @@ public class MetadataIndexTemplateServiceTests extends ESSingleNodeTestCase {
         ClusterState state2 = ClusterState.builder(ClusterState.EMPTY_STATE)
             .metadata(Metadata.builder().put("1", it).put("c1", componentTemplate).build())
             .build();
-        e = expectThrows(IllegalArgumentException.class, () -> MetadataIndexTemplateService.resolveAliases(state2.metadata(), "1", true));
+        e = expectThrows(IllegalArgumentException.class, () -> MetadataIndexTemplateService.resolveAliases(state2.metadata(), "1"));
         assertThat(e.getMessage(), equalTo("template [1] has alias and data stream definitions"));
-        // Ignoring validation
-        assertThat(MetadataIndexTemplateService.resolveAliases(state2.metadata(), "1", false), equalTo(List.of(a1)));
-        assertWarnings("template [1] has alias and data stream definitions");
     }
 
     public void testAddInvalidTemplate() throws Exception {