Merge pull request #123543 from jiahuif-forks/feature/validating-admission-policy/excluded-resources

ValidatingAdmissionPolicy: exclude brink-able resources.
This commit is contained in:
Kubernetes Prow Robot 2024-03-05 13:45:01 -08:00 committed by GitHub
commit df1eccae38
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 313 additions and 31 deletions

View File

@ -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 {

View File

@ -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)
}

View File

@ -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)
}
}

View File

@ -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)

View File

@ -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)

View File

@ -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)

View File

@ -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
}

View File

@ -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)
}

View File

@ -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)

View File

@ -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 {

View File

@ -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{

View File

@ -0,0 +1,106 @@
/*
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 cel
import (
"fmt"
"slices"
"strings"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/discovery"
apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
"k8s.io/kubernetes/pkg/kubeapiserver/admission/exclusion"
"k8s.io/kubernetes/test/integration/framework"
)
// TestExcludedResources is an open-box test to ensure that a resource that is not persisted
// is either in the allow-list or block-list of the CEL admission.
//
// see staging/src/k8s.io/apiserver/pkg/admission/exclusion/exclusion.go
// for the lists that the test is about.
func TestExcludedResources(t *testing.T) {
server, err := apiservertesting.StartTestServer(t,
nil, // no extra options
// enable all APIs to cover all defined resources
// note that it does not require feature flags enabled to
// discovery GVRs of disabled features.
[]string{"--runtime-config=api/all=true"},
framework.SharedEtcd())
if err != nil {
t.Fatal(err)
}
defer server.TearDownFn()
config := server.ClientConfig
discoveryClient, err := discovery.NewDiscoveryClientForConfig(config)
if err != nil {
t.Fatal(err)
}
_, resourceLists, _, err := discoveryClient.GroupsAndMaybeResources()
if err != nil {
t.Fatal(err)
}
interestedGRs := sets.New[schema.GroupResource]()
interestedVerbCombinations := []metav1.Verbs{
{"create"},
{"create", "get"},
}
for _, rl := range resourceLists {
for _, r := range rl.APIResources {
slices.Sort(r.Verbs)
for _, c := range interestedVerbCombinations {
if slices.Equal(r.Verbs, c) {
gv, err := schema.ParseGroupVersion(rl.GroupVersion)
if err != nil {
t.Fatalf("internal error: cannot parse GV from %q: %v", rl.GroupVersion, err)
}
interestedGRs.Insert(gv.WithResource(r.Name).GroupResource())
}
}
}
}
existing := sets.New[schema.GroupResource]()
existing.Insert(exclusion.Included()...)
existing.Insert(exclusion.Excluded()...)
shouldAdd, shouldRemove := interestedGRs.Difference(existing), existing.Difference(interestedGRs)
if shouldAdd.Len() > 0 {
t.Errorf("the following resources should either be in Included or Excluded in\n"+
"pkg/kubeapiserver/admission/exclusion/resources.go\n%s",
formatGRs(shouldAdd.UnsortedList()),
)
}
if shouldRemove.Len() > 0 {
t.Errorf("the following resources are in pkg/kubeapiserver/admission/exclusion/resources.go\n"+
"but does not seem to be transient.\n%s",
formatGRs(shouldRemove.UnsortedList()))
}
}
func formatGRs(grs []schema.GroupResource) string {
lines := make([]string, 0, len(grs))
for _, gvr := range grs {
item := fmt.Sprintf("%#v,", gvr)
lines = append(lines, item)
}
slices.Sort(lines)
return strings.Join(lines, "\n")
}

View File

@ -30,6 +30,7 @@ import (
"text/template"
"time"
authenticationv1 "k8s.io/api/authentication/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
"k8s.io/apiextensions-apiserver/test/integration/fixtures"
@ -39,6 +40,7 @@ import (
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/rest"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/utils/ptr"
"k8s.io/kubernetes/cmd/kube-apiserver/app/options"
apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
@ -1701,6 +1703,71 @@ func Test_ValidatingAdmissionPolicy_MatchWithMatchPolicyExact(t *testing.T) {
}
}
func Test_ValidatingAdmissionPolicy_MatchExcludedResource(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true)()
server, err := apiservertesting.StartTestServer(t, nil, []string{
"--enable-admission-plugins", "ValidatingAdmissionPolicy",
}, framework.SharedEtcd())
if err != nil {
t.Fatal(err)
}
defer server.TearDownFn()
config := server.ClientConfig
client, err := clientset.NewForConfig(config)
if err != nil {
t.Fatal(err)
}
policy := withValidations([]admissionregistrationv1beta1.Validation{
{
Expression: "false",
Message: "try to deny SelfSubjectReview",
},
}, withFailurePolicy(admissionregistrationv1beta1.Fail, makePolicy("match-excluded-resources")))
policy.Spec.MatchConstraints = &admissionregistrationv1beta1.MatchResources{
MatchPolicy: ptr.To(admissionregistrationv1beta1.Exact),
ResourceRules: []admissionregistrationv1beta1.NamedRuleWithOperations{
{
RuleWithOperations: admissionregistrationv1beta1.RuleWithOperations{
Operations: []admissionregistrationv1.OperationType{
"*",
},
Rule: admissionregistrationv1.Rule{
APIGroups: []string{
"authentication.k8s.io",
},
APIVersions: []string{
"v1",
},
Resources: []string{
"selfsubjectreviews",
},
},
},
},
},
}
policy = withWaitReadyConstraintAndExpression(policy)
if _, err := client.AdmissionregistrationV1beta1().ValidatingAdmissionPolicies().Create(context.Background(), policy, metav1.CreateOptions{}); err != nil {
t.Fatalf("fail to create policy: %v", err)
}
policyBinding := makeBinding("match-by-match-policy-exact-binding", "match-excluded-resources", "")
if err := createAndWaitReady(t, client, policyBinding, nil); err != nil {
t.Fatalf("fail to create and wait for binding: %v", err)
}
r, err := client.AuthenticationV1().SelfSubjectReviews().Create(context.Background(), &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{})
if err != nil {
t.Fatalf("unexpected denied SelfSubjectReview: %v", err)
}
// confidence check the returned user info.
if len(r.Status.UserInfo.UID) == 0 {
t.Errorf("unexpected invalid user info: %v", r.Status.UserInfo)
}
}
// Test_ValidatingAdmissionPolicy_PolicyDeletedThenRecreated validates that deleting a ValidatingAdmissionPolicy
// removes the policy from the apiserver admission chain and recreating it re-enables it.
func Test_ValidatingAdmissionPolicy_PolicyDeletedThenRecreated(t *testing.T) {