chore: adding annotation to generate VAPB right away once the waiting window is over to protect against clock skews

pkg/controller/constraint/constraint_controller.go

@@ -65,9 +65,12 @@ import (
 
 const (
 	BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until"
+	VAPBGenerationAnnotation           = "gatekeeper.sh/vapb-generation"
 	ErrGenerateVAPBState               = "error"
 	GeneratedVAPBState                 = "generated"
 	WaitVAPBState                      = "waiting"
+	VAPBGenerationBlocked              = "blocked"
+	VAPBGenerationUnblocked            = "unblocked"
 )
 
 var (
@@ -541,20 +544,22 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 				shouldGenerateVAPB = false
 			default:
 				// reconcile for vapb generation if annotation is not set
-				if ct.Annotations == nil || ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" {
+				if ct.Annotations == nil || (ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" && ct.Annotations[VAPBGenerationAnnotation] != "unblocked") {
 					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, err, "could not parse timestamp")
-				}
-				if t.After(time.Now()) {
-					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)
+				if ct.Annotations[VAPBGenerationAnnotation] == "" || ct.Annotations[VAPBGenerationAnnotation] == VAPBGenerationBlocked {
+					// 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")
+					}
+					if t.After(time.Now()) {
+						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)
+					}
 				}
 			}
 		}

pkg/controller/constrainttemplate/constrainttemplate_controller.go

