From 18347414127be839639d44296163310bd9e284ec Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Thu, 7 Nov 2024 15:08:26 -0800 Subject: [PATCH] feat: add generate operation and wait for VAPB generation (#3573) Signed-off-by: Jaydip Gabani --- Makefile | 3 +- cmd/build/helmify/kustomize-for-helm.yaml | 2 + cmd/build/helmify/replacements.go | 4 + cmd/build/helmify/static/README.md | 1 + manifest_staging/charts/gatekeeper/README.md | 1 + .../gatekeeper-audit-deployment.yaml | 4 + .../constraint/constraint_controller.go | 256 ++++++++------ .../constrainttemplate/constants.go | 2 + .../constrainttemplate_controller.go | 314 +++++++++++------- .../constrainttemplate_controller_test.go | 88 ++++- .../mutators/core/reconciler_test.go | 14 +- pkg/operations/operations.go | 2 + pkg/operations/operations_test.go | 2 +- 13 files changed, 439 insertions(+), 254 deletions(-) diff --git a/Makefile b/Makefile index 285b29da4a6..a704b2a3c5d 100644 --- a/Makefile +++ b/Makefile @@ -73,8 +73,6 @@ MANAGER_IMAGE_PATCH := "apiVersion: apps/v1\ \n - --disable-opa-builtin=http.send\ \n - --log-mutations\ \n - --mutation-annotations\ -\n - --default-create-vap-for-templates=${GENERATE_VAP}\ -\n - --default-create-vap-binding-for-constraints=${GENERATE_VAPBINDING}\ \n - --log-level=${LOG_LEVEL}\ \n---\ \napiVersion: apps/v1\ @@ -94,6 +92,7 @@ MANAGER_IMAGE_PATCH := "apiVersion: apps/v1\ \n - --operation=audit\ \n - --operation=status\ \n - --operation=mutation-status\ +\n - --operation=generate\ \n - --audit-chunk-size=500\ \n - --logtostderr\ \n - --default-create-vap-for-templates=${GENERATE_VAP}\ diff --git a/cmd/build/helmify/kustomize-for-helm.yaml b/cmd/build/helmify/kustomize-for-helm.yaml index b8a244eda18..8a2c01d646a 100644 --- a/cmd/build/helmify/kustomize-for-helm.yaml +++ b/cmd/build/helmify/kustomize-for-helm.yaml @@ -178,6 +178,7 @@ spec: - --audit-events-involved-namespace={{ .Values.auditEventsInvolvedNamespace }} - --operation=audit - --operation=status + - --operation=generate - HELMSUBST_DEPLOYMENT_AUDIT_PUBSUB_ARGS - HELMSUBST_MUTATION_STATUS_ENABLED_ARG - --logtostderr @@ -192,6 +193,7 @@ spec: - --enable-k8s-native-validation={{ .Values.enableK8sNativeValidation }} - HELMSUBST_DEPLOYMENT_DEFAULT_CREATE_VAP_FOR_TEMPLATES - HELMSUBST_DEPLOYMENT_DEFAULT_CREATE_VAPB_FOR_CONSTRAINTS + - HELMSUBST_DEPLOYMENT_AUDIT_DEFAULT_WAIT_VAPB_GENERATION imagePullPolicy: "{{ .Values.image.pullPolicy }}" HELMSUBST_AUDIT_CONTROLLER_MANAGER_DEPLOYMENT_IMAGE_RELEASE: "" ports: diff --git a/cmd/build/helmify/replacements.go b/cmd/build/helmify/replacements.go index ee18c3bb05c..20afb2c7a4e 100644 --- a/cmd/build/helmify/replacements.go +++ b/cmd/build/helmify/replacements.go @@ -112,6 +112,10 @@ var replacements = map[string]string{ "- HELMSUBST_MUTATION_STATUS_ENABLED_ARG": `{{ if not .Values.disableMutation}}- --operation=mutation-status{{- end }}`, + "- HELMSUBST_DEPLOYMENT_AUDIT_DEFAULT_WAIT_VAPB_GENERATION": `{{ if hasKey .Values "defaultWaitForVAPBGeneration"}} + - --default-wait-for-vapb-generation={{ .Values.defaultWaitForVAPBGeneration }} + {{- end }}`, + "- HELMSUBST_DEPLOYMENT_AUDIT_PUBSUB_ARGS": `{{ if hasKey .Values.audit "enablePubsub" }} - --enable-pub-sub={{ .Values.audit.enablePubsub }} {{- end }} diff --git a/cmd/build/helmify/static/README.md b/cmd/build/helmify/static/README.md index a6ec18a0169..92a7fa72897 100644 --- a/cmd/build/helmify/static/README.md +++ b/cmd/build/helmify/static/README.md @@ -171,6 +171,7 @@ information._ | enableK8sNativeValidation | Enable the K8s Native Validating driver to allow constraint templates to use rules written in VAP-style CEL (beta feature) | `true` | | defaultCreateVAPForTemplates | (alpha) Create VAP resource for template containing VAP-style CEL source. Allowed values are false: do not create Validating Admission Policy unless generateVAP: true is set on constraint template explicitly, true: create Validating Admission Policy unless generateVAP: false is set on constraint template explicitly. | `false` | | defaultCreateVAPBindingForConstraints | (alpha) Create VAPBinding resource for constraint of the template containing VAP-style CEL source. Allowed values are false: do not create Validating Admission Policy Binding, true: create Validating Admission Policy Binding. | `false` | +| defaultWaitForVAPBGeneration | (alpha) Wait time in seconds before generating a ValidatingAdmissionPolicyBinding after a constraint CRD is created. | `30` | | auditEventsInvolvedNamespace | Emit audit events for each violation in the involved objects namespace, the default (false) generates events in the namespace Gatekeeper is installed in. Audit events from cluster-scoped resources will continue to generate events in the namespace that Gatekeeper is installed in | `false` | | admissionEventsInvolvedNamespace | Emit admission events for each violation in the involved objects namespace, the default (false) generates events in the namespace Gatekeeper is installed in. Admission events from cluster-scoped resources will continue to generate events in the namespace that Gatekeeper is installed in | `false` | | logDenies | Log detailed info on each deny | `false` | diff --git a/manifest_staging/charts/gatekeeper/README.md b/manifest_staging/charts/gatekeeper/README.md index a6ec18a0169..92a7fa72897 100644 --- a/manifest_staging/charts/gatekeeper/README.md +++ b/manifest_staging/charts/gatekeeper/README.md @@ -171,6 +171,7 @@ information._ | enableK8sNativeValidation | Enable the K8s Native Validating driver to allow constraint templates to use rules written in VAP-style CEL (beta feature) | `true` | | defaultCreateVAPForTemplates | (alpha) Create VAP resource for template containing VAP-style CEL source. Allowed values are false: do not create Validating Admission Policy unless generateVAP: true is set on constraint template explicitly, true: create Validating Admission Policy unless generateVAP: false is set on constraint template explicitly. | `false` | | defaultCreateVAPBindingForConstraints | (alpha) Create VAPBinding resource for constraint of the template containing VAP-style CEL source. Allowed values are false: do not create Validating Admission Policy Binding, true: create Validating Admission Policy Binding. | `false` | +| defaultWaitForVAPBGeneration | (alpha) Wait time in seconds before generating a ValidatingAdmissionPolicyBinding after a constraint CRD is created. | `30` | | auditEventsInvolvedNamespace | Emit audit events for each violation in the involved objects namespace, the default (false) generates events in the namespace Gatekeeper is installed in. Audit events from cluster-scoped resources will continue to generate events in the namespace that Gatekeeper is installed in | `false` | | admissionEventsInvolvedNamespace | Emit admission events for each violation in the involved objects namespace, the default (false) generates events in the namespace Gatekeeper is installed in. Admission events from cluster-scoped resources will continue to generate events in the namespace that Gatekeeper is installed in | `false` | | logDenies | Log detailed info on each deny | `false` | diff --git a/manifest_staging/charts/gatekeeper/templates/gatekeeper-audit-deployment.yaml b/manifest_staging/charts/gatekeeper/templates/gatekeeper-audit-deployment.yaml index 90c32af5349..8f508b9add9 100644 --- a/manifest_staging/charts/gatekeeper/templates/gatekeeper-audit-deployment.yaml +++ b/manifest_staging/charts/gatekeeper/templates/gatekeeper-audit-deployment.yaml @@ -69,6 +69,7 @@ spec: - --audit-events-involved-namespace={{ .Values.auditEventsInvolvedNamespace }} - --operation=audit - --operation=status + - --operation=generate {{ if hasKey .Values.audit "enablePubsub" }} - --enable-pub-sub={{ .Values.audit.enablePubsub }} {{- end }} @@ -103,6 +104,9 @@ spec: {{- if hasKey .Values "defaultCreateVAPBindingForConstraints"}} - --default-create-vap-binding-for-constraints={{ .Values.defaultCreateVAPBindingForConstraints }} {{- end }} + {{ if hasKey .Values "defaultWaitForVAPBGeneration"}} + - --default-wait-for-vapb-generation={{ .Values.defaultWaitForVAPBGeneration }} + {{- end }} command: - /manager env: diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 196599fea30..fb90cec68fb 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -23,6 +23,7 @@ import ( "reflect" "strings" "sync" + "time" "github.com/go-logr/logr" "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1beta1" @@ -62,11 +63,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ) +const ( + BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until" +) + var ( - log = logf.Log.V(logging.DebugLevel).WithName("controller").WithValues(logging.Process, "constraint_controller") - discoveryErr *apiutil.ErrResourceDiscoveryFailed - DefaultGenerateVAPB = flag.Bool("default-create-vap-binding-for-constraints", false, "(alpha) Create VAPBinding resource for constraint of the template containing VAP-style CEL source. Allowed values are false: do not create Validating Admission Policy Binding, true: create Validating Admission Policy Binding.") - DefaultGenerateVAP = flag.Bool("default-create-vap-for-templates", false, "(alpha) Create VAP resource for template containing VAP-style CEL source. Allowed values are false: do not create Validating Admission Policy unless generateVAP: true is set on constraint template explicitly, true: create Validating Admission Policy unless generateVAP: false is set on constraint template explicitly.") + log = logf.Log.V(logging.DebugLevel).WithName("controller").WithValues(logging.Process, "constraint_controller") + discoveryErr *apiutil.ErrResourceDiscoveryFailed + DefaultGenerateVAPB = flag.Bool("default-create-vap-binding-for-constraints", false, "(alpha) Create VAPBinding resource for constraint of the template containing VAP-style CEL source. Allowed values are false: do not create Validating Admission Policy Binding, true: create Validating Admission Policy Binding.") + DefaultGenerateVAP = flag.Bool("default-create-vap-for-templates", false, "(alpha) Create VAP resource for template containing VAP-style CEL source. Allowed values are false: do not create Validating Admission Policy unless generateVAP: true is set on constraint template explicitly, true: create Validating Admission Policy unless generateVAP: false is set on constraint template explicitly.") + DefaultWaitForVAPBGeneration = flag.Int("default-wait-for-vapb-generation", 30, "(alpha) Wait time in seconds before generating a ValidatingAdmissionPolicyBinding after a constraint CRD is created.") ) var ( @@ -280,12 +286,6 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R } }() - ct := &v1beta1.ConstraintTemplate{} - err = r.reader.Get(ctx, types.NamespacedName{Name: strings.ToLower(instance.GetKind())}, ct) - if err != nil { - return reconcile.Result{}, err - } - if !deleted { r.log.Info("handling constraint update", "instance", instance) status, err := r.getOrCreatePodStatus(ctx, instance) @@ -302,105 +302,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R if err != nil { return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not validate enforcement actions") } - generateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance) - if err != nil { - log.Error(err, "could not determine if ValidatingAdmissionPolicyBinding should be generated") - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ValidatingAdmissionPolicyBinding should be generated") - } - isAPIEnabled := false - var groupVersion *schema.GroupVersion - if generateVAPB { - isAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&log) - } - if generateVAPB { - if !isAPIEnabled { - log.Error(ErrValidatingAdmissionPolicyAPIDisabled, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName()) - _ = r.reportErrorOnConstraintStatus(ctx, status, ErrValidatingAdmissionPolicyAPIDisabled, "cannot generate ValidatingAdmissionPolicyBinding") - generateVAPB = false - } else { - unversionedCT := &templates.ConstraintTemplate{} - if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not convert ConstraintTemplate to unversioned") - } - hasVAP, err := ShouldGenerateVAP(unversionedCT) - switch { - case errors.Is(err, celSchema.ErrCodeNotDefined): - generateVAPB = false - case err != nil: - log.Error(err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy", "constraint", instance.GetName(), "constraint_template", ct.GetName()) - _ = r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy") - generateVAPB = false - case !hasVAP: - log.Error(ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName(), "constraint_template", ct.GetName()) - _ = r.reportErrorOnConstraintStatus(ctx, status, ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding") - generateVAPB = false - default: - } - } - } - r.log.Info("constraint controller", "generateVAPB", generateVAPB) - // generate vapbinding resources - if generateVAPB && groupVersion != nil { - currentVapBinding, err := vapBindingForVersion(*groupVersion) - if err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") - } - vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName()) - log.Info("check if vapbinding exists", "vapBindingName", vapBindingName) - if err := r.reader.Get(ctx, types.NamespacedName{Name: vapBindingName}, currentVapBinding); err != nil { - if !apierrors.IsNotFound(err) && !errors.As(err, &discoveryErr) && !meta.IsNoMatchError(err) { - return reconcile.Result{}, err - } - currentVapBinding = nil - } - transformedVapBinding, err := transform.ConstraintToBinding(instance, VAPEnforcementActions) - if err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not transform constraint to ValidatingAdmissionPolicyBinding") - } - newVapBinding, err := getRunTimeVAPBinding(groupVersion, transformedVapBinding, currentVapBinding) - if err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding object with runtime group version") - } - - if err := controllerutil.SetControllerReference(instance, newVapBinding, r.scheme); err != nil { - return reconcile.Result{}, err - } - - if currentVapBinding == nil { - log.Info("creating vapbinding") - if err := r.writer.Create(ctx, newVapBinding); err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not create ValidatingAdmissionPolicyBinding: %s", vapBindingName)) - } - } else if !reflect.DeepEqual(currentVapBinding, newVapBinding) { - log.Info("updating vapbinding") - if err := r.writer.Update(ctx, newVapBinding); err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) - } - } - } - // do not generate vapbinding resources - // remove if exists - if !generateVAPB && groupVersion != nil { - currentVapBinding, err := vapBindingForVersion(*groupVersion) - if err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") - } - vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName()) - log.Info("check if vapbinding exists", "vapBindingName", vapBindingName) - if err := r.reader.Get(ctx, types.NamespacedName{Name: vapBindingName}, currentVapBinding); err != nil { - if !apierrors.IsNotFound(err) && !errors.As(err, &discoveryErr) && !meta.IsNoMatchError(err) { - return reconcile.Result{}, err - } - currentVapBinding = nil - } - if currentVapBinding != nil { - log.Info("deleting vapbinding") - if err := r.writer.Delete(ctx, currentVapBinding); err != nil { - return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName)) - } - } - } if err := r.cacheConstraint(ctx, instance); err != nil { r.constraintsCache.addConstraintKey(constraintKey, tags{ enforcementAction: enforcementAction, @@ -423,6 +325,14 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R status: metrics.ActiveStatus, }) reportMetrics = true + requeueAfter, err := r.manageVAPB(ctx, enforcementAction, instance, status) + if err != nil { + return reconcile.Result{RequeueAfter: requeueAfter}, err + } + if requeueAfter != time.Duration(0) { + log.Info("requeueing after", "requeueAfter", requeueAfter) + return reconcile.Result{RequeueAfter: requeueAfter}, nil + } } else { r.log.Info("handling constraint delete", "instance", instance) if _, err := r.cfClient.RemoveConstraint(ctx, instance); err != nil { @@ -567,6 +477,136 @@ func (r *ReconcileConstraint) reportErrorOnConstraintStatus(ctx context.Context, return err } +func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction util.EnforcementAction, instance *unstructured.Unstructured, status *constraintstatusv1beta1.ConstraintPodStatus) (time.Duration, error) { + noDelay := time.Duration(0) + if !operations.IsAssigned(operations.Generate) { + log.Info("generate operation is not assigned, ValidatingAdmissionPolicyBinding resource will not be generated") + return noDelay, nil + } + ct := &v1beta1.ConstraintTemplate{} + err := r.reader.Get(ctx, types.NamespacedName{Name: strings.ToLower(instance.GetKind())}, ct) + if err != nil { + return noDelay, err + } + + generateVAPB, 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 + var groupVersion *schema.GroupVersion + if generateVAPB { + isAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&log) + } + if generateVAPB { + if !isAPIEnabled { + log.Error(ErrValidatingAdmissionPolicyAPIDisabled, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName()) + _ = r.reportErrorOnConstraintStatus(ctx, status, ErrValidatingAdmissionPolicyAPIDisabled, "cannot generate ValidatingAdmissionPolicyBinding") + generateVAPB = 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 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 + 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 + default: + // reconcile for vapb generation if annotation is not set + if ct.Annotations == nil || ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" { + 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()) { + return time.Until(t), nil + } + } + } + } + + r.log.Info("constraint controller", "generateVAPB", generateVAPB) + // generate vapbinding resources + if generateVAPB && groupVersion != nil { + currentVapBinding, err := vapBindingForVersion(*groupVersion) + if err != nil { + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") + } + vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName()) + log.Info("check if vapbinding exists", "vapBindingName", vapBindingName) + if err := r.reader.Get(ctx, types.NamespacedName{Name: vapBindingName}, currentVapBinding); err != nil { + if !apierrors.IsNotFound(err) { + return noDelay, err + } + currentVapBinding = nil + } + transformedVapBinding, err := transform.ConstraintToBinding(instance, VAPEnforcementActions) + if err != nil { + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not transform constraint to ValidatingAdmissionPolicyBinding") + } + + newVapBinding, err := getRunTimeVAPBinding(groupVersion, transformedVapBinding, currentVapBinding) + if err != nil { + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding object with runtime group version") + } + + if err := controllerutil.SetControllerReference(instance, newVapBinding, r.scheme); err != nil { + return noDelay, err + } + + if currentVapBinding == nil { + log.Info("creating vapbinding") + if err := r.writer.Create(ctx, newVapBinding); err != nil { + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not create ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + } + } else if !reflect.DeepEqual(currentVapBinding, newVapBinding) { + log.Info("updating vapbinding") + if err := r.writer.Update(ctx, newVapBinding); err != nil { + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + } + } + } + // do not generate vapbinding resources + // remove if exists + if !generateVAPB && groupVersion != nil { + currentVapBinding, err := vapBindingForVersion(*groupVersion) + if err != nil { + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") + } + vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName()) + log.Info("check if vapbinding exists", "vapBindingName", vapBindingName) + if err := r.reader.Get(ctx, types.NamespacedName{Name: vapBindingName}, currentVapBinding); err != nil { + if !apierrors.IsNotFound(err) && !errors.As(err, &discoveryErr) && !meta.IsNoMatchError(err) { + return noDelay, err + } + currentVapBinding = nil + } + if currentVapBinding != nil { + log.Info("deleting vapbinding") + if err := r.writer.Delete(ctx, currentVapBinding); err != nil { + return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName)) + } + } + } + return noDelay, nil +} + func NewConstraintsCache() *ConstraintsCache { return &ConstraintsCache{ cache: make(map[string]tags), diff --git a/pkg/controller/constrainttemplate/constants.go b/pkg/controller/constrainttemplate/constants.go index ccd1eb9fa17..e423a9ff7ed 100644 --- a/pkg/controller/constrainttemplate/constants.go +++ b/pkg/controller/constrainttemplate/constants.go @@ -9,4 +9,6 @@ const ( ErrConversionCode = "conversion_error" // ErrIngestCode indicates a problem ingesting a ConstraintTemplate Rego code. ErrIngestCode = "ingest_error" + // ErrParseCode indicates a problem parsing a ConstraintTemplate. + ErrParseCode = "parse_error" ) diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index e331d03f752..371c1c9a06e 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -22,6 +22,7 @@ import ( "reflect" "time" + "github.com/go-logr/logr" "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1beta1" constraintclient "github.com/open-policy-agent/frameworks/constraint/pkg/client" "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" @@ -314,6 +315,14 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec return result, err } + unversionedCT := &templates.ConstraintTemplate{} + if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil { + logger.Error(err, "conversion error") + r.metrics.registry.add(request.NamespacedName, metrics.ErrorStatus) + logError(request.NamespacedName.Name) + return reconcile.Result{}, err + } + status, err := r.getOrCreatePodStatus(ctx, ct.Name) if err != nil { logger.Info("could not get/create pod status object", "error", err) @@ -323,14 +332,6 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec status.Status.ObservedGeneration = ct.GetGeneration() status.Status.Errors = nil - unversionedCT := &templates.ConstraintTemplate{} - if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil { - logger.Error(err, "conversion error") - r.metrics.registry.add(request.NamespacedName, metrics.ErrorStatus) - logError(request.NamespacedName.Name) - return reconcile.Result{}, err - } - unversionedProposedCRD, err := r.cfClient.CreateCRD(ctx, unversionedCT) if err != nil { logger.Error(err, "CRD creation error") @@ -378,13 +379,8 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec r.metrics.registry.add(request.NamespacedName, metrics.ErrorStatus) return reconcile.Result{}, err } - generateVap, err := constraint.ShouldGenerateVAP(unversionedCT) - if err != nil && !errors.Is(err, celSchema.ErrCodeNotDefined) { - logger.Error(err, "generateVap error") - } - logger.Info("generateVap", "r.generateVap", generateVap) - result, err := r.handleUpdate(ctx, ct, unversionedCT, proposedCRD, currentCRD, status, generateVap) + result, err := r.handleUpdate(ctx, ct, unversionedCT, proposedCRD, currentCRD, status) if err != nil { logger.Error(err, "handle update error") logError(request.NamespacedName.Name) @@ -417,7 +413,6 @@ func (r *ReconcileConstraintTemplate) handleUpdate( unversionedCT *templates.ConstraintTemplate, proposedCRD, currentCRD *apiextensionsv1.CustomResourceDefinition, status *statusv1beta1.ConstraintTemplatePodStatus, - generateVap bool, ) (reconcile.Result, error) { name := proposedCRD.GetName() logger := logger.WithValues("name", ct.GetName(), "crdName", name) @@ -445,131 +440,25 @@ func (r *ReconcileConstraintTemplate) handleUpdate( t := r.tracker.For(gvkConstraintTemplate) t.Observe(unversionedCT) - var newCRD *apiextensionsv1.CustomResourceDefinition - if currentCRD == nil { - newCRD = proposedCRD.DeepCopy() - } else { - newCRD = currentCRD.DeepCopy() - newCRD.Spec = proposedCRD.Spec + generateVap, err := constraint.ShouldGenerateVAP(unversionedCT) + if err != nil && !errors.Is(err, celSchema.ErrCodeNotDefined) { + logger.Error(err, "generateVap error") } - if err := controllerutil.SetControllerReference(ct, newCRD, r.scheme); err != nil { + if err := r.generateCRD(ctx, ct, proposedCRD, currentCRD, status, logger, generateVap); err != nil { return reconcile.Result{}, err } - if currentCRD == nil { - logger.Info("creating crd") - if err := r.Create(ctx, newCRD); err != nil { - err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not create CRD", status, err) - return reconcile.Result{}, err - } - } else if !reflect.DeepEqual(newCRD, currentCRD) { - logger.Info("updating crd") - if err := r.Update(ctx, newCRD); err != nil { - err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not update CRD", status, err) - return reconcile.Result{}, err - } - } // This must go after CRD creation/update as otherwise AddWatch will always fail logger.Info("making sure constraint is in watcher registry") if err := r.addWatch(ctx, makeGvk(ct.Spec.CRD.Spec.Names.Kind)); err != nil { logger.Error(err, "error adding template to watch registry") return reconcile.Result{}, err } - isVapAPIEnabled := false - var groupVersion *schema.GroupVersion - if generateVap { - isVapAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&logger) - } - logger.Info("isVapAPIEnabled", "isVapAPIEnabled", isVapAPIEnabled) - logger.Info("groupVersion", "groupVersion", groupVersion) - if generateVap && (!isVapAPIEnabled || groupVersion == nil) { - logger.Error(constraint.ErrValidatingAdmissionPolicyAPIDisabled, "ValidatingAdmissionPolicy resource cannot be generated for ConstraintTemplate", "name", ct.GetName()) - err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "ValidatingAdmissionPolicy resource cannot be generated for ConstraintTemplate", status, constraint.ErrValidatingAdmissionPolicyAPIDisabled) - return reconcile.Result{}, err - } - // generating vap resources - if generateVap && isVapAPIEnabled && groupVersion != nil { - currentVap, err := vapForVersion(groupVersion) - if err != nil { - logger.Error(err, "error getting vap object with respective groupVersion") - err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get VAP with runtime group version", status, err) - return reconcile.Result{}, err - } - vapName := fmt.Sprintf("gatekeeper-%s", unversionedCT.GetName()) - logger.Info("check if vap exists", "vapName", vapName) - if err := r.Get(ctx, types.NamespacedName{Name: vapName}, currentVap); err != nil { - if !apierrors.IsNotFound(err) && !errors.As(err, &discoveryErr) && !meta.IsNoMatchError(err) { - return reconcile.Result{}, err - } - currentVap = nil - } - logger.Info("get vap", "vapName", vapName, "currentVap", currentVap) - transformedVap, err := transform.TemplateToPolicyDefinition(unversionedCT) - if err != nil { - logger.Error(err, "transform to vap error", "vapName", vapName) - err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not transform to vap object", status, err) - return reconcile.Result{}, err - } - - newVap, err := getRunTimeVAP(groupVersion, transformedVap, currentVap) - if err != nil { - logger.Error(err, "getRunTimeVAP error", "vapName", vapName) - err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get runtime vap object", status, err) - return reconcile.Result{}, err - } - - if err := controllerutil.SetControllerReference(ct, newVap, r.scheme); err != nil { - return reconcile.Result{}, err - } - if currentVap == nil { - logger.Info("creating vap", "vapName", vapName) - if err := r.Create(ctx, newVap); err != nil { - logger.Info("creating vap error", "vapName", vapName, "error", err) - err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not create vap object", status, err) - return reconcile.Result{}, err - } - // after vap is created, trigger update event for all constraints - if err := r.triggerConstraintEvents(ctx, ct, status); err != nil { - return reconcile.Result{}, err - } - } else if !reflect.DeepEqual(currentVap, newVap) { - logger.Info("updating vap") - if err := r.Update(ctx, newVap); err != nil { - err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not update vap object", status, err) - return reconcile.Result{}, err - } - } - } - // do not generate vap resources - // remove if exists - if !generateVap && isVapAPIEnabled && groupVersion != nil { - currentVap, err := vapForVersion(groupVersion) - if err != nil { - logger.Error(err, "error getting vap object with respective groupVersion") - err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get VAP with correct group version", status, err) - return reconcile.Result{}, err - } - vapName := fmt.Sprintf("gatekeeper-%s", unversionedCT.GetName()) - logger.Info("check if vap exists", "vapName", vapName) - if err := r.Get(ctx, types.NamespacedName{Name: vapName}, currentVap); err != nil { - if !apierrors.IsNotFound(err) && !errors.As(err, &discoveryErr) && !meta.IsNoMatchError(err) { - return reconcile.Result{}, err - } - currentVap = nil - } - if currentVap != nil { - logger.Info("deleting vap") - if err := r.Delete(ctx, currentVap); err != nil { - err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not delete vap object", status, err) - return reconcile.Result{}, err - } - // after vap is deleted, trigger update event for all constraints - if err := r.triggerConstraintEvents(ctx, ct, status); err != nil { - return reconcile.Result{}, err - } - } + err = r.manageVAP(ctx, ct, unversionedCT, status, logger, generateVap) + if err != nil { + return reconcile.Result{}, err } if err := r.Update(ctx, status); err != nil { logger.Error(err, "update ct pod status error") @@ -848,3 +737,172 @@ 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 { + if !operations.IsAssigned(operations.Generate) { + return nil + } + var newCRD *apiextensionsv1.CustomResourceDefinition + if currentCRD == nil { + newCRD = proposedCRD.DeepCopy() + } else { + newCRD = currentCRD.DeepCopy() + newCRD.Spec = proposedCRD.Spec + } + + if err := controllerutil.SetControllerReference(ct, newCRD, r.scheme); err != nil { + return err + } + + if currentCRD == nil { + logger.Info("creating crd") + if err := r.Create(ctx, newCRD); err != nil { + err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not create CRD", status, err) + return err + } + } else if !reflect.DeepEqual(newCRD, currentCRD) { + logger.Info("updating crd") + if err := r.Update(ctx, newCRD); err != nil { + err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not update CRD", status, err) + return err + } + } + if !generateVAP { + return nil + } + + // 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) + if err != nil { + err = r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not annotate with timestamp to block VAPB generation", status, err) + } + return err +} + +func (r *ReconcileConstraintTemplate) manageVAP(ctx context.Context, ct *v1beta1.ConstraintTemplate, unversionedCT *templates.ConstraintTemplate, status *statusv1beta1.ConstraintTemplatePodStatus, logger logr.Logger, generateVap bool) error { + if !operations.IsAssigned(operations.Generate) { + logger.Info("generate operation is not assigned, ValidatingAdmissionPolicy resource will not be generated") + return nil + } + isVapAPIEnabled := false + var groupVersion *schema.GroupVersion + logger.Info("generateVap", "r.generateVap", generateVap) + + isVapAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&logger) + logger.Info("isVapAPIEnabled", "isVapAPIEnabled", isVapAPIEnabled) + logger.Info("groupVersion", "groupVersion", groupVersion) + + if generateVap && (!isVapAPIEnabled || groupVersion == nil) { + logger.Error(constraint.ErrValidatingAdmissionPolicyAPIDisabled, "ValidatingAdmissionPolicy resource cannot be generated for ConstraintTemplate", "name", ct.GetName()) + err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "ValidatingAdmissionPolicy resource cannot be generated for ConstraintTemplate", status, constraint.ErrValidatingAdmissionPolicyAPIDisabled) + return err + } + // generating VAP resources + if generateVap && isVapAPIEnabled && groupVersion != nil { + currentVap, err := vapForVersion(groupVersion) + if err != nil { + logger.Error(err, "error getting VAP object with respective groupVersion") + err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get VAP with runtime group version", status, err) + return err + } + vapName := fmt.Sprintf("gatekeeper-%s", unversionedCT.GetName()) + logger.Info("check if VAP exists", "vapName", vapName) + if err := r.Get(ctx, types.NamespacedName{Name: vapName}, currentVap); err != nil { + if !apierrors.IsNotFound(err) && !errors.As(err, &discoveryErr) && !meta.IsNoMatchError(err) { + err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get VAP object", status, err) + return err + } + currentVap = nil + } + logger.Info("get VAP", "vapName", vapName, "currentVap", currentVap) + transformedVap, err := transform.TemplateToPolicyDefinition(unversionedCT) + if err != nil { + logger.Error(err, "transform to VAP error", "vapName", vapName) + err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not transform to VAP object", status, err) + return err + } + + newVap, err := getRunTimeVAP(groupVersion, transformedVap, currentVap) + if err != nil { + logger.Error(err, "getRunTimeVAP error", "vapName", vapName) + err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get runtime VAP object", status, err) + return err + } + + if err := controllerutil.SetControllerReference(ct, newVap, r.scheme); err != nil { + return err + } + + if currentVap == nil { + logger.Info("creating VAP", "vapName", vapName) + if err := r.Create(ctx, newVap); err != nil { + logger.Info("creating VAP error", "vapName", vapName, "error", err) + err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not create VAP object", status, err) + return err + } + // after VAP is created, trigger update event for all constraints + if err := r.triggerConstraintEvents(ctx, ct, status); err != nil { + return err + } + } else if !reflect.DeepEqual(currentVap, newVap) { + logger.Info("updating VAP") + if err := r.Update(ctx, newVap); err != nil { + err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not update VAP object", status, err) + return err + } + } + } + // do not generate VAP resources + // remove if exists + if !generateVap && isVapAPIEnabled && groupVersion != nil { + currentVap, err := vapForVersion(groupVersion) + if err != nil { + logger.Error(err, "error getting VAP object with respective groupVersion") + err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get VAP with correct group version", status, err) + return err + } + vapName := fmt.Sprintf("gatekeeper-%s", unversionedCT.GetName()) + logger.Info("check if VAP exists", "vapName", vapName) + if err := r.Get(ctx, types.NamespacedName{Name: vapName}, currentVap); err != nil { + if !apierrors.IsNotFound(err) && !errors.As(err, &discoveryErr) && !meta.IsNoMatchError(err) { + return err + } + currentVap = nil + } + if currentVap != nil { + logger.Info("deleting VAP") + if err := r.Delete(ctx, currentVap); err != nil { + err := r.reportErrorOnCTStatus(ctx, ErrUpdateCode, "Could not delete VAP object", status, err) + return err + } + // after VAP is deleted, trigger update event for all constraints + if err := r.triggerConstraintEvents(ctx, ct, status); err != nil { + return err + } + } + } + return nil +} + +// 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 { + 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 + } + // 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 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) +} diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index 895043b253f..9010f61f640 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -21,6 +21,7 @@ import ( "fmt" "strings" "testing" + "time" templatesv1 "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1" "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1beta1" @@ -305,6 +306,20 @@ func TestReconcile(t *testing.T) { if err != nil { t.Fatal(err) } + + logger.Info("Running Test: block-vapb-generation-until annotation should not be present") + err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { + return true + }, func() error { + ct := &v1beta1.ConstraintTemplate{} + if err := c.Get(ctx, types.NamespacedName{Name: denyall + strings.ToLower(suffix)}, ct); err != nil { + return err + } + if _, ok := ct.GetAnnotations()[constraint.BlockVAPBGenerationUntilAnnotation]; ok { + return fmt.Errorf("unexpected %s annotations on CT", constraint.BlockVAPBGenerationUntilAnnotation) + } + return nil + }) }) t.Run("Vap should be created with v1beta1", func(t *testing.T) { @@ -361,7 +376,7 @@ func TestReconcile(t *testing.T) { t.Run("Vap should not be created for rego only template", func(t *testing.T) { suffix := "VapShouldNotBeCreatedForRegoOnlyTemplate" - logger.Info("Running test: Vap should not be created") + logger.Info("Running test: Vap should not be created for rego only template") constraintTemplate := makeReconcileConstraintTemplate(suffix) t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, expectedCRD(suffix))) testutils.CreateThenCleanup(ctx, t, c, constraintTemplate) @@ -388,7 +403,7 @@ func TestReconcile(t *testing.T) { t.Run("Vap should not be created for only rego engine", func(t *testing.T) { suffix := "VapShouldNotBeCreatedForOnlyRegoEngine" - logger.Info("Running test: Vap should not be created") + logger.Info("Running test: Vap should not be created for only rego engine") constraintTemplate := makeReconcileConstraintTemplateWithRegoEngine(suffix) t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, expectedCRD(suffix))) testutils.CreateThenCleanup(ctx, t, c, constraintTemplate) @@ -623,9 +638,9 @@ func TestReconcile(t *testing.T) { } }) - t.Run("VapBinding should be created with VAP enforcement Point", func(t *testing.T) { + t.Run("VapBinding should be created with VAP enforcement point after default wait", func(t *testing.T) { suffix := "VapBindingShouldBeCreatedWithVAPEnforcementPoint" - logger.Info("Running test: VapBinding should be created with VAP enforcement point") + logger.Info("Running test: VapBinding should be created with VAP enforcement point after default wait") constraint.DefaultGenerateVAPB = ptr.To[bool](false) constraintTemplate := makeReconcileConstraintTemplateForVap(suffix, ptr.To[bool](true)) cstr := newDenyAllCstrWithScopedEA(suffix, util.VAPEnforcementPoint) @@ -644,12 +659,36 @@ func TestReconcile(t *testing.T) { err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { return true }, func() error { + ct := &v1beta1.ConstraintTemplate{} + if err := c.Get(ctx, types.NamespacedName{Name: denyall + strings.ToLower(suffix)}, ct); err != nil { + return err + } + timestamp := ct.GetAnnotations()[constraint.BlockVAPBGenerationUntilAnnotation] + 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: vapBindingName}, vapBinding); err != nil { return err } - return nil + blockTime, err := time.Parse(time.RFC3339, timestamp) + if err != nil { + return err + } + vapBindingCreationTime := vapBinding.GetCreationTimestamp().Time + if vapBindingCreationTime.Before(blockTime) { + return fmt.Errorf("VAPBinding should be created after default wait") + } + + if err := c.Delete(ctx, cstr); err != nil { + return err + } + return c.Delete(ctx, vapBinding) }) if err != nil { t.Fatal(err) @@ -716,11 +755,28 @@ func TestReconcile(t *testing.T) { if err != nil { t.Fatal(err) } + + logger.Info("Running test: VAP ConstraintTemplate should have block-VAPB-generation-until annotation") + err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { + return true + }, func() error { + ct := &v1beta1.ConstraintTemplate{} + if err := c.Get(ctx, types.NamespacedName{Name: denyall + strings.ToLower(suffix)}, ct); err != nil { + return err + } + if ct.GetAnnotations()[constraint.BlockVAPBGenerationUntilAnnotation] == "" { + return fmt.Errorf("expected %s annotations on CT", constraint.BlockVAPBGenerationUntilAnnotation) + } + return nil + }) + if err != nil { + t.Fatal(err) + } }) - t.Run("VapBinding should be created with v1", func(t *testing.T) { + t.Run("VapBinding should be created with v1 with some delay after constraint CRD is available", func(t *testing.T) { suffix := "VapBindingShouldBeCreatedV1" - logger.Info("Running test: VapBinding should be created with v1") + logger.Info("Running test: VapBinding should be created with v1 with some delay after constraint CRD is available") constraint.DefaultGenerateVAPB = ptr.To[bool](true) transform.GroupVersion = &admissionregistrationv1.SchemeGroupVersion constraintTemplate := makeReconcileConstraintTemplateForVap(suffix, ptr.To[bool](true)) @@ -740,11 +796,27 @@ func TestReconcile(t *testing.T) { err = retry.OnError(testutils.ConstantRetry, func(_ error) bool { return true }, func() error { + ct := &v1beta1.ConstraintTemplate{} + if err := c.Get(ctx, types.NamespacedName{Name: denyall + strings.ToLower(suffix)}, ct); err != nil { + return err + } + timestamp := ct.GetAnnotations()[constraint.BlockVAPBGenerationUntilAnnotation] + if timestamp == "" { + return fmt.Errorf("expected %s annotations on CT", constraint.BlockVAPBGenerationUntilAnnotation) + } // check if vapbinding resource exists now vapBinding := &admissionregistrationv1.ValidatingAdmissionPolicyBinding{} if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil { return err } + blockTime, err := time.Parse(time.RFC3339, timestamp) + if err != nil { + return err + } + vapBindingCreationTime := vapBinding.GetCreationTimestamp().Time + if vapBindingCreationTime.Before(blockTime) { + return fmt.Errorf("VAPBinding should not be created before the timestamp") + } return nil }) if err != nil { @@ -911,7 +983,7 @@ func TestReconcile(t *testing.T) { } }) - t.Run("Revew request initiated from an enforcement point not supported by client should result in error", func(t *testing.T) { + t.Run("Review request initiated from an enforcement point not supported by client should result in error", func(t *testing.T) { suffix := "ReviewResultsInError" logger.Info("Running test: Review request initiated from an enforcement point not supported by client should result in error") diff --git a/pkg/controller/mutators/core/reconciler_test.go b/pkg/controller/mutators/core/reconciler_test.go index 1ba3a2822ec..bcebe7acdf2 100644 --- a/pkg/controller/mutators/core/reconciler_test.go +++ b/pkg/controller/mutators/core/reconciler_test.go @@ -402,7 +402,7 @@ func TestReconciler_Reconcile(t *testing.T) { }, Status: statusv1beta1.MutatorPodStatusStatus{ ID: "no-pod", - Operations: []string{"audit", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, + Operations: []string{"audit", "generate", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, Enforced: true, Errors: nil, }, @@ -434,7 +434,7 @@ func TestReconciler_Reconcile(t *testing.T) { }, Status: statusv1beta1.MutatorPodStatusStatus{ ID: "no-pod", - Operations: []string{"audit", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, + Operations: []string{"audit", "generate", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, Enforced: true, Errors: nil, }, @@ -494,7 +494,7 @@ func TestReconciler_Reconcile(t *testing.T) { }, Status: statusv1beta1.MutatorPodStatusStatus{ ID: "no-pod", - Operations: []string{"audit", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, + Operations: []string{"audit", "generate", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, Enforced: false, Errors: []statusv1beta1.MutatorError{{Message: newErrSome(1).Error()}}, }, @@ -535,7 +535,7 @@ func TestReconciler_Reconcile(t *testing.T) { }, Status: statusv1beta1.MutatorPodStatusStatus{ ID: "no-pod", - Operations: []string{"audit", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, + Operations: []string{"audit", "generate", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, Enforced: false, Errors: []statusv1beta1.MutatorError{ { @@ -634,7 +634,7 @@ func TestReconciler_Reconcile(t *testing.T) { }, Status: statusv1beta1.MutatorPodStatusStatus{ ID: "no-pod", - Operations: []string{"audit", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, + Operations: []string{"audit", "generate", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, Enforced: false, Errors: []statusv1beta1.MutatorError{ { @@ -685,7 +685,7 @@ func TestReconciler_Reconcile(t *testing.T) { }, Status: statusv1beta1.MutatorPodStatusStatus{ ID: "no-pod", - Operations: []string{"audit", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, + Operations: []string{"audit", "generate", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, Enforced: true, Errors: nil, }, @@ -940,7 +940,7 @@ func TestReconciler_Reconcile_DeletePodStatus(t *testing.T) { }, Status: statusv1beta1.MutatorPodStatusStatus{ ID: "no-pod", - Operations: []string{"audit", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, + Operations: []string{"audit", "generate", "mutation-controller", "mutation-status", "mutation-webhook", "status", "webhook"}, Enforced: true, }, } diff --git a/pkg/operations/operations.go b/pkg/operations/operations.go index 100f19bfcea..13088984540 100644 --- a/pkg/operations/operations.go +++ b/pkg/operations/operations.go @@ -21,6 +21,7 @@ const ( MutationWebhook = Operation("mutation-webhook") Status = Operation("status") Webhook = Operation("webhook") + Generate = Operation("generate") ) var ( @@ -28,6 +29,7 @@ var ( // a pod. It is NOT intended to be mutated. allOperations = []Operation{ Audit, + Generate, MutationController, MutationStatus, MutationWebhook, diff --git a/pkg/operations/operations_test.go b/pkg/operations/operations_test.go index 1865a6c630e..b9df2a7f1dc 100644 --- a/pkg/operations/operations_test.go +++ b/pkg/operations/operations_test.go @@ -15,7 +15,7 @@ func Test_Flags(t *testing.T) { }{ "default": { input: []string{}, - expected: map[Operation]bool{Audit: true, Webhook: true, Status: true, MutationStatus: true, MutationWebhook: true, MutationController: true}, + expected: map[Operation]bool{Audit: true, Webhook: true, Status: true, MutationStatus: true, MutationWebhook: true, MutationController: true, Generate: true}, }, "multiple": { input: []string{"-operation", "audit", "-operation", "webhook"},