From 64ee859aa82c17daa8037e4e90e066ae4582d653 Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Wed, 28 Feb 2024 15:31:44 -0800 Subject: [PATCH] make ValidatingAdmissionPolicy ignore excluded resources. --- .../admission/plugin/policy/generic/plugin.go | 9 ++- .../plugin/policy/validating/plugin.go | 1 + .../plugin/policy/validating/validator.go | 1 - .../cel/validatingadmissionpolicy_test.go | 67 +++++++++++++++++++ 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/generic/plugin.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/generic/plugin.go index 6dafd014245..3e6faa644b6 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/generic/plugin.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/generic/plugin.go @@ -26,6 +26,7 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/initializer" "k8s.io/apiserver/pkg/admission/plugin/policy/matching" + "k8s.io/apiserver/pkg/admission/resourcefilter" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/client-go/dynamic" "k8s.io/client-go/informers" @@ -52,6 +53,7 @@ type Plugin[H any] struct { client kubernetes.Interface restMapper meta.RESTMapper dynamicClient dynamic.Interface + resourceFilter resourcefilter.Interface // optional stopCh <-chan struct{} authorizer authorizer.Authorizer enabled bool @@ -64,6 +66,7 @@ var ( _ initializer.WantsDynamicClient = &Plugin[any]{} _ initializer.WantsDrainedNotification = &Plugin[any]{} _ initializer.WantsAuthorizer = &Plugin[any]{} + _ initializer.WantsResourceFilter = &Plugin[any]{} _ admission.InitializationValidator = &Plugin[any]{} ) @@ -111,6 +114,10 @@ func (c *Plugin[H]) SetEnabled(enabled bool) { c.enabled = enabled } +func (c *Plugin[H]) SetResourceFilter(filter resourcefilter.Interface) { + c.resourceFilter = filter +} + // 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 +184,7 @@ func (c *Plugin[H]) Dispatch( ) (err error) { if !c.enabled { return nil - } else if isPolicyResource(a) { + } else if isPolicyResource(a) || (c.resourceFilter != nil && !c.resourceFilter.ShouldHandle(a)) { return nil } else if !c.WaitForReady() { return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request")) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go index 56b22fc04f2..5880af4fe9b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go +++ b/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.WantsResourceFilter = &Plugin{} func NewPlugin(_ io.Reader) *Plugin { handler := admission.NewHandler(admission.Connect, admission.Create, admission.Delete, admission.Update) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator.go index 64d5fdc476a..da2ea1c10f8 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator.go +++ b/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 { diff --git a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go index c638e0b3848..9ee7594a3d1 100644 --- a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go +++ b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go @@ -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) {