Use per-policy marker names for VAP integration tests.

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).
This commit is contained in:
Ben Luddy 2025-06-24 13:06:21 -04:00 committed by Michal Wozniak
parent 5ea44b894a
commit fc02d1eaf5

View File

@ -64,6 +64,8 @@ import (
authorizationv1 "k8s.io/api/authorization/v1" authorizationv1 "k8s.io/api/authorization/v1"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1" rbacv1 "k8s.io/api/rbac/v1"
utilrand "k8s.io/apimachinery/pkg/util/rand"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
) )
// Test_ValidateNamespace_NoParams tests a ValidatingAdmissionPolicy that validates creation of a Namespace with no params. // Test_ValidateNamespace_NoParams tests a ValidatingAdmissionPolicy that validates creation of a Namespace with no params.
@ -2870,8 +2872,17 @@ func serviceAccountClient(namespace, name string) clientFn {
func withWaitReadyConstraintAndExpression(policy *admissionregistrationv1.ValidatingAdmissionPolicy) *admissionregistrationv1.ValidatingAdmissionPolicy { func withWaitReadyConstraintAndExpression(policy *admissionregistrationv1.ValidatingAdmissionPolicy) *admissionregistrationv1.ValidatingAdmissionPolicy {
policy = policy.DeepCopy() 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{ policy.Spec.MatchConstraints.ResourceRules = append(policy.Spec.MatchConstraints.ResourceRules, admissionregistrationv1.NamedRuleWithOperations{
ResourceNames: []string{"test-marker"}, ResourceNames: []string{testMarkerName},
RuleWithOperations: admissionregistrationv1.RuleWithOperations{ RuleWithOperations: admissionregistrationv1.RuleWithOperations{
Operations: []admissionregistrationv1.OperationType{ Operations: []admissionregistrationv1.OperationType{
"UPDATE", "UPDATE",
@ -2890,7 +2901,7 @@ func withWaitReadyConstraintAndExpression(policy *admissionregistrationv1.Valida
}, },
}) })
policy.Spec.Validations = append([]admissionregistrationv1.Validation{{ 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", Message: "marker denied; policy is ready",
}}, policy.Spec.Validations...) }}, policy.Spec.Validations...)
return policy return policy
@ -2905,14 +2916,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 { 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() { defer func() {
err := client.CoreV1().Endpoints(ns).Delete(context.TODO(), marker.Name, metav1.DeleteOptions{}) err := client.CoreV1().Endpoints(ns).Delete(context.TODO(), marker.Name, metav1.DeleteOptions{})
if err != nil { if err != nil {
t.Logf("error deleting marker: %v", err) 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 { if err != nil {
return err return err
} }