@@ -460,7 +460,8 @@ func (r *ReconcileConstraintTemplate) handleUpdate(
 		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 {
+	var requeueAfter time.Duration
+	if err := r.generateCRD(ctx, ct, proposedCRD, currentCRD, status, logger, generateVap, &requeueAfter); err != nil {
 		return reconcile.Result{}, err
 	}
 
@@ -479,7 +480,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate(
 		logger.Error(err, "update ct pod status error")
 		return reconcile.Result{Requeue: true}, nil
 	}
-	return reconcile.Result{}, nil
+	return reconcile.Result{RequeueAfter: requeueAfter}, nil
 }
 
 func (r *ReconcileConstraintTemplate) handleDelete(
@@ -757,7 +758,7 @@ func v1beta1ToV1(v1beta1Obj *admissionregistrationv1beta1.ValidatingAdmissionPol
 	return obj, nil
 }
 
-func (r *ReconcileConstraintTemplate) generateCRD(ctx context.Context, ct *v1beta1.ConstraintTemplate, proposedCRD, currentCRD *apiextensionsv1.CustomResourceDefinition, status *statusv1beta1.ConstraintTemplatePodStatus, logger logr.Logger, generateVAP bool) error {
+func (r *ReconcileConstraintTemplate) generateCRD(ctx context.Context, ct *v1beta1.ConstraintTemplate, proposedCRD, currentCRD *apiextensionsv1.CustomResourceDefinition, status *statusv1beta1.ConstraintTemplatePodStatus, logger logr.Logger, generateVAP bool, requeueAfter *time.Duration) error {
 	if !operations.IsAssigned(operations.Generate) {
 		return nil
 	}
@@ -789,9 +790,9 @@ func (r *ReconcileConstraintTemplate) generateCRD(ctx context.Context, ct *v1bet
 	if !generateVAP {
 		return nil
 	}
-
+	var err error
 	// We add the annotation as a follow-on update to be sure the timestamp is set relative to a time after the CRD is successfully created. Creating the CRD with a delay timestamp already set would not account for request latency.
-	err := r.updateTemplateWithBlockVAPBGenerationAnnotations(ctx, ct)
+	*requeueAfter, err = r.updateTemplateWithBlockVAPBGenerationAnnotations(ctx, ct)
 	if err != nil {
 		err = r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not annotate with timestamp to block VAPB generation", status, err)
 	}
@@ -907,23 +908,32 @@ func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1
 
 // updateTemplateWithBlockVAPBGenerationAnnotations updates the ConstraintTemplate with an annotation to block VAPB generation until specific time
 // This is to avoid the issue where the VAPB is generated before the CRD is cached in the API server.
-func (r *ReconcileConstraintTemplate) updateTemplateWithBlockVAPBGenerationAnnotations(ctx context.Context, ct *v1beta1.ConstraintTemplate) error {
+func (r *ReconcileConstraintTemplate) updateTemplateWithBlockVAPBGenerationAnnotations(ctx context.Context, ct *v1beta1.ConstraintTemplate) (time.Duration, error) {
+	noRequeue := time.Duration(0)
+	if ct.Annotations != nil && ct.Annotations[constraint.VAPBGenerationAnnotation] == constraint.VAPBGenerationUnblocked {
+		return noRequeue, nil
+	}
 	currentTime := time.Now()
 	if ct.Annotations != nil && ct.Annotations[constraint.BlockVAPBGenerationUntilAnnotation] != "" {
 		until := ct.Annotations[constraint.BlockVAPBGenerationUntilAnnotation]
 		t, err := time.Parse(time.RFC3339, until)
 		if err != nil {
-			return err
+			return noRequeue, err
 		}
 		// if wait time is within the time window to generate vap binding, do not update the annotation
 		// otherwise update the annotation with the current time + wait time. This prevents clock skew from preventing generation on task reschedule.
 		if t.Before(currentTime.Add(time.Duration(*constraint.DefaultWaitForVAPBGeneration) * time.Second)) {
-			return nil
+			if t.Before(currentTime) {
+				ct.Annotations[constraint.VAPBGenerationAnnotation] = constraint.VAPBGenerationUnblocked
+				return noRequeue, r.Update(ctx, ct)
+			}
+			return t.Sub(currentTime), nil
 		}
 	}
 	if ct.Annotations == nil {
 		ct.Annotations = make(map[string]string)
 	}
 	ct.Annotations[constraint.BlockVAPBGenerationUntilAnnotation] = currentTime.Add(time.Duration(*constraint.DefaultWaitForVAPBGeneration) * time.Second).Format(time.RFC3339)
-	return r.Update(ctx, ct)
+	ct.Annotations[constraint.VAPBGenerationAnnotation] = constraint.VAPBGenerationBlocked
+	return time.Duration(*constraint.DefaultWaitForVAPBGeneration) * time.Second, r.Update(ctx, ct)
 }

pkg/controller/constrainttemplate/constrainttemplate_controller_test.go

@@ -306,7 +306,7 @@ func TestReconcile(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		logger.Info("Running Test: block-vapb-generation-until annotation should not be present")
+		logger.Info("Running Test: vapb annotation should not be present on constraint template")
 		err = retry.OnError(testutils.ConstantRetry, func(_ error) bool {
 			return true
 		}, func() error {
@@ -317,6 +317,9 @@ func TestReconcile(t *testing.T) {
 			if _, ok := ct.GetAnnotations()[constraint.BlockVAPBGenerationUntilAnnotation]; ok {
 				return fmt.Errorf("unexpected %s annotations on CT", constraint.BlockVAPBGenerationUntilAnnotation)
 			}
+			if _, ok := ct.GetAnnotations()[constraint.VAPBGenerationAnnotation]; ok {
+				return fmt.Errorf("unexpected %s annotations on CT", constraint.VAPBGenerationAnnotation)
+			}
 			return nil
 		})
 	})
@@ -728,7 +731,6 @@ func TestReconcile(t *testing.T) {
 			if timestamp == "" {
 				return fmt.Errorf("expected %s annotations on CT", constraint.BlockVAPBGenerationUntilAnnotation)
 			}
-			// check if vapbinding resource exists now
 			if err := c.Get(ctx, types.NamespacedName{Name: cstr.GetName()}, cstr); err != nil {
 				return err
 			}
@@ -745,6 +747,9 @@ func TestReconcile(t *testing.T) {
 			if vapBindingCreationTime.Before(blockTime) {
 				return fmt.Errorf("VAPBinding should be created after default wait")
 			}
+			if ct.GetAnnotations()[constraint.VAPBGenerationAnnotation] == constraint.VAPBGenerationUnblocked {
+				return fmt.Errorf("expected %s annotations on CT to be unblocked", constraint.VAPBGenerationAnnotation)
+			}
 			return nil
 		})
 		if err != nil {
@@ -812,7 +817,7 @@ func TestReconcile(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		logger.Info("Running test: VAP ConstraintTemplate should have block-VAPB-generation-until annotation")
+		logger.Info("Running test: VAP ConstraintTemplate should have VAPB annotation")
 		err = retry.OnError(testutils.ConstantRetry, func(_ error) bool {
 			return true
 		}, func() error {
@@ -823,6 +828,9 @@ func TestReconcile(t *testing.T) {
 			if ct.GetAnnotations()[constraint.BlockVAPBGenerationUntilAnnotation] == "" {
 				return fmt.Errorf("expected %s annotations on CT", constraint.BlockVAPBGenerationUntilAnnotation)
 			}
+			if ct.GetAnnotations()[constraint.VAPBGenerationAnnotation] == "" {
+				return fmt.Errorf("expected %s annotations on CT", constraint.VAPBGenerationAnnotation)
+			}
 			return nil
 		})
 		if err != nil {
@@ -873,6 +881,9 @@ func TestReconcile(t *testing.T) {
 			if vapBindingCreationTime.Before(blockTime) {
 				return fmt.Errorf("VAPBinding should not be created before the timestamp")
 			}
+			if ct.GetAnnotations()[constraint.VAPBGenerationAnnotation] == constraint.VAPBGenerationUnblocked {
+				return fmt.Errorf("expected %s annotations on CT to be unblocked", constraint.VAPBGenerationAnnotation)
+			}
 			if err := c.Delete(ctx, cstr); err != nil {
 				return err
 			}