Move Security to use auto-managed system indices #68375
Conversation
Backport of elastic#67114. Part of elastic#61656. Change the Security plugin so that its system indices are managed automatically by the system indices infrastructure. Also add an `origin` field to `CreateIndexRequest` and `UpdateSettingsRequest`.
pugnascotia
added
>refactoring
:Security/Security
|
Pinging @elastic/es-security (Team:Security) |
|
@elasticmachine update branch |
|
@jaymode / @albertzaharovits there was a test failure in this backport: I think it's legitimate but it's intermittent - looks like in an upgrade scenario, a test tried to create an API key, but it couldn't because the mappings on I could reinstate that behaviour in
Or, I could change Thoughts appreciated. |
|
In these situations, I would prefer to be able to support these features in a mixed version cluster if the changes to the index can be applied (the index change doesn't require a feature only available in a new version). For the purposes of this PR, I'd put the behavior into As a later TODO I think we could generalize this behavior into a special system index class/client that would enable non-breaking updates during rolling upgrades independent of the master version. |
I agree. |
| @@ -219,6 +226,19 @@ private boolean checkIndexAvailable(ClusterState state) { | |||
| } | |||
| } | |||
|
|
|||
| private boolean checkIndexMappingUpToDate(ClusterState clusterState) { | |||
| return checkIndexMappingVersionMatches(clusterState, Version.CURRENT::equals); | |||
There was a problem hiding this comment.
Ah, good catch 👍
In `checkIndexMappingUpToDate(ClusterState)`, use `onOrAfter()` instead of `current()` when checking whether an index is up-to-date.
| // different mappings, settings etc. This is so that rolling upgrade scenarios still work. | ||
| // We check this via the request's origin. Eventually, `SystemIndexManager` will reconfigure | ||
| // the index to the latest settings. | ||
| if (isSystemIndex && Strings.isNullOrEmpty(request.origin())) { |
There was a problem hiding this comment.
This is pretty minor but request.origin() should never be null based on my reading of the code. Is this correct? If it should never be null, then I believe we should just call request.origin().isEmpty() here and the other two places.
While backporting elastic#67114 via elastic#68375, I realised that there are existing upgrade scenarios that expect the `SecurityIndexManager` to update index mappings, so in the backport PR, this capability was reinstated. This commit does the same in `master`.
I forgot to update the serialisation version checks in `CreateIndexRequest` and `UpdateSettingsRequest` after merging the backport #68375. Fix it.
Backport of #67114. Part of #61656.
Change the Security plugin so that its system indices are managed automatically
by the system indices infrastructure.
Also add an
originfield toCreateIndexRequestandUpdateSettingsRequest.