Add test cases for ValidatingAdmissionPolicy

pkg/features/kube_features.go

@@ -1206,7 +1206,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	genericfeatures.APIResponseCompression: {Default: true, PreRelease: featuregate.Beta},
 
-	genericfeatures.ValidatingAdmissionPolicy: {Default: false, PreRelease: featuregate.Alpha},
+	genericfeatures.ValidatingAdmissionPolicy: {Default: false, PreRelease: featuregate.Beta},
 
 	genericfeatures.CustomResourceValidationExpressions: {Default: true, PreRelease: featuregate.Beta},
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/google/cel-go/cel"
 
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/admission"
 	celconfig "k8s.io/apiserver/pkg/apis/cel"
 	"k8s.io/apiserver/pkg/cel/environment"
@@ -141,7 +142,7 @@ func TestCompositedPolicies(t *testing.T) {
 			if costBudget == 0 {
 				costBudget = celconfig.RuntimeCELCostBudget
 			}
-			result, _, err := f.ForInput(context.Background(), versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, nil, costBudget)
+			result, _, err := f.ForInput(context.Background(), versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes, v1.GroupVersionResource(tc.attributes.GetResource()), v1.GroupVersionKind(versionedAttr.VersionedKind)), optionalVars, nil, costBudget)
 			if !tc.expectErr && err != nil {
 				t.Fatalf("failed evaluation: %v", err)
 			}

staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter.go

@@ -253,10 +253,13 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione
 }
 
 // TODO: to reuse https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go#L154
-func CreateAdmissionRequest(attr admission.Attributes) *admissionv1.AdmissionRequest {
-	// FIXME: how to get resource GVK, GVR and subresource?
-	gvk := attr.GetKind()
-	gvr := attr.GetResource()
+func CreateAdmissionRequest(attr admission.Attributes, equivalentGVR metav1.GroupVersionResource, equivalentKind metav1.GroupVersionKind) *admissionv1.AdmissionRequest {
+	// Attempting to use same logic as webhook for constructing resource
+	// GVK, GVR, subresource
+	// Use the GVK, GVR that the matcher decided was equivalent to that of the request
+	// https://github.com/kubernetes/kubernetes/blob/90c362b3430bcbbf8f245fadbcd521dab39f1d7c/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go#L182-L210
+	gvk := equivalentKind
+	gvr := equivalentGVR
 	subresource := attr.GetSubresource()
 
 	requestGVK := attr.GetKind()

staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go

@@ -787,7 +787,7 @@ func TestFilter(t *testing.T) {
 
 			optionalVars := OptionalVariableBindings{VersionedParams: tc.params, Authorizer: tc.authorizer}
 			ctx := context.TODO()
-			evalResults, _, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, CreateNamespaceObject(tc.namespaceObject), celconfig.RuntimeCELCostBudget)
+			evalResults, _, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes, metav1.GroupVersionResource(versionedAttr.GetResource()), metav1.GroupVersionKind(versionedAttr.VersionedKind)), optionalVars, CreateNamespaceObject(tc.namespaceObject), celconfig.RuntimeCELCostBudget)
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
@@ -933,7 +933,7 @@ func TestRuntimeCELCostBudget(t *testing.T) {
 			}
 			optionalVars := OptionalVariableBindings{VersionedParams: tc.params, Authorizer: tc.authorizer}
 			ctx := context.TODO()
-			evalResults, remaining, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, nil, tc.testRuntimeCELCostBudget)
+			evalResults, remaining, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes, metav1.GroupVersionResource(versionedAttr.GetResource()), metav1.GroupVersionKind(versionedAttr.VersionedKind)), optionalVars, nil, tc.testRuntimeCELCostBudget)
 			if tc.exceedBudget && err == nil {
 				t.Errorf("Expected RuntimeCELCostBudge to be exceeded but got nil")
 			}

staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller.go

