From 8b5048ab8812481b7c4d6bbd329608f4f517d8a9 Mon Sep 17 00:00:00 2001 From: Rita Zhang Date: Wed, 14 Feb 2024 17:07:02 -0800 Subject: [PATCH] address review comments Signed-off-by: Rita Zhang --- .github/workflows/workflow.yaml | 6 +- .../constraint/constraint_controller.go | 87 ++++++++----------- .../constrainttemplate_controller.go | 54 ++++-------- .../constrainttemplate_controller_test.go | 16 ++-- test/bats/test.bats | 3 +- test/testutils/controller.go | 6 +- .../client/drivers/k8scel/schema/schema.go | 33 +++---- .../drivers/k8scel/transform/cel_snippets.go | 50 +++++------ .../k8scel/transform/make_vap_objects.go | 53 +++++------ 9 files changed, 137 insertions(+), 171 deletions(-) diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml index df56c449f00..853b1074147 100644 --- a/.github/workflows/workflow.yaml +++ b/.github/workflows/workflow.yaml @@ -138,7 +138,7 @@ jobs: build_test: name: "Build and Test" runs-on: ubuntu-22.04 - timeout-minutes: 15 + timeout-minutes: 20 strategy: matrix: KUBERNETES_VERSION: ["1.25.8", "1.26.3", "1.27.1", "1.28.0"] @@ -177,7 +177,7 @@ jobs: IMG=gatekeeper-e2e:latest \ USE_LOCAL_IMG=true - make test-e2e + make test-e2e KUBERNETES_VERSION=${{ matrix.KUBERNETES_VERSION }} - name: Save logs if: ${{ always() }} @@ -263,7 +263,7 @@ jobs: build_test_generator_expansion: name: "[Generator Resource Expansion] Build and Test" runs-on: ubuntu-22.04 - timeout-minutes: 15 + timeout-minutes: 20 steps: - name: Harden Runner diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 38f805f4787..d9df7b335c2 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "reflect" "strings" "sync" @@ -26,7 +27,6 @@ import ( v1beta1 "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/client/drivers/k8scel/transform" - "github.com/open-policy-agent/frameworks/constraint/pkg/core/constraints" constraintstatusv1beta1 "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process" "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/constraintstatus" @@ -36,7 +36,7 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" - admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" + admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -59,6 +59,8 @@ import ( var log = logf.Log.WithName("controller").WithValues(logging.Process, "constraint_controller") +var vapMux sync.RWMutex + var VapAPIEnabled *bool var VapEnforcement VapFlagType @@ -85,6 +87,15 @@ func (v *VapFlagType) Set(value string) error { return fmt.Errorf("invalid value %s. Allowed values are %s, %s, %s", value, VapFlagNone, VapFlagGatekeeperDefault, VapFlagVapDefault) } +// setting defaults when not set; required for unit test +func (v *VapFlagType) SetDefaultIfEmpty() { + if *v == "" { + *v = VapFlagType(VapFlagGatekeeperDefault) + VapAPIEnabled = new(bool) + *VapAPIEnabled = true + } +} + type Adder struct { CFClient *constraintclient.Client ConstraintsCache *ConstraintsCache @@ -145,9 +156,8 @@ type ConstraintsCache struct { } type tags struct { - enforcementAction util.EnforcementAction - status metrics.Status - generateVapBinding bool + enforcementAction util.EnforcementAction + status metrics.Status } // newReconciler returns a new reconcile.Reconciler. @@ -230,8 +240,7 @@ type ReconcileConstraint struct { // that would otherwise trigger a watch. The bool returns whether // the function was executed, which can be used to determine // whether the reconciler should infer the object has been deleted - ifWatching func(schema.GroupVersionKind, func() error) (bool, error) - generateVapBinding bool + ifWatching func(schema.GroupVersionKind, func() error) (bool, error) } // +kubebuilder:rbac:groups=constraints.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete @@ -263,6 +272,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R return reconcile.Result{}, nil } + generateVapBinding := false deleted := false instance := &unstructured.Unstructured{} instance.SetGroupVersionKind(gvk) @@ -294,7 +304,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R } // unless constraint vap label is false, default to parent if useVap == "no" { - r.generateVapBinding = false + generateVapBinding = false } else { log.Info("constraint resource use-vap label is not no; will default to parent constraint template label") parentCTUseVap, err := r.getCTVapLabel(ctx, instance.GetKind()) @@ -303,8 +313,8 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R return reconcile.Result{}, err } log.Info("constraint resource", "parentCTUseVap", parentCTUseVap) - r.generateVapBinding = ShouldGenerateVap(parentCTUseVap) - log.Info("constraint resource", "generateVapBinding", r.generateVapBinding) + generateVapBinding = ShouldGenerateVap(parentCTUseVap) + log.Info("constraint resource", "generateVapBinding", generateVapBinding) } constraintKey := strings.Join([]string{instance.GetKind(), instance.GetName()}, "/") enforcementAction, err := util.GetEnforcementAction(instance.Object) @@ -329,23 +339,20 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R status.Status.ConstraintUID = instance.GetUID() status.Status.ObservedGeneration = instance.GetGeneration() status.Status.Errors = nil - cachedTags := r.constraintsCache.getTagsByConstraintKey(constraintKey) - cachedGenerateVapBinding := cachedTags.generateVapBinding - log.Info("constraint", "cachedGenerateVapBinding", cachedGenerateVapBinding) - if c, err := r.cfClient.GetConstraint(instance); err != nil || !constraints.SemanticEqual(instance, c) || r.generateVapBinding != cachedGenerateVapBinding { + if c, err := r.cfClient.GetConstraint(instance); err != nil || !reflect.DeepEqual(instance, c) { // generate vapbinding resources - if r.generateVapBinding && IsVapAPIEnabled() { - currentVapBinding := &admissionregistrationv1alpha1.ValidatingAdmissionPolicyBinding{} + if generateVapBinding && IsVapAPIEnabled() { + currentVapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} 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) { + if !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "failed to get API group resources") { return reconcile.Result{}, err } currentVapBinding = nil } - newVapBinding := &admissionregistrationv1alpha1.ValidatingAdmissionPolicyBinding{} + newVapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} transformedVapBinding, err := transform.ConstraintToBinding(instance) if err != nil { status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: err.Error()}) @@ -375,18 +382,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R return reconcile.Result{}, err } } else { - uc, err := runtime.DefaultUnstructuredConverter.ToUnstructured(currentVapBinding) - if err != nil { - log.Error(err, "error converting to unstructured") - return reconcile.Result{}, err - } - un, err := runtime.DefaultUnstructuredConverter.ToUnstructured(newVapBinding) - if err != nil { - log.Error(err, "error converting to unstructured") - return reconcile.Result{}, err - } - - if !constraints.SemanticEqual(&unstructured.Unstructured{Object: un}, &unstructured.Unstructured{Object: uc}) { + if !reflect.DeepEqual(currentVapBinding, newVapBinding) { log.Info("updating vapbinding") if err := r.writer.Update(ctx, newVapBinding); err != nil { status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: err.Error()}) @@ -400,12 +396,12 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R } // do not generate vapbinding resources // remove if exists - if !r.generateVapBinding && r.generateVapBinding != cachedGenerateVapBinding && IsVapAPIEnabled() { - currentVapBinding := &admissionregistrationv1alpha1.ValidatingAdmissionPolicyBinding{} + if !generateVapBinding && IsVapAPIEnabled() { + currentVapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} 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) { + if !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "failed to get API group resources") { return reconcile.Result{}, err } currentVapBinding = nil @@ -423,9 +419,8 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R } if err := r.cacheConstraint(ctx, instance); err != nil { r.constraintsCache.addConstraintKey(constraintKey, tags{ - enforcementAction: enforcementAction, - status: metrics.ErrorStatus, - generateVapBinding: r.generateVapBinding, + enforcementAction: enforcementAction, + status: metrics.ErrorStatus, }) status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: err.Error()}) if err2 := r.writer.Update(ctx, status); err2 != nil { @@ -444,9 +439,8 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R // adding constraint to cache and sending metrics r.constraintsCache.addConstraintKey(constraintKey, tags{ - enforcementAction: enforcementAction, - status: metrics.ActiveStatus, - generateVapBinding: r.generateVapBinding, + enforcementAction: enforcementAction, + status: metrics.ActiveStatus, }) reportMetrics = true } else { @@ -586,9 +580,8 @@ func (c *ConstraintsCache) addConstraintKey(constraintKey string, t tags) { defer c.mux.Unlock() c.cache[constraintKey] = tags{ - enforcementAction: t.enforcementAction, - status: t.status, - generateVapBinding: t.generateVapBinding, + enforcementAction: t.enforcementAction, + status: t.status, } } @@ -599,13 +592,6 @@ func (c *ConstraintsCache) deleteConstraintKey(constraintKey string) { delete(c.cache, constraintKey) } -func (c *ConstraintsCache) getTagsByConstraintKey(constraintKey string) tags { - c.mux.Lock() - defer c.mux.Unlock() - - return c.cache[constraintKey] -} - func (c *ConstraintsCache) reportTotalConstraints(ctx context.Context, reporter StatsReporter) { c.mux.RLock() defer c.mux.RUnlock() @@ -640,6 +626,9 @@ func ShouldGenerateVap(useVapLabel string) bool { } func IsVapAPIEnabled() bool { + vapMux.Lock() + defer vapMux.Unlock() + if VapAPIEnabled != nil { return *VapAPIEnabled } diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index 7de19061fbc..c6c210184cb 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -19,12 +19,12 @@ import ( "context" "fmt" "reflect" + "strings" "time" "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/client/drivers/k8scel/transform" - "github.com/open-policy-agent/frameworks/constraint/pkg/core/constraints" "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" statusv1beta1 "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/constraint" @@ -37,7 +37,7 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/util" "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" errorpkg "github.com/pkg/errors" - admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" + admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -116,12 +116,7 @@ func (a *Adder) InjectGetPod(getPod func(context.Context) (*corev1.Pod, error)) // regEvents is the channel registered by Registrar to put the events in // cstrEvents and regEvents point to same event channel except for testing. func newReconciler(mgr manager.Manager, cfClient *constraintclient.Client, wm *watch.Manager, cs *watch.ControllerSwitch, tracker *readiness.Tracker, cstrEvents <-chan event.GenericEvent, regEvents chan<- event.GenericEvent, getPod func(context.Context) (*corev1.Pod, error)) (*ReconcileConstraintTemplate, error) { - // setting defaults when not set already; required for unit test - if constraint.VapEnforcement == "" { - constraint.VapEnforcement = constraint.VapFlagGatekeeperDefault - constraint.VapAPIEnabled = new(bool) - *constraint.VapAPIEnabled = true - } + constraint.VapEnforcement.SetDefaultIfEmpty() // constraintsCache contains total number of constraints and shared mutex and vap label constraintsCache := constraint.NewConstraintsCache() @@ -249,7 +244,6 @@ type ReconcileConstraintTemplate struct { metrics *reporter tracker *readiness.Tracker getPod func(context.Context) (*corev1.Pod, error) - generateVap bool cstrEvents chan event.GenericEvent } @@ -286,20 +280,21 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec } deleted = true } + generateVap := false labels := ct.GetLabels() logger.Info("constraint template resource", "labels", labels) useVap, ok := labels[constraint.VapGenerationLabel] if !ok { logger.Info("constraint template resource does not have a label for use-vap; will default to flag behavior", "VapEnforcement", constraint.VapEnforcement) - r.generateVap = constraint.ShouldGenerateVap("") + generateVap = constraint.ShouldGenerateVap("") } else { logger.Info("constraint template resource", "useVap", useVap) - r.generateVap = constraint.ShouldGenerateVap(useVap) + generateVap = constraint.ShouldGenerateVap(useVap) if useVap != "no" && useVap != "yes" { logger.Error(fmt.Errorf("constraint template resource has an invalid value for %s, allowed values are yes and no", constraint.VapGenerationLabel), "constraint template resource has an invalid label value") } } - logger.Info("generateVap", "r.generateVap", r.generateVap) + logger.Info("generateVap", "r.generateVap", generateVap) deleted = deleted || !ct.GetDeletionTimestamp().IsZero() @@ -395,7 +390,7 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec return reconcile.Result{}, err } - result, err := r.handleUpdate(ctx, ct, unversionedCT, proposedCRD, currentCRD, status) + result, err := r.handleUpdate(ctx, ct, unversionedCT, proposedCRD, currentCRD, status, generateVap) if err != nil { logger.Error(err, "handle update error") logError(request.NamespacedName.Name) @@ -428,6 +423,7 @@ 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) @@ -487,20 +483,18 @@ func (r *ReconcileConstraintTemplate) handleUpdate( return reconcile.Result{}, err } // generating vap resources - if r.generateVap && constraint.IsVapAPIEnabled() { - currentVap := &admissionregistrationv1alpha1.ValidatingAdmissionPolicy{} + if generateVap && constraint.IsVapAPIEnabled() { + currentVap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{} 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 { - logger.Info("get vap error", "vapName", vapName, "error", err) - if !errors.IsNotFound(err) { return reconcile.Result{}, err } currentVap = nil } logger.Info("get vap", "vapName", vapName, "currentVap", currentVap) - newVap := &admissionregistrationv1alpha1.ValidatingAdmissionPolicy{} + newVap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{} transformedVap, err := transform.TemplateToPolicyDefinition(unversionedCT) if err != nil { logger.Info("transform to vap error", "vapName", vapName, "error", err) @@ -521,11 +515,6 @@ func (r *ReconcileConstraintTemplate) handleUpdate( } if currentVap == nil { - // wait for paramKindCRD to exist first - paramKindCRD := &apiextensionsv1.CustomResourceDefinition{} - if err := r.Get(ctx, types.NamespacedName{Name: name}, paramKindCRD); err != nil { - return reconcile.Result{}, err - } logger.Info("creating vap", "vapName", vapName) if err := r.Create(ctx, newVap); err != nil { logger.Info("creating vap error", "vapName", vapName, "error", err) @@ -535,18 +524,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( return reconcile.Result{}, err } } else { - uc, err := runtime.DefaultUnstructuredConverter.ToUnstructured(currentVap) - if err != nil { - logger.Error(err, "error converting to unstructured") - return reconcile.Result{}, err - } - un, err := runtime.DefaultUnstructuredConverter.ToUnstructured(newVap) - if err != nil { - logger.Error(err, "error converting to unstructured") - return reconcile.Result{}, err - } - - if !constraints.SemanticEqual(&unstructured.Unstructured{Object: un}, &unstructured.Unstructured{Object: uc}) { + if !reflect.DeepEqual(currentVap, newVap) { logger.Info("updating vap") if err := r.Update(ctx, newVap); err != nil { updateErr := &v1beta1.CreateCRDError{Code: ErrUpdateCode, Message: err.Error()} @@ -577,12 +555,12 @@ func (r *ReconcileConstraintTemplate) handleUpdate( } // do not generate vap resources // remove if exists - if !r.generateVap && constraint.IsVapAPIEnabled() { - currentVap := &admissionregistrationv1alpha1.ValidatingAdmissionPolicy{} + if !generateVap && constraint.IsVapAPIEnabled() { + currentVap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{} 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 !errors.IsNotFound(err) { + if !errors.IsNotFound(err) && !strings.Contains(err.Error(), "failed to get API group resources") { return reconcile.Result{}, err } currentVap = nil diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index c28715e5fc8..501131642ab 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -39,7 +39,7 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/test/testutils" "golang.org/x/net/context" admissionv1 "k8s.io/api/admission/v1" - admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" + admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" corev1 "k8s.io/api/core/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -272,7 +272,7 @@ func TestReconcile(t *testing.T) { return true }, func() error { // check if vap resource exists now - vap := &admissionregistrationv1alpha1.ValidatingAdmissionPolicy{} + vap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{} vapName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) if err := c.Get(ctx, types.NamespacedName{Name: vapName}, vap); err != nil { return err @@ -299,7 +299,7 @@ func TestReconcile(t *testing.T) { return true }, func() error { // check if vap resource exists now - vap := &admissionregistrationv1alpha1.ValidatingAdmissionPolicy{} + vap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{} vapName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) if err := c.Get(ctx, types.NamespacedName{Name: vapName}, vap); err != nil { if !apierrors.IsNotFound(err) { @@ -327,7 +327,7 @@ func TestReconcile(t *testing.T) { return true }, func() error { // check if vap resource exists now - vap := &admissionregistrationv1alpha1.ValidatingAdmissionPolicy{} + vap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{} vapName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) if err := c.Get(ctx, types.NamespacedName{Name: vapName}, vap); err != nil { if !apierrors.IsNotFound(err) { @@ -341,7 +341,7 @@ func TestReconcile(t *testing.T) { t.Fatal(err) } }) - /// TODO(ritazh): uncomment this test after the fix for https://github.com/kubernetes/kubernetes/issues/122658 makes its way to a k8s release + // TODO(ritazh): uncomment this test after the fix for https://github.com/kubernetes/kubernetes/issues/122658 makes its way to a k8s release // t.Run("VapBinding should be created", func(t *testing.T) { // suffix := "VapBindingShouldBeCreated" @@ -349,7 +349,7 @@ func TestReconcile(t *testing.T) { // labels := map[string]string{ // "gatekeeper.sh/use-vap": "yes", // } - // constraintTemplate := makeReconcileConstraintTemplateWithLabel(suffix, labels) + // constraintTemplate := makeReconcileConstraintTemplateForVap(suffix, labels) // cstr := newDenyAllCstrWithLabel(suffix, labels) // t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, cstr)) @@ -365,7 +365,7 @@ func TestReconcile(t *testing.T) { // return err // } // // check if vap resource exists now - // vap := &admissionregistrationv1alpha1.ValidatingAdmissionPolicy{} + // vap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{} // vapName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) // if err := c.Get(ctx, types.NamespacedName{Name: vapName}, vap); err != nil { // return err @@ -374,7 +374,7 @@ func TestReconcile(t *testing.T) { // return err // } // // check if vapbinding resource exists now - // vapBinding := &admissionregistrationv1alpha1.ValidatingAdmissionPolicyBinding{} + // vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{} // vapBindingName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix)) // if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil { // return err diff --git a/test/bats/test.bats b/test/bats/test.bats index 773d032b827..df8e4277957 100644 --- a/test/bats/test.bats +++ b/test/bats/test.bats @@ -71,7 +71,8 @@ teardown_file() { #kubectl delete --ignore-not-found -f ${BATS_TESTS_DIR}/bad/bad_ns.yaml @test "vap test" { - if [ -z $ENABLE_VAP_TESTS ]; then + minor_version=$(echo "$KUBERNETES_VERSION" | cut -d'.' -f2) + if [ "$minor_version" -lt 28 ]; then skip "skipping vap tests" fi local api="$(kubectl api-resources | grep validatingadmission)" diff --git a/test/testutils/controller.go b/test/testutils/controller.go index f243a8af9f5..b3fff214e0f 100644 --- a/test/testutils/controller.go +++ b/test/testutils/controller.go @@ -16,7 +16,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/apis" "github.com/open-policy-agent/gatekeeper/v3/pkg/fakes" "github.com/open-policy-agent/gatekeeper/v3/pkg/target" - admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -145,10 +144,7 @@ func StartControlPlane(m *testing.M, cfg **rest.Config, testerDepth int) { if err := apis.AddToScheme(scheme.Scheme); err != nil { log.Fatal(err) } - - if err := admissionregistrationv1alpha1.AddToScheme(scheme.Scheme); err != nil { - log.Fatal(err) - } + if err := admissionregistrationv1beta1.AddToScheme(scheme.Scheme); err != nil { log.Fatal(err) } diff --git a/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/schema/schema.go b/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/schema/schema.go index 63f9c2b3a8c..f67f65e0997 100644 --- a/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/schema/schema.go +++ b/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/schema/schema.go @@ -7,7 +7,7 @@ import ( "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" admissionv1 "k8s.io/api/admissionregistration/v1" - admissionv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" + admissionv1beta1 "k8s.io/api/admissionregistration/v1beta1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/admission/plugin/cel" "k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy" @@ -99,14 +99,14 @@ func (in *Source) GetMatchConditions() ([]cel.ExpressionAccessor, error) { return matchConditions, nil } -func (in *Source) GetV1Alpha1MatchConditions() ([]admissionv1alpha1.MatchCondition, error) { +func (in *Source) GetV1Beta1MatchConditions() ([]admissionv1beta1.MatchCondition, error) { if err := in.validateMatchConditions(); err != nil { return nil, err } - var matchConditions []admissionv1alpha1.MatchCondition + var matchConditions []admissionv1beta1.MatchCondition for _, mc := range in.MatchConditions { - matchConditions = append(matchConditions, admissionv1alpha1.MatchCondition{ + matchConditions = append(matchConditions, admissionv1beta1.MatchCondition{ Name: mc.Name, Expression: mc.Expression, }) @@ -142,14 +142,14 @@ func (in *Source) GetVariables() ([]cel.NamedExpressionAccessor, error) { return vars, nil } -func (in *Source) GetV1Alpha1Variables() ([]admissionv1alpha1.Variable, error) { +func (in *Source) GetV1Beta1Variables() ([]admissionv1beta1.Variable, error) { if err := in.validateVariables(); err != nil { return nil, err } - var variables []admissionv1alpha1.Variable + var variables []admissionv1beta1.Variable for _, v := range in.Variables { - variables = append(variables, admissionv1alpha1.Variable{ + variables = append(variables, admissionv1beta1.Variable{ Name: v.Name, Expression: v.Expression, }) @@ -169,10 +169,10 @@ func (in *Source) GetValidations() ([]cel.ExpressionAccessor, error) { return validations, nil } -func (in *Source) GetV1Alpha1Validatons() ([]admissionv1alpha1.Validation, error) { - var validations []admissionv1alpha1.Validation +func (in *Source) GetV1Beta1Validatons() ([]admissionv1beta1.Validation, error) { + var validations []admissionv1beta1.Validation for _, v := range in.Validations { - validations = append(validations, admissionv1alpha1.Validation{ + validations = append(validations, admissionv1beta1.Validation{ Expression: v.Expression, Message: v.Message, MessageExpression: v.MessageExpression, @@ -213,18 +213,19 @@ func (in *Source) GetFailurePolicy() (*admissionv1.FailurePolicyType, error) { return &out, nil } -func (in *Source) GetV1alpha1FailurePolicy() (*admissionv1alpha1.FailurePolicyType, error) { +func (in *Source) GetV1Beta1FailurePolicy() (*admissionv1beta1.FailurePolicyType, error) { + var out admissionv1beta1.FailurePolicyType + /// TODO(ritazh): default for now until the feature is safe to fail close if in.FailurePolicy == nil { - return nil, nil + out = admissionv1beta1.Ignore + return &out, nil } - var out admissionv1alpha1.FailurePolicyType - switch *in.FailurePolicy { case string(admissionv1.Fail): - out = admissionv1alpha1.Fail + out = admissionv1beta1.Fail case string(admissionv1.Ignore): - out = admissionv1alpha1.Ignore + out = admissionv1beta1.Ignore default: return nil, fmt.Errorf("%w: unrecognized failure policy: %s", ErrBadFailurePolicy, *in.FailurePolicy) } diff --git a/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform/cel_snippets.go b/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform/cel_snippets.go index d61e6bbd8dd..f6d0ed0c108 100644 --- a/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform/cel_snippets.go +++ b/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform/cel_snippets.go @@ -2,7 +2,7 @@ package transform import ( "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/schema" - admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" + admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" "k8s.io/apiserver/pkg/admission/plugin/cel" "k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy" "k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions" @@ -84,15 +84,15 @@ const ( ` ) -func MatchExcludedNamespacesGlobV1Alpha1() admissionregistrationv1alpha1.MatchCondition { - return admissionregistrationv1alpha1.MatchCondition{ +func MatchExcludedNamespacesGlobV1Beta1() admissionregistrationv1beta1.MatchCondition { + return admissionregistrationv1beta1.MatchCondition{ Name: "gatekeeper_internal_match_excluded_namespaces", Expression: matchExcludedNamespacesGlob, } } func MatchExcludedNamespacesGlobCEL() []cel.ExpressionAccessor { - mc := MatchExcludedNamespacesGlobV1Alpha1() + mc := MatchExcludedNamespacesGlobV1Beta1() return []cel.ExpressionAccessor{ &matchconditions.MatchCondition{ Name: mc.Name, @@ -101,15 +101,15 @@ func MatchExcludedNamespacesGlobCEL() []cel.ExpressionAccessor { } } -func MatchNamespacesGlobV1Alpha1() admissionregistrationv1alpha1.MatchCondition { - return admissionregistrationv1alpha1.MatchCondition{ +func MatchNamespacesGlobV1Beta1() admissionregistrationv1beta1.MatchCondition { + return admissionregistrationv1beta1.MatchCondition{ Name: "gatekeeper_internal_match_namespaces", Expression: matchNamespacesGlob, } } func MatchNamespacesGlobCEL() []cel.ExpressionAccessor { - mc := MatchNamespacesGlobV1Alpha1() + mc := MatchNamespacesGlobV1Beta1() return []cel.ExpressionAccessor{ &matchconditions.MatchCondition{ Name: mc.Name, @@ -118,15 +118,15 @@ func MatchNamespacesGlobCEL() []cel.ExpressionAccessor { } } -func MatchNameGlobV1Alpha1() admissionregistrationv1alpha1.MatchCondition { - return admissionregistrationv1alpha1.MatchCondition{ +func MatchNameGlobV1Beta1() admissionregistrationv1beta1.MatchCondition { + return admissionregistrationv1beta1.MatchCondition{ Name: "gatekeeper_internal_match_name", Expression: matchNameGlob, } } func MatchNameGlobCEL() []cel.ExpressionAccessor { - mc := MatchNameGlobV1Alpha1() + mc := MatchNameGlobV1Beta1() return []cel.ExpressionAccessor{ &matchconditions.MatchCondition{ Name: mc.Name, @@ -135,15 +135,15 @@ func MatchNameGlobCEL() []cel.ExpressionAccessor { } } -func MatchKindsV1Alpha1() admissionregistrationv1alpha1.MatchCondition { - return admissionregistrationv1alpha1.MatchCondition{ +func MatchKindsV1Beta1() admissionregistrationv1beta1.MatchCondition { + return admissionregistrationv1beta1.MatchCondition{ Name: "gatekeeper_internal_match_kinds", Expression: matchKinds, } } func MatchKindsCEL() []cel.ExpressionAccessor { - mc := MatchKindsV1Alpha1() + mc := MatchKindsV1Beta1() return []cel.ExpressionAccessor{ &matchconditions.MatchCondition{ Name: mc.Name, @@ -152,15 +152,15 @@ func MatchKindsCEL() []cel.ExpressionAccessor { } } -func BindParamsV1Alpha1() admissionregistrationv1alpha1.Variable { - return admissionregistrationv1alpha1.Variable{ +func BindParamsV1Beta1() admissionregistrationv1beta1.Variable { + return admissionregistrationv1beta1.Variable{ Name: schema.ParamsName, Expression: "!has(params.spec) ? null : !has(params.spec.parameters) ? null: params.spec.parameters", } } func BindParamsCEL() []cel.NamedExpressionAccessor { - v := BindParamsV1Alpha1() + v := BindParamsV1Beta1() return []cel.NamedExpressionAccessor{ &validatingadmissionpolicy.Variable{ Name: v.Name, @@ -169,12 +169,12 @@ func BindParamsCEL() []cel.NamedExpressionAccessor { } } -func AllMatchersV1Alpha1() []admissionregistrationv1alpha1.MatchCondition { - return []admissionregistrationv1alpha1.MatchCondition{ - MatchExcludedNamespacesGlobV1Alpha1(), - MatchNamespacesGlobV1Alpha1(), - MatchNameGlobV1Alpha1(), - MatchKindsV1Alpha1(), +func AllMatchersV1Beta1() []admissionregistrationv1beta1.MatchCondition { + return []admissionregistrationv1beta1.MatchCondition{ + MatchExcludedNamespacesGlobV1Beta1(), + MatchNamespacesGlobV1Beta1(), + MatchNameGlobV1Beta1(), + MatchKindsV1Beta1(), } } @@ -182,8 +182,8 @@ func AllVariablesCEL() []cel.NamedExpressionAccessor { return BindParamsCEL() } -func AllVariablesV1Alpha1() []admissionregistrationv1alpha1.Variable { - return []admissionregistrationv1alpha1.Variable{ - BindParamsV1Alpha1(), +func AllVariablesV1Beta1() []admissionregistrationv1beta1.Variable { + return []admissionregistrationv1beta1.Variable{ + BindParamsV1Beta1(), } } diff --git a/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go b/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go index d380f44effd..002f2091709 100644 --- a/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go +++ b/vendor/github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go @@ -8,56 +8,57 @@ import ( templatesv1beta1 "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1beta1" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/schema" "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" - admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" + admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" ) -func TemplateToPolicyDefinition(template *templates.ConstraintTemplate) (*admissionregistrationv1alpha1.ValidatingAdmissionPolicy, error) { +func TemplateToPolicyDefinition(template *templates.ConstraintTemplate) (*admissionregistrationv1beta1.ValidatingAdmissionPolicy, error) { source, err := schema.GetSourceFromTemplate(template) if err != nil { return nil, err } - matchConditions, err := source.GetV1Alpha1MatchConditions() + matchConditions, err := source.GetV1Beta1MatchConditions() if err != nil { return nil, err } - matchConditions = append(matchConditions, AllMatchersV1Alpha1()...) + matchConditions = append(matchConditions, AllMatchersV1Beta1()...) - validations, err := source.GetV1Alpha1Validatons() + validations, err := source.GetV1Beta1Validatons() if err != nil { return nil, err } - variables, err := source.GetV1Alpha1Variables() + variables, err := source.GetV1Beta1Variables() if err != nil { return nil, err } - variables = append(variables, AllVariablesV1Alpha1()...) + variables = append(variables, AllVariablesV1Beta1()...) - failurePolicy, err := source.GetV1alpha1FailurePolicy() + failurePolicy, err := source.GetV1Beta1FailurePolicy() if err != nil { return nil, err } - policy := &admissionregistrationv1alpha1.ValidatingAdmissionPolicy{ + policy := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("gatekeeper-%s", template.GetName()), }, - Spec: admissionregistrationv1alpha1.ValidatingAdmissionPolicySpec{ - ParamKind: &admissionregistrationv1alpha1.ParamKind{ + Spec: admissionregistrationv1beta1.ValidatingAdmissionPolicySpec{ + ParamKind: &admissionregistrationv1beta1.ParamKind{ APIVersion: fmt.Sprintf("%s/%s", apiconstraints.Group, templatesv1beta1.SchemeGroupVersion.Version), Kind: template.Spec.CRD.Spec.Names.Kind, }, - MatchConstraints: &admissionregistrationv1alpha1.MatchResources{ - ResourceRules: []admissionregistrationv1alpha1.NamedRuleWithOperations{ + MatchConstraints: &admissionregistrationv1beta1.MatchResources{ + ResourceRules: []admissionregistrationv1beta1.NamedRuleWithOperations{ { - RuleWithOperations: admissionregistrationv1alpha1.RuleWithOperations{ - Operations: []admissionregistrationv1alpha1.OperationType{admissionregistrationv1alpha1.OperationAll}, - Rule: admissionregistrationv1alpha1.Rule{APIGroups: []string{"*"}, APIVersions: []string{"*"}, Resources: []string{"*"}}, + RuleWithOperations: admissionregistrationv1beta1.RuleWithOperations{ + /// TODO(ritazh): default for now until we can safely expose these to users + Operations: []admissionregistrationv1beta1.OperationType{admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update}, + Rule: admissionregistrationv1beta1.Rule{APIGroups: []string{"*"}, APIVersions: []string{"*"}, Resources: []string{"*"}}, }, }, }, @@ -72,34 +73,34 @@ func TemplateToPolicyDefinition(template *templates.ConstraintTemplate) (*admiss return policy, nil } -func ConstraintToBinding(constraint *unstructured.Unstructured) (*admissionregistrationv1alpha1.ValidatingAdmissionPolicyBinding, error) { +func ConstraintToBinding(constraint *unstructured.Unstructured) (*admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding, error) { enforcementActionStr, err := apiconstraints.GetEnforcementAction(constraint) if err != nil { return nil, err } - var enforcementAction admissionregistrationv1alpha1.ValidationAction + var enforcementAction admissionregistrationv1beta1.ValidationAction switch enforcementActionStr { case apiconstraints.EnforcementActionDeny: - enforcementAction = admissionregistrationv1alpha1.Deny + enforcementAction = admissionregistrationv1beta1.Deny case "warn": - enforcementAction = admissionregistrationv1alpha1.Warn + enforcementAction = admissionregistrationv1beta1.Warn default: return nil, fmt.Errorf("%w: unrecognized enforcement action %s, must be `warn` or `deny`", ErrBadEnforcementAction, enforcementActionStr) } - binding := &admissionregistrationv1alpha1.ValidatingAdmissionPolicyBinding{ + binding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("gatekeeper-%s", constraint.GetName()), }, - Spec: admissionregistrationv1alpha1.ValidatingAdmissionPolicyBindingSpec{ + Spec: admissionregistrationv1beta1.ValidatingAdmissionPolicyBindingSpec{ PolicyName: fmt.Sprintf("gatekeeper-%s", strings.ToLower(constraint.GetKind())), - ParamRef: &admissionregistrationv1alpha1.ParamRef{ + ParamRef: &admissionregistrationv1beta1.ParamRef{ Name: constraint.GetName(), - ParameterNotFoundAction: ptr.To[admissionregistrationv1alpha1.ParameterNotFoundActionType](admissionregistrationv1alpha1.AllowAction), + ParameterNotFoundAction: ptr.To[admissionregistrationv1beta1.ParameterNotFoundActionType](admissionregistrationv1beta1.AllowAction), }, - MatchResources: &admissionregistrationv1alpha1.MatchResources{}, - ValidationActions: []admissionregistrationv1alpha1.ValidationAction{enforcementAction}, + MatchResources: &admissionregistrationv1beta1.MatchResources{}, + ValidationActions: []admissionregistrationv1beta1.ValidationAction{enforcementAction}, }, } objectSelectorMap, found, err := unstructured.NestedMap(constraint.Object, "spec", "match", "labelSelector")