feat: add default-failure-policy flag to fix #4567#4581
Closed
Mallikarjunadevops wants to merge 1 commit into
Closed
feat: add default-failure-policy flag to fix #4567#4581Mallikarjunadevops wants to merge 1 commit into
Mallikarjunadevops wants to merge 1 commit into
Conversation
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a configurable “default failure policy” for CEL-based schema generation and plumbs it through Gatekeeper’s manifests/Helm chart so installs can choose between Fail and Ignore.
Changes:
- Introduces a global
--default-failure-policyflag and uses it whenSource.FailurePolicyis unset. - Adds unit tests covering defaulting behavior for both v1 and v1beta1 failure policy types.
- Exposes the setting via Helm values and wires it into controller-manager/audit deployments and manager manifests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/drivers/k8scel/schema/schema.go | Adds default failure policy flag and defaulting logic in failure policy getters. |
| pkg/drivers/k8scel/schema/schema_test.go | Adds tests validating defaulting behavior for v1 and v1beta1 failure policies. |
| manifest_staging/charts/gatekeeper/values.yaml | Adds defaultFailurePolicy chart value. |
| manifest_staging/charts/gatekeeper/templates/gatekeeper-controller-manager-deployment.yaml | Passes --default-failure-policy arg from values into controller-manager deployment. |
| manifest_staging/charts/gatekeeper/templates/gatekeeper-audit-deployment.yaml | Passes --default-failure-policy arg from values into audit deployment. |
| config/manager/manager.yaml | Adds --default-failure-policy=Fail to manager containers. |
| {{- if not .Values.controllerManager.disableGenerateOperation }} | ||
| - --operation=generate | ||
| {{- end }} | ||
| {{ if hasKey .Values "defaultFailurePolicy" }}- --default-failure-policy={{ .Values.defaultFailurePolicy }}{{- end }} |
| {{- if not .Values.audit.disableStatusOperation }} | ||
| - --operation=status | ||
| {{- end }} | ||
| {{ if hasKey .Values "defaultFailurePolicy" }}- --default-failure-policy={{ .Values.defaultFailurePolicy }}{{- end }} |
| package schema | ||
|
|
||
| import ( | ||
| "flag" |
| ObjectName = "anyObject" | ||
| ) | ||
|
|
||
| var DefaultFailurePolicy = flag.String("default-failure-policy", "Fail", "Default failure policy for ConstraintTemplate and ValidatingAdmissionPolicy. Can be set to 'Fail' or 'Ignore'.") |
Comment on lines
202
to
211
| func (in *Source) GetFailurePolicy() (*admissionv1.FailurePolicyType, error) { | ||
| if in.FailurePolicy == nil { | ||
| return nil, nil | ||
| var out admissionv1.FailurePolicyType | ||
| if DefaultFailurePolicy != nil && *DefaultFailurePolicy == string(admissionv1.Ignore) { | ||
| out = admissionv1.Ignore | ||
| } else { | ||
| out = admissionv1.Fail | ||
| } | ||
| return &out, nil | ||
| } |
Comment on lines
227
to
236
| func (in *Source) GetV1Beta1FailurePolicy() (*admissionv1beta1.FailurePolicyType, error) { | ||
| var out admissionv1beta1.FailurePolicyType | ||
| if in.FailurePolicy == nil { | ||
| out = admissionv1beta1.Fail | ||
| if DefaultFailurePolicy != nil && *DefaultFailurePolicy == string(admissionv1beta1.Ignore) { | ||
| out = admissionv1beta1.Ignore | ||
| } else { | ||
| out = admissionv1beta1.Fail | ||
| } | ||
| return &out, nil | ||
| } |
Comment on lines
+285
to
+289
| func TestGetFailurePolicy(t *testing.T) { | ||
| originalDefault := *DefaultFailurePolicy | ||
| defer func() { | ||
| *DefaultFailurePolicy = originalDefault | ||
| }() |
Comment on lines
+323
to
+326
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| *DefaultFailurePolicy = tt.defaultFailurePolicy | ||
| got, err := tt.source.GetFailurePolicy() |
Contributor
|
closing this in favor of #4568, thanks for raising the pr. |
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.
Fixes #4567 This pull request introduces a global
--default-failure-policyflag to configure the default failure policy for admission requests and VAPs when aConstraintTemplatedoes not explicitly define one.Changes:
DefaultFailurePolicytopkg/drivers/k8scel/schema/schema.go.GetV1Beta1FailurePolicy()andGetFailurePolicy()to use the new flag as a fallback instead of the hardcodedFail.--default-failure-policyflag to the controller manager and audit deployments invalues.yamland raw manifests.schema_test.goto ensure correct fallback logic.Testing Performed
--default-failure-policy) behaves perfectly as a fallback, while respecting any explicitly defined template failure policies.TestGetFailurePolicyandTestGetV1Beta1FailurePolicytopkg/drivers/k8scel/schema/schema_test.gochecking all four edge cases (fallback when default is Fail/Ignore, explicit overrides).$ go test ./pkg/drivers/k8scel/schema ok github.com/open-policy-agent/gatekeeper/v3/pkg/drivers/k8scel/schema 4.239s