refactor: extract operation-to-capability wiring plan with table-driven tests

[shashankvarma499]

Description

Fixes #4776.

Extracts a testable operations.Plan that maps --operation values and feature flags to the clients/systems/runnables/webhooks/controllers constructed by setupControllers. Production wiring now consumes operations.CurrentPlan, so tests and process startup cannot drift.

The plan makes previously-undetected wiring contradictions explicit and testable, e.g.:

• status builds the constraint client with no enforcement point.
• generate owns CRD/VAP/VAPB behavior but does not start the ConstraintTemplate/Constraint reconcilers.

Changes

• pkg/operations/plan.go — new Plan, Features, NewPlan, CurrentPlan, and Plan.Validate.
• pkg/operations/plan_test.go — table-driven suite covering all 7 isolated operations, shipped combinations (audit+status+mutation-status+generate, webhook+mutation-webhook, default all-operations), and feature toggles (expansion, external data, violation/admission export, VAP scope sync, k8s native validation).
• pkg/operations/operations.go — add AssignedSet() and shared hasValidationOperations/hasMutationOperations helpers.
• main.go — setupControllers now consumes CurrentPlan.

Tests

• go test ./pkg/operations/... -count=1 — pass
• go build . — pass
• go vet ./pkg/operations/... . — pass

(Controller tests requiring envtest/kubebuilder binaries were not run — those assets aren't present in this environment; unrelated to this change.)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: shashankvarma499 / name: Shashank Varma (6c0c32d)

[JaydipGabani]

The new plan tests pass, but they do not verify the real registration path and they encode the two known operation contradictions as expected behavior. status still cannot start because it creates a constraint client with no enforcement point, and generate still omits the reconcilers that own generation. Please make production controller registration consume the plan (or exercise actual registration) and assert the documented behavior for both isolated operations.

[JaydipGabani]

ControllerPlan is not consumed by production code. setupControllers still calls controller.AddToManager unconditionally, and each adder independently reads the global operation and feature flags. A guard here can therefore diverge from actual registration while every table test remains green. Please make registration consume this plan, or add the required focused test around real setup/registration that observes which controllers were actually added.

[JaydipGabani]

status makes hasValidation true, but neither enforcement flag is set. setupControllers then calls constraintclient.NewClient with an empty EnforcementPoints(), which returns must specify at least one enforcement point; Plan.Validate() does not catch this. This keeps --operation=status unable to start and the new status row blesses the regression. status must not construct the validation client and ingestion stack solely for status aggregation.

[JaydipGabani]

generate is excluded from hasValidation, so a generate-only plan sets both ConstraintTemplate and Constraint false. Those reconcilers own CRD/VAP/VAPB generation, so the documented operation still does nothing. The table test currently codifies this known bug instead of failing on it. Please model generation as its own controller and compilation capability rather than as an audit or webhook enforcement point.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings affect status wiring, generation behavior, and controller registration.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Refactors operation-to-capability wiring into a testable operations.Plan and integrates it into startup setup.

Changes:

• Adds plan types, feature handling, validation, and table-driven tests.
• Adds operation snapshots and shared predicates.
• Updates startup wiring to consume CurrentPlan.

File summaries

File	Summary
pkg/operations/plan.go	Defines capability planning. Findings include invalid status-only client allocation, unused controller-plan flags, and omitted scope-sync gating.
pkg/operations/plan_test.go	Tests operations and feature combinations; generate-only expectations omit required reconcilers.
pkg/operations/operations.go	Adds AssignedSet() and shared operation predicates.
main.go	Uses CurrentPlan, but controller registration still iterates global injectors instead of plan-controlled controllers.

Review details

Suppressed comments (4)

pkg/operations/plan.go:135

• These capabilities ignore SyncVAPEnforcementScope. When generation is enabled but scope synchronization is disabled, setupControllers still allocates both the webhook-config cache and the ConstraintTemplate event channel, although the webhook-config controller and all event paths gate on the same flag. The feature-toggle case therefore does not prove optional dependencies are absent; include the scope flag in these two capabilities.

			WebhookConfigCache:       ops[Generate],
			ConstraintTemplateEvents: ops[Generate],

pkg/operations/plan.go:155

• With only Generate selected, hasValidation is false, so this plan leaves both reconciliation paths disabled. setupControllers still delegates to adders whose HasValidationOperations guards return early, meaning generate-only never reconciles ConstraintTemplates/Constraints and cannot create the CRDs or VAP/VAPB resources that the generate operation documents (#4771). Generation needs an explicit capability separate from audit/webhook enforcement rather than preserving this contradictory expectation.

			ConstraintTemplate:       hasValidation,
			Constraint:               hasValidation,

pkg/operations/plan.go:89

• The new ControllerPlan flags are not consumed by production registration: main.go still passes every injector to controller.AddToManager, and each adder independently checks the global operation/feature flags. Thus this is a second mapping that tests can validate while runtime registration follows different predicates, so the claimed single source of truth can drift. Pass the plan into registration or make the adders derive their guards from it.

// ControllerPlan describes controller registration.
type ControllerPlan struct {
	ConstraintTemplate       bool
	Constraint               bool
	Config                   bool

pkg/operations/plan_test.go:377

• Every row here calls only NewPlan and Validate; no test invokes setupControllers, controller.AddToManager, or an individual registration path. This cannot verify the required non-nil dependencies at registration time or catch a production guard regression, so the suite does not meet the real setup/registration coverage required for this refactor.

			got := NewPlan(tc.ops, tc.features)
			if diff := cmp.Diff(tc.want, got); diff != "" {
				t.Errorf("NewPlan() mismatch (-want +got):\n%s", diff)
			}
			if err := got.Validate(); err != nil {

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.