Merge pull request kubernetes#123543 from jiahuif-forks/feature/valid…

…ating-admission-policy/excluded-resources

ValidatingAdmissionPolicy: exclude brink-able resources.

pkg/kubeapiserver/admission/config.go

@@ -29,13 +29,14 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	webhookinit "k8s.io/apiserver/pkg/admission/plugin/webhook/initializer"
 	genericapiserver "k8s.io/apiserver/pkg/server"
-	egressselector "k8s.io/apiserver/pkg/server/egressselector"
+	"k8s.io/apiserver/pkg/server/egressselector"
 	"k8s.io/apiserver/pkg/util/webhook"
 	cacheddiscovery "k8s.io/client-go/discovery/cached/memory"
 	externalinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/restmapper"
+	"k8s.io/kubernetes/pkg/kubeapiserver/admission/exclusion"
 	quotainstall "k8s.io/kubernetes/pkg/quota/v1/install"
 )
 
@@ -69,6 +70,7 @@ func (c *Config) New(proxyTransport *http.Transport, egressSelector *egressselec
 		cloudConfig,
 		discoveryRESTMapper,
 		quotainstall.NewQuotaConfigurationForAdmission(),
+		exclusion.Excluded(),
 	)
 
 	admissionPostStartHook := func(context genericapiserver.PostStartHookContext) error {

pkg/kubeapiserver/admission/exclusion/resources.go

@@ -0,0 +1,66 @@
+/*
+Copyright 2024 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 exclusion
+
+import (
+	"slices"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+// include is the list of resources that the expression-based admission controllers
+// should intercept.
+// The version is omitted, all versions of the same GroupResource are treated the same.
+// If a resource is transient, i.e., not persisted in the storage, the resource must be
+// in either include or excluded list.
+var included = []schema.GroupResource{
+	{Group: "", Resource: "bindings"},
+	{Group: "", Resource: "pods/attach"},
+	{Group: "", Resource: "pods/binding"},
+	{Group: "", Resource: "pods/eviction"},
+	{Group: "", Resource: "pods/exec"},
+	{Group: "", Resource: "pods/portforward"},
+
+	// ref: https://github.com/kubernetes/kubernetes/issues/122205#issuecomment-1927390823
+	{Group: "", Resource: "serviceaccounts/token"},
+}
+
+// excluded is the list of resources that the expression-based admission controllers
+// should ignore.
+// The version is omitted, all versions of the same GroupResource are treated the same.
+var excluded = []schema.GroupResource{
+	// BEGIN interception of these non-persisted resources may break the cluster
+	{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"},
+	{Group: "authentication.k8s.io", Resource: "tokenreviews"},
+	{Group: "authorization.k8s.io", Resource: "localsubjectaccessreviews"},
+	{Group: "authorization.k8s.io", Resource: "selfsubjectaccessreviews"},
+	{Group: "authorization.k8s.io", Resource: "selfsubjectrulesreviews"},
+	{Group: "authorization.k8s.io", Resource: "subjectaccessreviews"},
+	// END interception of these non-persisted resources may break the cluster
+}
+
+// Included returns a copy of the list of resources that the expression-based admission controllers
+// should intercept.
+func Included() []schema.GroupResource {
+	return slices.Clone(included)
+}
+
+// Excluded returns a copy of the list of resources that the expression-based admission controllers
+// should ignore.
+func Excluded() []schema.GroupResource {
+	return slices.Clone(excluded)
+}

pkg/kubeapiserver/admission/initializer.go

@@ -18,6 +18,7 @@ package admission
 
 import (
 	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/initializer"
 	quota "k8s.io/apiserver/pkg/quota/v1"
@@ -32,9 +33,10 @@ type WantsCloudConfig interface {
 
 // PluginInitializer is used for initialization of the Kubernetes specific admission plugins.
 type PluginInitializer struct {
-	cloudConfig        []byte
-	restMapper         meta.RESTMapper
-	quotaConfiguration quota.Configuration
+	cloudConfig                []byte
+	restMapper                 meta.RESTMapper
+	quotaConfiguration         quota.Configuration
+	excludedAdmissionResources []schema.GroupResource
 }
 
 var _ admission.PluginInitializer = &PluginInitializer{}
@@ -46,11 +48,13 @@ func NewPluginInitializer(
 	cloudConfig []byte,
 	restMapper meta.RESTMapper,
 	quotaConfiguration quota.Configuration,
+	excludedAdmissionResources []schema.GroupResource,
 ) *PluginInitializer {
 	return &PluginInitializer{
-		cloudConfig:        cloudConfig,
-		restMapper:         restMapper,
-		quotaConfiguration: quotaConfiguration,
+		cloudConfig:                cloudConfig,
+		restMapper:                 restMapper,
+		quotaConfiguration:         quotaConfiguration,
+		excludedAdmissionResources: excludedAdmissionResources,
 	}
 }
 
@@ -68,4 +72,8 @@ func (i *PluginInitializer) Initialize(plugin admission.Interface) {
 	if wants, ok := plugin.(initializer.WantsQuotaConfiguration); ok {
 		wants.SetQuotaConfiguration(i.quotaConfiguration)
 	}
+
+	if wants, ok := plugin.(initializer.WantsExcludedAdmissionResources); ok {
+		wants.SetExcludedAdmissionResources(i.excludedAdmissionResources)
+	}
 }

pkg/kubeapiserver/admission/initializer_test.go

@@ -49,7 +49,7 @@ func (p *WantsCloudConfigAdmissionPlugin) SetCloudConfig(cloudConfig []byte) {
 
 func TestCloudConfigAdmissionPlugin(t *testing.T) {
 	cloudConfig := []byte("cloud-configuration")
-	initializer := NewPluginInitializer(cloudConfig, nil, nil)
+	initializer := NewPluginInitializer(cloudConfig, nil, nil, nil)
 	wantsCloudConfigAdmission := &WantsCloudConfigAdmissionPlugin{}
 	initializer.Initialize(wantsCloudConfigAdmission)
 
@@ -94,7 +94,7 @@ func (p *WantsRESTMapperAdmissionPlugin) SetRESTMapper(mapper meta.RESTMapper) {
 
 func TestRESTMapperAdmissionPlugin(t *testing.T) {
 	mapper := doNothingRESTMapper{}
-	initializer := NewPluginInitializer(nil, mapper, nil)
+	initializer := NewPluginInitializer(nil, mapper, nil, nil)
 	wantsRESTMapperAdmission := &WantsRESTMapperAdmissionPlugin{}
 	initializer.Initialize(wantsRESTMapperAdmission)
 
@@ -121,7 +121,7 @@ func (p *WantsQuotaConfigurationAdmissionPlugin) SetQuotaConfiguration(config qu
 
 func TestQuotaConfigurationAdmissionPlugin(t *testing.T) {
 	config := doNothingQuotaConfiguration{}
-	initializer := NewPluginInitializer(nil, nil, config)
+	initializer := NewPluginInitializer(nil, nil, config, nil)
 	wantsQuotaConfigurationAdmission := &WantsQuotaConfigurationAdmissionPlugin{}
 	initializer.Initialize(wantsQuotaConfigurationAdmission)
 

plugin/pkg/admission/gc/gc_admission_test.go

@@ -139,7 +139,7 @@ func newGCPermissionsEnforcement() (*gcPermissionsEnforcement, error) {
 		return nil, fmt.Errorf("unexpected error while constructing resource list from fake discovery client: %v", err)
 	}
 	restMapper := restmapper.NewDiscoveryRESTMapper(restMapperRes)
-	pluginInitializer := kubeadmission.NewPluginInitializer(nil, restMapper, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(nil, restMapper, nil, nil)
 	initializersChain := admission.PluginInitializers{}
 	initializersChain = append(initializersChain, genericPluginInitializer)
 	initializersChain = append(initializersChain, pluginInitializer)

plugin/pkg/admission/resourcequota/admission_test.go

@@ -115,7 +115,7 @@ func createHandlerWithConfig(kubeClient kubernetes.Interface, informerFactory in
 
 	initializers := admission.PluginInitializers{
 		genericadmissioninitializer.New(kubeClient, nil, informerFactory, nil, nil, stopCh),
-		kubeapiserveradmission.NewPluginInitializer(nil, nil, quotaConfiguration),
+		kubeapiserveradmission.NewPluginInitializer(nil, nil, quotaConfiguration, nil),
 	}
 	initializers.Initialize(handler)
 

staging/src/k8s.io/apiserver/pkg/admission/initializer/interfaces.go

@@ -18,6 +18,7 @@ package initializer
 
 import (
 	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/cel/openapi/resolver"
@@ -89,3 +90,10 @@ type WantsSchemaResolver interface {
 	SetSchemaResolver(resolver resolver.SchemaResolver)
 	admission.InitializationValidator
 }
+
+// WantsExcludedAdmissionResources defines a function which sets the ExcludedAdmissionResources
+// for an admission plugin that needs it.
+type WantsExcludedAdmissionResources interface {
+	SetExcludedAdmissionResources(excludedAdmissionResources []schema.GroupResource)
+	admission.InitializationValidator
+}

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

@@ -21,8 +21,11 @@ import (
 	"errors"
 	"fmt"
 
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/initializer"
 	"k8s.io/apiserver/pkg/admission/plugin/policy/matching"
@@ -36,6 +39,15 @@ import (
 type sourceFactory[H any] func(informers.SharedInformerFactory, kubernetes.Interface, dynamic.Interface, meta.RESTMapper) Source[H]
 type dispatcherFactory[H any] func(authorizer.Authorizer, *matching.Matcher) Dispatcher[H]
 
+// admissionResources is the list of resources related to CEL-based admission
+// features.
+var admissionResources = []schema.GroupResource{
+	{Group: admissionregistrationv1.GroupName, Resource: "validatingadmissionpolicies"},
+	{Group: admissionregistrationv1.GroupName, Resource: "validatingadmissionpolicybindings"},
+	{Group: admissionregistrationv1.GroupName, Resource: "mutatingadmissionpolicies"},
+	{Group: admissionregistrationv1.GroupName, Resource: "mutatingadmissionpolicybindings"},
+}
+
 // AdmissionPolicyManager is an abstract admission plugin with all the
 // infrastructure to define Admit or Validate on-top.
 type Plugin[H any] struct {
@@ -48,13 +60,14 @@ type Plugin[H any] struct {
 	dispatcher Dispatcher[H]
 	matcher    *matching.Matcher
 
-	informerFactory informers.SharedInformerFactory
-	client          kubernetes.Interface
-	restMapper      meta.RESTMapper
-	dynamicClient   dynamic.Interface
-	stopCh          <-chan struct{}
-	authorizer      authorizer.Authorizer
-	enabled         bool
+	informerFactory   informers.SharedInformerFactory
+	client            kubernetes.Interface
+	restMapper        meta.RESTMapper
+	dynamicClient     dynamic.Interface
+	excludedResources sets.Set[schema.GroupResource]
+	stopCh            <-chan struct{}
+	authorizer        authorizer.Authorizer
+	enabled           bool
 }
 
 var (
@@ -64,6 +77,7 @@ var (
 	_ initializer.WantsDynamicClient               = &Plugin[any]{}
 	_ initializer.WantsDrainedNotification         = &Plugin[any]{}
 	_ initializer.WantsAuthorizer                  = &Plugin[any]{}
+	_ initializer.WantsExcludedAdmissionResources  = &Plugin[any]{}
 	_ admission.InitializationValidator            = &Plugin[any]{}
 )
 
@@ -76,6 +90,9 @@ func NewPlugin[H any](
 		Handler:           handler,
 		sourceFactory:     sourceFactory,
 		dispatcherFactory: dispatcherFactory,
+
+		// always exclude admission/mutating policies and bindings
+		excludedResources: sets.New(admissionResources...),
 	}
 }
 
@@ -111,6 +128,10 @@ func (c *Plugin[H]) SetEnabled(enabled bool) {
 	c.enabled = enabled
 }
 
+func (c *Plugin[H]) SetExcludedAdmissionResources(excludedResources []schema.GroupResource) {
+	c.excludedResources.Insert(excludedResources...)
+}
+
 // ValidateInitialization - once clientset and informer factory are provided, creates and starts the admission controller
 func (c *Plugin[H]) ValidateInitialization() error {
 	// By default enabled is set to false. It is up to types which embed this
@@ -177,7 +198,7 @@ func (c *Plugin[H]) Dispatch(
 ) (err error) {
 	if !c.enabled {
 		return nil
-	} else if isPolicyResource(a) {
+	} else if c.shouldIgnoreResource(a) {
 		return nil
 	} else if !c.WaitForReady() {
 		return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request"))
@@ -186,14 +207,9 @@ func (c *Plugin[H]) Dispatch(
 	return c.dispatcher.Dispatch(ctx, a, o, c.source.Hooks())
 }
 
-func isPolicyResource(attr admission.Attributes) bool {
-	gvk := attr.GetResource()
-	if gvk.Group == "admissionregistration.k8s.io" {
-		if gvk.Resource == "validatingadmissionpolicies" || gvk.Resource == "validatingadmissionpolicybindings" {
-			return true
-		} else if gvk.Resource == "mutatingadmissionpolicies" || gvk.Resource == "mutatingadmissionpolicybindings" {
-			return true
-		}
-	}
-	return false
+func (c *Plugin[H]) shouldIgnoreResource(attr admission.Attributes) bool {
+	gvr := attr.GetResource()
+	// exclusion decision ignores the version.
+	gr := gvr.GroupResource()
+	return c.excludedResources.Has(gr)
 }

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

@@ -74,6 +74,7 @@ type Plugin struct {
 var _ admission.Interface = &Plugin{}
 var _ admission.ValidationInterface = &Plugin{}
 var _ initializer.WantsFeatures = &Plugin{}
+var _ initializer.WantsExcludedAdmissionResources = &Plugin{}
 
 func NewPlugin(_ io.Reader) *Plugin {
 	handler := admission.NewHandler(admission.Connect, admission.Create, admission.Delete, admission.Update)

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

@@ -80,7 +80,6 @@ func (v *validator) Validate(ctx context.Context, matchedResource schema.GroupVe
 	} else {
 		f = *v.failPolicy
 	}
-
 	if v.celMatcher != nil {
 		matchResults := v.celMatcher.Match(ctx, versionedAttr, versionedParams, authz)
 		if matchResults.Error != nil {

test/integration/apiserver/cel/admission_test_util.go

@@ -143,6 +143,15 @@ var (
 		gvr("admissionregistration.k8s.io", "v1beta1", "validatingadmissionpolicies"):         true,
 		gvr("admissionregistration.k8s.io", "v1beta1", "validatingadmissionpolicies/status"):  true,
 		gvr("admissionregistration.k8s.io", "v1beta1", "validatingadmissionpolicybindings"):   true,
+		// transient resource exemption
+		gvr("authentication.k8s.io", "v1", "selfsubjectreviews"):       true,
+		gvr("authentication.k8s.io", "v1beta1", "selfsubjectreviews"):  true,
+		gvr("authentication.k8s.io", "v1alpha1", "selfsubjectreviews"): true,
+		gvr("authentication.k8s.io", "v1", "tokenreviews"):             true,
+		gvr("authorization.k8s.io", "v1", "localsubjectaccessreviews"): true,
+		gvr("authorization.k8s.io", "v1", "selfsubjectaccessreviews"):  true,
+		gvr("authorization.k8s.io", "v1", "selfsubjectrulesreviews"):   true,
+		gvr("authorization.k8s.io", "v1", "subjectaccessreviews"):      true,
 	}
 
 	parentResources = map[schema.GroupVersionResource]schema.GroupVersionResource{