Use per-policy marker names for VAP integration tests.

benluddy · benluddy · commit 190c8c7abc5d · 2025-06-25T11:17:20.000-04:00
Writes to policy resources don't instantaneously take effect in admission. ValidatingAdmissionPolicy
integration tests determine that the policies under test have taken effect by adding a sentinel
policy rule and polling until that rule is applied to a request.

If the marker resource names are the same for each test case in a series of test cases, then
observing a policy's effect on a marker request only indicates that _any_ test policy is in effect,
but it's not necessarily the policy the current test case is waiting for. For example:

1. Test 1 creates a policy and binding.

2. The policy and binding are observed by the admission plugin and take effect.

3. Test 1 observes that a policy is in effect via marker requests.

4. Test 1 exercises the behavior under test and successfully deletes the policy and binding it
created.

5. Test 2 creates a policy and binding.

6. Test 2 observes that a policy is in effect via marker requests, but the policy in effect is still
the one created by Test 1.

7. Test 2 exercises the behavior under test, which fails because it was evaluated against Test 1's
policy.

Generating a per-policy name for the marker resource in each test resolves the timing issue. In the
example, step (6) will not proceed until the admission plugin has observed the policy and binding
created in (5).
diff --git a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go
@@ -65,6 +65,8 @@ import (
 	authorizationv1 "k8s.io/api/authorization/v1"
 	v1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	utilrand "k8s.io/apimachinery/pkg/util/rand"
+	utilvalidation "k8s.io/apimachinery/pkg/util/validation"
 )
 
 // Short term fix to refresh the policy source cache faster for tests
@@ -3004,8 +3006,17 @@ func secondaryAuthorizationServiceAccountClient(t *testing.T, adminClient *clien
 
 func withWaitReadyConstraintAndExpression(policy *admissionregistrationv1.ValidatingAdmissionPolicy) *admissionregistrationv1.ValidatingAdmissionPolicy {
 	policy = policy.DeepCopy()
+
+	testMarkerName := fmt.Sprintf("test-marker-%s", utilrand.String(utilvalidation.DNS1123SubdomainMaxLength-len("test-marker-")))
+	annotations := policy.GetAnnotations()
+	if annotations == nil {
+		annotations = make(map[string]string)
+	}
+	annotations["test-marker-name"] = testMarkerName
+	policy.SetAnnotations(annotations)
+
 	policy.Spec.MatchConstraints.ResourceRules = append(policy.Spec.MatchConstraints.ResourceRules, admissionregistrationv1.NamedRuleWithOperations{
-		ResourceNames: []string{"test-marker"},
+		ResourceNames: []string{testMarkerName},
 		RuleWithOperations: admissionregistrationv1.RuleWithOperations{
 			Operations: []admissionregistrationv1.OperationType{
 				"UPDATE",
@@ -3024,7 +3035,7 @@ func withWaitReadyConstraintAndExpression(policy *admissionregistrationv1.Valida
 		},
 	})
 	policy.Spec.Validations = append([]admissionregistrationv1.Validation{{
-		Expression: "object.metadata.name != 'test-marker'",
+		Expression: fmt.Sprintf("object.metadata.name != '%s'", testMarkerName),
 		Message:    "marker denied; policy is ready",
 	}}, policy.Spec.Validations...)
 	return policy
@@ -3039,14 +3050,23 @@ func createAndWaitReadyNamespaced(t *testing.T, client clientset.Interface, bind
 }
 
 func createAndWaitReadyNamespacedWithWarnHandler(t *testing.T, client clientset.Interface, binding *admissionregistrationv1.ValidatingAdmissionPolicyBinding, matchLabels map[string]string, ns string, handler *warningHandler) error {
-	marker := &v1.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "test-marker", Namespace: ns, Labels: matchLabels}}
+	policy, err := client.AdmissionregistrationV1().ValidatingAdmissionPolicies().Get(context.TODO(), binding.Spec.PolicyName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	testMarkerName := "test-marker"
+	if testMarkerNameAnnotation, ok := policy.GetAnnotations()["test-marker-name"]; ok {
+		testMarkerName = testMarkerNameAnnotation
+	}
+
+	marker := &v1.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: testMarkerName, Namespace: ns, Labels: matchLabels}}
 	defer func() {
 		err := client.CoreV1().Endpoints(ns).Delete(context.TODO(), marker.Name, metav1.DeleteOptions{})
 		if err != nil {
 			t.Logf("error deleting marker: %v", err)
 		}
 	}()
-	marker, err := client.CoreV1().Endpoints(ns).Create(context.TODO(), marker, metav1.CreateOptions{})
+	marker, err = client.CoreV1().Endpoints(ns).Create(context.TODO(), marker, metav1.CreateOptions{})
 	if err != nil {
 		return err
 	}
