From e2977889b80fcaa02f5c7d5814117e796f2580e2 Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Tue, 17 Jan 2017 08:51:04 -0600 Subject: [PATCH] Allow comma delimited array settings to have a space after each entry (#22591) Previously, certain settings that could take multiple comma delimited values would pick up incorrect values for all entries but the first if each comma separated value was followed by a whitespace character. For example, the multi-value "A,B,C" would be correctly parsed as ["A", "B", "C"] but the multi-value "A, B, C" would be incorrectly parsed as ["A", " B", " C"]. This commit allows a comma separated list to have whitespace characters after each entry. The specific settings that were affected by this are: cluster.routing.allocation.awareness.attributes index.routing.allocation.require.* index.routing.allocation.include.* index.routing.allocation.exclude.* cluster.routing.allocation.require.* cluster.routing.allocation.include.* cluster.routing.allocation.exclude.* http.cors.allow-methods http.cors.allow-headers For the allocation filtering related settings, this commit also provides validation of each specified entry if the filtering is done by _ip, _host_ip, or _publish_ip, to ensure that each entry is a valid IP address. Closes #22297 --- .../cluster/metadata/IndexMetaData.java | 7 +-- .../cluster/node/DiscoveryNodeFilters.java | 23 ++++++++- .../decider/AwarenessAllocationDecider.java | 4 +- .../decider/FilterAllocationDecider.java | 7 +-- .../java/org/elasticsearch/node/Node.java | 11 +++- .../allocation/FilteringAllocationIT.java | 12 +++++ .../node/DiscoveryNodeFiltersTests.java | 11 ++++ .../allocation/AwarenessAllocationTests.java | 50 +++++++++++++++++++ .../FilterAllocationDeciderTests.java | 14 ++++++ .../netty4/Netty4HttpServerTransport.java | 4 +- .../Netty4HttpServerTransportTests.java | 6 ++- .../org/elasticsearch/node/NodeTests.java | 37 ++++++++++++++ 12 files changed, 172 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java index 8c2dc3d47ed..7e0b7e419f4 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java @@ -70,6 +70,7 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; +import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.IP_VALIDATOR; import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.AND; import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.OR; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; @@ -242,11 +243,11 @@ public class IndexMetaData implements Diffable, ToXContent { public static final String INDEX_ROUTING_INCLUDE_GROUP_PREFIX = "index.routing.allocation.include"; public static final String INDEX_ROUTING_EXCLUDE_GROUP_PREFIX = "index.routing.allocation.exclude"; public static final Setting INDEX_ROUTING_REQUIRE_GROUP_SETTING = - Setting.groupSetting(INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".", Property.Dynamic, Property.IndexScope); + Setting.groupSetting(INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.IndexScope); public static final Setting INDEX_ROUTING_INCLUDE_GROUP_SETTING = - Setting.groupSetting(INDEX_ROUTING_INCLUDE_GROUP_PREFIX + ".", Property.Dynamic, Property.IndexScope); + Setting.groupSetting(INDEX_ROUTING_INCLUDE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.IndexScope); public static final Setting INDEX_ROUTING_EXCLUDE_GROUP_SETTING = - Setting.groupSetting(INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".", Property.Dynamic, Property.IndexScope); + Setting.groupSetting(INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.IndexScope); public static final Setting INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING = Setting.groupSetting("index.routing.allocation.initial_recovery."); // this is only setable internally not a registered setting!! diff --git a/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java b/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java index fad86caa7cc..3fcfdc08722 100644 --- a/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java +++ b/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java @@ -21,6 +21,7 @@ package org.elasticsearch.cluster.node; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Settings; @@ -28,6 +29,7 @@ import org.elasticsearch.common.transport.TransportAddress; import java.util.HashMap; import java.util.Map; +import java.util.function.Consumer; public class DiscoveryNodeFilters { @@ -36,6 +38,25 @@ public class DiscoveryNodeFilters { OR } + /** + * Validates the IP addresses in a group of {@link Settings} by looking for the keys + * "_ip", "_host_ip", and "_publish_ip" and ensuring each of their comma separated values + * is a valid IP address. + */ + public static final Consumer IP_VALIDATOR = (settings) -> { + Map settingsMap = settings.getAsMap(); + for (Map.Entry entry : settingsMap.entrySet()) { + String propertyKey = entry.getKey(); + if ("_ip".equals(propertyKey) || "_host_ip".equals(propertyKey) || "_publish_ip".equals(propertyKey)) { + for (String value : Strings.tokenizeToStringArray(entry.getValue(), ",")) { + if (InetAddresses.isInetAddress(value) == false) { + throw new IllegalArgumentException("invalid IP address [" + value + "] for [" + propertyKey + "]"); + } + } + } + } + }; + public static DiscoveryNodeFilters buildFromSettings(OpType opType, String prefix, Settings settings) { return buildFromKeyValue(opType, settings.getByPrefix(prefix).getAsMap()); } @@ -43,7 +64,7 @@ public class DiscoveryNodeFilters { public static DiscoveryNodeFilters buildFromKeyValue(OpType opType, Map filters) { Map bFilters = new HashMap<>(); for (Map.Entry entry : filters.entrySet()) { - String[] values = Strings.splitStringByCommaToArray(entry.getValue()); + String[] values = Strings.tokenizeToStringArray(entry.getValue(), ","); if (values.length > 0) { bFilters.put(entry.getKey(), values); } diff --git a/core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AwarenessAllocationDecider.java b/core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AwarenessAllocationDecider.java index 4fa3225f468..4160fd224aa 100644 --- a/core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AwarenessAllocationDecider.java +++ b/core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AwarenessAllocationDecider.java @@ -78,12 +78,12 @@ public class AwarenessAllocationDecider extends AllocationDecider { public static final String NAME = "awareness"; public static final Setting CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING = - new Setting<>("cluster.routing.allocation.awareness.attributes", "", Strings::splitStringByCommaToArray , Property.Dynamic, + new Setting<>("cluster.routing.allocation.awareness.attributes", "", s -> Strings.tokenizeToStringArray(s, ","), Property.Dynamic, Property.NodeScope); public static final Setting CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING = Setting.groupSetting("cluster.routing.allocation.awareness.force.", Property.Dynamic, Property.NodeScope); - private String[] awarenessAttributes; + private volatile String[] awarenessAttributes; private volatile Map forcedAwarenessAttributes; diff --git a/core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java b/core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java index 855c570a252..85069392eb6 100644 --- a/core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java +++ b/core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; +import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.IP_VALIDATOR; import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.AND; import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.OR; @@ -68,11 +69,11 @@ public class FilterAllocationDecider extends AllocationDecider { private static final String CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX = "cluster.routing.allocation.include"; private static final String CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX = "cluster.routing.allocation.exclude"; public static final Setting CLUSTER_ROUTING_REQUIRE_GROUP_SETTING = - Setting.groupSetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", Property.Dynamic, Property.NodeScope); + Setting.groupSetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.NodeScope); public static final Setting CLUSTER_ROUTING_INCLUDE_GROUP_SETTING = - Setting.groupSetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", Property.Dynamic, Property.NodeScope); + Setting.groupSetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.NodeScope); public static final Setting CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING = - Setting.groupSetting(CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX + ".", Property.Dynamic, Property.NodeScope); + Setting.groupSetting(CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.NodeScope); private volatile DiscoveryNodeFilters clusterRequireFilters; private volatile DiscoveryNodeFilters clusterIncludeFilters; diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 97ab20c7767..05ea6a9bda8 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -181,7 +181,16 @@ public class Node implements Closeable { */ public static final Setting NODE_LOCAL_STORAGE_SETTING = Setting.boolSetting("node.local_storage", true, Property.NodeScope); public static final Setting NODE_NAME_SETTING = Setting.simpleString("node.name", Property.NodeScope); - public static final Setting NODE_ATTRIBUTES = Setting.groupSetting("node.attr.", Property.NodeScope); + public static final Setting NODE_ATTRIBUTES = Setting.groupSetting("node.attr.", (settings) -> { + Map settingsMap = settings.getAsMap(); + for (Map.Entry entry : settingsMap.entrySet()) { + String value = entry.getValue(); + if (Character.isWhitespace(value.charAt(0)) || Character.isWhitespace(value.charAt(value.length() - 1))) { + throw new IllegalArgumentException("node.attr." + entry.getKey() + " cannot have leading or trailing whitespace " + + "[" + value + "]"); + } + } + }, Property.NodeScope); public static final Setting BREAKER_TYPE_KEY = new Setting<>("indices.breaker.type", "hierarchy", (s) -> { switch (s) { case "hierarchy": diff --git a/core/src/test/java/org/elasticsearch/cluster/allocation/FilteringAllocationIT.java b/core/src/test/java/org/elasticsearch/cluster/allocation/FilteringAllocationIT.java index 03866269cae..93771019ef4 100644 --- a/core/src/test/java/org/elasticsearch/cluster/allocation/FilteringAllocationIT.java +++ b/core/src/test/java/org/elasticsearch/cluster/allocation/FilteringAllocationIT.java @@ -24,8 +24,10 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.routing.IndexRoutingTable; import org.elasticsearch.cluster.routing.IndexShardRoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.test.ESIntegTestCase; @@ -144,5 +146,15 @@ public class FilteringAllocationIT extends ESIntegTestCase { clusterState = client().admin().cluster().prepareState().execute().actionGet().getState(); assertThat(clusterState.routingTable().index("test").numberOfNodesShardsAreAllocatedOn(), equalTo(2)); } + + public void testInvalidIPFilterClusterSettings() { + String ipKey = randomFrom("_ip", "_host_ip", "_publish_ip"); + Setting filterSetting = randomFrom(FilterAllocationDecider.CLUSTER_ROUTING_REQUIRE_GROUP_SETTING, + FilterAllocationDecider.CLUSTER_ROUTING_INCLUDE_GROUP_SETTING, FilterAllocationDecider.CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings() + .setTransientSettings(Settings.builder().put(filterSetting.getKey() + ipKey, "192.168.1.1.")) + .execute().actionGet()); + assertEquals("invalid IP address [192.168.1.1.] for [" + ipKey + "]", e.getMessage()); + } } diff --git a/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeFiltersTests.java b/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeFiltersTests.java index 1f226427237..c78de758d96 100644 --- a/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeFiltersTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeFiltersTests.java @@ -245,6 +245,17 @@ public class DiscoveryNodeFiltersTests extends ESTestCase { assertThat(filters.match(node), equalTo(true)); } + public void testCommaSeparatedValuesTrimmed() { + DiscoveryNode node = new DiscoveryNode("", "", "", "", "192.1.1.54", localAddress, singletonMap("tag", "B"), emptySet(), null); + + Settings settings = shuffleSettings(Settings.builder() + .put("xxx." + randomFrom("_ip", "_host_ip", "_publish_ip"), "192.1.1.1, 192.1.1.54") + .put("xxx.tag", "A, B") + .build()); + DiscoveryNodeFilters filters = DiscoveryNodeFilters.buildFromSettings(OR, "xxx.", settings); + assertTrue(filters.match(node)); + } + private Settings shuffleSettings(Settings source) { Settings.Builder settings = Settings.builder(); List keys = new ArrayList<>(source.getAsMap().keySet()); diff --git a/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AwarenessAllocationTests.java b/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AwarenessAllocationTests.java index fec0a33b917..2c1ec07c7fa 100644 --- a/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AwarenessAllocationTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AwarenessAllocationTests.java @@ -32,10 +32,14 @@ import org.elasticsearch.cluster.routing.ShardRoutingState; import org.elasticsearch.cluster.routing.allocation.command.AllocationCommands; import org.elasticsearch.cluster.routing.allocation.command.CancelAllocationCommand; import org.elasticsearch.cluster.routing.allocation.command.MoveAllocationCommand; +import org.elasticsearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.ClusterRebalanceAllocationDecider; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import java.util.HashMap; +import java.util.Map; + import static java.util.Collections.singletonMap; import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; import static org.elasticsearch.cluster.routing.ShardRoutingState.RELOCATING; @@ -803,4 +807,50 @@ public class AwarenessAllocationTests extends ESAllocationTestCase { assertThat(clusterState.getRoutingNodes().shardsWithState(INITIALIZING).size(), equalTo(4)); // +1 for relocating shard. assertThat(clusterState.getRoutingNodes().shardsWithState(UNASSIGNED).size(), equalTo(1)); // Still 1 unassigned. } + + public void testMultipleAwarenessAttributes() { + AllocationService strategy = createAllocationService(Settings.builder() + .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "zone, rack") + .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.getKey() + "zone.values", "a, b") + .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.getKey() + "rack.values", "c, d") + .put(ClusterRebalanceAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ALLOW_REBALANCE_SETTING.getKey(), "always") + .build()); + + logger.info("Building initial routing table for 'testUnbalancedZones'"); + + MetaData metaData = MetaData.builder() + .put(IndexMetaData.builder("test").settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(1)) + .build(); + + RoutingTable initialRoutingTable = RoutingTable.builder().addAsNew(metaData.index("test")).build(); + + ClusterState clusterState = ClusterState.builder( + org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY) + ).metaData(metaData).routingTable(initialRoutingTable).build(); + + logger.info("--> adding two nodes in different zones and do rerouting"); + Map nodeAAttributes = new HashMap<>(); + nodeAAttributes.put("zone", "a"); + nodeAAttributes.put("rack", "c"); + Map nodeBAttributes = new HashMap<>(); + nodeBAttributes.put("zone", "b"); + nodeBAttributes.put("rack", "d"); + clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() + .add(newNode("A-0", nodeAAttributes)) + .add(newNode("B-0", nodeBAttributes)) + ).build(); + clusterState = strategy.reroute(clusterState, "reroute"); + assertThat(clusterState.getRoutingNodes().shardsWithState(STARTED).size(), equalTo(0)); + assertThat(clusterState.getRoutingNodes().shardsWithState(INITIALIZING).size(), equalTo(1)); + + logger.info("--> start the shards (primaries)"); + clusterState = strategy.applyStartedShards(clusterState, clusterState.getRoutingNodes().shardsWithState(INITIALIZING)); + assertThat(clusterState.getRoutingNodes().shardsWithState(STARTED).size(), equalTo(1)); + assertThat(clusterState.getRoutingNodes().shardsWithState(INITIALIZING).size(), equalTo(1)); + + clusterState = strategy.applyStartedShards(clusterState, clusterState.getRoutingNodes().shardsWithState(INITIALIZING)); + logger.info("--> all replicas are allocated and started since we have one node in each zone and rack"); + assertThat(clusterState.getRoutingNodes().shardsWithState(STARTED).size(), equalTo(2)); + assertThat(clusterState.getRoutingNodes().shardsWithState(INITIALIZING).size(), equalTo(0)); + } } diff --git a/core/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterAllocationDeciderTests.java b/core/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterAllocationDeciderTests.java index 5ec162eb719..d4ec30f6e51 100644 --- a/core/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterAllocationDeciderTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterAllocationDeciderTests.java @@ -37,6 +37,8 @@ import org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDeci import org.elasticsearch.cluster.routing.allocation.decider.ReplicaAfterPrimaryActiveAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.SameShardAllocationDecider; import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.IndexScopedSettings; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.snapshots.Snapshot; import org.elasticsearch.snapshots.SnapshotId; @@ -191,4 +193,16 @@ public class FilterAllocationDeciderTests extends ESAllocationTestCase { .build(); return service.reroute(clusterState, "reroute", false); } + + public void testInvalidIPFilter() { + String ipKey = randomFrom("_ip", "_host_ip", "_publish_ip"); + Setting filterSetting = randomFrom(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING, + IndexMetaData.INDEX_ROUTING_INCLUDE_GROUP_SETTING, IndexMetaData.INDEX_ROUTING_EXCLUDE_GROUP_SETTING); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + IndexScopedSettings indexScopedSettings = new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); + indexScopedSettings.updateDynamicSettings(Settings.builder().put(filterSetting.getKey() + ipKey, "192..168.1.1").build(), + Settings.builder().put(Settings.EMPTY), Settings.builder(), "test ip validation"); + }); + assertEquals("invalid IP address [192..168.1.1] for [" + ipKey + "]", e.getMessage()); + } } diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java index 10a0a159c23..b9a81010a72 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java @@ -406,14 +406,14 @@ public class Netty4HttpServerTransport extends AbstractLifecycleComponent implem if (SETTING_CORS_ALLOW_CREDENTIALS.get(settings)) { builder.allowCredentials(); } - String[] strMethods = Strings.splitStringByCommaToArray(SETTING_CORS_ALLOW_METHODS.get(settings)); + String[] strMethods = Strings.tokenizeToStringArray(SETTING_CORS_ALLOW_METHODS.get(settings), ","); HttpMethod[] methods = Arrays.asList(strMethods) .stream() .map(HttpMethod::valueOf) .toArray(size -> new HttpMethod[size]); return builder.allowedRequestMethods(methods) .maxAge(SETTING_CORS_MAX_AGE.get(settings)) - .allowedRequestHeaders(Strings.splitStringByCommaToArray(SETTING_CORS_ALLOW_HEADERS.get(settings))) + .allowedRequestHeaders(Strings.tokenizeToStringArray(SETTING_CORS_ALLOW_HEADERS.get(settings), ",")) .shortCircuit() .build(); } diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java b/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java index e3dd6d8a78e..69dcaad9dd3 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java @@ -51,6 +51,7 @@ import java.util.HashSet; import java.util.Set; import java.util.stream.Collectors; +import static org.elasticsearch.common.Strings.collectionToDelimitedString; import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_CREDENTIALS; import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_HEADERS; import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_METHODS; @@ -89,11 +90,12 @@ public class Netty4HttpServerTransportTests extends ESTestCase { public void testCorsConfig() { final Set methods = new HashSet<>(Arrays.asList("get", "options", "post")); final Set headers = new HashSet<>(Arrays.asList("Content-Type", "Content-Length")); + final String suffix = randomBoolean() ? " " : ""; // sometimes have a leading whitespace between comma delimited elements final Settings settings = Settings.builder() .put(SETTING_CORS_ENABLED.getKey(), true) .put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "*") - .put(SETTING_CORS_ALLOW_METHODS.getKey(), Strings.collectionToCommaDelimitedString(methods)) - .put(SETTING_CORS_ALLOW_HEADERS.getKey(), Strings.collectionToCommaDelimitedString(headers)) + .put(SETTING_CORS_ALLOW_METHODS.getKey(), collectionToDelimitedString(methods, ",", suffix, "")) + .put(SETTING_CORS_ALLOW_HEADERS.getKey(), collectionToDelimitedString(headers, ",", suffix, "")) .put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true) .build(); final Netty4CorsConfig corsConfig = Netty4HttpServerTransport.buildCorsConfig(settings); diff --git a/test/framework/src/main/java/org/elasticsearch/node/NodeTests.java b/test/framework/src/main/java/org/elasticsearch/node/NodeTests.java index 9203b632cba..69291ccaba6 100644 --- a/test/framework/src/main/java/org/elasticsearch/node/NodeTests.java +++ b/test/framework/src/main/java/org/elasticsearch/node/NodeTests.java @@ -138,4 +138,41 @@ public class NodeTests extends ESTestCase { } + public void testNodeAttributes() throws IOException { + String attr = randomAsciiOfLength(5); + Settings.Builder settings = baseSettings().put(Node.NODE_ATTRIBUTES.getKey() + "test_attr", attr); + try (Node node = new MockNode(settings.build(), Collections.singleton(MockTcpTransportPlugin.class))) { + final Settings nodeSettings = randomBoolean() ? node.settings() : node.getEnvironment().settings(); + assertEquals(attr, Node.NODE_ATTRIBUTES.get(nodeSettings).getAsMap().get("test_attr")); + } + + // leading whitespace not allowed + attr = " leading"; + settings = baseSettings().put(Node.NODE_ATTRIBUTES.getKey() + "test_attr", attr); + try (Node node = new MockNode(settings.build(), Collections.singleton(MockTcpTransportPlugin.class))) { + fail("should not allow a node attribute with leading whitespace"); + } catch (IllegalArgumentException e) { + assertEquals("node.attr.test_attr cannot have leading or trailing whitespace [ leading]", e.getMessage()); + } + + // trailing whitespace not allowed + attr = "trailing "; + settings = baseSettings().put(Node.NODE_ATTRIBUTES.getKey() + "test_attr", attr); + try (Node node = new MockNode(settings.build(), Collections.singleton(MockTcpTransportPlugin.class))) { + fail("should not allow a node attribute with trailing whitespace"); + } catch (IllegalArgumentException e) { + assertEquals("node.attr.test_attr cannot have leading or trailing whitespace [trailing ]", e.getMessage()); + } + } + + private static Settings.Builder baseSettings() { + final Path tempDir = createTempDir(); + return Settings.builder() + .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", randomLong())) + .put(Environment.PATH_HOME_SETTING.getKey(), tempDir) + .put(NetworkModule.HTTP_ENABLED.getKey(), false) + .put("transport.type", "mock-socket-network") + .put(Node.NODE_DATA_SETTING.getKey(), true); + } + }