@@ -244,7 +244,7 @@ func (c *celAdmissionController) Validate(
 		var versionedAttr *admission.VersionedAttributes
 
 		definition := definitionInfo.lastReconciledValue
-		matches, matchKind, err := c.policyController.matcher.DefinitionMatches(a, o, definition)
+		matches, matchResource, matchKind, err := c.policyController.matcher.DefinitionMatches(a, o, definition)
 		if err != nil {
 			// Configuration error.
 			addConfigError(err, definition, nil)
@@ -323,7 +323,7 @@ func (c *celAdmissionController) Validate(
 						nested: param,
 					}
 				}
-				validationResults = append(validationResults, bindingInfo.validator.Validate(ctx, versionedAttr, p, namespace, celconfig.RuntimeCELCostBudget, authz))
+				validationResults = append(validationResults, bindingInfo.validator.Validate(ctx, matchResource, versionedAttr, p, namespace, celconfig.RuntimeCELCostBudget, authz))
 			}
 
 			for _, validationResult := range validationResults {

staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/interface.go

@@ -86,7 +86,7 @@ type Matcher interface {
 
 	// DefinitionMatches says whether this policy definition matches the provided admission
 	// resource request
-	DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionKind, error)
+	DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error)
 
 	// BindingMatches says whether this policy definition matches the provided admission
 	// resource request
