From d8be7aa9ca99070e42cdef37b8c4af07b754520e Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Wed, 8 Mar 2023 17:36:11 -0800 Subject: [PATCH] implement message expression. --- .../validation/validation.go | 52 ++-- .../validation/validation_test.go | 33 ++- .../pkg/admission/plugin/cel/filter.go | 50 +++- .../pkg/admission/plugin/cel/filter_test.go | 59 +++- .../pkg/admission/plugin/cel/interface.go | 5 +- .../admission_test.go | 13 +- .../controller_reconcile.go | 17 +- .../validatingadmissionpolicy/message.go | 36 +++ .../validatingadmissionpolicy/validator.go | 66 ++++- .../validator_test.go | 280 +++++++++++++++++- 10 files changed, 545 insertions(+), 66 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/message.go diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index c53b4b12667..fe9f60fa988 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -782,26 +782,24 @@ func validateValidation(v *admissionregistration.Validation, paramKind *admissio var allErrors field.ErrorList trimmedExpression := strings.TrimSpace(v.Expression) trimmedMsg := strings.TrimSpace(v.Message) + trimmedMessageExpression := strings.TrimSpace(v.MessageExpression) if len(trimmedExpression) == 0 { allErrors = append(allErrors, field.Required(fldPath.Child("expression"), "expression is not specified")) } else { - result := plugincel.CompileCELExpression(&validatingadmissionpolicy.ValidationCondition{ - Expression: trimmedExpression, - Message: v.Message, - Reason: v.Reason, - }, plugincel.OptionalVariableDeclarations{HasParams: paramKind != nil, HasAuthorizer: true}, celconfig.PerCallLimit) - if result.Error != nil { - switch result.Error.Type { - case cel.ErrorTypeRequired: - allErrors = append(allErrors, field.Required(fldPath.Child("expression"), result.Error.Detail)) - case cel.ErrorTypeInvalid: - allErrors = append(allErrors, field.Invalid(fldPath.Child("expression"), v.Expression, result.Error.Detail)) - case cel.ErrorTypeInternal: - allErrors = append(allErrors, field.InternalError(fldPath.Child("expression"), result.Error)) - default: - allErrors = append(allErrors, field.InternalError(fldPath.Child("expression"), fmt.Errorf("unsupported error type: %w", result.Error))) - } - } + allErrors = append(allErrors, validateCELExpression(v.Expression, plugincel.OptionalVariableDeclarations{ + HasParams: paramKind != nil, + HasAuthorizer: true, + }, fldPath.Child("expression"))...) + } + if len(v.MessageExpression) > 0 && len(trimmedMessageExpression) == 0 { + allErrors = append(allErrors, field.Invalid(fldPath.Child("messageExpression"), v.MessageExpression, "must be non-empty if specified")) + } else if len(trimmedMessageExpression) != 0 { + // use v.MessageExpression instead of trimmedMessageExpression so that + // the compiler output shows the correct column. + allErrors = append(allErrors, validateCELExpression(v.MessageExpression, plugincel.OptionalVariableDeclarations{ + HasParams: paramKind != nil, + HasAuthorizer: false, + }, fldPath.Child("messageExpression"))...) } if len(v.Message) > 0 && len(trimmedMsg) == 0 { allErrors = append(allErrors, field.Invalid(fldPath.Child("message"), v.Message, "message must be non-empty if specified")) @@ -816,6 +814,26 @@ func validateValidation(v *admissionregistration.Validation, paramKind *admissio return allErrors } +func validateCELExpression(expression string, variables plugincel.OptionalVariableDeclarations, fldPath *field.Path) field.ErrorList { + var allErrors field.ErrorList + result := plugincel.CompileCELExpression(&validatingadmissionpolicy.ValidationCondition{ + Expression: expression, + }, variables, celconfig.PerCallLimit) + if result.Error != nil { + switch result.Error.Type { + case cel.ErrorTypeRequired: + allErrors = append(allErrors, field.Required(fldPath, result.Error.Detail)) + case cel.ErrorTypeInvalid: + allErrors = append(allErrors, field.Invalid(fldPath, expression, result.Error.Detail)) + case cel.ErrorTypeInternal: + allErrors = append(allErrors, field.InternalError(fldPath, result.Error)) + default: + allErrors = append(allErrors, field.InternalError(fldPath, fmt.Errorf("unsupported error type: %w", result.Error))) + } + } + return allErrors +} + func validateAuditAnnotation(meta metav1.ObjectMeta, v *admissionregistration.AuditAnnotation, paramKind *admissionregistration.ParamKind, fldPath *field.Path) field.ErrorList { var allErrors field.ErrorList if len(meta.GetName()) != 0 { diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index b442cd82239..575d7502064 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -2564,7 +2564,38 @@ func TestValidateValidatingAdmissionPolicy(t *testing.T) { }, }, }, - expectedError: `spec.validations[0].expression: Invalid value: "object.x in [1, 2, ": compilation failed: ERROR: :1:19: Syntax error: missing ']' at '`, + expectedError: `spec.validations[0].expression: Invalid value: "object.x in [1, 2, ": compilation failed: ERROR: :1:20: Syntax error: missing ']' at '`, + }, + { + name: "invalid messageExpression", + config: &admissionregistration.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config", + }, + Spec: admissionregistration.ValidatingAdmissionPolicySpec{ + Validations: []admissionregistration.Validation{ + { + Expression: "true", + MessageExpression: "object.x in [1, 2, ", + }, + }, + MatchConstraints: &admissionregistration.MatchResources{ + ResourceRules: []admissionregistration.NamedRuleWithOperations{ + { + RuleWithOperations: admissionregistration.RuleWithOperations{ + Operations: []admissionregistration.OperationType{"CREATE"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*/*"}, + }, + }, + }, + }, + }, + }, + }, + expectedError: `spec.validations[0].messageExpression: Invalid value: "object.x in [1, 2, ": compilation failed: ERROR: :1:20: Syntax error: missing ']' at '`, }, { name: "invalid auditAnnotations key due to key name", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter.go index cb9d4702e73..6e504897c5a 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter.go @@ -18,7 +18,6 @@ package cel import ( "context" - "errors" "fmt" "math" "reflect" @@ -32,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/cel/library" ) @@ -78,6 +78,9 @@ func (a *evaluationActivation) Parent() interpreter.Activation { func (c *filterCompiler) Compile(expressionAccessors []ExpressionAccessor, options OptionalVariableDeclarations, perCallLimit uint64) Filter { compilationResults := make([]CompilationResult, len(expressionAccessors)) for i, expressionAccessor := range expressionAccessors { + if expressionAccessor == nil { + continue + } compilationResults[i] = CompileCELExpression(expressionAccessor, options, perCallLimit) } return NewFilter(compilationResults) @@ -119,24 +122,24 @@ func objectToResolveVal(r runtime.Object) (interface{}, error) { // ForInput evaluates the compiled CEL expressions converting them into CELEvaluations // errors per evaluation are returned on the Evaluation object // runtimeCELCostBudget was added for testing purpose only. Callers should always use const RuntimeCELCostBudget from k8s.io/apiserver/pkg/apis/cel/config.go as input. -func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.VersionedAttributes, request *admissionv1.AdmissionRequest, inputs OptionalVariableBindings, runtimeCELCostBudget int64) ([]EvaluationResult, error) { +func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.VersionedAttributes, request *admissionv1.AdmissionRequest, inputs OptionalVariableBindings, runtimeCELCostBudget int64) ([]EvaluationResult, int64, error) { // TODO: replace unstructured with ref.Val for CEL variables when native type support is available evaluations := make([]EvaluationResult, len(f.compilationResults)) var err error oldObjectVal, err := objectToResolveVal(versionedAttr.VersionedOldObject) if err != nil { - return nil, err + return nil, -1, err } objectVal, err := objectToResolveVal(versionedAttr.VersionedObject) if err != nil { - return nil, err + return nil, -1, err } var paramsVal, authorizerVal, requestResourceAuthorizerVal any if inputs.VersionedParams != nil { paramsVal, err = objectToResolveVal(inputs.VersionedParams) if err != nil { - return nil, err + return nil, -1, err } } @@ -147,7 +150,7 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione requestVal, err := convertObjectToUnstructured(request) if err != nil { - return nil, err + return nil, -1, err } va := &evaluationActivation{ object: objectVal, @@ -161,13 +164,22 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione remainingBudget := runtimeCELCostBudget for i, compilationResult := range f.compilationResults { var evaluation = &evaluations[i] + if compilationResult.ExpressionAccessor == nil { // in case of placeholder + continue + } evaluation.ExpressionAccessor = compilationResult.ExpressionAccessor if compilationResult.Error != nil { - evaluation.Error = errors.New(fmt.Sprintf("compilation error: %v", compilationResult.Error)) + evaluation.Error = &cel.Error{ + Type: cel.ErrorTypeInvalid, + Detail: fmt.Sprintf("compilation error: %v", compilationResult.Error), + } continue } if compilationResult.Program == nil { - evaluation.Error = errors.New("unexpected internal error compiling expression") + evaluation.Error = &cel.Error{ + Type: cel.ErrorTypeInternal, + Detail: fmt.Sprintf("unexpected internal error compiling expression"), + } continue } t1 := time.Now() @@ -175,26 +187,38 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione elapsed := time.Since(t1) evaluation.Elapsed = elapsed if evalDetails == nil { - return nil, errors.New(fmt.Sprintf("runtime cost could not be calculated for expression: %v, no further expression will be run", compilationResult.ExpressionAccessor.GetExpression())) + return nil, -1, &cel.Error{ + Type: cel.ErrorTypeInternal, + Detail: fmt.Sprintf("runtime cost could not be calculated for expression: %v, no further expression will be run", compilationResult.ExpressionAccessor.GetExpression()), + } } else { rtCost := evalDetails.ActualCost() if rtCost == nil { - return nil, errors.New(fmt.Sprintf("runtime cost could not be calculated for expression: %v, no further expression will be run", compilationResult.ExpressionAccessor.GetExpression())) + return nil, -1, &cel.Error{ + Type: cel.ErrorTypeInvalid, + Detail: fmt.Sprintf("runtime cost could not be calculated for expression: %v, no further expression will be run", compilationResult.ExpressionAccessor.GetExpression()), + } } else { if *rtCost > math.MaxInt64 || int64(*rtCost) > remainingBudget { - return nil, errors.New(fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run")) + return nil, -1, &cel.Error{ + Type: cel.ErrorTypeInvalid, + Detail: fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run"), + } } remainingBudget -= int64(*rtCost) } } if err != nil { - evaluation.Error = errors.New(fmt.Sprintf("expression '%v' resulted in error: %v", compilationResult.ExpressionAccessor.GetExpression(), err)) + evaluation.Error = &cel.Error{ + Type: cel.ErrorTypeInvalid, + Detail: fmt.Sprintf("expression '%v' resulted in error: %v", compilationResult.ExpressionAccessor.GetExpression(), err), + } } else { evaluation.EvalResult = evalResult } } - return evaluations, nil + return evaluations, remainingBudget, nil } // TODO: to reuse https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go#L154 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 07627a576af..f7ddc0b04bc 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 @@ -38,6 +38,7 @@ import ( "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" apiservercel "k8s.io/apiserver/pkg/cel" + "k8s.io/utils/pointer" ) type condition struct { @@ -661,7 +662,7 @@ func TestFilter(t *testing.T) { optionalVars := OptionalVariableBindings{VersionedParams: tc.params, Authorizer: tc.authorizer} ctx := context.TODO() - evalResults, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, celconfig.RuntimeCELCostBudget) + evalResults, _, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, celconfig.RuntimeCELCostBudget) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -697,6 +698,7 @@ func TestRuntimeCELCostBudget(t *testing.T) { authorizer authorizer.Authorizer testRuntimeCELCostBudget int64 exceedBudget bool + expectRemainingBudget *int64 }{ { name: "expression exceed RuntimeCELCostBudget at fist expression", @@ -739,10 +741,49 @@ func TestRuntimeCELCostBudget(t *testing.T) { Expression: "object.subsets.size() > 2", }, }, - attributes: newValidAttribute(nil, false), - hasParamKind: true, - params: configMapParams, - exceedBudget: false, + attributes: newValidAttribute(nil, false), + hasParamKind: true, + params: configMapParams, + exceedBudget: false, + testRuntimeCELCostBudget: 10, + expectRemainingBudget: pointer.Int64(4), // 10 - 6 + }, + { + name: "test RuntimeCELCostBudge exactly covers", + validations: []ExpressionAccessor{ + &condition{ + Expression: "oldObject != null", + }, + &condition{ + Expression: "object.subsets.size() > 2", + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: true, + params: configMapParams, + exceedBudget: false, + testRuntimeCELCostBudget: 6, + expectRemainingBudget: pointer.Int64(0), + }, + { + name: "test RuntimeCELCostBudge exactly covers then constant", + validations: []ExpressionAccessor{ + &condition{ + Expression: "oldObject != null", + }, + &condition{ + Expression: "object.subsets.size() > 2", + }, + &condition{ + Expression: "true", // zero cost + }, + }, + attributes: newValidAttribute(nil, false), + hasParamKind: true, + params: configMapParams, + exceedBudget: false, + testRuntimeCELCostBudget: 6, + expectRemainingBudget: pointer.Int64(0), }, } @@ -767,19 +808,25 @@ func TestRuntimeCELCostBudget(t *testing.T) { } optionalVars := OptionalVariableBindings{VersionedParams: tc.params, Authorizer: tc.authorizer} ctx := context.TODO() - evalResults, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, tc.testRuntimeCELCostBudget) + evalResults, remaining, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, tc.testRuntimeCELCostBudget) 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") { t.Errorf("Expected RuntimeCELCostBudge exceeded error but got: %v", err) } + if err != nil && remaining != -1 { + t.Errorf("expected -1 remaining when error, but got %d", remaining) + } if err != nil && !tc.exceedBudget { t.Fatalf("unexpected error: %v", err) } if tc.exceedBudget && len(evalResults) != 0 { t.Fatalf("unexpected result returned: %v", evalResults) } + if tc.expectRemainingBudget != nil && *tc.expectRemainingBudget != remaining { + t.Errorf("wrong remaining budget, expect %d, but got %d", *tc.expectRemainingBudget, remaining) + } }) } } 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 0b11f247fe4..26535536942 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 @@ -92,9 +92,10 @@ type OptionalVariableBindings struct { // by the underlying CEL code (which is indicated by the match criteria of a policy definition). // versionedParams may be nil. type Filter interface { - // ForInput converts compiled CEL-typed values into evaluated CEL-typed values + // ForInput converts compiled CEL-typed values into evaluated CEL-typed value. // runtimeCELCostBudget was added for testing purpose only. Callers should always use const RuntimeCELCostBudget from k8s.io/apiserver/pkg/apis/cel/config.go as input. - ForInput(ctx context.Context, versionedAttr *admission.VersionedAttributes, request *v1.AdmissionRequest, optionalVars OptionalVariableBindings, runtimeCELCostBudget int64) ([]EvaluationResult, error) + // If cost budget is calculated, the filter should return the remaining budget. + ForInput(ctx context.Context, versionedAttr *admission.VersionedAttributes, request *v1.AdmissionRequest, optionalVars OptionalVariableBindings, runtimeCELCostBudget int64) ([]EvaluationResult, int64, error) // CompilationErrors returns a list of errors from the compilation of the evaluator CompilationErrors() []error diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go index e5f3257c225..6b364436fb9 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go @@ -212,7 +212,7 @@ func (f *fakeCompiler) Compile( options cel.OptionalVariableDeclarations, perCallLimit uint64, ) cel.Filter { - if len(expressions) > 0 { + if len(expressions) > 0 && expressions[0] != nil { key := expressions[0].GetExpression() if fun, ok := f.CompileFuncs[key]; ok { return fun(expressions, options) @@ -252,8 +252,8 @@ type fakeFilter struct { keyId string } -func (f *fakeFilter) ForInput(ctx context.Context, versionedAttr *admission.VersionedAttributes, request *admissionv1.AdmissionRequest, inputs cel.OptionalVariableBindings, runtimeCELCostBudget int64) ([]cel.EvaluationResult, error) { - return []cel.EvaluationResult{}, nil +func (f *fakeFilter) ForInput(ctx context.Context, versionedAttr *admission.VersionedAttributes, request *admissionv1.AdmissionRequest, inputs cel.OptionalVariableBindings, runtimeCELCostBudget int64) ([]cel.EvaluationResult, int64, error) { + return []cel.EvaluationResult{}, 0, nil } func (f *fakeFilter) CompilationErrors() []error { @@ -263,8 +263,8 @@ func (f *fakeFilter) CompilationErrors() []error { var _ Validator = &fakeValidator{} type fakeValidator struct { - validationFilter, auditAnnotationFilter *fakeFilter - ValidateFunc func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult + validationFilter, auditAnnotationFilter, messageFilter *fakeFilter + ValidateFunc func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult } func (f *fakeValidator) RegisterDefinition(definition *v1alpha1.ValidatingAdmissionPolicy, validateFunc func(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult) { @@ -418,10 +418,11 @@ func setupTestCommon(t *testing.T, compiler cel.FilterCompiler, matcher Matcher, // Override compiler used by controller for tests controller = handler.evaluator.(*celAdmissionController) controller.policyController.filterCompiler = compiler - controller.policyController.newValidator = func(validationFilter, auditAnnotationFilter cel.Filter, fail *admissionRegistrationv1.FailurePolicyType, authorizer authorizer.Authorizer) Validator { + controller.policyController.newValidator = func(validationFilter, auditAnnotationFilter, messageFilter cel.Filter, fail *admissionRegistrationv1.FailurePolicyType, authorizer authorizer.Authorizer) Validator { f := validationFilter.(*fakeFilter) v := validatorMap[f.keyId] v.validationFilter = f + v.messageFilter = f v.auditAnnotationFilter = auditAnnotationFilter.(*fakeFilter) return v } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go index 7f9c0a50ff0..62ab0c060d5 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go @@ -92,7 +92,7 @@ type policyController struct { authz authorizer.Authorizer } -type newValidator func(validationFilter cel.Filter, auditAnnotationFilter cel.Filter, failurePolicy *v1.FailurePolicyType, authorizer authorizer.Authorizer) Validator +type newValidator func(validationFilter cel.Filter, auditAnnotationFilter cel.Filter, messageFilter cel.Filter, failurePolicy *v1.FailurePolicyType, authorizer authorizer.Authorizer) Validator func newPolicyController( restMapper meta.RESTMapper, @@ -459,9 +459,11 @@ func (c *policyController) latestPolicyData() []policyData { hasParam = true } optionalVars := cel.OptionalVariableDeclarations{HasParams: hasParam, HasAuthorizer: true} + expressionOptionalVars := cel.OptionalVariableDeclarations{HasParams: hasParam, HasAuthorizer: false} bindingInfo.validator = c.newValidator( c.filterCompiler.Compile(convertv1alpha1Validations(definitionInfo.lastReconciledValue.Spec.Validations), optionalVars, celconfig.PerCallLimit), c.filterCompiler.Compile(convertv1alpha1AuditAnnotations(definitionInfo.lastReconciledValue.Spec.AuditAnnotations), optionalVars, celconfig.PerCallLimit), + c.filterCompiler.Compile(convertV1Alpha1MessageExpressions(definitionInfo.lastReconciledValue.Spec.Validations), expressionOptionalVars, celconfig.PerCallLimit), convertv1alpha1FailurePolicyTypeTov1FailurePolicyType(definitionInfo.lastReconciledValue.Spec.FailurePolicy), c.authz, ) @@ -514,6 +516,19 @@ func convertv1alpha1Validations(inputValidations []v1alpha1.Validation) []cel.Ex return celExpressionAccessor } +func convertV1Alpha1MessageExpressions(inputValidations []v1alpha1.Validation) []cel.ExpressionAccessor { + celExpressionAccessor := make([]cel.ExpressionAccessor, len(inputValidations)) + for i, validation := range inputValidations { + if validation.MessageExpression != "" { + condition := MessageExpressionCondition{ + MessageExpression: validation.MessageExpression, + } + celExpressionAccessor[i] = &condition + } + } + return celExpressionAccessor +} + func convertv1alpha1AuditAnnotations(inputValidations []v1alpha1.AuditAnnotation) []cel.ExpressionAccessor { celExpressionAccessor := make([]cel.ExpressionAccessor, len(inputValidations)) for i, validation := range inputValidations { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/message.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/message.go new file mode 100644 index 00000000000..772891e3c8b --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/message.go @@ -0,0 +1,36 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validatingadmissionpolicy + +import ( + celgo "github.com/google/cel-go/cel" + "k8s.io/apiserver/pkg/admission/plugin/cel" +) + +var _ cel.ExpressionAccessor = (*MessageExpressionCondition)(nil) + +type MessageExpressionCondition struct { + MessageExpression string +} + +func (m *MessageExpressionCondition) GetExpression() string { + return m.MessageExpression +} + +func (m *MessageExpressionCondition) ReturnTypes() []*celgo.Type { + return []*celgo.Type{celgo.StringType} +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator.go index 02fe24333a0..fcb901ed6e1 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator.go @@ -30,21 +30,25 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/plugin/cel" + celconfig "k8s.io/apiserver/pkg/apis/cel" "k8s.io/apiserver/pkg/authorization/authorizer" + apiservercel "k8s.io/apiserver/pkg/cel" ) // validator implements the Validator interface type validator struct { validationFilter cel.Filter auditAnnotationFilter cel.Filter + messageFilter cel.Filter failPolicy *v1.FailurePolicyType authorizer authorizer.Authorizer } -func NewValidator(validationFilter, auditAnnotationFilter cel.Filter, failPolicy *v1.FailurePolicyType, authorizer authorizer.Authorizer) Validator { +func NewValidator(validationFilter, auditAnnotationFilter, messageFilter cel.Filter, failPolicy *v1.FailurePolicyType, authorizer authorizer.Authorizer) Validator { return &validator{ validationFilter: validationFilter, auditAnnotationFilter: auditAnnotationFilter, + messageFilter: messageFilter, failPolicy: failPolicy, authorizer: authorizer, } @@ -75,7 +79,9 @@ func (v *validator) Validate(ctx context.Context, versionedAttr *admission.Versi } optionalVars := cel.OptionalVariableBindings{VersionedParams: versionedParams, Authorizer: v.authorizer} - evalResults, err := v.validationFilter.ForInput(ctx, versionedAttr, cel.CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, runtimeCELCostBudget) + expressionOptionalVars := cel.OptionalVariableBindings{VersionedParams: versionedParams} + admissionRequest := cel.CreateAdmissionRequest(versionedAttr.Attributes) + evalResults, remainingBudget, err := v.validationFilter.ForInput(ctx, versionedAttr, admissionRequest, optionalVars, runtimeCELCostBudget) if err != nil { return ValidateResult{ Decisions: []PolicyDecision{ @@ -88,7 +94,7 @@ func (v *validator) Validate(ctx context.Context, versionedAttr *admission.Versi } } decisions := make([]PolicyDecision, len(evalResults)) - + messageResults, _, err := v.messageFilter.ForInput(ctx, versionedAttr, admissionRequest, expressionOptionalVars, remainingBudget) for i, evalResult := range evalResults { var decision = &decisions[i] // TODO: move this to generics @@ -101,10 +107,23 @@ func (v *validator) Validate(ctx context.Context, versionedAttr *admission.Versi continue } + var messageResult *cel.EvaluationResult + var messageError *apiservercel.Error + if len(messageResults) > i { + messageResult = &messageResults[i] + } + messageError, _ = err.(*apiservercel.Error) if evalResult.Error != nil { decision.Action = policyDecisionActionForError(f) decision.Evaluation = EvalError decision.Message = evalResult.Error.Error() + } else if messageError != nil && + (messageError.Type == apiservercel.ErrorTypeInternal || + (messageError.Type == apiservercel.ErrorTypeInvalid && + strings.HasPrefix(messageError.Detail, "validation failed due to running out of cost budget"))) { + decision.Action = policyDecisionActionForError(f) + decision.Evaluation = EvalError + decision.Message = fmt.Sprintf("failed messageExpression: %s", err) } else if evalResult.EvalResult != celtypes.True { decision.Action = ActionDeny if validation.Reason == nil { @@ -112,11 +131,39 @@ func (v *validator) Validate(ctx context.Context, versionedAttr *admission.Versi } else { decision.Reason = *validation.Reason } - if len(validation.Message) > 0 { - decision.Message = strings.TrimSpace(validation.Message) - } else { - decision.Message = fmt.Sprintf("failed expression: %v", strings.TrimSpace(validation.Expression)) + // decide the failure message + var message string + // attempt to set message with messageExpression result + if messageResult != nil && messageResult.Error == nil && messageResult.EvalResult != nil { + // also fallback if the eval result is non-string (including null) or + // whitespaces. + if message, ok = messageResult.EvalResult.Value().(string); ok { + message = strings.TrimSpace(message) + // deny excessively long message from EvalResult + if len(message) > celconfig.MaxEvaluatedMessageExpressionSizeBytes { + klog.V(2).InfoS("excessively long message denied", "message", message) + message = "" + } + // deny message that contains newlines + if strings.ContainsAny(message, "\n") { + klog.V(2).InfoS("multi-line message denied", "message", message) + message = "" + } + } } + if messageResult != nil && messageResult.Error != nil { + // log any error with messageExpression + klog.V(2).ErrorS(messageResult.Error, "error while evaluating messageExpression") + } + // fallback to set message to the custom message + if message == "" && len(validation.Message) > 0 { + message = strings.TrimSpace(validation.Message) + } + // fallback to use the expression to compose a message + if message == "" { + message = fmt.Sprintf("failed expression: %v", strings.TrimSpace(validation.Expression)) + } + decision.Message = message } else { decision.Action = ActionAdmit decision.Evaluation = EvalAdmit @@ -124,7 +171,7 @@ func (v *validator) Validate(ctx context.Context, versionedAttr *admission.Versi } options := cel.OptionalVariableBindings{VersionedParams: versionedParams} - auditAnnotationEvalResults, err := v.auditAnnotationFilter.ForInput(ctx, versionedAttr, cel.CreateAdmissionRequest(versionedAttr.Attributes), options, runtimeCELCostBudget) + auditAnnotationEvalResults, _, err := v.auditAnnotationFilter.ForInput(ctx, versionedAttr, cel.CreateAdmissionRequest(versionedAttr.Attributes), options, runtimeCELCostBudget) if err != nil { return ValidateResult{ Decisions: []PolicyDecision{ @@ -139,6 +186,9 @@ func (v *validator) Validate(ctx context.Context, versionedAttr *admission.Versi auditAnnotationResults := make([]PolicyAuditAnnotation, len(auditAnnotationEvalResults)) for i, evalResult := range auditAnnotationEvalResults { + if evalResult.ExpressionAccessor == nil { + continue + } var auditAnnotationResult = &auditAnnotationResults[i] // TODO: move this to generics validation, ok := evalResult.ExpressionAccessor.(*AuditAnnotationCondition) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator_test.go index 78de36d615a..33e1796f8b0 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/validator_test.go @@ -34,6 +34,7 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/plugin/cel" celconfig "k8s.io/apiserver/pkg/apis/cel" + apiservercel "k8s.io/apiserver/pkg/cel" ) var _ cel.Filter = &fakeCelFilter{} @@ -43,11 +44,17 @@ type fakeCelFilter struct { throwError bool } -func (f *fakeCelFilter) ForInput(context.Context, *admission.VersionedAttributes, *admissionv1.AdmissionRequest, cel.OptionalVariableBindings, int64) ([]cel.EvaluationResult, error) { - if f.throwError { - return nil, errors.New("test error") +func (f *fakeCelFilter) ForInput(_ context.Context, _ *admission.VersionedAttributes, _ *admissionv1.AdmissionRequest, _ cel.OptionalVariableBindings, costBudget int64) ([]cel.EvaluationResult, int64, error) { + if costBudget <= 0 { // this filter will cost 1, so cost = 0 means fail. + return nil, -1, &apiservercel.Error{ + Type: apiservercel.ErrorTypeInvalid, + Detail: fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run"), + } } - return f.evaluations, nil + if f.throwError { + return nil, -1, errors.New("test error") + } + return f.evaluations, costBudget - 1, nil } func (f *fakeCelFilter) CompilationErrors() []error { @@ -65,13 +72,15 @@ func TestValidate(t *testing.T) { fakeVersionedAttr, _ := admission.NewVersionedAttributes(fakeAttr, schema.GroupVersionKind{}, nil) cases := []struct { - name string - failPolicy *v1.FailurePolicyType - evaluations []cel.EvaluationResult - auditEvaluations []cel.EvaluationResult - policyDecision []PolicyDecision - auditAnnotations []PolicyAuditAnnotation - throwError bool + name string + failPolicy *v1.FailurePolicyType + evaluations []cel.EvaluationResult + messageEvaluations []cel.EvaluationResult + auditEvaluations []cel.EvaluationResult + policyDecision []PolicyDecision + auditAnnotations []PolicyAuditAnnotation + throwError bool + costBudget int64 // leave zero to use default }{ { name: "test pass", @@ -572,6 +581,244 @@ func TestValidate(t *testing.T) { }, failPolicy: &ignore, }, + { + name: "messageExpression successful, empty message", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.String("evaluated message"), + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "evaluated message", + Reason: forbiddenReason, + }, + }, + }, + { + name: "messageExpression for multiple validations", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "I am not overwritten", + }, + }, + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "I am overwritten", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + {}, + { + EvalResult: celtypes.String("evaluated message"), + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "I am not overwritten", + Reason: forbiddenReason, + }, + { + Action: ActionDeny, + Message: "evaluated message", + Reason: forbiddenReason, + }, + }, + }, + { + name: "messageExpression successful, overwritten message", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "I am overwritten", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.String("evaluated message"), + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "evaluated message", + Reason: forbiddenReason, + }, + }, + }, + { + name: "messageExpression user failure", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "test1", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + { + Error: &apiservercel.Error{Type: apiservercel.ErrorTypeInvalid}, + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "test1", // original message used + Reason: forbiddenReason, + }, + }, + }, + { + name: "messageExpression eval to empty", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "test1", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.String(" "), + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "test1", + Reason: forbiddenReason, + }, + }, + }, + { + name: "messageExpression eval to multi-line", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "test1", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.String("hello\nthere"), + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "test1", + Reason: forbiddenReason, + }, + }, + }, + { + name: "messageExpression eval result too long", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "test1", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.String(strings.Repeat("x", 5*1024+1)), + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "test1", + Reason: forbiddenReason, + }, + }, + }, + { + name: "messageExpression eval to null", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "test1", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.NullValue, + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "test1", + Reason: forbiddenReason, + }, + }, + }, + { + name: "messageExpression out of budget after successful eval of expression", + evaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.False, + ExpressionAccessor: &ValidationCondition{ + Reason: &forbiddenReason, + Expression: "this.expression == unit.test", + Message: "test1", + }, + }, + }, + messageEvaluations: []cel.EvaluationResult{ + { + EvalResult: celtypes.StringType, // does not matter + }, + }, + policyDecision: []PolicyDecision{ + { + Action: ActionDeny, + Message: "running out of cost budget", + }, + }, + costBudget: 1, // shared between expression and messageExpression, needs 1 + 1 = 2 in total + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -581,13 +828,21 @@ func TestValidate(t *testing.T) { evaluations: tc.evaluations, throwError: tc.throwError, }, + messageFilter: &fakeCelFilter{ + evaluations: tc.messageEvaluations, + throwError: tc.throwError, + }, auditAnnotationFilter: &fakeCelFilter{ evaluations: tc.auditEvaluations, throwError: tc.throwError, }, } ctx := context.TODO() - validateResult := v.Validate(ctx, fakeVersionedAttr, nil, celconfig.RuntimeCELCostBudget) + var budget int64 = celconfig.RuntimeCELCostBudget + if tc.costBudget != 0 { + budget = tc.costBudget + } + validateResult := v.Validate(ctx, fakeVersionedAttr, nil, budget) require.Equal(t, len(validateResult.Decisions), len(tc.policyDecision)) @@ -630,6 +885,7 @@ func TestContextCanceled(t *testing.T) { v := validator{ failPolicy: &fail, validationFilter: f, + messageFilter: f, auditAnnotationFilter: &fakeCelFilter{ evaluations: nil, throwError: false,