chore: adding enforcement point status, vapgeneratestatus (#3686)

Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>

apis/status/v1beta1/constraintpodstatus_types.go

@@ -39,11 +39,12 @@ type ConstraintPodStatusStatus struct {
 	// Storing the constraint UID allows us to detect drift, such as
 	// when a constraint has been recreated after its CRD was deleted
 	// out from under it, interrupting the watch
-	ConstraintUID      types.UID `json:"constraintUID,omitempty"`
-	Operations         []string  `json:"operations,omitempty"`
-	Enforced           bool      `json:"enforced,omitempty"`
-	Errors             []Error   `json:"errors,omitempty"`
-	ObservedGeneration int64     `json:"observedGeneration,omitempty"`
+	ConstraintUID           types.UID                `json:"constraintUID,omitempty"`
+	Operations              []string                 `json:"operations,omitempty"`
+	Enforced                bool                     `json:"enforced,omitempty"`
+	Errors                  []Error                  `json:"errors,omitempty"`
+	ObservedGeneration      int64                    `json:"observedGeneration,omitempty"`
+	EnforcementPointsStatus []EnforcementPointStatus `json:"enforcementPointsStatus,omitempty"`
 }
 
 // Error represents a single error caught while adding a constraint to engine.
@@ -53,6 +54,14 @@ type Error struct {
 	Location string `json:"location,omitempty"`
 }
 
+// EnforcementPointStatus represents the status of a single enforcement point.
+type EnforcementPointStatus struct {
+	EnforcementPoint   string `json:"enforcementPoint"`
+	State              string `json:"state"`
+	Message            string `json:"message,omitempty"`
+	ObservedGeneration int64  `json:"observedGeneration,omitempty"`
+}
+
 // +kubebuilder:object:root=true
 // +kubebuilder:resource:scope=Namespaced
 

apis/status/v1beta1/constrainttemplatepodstatus_types.go

@@ -29,11 +29,19 @@ import (
 // ConstraintTemplatePodStatusStatus defines the observed state of ConstraintTemplatePodStatus.
 type ConstraintTemplatePodStatusStatus struct {
 	// Important: Run "make" to regenerate code after modifying this file
-	ID                 string                             `json:"id,omitempty"`
-	TemplateUID        types.UID                          `json:"templateUID,omitempty"`
-	Operations         []string                           `json:"operations,omitempty"`
-	ObservedGeneration int64                              `json:"observedGeneration,omitempty"`
-	Errors             []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"`
+	ID                  string                             `json:"id,omitempty"`
+	TemplateUID         types.UID                          `json:"templateUID,omitempty"`
+	Operations          []string                           `json:"operations,omitempty"`
+	ObservedGeneration  int64                              `json:"observedGeneration,omitempty"`
+	Errors              []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"`
+	VAPGenerationStatus *VAPGenerationStatus               `json:"vapGenerationStatus,omitempty"`
+}
+
+// VAPGenerationStatus represents the status of VAP generation.
+type VAPGenerationStatus struct {
+	State              string `json:"state,omitempty"`
+	ObservedGeneration int64  `json:"observedGeneration,omitempty"`
+	Warning            string `json:"warning,omitempty"`
 }
 
 // +kubebuilder:object:root=true

config/crd/bases/status.gatekeeper.sh_constraintpodstatuses.yaml

@@ -48,6 +48,25 @@ spec:
                 type: string
               enforced:
                 type: boolean
+              enforcementPointsStatus:
+                items:
+                  description: EnforcementPointStatus represents the status of a single
+                    enforcement point.
+                  properties:
+                    enforcementPoint:
+                      type: string
+                    message:
+                      type: string
+                    observedGeneration:
+                      format: int64
+                      type: integer
+                    state:
+                      type: string
+                  required:
+                  - enforcementPoint
+                  - state
+                  type: object
+                type: array
               errors:
                 items:
                   description: Error represents a single error caught while adding

config/crd/bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml

@@ -74,6 +74,17 @@ spec:
                   don't ONLY use UUIDs, this is an alias to string.  Being a type captures
                   intent and helps make sure that UIDs and names do not get conflated.
                 type: string
+              vapGenerationStatus:
+                description: VAPGenerationStatus represents the status of VAP generation.
+                properties:
+                  observedGeneration:
+                    format: int64
+                    type: integer
+                  state:
+                    type: string
+                  warning:
+                    type: string
+                type: object
             type: object
         type: object
     served: true

manifest_staging/charts/gatekeeper/crds/constraintpodstatus-customresourcedefinition.yaml

@@ -50,6 +50,24 @@ spec:
                 type: string
               enforced:
                 type: boolean
+              enforcementPointsStatus:
+                items:
+                  description: EnforcementPointStatus represents the status of a single enforcement point.
+                  properties:
+                    enforcementPoint:
+                      type: string
+                    message:
+                      type: string
+                    observedGeneration:
+                      format: int64
+                      type: integer
+                    state:
+                      type: string
+                  required:
+                  - enforcementPoint
+                  - state
+                  type: object
+                type: array
               errors:
                 items:
                   description: Error represents a single error caught while adding a constraint to engine.

manifest_staging/charts/gatekeeper/crds/constrainttemplatepodstatus-customresourcedefinition.yaml

@@ -73,6 +73,17 @@ spec:
                   don't ONLY use UUIDs, this is an alias to string.  Being a type captures
                   intent and helps make sure that UIDs and names do not get conflated.
                 type: string
+              vapGenerationStatus:
+                description: VAPGenerationStatus represents the status of VAP generation.
+                properties:
+                  observedGeneration:
+                    format: int64
+                    type: integer
+                  state:
+                    type: string
+                  warning:
+                    type: string
+                type: object
             type: object
         type: object
     served: true

manifest_staging/deploy/gatekeeper.yaml

@@ -2660,6 +2660,24 @@ spec:
                 type: string
               enforced:
                 type: boolean
+              enforcementPointsStatus:
+                items:
+                  description: EnforcementPointStatus represents the status of a single enforcement point.
+                  properties:
+                    enforcementPoint:
+                      type: string
+                    message:
+                      type: string
+                    observedGeneration:
+                      format: int64
+                      type: integer
+                    state:
+                      type: string
+                  required:
+                  - enforcementPoint
+                  - state
+                  type: object
+                type: array
               errors:
                 items:
                   description: Error represents a single error caught while adding a constraint to engine.
@@ -2763,6 +2781,17 @@ spec:
                   don't ONLY use UUIDs, this is an alias to string.  Being a type captures
                   intent and helps make sure that UIDs and names do not get conflated.
                 type: string
+              vapGenerationStatus:
+                description: VAPGenerationStatus represents the status of VAP generation.
+                properties:
+                  observedGeneration:
+                    format: int64
+                    type: integer
+                  state:
+                    type: string
+                  warning:
+                    type: string
+                type: object
             type: object
         type: object
     served: true

pkg/controller/constraint/constraint_controller.go

@@ -65,6 +65,9 @@ import (
 
 const (
 	BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until"
+	ErrGenerateVAPBState               = "error"
+	GeneratedVAPBState                 = "generated"
+	WaitVAPBState                      = "waiting"
 )
 
 var (
@@ -489,39 +492,39 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 		return noDelay, err
 	}
 
-	generateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance)
+	shouldGenerateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance)
 	if err != nil {
 		log.Error(err, "could not determine if ValidatingAdmissionPolicyBinding should be generated")
 		return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ValidatingAdmissionPolicyBinding should be generated")
 	}
 	isAPIEnabled := false
 	var groupVersion *schema.GroupVersion
-	if generateVAPB {
+	if shouldGenerateVAPB {
 		isAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&log)
 	}
-	if generateVAPB {
+	if shouldGenerateVAPB {
 		if !isAPIEnabled {
 			log.Error(ErrValidatingAdmissionPolicyAPIDisabled, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName())
-			_ = r.reportErrorOnConstraintStatus(ctx, status, ErrValidatingAdmissionPolicyAPIDisabled, "cannot generate ValidatingAdmissionPolicyBinding")
-			generateVAPB = false
+			status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("cannot generate ValidatingAdmissionPolicyBinding: %s", ErrValidatingAdmissionPolicyAPIDisabled)})
+			shouldGenerateVAPB = false
 		} else {
 			unversionedCT := &templates.ConstraintTemplate{}
 			if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil {
 				return noDelay, r.reportErrorOnConstraintStatus(ctx, status, 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):
+				updateEnforcementPointStatus(status, util.VAPEnforcementPoint, ErrGenerateVAPBState, err.Error(), instance.GetGeneration())
+				shouldGenerateVAPB = false
 			case err != nil:
 				log.Error(err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy", "constraint", instance.GetName(), "constraint_template", unversionedCT.GetName())
-				_ = r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy")
-				generateVAPB = false
+				status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy: %s", err)})
+				shouldGenerateVAPB = false
 			case !hasVAP:
 				log.Error(ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName(), "constraint_template", unversionedCT.GetName())
-				_ = r.reportErrorOnConstraintStatus(ctx, status, ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding")
-				generateVAPB = false
+				status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("cannot generate ValidatingAdmissionPolicyBinding: %s", ErrVAPConditionsNotSatisfied)})
+				shouldGenerateVAPB = false
 			default:
 				// reconcile for vapb generation if annotation is not set
 				if ct.Annotations == nil || ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" {
@@ -535,15 +538,17 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 					return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not parse timestamp")
 				}
 				if t.After(time.Now()) {
-					return time.Until(t), nil
+					wait := time.Until(t)
+					updateEnforcementPointStatus(status, util.VAPEnforcementPoint, WaitVAPBState, fmt.Sprintf("waiting for %s before generating ValidatingAdmissionPolicyBinding to make sure api-server has cached constraint CRD", wait), instance.GetGeneration())
+					return wait, r.writer.Update(ctx, status)
 				}
 			}
 		}
 	}
 