@@ -109,5 +109,5 @@ type ValidateResult struct {
 type Validator interface {
 	// Validate is used to take cel evaluations and convert into decisions
 	// runtimeCELCostBudget was added for testing purpose only. Callers should always use const RuntimeCELCostBudget from k8s.io/apiserver/pkg/apis/cel/config.go as input.
-	Validate(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *corev1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult
+	Validate(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *corev1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matcher.go

@@ -63,7 +63,7 @@ func (c *matcher) ValidateInitialization() error {
 }
 
 // DefinitionMatches returns whether this ValidatingAdmissionPolicy matches the provided admission resource request
-func (c *matcher) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionKind, error) {
+func (c *matcher) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error) {
 	criteria := matchCriteria{constraints: definition.Spec.MatchConstraints}
 	return c.Matcher.Matches(a, o, &criteria)
 }
@@ -74,7 +74,7 @@ func (c *matcher) BindingMatches(a admission.Attributes, o admission.ObjectInter
 		return true, nil
 	}
 	criteria := matchCriteria{constraints: binding.Spec.MatchResources}
-	isMatch, _, err := c.Matcher.Matches(a, o, &criteria)
+	isMatch, _, _, err := c.Matcher.Matches(a, o, &criteria)
 	return isMatch, err
 }
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matching/matching.go

@@ -71,56 +71,60 @@ func (m *Matcher) ValidateInitialization() error {
 	return nil
 }
 
-func (m *Matcher) Matches(attr admission.Attributes, o admission.ObjectInterfaces, criteria MatchCriteria) (bool, schema.GroupVersionKind, error) {
+func (m *Matcher) Matches(attr admission.Attributes, o admission.ObjectInterfaces, criteria MatchCriteria) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error) {
 	matches, matchNsErr := m.namespaceMatcher.MatchNamespaceSelector(criteria, attr)
 	// Should not return an error here for policy which do not apply to the request, even if err is an unexpected scenario.
 	if !matches && matchNsErr == nil {
-		return false, schema.GroupVersionKind{}, nil
+		return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, nil
 	}
 
 	matches, matchObjErr := m.objectMatcher.MatchObjectSelector(criteria, attr)
 	// Should not return an error here for policy which do not apply to the request, even if err is an unexpected scenario.
 	if !matches && matchObjErr == nil {
-		return false, schema.GroupVersionKind{}, nil
+		return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, nil
 	}
 
 	matchResources := criteria.GetMatchResources()
 	matchPolicy := matchResources.MatchPolicy
-	if isExcluded, _, err := matchesResourceRules(matchResources.ExcludeResourceRules, matchPolicy, attr, o); isExcluded || err != nil {
-		return false, schema.GroupVersionKind{}, err
+	if isExcluded, _, _, err := matchesResourceRules(matchResources.ExcludeResourceRules, matchPolicy, attr, o); isExcluded || err != nil {
+		return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, err
 	}
 
 	var (
-		isMatch   bool
-		matchKind schema.GroupVersionKind
-		matchErr  error
+		isMatch       bool
+		matchResource schema.GroupVersionResource
+		matchKind     schema.GroupVersionKind
+		matchErr      error
 	)
 	if len(matchResources.ResourceRules) == 0 {
 		isMatch = true
 		matchKind = attr.GetKind()
+		matchResource = attr.GetResource()
 	} else {
-		isMatch, matchKind, matchErr = matchesResourceRules(matchResources.ResourceRules, matchPolicy, attr, o)
+		isMatch, matchResource, matchKind, matchErr = matchesResourceRules(matchResources.ResourceRules, matchPolicy, attr, o)
 	}
 	if matchErr != nil {
-		return false, schema.GroupVersionKind{}, matchErr
+		return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, matchErr
 	}
 	if !isMatch {
-		return false, schema.GroupVersionKind{}, nil
+		return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, nil
 	}
 
 	// now that we know this applies to this request otherwise, if there were selector errors, return them
 	if matchNsErr != nil {
-		return false, schema.GroupVersionKind{}, matchNsErr
+		return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, matchNsErr
 	}
 	if matchObjErr != nil {
-		return false, schema.GroupVersionKind{}, matchObjErr
+		return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, matchObjErr
 	}
 
-	return true, matchKind, nil
+	return true, matchResource, matchKind, nil
 }
 
-func matchesResourceRules(namedRules []v1beta1.NamedRuleWithOperations, matchPolicy *v1beta1.MatchPolicyType, attr admission.Attributes, o admission.ObjectInterfaces) (bool, schema.GroupVersionKind, error) {
+func matchesResourceRules(namedRules []v1beta1.NamedRuleWithOperations, matchPolicy *v1beta1.MatchPolicyType, attr admission.Attributes, o admission.ObjectInterfaces) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error) {
 	matchKind := attr.GetKind()
+	matchResource := attr.GetResource()
+
 	for _, namedRule := range namedRules {
 		rule := v1.RuleWithOperations(namedRule.RuleWithOperations)
 		ruleMatcher := rules.Matcher{
@@ -132,22 +136,22 @@ func matchesResourceRules(namedRules []v1beta1.NamedRuleWithOperations, matchPol
 		}
 		// an empty name list always matches
 		if len(namedRule.ResourceNames) == 0 {
-			return true, matchKind, nil
+			return true, matchResource, matchKind, nil
 		}
 		// TODO: GetName() can return an empty string if the user is relying on
 		// the API server to generate the name... figure out what to do for this edge case
 		name := attr.GetName()
 		for _, matchedName := range namedRule.ResourceNames {
 			if name == matchedName {
-				return true, matchKind, nil
+				return true, matchResource, matchKind, nil
 			}
 		}
 	}
 
 	// if match policy is undefined or exact, don't perform fuzzy matching
 	// note that defaulting to fuzzy matching is set by the API
 	if matchPolicy == nil || *matchPolicy == v1beta1.Exact {
-		return false, schema.GroupVersionKind{}, nil
+		return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, nil
 	}
 
 	attrWithOverride := &attrWithResourceOverride{Attributes: attr}
@@ -169,24 +173,24 @@ func matchesResourceRules(namedRules []v1beta1.NamedRuleWithOperations, matchPol
 			}
 			matchKind = o.GetEquivalentResourceMapper().KindFor(equivalent, attr.GetSubresource())
 			if matchKind.Empty() {
-				return false, schema.GroupVersionKind{}, fmt.Errorf("unable to convert to %v: unknown kind", equivalent)
+				return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, fmt.Errorf("unable to convert to %v: unknown kind", equivalent)
 			}
 			// an empty name list always matches
 			if len(namedRule.ResourceNames) == 0 {
-				return true, matchKind, nil
+				return true, equivalent, matchKind, nil
 			}
 
 			// TODO: GetName() can return an empty string if the user is relying on
 			// the API server to generate the name... figure out what to do for this edge case
 			name := attr.GetName()
 			for _, matchedName := range namedRule.ResourceNames {
 				if name == matchedName {
-					return true, matchKind, nil
+					return true, equivalent, matchKind, nil
 				}
 			}
 		}
 	}
-	return false, schema.GroupVersionKind{}, nil
+	return false, schema.GroupVersionResource{}, schema.GroupVersionKind{}, nil
 }
 
 type attrWithResourceOverride struct {

staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matching/matching_test.go

@@ -98,9 +98,10 @@ func TestMatcher(t *testing.T) {
 		criteria *v1beta1.MatchResources
 		attrs    admission.Attributes
 
-		expectMatches   bool
-		expectMatchKind schema.GroupVersionKind
-		expectErr       string
+		expectMatches       bool
+		expectMatchKind     schema.GroupVersionKind
+		expectMatchResource schema.GroupVersionResource
+		expectErr           string
 	}{
 		{
 			name:          "no rules (just write)",
@@ -204,9 +205,10 @@ func TestMatcher(t *testing.T) {
 						Rule:       v1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1beta1"}, Resources: []string{"deployments"}, Scope: &allScopes},
 					},
 				}}},
-			attrs:           admission.NewAttributesRecord(nil, nil, gvk("apps", "v1", "Deployment"), "ns", "name", gvr("apps", "v1", "deployments"), "", admission.Create, &metav1.CreateOptions{}, false, nil),
-			expectMatches:   true,
-			expectMatchKind: gvk("extensions", "v1beta1", "Deployment"),
+			attrs:               admission.NewAttributesRecord(nil, nil, gvk("apps", "v1", "Deployment"), "ns", "name", gvr("apps", "v1", "deployments"), "", admission.Create, &metav1.CreateOptions{}, false, nil),
+			expectMatches:       true,
+			expectMatchResource: gvr("extensions", "v1beta1", "deployments"),
+			expectMatchKind:     gvk("extensions", "v1beta1", "Deployment"),
 		},
 		{
 			name: "specific rules, equivalent match, prefer apps",
@@ -225,9 +227,10 @@ func TestMatcher(t *testing.T) {
 						Rule:       v1.Rule{APIGroups: []string{"extensions"}, APIVersions: []string{"v1beta1"}, Resources: []string{"deployments"}, Scope: &allScopes},
 					},
 				}}},
