diff --git a/cmd/kube-controller-manager/app/validatingadmissionpolicystatus.go b/cmd/kube-controller-manager/app/validatingadmissionpolicystatus.go index 9f4c78c52ba..2fd14589f5f 100644 --- a/cmd/kube-controller-manager/app/validatingadmissionpolicystatus.go +++ b/cmd/kube-controller-manager/app/validatingadmissionpolicystatus.go @@ -19,10 +19,11 @@ package app import ( "context" + apiextensionsscheme "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme" pluginvalidatingadmissionpolicy "k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy" "k8s.io/apiserver/pkg/cel/openapi/resolver" genericfeatures "k8s.io/apiserver/pkg/features" - "k8s.io/client-go/kubernetes/scheme" + k8sscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/component-base/featuregate" "k8s.io/controller-manager/controller" "k8s.io/kubernetes/cmd/kube-controller-manager/names" @@ -42,8 +43,12 @@ func newValidatingAdmissionPolicyStatusControllerDescriptor() *ControllerDescrip func startValidatingAdmissionPolicyStatusController(ctx context.Context, controllerContext ControllerContext, controllerName string) (controller.Interface, bool, error) { // KCM won't start the controller without the feature gate set. + + schemaResolver := resolver.NewDefinitionsSchemaResolver(openapi.GetOpenAPIDefinitions, k8sscheme.Scheme, apiextensionsscheme.Scheme). + Combine(&resolver.ClientDiscoveryResolver{Discovery: controllerContext.ClientBuilder.DiscoveryClientOrDie(names.ValidatingAdmissionPolicyStatusController)}) + typeChecker := &pluginvalidatingadmissionpolicy.TypeChecker{ - SchemaResolver: resolver.NewDefinitionsSchemaResolver(scheme.Scheme, openapi.GetOpenAPIDefinitions), + SchemaResolver: schemaResolver, RestMapper: controllerContext.RESTMapper, } c, err := validatingadmissionpolicystatus.NewController( diff --git a/pkg/controller/validatingadmissionpolicystatus/controller_test.go b/pkg/controller/validatingadmissionpolicystatus/controller_test.go index 6eb684e74c6..74af1be10a7 100644 --- a/pkg/controller/validatingadmissionpolicystatus/controller_test.go +++ b/pkg/controller/validatingadmissionpolicystatus/controller_test.go @@ -103,7 +103,7 @@ func TestTypeChecking(t *testing.T) { client := fake.NewSimpleClientset(policy) informerFactory := informers.NewSharedInformerFactory(client, 0) typeChecker := &validatingadmissionpolicy.TypeChecker{ - SchemaResolver: resolver.NewDefinitionsSchemaResolver(scheme.Scheme, openapi.GetOpenAPIDefinitions), + SchemaResolver: resolver.NewDefinitionsSchemaResolver(openapi.GetOpenAPIDefinitions, scheme.Scheme), RestMapper: testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme), } controller, err := NewController( diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/typechecking.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/typechecking.go index 6d73e237b07..86c8479c3a4 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/typechecking.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/typechecking.go @@ -238,7 +238,7 @@ func (c *TypeChecker) typesToCheck(p *v1beta1.ValidatingAdmissionPolicy) []schem if p.Spec.MatchConstraints == nil || len(p.Spec.MatchConstraints.ResourceRules) == 0 { return nil } - + restMapperRefreshAttempted := false // at most once per policy, refresh RESTMapper and retry resolution. for _, rule := range p.Spec.MatchConstraints.ResourceRules { groups := extractGroups(&rule.Rule) if len(groups) == 0 { @@ -268,7 +268,16 @@ func (c *TypeChecker) typesToCheck(p *v1beta1.ValidatingAdmissionPolicy) []schem } resolved, err := c.RestMapper.KindsFor(gvr) if err != nil { - continue + if restMapperRefreshAttempted { + // RESTMapper refresh happens at most once per policy + continue + } + c.tryRefreshRESTMapper() + restMapperRefreshAttempted = true + resolved, err = c.RestMapper.KindsFor(gvr) + if err != nil { + continue + } } for _, r := range resolved { if !r.Empty() { @@ -344,6 +353,13 @@ func sortGVKList(list []schema.GroupVersionKind) []schema.GroupVersionKind { return list } +// tryRefreshRESTMapper refreshes the RESTMapper if it supports refreshing. +func (c *TypeChecker) tryRefreshRESTMapper() { + if r, ok := c.RestMapper.(meta.ResettableRESTMapper); ok { + r.Reset() + } +} + func buildEnv(hasParams bool, hasAuthorizer bool, types typeOverwrite) (*cel.Env, error) { baseEnv := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()) requestType := plugincel.BuildRequestType() diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/combined.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/combined.go new file mode 100644 index 00000000000..8a1832097e3 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/combined.go @@ -0,0 +1,45 @@ +/* +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 resolver + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/kube-openapi/pkg/validation/spec" +) + +// Combine combines the DefinitionsSchemaResolver with a secondary schema resolver. +// The resulting schema resolver uses the DefinitionsSchemaResolver for a GVK that DefinitionsSchemaResolver knows, +// and the secondary otherwise. +func (d *DefinitionsSchemaResolver) Combine(secondary SchemaResolver) SchemaResolver { + return &combinedSchemaResolver{definitions: d, secondary: secondary} +} + +type combinedSchemaResolver struct { + definitions *DefinitionsSchemaResolver + secondary SchemaResolver +} + +// ResolveSchema takes a GroupVersionKind (GVK) and returns the OpenAPI schema +// identified by the GVK. +// If the DefinitionsSchemaResolver knows the gvk, the DefinitionsSchemaResolver handles the resolution, +// otherwise, the secondary does. +func (r *combinedSchemaResolver) ResolveSchema(gvk schema.GroupVersionKind) (*spec.Schema, error) { + if _, ok := r.definitions.gvkToSchema[gvk]; ok { + return r.definitions.ResolveSchema(gvk) + } + return r.secondary.ResolveSchema(gvk) +} diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/definitions.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/definitions.go index df7357f7785..9641a2421e3 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/definitions.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/definitions.go @@ -35,11 +35,11 @@ type DefinitionsSchemaResolver struct { // NewDefinitionsSchemaResolver creates a new DefinitionsSchemaResolver. // An example working setup: -// scheme = "k8s.io/client-go/kubernetes/scheme".Scheme // getDefinitions = "k8s.io/kubernetes/pkg/generated/openapi".GetOpenAPIDefinitions -func NewDefinitionsSchemaResolver(scheme *runtime.Scheme, getDefinitions common.GetOpenAPIDefinitions) *DefinitionsSchemaResolver { +// scheme = "k8s.io/client-go/kubernetes/scheme".Scheme +func NewDefinitionsSchemaResolver(getDefinitions common.GetOpenAPIDefinitions, schemes ...*runtime.Scheme) *DefinitionsSchemaResolver { gvkToSchema := make(map[schema.GroupVersionKind]*spec.Schema) - namer := openapi.NewDefinitionNamer(scheme) + namer := openapi.NewDefinitionNamer(schemes...) defs := getDefinitions(func(path string) spec.Ref { return spec.MustCreateRef(path) }) diff --git a/test/e2e/apimachinery/validatingadmissionpolicy.go b/test/e2e/apimachinery/validatingadmissionpolicy.go index 836e2cfb1a5..5aaf91c0dc8 100644 --- a/test/e2e/apimachinery/validatingadmissionpolicy.go +++ b/test/e2e/apimachinery/validatingadmissionpolicy.go @@ -28,11 +28,15 @@ import ( admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/features" clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/openapi3" "k8s.io/kubernetes/test/e2e/framework" admissionapi "k8s.io/pod-security-admission/api" ) @@ -42,6 +46,7 @@ var _ = SIGDescribe("ValidatingAdmissionPolicy [Privileged:ClusterAdmin]", frame f.NamespacePodSecurityLevel = admissionapi.LevelBaseline var client clientset.Interface + var extensionsClient apiextensionsclientset.Interface ginkgo.BeforeEach(func() { var err error @@ -52,6 +57,8 @@ var _ = SIGDescribe("ValidatingAdmissionPolicy [Privileged:ClusterAdmin]", frame // TODO: feature check should fail after GA graduation ginkgo.Skip(fmt.Sprintf("server does not support ValidatingAdmissionPolicy v1beta1: %v, feature gate not enabled?", err)) } + extensionsClient, err = apiextensionsclientset.NewForConfig(f.ClientConfig()) + framework.ExpectNoError(err, "initializing api-extensions client") }) ginkgo.BeforeEach(func(ctx context.Context) { @@ -256,6 +263,98 @@ var _ = SIGDescribe("ValidatingAdmissionPolicy [Privileged:ClusterAdmin]", frame framework.ExpectNoError(err, "create non-replicated ReplicaSet") }) }) + + ginkgo.It("should type check a CRD", func(ctx context.Context) { + crd := crontabExampleCRD() + crd.Spec.Group = "stable." + f.UniqueName + crd.Name = crd.Spec.Names.Plural + "." + crd.Spec.Group + var policy *admissionregistrationv1beta1.ValidatingAdmissionPolicy + ginkgo.By("creating the CRD", func() { + var err error + crd, err = extensionsClient.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, crd, metav1.CreateOptions{}) + framework.ExpectNoError(err, "create CRD") + err = wait.PollUntilContextCancel(ctx, 100*time.Millisecond, true, func(ctx context.Context) (done bool, err error) { + // wait for the CRD to be published. + root := openapi3.NewRoot(client.Discovery().OpenAPIV3()) + _, err = root.GVSpec(schema.GroupVersion{Group: crd.Spec.Group, Version: "v1"}) + return err == nil, nil + }) + framework.ExpectNoError(err, "wait for CRD.") + ginkgo.DeferCleanup(func(ctx context.Context, name string) error { + return extensionsClient.ApiextensionsV1().CustomResourceDefinitions().Delete(ctx, name, metav1.DeleteOptions{}) + }, crd.Name) + }) + ginkgo.By("creating a vaild policy for crontabs", func() { + policy = newValidatingAdmissionPolicyBuilder(f.UniqueName+".correct-crd-policy.example.com"). + MatchUniqueNamespace(f.UniqueName). + StartResourceRule(). + MatchResource([]string{crd.Spec.Group}, []string{"v1"}, []string{"crontabs"}). + EndResourceRule(). + WithValidation(admissionregistrationv1beta1.Validation{ + Expression: "object.spec.replicas > 1", + }). + Build() + policy, err := client.AdmissionregistrationV1beta1().ValidatingAdmissionPolicies().Create(ctx, policy, metav1.CreateOptions{}) + framework.ExpectNoError(err, "create policy") + ginkgo.DeferCleanup(func(ctx context.Context, name string) error { + return client.AdmissionregistrationV1alpha1().ValidatingAdmissionPolicies().Delete(ctx, name, metav1.DeleteOptions{}) + }, policy.Name) + }) + ginkgo.By("waiting for the type check to finish without warnings", func() { + err := wait.PollUntilContextCancel(ctx, 100*time.Millisecond, true, func(ctx context.Context) (done bool, err error) { + policy, err = client.AdmissionregistrationV1beta1().ValidatingAdmissionPolicies().Get(ctx, policy.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + if policy.Status.TypeChecking != nil { + return true, nil + } + return false, nil + }) + framework.ExpectNoError(err, "wait for type checking") + gomega.Expect(policy.Status.TypeChecking.ExpressionWarnings).To(gomega.BeEmpty(), "expect no warnings") + }) + ginkgo.By("creating a policy with type-confused expressions for crontabs", func() { + policy = newValidatingAdmissionPolicyBuilder(f.UniqueName+".confused-crd-policy.example.com"). + MatchUniqueNamespace(f.UniqueName). + StartResourceRule(). + MatchResource([]string{crd.Spec.Group}, []string{"v1"}, []string{"crontabs"}). + EndResourceRule(). + WithValidation(admissionregistrationv1beta1.Validation{ + Expression: "object.spec.replicas > '1'", // type confusion + }). + WithValidation(admissionregistrationv1beta1.Validation{ + Expression: "object.spec.maxRetries < 10", // not yet existing field + }). + Build() + policy, err := client.AdmissionregistrationV1beta1().ValidatingAdmissionPolicies().Create(ctx, policy, metav1.CreateOptions{}) + framework.ExpectNoError(err, "create policy") + ginkgo.DeferCleanup(func(ctx context.Context, name string) error { + return client.AdmissionregistrationV1alpha1().ValidatingAdmissionPolicies().Delete(ctx, name, metav1.DeleteOptions{}) + }, policy.Name) + }) + ginkgo.By("waiting for the type check to finish with warnings", func() { + err := wait.PollUntilContextCancel(ctx, 100*time.Millisecond, true, func(ctx context.Context) (done bool, err error) { + policy, err = client.AdmissionregistrationV1beta1().ValidatingAdmissionPolicies().Get(ctx, policy.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + if policy.Status.TypeChecking != nil { + return true, nil + } + return false, nil + }) + framework.ExpectNoError(err, "wait for type checking") + + gomega.Expect(policy.Status.TypeChecking.ExpressionWarnings).To(gomega.HaveLen(2)) + warning := policy.Status.TypeChecking.ExpressionWarnings[0] + gomega.Expect(warning.FieldRef).To(gomega.Equal("spec.validations[0].expression")) + gomega.Expect(warning.Warning).To(gomega.ContainSubstring("found no matching overload for '_>_' applied to '(int, string)'")) + warning = policy.Status.TypeChecking.ExpressionWarnings[1] + gomega.Expect(warning.FieldRef).To(gomega.Equal("spec.validations[1].expression")) + gomega.Expect(warning.Warning).To(gomega.ContainSubstring("undefined field 'maxRetries'")) + }) + }) }) func createBinding(bindingName string, uniqueLabel string, policyName string) *admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding { @@ -408,3 +507,48 @@ func (b *validatingAdmissionPolicyBuilder) WithVariable(variable admissionregist func (b *validatingAdmissionPolicyBuilder) Build() *admissionregistrationv1beta1.ValidatingAdmissionPolicy { return b.policy } + +func crontabExampleCRD() *apiextensionsv1.CustomResourceDefinition { + return &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "crontabs.stable.example.com", + }, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: "stable.example.com", + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ + { + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensionsv1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "cronSpec": { + Type: "string", + }, + "image": { + Type: "string", + }, + "replicas": { + Type: "integer", + }, + }, + }, + }, + }}, + }, + }, + Scope: apiextensionsv1.NamespaceScoped, + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "crontabs", + Singular: "crontab", + Kind: "CronTab", + ShortNames: []string{"ct"}, + }, + }, + } +} diff --git a/test/integration/apiserver/cel/typeresolution_test.go b/test/integration/apiserver/cel/typeresolution_test.go index 76d4a42296f..869921c1226 100644 --- a/test/integration/apiserver/cel/typeresolution_test.go +++ b/test/integration/apiserver/cel/typeresolution_test.go @@ -79,7 +79,7 @@ func TestTypeResolver(t *testing.T) { } }(crd) discoveryResolver := &resolver.ClientDiscoveryResolver{Discovery: client.Discovery()} - definitionsResolver := resolver.NewDefinitionsSchemaResolver(k8sscheme.Scheme, openapi.GetOpenAPIDefinitions) + definitionsResolver := resolver.NewDefinitionsSchemaResolver(openapi.GetOpenAPIDefinitions, k8sscheme.Scheme) // wait until the CRD schema is published at the OpenAPI v3 endpoint err = wait.PollImmediate(time.Second, time.Minute, func() (done bool, err error) { p, err := client.OpenAPIV3().Paths() @@ -330,7 +330,7 @@ func TestBuiltinResolution(t *testing.T) { }{ { name: "definitions", - resolver: resolver.NewDefinitionsSchemaResolver(k8sscheme.Scheme, openapi.GetOpenAPIDefinitions), + resolver: resolver.NewDefinitionsSchemaResolver(openapi.GetOpenAPIDefinitions, k8sscheme.Scheme), scheme: buildTestScheme(), }, {