From 26bfc0fb7c03dd2cdc1a7c4a40b41423c72742d1 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Wed, 20 Nov 2024 17:59:05 -0800 Subject: [PATCH] chore: adding enforcement point status, vapgeneratestatus (#3686) Signed-off-by: Jaydip Gabani --- .../v1beta1/constraintpodstatus_types.go | 19 +- .../constrainttemplatepodstatus_types.go | 18 +- apis/status/v1beta1/zz_generated.deepcopy.go | 40 ++++ ...s.gatekeeper.sh_constraintpodstatuses.yaml | 19 ++ ...eper.sh_constrainttemplatepodstatuses.yaml | 11 ++ ...intpodstatus-customresourcedefinition.yaml | 18 ++ ...atepodstatus-customresourcedefinition.yaml | 11 ++ manifest_staging/deploy/gatekeeper.yaml | 29 +++ .../constraint/constraint_controller.go | 61 ++++-- .../constraint/constraint_controller_test.go | 12 -- .../constrainttemplate/constants.go | 7 + .../constrainttemplate_controller.go | 5 +- .../constrainttemplate_controller_test.go | 175 +++++++++++++++--- pkg/drivers/k8scel/schema/errors.go | 3 +- pkg/drivers/k8scel/schema/schema.go | 2 +- 15 files changed, 364 insertions(+), 66 deletions(-) diff --git a/apis/status/v1beta1/constraintpodstatus_types.go b/apis/status/v1beta1/constraintpodstatus_types.go index 292a298d154..156699c5e4f 100644 --- a/apis/status/v1beta1/constraintpodstatus_types.go +++ b/apis/status/v1beta1/constraintpodstatus_types.go @@ -39,11 +39,12 @@ type ConstraintPodStatusStatus struct { // Storing the constraint UID allows us to detect drift, such as // when a constraint has been recreated after its CRD was deleted // out from under it, interrupting the watch - ConstraintUID types.UID `json:"constraintUID,omitempty"` - Operations []string `json:"operations,omitempty"` - Enforced bool `json:"enforced,omitempty"` - Errors []Error `json:"errors,omitempty"` - ObservedGeneration int64 `json:"observedGeneration,omitempty"` + ConstraintUID types.UID `json:"constraintUID,omitempty"` + Operations []string `json:"operations,omitempty"` + Enforced bool `json:"enforced,omitempty"` + Errors []Error `json:"errors,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + EnforcementPointsStatus []EnforcementPointStatus `json:"enforcementPointsStatus,omitempty"` } // Error represents a single error caught while adding a constraint to engine. @@ -53,6 +54,14 @@ type Error struct { Location string `json:"location,omitempty"` } +// EnforcementPointStatus represents the status of a single enforcement point. +type EnforcementPointStatus struct { + EnforcementPoint string `json:"enforcementPoint"` + State string `json:"state"` + Message string `json:"message,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` +} + // +kubebuilder:object:root=true // +kubebuilder:resource:scope=Namespaced diff --git a/apis/status/v1beta1/constrainttemplatepodstatus_types.go b/apis/status/v1beta1/constrainttemplatepodstatus_types.go index 1850ec7de60..a06c6696417 100644 --- a/apis/status/v1beta1/constrainttemplatepodstatus_types.go +++ b/apis/status/v1beta1/constrainttemplatepodstatus_types.go @@ -29,11 +29,19 @@ import ( // ConstraintTemplatePodStatusStatus defines the observed state of ConstraintTemplatePodStatus. type ConstraintTemplatePodStatusStatus struct { // Important: Run "make" to regenerate code after modifying this file - ID string `json:"id,omitempty"` - TemplateUID types.UID `json:"templateUID,omitempty"` - Operations []string `json:"operations,omitempty"` - ObservedGeneration int64 `json:"observedGeneration,omitempty"` - Errors []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"` + ID string `json:"id,omitempty"` + TemplateUID types.UID `json:"templateUID,omitempty"` + Operations []string `json:"operations,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + Errors []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"` + VAPGenerationStatus *VAPGenerationStatus `json:"vapGenerationStatus,omitempty"` +} + +// VAPGenerationStatus represents the status of VAP generation. +type VAPGenerationStatus struct { + State string `json:"state,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + Warning string `json:"warning,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/status/v1beta1/zz_generated.deepcopy.go b/apis/status/v1beta1/zz_generated.deepcopy.go index fa401e6ced1..0249b331f42 100644 --- a/apis/status/v1beta1/zz_generated.deepcopy.go +++ b/apis/status/v1beta1/zz_generated.deepcopy.go @@ -199,6 +199,11 @@ func (in *ConstraintPodStatusStatus) DeepCopyInto(out *ConstraintPodStatusStatus *out = make([]Error, len(*in)) copy(*out, *in) } + if in.EnforcementPointsStatus != nil { + in, out := &in.EnforcementPointsStatus, &out.EnforcementPointsStatus + *out = make([]EnforcementPointStatus, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConstraintPodStatusStatus. @@ -288,6 +293,11 @@ func (in *ConstraintTemplatePodStatusStatus) DeepCopyInto(out *ConstraintTemplat } } } + if in.VAPGenerationStatus != nil { + in, out := &in.VAPGenerationStatus, &out.VAPGenerationStatus + *out = new(VAPGenerationStatus) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConstraintTemplatePodStatusStatus. @@ -300,6 +310,21 @@ func (in *ConstraintTemplatePodStatusStatus) DeepCopy() *ConstraintTemplatePodSt return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EnforcementPointStatus) DeepCopyInto(out *EnforcementPointStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EnforcementPointStatus. +func (in *EnforcementPointStatus) DeepCopy() *EnforcementPointStatus { + if in == nil { + return nil + } + out := new(EnforcementPointStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Error) DeepCopyInto(out *Error) { *out = *in @@ -516,3 +541,18 @@ func (in *MutatorPodStatusStatus) DeepCopy() *MutatorPodStatusStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VAPGenerationStatus) DeepCopyInto(out *VAPGenerationStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VAPGenerationStatus. +func (in *VAPGenerationStatus) DeepCopy() *VAPGenerationStatus { + if in == nil { + return nil + } + out := new(VAPGenerationStatus) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml index f9678eb0f0a..3216c03aaab 100644 --- a/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml @@ -48,6 +48,25 @@ spec: type: string enforced: type: boolean + enforcementPointsStatus: + items: + description: EnforcementPointStatus represents the status of a single + enforcement point. + properties: + enforcementPoint: + type: string + message: + type: string + observedGeneration: + format: int64 + type: integer + state: + type: string + required: + - enforcementPoint + - state + type: object + type: array errors: items: description: Error represents a single error caught while adding diff --git a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml index 9030379a557..69b04663e0a 100644 --- a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml @@ -74,6 +74,17 @@ spec: don't ONLY use UUIDs, this is an alias to string. Being a type captures intent and helps make sure that UIDs and names do not get conflated. type: string + vapGenerationStatus: + description: VAPGenerationStatus represents the status of VAP generation. + properties: + observedGeneration: + format: int64 + type: integer + state: + type: string + warning: + type: string + type: object type: object type: object served: true diff --git a/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml index 85942c0dbcc..9caedd58716 100644 --- a/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml @@ -50,6 +50,24 @@ spec: type: string enforced: type: boolean + enforcementPointsStatus: + items: + description: EnforcementPointStatus represents the status of a single enforcement point. + properties: + enforcementPoint: + type: string + message: + type: string + observedGeneration: + format: int64 + type: integer + state: + type: string + required: + - enforcementPoint + - state + type: object + type: array errors: items: description: Error represents a single error caught while adding a constraint to engine. diff --git a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml index 2d4bd1c8bf2..09b0b9c64e8 100644 --- a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml @@ -73,6 +73,17 @@ spec: don't ONLY use UUIDs, this is an alias to string. Being a type captures intent and helps make sure that UIDs and names do not get conflated. type: string + vapGenerationStatus: + description: VAPGenerationStatus represents the status of VAP generation. + properties: + observedGeneration: + format: int64 + type: integer + state: + type: string + warning: + type: string + type: object type: object type: object served: true diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index a45835556c0..b98798f1955 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2660,6 +2660,24 @@ spec: type: string enforced: type: boolean + enforcementPointsStatus: + items: + description: EnforcementPointStatus represents the status of a single enforcement point. + properties: + enforcementPoint: + type: string + message: + type: string + observedGeneration: + format: int64 + type: integer + state: + type: string + required: + - enforcementPoint + - state + type: object + type: array errors: items: description: Error represents a single error caught while adding a constraint to engine. @@ -2763,6 +2781,17 @@ spec: don't ONLY use UUIDs, this is an alias to string. Being a type captures intent and helps make sure that UIDs and names do not get conflated. type: string + vapGenerationStatus: + description: VAPGenerationStatus represents the status of VAP generation. + properties: + observedGeneration: + format: int64 + type: integer + state: + type: string + warning: + type: string + type: object type: object type: object served: true diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index fb90cec68fb..e02b4f16584 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -65,6 +65,9 @@ import ( const ( BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until" + ErrGenerateVAPBState = "error" + GeneratedVAPBState = "generated" + WaitVAPBState = "waiting" ) var ( @@ -489,21 +492,21 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction return noDelay, err } - generateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance) + shouldGenerateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance) if err != nil { log.Error(err, "could not determine if ValidatingAdmissionPolicyBinding should be generated") return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ValidatingAdmissionPolicyBinding should be generated") } isAPIEnabled := false var groupVersion *schema.GroupVersion - if generateVAPB { + if shouldGenerateVAPB { isAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&log) } - if generateVAPB { + if shouldGenerateVAPB { if !isAPIEnabled { log.Error(ErrValidatingAdmissionPolicyAPIDisabled, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName()) - _ = r.reportErrorOnConstraintStatus(ctx, status, ErrValidatingAdmissionPolicyAPIDisabled, "cannot generate ValidatingAdmissionPolicyBinding") - generateVAPB = false + status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("cannot generate ValidatingAdmissionPolicyBinding: %s", ErrValidatingAdmissionPolicyAPIDisabled)}) + shouldGenerateVAPB = false } else { unversionedCT := &templates.ConstraintTemplate{} if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil { @@ -511,17 +514,17 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction } hasVAP, err := ShouldGenerateVAP(unversionedCT) switch { - case errors.Is(err, celSchema.ErrCodeNotDefined): - // TODO jgabani: follow up with enforcementPointStatus field under bypod to not swallow this error. - generateVAPB = false + case errors.Is(err, celSchema.ErrCELEngineMissing): + updateEnforcementPointStatus(status, util.VAPEnforcementPoint, ErrGenerateVAPBState, err.Error(), instance.GetGeneration()) + shouldGenerateVAPB = false case err != nil: log.Error(err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy", "constraint", instance.GetName(), "constraint_template", unversionedCT.GetName()) - _ = r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy") - generateVAPB = false + status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy: %s", err)}) + shouldGenerateVAPB = false case !hasVAP: log.Error(ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName(), "constraint_template", unversionedCT.GetName()) - _ = r.reportErrorOnConstraintStatus(ctx, status, ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding") - generateVAPB = false + status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("cannot generate ValidatingAdmissionPolicyBinding: %s", ErrVAPConditionsNotSatisfied)}) + shouldGenerateVAPB = false default: // reconcile for vapb generation if annotation is not set if ct.Annotations == nil || ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" { @@ -535,15 +538,17 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not parse timestamp") } if t.After(time.Now()) { - return time.Until(t), nil + wait := time.Until(t) + updateEnforcementPointStatus(status, util.VAPEnforcementPoint, WaitVAPBState, fmt.Sprintf("waiting for %s before generating ValidatingAdmissionPolicyBinding to make sure api-server has cached constraint CRD", wait), instance.GetGeneration()) + return wait, r.writer.Update(ctx, status) } } } } - r.log.Info("constraint controller", "generateVAPB", generateVAPB) + r.log.Info("constraint controller", "generateVAPB", shouldGenerateVAPB) // generate vapbinding resources - if generateVAPB && groupVersion != nil { + if shouldGenerateVAPB && groupVersion != nil { currentVapBinding, err := vapBindingForVersion(*groupVersion) if err != nil { return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") @@ -581,10 +586,11 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } + updateEnforcementPointStatus(status, util.VAPEnforcementPoint, GeneratedVAPBState, "", instance.GetGeneration()) } // do not generate vapbinding resources // remove if exists - if !generateVAPB && groupVersion != nil { + if !shouldGenerateVAPB && groupVersion != nil { currentVapBinding, err := vapBindingForVersion(*groupVersion) if err != nil { return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") @@ -602,9 +608,10 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction if err := r.writer.Delete(ctx, currentVapBinding); err != nil { return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } + cleanEnforcementPointStatus(status, util.VAPEnforcementPoint) } } - return noDelay, nil + return noDelay, r.writer.Update(ctx, status) } func NewConstraintsCache() *ConstraintsCache { @@ -726,3 +733,23 @@ func v1beta1ToV1(v1beta1Obj *admissionregistrationv1beta1.ValidatingAdmissionPol return obj, nil } + +func updateEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string, state string, message string, observedGeneration int64) { + enforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: enforcementPoint, State: state, ObservedGeneration: observedGeneration, Message: message} + for i, ep := range status.Status.EnforcementPointsStatus { + if ep.EnforcementPoint == enforcementPoint { + status.Status.EnforcementPointsStatus[i] = enforcementPointStatus + return + } + } + status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, enforcementPointStatus) +} + +func cleanEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string) { + for i, ep := range status.Status.EnforcementPointsStatus { + if ep.EnforcementPoint == enforcementPoint { + status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus[:i], status.Status.EnforcementPointsStatus[i+1:]...) + return + } + } +} diff --git a/pkg/controller/constraint/constraint_controller_test.go b/pkg/controller/constraint/constraint_controller_test.go index ad244d39bd5..7bdce693577 100644 --- a/pkg/controller/constraint/constraint_controller_test.go +++ b/pkg/controller/constraint/constraint_controller_test.go @@ -511,18 +511,6 @@ func TestShouldGenerateVAP(t *testing.T) { expected: false, wantErr: false, }, - { - name: "missing, default 'yes'", - template: makeTemplateWithCELEngine(nil), - vapDefault: true, - expected: true, - }, - { - name: "missing, default 'no'", - template: makeTemplateWithCELEngine(nil), - vapDefault: false, - expected: false, - }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/pkg/controller/constrainttemplate/constants.go b/pkg/controller/constrainttemplate/constants.go index e423a9ff7ed..8f4ebae888f 100644 --- a/pkg/controller/constrainttemplate/constants.go +++ b/pkg/controller/constrainttemplate/constants.go @@ -12,3 +12,10 @@ const ( // ErrParseCode indicates a problem parsing a ConstraintTemplate. ErrParseCode = "parse_error" ) + +const ( + // ErrGenerateVAPState indicates a problem generating a VAP. + ErrGenerateVAPState = "error" + // GeneratedVAPState indicates a VAP was generated. + GeneratedVAPState = "generated" +) diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index 371c1c9a06e..ff69d8a972e 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -441,8 +441,9 @@ func (r *ReconcileConstraintTemplate) handleUpdate( t.Observe(unversionedCT) generateVap, err := constraint.ShouldGenerateVAP(unversionedCT) - if err != nil && !errors.Is(err, celSchema.ErrCodeNotDefined) { + if err != nil && !errors.Is(err, celSchema.ErrCELEngineMissing) { logger.Error(err, "generateVap error") + status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{State: ErrGenerateVAPState, ObservedGeneration: ct.GetGeneration(), Warning: fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.Error())} } if err := r.generateCRD(ctx, ct, proposedCRD, currentCRD, status, logger, generateVap); err != nil { @@ -851,6 +852,7 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 return err } } + status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{State: GeneratedVAPState, ObservedGeneration: ct.GetGeneration(), Warning: ""} } // do not generate VAP resources // remove if exists @@ -875,6 +877,7 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not delete VAP object", status, err) return err } + status.Status.VAPGenerationStatus = nil // after VAP is deleted, trigger update event for all constraints if err := r.triggerConstraintEvents(ctx, ct, status); err != nil { return err diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index 9010f61f640..1e28caa924a 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -62,9 +62,8 @@ import ( ) const ( - DenyAll = "DenyAll" - denyall = "denyall" - vapBindingName = "gatekeeper-denyallconstraint" + DenyAll = "DenyAll" + denyall = "denyall" ) func makeReconcileConstraintTemplate(suffix string) *v1beta1.ConstraintTemplate { @@ -377,6 +376,7 @@ func TestReconcile(t *testing.T) { suffix := "VapShouldNotBeCreatedForRegoOnlyTemplate" logger.Info("Running test: Vap should not be created for rego only template") + constraint.DefaultGenerateVAP = ptr.To[bool](true) constraintTemplate := makeReconcileConstraintTemplate(suffix) t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, expectedCRD(suffix))) testutils.CreateThenCleanup(ctx, t, c, constraintTemplate) @@ -398,6 +398,48 @@ func TestReconcile(t *testing.T) { if err != nil { t.Fatal(err) } + + logger.Info("Running test: EnforcementPointStatus should indicate missing CEL engine for constraint using VAP enforcementPoint with rego templates") + cstr := newDenyAllCstrWithScopedEA(suffix, util.VAPEnforcementPoint) + err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { + return true + }, func() error { + return c.Create(ctx, cstr) + }) + if err != nil { + logger.Error(err, "create cstr") + t.Fatal(err) + } + + err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { + return true + }, func() error { + statusObj := &statusv1beta1.ConstraintPodStatus{} + sName, err := statusv1beta1.KeyForConstraint(util.GetPodName(), cstr) + if err != nil { + return err + } + key := types.NamespacedName{Name: sName, Namespace: util.GetNamespace()} + if err := c.Get(ctx, key, statusObj); err != nil { + return err + } + + for _, ep := range statusObj.Status.EnforcementPointsStatus { + if ep.EnforcementPoint == util.VAPEnforcementPoint { + if ep.Message == "" { + return fmt.Errorf("expected message") + } + if ep.State != constraint.ErrGenerateVAPBState { + return fmt.Errorf("expected error code") + } + return nil + } + } + return fmt.Errorf("expected enforcement point status") + }) + if err != nil { + t.Fatal(err) + } }) t.Run("Vap should not be created for only rego engine", func(t *testing.T) { @@ -476,6 +518,29 @@ func TestReconcile(t *testing.T) { if err != nil { t.Fatal(err) } + + logger.Info("Running test: constraint template should have vap generated status state set to generated") + err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { + return true + }, func() error { + statusObj := &statusv1beta1.ConstraintTemplatePodStatus{} + sName, err := statusv1beta1.KeyForConstraintTemplate(util.GetPodName(), constraintTemplate.GetName()) + if err != nil { + return err + } + key := types.NamespacedName{Name: sName, Namespace: util.GetNamespace()} + if err := c.Get(ctx, key, statusObj); err != nil { + return err + } + + if statusObj.Status.VAPGenerationStatus == nil || statusObj.Status.VAPGenerationStatus.State != GeneratedVAPState { + return fmt.Errorf("Expected VAP generation status state to be %s", GeneratedVAPState) + } + return nil + }) + if err != nil { + t.Fatal(err) + } }) t.Run("VapBinding should not be created", func(t *testing.T) { @@ -501,8 +566,7 @@ func TestReconcile(t *testing.T) { }, func() error { // check if vapbinding resource exists now vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} - vapBindingName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) - if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil { + if err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("gatekeeper-%s", cstr.GetName())}, vapBinding); err != nil { if !apierrors.IsNotFound(err) { return err } @@ -538,8 +602,7 @@ func TestReconcile(t *testing.T) { }, func() error { // check if vapbinding resource exists now vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} - vapBindingName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) - if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil { + if err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("gatekeeper-%s", cstr.GetName())}, vapBinding); err != nil { if !apierrors.IsNotFound(err) { return err } @@ -624,8 +687,7 @@ func TestReconcile(t *testing.T) { return true }, func() error { vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} - vapBindingName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) - if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil { + if err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("gatekeeper-%s", cstr.GetName())}, vapBinding); err != nil { if !apierrors.IsNotFound(err) { return err } @@ -673,7 +735,7 @@ func TestReconcile(t *testing.T) { } // check if vapbinding resource exists now vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} - if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil { + if err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("gatekeeper-%s", cstr.GetName())}, vapBinding); err != nil { return err } blockTime, err := time.Parse(time.RFC3339, timestamp) @@ -718,8 +780,7 @@ func TestReconcile(t *testing.T) { }, func() error { // check if vapbinding resource exists now vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} - vapBindingName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) - if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil { + if err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("gatekeeper-%s", cstr.GetName())}, vapBinding); err != nil { if !apierrors.IsNotFound(err) { return err } @@ -804,20 +865,87 @@ func TestReconcile(t *testing.T) { if timestamp == "" { return fmt.Errorf("expected %s annotations on CT", constraint.BlockVAPBGenerationUntilAnnotation) } - // check if vapbinding resource exists now - vapBinding := &admissionregistrationv1.ValidatingAdmissionPolicyBinding{} - if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil { - return err - } blockTime, err := time.Parse(time.RFC3339, timestamp) if err != nil { return err } + // check if vapbinding resource exists now + vapBinding := &admissionregistrationv1.ValidatingAdmissionPolicyBinding{} + if err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("gatekeeper-%s", cstr.GetName())}, vapBinding); err != nil { + return err + } vapBindingCreationTime := vapBinding.GetCreationTimestamp().Time if vapBindingCreationTime.Before(blockTime) { return fmt.Errorf("VAPBinding should not be created before the timestamp") } - return nil + if err := c.Delete(ctx, cstr); err != nil { + return err + } + return c.Delete(ctx, vapBinding) + }) + if err != nil { + t.Fatal(err) + } + }) + + t.Run("VapBinding should be created with v1 without warnings in enforcementPointsStatus", func(t *testing.T) { + suffix := "VapBindingShouldBeCreatedV1EnforcementPointsStatus" + logger.Info("Running test: VapBinding should be created with v1 without warnings in enforcementPointsStatus") + constraint.DefaultGenerateVAPB = ptr.To[bool](true) + transform.GroupVersion = &admissionregistrationv1.SchemeGroupVersion + constraintTemplate := makeReconcileConstraintTemplateForVap(suffix, ptr.To[bool](true)) + cstr := newDenyAllCstr(suffix) + t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, expectedCRD(suffix))) + testutils.CreateThenCleanup(ctx, t, c, constraintTemplate) + + err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { + return true + }, func() error { + return c.Create(ctx, cstr) + }) + if err != nil { + logger.Error(err, "create cstr") + t.Fatal(err) + } + err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { + return true + }, func() error { + statusObj := &statusv1beta1.ConstraintTemplatePodStatus{} + sName, err := statusv1beta1.KeyForConstraintTemplate(util.GetPodName(), constraintTemplate.GetName()) + if err != nil { + return err + } + key := types.NamespacedName{Name: sName, Namespace: util.GetNamespace()} + if err := c.Get(ctx, key, statusObj); err != nil { + return err + } + + if statusObj.Status.VAPGenerationStatus == nil || statusObj.Status.VAPGenerationStatus.State != GeneratedVAPState { + return fmt.Errorf("Expected VAP generation status state to be %s", GeneratedVAPState) + } + + cStatusObj := &statusv1beta1.ConstraintPodStatus{} + name, err := statusv1beta1.KeyForConstraint(util.GetPodName(), cstr) + if err != nil { + return err + } + key = types.NamespacedName{Name: name, Namespace: util.GetNamespace()} + if err := c.Get(ctx, key, cStatusObj); err != nil { + return err + } + + for _, ep := range cStatusObj.Status.EnforcementPointsStatus { + if ep.EnforcementPoint == util.VAPEnforcementPoint { + if ep.Message != "" { + return fmt.Errorf("message not expected") + } + if ep.State != constraint.GeneratedVAPBState { + return fmt.Errorf("expected state to be %s", constraint.GeneratedVAPBState) + } + break + } + } + return c.Delete(ctx, cstr) }) if err != nil { t.Fatal(err) @@ -1037,7 +1165,7 @@ func TestReconcile(t *testing.T) { err = retry.OnError(testutils.ConstantRetry, func(error) bool { return true }, func() error { - err := c.Get(ctx, types.NamespacedName{Name: "denyallconstraint"}, cstr) + err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("cstr-%s", strings.ToLower(suffix))}, cstr) if err != nil { return err } @@ -1344,6 +1472,7 @@ violation[{"msg": "denied!"}] { // Create the constraint for constraint template cstr := newDenyAllCstr("") + cstr.SetName("denyallconstraint") err = c.Create(ctx, cstr) if err != nil { t.Fatal(err) @@ -1444,7 +1573,7 @@ func constraintEnforced(ctx context.Context, c client.Client, suffix string) err return true }, func() error { cstr := newDenyAllCstr(suffix) - err := c.Get(ctx, types.NamespacedName{Name: "denyallconstraint"}, cstr) + err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("cstr-%s", strings.ToLower(suffix))}, cstr) if err != nil { return err } @@ -1465,7 +1594,7 @@ func isConstraintStatuErrorAsExpected(ctx context.Context, c client.Client, suff return true }, func() error { cstr := newDenyAllCstr(suffix) - err := c.Get(ctx, types.NamespacedName{Name: "denyallconstraint"}, cstr) + err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("cstr-%s", strings.ToLower(suffix))}, cstr) if err != nil { return err } @@ -1498,7 +1627,7 @@ func newDenyAllCstr(suffix string) *unstructured.Unstructured { Version: "v1beta1", Kind: DenyAll + suffix, }) - cstr.SetName("denyallconstraint") + cstr.SetName(fmt.Sprintf("cstr-%s", strings.ToLower(suffix))) return cstr } @@ -1525,7 +1654,7 @@ func newDenyAllCstrWithScopedEA(suffix string, ep ...string) *unstructured.Unstr Version: "v1beta1", Kind: DenyAll + suffix, }) - cstr.SetName("denyallconstraint") + cstr.SetName(fmt.Sprintf("cstr-%s", strings.ToLower(suffix))) return cstr } diff --git a/pkg/drivers/k8scel/schema/errors.go b/pkg/drivers/k8scel/schema/errors.go index f735a2997a6..6dff5f98510 100644 --- a/pkg/drivers/k8scel/schema/errors.go +++ b/pkg/drivers/k8scel/schema/errors.go @@ -6,8 +6,7 @@ var ( ErrBadMatchCondition = errors.New("invalid match condition") ErrBadVariable = errors.New("invalid variable definition") ErrBadFailurePolicy = errors.New("invalid failure policy") - ErrCodeNotDefined = errors.New("K8sNativeValidation code not defined") + ErrCELEngineMissing = errors.New("K8sNativeValidation engine is missing") ErrOneTargetAllowed = errors.New("wrong number of targets defined, only 1 target allowed") ErrBadType = errors.New("could not recognize the type") - ErrMissingField = errors.New("K8sNativeValidation source missing required field") ) diff --git a/pkg/drivers/k8scel/schema/schema.go b/pkg/drivers/k8scel/schema/schema.go index afe01db2259..575030414f1 100644 --- a/pkg/drivers/k8scel/schema/schema.go +++ b/pkg/drivers/k8scel/schema/schema.go @@ -288,7 +288,7 @@ func GetSourceFromTemplate(ct *templates.ConstraintTemplate) (*Source, error) { break } if source == nil { - return nil, ErrCodeNotDefined + return nil, ErrCELEngineMissing } return source, nil }