-			attrs:           admission.NewAttributesRecord(nil, nil, gvk("apps", "v1", "Deployment"), "ns", "name", gvr("apps", "v1", "deployments"), "", admission.Create, &metav1.CreateOptions{}, false, nil),
-			expectMatches:   true,
-			expectMatchKind: gvk("apps", "v1beta1", "Deployment"),
+			attrs:               admission.NewAttributesRecord(nil, nil, gvk("apps", "v1", "Deployment"), "ns", "name", gvr("apps", "v1", "deployments"), "", admission.Create, &metav1.CreateOptions{}, false, nil),
+			expectMatches:       true,
+			expectMatchResource: gvr("apps", "v1beta1", "deployments"),
+			expectMatchKind:     gvk("apps", "v1beta1", "Deployment"),
 		},
 
 		{
@@ -311,9 +314,10 @@ func TestMatcher(t *testing.T) {
 						Rule:       v1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1beta1"}, Resources: []string{"deployments", "deployments/scale"}, Scope: &allScopes},
 					},
 				}}},
-			attrs:           admission.NewAttributesRecord(nil, nil, gvk("autoscaling", "v1", "Scale"), "ns", "name", gvr("apps", "v1", "deployments"), "scale", admission.Create, &metav1.CreateOptions{}, false, nil),
-			expectMatches:   true,
-			expectMatchKind: gvk("extensions", "v1beta1", "Scale"),
+			attrs:               admission.NewAttributesRecord(nil, nil, gvk("autoscaling", "v1", "Scale"), "ns", "name", gvr("apps", "v1", "deployments"), "scale", admission.Create, &metav1.CreateOptions{}, false, nil),
+			expectMatches:       true,
+			expectMatchResource: gvr("extensions", "v1beta1", "deployments"),
+			expectMatchKind:     gvk("extensions", "v1beta1", "Scale"),
 		},
 		{
 			name: "specific rules, subresource equivalent match, prefer apps",
@@ -332,9 +336,10 @@ func TestMatcher(t *testing.T) {
 						Rule:       v1.Rule{APIGroups: []string{"extensions"}, APIVersions: []string{"v1beta1"}, Resources: []string{"deployments", "deployments/scale"}, Scope: &allScopes},
 					},
 				}}},
