diff --git a/pkg/apis/admissionregistration/fuzzer/fuzzer.go b/pkg/apis/admissionregistration/fuzzer/fuzzer.go index 111e3494039..ca675e9b19b 100644 --- a/pkg/apis/admissionregistration/fuzzer/fuzzer.go +++ b/pkg/apis/admissionregistration/fuzzer/fuzzer.go @@ -98,5 +98,14 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { obj.MatchPolicy = &m } }, + func(obj *admissionregistration.ParamRef, c fuzz.Continue) { + c.FuzzNoCustom(obj) // fuzz self without calling this function again + + // Populate required field + if obj.ParameterNotFoundAction == nil { + v := admissionregistration.DenyAction + obj.ParameterNotFoundAction = &v + } + }, } } diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index 2f250653980..578e8d52fe9 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/pkg/apis/admissionregistration/types.go @@ -76,6 +76,18 @@ const ( AllScopes ScopeType = "*" ) +// ParameterNotFoundActionType specifies a failure policy that defines how a binding +// is evaluated when the param referred by its perNamespaceParamRef is not found. +type ParameterNotFoundActionType string + +const ( + // Allow means all requests will be admitted if no param resources + // could be found. + AllowAction ParameterNotFoundActionType = "Allow" + // Deny means all requests will be denied if no param resources are found. + DenyAction ParameterNotFoundActionType = "Deny" +) + // FailurePolicyType specifies the type of failure policy type FailurePolicyType string @@ -401,6 +413,15 @@ type AuditAnnotation struct { // ValidatingAdmissionPolicyBinding binds the ValidatingAdmissionPolicy with paramerized resources. // ValidatingAdmissionPolicyBinding and parameter CRDs together define how cluster administrators configure policies for clusters. +// +// For a given admission request, each binding will cause its policy to be +// evaluated N times, where N is 1 for policies/bindings that don't use +// params, otherwise N is the number of parameters selected by the binding. +// +// The CEL expressions of a policy must have a computed CEL cost below the maximum +// CEL budget. Each evaluation of the policy is given an independent CEL cost budget. +// Adding/removing policies, bindings, or params can not affect whether a +// given (policy, binding, param) combination is within its own CEL budget. type ValidatingAdmissionPolicyBinding struct { metav1.TypeMeta // Standard object metadata; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata. @@ -430,9 +451,10 @@ type ValidatingAdmissionPolicyBindingSpec struct { // Required. PolicyName string - // ParamRef specifies the parameter resource used to configure the admission control policy. + // paramRef specifies the parameter resource used to configure the admission control policy. // It should point to a resource of the type specified in ParamKind of the bound ValidatingAdmissionPolicy. // If the policy specifies a ParamKind and the resource referred to by ParamRef does not exist, this binding is considered mis-configured and the FailurePolicy of the ValidatingAdmissionPolicy applied. + // If the policy does not specify a ParamKind then this field is ignored, and the rules are evaluated without a param. // +optional ParamRef *ParamRef @@ -486,14 +508,63 @@ type ValidatingAdmissionPolicyBindingSpec struct { ValidationActions []ValidationAction } -// ParamRef references a parameter resource +// ParamRef describes how to locate the params to be used as input to +// expressions of rules applied by a policy binding. +// +structType=atomic type ParamRef struct { - // Name of the resource being referenced. + // name is the name of the resource being referenced. + // + // One of `name` or `selector` must be set, but `name` and `selector` are + // mutually exclusive properties. If one is set, the other must be unset. + // + // A single parameter used for all admission requests can be configured + // by setting the `name` field, leaving `selector` blank, and setting namespace + // if `paramKind` is namespace-scoped. + // + // +optional Name string - // Namespace of the referenced resource. - // Should be empty for the cluster-scoped resources + + // namespace is the namespace of the referenced resource. Allows limiting + // the search for params to a specific namespace. Applies to both `name` and + // `selector` fields. + // + // A per-namespace parameter may be used by specifying a namespace-scoped + // `paramKind` in the policy and leaving this field empty. + // + // - If `paramKind` is cluster-scoped, this field MUST be unset. Setting this + // field results in a configuration error. + // + // - If `paramKind` is namespace-scoped, the namespace of the object being + // evaluated for admission will be used when this field is left unset. Take + // care that if this is left empty the binding must not match any cluster-scoped + // resources, which will result in an error. + // // +optional Namespace string + + // selector can be used to match multiple param objects based on their labels. + // Supply selector: {} to match all resources of the ParamKind. + // + // If multiple params are found, they are all evaluated with the policy expressions + // and the results are ANDed together. + // + // One of `name` or `selector` must be set, but `name` and `selector` are + // mutually exclusive properties. If one is set, the other must be unset. + // + // +optional + Selector *metav1.LabelSelector + + // parameterNotFoundAction controls the behavior of the binding when the resource + // exists, and name or selector is valid, but there are no parameters + // matched by the binding. If the value is set to `Allow`, then no + // matched parameters will be treated as successful validation by the binding. + // If set to `Deny`, then no matched parameters will be subject to the + // `failurePolicy` of the policy. + // + // Allowed values are `Allow` or `Deny` + // + // Required + ParameterNotFoundAction *ParameterNotFoundActionType } // MatchResources decides whether to run the admission control policy on an object based diff --git a/pkg/apis/admissionregistration/v1alpha1/defaults.go b/pkg/apis/admissionregistration/v1alpha1/defaults.go index 85d12b128f7..1abb61b2c0c 100644 --- a/pkg/apis/admissionregistration/v1alpha1/defaults.go +++ b/pkg/apis/admissionregistration/v1alpha1/defaults.go @@ -49,3 +49,11 @@ func SetDefaults_MatchResources(obj *admissionregistrationv1alpha1.MatchResource obj.ObjectSelector = &selector } } + +// SetDefaults_ParamRef sets defaults for ParamRef +func SetDefaults_ParamRef(obj *admissionregistrationv1alpha1.ParamRef) { + if obj.ParameterNotFoundAction == nil { + v := admissionregistrationv1alpha1.DenyAction + obj.ParameterNotFoundAction = &v + } +} diff --git a/pkg/apis/admissionregistration/v1alpha1/defaults_test.go b/pkg/apis/admissionregistration/v1alpha1/defaults_test.go index 9315118dd8b..3c09d708d5f 100644 --- a/pkg/apis/admissionregistration/v1alpha1/defaults_test.go +++ b/pkg/apis/admissionregistration/v1alpha1/defaults_test.go @@ -116,3 +116,58 @@ func TestDefaultAdmissionPolicy(t *testing.T) { }) } } + +func TestDefaultAdmissionPolicyBinding(t *testing.T) { + denyAction := v1alpha1.DenyAction + equivalent := v1alpha1.Equivalent + + tests := []struct { + name string + original runtime.Object + expected runtime.Object + }{ + { + name: "ValidatingAdmissionPolicyBinding.ParamRef.ParameterNotFoundAction", + original: &v1alpha1.ValidatingAdmissionPolicyBinding{ + Spec: v1alpha1.ValidatingAdmissionPolicyBindingSpec{ + ParamRef: &v1alpha1.ParamRef{}, + }, + }, + expected: &v1alpha1.ValidatingAdmissionPolicyBinding{ + Spec: v1alpha1.ValidatingAdmissionPolicyBindingSpec{ + ParamRef: &v1alpha1.ParamRef{ + ParameterNotFoundAction: &denyAction, + }, + }, + }, + }, + { + name: "ValidatingAdmissionPolicyBinding.MatchResources", + original: &v1alpha1.ValidatingAdmissionPolicyBinding{ + Spec: v1alpha1.ValidatingAdmissionPolicyBindingSpec{ + MatchResources: &v1alpha1.MatchResources{}, + }, + }, + expected: &v1alpha1.ValidatingAdmissionPolicyBinding{ + Spec: v1alpha1.ValidatingAdmissionPolicyBindingSpec{ + MatchResources: &v1alpha1.MatchResources{ + NamespaceSelector: &metav1.LabelSelector{}, + ObjectSelector: &metav1.LabelSelector{}, + MatchPolicy: &equivalent, + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + original := test.original + expected := test.expected + legacyscheme.Scheme.Default(original) + if !apiequality.Semantic.DeepEqual(original, expected) { + t.Error(cmp.Diff(expected, original)) + } + }) + } +} diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index 1f54ab725f2..4ea4ffe78ce 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -1151,9 +1151,38 @@ func validateParamRef(pr *admissionregistration.ParamRef, fldPath *field.Path) f if pr == nil { return allErrors } - for _, msg := range path.ValidatePathSegmentName(pr.Name, false) { - allErrors = append(allErrors, field.Invalid(fldPath.Child("name"), pr.Name, msg)) + + if len(pr.Name) > 0 { + for _, msg := range path.ValidatePathSegmentName(pr.Name, false) { + allErrors = append(allErrors, field.Invalid(fldPath.Child("name"), pr.Name, msg)) + } + + if pr.Selector != nil { + allErrors = append(allErrors, field.Forbidden(fldPath.Child("name"), `name and selector are mutually exclusive`)) + } } + + if pr.Selector != nil { + labelSelectorValidationOpts := metav1validation.LabelSelectorValidationOptions{} + allErrors = append(allErrors, metav1validation.ValidateLabelSelector(pr.Selector, labelSelectorValidationOpts, fldPath.Child("selector"))...) + + if len(pr.Name) > 0 { + allErrors = append(allErrors, field.Forbidden(fldPath.Child("selector"), `name and selector are mutually exclusive`)) + } + } + + if len(pr.Name) == 0 && pr.Selector == nil { + allErrors = append(allErrors, field.Required(fldPath, `one of name or selector must be specified`)) + } + + if pr.ParameterNotFoundAction == nil || len(*pr.ParameterNotFoundAction) == 0 { + allErrors = append(allErrors, field.Required(fldPath.Child("parameterNotFoundAction"), "")) + } else { + if *pr.ParameterNotFoundAction != admissionregistration.DenyAction && *pr.ParameterNotFoundAction != admissionregistration.AllowAction { + allErrors = append(allErrors, field.NotSupported(fldPath.Child("parameterNotFoundAction"), pr.ParameterNotFoundAction, []string{string(admissionregistration.DenyAction), string(admissionregistration.AllowAction)})) + } + } + return allErrors } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index 2c7a13d770f..8b2d50daa95 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -32,6 +32,8 @@ import ( "k8s.io/kubernetes/pkg/apis/admissionregistration" ) +func ptr[T any](v T) *T { return &v } + func strPtr(s string) *string { return &s } func int32Ptr(i int32) *int32 { return &i } @@ -3517,7 +3519,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, MatchResources: &admissionregistration.MatchResources{ MatchPolicy: func() *admissionregistration.MatchPolicyType { @@ -3537,7 +3540,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, MatchResources: &admissionregistration.MatchResources{ ResourceRules: []admissionregistration.NamedRuleWithOperations{{ @@ -3591,7 +3595,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, MatchResources: &admissionregistration.MatchResources{ ResourceRules: []admissionregistration.NamedRuleWithOperations{{ RuleWithOperations: admissionregistration.RuleWithOperations{ @@ -3616,7 +3621,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, MatchResources: &admissionregistration.MatchResources{ ResourceRules: []admissionregistration.NamedRuleWithOperations{{ RuleWithOperations: admissionregistration.RuleWithOperations{ @@ -3641,7 +3647,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3668,7 +3675,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3704,7 +3712,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3731,7 +3740,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3758,7 +3768,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3794,7 +3805,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3821,7 +3833,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3848,7 +3861,8 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny, admissionregistration.Deny}, }, @@ -3863,12 +3877,80 @@ func TestValidateValidatingAdmissionPolicyBinding(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.ValidationAction("illegal")}, }, }, expectedError: `Unsupported value: "illegal": supported values: "Audit", "Deny", "Warn"`, + }, { + name: "paramRef selector must not be set when name is set", + config: &admissionregistration.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "xyzlimit-scale.example.com", + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + ParamRef: &admissionregistration.ParamRef{ + Name: "xyzlimit-scale-setting.example.com", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "label": "value", + }, + }, + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), + }, + }, + }, + expectedError: `spec.paramRef.name: Forbidden: name and selector are mutually exclusive, spec.paramRef.selector: Forbidden: name and selector are mutually exclusive`, + }, { + name: "paramRef parameterNotFoundAction must be set", + config: &admissionregistration.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "xyzlimit-scale.example.com", + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + ParamRef: &admissionregistration.ParamRef{ + Name: "xyzlimit-scale-setting.example.com", + }, + }, + }, + expectedError: "spec.paramRef.parameterNotFoundAction: Required value", + }, { + name: "paramRef parameterNotFoundAction must be an valid value", + config: &admissionregistration.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "xyzlimit-scale.example.com", + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + ParamRef: &admissionregistration.ParamRef{ + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.ParameterNotFoundActionType("invalid")), + }, + }, + }, + expectedError: `spec.paramRef.parameterNotFoundAction: Unsupported value: "invalid": supported values: "Deny", "Allow"`, + }, { + name: "paramRef one of name or selector", + config: &admissionregistration.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "xyzlimit-scale.example.com", + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + ParamRef: &admissionregistration.ParamRef{ + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), + }, + }, + }, + expectedError: `one of name or selector must be specified`, }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -3903,7 +3985,8 @@ func TestValidateValidatingAdmissionPolicyBindingUpdate(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3937,7 +4020,8 @@ func TestValidateValidatingAdmissionPolicyBindingUpdate(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ @@ -3973,7 +4057,8 @@ func TestValidateValidatingAdmissionPolicyBindingUpdate(t *testing.T) { Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "xyzlimit-scale.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "xyzlimit-scale-setting.example.com", + Name: "xyzlimit-scale-setting.example.com", + ParameterNotFoundAction: ptr(admissionregistration.DenyAction), }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{ diff --git a/pkg/printers/internalversion/printers.go b/pkg/printers/internalversion/printers.go index 3d309dce3bb..138e38e025c 100644 --- a/pkg/printers/internalversion/printers.go +++ b/pkg/printers/internalversion/printers.go @@ -1673,13 +1673,18 @@ func printValidatingAdmissionPolicyBinding(obj *admissionregistration.Validating Object: runtime.RawExtension{Object: obj}, } paramName := "" - if obj.Spec.ParamRef != nil { - if obj.Spec.ParamRef.Namespace != "" { - paramName = obj.Spec.ParamRef.Namespace + "/" + obj.Spec.ParamRef.Name - } else { - paramName = obj.Spec.ParamRef.Name + if pr := obj.Spec.ParamRef; pr != nil { + if len(pr.Name) > 0 { + if pr.Namespace != "" { + paramName = pr.Namespace + "/" + pr.Name + } else { + // Can't tell from here if param is cluster-scoped, so all + // params without names get * namespace + paramName = "*/" + pr.Name + } + } else if pr.Selector != nil { + paramName = pr.Selector.String() } - } row.Cells = append(row.Cells, obj.Name, obj.Spec.PolicyName, paramName, translateTimestampSince(obj.CreationTimestamp)) return []metav1.TableRow{row}, nil diff --git a/pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz.go b/pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz.go index a68e6c727e1..49898e694bd 100644 --- a/pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz.go +++ b/pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz.go @@ -85,15 +85,21 @@ func (v *validatingAdmissionPolicyBindingStrategy) authorize(ctx context.Context } } - paramRef := binding.Spec.ParamRef + var attrs authorizer.AttributesRecord - // require that the user can read (verb "get") the referred resource. - attrs := authorizer.AttributesRecord{ + paramRef := binding.Spec.ParamRef + verb := "get" + + if len(paramRef.Name) == 0 { + verb = "list" + } + + attrs = authorizer.AttributesRecord{ User: user, - Verb: "get", + Verb: verb, ResourceRequest: true, Name: paramRef.Name, - Namespace: paramRef.Namespace, + Namespace: paramRef.Namespace, // if empty, no namespace indicates get across all namespaces APIGroup: apiGroup, APIVersion: apiVersion, Resource: resource, @@ -104,7 +110,8 @@ func (v *validatingAdmissionPolicyBindingStrategy) authorize(ctx context.Context return err } if d != authorizer.DecisionAllow { - return fmt.Errorf(`user %v does not have "get" permission on the object referenced by paramRef`, user) + return fmt.Errorf(`user %v does not have "%v" permission on the object referenced by paramRef`, verb, user) } + return nil } diff --git a/pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz_test.go b/pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz_test.go index f42fcd380d2..c72e9f92d2a 100644 --- a/pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz_test.go +++ b/pkg/registry/admissionregistration/validatingadmissionpolicybinding/authz_test.go @@ -103,19 +103,46 @@ func TestAuthorization(t *testing.T) { strategy := NewStrategy(tc.auth, tc.policyGetter, tc.resourceResolver) t.Run("create", func(t *testing.T) { ctx := request.WithUser(context.Background(), tc.userInfo) - errs := strategy.Validate(ctx, validPolicyBinding()) - if len(errs) > 0 != tc.expectErr { - t.Errorf("expected error: %v but got error: %v", tc.expectErr, errs) + for _, obj := range validPolicyBindings() { + errs := strategy.Validate(ctx, obj) + if len(errs) > 0 != tc.expectErr { + t.Errorf("expected error: %v but got error: %v", tc.expectErr, errs) + } } }) t.Run("update", func(t *testing.T) { ctx := request.WithUser(context.Background(), tc.userInfo) - obj := validPolicyBinding() - objWithChangedParamRef := obj.DeepCopy() - objWithChangedParamRef.Spec.ParamRef.Name = "changed" - errs := strategy.ValidateUpdate(ctx, obj, objWithChangedParamRef) - if len(errs) > 0 != tc.expectErr { - t.Errorf("expected error: %v but got error: %v", tc.expectErr, errs) + for _, obj := range validPolicyBindings() { + objWithChangedParamRef := obj.DeepCopy() + if pr := objWithChangedParamRef.Spec.ParamRef; pr != nil { + if len(pr.Name) > 0 { + pr.Name = "changed" + } + + if pr.Selector != nil { + pr.Selector = &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "changed": "value", + }, + } + } + + if len(pr.Namespace) > 0 { + pr.Namespace = "othernamespace" + } + + if pr.ParameterNotFoundAction == nil || *pr.ParameterNotFoundAction == admissionregistration.AllowAction { + v := admissionregistration.DenyAction + pr.ParameterNotFoundAction = &v + } else { + v := admissionregistration.AllowAction + pr.ParameterNotFoundAction = &v + } + } + errs := strategy.ValidateUpdate(ctx, obj, objWithChangedParamRef) + if len(errs) > 0 != tc.expectErr { + t.Errorf("expected error: %v but got error: %v", tc.expectErr, errs) + } } }) }) diff --git a/pkg/registry/admissionregistration/validatingadmissionpolicybinding/storage/storage_test.go b/pkg/registry/admissionregistration/validatingadmissionpolicybinding/storage/storage_test.go index db4af525be0..f177f57ea67 100644 --- a/pkg/registry/admissionregistration/validatingadmissionpolicybinding/storage/storage_test.go +++ b/pkg/registry/admissionregistration/validatingadmissionpolicybinding/storage/storage_test.go @@ -37,121 +37,175 @@ import ( ) func TestCreate(t *testing.T) { - storage, server := newInsecureStorage(t) - defer server.Terminate(t) - defer storage.Store.DestroyFunc() - test := genericregistrytest.New(t, storage.Store).ClusterScope() - configuration := validPolicyBinding() - test.TestCreate( - // valid - configuration, - // invalid - newPolicyBinding(""), - ) + for _, configuration := range validPolicyBindings() { + t.Run(configuration.Name, func(t *testing.T) { + storage, server := newInsecureStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + test := genericregistrytest.New(t, storage.Store).ClusterScope() + + test.TestCreate( + // valid + configuration, + // invalid + newPolicyBinding(""), + ) + }) + } } func TestUpdate(t *testing.T) { - storage, server := newInsecureStorage(t) - defer server.Terminate(t) - defer storage.Store.DestroyFunc() - test := genericregistrytest.New(t, storage.Store).ClusterScope() - - test.TestUpdate( - // valid - validPolicyBinding(), - // updateFunc - func(obj runtime.Object) runtime.Object { - object := obj.(*admissionregistration.ValidatingAdmissionPolicyBinding) - object.Labels = map[string]string{"c": "d"} - return object - }, - // invalid updateFunc - func(obj runtime.Object) runtime.Object { - object := obj.(*admissionregistration.ValidatingAdmissionPolicyBinding) - object.Name = "" - return object - }, - ) + for _, b := range validPolicyBindings() { + storage, server := newInsecureStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + t.Run(b.Name, func(t *testing.T) { + test := genericregistrytest.New(t, storage.Store).ClusterScope() + test.TestUpdate( + // valid + b, + // updateFunc + func(obj runtime.Object) runtime.Object { + object := obj.(*admissionregistration.ValidatingAdmissionPolicyBinding) + object.Labels = map[string]string{"c": "d"} + return object + }, + // invalid updateFunc + func(obj runtime.Object) runtime.Object { + object := obj.(*admissionregistration.ValidatingAdmissionPolicyBinding) + object.Name = "" + return object + }, + ) + }) + } } func TestGet(t *testing.T) { - storage, server := newInsecureStorage(t) - defer server.Terminate(t) - defer storage.Store.DestroyFunc() - test := genericregistrytest.New(t, storage.Store).ClusterScope() - test.TestGet(validPolicyBinding()) + for _, b := range validPolicyBindings() { + t.Run(b.Name, func(t *testing.T) { + storage, server := newInsecureStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + + test := genericregistrytest.New(t, storage.Store).ClusterScope() + test.TestGet(b) + }) + } } func TestList(t *testing.T) { - storage, server := newInsecureStorage(t) - defer server.Terminate(t) - defer storage.Store.DestroyFunc() - test := genericregistrytest.New(t, storage.Store).ClusterScope() - test.TestList(validPolicyBinding()) + for _, b := range validPolicyBindings() { + t.Run(b.Name, func(t *testing.T) { + storage, server := newInsecureStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + test := genericregistrytest.New(t, storage.Store).ClusterScope() + test.TestList(b) + }) + } } func TestDelete(t *testing.T) { - storage, server := newInsecureStorage(t) - defer server.Terminate(t) - defer storage.Store.DestroyFunc() - test := genericregistrytest.New(t, storage.Store).ClusterScope() - test.TestDelete(validPolicyBinding()) + for _, b := range validPolicyBindings() { + t.Run(b.Name, func(t *testing.T) { + storage, server := newInsecureStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + + test := genericregistrytest.New(t, storage.Store).ClusterScope() + test.TestDelete(b) + }) + } } func TestWatch(t *testing.T) { - storage, server := newInsecureStorage(t) - defer server.Terminate(t) - defer storage.Store.DestroyFunc() - test := genericregistrytest.New(t, storage.Store).ClusterScope() - test.TestWatch( - validPolicyBinding(), - []labels.Set{}, - []labels.Set{ - {"hoo": "bar"}, - }, - []fields.Set{ - {"metadata.name": "foo"}, - }, - []fields.Set{ - {"metadata.name": "nomatch"}, - }, - ) + for _, b := range validPolicyBindings() { + t.Run(b.Name, func(t *testing.T) { + storage, server := newInsecureStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + test := genericregistrytest.New(t, storage.Store).ClusterScope() + test.TestWatch( + b, + []labels.Set{}, + []labels.Set{ + {"hoo": "bar"}, + }, + []fields.Set{ + {"metadata.name": b.Name}, + }, + []fields.Set{ + {"metadata.name": "nomatch"}, + }, + ) + }) + } } -func validPolicyBinding() *admissionregistration.ValidatingAdmissionPolicyBinding { - return &admissionregistration.ValidatingAdmissionPolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - }, - Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ - PolicyName: "replicalimit-policy.example.com", - ParamRef: &admissionregistration.ParamRef{ - Name: "param-test", +func validPolicyBindings() []*admissionregistration.ValidatingAdmissionPolicyBinding { + denyAction := admissionregistration.DenyAction + return []*admissionregistration.ValidatingAdmissionPolicyBinding{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", }, - ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, - MatchResources: &admissionregistration.MatchResources{ - MatchPolicy: func() *admissionregistration.MatchPolicyType { - r := admissionregistration.MatchPolicyType("Exact") - return &r - }(), - ObjectSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"a": "b"}, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "replicalimit-policy.example.com", + ParamRef: &admissionregistration.ParamRef{ + Name: "replica-limit-test.example.com", + ParameterNotFoundAction: &denyAction, }, - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"a": "b"}, + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-clusterwide", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "replicalimit-policy.example.com", + ParamRef: &admissionregistration.ParamRef{ + Name: "replica-limit-test.example.com", + Namespace: "default", + ParameterNotFoundAction: &denyAction, }, - ResourceRules: []admissionregistration.NamedRuleWithOperations{ - { - RuleWithOperations: admissionregistration.RuleWithOperations{ - Operations: []admissionregistration.OperationType{"CREATE"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"a"}, - }, + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-selector", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "replicalimit-policy.example.com", + ParamRef: &admissionregistration.ParamRef{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "label": "value", }, }, + ParameterNotFoundAction: &denyAction, }, + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-selector-clusterwide", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "replicalimit-policy.example.com", + ParamRef: &admissionregistration.ParamRef{ + Namespace: "mynamespace", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "label": "value", + }, + }, + ParameterNotFoundAction: &denyAction, + }, + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, }, }, } @@ -166,7 +220,8 @@ func newPolicyBinding(name string) *admissionregistration.ValidatingAdmissionPol Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ PolicyName: "replicalimit-policy.example.com", ParamRef: &admissionregistration.ParamRef{ - Name: "param-test", + Name: "param-test", + Namespace: "default", }, ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, MatchResources: &admissionregistration.MatchResources{}, diff --git a/pkg/registry/admissionregistration/validatingadmissionpolicybinding/strategy_test.go b/pkg/registry/admissionregistration/validatingadmissionpolicybinding/strategy_test.go index 57d4cedd2c0..55b310db31a 100644 --- a/pkg/registry/admissionregistration/validatingadmissionpolicybinding/strategy_test.go +++ b/pkg/registry/admissionregistration/validatingadmissionpolicybinding/strategy_test.go @@ -35,32 +35,87 @@ func TestPolicyBindingStrategy(t *testing.T) { t.Errorf("PolicyBinding should not allow create on update") } - configuration := validPolicyBinding() - strategy.PrepareForCreate(ctx, configuration) - errs := strategy.Validate(ctx, configuration) - if len(errs) != 0 { - t.Errorf("Unexpected error validating %v", errs) - } - invalidConfiguration := &admissionregistration.ValidatingAdmissionPolicyBinding{ - ObjectMeta: metav1.ObjectMeta{Name: ""}, - } - strategy.PrepareForUpdate(ctx, invalidConfiguration, configuration) - errs = strategy.ValidateUpdate(ctx, invalidConfiguration, configuration) - if len(errs) == 0 { - t.Errorf("Expected a validation error") + for _, configuration := range validPolicyBindings() { + strategy.PrepareForCreate(ctx, configuration) + errs := strategy.Validate(ctx, configuration) + if len(errs) != 0 { + t.Errorf("Unexpected error validating %v", errs) + } + invalidConfiguration := &admissionregistration.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{Name: ""}, + } + strategy.PrepareForUpdate(ctx, invalidConfiguration, configuration) + errs = strategy.ValidateUpdate(ctx, invalidConfiguration, configuration) + if len(errs) == 0 { + t.Errorf("Expected a validation error") + } } } -func validPolicyBinding() *admissionregistration.ValidatingAdmissionPolicyBinding { - return &admissionregistration.ValidatingAdmissionPolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - }, - Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ - PolicyName: "replicalimit-policy.example.com", - ParamRef: &admissionregistration.ParamRef{ - Name: "replica-limit-test.example.com", + +func validPolicyBindings() []*admissionregistration.ValidatingAdmissionPolicyBinding { + denyAction := admissionregistration.DenyAction + return []*admissionregistration.ValidatingAdmissionPolicyBinding{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "replicalimit-policy.example.com", + ParamRef: &admissionregistration.ParamRef{ + Name: "replica-limit-test.example.com", + ParameterNotFoundAction: &denyAction, + }, + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-clusterwide", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "replicalimit-policy.example.com", + ParamRef: &admissionregistration.ParamRef{ + Name: "replica-limit-test.example.com", + Namespace: "default", + ParameterNotFoundAction: &denyAction, + }, + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-selector", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "replicalimit-policy.example.com", + ParamRef: &admissionregistration.ParamRef{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "label": "value", + }, + }, + ParameterNotFoundAction: &denyAction, + }, + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-selector-clusterwide", + }, + Spec: admissionregistration.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "replicalimit-policy.example.com", + ParamRef: &admissionregistration.ParamRef{ + Namespace: "mynamespace", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "label": "value", + }, + }, + ParameterNotFoundAction: &denyAction, + }, + ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, }, - ValidationActions: []admissionregistration.ValidationAction{admissionregistration.Deny}, }, } } diff --git a/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go b/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go index 3b0745cc6d5..575456c8386 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go +++ b/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go @@ -39,6 +39,18 @@ const ( AllScopes ScopeType = v1.AllScopes ) +// ParameterNotFoundActionType specifies a failure policy that defines how a binding +// is evaluated when the param referred by its perNamespaceParamRef is not found. +// +enum +type ParameterNotFoundActionType string + +const ( + // Ignore means that an error finding params for a binding is ignored + AllowAction ParameterNotFoundActionType = "Allow" + // Fail means that an error finding params for a binding is ignored + DenyAction ParameterNotFoundActionType = "Deny" +) + // FailurePolicyType specifies a failure policy that defines how unrecognized errors from the admission endpoint are handled. // +enum type FailurePolicyType string @@ -363,6 +375,15 @@ type AuditAnnotation struct { // ValidatingAdmissionPolicyBinding binds the ValidatingAdmissionPolicy with paramerized resources. // ValidatingAdmissionPolicyBinding and parameter CRDs together define how cluster administrators configure policies for clusters. +// +// For a given admission request, each binding will cause its policy to be +// evaluated N times, where N is 1 for policies/bindings that don't use +// params, otherwise N is the number of parameters selected by the binding. +// +// The CEL expressions of a policy must have a computed CEL cost below the maximum +// CEL budget. Each evaluation of the policy is given an independent CEL cost budget. +// Adding/removing policies, bindings, or params can not affect whether a +// given (policy, binding, param) combination is within its own CEL budget. type ValidatingAdmissionPolicyBinding struct { metav1.TypeMeta `json:",inline"` // Standard object metadata; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata. @@ -393,9 +414,10 @@ type ValidatingAdmissionPolicyBindingSpec struct { // Required. PolicyName string `json:"policyName,omitempty" protobuf:"bytes,1,rep,name=policyName"` - // ParamRef specifies the parameter resource used to configure the admission control policy. + // paramRef specifies the parameter resource used to configure the admission control policy. // It should point to a resource of the type specified in ParamKind of the bound ValidatingAdmissionPolicy. // If the policy specifies a ParamKind and the resource referred to by ParamRef does not exist, this binding is considered mis-configured and the FailurePolicy of the ValidatingAdmissionPolicy applied. + // If the policy does not specify a ParamKind then this field is ignored, and the rules are evaluated without a param. // +optional ParamRef *ParamRef `json:"paramRef,omitempty" protobuf:"bytes,2,rep,name=paramRef"` @@ -450,15 +472,59 @@ type ValidatingAdmissionPolicyBindingSpec struct { ValidationActions []ValidationAction `json:"validationActions,omitempty" protobuf:"bytes,4,rep,name=validationActions"` } -// ParamRef references a parameter resource +// ParamRef describes how to locate the params to be used as input to +// expressions of rules applied by a policy binding. // +structType=atomic type ParamRef struct { - // Name of the resource being referenced. + // `name` is the name of the resource being referenced. + // + // `name` and `selector` are mutually exclusive properties. If one is set, + // the other must be unset. + // + // +optional Name string `json:"name,omitempty" protobuf:"bytes,1,rep,name=name"` - // Namespace of the referenced resource. - // Should be empty for the cluster-scoped resources + + // namespace is the namespace of the referenced resource. Allows limiting + // the search for params to a specific namespace. Applies to both `name` and + // `selector` fields. + // + // A per-namespace parameter may be used by specifying a namespace-scoped + // `paramKind` in the policy and leaving this field empty. + // + // - If `paramKind` is cluster-scoped, this field MUST be unset. Setting this + // field results in a configuration error. + // + // - If `paramKind` is namespace-scoped, the namespace of the object being + // evaluated for admission will be used when this field is left unset. Take + // care that if this is left empty the binding must not match any cluster-scoped + // resources, which will result in an error. + // // +optional Namespace string `json:"namespace,omitempty" protobuf:"bytes,2,rep,name=namespace"` + + // selector can be used to match multiple param objects based on their labels. + // Supply selector: {} to match all resources of the ParamKind. + // + // If multiple params are found, they are all evaluated with the policy expressions + // and the results are ANDed together. + // + // One of `name` or `selector` must be set, but `name` and `selector` are + // mutually exclusive properties. If one is set, the other must be unset. + // + // +optional + Selector *metav1.LabelSelector `json:"selector,omitempty" protobuf:"bytes,3,rep,name=selector"` + + // `parameterNotFoundAction` controls the behavior of the binding when the resource + // exists, and name or selector is valid, but there are no parameters + // matched by the binding. If the value is set to `Allow`, then no + // matched parameters will be treated as successful validation by the binding. + // If set to `Deny`, then no matched parameters will be subject to the + // `failurePolicy` of the policy. + // + // Allowed values are `Allow` or `Deny` + // Default to `Deny` + // +optional + ParameterNotFoundAction *ParameterNotFoundActionType `json:"parameterNotFoundAction,omitempty" protobuf:"bytes,4,rep,name=parameterNotFoundAction"` } // MatchResources decides whether to run the admission control policy on an object based