String settings leniently accept lists

[DaveCTurner]

If you got here from the good first issue label, see this comment.

For example, the following elasticsearch.yml ...

node.name:
- node-a
- node-b

... starts up a node called [node-a, node-b]:

[2018-08-24T17:08:43,807][INFO ][o.e.n.Node               ] [[node-a, node-b]] initializing ...

In fact this is what's happening in #30946. The following API call...

PUT _cluster/settings
{
  "transient" : {
    "cluster.routing.allocation.exclude._name" : [ "elasticsearch-data-0" ]
  }
}

... sets cluster.routing.allocation.exclude._name to the string [elasticsearch-data-0], which then does not match the node called elasticsearch-data-0 so the allocation is not filtered as expected.

[elasticmachine]

Pinging @elastic/es-core-infra

[original-brownbear]

The fix broke 6.x Tribe tests, reopening ...
see #33660

[original-brownbear]

@s1monw @jasontedor question here:

Is it true that the only reason we can't do 6075e15 is because they break the "dummy settings" used by Tribe here: https://github.com/elastic/elasticsearch/blob/6.x/modules/tribe/src/main/java/org/elasticsearch/tribe/TribePlugin.java#L177
... because those always assume String / scalar type for the dummies even if the key would in fact contain a list?

Or is this kind of hard validation a problem beyond just Tribe and we want to allow this kind of "validation" or mixed list and scalar settings in general?

[bczifra]

A use can specify node.attr.foo : [ "bar", "baz" ] in elasticsearch.yml, and Elasticsearch starts up without logging anything at all indicating that this is not supported behavior, so setting "index.routing.allocation.require.foo": "bar" won't match "[bar, baz]" and the index shards won't be allocated. This can be particularly confusing because allocation awareness attributes do support multiple values.

[gwbrown]

@original-brownbear Did you ever get an answer to your question? Can we reintroduce the fix now that Tribe has been removed?

[original-brownbear]

@gwbrown

Did you ever get an answer to your question? Can we reintroduce the fix now that Tribe has been removed?

No but I think we can give the fix another go, it did only affect Tribe in the past. Do you want/need this in the near future? (should be trivial to open a new PR that is the same as the original fix)

[gwbrown]

Thanks! I have no particular need for this - this is part of core/infra's issue cleanup and I was just checking on the current status. Sounds like we just need someone to make sure your original patch from #33503 is up to date for current master and re-submit.

[amitds1997]

If no one's working on this, can I give this a shot?

[rjernst]

@amitds1997 You are welcome to try. Note that this may be a little tricky, though I don't want to deter you from attempting it. I believe the underlying problem is Settings.get calls toString() internally. This is so other types (eg numbers) always get returned as Strings. Fixing will probably involve adjusting that assumption.

[amitds1997]

@rjernst What I gathered from gwbrown's comment is that we have to re-implement the original patch against the master. Please let me know if I have missed out on anything not present in that patch.

[rjernst]

@amitds1997 That could be it, I'm not sure. It should be simple to test, though.

[mosche]

Still an issue, discussing with Core/Infra to prioritise

[elasticsearchmachine]

Pinging @elastic/es-core-infra (Team:Core/Infra)

[mosche]

Note: This will require a deprecation warning to prevent clusters from breaking when upgrading.