diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go index 2ae483e69e2..ebbb51bbbb7 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go +++ b/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) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter.go index 9cce78e460a..3e2a63e75ca 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter.go +++ b/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() diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go index 011d0606a70..390c5d780ae 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go +++ b/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") } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go index f77f6ae4eec..f86b6553587 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go @@ -318,10 +318,10 @@ var _ Validator = &fakeValidator{} type fakeValidator struct { validationFilter, auditAnnotationFilter, messageFilter *fakeFilter - ValidateFunc func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult + ValidateFunc func(ctx context.Context, matchResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult } -func (f *fakeValidator) RegisterDefinition(definition *v1beta1.ValidatingAdmissionPolicy, validateFunc func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult) { +func (f *fakeValidator) RegisterDefinition(definition *v1beta1.ValidatingAdmissionPolicy, validateFunc func(ctx context.Context, matchResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult) { //Key must be something that we can decipher from the inputs to Validate so using message which will be on the validationCondition object of evalResult var key string if len(definition.Spec.Validations) > 0 { @@ -338,8 +338,8 @@ func (f *fakeValidator) RegisterDefinition(definition *v1beta1.ValidatingAdmissi validatorMap[key] = f } -func (f *fakeValidator) Validate(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { - return f.ValidateFunc(ctx, versionedAttr, versionedParams, namespace, runtimeCELCostBudget, authz) +func (f *fakeValidator) Validate(ctx context.Context, matchResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + return f.ValidateFunc(ctx, matchResource, versionedAttr, versionedParams, namespace, runtimeCELCostBudget, authz) } var _ Matcher = &fakeMatcher{} @@ -390,18 +390,18 @@ func (f *fakeMatcher) RegisterBinding(binding *v1beta1.ValidatingAdmissionPolicy // Matches says whether this policy definition matches the provided admission // resource request -func (f *fakeMatcher) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionKind, error) { +func (f *fakeMatcher) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error) { namespace, name := definition.Namespace, definition.Name key := namespacedName{ name: name, namespace: namespace, } if fun, ok := f.DefinitionMatchFuncs[key]; ok { - return fun(definition, a), a.GetKind(), nil + return fun(definition, a), a.GetResource(), a.GetKind(), nil } // Default is match everything - return f.DefaultMatch, a.GetKind(), nil + return f.DefaultMatch, a.GetResource(), a.GetKind(), nil } // Matches says whether this policy definition matches the provided admission @@ -834,7 +834,7 @@ func TestBasicPolicyDefinitionFailure(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -904,7 +904,7 @@ func TestDefinitionDoesntMatch(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1019,7 +1019,7 @@ func TestReconfigureBinding(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1130,7 +1130,7 @@ func TestRemoveDefinition(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1199,7 +1199,7 @@ func TestRemoveBinding(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1309,7 +1309,7 @@ func TestInvalidParamSourceInstanceName(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1370,7 +1370,7 @@ func TestEmptyParamRef(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { // Versioned params must be nil to pass the test if versionedParams != nil { return ValidateResult{ @@ -1447,7 +1447,7 @@ func TestEmptyParamSource(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1549,7 +1549,7 @@ func TestMultiplePoliciesSharedParamType(t *testing.T) { } }) - validator1.RegisterDefinition(&policy1, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator1.RegisterDefinition(&policy1, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { evaluations1.Add(1) return ValidateResult{ Decisions: []PolicyDecision{ @@ -1568,7 +1568,7 @@ func TestMultiplePoliciesSharedParamType(t *testing.T) { } }) - validator2.RegisterDefinition(&policy2, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator2.RegisterDefinition(&policy2, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { evaluations2.Add(1) return ValidateResult{ Decisions: []PolicyDecision{ @@ -1678,7 +1678,7 @@ func TestNativeTypeParam(t *testing.T) { } }) - validator.RegisterDefinition(&nativeTypeParamPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(&nativeTypeParamPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { evaluations.Add(1) if _, ok := versionedParams.(*v1.ConfigMap); ok { return ValidateResult{ @@ -1760,7 +1760,7 @@ func TestAuditValidationAction(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1831,7 +1831,7 @@ func TestWarnValidationAction(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1890,7 +1890,7 @@ func TestAllValidationActions(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1977,7 +1977,7 @@ func TestNamespaceParamRefName(t *testing.T) { lock := sync.Mutex{} observedParamNamespaces := []string{} - validator.RegisterDefinition(&nativeTypeParamPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(&nativeTypeParamPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { lock.Lock() defer lock.Unlock() @@ -2264,7 +2264,7 @@ func testParamRefCase(t *testing.T, paramIsClusterScoped, nameIsSet, namespaceIs } }) - validator.RegisterDefinition(&policy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(&policy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { observeParam(versionedParams) return ValidateResult{ Decisions: []PolicyDecision{ @@ -2505,7 +2505,7 @@ func TestNamespaceParamRefClusterScopedParamError(t *testing.T) { } }) - validator.RegisterDefinition(&nativeTypeParamPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(&nativeTypeParamPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { evaluations.Add(1) if _, ok := versionedParams.(*v1beta1.ValidatingAdmissionPolicy); ok { return ValidateResult{ @@ -2570,7 +2570,7 @@ func TestAuditAnnotations(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *v1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { o, err := meta.Accessor(versionedParams) if err != nil { t.Fatal(err) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller.go index a14412856c2..46b76e06d5f 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller.go +++ b/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 { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/interface.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/interface.go index a7ba0d370e2..206fc137831 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/interface.go +++ b/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 } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matcher.go index 8c1757c852f..397f2c26714 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matcher.go +++ b/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 } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matching/matching.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matching/matching.go index ce81de36f6b..ebdb61db889 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matching/matching.go +++ b/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,14 +136,14 @@ 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 } } } @@ -147,7 +151,7 @@ func matchesResourceRules(namedRules []v1beta1.NamedRuleWithOperations, matchPol // 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,11 +173,11 @@ 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 @@ -181,12 +185,12 @@ func matchesResourceRules(namedRules []v1beta1.NamedRuleWithOperations, matchPol 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 { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matching/matching_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matching/matching_test.go index f6bd39ba4f4..e763eb3d86b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/matching/matching_test.go +++ b/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) + } + } }) } } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator.go index b21e6495b4a..9630a497471 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/plugin/cel" "k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions" @@ -72,7 +73,7 @@ func auditAnnotationEvaluationForError(f v1.FailurePolicyType) PolicyAuditAnnota // Validate takes a list of Evaluation and a failure policy and converts them into actionable PolicyDecisions // runtimeCELCostBudget was added for testing purpose only. Callers should always use const RuntimeCELCostBudget from k8s.io/apiserver/pkg/apis/cel/config.go as input. -func (v *validator) Validate(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *corev1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { +func (v *validator) Validate(ctx context.Context, matchedResource schema.GroupVersionResource, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, namespace *corev1.Namespace, runtimeCELCostBudget int64, authz authorizer.Authorizer) ValidateResult { var f v1.FailurePolicyType if v.failPolicy == nil { f = v1.Fail @@ -102,7 +103,7 @@ func (v *validator) Validate(ctx context.Context, versionedAttr *admission.Versi optionalVars := cel.OptionalVariableBindings{VersionedParams: versionedParams, Authorizer: authz} expressionOptionalVars := cel.OptionalVariableBindings{VersionedParams: versionedParams} - admissionRequest := cel.CreateAdmissionRequest(versionedAttr.Attributes) + admissionRequest := cel.CreateAdmissionRequest(versionedAttr.Attributes, metav1.GroupVersionResource(matchedResource), metav1.GroupVersionKind(versionedAttr.VersionedKind)) // Decide which fields are exposed ns := cel.CreateNamespaceObject(namespace) evalResults, remainingBudget, err := v.validationFilter.ForInput(ctx, versionedAttr, admissionRequest, optionalVars, ns, runtimeCELCostBudget) @@ -195,7 +196,7 @@ func (v *validator) Validate(ctx context.Context, versionedAttr *admission.Versi } options := cel.OptionalVariableBindings{VersionedParams: versionedParams} - auditAnnotationEvalResults, _, err := v.auditAnnotationFilter.ForInput(ctx, versionedAttr, cel.CreateAdmissionRequest(versionedAttr.Attributes), options, namespace, runtimeCELCostBudget) + auditAnnotationEvalResults, _, err := v.auditAnnotationFilter.ForInput(ctx, versionedAttr, admissionRequest, options, namespace, runtimeCELCostBudget) if err != nil { return ValidateResult{ Decisions: []PolicyDecision{ diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator_test.go index ab9c12c7275..d1e5753afe6 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator_test.go @@ -893,7 +893,7 @@ func TestValidate(t *testing.T) { if tc.costBudget != 0 { budget = tc.costBudget } - validateResult := v.Validate(ctx, fakeVersionedAttr, nil, nil, budget, nil) + validateResult := v.Validate(ctx, fakeVersionedAttr.GetResource(), fakeVersionedAttr, nil, nil, budget, nil) require.Equal(t, len(validateResult.Decisions), len(tc.policyDecision)) @@ -945,7 +945,7 @@ func TestContextCanceled(t *testing.T) { } ctx, cancel := context.WithCancel(context.TODO()) cancel() - validationResult := v.Validate(ctx, fakeVersionedAttr, nil, nil, celconfig.RuntimeCELCostBudget, nil) + validationResult := v.Validate(ctx, fakeVersionedAttr.GetResource(), fakeVersionedAttr, nil, nil, celconfig.RuntimeCELCostBudget, nil) if len(validationResult.Decisions) != 1 || !strings.Contains(validationResult.Decisions[0].Message, "operation interrupted") { t.Errorf("Expected 'operation interrupted' but got %v", validationResult.Decisions) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions/matcher.go index e9c3fb8f71c..21dd28f6c24 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions/matcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions/matcher.go @@ -26,6 +26,7 @@ import ( celtypes "github.com/google/cel-go/common/types" v1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/admission" @@ -78,7 +79,7 @@ func NewMatcher(filter celplugin.Filter, failPolicy *v1.FailurePolicyType, match func (m *matcher) Match(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, authz authorizer.Authorizer) MatchResult { t := time.Now() - evalResults, _, err := m.filter.ForInput(ctx, versionedAttr, celplugin.CreateAdmissionRequest(versionedAttr.Attributes), celplugin.OptionalVariableBindings{ + evalResults, _, err := m.filter.ForInput(ctx, versionedAttr, celplugin.CreateAdmissionRequest(versionedAttr.Attributes, metav1.GroupVersionResource(versionedAttr.GetResource()), metav1.GroupVersionKind(versionedAttr.VersionedKind)), celplugin.OptionalVariableBindings{ VersionedParams: versionedParams, Authorizer: authz, }, nil, celconfig.RuntimeCELCostBudgetMatchConditions)