Fix regular indices not on frozen tier validate value

build.gradle

@@ -189,8 +189,8 @@ tasks.register("verifyVersions") {
  * after the backport of the backcompat code is complete.
  */
 
-boolean bwc_tests_enabled = true
-String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
+boolean bwc_tests_enabled = false
+String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/70205" /* place a PR link here when committing bwc changes */
 /*
  * FIPS 140-2 behavior was fixed in 7.11.0. Before that there is no way to run elasticsearch in a
  * JVM that is properly configured to be in fips mode with BCFIPS. For now we need to disable

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java

@@ -81,7 +81,7 @@ public void updateSettings(final UpdateSettingsClusterStateUpdateRequest request
 
         indexScopedSettings.validate(
                 normalizedSettings.filter(s -> Regex.isSimpleMatchPattern(s) == false), // don't validate wildcards
-                false, // don't validate dependencies here we check it below never allow to change the number of shards
+                false, // don't validate values here we check it below never allow to change the number of shards
                 true); // validate internal or private index settings
         for (String key : normalizedSettings.keySet()) {
             Setting setting = indexScopedSettings.get(key);

server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -414,55 +414,55 @@ public synchronized <T> void addSettingsUpdateConsumer(Setting<T> setting, Consu
      * Validates that all settings are registered and valid.
      *
      * @param settings             the settings to validate
-     * @param validateDependencies true if dependent settings should be validated
+     * @param validateValues       true if values should be validated, otherwise only keys are validated
      * @see Setting#getSettingsDependencies(String)
      */
-    public final void validate(final Settings settings, final boolean validateDependencies) {
-        validate(settings, validateDependencies, false, false);
+    public final void validate(final Settings settings, final boolean validateValues) {
+        validate(settings, validateValues, false, false);
     }
 
     /**
      * Validates that all settings are registered and valid.
      *
      * @param settings                       the settings to validate
-     * @param validateDependencies           true if dependent settings should be validated
+     * @param validateValues                 true if values should be validated, otherwise only keys are validated
      * @param validateInternalOrPrivateIndex true if internal index settings should be validated
      * @see Setting#getSettingsDependencies(String)
      */
-    public final void validate(final Settings settings, final boolean validateDependencies, final boolean validateInternalOrPrivateIndex) {
-        validate(settings, validateDependencies, false, false, validateInternalOrPrivateIndex);
+    public final void validate(final Settings settings, final boolean validateValues, final boolean validateInternalOrPrivateIndex) {
+        validate(settings, validateValues, false, false, validateInternalOrPrivateIndex);
     }
 
     /**
      * Validates that all settings are registered and valid.
      *
      * @param settings               the settings
-     * @param validateDependencies   true if dependent settings should be validated
+     * @param validateValues         true if values should be validated, otherwise only keys are validated
      * @param ignorePrivateSettings  true if private settings should be ignored during validation
      * @param ignoreArchivedSettings true if archived settings should be ignored during validation
      * @see Setting#getSettingsDependencies(String)
      */
     public final void validate(
             final Settings settings,
-            final boolean validateDependencies,
+            final boolean validateValues,
             final boolean ignorePrivateSettings,
             final boolean ignoreArchivedSettings) {
-        validate(settings, validateDependencies, ignorePrivateSettings, ignoreArchivedSettings, false);
+        validate(settings, validateValues, ignorePrivateSettings, ignoreArchivedSettings, false);
     }
 
     /**
      * Validates that all settings are registered and valid.
      *
      * @param settings                       the settings
-     * @param validateDependencies           true if dependent settings should be validated
+     * @param validateValues                 true if values should be validated, otherwise only keys are validated
      * @param ignorePrivateSettings          true if private settings should be ignored during validation
      * @param ignoreArchivedSettings         true if archived settings should be ignored during validation
      * @param validateInternalOrPrivateIndex true if index internal settings should be validated
      * @see Setting#getSettingsDependencies(String)
      */
     public final void validate(
             final Settings settings,
-            final boolean validateDependencies,
+            final boolean validateValues,
             final boolean ignorePrivateSettings,
             final boolean ignoreArchivedSettings,
             final boolean validateInternalOrPrivateIndex) {
@@ -476,7 +476,7 @@ public final void validate(
                 continue;
             }
             try {
-                validate(key, settings, validateDependencies, validateInternalOrPrivateIndex);
+                validate(key, settings, validateValues, validateInternalOrPrivateIndex);
             } catch (final RuntimeException ex) {
                 exceptions.add(ex);
             }
@@ -489,7 +489,7 @@ public final void validate(
      *
      * @param key the key of the setting to validate
      * @param settings the settings
-     * @param validateDependencies true if dependent settings should be validated
+     * @param validateDependencies true if dependent settings and settings where validation has dependencies should be validated
      * @throws IllegalArgumentException if the setting is invalid
      */
     void validate(final String key, final Settings settings, final boolean validateDependencies) {
@@ -501,12 +501,12 @@ void validate(final String key, final Settings settings, final boolean validateD
      *
      * @param key                            the key of the setting to validate
      * @param settings                       the settings
-     * @param validateDependencies           true if dependent settings should be validated
+     * @param validateValue                  true if value should be validated, otherwise only keys are validated
      * @param validateInternalOrPrivateIndex true if internal index settings should be validated
      * @throws IllegalArgumentException if the setting is invalid
      */
     void validate(
-            final String key, final Settings settings, final boolean validateDependencies, final boolean validateInternalOrPrivateIndex) {
+            final String key, final Settings settings, final boolean validateValue, final boolean validateInternalOrPrivateIndex) {
         Setting<?> setting = getRaw(key);
         if (setting == null) {
             LevenshteinDistance ld = new LevenshteinDistance();
@@ -537,7 +537,7 @@ void validate(
             if (setting.hasComplexMatcher()) {
                 setting = setting.getConcreteSetting(key);
             }
-            if (validateDependencies && settingsDependencies.isEmpty() == false) {
+            if (validateValue && settingsDependencies.isEmpty() == false) {
                 for (final Setting.SettingDependency settingDependency : settingsDependencies) {
                     final Setting<?> dependency = settingDependency.getSetting();
                     // validate the dependent setting is set
@@ -564,7 +564,9 @@ void validate(
                 }
             }
         }
-        setting.get(settings);
+        if (validateValue) {
+            setting.get(settings);
+        }
     }
 
     /**

server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java

@@ -82,6 +82,12 @@ public T get(Settings settings) {
         }
     }
 
+    @Override
+    void validateWithoutDependencies(Settings settings) {
+        // secure settings have no Setting.Validator
+        get(settings);
+    }
+
     /**
      * Returns the digest of this secure setting's value or {@code null} if the setting is missing (inside the keystore). This method can be
      * called even after the {@code SecureSettings} have been closed, unlike {@code #get(Settings)}. The digest is used to check for changes

server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -26,9 +26,11 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
@@ -259,6 +261,58 @@ public void testDependentSettingsWithFallback() {
         service.validate(Settings.builder().put("fallback.test.name", "test").put("foo.test.bar", 7).build(), true);
     }
 
+    public void testValidateValue() {
+        final boolean nodeSetting = randomBoolean();
+        final String prefix = nodeSetting ? "" : "index.";
+        final Property scopeProperty = nodeSetting ? Property.NodeScope : Property.IndexScope;
+        final Setting.Validator<String> baseValidator = s -> {
+            if (s.length() > 1) {
+                throw new IllegalArgumentException("too long");
+            }
+        };
+        final Setting<String> baseSetting = Setting.simpleString(prefix + "foo.base", baseValidator,
+            Property.Dynamic, scopeProperty);
+        final Setting.Validator<String> dependingValidator = new Setting.Validator<String>() {
+            @Override
+            public void validate(String value) {
+            }
+
+            @Override
+            public void validate(String value, Map<Setting<?>, Object> settings, boolean isPresent) {
+                if (Objects.equals(value, settings.get(baseSetting)) == false) {
+                    throw new IllegalArgumentException("must have same value");
+                }
+            }
+
+            @Override
+            public Iterator<Setting<?>> settings() {
+                return List.<Setting<?>>of(baseSetting).iterator();
+            }
+        };
+        final Setting<String> dependingSetting = Setting.simpleString(prefix + "foo.depending", dependingValidator, scopeProperty);
+
+        final AbstractScopedSettings service = nodeSetting ? new ClusterSettings(Settings.EMPTY, Set.of(baseSetting, dependingSetting)) :
+            new IndexScopedSettings(Settings.EMPTY, Set.of(baseSetting, dependingSetting));
+
+        service.validate(Settings.builder().put(baseSetting.getKey(), "1").put(dependingSetting.getKey(), 1).build(), true);
+        service.validate(Settings.builder().put(dependingSetting.getKey(), "1").build(), false);
+        final IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> service.validate(Settings.builder().put(dependingSetting.getKey(), "1").build(), true));
+        assertThat(e.getMessage(), equalTo("must have same value"));
+
+        final IllegalArgumentException e2 = expectThrows(
+            IllegalArgumentException.class,
+            () -> service.validate(Settings.builder().put(baseSetting.getKey(), "2").put(dependingSetting.getKey(), "1").build(), true));
+        assertThat(e2.getMessage(), equalTo("must have same value"));
+
+        service.validate(Settings.builder().put(baseSetting.getKey(), "22").build(), false);
+        final IllegalArgumentException e3 = expectThrows(
+            IllegalArgumentException.class,
+            () -> service.validate(Settings.builder().put(baseSetting.getKey(), "22").build(), true));
+        assertThat(e3.getMessage(), equalTo("too long"));
+    }
+
     public void testTupleAffixUpdateConsumer() {
         String prefix = randomAlphaOfLength(3) + "foo.";
         String intSuffix = randomAlphaOfLength(3);
@@ -850,16 +904,16 @@ public void testValidate() {
         assertEquals("unknown setting [i.am.not.a.setting]" + unknownMsgSuffix, e.getMessage());
 
         e = expectThrows(IllegalArgumentException.class, () ->
-            settings.validate(Settings.builder().put("index.store.type", "boom").put("index.number_of_replicas", true).build(), false));
+            settings.validate(Settings.builder().put("index.store.type", "boom").put("index.number_of_replicas", true).build(), true));
         assertEquals("Failed to parse value [true] for setting [index.number_of_replicas]", e.getMessage());
 
         e = expectThrows(IllegalArgumentException.class, () ->
-            settings.validate("index.number_of_replicas", Settings.builder().put("index.number_of_replicas", "true").build(), false));
+            settings.validate("index.number_of_replicas", Settings.builder().put("index.number_of_replicas", "true").build(), true));
         assertEquals("Failed to parse value [true] for setting [index.number_of_replicas]", e.getMessage());
 
         e = expectThrows(IllegalArgumentException.class, () ->
             settings.validate("index.similarity.boolean.type", Settings.builder().put("index.similarity.boolean.type", "mine").build(),
-                false));
+                true));
         assertEquals("illegal value for [index.similarity.boolean] cannot redefine built-in similarity", e.getMessage());
     }
 
@@ -967,7 +1021,7 @@ public void testLoggingUpdates() {
             IllegalArgumentException ex =
                 expectThrows(
                     IllegalArgumentException.class,
-                    () -> settings.validate(Settings.builder().put("logger._root", "boom").build(), false));
+                    () -> settings.validate(Settings.builder().put("logger._root", "boom").build(), true));
             assertEquals("Unknown level constant [BOOM].", ex.getMessage());
             assertEquals(level, LogManager.getRootLogger().getLevel());
             settings.applySettings(Settings.builder().put("logger._root", "TRACE").build());

x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierIT.java

@@ -7,6 +7,7 @@
 
 package org.elasticsearch.xpack.cluster.routing.allocation;
 
+import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest;
 import org.elasticsearch.action.admin.indices.shrink.ResizeType;
 import org.elasticsearch.action.admin.indices.template.put.PutComposableIndexTemplateAction;
 import org.elasticsearch.cluster.health.ClusterHealthStatus;
@@ -24,6 +25,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Map;
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
@@ -267,6 +269,35 @@ public void testTierFilteringIgnoredByFilterAllocationDecider() {
             .get();
     }
 
+    public void testIllegalOnFrozen() {
+        startDataNode();
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> createIndex(index, Settings.builder()
+                .put("index.number_of_shards", 1)
+                .put("index.number_of_replicas", 0)
+                .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, DataTier.DATA_FROZEN)
+                .build()));
+        assertThat(e.getMessage(), equalTo("[data_frozen] tier can only be used for partial searchable snapshots"));
+
+        String initialTier = randomFrom(DataTier.DATA_HOT, DataTier.DATA_WARM, DataTier.DATA_COLD);
+        createIndex(index, Settings.builder()
+            .put("index.number_of_shards", 1)
+            .put("index.number_of_replicas", 0)
+            .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, initialTier)
+            .build());
+
+        IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, () -> updatePreference(DataTier.DATA_FROZEN));
+        assertThat(e2.getMessage(), equalTo("[data_frozen] tier can only be used for partial searchable snapshots"));
+
+        updatePreference(randomValueOtherThan(initialTier, () -> randomFrom(DataTier.DATA_HOT, DataTier.DATA_WARM, DataTier.DATA_COLD)));
+    }
+
+    private void updatePreference(String tier) {
+        client().admin().indices().updateSettings(new UpdateSettingsRequest(index)
+            .settings(Map.of(DataTierAllocationDecider.INDEX_ROUTING_PREFER, tier))).actionGet();
+    }
+
     private DataTiersFeatureSetUsage getUsage() {
         XPackUsageResponse usages = new XPackUsageRequestBuilder(client()).execute().actionGet();
         return usages.getUsages().stream()