refactor: conditionally create controller dependencies based on operations

[Copilot]

What this PR does / why we need it:

Controllers were initializing all dependencies regardless of assigned operations, causing failures when pods ran with limited operation sets (e.g., mutation-only pods failing to initialize OPA client).

Changes:

• main.go: Conditionally create dependencies based on operations using operations.IsAssigned() directly:
  • OPA client: audit or webhook operations OR external data enabled
  • MutationSystem: mutation operations OR expansion enabled
  • ExpansionSystem: expansion enabled
  • ProviderCache: external data enabled
  • CacheManager: audit or webhook operations enabled
  • WebhookConfigCache/CtEvents: generate operation assigned
  • CertWatcher: shared between OPA and mutation (no duplication)
  • Preserved original code comments for documentation
• controller.go: Nil-check dependencies before injection
• audit/controller.go: Added nil check for OPA client before initializing audit manager
• webhook/webhook.go: Added nil check for OPA client and mutation system before initializing webhooks

Example:

// Before: always created
client, err := constraintclient.NewClient(cfArgs...)
mutationSystem := mutation.NewSystem(mutationOpts)

// After: conditional
var client *constraintclient.Client
if operations.IsAssigned(operations.Audit) || operations.IsAssigned(operations.Webhook) || *externaldata.ExternalDataEnabled {
    client, err = constraintclient.NewClient(cfArgs...)
}

var mutationSystem *mutation.System  
if operations.IsAssigned(operations.MutationWebhook) || operations.IsAssigned(operations.MutationController) || operations.IsAssigned(operations.MutationStatus) || *expansion.ExpansionEnabled {
    mutationSystem = mutation.NewSystem(mutationOpts)
}

Which issue(s) this PR fixes:
Fixes #4051
Fixes #3964

Special notes for your reviewer:

• Default behavior (all operations enabled) unchanged - backward compatible
• Controllers retain operation checks as defense in depth
• Export system always created (no operation flag)
• Uses operations.IsAssigned() directly for cleaner, more readable code
• Nil checks added at multiple layers (controller, audit, webhook) for robustness
• Status operation does NOT require OPA client or cache manager - status controllers only aggregate status CRDs without interacting with the OPA constraint framework
• Original code comments preserved for documentation and maintainability

Original prompt

Look at the issues - #4051 and #3964, the problem currently is

in main.go controller dependencies are getting injected regardless of the operations and then all controllers are spun up. For exmaple, for operation mutation and validation OPA client dependency is getting injected, however OPA client dependency is only required for validaiton. So pod running with only operation mutation will error out saying there is no OPA client initialize. SImilarly, the same problem exists for pod running only with operation=generate. The goal is to reorganize controller dependencies with operations and only inject those dependencies in controller when operations are enabled.

for example all mutation related dependencies only be added when mutation operation is enabled. While fixing this change make sure to -

1. Scan deep through all the code to understand what each operation is intended for and what controllers and/or section of gatekeeper code (mutation, validation, audit, generate, status etc) is dependent on these operations
2. Do not update or remove any operations or any functionality, this is purely refactoring change.
3. Add missing tests for adding controller deps based on operations if there are any missing changes
4. Explain refactoring in the PR comments if you can and also explain reason behind your decisions on the PR comments if you can.
5. Make minimum code comments while raising this PR.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[JaydipGabani]

@copilot 1. changes in operations.go are not need, use IsAssigned whenever you want to check for presence of an operations.
2. Audit opreration also required opaclient since it invokes constraint template framework, make sure that audit controller dependencies are injected properly.
3. Use nil check similarly to controller.AddToManager in audit.addtomanager and webhook.addtomanager.

Re-analyze and think hard about whole operation and controller dependencies integration, double check you changes so far and the future changes as well to make sure all the dependencies are properly initiated for only required operaitons. Focus on code readbility, clean and optimized code. Do not add unnecessary comments. Remove not needed tests.

[Copilot]

@copilot 1. changes in operations.go are not need, use IsAssigned whenever you want to check for presence of an operations.
2. Audit opreration also required opaclient since it invokes constraint temp...

Done in commit c9ef9e9:

1. Removed helper functions from operations.go - now using IsAssigned() directly throughout
2. Audit operation correctly uses OPA client (included in audit/webhook/status checks)
3. Added nil checks in audit.AddToManager() and webhook.AddToManager() similar to controller.AddToManager()
4. Removed unnecessary comments and cleaned up code

All dependencies are now properly initialized only for required operations.

[JaydipGabani]

@copilot main.go:385 does operation status require opaclient?

[Copilot]

@copilot main.go:385 does operation status require opaclient?

No, status operation does not require OPA client. Fixed in commit 1a6834b.

Status controllers (constraintstatus, configstatus, mutatorstatus, etc.) only aggregate status CRDs - they don't interact with the OPA constraint framework. The CFClient field in some Adder structs is injected but never used in reconciliation logic. Removed status from both OPA client and cache manager dependencies.

[JaydipGabani]

@copilot please address these changes.

[JaydipGabani]

preserve the original comments as much as possible.

[Copilot]

Restored original comments in commit 094d605. All removed comments have been preserved including:

• "Block until the setup (certificate generation) finishes."
• "certWatcher is used to watch for changes to Gatekeeper's certificate and key files."
• "register the client cert watcher to the driver"
• "register the client cert watcher to the mutation system"
• "processExcluder is used for namespace exclusion for specified processes in config"
• "Setup all Controllers"
• "Events ch will be used to receive events from dynamic watches registered via the registrar below."

[Copilot]

@copilot please address these changes.

Addressed in commit 094d605 - restored all original comments as requested.

[Copilot]

Pull Request Overview

This PR refactors controller dependency initialization in main.go to conditionally create dependencies based on assigned operations. This addresses issues where pods running with limited operation sets would fail to initialize unnecessary dependencies (e.g., mutation-only pods failing to initialize OPA client).

Key changes:

• Conditional creation of OPA client, mutation system, expansion system, provider cache, cache manager, and webhook config cache based on operation assignments
• Nil-check guards added in controller injection, webhook setup, and audit manager initialization
• Maintains backward compatibility with default behavior (all operations enabled)

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
main.go	Conditionally creates controller dependencies based on operations using operations.IsAssigned() checks
pkg/controller/controller.go	Adds nil checks before dependency injection to handle optional dependencies
pkg/webhook/webhook.go	Adds nil check to skip webhook setup when neither OPA client nor mutation system are available
pkg/audit/controller.go	Adds nil check to skip audit manager initialization when OPA client is unavailable

[Copilot]

The conditional creation of expansionSystem introduces a potential nil pointer dereference bug. When *expansion.ExpansionEnabled is false, expansionSystem will be nil, but it's still passed to webhook and audit handlers which call .Expand() on it without nil checks (see pkg/webhook/policy.go:621 and pkg/audit/manager.go:719).

Solution: Always create the expansionSystem (even when expansion is disabled), similar to how it was done before this refactoring:

// Always create expansion system, even if expansion is disabled
// The system handles nil mutationSystem gracefully
if needsMutationSystem {
    expansionSystem = expansion.NewSystem(mutationSystem)
} else {
    expansionSystem = expansion.NewSystem(nil)
}

Or more concisely, remove the conditional and always create it:

expansionSystem = expansion.NewSystem(mutationSystem)

The expansion.System is designed to work even when no expansion templates are defined - it will simply return empty results.

Suggested change

	if needsExpansionSystem {
	expansionSystem = expansion.NewSystem(mutationSystem)
	}
	expansionSystem = expansion.NewSystem(mutationSystem)