diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 55fb3c3740f..2ea91d488d0 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/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) + } } } } diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index 03ff303361d..8d68c006377 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/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) } diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index c0606e4b689..3a243525156 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/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 }