fix: fixing error reporting for templates without CEL #3493
Conversation
…gent#3492 Signed-off-by: Jaydip Gabani <[email protected]>
| if err != nil { | ||
| switch { | ||
| case errors.Is(err, celSchema.ErrCodeNotDefined): | ||
| generateVAPB = false |
There was a problem hiding this comment.
do we want to log warning here?
log.V(1).Info("Warning: Template is not configured to create ValidatingAadmissionPolicy, cannot create ValidatingAdmissionPolicyBinding")
would this be leaking information intended for template to constraints?
There was a problem hiding this comment.
IMO we should log and update status ONLY if code.engine is K8sNativeValidation. Otherwise this error will appear for all older rego CTs as code.source was introduced recently.
e.g. of older regos CTs:
targets:
- target: admission.k8s.gatekeeper.sh
rego: |
package k8sallowedreposThere was a problem hiding this comment.
I don't see changes that address
IMO we should log and update status ONLY if code.engine is K8sNativeValidation.
This would require updating framework code I think. https://github.com/open-policy-agent/frameworks/blob/aa99306df54ee1e75380507503a547f22686b11d/constraint/pkg/client/drivers/k8scel/schema/schema.go#L291 to only return this error when code.engine == Name
This isnt blocking and can be added as a followup.
Signed-off-by: Jaydip Gabani <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3493 +/- ##
==========================================
- Coverage 54.49% 48.03% -6.47%
==========================================
Files 134 219 +85
Lines 12329 15169 +2840
==========================================
+ Hits 6719 7286 +567
- Misses 5116 7069 +1953
- Partials 494 814 +320
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…ent#3493) Signed-off-by: Jaydip Gabani <[email protected]> (cherry picked from commit 4eb7dcd)
…ent#3493) Signed-off-by: Jaydip Gabani <[email protected]> (cherry picked from commit 4eb7dcd)
What this PR does / why we need it:
This PR makes sure that VAPBinding generation error are reported only on constraint that belong to CT where
Code[_].engine == K8sNativeValidationWhich issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #3492
Special notes for your reviewer: