No error raised on invalid input to cluster.routing.allocation.exclude._name

[jimmypw]

Elasticsearch version (bin/elasticsearch --version):
7.14.1

Plugins installed: []
none

JVM version (java -version):
bundled with .deb

/usr/share/elasticsearch/jdk/bin/java -version
openjdk version "16.0.2" 2021-07-20
OpenJDK Runtime Environment Temurin-16.0.2+7 (build 16.0.2+7)
OpenJDK 64-Bit Server VM Temurin-16.0.2+7 (build 16.0.2+7, mixed mode, sharing)

OS version (uname -a if on a Unix-like system):
Linux elk00001 4.19.0-17-amd64 #1 SMP Debian 4.19.194-3 (2021-07-18) x86_64 GNU/Linux

Description of the problem including expected versus actual behavior:
The cluster.routing.allocation.exclude._name setting directive of PUT /_cluster/settings accepts non-string values. I am able to submit an array in to this setting which is not honoured, nor is an error raised. The documentation specifies a comma separated string must be provided. https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-cluster.html#cluster-routing-settings

Steps to reproduce:

curl -u 'elastic:<redacted>' -X PUT "localhost:9200/_cluster/settings?pretty" -H 'Content-Type: application/json' -d'
{
  "transient" : {
    "cluster.routing.allocation.exclude._name" : [ "elk00001","elk00002","elk00003"]
  }
}'
response:
{
  "acknowledged" : true,
  "persistent" : { },
  "transient" : {
    "cluster" : {
      "routing" : {
        "allocation" : {
          "exclude" : {
            "_name" : [
              "elk00001",
              "elk00002",
              "elk00003"
            ]
          }
        }
      }
    }
  }
}

root@elk00001:~# curl -u 'elastic:<redacted>' localhost:9200/_cluster/settings?pretty
{
  "persistent" : { },
  "transient" : {
    "cluster" : {
      "routing" : {
        "allocation" : {
          "exclude" : {
            "_name" : [
              "elk00001",
              "elk00002",
              "elk00003"
            ]
          }
        }
      }
    }
  }
}

Provide logs (if relevant):

N/A

[elasticmachine]

Pinging @elastic/es-distributed (Team:Distributed)

[Leaf-Lin]

Just clarify for my own understanding. The correct way to do this is to specify a comma-separated string as documented instead of an array:

curl -u 'elastic:<redacted>' -X PUT "localhost:9200/_cluster/settings?pretty" -H 'Content-Type: application/json' -d'
{
  "transient" : {
    "cluster.routing.allocation.exclude._name" : "elk00001,elk00002,elk00003"
  }
}'

[DaveCTurner]

Although the docs are technically correct, most settings that expect a list of values can be either a comma-separated string or a genuine list as the OP tried. I think rather than raising an error we should accept both for these settings too, partly for consistency and partly because it'd be a breaking change to start to reject genuine lists even though they don't quite work as expected.

The fundamental problem with these settings is that we're parsing the value of the setting ourselves with Strings.tokenizeToStringArray rather than using the setting framework's listSetting functionality to do it properly. I believe the solution is to make the following change to all the relevant settings and then chase through the compile errors:

diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java
index d1b575e5c9f..9657e7647ae 100644
--- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java
+++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java
@@ -60,9 +60,9 @@ public class FilterAllocationDecider extends AllocationDecider {
     private static final String CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX = "cluster.routing.allocation.require";
     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.AffixSetting<String> CLUSTER_ROUTING_REQUIRE_GROUP_SETTING =
+    public static final Setting.AffixSetting<List<String>> CLUSTER_ROUTING_REQUIRE_GROUP_SETTING =
         Setting.prefixKeySetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", key ->
-            Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
+            Setting.listSetting(key, Function.identity(), s -> List.of(), s -> IP_VALIDATOR.accept(key, s)));
     public static final Setting.AffixSetting<String> CLUSTER_ROUTING_INCLUDE_GROUP_SETTING =
         Setting.prefixKeySetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", key ->
             Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));

[idegtiarenko]

I see the similar issue is affecting index.routing.allocation.* in index settings. I assume that code needs to be updated as well