fix: allow mutation-webhook only operation without constraint client …#4469
fix: allow mutation-webhook only operation without constraint client …#4469adiazny wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR cherry-picks the upstream fix to let Gatekeeper run with --operation=mutation-webhook only by avoiding unnecessary constraint/OPA client initialization and making the cache manager tolerant of having no constraint client.
Changes:
- Only initialize the constraint/OPA client in
main.gowhen validation operations are enabled. - Default
CacheManager’sCfClientto a no-op implementation when none is provided. - Extend cache manager tests to cover behavior with a no-op
CfClientand adjust assertions accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
main.go |
Skips constraint client creation when validation operations aren’t enabled; passes no client to cache manager in mutation-only mode. |
pkg/cachemanager/cachemanager.go |
Introduces noopCFDataClient and defaults CfClient to it when unset. |
pkg/cachemanager/cachemanager_test.go |
Adds tests for no-op client cases and makes assertions conditional on fake client presence. |
| cfClient := config.CfClient | ||
| if cfClient == nil { | ||
| cfClient = &noopCFDataClient{} | ||
| } |
There was a problem hiding this comment.
config.CfClient is an interface, so a typed-nil (e.g. (*constraintclient.Client)(nil) assigned to CFDataClient) will make cfClient != nil here, and the no-op fallback won’t apply. That can still lead to panics later when c.cfClient.AddData/RemoveData is invoked. Consider normalizing the interface value (e.g., via reflect.ValueOf(cfClient).IsNil() for nilable kinds, or a small helper) before deciding whether to use noopCFDataClient.
…pen-policy-agent#4423) Signed-off-by: Alan Diaz <alan.diaz@jpmchase.com> (cherry picked from commit 70d79e4) Signed-off-by: Alan Diaz <alan.diaz@jpmchase.com>
adiazny
force-pushed
the
backport-fix-PR4423-release-3.21
branch
from
5c437e5 to
33dcbec
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-3.21 #4469 +/- ##
===============================================
Coverage ? 40.71%
===============================================
Files ? 251
Lines ? 17734
Branches ? 0
===============================================
Hits ? 7220
Misses ? 9889
Partials ? 625
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ogulcanaydogan
left a comment
ogulcanaydogan
left a comment
There was a problem hiding this comment.
Clean cherry-pick. The key change is guarding the constraint client initialization behind HasValidationOperations(), so mutation-only deployments don't need it.
Wrapping the rego driver and K8s CEL setup in the validation check makes sense since those are only needed for constraint evaluation.
LGTM
|
This PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
What this PR does / why we need it: cherry-pick #4423 fix: allow mutation-webhook only operation without constraint client for release-3.21
Which issue(s) this PR fixes:
Fixes #3928