Add transport action immutable state checks #88491
Conversation
grcevski
added
>non-issue
:Core/Infra/Core
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
|
||
| if (errors.isEmpty() == false) { | ||
| throw new IllegalArgumentException( | ||
| format("Failed to process request [%s] with errors: %s", request, String.join(System.lineSeparator(), errors)) |
There was a problem hiding this comment.
I don't know if we have multiline error messages in other exceptions. Will this IAE propgate through a user request (that would return a 400)? eg a user tries to add an ILM policy with an already reserved name, then they get this exception. If we want to go with newlines, I think we need to make sure this gets serialized correctly (ie the newline needs to be escaped in the json response).
Also, I'm not sure System.lineSeparator is necessary? At least for json only \n would be expected.
There was a problem hiding this comment.
OK, sounds good. I'll wrap the errors as an array with [] and commas. That should solve the windows line separator test issue too.
There was a problem hiding this comment.
Will this IAE propgate through a user request (that would return a 400)?
Yes it does go all the way back to the user that issued the REST request.
|
|
||
| assertTrue( | ||
| expectThrows(IllegalArgumentException.class, () -> action.validateForImmutableState(request, clusterState)).getMessage() | ||
| .contains("with errors: [a] set as read-only by [namespace_one]\n" + "[e] set as read-only by [namespace_two]") |
There was a problem hiding this comment.
This assertion will break on windows since \n is hardcoded (at least with the current exception message)
|
I switched it to use similar multi line reporting as in PluginsService.java |
* upstream/master: (2974 commits) Reserved cluster state service (elastic#88527) Add transport action immutable state checks (elastic#88491) Remove suggest flag from index stats docs (elastic#85479) Polling cluster formation state for master-is-stable health indicator (elastic#88397) Add test execution guide in yamlRestTest asciidoc (elastic#88490) Add troubleshooting guide for corrupt repository (elastic#88391) [Transform] Finetune Schedule to be less noisy on retry and retry slower (elastic#88531) Updatable API keys - auto-update legacy RDs (elastic#88514) Fix typo in TransportForceMergeAction and TransportClearIndicesCacheA… (elastic#88064) Fixed NullPointerException on bulk request (elastic#88358) Avoid needless index metadata builders during reroute (elastic#88506) Set metadata on request in API key noop test (elastic#88507) Fix passing positional args to ES in Docker (elastic#88502) Improve description for task api detailed param (elastic#88493) Support cartesian shape with doc values (elastic#88487) Promote usage of Subjects in Authentication class (elastic#88494) Add CCx 2.0 feature flag (elastic#88451) Reword the watcher 'always' and 'never' condition docs (elastic#86105) Simplify azure discovery installation docs (elastic#88404) Breakup FIPS CI testing jobs ... # Conflicts: # server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java # x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
This PR is splitting the commit to add the code that verifies if a master node action is modifying state that was set in operator mode.
Relates to #88224