-			attrs:           admission.NewAttributesRecord(nil, nil, gvk("autoscaling", "v1", "Scale"), "ns", "name", gvr("apps", "v1", "deployments"), "scale", admission.Create, &metav1.CreateOptions{}, false, nil),
-			expectMatches:   true,
-			expectMatchKind: gvk("apps", "v1beta1", "Scale"),
+			attrs:               admission.NewAttributesRecord(nil, nil, gvk("autoscaling", "v1", "Scale"), "ns", "name", gvr("apps", "v1", "deployments"), "scale", admission.Create, &metav1.CreateOptions{}, false, nil),
+			expectMatches:       true,
+			expectMatchResource: gvr("apps", "v1beta1", "deployments"),
+			expectMatchKind:     gvk("apps", "v1beta1", "Scale"),
 		},
 		{
 			name: "specific rules, prefer exact match and name match",
@@ -380,9 +385,10 @@ func TestMatcher(t *testing.T) {
 						Rule:       v1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1"}, Resources: []string{"deployments", "deployments/scale"}, Scope: &allScopes},
 					},
 				}}},
-			attrs:           admission.NewAttributesRecord(nil, nil, gvk("autoscaling", "v1", "Scale"), "ns", "name", gvr("extensions", "v1beta1", "deployments"), "scale", admission.Create, &metav1.CreateOptions{}, false, nil),
-			expectMatches:   true,
-			expectMatchKind: gvk("autoscaling", "v1", "Scale"),
+			attrs:               admission.NewAttributesRecord(nil, nil, gvk("autoscaling", "v1", "Scale"), "ns", "name", gvr("extensions", "v1beta1", "deployments"), "scale", admission.Create, &metav1.CreateOptions{}, false, nil),
+			expectMatches:       true,
+			expectMatchResource: gvr("apps", "v1", "deployments"),
+			expectMatchKind:     gvk("autoscaling", "v1", "Scale"),
 		},
 		{
 			name: "specific rules, subresource equivalent match, prefer extensions and name match miss",
@@ -536,7 +542,7 @@ func TestMatcher(t *testing.T) {
 
 	for _, testcase := range testcases {
 		t.Run(testcase.name, func(t *testing.T) {
-			matches, matchKind, err := a.Matches(testcase.attrs, interfaces, &fakeCriteria{matchResources: *testcase.criteria})
+			matches, matchResource, matchKind, err := a.Matches(testcase.attrs, interfaces, &fakeCriteria{matchResources: *testcase.criteria})
 			if err != nil {
 				if len(testcase.expectErr) == 0 {
 					t.Fatal(err)
@@ -558,6 +564,22 @@ func TestMatcher(t *testing.T) {
 			if matches != testcase.expectMatches {
 				t.Fatalf("expected matches = %v; got %v", testcase.expectMatches, matches)
 			}
+
+			expectResource := testcase.expectMatchResource
+			if !expectResource.Empty() && !matches {
+				t.Fatalf("expectResource is non-empty, but did not match")
+			} else if expectResource.Empty() {
+				// Test for exact match by default. Tests that expect an equivalent
+				// resource to match should explicitly state so by supplying
+				// expectMatchResource
+				expectResource = testcase.attrs.GetResource()
+			}
+
+			if matches {
+				if matchResource != expectResource {
+					t.Fatalf("expected matchResource %v, got %v", expectResource, matchResource)
+				}
+			}
 		})
 	}
 }