Skip to content

Commit

Permalink
chore: adding annotation to generate VAPB right away once the waiting…
Browse files Browse the repository at this point in the history 
… window is over to protect against clock skews (#3773)

Signed-off-by: Jaydip Gabani <[email protected]>
  • Loading branch information
@JaydipGabani
JaydipGabani authored Feb 3, 2025
1 parent 08196f9 commit 5738672
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 23 deletions.
27 changes: 16 additions & 11 deletions pkg/controller/constraint/constraint_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,12 @@ import (

const (
BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until"
VAPBGenerationAnnotation = "gatekeeper.sh/vapb-generation-state"
ErrGenerateVAPBState = "error"
GeneratedVAPBState = "generated"
WaitVAPBState = "waiting"
VAPBGenerationBlocked = "blocked"
VAPBGenerationUnblocked = "unblocked"
)

var (
Expand Down Expand Up @@ -528,20 +531,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)
}
}
}
}
Expand Down
28 changes: 19 additions & 9 deletions pkg/controller/constrainttemplate/constrainttemplate_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,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
}

Expand All @@ -459,7 +460,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(
Expand Down Expand Up @@ -737,7 +738,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
}
Expand Down Expand Up @@ -769,9 +770,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)
}
Expand Down Expand Up @@ -887,23 +888,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)
}
21 changes: 18 additions & 3 deletions pkg/controller/constrainttemplate/constrainttemplate_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,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 {
Expand All @@ -315,6 +315,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
})
})
Expand Down Expand Up @@ -726,13 +729,14 @@ 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
}
// check if vapbinding resource exists now
vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{}
if err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("gatekeeper-%s", cstr.GetName())}, vapBinding); err != nil {
// Since tests retries 3000 times at 100 retries per second, adding sleep makes sure that this test gets covarage time > 30s to cover the default wait.
time.Sleep(10 * time.Millisecond)
return err
}
blockTime, err := time.Parse(time.RFC3339, timestamp)
Expand All @@ -743,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 {
Expand Down Expand Up @@ -810,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 {
Expand All @@ -821,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 {
Expand Down Expand Up @@ -865,12 +875,17 @@ func TestReconcile(t *testing.T) {
// 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 {
// Since tests retries 3000 times at 100 retries per second, adding sleep makes sure that this test gets covarage time > 30s to cover the default wait.
time.Sleep(10 * time.Millisecond)
return err
}
vapBindingCreationTime := vapBinding.GetCreationTimestamp().Time
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
}
Expand Down

0 comments on commit 5738672

Please sign in to comment.