From d6e4115ead6b93d2accf688876471231b365ceae Mon Sep 17 00:00:00 2001 From: Cici Huang Date: Wed, 1 May 2024 16:26:41 -0700 Subject: [PATCH] Adding the feature gates to fix cost for VAP and webhook matchConditions. --- .../validation/validation.go | 43 +- .../validation/validation_test.go | 20 +- pkg/features/kube_features.go | 4 + .../apiextensions/validation/validation.go | 6 +- .../validation/validation_test.go | 6 +- .../apiserver/schema/cel/compilation_test.go | 6 +- .../pkg/apiserver/schema/cel/validation.go | 3 +- .../apiserver/schema/cel/validation_test.go | 44 ++- .../pkg/admission/plugin/cel/compile.go | 64 +-- .../pkg/admission/plugin/cel/compile_test.go | 4 +- .../admission/plugin/cel/composition_test.go | 52 ++- .../pkg/admission/plugin/cel/filter_test.go | 312 ++++++++++++++- .../pkg/admission/plugin/cel/interface.go | 4 +- .../plugin/policy/validating/plugin.go | 27 +- .../plugin/policy/validating/typechecking.go | 5 +- .../policy/validating/validator_test.go | 2 +- .../pkg/admission/plugin/webhook/accessors.go | 12 + .../plugin/webhook/generic/webhook.go | 6 +- .../apis/apiserver/validation/validation.go | 6 +- .../apiserver/validation/validation_test.go | 2 +- .../pkg/authentication/cel/compile_test.go | 8 +- .../pkg/authorization/cel/compile_test.go | 2 +- .../apiserver/pkg/cel/environment/base.go | 47 ++- .../pkg/cel/environment/base_test.go | 6 +- .../pkg/cel/environment/environment_test.go | 2 +- .../apiserver/pkg/cel/lazy/lazy_test.go | 2 +- .../apiserver/pkg/cel/mutation/env_test.go | 2 +- .../unstructured/typeresolver_test.go | 2 +- .../pkg/cel/openapi/compiling_test.go | 2 +- .../apiserver/pkg/features/kube_features.go | 22 ++ .../structured/namedresources/cel/compile.go | 3 +- .../admissionwebhook/match_conditions_test.go | 368 +++++++++++++++++- .../apiserver/cel/typeresolution_test.go | 2 +- .../cel/validatingadmissionpolicy_test.go | 229 ++++++++++- 34 files changed, 1199 insertions(+), 126 deletions(-) diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index 4695b29eea8..60ab6f1fa22 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -35,6 +35,8 @@ import ( "k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions" "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/cel/environment" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/util/jsonpath" @@ -221,6 +223,7 @@ func ValidateValidatingWebhookConfiguration(e *admissionregistration.ValidatingW requireRecognizedAdmissionReviewVersion: true, requireUniqueWebhookNames: true, allowInvalidLabelValueInSelector: false, + strictCostEnforcement: utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForWebhooks), }) } @@ -250,6 +253,7 @@ func ValidateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebho requireRecognizedAdmissionReviewVersion: true, requireUniqueWebhookNames: true, allowInvalidLabelValueInSelector: false, + strictCostEnforcement: utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForWebhooks), }) } @@ -261,6 +265,7 @@ type validationOptions struct { requireUniqueWebhookNames bool allowInvalidLabelValueInSelector bool preexistingExpressions preexistingExpressions + strictCostEnforcement bool } type preexistingExpressions struct { @@ -687,6 +692,7 @@ func ValidateValidatingWebhookConfigurationUpdate(newC, oldC *admissionregistrat requireUniqueWebhookNames: validatingHasUniqueWebhookNames(oldC.Webhooks), allowInvalidLabelValueInSelector: validatingWebhookHasInvalidLabelValueInSelector(oldC.Webhooks), preexistingExpressions: findValidatingPreexistingExpressions(oldC), + strictCostEnforcement: utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForWebhooks), }) } @@ -700,6 +706,7 @@ func ValidateMutatingWebhookConfigurationUpdate(newC, oldC *admissionregistratio requireUniqueWebhookNames: mutatingHasUniqueWebhookNames(oldC.Webhooks), allowInvalidLabelValueInSelector: mutatingWebhookHasInvalidLabelValueInSelector(oldC.Webhooks), preexistingExpressions: findMutatingPreexistingExpressions(oldC), + strictCostEnforcement: utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForWebhooks), }) } @@ -713,7 +720,7 @@ const ( // ValidateValidatingAdmissionPolicy validates a ValidatingAdmissionPolicy before creation. func ValidateValidatingAdmissionPolicy(p *admissionregistration.ValidatingAdmissionPolicy) field.ErrorList { - return validateValidatingAdmissionPolicy(p, validationOptions{ignoreMatchConditions: false}) + return validateValidatingAdmissionPolicy(p, validationOptions{ignoreMatchConditions: false, strictCostEnforcement: utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForVAP)}) } func validateValidatingAdmissionPolicy(p *admissionregistration.ValidatingAdmissionPolicy, opts validationOptions) field.ErrorList { @@ -728,7 +735,7 @@ func validateValidatingAdmissionPolicySpec(meta metav1.ObjectMeta, spec *admissi getCompiler := func() plugincel.Compiler { if compiler == nil { needsComposition := len(spec.Variables) > 0 - compiler = createCompiler(needsComposition) + compiler = createCompiler(needsComposition, opts.strictCostEnforcement) } return compiler } @@ -973,6 +980,7 @@ func validateVariable(compiler plugincel.Compiler, v *admissionregistration.Vari result := compiler.CompileAndStoreVariable(variable, plugincel.OptionalVariableDeclarations{ HasParams: paramKind != nil, HasAuthorizer: true, + StrictCost: opts.strictCostEnforcement, }, envType) if result.Error != nil { allErrors = append(allErrors, convertCELErrorToValidationError(fldPath.Child("expression"), variable, result.Error)) @@ -1047,6 +1055,7 @@ func validateValidationExpression(compiler plugincel.Compiler, expression string }, plugincel.OptionalVariableDeclarations{ HasParams: hasParams, HasAuthorizer: true, + StrictCost: opts.strictCostEnforcement, }, envType, fldPath) } @@ -1055,11 +1064,18 @@ func validateMatchConditionsExpression(expression string, opts validationOptions if opts.preexistingExpressions.matchConditionExpressions.Has(expression) { envType = environment.StoredExpressions } - return validateCELCondition(statelessCELCompiler, &matchconditions.MatchCondition{ + var compiler plugincel.Compiler + if opts.strictCostEnforcement { + compiler = strictStatelessCELCompiler + } else { + compiler = nonStrictStatelessCELCompiler + } + return validateCELCondition(compiler, &matchconditions.MatchCondition{ Expression: expression, }, plugincel.OptionalVariableDeclarations{ HasParams: opts.allowParamsInMatchConditions, HasAuthorizer: true, + StrictCost: opts.strictCostEnforcement, }, envType, fldPath) } @@ -1073,6 +1089,7 @@ func validateMessageExpression(compiler plugincel.Compiler, expression string, o }, plugincel.OptionalVariableDeclarations{ HasParams: opts.allowParamsInMatchConditions, HasAuthorizer: false, + StrictCost: opts.strictCostEnforcement, }, envType, fldPath) } @@ -1097,7 +1114,7 @@ func validateAuditAnnotation(compiler plugincel.Compiler, meta metav1.ObjectMeta } result := compiler.CompileCELExpression(&validatingadmissionpolicy.AuditAnnotationCondition{ ValueExpression: trimmedValueExpression, - }, plugincel.OptionalVariableDeclarations{HasParams: paramKind != nil, HasAuthorizer: true}, envType) + }, plugincel.OptionalVariableDeclarations{HasParams: paramKind != nil, HasAuthorizer: true, StrictCost: opts.strictCostEnforcement}, envType) if result.Error != nil { switch result.Error.Type { case cel.ErrorTypeRequired: @@ -1191,6 +1208,7 @@ func ValidateValidatingAdmissionPolicyUpdate(newC, oldC *admissionregistration.V return validateValidatingAdmissionPolicy(newC, validationOptions{ ignoreMatchConditions: ignoreValidatingAdmissionPolicyMatchConditions(newC, oldC), preexistingExpressions: findValidatingPolicyPreexistingExpressions(oldC), + strictCostEnforcement: utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForVAP), }) } @@ -1250,17 +1268,24 @@ func validateFieldRef(fieldRef string, fldPath *field.Path) field.ErrorList { // statelessCELCompiler does not support variable composition (and thus is stateless). It should be used when // variable composition is not allowed, for example, when validating MatchConditions. -var statelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) +// strictStatelessCELCompiler is a cel Compiler that enforces strict cost enforcement. +// nonStrictStatelessCELCompiler is a cel Compiler that does not enforce strict cost enforcement. +var strictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) +var nonStrictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false)) -func createCompiler(allowComposition bool) plugincel.Compiler { +func createCompiler(allowComposition, strictCost bool) plugincel.Compiler { if !allowComposition { - return statelessCELCompiler + if strictCost { + return strictStatelessCELCompiler + } else { + return nonStrictStatelessCELCompiler + } } - compiler, err := plugincel.NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler, err := plugincel.NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), strictCost)) if err != nil { // should never happen, but cannot panic either. utilruntime.HandleError(err) - return plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + return plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), strictCost)) } return compiler } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index 8b2d50daa95..23e29bc5b61 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -29,6 +29,8 @@ import ( plugincel "k8s.io/apiserver/pkg/admission/plugin/cel" "k8s.io/apiserver/pkg/cel/environment" "k8s.io/apiserver/pkg/cel/library" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/apis/admissionregistration" ) @@ -3412,8 +3414,9 @@ func TestValidateValidatingAdmissionPolicyUpdate(t *testing.T) { }, // TODO: CustomAuditAnnotations: string valueExpression with {oldObject} is allowed } + strictCost := utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForVAP) // Include the test library, which includes the test() function in the storage environment during test - base := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()) + base := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), strictCost) extended, err := base.Extend(environment.VersionedOptions{ IntroducedVersion: version.MustParseGeneric("1.999"), EnvOptions: []cel.EnvOption{library.Test()}, @@ -3421,10 +3424,17 @@ func TestValidateValidatingAdmissionPolicyUpdate(t *testing.T) { if err != nil { t.Fatal(err) } - statelessCELCompiler = plugincel.NewCompiler(extended) - defer func() { - statelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) - }() + if strictCost { + strictStatelessCELCompiler = plugincel.NewCompiler(extended) + defer func() { + strictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), strictCost)) + }() + } else { + nonStrictStatelessCELCompiler = plugincel.NewCompiler(extended) + defer func() { + nonStrictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), strictCost)) + }() + } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 6db15d49d86..ab6ea529985 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -1270,6 +1270,10 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS genericfeatures.StorageVersionHash: {Default: true, PreRelease: featuregate.Beta}, + genericfeatures.StrictCostEnforcementForVAP: {Default: false, PreRelease: featuregate.Beta}, + + genericfeatures.StrictCostEnforcementForWebhooks: {Default: false, PreRelease: featuregate.Beta}, + genericfeatures.StructuredAuthenticationConfiguration: {Default: true, PreRelease: featuregate.Beta}, genericfeatures.StructuredAuthorizationConfiguration: {Default: true, PreRelease: featuregate.Beta}, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index 970a6519c1a..7133776fbbe 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -92,7 +92,8 @@ func ValidateCustomResourceDefinition(ctx context.Context, obj *apiextensions.Cu requirePrunedDefaults: true, requireAtomicSetType: true, requireMapListKeysMapSetValidation: true, - celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()), + // strictCost is always true to enforce cost limits. + celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), } allErrs := genericvalidation.ValidateObjectMeta(&obj.ObjectMeta, false, nameValidationFn, field.NewPath("metadata")) @@ -231,7 +232,8 @@ func ValidateCustomResourceDefinitionUpdate(ctx context.Context, obj, oldObj *ap requireMapListKeysMapSetValidation: requireMapListKeysMapSetValidation(&oldObj.Spec), preexistingExpressions: findPreexistingExpressions(&oldObj.Spec), versionsWithUnchangedSchemas: findVersionsWithUnchangedSchemas(obj, oldObj), - celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()), + // strictCost is always true to enforce cost limits. + celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), } return validateCustomResourceDefinitionUpdate(ctx, obj, oldObj, opts) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index f68088b3dfe..cf8edcdc49a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go @@ -7312,7 +7312,7 @@ func TestValidateCustomResourceDefinitionValidationRuleCompatibility(t *testing. } // Include the test library, which includes the test() function in the storage environment during test - base := environment.MustBaseEnvSet(version.MajorMinor(1, 998)) + base := environment.MustBaseEnvSet(version.MajorMinor(1, 998), true) envSet, err := base.Extend(environment.VersionedOptions{ IntroducedVersion: version.MajorMinor(1, 999), EnvOptions: []cel.EnvOption{library.Test()}, @@ -10104,7 +10104,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctx := context.TODO() if tt.opts.celEnvironmentSet == nil { - tt.opts.celEnvironmentSet = environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()) + tt.opts.celEnvironmentSet = environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true) } got := validateCustomResourceDefinitionValidation(ctx, &tt.input, tt.statusEnabled, tt.opts, field.NewPath("spec", "validation")) @@ -10635,7 +10635,7 @@ func TestCelContext(t *testing.T) { celContext := RootCELContext(tt.schema) celContext.converter = converter opts := validationOptions{ - celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()), + celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), } openAPIV3Schema := &specStandardValidatorV3{ allowDefaults: opts.allowDefaults, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go index dc501189b55..ac9149c2482 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go @@ -850,7 +850,7 @@ func TestCelCompilation(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { - env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()).Extend( + env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true).Extend( environment.VersionedOptions{ IntroducedVersion: version.MajorMinor(1, 999), EnvOptions: []celgo.EnvOption{celgo.Lib(&fakeLib{})}, @@ -1327,7 +1327,7 @@ func genMapWithCustomItemRule(item *schema.Structural, rule string) func(maxProp // if expectedCostExceedsLimit is non-zero. Typically, only expectedCost or expectedCostExceedsLimit is non-zero, not both. func schemaChecker(schema *schema.Structural, expectedCost uint64, expectedCostExceedsLimit uint64, t *testing.T) func(t *testing.T) { return func(t *testing.T) { - compilationResults, err := Compile(schema, model.SchemaDeclType(schema, false), celconfig.PerCallLimit, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()), NewExpressionsEnvLoader()) + compilationResults, err := Compile(schema, model.SchemaDeclType(schema, false), celconfig.PerCallLimit, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), NewExpressionsEnvLoader()) if err != nil { t.Fatalf("Expected no error, got: %v", err) } @@ -1885,7 +1885,7 @@ func TestCostEstimation(t *testing.T) { } func BenchmarkCompile(b *testing.B) { - env := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()) // prepare the environment + env := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true) // prepare the environment s := genArrayWithRule("number", "true")(nil) b.ReportAllocs() b.ResetTimer() diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go index b4c8afa9af7..d9b595805ba 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go @@ -90,7 +90,8 @@ func NewValidator(s *schema.Structural, isResourceRoot bool, perCallLimit uint64 // exist. declType is expected to be a CEL DeclType corresponding to the structural schema. // perCallLimit was added for testing purpose only. Callers should always use const PerCallLimit from k8s.io/apiserver/pkg/apis/cel/config.go as input. func validator(s *schema.Structural, isResourceRoot bool, declType *cel.DeclType, perCallLimit uint64) *Validator { - compiledRules, err := Compile(s, declType, perCallLimit, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()), StoredExpressionsEnvLoader()) + // strictCost is always true to enforce cost limits. + compiledRules, err := Compile(s, declType, perCallLimit, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), StoredExpressionsEnvLoader()) var itemsValidator, additionalPropertiesValidator *Validator var propertiesValidators map[string]Validator if s.Items != nil { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go index 46ed0105259..1041aed7ada 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go @@ -61,6 +61,7 @@ func TestValidationExpressions(t *testing.T) { costBudget int64 isRoot bool expectSkipped bool + expectedCost int64 }{ // tests where val1 and val2 are equal but val3 is different // equality, comparisons and type specific functions @@ -2006,6 +2007,38 @@ func TestValidationExpressions(t *testing.T) { `quantity(self.val1).isInteger()`, }, }, + {name: "cost for extended lib calculated correctly: isSorted", + obj: objs("20", "200M"), + schema: schemas(stringType, stringType), + valid: []string{ + "[1,2,3,4].isSorted()", + }, + expectedCost: 4, + }, + {name: "cost for extended lib calculated correctly: url", + obj: objs("20", "200M"), + schema: schemas(stringType, stringType), + valid: []string{ + "url('https:://kubernetes.io/').getHostname() != 'test'", + }, + expectedCost: 4, + }, + {name: "cost for extended lib calculated correctly: split", + obj: objs("20", "200M"), + schema: schemas(stringType, stringType), + valid: []string{ + "size('abc 123 def 123'.split(' ')) > 0", + }, + expectedCost: 5, + }, + {name: "cost for extended lib calculated correctly: join", + obj: objs("20", "200M"), + schema: schemas(stringType, stringType), + valid: []string{ + "size(['aa', 'bb', 'cc', 'd', 'e', 'f', 'g', 'h', 'i', 'j'].join(' ')) > 0", + }, + expectedCost: 7, + }, } for i := range tests { @@ -2013,7 +2046,9 @@ func TestValidationExpressions(t *testing.T) { t.Run(tests[i].name, func(t *testing.T) { t.Parallel() tt := tests[i] - tt.costBudget = celconfig.RuntimeCELCostBudget + if tt.costBudget == 0 { + tt.costBudget = celconfig.RuntimeCELCostBudget + } ctx := context.TODO() for j := range tt.valid { validRule := tt.valid[j] @@ -2032,6 +2067,13 @@ func TestValidationExpressions(t *testing.T) { for _, err := range errs { t.Errorf("unexpected error: %v", err) } + + if tt.expectedCost != 0 { + if remainingBudget != tt.costBudget-tt.expectedCost { + t.Errorf("expected cost to be %d, but got %d", tt.expectedCost, tt.costBudget-remainingBudget) + } + return + } if tt.expectSkipped { // Skipped validations should have no cost. The only possible false positive here would be the CEL expression 'true'. if remainingBudget != tt.costBudget { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go index b7b589d273a..bb5e233d453 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go @@ -222,40 +222,48 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor, op func mustBuildEnvs(baseEnv *environment.EnvSet) variableDeclEnvs { requestType := BuildRequestType() namespaceType := BuildNamespaceType() - envs := make(variableDeclEnvs, 4) // since the number of variable combinations is small, pre-build a environment for each + envs := make(variableDeclEnvs, 8) // since the number of variable combinations is small, pre-build a environment for each for _, hasParams := range []bool{false, true} { for _, hasAuthorizer := range []bool{false, true} { - var envOpts []cel.EnvOption - if hasParams { - envOpts = append(envOpts, cel.Variable(ParamsVarName, cel.DynType)) - } - if hasAuthorizer { + for _, strictCost := range []bool{false, true} { + var envOpts []cel.EnvOption + if hasParams { + envOpts = append(envOpts, cel.Variable(ParamsVarName, cel.DynType)) + } + if hasAuthorizer { + envOpts = append(envOpts, + cel.Variable(AuthorizerVarName, library.AuthorizerType), + cel.Variable(RequestResourceAuthorizerVarName, library.ResourceCheckType)) + } envOpts = append(envOpts, - cel.Variable(AuthorizerVarName, library.AuthorizerType), - cel.Variable(RequestResourceAuthorizerVarName, library.ResourceCheckType)) - } - envOpts = append(envOpts, - cel.Variable(ObjectVarName, cel.DynType), - cel.Variable(OldObjectVarName, cel.DynType), - cel.Variable(NamespaceVarName, namespaceType.CelType()), - cel.Variable(RequestVarName, requestType.CelType())) + cel.Variable(ObjectVarName, cel.DynType), + cel.Variable(OldObjectVarName, cel.DynType), + cel.Variable(NamespaceVarName, namespaceType.CelType()), + cel.Variable(RequestVarName, requestType.CelType())) - extended, err := baseEnv.Extend( - environment.VersionedOptions{ - // Feature epoch was actually 1.26, but we artificially set it to 1.0 because these - // options should always be present. - IntroducedVersion: version.MajorMinor(1, 0), - EnvOptions: envOpts, - DeclTypes: []*apiservercel.DeclType{ - namespaceType, - requestType, + extended, err := baseEnv.Extend( + environment.VersionedOptions{ + // Feature epoch was actually 1.26, but we artificially set it to 1.0 because these + // options should always be present. + IntroducedVersion: version.MajorMinor(1, 0), + EnvOptions: envOpts, + DeclTypes: []*apiservercel.DeclType{ + namespaceType, + requestType, + }, }, - }, - ) - if err != nil { - panic(fmt.Sprintf("environment misconfigured: %v", err)) + ) + if err != nil { + panic(fmt.Sprintf("environment misconfigured: %v", err)) + } + if strictCost { + extended, err = extended.Extend(environment.StrictCostOpt) + if err != nil { + panic(fmt.Sprintf("environment misconfigured: %v", err)) + } + } + envs[OptionalVariableDeclarations{HasParams: hasParams, HasAuthorizer: hasAuthorizer, StrictCost: strictCost}] = extended } - envs[OptionalVariableDeclarations{HasParams: hasParams, HasAuthorizer: hasAuthorizer}] = extended } } return envs diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile_test.go index 5caf82858a4..2059cc52486 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile_test.go @@ -178,7 +178,7 @@ func TestCompileValidatingPolicyExpression(t *testing.T) { } // Include the test library, which includes the test() function in the storage environment during test - base := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()) + base := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true) extended, err := base.Extend(environment.VersionedOptions{ IntroducedVersion: version.MajorMinor(1, 999), EnvOptions: []celgo.EnvOption{library.Test()}, @@ -254,7 +254,7 @@ func TestCompileValidatingPolicyExpression(t *testing.T) { } func BenchmarkCompile(b *testing.B) { - compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) b.ResetTimer() for i := 0; i < b.N; i++ { options := OptionalVariableDeclarations{HasParams: rand.Int()%2 == 0, HasAuthorizer: rand.Int()%2 == 0} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go index aedc7c969f0..53d9b861284 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go @@ -48,14 +48,15 @@ func (t *testVariable) GetName() string { func TestCompositedPolicies(t *testing.T) { cases := []struct { - name string - variables []NamedExpressionAccessor - expression string - attributes admission.Attributes - expectedResult any - expectErr bool - expectedErrorMessage string - runtimeCostBudget int64 + name string + variables []NamedExpressionAccessor + expression string + attributes admission.Attributes + expectedResult any + expectErr bool + expectedErrorMessage string + runtimeCostBudget int64 + strictCostEnforcement bool }{ { name: "simple", @@ -185,16 +186,45 @@ func TestCompositedPolicies(t *testing.T) { expectErr: true, expectedErrorMessage: "found no matching overload for '_==_' applied to '(string, int)'", }, + { + name: "with strictCostEnforcement on: exceeds cost budget", + variables: []NamedExpressionAccessor{ + &testVariable{ + name: "dict", + expression: "'abc 123 def 123'.split(' ')", + }, + }, + attributes: endpointCreateAttributes(), + expression: "size(variables.dict) > 0", + expectErr: true, + expectedErrorMessage: "validation failed due to running out of cost budget, no further validation rules will be run", + runtimeCostBudget: 5, + strictCostEnforcement: true, + }, + { + name: "with strictCostEnforcement off: not exceed cost budget", + variables: []NamedExpressionAccessor{ + &testVariable{ + name: "dict", + expression: "'abc 123 def 123'.split(' ')", + }, + }, + attributes: endpointCreateAttributes(), + expression: "size(variables.dict) > 0", + expectedResult: true, + runtimeCostBudget: 5, + strictCostEnforcement: false, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - compiler, err := NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler, err := NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), tc.strictCostEnforcement)) if err != nil { t.Fatal(err) } - compiler.CompileAndStoreVariables(tc.variables, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.NewExpressions) + compiler.CompileAndStoreVariables(tc.variables, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false, StrictCost: tc.strictCostEnforcement}, environment.NewExpressions) validations := []ExpressionAccessor{&condition{Expression: tc.expression}} - f := compiler.Compile(validations, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.NewExpressions) + f := compiler.Compile(validations, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false, StrictCost: tc.strictCostEnforcement}, environment.NewExpressions) versionedAttr, err := admission.NewVersionedAttributes(tc.attributes, tc.attributes.GetKind(), newObjectInterfacesForTest()) if err != nil { t.Fatal(err) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go index 390c5d780ae..8ce7d470955 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go @@ -93,8 +93,8 @@ func TestCompile(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - c := filterCompiler{compiler: NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))} - e := c.Compile(tc.validation, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.NewExpressions) + c := filterCompiler{compiler: NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))} + e := c.Compile(tc.validation, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false, StrictCost: true}, environment.NewExpressions) if e == nil { t.Fatalf("unexpected nil validator") } @@ -182,6 +182,7 @@ func TestFilter(t *testing.T) { authorizer authorizer.Authorizer testPerCallLimit uint64 namespaceObject *corev1.Namespace + strictCost bool }{ { name: "valid syntax for object", @@ -762,7 +763,7 @@ func TestFilter(t *testing.T) { if tc.testPerCallLimit == 0 { tc.testPerCallLimit = celconfig.PerCallLimit } - env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()).Extend( + env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), tc.strictCost).Extend( environment.VersionedOptions{ IntroducedVersion: environment.DefaultCompatibilityVersion(), ProgramOptions: []celgo.ProgramOption{celgo.CostLimit(tc.testPerCallLimit)}, @@ -772,7 +773,7 @@ func TestFilter(t *testing.T) { t.Fatal(err) } c := NewFilterCompiler(env) - f := c.Compile(tc.validations, OptionalVariableDeclarations{HasParams: tc.hasParamKind, HasAuthorizer: tc.authorizer != nil}, environment.NewExpressions) + f := c.Compile(tc.validations, OptionalVariableDeclarations{HasParams: tc.hasParamKind, HasAuthorizer: tc.authorizer != nil, StrictCost: tc.strictCost}, environment.NewExpressions) if f == nil { t.Fatalf("unexpected nil validator") } @@ -815,15 +816,17 @@ func TestRuntimeCELCostBudget(t *testing.T) { } cases := []struct { - name string - attributes admission.Attributes - params runtime.Object - validations []ExpressionAccessor - hasParamKind bool - authorizer authorizer.Authorizer - testRuntimeCELCostBudget int64 - exceedBudget bool - expectRemainingBudget *int64 + name string + attributes admission.Attributes + params runtime.Object + validations []ExpressionAccessor + hasParamKind bool + authorizer authorizer.Authorizer + testRuntimeCELCostBudget int64 + exceedBudget bool + expectRemainingBudget *int64 + enableStrictCostEnforcement bool + exceedPerCallLimit bool }{ { name: "expression exceed RuntimeCELCostBudget at fist expression", @@ -910,12 +913,275 @@ func TestRuntimeCELCostBudget(t *testing.T) { testRuntimeCELCostBudget: 6, expectRemainingBudget: pointer.Int64(0), }, + { + name: "Extended library cost: authz check", + validations: []ExpressionAccessor{ + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed()", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 6, + expectRemainingBudget: pointer.Int64(0), + authorizer: denyAll, + }, + { + name: "Extended library cost: isSorted()", + validations: []ExpressionAccessor{ + &condition{ + Expression: "[1,2,3,4].isSorted()", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 1, + expectRemainingBudget: pointer.Int64(0), + }, + { + name: "Extended library cost: url", + validations: []ExpressionAccessor{ + &condition{ + Expression: "url('https:://kubernetes.io/').getHostname() == 'kubernetes.io'", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 2, + expectRemainingBudget: pointer.Int64(0), + }, + { + name: "Extended library cost: split", + validations: []ExpressionAccessor{ + &condition{ + Expression: "size('abc 123 def 123'.split(' ')) > 0", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 3, + expectRemainingBudget: pointer.Int64(0), + }, + { + name: "Extended library cost: join", + validations: []ExpressionAccessor{ + &condition{ + Expression: "size(['aa', 'bb', 'cc', 'd', 'e', 'f', 'g', 'h', 'i', 'j'].join(' ')) > 0", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 3, + expectRemainingBudget: pointer.Int64(0), + }, + { + name: "Extended library cost: find", + validations: []ExpressionAccessor{ + &condition{ + Expression: "size('abc 123 def 123'.find('123')) > 0", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 3, + expectRemainingBudget: pointer.Int64(0), + }, + { + name: "Extended library cost: quantity", + validations: []ExpressionAccessor{ + &condition{ + Expression: "quantity(\"200M\") == quantity(\"0.2G\") && quantity(\"0.2G\") == quantity(\"200M\")", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 6, + expectRemainingBudget: pointer.Int64(0), + }, + { + name: "With StrictCostEnforcementForVAP enabled: expression exceed RuntimeCELCostBudget at fist expression", + validations: []ExpressionAccessor{ + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed()", + }, + &condition{ + Expression: "has(object.subsets)", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + testRuntimeCELCostBudget: 35000, + exceedBudget: true, + authorizer: denyAll, + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: expression exceed RuntimeCELCostBudget at last expression", + validations: []ExpressionAccessor{ + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed()", + }, + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed()", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + testRuntimeCELCostBudget: 700000, + exceedBudget: true, + authorizer: denyAll, + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: test RuntimeCELCostBudge is not exceed", + validations: []ExpressionAccessor{ + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed()", + }, + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed()", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: true, + params: configMapParams, + exceedBudget: false, + testRuntimeCELCostBudget: 700011, + expectRemainingBudget: pointer.Int64(1), // 700011 - 700010 + authorizer: denyAll, + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: test RuntimeCELCostBudge exactly covers", + validations: []ExpressionAccessor{ + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed()", + }, + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed()", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: true, + params: configMapParams, + exceedBudget: false, + testRuntimeCELCostBudget: 700010, + expectRemainingBudget: pointer.Int64(0), + authorizer: denyAll, + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: per call limit exceeds", + validations: []ExpressionAccessor{ + &condition{ + Expression: "!authorizer.group('').resource('endpoints').check('create').allowed() && !authorizer.group('').resource('endpoints').check('create').allowed() && !authorizer.group('').resource('endpoints').check('create').allowed()", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: true, + params: configMapParams, + authorizer: denyAll, + exceedPerCallLimit: true, + testRuntimeCELCostBudget: -1, + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: Extended library cost: isSorted()", + validations: []ExpressionAccessor{ + &condition{ + Expression: "[1,2,3,4].isSorted()", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 4, + expectRemainingBudget: pointer.Int64(0), + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: Extended library cost: url", + validations: []ExpressionAccessor{ + &condition{ + Expression: "url('https:://kubernetes.io/').getHostname() == 'kubernetes.io'", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 4, + expectRemainingBudget: pointer.Int64(0), + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: Extended library cost: split", + validations: []ExpressionAccessor{ + &condition{ + Expression: "size('abc 123 def 123'.split(' ')) > 0", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 5, + expectRemainingBudget: pointer.Int64(0), + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: Extended library cost: join", + validations: []ExpressionAccessor{ + &condition{ + Expression: "size(['aa', 'bb', 'cc', 'd', 'e', 'f', 'g', 'h', 'i', 'j'].join(' ')) > 0", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 7, + expectRemainingBudget: pointer.Int64(0), + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: Extended library cost: find", + validations: []ExpressionAccessor{ + &condition{ + Expression: "size('abc 123 def 123'.find('123')) > 0", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 4, + expectRemainingBudget: pointer.Int64(0), + enableStrictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP enabled: Extended library cost: quantity", + validations: []ExpressionAccessor{ + &condition{ + Expression: "quantity(\"200M\") == quantity(\"0.2G\") && quantity(\"0.2G\") == quantity(\"200M\")", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: false, + exceedBudget: false, + testRuntimeCELCostBudget: 6, + expectRemainingBudget: pointer.Int64(0), + enableStrictCostEnforcement: true, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - c := filterCompiler{compiler: NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))} - f := c.Compile(tc.validations, OptionalVariableDeclarations{HasParams: tc.hasParamKind, HasAuthorizer: false}, environment.NewExpressions) + c := filterCompiler{compiler: NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), tc.enableStrictCostEnforcement))} + f := c.Compile(tc.validations, OptionalVariableDeclarations{HasParams: tc.hasParamKind, HasAuthorizer: true, StrictCost: tc.enableStrictCostEnforcement}, environment.NewExpressions) if f == nil { t.Fatalf("unexpected nil validator") } @@ -928,16 +1194,28 @@ func TestRuntimeCELCostBudget(t *testing.T) { t.Fatalf("unexpected error on conversion: %v", err) } - if tc.testRuntimeCELCostBudget == 0 { + if tc.testRuntimeCELCostBudget < 0 { tc.testRuntimeCELCostBudget = celconfig.RuntimeCELCostBudget } optionalVars := OptionalVariableBindings{VersionedParams: tc.params, Authorizer: tc.authorizer} ctx := context.TODO() evalResults, remaining, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes, metav1.GroupVersionResource(versionedAttr.GetResource()), metav1.GroupVersionKind(versionedAttr.VersionedKind)), optionalVars, nil, tc.testRuntimeCELCostBudget) + if tc.exceedPerCallLimit { + hasCostErr := false + for _, evalResult := range evalResults { + if evalResult.Error != nil && strings.Contains(evalResult.Error.Error(), "operation cancelled: actual cost limit exceeded") { + hasCostErr = true + break + } + } + if !hasCostErr { + t.Errorf("Expected per call limit exceeded error but didn't get one") + } + } if tc.exceedBudget && err == nil { t.Errorf("Expected RuntimeCELCostBudge to be exceeded but got nil") } - if tc.exceedBudget && !strings.Contains(err.Error(), "validation failed due to running out of cost budget, no further validation rules will be run") { + if tc.exceedBudget && err != nil && !strings.Contains(err.Error(), "validation failed due to running out of cost budget, no further validation rules will be run") { t.Errorf("Expected RuntimeCELCostBudge exceeded error but got: %v", err) } if err != nil && remaining != -1 { 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 c9f4e63369f..ae61dc826c4 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 @@ -57,10 +57,12 @@ type OptionalVariableDeclarations struct { // HasParams specifies if the "params" variable is declared. // The "params" variable may still be bound to "null" when declared. HasParams bool - // HasAuthorizer specifies if the"authorizer" and "authorizer.requestResource" + // HasAuthorizer specifies if the "authorizer" and "authorizer.requestResource" // variables are declared. When declared, the authorizer variables are // expected to be non-null. HasAuthorizer bool + // StrictCost specifies if the CEL cost limitation is strict for extended libraries as well as native libraries. + StrictCost bool } // FilterCompiler contains a function to assist with converting types and values to/from CEL-typed values. diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go index c286cffbdc8..fb097737a8c 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go @@ -31,6 +31,7 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/cel/environment" "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" @@ -43,13 +44,21 @@ const ( ) var ( - compositionEnvTemplate *cel.CompositionEnv = func() *cel.CompositionEnv { - compositionEnvTemplate, err := cel.NewCompositionEnv(cel.VariablesTypeName, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compositionEnvTemplateWithStrictCost *cel.CompositionEnv = func() *cel.CompositionEnv { + compositionEnvTemplateWithStrictCost, err := cel.NewCompositionEnv(cel.VariablesTypeName, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) if err != nil { panic(err) } - return compositionEnvTemplate + return compositionEnvTemplateWithStrictCost + }() + compositionEnvTemplateWithoutStrictCost *cel.CompositionEnv = func() *cel.CompositionEnv { + compositionEnvTemplateWithoutStrictCost, err := cel.NewCompositionEnv(cel.VariablesTypeName, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false)) + if err != nil { + panic(err) + } + + return compositionEnvTemplateWithoutStrictCost }() ) @@ -114,12 +123,18 @@ func compilePolicy(policy *Policy) Validator { if policy.Spec.ParamKind != nil { hasParam = true } - optionalVars := cel.OptionalVariableDeclarations{HasParams: hasParam, HasAuthorizer: true} - expressionOptionalVars := cel.OptionalVariableDeclarations{HasParams: hasParam, HasAuthorizer: false} + strictCost := utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForVAP) + optionalVars := cel.OptionalVariableDeclarations{HasParams: hasParam, HasAuthorizer: true, StrictCost: strictCost} + expressionOptionalVars := cel.OptionalVariableDeclarations{HasParams: hasParam, HasAuthorizer: false, StrictCost: strictCost} failurePolicy := policy.Spec.FailurePolicy var matcher matchconditions.Matcher = nil matchConditions := policy.Spec.MatchConditions - + var compositionEnvTemplate *cel.CompositionEnv + if strictCost { + compositionEnvTemplate = compositionEnvTemplateWithStrictCost + } else { + compositionEnvTemplate = compositionEnvTemplateWithoutStrictCost + } filterCompiler := cel.NewCompositedCompilerFromTemplate(compositionEnvTemplate) filterCompiler.CompileAndStoreVariables(convertv1beta1Variables(policy.Spec.Variables), optionalVars, environment.StoredExpressions) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking.go index 16184b4badd..192be9621bd 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking.go @@ -39,6 +39,8 @@ import ( "k8s.io/apiserver/pkg/cel/library" "k8s.io/apiserver/pkg/cel/openapi" "k8s.io/apiserver/pkg/cel/openapi/resolver" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" ) @@ -210,6 +212,7 @@ func (c *TypeChecker) CheckExpression(ctx *TypeCheckingContext, expression strin options := plugincel.OptionalVariableDeclarations{ HasParams: ctx.paramDeclType != nil, HasAuthorizer: true, + StrictCost: utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForVAP), } compiler.CompileAndStoreVariables(convertv1beta1Variables(ctx.variables), options, environment.StoredExpressions) result := compiler.CompileCELExpression(celExpression(expression), options, environment.StoredExpressions) @@ -391,7 +394,7 @@ func (c *TypeChecker) tryRefreshRESTMapper() { } func buildEnvSet(hasParams bool, hasAuthorizer bool, types typeOverwrite) (*environment.EnvSet, error) { - baseEnv := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()) + baseEnv := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForVAP)) requestType := plugincel.BuildRequestType() namespaceType := plugincel.BuildNamespaceType() diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator_test.go index ba4f1ca86c7..99b3556b544 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator_test.go @@ -931,7 +931,7 @@ func TestContextCanceled(t *testing.T) { fakeAttr := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "default", "foo", schema.GroupVersionResource{}, "", admission.Create, nil, false, nil) fakeVersionedAttr, _ := admission.NewVersionedAttributes(fakeAttr, schema.GroupVersionKind{}, nil) - fc := cel.NewFilterCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + fc := cel.NewFilterCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) f := fc.Compile([]cel.ExpressionAccessor{&ValidationCondition{Expression: "[1,2,3,4,5,6,7,8,9,10].map(x, [1,2,3,4,5,6,7,8,9,10].map(y, x*y)) == []"}}, cel.OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.StoredExpressions) v := validator{ failPolicy: &fail, diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go index e60d245a621..f23580cc09f 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go @@ -27,6 +27,8 @@ import ( "k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace" "k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object" "k8s.io/apiserver/pkg/cel/environment" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" webhookutil "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/rest" ) @@ -139,11 +141,16 @@ func (m *mutatingWebhookAccessor) GetCompiledMatcher(compiler cel.FilterCompiler Expression: matchCondition.Expression, } } + strictCost := false + if utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForWebhooks) { + strictCost = true + } m.compiledMatcher = matchconditions.NewMatcher(compiler.Compile( expressions, cel.OptionalVariableDeclarations{ HasParams: false, HasAuthorizer: true, + StrictCost: strictCost, }, environment.StoredExpressions, ), m.FailurePolicy, "webhook", "admit", m.Name) @@ -267,11 +274,16 @@ func (v *validatingWebhookAccessor) GetCompiledMatcher(compiler cel.FilterCompil Expression: matchCondition.Expression, } } + strictCost := false + if utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForWebhooks) { + strictCost = true + } v.compiledMatcher = matchconditions.NewMatcher(compiler.Compile( expressions, cel.OptionalVariableDeclarations{ HasParams: false, HasAuthorizer: true, + StrictCost: strictCost, }, environment.StoredExpressions, ), v.FailurePolicy, "webhook", "validating", v.Name) 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 6a513f1c11a..f067b3f723c 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 @@ -21,7 +21,6 @@ import ( "fmt" "io" - admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" "k8s.io/klog/v2" admissionv1 "k8s.io/api/admission/v1" @@ -31,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" genericadmissioninit "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/webhook" "k8s.io/apiserver/pkg/admission/plugin/webhook/config" @@ -39,6 +39,8 @@ import ( "k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/cel/environment" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" webhookutil "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/informers" clientset "k8s.io/client-go/kubernetes" @@ -100,7 +102,7 @@ func NewWebhook(handler *admission.Handler, configFile io.Reader, sourceFactory namespaceMatcher: &namespace.Matcher{}, objectMatcher: &object.Matcher{}, dispatcher: dispatcherFactory(&cm), - filterCompiler: cel.NewFilterCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())), + filterCompiler: cel.NewFilterCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcementForWebhooks))), }, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go index 35ee8a4503c..471eb4a7410 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go @@ -91,7 +91,8 @@ func CompileAndValidateJWTAuthenticator(authenticator api.JWTAuthenticator, disa func validateJWTAuthenticator(authenticator api.JWTAuthenticator, fldPath *field.Path, disallowedIssuers sets.Set[string], structuredAuthnFeatureEnabled bool) (authenticationcel.CELMapper, field.ErrorList) { var allErrs field.ErrorList - compiler := authenticationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + // strictCost is set to true which enables the strict cost for CEL validation. + compiler := authenticationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) state := &validationState{} allErrs = append(allErrs, validateIssuer(authenticator.Issuer, disallowedIssuers, fldPath.Child("issuer"))...) @@ -722,7 +723,8 @@ func compileMatchConditions(matchConditions []api.WebhookMatchCondition, fldPath return nil, allErrs } - compiler := authorizationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + // strictCost is set to true which enables the strict cost for CEL validation. + compiler := authorizationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) seenExpressions := sets.NewString() var compilationResults []authorizationcel.CompilationResult diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go index 7fc442f4922..aa603b870d8 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go @@ -43,7 +43,7 @@ import ( ) var ( - compiler = authenticationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler = authenticationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) ) func TestValidateAuthenticationConfiguration(t *testing.T) { diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/cel/compile_test.go b/staging/src/k8s.io/apiserver/pkg/authentication/cel/compile_test.go index c8659aa830b..5615f4766cc 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/cel/compile_test.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/cel/compile_test.go @@ -57,7 +57,7 @@ func TestCompileClaimsExpression(t *testing.T) { }, } - compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -86,7 +86,7 @@ func TestCompileUserExpression(t *testing.T) { }, } - compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -135,7 +135,7 @@ func TestCompileClaimsExpressionError(t *testing.T) { }, } - compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -205,7 +205,7 @@ func TestCompileUserExpressionError(t *testing.T) { }, } - compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go index ec7975b90e0..aecb3146384 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go @@ -59,7 +59,7 @@ func TestCompileCELExpression(t *testing.T) { }, } - compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go b/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go index 5eacb61050c..74453a7ef63 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go @@ -46,7 +46,9 @@ func DefaultCompatibilityVersion() *version.Version { return version.MajorMinor(1, 29) } -var baseOpts = []VersionedOptions{ +var baseOpts = append(baseOptsWithoutStrictCost, StrictCostOpt) + +var baseOptsWithoutStrictCost = []VersionedOptions{ { // CEL epoch was actually 1.23, but we artificially set it to 1.0 because these // options should always be present. @@ -139,6 +141,14 @@ var baseOpts = []VersionedOptions{ }, } +var StrictCostOpt = VersionedOptions{ + // This is to configure the cost calculation for extended libraries + IntroducedVersion: version.MajorMinor(1, 0), + ProgramOptions: []cel.ProgramOption{ + cel.CostTracking(&library.CostEstimator{}), + }, +} + // MustBaseEnvSet returns the common CEL base environments for Kubernetes for Version, or panics // if the version is nil, or does not have major and minor components. // @@ -148,7 +158,8 @@ var baseOpts = []VersionedOptions{ // The returned environment contains no CEL variable definitions or custom type declarations and // should be extended to construct environments with the appropriate variable definitions, // type declarations and any other needed configuration. -func MustBaseEnvSet(ver *version.Version) *EnvSet { +// strictCost is used to determine whether to enforce strict cost calculation for CEL expressions. +func MustBaseEnvSet(ver *version.Version, strictCost bool) *EnvSet { if ver == nil { panic("version must be non-nil") } @@ -156,19 +167,33 @@ func MustBaseEnvSet(ver *version.Version) *EnvSet { panic(fmt.Sprintf("version must contain an major and minor component, but got: %s", ver.String())) } key := strconv.FormatUint(uint64(ver.Major()), 10) + "." + strconv.FormatUint(uint64(ver.Minor()), 10) - if entry, ok := baseEnvs.Load(key); ok { - return entry.(*EnvSet) + var entry interface{} + if strictCost { + if entry, ok := baseEnvs.Load(key); ok { + return entry.(*EnvSet) + } + entry, _, _ = baseEnvsSingleflight.Do(key, func() (interface{}, error) { + entry := mustNewEnvSet(ver, baseOpts) + baseEnvs.Store(key, entry) + return entry, nil + }) + } else { + if entry, ok := baseEnvsWithOption.Load(key); ok { + return entry.(*EnvSet) + } + entry, _, _ = baseEnvsWithOptionSingleflight.Do(key, func() (interface{}, error) { + entry := mustNewEnvSet(ver, baseOptsWithoutStrictCost) + baseEnvsWithOption.Store(key, entry) + return entry, nil + }) } - entry, _, _ := baseEnvsSingleflight.Do(key, func() (interface{}, error) { - entry := mustNewEnvSet(ver, baseOpts) - baseEnvs.Store(key, entry) - return entry, nil - }) return entry.(*EnvSet) } var ( - baseEnvs = sync.Map{} - baseEnvsSingleflight = &singleflight.Group{} + baseEnvs = sync.Map{} + baseEnvsWithOption = sync.Map{} + baseEnvsSingleflight = &singleflight.Group{} + baseEnvsWithOptionSingleflight = &singleflight.Group{} ) diff --git a/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go b/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go index 4bfb764576e..a21891115cf 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go @@ -29,10 +29,10 @@ import ( // a cached environment is loaded for each MustBaseEnvSet call. func BenchmarkLoadBaseEnv(b *testing.B) { ver := DefaultCompatibilityVersion() - MustBaseEnvSet(ver) + MustBaseEnvSet(ver, true) b.ResetTimer() for i := 0; i < b.N; i++ { - MustBaseEnvSet(ver) + MustBaseEnvSet(ver, true) } } @@ -41,7 +41,7 @@ func BenchmarkLoadBaseEnv(b *testing.B) { func BenchmarkLoadBaseEnvDifferentVersions(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - MustBaseEnvSet(version.MajorMinor(1, uint(i))) + MustBaseEnvSet(version.MajorMinor(1, uint(i)), true) } } diff --git a/staging/src/k8s.io/apiserver/pkg/cel/environment/environment_test.go b/staging/src/k8s.io/apiserver/pkg/cel/environment/environment_test.go index e5b29371aca..88e2c521d95 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/environment/environment_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/environment/environment_test.go @@ -262,7 +262,7 @@ func TestBaseEnvironment(t *testing.T) { for _, tv := range tc.typeVersionCombinations { t.Run(fmt.Sprintf("version=%s,envType=%s", tv.version.String(), tv.envType), func(t *testing.T) { - envSet := MustBaseEnvSet(tv.version) + envSet := MustBaseEnvSet(tv.version, true) if tc.opts != nil { var err error envSet, err = envSet.Extend(tc.opts...) diff --git a/staging/src/k8s.io/apiserver/pkg/cel/lazy/lazy_test.go b/staging/src/k8s.io/apiserver/pkg/cel/lazy/lazy_test.go index 19de8e99144..7650e03ba3f 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/lazy/lazy_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/lazy/lazy_test.go @@ -129,7 +129,7 @@ func compileAndRun(env *cel.Env, activation *testActivation, exp string) (ref.Va func buildTestEnv() (*cel.Env, *apiservercel.DeclType, error) { variablesType := apiservercel.NewMapType(apiservercel.StringType, apiservercel.AnyType, 0) variablesType.Fields = make(map[string]*apiservercel.DeclField) - envSet, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()).Extend( + envSet, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true).Extend( environment.VersionedOptions{ IntroducedVersion: version.MajorMinor(1, 28), EnvOptions: []cel.EnvOption{ diff --git a/staging/src/k8s.io/apiserver/pkg/cel/mutation/env_test.go b/staging/src/k8s.io/apiserver/pkg/cel/mutation/env_test.go index 48bd85c9248..911ae5db2b4 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/mutation/env_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/mutation/env_test.go @@ -28,7 +28,7 @@ import ( // mustCreateEnv creates the default env for testing, with given option. // it fatally fails the test if the env fails to set up. func mustCreateEnv(t testing.TB, envOptions ...cel.EnvOption) *cel.Env { - envSet, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()). + envSet, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true). Extend(environment.VersionedOptions{ IntroducedVersion: version.MajorMinor(1, 30), EnvOptions: envOptions, diff --git a/staging/src/k8s.io/apiserver/pkg/cel/mutation/unstructured/typeresolver_test.go b/staging/src/k8s.io/apiserver/pkg/cel/mutation/unstructured/typeresolver_test.go index 4c747761422..7ca743b27c3 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/mutation/unstructured/typeresolver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/mutation/unstructured/typeresolver_test.go @@ -140,7 +140,7 @@ func TestTypeProvider(t *testing.T) { } } func mustCreateEnv(t testing.TB, envOptions ...cel.EnvOption) *cel.Env { - envSet, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()). + envSet, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true). Extend(environment.VersionedOptions{ IntroducedVersion: version.MajorMinor(1, 30), EnvOptions: envOptions, diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/compiling_test.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/compiling_test.go index 1799c2b0da7..ca6db71a427 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/compiling_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/compiling_test.go @@ -106,7 +106,7 @@ func buildTestEnv() (*cel.Env, error) { fooType := common.SchemaDeclType(simpleMapSchema("foo", spec.StringProperty()), true).MaybeAssignTypeName("fooType") barType := common.SchemaDeclType(simpleMapSchema("bar", spec.Int64Property()), true).MaybeAssignTypeName("barType") - env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()).Extend( + env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true).Extend( environment.VersionedOptions{ IntroducedVersion: version.MajorMinor(1, 26), EnvOptions: []cel.EnvOption{ diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index dbd41b8c5f0..bae04d95458 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -220,6 +220,24 @@ const ( // if the generated name conflicts with an existing resource name, up to a maximum number of 7 retries. RetryGenerateName featuregate.Feature = "RetryGenerateName" + // owner: @cici37 + // alpha: v1.30 + // + // StrictCostEnforcementForVAP is used to apply strict CEL cost validation for ValidatingAdmissionPolicy. + // It will be set to off by default for certain time of period to prevent the impact on the existing users. + // It is strongly recommended to enable this feature gate as early as possible. + // The strict cost is specific for the extended libraries whose cost defined under k8s/apiserver/pkg/cel/library. + StrictCostEnforcementForVAP featuregate.Feature = "StrictCostEnforcementForVAP" + + // owner: @cici37 + // alpha: v1.30 + // + // StrictCostEnforcementForWebhooks is used to apply strict CEL cost validation for matchConditions in Webhooks. + // It will be set to off by default for certain time of period to prevent the impact on the existing users. + // It is strongly recommended to enable this feature gate as early as possible. + // The strict cost is specific for the extended libraries whose cost defined under k8s/apiserver/pkg/cel/library. + StrictCostEnforcementForWebhooks featuregate.Feature = "StrictCostEnforcementForWebhooks" + // owner: @caesarxuchao @roycaihw // alpha: v1.20 // @@ -347,6 +365,10 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS StorageVersionHash: {Default: true, PreRelease: featuregate.Beta}, + StrictCostEnforcementForVAP: {Default: false, PreRelease: featuregate.Beta}, + + StrictCostEnforcementForWebhooks: {Default: false, PreRelease: featuregate.Beta}, + StructuredAuthenticationConfiguration: {Default: true, PreRelease: featuregate.Beta}, StructuredAuthorizationConfiguration: {Default: true, PreRelease: featuregate.Beta}, diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/namedresources/cel/compile.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/namedresources/cel/compile.go index 755dcbff10b..d4beb0ce698 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/namedresources/cel/compile.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/namedresources/cel/compile.go @@ -184,7 +184,8 @@ func (c CompilationResult) Evaluate(ctx context.Context, attributes []resourceap } func mustBuildEnv() *environment.EnvSet { - envset := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()) + // strictCost is always true to enforce cost limits. + envset := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true) versioned := []environment.VersionedOptions{ { // Feature epoch was actually 1.30, but we artificially set it to 1.0 because these diff --git a/test/integration/apiserver/admissionwebhook/match_conditions_test.go b/test/integration/apiserver/admissionwebhook/match_conditions_test.go index 4ff64102342..fc5d0c7f9ad 100644 --- a/test/integration/apiserver/admissionwebhook/match_conditions_test.go +++ b/test/integration/apiserver/admissionwebhook/match_conditions_test.go @@ -25,6 +25,7 @@ import ( "net/http" "net/http/httptest" "strconv" + "strings" "sync" "testing" "time" @@ -36,7 +37,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" + featuregatetesting "k8s.io/component-base/featuregate/testing" apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" "k8s.io/kubernetes/test/integration/framework" ) @@ -107,7 +111,7 @@ func newMatchConditionHandler(recorder *admissionRecorder) http.Handler { // TestMatchConditions tests ValidatingWebhookConfigurations and MutatingWebhookConfigurations that validates different cases of matchCondition fields func TestMatchConditions(t *testing.T) { - + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.StrictCostEnforcementForWebhooks, false) fail := admissionregistrationv1.Fail ignore := admissionregistrationv1.Ignore @@ -118,6 +122,7 @@ func TestMatchConditions(t *testing.T) { matchedPods []*corev1.Pod expectErrorPod bool failPolicy *admissionregistrationv1.FailurePolicyType + errMessage string }{ { name: "pods in namespace kube-system is ignored", @@ -284,6 +289,35 @@ func TestMatchConditions(t *testing.T) { matchConditionsTestPod("test2", "default"), }, }, + { + name: "without strict cost enforcement: Authz check does not exceed per call limit", + matchConditions: []admissionregistrationv1.MatchCondition{ + { + Name: "test1", + Expression: "authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed()", + }, + }, + pods: []*corev1.Pod{ + matchConditionsTestPod("test1", "kube-system"), + }, + matchedPods: []*corev1.Pod{ + matchConditionsTestPod("test1", "kube-system"), + }, + failPolicy: &fail, + expectErrorPod: false, + }, + { + name: "without strict cost enforcement: Authz check does not exceed overall cost limit", + matchConditions: generateMatchConditionsWithAuthzCheck(8, "authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed()"), + pods: []*corev1.Pod{ + matchConditionsTestPod("test1", "kube-system"), + }, + matchedPods: []*corev1.Pod{ + matchConditionsTestPod("test1", "kube-system"), + }, + failPolicy: &fail, + expectErrorPod: false, + }, } roots := x509.NewCertPool() @@ -432,6 +466,8 @@ func TestMatchConditions(t *testing.T) { t.Fatalf("unexpected error creating test pod: %v", err) } else if testcase.expectErrorPod && err == nil { t.Fatal("expected error creating pods") + } else if testcase.expectErrorPod && err != nil && !strings.Contains(err.Error(), testcase.errMessage) { + t.Fatalf("expected error message includes: %v, but get %v", testcase.errMessage, err) } } @@ -448,8 +484,324 @@ func TestMatchConditions(t *testing.T) { } } - //Reset and rerun against mutating webhook configuration - //TODO: private helper function for validation after creating vwh or mwh + // Reset and rerun against mutating webhook configuration + // TODO: private helper function for validation after creating vwh or mwh + upCh = recorder.Reset() + err = client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(context.TODO(), validatingcfg.GetName(), metav1.DeleteOptions{}) + if err != nil { + t.Fatal(err) + } else { + vhwHasBeenCleanedUp = true + } + + mutatingwebhook := &admissionregistrationv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "admission.integration.test", + }, + Webhooks: []admissionregistrationv1.MutatingWebhook{ + { + Name: "admission.integration.test", + Rules: []admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"pods"}, + }, + }}, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + URL: &endpoint, + CABundle: localhostCert, + }, + // ignore pods in the marker namespace + NamespaceSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: corev1.LabelMetadataName, + Operator: metav1.LabelSelectorOpNotIn, + Values: []string{"marker"}, + }, + }}, + FailurePolicy: testcase.failPolicy, + SideEffects: &noSideEffects, + AdmissionReviewVersions: []string{"v1"}, + MatchConditions: testcase.matchConditions, + }, + { + Name: "admission.integration.test.marker", + Rules: []admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{APIGroups: []string{""}, APIVersions: []string{"v1"}, Resources: []string{"pods"}}, + }}, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + URL: &markerEndpoint, + CABundle: localhostCert, + }, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{ + corev1.LabelMetadataName: "marker", + }}, + ObjectSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"marker": "true"}}, + FailurePolicy: testcase.failPolicy, + SideEffects: &noSideEffects, + AdmissionReviewVersions: []string{"v1"}, + }, + }, + } + + mutatingcfg, err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().Create(context.TODO(), mutatingwebhook, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + defer func() { + err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(context.TODO(), mutatingcfg.GetName(), metav1.DeleteOptions{}) + if err != nil { + t.Fatal(err) + } + }() + + // wait until new webhook is called the first time + if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*5, wait.ForeverTestTimeout, true, func(_ context.Context) (bool, error) { + _, err = client.CoreV1().Pods(markerNs).Patch(context.TODO(), marker.Name, types.JSONPatchType, []byte("[]"), metav1.PatchOptions{}) + select { + case <-upCh: + return true, nil + default: + t.Logf("Waiting for webhook to become effective, getting marker object: %v", err) + return false, nil + } + }); err != nil { + t.Fatal(err) + } + + for _, pod := range testcase.pods { + _, err = client.CoreV1().Pods(pod.Namespace).Create(context.TODO(), pod, dryRunCreate) + if testcase.expectErrorPod == false && err != nil { + t.Fatalf("unexpected error creating test pod: %v", err) + } else if testcase.expectErrorPod == true && err == nil { + t.Fatal("expected error creating pods") + } + } + + if len(recorder.requests) != len(testcase.matchedPods) { + t.Errorf("unexpected requests %v, expected %v", recorder.requests, testcase.matchedPods) + } + + for i, request := range recorder.requests { + if request.Name != testcase.matchedPods[i].Name { + t.Errorf("unexpected pod name %v, expected %v", request.Name, testcase.matchedPods[i].Name) + } + if request.Namespace != testcase.matchedPods[i].Namespace { + t.Errorf("unexpected pod namespace %v, expected %v", request.Namespace, testcase.matchedPods[i].Namespace) + } + } + }) + } +} + +func TestMatchConditionsWithStrictCostEnforcement(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.StrictCostEnforcementForWebhooks, true) + + testcases := []struct { + name string + matchConditions []admissionregistrationv1.MatchCondition + pods []*corev1.Pod + matchedPods []*corev1.Pod + expectErrorPod bool + failPolicy *admissionregistrationv1.FailurePolicyType + errMessage string + }{ + { + name: "with strict cost enforcement: exceed per call limit should reject request with fail policy fail", + matchConditions: []admissionregistrationv1.MatchCondition{ + { + Name: "test1", + Expression: "authorizer.group('').resource('pods').namespace('default').check('create').allowed() && authorizer.group('').resource('pods').namespace('default').check('create').allowed() && authorizer.group('').resource('pods').namespace('default').check('create').allowed()", + }, + }, + pods: []*corev1.Pod{ + matchConditionsTestPod("test1", "default"), + }, + matchedPods: []*corev1.Pod{}, + expectErrorPod: true, + errMessage: "operation cancelled: actual cost limit exceeded", + }, + { + name: "with strict cost enforcement: exceed overall cost limit should reject request with fail policy fail", + matchConditions: generateMatchConditionsWithAuthzCheck(8, "authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed()"), + pods: []*corev1.Pod{ + matchConditionsTestPod("test1", "kube-system"), + }, + matchedPods: []*corev1.Pod{}, + expectErrorPod: true, + errMessage: "validation failed due to running out of cost budget, no further validation rules will be run", + }, + } + + roots := x509.NewCertPool() + if !roots.AppendCertsFromPEM(localhostCert) { + t.Fatal("Failed to append Cert from PEM") + } + cert, err := tls.X509KeyPair(localhostCert, localhostKey) + if err != nil { + t.Fatalf("Failed to build cert with error: %+v", err) + } + + recorder := &admissionRecorder{requests: []*admissionv1.AdmissionRequest{}} + + webhookServer := httptest.NewUnstartedServer(newMatchConditionHandler(recorder)) + webhookServer.TLS = &tls.Config{ + RootCAs: roots, + Certificates: []tls.Certificate{cert}, + } + webhookServer.StartTLS() + defer webhookServer.Close() + + dryRunCreate := metav1.CreateOptions{ + DryRun: []string{metav1.DryRunAll}, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + upCh := recorder.Reset() + server, err := apiservertesting.StartTestServer(t, nil, []string{ + "--disable-admission-plugins=ServiceAccount", + }, framework.SharedEtcd()) + if err != nil { + t.Fatal(err) + } + defer server.TearDownFn() + + config := server.ClientConfig + + client, err := clientset.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + + // Write markers to a separate namespace to avoid cross-talk + markerNs := "marker" + _, err = client.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: markerNs}}, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + // Create a marker object to use to check for the webhook configurations to be ready. + marker, err := client.CoreV1().Pods(markerNs).Create(context.TODO(), newMarkerPod(markerNs), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + endpoint := webhookServer.URL + markerEndpoint := webhookServer.URL + "/marker" + validatingwebhook := &admissionregistrationv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "admission.integration.test", + }, + Webhooks: []admissionregistrationv1.ValidatingWebhook{ + { + Name: "admission.integration.test", + Rules: []admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"pods"}, + }, + }}, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + URL: &endpoint, + CABundle: localhostCert, + }, + // ignore pods in the marker namespace + NamespaceSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: corev1.LabelMetadataName, + Operator: metav1.LabelSelectorOpNotIn, + Values: []string{"marker"}, + }, + }}, + FailurePolicy: testcase.failPolicy, + SideEffects: &noSideEffects, + AdmissionReviewVersions: []string{"v1"}, + MatchConditions: testcase.matchConditions, + }, + { + Name: "admission.integration.test.marker", + Rules: []admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{APIGroups: []string{""}, APIVersions: []string{"v1"}, Resources: []string{"pods"}}, + }}, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + URL: &markerEndpoint, + CABundle: localhostCert, + }, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{ + corev1.LabelMetadataName: "marker", + }}, + ObjectSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"marker": "true"}}, + FailurePolicy: testcase.failPolicy, + SideEffects: &noSideEffects, + AdmissionReviewVersions: []string{"v1"}, + }, + }, + } + + validatingcfg, err := client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Create(context.TODO(), validatingwebhook, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + vhwHasBeenCleanedUp := false + defer func() { + if !vhwHasBeenCleanedUp { + err := client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(context.TODO(), validatingcfg.GetName(), metav1.DeleteOptions{}) + if err != nil { + t.Fatal(err) + } + } + }() + + // wait until new webhook is called the first time + if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*5, wait.ForeverTestTimeout, true, func(_ context.Context) (bool, error) { + _, err = client.CoreV1().Pods(markerNs).Patch(context.TODO(), marker.Name, types.JSONPatchType, []byte("[]"), metav1.PatchOptions{}) + select { + case <-upCh: + return true, nil + default: + t.Logf("Waiting for webhook to become effective, getting marker object: %v", err) + return false, nil + } + }); err != nil { + t.Fatal(err) + } + + for _, pod := range testcase.pods { + _, err := client.CoreV1().Pods(pod.Namespace).Create(context.TODO(), pod, dryRunCreate) + if !testcase.expectErrorPod && err != nil { + t.Fatalf("unexpected error creating test pod: %v", err) + } else if testcase.expectErrorPod && err == nil { + t.Fatal("expected error creating pods") + } else if testcase.expectErrorPod && err != nil && !strings.Contains(err.Error(), testcase.errMessage) { + t.Fatalf("expected error message includes: %v, but get %v", testcase.errMessage, err) + } + } + + if len(recorder.requests) != len(testcase.matchedPods) { + t.Errorf("unexpected requests %v, expected %v", recorder.requests, testcase.matchedPods) + } + + for i, request := range recorder.requests { + if request.Name != testcase.matchedPods[i].Name { + t.Errorf("unexpected pod name %v, expected %v", request.Name, testcase.matchedPods[i].Name) + } + if request.Namespace != testcase.matchedPods[i].Namespace { + t.Errorf("unexpected pod namespace %v, expected %v", request.Namespace, testcase.matchedPods[i].Namespace) + } + } + + // Reset and rerun against mutating webhook configuration + // TODO: private helper function for validation after creating vwh or mwh upCh = recorder.Reset() err = client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(context.TODO(), validatingcfg.GetName(), metav1.DeleteOptions{}) if err != nil { @@ -777,3 +1129,13 @@ func repeatedMatchConditions(size int) []admissionregistrationv1.MatchCondition } return matchConditions } + +// generate n matchConditions with provided expression +func generateMatchConditionsWithAuthzCheck(num int, exp string) []admissionregistrationv1.MatchCondition { + var conditions = make([]admissionregistrationv1.MatchCondition, num) + for i := 0; i < num; i++ { + conditions[i].Name = "test" + strconv.Itoa(i) + conditions[i].Expression = exp + } + return conditions +} diff --git a/test/integration/apiserver/cel/typeresolution_test.go b/test/integration/apiserver/cel/typeresolution_test.go index b8685e4c856..be04edbdf6b 100644 --- a/test/integration/apiserver/cel/typeresolution_test.go +++ b/test/integration/apiserver/cel/typeresolution_test.go @@ -376,7 +376,7 @@ func TestBuiltinResolution(t *testing.T) { // with the practical defaults. // `self` is defined as the object being evaluated against. func simpleCompileCEL(schema *spec.Schema, expression string) (cel.Program, error) { - env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()).Env(environment.NewExpressions) + env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true).Env(environment.NewExpressions) if err != nil { return nil, err } diff --git a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go index 7388fd7969b..c27cc1754fe 100644 --- a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go +++ b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go @@ -2131,6 +2131,217 @@ func Test_ValidatingAdmissionPolicy_ParamResourceDeletedThenRecreated(t *testing } } +// Test_CostLimitForValidation tests the cost limit set for a ValidatingAdmissionPolicy. +func Test_CostLimitForValidation(t *testing.T) { + testcases := []struct { + name string + policy *admissionregistrationv1.ValidatingAdmissionPolicy + policyBinding *admissionregistrationv1.ValidatingAdmissionPolicyBinding + namespace *v1.Namespace + err string + failureReason metav1.StatusReason + strictCostEnforcement bool + }{ + { + name: "With StrictCostEnforcementForVAP: Single expression exceeds per call cost limit for native library", + policy: withValidations([]admissionregistrationv1.Validation{ + { + Expression: "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(x, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(y, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z2, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z3, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z4, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z5, int('1'.find('[0-9]*')) < 100)))))))", + }, + }, withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix")))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + err: "operation cancelled: actual cost limit exceeded", + failureReason: metav1.StatusReasonInvalid, + strictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP: Expression exceeds per call cost limit for extended library", + policy: withValidations([]admissionregistrationv1.Validation{ + { + + Expression: "authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed()", + }, + }, withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix")))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + err: "operation cancelled: actual cost limit exceeded", + failureReason: metav1.StatusReasonInvalid, + strictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP: Expression exceeds per call cost limit for extended library in variables", + policy: withVariables([]admissionregistrationv1.Variable{ + { + Name: "authzCheck", + Expression: "authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed()", + }, + }, withValidations([]admissionregistrationv1.Validation{ + { + Expression: "variables.authzCheck", + }, + }, withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix"))))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + err: "operation cancelled: actual cost limit exceeded", + failureReason: metav1.StatusReasonInvalid, + strictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP: Expression exceeds per call cost limit for extended library in matchConditions", + policy: withMatchConditions([]admissionregistrationv1.MatchCondition{ + { + Name: "test", + Expression: "authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed()", + }, + }, withValidations([]admissionregistrationv1.Validation{ + { + Expression: "true", + }, + }, withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix"))))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + err: "operation cancelled: actual cost limit exceeded", + failureReason: metav1.StatusReasonInvalid, + strictCostEnforcement: true, + }, + { + name: "With StrictCostEnforcementForVAP: Expression exceeds per policy cost limit for extended library", + policy: withValidations(generateValidationsWithAuthzCheck(29, "authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed()"), withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix")))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + err: "validation failed due to running out of cost budget, no further validation rules will be run", + failureReason: metav1.StatusReasonInvalid, + strictCostEnforcement: true, + }, + { + name: "Without StrictCostEnforcementForVAP: Single expression exceeds per call cost limit for native library", + policy: withValidations([]admissionregistrationv1.Validation{ + { + Expression: "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(x, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(y, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z2, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z3, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z4, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].all(z5, int('1'.find('[0-9]*')) < 100)))))))", + }, + }, withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix")))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + err: "operation cancelled: actual cost limit exceeded", + failureReason: metav1.StatusReasonInvalid, + strictCostEnforcement: false, + }, + { + name: "Without StrictCostEnforcementForVAP: Expression does not exceed per call cost limit for extended library", + policy: withValidations([]admissionregistrationv1.Validation{ + { + + Expression: "authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed()", + }, + }, withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix")))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + strictCostEnforcement: false, + }, + { + name: "Without StrictCostEnforcementForVAP: Expression does not exceed per call cost limit for extended library in variables", + policy: withVariables([]admissionregistrationv1.Variable{ + { + Name: "authzCheck", + Expression: "authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed() && authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed()", + }, + }, withValidations([]admissionregistrationv1.Validation{ + { + Expression: "variables.authzCheck", + }, + }, withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix"))))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + strictCostEnforcement: false, + }, + { + name: "Without StrictCostEnforcementForVAP: Expression does not exceed per policy cost limit for extended library", + policy: withValidations(generateValidationsWithAuthzCheck(29, "authorizer.group('apps').resource('deployments').subresource('status').namespace('test').name('backend').check('create').allowed()"), withFailurePolicy(admissionregistrationv1.Fail, withNamespaceMatch(makePolicy("validate-namespace-suffix")))), + policyBinding: makeBinding("validate-namespace-suffix-binding", "validate-namespace-suffix", ""), + namespace: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-k8s", + }, + }, + strictCostEnforcement: false, + }, + } + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.StrictCostEnforcementForVAP, testcase.strictCostEnforcement) + + server, err := apiservertesting.StartTestServer(t, nil, []string{ + "--enable-admission-plugins", "ValidatingAdmissionPolicy", + }, framework.SharedEtcd()) + if err != nil { + t.Fatal(err) + } + defer server.TearDownFn() + + config := server.ClientConfig + + client, err := clientset.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + policy := withWaitReadyConstraintAndExpression(testcase.policy) + if _, err := client.AdmissionregistrationV1().ValidatingAdmissionPolicies().Create(context.TODO(), policy, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + if err := createAndWaitReady(t, client, testcase.policyBinding, nil); err != nil { + t.Fatal(err) + } + + _, err = client.CoreV1().Namespaces().Create(context.TODO(), testcase.namespace, metav1.CreateOptions{}) + + checkExpectedError(t, err, testcase.err) + checkFailureReason(t, err, testcase.failureReason) + }) + } +} + +// generate n validation rules with provided expression +func generateValidationsWithAuthzCheck(num int, exp string) []admissionregistrationv1.Validation { + var validations = make([]admissionregistrationv1.Validation, num) + for i := 0; i < num; i++ { + validations[i].Expression = exp + } + return validations +} + // TestCRDParams tests that a CustomResource can be used as a param resource for a ValidatingAdmissionPolicy. func TestCRDParams(t *testing.T) { testcases := []struct { @@ -2722,6 +2933,12 @@ func createAndWaitReadyNamespacedWithWarnHandler(t *testing.T, client clientset. } else if err != nil && strings.Contains(err.Error(), "not yet synced to use for admission") { t.Logf("waiting for policy to be ready. Marker: %v. Admission not synced yet: %v", marker, err) return false, nil + } else if err != nil && strings.Contains(err.Error(), "validation failed due to running out of cost budget, no further validation rules will be run") { + t.Logf("policy is ready and exceeds the budget: %v", err) + return true, nil + } else if err != nil && strings.Contains(err.Error(), "operation cancelled: actual cost limit exceeded") { + t.Logf("policy is ready and exceeds the budget: %v", err) + return true, nil } else { t.Logf("waiting for policy to be ready. Marker: %v, Last marker patch response: %v", marker, err) return false, err @@ -2856,6 +3073,16 @@ func withValidations(validations []admissionregistrationv1.Validation, policy *a return policy } +func withVariables(variables []admissionregistrationv1.Variable, policy *admissionregistrationv1.ValidatingAdmissionPolicy) *admissionregistrationv1.ValidatingAdmissionPolicy { + policy.Spec.Variables = variables + return policy +} + +func withMatchConditions(matchConditions []admissionregistrationv1.MatchCondition, policy *admissionregistrationv1.ValidatingAdmissionPolicy) *admissionregistrationv1.ValidatingAdmissionPolicy { + policy.Spec.MatchConditions = matchConditions + return policy +} + func withAuditAnnotations(auditAnnotations []admissionregistrationv1.AuditAnnotation, policy *admissionregistrationv1.ValidatingAdmissionPolicy) *admissionregistrationv1.ValidatingAdmissionPolicy { policy.Spec.AuditAnnotations = auditAnnotations return policy @@ -2995,7 +3222,7 @@ func checkExpectedError(t *testing.T, err error, expectedErr string) { t.Fatal("got error but expected none") } - if err.Error() != expectedErr { + if !strings.Contains(err.Error(), expectedErr) { t.Logf("actual validation error: %v", err) t.Logf("expected validation error: %v", expectedErr) t.Error("unexpected validation error")