TemplateUpgradeService runs updates under existing ThreadContext

[tvernum]

The TemplateUpgradeService has a high level flow of:

• receive ClusterChangedEvent
• check preconditions (global block, master node, etc)
• gather necessary updates from registered upgraders (plugins)
• apply template updates on the generic thread pool.

However

• the ClusterChangedEvent comes in with the same ThreadContext as the action that triggered the event (which might be a node join/leave, but it also might be a settings change or index create/delete over REST).
• The generic threadpool execute preserves the ThreadContext from the calling code.

Consequently, the template update runs with a ThreadContext that matches the original triggering action.
If X-Pack Security is enabled, that means that update which should run as _system might attempt to run as the user which authenticated to the Rest API. That user may not have privileges to perform that update.

[elasticmachine]

Pinging @elastic/es-core-infra

[elasticmachine]

Pinging @elastic/es-security

[jaymode]

@tvernum I am removing the discuss label as I am not sure that discussion is needed about this issue. Please add it back and comment about what needs to be discussed if you still believe there is something to discuss.

[tvernum]

@ywelsch And I discussed this just after I raised it, and believed that it would be good to discuss this more widely.
We were of the view that cluster state events should be published in a system context (which they probably are on non-master nodes, but the master node shortcuts its own update event).

[jaymode]

Thanks for the background @tvernum. I moved forward with merging #30621 as an interim fix; the idea that cluster state events should be published in a context unrelated to a user makes sense to me. I would have concerns about making such a change in a bugfix (ie 6.3.x) release as there are unknowns as to what this change would affect.

One thing that I'd like to ensure we are on the same page with is that we currently use the term "system context" to mean a context that is used for normal actions (non internal:) that are executed by elasticsearch itself and not due to a specific user action.

[elasticmachine]

Pinging @elastic/es-distributed

[bleskes]

IMO we should make all cluster state handling (appliers + cluster state update tasks) user agnostics - i.e., run with no user context. The reason is that it will take a lot of effort to define and maintain a model here. Cluster state appliers can be called on the master after committing a cluster state, where we can potentially carry things over easily, but they can also be called as a result of an incoming cluster state from the master. Another complication is that both the cluster state update task and the incoming cluster state queue support batching. If we want to make things "correct" we need to make sure that those batches only batch changes that are made by the same (or equivalent) user context. This means we'll need to transfer the user context that have caused a cluster state to change via the publishing mechanism and persist it in the pending cluster state queue. We will also need to change the batching in the cluster state update task executor to be aware of the thread contexts.

All in all - it's really complicated and I would really like to avoid it. So far I didn't hear a very compelling need, so I suggest we go with the model of "all cluster state changes/code runs under the system user". This doesn't mean that cluster state update tasks listeners can't restore the context when they get a response.

[ywelsch]

Closed by #31241