refactor: construct mutation dependencies only when mutation operations run - #4787
Closed
longxiucai wants to merge 1 commit into
Closed
longxiucai wants to merge 1 commit into
longxiucai wants to merge 1 commit into
Conversation
…ns run Today setupControllers always constructs the mutation system and the mutator instances Adder registers its conflict-routing channels and runnable before the individual controllers check mutation.Enabled(). Non-mutation processes therefore carry an unused mutation system and runnable. Construct the mutation system only when a mutation operation is assigned, and wire the external-data provider cache and client cert watcher into it only in that case; they keep serving the validation client independently. Return from the instances Adder before creating any conflict-routing state when mutation is disabled. Workload expansion remains safe: expansion.System already accepts a nil mutation system and skips applying mutators. Fixes open-policy-agent#4772 Signed-off-by: longyuxiang <longyuxiang@kylinos.cn>
Author
|
Closing in favor of #4777, which covers the same change and was opened first. No need to review this one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Part of the controller dependency cleanup tracked in #3964.
setupControllersalways constructs the mutation system, andpkg/controller/mutators/instances.Adder.Addcreates theconflict-routing channels and registers the
routeConflictEventsrunnable before the individual mutator controllers check
mutation.Enabled(). Non-mutation processes therefore carry an unusedmutation system and runnable.
Changes:
assigned (
mutation.Enabled()), via a small helper that is unittested.
the mutation options only in that case; they keep serving the
validation client independently.
state when mutation is disabled.
Workload expansion remains safe:
expansion.Systemalready accepts anil mutation system and skips applying mutators, which is covered by
the existing expansion tests.
Which issue(s) this PR fixes:
Fixes #4772
Special notes for your reviewer:
TestNewMutationSystemcovers mutation-disabled and mutation-enabledoperation sets; the disabled case fails if unconditional construction
is restored.
TestAddSkipsSetupWhenMutationDisabledproves the instances Adderreturns before touching the manager when mutation is disabled.
go test ./pkg/mutation/... ./pkg/expansion/...passes.