Improve error messaging for validating admission policy authz

pkg/registry/admissionregistration/validatingadmissionpolicy/authz_test.go

@@ -20,10 +20,12 @@ import (
 	"context"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/endpoints/request"
+	"k8s.io/kubernetes/pkg/apis/admissionregistration"
 	"k8s.io/kubernetes/pkg/registry/admissionregistration/resolver"
 )
 
@@ -31,6 +33,7 @@ func TestAuthorization(t *testing.T) {
 	for _, tc := range []struct {
 		name             string
 		userInfo         user.Info
+		obj              *admissionregistration.ValidatingAdmissionPolicy
 		auth             AuthFunc
 		resourceResolver resolver.ResourceResolverFunc
 		expectErr        bool
@@ -39,6 +42,7 @@ func TestAuthorization(t *testing.T) {
 			name:      "superuser",
 			userInfo:  &user.DefaultInfo{Groups: []string{user.SystemPrivilegedGroup}},
 			expectErr: false, // success despite always-denying authorizer
+			obj:       validValidatingAdmissionPolicy(),
 			auth: func(ctx context.Context, a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
 				return authorizer.DecisionDeny, "", nil
 			},
@@ -46,6 +50,7 @@ func TestAuthorization(t *testing.T) {
 		{
 			name:     "authorized",
 			userInfo: &user.DefaultInfo{Groups: []string{user.AllAuthenticated}},
+			obj:      validValidatingAdmissionPolicy(),
 			auth: func(ctx context.Context, a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
 				if a.GetResource() == "replicalimits" {
 					return authorizer.DecisionAllow, "", nil
@@ -64,6 +69,7 @@ func TestAuthorization(t *testing.T) {
 		{
 			name:     "denied",
 			userInfo: &user.DefaultInfo{Groups: []string{user.AllAuthenticated}},
+			obj:      validValidatingAdmissionPolicy(),
 			auth: func(ctx context.Context, a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
 				if a.GetResource() == "configmaps" {
 					return authorizer.DecisionAllow, "", nil
@@ -79,22 +85,36 @@ func TestAuthorization(t *testing.T) {
 			},
 			expectErr: true,
 		},
+		{
+			name:     "param not found",
+			userInfo: &user.DefaultInfo{Groups: []string{user.AllAuthenticated}},
+			obj:      validValidatingAdmissionPolicy(),
+			auth: func(ctx context.Context, a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+				if a.GetResource() == "replicalimits" {
+					return authorizer.DecisionAllow, "", nil
+				}
+				return authorizer.DecisionDeny, "", nil
+			},
+			resourceResolver: func(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
+				return schema.GroupVersionResource{}, &meta.NoKindMatchError{GroupKind: gvk.GroupKind(), SearchedVersions: []string{gvk.Version}}
+			},
+			expectErr: true,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			strategy := NewStrategy(tc.auth, tc.resourceResolver)
 			t.Run("create", func(t *testing.T) {
 				ctx := request.WithUser(context.Background(), tc.userInfo)
-				errs := strategy.Validate(ctx, validValidatingAdmissionPolicy())
+				errs := strategy.Validate(ctx, tc.obj)
 				if len(errs) > 0 != tc.expectErr {
 					t.Errorf("expected error: %v but got error: %v", tc.expectErr, errs)
 				}
 			})
 			t.Run("update", func(t *testing.T) {
 				ctx := request.WithUser(context.Background(), tc.userInfo)
-				obj := validValidatingAdmissionPolicy()
-				objWithUpdatedParamKind := obj.DeepCopy()
+				objWithUpdatedParamKind := tc.obj.DeepCopy()
 				objWithUpdatedParamKind.Spec.ParamKind.APIVersion += "1"
-				errs := strategy.ValidateUpdate(ctx, obj, objWithUpdatedParamKind)
+				errs := strategy.ValidateUpdate(ctx, tc.obj, objWithUpdatedParamKind)
 				if len(errs) > 0 != tc.expectErr {
 					t.Errorf("expected error: %v but got error: %v", tc.expectErr, errs)
 				}

pkg/registry/admissionregistration/validatingadmissionpolicy/storage/storage_test.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing"
@@ -201,7 +202,7 @@ func newValidatingAdmissionPolicy(name string) *admissionregistration.Validating
 }
 
 func newInsecureStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) {
-	return newStorage(t, nil, nil)
+	return newStorage(t, nil, replicaLimitsResolver)
 }
 
 func newStorage(t *testing.T, authorizer authorizer.Authorizer, resourceResolver resolver.ResourceResolver) (*REST, *etcd3testing.EtcdTestServer) {
@@ -225,3 +226,11 @@ func TestCategories(t *testing.T) {
 	expected := []string{"api-extensions"}
 	registrytest.AssertCategories(t, storage, expected)
 }
+
+var replicaLimitsResolver resolver.ResourceResolverFunc = func(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
+	return schema.GroupVersionResource{
+		Group:    "rules.example.com",
+		Version:  "v1",
+		Resource: "replicalimits",
+	}, nil
+}

pkg/registry/admissionregistration/validatingadmissionpolicy/strategy_test.go

@@ -20,12 +20,14 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/kubernetes/pkg/apis/admissionregistration"
+	"k8s.io/kubernetes/pkg/registry/admissionregistration/resolver"
 )
 
 func TestValidatingAdmissionPolicyStrategy(t *testing.T) {
-	strategy := NewStrategy(nil, nil)
+	strategy := NewStrategy(nil, replicaLimitsResolver)
 	ctx := genericapirequest.NewDefaultContext()
 	if strategy.NamespaceScoped() {
 		t.Error("ValidatingAdmissionPolicy strategy must be cluster scoped")
@@ -49,6 +51,15 @@ func TestValidatingAdmissionPolicyStrategy(t *testing.T) {
 		t.Errorf("Expected a validation error")
 	}
 }
+
+var replicaLimitsResolver resolver.ResourceResolverFunc = func(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
+	return schema.GroupVersionResource{
+		Group:    "rules.example.com",
+		Version:  "v1",
+		Resource: "replicalimits",
+	}, nil
+}
+
 func validValidatingAdmissionPolicy() *admissionregistration.ValidatingAdmissionPolicy {
 	ignore := admissionregistration.Ignore
 	return &admissionregistration.ValidatingAdmissionPolicy{

pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz.go

@@ -55,7 +55,10 @@ func (v *validatingAdmissionPolicyBindingStrategy) authorizeUpdate(ctx context.C
 }
 
 func (v *validatingAdmissionPolicyBindingStrategy) authorize(ctx context.Context, binding *admissionregistration.ValidatingAdmissionPolicyBinding) error {
-	if v.authorizer == nil || v.resourceResolver == nil || binding.Spec.ParamRef == nil {
+	if v.resourceResolver == nil {
+		return fmt.Errorf(`unexpected internal error: resourceResolver is nil`)
+	}
+	if v.authorizer == nil || binding.Spec.ParamRef == nil {
 		return nil
 	}
 
@@ -72,13 +75,21 @@ func (v *validatingAdmissionPolicyBindingStrategy) authorize(ctx context.Context
 	// default to requiring permissions on all group/version/resources
 	resource, apiGroup, apiVersion := "*", "*", "*"
 
-	if policy, err := v.policyGetter.GetValidatingAdmissionPolicy(ctx, binding.Spec.PolicyName); err == nil && policy.Spec.ParamKind != nil {
+	var policyErr, gvParseErr, gvrResolveErr error
+
+	var policy *admissionregistration.ValidatingAdmissionPolicy
+	policy, policyErr = v.policyGetter.GetValidatingAdmissionPolicy(ctx, binding.Spec.PolicyName)
+	if policyErr == nil && policy.Spec.ParamKind != nil {
 		paramKind := policy.Spec.ParamKind
-		if gv, err := schema.ParseGroupVersion(paramKind.APIVersion); err == nil {
+		var gv schema.GroupVersion
+		gv, gvParseErr = schema.ParseGroupVersion(paramKind.APIVersion)
+		if gvParseErr == nil {
 			// we only need to authorize the parsed group/version
 			apiGroup = gv.Group
 			apiVersion = gv.Version
-			if gvr, err := v.resourceResolver.Resolve(gv.WithKind(paramKind.Kind)); err == nil {
+			var gvr schema.GroupVersionResource
+			gvr, gvrResolveErr = v.resourceResolver.Resolve(gv.WithKind(paramKind.Kind))
+			if gvrResolveErr == nil {
 				// we only need to authorize the resolved resource
 				resource = gvr.Resource
 			}
@@ -107,9 +118,18 @@ func (v *validatingAdmissionPolicyBindingStrategy) authorize(ctx context.Context
 
 	d, _, err := v.authorizer.Authorize(ctx, attrs)
 	if err != nil {
-		return err
+		return fmt.Errorf(`failed to authorize request: %w`, err)
 	}
 	if d != authorizer.DecisionAllow {
+		if policyErr != nil {
+			return fmt.Errorf(`unable to get policy %s to determine minimum required permissions and user %v does not have "%v" permission for all groups, versions and resources`, binding.Spec.PolicyName, user, verb)
+		}
+		if gvParseErr != nil {
+			return fmt.Errorf(`unable to parse paramKind %v to determine minimum required permissions and user %v does not have "%v" permission for all groups, versions and resources`, policy.Spec.ParamKind, user, verb)
+		}
+		if gvrResolveErr != nil {
+			return fmt.Errorf(`unable to resolve paramKind %v to determine minimum required permissions and user %v does not have "%v" permission for all groups, versions and resources`, policy.Spec.ParamKind, user, verb)
+		}
 		return fmt.Errorf(`user %v does not have "%v" permission on the object referenced by paramRef`, user, verb)
 	}
 

pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz_test.go

@@ -18,8 +18,12 @@ package validatingadmissionpolicybinding
 
 import (
 	"context"
+	"errors"
+	"fmt"
+	"strings"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/authentication/user"
@@ -31,17 +35,16 @@ import (
 
 func TestAuthorization(t *testing.T) {
 	for _, tc := range []struct {
-		name             string
-		userInfo         user.Info
-		auth             AuthFunc
-		policyGetter     PolicyGetterFunc
-		resourceResolver resolver.ResourceResolverFunc
-		expectErr        bool
+		name              string
+		userInfo          user.Info
+		auth              AuthFunc
+		policyGetter      PolicyGetterFunc
+		resourceResolver  resolver.ResourceResolverFunc
+		expectErrContains string
 	}{
 		{
-			name:      "superuser",
-			userInfo:  &user.DefaultInfo{Groups: []string{user.SystemPrivilegedGroup}},
-			expectErr: false, // success despite always-denying authorizer
+			name:     "superuser", // success despite always-denying authorizer
+			userInfo: &user.DefaultInfo{Groups: []string{user.SystemPrivilegedGroup}},
 			auth: func(ctx context.Context, a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
 				return authorizer.DecisionDeny, "", nil
 			},
@@ -70,7 +73,6 @@ func TestAuthorization(t *testing.T) {
 					Resource: "configmaps",
 				}, nil
 			},
-			expectErr: false,
 		},
 		{
 			name:     "denied",
@@ -96,7 +98,76 @@ func TestAuthorization(t *testing.T) {
 					Resource: "params",
 				}, nil
 			},
-			expectErr: true,
+			expectErrContains: "permission on the object referenced by paramRef",
+		},
+		{
+			name:     "unable to parse paramRef",
+			userInfo: &user.DefaultInfo{Groups: []string{user.AllAuthenticated}},
+			auth: func(ctx context.Context, a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+				if a.GetResource() == "configmaps" {
+					return authorizer.DecisionAllow, "", nil
+				}
+				return authorizer.DecisionDeny, "", nil
+			},
+			policyGetter: func(ctx context.Context, name string) (*admissionregistration.ValidatingAdmissionPolicy, error) {
+				return &admissionregistration.ValidatingAdmissionPolicy{
+					ObjectMeta: metav1.ObjectMeta{Name: "replicalimit-policy.example.com"},
+					Spec: admissionregistration.ValidatingAdmissionPolicySpec{
+						ParamKind: &admissionregistration.ParamKind{Kind: "ConfigMap", APIVersion: "invalid"},
+					},
+				}, nil
+			},
+			resourceResolver: func(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
+				return schema.GroupVersionResource{
+					Group:    "",
+					Version:  "v1",
+					Resource: "configmaps",
+				}, nil
+			},
+			expectErrContains: "unable to parse paramKind &{foo.example.com/v1 Params} to determine minimum required permissions",
+		},
+		{
+			name:     "unable to resolve param",
+			userInfo: &user.DefaultInfo{Groups: []string{user.AllAuthenticated}},
+			auth: func(ctx context.Context, a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+				if a.GetResource() == "configmaps" {
+					return authorizer.DecisionAllow, "", nil
+				}
+				return authorizer.DecisionDeny, "", nil
+			},
+			policyGetter: func(ctx context.Context, name string) (*admissionregistration.ValidatingAdmissionPolicy, error) {
+				return &admissionregistration.ValidatingAdmissionPolicy{
+					ObjectMeta: metav1.ObjectMeta{Name: "replicalimit-policy.example.com"},
+					Spec: admissionregistration.ValidatingAdmissionPolicySpec{
+						ParamKind: &admissionregistration.ParamKind{Kind: "Params", APIVersion: "foo.example.com/v1"},
+					},
+				}, nil
+			},
+			resourceResolver: func(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
+				return schema.GroupVersionResource{}, &meta.NoKindMatchError{GroupKind: gvk.GroupKind(), SearchedVersions: []string{gvk.Version}}
+			},
+			expectErrContains: "unable to resolve paramKind &{foo.example.com/v1 Params} to determine minimum required permissions",
+		},
+		{
+			name:     "unable to get policy",
+			userInfo: &user.DefaultInfo{Groups: []string{user.AllAuthenticated}},
+			auth: func(ctx context.Context, a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+				if a.GetResource() == "configmaps" {
+					return authorizer.DecisionAllow, "", nil
+				}
+				return authorizer.DecisionDeny, "", nil
+			},
+			policyGetter: func(ctx context.Context, name string) (*admissionregistration.ValidatingAdmissionPolicy, error) {
+				return nil, fmt.Errorf("no such policy")
+			},
+			resourceResolver: func(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
+				return schema.GroupVersionResource{
+					Group:    "",
+					Version:  "v1",
+					Resource: "configmaps",
+				}, nil
+			},
+			expectErrContains: "unable to get policy replicalimit-policy.example.com to determine minimum required permissions",
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
@@ -105,8 +176,8 @@ func TestAuthorization(t *testing.T) {
 				ctx := request.WithUser(context.Background(), tc.userInfo)
 				for _, obj := range validPolicyBindings() {
 					errs := strategy.Validate(ctx, obj)
-					if len(errs) > 0 != tc.expectErr {
-						t.Errorf("expected error: %v but got error: %v", tc.expectErr, errs)
+					if len(errs) > 0 && !strings.Contains(errors.Join(errs.ToAggregate().Errors()...).Error(), tc.expectErrContains) {
+						t.Errorf("expected error to contain: %v but got error: %v", tc.expectErrContains, errs)
 					}
 				}
 			})
@@ -140,8 +211,8 @@ func TestAuthorization(t *testing.T) {
 						}
 					}
 					errs := strategy.ValidateUpdate(ctx, obj, objWithChangedParamRef)
-					if len(errs) > 0 != tc.expectErr {
-						t.Errorf("expected error: %v but got error: %v", tc.expectErr, errs)
+					if len(errs) > 0 && !strings.Contains(errors.Join(errs.ToAggregate().Errors()...).Error(), tc.expectErrContains) {
+						t.Errorf("expected error to contain: %v but got error: %v", tc.expectErrContains, errs)
 					}
 				}
 			})

pkg/registry/admissionregistration/validatingadmissionpolicybinding/storage/storage_test.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing"
@@ -230,7 +231,7 @@ func newPolicyBinding(name string) *admissionregistration.ValidatingAdmissionPol
 }
 
 func newInsecureStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) {
-	return newStorage(t, nil, nil, nil)
+	return newStorage(t, nil, nil, replicaLimitsResolver)
 }
 
 func newStorage(t *testing.T, authorizer authorizer.Authorizer, policyGetter PolicyGetter, resourceResolver resolver.ResourceResolver) (*REST, *etcd3testing.EtcdTestServer) {
@@ -254,3 +255,11 @@ func TestCategories(t *testing.T) {
 	expected := []string{"api-extensions"}
 	registrytest.AssertCategories(t, storage, expected)
 }
+
+var replicaLimitsResolver resolver.ResourceResolverFunc = func(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
+	return schema.GroupVersionResource{
+		Group:    "rules.example.com",
+		Version:  "v1",
+		Resource: "replicalimits",
+	}, nil
+}

pkg/registry/admissionregistration/validatingadmissionpolicybinding/strategy_test.go

@@ -20,13 +20,15 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+	"k8s.io/kubernetes/pkg/registry/admissionregistration/resolver"
 
 	"k8s.io/kubernetes/pkg/apis/admissionregistration"
 )
 
 func TestPolicyBindingStrategy(t *testing.T) {
-	strategy := NewStrategy(nil, nil, nil)
+	strategy := NewStrategy(nil, nil, replicaLimitsResolver)
 	ctx := genericapirequest.NewDefaultContext()
 	if strategy.NamespaceScoped() {
 		t.Error("PolicyBinding strategy must be cluster scoped")
@@ -52,6 +54,14 @@ func TestPolicyBindingStrategy(t *testing.T) {
 	}
 }
 
+var replicaLimitsResolver resolver.ResourceResolverFunc = func(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
+	return schema.GroupVersionResource{
+		Group:    "rules.example.com",
+		Version:  "v1",
+		Resource: "replicalimits",
+	}, nil
+}
+
 func validPolicyBindings() []*admissionregistration.ValidatingAdmissionPolicyBinding {
 	denyAction := admissionregistration.DenyAction
 	return []*admissionregistration.ValidatingAdmissionPolicyBinding{

staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer.go

@@ -1,152 +0,0 @@
-/*
-Copyright 2023 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package validating
-
-import (
-	"context"
-	"encoding/json"
-	"sort"
-	"strings"
-
-	"k8s.io/apimachinery/pkg/fields"
-	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apiserver/pkg/authentication/user"
-	"k8s.io/apiserver/pkg/authorization/authorizer"
-)
-
-type authzResult struct {
-	authorized authorizer.Decision
-	reason     string
-	err        error
-}
-
-type cachingAuthorizer struct {
-	authorizer authorizer.Authorizer
-	decisions  map[string]authzResult
-}
-
-func newCachingAuthorizer(in authorizer.Authorizer) authorizer.Authorizer {
-	return &cachingAuthorizer{
-		authorizer: in,
-		decisions:  make(map[string]authzResult),
-	}
-}
-
-// The attribute accessors known to cache key construction. If this fails to compile, the cache
-// implementation may need to be updated.
-var _ authorizer.Attributes = (interface {
-	GetUser() user.Info
-	GetVerb() string
-	IsReadOnly() bool
-	GetNamespace() string
-	GetResource() string
-	GetSubresource() string
-	GetName() string
-	GetAPIGroup() string
-	GetAPIVersion() string
-	IsResourceRequest() bool
-	GetPath() string
-	GetFieldSelector() (fields.Requirements, error)
-	GetLabelSelector() (labels.Requirements, error)
-})(nil)
-
-// The user info accessors known to cache key construction. If this fails to compile, the cache
-// implementation may need to be updated.
-var _ user.Info = (interface {
-	GetName() string
-	GetUID() string
-	GetGroups() []string
-	GetExtra() map[string][]string
-})(nil)
-
-// Authorize returns an authorization decision by delegating to another Authorizer. If an equivalent
-// check has already been performed, a cached result is returned. Not safe for concurrent use.
-func (ca *cachingAuthorizer) Authorize(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
-	type SerializableAttributes struct {
-		authorizer.AttributesRecord
-		LabelSelector string
-	}
-
-	serializableAttributes := SerializableAttributes{
-		AttributesRecord: authorizer.AttributesRecord{
-			Verb:            a.GetVerb(),
-			Namespace:       a.GetNamespace(),
-			APIGroup:        a.GetAPIGroup(),
-			APIVersion:      a.GetAPIVersion(),
-			Resource:        a.GetResource(),
-			Subresource:     a.GetSubresource(),
-			Name:            a.GetName(),
-			ResourceRequest: a.IsResourceRequest(),
-			Path:            a.GetPath(),
-		},
-	}
-	// in the error case, we won't honor this field selector, so the cache doesn't need it.
-	if fieldSelector, err := a.GetFieldSelector(); len(fieldSelector) > 0 {
-		serializableAttributes.FieldSelectorRequirements, serializableAttributes.FieldSelectorParsingErr = fieldSelector, err
-	}
-	if labelSelector, _ := a.GetLabelSelector(); len(labelSelector) > 0 {
-		// the labels requirements have private elements so those don't help us serialize to a unique key
-		serializableAttributes.LabelSelector = labelSelector.String()
-	}
-
-	if u := a.GetUser(); u != nil {
-		di := &user.DefaultInfo{
-			Name: u.GetName(),
-			UID:  u.GetUID(),
-		}
-
-		// Differently-ordered groups or extras could cause otherwise-equivalent checks to
-		// have distinct cache keys.
-		if groups := u.GetGroups(); len(groups) > 0 {
-			di.Groups = make([]string, len(groups))
-			copy(di.Groups, groups)
-			sort.Strings(di.Groups)
-		}
-
-		if extra := u.GetExtra(); len(extra) > 0 {
-			di.Extra = make(map[string][]string, len(extra))
-			for k, vs := range extra {
-				vdupe := make([]string, len(vs))
-				copy(vdupe, vs)
-				sort.Strings(vdupe)
-				di.Extra[k] = vdupe
-			}
-		}
-
-		serializableAttributes.User = di
-	}
-
-	var b strings.Builder
-	if err := json.NewEncoder(&b).Encode(serializableAttributes); err != nil {
-		return authorizer.DecisionNoOpinion, "", err
-	}
-	key := b.String()
-
-	if cached, ok := ca.decisions[key]; ok {
-		return cached.authorized, cached.reason, cached.err
-	}
-
-	authorized, reason, err := ca.authorizer.Authorize(ctx, a)
-
-	ca.decisions[key] = authzResult{
-		authorized: authorized,
-		reason:     reason,
-		err:        err,
-	}
-
-	return authorized, reason, err
-}

staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer_test.go

@@ -1,523 +0,0 @@
-/*
-Copyright 2023 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package validating
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"testing"
-
-	"k8s.io/apimachinery/pkg/fields"
-	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apiserver/pkg/authentication/user"
-	"k8s.io/apiserver/pkg/authorization/authorizer"
-	genericfeatures "k8s.io/apiserver/pkg/features"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	featuregatetesting "k8s.io/component-base/featuregate/testing"
-)
-
-func mustParseLabelSelector(str string) labels.Requirements {
-	ret, err := labels.Parse(str)
-	if err != nil {
-		panic(err)
-	}
-	retRequirements, _ /*selectable*/ := ret.Requirements()
-	return retRequirements
-}
-
-func TestCachingAuthorizer(t *testing.T) {
-	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true)
-
-	type result struct {
-		decision authorizer.Decision
-		reason   string
-		error    error
-	}
-
-	type invocation struct {
-		attributes authorizer.Attributes
-		expected   result
-	}
-
-	for _, tc := range []struct {
-		name    string
-		calls   []invocation
-		backend []result
-	}{
-		{
-			name: "hit",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{Name: "test name"},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					attributes: authorizer.AttributesRecord{Name: "test name"},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-			},
-		},
-		{
-			name: "hit with differently-ordered groups",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						User: &user.DefaultInfo{
-							Groups: []string{"a", "b", "c"},
-						},
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					attributes: authorizer.AttributesRecord{
-						User: &user.DefaultInfo{
-							Groups: []string{"c", "b", "a"},
-						},
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-			},
-		},
-		{
-			name: "hit with differently-ordered extra",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						User: &user.DefaultInfo{
-							Extra: map[string][]string{
-								"k": {"a", "b", "c"},
-							},
-						},
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					attributes: authorizer.AttributesRecord{
-						User: &user.DefaultInfo{
-							Extra: map[string][]string{
-								"k": {"c", "b", "a"},
-							},
-						},
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-			},
-		},
-		{
-			name: "miss due to different name",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{Name: "alpha"},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason alpha",
-						error:    fmt.Errorf("test error alpha"),
-					},
-				},
-				{
-					attributes: authorizer.AttributesRecord{Name: "beta"},
-					expected: result{
-						decision: authorizer.DecisionDeny,
-						reason:   "test reason beta",
-						error:    fmt.Errorf("test error beta"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason alpha",
-					error:    fmt.Errorf("test error alpha"),
-				},
-				{
-					decision: authorizer.DecisionDeny,
-					reason:   "test reason beta",
-					error:    fmt.Errorf("test error beta"),
-				},
-			},
-		},
-		{
-			name: "miss due to different user",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						User: &user.DefaultInfo{Name: "alpha"},
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason alpha",
-						error:    fmt.Errorf("test error alpha"),
-					},
-				},
-				{
-					attributes: authorizer.AttributesRecord{
-						User: &user.DefaultInfo{Name: "beta"},
-					},
-					expected: result{
-						decision: authorizer.DecisionDeny,
-						reason:   "test reason beta",
-						error:    fmt.Errorf("test error beta"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason alpha",
-					error:    fmt.Errorf("test error alpha"),
-				},
-				{
-					decision: authorizer.DecisionDeny,
-					reason:   "test reason beta",
-					error:    fmt.Errorf("test error beta"),
-				},
-			},
-		},
-		{
-			name: "honor good field selector",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						Name:                      "test name",
-						FieldSelectorRequirements: fields.ParseSelectorOrDie("foo=bar").Requirements(),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					attributes: authorizer.AttributesRecord{
-						Name: "test name",
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason 2",
-						error:    fmt.Errorf("test error 2"),
-					},
-				},
-				{
-					// now this should be cached
-					attributes: authorizer.AttributesRecord{
-						Name:                      "test name",
-						FieldSelectorRequirements: fields.ParseSelectorOrDie("foo=bar").Requirements(),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason 2",
-					error:    fmt.Errorf("test error 2"),
-				},
-			},
-		},
-		{
-			name: "ignore malformed field selector first",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						Name:                    "test name",
-						FieldSelectorParsingErr: errors.New("malformed"),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					// notice that this does not have the malformed field selector.
-					// it should use the cached result
-					attributes: authorizer.AttributesRecord{
-						Name: "test name",
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-			},
-		},
-		{
-			name: "ignore malformed field selector second",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						Name: "test name",
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					// this should use the broader cached value because the selector will be ignored
-					attributes: authorizer.AttributesRecord{
-						Name:                    "test name",
-						FieldSelectorParsingErr: errors.New("malformed"),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-			},
-		},
-
-		{
-			name: "honor good label selector",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						Name:                      "test name",
-						LabelSelectorRequirements: mustParseLabelSelector("foo=bar"),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					attributes: authorizer.AttributesRecord{
-						Name: "test name",
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason 2",
-						error:    fmt.Errorf("test error 2"),
-					},
-				},
-				{
-					// now this should be cached
-					attributes: authorizer.AttributesRecord{
-						Name:                      "test name",
-						LabelSelectorRequirements: mustParseLabelSelector("foo=bar"),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					attributes: authorizer.AttributesRecord{
-						Name:                      "test name",
-						LabelSelectorRequirements: mustParseLabelSelector("diff=zero"),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason 3",
-						error:    fmt.Errorf("test error 3"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason 2",
-					error:    fmt.Errorf("test error 2"),
-				},
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason 3",
-					error:    fmt.Errorf("test error 3"),
-				},
-			},
-		},
-		{
-			name: "ignore malformed label selector first",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						Name:                    "test name",
-						LabelSelectorParsingErr: errors.New("malformed mess"),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					// notice that this does not have the malformed field selector.
-					// it should use the cached result
-					attributes: authorizer.AttributesRecord{
-						Name: "test name",
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-			},
-		},
-		{
-			name: "ignore malformed label selector second",
-			calls: []invocation{
-				{
-					attributes: authorizer.AttributesRecord{
-						Name: "test name",
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-				{
-					// this should use the broader cached value because the selector will be ignored
-					attributes: authorizer.AttributesRecord{
-						Name:                    "test name",
-						LabelSelectorParsingErr: errors.New("malformed mess"),
-					},
-					expected: result{
-						decision: authorizer.DecisionAllow,
-						reason:   "test reason",
-						error:    fmt.Errorf("test error"),
-					},
-				},
-			},
-			backend: []result{
-				{
-					decision: authorizer.DecisionAllow,
-					reason:   "test reason",
-					error:    fmt.Errorf("test error"),
-				},
-			},
-		},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			var misses int
-			frontend := newCachingAuthorizer(func() authorizer.Authorizer {
-				return authorizer.AuthorizerFunc(func(_ context.Context, attributes authorizer.Attributes) (authorizer.Decision, string, error) {
-					if misses >= len(tc.backend) {
-						t.Fatalf("got more than expected %d backend invocations", len(tc.backend))
-					}
-					result := tc.backend[misses]
-					misses++
-					return result.decision, result.reason, result.error
-				})
-			}())
-
-			for i, invocation := range tc.calls {
-				decision, reason, err := frontend.Authorize(context.TODO(), invocation.attributes)
-				if decision != invocation.expected.decision {
-					t.Errorf("(call %d of %d) expected decision %v, got %v", i+1, len(tc.calls), invocation.expected.decision, decision)
-				}
-				if reason != invocation.expected.reason {
-					t.Errorf("(call %d of %d) expected reason %q, got %q", i+1, len(tc.calls), invocation.expected.reason, reason)
-				}
-				if err.Error() != invocation.expected.error.Error() {
-					t.Errorf("(call %d of %d) expected error %q, got %q", i+1, len(tc.calls), invocation.expected.error.Error(), err.Error())
-				}
-			}
-
-			if len(tc.backend) > misses {
-				t.Errorf("expected %d backend invocations, got %d", len(tc.backend), misses)
-			}
-		})
-	}
-}

staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator.go

@@ -41,13 +41,13 @@ import (
 // validator implements the Validator interface
 type validator struct {
 	celMatcher            matchconditions.Matcher
-	validationFilter      cel.Filter
-	auditAnnotationFilter cel.Filter
-	messageFilter         cel.Filter
+	validationFilter      cel.ConditionEvaluator
+	auditAnnotationFilter cel.ConditionEvaluator
+	messageFilter         cel.ConditionEvaluator
 	failPolicy            *v1.FailurePolicyType
 }
 
-func NewValidator(validationFilter cel.Filter, celMatcher matchconditions.Matcher, auditAnnotationFilter, messageFilter cel.Filter, failPolicy *v1.FailurePolicyType) Validator {
+func NewValidator(validationFilter cel.ConditionEvaluator, celMatcher matchconditions.Matcher, auditAnnotationFilter, messageFilter cel.ConditionEvaluator, failPolicy *v1.FailurePolicyType) Validator {
 	return &validator{
 		celMatcher:            celMatcher,
 		validationFilter:      validationFilter,

staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator_test.go

@@ -41,7 +41,7 @@ import (
 	"k8s.io/apiserver/pkg/cel/environment"
 )
 
-var _ cel.Filter = &fakeCelFilter{}
+var _ cel.ConditionEvaluator = &fakeCelFilter{}
 
 type fakeCelFilter struct {
 	evaluations []cel.EvaluationResult
@@ -932,8 +932,8 @@ func TestContextCanceled(t *testing.T) {
 
 	fakeAttr := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "default", "foo", schema.GroupVersionResource{}, "", admission.Create, nil, false, nil)
 	fakeVersionedAttr, _ := admission.NewVersionedAttributes(fakeAttr, schema.GroupVersionKind{}, nil)
-	fc := cel.NewFilterCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
-	f := fc.Compile([]cel.ExpressionAccessor{&ValidationCondition{Expression: "[1,2,3,4,5,6,7,8,9,10].map(x, [1,2,3,4,5,6,7,8,9,10].map(y, x*y)) == []"}}, cel.OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.StoredExpressions)
+	fc := cel.NewConditionCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
+	f := fc.CompileCondition([]cel.ExpressionAccessor{&ValidationCondition{Expression: "[1,2,3,4,5,6,7,8,9,10].map(x, [1,2,3,4,5,6,7,8,9,10].map(y, x*y)) == []"}}, cel.OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.StoredExpressions)
 	v := validator{
 		failPolicy:       &fail,
 		celMatcher:       &fakeCELMatcher{matches: true},