From a492dcd051c3791d60ab0303d4a7be6b7707811e Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Fri, 8 Nov 2024 22:01:22 +0000 Subject: [PATCH 01/13] adding enforcement point status, vapgeneratestatus Signed-off-by: Jaydip Gabani --- .../v1beta1/constraintpodstatus_types.go | 18 +- .../constrainttemplatepodstatus_types.go | 17 +- apis/status/v1beta1/zz_generated.deepcopy.go | 36 ++++ ...vkmanifest.gatekeeper.sh_gvkmanifests.yaml | 52 ++++++ ...s.gatekeeper.sh_constraintpodstatuses.yaml | 16 ++ ...eper.sh_constrainttemplatepodstatuses.yaml | 8 + ...intpodstatus-customresourcedefinition.yaml | 15 ++ ...atepodstatus-customresourcedefinition.yaml | 8 + manifest_staging/deploy/gatekeeper.yaml | 23 +++ .../constraint/constraint_controller.go | 72 ++++---- .../constraint/constraint_controller_test.go | 2 +- .../constrainttemplate_controller.go | 9 +- .../constrainttemplate_controller_test.go | 154 ++++++++++++++++++ pkg/drivers/k8scel/schema/errors.go | 3 +- pkg/drivers/k8scel/schema/schema.go | 2 +- 15 files changed, 390 insertions(+), 45 deletions(-) create mode 100644 config/crd/bases/gvkmanifest.gatekeeper.sh_gvkmanifests.yaml diff --git a/apis/status/v1beta1/constraintpodstatus_types.go b/apis/status/v1beta1/constraintpodstatus_types.go index 292a298d154..3a4c38d4d52 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,13 @@ type Error struct { Location string `json:"location,omitempty"` } +// EnforcementPointStatus represents the status of a single enforcement point. +type EnforcementPointStatus struct { + EnforcementPoint string `json:"enforcementPoint"` + Enforced bool `json:"enforced"` + Message string `json:"message,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..ed7689f87bb 100644 --- a/apis/status/v1beta1/constrainttemplatepodstatus_types.go +++ b/apis/status/v1beta1/constrainttemplatepodstatus_types.go @@ -29,11 +29,18 @@ 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 { + Generated bool `json:"generated,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..6b165de0856 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,7 @@ func (in *ConstraintTemplatePodStatusStatus) DeepCopyInto(out *ConstraintTemplat } } } + out.VAPGenerationStatus = in.VAPGenerationStatus } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConstraintTemplatePodStatusStatus. @@ -300,6 +306,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 +537,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/gvkmanifest.gatekeeper.sh_gvkmanifests.yaml b/config/crd/bases/gvkmanifest.gatekeeper.sh_gvkmanifests.yaml new file mode 100644 index 00000000000..07e9ee1019f --- /dev/null +++ b/config/crd/bases/gvkmanifest.gatekeeper.sh_gvkmanifests.yaml @@ -0,0 +1,52 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: gvkmanifests.gvkmanifest.gatekeeper.sh +spec: + group: gvkmanifest.gatekeeper.sh + names: + kind: GVKManifest + listKind: GVKManifestList + plural: gvkmanifests + singular: gvkmanifest + scope: Cluster + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: GVKManifest is the Schema for the GVKManifest API. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + properties: + groups: + additionalProperties: + additionalProperties: + items: + type: string + type: array + type: object + type: object + type: object + type: object + served: true + storage: true diff --git a/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml index f9678eb0f0a..0dce1d32093 100644 --- a/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml @@ -48,6 +48,22 @@ spec: type: string enforced: type: boolean + enforcementPointsStatus: + items: + description: EnforcementPointStatus represents the status of a single + enforcement point. + properties: + enforced: + type: boolean + enforcementPoint: + type: string + message: + type: string + required: + - enforced + - enforcementPoint + 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..7a50c63c8f3 100644 --- a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml @@ -74,6 +74,14 @@ 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: + generated: + type: boolean + 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..2ade6ab11a3 100644 --- a/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml @@ -50,6 +50,21 @@ spec: type: string enforced: type: boolean + enforcementPointsStatus: + items: + description: EnforcementPointStatus represents the status of a single enforcement point. + properties: + enforced: + type: boolean + enforcementPoint: + type: string + message: + type: string + required: + - enforced + - enforcementPoint + 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..bc2defb6b0c 100644 --- a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml @@ -73,6 +73,14 @@ 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: + generated: + type: boolean + 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..c1ddf841d58 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2660,6 +2660,21 @@ spec: type: string enforced: type: boolean + enforcementPointsStatus: + items: + description: EnforcementPointStatus represents the status of a single enforcement point. + properties: + enforced: + type: boolean + enforcementPoint: + type: string + message: + type: string + required: + - enforced + - enforcementPoint + type: object + type: array errors: items: description: Error represents a single error caught while adding a constraint to engine. @@ -2763,6 +2778,14 @@ 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: + generated: + type: boolean + 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..21ad19879c8 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -296,11 +296,12 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R status.Status.ConstraintUID = instance.GetUID() status.Status.ObservedGeneration = instance.GetGeneration() status.Status.Errors = nil + status.Status.EnforcementPointsStatus = nil if c, err := r.cfClient.GetConstraint(instance); err != nil || !reflect.DeepEqual(instance, c) { err := util.ValidateEnforcementAction(enforcementAction, instance.Object) if err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not validate enforcement actions") + return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, "could not validate enforcement actions") } if err := r.cacheConstraint(ctx, instance); err != nil { @@ -309,7 +310,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R status: metrics.ErrorStatus, }) reportMetrics = true - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not cache constraint") + return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, "could not cache constraint") } logAddition(r.log, instance, enforcementAction) } @@ -416,7 +417,7 @@ func (r *ReconcileConstraint) getOrCreatePodStatus(ctx context.Context, constrai func ShouldGenerateVAP(ct *templates.ConstraintTemplate) (bool, error) { source, err := celSchema.GetSourceFromTemplate(ct) if err != nil { - return false, err + return *DefaultGenerateVAP, err } if source.GenerateVAP == nil { return *DefaultGenerateVAP, nil @@ -468,8 +469,9 @@ func (r *ReconcileConstraint) cacheConstraint(ctx context.Context, instance *uns return nil } -func (r *ReconcileConstraint) reportErrorOnConstraintStatus(ctx context.Context, status *constraintstatusv1beta1.ConstraintPodStatus, err error, message string) error { +func (r *ReconcileConstraint) reportErrorOnConstraintStatus(ctx context.Context, status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPointStatus constraintstatusv1beta1.EnforcementPointStatus, err error, message string) error { status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("%s: %s", message, err)}) + status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, enforcementPointStatus) if err2 := r.writer.Update(ctx, status); err2 != nil { log.Error(err2, message, "error", "could not update constraint status") return errorpkg.Wrapf(err, fmt.Sprintf("%s, could not update constraint status: %s", message, err2)) @@ -483,56 +485,58 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction log.Info("generate operation is not assigned, ValidatingAdmissionPolicyBinding resource will not be generated") return noDelay, nil } + vapEnforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: util.VAPEnforcementPoint, Enforced: false} ct := &v1beta1.ConstraintTemplate{} err := r.reader.Get(ctx, types.NamespacedName{Name: strings.ToLower(instance.GetKind())}, ct) if err != nil { 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") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, "could not determine if ValidatingAdmissionPolicyBinding should be generated") } isAPIEnabled := false + couldGenerateVAPB := shouldGenerateVAPB 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)}) + couldGenerateVAPB = false } else { unversionedCT := &templates.ConstraintTemplate{} if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not convert ConstraintTemplate to unversioned") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not convert ConstraintTemplate to unversioned") } 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): + vapEnforcementPointStatus.Message = err.Error() + couldGenerateVAPB = 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)}) + couldGenerateVAPB = 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)}) + couldGenerateVAPB = false default: // reconcile for vapb generation if annotation is not set if ct.Annotations == nil || ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, errors.New("annotation to wait for ValidatingAdmissionPolicyBinding generation not found"), "could not find annotation to wait for ValidatingAdmissionPolicyBinding generation") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, errors.New("annotation to wait for ValidatingAdmissionPolicyBinding generation not found"), "could not find annotation to wait for ValidatingAdmissionPolicyBinding generation") } // waiting for sometime before generating vapbinding, gives api-server time to cache CRDs timestamp := ct.Annotations[BlockVAPBGenerationUntilAnnotation] t, err := time.Parse(time.RFC3339, timestamp) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not parse timestamp") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not parse timestamp") } if t.After(time.Now()) { return time.Until(t), nil @@ -541,12 +545,12 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction } } - r.log.Info("constraint controller", "generateVAPB", generateVAPB) + r.log.Info("constraint controller", "generateVAPB", couldGenerateVAPB) // generate vapbinding resources - if generateVAPB && groupVersion != nil { + if couldGenerateVAPB && groupVersion != nil { currentVapBinding, err := vapBindingForVersion(*groupVersion) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not get ValidatingAdmissionPolicyBinding API version") } vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName()) log.Info("check if vapbinding exists", "vapBindingName", vapBindingName) @@ -558,12 +562,12 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction } transformedVapBinding, err := transform.ConstraintToBinding(instance, VAPEnforcementActions) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not transform constraint to ValidatingAdmissionPolicyBinding") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not transform constraint to ValidatingAdmissionPolicyBinding") } newVapBinding, err := getRunTimeVAPBinding(groupVersion, transformedVapBinding, currentVapBinding) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding object with runtime group version") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not get ValidatingAdmissionPolicyBinding object with runtime group version") } if err := controllerutil.SetControllerReference(instance, newVapBinding, r.scheme); err != nil { @@ -573,21 +577,22 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction if currentVapBinding == nil { log.Info("creating vapbinding") if err := r.writer.Create(ctx, newVapBinding); err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not create ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, fmt.Sprintf("could not create ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } else if !reflect.DeepEqual(currentVapBinding, newVapBinding) { log.Info("updating vapbinding") if err := r.writer.Update(ctx, newVapBinding); err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } + vapEnforcementPointStatus.Enforced = true } // do not generate vapbinding resources // remove if exists - if !generateVAPB && groupVersion != nil { + if !couldGenerateVAPB && groupVersion != nil { currentVapBinding, err := vapBindingForVersion(*groupVersion) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, "could not get ValidatingAdmissionPolicyBinding API version") } vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName()) log.Info("check if vapbinding exists", "vapBindingName", vapBindingName) @@ -600,10 +605,17 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction if currentVapBinding != nil { log.Info("deleting vapbinding") if err := r.writer.Delete(ctx, currentVapBinding); err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } } + if shouldGenerateVAPB { + status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, vapEnforcementPointStatus) + log.Info("updating constraint status with enforcement point status", "status", status.Status) + if err := r.writer.Update(ctx, status); err != nil { + return noDelay, err + } + } return noDelay, nil } diff --git a/pkg/controller/constraint/constraint_controller_test.go b/pkg/controller/constraint/constraint_controller_test.go index ad244d39bd5..cea0a13ff2a 100644 --- a/pkg/controller/constraint/constraint_controller_test.go +++ b/pkg/controller/constraint/constraint_controller_test.go @@ -615,7 +615,7 @@ func TestReportErrorOnConstraintStatus(t *testing.T) { writer: writer, } - err := r.reportErrorOnConstraintStatus(context.TODO(), tt.status, tt.err, tt.message) + err := r.reportErrorOnConstraintStatus(context.TODO(), tt.status, constraintstatusv1beta1.EnforcementPointStatus{}, tt.err, tt.message) if err == nil || err.Error() != tt.expectedError.Error() { t.Errorf("expected error %v, got %v", tt.expectedError, err) } diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index 371c1c9a06e..28f6fdf3b89 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -331,6 +331,7 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec status.Status.TemplateUID = ct.GetUID() status.Status.ObservedGeneration = ct.GetGeneration() status.Status.Errors = nil + status.Status.VAPGenerationStatus = statusv1beta1.VAPGenerationStatus{} unversionedProposedCRD, err := r.cfClient.CreateCRD(ctx, unversionedCT) if err != nil { @@ -441,8 +442,13 @@ func (r *ReconcileConstraintTemplate) handleUpdate( t.Observe(unversionedCT) generateVap, err := constraint.ShouldGenerateVAP(unversionedCT) - if err != nil && !errors.Is(err, celSchema.ErrCodeNotDefined) { + if err != nil { logger.Error(err, "generateVap error") + if generateVap { + generateVap = false + status.Status.VAPGenerationStatus.Generated = false + status.Status.VAPGenerationStatus.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 +857,7 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 return err } } + status.Status.VAPGenerationStatus.Generated = true } // do not generate VAP resources // remove if exists diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index 9010f61f640..672ebbb9b31 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -377,6 +377,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 +399,72 @@ func TestReconcile(t *testing.T) { if err != nil { t.Fatal(err) } + + logger.Info("Running test: Warning should be present on constrainttemplate for not able to generate VAP") + 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.Warning == "" { + return fmt.Errorf("expected warning message") + } + return nil + }) + 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.Enforced { + return fmt.Errorf("expected enforced to be false") + } + return nil + } + } + return fmt.Errorf("expected enforcement point status") + }) + if err != nil { + t.Fatal(err) + } + constraint.DefaultGenerateVAP = ptr.To[bool](false) }) t.Run("Vap should not be created for only rego engine", func(t *testing.T) { @@ -476,6 +543,29 @@ func TestReconcile(t *testing.T) { if err != nil { t.Fatal(err) } + + logger.Info("Running test: constraint template should have vap generated status set to true") + 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.Generated { + return fmt.Errorf("Expected VAP generation status to be true") + } + return nil + }) + if err != nil { + t.Fatal(err) + } }) t.Run("VapBinding should not be created", func(t *testing.T) { @@ -824,6 +914,70 @@ func TestReconcile(t *testing.T) { } }) + 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.Generated { + return fmt.Errorf("Expected VAP generation status to be true") + } + + 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.Enforced { + return fmt.Errorf("expected enforced to be true") + } + return nil + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } + }) + t.Run("Constraint is marked as enforced", func(t *testing.T) { suffix := "MarkedEnforced" 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 } From c72db950ef3948e8d147c71fd22abba0ca42f26c Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Fri, 8 Nov 2024 22:03:52 +0000 Subject: [PATCH 02/13] removing not needed file Signed-off-by: Jaydip Gabani --- ...vkmanifest.gatekeeper.sh_gvkmanifests.yaml | 52 ------------------- 1 file changed, 52 deletions(-) delete mode 100644 config/crd/bases/gvkmanifest.gatekeeper.sh_gvkmanifests.yaml diff --git a/config/crd/bases/gvkmanifest.gatekeeper.sh_gvkmanifests.yaml b/config/crd/bases/gvkmanifest.gatekeeper.sh_gvkmanifests.yaml deleted file mode 100644 index 07e9ee1019f..00000000000 --- a/config/crd/bases/gvkmanifest.gatekeeper.sh_gvkmanifests.yaml +++ /dev/null @@ -1,52 +0,0 @@ ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.14.0 - name: gvkmanifests.gvkmanifest.gatekeeper.sh -spec: - group: gvkmanifest.gatekeeper.sh - names: - kind: GVKManifest - listKind: GVKManifestList - plural: gvkmanifests - singular: gvkmanifest - scope: Cluster - versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - description: GVKManifest is the Schema for the GVKManifest API. - properties: - apiVersion: - description: |- - APIVersion defines the versioned schema of this representation of an object. - Servers should convert recognized schemas to the latest internal value, and - may reject unrecognized values. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources - type: string - kind: - description: |- - Kind is a string value representing the REST resource this object represents. - Servers may infer this from the endpoint the client submits requests to. - Cannot be updated. - In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds - type: string - metadata: - type: object - spec: - properties: - groups: - additionalProperties: - additionalProperties: - items: - type: string - type: array - type: object - type: object - type: object - type: object - served: true - storage: true From 8c273630d77931c43a5d19bcde92f9437e56bef3 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Sat, 9 Nov 2024 00:45:26 +0000 Subject: [PATCH 03/13] fixing unit tests Signed-off-by: Jaydip Gabani --- pkg/controller/constraint/constraint_controller_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/constraint/constraint_controller_test.go b/pkg/controller/constraint/constraint_controller_test.go index cea0a13ff2a..5811276f946 100644 --- a/pkg/controller/constraint/constraint_controller_test.go +++ b/pkg/controller/constraint/constraint_controller_test.go @@ -445,14 +445,14 @@ func TestShouldGenerateVAP(t *testing.T) { }, }, vapDefault: true, - expected: false, + expected: true, wantErr: true, }, { name: "template with only Rego engine", template: makeTemplateWithRegoEngine(), vapDefault: true, - expected: false, + expected: true, wantErr: true, }, { From 0e40ed255ab18bd61480d2bf035f8512951bba25 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Thu, 14 Nov 2024 00:37:47 +0000 Subject: [PATCH 04/13] fixing unit test Signed-off-by: Jaydip Gabani --- .../constrainttemplate_controller_test.go | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index 672ebbb9b31..972aab30ed7 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 { @@ -423,7 +422,7 @@ func TestReconcile(t *testing.T) { t.Fatal(err) } - logger.Info("Running test: EnforcementPointStatus should indicate missing CEL engine for constraint using VAP enforcementpoint with rego templates") + 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 @@ -591,8 +590,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 } @@ -628,8 +626,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 } @@ -714,8 +711,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 } @@ -763,7 +759,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) @@ -808,8 +804,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 } @@ -894,20 +889,23 @@ 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) @@ -968,10 +966,10 @@ func TestReconcile(t *testing.T) { if !ep.Enforced { return fmt.Errorf("expected enforced to be true") } - return nil + break } } - return nil + return c.Delete(ctx, cstr) }) if err != nil { t.Fatal(err) @@ -1191,7 +1189,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 } @@ -1498,6 +1496,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) @@ -1598,7 +1597,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 } @@ -1619,7 +1618,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 } @@ -1652,7 +1651,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 } @@ -1679,7 +1678,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 } From 0238c737b5a69c6f6058339289eaf957e6fc504b Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Thu, 14 Nov 2024 22:50:02 +0000 Subject: [PATCH 05/13] adding observegeneration for enforcement point status Signed-off-by: Jaydip Gabani --- .../v1beta1/constraintpodstatus_types.go | 7 ++- .../constrainttemplatepodstatus_types.go | 4 +- ...s.gatekeeper.sh_constraintpodstatuses.yaml | 9 ++- ...eper.sh_constrainttemplatepodstatuses.yaml | 4 +- ...intpodstatus-customresourcedefinition.yaml | 9 ++- ...atepodstatus-customresourcedefinition.yaml | 4 +- manifest_staging/deploy/gatekeeper.yaml | 13 ++-- .../constraint/constraint_controller.go | 61 ++++++++++++------- .../constraint/constraint_controller_test.go | 2 +- .../constrainttemplate/constants.go | 7 +++ .../constrainttemplate_controller.go | 4 +- .../constrainttemplate_controller_test.go | 18 +++--- 12 files changed, 89 insertions(+), 53 deletions(-) diff --git a/apis/status/v1beta1/constraintpodstatus_types.go b/apis/status/v1beta1/constraintpodstatus_types.go index 3a4c38d4d52..7b4176a1ef5 100644 --- a/apis/status/v1beta1/constraintpodstatus_types.go +++ b/apis/status/v1beta1/constraintpodstatus_types.go @@ -56,9 +56,10 @@ type Error struct { // EnforcementPointStatus represents the status of a single enforcement point. type EnforcementPointStatus struct { - EnforcementPoint string `json:"enforcementPoint"` - Enforced bool `json:"enforced"` - Message string `json:"message,omitempty"` + EnforcementPoint string `json:"enforcementPoint"` + Code string `json:"code"` + Message string `json:"message,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/status/v1beta1/constrainttemplatepodstatus_types.go b/apis/status/v1beta1/constrainttemplatepodstatus_types.go index ed7689f87bb..256cea7fca0 100644 --- a/apis/status/v1beta1/constrainttemplatepodstatus_types.go +++ b/apis/status/v1beta1/constrainttemplatepodstatus_types.go @@ -39,8 +39,8 @@ type ConstraintTemplatePodStatusStatus struct { // VAPGenerationStatus represents the status of VAP generation. type VAPGenerationStatus struct { - Generated bool `json:"generated,omitempty"` - Warning string `json:"warning,omitempty"` + Code string `json:"code,omitempty"` + Warning string `json:"warning,omitempty"` } // +kubebuilder:object:root=true diff --git a/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml index 0dce1d32093..bf3fa089ee2 100644 --- a/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml @@ -53,14 +53,17 @@ spec: description: EnforcementPointStatus represents the status of a single enforcement point. properties: - enforced: - type: boolean + code: + type: string enforcementPoint: type: string message: type: string + observedGeneration: + format: int64 + type: integer required: - - enforced + - code - enforcementPoint type: object type: array diff --git a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml index 7a50c63c8f3..c64d1162897 100644 --- a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml @@ -77,8 +77,8 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: - generated: - type: boolean + code: + type: string warning: type: string type: object diff --git a/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml index 2ade6ab11a3..c34816c277e 100644 --- a/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml @@ -54,14 +54,17 @@ spec: items: description: EnforcementPointStatus represents the status of a single enforcement point. properties: - enforced: - type: boolean + code: + type: string enforcementPoint: type: string message: type: string + observedGeneration: + format: int64 + type: integer required: - - enforced + - code - enforcementPoint type: object type: array diff --git a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml index bc2defb6b0c..96b2aa6876e 100644 --- a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml @@ -76,8 +76,8 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: - generated: - type: boolean + code: + type: string warning: type: string type: object diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index c1ddf841d58..b97086aa91a 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2664,14 +2664,17 @@ spec: items: description: EnforcementPointStatus represents the status of a single enforcement point. properties: - enforced: - type: boolean + code: + type: string enforcementPoint: type: string message: type: string + observedGeneration: + format: int64 + type: integer required: - - enforced + - code - enforcementPoint type: object type: array @@ -2781,8 +2784,8 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: - generated: - type: boolean + code: + type: string warning: type: string type: object diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 21ad19879c8..9a305494a5d 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" + ErrGenerateVAPBCode = "errror" + GeneratedVAPBCode = "generated" + WaitVAPBCode = "waiting" ) var ( @@ -296,12 +299,11 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R status.Status.ConstraintUID = instance.GetUID() status.Status.ObservedGeneration = instance.GetGeneration() status.Status.Errors = nil - status.Status.EnforcementPointsStatus = nil if c, err := r.cfClient.GetConstraint(instance); err != nil || !reflect.DeepEqual(instance, c) { err := util.ValidateEnforcementAction(enforcementAction, instance.Object) if err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, "could not validate enforcement actions") + return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not validate enforcement actions") } if err := r.cacheConstraint(ctx, instance); err != nil { @@ -310,7 +312,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R status: metrics.ErrorStatus, }) reportMetrics = true - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, "could not cache constraint") + return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not cache constraint") } logAddition(r.log, instance, enforcementAction) } @@ -469,9 +471,8 @@ func (r *ReconcileConstraint) cacheConstraint(ctx context.Context, instance *uns return nil } -func (r *ReconcileConstraint) reportErrorOnConstraintStatus(ctx context.Context, status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPointStatus constraintstatusv1beta1.EnforcementPointStatus, err error, message string) error { +func (r *ReconcileConstraint) reportErrorOnConstraintStatus(ctx context.Context, status *constraintstatusv1beta1.ConstraintPodStatus, err error, message string) error { status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("%s: %s", message, err)}) - status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, enforcementPointStatus) if err2 := r.writer.Update(ctx, status); err2 != nil { log.Error(err2, message, "error", "could not update constraint status") return errorpkg.Wrapf(err, fmt.Sprintf("%s, could not update constraint status: %s", message, err2)) @@ -485,17 +486,31 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction log.Info("generate operation is not assigned, ValidatingAdmissionPolicyBinding resource will not be generated") return noDelay, nil } - vapEnforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: util.VAPEnforcementPoint, Enforced: false} ct := &v1beta1.ConstraintTemplate{} err := r.reader.Get(ctx, types.NamespacedName{Name: strings.ToLower(instance.GetKind())}, ct) if err != nil { return noDelay, err } + vapEnforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: util.VAPEnforcementPoint, Code: ErrGenerateVAPBCode, ObservedGeneration: instance.GetGeneration()} + vapEnforcementPointStatusIndex := -1 + + for i, ep := range status.Status.EnforcementPointsStatus { + if ep.EnforcementPoint == util.VAPEnforcementPoint { + status.Status.EnforcementPointsStatus[i] = vapEnforcementPointStatus + vapEnforcementPointStatusIndex = i + break + } + } + if vapEnforcementPointStatusIndex == -1 { + status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, vapEnforcementPointStatus) + vapEnforcementPointStatusIndex = len(status.Status.EnforcementPointsStatus) - 1 + } + 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, constraintstatusv1beta1.EnforcementPointStatus{}, 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 couldGenerateVAPB := shouldGenerateVAPB @@ -511,12 +526,12 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction } else { unversionedCT := &templates.ConstraintTemplate{} if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not convert ConstraintTemplate to unversioned") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not convert ConstraintTemplate to unversioned") } hasVAP, err := ShouldGenerateVAP(unversionedCT) switch { case errors.Is(err, celSchema.ErrCELEngineMissing): - vapEnforcementPointStatus.Message = err.Error() + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = err.Error() couldGenerateVAPB = false case err != nil: log.Error(err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy", "constraint", instance.GetName(), "constraint_template", unversionedCT.GetName()) @@ -529,17 +544,21 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction default: // reconcile for vapb generation if annotation is not set if ct.Annotations == nil || ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, errors.New("annotation to wait for ValidatingAdmissionPolicyBinding generation not found"), "could not find annotation to wait for ValidatingAdmissionPolicyBinding generation") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, errors.New("annotation to wait for ValidatingAdmissionPolicyBinding generation not found"), "could not find annotation to wait for ValidatingAdmissionPolicyBinding generation") } // waiting for sometime before generating vapbinding, gives api-server time to cache CRDs timestamp := ct.Annotations[BlockVAPBGenerationUntilAnnotation] t, err := time.Parse(time.RFC3339, timestamp) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not parse timestamp") + 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) + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Code = WaitVAPBCode + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = fmt.Sprintf("waiting for %s before generating ValidatingAdmissionPolicyBinding to make sure api-server has cached constraint CRD", wait) + err := r.writer.Update(ctx, status) + return wait, err } } } @@ -550,7 +569,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction if couldGenerateVAPB && groupVersion != nil { currentVapBinding, err := vapBindingForVersion(*groupVersion) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not get ValidatingAdmissionPolicyBinding API version") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") } vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName()) log.Info("check if vapbinding exists", "vapBindingName", vapBindingName) @@ -562,12 +581,12 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction } transformedVapBinding, err := transform.ConstraintToBinding(instance, VAPEnforcementActions) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not transform constraint to ValidatingAdmissionPolicyBinding") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not transform constraint to ValidatingAdmissionPolicyBinding") } newVapBinding, err := getRunTimeVAPBinding(groupVersion, transformedVapBinding, currentVapBinding) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, "could not get ValidatingAdmissionPolicyBinding object with runtime group version") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding object with runtime group version") } if err := controllerutil.SetControllerReference(instance, newVapBinding, r.scheme); err != nil { @@ -577,22 +596,23 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction if currentVapBinding == nil { log.Info("creating vapbinding") if err := r.writer.Create(ctx, newVapBinding); err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, fmt.Sprintf("could not create ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not create ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } else if !reflect.DeepEqual(currentVapBinding, newVapBinding) { log.Info("updating vapbinding") if err := r.writer.Update(ctx, newVapBinding); err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, vapEnforcementPointStatus, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } - vapEnforcementPointStatus.Enforced = true + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Code = GeneratedVAPBCode + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = "" } // do not generate vapbinding resources // remove if exists if !couldGenerateVAPB && groupVersion != nil { currentVapBinding, err := vapBindingForVersion(*groupVersion) if err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, "could not get ValidatingAdmissionPolicyBinding API version") + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") } vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName()) log.Info("check if vapbinding exists", "vapBindingName", vapBindingName) @@ -605,12 +625,11 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction if currentVapBinding != nil { log.Info("deleting vapbinding") if err := r.writer.Delete(ctx, currentVapBinding); err != nil { - return noDelay, r.reportErrorOnConstraintStatus(ctx, status, constraintstatusv1beta1.EnforcementPointStatus{}, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } } if shouldGenerateVAPB { - status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, vapEnforcementPointStatus) log.Info("updating constraint status with enforcement point status", "status", status.Status) if err := r.writer.Update(ctx, status); err != nil { return noDelay, err diff --git a/pkg/controller/constraint/constraint_controller_test.go b/pkg/controller/constraint/constraint_controller_test.go index 5811276f946..6fe288a34f6 100644 --- a/pkg/controller/constraint/constraint_controller_test.go +++ b/pkg/controller/constraint/constraint_controller_test.go @@ -615,7 +615,7 @@ func TestReportErrorOnConstraintStatus(t *testing.T) { writer: writer, } - err := r.reportErrorOnConstraintStatus(context.TODO(), tt.status, constraintstatusv1beta1.EnforcementPointStatus{}, tt.err, tt.message) + err := r.reportErrorOnConstraintStatus(context.TODO(), tt.status, tt.err, tt.message) if err == nil || err.Error() != tt.expectedError.Error() { t.Errorf("expected error %v, got %v", tt.expectedError, err) } diff --git a/pkg/controller/constrainttemplate/constants.go b/pkg/controller/constrainttemplate/constants.go index e423a9ff7ed..46080d0f0ac 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 ( + // ErrGenerateVAPCode indicates a problem generating a VAP. + ErrGenerateVAPCode = "errror" + // GeneratedVAPCode indicates a VAP was generated. + GeneratedVAPCode = "generated" +) diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index 28f6fdf3b89..d9261aca56f 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -446,7 +446,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( logger.Error(err, "generateVap error") if generateVap { generateVap = false - status.Status.VAPGenerationStatus.Generated = false + status.Status.VAPGenerationStatus.Code = ErrGenerateVAPCode status.Status.VAPGenerationStatus.Warning = fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.Error()) } } @@ -857,7 +857,7 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 return err } } - status.Status.VAPGenerationStatus.Generated = true + status.Status.VAPGenerationStatus.Code = GeneratedVAPCode } // do not generate VAP resources // remove if exists diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index 972aab30ed7..c5f74c53517 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -452,8 +452,8 @@ func TestReconcile(t *testing.T) { if ep.Message == "" { return fmt.Errorf("expected message") } - if ep.Enforced { - return fmt.Errorf("expected enforced to be false") + if ep.Code != ErrGenerateVAPCode { + return fmt.Errorf("expected error code") } return nil } @@ -543,7 +543,7 @@ func TestReconcile(t *testing.T) { t.Fatal(err) } - logger.Info("Running test: constraint template should have vap generated status set to true") + logger.Info("Running test: constraint template should have vap generated status code set to generated") err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { return true }, func() error { @@ -557,8 +557,8 @@ func TestReconcile(t *testing.T) { return err } - if !statusObj.Status.VAPGenerationStatus.Generated { - return fmt.Errorf("Expected VAP generation status to be true") + if statusObj.Status.VAPGenerationStatus.Code != GeneratedVAPCode { + return fmt.Errorf("Expected VAP generation status code to be %s", GeneratedVAPCode) } return nil }) @@ -944,8 +944,8 @@ func TestReconcile(t *testing.T) { return err } - if !statusObj.Status.VAPGenerationStatus.Generated { - return fmt.Errorf("Expected VAP generation status to be true") + if statusObj.Status.VAPGenerationStatus.Code != GeneratedVAPCode { + return fmt.Errorf("Expected VAP generation status code to be %s", GeneratedVAPCode) } cStatusObj := &statusv1beta1.ConstraintPodStatus{} @@ -963,8 +963,8 @@ func TestReconcile(t *testing.T) { if ep.Message != "" { return fmt.Errorf("message not expected") } - if !ep.Enforced { - return fmt.Errorf("expected enforced to be true") + if ep.Code != constraint.GeneratedVAPBCode { + return fmt.Errorf("expected code to be %s", constraint.GeneratedVAPBCode) } break } From a4f17a369489df3cd26e344e3435a1e70ced4322 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Fri, 15 Nov 2024 17:23:08 +0000 Subject: [PATCH 06/13] renaming code -> state field Signed-off-by: Jaydip Gabani --- apis/status/v1beta1/constraintpodstatus_types.go | 2 +- .../v1beta1/constrainttemplatepodstatus_types.go | 2 +- ...atus.gatekeeper.sh_constraintpodstatuses.yaml | 6 +++--- ...ekeeper.sh_constrainttemplatepodstatuses.yaml | 2 +- ...traintpodstatus-customresourcedefinition.yaml | 6 +++--- ...mplatepodstatus-customresourcedefinition.yaml | 2 +- manifest_staging/deploy/gatekeeper.yaml | 8 ++++---- .../constraint/constraint_controller.go | 12 ++++++------ pkg/controller/constrainttemplate/constants.go | 8 ++++---- .../constrainttemplate_controller.go | 4 ++-- .../constrainttemplate_controller_test.go | 16 ++++++++-------- 11 files changed, 34 insertions(+), 34 deletions(-) diff --git a/apis/status/v1beta1/constraintpodstatus_types.go b/apis/status/v1beta1/constraintpodstatus_types.go index 7b4176a1ef5..156699c5e4f 100644 --- a/apis/status/v1beta1/constraintpodstatus_types.go +++ b/apis/status/v1beta1/constraintpodstatus_types.go @@ -57,7 +57,7 @@ type Error struct { // EnforcementPointStatus represents the status of a single enforcement point. type EnforcementPointStatus struct { EnforcementPoint string `json:"enforcementPoint"` - Code string `json:"code"` + State string `json:"state"` Message string `json:"message,omitempty"` ObservedGeneration int64 `json:"observedGeneration,omitempty"` } diff --git a/apis/status/v1beta1/constrainttemplatepodstatus_types.go b/apis/status/v1beta1/constrainttemplatepodstatus_types.go index 256cea7fca0..1caa411cdcd 100644 --- a/apis/status/v1beta1/constrainttemplatepodstatus_types.go +++ b/apis/status/v1beta1/constrainttemplatepodstatus_types.go @@ -39,7 +39,7 @@ type ConstraintTemplatePodStatusStatus struct { // VAPGenerationStatus represents the status of VAP generation. type VAPGenerationStatus struct { - Code string `json:"code,omitempty"` + State string `json:"state,omitempty"` Warning string `json:"warning,omitempty"` } diff --git a/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml index bf3fa089ee2..3216c03aaab 100644 --- a/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml @@ -53,8 +53,6 @@ spec: description: EnforcementPointStatus represents the status of a single enforcement point. properties: - code: - type: string enforcementPoint: type: string message: @@ -62,9 +60,11 @@ spec: observedGeneration: format: int64 type: integer + state: + type: string required: - - code - enforcementPoint + - state type: object type: array errors: diff --git a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml index c64d1162897..29bbc7fdae7 100644 --- a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml @@ -77,7 +77,7 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: - code: + state: type: string warning: type: string diff --git a/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml index c34816c277e..9caedd58716 100644 --- a/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml @@ -54,8 +54,6 @@ spec: items: description: EnforcementPointStatus represents the status of a single enforcement point. properties: - code: - type: string enforcementPoint: type: string message: @@ -63,9 +61,11 @@ spec: observedGeneration: format: int64 type: integer + state: + type: string required: - - code - enforcementPoint + - state type: object type: array errors: diff --git a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml index 96b2aa6876e..238d5987dc4 100644 --- a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml @@ -76,7 +76,7 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: - code: + state: type: string warning: type: string diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index b97086aa91a..744a61ce966 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2664,8 +2664,6 @@ spec: items: description: EnforcementPointStatus represents the status of a single enforcement point. properties: - code: - type: string enforcementPoint: type: string message: @@ -2673,9 +2671,11 @@ spec: observedGeneration: format: int64 type: integer + state: + type: string required: - - code - enforcementPoint + - state type: object type: array errors: @@ -2784,7 +2784,7 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: - code: + state: type: string warning: type: string diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 9a305494a5d..55ee9562dba 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -65,9 +65,9 @@ import ( const ( BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until" - ErrGenerateVAPBCode = "errror" - GeneratedVAPBCode = "generated" - WaitVAPBCode = "waiting" + ErrGenerateVAPBState = "errror" + GeneratedVAPBState = "generated" + WaitVAPBState = "waiting" ) var ( @@ -492,7 +492,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction return noDelay, err } - vapEnforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: util.VAPEnforcementPoint, Code: ErrGenerateVAPBCode, ObservedGeneration: instance.GetGeneration()} + vapEnforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: util.VAPEnforcementPoint, State: ErrGenerateVAPBState, ObservedGeneration: instance.GetGeneration()} vapEnforcementPointStatusIndex := -1 for i, ep := range status.Status.EnforcementPointsStatus { @@ -555,7 +555,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction } if t.After(time.Now()) { wait := time.Until(t) - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Code = WaitVAPBCode + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].State = WaitVAPBState status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = fmt.Sprintf("waiting for %s before generating ValidatingAdmissionPolicyBinding to make sure api-server has cached constraint CRD", wait) err := r.writer.Update(ctx, status) return wait, err @@ -604,7 +604,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Code = GeneratedVAPBCode + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].State = GeneratedVAPBState status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = "" } // do not generate vapbinding resources diff --git a/pkg/controller/constrainttemplate/constants.go b/pkg/controller/constrainttemplate/constants.go index 46080d0f0ac..7265222d568 100644 --- a/pkg/controller/constrainttemplate/constants.go +++ b/pkg/controller/constrainttemplate/constants.go @@ -14,8 +14,8 @@ const ( ) const ( - // ErrGenerateVAPCode indicates a problem generating a VAP. - ErrGenerateVAPCode = "errror" - // GeneratedVAPCode indicates a VAP was generated. - GeneratedVAPCode = "generated" + // ErrGenerateVAPState indicates a problem generating a VAP. + ErrGenerateVAPState = "errror" + // 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 d9261aca56f..b8c6f5bab7b 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -446,7 +446,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( logger.Error(err, "generateVap error") if generateVap { generateVap = false - status.Status.VAPGenerationStatus.Code = ErrGenerateVAPCode + status.Status.VAPGenerationStatus.State = ErrGenerateVAPState status.Status.VAPGenerationStatus.Warning = fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.Error()) } } @@ -857,7 +857,7 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 return err } } - status.Status.VAPGenerationStatus.Code = GeneratedVAPCode + status.Status.VAPGenerationStatus.State = GeneratedVAPState } // do not generate VAP resources // remove if exists diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index c5f74c53517..f8b556da3e3 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -452,7 +452,7 @@ func TestReconcile(t *testing.T) { if ep.Message == "" { return fmt.Errorf("expected message") } - if ep.Code != ErrGenerateVAPCode { + if ep.State != ErrGenerateVAPState { return fmt.Errorf("expected error code") } return nil @@ -543,7 +543,7 @@ func TestReconcile(t *testing.T) { t.Fatal(err) } - logger.Info("Running test: constraint template should have vap generated status code set to generated") + 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 { @@ -557,8 +557,8 @@ func TestReconcile(t *testing.T) { return err } - if statusObj.Status.VAPGenerationStatus.Code != GeneratedVAPCode { - return fmt.Errorf("Expected VAP generation status code to be %s", GeneratedVAPCode) + if statusObj.Status.VAPGenerationStatus.State != GeneratedVAPState { + return fmt.Errorf("Expected VAP generation status state to be %s", GeneratedVAPState) } return nil }) @@ -944,8 +944,8 @@ func TestReconcile(t *testing.T) { return err } - if statusObj.Status.VAPGenerationStatus.Code != GeneratedVAPCode { - return fmt.Errorf("Expected VAP generation status code to be %s", GeneratedVAPCode) + if statusObj.Status.VAPGenerationStatus.State != GeneratedVAPState { + return fmt.Errorf("Expected VAP generation status state to be %s", GeneratedVAPState) } cStatusObj := &statusv1beta1.ConstraintPodStatus{} @@ -963,8 +963,8 @@ func TestReconcile(t *testing.T) { if ep.Message != "" { return fmt.Errorf("message not expected") } - if ep.Code != constraint.GeneratedVAPBCode { - return fmt.Errorf("expected code to be %s", constraint.GeneratedVAPBCode) + if ep.State != constraint.GeneratedVAPBState { + return fmt.Errorf("expected state to be %s", constraint.GeneratedVAPBState) } break } From ae48233569b25bd022b3b707979c3bd2ff2c2ae3 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Sat, 16 Nov 2024 00:15:22 +0000 Subject: [PATCH 07/13] adding observed generation and not wiping CT VAPgeneration status Signed-off-by: Jaydip Gabani --- apis/status/v1beta1/constrainttemplatepodstatus_types.go | 7 ++++--- apis/status/v1beta1/zz_generated.deepcopy.go | 6 +++++- ...status.gatekeeper.sh_constrainttemplatepodstatuses.yaml | 3 +++ ...nstrainttemplatepodstatus-customresourcedefinition.yaml | 3 +++ manifest_staging/deploy/gatekeeper.yaml | 3 +++ pkg/controller/constrainttemplate/constants.go | 2 +- .../constrainttemplate/constrainttemplate_controller.go | 4 +++- 7 files changed, 22 insertions(+), 6 deletions(-) diff --git a/apis/status/v1beta1/constrainttemplatepodstatus_types.go b/apis/status/v1beta1/constrainttemplatepodstatus_types.go index 1caa411cdcd..a06c6696417 100644 --- a/apis/status/v1beta1/constrainttemplatepodstatus_types.go +++ b/apis/status/v1beta1/constrainttemplatepodstatus_types.go @@ -34,13 +34,14 @@ type ConstraintTemplatePodStatusStatus struct { Operations []string `json:"operations,omitempty"` ObservedGeneration int64 `json:"observedGeneration,omitempty"` Errors []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"` - VAPGenerationStatus VAPGenerationStatus `json:"vapGenerationStatus,omitempty"` + VAPGenerationStatus *VAPGenerationStatus `json:"vapGenerationStatus,omitempty"` } // VAPGenerationStatus represents the status of VAP generation. type VAPGenerationStatus struct { - State string `json:"state,omitempty"` - Warning string `json:"warning,omitempty"` + 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 6b165de0856..0249b331f42 100644 --- a/apis/status/v1beta1/zz_generated.deepcopy.go +++ b/apis/status/v1beta1/zz_generated.deepcopy.go @@ -293,7 +293,11 @@ func (in *ConstraintTemplatePodStatusStatus) DeepCopyInto(out *ConstraintTemplat } } } - out.VAPGenerationStatus = in.VAPGenerationStatus + 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. diff --git a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml index 29bbc7fdae7..69b04663e0a 100644 --- a/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml @@ -77,6 +77,9 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: + observedGeneration: + format: int64 + type: integer state: type: string warning: diff --git a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml index 238d5987dc4..09b0b9c64e8 100644 --- a/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml @@ -76,6 +76,9 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: + observedGeneration: + format: int64 + type: integer state: type: string warning: diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index 744a61ce966..b98798f1955 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2784,6 +2784,9 @@ spec: vapGenerationStatus: description: VAPGenerationStatus represents the status of VAP generation. properties: + observedGeneration: + format: int64 + type: integer state: type: string warning: diff --git a/pkg/controller/constrainttemplate/constants.go b/pkg/controller/constrainttemplate/constants.go index 7265222d568..8f4ebae888f 100644 --- a/pkg/controller/constrainttemplate/constants.go +++ b/pkg/controller/constrainttemplate/constants.go @@ -15,7 +15,7 @@ const ( const ( // ErrGenerateVAPState indicates a problem generating a VAP. - ErrGenerateVAPState = "errror" + 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 b8c6f5bab7b..df29c8e79f8 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -331,7 +331,6 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec status.Status.TemplateUID = ct.GetUID() status.Status.ObservedGeneration = ct.GetGeneration() status.Status.Errors = nil - status.Status.VAPGenerationStatus = statusv1beta1.VAPGenerationStatus{} unversionedProposedCRD, err := r.cfClient.CreateCRD(ctx, unversionedCT) if err != nil { @@ -447,6 +446,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( if generateVap { generateVap = false status.Status.VAPGenerationStatus.State = ErrGenerateVAPState + status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() status.Status.VAPGenerationStatus.Warning = fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.Error()) } } @@ -858,6 +858,8 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 } } status.Status.VAPGenerationStatus.State = GeneratedVAPState + status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() + status.Status.VAPGenerationStatus.Warning = "" } // do not generate VAP resources // remove if exists From 8ef8278eddcc98c96cc1035747ca82c032d40e72 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Sat, 16 Nov 2024 09:58:05 +0000 Subject: [PATCH 08/13] updating vap status on delete, fixing tests Signed-off-by: Jaydip Gabani --- .../constraint/constraint_controller.go | 29 ++++++++----------- .../constrainttemplate/constants.go | 2 ++ .../constrainttemplate_controller.go | 11 +++++++ 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 55ee9562dba..ebcd2f4b553 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -68,6 +68,7 @@ const ( ErrGenerateVAPBState = "errror" GeneratedVAPBState = "generated" WaitVAPBState = "waiting" + DeletedVAPBState = "deleted" ) var ( @@ -513,7 +514,6 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ValidatingAdmissionPolicyBinding should be generated") } isAPIEnabled := false - couldGenerateVAPB := shouldGenerateVAPB var groupVersion *schema.GroupVersion if shouldGenerateVAPB { isAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&log) @@ -522,7 +522,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction if !isAPIEnabled { log.Error(ErrValidatingAdmissionPolicyAPIDisabled, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName()) status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("cannot generate ValidatingAdmissionPolicyBinding: %s", ErrValidatingAdmissionPolicyAPIDisabled)}) - couldGenerateVAPB = false + shouldGenerateVAPB = false } else { unversionedCT := &templates.ConstraintTemplate{} if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil { @@ -532,15 +532,15 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction switch { case errors.Is(err, celSchema.ErrCELEngineMissing): status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = err.Error() - couldGenerateVAPB = false + 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()) status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy: %s", err)}) - couldGenerateVAPB = false + shouldGenerateVAPB = false case !hasVAP: log.Error(ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName(), "constraint_template", unversionedCT.GetName()) status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("cannot generate ValidatingAdmissionPolicyBinding: %s", ErrVAPConditionsNotSatisfied)}) - couldGenerateVAPB = false + shouldGenerateVAPB = false default: // reconcile for vapb generation if annotation is not set if ct.Annotations == nil || ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" { @@ -557,16 +557,15 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction wait := time.Until(t) status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].State = WaitVAPBState status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = fmt.Sprintf("waiting for %s before generating ValidatingAdmissionPolicyBinding to make sure api-server has cached constraint CRD", wait) - err := r.writer.Update(ctx, status) - return wait, err + return wait, r.writer.Update(ctx, status) } } } } - r.log.Info("constraint controller", "generateVAPB", couldGenerateVAPB) + r.log.Info("constraint controller", "generateVAPB", shouldGenerateVAPB) // generate vapbinding resources - if couldGenerateVAPB && 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") @@ -609,7 +608,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction } // do not generate vapbinding resources // remove if exists - if !couldGenerateVAPB && 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") @@ -627,15 +626,11 @@ 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)) } + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].State = DeletedVAPBState + status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = "" } } - if shouldGenerateVAPB { - log.Info("updating constraint status with enforcement point status", "status", status.Status) - if err := r.writer.Update(ctx, status); err != nil { - return noDelay, err - } - } - return noDelay, nil + return noDelay, r.writer.Update(ctx, status) } func NewConstraintsCache() *ConstraintsCache { diff --git a/pkg/controller/constrainttemplate/constants.go b/pkg/controller/constrainttemplate/constants.go index 8f4ebae888f..fecd8b80af2 100644 --- a/pkg/controller/constrainttemplate/constants.go +++ b/pkg/controller/constrainttemplate/constants.go @@ -18,4 +18,6 @@ const ( ErrGenerateVAPState = "error" // GeneratedVAPState indicates a VAP was generated. GeneratedVAPState = "generated" + // DeletedVAPState indicates a VAP was deleted. + DeletedVAPState = "deleted" ) diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index df29c8e79f8..7bc429f6dc4 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -445,6 +445,9 @@ func (r *ReconcileConstraintTemplate) handleUpdate( logger.Error(err, "generateVap error") if generateVap { generateVap = false + if status.Status.VAPGenerationStatus == nil { + status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{} + } status.Status.VAPGenerationStatus.State = ErrGenerateVAPState status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() status.Status.VAPGenerationStatus.Warning = fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.Error()) @@ -857,6 +860,9 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 return err } } + if status.Status.VAPGenerationStatus == nil { + status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{} + } status.Status.VAPGenerationStatus.State = GeneratedVAPState status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() status.Status.VAPGenerationStatus.Warning = "" @@ -884,6 +890,11 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not delete VAP object", status, err) return err } + if status.Status.VAPGenerationStatus != nil { + status.Status.VAPGenerationStatus.State = DeletedVAPState + status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() + status.Status.VAPGenerationStatus.Warning = "" + } // after VAP is deleted, trigger update event for all constraints if err := r.triggerConstraintEvents(ctx, ct, status); err != nil { return err From b25ca5390ea00ea53fc414e7e23eee94a36a1a7a Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Mon, 18 Nov 2024 06:13:47 +0000 Subject: [PATCH 09/13] fixing test Signed-off-by: Jaydip Gabani --- .../v1beta1/constrainttemplatepodstatus_types.go | 2 +- apis/status/v1beta1/zz_generated.deepcopy.go | 6 +----- pkg/controller/constraint/constraint_controller.go | 2 +- .../constrainttemplate_controller.go | 14 +++----------- .../constrainttemplate_controller_test.go | 3 +-- 5 files changed, 7 insertions(+), 20 deletions(-) diff --git a/apis/status/v1beta1/constrainttemplatepodstatus_types.go b/apis/status/v1beta1/constrainttemplatepodstatus_types.go index a06c6696417..418de3e761e 100644 --- a/apis/status/v1beta1/constrainttemplatepodstatus_types.go +++ b/apis/status/v1beta1/constrainttemplatepodstatus_types.go @@ -34,7 +34,7 @@ type ConstraintTemplatePodStatusStatus struct { Operations []string `json:"operations,omitempty"` ObservedGeneration int64 `json:"observedGeneration,omitempty"` Errors []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"` - VAPGenerationStatus *VAPGenerationStatus `json:"vapGenerationStatus,omitempty"` + VAPGenerationStatus VAPGenerationStatus `json:"vapGenerationStatus,omitempty"` } // VAPGenerationStatus represents the status of VAP generation. diff --git a/apis/status/v1beta1/zz_generated.deepcopy.go b/apis/status/v1beta1/zz_generated.deepcopy.go index 0249b331f42..6b165de0856 100644 --- a/apis/status/v1beta1/zz_generated.deepcopy.go +++ b/apis/status/v1beta1/zz_generated.deepcopy.go @@ -293,11 +293,7 @@ func (in *ConstraintTemplatePodStatusStatus) DeepCopyInto(out *ConstraintTemplat } } } - if in.VAPGenerationStatus != nil { - in, out := &in.VAPGenerationStatus, &out.VAPGenerationStatus - *out = new(VAPGenerationStatus) - **out = **in - } + out.VAPGenerationStatus = in.VAPGenerationStatus } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConstraintTemplatePodStatusStatus. diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index ebcd2f4b553..ee683a4baf6 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -65,7 +65,7 @@ import ( const ( BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until" - ErrGenerateVAPBState = "errror" + ErrGenerateVAPBState = "error" GeneratedVAPBState = "generated" WaitVAPBState = "waiting" DeletedVAPBState = "deleted" diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index 7bc429f6dc4..ae703e31a0a 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -445,9 +445,6 @@ func (r *ReconcileConstraintTemplate) handleUpdate( logger.Error(err, "generateVap error") if generateVap { generateVap = false - if status.Status.VAPGenerationStatus == nil { - status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{} - } status.Status.VAPGenerationStatus.State = ErrGenerateVAPState status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() status.Status.VAPGenerationStatus.Warning = fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.Error()) @@ -860,9 +857,6 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 return err } } - if status.Status.VAPGenerationStatus == nil { - status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{} - } status.Status.VAPGenerationStatus.State = GeneratedVAPState status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() status.Status.VAPGenerationStatus.Warning = "" @@ -890,11 +884,9 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not delete VAP object", status, err) return err } - if status.Status.VAPGenerationStatus != nil { - status.Status.VAPGenerationStatus.State = DeletedVAPState - status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() - status.Status.VAPGenerationStatus.Warning = "" - } + status.Status.VAPGenerationStatus.State = DeletedVAPState + status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() + status.Status.VAPGenerationStatus.Warning = "" // 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 f8b556da3e3..98f8b155740 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -452,7 +452,7 @@ func TestReconcile(t *testing.T) { if ep.Message == "" { return fmt.Errorf("expected message") } - if ep.State != ErrGenerateVAPState { + if ep.State != constraint.ErrGenerateVAPBState { return fmt.Errorf("expected error code") } return nil @@ -463,7 +463,6 @@ func TestReconcile(t *testing.T) { if err != nil { t.Fatal(err) } - constraint.DefaultGenerateVAP = ptr.To[bool](false) }) t.Run("Vap should not be created for only rego engine", func(t *testing.T) { From c33b601fcd528a87aa0af49e2e0cd6af081e8047 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Tue, 19 Nov 2024 01:01:49 +0000 Subject: [PATCH 10/13] remobing delete status for vap EP Signed-off-by: Jaydip Gabani --- .../constrainttemplatepodstatus_types.go | 2 +- apis/status/v1beta1/zz_generated.deepcopy.go | 6 ++- .../constraint/constraint_controller.go | 46 ++++++++++--------- .../constrainttemplate_controller.go | 12 ++--- .../constrainttemplate_controller_test.go | 6 +-- 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/apis/status/v1beta1/constrainttemplatepodstatus_types.go b/apis/status/v1beta1/constrainttemplatepodstatus_types.go index 418de3e761e..a06c6696417 100644 --- a/apis/status/v1beta1/constrainttemplatepodstatus_types.go +++ b/apis/status/v1beta1/constrainttemplatepodstatus_types.go @@ -34,7 +34,7 @@ type ConstraintTemplatePodStatusStatus struct { Operations []string `json:"operations,omitempty"` ObservedGeneration int64 `json:"observedGeneration,omitempty"` Errors []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"` - VAPGenerationStatus VAPGenerationStatus `json:"vapGenerationStatus,omitempty"` + VAPGenerationStatus *VAPGenerationStatus `json:"vapGenerationStatus,omitempty"` } // VAPGenerationStatus represents the status of VAP generation. diff --git a/apis/status/v1beta1/zz_generated.deepcopy.go b/apis/status/v1beta1/zz_generated.deepcopy.go index 6b165de0856..0249b331f42 100644 --- a/apis/status/v1beta1/zz_generated.deepcopy.go +++ b/apis/status/v1beta1/zz_generated.deepcopy.go @@ -293,7 +293,11 @@ func (in *ConstraintTemplatePodStatusStatus) DeepCopyInto(out *ConstraintTemplat } } } - out.VAPGenerationStatus = in.VAPGenerationStatus + 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. diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index ee683a4baf6..4b1fa156455 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -493,21 +493,6 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction return noDelay, err } - vapEnforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: util.VAPEnforcementPoint, State: ErrGenerateVAPBState, ObservedGeneration: instance.GetGeneration()} - vapEnforcementPointStatusIndex := -1 - - for i, ep := range status.Status.EnforcementPointsStatus { - if ep.EnforcementPoint == util.VAPEnforcementPoint { - status.Status.EnforcementPointsStatus[i] = vapEnforcementPointStatus - vapEnforcementPointStatusIndex = i - break - } - } - if vapEnforcementPointStatusIndex == -1 { - status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, vapEnforcementPointStatus) - vapEnforcementPointStatusIndex = len(status.Status.EnforcementPointsStatus) - 1 - } - shouldGenerateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance) if err != nil { log.Error(err, "could not determine if ValidatingAdmissionPolicyBinding should be generated") @@ -531,7 +516,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction hasVAP, err := ShouldGenerateVAP(unversionedCT) switch { case errors.Is(err, celSchema.ErrCELEngineMissing): - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = err.Error() + 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()) @@ -555,8 +540,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction } if t.After(time.Now()) { wait := time.Until(t) - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].State = WaitVAPBState - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = fmt.Sprintf("waiting for %s before generating ValidatingAdmissionPolicyBinding to make sure api-server has cached constraint CRD", wait) + 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) } } @@ -603,8 +587,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) } } - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].State = GeneratedVAPBState - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = "" + updateEnforcementPointStatus(status, util.VAPEnforcementPoint, GeneratedVAPBState, "", instance.GetGeneration()) } // do not generate vapbinding resources // remove if exists @@ -626,8 +609,7 @@ 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)) } - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].State = DeletedVAPBState - status.Status.EnforcementPointsStatus[vapEnforcementPointStatusIndex].Message = "" + cleanEnforcementPointStatus(status, util.VAPEnforcementPoint) } } return noDelay, r.writer.Update(ctx, status) @@ -752,3 +734,23 @@ func v1beta1ToV1(v1beta1Obj *admissionregistrationv1beta1.ValidatingAdmissionPol return obj, nil } + +func updateEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string, state string, message string, observedGeneration int64) { + vapEnforcementPointStatus := 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] = vapEnforcementPointStatus + return + } + } + status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, vapEnforcementPointStatus) +} + +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/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index ae703e31a0a..d4b4e885b92 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -445,9 +445,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( logger.Error(err, "generateVap error") if generateVap { generateVap = false - status.Status.VAPGenerationStatus.State = ErrGenerateVAPState - status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() - status.Status.VAPGenerationStatus.Warning = fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.Error()) + status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{State: ErrGenerateVAPState, ObservedGeneration: ct.GetGeneration(), Warning: fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.Error())} } } @@ -857,9 +855,7 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1 return err } } - status.Status.VAPGenerationStatus.State = GeneratedVAPState - status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() - status.Status.VAPGenerationStatus.Warning = "" + status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{State: GeneratedVAPState, ObservedGeneration: ct.GetGeneration(), Warning: ""} } // do not generate VAP resources // remove if exists @@ -884,9 +880,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.State = DeletedVAPState - status.Status.VAPGenerationStatus.ObservedGeneration = ct.GetGeneration() - status.Status.VAPGenerationStatus.Warning = "" + 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 98f8b155740..de40949602c 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -413,7 +413,7 @@ func TestReconcile(t *testing.T) { return err } - if statusObj.Status.VAPGenerationStatus.Warning == "" { + if statusObj.Status.VAPGenerationStatus == nil || statusObj.Status.VAPGenerationStatus.Warning == "" { return fmt.Errorf("expected warning message") } return nil @@ -556,7 +556,7 @@ func TestReconcile(t *testing.T) { return err } - if statusObj.Status.VAPGenerationStatus.State != GeneratedVAPState { + if statusObj.Status.VAPGenerationStatus == nil || statusObj.Status.VAPGenerationStatus.State != GeneratedVAPState { return fmt.Errorf("Expected VAP generation status state to be %s", GeneratedVAPState) } return nil @@ -943,7 +943,7 @@ func TestReconcile(t *testing.T) { return err } - if statusObj.Status.VAPGenerationStatus.State != GeneratedVAPState { + if statusObj.Status.VAPGenerationStatus == nil || statusObj.Status.VAPGenerationStatus.State != GeneratedVAPState { return fmt.Errorf("Expected VAP generation status state to be %s", GeneratedVAPState) } From 5dff00d57af6f275ee7d4934af5addd5f37c4317 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Tue, 19 Nov 2024 01:04:44 +0000 Subject: [PATCH 11/13] removing unused constant Signed-off-by: Jaydip Gabani --- pkg/controller/constraint/constraint_controller.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 4b1fa156455..41c482d00fb 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -68,7 +68,6 @@ const ( ErrGenerateVAPBState = "error" GeneratedVAPBState = "generated" WaitVAPBState = "waiting" - DeletedVAPBState = "deleted" ) var ( From 1afa5ac4cd6ea1b77fc8d52f4ebc5d2a30e11360 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Wed, 20 Nov 2024 19:26:31 +0000 Subject: [PATCH 12/13] updating shouldgeneratevap Signed-off-by: Jaydip Gabani --- .../constraint/constraint_controller.go | 2 +- .../constraint/constraint_controller_test.go | 16 ++----------- .../constrainttemplate_controller.go | 7 ++---- .../constrainttemplate_controller_test.go | 23 ------------------- 4 files changed, 5 insertions(+), 43 deletions(-) diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 41c482d00fb..277a130edac 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -419,7 +419,7 @@ func (r *ReconcileConstraint) getOrCreatePodStatus(ctx context.Context, constrai func ShouldGenerateVAP(ct *templates.ConstraintTemplate) (bool, error) { source, err := celSchema.GetSourceFromTemplate(ct) if err != nil { - return *DefaultGenerateVAP, err + return false, err } if source.GenerateVAP == nil { return *DefaultGenerateVAP, nil diff --git a/pkg/controller/constraint/constraint_controller_test.go b/pkg/controller/constraint/constraint_controller_test.go index 6fe288a34f6..7bdce693577 100644 --- a/pkg/controller/constraint/constraint_controller_test.go +++ b/pkg/controller/constraint/constraint_controller_test.go @@ -445,14 +445,14 @@ func TestShouldGenerateVAP(t *testing.T) { }, }, vapDefault: true, - expected: true, + expected: false, wantErr: true, }, { name: "template with only Rego engine", template: makeTemplateWithRegoEngine(), vapDefault: true, - expected: true, + expected: false, wantErr: true, }, { @@ -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/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index d4b4e885b92..ff69d8a972e 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -441,12 +441,9 @@ func (r *ReconcileConstraintTemplate) handleUpdate( t.Observe(unversionedCT) generateVap, err := constraint.ShouldGenerateVAP(unversionedCT) - if err != nil { + if err != nil && !errors.Is(err, celSchema.ErrCELEngineMissing) { logger.Error(err, "generateVap error") - if generateVap { - generateVap = false - status.Status.VAPGenerationStatus = &statusv1beta1.VAPGenerationStatus{State: ErrGenerateVAPState, ObservedGeneration: ct.GetGeneration(), Warning: fmt.Sprintf("ValidatingAdmissionPolicy is not generated: %s", err.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 { diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index de40949602c..1e28caa924a 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -399,29 +399,6 @@ func TestReconcile(t *testing.T) { t.Fatal(err) } - logger.Info("Running test: Warning should be present on constrainttemplate for not able to generate VAP") - 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.Warning == "" { - return fmt.Errorf("expected warning message") - } - return nil - }) - 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 { From 45a53f7dab2aca0b501309548890ffd8bca40313 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Thu, 21 Nov 2024 01:29:49 +0000 Subject: [PATCH 13/13] removing deletedVAPState constraint Signed-off-by: Jaydip Gabani --- pkg/controller/constraint/constraint_controller.go | 6 +++--- pkg/controller/constrainttemplate/constants.go | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 277a130edac..e02b4f16584 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -735,14 +735,14 @@ func v1beta1ToV1(v1beta1Obj *admissionregistrationv1beta1.ValidatingAdmissionPol } func updateEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string, state string, message string, observedGeneration int64) { - vapEnforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: enforcementPoint, State: state, ObservedGeneration: observedGeneration, Message: message} + 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] = vapEnforcementPointStatus + status.Status.EnforcementPointsStatus[i] = enforcementPointStatus return } } - status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, vapEnforcementPointStatus) + status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, enforcementPointStatus) } func cleanEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string) { diff --git a/pkg/controller/constrainttemplate/constants.go b/pkg/controller/constrainttemplate/constants.go index fecd8b80af2..8f4ebae888f 100644 --- a/pkg/controller/constrainttemplate/constants.go +++ b/pkg/controller/constrainttemplate/constants.go @@ -18,6 +18,4 @@ const ( ErrGenerateVAPState = "error" // GeneratedVAPState indicates a VAP was generated. GeneratedVAPState = "generated" - // DeletedVAPState indicates a VAP was deleted. - DeletedVAPState = "deleted" )