chore: adding enforcement point status, vapgeneratestatus

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,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 {
+	State   string `json:"state,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,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:
+                  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,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:
+                  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,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:
+                  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               = "errror"
+	GeneratedVAPBState                 = "generated"
+	WaitVAPBState                      = "waiting"
 )
 
 var (
@@ -416,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 false, err
+		return *DefaultGenerateVAP, err
 	}
 	if source.GenerateVAP == nil {
 		return *DefaultGenerateVAP, nil
@@ -489,39 +492,55 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 		return noDelay, err
 	}
 
-	generateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance)
+	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")
 		return noDelay, r.reportErrorOnConstraintStatus(ctx, status, 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")
 			}
 			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):
+				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())
-				_ = 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] == "" {
@@ -535,15 +554,19 @@ 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)
+					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
 				}
 			}
 		}
 	}
 
-	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")
@@ -581,10 +604,12 @@ 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 = ""
 	}
 	// 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")
@@ -604,6 +629,12 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 			}
 		}
 	}
+	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
 }
 

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,
 		},
 		{