From c400002facd01a8b0fc10ca4f5a66c8b8abd94c4 Mon Sep 17 00:00:00 2001 From: Cici Huang Date: Fri, 3 Mar 2023 00:18:47 +0000 Subject: [PATCH] Apply context cancellation to ValidatingAdmissionPolicy. --- .../pkg/admission/plugin/cel/filter.go | 5 ++-- .../pkg/admission/plugin/cel/filter_test.go | 6 ++-- .../pkg/admission/plugin/cel/interface.go | 3 +- .../admission_test.go | 30 +++++++++---------- .../validatingadmissionpolicy/controller.go | 2 +- .../validatingadmissionpolicy/interface.go | 4 ++- .../validatingadmissionpolicy/validator.go | 5 ++-- .../validator_test.go | 6 ++-- 8 files changed, 35 insertions(+), 26 deletions(-) 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 6531fecc835..1b516f6724f 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 @@ -17,6 +17,7 @@ limitations under the License. package cel import ( + "context" "errors" "fmt" "math" @@ -119,7 +120,7 @@ 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(versionedAttr *generic.VersionedAttributes, request *admissionv1.AdmissionRequest, inputs OptionalVariableBindings, runtimeCELCostBudget int64) ([]EvaluationResult, error) { +func (f *filter) ForInput(ctx context.Context, versionedAttr *generic.VersionedAttributes, request *admissionv1.AdmissionRequest, inputs OptionalVariableBindings, runtimeCELCostBudget int64) ([]EvaluationResult, 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 @@ -171,7 +172,7 @@ func (f *filter) ForInput(versionedAttr *generic.VersionedAttributes, request *a continue } t1 := time.Now() - evalResult, evalDetails, err := compilationResult.Program.Eval(va) + evalResult, evalDetails, err := compilationResult.Program.ContextEval(ctx, va) elapsed := time.Since(t1) evaluation.Elapsed = elapsed if evalDetails == nil { 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 99e0f56f8e8..3a2871d5a34 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 @@ -661,7 +661,8 @@ func TestFilter(t *testing.T) { } optionalVars := OptionalVariableBindings{VersionedParams: tc.params, Authorizer: tc.authorizer} - evalResults, err := f.ForInput(versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, celconfig.RuntimeCELCostBudget) + ctx := context.TODO() + evalResults, err := f.ForInput(ctx, versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, celconfig.RuntimeCELCostBudget) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -766,7 +767,8 @@ func TestRuntimeCELCostBudget(t *testing.T) { tc.testRuntimeCELCostBudget = celconfig.RuntimeCELCostBudget } optionalVars := OptionalVariableBindings{VersionedParams: tc.params, Authorizer: tc.authorizer} - evalResults, err := f.ForInput(versionedAttr, CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, tc.testRuntimeCELCostBudget) + ctx := context.TODO() + evalResults, 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") } 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 fe5702c0afa..1c7fad6aa7f 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/interface.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/interface.go @@ -17,6 +17,7 @@ limitations under the License. package cel import ( + "context" "time" "github.com/google/cel-go/cel" @@ -93,7 +94,7 @@ type OptionalVariableBindings struct { type Filter interface { // ForInput converts compiled CEL-typed values into evaluated CEL-typed values // 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(versionedAttr *generic.VersionedAttributes, request *v1.AdmissionRequest, optionalVars OptionalVariableBindings, runtimeCELCostBudget int64) ([]EvaluationResult, error) + ForInput(ctx context.Context, versionedAttr *generic.VersionedAttributes, request *v1.AdmissionRequest, optionalVars OptionalVariableBindings, runtimeCELCostBudget int64) ([]EvaluationResult, 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 79d78bb266b..5a1b0995de9 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 @@ -253,7 +253,7 @@ type fakeFilter struct { keyId string } -func (f *fakeFilter) ForInput(versionedAttr *whgeneric.VersionedAttributes, request *admissionv1.AdmissionRequest, inputs cel.OptionalVariableBindings, runtimeCELCostBudget int64) ([]cel.EvaluationResult, error) { +func (f *fakeFilter) ForInput(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, request *admissionv1.AdmissionRequest, inputs cel.OptionalVariableBindings, runtimeCELCostBudget int64) ([]cel.EvaluationResult, error) { return []cel.EvaluationResult{}, nil } @@ -265,10 +265,10 @@ var _ Validator = &fakeValidator{} type fakeValidator struct { validationFilter, auditAnnotationFilter *fakeFilter - ValidateFunc func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult + ValidateFunc func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult } -func (f *fakeValidator) RegisterDefinition(definition *v1alpha1.ValidatingAdmissionPolicy, validateFunc func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult) { +func (f *fakeValidator) RegisterDefinition(definition *v1alpha1.ValidatingAdmissionPolicy, validateFunc func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult) { //Key must be something that we can decipher from the inputs to Validate so using message which will be on the validationCondition object of evalResult var key string if len(definition.Spec.Validations) > 0 { @@ -285,8 +285,8 @@ func (f *fakeValidator) RegisterDefinition(definition *v1alpha1.ValidatingAdmiss validatorMap[key] = f } -func (f *fakeValidator) Validate(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { - return f.ValidateFunc(versionedAttr, versionedParams, runtimeCELCostBudget) +func (f *fakeValidator) Validate(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + return f.ValidateFunc(ctx, versionedAttr, versionedParams, runtimeCELCostBudget) } var _ Matcher = &fakeMatcher{} @@ -769,7 +769,7 @@ func TestBasicPolicyDefinitionFailure(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -839,7 +839,7 @@ func TestDefinitionDoesntMatch(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -952,7 +952,7 @@ func TestReconfigureBinding(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1062,7 +1062,7 @@ func TestRemoveDefinition(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1131,7 +1131,7 @@ func TestRemoveBinding(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1241,7 +1241,7 @@ func TestInvalidParamSourceInstanceName(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1309,7 +1309,7 @@ func TestEmptyParamSource(t *testing.T) { } }) - validator.RegisterDefinition(denyPolicy, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator.RegisterDefinition(denyPolicy, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { return ValidateResult{ Decisions: []PolicyDecision{ { @@ -1411,7 +1411,7 @@ func TestMultiplePoliciesSharedParamType(t *testing.T) { } }) - validator1.RegisterDefinition(&policy1, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator1.RegisterDefinition(&policy1, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { evaluations1.Add(1) return ValidateResult{ Decisions: []PolicyDecision{ @@ -1430,7 +1430,7 @@ func TestMultiplePoliciesSharedParamType(t *testing.T) { } }) - validator2.RegisterDefinition(&policy2, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator2.RegisterDefinition(&policy2, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { evaluations2.Add(1) return ValidateResult{ Decisions: []PolicyDecision{ @@ -1540,7 +1540,7 @@ func TestNativeTypeParam(t *testing.T) { } }) - validator.RegisterDefinition(&nativeTypeParamPolicy, func(versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { + validator.RegisterDefinition(&nativeTypeParamPolicy, func(ctx context.Context, versionedAttr *whgeneric.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { evaluations.Add(1) if _, ok := versionedParams.(*v1.ConfigMap); ok { return ValidateResult{ diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller.go index 7cdb86e4d0a..3bbb8e70db3 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller.go @@ -329,7 +329,7 @@ func (c *celAdmissionController) Validate( versionedAttr = va } - validationResult := bindingInfo.validator.Validate(versionedAttr, param, celconfig.RuntimeCELCostBudget) + validationResult := bindingInfo.validator.Validate(ctx, versionedAttr, param, celconfig.RuntimeCELCostBudget) if err != nil { // runtime error. Apply failure policy wrappedError := fmt.Errorf("failed to evaluate CEL expression: %w", err) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/interface.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/interface.go index 072b7e94aa0..4d5a71b2381 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/interface.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/interface.go @@ -17,6 +17,8 @@ limitations under the License. package validatingadmissionpolicy import ( + "context" + celgo "github.com/google/cel-go/cel" "k8s.io/api/admissionregistration/v1alpha1" @@ -84,5 +86,5 @@ type ValidateResult struct { type Validator interface { // Validate is used to take cel evaluations and convert into decisions // runtimeCELCostBudget was added for testing purpose only. Callers should always use const RuntimeCELCostBudget from k8s.io/apiserver/pkg/apis/cel/config.go as input. - Validate(versionedAttr *generic.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult + Validate(ctx context.Context, versionedAttr *generic.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult } 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 a1ee66b57a4..8e0bb853a05 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 @@ -17,6 +17,7 @@ limitations under the License. package validatingadmissionpolicy import ( + "context" "fmt" "strings" @@ -65,7 +66,7 @@ func auditAnnotationEvaluationForError(f v1.FailurePolicyType) PolicyAuditAnnota // Validate takes a list of Evaluation and a failure policy and converts them into actionable PolicyDecisions // 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 (v *validator) Validate(versionedAttr *generic.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { +func (v *validator) Validate(ctx context.Context, versionedAttr *generic.VersionedAttributes, versionedParams runtime.Object, runtimeCELCostBudget int64) ValidateResult { var f v1.FailurePolicyType if v.failPolicy == nil { f = v1.Fail @@ -74,7 +75,7 @@ func (v *validator) Validate(versionedAttr *generic.VersionedAttributes, version } optionalVars := cel.OptionalVariableBindings{VersionedParams: versionedParams, Authorizer: v.authorizer} - evalResults, err := v.validationFilter.ForInput(versionedAttr, cel.CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, runtimeCELCostBudget) + evalResults, err := v.validationFilter.ForInput(ctx context.Context, versionedAttr, cel.CreateAdmissionRequest(versionedAttr.Attributes), optionalVars, runtimeCELCostBudget) if err != nil { return ValidateResult{ Decisions: []PolicyDecision{ 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 706e1987b54..24afd11e8b8 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 @@ -17,6 +17,7 @@ limitations under the License. package validatingadmissionpolicy import ( + "context" "errors" "fmt" "strings" @@ -43,7 +44,7 @@ type fakeCelFilter struct { throwError bool } -func (f *fakeCelFilter) ForInput(*generic.VersionedAttributes, *admissionv1.AdmissionRequest, cel.OptionalVariableBindings, int64) ([]cel.EvaluationResult, error) { +func (f *fakeCelFilter) ForInput(context.Context, *generic.VersionedAttributes, *admissionv1.AdmissionRequest, cel.OptionalVariableBindings, int64) ([]cel.EvaluationResult, error) { if f.throwError { return nil, errors.New("test error") } @@ -586,7 +587,8 @@ func TestValidate(t *testing.T) { throwError: tc.throwError, }, } - validateResult := v.Validate(fakeVersionedAttr, nil, celconfig.RuntimeCELCostBudget) + ctx := context.TODO() + validateResult := v.Validate(ctx, fakeVersionedAttr, nil, celconfig.RuntimeCELCostBudget) require.Equal(t, len(validateResult.Decisions), len(tc.policyDecision))