diff --git a/pkg/kubeapiserver/options/plugins.go b/pkg/kubeapiserver/options/plugins.go index 333b523cfe9..9501de708d9 100644 --- a/pkg/kubeapiserver/options/plugins.go +++ b/pkg/kubeapiserver/options/plugins.go @@ -20,6 +20,7 @@ package options // This should probably be part of some configuration fed into the build for a // given binary target. import ( + validatingpolicy "k8s.io/apiserver/pkg/admission/plugin/cel" // Admission policies "k8s.io/kubernetes/plugin/pkg/admission/admit" "k8s.io/kubernetes/plugin/pkg/admission/alwayspullimages" @@ -97,6 +98,7 @@ var AllOrderedPlugins = []string{ // webhook, resourcequota, and deny plugins must go at the end mutatingwebhook.PluginName, // MutatingAdmissionWebhook + validatingpolicy.PluginName, // ValidatingAdmissionPolicy validatingwebhook.PluginName, // ValidatingAdmissionWebhook resourcequota.PluginName, // ResourceQuota deny.PluginName, // AlwaysDeny @@ -159,6 +161,7 @@ func DefaultOffAdmissionPlugins() sets.String { certsubjectrestriction.PluginName, // CertificateSubjectRestriction defaultingressclass.PluginName, // DefaultIngressClass podsecurity.PluginName, // PodSecurity + validatingpolicy.PluginName, // ValidatingAdmissionPolicy, only active when feature gate CELValidatingAdmission is enabled ) return sets.NewString(AllOrderedPlugins...).Difference(defaultOnPlugins) diff --git a/pkg/kubeapiserver/options/plugins_test.go b/pkg/kubeapiserver/options/plugins_test.go index 5eb5c6c4a06..81784dd75c4 100644 --- a/pkg/kubeapiserver/options/plugins_test.go +++ b/pkg/kubeapiserver/options/plugins_test.go @@ -24,7 +24,7 @@ import ( func TestAdmissionPluginOrder(t *testing.T) { // Ensure the last four admission plugins listed are webhooks, quota, and deny allplugins := strings.Join(AllOrderedPlugins, ",") - expectSuffix := ",MutatingAdmissionWebhook,ValidatingAdmissionWebhook,ResourceQuota,AlwaysDeny" + expectSuffix := ",MutatingAdmissionWebhook,ValidatingAdmissionPolicy,ValidatingAdmissionWebhook,ResourceQuota,AlwaysDeny" if !strings.HasSuffix(allplugins, expectSuffix) { t.Fatalf("AllOrderedPlugins must end with ...%s", expectSuffix) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go index d7628a515b3..2cede89841a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go @@ -75,7 +75,7 @@ var ( initEnvErr error ) -// This func is duplicated in k8s.io/apiserver/pkg/admission/plugin/cel/internal/implementation.go +// This func is duplicated in k8s.io/apiserver/pkg/admission/plugin/cel/validator.go // If any changes are made here, consider to make the same changes there as well. func getBaseEnv() (*cel.Env, error) { initEnvOnce.Do(func() { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/admission.go index 50f679c857e..405cc11b5a1 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/admission.go @@ -21,9 +21,16 @@ import ( "errors" "fmt" "io" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apiserver/pkg/features" + "k8s.io/client-go/dynamic" + "k8s.io/component-base/featuregate" "time" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/admission/initializer" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" ) @@ -38,7 +45,7 @@ import ( const ( // PluginName indicates the name of admission plug-in - PluginName = "CEL" + PluginName = "ValidatingAdmissionPolicy" ) // Register registers a plugin @@ -54,28 +61,89 @@ func Register(plugins *admission.Plugins) { type celAdmissionPlugin struct { evaluator CELPolicyEvaluator + + inspectedFeatureGates bool + enabled bool + + // Injected Dependencies + informerFactory informers.SharedInformerFactory + client kubernetes.Interface + restMapper meta.RESTMapper + dynamicClient dynamic.Interface + stopCh <-chan struct{} } -var _ WantsCELPolicyEvaluator = &celAdmissionPlugin{} +var _ initializer.WantsExternalKubeInformerFactory = &celAdmissionPlugin{} +var _ initializer.WantsExternalKubeClientSet = &celAdmissionPlugin{} +var _ initializer.WantsRESTMapper = &celAdmissionPlugin{} +var _ initializer.WantsDynamicClient = &celAdmissionPlugin{} +var _ initializer.WantsDrainedNotification = &celAdmissionPlugin{} + +var _ admission.InitializationValidator = &celAdmissionPlugin{} var _ admission.ValidationInterface = &celAdmissionPlugin{} -func NewPlugin() (*celAdmissionPlugin, error) { +func NewPlugin() (admission.Interface, error) { result := &celAdmissionPlugin{} return result, nil } -func (c *celAdmissionPlugin) SetCELPolicyEvaluator(evaluator CELPolicyEvaluator) { - c.evaluator = evaluator +func (c *celAdmissionPlugin) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) { + c.informerFactory = f } -// Once clientset and informer factory are provided, creates and starts the -// admission controller +func (c *celAdmissionPlugin) SetExternalKubeClientSet(client kubernetes.Interface) { + c.client = client +} + +func (c *celAdmissionPlugin) SetRESTMapper(mapper meta.RESTMapper) { + c.restMapper = mapper +} + +func (c *celAdmissionPlugin) SetDynamicClient(client dynamic.Interface) { + c.dynamicClient = client +} + +func (c *celAdmissionPlugin) SetDrainedNotification(stopCh <-chan struct{}) { + c.stopCh = stopCh +} + +func (c *celAdmissionPlugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { + if featureGates.Enabled(features.CELValidatingAdmission) { + c.enabled = true + } + c.inspectedFeatureGates = true +} + +// ValidateInitialization - once clientset and informer factory are provided, creates and starts the admission controller func (c *celAdmissionPlugin) ValidateInitialization() error { - if c.evaluator != nil { + if !c.inspectedFeatureGates { + return fmt.Errorf("%s did not see feature gates", PluginName) + } + if !c.enabled { return nil } + if c.informerFactory == nil { + return errors.New("missing informer factory") + } + if c.client == nil { + return errors.New("missing kubernetes client") + } + if c.restMapper == nil { + return errors.New("missing rest mapper") + } + if c.dynamicClient == nil { + return errors.New("missing dynamic client") + } + if c.stopCh == nil { + return errors.New("missing stop channel") + } + c.evaluator = NewAdmissionController(c.informerFactory, c.client, c.restMapper, c.dynamicClient) + if err := c.evaluator.ValidateInitialization(); err != nil { + return err + } - return errors.New("CELPolicyEvaluator not injected") + go c.evaluator.Run(c.stopCh) + return nil } //////////////////////////////////////////////////////////////////////////////// @@ -91,13 +159,32 @@ func (c *celAdmissionPlugin) Validate( a admission.Attributes, o admission.ObjectInterfaces, ) (err error) { + if !c.enabled { + return nil + } deadlined, cancel := context.WithTimeout(ctx, 2*time.Second) defer cancel() + // isPolicyResource determines if an admission.Attributes object is describing + // the admission of a ValidatingAdmissionPolicy or a ValidatingAdmissionPolicyBinding + if isPolicyResource(a) { + return + } + if !cache.WaitForNamedCacheSync("cel-admission-plugin", deadlined.Done(), c.evaluator.HasSynced) { return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request")) } return c.evaluator.Validate(ctx, a, o) } + +func isPolicyResource(attr admission.Attributes) bool { + gvk := attr.GetResource() + if gvk.Group == "admissionregistration.k8s.io" { + if gvk.Resource == "validatingadmissionpolicies" || gvk.Resource == "validatingadmissionpolicybindings" { + return true + } + } + return false +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/admission_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/admission_test.go index 16b86a8fa4d..c20ff00e192 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/admission_test.go @@ -24,62 +24,208 @@ import ( "time" "github.com/stretchr/testify/require" + "k8s.io/api/admissionregistration/v1alpha1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/apimachinery/pkg/watch" - "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/admission/initializer" "k8s.io/apiserver/pkg/admission/plugin/cel/internal/generic" + "k8s.io/apiserver/pkg/features" dynamicfake "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" clienttesting "k8s.io/client-go/testing" - "k8s.io/client-go/tools/cache" + "k8s.io/component-base/featuregate" ) var ( - scheme *runtime.Scheme = runtime.NewScheme() - codecs serializer.CodecFactory = serializer.NewCodecFactory(scheme) + scheme *runtime.Scheme = func() *runtime.Scheme { + res := runtime.NewScheme() + res.AddKnownTypeWithName(paramsGVK, &unstructured.Unstructured{}) + res.AddKnownTypeWithName(schema.GroupVersionKind{ + Group: paramsGVK.Group, + Version: paramsGVK.Version, + Kind: paramsGVK.Kind + "List", + }, &unstructured.UnstructuredList{}) + + if err := v1alpha1.AddToScheme(res); err != nil { + panic(err) + } + return res + }() paramsGVK schema.GroupVersionKind = schema.GroupVersionKind{ Group: "example.com", Version: "v1", Kind: "ParamsConfig", } - fakeRestMapper *meta.DefaultRESTMapper = meta.NewDefaultRESTMapper([]schema.GroupVersion{ - { - Group: "", - Version: "v1", + + fakeRestMapper *meta.DefaultRESTMapper = func() *meta.DefaultRESTMapper { + res := meta.NewDefaultRESTMapper([]schema.GroupVersion{ + { + Group: "", + Version: "v1", + }, + }) + + res.Add(paramsGVK, meta.RESTScopeNamespace) + res.Add(definitionGVK, meta.RESTScopeRoot) + res.Add(bindingGVK, meta.RESTScopeRoot) + return res + }() + + definitionGVK schema.GroupVersionKind = must3(scheme.ObjectKinds(&v1alpha1.ValidatingAdmissionPolicy{}))[0] + bindingGVK schema.GroupVersionKind = must3(scheme.ObjectKinds(&v1alpha1.ValidatingAdmissionPolicyBinding{}))[0] + + definitionsGVR schema.GroupVersionResource = must(fakeRestMapper.RESTMapping(definitionGVK.GroupKind(), definitionGVK.Version)).Resource + bindingsGVR schema.GroupVersionResource = must(fakeRestMapper.RESTMapping(bindingGVK.GroupKind(), bindingGVK.Version)).Resource + + // Common objects + denyPolicy *v1alpha1.ValidatingAdmissionPolicy = &v1alpha1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "denypolicy.example.com", + ResourceVersion: "1", }, - }) + Spec: v1alpha1.ValidatingAdmissionPolicySpec{ + ParamKind: &v1alpha1.ParamKind{ + APIVersion: paramsGVK.GroupVersion().String(), + Kind: paramsGVK.Kind, + }, + FailurePolicy: ptrTo(v1alpha1.Fail), + }, + } - definitionGVK schema.GroupVersionKind = (&FakePolicyDefinition{}).GroupVersionKind() - bindingGVK schema.GroupVersionKind = (&FakePolicyBinding{}).GroupVersionKind() + fakeParams *unstructured.Unstructured = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": paramsGVK.GroupVersion().String(), + "kind": paramsGVK.Kind, + "metadata": map[string]interface{}{ + "name": "replicas-test.example.com", + "resourceVersion": "1", + }, + "maxReplicas": int64(3), + }, + } - definitionsGVR schema.GroupVersionResource = definitionGVK.GroupVersion().WithResource("policydefinitions") - bindingsGVR schema.GroupVersionResource = bindingGVK.GroupVersion().WithResource("policybindings") + denyBinding *v1alpha1.ValidatingAdmissionPolicyBinding = &v1alpha1.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "denybinding.example.com", + ResourceVersion: "1", + }, + Spec: v1alpha1.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: denyPolicy.Name, + ParamRef: &v1alpha1.ParamRef{ + Name: fakeParams.GetName(), + Namespace: fakeParams.GetNamespace(), + }, + }, + } + denyBindingWithNoParamRef *v1alpha1.ValidatingAdmissionPolicyBinding = &v1alpha1.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "denybinding.example.com", + ResourceVersion: "1", + }, + Spec: v1alpha1.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: denyPolicy.Name, + }, + } ) -func init() { - fakeRestMapper.Add(definitionGVK, meta.RESTScopeRoot) - fakeRestMapper.Add(bindingGVK, meta.RESTScopeNamespace) - fakeRestMapper.Add(paramsGVK, meta.RESTScopeNamespace) +// Interface which has fake compile and match functionality for use in tests +// So that we can test the controller without pulling in any CEL functionality +type fakeCompiler struct { + DefaultMatch bool + CompileFuncs map[string]func(*v1alpha1.ValidatingAdmissionPolicy) Validator + DefinitionMatchFuncs map[string]func(*v1alpha1.ValidatingAdmissionPolicy, admission.Attributes) bool + BindingMatchFuncs map[string]func(*v1alpha1.ValidatingAdmissionPolicyBinding, admission.Attributes) bool +} - scheme.AddKnownTypeWithName(definitionGVK, &FakePolicyDefinition{}) - scheme.AddKnownTypeWithName(bindingGVK, &FakePolicyBinding{}) +var _ ValidatorCompiler = &fakeCompiler{} - scheme.AddKnownTypeWithName((&FakePolicyDefinitionList{}).GroupVersionKind(), &FakePolicyDefinitionList{}) - scheme.AddKnownTypeWithName((&FakePolicyBindingList{}).GroupVersionKind(), &FakePolicyBindingList{}) +func (f *fakeCompiler) HasSynced() bool { + return true +} - scheme.AddKnownTypeWithName(paramsGVK, &unstructured.Unstructured{}) - scheme.AddKnownTypeWithName(schema.GroupVersionKind{ - Group: paramsGVK.Group, - Version: paramsGVK.Version, - Kind: paramsGVK.Kind + "List", - }, &unstructured.UnstructuredList{}) +func (f *fakeCompiler) ValidateInitialization() error { + return nil +} + +// Matches says whether this policy definition matches the provided admission +// resource request +func (f *fakeCompiler) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1alpha1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionKind, error) { + namespace, name := definition.Namespace, definition.Name + key := namespace + "/" + name + if fun, ok := f.DefinitionMatchFuncs[key]; ok { + return fun(definition, a), schema.GroupVersionKind{}, nil + } + + // Default is match everything + return f.DefaultMatch, schema.GroupVersionKind{}, nil +} + +// Matches says whether this policy definition matches the provided admission +// resource request +func (f *fakeCompiler) BindingMatches(a admission.Attributes, o admission.ObjectInterfaces, binding *v1alpha1.ValidatingAdmissionPolicyBinding) (bool, error) { + namespace, name := binding.Namespace, binding.Name + key := namespace + "/" + name + if fun, ok := f.BindingMatchFuncs[key]; ok { + return fun(binding, a), nil + } + + // Default is match everything + return f.DefaultMatch, nil +} + +func (f *fakeCompiler) Compile( + definition *v1alpha1.ValidatingAdmissionPolicy, +) Validator { + namespace, name := definition.Namespace, definition.Name + + key := namespace + "/" + name + if fun, ok := f.CompileFuncs[key]; ok { + return fun(definition) + } + + return nil +} + +func (f *fakeCompiler) RegisterDefinition(definition *v1alpha1.ValidatingAdmissionPolicy, compileFunc func(*v1alpha1.ValidatingAdmissionPolicy) Validator, matchFunc func(*v1alpha1.ValidatingAdmissionPolicy, admission.Attributes) bool) { + namespace, name := definition.Namespace, definition.Name + key := namespace + "/" + name + if compileFunc != nil { + + if f.CompileFuncs == nil { + f.CompileFuncs = make(map[string]func(*v1alpha1.ValidatingAdmissionPolicy) Validator) + } + f.CompileFuncs[key] = compileFunc + } + + if matchFunc != nil { + if f.DefinitionMatchFuncs == nil { + f.DefinitionMatchFuncs = make(map[string]func(*v1alpha1.ValidatingAdmissionPolicy, admission.Attributes) bool) + } + f.DefinitionMatchFuncs[key] = matchFunc + } +} + +func (f *fakeCompiler) RegisterBinding(binding *v1alpha1.ValidatingAdmissionPolicyBinding, matchFunc func(*v1alpha1.ValidatingAdmissionPolicyBinding, admission.Attributes) bool) { + namespace, name := binding.Namespace, binding.Name + key := namespace + "/" + name + + if matchFunc != nil { + if f.BindingMatchFuncs == nil { + f.BindingMatchFuncs = make(map[string]func(*v1alpha1.ValidatingAdmissionPolicyBinding, admission.Attributes) bool) + } + f.BindingMatchFuncs[key] = matchFunc + } +} + +func setupFakeTest(t *testing.T, comp *fakeCompiler) (plugin admission.ValidationInterface, paramTracker, policyTracker clienttesting.ObjectTracker, controller *celAdmissionController) { + return setupTestCommon(t, comp) } // Starts CEL admission controller and sets up a plugin configured with it as well @@ -89,54 +235,48 @@ func init() { // support multiple types of params this function needs to be augmented // // PolicyTracker expects FakePolicyDefinition and FakePolicyBinding types -func setupTest(t *testing.T) (plugin admission.ValidationInterface, paramTracker, policyTracker clienttesting.ObjectTracker, controller *celAdmissionController) { +func setupTestCommon(t *testing.T, compiler ValidatorCompiler) (plugin admission.ValidationInterface, paramTracker, policyTracker clienttesting.ObjectTracker, controller *celAdmissionController) { testContext, testContextCancel := context.WithCancel(context.Background()) t.Cleanup(testContextCancel) dynamicClient := dynamicfake.NewSimpleDynamicClient(scheme) - tracker := clienttesting.NewObjectTracker(scheme, codecs.UniversalDecoder()) - // Set up fake informers that return instances of mock Policy definitoins - // and mock policy bindings - fakeDefinitionsInformer := cache.NewSharedIndexInformer(&cache.ListWatch{ - ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return tracker.List(definitionsGVR, definitionGVK, "") - }, - WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return tracker.Watch(definitionsGVR, "") - }, - }, &FakePolicyDefinition{}, 30*time.Second, nil) + fakeClient := fake.NewSimpleClientset() + fakeInformerFactory := informers.NewSharedInformerFactory(fakeClient, time.Second) + featureGate := featuregate.NewFeatureGate() + err := featureGate.Add(map[featuregate.Feature]featuregate.FeatureSpec{ + features.CELValidatingAdmission: { + Default: true, PreRelease: featuregate.Alpha}}) + if err != nil { + // FIXME: handle error. + panic("Unexpected error") + } + err = featureGate.SetFromMap(map[string]bool{string(features.CELValidatingAdmission): true}) + if err != nil { + // FIXME: handle error. + panic("Unexpected error.") + } - fakeBindingsInformer := cache.NewSharedIndexInformer(&cache.ListWatch{ - ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return tracker.List(bindingsGVR, bindingGVK, "") - }, - WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return tracker.Watch(bindingsGVR, "") - }, - }, &FakePolicyBinding{}, 30*time.Second, nil) + handler := &celAdmissionPlugin{enabled: true} - go fakeDefinitionsInformer.Run(testContext.Done()) - go fakeBindingsInformer.Run(testContext.Done()) - - admissionController := NewAdmissionController( - fakeDefinitionsInformer, - fakeBindingsInformer, - nil, // objectConverter is unused by the `FakePolicyDefinition` compile func - fakeRestMapper, - dynamicClient, - ).(*celAdmissionController) - - handler, err := NewPlugin() - require.NoError(t, err) - - pluginInitializer := NewPluginInitializer(admissionController) - pluginInitializer.Initialize(handler) + genericInitializer := initializer.New(fakeClient, dynamicClient, fakeInformerFactory, nil, featureGate, testContext.Done()) + genericInitializer.Initialize(handler) + handler.SetRESTMapper(fakeRestMapper) err = admission.ValidateInitialization(handler) require.NoError(t, err) + require.True(t, handler.enabled) - go admissionController.Run(testContext.Done()) - return handler, dynamicClient.Tracker(), tracker, admissionController + // Override compiler used by controller for tests + handler.evaluator.(*celAdmissionController).validatorCompiler = compiler + + // Make sure to start the fake informers + fakeInformerFactory.Start(testContext.Done()) + t.Cleanup(func() { + testContextCancel() + // wait for informer factory to shutdown + fakeInformerFactory.Shutdown() + }) + return handler, dynamicClient.Tracker(), fakeClient.Tracker(), handler.evaluator.(*celAdmissionController) } // Gets the last reconciled value in the controller of an object with the same @@ -154,9 +294,13 @@ func (c *celAdmissionController) getCurrentObject(obj runtime.Object) (runtime.O switch obj.(type) { case *unstructured.Unstructured: - paramSource := obj.GetObjectKind().GroupVersionKind() + paramSourceGVK := obj.GetObjectKind().GroupVersionKind() + paramKind := v1alpha1.ParamKind{ + APIVersion: paramSourceGVK.GroupVersion().String(), + Kind: paramSourceGVK.Kind, + } var paramInformer generic.Informer[*unstructured.Unstructured] - if paramInfo, ok := c.paramsCRDControllers[paramSource]; ok { + if paramInfo, ok := c.paramsCRDControllers[paramKind]; ok { paramInformer = paramInfo.controller.Informer() } else { // Treat unknown CRD the same as not found @@ -173,17 +317,17 @@ func (c *celAdmissionController) getCurrentObject(obj runtime.Object) (runtime.O } return item, nil - case PolicyBinding: - namespacedName := accessor.GetNamespace() + "/" + accessor.GetName() - info, ok := c.bindingInfos[namespacedName] + case *v1alpha1.ValidatingAdmissionPolicyBinding: + nn := getNamespaceName(accessor.GetNamespace(), accessor.GetName()) + info, ok := c.bindingInfos[nn] if !ok { return nil, nil } return info.lastReconciledValue, nil - case PolicyDefinition: - namespacedName := accessor.GetNamespace() + "/" + accessor.GetName() - info, ok := c.definitionInfo[namespacedName] + case *v1alpha1.ValidatingAdmissionPolicy: + nn := getNamespaceName(accessor.GetNamespace(), accessor.GetName()) + info, ok := c.definitionInfo[nn] if !ok { return nil, nil } @@ -197,7 +341,7 @@ func (c *celAdmissionController) getCurrentObject(obj runtime.Object) (runtime.O // Waits for the given objects to have been the latest reconciled values of // their gvk/name in the controller func waitForReconcile(ctx context.Context, controller *celAdmissionController, objects ...runtime.Object) error { - return wait.PollWithContext(ctx, 200*time.Millisecond, 3*time.Hour, func(ctx context.Context) (done bool, err error) { + return wait.PollWithContext(ctx, 200*time.Millisecond, 5*time.Second, func(ctx context.Context) (done bool, err error) { for _, obj := range objects { currentValue, err := controller.getCurrentObject(obj) if err != nil { @@ -254,12 +398,26 @@ func attributeRecord( old, new runtime.Object, operation admission.Operation, ) admission.Attributes { + if old == nil && new == nil { + panic("both `old` and `new` may not be nil") + } + accessor, err := meta.Accessor(new) if err != nil { panic(err) } - gvk := new.GetObjectKind().GroupVersionKind() + // one of old/new may be nil, but not both + example := new + if example == nil { + example = old + } + + gvk := example.GetObjectKind().GroupVersionKind() + if gvk.Empty() { + // If gvk is not populated, try to fetch it from the scheme + gvk = must3(scheme.ObjectKinds(example))[0] + } mapping, err := fakeRestMapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil { panic(err) @@ -284,6 +442,20 @@ func ptrTo[T any](obj T) *T { return &obj } +func must[T any](val T, err error) T { + if err != nil { + panic(err) + } + return val +} + +func must3[T any, I any](val T, _ I, err error) T { + if err != nil { + panic(err) + } + return val +} + //////////////////////////////////////////////////////////////////////////////// // Functionality Tests //////////////////////////////////////////////////////////////////////////////// @@ -293,65 +465,25 @@ func TestBasicPolicyDefinitionFailure(t *testing.T) { defer testContextCancel() datalock := sync.Mutex{} - passedParams := []*unstructured.Unstructured{} numCompiles := 0 - fakeParams := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": paramsGVK.GroupVersion().String(), - "kind": paramsGVK.Kind, - "metadata": map[string]interface{}{ - "name": "replicas-test.example.com", - "resourceVersion": "1", - }, - "maxReplicas": int64(3), - }, + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, } + compiler.RegisterDefinition(denyPolicy, func(policy *v1alpha1.ValidatingAdmissionPolicy) Validator { + datalock.Lock() + numCompiles += 1 + datalock.Unlock() - // Push some fake - denyPolicy := &FakePolicyDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denypolicy.example.com", - ResourceVersion: "1", - }, - CompileFunc: ptrTo(func(converter ObjectConverter) (EvaluatorFunc, error) { - datalock.Lock() - numCompiles += 1 - datalock.Unlock() + return testValidator{} + }, nil) - return func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision { - datalock.Lock() - passedParams = append(passedParams, params) - datalock.Unlock() - - // Policy always denies - return []PolicyDecision{ - { - Kind: Deny, - Message: "Denied", - }, - } - }, nil - }), - ParamSource: ¶msGVK, - FailurePolicy: Fail, - } - - denyBinding := &FakePolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denybinding.example.com", - Namespace: "", - ResourceVersion: "1", - }, - Params: "replicas-test.example.com", - Policy: "denypolicy.example.com", - } - - handler, paramTracker, tracker, controller := setupTest(t) + handler, paramTracker, tracker, controller := setupFakeTest(t, compiler) require.NoError(t, paramTracker.Add(fakeParams)) - require.NoError(t, tracker.Add(denyPolicy)) - require.NoError(t, tracker.Add(denyBinding)) + require.NoError(t, tracker.Create(definitionsGVR, denyPolicy, denyPolicy.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, denyBinding, denyBinding.Namespace)) // Wait for controller to reconcile given objects require.NoError(t, @@ -363,19 +495,34 @@ func TestBasicPolicyDefinitionFailure(t *testing.T) { testContext, // Object is irrelevant/unchecked for this test. Just test that // the evaluator is executed, and returns a denial - attributeRecord(nil, denyBinding, admission.Create), + attributeRecord(nil, fakeParams, admission.Create), &admission.RuntimeObjectInterfaces{}, ) - require.ErrorContains(t, err, `{"kind":"Deny","message":"Denied"}`) + require.ErrorContains(t, err, `Denied`) +} - require.Equal(t, []*unstructured.Unstructured{fakeParams}, passedParams) +type testValidator struct { +} + +func (v testValidator) Validate(a admission.Attributes, o admission.ObjectInterfaces, params runtime.Object, matchKind schema.GroupVersionKind) ([]policyDecision, error) { + // Policy always denies + return []policyDecision{ + { + kind: deny, + message: "Denied", + }, + }, nil } // Shows that if a definition does not match the input, it will not be used. // But with a different input it will be used. func TestDefinitionDoesntMatch(t *testing.T) { - handler, paramTracker, tracker, controller := setupTest(t) + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, + } + handler, paramTracker, tracker, controller := setupFakeTest(t, compiler) testContext, testContextCancel := context.WithCancel(context.Background()) defer testContextCancel() @@ -383,12 +530,15 @@ func TestDefinitionDoesntMatch(t *testing.T) { passedParams := []*unstructured.Unstructured{} numCompiles := 0 - denyPolicy := &FakePolicyDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denypolicy.example.com", - ResourceVersion: "1", - }, - MatchFunc: ptrTo(func(a admission.Attributes) bool { + compiler.RegisterDefinition(denyPolicy, + func(vap *v1alpha1.ValidatingAdmissionPolicy) Validator { + datalock.Lock() + numCompiles += 1 + datalock.Unlock() + + return testValidator{} + + }, func(vap *v1alpha1.ValidatingAdmissionPolicy, a admission.Attributes) bool { // Match names with even-numbered length obj := a.GetObject() @@ -399,55 +549,11 @@ func TestDefinitionDoesntMatch(t *testing.T) { } return len(accessor.GetName())%2 == 0 - }), - CompileFunc: ptrTo(func(converter ObjectConverter) (EvaluatorFunc, error) { - datalock.Lock() - numCompiles += 1 - datalock.Unlock() - - return func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision { - datalock.Lock() - passedParams = append(passedParams, params) - datalock.Unlock() - - // Policy always denies - return []PolicyDecision{ - { - Kind: Deny, - Message: "Denied", - }, - } - }, nil - }), - ParamSource: ¶msGVK, - FailurePolicy: Fail, - } - - fakeParams := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": paramsGVK.GroupVersion().String(), - "kind": paramsGVK.Kind, - "metadata": map[string]interface{}{ - "name": "replicas-test.example.com", - "resourceVersion": "1", - }, - "maxReplicas": int64(3), - }, - } - - denyBinding := &FakePolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denybinding.example.com", - Namespace: "", - ResourceVersion: "1", - }, - Params: "replicas-test.example.com", - Policy: "denypolicy.example.com", - } + }) require.NoError(t, paramTracker.Add(fakeParams)) - require.NoError(t, tracker.Add(denyPolicy)) - require.NoError(t, tracker.Add(denyBinding)) + require.NoError(t, tracker.Create(definitionsGVR, denyPolicy, denyPolicy.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, denyBinding, denyBinding.Namespace)) // Wait for controller to reconcile given objects require.NoError(t, @@ -493,31 +599,24 @@ func TestDefinitionDoesntMatch(t *testing.T) { attributeRecord( nil, matchingParams, admission.Create), &admission.RuntimeObjectInterfaces{}), - `{"kind":"Deny","message":"Denied"}`) + `Denied`) require.Equal(t, numCompiles, 1) - require.Equal(t, passedParams, []*unstructured.Unstructured{fakeParams}) } func TestReconfigureBinding(t *testing.T) { testContext, testContextCancel := context.WithCancel(context.Background()) defer testContextCancel() - datalock := sync.Mutex{} - passedParams := []*unstructured.Unstructured{} - numCompiles := 0 - - fakeParams := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": paramsGVK.GroupVersion().String(), - "kind": paramsGVK.Kind, - "metadata": map[string]interface{}{ - "name": "replicas-test.example.com", - "resourceVersion": "1", - }, - "maxReplicas": int64(3), - }, + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, } + handler, paramTracker, tracker, controller := setupFakeTest(t, compiler) + + datalock := sync.Mutex{} + numCompiles := 0 + fakeParams2 := &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": paramsGVK.GroupVersion().String(), @@ -530,57 +629,33 @@ func TestReconfigureBinding(t *testing.T) { }, } - denyPolicy := &FakePolicyDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denypolicy.example.com", - ResourceVersion: "1", - }, - CompileFunc: ptrTo(func(converter ObjectConverter) (EvaluatorFunc, error) { + compiler.RegisterDefinition(denyPolicy, + func(vap *v1alpha1.ValidatingAdmissionPolicy) Validator { datalock.Lock() numCompiles += 1 datalock.Unlock() - return func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision { - datalock.Lock() - passedParams = append(passedParams, params) - datalock.Unlock() + return testValidator{} - // Policy always denies - return []PolicyDecision{ - { - Kind: Deny, - Message: "Denied", - }, - } - }, nil - }), - ParamSource: ¶msGVK, - FailurePolicy: Fail, - } + }, nil) - denyBinding := &FakePolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denybinding.example.com", - ResourceVersion: "1", - }, - Params: "replicas-test.example.com", - Policy: "denypolicy.example.com", - } - - denyBinding2 := &FakePolicyBinding{ + denyBinding2 := &v1alpha1.ValidatingAdmissionPolicyBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "denybinding.example.com", ResourceVersion: "2", }, - Params: "replicas-test2.example.com", - Policy: "denypolicy.example.com", + Spec: v1alpha1.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: denyPolicy.Name, + ParamRef: &v1alpha1.ParamRef{ + Name: fakeParams2.GetName(), + Namespace: fakeParams2.GetNamespace(), + }, + }, } - handler, paramTracker, tracker, controller := setupTest(t) - require.NoError(t, paramTracker.Add(fakeParams)) - require.NoError(t, tracker.Add(denyPolicy)) - require.NoError(t, tracker.Add(denyBinding)) + require.NoError(t, tracker.Create(definitionsGVR, denyPolicy, denyPolicy.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, denyBinding, denyBinding.Namespace)) // Wait for controller to reconcile given objects require.NoError(t, @@ -588,23 +663,21 @@ func TestReconfigureBinding(t *testing.T) { testContext, controller, fakeParams, denyBinding, denyPolicy)) - ar := attributeRecord(nil, denyBinding, admission.Create) - err := handler.Validate( testContext, - attributeRecord(nil, denyBinding, admission.Create), + attributeRecord(nil, fakeParams, admission.Create), &admission.RuntimeObjectInterfaces{}, ) // Expect validation to fail for first time due to binding unconditionally // failing - require.ErrorContains(t, err, `{"kind":"Deny","message":"Denied"}`, "expect policy validation error") + require.ErrorContains(t, err, `Denied`, "expect policy validation error") // Expect `Compile` only called once require.Equal(t, 1, numCompiles, "expect `Compile` to be called only once") // Show Evaluator was called - require.Len(t, passedParams, 1, "expect evaluator is called due to proper configuration") + //require.Len(t, passedParams, 1, "expect evaluator is called due to proper configuration") // Update the tracker to point at different params require.NoError(t, tracker.Update(bindingsGVR, denyBinding2, "")) @@ -615,13 +688,13 @@ func TestReconfigureBinding(t *testing.T) { err = handler.Validate( testContext, - ar, + attributeRecord(nil, fakeParams, admission.Create), &admission.RuntimeObjectInterfaces{}, ) - require.ErrorContains(t, err, `{"decision":{"kind":"Deny","message":"configuration error: replicas-test2.example.com not found"}`) + require.ErrorContains(t, err, `failed to configure binding: replicas-test2.example.com not found`) require.Equal(t, 1, numCompiles, "expect compile is not called when there is configuration error") - require.Len(t, passedParams, 1, "expect evaluator was not called when there is configuration error") + //require.Len(t, passedParams, 1, "expect evaluator was not called when there is configuration error") // Add the missing params require.NoError(t, paramTracker.Add(fakeParams2)) @@ -632,13 +705,13 @@ func TestReconfigureBinding(t *testing.T) { // Expect validation to now fail again. err = handler.Validate( testContext, - ar, + attributeRecord(nil, fakeParams, admission.Create), &admission.RuntimeObjectInterfaces{}, ) // Expect validation to fail the third time due to validation failure - require.ErrorContains(t, err, `{"kind":"Deny","message":"Denied"}`, "expected a true policy failure, not a configuration error") - require.Equal(t, []*unstructured.Unstructured{fakeParams, fakeParams2}, passedParams, "expected call to `Validate` to cause call to evaluator") + require.ErrorContains(t, err, `Denied`, "expected a true policy failure, not a configuration error") + //require.Equal(t, []*unstructured.Unstructured{fakeParams, fakeParams2}, passedParams, "expected call to `Validate` to cause call to evaluator") require.Equal(t, 2, numCompiles, "expect changing binding causes a recompile") } @@ -647,64 +720,26 @@ func TestRemoveDefinition(t *testing.T) { testContext, testContextCancel := context.WithCancel(context.Background()) defer testContextCancel() + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, + } + handler, paramTracker, tracker, controller := setupFakeTest(t, compiler) + datalock := sync.Mutex{} - passedParams := []*unstructured.Unstructured{} numCompiles := 0 - fakeParams := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": paramsGVK.GroupVersion().String(), - "kind": paramsGVK.Kind, - "metadata": map[string]interface{}{ - "name": "replicas-test.example.com", - "resourceVersion": "1", - }, - "maxReplicas": int64(3), - }, - } + compiler.RegisterDefinition(denyPolicy, func(vap *v1alpha1.ValidatingAdmissionPolicy) Validator { + datalock.Lock() + numCompiles += 1 + datalock.Unlock() - denyPolicy := &FakePolicyDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denypolicy.example.com", - ResourceVersion: "1", - }, - CompileFunc: ptrTo(func(converter ObjectConverter) (EvaluatorFunc, error) { - datalock.Lock() - numCompiles += 1 - datalock.Unlock() - - return func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision { - datalock.Lock() - passedParams = append(passedParams, params) - datalock.Unlock() - - // Policy always denies - return []PolicyDecision{ - { - Kind: Deny, - Message: "Denied", - }, - } - }, nil - }), - ParamSource: ¶msGVK, - FailurePolicy: Fail, - } - denyBinding := &FakePolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denybinding.example.com", - Namespace: "", - ResourceVersion: "1", - }, - Params: "replicas-test.example.com", - Policy: "denypolicy.example.com", - } - - handler, paramTracker, tracker, controller := setupTest(t) + return testValidator{} + }, nil) require.NoError(t, paramTracker.Add(fakeParams)) - require.NoError(t, tracker.Add(denyPolicy)) - require.NoError(t, tracker.Add(denyBinding)) + require.NoError(t, tracker.Create(definitionsGVR, denyPolicy, denyPolicy.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, denyBinding, denyBinding.Namespace)) // Wait for controller to reconcile given objects require.NoError(t, @@ -712,16 +747,15 @@ func TestRemoveDefinition(t *testing.T) { testContext, controller, fakeParams, denyBinding, denyPolicy)) - record := attributeRecord(nil, denyBinding, admission.Create) + record := attributeRecord(nil, fakeParams, admission.Create) require.ErrorContains(t, handler.Validate( testContext, record, &admission.RuntimeObjectInterfaces{}, ), - `{"kind":"Deny","message":"Denied"}`) + `Denied`) - require.Equal(t, []*unstructured.Unstructured{fakeParams}, passedParams) require.NoError(t, tracker.Delete(definitionsGVR, denyPolicy.Namespace, denyPolicy.Name)) require.NoError(t, waitForReconcileDeletion(testContext, controller, denyPolicy)) @@ -732,74 +766,32 @@ func TestRemoveDefinition(t *testing.T) { record, &admission.RuntimeObjectInterfaces{}, )) - } // Shows that a binding which is in effect will stop being in effect when removed func TestRemoveBinding(t *testing.T) { testContext, testContextCancel := context.WithCancel(context.Background()) defer testContextCancel() + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, + } + handler, paramTracker, tracker, controller := setupFakeTest(t, compiler) datalock := sync.Mutex{} - passedParams := []*unstructured.Unstructured{} numCompiles := 0 - fakeParams := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": paramsGVK.GroupVersion().String(), - "kind": paramsGVK.Kind, - "metadata": map[string]interface{}{ - "name": "replicas-test.example.com", - "resourceVersion": "1", - }, - "maxReplicas": int64(3), - }, - } + compiler.RegisterDefinition(denyPolicy, func(vap *v1alpha1.ValidatingAdmissionPolicy) Validator { + datalock.Lock() + numCompiles += 1 + datalock.Unlock() - // Push some fake - denyPolicy := &FakePolicyDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denypolicy.example.com", - ResourceVersion: "1", - }, - CompileFunc: ptrTo(func(converter ObjectConverter) (EvaluatorFunc, error) { - datalock.Lock() - numCompiles += 1 - datalock.Unlock() - - return func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision { - datalock.Lock() - passedParams = append(passedParams, params) - datalock.Unlock() - - // Policy always denies - return []PolicyDecision{ - { - Kind: Deny, - Message: "Denied", - }, - } - }, nil - }), - ParamSource: ¶msGVK, - FailurePolicy: Fail, - } - - denyBinding := &FakePolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denybinding.example.com", - Namespace: "", - ResourceVersion: "1", - }, - Params: "replicas-test.example.com", - Policy: "denypolicy.example.com", - } - - handler, paramTracker, tracker, controller := setupTest(t) + return testValidator{} + }, nil) require.NoError(t, paramTracker.Add(fakeParams)) - require.NoError(t, tracker.Add(denyPolicy)) - require.NoError(t, tracker.Add(denyBinding)) + require.NoError(t, tracker.Create(definitionsGVR, denyPolicy, denyPolicy.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, denyBinding, denyBinding.Namespace)) // Wait for controller to reconcile given objects require.NoError(t, @@ -807,7 +799,7 @@ func TestRemoveBinding(t *testing.T) { testContext, controller, fakeParams, denyBinding, denyPolicy)) - record := attributeRecord(nil, denyBinding, admission.Create) + record := attributeRecord(nil, fakeParams, admission.Create) require.ErrorContains(t, handler.Validate( @@ -815,19 +807,11 @@ func TestRemoveBinding(t *testing.T) { record, &admission.RuntimeObjectInterfaces{}, ), - `{"kind":"Deny","message":"Denied"}`) + `Denied`) - require.Equal(t, []*unstructured.Unstructured{fakeParams}, passedParams) + //require.Equal(t, []*unstructured.Unstructured{fakeParams}, passedParams) require.NoError(t, tracker.Delete(bindingsGVR, denyBinding.Namespace, denyBinding.Name)) require.NoError(t, waitForReconcileDeletion(testContext, controller, denyBinding)) - - require.ErrorContains(t, handler.Validate( - testContext, - // Object is irrelevant/unchecked for this test. Just test that - // the evaluator is executed, and returns a denial - record, - &admission.RuntimeObjectInterfaces{}, - ), `{"decision":{"kind":"Deny","message":"configuration error: no bindings found"}`) } // Shows that an error is surfaced if a paramSource specified in a binding does @@ -835,59 +819,38 @@ func TestRemoveBinding(t *testing.T) { func TestInvalidParamSourceGVK(t *testing.T) { testContext, testContextCancel := context.WithCancel(context.Background()) defer testContextCancel() - handler, _, tracker, controller := setupTest(t) + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, + } + handler, _, tracker, controller := setupFakeTest(t, compiler) passedParams := make(chan *unstructured.Unstructured) - denyPolicy := &FakePolicyDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denypolicy.example.com", - Namespace: "", - ResourceVersion: "1", - }, - CompileFunc: ptrTo(func(converter ObjectConverter) (EvaluatorFunc, error) { - return func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision { - // Policy always denies - return []PolicyDecision{ - { - Kind: Deny, - Message: "Denied", - }, - } - }, nil - }), - ParamSource: ptrTo(paramsGVK.GroupVersion().WithKind("BadParamKind")), - FailurePolicy: Fail, + badPolicy := *denyPolicy + badPolicy.Spec.ParamKind = &v1alpha1.ParamKind{ + APIVersion: paramsGVK.GroupVersion().String(), + Kind: "BadParamKind", } - denyBinding := &FakePolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denybinding.example.com", - Namespace: "", - ResourceVersion: "1", - }, - Params: "replicas-test.example.com", - Policy: "denypolicy.example.com", - } - - require.NoError(t, tracker.Add(denyPolicy)) - require.NoError(t, tracker.Add(denyBinding)) + require.NoError(t, tracker.Create(definitionsGVR, &badPolicy, badPolicy.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, denyBinding, denyBinding.Namespace)) // Wait for controller to reconcile given objects require.NoError(t, waitForReconcile( testContext, controller, - denyBinding, denyPolicy)) + denyBinding, &badPolicy)) err := handler.Validate( testContext, - attributeRecord(nil, denyBinding, admission.Create), + attributeRecord(nil, fakeParams, admission.Create), &admission.RuntimeObjectInterfaces{}, ) // expect the specific error to be that the param was not found, not that CRD // is not existing require.ErrorContains(t, err, - `{"decision":{"kind":"Deny","message":"configuration error: failed to find resource for param source: 'example.com/v1, Kind=BadParamKind'"}`) + `failed to configure policy: failed to find resource referenced by paramKind: 'example.com/v1, Kind=BadParamKind'`) close(passedParams) require.Len(t, passedParams, 0) @@ -898,52 +861,26 @@ func TestInvalidParamSourceGVK(t *testing.T) { func TestInvalidParamSourceInstanceName(t *testing.T) { testContext, testContextCancel := context.WithCancel(context.Background()) defer testContextCancel() - handler, _, tracker, controller := setupTest(t) + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, + } + handler, _, tracker, controller := setupFakeTest(t, compiler) datalock := sync.Mutex{} passedParams := []*unstructured.Unstructured{} numCompiles := 0 - denyPolicy := &FakePolicyDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denypolicy.example.com", - ResourceVersion: "1", - }, - CompileFunc: ptrTo(func(converter ObjectConverter) (EvaluatorFunc, error) { - datalock.Lock() - numCompiles += 1 - datalock.Unlock() + compiler.RegisterDefinition(denyPolicy, func(vap *v1alpha1.ValidatingAdmissionPolicy) Validator { + datalock.Lock() + numCompiles += 1 + datalock.Unlock() - return func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision { - datalock.Lock() - passedParams = append(passedParams, params) - datalock.Unlock() + return testValidator{} + }, nil) - // Policy always denies - return []PolicyDecision{ - { - Kind: Deny, - Message: "Denied", - }, - } - }, nil - }), - ParamSource: ¶msGVK, - FailurePolicy: Fail, - } - - denyBinding := &FakePolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denybinding.example.com", - Namespace: "", - ResourceVersion: "1", - }, - Params: "replicas-test.example.com", - Policy: "denypolicy.example.com", - } - - require.NoError(t, tracker.Add(denyPolicy)) - require.NoError(t, tracker.Add(denyBinding)) + require.NoError(t, tracker.Create(definitionsGVR, denyPolicy, denyPolicy.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, denyBinding, denyBinding.Namespace)) // Wait for controller to reconcile given objects require.NoError(t, @@ -953,14 +890,14 @@ func TestInvalidParamSourceInstanceName(t *testing.T) { err := handler.Validate( testContext, - attributeRecord(nil, denyBinding, admission.Create), + attributeRecord(nil, fakeParams, admission.Create), &admission.RuntimeObjectInterfaces{}, ) // expect the specific error to be that the param was not found, not that CRD // is not existing require.ErrorContains(t, err, - `{"decision":{"kind":"Deny","message":"configuration error: replicas-test.example.com not found"}`) + `failed to configure binding: replicas-test.example.com not found`) require.Len(t, passedParams, 0) } @@ -972,54 +909,29 @@ func TestInvalidParamSourceInstanceName(t *testing.T) { func TestEmptyParamSource(t *testing.T) { testContext, testContextCancel := context.WithCancel(context.Background()) defer testContextCancel() + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, + } + handler, _, tracker, controller := setupFakeTest(t, compiler) datalock := sync.Mutex{} - passedParams := []*unstructured.Unstructured{} numCompiles := 0 // Push some fake - denyPolicy := &FakePolicyDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denypolicy.example.com", - ResourceVersion: "1", - }, - CompileFunc: ptrTo(func(converter ObjectConverter) (EvaluatorFunc, error) { - datalock.Lock() - numCompiles += 1 - datalock.Unlock() + noParamSourcePolicy := *denyPolicy + noParamSourcePolicy.Spec.ParamKind = nil - return func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision { - datalock.Lock() - passedParams = append(passedParams, params) - datalock.Unlock() + compiler.RegisterDefinition(&noParamSourcePolicy, func(vap *v1alpha1.ValidatingAdmissionPolicy) Validator { + datalock.Lock() + numCompiles += 1 + datalock.Unlock() - // Policy always denies - return []PolicyDecision{ - { - Kind: Deny, - Message: "Denied", - }, - } - }, nil - }), - ParamSource: nil, - FailurePolicy: Fail, - } + return testValidator{} + }, nil) - denyBinding := &FakePolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "denybinding.example.com", - Namespace: "", - ResourceVersion: "1", - }, - Params: "replicas-test.example.com", - Policy: "denypolicy.example.com", - } - - handler, _, tracker, controller := setupTest(t) - - require.NoError(t, tracker.Add(denyPolicy)) - require.NoError(t, tracker.Add(denyBinding)) + require.NoError(t, tracker.Create(definitionsGVR, &noParamSourcePolicy, noParamSourcePolicy.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, denyBindingWithNoParamRef, denyBindingWithNoParamRef.Namespace)) // Wait for controller to reconcile given objects require.NoError(t, @@ -1031,11 +943,10 @@ func TestEmptyParamSource(t *testing.T) { testContext, // Object is irrelevant/unchecked for this test. Just test that // the evaluator is executed, and returns a denial - attributeRecord(nil, denyBinding, admission.Create), + attributeRecord(nil, fakeParams, admission.Create), &admission.RuntimeObjectInterfaces{}, ) - require.ErrorContains(t, err, `{"kind":"Deny","message":"Denied"}`) + require.ErrorContains(t, err, `Denied`) require.Equal(t, 1, numCompiles) - require.Equal(t, []*unstructured.Unstructured{nil}, passedParams) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compiler.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compiler.go index dac2f2ab05c..3adb6623e5c 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compiler.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compiler.go @@ -201,6 +201,17 @@ func CompileValidatingPolicyExpression(validationExpression string, hasParams bo }, } } + + _, err = cel.AstToCheckedExpr(ast) + if err != nil { + // should be impossible since env.Compile returned no issues + return CompilationResult{ + Error: &apiservercel.Error{ + Type: apiservercel.ErrorTypeInternal, + Detail: "unexpected compilation error: " + err.Error(), + }, + } + } prog, err := env.Program(ast, cel.EvalOptions(cel.OptOptimize), cel.OptimizeRegex(library.ExtensionLibRegexOptimizations...), diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/controller.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/controller.go index 150940747e1..8c597e08892 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/controller.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/controller.go @@ -21,28 +21,35 @@ import ( "errors" "fmt" "sync" + "sync/atomic" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "k8s.io/apiserver/pkg/admission/plugin/cel/matching" + + "k8s.io/api/admissionregistration/v1alpha1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "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/plugin/cel/internal/generic" "k8s.io/client-go/dynamic" - "k8s.io/client-go/tools/cache" - "k8s.io/klog/v2" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" ) +var _ CELPolicyEvaluator = &celAdmissionController{} + // celAdmissionController is the top-level controller for admission control using CEL // it is responsible for watching policy definitions, bindings, and config param CRDs type celAdmissionController struct { // Context under which the controller runs runningContext context.Context - policyDefinitionsController generic.Controller[PolicyDefinition] - policyBindingController generic.Controller[PolicyBinding] + policyDefinitionsController generic.Controller[*v1alpha1.ValidatingAdmissionPolicy] + policyBindingController generic.Controller[*v1alpha1.ValidatingAdmissionPolicyBinding] // dynamicclient used to create informers to watch the param crd types dynamicClient dynamic.Interface @@ -50,7 +57,7 @@ type celAdmissionController struct { // Provided to the policy's Compile function as an injected dependency to // assist with compiling its expressions to CEL - objectConverter ObjectConverter + validatorCompiler ValidatorCompiler // Lock which protects: // - definitionInfo @@ -61,21 +68,26 @@ type celAdmissionController struct { mutex sync.RWMutex // controller and metadata - paramsCRDControllers map[schema.GroupVersionKind]*paramInfo + paramsCRDControllers map[v1alpha1.ParamKind]*paramInfo // Index for each definition namespace/name, contains all binding // namespace/names known to exist for that definition - definitionInfo map[string]*definitionInfo + definitionInfo map[namespacedName]*definitionInfo // Index for each bindings namespace/name. Contains compiled templates // for the binding depending on the policy/param combination. - bindingInfos map[string]*bindingInfo + bindingInfos map[namespacedName]*bindingInfo // Map from namespace/name of a definition to a set of namespace/name // of bindings which depend on it. // All keys must have at least one dependent binding // All binding names MUST exist as a key bindingInfos - definitionsToBindings map[string]sets.String + definitionsToBindings map[namespacedName]sets.Set[namespacedName] +} + +// namespaceName is used as a key in definitionInfo and bindingInfos +type namespacedName struct { + namespace, name string } type definitionInfo struct { @@ -86,16 +98,16 @@ type definitionInfo struct { // Last value seen by this controller to be used in policy enforcement // May not be nil - lastReconciledValue PolicyDefinition + lastReconciledValue *v1alpha1.ValidatingAdmissionPolicy } type bindingInfo struct { - // Compiled CEL expression turned into an evaluator - evaluator EvaluatorFunc + // Compiled CEL expression turned into an validator + validator atomic.Pointer[Validator] // Last value seen by this controller to be used in policy enforcement // May not be nil - lastReconciledValue PolicyBinding + lastReconciledValue *v1alpha1.ValidatingAdmissionPolicyBinding } type paramInfo struct { @@ -106,31 +118,33 @@ type paramInfo struct { stop func() // Policy Definitions which refer to this param CRD - dependentDefinitions sets.String + dependentDefinitions sets.Set[namespacedName] } func NewAdmissionController( - // Informers - policyDefinitionsInformer cache.SharedIndexInformer, - policyBindingInformer cache.SharedIndexInformer, - // Injected Dependencies - objectConverter ObjectConverter, + informerFactory informers.SharedInformerFactory, + client kubernetes.Interface, restMapper meta.RESTMapper, dynamicClient dynamic.Interface, ) CELPolicyEvaluator { + matcher := matching.NewMatcher(informerFactory.Core().V1().Namespaces().Lister(), client) + validatorCompiler := &CELValidatorCompiler{ + Matcher: matcher, + } c := &celAdmissionController{ - definitionInfo: make(map[string]*definitionInfo), - bindingInfos: make(map[string]*bindingInfo), - paramsCRDControllers: make(map[schema.GroupVersionKind]*paramInfo), - definitionsToBindings: make(map[string]sets.String), + definitionInfo: make(map[namespacedName]*definitionInfo), + bindingInfos: make(map[namespacedName]*bindingInfo), + paramsCRDControllers: make(map[v1alpha1.ParamKind]*paramInfo), + definitionsToBindings: make(map[namespacedName]sets.Set[namespacedName]), dynamicClient: dynamicClient, - objectConverter: objectConverter, + validatorCompiler: validatorCompiler, restMapper: restMapper, } c.policyDefinitionsController = generic.NewController( - generic.NewInformer[PolicyDefinition](policyDefinitionsInformer), + generic.NewInformer[*v1alpha1.ValidatingAdmissionPolicy]( + informerFactory.Admissionregistration().V1alpha1().ValidatingAdmissionPolicies().Informer()), c.reconcilePolicyDefinition, generic.ControllerOptions{ Workers: 1, @@ -138,7 +152,8 @@ func NewAdmissionController( }, ) c.policyBindingController = generic.NewController( - generic.NewInformer[PolicyBinding](policyBindingInformer), + generic.NewInformer[*v1alpha1.ValidatingAdmissionPolicyBinding]( + informerFactory.Admissionregistration().V1alpha1().ValidatingAdmissionPolicyBindings().Informer()), c.reconcilePolicyBinding, generic.ControllerOptions{ Workers: 1, @@ -187,30 +202,57 @@ func (c *celAdmissionController) Validate( c.mutex.RLock() defer c.mutex.RUnlock() - var allDecisions []PolicyDecisionWithMetadata = nil + var deniedDecisions []policyDecisionWithMetadata - addConfigError := func(err error, definition PolicyDefinition, binding PolicyBinding) { - wrappedError := fmt.Errorf("configuration error: %w", err) - switch p := definition.GetFailurePolicy(); p { - case Ignore: - klog.Info(wrappedError) + addConfigError := func(err error, definition *v1alpha1.ValidatingAdmissionPolicy, binding *v1alpha1.ValidatingAdmissionPolicyBinding) { + // we always default the FailurePolicy if it is unset and validate it in API level + var policy v1alpha1.FailurePolicyType + if definition.Spec.FailurePolicy == nil { + policy = v1alpha1.Fail + } else { + policy = *definition.Spec.FailurePolicy + } + + // apply FailurePolicy specified in ValidatingAdmissionPolicy, the default would be Fail + switch policy { + case v1alpha1.Ignore: + // TODO: add metrics for ignored error here return - case Fail: - allDecisions = append(allDecisions, PolicyDecisionWithMetadata{ - PolicyDecision: PolicyDecision{ - Kind: Deny, - Message: wrappedError.Error(), + case v1alpha1.Fail: + var message string + if binding == nil { + message = fmt.Errorf("failed to configure policy: %w", err).Error() + } else { + message = fmt.Errorf("failed to configure binding: %w", err).Error() + } + deniedDecisions = append(deniedDecisions, policyDecisionWithMetadata{ + policyDecision: policyDecision{ + kind: deny, + message: message, }, - Definition: definition, - Binding: binding, + definition: definition, + binding: binding, }) default: - utilruntime.HandleError(fmt.Errorf("unrecognized failure policy: '%v'", p)) + deniedDecisions = append(deniedDecisions, policyDecisionWithMetadata{ + policyDecision: policyDecision{ + kind: deny, + message: fmt.Errorf("unrecognized failure policy: '%v'", policy).Error(), + }, + definition: definition, + binding: binding, + }) } } for definitionNamespacedName, definitionInfo := range c.definitionInfo { definition := definitionInfo.lastReconciledValue - if !definition.Matches(a) { + matches, matchKind, err := c.validatorCompiler.DefinitionMatches(a, o, definition) + if err != nil { + // Configuration error. + addConfigError(err, definition, nil) + continue + } + if !matches { // Policy definition does not match request continue } else if definitionInfo.configurationError != nil { @@ -221,8 +263,6 @@ func (c *celAdmissionController) Validate( dependentBindings := c.definitionsToBindings[definitionNamespacedName] if len(dependentBindings) == 0 { - // Definition has no known bindings yet. - addConfigError(errors.New("no bindings found"), definition, nil) continue } @@ -231,40 +271,61 @@ func (c *celAdmissionController) Validate( // be a bindingInfo for it bindingInfo := c.bindingInfos[namespacedBindingName] binding := bindingInfo.lastReconciledValue - if !binding.Matches(a) { + matches, err := c.validatorCompiler.BindingMatches(a, o, binding) + if err != nil { + // Configuration error. + addConfigError(err, definition, binding) + continue + } + if !matches { continue } var param *unstructured.Unstructured - // If definition has no paramsource, always provide nil params to - // evaluator. If binding specifies a params to use they are ignored. - // Done this way so you can configure params before definition is ready. - if paramSource := definition.GetParamSource(); paramSource != nil { - paramsNamespace, paramsName := binding.GetTargetParams() + // If definition has paramKind, paramRef is required in binding. + // If definition has no paramKind, paramRef set in binding will be ignored. + paramKind := definition.Spec.ParamKind + paramRef := binding.Spec.ParamRef + if paramKind != nil && paramRef != nil { // Find the params referred by the binding by looking its name up // in our informer for its CRD - paramInfo, ok := c.paramsCRDControllers[*paramSource] + paramInfo, ok := c.paramsCRDControllers[*paramKind] if !ok { - addConfigError(fmt.Errorf("paramSource kind `%v` not known", - paramSource.String()), definition, binding) + addConfigError(fmt.Errorf("paramKind kind `%v` not known", + paramKind.String()), definition, binding) continue } - if len(paramsNamespace) == 0 { - param, err = paramInfo.controller.Informer().Get(paramsName) + // If the param informer for this admission policy has not yet + // had time to perform an initial listing, don't attempt to use + // it. + //!TOOD(alexzielenski): add a wait for a very short amount of + // time for the cache to sync + if !paramInfo.controller.HasSynced() { + addConfigError(fmt.Errorf("paramKind kind `%v` not yet synced to use for admission", + paramKind.String()), definition, binding) + continue + } + + if len(paramRef.Namespace) == 0 { + param, err = paramInfo.controller.Informer().Get(paramRef.Name) } else { - param, err = paramInfo.controller.Informer().Namespaced(paramsNamespace).Get(paramsName) + param, err = paramInfo.controller.Informer().Namespaced(paramRef.Namespace).Get(paramRef.Name) } if err != nil { // Apply failure policy addConfigError(err, definition, binding) - if k8serrors.IsNotFound(err) { - // Param doesnt exist yet? - // Maybe just have to wait a bit. + if k8serrors.IsInvalid(err) { + // Param mis-configured + // require to set paramRef.namespace for namespaced resource and unset paramRef.namespace for cluster scoped resource + continue + } else if k8serrors.IsNotFound(err) { + // Param not yet available. User may need to wait a bit + // before being able to use it for validation. continue } @@ -274,47 +335,69 @@ func (c *celAdmissionController) Validate( } } - if bindingInfo.evaluator == nil { + validator := bindingInfo.validator.Load() + if validator == nil { // Compile policy definition using binding - bindingInfo.evaluator, err = definition.Compile(c.objectConverter, c.restMapper) - if err != nil { - // compilation error. Apply failure policy - wrappedError := fmt.Errorf("failed to compile CEL expression: %w", err) - addConfigError(wrappedError, definition, binding) - continue - } - c.bindingInfos[namespacedBindingName] = bindingInfo + newValidator := c.validatorCompiler.Compile(definition) + validator = &newValidator + + bindingInfo.validator.Store(validator) + } + + decisions, err := (*validator).Validate(a, o, param, matchKind) + if err != nil { + // runtime error. Apply failure policy + wrappedError := fmt.Errorf("failed to evaluate CEL expression: %w", err) + addConfigError(wrappedError, definition, binding) + continue } - decisions := bindingInfo.evaluator(a, param) for _, decision := range decisions { - switch decision.Kind { - case Admit: - // Do nothing - case Deny: - allDecisions = append(allDecisions, PolicyDecisionWithMetadata{ - Definition: definition, - Binding: binding, - PolicyDecision: decision, + switch decision.kind { + case admit: + // TODO: add metrics for ignored error here + case deny: + deniedDecisions = append(deniedDecisions, policyDecisionWithMetadata{ + definition: definition, + binding: binding, + policyDecision: decision, }) default: - // unrecognized decision. ignore + return fmt.Errorf("unrecognized evaluation decision '%s' for ValidatingAdmissionPolicyBinding '%s' with ValidatingAdmissionPolicy '%s'", + decision.kind, binding.Name, definition.Name) } } } } - if len(allDecisions) > 0 { - return k8serrors.NewConflict( - a.GetResource().GroupResource(), a.GetName(), - &PolicyError{ - Decisions: allDecisions, - }) + if len(deniedDecisions) > 0 { + // TODO: refactor admission.NewForbidden so the name extraction is reusable but the code/reason is customizable + var message string + deniedDecision := deniedDecisions[0] + if deniedDecision.binding != nil { + message = fmt.Sprintf("ValidatingAdmissionPolicy '%s' with binding '%s' denied request: %s", deniedDecision.definition.Name, deniedDecision.binding.Name, deniedDecision.message) + } else { + message = fmt.Sprintf("ValidatingAdmissionPolicy '%s' denied request: %s", deniedDecision.definition.Name, deniedDecision.message) + } + err := admission.NewForbidden(a, errors.New(message)).(*k8serrors.StatusError) + reason := deniedDecision.reason + if len(reason) == 0 { + reason = metav1.StatusReasonInvalid + } + err.ErrStatus.Reason = reason + err.ErrStatus.Code = reasonToCode(reason) + err.ErrStatus.Details.Causes = append(err.ErrStatus.Details.Causes, metav1.StatusCause{Message: message}) + return err } return nil } + func (c *celAdmissionController) HasSynced() bool { return c.policyBindingController.HasSynced() && c.policyDefinitionsController.HasSynced() } + +func (c *celAdmissionController) ValidateInitialization() error { + return c.validatorCompiler.ValidateInitialization() +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/controller_reconcile.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/controller_reconcile.go index d0aa7d1ae78..580941172f7 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/controller_reconcile.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/controller_reconcile.go @@ -21,6 +21,7 @@ import ( "fmt" "time" + "k8s.io/api/admissionregistration/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -30,28 +31,27 @@ import ( "k8s.io/client-go/tools/cache" ) -func (c *celAdmissionController) reconcilePolicyDefinition(namespace, name string, definition PolicyDefinition) error { +func (c *celAdmissionController) reconcilePolicyDefinition(namespace, name string, definition *v1alpha1.ValidatingAdmissionPolicy) error { c.mutex.Lock() defer c.mutex.Unlock() - // Namespace for policydefinition is empty. Leaving usage here for compatibility - // with future NamespacedPolicyDefinition - namespacedName := namespace + "/" + name - info, ok := c.definitionInfo[namespacedName] + // Namespace for policydefinition is empty. + nn := getNamespaceName(namespace, name) + info, ok := c.definitionInfo[nn] if !ok { info = &definitionInfo{} - c.definitionInfo[namespacedName] = info + c.definitionInfo[nn] = info } - var paramSource *schema.GroupVersionKind + var paramSource *v1alpha1.ParamKind if definition != nil { - paramSource = definition.GetParamSource() + paramSource = definition.Spec.ParamKind } // If param source has changed, remove definition as dependent of old params // If there are no more dependents of old param, stop and clean up controller - if info.lastReconciledValue != nil && info.lastReconciledValue.GetParamSource() != nil { - oldParamSource := *info.lastReconciledValue.GetParamSource() + if info.lastReconciledValue != nil && info.lastReconciledValue.Spec.ParamKind != nil { + oldParamSource := *info.lastReconciledValue.Spec.ParamKind // If we are: // - switching from having a param to not having a param (includes deletion) @@ -59,7 +59,7 @@ func (c *celAdmissionController) reconcilePolicyDefinition(namespace, name strin // we remove dependency on the controller. if paramSource == nil || *paramSource != oldParamSource { if oldParamInfo, ok := c.paramsCRDControllers[oldParamSource]; ok { - oldParamInfo.dependentDefinitions.Delete(namespacedName) + oldParamInfo.dependentDefinitions.Delete(nn) if len(oldParamInfo.dependentDefinitions) == 0 { oldParamInfo.stop() delete(c.paramsCRDControllers, oldParamSource) @@ -70,14 +70,14 @@ func (c *celAdmissionController) reconcilePolicyDefinition(namespace, name strin // Reset all previously compiled evaluators in case something relevant in // definition has changed. - for key := range c.definitionsToBindings[namespacedName] { + for key := range c.definitionsToBindings[nn] { bindingInfo := c.bindingInfos[key] - bindingInfo.evaluator = nil + bindingInfo.validator.Store(nil) c.bindingInfos[key] = bindingInfo } if definition == nil { - delete(c.definitionInfo, namespacedName) + delete(c.definitionInfo, nn) return nil } @@ -91,12 +91,28 @@ func (c *celAdmissionController) reconcilePolicyDefinition(namespace, name strin } // find GVR for params - paramsGVR, err := c.restMapper.RESTMapping(paramSource.GroupKind(), paramSource.Version) + // Parse param source into a GVK + + paramSourceGV, err := schema.ParseGroupVersion(paramSource.APIVersion) + if err != nil { + // Failed to resolve. Return error so we retry again (rate limited) + // Save a record of this definition with an evaluator that unconditionally + info.configurationError = fmt.Errorf("failed to parse apiVersion of paramKind '%v' with error: %w", paramSource.String(), err) + + // Return nil, since this error cannot be resolved by waiting more time + return nil + } + + paramsGVR, err := c.restMapper.RESTMapping(schema.GroupKind{ + Group: paramSourceGV.Group, + Kind: paramSource.Kind, + }, paramSourceGV.Version) + if err != nil { // Failed to resolve. Return error so we retry again (rate limited) // Save a record of this definition with an evaluator that unconditionally // - info.configurationError = fmt.Errorf("failed to find resource for param source: '%v'", paramSource.String()) + info.configurationError = fmt.Errorf("failed to find resource referenced by paramKind: '%v'", paramSourceGV.WithKind(paramSource.Kind)) return info.configurationError } @@ -126,7 +142,7 @@ func (c *celAdmissionController) reconcilePolicyDefinition(namespace, name strin c.paramsCRDControllers[*paramSource] = ¶mInfo{ controller: controller, stop: instanceCancel, - dependentDefinitions: sets.NewString(namespacedName), + dependentDefinitions: sets.New(nn), } go informer.Informer().Run(instanceContext.Done()) @@ -136,37 +152,37 @@ func (c *celAdmissionController) reconcilePolicyDefinition(namespace, name strin return nil } -func (c *celAdmissionController) reconcilePolicyBinding(namespace, name string, binding PolicyBinding) error { +func (c *celAdmissionController) reconcilePolicyBinding(namespace, name string, binding *v1alpha1.ValidatingAdmissionPolicyBinding) error { c.mutex.Lock() defer c.mutex.Unlock() // Namespace for PolicyBinding is empty. In the future a namespaced binding // may be added // https://github.com/kubernetes/enhancements/blob/bf5c3c81ea2081d60c1dc7c832faa98479e06209/keps/sig-api-machinery/3488-cel-admission-control/README.md?plain=1#L1042 - namespacedName := namespace + "/" + name - info, ok := c.bindingInfos[namespacedName] + nn := getNamespaceName(namespace, name) + info, ok := c.bindingInfos[nn] if !ok { info = &bindingInfo{} - c.bindingInfos[namespacedName] = info + c.bindingInfos[nn] = info } - oldNamespacedDefinitionName := "" + var oldNamespacedDefinitionName namespacedName if info.lastReconciledValue != nil { - oldefinitionNamespace, oldefinitionName := info.lastReconciledValue.GetTargetDefinition() - oldNamespacedDefinitionName = oldefinitionNamespace + "/" + oldefinitionName + // All validating policies are cluster-scoped so have empty namespace + oldNamespacedDefinitionName = getNamespaceName("", info.lastReconciledValue.Spec.PolicyName) } - namespacedDefinitionName := "" + var namespacedDefinitionName namespacedName if binding != nil { - newDefinitionNamespace, newDefinitionName := binding.GetTargetDefinition() - namespacedDefinitionName = newDefinitionNamespace + "/" + newDefinitionName + // All validating policies are cluster-scoped so have empty namespace + namespacedDefinitionName = getNamespaceName("", binding.Spec.PolicyName) } // Remove record of binding from old definition if the referred policy // has changed if oldNamespacedDefinitionName != namespacedDefinitionName { if dependentBindings, ok := c.definitionsToBindings[oldNamespacedDefinitionName]; ok { - dependentBindings.Delete(namespacedName) + dependentBindings.Delete(nn) // if there are no more dependent bindings, remove knowledge of the // definition altogether @@ -177,19 +193,19 @@ func (c *celAdmissionController) reconcilePolicyBinding(namespace, name string, } if binding == nil { - delete(c.bindingInfos, namespacedName) + delete(c.bindingInfos, nn) return nil } // Add record of binding to new definition if dependentBindings, ok := c.definitionsToBindings[namespacedDefinitionName]; ok { - dependentBindings.Insert(namespacedName) + dependentBindings.Insert(nn) } else { - c.definitionsToBindings[namespacedDefinitionName] = sets.NewString(namespacedName) + c.definitionsToBindings[namespacedDefinitionName] = sets.New(nn) } // Remove compiled template for old binding - info.evaluator = nil + info.validator.Store(nil) info.lastReconciledValue = binding return nil } @@ -201,3 +217,10 @@ func (c *celAdmissionController) reconcileParams(namespace, name string, params // checker errors to the status of the resources. return nil } + +func getNamespaceName(namespace, name string) namespacedName { + return namespacedName{ + namespace: namespace, + name: name, + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/fake.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/fake.go deleted file mode 100644 index 9163f0e9e8a..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/fake.go +++ /dev/null @@ -1,258 +0,0 @@ -/* -Copyright 2022 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 ( - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apiserver/pkg/admission" -) - -//////////////////////////////////////////////////////////////////////////////// -// Fake Policy Definitions -//////////////////////////////////////////////////////////////////////////////// - -type FakePolicyDefinition struct { - metav1.TypeMeta - metav1.ObjectMeta - - // Function called when `Matches` is called - // If nil, a default function that always returns true is used - // Specified as a function pointer so that this type is still comparable - MatchFunc *func(admission.Attributes) bool `json:"-"` - - // Func invoked for implementation of `Compile` - // Specified as a function pointer so that this type is still comparable - CompileFunc *func(converter ObjectConverter) (EvaluatorFunc, error) `json:"-"` - - // GVK to return when ParamSource() is called - ParamSource *schema.GroupVersionKind `json:"paramSource"` - - FailurePolicy FailurePolicy `json:"failurePolicy"` -} - -var _ PolicyDefinition = &FakePolicyDefinition{} - -func (f *FakePolicyDefinition) SetGroupVersionKind(kind schema.GroupVersionKind) { - f.TypeMeta.APIVersion = kind.GroupVersion().String() - f.TypeMeta.Kind = kind.Kind -} - -func (f *FakePolicyDefinition) GroupVersionKind() schema.GroupVersionKind { - parsedGV, err := schema.ParseGroupVersion(f.TypeMeta.APIVersion) - if err != nil || f.TypeMeta.Kind == "" || parsedGV.Empty() { - return schema.GroupVersionKind{ - Group: "admission.k8s.io", - Version: "v1alpha1", - Kind: "PolicyDefinition", - } - } - return schema.GroupVersionKind{ - Group: parsedGV.Group, - Version: parsedGV.Version, - Kind: f.TypeMeta.Kind, - } -} - -func (f *FakePolicyDefinition) GetObjectKind() schema.ObjectKind { - return f -} - -func (f *FakePolicyDefinition) DeepCopyObject() runtime.Object { - copied := *f - f.ObjectMeta.DeepCopyInto(&copied.ObjectMeta) - return &copied -} - -func (f *FakePolicyDefinition) GetName() string { - return f.ObjectMeta.Name -} - -func (f *FakePolicyDefinition) GetNamespace() string { - return f.ObjectMeta.Namespace -} - -func (f *FakePolicyDefinition) Matches(a admission.Attributes) bool { - if f.MatchFunc == nil || *f.MatchFunc == nil { - return true - } - return (*f.MatchFunc)(a) -} - -func (f *FakePolicyDefinition) Compile( - converter ObjectConverter, - mapper meta.RESTMapper, -) (EvaluatorFunc, error) { - if f.CompileFunc == nil || *f.CompileFunc == nil { - panic("must provide a CompileFunc to policy definition") - } - return (*f.CompileFunc)(converter) -} - -func (f *FakePolicyDefinition) GetParamSource() *schema.GroupVersionKind { - return f.ParamSource -} - -func (f *FakePolicyDefinition) GetFailurePolicy() FailurePolicy { - return f.FailurePolicy -} - -//////////////////////////////////////////////////////////////////////////////// -// Fake Policy Binding -//////////////////////////////////////////////////////////////////////////////// - -type FakePolicyBinding struct { - metav1.TypeMeta - metav1.ObjectMeta - - // Specified as a function pointer so that this type is still comparable - MatchFunc *func(admission.Attributes) bool `json:"-"` - Params string `json:"params"` - Policy string `json:"policy"` -} - -var _ PolicyBinding = &FakePolicyBinding{} - -func (f *FakePolicyBinding) SetGroupVersionKind(kind schema.GroupVersionKind) { - f.TypeMeta.APIVersion = kind.GroupVersion().String() - f.TypeMeta.Kind = kind.Kind -} - -func (f *FakePolicyBinding) GroupVersionKind() schema.GroupVersionKind { - parsedGV, err := schema.ParseGroupVersion(f.TypeMeta.APIVersion) - if err != nil || f.TypeMeta.Kind == "" || parsedGV.Empty() { - return schema.GroupVersionKind{ - Group: "admission.k8s.io", - Version: "v1alpha1", - Kind: "PolicyBinding", - } - } - return schema.GroupVersionKind{ - Group: parsedGV.Group, - Version: parsedGV.Version, - Kind: f.TypeMeta.Kind, - } -} - -func (f *FakePolicyBinding) GetObjectKind() schema.ObjectKind { - return f -} - -func (f *FakePolicyBinding) DeepCopyObject() runtime.Object { - copied := *f - f.ObjectMeta.DeepCopyInto(&copied.ObjectMeta) - return &copied -} - -func (f *FakePolicyBinding) Matches(a admission.Attributes) bool { - if f.MatchFunc == nil || *f.MatchFunc == nil { - return true - } - return (*f.MatchFunc)(a) -} - -func (f *FakePolicyBinding) GetTargetDefinition() (namespace, name string) { - return f.Namespace, f.Policy -} - -func (f *FakePolicyBinding) GetTargetParams() (namespace, name string) { - return f.Namespace, f.Params -} - -/// List Types - -type FakePolicyDefinitionList struct { - metav1.TypeMeta - metav1.ListMeta - - Items []FakePolicyDefinition -} - -func (f *FakePolicyDefinitionList) SetGroupVersionKind(kind schema.GroupVersionKind) { - f.TypeMeta.APIVersion = kind.GroupVersion().String() - f.TypeMeta.Kind = kind.Kind -} - -func (f *FakePolicyDefinitionList) GroupVersionKind() schema.GroupVersionKind { - parsedGV, err := schema.ParseGroupVersion(f.TypeMeta.APIVersion) - if err != nil || f.TypeMeta.Kind == "" || parsedGV.Empty() { - return schema.GroupVersionKind{ - Group: "admission.k8s.io", - Version: "v1alpha1", - Kind: "PolicyDefinitionList", - } - } - return schema.GroupVersionKind{ - Group: parsedGV.Group, - Version: parsedGV.Version, - Kind: f.TypeMeta.Kind, - } -} - -func (f *FakePolicyDefinitionList) GetObjectKind() schema.ObjectKind { - return f -} - -func (f *FakePolicyDefinitionList) DeepCopyObject() runtime.Object { - copied := *f - f.ListMeta.DeepCopyInto(&copied.ListMeta) - copied.Items = make([]FakePolicyDefinition, len(f.Items)) - copy(copied.Items, f.Items) - return &copied -} - -type FakePolicyBindingList struct { - metav1.TypeMeta - metav1.ListMeta - - Items []FakePolicyBinding -} - -func (f *FakePolicyBindingList) SetGroupVersionKind(kind schema.GroupVersionKind) { - f.TypeMeta.APIVersion = kind.GroupVersion().String() - f.TypeMeta.Kind = kind.Kind -} - -func (f *FakePolicyBindingList) GroupVersionKind() schema.GroupVersionKind { - parsedGV, err := schema.ParseGroupVersion(f.TypeMeta.APIVersion) - if err != nil || f.TypeMeta.Kind == "" || parsedGV.Empty() { - return schema.GroupVersionKind{ - Group: "admission.k8s.io", - Version: "v1alpha1", - Kind: "PolicyBindingList", - } - } - return schema.GroupVersionKind{ - Group: parsedGV.Group, - Version: parsedGV.Version, - Kind: f.TypeMeta.Kind, - } -} - -func (f *FakePolicyBindingList) GetObjectKind() schema.ObjectKind { - return f -} - -func (f *FakePolicyBindingList) DeepCopyObject() runtime.Object { - copied := *f - f.ListMeta.DeepCopyInto(&copied.ListMeta) - copied.Items = make([]FakePolicyBinding, len(f.Items)) - copy(copied.Items, f.Items) - return &copied -} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/initializer.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/initializer.go index 18fa3e119d0..f35bd571663 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/initializer.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/initializer.go @@ -18,38 +18,13 @@ package cel import ( "context" - "k8s.io/apiserver/pkg/admission" ) type CELPolicyEvaluator interface { + admission.InitializationValidator + Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error HasSynced() bool -} - -// NewPluginInitializer creates a plugin initializer which dependency injects a -// singleton cel admission controller into the plugins which desire it -func NewPluginInitializer(validator CELPolicyEvaluator) *PluginInitializer { - return &PluginInitializer{validator: validator} -} - -// WantsCELPolicyEvaluator gives the ability to have the shared -// CEL Admission Controller dependency injected at initialization-time. -type WantsCELPolicyEvaluator interface { - SetCELPolicyEvaluator(CELPolicyEvaluator) -} - -// PluginInitializer is used for initialization of the webhook admission plugin. -type PluginInitializer struct { - validator CELPolicyEvaluator -} - -var _ admission.PluginInitializer = &PluginInitializer{} - -// Initialize checks the initialization interfaces implemented by each plugin -// and provide the appropriate initialization data -func (i *PluginInitializer) Initialize(plugin admission.Interface) { - if wants, ok := plugin.(WantsCELPolicyEvaluator); ok { - wants.SetCELPolicyEvaluator(i.validator) - } + Run(stopCh <-chan struct{}) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/interface.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/interface.go index d23ef46b838..8a20110d48e 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/interface.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/interface.go @@ -17,92 +17,34 @@ limitations under the License. package cel import ( - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/api/admissionregistration/v1alpha1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/cel" - - "github.com/google/cel-go/common/types/ref" ) -type FailurePolicy string - -const ( - Fail FailurePolicy = "Fail" - Ignore FailurePolicy = "Ignore" -) - -// EvaluatorFunc represents the AND of one or more compiled CEL expression's -// evaluators `params` may be nil if definition does not specify a paramsource -type EvaluatorFunc func(a admission.Attributes, params *unstructured.Unstructured) []PolicyDecision - -// ObjectConverter is Dependency Injected into the PolicyDefinition's `Compile` -// function to assist with converting types and values to/from CEL-typed values. -type ObjectConverter interface { - // DeclForResource looks up the openapi or JSONSchemaProps, structural schema, etc. - // and compiles it into something that can be used to turn objects into CEL - // values - DeclForResource(gvr schema.GroupVersionResource) (*cel.DeclType, error) - - // ValueForObject takes a Kubernetes Object and uses the CEL DeclType - // to transform it into a CEL value. - // Object may be a typed native object or an unstructured object - ValueForObject(value runtime.Object, decl *cel.DeclType) (ref.Val, error) +// Validator defines the func used to validate the cel expressions +// matchKind provides the GroupVersionKind that the object should be +// validated by CEL expressions as. +type Validator interface { + Validate(a admission.Attributes, o admission.ObjectInterfaces, versionedParams runtime.Object, matchKind schema.GroupVersionKind) ([]policyDecision, error) } -// PolicyDefinition is an interface for internal policy binding type. -// Implemented by mock/testing types, and to be implemented by the public API -// types once they have completed API review. -// -// The interface closely mirrors the format and functionality of the -// PolicyDefinition proposed in the KEP. -type PolicyDefinition interface { - runtime.Object +// ValidatorCompiler is Dependency Injected into the PolicyDefinition's `Compile` +// function to assist with converting types and values to/from CEL-typed values. +type ValidatorCompiler interface { + admission.InitializationValidator // Matches says whether this policy definition matches the provided admission // resource request - Matches(a admission.Attributes) bool + DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1alpha1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionKind, error) - Compile( - // Definition is provided with a converter which may be used by the - // return evaluator function to convert objects into CEL-typed objects - objectConverter ObjectConverter, - // Injected RESTMapper to assist with compilation - mapper meta.RESTMapper, - ) (EvaluatorFunc, error) - - // GetParamSource returns the GVK for the CRD used as the source of - // parameters used in the evaluation of instances of this policy - // May return nil if there is no paramsource for this definition. - GetParamSource() *schema.GroupVersionKind - - // GetFailurePolicy returns how an object should be treated during an - // admission when there is a configuration error preventing CEL evaluation - GetFailurePolicy() FailurePolicy -} - -// PolicyBinding is an interface for internal policy binding type. Implemented -// by mock/testing types, and to be implemented by the public API types once -// they have completed API review. -// -// The interface closely mirrors the format and functionality of the -// PolicyBinding proposed in the KEP. -type PolicyBinding interface { - runtime.Object - - // Matches says whether this policy binding matches the provided admission + // Matches says whether this policy definition matches the provided admission // resource request - Matches(a admission.Attributes) bool + BindingMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1alpha1.ValidatingAdmissionPolicyBinding) (bool, error) - // GetTargetDefinition returns the Namespace/Name of Policy Definition used - // by this binding. - GetTargetDefinition() (namespace, name string) - - // GetTargetParams returns the Namespace/Name of instance of TargetDefinition's - // ParamSource to be provided to the CEL expressions of the definition during - // evaluation. - // If TargetDefinition has nil ParamSource, this is ignored. - GetTargetParams() (namespace, name string) + // Compile is used for the cel expression compilation + Compile( + policy *v1alpha1.ValidatingAdmissionPolicy, + ) Validator } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/internal/generic/controller.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/internal/generic/controller.go index 10e42b5009c..bd5ea818d67 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/internal/generic/controller.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/internal/generic/controller.go @@ -52,7 +52,7 @@ type ControllerOptions struct { Workers uint } -func (c controller[T]) Informer() Informer[T] { +func (c *controller[T]) Informer() Informer[T] { return c.informer } @@ -73,7 +73,7 @@ func NewController[T runtime.Object]( options: options, informer: informer, reconciler: reconciler, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), options.Name), + queue: nil, } } @@ -84,10 +84,18 @@ func (c *controller[T]) Run(ctx context.Context) error { klog.Infof("starting %s", c.options.Name) defer klog.Infof("stopping %s", c.options.Name) + c.queue = workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), c.options.Name) + + // Forcefully shutdown workqueue. Drop any enqueued items. + // Important to do this in a `defer` at the start of `Run`. + // Otherwise, if there are any early returns without calling this, we + // would never shut down the workqueue + defer c.queue.ShutDown() + enqueue := func(obj interface{}) { var key string var err error - if key, err = cache.MetaNamespaceKeyFunc(obj); err != nil { + if key, err = cache.DeletionHandlingMetaNamespaceKeyFunc(obj); err != nil { utilruntime.HandleError(err) return } @@ -185,7 +193,7 @@ func (c *controller[T]) HasSynced() bool { func (c *controller[T]) runWorker() { for { - obj, shutdown := c.queue.Get() + key, shutdown := c.queue.Get() if shutdown { return } @@ -221,9 +229,9 @@ func (c *controller[T]) runWorker() { // Finally, if no error occurs we Forget this item so it is allowed // to be re-enqueued without a long rate limit c.queue.Forget(obj) - klog.Infof("Successfully synced '%s'", key) + klog.V(4).Infof("syncAdmissionPolicy(%q)", key) return nil - }(obj) + }(key) if err != nil { utilruntime.HandleError(err) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/policy_decision.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/policy_decision.go index 800da35498b..768fb72e4e9 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/policy_decision.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/policy_decision.go @@ -17,37 +17,43 @@ limitations under the License. package cel import ( - "encoding/json" - "fmt" + "net/http" + + "k8s.io/api/admissionregistration/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -type PolicyDecisionKind string +type policyDecisionKind string const ( - Admit PolicyDecisionKind = "Admit" - Deny PolicyDecisionKind = "Deny" + admit policyDecisionKind = "admit" + deny policyDecisionKind = "deny" ) -type PolicyDecision struct { - Kind PolicyDecisionKind `json:"kind"` - Message any `json:"message"` +type policyDecision struct { + kind policyDecisionKind + message string + reason metav1.StatusReason } -type PolicyDecisionWithMetadata struct { - PolicyDecision `json:"decision"` - Definition PolicyDefinition `json:"definition"` - Binding PolicyBinding `json:"binding"` +type policyDecisionWithMetadata struct { + policyDecision + definition *v1alpha1.ValidatingAdmissionPolicy + binding *v1alpha1.ValidatingAdmissionPolicyBinding } -type PolicyError struct { - Decisions []PolicyDecisionWithMetadata -} - -func (p *PolicyError) Error() string { - // Just format the error as JSON - jsonText, err := json.Marshal(p.Decisions) - if err != nil { - return fmt.Sprintf("error formatting PolicyError: %s", err.Error()) +func reasonToCode(r metav1.StatusReason) int32 { + switch r { + case metav1.StatusReasonForbidden: + return http.StatusForbidden + case metav1.StatusReasonUnauthorized: + return http.StatusUnauthorized + case metav1.StatusReasonRequestEntityTooLarge: + return http.StatusRequestEntityTooLarge + case metav1.StatusReasonInvalid: + return http.StatusUnprocessableEntity + default: + // It should not reach here since we only allow above reason to be set from API level + return http.StatusUnprocessableEntity } - return string(jsonText) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/validator.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/validator.go new file mode 100644 index 00000000000..3284f01de2b --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/validator.go @@ -0,0 +1,310 @@ +/* +Copyright 2022 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" + "reflect" + "strings" + + celtypes "github.com/google/cel-go/common/types" + "github.com/google/cel-go/interpreter" + + admissionv1 "k8s.io/api/admission/v1" + "k8s.io/api/admissionregistration/v1alpha1" + authenticationv1 "k8s.io/api/authentication/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/admission/plugin/cel/matching" + "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" +) + +var _ ValidatorCompiler = &CELValidatorCompiler{} +var _ matching.MatchCriteria = &matchCriteria{} + +type matchCriteria struct { + constraints *v1alpha1.MatchResources +} + +// GetParsedNamespaceSelector returns the converted LabelSelector which implements labels.Selector +func (m *matchCriteria) GetParsedNamespaceSelector() (labels.Selector, error) { + return metav1.LabelSelectorAsSelector(m.constraints.NamespaceSelector) +} + +// GetParsedObjectSelector returns the converted LabelSelector which implements labels.Selector +func (m *matchCriteria) GetParsedObjectSelector() (labels.Selector, error) { + return metav1.LabelSelectorAsSelector(m.constraints.ObjectSelector) +} + +// GetMatchResources returns the matchConstraints +func (m *matchCriteria) GetMatchResources() v1alpha1.MatchResources { + return *m.constraints +} + +// CELValidatorCompiler implement the interface ValidatorCompiler. +type CELValidatorCompiler struct { + Matcher *matching.Matcher +} + +// DefinitionMatches returns whether this ValidatingAdmissionPolicy matches the provided admission resource request +func (c *CELValidatorCompiler) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1alpha1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionKind, error) { + criteria := matchCriteria{constraints: definition.Spec.MatchConstraints} + return c.Matcher.Matches(a, o, &criteria) +} + +// BindingMatches returns whether this ValidatingAdmissionPolicyBinding matches the provided admission resource request +func (c *CELValidatorCompiler) BindingMatches(a admission.Attributes, o admission.ObjectInterfaces, binding *v1alpha1.ValidatingAdmissionPolicyBinding) (bool, error) { + if binding.Spec.MatchResources == nil { + return true, nil + } + criteria := matchCriteria{constraints: binding.Spec.MatchResources} + isMatch, _, err := c.Matcher.Matches(a, o, &criteria) + return isMatch, err +} + +// ValidateInitialization checks if Matcher is initialized. +func (c *CELValidatorCompiler) ValidateInitialization() error { + return c.Matcher.ValidateInitialization() +} + +type validationActivation struct { + object, oldObject, params, request interface{} +} + +// ResolveName returns a value from the activation by qualified name, or false if the name +// could not be found. +func (a *validationActivation) ResolveName(name string) (interface{}, bool) { + switch name { + case ObjectVarName: + return a.object, true + case OldObjectVarName: + return a.oldObject, true + case ParamsVarName: + return a.params, true + case RequestVarName: + return a.request, true + default: + return nil, false + } +} + +// Parent returns the parent of the current activation, may be nil. +// If non-nil, the parent will be searched during resolve calls. +func (a *validationActivation) Parent() interpreter.Activation { + return nil +} + +// Compile compiles the cel expression defined in ValidatingAdmissionPolicy +func (c *CELValidatorCompiler) Compile(p *v1alpha1.ValidatingAdmissionPolicy) Validator { + if len(p.Spec.Validations) == 0 { + return nil + } + hasParam := false + if p.Spec.ParamKind != nil { + hasParam = true + } + compilationResults := make([]CompilationResult, len(p.Spec.Validations)) + for i, validation := range p.Spec.Validations { + compilationResults[i] = CompileValidatingPolicyExpression(validation.Expression, hasParam) + } + return &CELValidator{policy: p, compilationResults: compilationResults} +} + +// CELValidator implements the Validator interface +type CELValidator struct { + policy *v1alpha1.ValidatingAdmissionPolicy + compilationResults []CompilationResult +} + +func convertObjectToUnstructured(obj interface{}) (*unstructured.Unstructured, error) { + if obj == nil || reflect.ValueOf(obj).IsNil() { + return &unstructured.Unstructured{Object: nil}, nil + } + ret, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + if err != nil { + return nil, err + } + return &unstructured.Unstructured{Object: ret}, nil +} + +func objectToResolveVal(r runtime.Object) (interface{}, error) { + if r == nil { + return nil, nil + } + v, err := convertObjectToUnstructured(r) + if err != nil { + return nil, err + } + return v.Object, nil +} + +func policyDecisionKindForError(f v1alpha1.FailurePolicyType) policyDecisionKind { + if f == v1alpha1.Ignore { + return admit + } + return deny +} + +// Validate validates all cel expressions in Validator and returns a PolicyDecision for each CEL expression or returns an error. +// An error will be returned if failed to convert the object/oldObject/params/request to unstructured. +// Each PolicyDecision will have a decision and a message. +// policyDecision.message will be empty if the decision is allowed and no error met. +func (v *CELValidator) Validate(a admission.Attributes, o admission.ObjectInterfaces, versionedParams runtime.Object, matchKind schema.GroupVersionKind) ([]policyDecision, error) { + // TODO: replace unstructured with ref.Val for CEL variables when native type support is available + + decisions := make([]policyDecision, len(v.compilationResults)) + var err error + versionedAttr, err := generic.NewVersionedAttributes(a, matchKind, o) + if err != nil { + return nil, err + } + oldObjectVal, err := objectToResolveVal(versionedAttr.VersionedOldObject) + if err != nil { + return nil, err + } + objectVal, err := objectToResolveVal(versionedAttr.VersionedObject) + if err != nil { + return nil, err + } + paramsVal, err := objectToResolveVal(versionedParams) + if err != nil { + return nil, err + } + request := createAdmissionRequest(versionedAttr.Attributes) + requestVal, err := convertObjectToUnstructured(request) + if err != nil { + return nil, err + } + va := &validationActivation{ + object: objectVal, + oldObject: oldObjectVal, + params: paramsVal, + request: requestVal.Object, + } + + var f v1alpha1.FailurePolicyType + if v.policy.Spec.FailurePolicy == nil { + f = v1alpha1.Fail + } else { + f = *v.policy.Spec.FailurePolicy + } + + for i, compilationResult := range v.compilationResults { + validation := v.policy.Spec.Validations[i] + + var policyDecision = &decisions[i] + + if compilationResult.Error != nil { + policyDecision.kind = policyDecisionKindForError(f) + policyDecision.message = fmt.Sprintf("compilation error: %v", compilationResult.Error) + continue + } + if compilationResult.Program == nil { + policyDecision.kind = policyDecisionKindForError(f) + policyDecision.message = "unexpected internal error compiling expression" + continue + } + evalResult, _, err := compilationResult.Program.Eval(va) + if err != nil { + policyDecision.kind = policyDecisionKindForError(f) + policyDecision.message = fmt.Sprintf("expression '%v' resulted in error: %v", v.policy.Spec.Validations[i].Expression, err) + } else if evalResult != celtypes.True { + policyDecision.kind = deny + if validation.Reason == nil { + policyDecision.reason = metav1.StatusReasonInvalid + } else { + policyDecision.reason = *validation.Reason + } + if len(validation.Message) > 0 { + policyDecision.message = strings.TrimSpace(validation.Message) + } else { + policyDecision.message = fmt.Sprintf("failed expression: %v", strings.TrimSpace(validation.Expression)) + } + + } else { + policyDecision.kind = admit + } + } + + return decisions, nil +} + +func createAdmissionRequest(attr admission.Attributes) *admissionv1.AdmissionRequest { + // FIXME: how to get resource GVK, GVR and subresource? + gvk := attr.GetKind() + gvr := attr.GetResource() + subresource := attr.GetSubresource() + + requestGVK := attr.GetKind() + requestGVR := attr.GetResource() + requestSubResource := attr.GetSubresource() + + aUserInfo := attr.GetUserInfo() + var userInfo authenticationv1.UserInfo + if aUserInfo != nil { + userInfo = authenticationv1.UserInfo{ + Extra: make(map[string]authenticationv1.ExtraValue), + Groups: aUserInfo.GetGroups(), + UID: aUserInfo.GetUID(), + Username: aUserInfo.GetName(), + } + // Convert the extra information in the user object + for key, val := range aUserInfo.GetExtra() { + userInfo.Extra[key] = authenticationv1.ExtraValue(val) + } + } + + dryRun := attr.IsDryRun() + + return &admissionv1.AdmissionRequest{ + Kind: metav1.GroupVersionKind{ + Group: gvk.Group, + Kind: gvk.Kind, + Version: gvk.Version, + }, + Resource: metav1.GroupVersionResource{ + Group: gvr.Group, + Resource: gvr.Resource, + Version: gvr.Version, + }, + SubResource: subresource, + RequestKind: &metav1.GroupVersionKind{ + Group: requestGVK.Group, + Kind: requestGVK.Kind, + Version: requestGVK.Version, + }, + RequestResource: &metav1.GroupVersionResource{ + Group: requestGVR.Group, + Resource: requestGVR.Resource, + Version: requestGVR.Version, + }, + RequestSubResource: requestSubResource, + Name: attr.GetName(), + Namespace: attr.GetNamespace(), + Operation: admissionv1.Operation(attr.GetOperation()), + UserInfo: userInfo, + // Leave Object and OldObject unset since we don't provide access to them via request + DryRun: &dryRun, + Options: runtime.RawExtension{ + Object: attr.GetOperationOptions(), + }, + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/validator_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/validator_test.go new file mode 100644 index 00000000000..4d669173526 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/validator_test.go @@ -0,0 +1,572 @@ +/* +Copyright 2022 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 ( + "strings" + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/stretchr/testify/require" + + v1 "k8s.io/api/admissionregistration/v1" + "k8s.io/api/admissionregistration/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" +) + +func TestCompile(t *testing.T) { + cases := []struct { + name string + policy *v1alpha1.ValidatingAdmissionPolicy + errorExpressions map[string]string + }{ + { + name: "invalid syntax", + policy: &v1alpha1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: v1alpha1.ValidatingAdmissionPolicySpec{ + FailurePolicy: func() *v1alpha1.FailurePolicyType { + r := v1alpha1.FailurePolicyType("Fail") + return &r + }(), + ParamKind: &v1alpha1.ParamKind{ + APIVersion: "rules.example.com/v1", + Kind: "ReplicaLimit", + }, + Validations: []v1alpha1.Validation{ + { + Expression: "1 < 'asdf'", + }, + { + Expression: "1 < 2", + }, + }, + MatchConstraints: &v1alpha1.MatchResources{ + MatchPolicy: func() *v1alpha1.MatchPolicyType { + r := v1alpha1.MatchPolicyType("Exact") + return &r + }(), + ResourceRules: []v1alpha1.NamedRuleWithOperations{ + { + RuleWithOperations: v1alpha1.RuleWithOperations{ + Operations: []v1.OperationType{"CREATE"}, + Rule: v1.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a"}, + }, + }, + }, + }, + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + }, + }, + }, + errorExpressions: map[string]string{ + "1 < 'asdf'": "found no matching overload for '_<_' applied to '(int, string)", + }, + }, + { + name: "valid syntax", + policy: &v1alpha1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: v1alpha1.ValidatingAdmissionPolicySpec{ + FailurePolicy: func() *v1alpha1.FailurePolicyType { + r := v1alpha1.FailurePolicyType("Fail") + return &r + }(), + Validations: []v1alpha1.Validation{ + { + Expression: "1 < 2", + }, + { + Expression: "object.spec.string.matches('[0-9]+')", + }, + { + Expression: "request.kind.group == 'example.com' && request.kind.version == 'v1' && request.kind.kind == 'Fake'", + }, + }, + MatchConstraints: &v1alpha1.MatchResources{ + MatchPolicy: func() *v1alpha1.MatchPolicyType { + r := v1alpha1.MatchPolicyType("Exact") + return &r + }(), + ResourceRules: []v1alpha1.NamedRuleWithOperations{ + { + RuleWithOperations: v1alpha1.RuleWithOperations{ + Operations: []v1.OperationType{"CREATE"}, + Rule: v1.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a"}, + }, + }, + }, + }, + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + }, + }, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var c CELValidatorCompiler + validator := c.Compile(tc.policy) + if validator == nil { + t.Fatalf("unexpected nil validator") + } + validations := tc.policy.Spec.Validations + CompilationResults := validator.(*CELValidator).compilationResults + require.Equal(t, len(validations), len(CompilationResults)) + + meets := make([]bool, len(validations)) + for expr, expectErr := range tc.errorExpressions { + for i, result := range CompilationResults { + if validations[i].Expression == expr { + if result.Error == nil { + t.Errorf("Expect expression '%s' to contain error '%v' but got no error", expr, expectErr) + } else if !strings.Contains(result.Error.Error(), expectErr) { + t.Errorf("Expected validation '%s' error to contain '%v' but got: %v", expr, expectErr, result.Error) + } + meets[i] = true + } + } + } + for i, meet := range meets { + if !meet && CompilationResults[i].Error != nil { + t.Errorf("Unexpected err '%v' for expression '%s'", CompilationResults[i].Error, validations[i].Expression) + } + } + }) + } +} + +func getValidPolicy(validations []v1alpha1.Validation, params *v1alpha1.ParamKind, fp *v1alpha1.FailurePolicyType) *v1alpha1.ValidatingAdmissionPolicy { + if fp == nil { + fp = func() *v1alpha1.FailurePolicyType { + r := v1alpha1.FailurePolicyType("Fail") + return &r + }() + } + return &v1alpha1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: v1alpha1.ValidatingAdmissionPolicySpec{ + FailurePolicy: fp, + Validations: validations, + ParamKind: params, + MatchConstraints: &v1alpha1.MatchResources{ + MatchPolicy: func() *v1alpha1.MatchPolicyType { + r := v1alpha1.MatchPolicyType("Exact") + return &r + }(), + ResourceRules: []v1alpha1.NamedRuleWithOperations{ + { + RuleWithOperations: v1alpha1.RuleWithOperations{ + Operations: []v1.OperationType{"CREATE"}, + Rule: v1.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a"}, + }, + }, + }, + }, + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + }, + }, + } +} + +func generatedDecision(k policyDecisionKind, m string, r metav1.StatusReason) policyDecision { + return policyDecision{kind: k, message: m, reason: r} +} + +func TestValidate(t *testing.T) { + // we fake the paramKind in ValidatingAdmissionPolicy for testing since the params is directly passed from cel admission + // Inside validator.go, we only check if paramKind exists + hasParamKind := &v1alpha1.ParamKind{ + APIVersion: "v1", + Kind: "ConfigMap", + } + ignorePolicy := func() *v1alpha1.FailurePolicyType { + r := v1alpha1.FailurePolicyType("Ignore") + return &r + }() + forbiddenReason := func() *metav1.StatusReason { + r := metav1.StatusReasonForbidden + return &r + }() + + configMapParams := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Data: map[string]string{ + "fakeString": "fake", + }, + } + crdParams := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "testSize": 10, + }, + }, + } + podObject := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: corev1.PodSpec{ + NodeName: "testnode", + }, + } + + cases := []struct { + name string + policy *v1alpha1.ValidatingAdmissionPolicy + attributes admission.Attributes + params runtime.Object + policyDecisions []policyDecision + }{ + { + name: "valid syntax for object", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "has(object.subsets) && object.subsets.size() < 2", + }, + }, nil, nil), + attributes: newValidAttribute(nil, false), + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + }, + }, + { + name: "valid syntax for metadata", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "object.metadata.name == 'endpoints1'", + }, + }, nil, nil), + attributes: newValidAttribute(nil, false), + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + }, + }, + { + name: "valid syntax for oldObject", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "oldObject == null", + }, + { + Expression: "object != null", + }, + }, nil, nil), + attributes: newValidAttribute(nil, false), + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + generatedDecision(admit, "", ""), + }, + }, + { + name: "valid syntax for request", + policy: getValidPolicy([]v1alpha1.Validation{ + {Expression: "request.operation == 'CREATE'"}, + }, nil, nil), + attributes: newValidAttribute(nil, false), + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + }, + }, + { + name: "valid syntax for configMap", + policy: getValidPolicy([]v1alpha1.Validation{ + {Expression: "request.namespace != params.data.fakeString"}, + }, hasParamKind, nil), + attributes: newValidAttribute(nil, false), + params: configMapParams, + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + }, + }, + { + name: "test failure policy with Ignore", + policy: getValidPolicy([]v1alpha1.Validation{ + {Expression: "object.subsets.size() > 2"}, + }, hasParamKind, ignorePolicy), + attributes: newValidAttribute(nil, false), + params: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Data: map[string]string{ + "fakeString": "fake", + }, + }, + policyDecisions: []policyDecision{ + generatedDecision(deny, "failed expression: object.subsets.size() > 2", metav1.StatusReasonInvalid), + }, + }, + { + name: "test failure policy with multiple validations", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "has(object.subsets)", + }, + { + Expression: "object.subsets.size() > 2", + }, + }, hasParamKind, ignorePolicy), + attributes: newValidAttribute(nil, false), + params: configMapParams, + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + generatedDecision(deny, "failed expression: object.subsets.size() > 2", metav1.StatusReasonInvalid), + }, + }, + { + name: "test failure policy with multiple failed validations", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "oldObject != null", + }, + { + Expression: "object.subsets.size() > 2", + }, + }, hasParamKind, nil), + attributes: newValidAttribute(nil, false), + params: configMapParams, + policyDecisions: []policyDecision{ + generatedDecision(deny, "failed expression: oldObject != null", metav1.StatusReasonInvalid), + generatedDecision(deny, "failed expression: object.subsets.size() > 2", metav1.StatusReasonInvalid), + }, + }, + { + name: "test Object nul in delete", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "oldObject != null", + }, + { + Expression: "object == null", + }, + }, hasParamKind, nil), + attributes: newValidAttribute(nil, true), + params: configMapParams, + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + generatedDecision(admit, "", ""), + }, + }, + { + name: "test reason for failed validation", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "oldObject == null", + Reason: forbiddenReason, + }, + }, hasParamKind, nil), + attributes: newValidAttribute(nil, true), + params: configMapParams, + policyDecisions: []policyDecision{ + generatedDecision(deny, "failed expression: oldObject == null", metav1.StatusReasonForbidden), + }, + }, + { + name: "test message for failed validation", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "oldObject == null", + Reason: forbiddenReason, + Message: "old object should be present", + }, + }, hasParamKind, nil), + attributes: newValidAttribute(nil, true), + params: configMapParams, + policyDecisions: []policyDecision{ + generatedDecision(deny, "old object should be present", metav1.StatusReasonForbidden), + }, + }, + { + name: "test runtime error", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "oldObject.x == 100", + }, + }, hasParamKind, nil), + attributes: newValidAttribute(nil, true), + params: configMapParams, + policyDecisions: []policyDecision{ + generatedDecision(deny, "resulted in error", ""), + }, + }, + { + name: "test against crd param", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "object.subsets.size() < params.spec.testSize", + }, + }, hasParamKind, nil), + attributes: newValidAttribute(nil, false), + params: crdParams, + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + }, + }, + { + name: "test compile failure with FailurePolicy Fail", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "fail to compile test", + }, + { + Expression: "object.subsets.size() > params.spec.testSize", + }, + }, hasParamKind, nil), + attributes: newValidAttribute(nil, false), + params: crdParams, + policyDecisions: []policyDecision{ + generatedDecision(deny, "compilation error: compilation failed: ERROR: :1:6: Syntax error:", ""), + generatedDecision(deny, "failed expression: object.subsets.size() > params.spec.testSize", metav1.StatusReasonInvalid), + }, + }, + { + name: "test compile failure with FailurePolicy Ignore", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "fail to compile test", + }, + { + Expression: "object.subsets.size() > params.spec.testSize", + }, + }, hasParamKind, ignorePolicy), + attributes: newValidAttribute(nil, false), + params: crdParams, + policyDecisions: []policyDecision{ + generatedDecision(admit, "compilation error: compilation failed: ERROR:", ""), + generatedDecision(deny, "failed expression: object.subsets.size() > params.spec.testSize", metav1.StatusReasonInvalid), + }, + }, + { + name: "test pod", + policy: getValidPolicy([]v1alpha1.Validation{ + { + Expression: "object.spec.nodeName == 'testnode'", + }, + }, nil, nil), + attributes: newValidAttribute(&podObject, false), + params: crdParams, + policyDecisions: []policyDecision{ + generatedDecision(admit, "", ""), + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := CELValidatorCompiler{} + validator := c.Compile(tc.policy) + if validator == nil { + t.Fatalf("unexpected nil validator") + } + validations := tc.policy.Spec.Validations + CompilationResults := validator.(*CELValidator).compilationResults + require.Equal(t, len(validations), len(CompilationResults)) + + policyResults, err := validator.Validate(tc.attributes, newObjectInterfacesForTest(), tc.params, tc.attributes.GetKind()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + require.Equal(t, len(policyResults), len(tc.policyDecisions)) + for i, policyDecision := range tc.policyDecisions { + if policyDecision.kind != policyResults[i].kind { + t.Errorf("Expected policy decision kind '%v' but got '%v'", policyDecision.kind, policyResults[i].kind) + } + if !strings.Contains(policyResults[i].message, policyDecision.message) { + t.Errorf("Expected policy decision message contains '%v' but got '%v'", policyDecision.message, policyResults[i].message) + } + if policyDecision.reason != policyResults[i].reason { + t.Errorf("Expected policy decision reason '%v' but got '%v'", policyDecision.reason, policyResults[i].reason) + } + } + }) + } +} + +// newObjectInterfacesForTest returns an ObjectInterfaces appropriate for test cases in this file. +func newObjectInterfacesForTest() admission.ObjectInterfaces { + scheme := runtime.NewScheme() + corev1.AddToScheme(scheme) + return admission.NewObjectInterfacesFromScheme(scheme) +} + +func newValidAttribute(object runtime.Object, isDelete bool) admission.Attributes { + var oldObject runtime.Object + if !isDelete { + if object == nil { + object = &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "endpoints1", + }, + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{{IP: "127.0.0.0"}}, + }, + }, + } + } + } else { + object = nil + oldObject = &corev1.Endpoints{ + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{{IP: "127.0.0.0"}}, + }, + }, + } + } + return admission.NewAttributesRecord(object, oldObject, schema.GroupVersionKind{}, "default", "foo", schema.GroupVersionResource{}, "", admission.Create, &metav1.CreateOptions{}, false, nil) + +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go index e3297aed8f5..52df53af82b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go @@ -219,7 +219,7 @@ func (a *attrWithResourceOverride) GetResource() schema.GroupVersionResource { r // Dispatch is called by the downstream Validate or Admit methods. func (a *Webhook) Dispatch(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces) error { - if rules.IsWebhookConfigurationResource(attr) { + if rules.IsExemptAdmissionConfigurationResource(attr) { return nil } if !a.WaitForReady() { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules/rules.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules/rules.go index 924e79bcc9f..b926f65dc10 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules/rules.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules/rules.go @@ -116,12 +116,12 @@ func (r *Matcher) resource() bool { return false } -// IsWebhookConfigurationResource determines if an admission.Attributes object is describing -// the admission of a ValidatingWebhookConfiguration or a MutatingWebhookConfiguration -func IsWebhookConfigurationResource(attr admission.Attributes) bool { +// IsExemptAdmissionConfigurationResource determines if an admission.Attributes object is describing +// the admission of a ValidatingWebhookConfiguration or a MutatingWebhookConfiguration or a ValidatingAdmissionPolicy or a ValidatingAdmissionPolicyBinding +func IsExemptAdmissionConfigurationResource(attr admission.Attributes) bool { gvk := attr.GetKind() if gvk.Group == "admissionregistration.k8s.io" { - if gvk.Kind == "ValidatingWebhookConfiguration" || gvk.Kind == "MutatingWebhookConfiguration" { + if gvk.Kind == "ValidatingWebhookConfiguration" || gvk.Kind == "MutatingWebhookConfiguration" || gvk.Kind == "ValidatingAdmissionPolicy" || gvk.Kind == "ValidatingAdmissionPolicyBinding" { return true } } diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/admission.go b/staging/src/k8s.io/apiserver/pkg/server/options/admission.go index 2061b886b25..1159e297d65 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/admission.go @@ -28,6 +28,7 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/initializer" admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" + "k8s.io/apiserver/pkg/admission/plugin/cel" "k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle" mutatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating" validatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/validating" @@ -86,7 +87,7 @@ func NewAdmissionOptions() *AdmissionOptions { // admission plugins. The apiserver always runs the validating ones // after all the mutating ones, so their relative order in this list // doesn't matter. - RecommendedPluginOrder: []string{lifecycle.PluginName, mutatingwebhook.PluginName, validatingwebhook.PluginName}, + RecommendedPluginOrder: []string{lifecycle.PluginName, mutatingwebhook.PluginName, cel.PluginName, validatingwebhook.PluginName}, DefaultOffPlugins: sets.NewString(), } server.RegisterAllAdmissionPlugins(options.Plugins) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/admission_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/admission_test.go index 13a8176a15b..da600a5a3e4 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/admission_test.go @@ -36,7 +36,7 @@ func TestEnabledPluginNames(t *testing.T) { }{ // scenario 0: check if a call to enabledPluginNames sets expected values. { - expectedPluginNames: []string{"NamespaceLifecycle", "MutatingAdmissionWebhook", "ValidatingAdmissionWebhook"}, + expectedPluginNames: []string{"NamespaceLifecycle", "MutatingAdmissionWebhook", "ValidatingAdmissionPolicy", "ValidatingAdmissionWebhook"}, }, // scenario 1: use default off plugins if no specified diff --git a/staging/src/k8s.io/apiserver/pkg/server/plugins.go b/staging/src/k8s.io/apiserver/pkg/server/plugins.go index 7a9094337db..adb83a908ef 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/plugins.go +++ b/staging/src/k8s.io/apiserver/pkg/server/plugins.go @@ -19,6 +19,7 @@ package server // This file exists to force the desired plugin implementations to be linked into genericapi pkg. import ( "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/admission/plugin/cel" "k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle" mutatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating" validatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/validating" @@ -29,4 +30,5 @@ func RegisterAllAdmissionPlugins(plugins *admission.Plugins) { lifecycle.Register(plugins) validatingwebhook.Register(plugins) mutatingwebhook.Register(plugins) + cel.Register(plugins) } diff --git a/test/integration/apiserver/admissionwebhook/admission_test.go b/test/integration/apiserver/admissionwebhook/admission_test.go index 68440bc4d45..1d36e5260e2 100644 --- a/test/integration/apiserver/admissionwebhook/admission_test.go +++ b/test/integration/apiserver/admissionwebhook/admission_test.go @@ -138,10 +138,12 @@ var ( // admissionExemptResources lists objects which are exempt from admission validation/mutation, // only resources exempted from admission processing by API server should be listed here. admissionExemptResources = map[schema.GroupVersionResource]bool{ - gvr("admissionregistration.k8s.io", "v1beta1", "mutatingwebhookconfigurations"): true, - gvr("admissionregistration.k8s.io", "v1beta1", "validatingwebhookconfigurations"): true, - gvr("admissionregistration.k8s.io", "v1", "mutatingwebhookconfigurations"): true, - gvr("admissionregistration.k8s.io", "v1", "validatingwebhookconfigurations"): true, + gvr("admissionregistration.k8s.io", "v1beta1", "mutatingwebhookconfigurations"): true, + gvr("admissionregistration.k8s.io", "v1beta1", "validatingwebhookconfigurations"): true, + gvr("admissionregistration.k8s.io", "v1", "mutatingwebhookconfigurations"): true, + gvr("admissionregistration.k8s.io", "v1", "validatingwebhookconfigurations"): true, + gvr("admissionregistration.k8s.io", "v1alpha1", "validatingadmissionpolicies"): true, + gvr("admissionregistration.k8s.io", "v1alpha1", "validatingadmissionpolicybindings"): true, } parentResources = map[schema.GroupVersionResource]schema.GroupVersionResource{