Allow setting validation against arbitrary types (#47264)

Today when settings validate, they can only validate against settings that are of the same type. While this strong-type is convenient from a development perspective, it is too limiting in that some settings need to validate against settings of a different type. For example, the list setting xpack.monitoring.exporters.<namespace>.host wants to validate that it is non-empty if and only if the string setting xpack.monitoring.exporters.<namespace>.type is "http". Today this is impossible since the settings validation framework only allows that setting to validate against other list settings. This commit increases the flexibility here to validate against settings of arbitrary type, at the expense of losing strong-typing during development.
elastic · Oct 2, 2019 · 52b97ec · 52b97ec
1 parent 2e3eb4b
commit 52b97ec
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 72 deletions.
diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java
@@ -68,6 +68,7 @@
 import java.util.EnumSet;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
@@ -154,16 +155,20 @@ static Setting<Integer> buildNumberOfShardsSetting() {
     public static final Setting<Integer> INDEX_ROUTING_PARTITION_SIZE_SETTING =
             Setting.intSetting(SETTING_ROUTING_PARTITION_SIZE, 1, 1, Property.IndexScope);
 
-    public static final Setting<Integer> INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING =
-        Setting.intSetting("index.number_of_routing_shards", INDEX_NUMBER_OF_SHARDS_SETTING,
-                           1, new Setting.Validator<Integer>() {
+    public static final Setting<Integer> INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING = Setting.intSetting(
+        "index.number_of_routing_shards",
+        INDEX_NUMBER_OF_SHARDS_SETTING,
+        1,
+        new Setting.Validator<Integer>() {
+
             @Override
-            public void validate(Integer value) {
+            public void validate(final Integer value) {
+
             }
 
             @Override
-            public void validate(Integer numRoutingShards, Map<Setting<Integer>, Integer> settings) {
-                Integer numShards = settings.get(INDEX_NUMBER_OF_SHARDS_SETTING);
+            public void validate(final Integer numRoutingShards, final Map<Setting<?>, Object> settings) {
+                int numShards = (int) settings.get(INDEX_NUMBER_OF_SHARDS_SETTING);
                 if (numRoutingShards < numShards) {
                     throw new IllegalArgumentException("index.number_of_routing_shards [" + numRoutingShards
                         + "] must be >= index.number_of_shards [" + numShards + "]");
@@ -172,10 +177,13 @@ public void validate(Integer numRoutingShards, Map<Setting<Integer>, Integer> se
             }
 
             @Override
-            public Iterator<Setting<Integer>> settings() {
-                return Collections.singleton(INDEX_NUMBER_OF_SHARDS_SETTING).iterator();
+            public Iterator<Setting<?>> settings() {
+                final List<Setting<?>> settings = Collections.singletonList(INDEX_NUMBER_OF_SHARDS_SETTING);
+                return settings.iterator();
             }
-        }, Property.IndexScope);
+
+        },
+        Property.IndexScope);
 
     public static final String SETTING_AUTO_EXPAND_REPLICAS = "index.auto_expand_replicas";
     public static final Setting<AutoExpandReplicas> INDEX_AUTO_EXPAND_REPLICAS_SETTING = AutoExpandReplicas.SETTING;

diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java
@@ -29,6 +29,7 @@
 
 import java.util.Arrays;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
@@ -109,68 +110,72 @@ static final class LowDiskWatermarkValidator implements Setting.Validator<String
 
         @Override
         public void validate(String value) {
+
         }
 
         @Override
-        public void validate(String value, Map<Setting<String>, String> settings) {
-            final String highWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
-            final String floodStageRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
+        public void validate(final String value, final Map<Setting<?>, Object> settings) {
+            final String highWatermarkRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
+            final String floodStageRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
             doValidate(value, highWatermarkRaw, floodStageRaw);
         }
 
         @Override
-        public Iterator<Setting<String>> settings() {
-            return Arrays.asList(
+        public Iterator<Setting<?>> settings() {
+            final List<Setting<?>> settings = Arrays.asList(
                     CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING,
-                    CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING)
-                    .iterator();
+                    CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
+            return settings.iterator();
         }
 
     }
 
     static final class HighDiskWatermarkValidator implements Setting.Validator<String> {
 
         @Override
-        public void validate(String value) {
+        public void validate(final String value) {
+
         }
 
         @Override
-        public void validate(String value, Map<Setting<String>, String> settings) {
-            final String lowWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);
-            final String floodStageRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
+        public void validate(final String value, final Map<Setting<?>, Object> settings) {
+            final String lowWatermarkRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);
+            final String floodStageRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
             doValidate(lowWatermarkRaw, value, floodStageRaw);
         }
 
         @Override
-        public Iterator<Setting<String>> settings() {
-            return Arrays.asList(
+        public Iterator<Setting<?>> settings() {
+            final List<Setting<?>> settings = Arrays.asList(
                     CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING,
-                    CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING)
-                    .iterator();
+                    CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
+            return settings.iterator();
         }
 
     }
 
     static final class FloodStageValidator implements Setting.Validator<String> {
 
         @Override
-        public void validate(String value) {
+        public void validate(final String value) {
+
         }
 
         @Override
-        public void validate(String value, Map<Setting<String>, String> settings) {
-            final String lowWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);
-            final String highWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
+        public void validate(final String value, final Map<Setting<?>, Object> settings) {
+            final String lowWatermarkRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);
+            final String highWatermarkRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
             doValidate(lowWatermarkRaw, highWatermarkRaw, value);
         }
 
         @Override
-        public Iterator<Setting<String>> settings() {
-            return Arrays.asList(
+        public Iterator<Setting<?>> settings() {
+            final List<Setting<?>> settings = Arrays.asList(
                     CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING,
-                    CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING)
-                    .iterator();
+                    CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
+            return settings.iterator();
         }
+
     }
 
     private static void doValidate(String low, String high, String flood) {

diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java
@@ -431,12 +431,12 @@ private T get(Settings settings, boolean validate) {
         try {
             T parsed = parser.apply(value);
             if (validate) {
-                final Iterator<Setting<T>> it = validator.settings();
-                final Map<Setting<T>, T> map;
+                final Iterator<Setting<?>> it = validator.settings();
+                final Map<Setting<?>, Object> map;
                 if (it.hasNext()) {
                     map = new HashMap<>();
                     while (it.hasNext()) {
-                        final Setting<T> setting = it.next();
+                        final Setting<?> setting = it.next();
                         map.put(setting, setting.get(settings, false)); // we have to disable validation or we will stack overflow
                     }
                 } else {
@@ -863,7 +863,7 @@ public interface Validator<T> {
          * @param value    the value of this setting
          * @param settings a map from the settings specified by {@link #settings()}} to their values
          */
-        default void validate(T value, Map<Setting<T>, T> settings) {
+        default void validate(T value, Map<Setting<?>, Object> settings) {
         }
 
         /**
@@ -873,7 +873,7 @@ default void validate(T value, Map<Setting<T>, T> settings) {
          *
          * @return the settings on which the validity of this setting depends.
          */
-        default Iterator<Setting<T>> settings() {
+        default Iterator<Setting<?>> settings() {
             return Collections.emptyIterator();
         }
 

diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java
@@ -324,8 +324,8 @@ public void validate(final String value) {
         }
 
         @Override
-        public void validate(final String value, final Map<Setting<String>, String> settings) {
-            final String requiredPipeline = settings.get(IndexSettings.REQUIRED_PIPELINE);
+        public void validate(final String value, final Map<Setting<?>, Object> settings) {
+            final String requiredPipeline = (String) settings.get(IndexSettings.REQUIRED_PIPELINE);
             if (value.equals(IngestService.NOOP_PIPELINE_NAME) == false
                 && requiredPipeline.equals(IngestService.NOOP_PIPELINE_NAME) == false) {
                 throw new IllegalArgumentException(
@@ -334,8 +334,9 @@ public void validate(final String value, final Map<Setting<String>, String> sett
         }
 
         @Override
-        public Iterator<Setting<String>> settings() {
-            return Collections.singletonList(REQUIRED_PIPELINE).iterator();
+        public Iterator<Setting<?>> settings() {
+            final List<Setting<?>> settings = Collections.singletonList(REQUIRED_PIPELINE);
+            return settings.iterator();
         }
 
     }
@@ -348,17 +349,18 @@ public void validate(final String value) {
         }
 
         @Override
-        public void validate(final String value, final Map<Setting<String>, String> settings) {
-            final String defaultPipeline = settings.get(IndexSettings.DEFAULT_PIPELINE);
+        public void validate(final String value, final Map<Setting<?>, Object> settings) {
+            final String defaultPipeline = (String) settings.get(IndexSettings.DEFAULT_PIPELINE);
             if (value.equals(IngestService.NOOP_PIPELINE_NAME) && defaultPipeline.equals(IngestService.NOOP_PIPELINE_NAME) == false) {
                 throw new IllegalArgumentException(
                     "index has a required pipeline [" + value + "] and a default pipeline [" + defaultPipeline + "]");
             }
         }
 
         @Override
-        public Iterator<Setting<String>> settings() {
-            return Collections.singletonList(DEFAULT_PIPELINE).iterator();
+        public Iterator<Setting<?>> settings() {
+            final List<Setting<?>> settings = Collections.singletonList(DEFAULT_PIPELINE);
+            return settings.iterator();
         }
 
     }

diff --git a/server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java b/server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java
@@ -28,6 +28,7 @@
 import org.elasticsearch.node.Node;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
@@ -77,45 +78,53 @@ public final class AutoQueueAdjustingExecutorBuilder extends ExecutorBuilder<Aut
             Integer.toString(minQueueSize),
             s -> Setting.parseInt(s, 0, minSizeKey),
             new Setting.Validator<Integer>() {
+
                 @Override
-                public void validate(Integer value) {
+                public void validate(final Integer value) {
+
                 }
 
                 @Override
-                public void validate(Integer value, Map<Setting<Integer>, Integer> settings) {
-                    if (value > settings.get(tempMaxQueueSizeSetting)) {
+                public void validate(final Integer value, final Map<Setting<?>, Object> settings) {
+                    if (value > (int) settings.get(tempMaxQueueSizeSetting)) {
                         throw new IllegalArgumentException("Failed to parse value [" + value + "] for setting [" + minSizeKey
                             + "] must be <= " + settings.get(tempMaxQueueSizeSetting));
                     }
                 }
 
                 @Override
-                public Iterator<Setting<Integer>> settings() {
-                    return Arrays.asList(tempMaxQueueSizeSetting).iterator();
+                public Iterator<Setting<?>> settings() {
+                    final List<Setting<?>> settings = Collections.singletonList(tempMaxQueueSizeSetting);
+                    return settings.iterator();
                 }
+
             },
             Setting.Property.NodeScope);
         this.maxQueueSizeSetting = new Setting<>(
                 maxSizeKey,
                 Integer.toString(maxQueueSize),
                 s -> Setting.parseInt(s, 0, maxSizeKey),
                 new Setting.Validator<Integer>() {
+
                     @Override
                     public void validate(Integer value) {
+
                     }
 
                     @Override
-                    public void validate(Integer value, Map<Setting<Integer>, Integer> settings) {
-                        if (value < settings.get(tempMinQueueSizeSetting)) {
+                    public void validate(final Integer value, final Map<Setting<?>, Object> settings) {
+                        if (value < (int) settings.get(tempMinQueueSizeSetting)) {
                             throw new IllegalArgumentException("Failed to parse value [" + value + "] for setting [" + minSizeKey
                                 + "] must be >= " + settings.get(tempMinQueueSizeSetting));
                         }
                     }
 
                     @Override
-                    public Iterator<Setting<Integer>> settings() {
-                        return Arrays.asList(tempMinQueueSizeSetting).iterator();
+                    public Iterator<Setting<?>> settings() {
+                        final List<Setting<?>> settings = Collections.singletonList(tempMinQueueSizeSetting);
+                        return settings.iterator();
                     }
+
                 },
                 Setting.Property.NodeScope);
         this.frameSizeSetting = Setting.intSetting(frameSizeKey, frameSize, 100, Setting.Property.NodeScope);