chore: updating VAP default failure policy to fail (#3702)

Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
open-policy-agent · Nov 22, 2024 · 754675b · 754675b
1 parent 26bfc0f
commit 754675b
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 13 deletions.
diff --git a/.github/workflows/test-gator.yaml b/.github/workflows/test-gator.yaml
@@ -33,7 +33,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        KUBERNETES_VERSION: ["1.27.13", "1.28.9", "1.29.4", "1.30.0"]
+        KUBERNETES_VERSION: ["1.29.10", "1.30.6", "1.31.2"]
     steps:
       - name: Harden Runner
         uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1

diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml
@@ -33,7 +33,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        KUBERNETES_VERSION: ["1.27.13", "1.28.9", "1.29.4", "1.30.0"]
+        KUBERNETES_VERSION: ["1.29.10", "1.30.6", "1.31.2"]
     steps:
       - name: Harden Runner
         uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1

diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go
@@ -364,6 +364,20 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R
 				return reconcile.Result{}, err
 			}
 		}
+		isAPIEnabled, groupVersion := transform.IsVapAPIEnabled(&log)
+		if isAPIEnabled {
+			currentVapBinding, err := vapBindingForVersion(*groupVersion)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
+			vapBindingName := getVAPBindingName(instance.GetName())
+			currentVapBinding.SetName(vapBindingName)
+			if err := r.writer.Delete(ctx, currentVapBinding); err != nil {
+				if !apierrors.IsNotFound(err) {
+					return reconcile.Result{}, err
+				}
+			}
+		}
 	}
 	return reconcile.Result{}, nil
 }
@@ -553,7 +567,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 		if err != nil {
 			return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version")
 		}
-		vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName())
+		vapBindingName := getVAPBindingName(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) {
@@ -595,7 +609,7 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
 		if err != nil {
 			return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version")
 		}
-		vapBindingName := fmt.Sprintf("gatekeeper-%s", instance.GetName())
+		vapBindingName := getVAPBindingName(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) {
@@ -753,3 +767,7 @@ func cleanEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodSt
 		}
 	}
 }
+
+func getVAPBindingName(constraintName string) string {
+	return fmt.Sprintf("gatekeeper-%s", constraintName)
+}
diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go
@@ -703,7 +703,6 @@ func TestReconcile(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 after default wait")
-		constraint.DefaultGenerateVAPB = ptr.To[bool](false)
 		constraintTemplate := makeReconcileConstraintTemplateForVap(suffix, ptr.To[bool](true))
 		cstr := newDenyAllCstrWithScopedEA(suffix, util.VAPEnforcementPoint)
 		t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, expectedCRD(suffix)))
@@ -746,11 +745,7 @@ func TestReconcile(t *testing.T) {
 			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)
+			return nil
 		})
 		if err != nil {
 			t.Fatal(err)

diff --git a/pkg/drivers/k8scel/schema/schema.go b/pkg/drivers/k8scel/schema/schema.go
@@ -217,9 +217,8 @@ func (in *Source) GetFailurePolicy() (*admissionv1.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 {
-		out = admissionv1beta1.Ignore
+		out = admissionv1beta1.Fail
 		return &out, nil
 	}
 
@@ -235,7 +234,7 @@ func (in *Source) GetV1Beta1FailurePolicy() (*admissionv1beta1.FailurePolicyType
 	return &out, nil
 }
 
-// ToUnstructured() is a convenience method for converting to unstructured.
+// MustToUnstructured() is a convenience method for converting to unstructured.
 // Intended for testing. It will panic on error.
 func (in *Source) MustToUnstructured() map[string]interface{} {
 	if in == nil {

diff --git a/test/bats/helpers.bash b/test/bats/helpers.bash
@@ -73,6 +73,21 @@ wait_for_process() {
   return 1
 }
 
+wait_for_error() {
+  wait_time="$1"
+  sleep_time="$2"
+  cmd="$3"
+  while [ "$wait_time" -gt 0 ]; do
+    if eval "$cmd"; then
+      sleep "$sleep_time"
+      wait_time=$((wait_time - sleep_time))
+    else
+      return 0
+    fi
+  done
+  return 1
+}
+
 get_ca_cert() {
   destination="$1"
   if [ $(kubectl get secret -n ${GATEKEEPER_NAMESPACE} gatekeeper-webhook-server-cert -o jsonpath='{.data.ca\.crt}' | wc -w) -eq 0 ]; then

diff --git a/test/bats/test.bats b/test/bats/test.bats
@@ -97,6 +97,10 @@ teardown_file() {
     kubectl delete --ignore-not-found -f ${BATS_TESTS_DIR}/constraints/all_ns_must_have_label_provided_vapbinding_scoped.yaml
 
     wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl delete --ignore-not-found -f ${BATS_TESTS_DIR}/templates/k8srequiredlabels_template_vap.yaml"
+    wait_for_error ${WAIT_TIME} ${SLEEP_TIME} "kubectl get ValidatingAdmissionPolicyBinding all-must-have-label-scoped"
+    wait_for_error ${WAIT_TIME} ${SLEEP_TIME} "kubectl get ValidatingAdmissionPolicyBinding all-must-have-label"
+    wait_for_error ${WAIT_TIME} ${SLEEP_TIME} "kubectl get ValidatingAdmissionPolicyBinding gatekeeper-all-must-have-label-scoped"
+    wait_for_error ${WAIT_TIME} ${SLEEP_TIME} "kubectl get ValidatingAdmissionPolicyBinding gatekeeper-all-must-have-label"
   fi
 }