-	r.log.Info("constraint controller", "generateVAPB", generateVAPB)
+	r.log.Info("constraint controller", "generateVAPB", shouldGenerateVAPB)
 	// generate vapbinding resources
-	if generateVAPB && groupVersion != nil {
+	if shouldGenerateVAPB && groupVersion != nil {
 		currentVapBinding, err := vapBindingForVersion(*groupVersion)
 		if err != nil {
 			return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version")
@@ -581,10 +586,11 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 				return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName))
 			}
 		}
+		updateEnforcementPointStatus(status, util.VAPEnforcementPoint, GeneratedVAPBState, "", instance.GetGeneration())
 	}
 	// do not generate vapbinding resources
 	// remove if exists
-	if !generateVAPB && groupVersion != nil {
+	if !shouldGenerateVAPB && groupVersion != nil {
 		currentVapBinding, err := vapBindingForVersion(*groupVersion)
 		if err != nil {
 			return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version")
@@ -602,9 +608,10 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 			if err := r.writer.Delete(ctx, currentVapBinding); err != nil {
 				return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName))
 			}
+			cleanEnforcementPointStatus(status, util.VAPEnforcementPoint)
 		}
 	}
-	return noDelay, nil
+	return noDelay, r.writer.Update(ctx, status)
 }
 
 func NewConstraintsCache() *ConstraintsCache {
@@ -726,3 +733,23 @@ func v1beta1ToV1(v1beta1Obj *admissionregistrationv1beta1.ValidatingAdmissionPol
 
 	return obj, nil
 }
+
+func updateEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string, state string, message string, observedGeneration int64) {
+	enforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: enforcementPoint, State: state, ObservedGeneration: observedGeneration, Message: message}
+	for i, ep := range status.Status.EnforcementPointsStatus {
+		if ep.EnforcementPoint == enforcementPoint {
+			status.Status.EnforcementPointsStatus[i] = enforcementPointStatus
+			return
+		}
+	}
+	status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, enforcementPointStatus)
+}
+
+func cleanEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string) {
+	for i, ep := range status.Status.EnforcementPointsStatus {
+		if ep.EnforcementPoint == enforcementPoint {
+			status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus[:i], status.Status.EnforcementPointsStatus[i+1:]...)
+			return
+		}
+	}
+}