honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Jason Tedor 2019-10-02 15:11:51 -05:00
parent 2e3eb4b24e
commit 52b97ec539
No known key found for this signature in database
GPG Key ID: FA89F05560F16BC5
7 changed files with 110 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java
View File

@ -68,6 +68,7 @@ import java.util.Collections;
import java.util.EnumSet; import java.util.EnumSet;
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
@ -154,16 +155,20 @@ public class IndexMetaData implements Diffable<IndexMetaData>, ToXContentFragmen
public static final Setting<Integer> INDEX_ROUTING_PARTITION_SIZE_SETTING = public static final Setting<Integer> INDEX_ROUTING_PARTITION_SIZE_SETTING =
Setting.intSetting(SETTING_ROUTING_PARTITION_SIZE, 1, 1, Property.IndexScope); Setting.intSetting(SETTING_ROUTING_PARTITION_SIZE, 1, 1, Property.IndexScope);
public static final Setting<Integer> INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING = public static final Setting<Integer> INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING = Setting.intSetting(
Setting.intSetting("index.number_of_routing_shards", INDEX_NUMBER_OF_SHARDS_SETTING, "index.number_of_routing_shards",
1, new Setting.Validator<Integer>() { INDEX_NUMBER_OF_SHARDS_SETTING,
1,
new Setting.Validator<Integer>() {
@Override @Override
public void validate(Integer value) { public void validate(final Integer value) {
} }
@Override @Override
public void validate(Integer numRoutingShards, Map<Setting<Integer>, Integer> settings) { public void validate(final Integer numRoutingShards, final Map<Setting<?>, Object> settings) {
Integer numShards = settings.get(INDEX_NUMBER_OF_SHARDS_SETTING); int numShards = (int) settings.get(INDEX_NUMBER_OF_SHARDS_SETTING);
if (numRoutingShards < numShards) { if (numRoutingShards < numShards) {
throw new IllegalArgumentException("index.number_of_routing_shards [" + numRoutingShards throw new IllegalArgumentException("index.number_of_routing_shards [" + numRoutingShards
+ "] must be >= index.number_of_shards [" + numShards + "]"); + "] must be >= index.number_of_shards [" + numShards + "]");
@ -172,10 +177,13 @@ public class IndexMetaData implements Diffable<IndexMetaData>, ToXContentFragmen
} }
@Override @Override
public Iterator<Setting<Integer>> settings() { public Iterator<Setting<?>> settings() {
return Collections.singleton(INDEX_NUMBER_OF_SHARDS_SETTING).iterator(); 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 String SETTING_AUTO_EXPAND_REPLICAS = "index.auto_expand_replicas";
public static final Setting<AutoExpandReplicas> INDEX_AUTO_EXPAND_REPLICAS_SETTING = AutoExpandReplicas.SETTING; public static final Setting<AutoExpandReplicas> INDEX_AUTO_EXPAND_REPLICAS_SETTING = AutoExpandReplicas.SETTING;

51
server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java
View File

@ -29,6 +29,7 @@ import org.elasticsearch.common.unit.TimeValue;
import java.util.Arrays; import java.util.Arrays;
import java.util.Iterator; import java.util.Iterator;
import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map; import java.util.Map;
@ -109,21 +110,22 @@ public class DiskThresholdSettings {
@Override @Override
public void validate(String value) { public void validate(String value) {
} }
@Override @Override
public void validate(String value, Map<Setting<String>, String> settings) { public void validate(final String value, final Map<Setting<?>, Object> settings) {
final String highWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING); final String highWatermarkRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
final String floodStageRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING); final String floodStageRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
doValidate(value, highWatermarkRaw, floodStageRaw); doValidate(value, highWatermarkRaw, floodStageRaw);
} }
@Override @Override
public Iterator<Setting<String>> settings() { public Iterator<Setting<?>> settings() {
return Arrays.asList( final List<Setting<?>> settings = Arrays.asList(
CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING, CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING,
CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING) CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
.iterator(); return settings.iterator();
} }
} }
@ -131,22 +133,23 @@ public class DiskThresholdSettings {
static final class HighDiskWatermarkValidator implements Setting.Validator<String> { static final class HighDiskWatermarkValidator implements Setting.Validator<String> {
@Override @Override
public void validate(String value) { public void validate(final String value) {
} }
@Override @Override
public void validate(String value, Map<Setting<String>, String> settings) { public void validate(final String value, final Map<Setting<?>, Object> settings) {
final String lowWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING); final String lowWatermarkRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);
final String floodStageRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING); final String floodStageRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
doValidate(lowWatermarkRaw, value, floodStageRaw); doValidate(lowWatermarkRaw, value, floodStageRaw);
} }
@Override @Override
public Iterator<Setting<String>> settings() { public Iterator<Setting<?>> settings() {
return Arrays.asList( final List<Setting<?>> settings = Arrays.asList(
CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING, CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING,
CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING) CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING);
.iterator(); return settings.iterator();
} }
} }
@ -154,23 +157,25 @@ public class DiskThresholdSettings {
static final class FloodStageValidator implements Setting.Validator<String> { static final class FloodStageValidator implements Setting.Validator<String> {
@Override @Override
public void validate(String value) { public void validate(final String value) {
} }
@Override @Override
public void validate(String value, Map<Setting<String>, String> settings) { public void validate(final String value, final Map<Setting<?>, Object> settings) {
final String lowWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING); final String lowWatermarkRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);
final String highWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING); final String highWatermarkRaw = (String) settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
doValidate(lowWatermarkRaw, highWatermarkRaw, value); doValidate(lowWatermarkRaw, highWatermarkRaw, value);
} }
@Override @Override
public Iterator<Setting<String>> settings() { public Iterator<Setting<?>> settings() {
return Arrays.asList( final List<Setting<?>> settings = Arrays.asList(
CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING, CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING,
CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING) CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
.iterator(); return settings.iterator();
} }
} }
private static void doValidate(String low, String high, String flood) { private static void doValidate(String low, String high, String flood) {

10
server/src/main/java/org/elasticsearch/common/settings/Setting.java
View File

@ -431,12 +431,12 @@ public class Setting<T> implements ToXContentObject {
try { try {
T parsed = parser.apply(value); T parsed = parser.apply(value);
if (validate) { if (validate) {
final Iterator<Setting<T>> it = validator.settings(); final Iterator<Setting<?>> it = validator.settings();
final Map<Setting<T>, T> map; final Map<Setting<?>, Object> map;
if (it.hasNext()) { if (it.hasNext()) {
map = new HashMap<>(); map = new HashMap<>();
while (it.hasNext()) { 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 map.put(setting, setting.get(settings, false)); // we have to disable validation or we will stack overflow
} }
} else { } else {
@ -863,7 +863,7 @@ public class Setting<T> implements ToXContentObject {
* @param value the value of this setting * @param value the value of this setting
* @param settings a map from the settings specified by {@link #settings()}} to their values * @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 @@ public class Setting<T> implements ToXContentObject {
* *
* @return the settings on which the validity of this setting depends. * @return the settings on which the validity of this setting depends.
*/ */
default Iterator<Setting<T>> settings() { default Iterator<Setting<?>> settings() {
return Collections.emptyIterator(); return Collections.emptyIterator();
} }

18
server/src/main/java/org/elasticsearch/index/IndexSettings.java
View File

@ -324,8 +324,8 @@ public final class IndexSettings {
} }
@Override @Override
public void validate(final String value, final Map<Setting<String>, String> settings) { public void validate(final String value, final Map<Setting<?>, Object> settings) {
final String requiredPipeline = settings.get(IndexSettings.REQUIRED_PIPELINE); final String requiredPipeline = (String) settings.get(IndexSettings.REQUIRED_PIPELINE);
if (value.equals(IngestService.NOOP_PIPELINE_NAME) == false if (value.equals(IngestService.NOOP_PIPELINE_NAME) == false
&& requiredPipeline.equals(IngestService.NOOP_PIPELINE_NAME) == false) { && requiredPipeline.equals(IngestService.NOOP_PIPELINE_NAME) == false) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
@ -334,8 +334,9 @@ public final class IndexSettings {
} }
@Override @Override
public Iterator<Setting<String>> settings() { public Iterator<Setting<?>> settings() {
return Collections.singletonList(REQUIRED_PIPELINE).iterator(); final List<Setting<?>> settings = Collections.singletonList(REQUIRED_PIPELINE);
return settings.iterator();
} }
} }
@ -348,8 +349,8 @@ public final class IndexSettings {
} }
@Override @Override
public void validate(final String value, final Map<Setting<String>, String> settings) { public void validate(final String value, final Map<Setting<?>, Object> settings) {
final String defaultPipeline = settings.get(IndexSettings.DEFAULT_PIPELINE); final String defaultPipeline = (String) settings.get(IndexSettings.DEFAULT_PIPELINE);
if (value.equals(IngestService.NOOP_PIPELINE_NAME) && defaultPipeline.equals(IngestService.NOOP_PIPELINE_NAME) == false) { if (value.equals(IngestService.NOOP_PIPELINE_NAME) && defaultPipeline.equals(IngestService.NOOP_PIPELINE_NAME) == false) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
"index has a required pipeline [" + value + "] and a default pipeline [" + defaultPipeline + "]"); "index has a required pipeline [" + value + "] and a default pipeline [" + defaultPipeline + "]");
@ -357,8 +358,9 @@ public final class IndexSettings {
} }
@Override @Override
public Iterator<Setting<String>> settings() { public Iterator<Setting<?>> settings() {
return Collections.singletonList(DEFAULT_PIPELINE).iterator(); final List<Setting<?>> settings = Collections.singletonList(DEFAULT_PIPELINE);
return settings.iterator();
} }
} }

27
server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java
View File

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

41
server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java
View File

@ -29,6 +29,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
@ -508,14 +509,17 @@ public class SettingsUpdaterTests extends ESTestCase {
private static Setting<String> invalidInIsolationSetting(int index) { private static Setting<String> invalidInIsolationSetting(int index) {
return Setting.simpleString("invalid.setting" + index, return Setting.simpleString("invalid.setting" + index,
new Setting.Validator<String>() { new Setting.Validator<String>() {
@Override @Override
public void validate(String value) { public void validate(final String value) {
throw new IllegalArgumentException("Invalid in isolation setting"); throw new IllegalArgumentException("Invalid in isolation setting");
} }
@Override @Override
public void validate(String value, Map<Setting<String>, String> settings) { public void validate(final String value, final Map<Setting<?>, Object> settings) {
} }
}, },
Property.NodeScope); Property.NodeScope);
} }
@ -523,52 +527,61 @@ public class SettingsUpdaterTests extends ESTestCase {
private static Setting<String> invalidWithDependenciesSetting(int index) { private static Setting<String> invalidWithDependenciesSetting(int index) {
return Setting.simpleString("invalid.setting" + index, return Setting.simpleString("invalid.setting" + index,
new Setting.Validator<String>() { new Setting.Validator<String>() {
@Override @Override
public void validate(String value) { public void validate(final String value) {
} }
@Override @Override
public void validate(String value, Map<Setting<String>, String> settings) { public void validate(final String value, final Map<Setting<?>, Object> settings) {
throw new IllegalArgumentException("Invalid with dependencies setting"); throw new IllegalArgumentException("Invalid with dependencies setting");
} }
}, },
Property.NodeScope); Property.NodeScope);
} }
private static class FooLowSettingValidator implements Setting.Validator<Integer> { private static class FooLowSettingValidator implements Setting.Validator<Integer> {
@Override @Override
public void validate(Integer value) { public void validate(final Integer value) {
} }
@Override @Override
public void validate(Integer low, Map<Setting<Integer>, Integer> settings) { public void validate(final Integer low, final Map<Setting<?>, Object> settings) {
if (settings.containsKey(SETTING_FOO_HIGH) && low > settings.get(SETTING_FOO_HIGH)) { if (settings.containsKey(SETTING_FOO_HIGH) && low > (int) settings.get(SETTING_FOO_HIGH)) {
throw new IllegalArgumentException("[low]=" + low + " is higher than [high]=" + settings.get(SETTING_FOO_HIGH)); throw new IllegalArgumentException("[low]=" + low + " is higher than [high]=" + settings.get(SETTING_FOO_HIGH));
} }
} }
@Override @Override
public Iterator<Setting<Integer>> settings() { public Iterator<Setting<?>> settings() {
return asList(SETTING_FOO_LOW, SETTING_FOO_HIGH).iterator(); final List<Setting<?>> settings = Collections.singletonList(SETTING_FOO_HIGH);
return settings.iterator();
} }
} }
private static class FooHighSettingValidator implements Setting.Validator<Integer> { private static class FooHighSettingValidator implements Setting.Validator<Integer> {
@Override @Override
public void validate(Integer value) { public void validate(final Integer value) {
} }
@Override @Override
public void validate(Integer high, Map<Setting<Integer>, Integer> settings) { public void validate(final Integer high, final Map<Setting<?>, Object> settings) {
if (settings.containsKey(SETTING_FOO_LOW) && high < settings.get(SETTING_FOO_LOW)) { if (settings.containsKey(SETTING_FOO_LOW) && high < (int) settings.get(SETTING_FOO_LOW)) {
throw new IllegalArgumentException("[high]=" + high + " is lower than [low]=" + settings.get(SETTING_FOO_LOW)); throw new IllegalArgumentException("[high]=" + high + " is lower than [low]=" + settings.get(SETTING_FOO_LOW));
} }
} }
@Override @Override
public Iterator<Setting<Integer>> settings() { public Iterator<Setting<?>> settings() {
return asList(SETTING_FOO_LOW, SETTING_FOO_HIGH).iterator(); final List<Setting<?>> settings = Collections.singletonList(SETTING_FOO_LOW);
return settings.iterator();
} }
} }
private static final Setting<Integer> SETTING_FOO_LOW = new Setting<>("foo.low", "10", private static final Setting<Integer> SETTING_FOO_LOW = new Setting<>("foo.low", "10",

9
server/src/test/java/org/elasticsearch/common/settings/SettingTests.java
View File

@ -208,13 +208,13 @@ public class SettingTests extends ESTestCase {
public static boolean invokedWithDependencies; public static boolean invokedWithDependencies;
@Override @Override
public void validate(String value) { public void validate(final String value) {
invokedInIsolation = true; invokedInIsolation = true;
assertThat(value, equalTo("foo.bar value")); assertThat(value, equalTo("foo.bar value"));
} }
@Override @Override
public void validate(String value, Map<Setting<String>, String> settings) { public void validate(final String value, final Map<Setting<?>, Object> settings) {
invokedWithDependencies = true; invokedWithDependencies = true;
assertTrue(settings.keySet().contains(BAZ_QUX_SETTING)); assertTrue(settings.keySet().contains(BAZ_QUX_SETTING));
assertThat(settings.get(BAZ_QUX_SETTING), equalTo("baz.qux value")); assertThat(settings.get(BAZ_QUX_SETTING), equalTo("baz.qux value"));
@ -223,8 +223,9 @@ public class SettingTests extends ESTestCase {
} }
@Override @Override
public Iterator<Setting<String>> settings() { public Iterator<Setting<?>> settings() {
return Arrays.asList(BAZ_QUX_SETTING, QUUX_QUUZ_SETTING).iterator(); final List<Setting<?>> settings = Arrays.asList(BAZ_QUX_SETTING, QUUX_QUUZ_SETTING);
return settings.iterator();
} }
} }