Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
adding annotation to generate VAPB right away once the waiting window…
Browse files Browse the repository at this point in the history
… is over to protect against clock skews

Signed-off-by: Jaydip Gabani <[email protected]>
JaydipGabani committed Jan 11, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent f1113cf commit c566741
Showing 3 changed files with 49 additions and 23 deletions.
27 changes: 16 additions & 11 deletions pkg/controller/constraint/constraint_controller.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
}
28 changes: 19 additions & 9 deletions pkg/controller/constrainttemplate/constrainttemplate_controller.go
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit c566741

Please sign in to comment.