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 bb5e233d453..06035f6b9e9 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 @@ -163,11 +163,12 @@ type variableDeclEnvs map[OptionalVariableDeclarations]*environment.EnvSet // CompileCELExpression returns a compiled CEL expression. // 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 (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor, options OptionalVariableDeclarations, envType environment.Type) CompilationResult { - resultError := func(errorString string, errType apiservercel.ErrorType) CompilationResult { + resultError := func(errorString string, errType apiservercel.ErrorType, cause error) CompilationResult { return CompilationResult{ Error: &apiservercel.Error{ Type: errType, Detail: errorString, + Cause: cause, }, ExpressionAccessor: expressionAccessor, } @@ -175,12 +176,12 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor, op env, err := c.varEnvs[options].Env(envType) if err != nil { - return resultError(fmt.Sprintf("unexpected error loading CEL environment: %v", err), apiservercel.ErrorTypeInternal) + return resultError(fmt.Sprintf("unexpected error loading CEL environment: %v", err), apiservercel.ErrorTypeInternal, nil) } ast, issues := env.Compile(expressionAccessor.GetExpression()) if issues != nil { - return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid) + return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid, apiservercel.NewCompilationError(issues)) } found := false returnTypes := expressionAccessor.ReturnTypes() @@ -198,19 +199,19 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor, op reason = fmt.Sprintf("must evaluate to one of %v", returnTypes) } - return resultError(reason, apiservercel.ErrorTypeInvalid) + return resultError(reason, apiservercel.ErrorTypeInvalid, nil) } _, err = cel.AstToCheckedExpr(ast) if err != nil { // should be impossible since env.Compile returned no issues - return resultError("unexpected compilation error: "+err.Error(), apiservercel.ErrorTypeInternal) + return resultError("unexpected compilation error: "+err.Error(), apiservercel.ErrorTypeInternal, nil) } prog, err := env.Program(ast, cel.InterruptCheckFrequency(celconfig.CheckFrequency), ) if err != nil { - return resultError("program instantiation failed: "+err.Error(), apiservercel.ErrorTypeInternal) + return resultError("program instantiation failed: "+err.Error(), apiservercel.ErrorTypeInternal, nil) } return CompilationResult{ Program: prog, 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 3e2a63e75ca..216a474d23e 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 @@ -192,6 +192,7 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione evaluation.Error = &cel.Error{ Type: cel.ErrorTypeInvalid, Detail: fmt.Sprintf("compilation error: %v", compilationResult.Error), + Cause: compilationResult.Error, } continue } @@ -211,6 +212,7 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione 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"), + Cause: cel.ErrOutOfBudget, } } remainingBudget -= compositionCost @@ -228,12 +230,14 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione 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()), + Cause: cel.ErrOutOfBudget, } } else { if *rtCost > math.MaxInt64 || int64(*rtCost) > remainingBudget { 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"), + Cause: cel.ErrOutOfBudget, } } remainingBudget -= int64(*rtCost) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/dispatcher.go index edf5a788673..f0601142530 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/dispatcher.go @@ -223,7 +223,7 @@ func (c *dispatcher) Dispatch(ctx context.Context, a admission.Attributes, o adm switch decision.Action { case ActionAdmit: if decision.Evaluation == EvalError { - celmetrics.Metrics.ObserveAdmissionWithError(ctx, decision.Elapsed, definition.Name, binding.Name, "active") + celmetrics.Metrics.ObserveAdmission(ctx, decision.Elapsed, definition.Name, binding.Name, ErrorType(&decision)) } case ActionDeny: for _, action := range binding.Spec.ValidationActions { @@ -234,13 +234,13 @@ func (c *dispatcher) Dispatch(ctx context.Context, a admission.Attributes, o adm Binding: binding, PolicyDecision: decision, }) - celmetrics.Metrics.ObserveRejection(ctx, decision.Elapsed, definition.Name, binding.Name, "active") + celmetrics.Metrics.ObserveRejection(ctx, decision.Elapsed, definition.Name, binding.Name, ErrorType(&decision)) case admissionregistrationv1.Audit: publishValidationFailureAnnotation(binding, i, decision, versionedAttr) - celmetrics.Metrics.ObserveAudit(ctx, decision.Elapsed, definition.Name, binding.Name, "active") + celmetrics.Metrics.ObserveAudit(ctx, decision.Elapsed, definition.Name, binding.Name, ErrorType(&decision)) case admissionregistrationv1.Warn: warning.AddWarning(ctx, "", fmt.Sprintf("Validation failed for ValidatingAdmissionPolicy '%s' with binding '%s': %s", definition.Name, binding.Name, decision.Message)) - celmetrics.Metrics.ObserveWarn(ctx, decision.Elapsed, definition.Name, binding.Name, "active") + celmetrics.Metrics.ObserveWarn(ctx, decision.Elapsed, definition.Name, binding.Name, ErrorType(&decision)) } } default: @@ -259,7 +259,7 @@ func (c *dispatcher) Dispatch(ctx context.Context, a admission.Attributes, o adm auditAnnotationCollector.add(auditAnnotation.Key, value) case AuditAnnotationActionError: // When failurePolicy=fail, audit annotation errors result in deny - deniedDecisions = append(deniedDecisions, policyDecisionWithMetadata{ + d := policyDecisionWithMetadata{ Definition: definition, Binding: binding, PolicyDecision: PolicyDecision{ @@ -268,8 +268,9 @@ func (c *dispatcher) Dispatch(ctx context.Context, a admission.Attributes, o adm Message: auditAnnotation.Error, Elapsed: auditAnnotation.Elapsed, }, - }) - celmetrics.Metrics.ObserveRejection(ctx, auditAnnotation.Elapsed, definition.Name, binding.Name, "active") + } + deniedDecisions = append(deniedDecisions, d) + celmetrics.Metrics.ObserveRejection(ctx, auditAnnotation.Elapsed, definition.Name, binding.Name, ErrorType(&d.PolicyDecision)) case AuditAnnotationActionExclude: // skip it default: return fmt.Errorf("unsupported AuditAnnotation Action: %s", auditAnnotation.Action) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/errors.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/errors.go new file mode 100644 index 00000000000..0fcf40ff0d4 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/errors.go @@ -0,0 +1,38 @@ +/* +Copyright 2024 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 validating + +import ( + "strings" + + celmetrics "k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics" +) + +// ErrorType decodes the error to determine the error type +// that the metrics understand. +func ErrorType(decision *PolicyDecision) celmetrics.ValidationErrorType { + if decision.Evaluation == EvalAdmit { + return celmetrics.ValidationNoError + } + if strings.HasPrefix(decision.Message, "compilation") { + return celmetrics.ValidationCompileError + } + if strings.HasPrefix(decision.Message, "validation failed due to running out of cost budget") { + return celmetrics.ValidatingOutOfBudget + } + return celmetrics.ValidatingInvalidError +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/errors.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/errors.go new file mode 100644 index 00000000000..4327252610a --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/errors.go @@ -0,0 +1,38 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +import ( + "errors" + + apiservercel "k8s.io/apiserver/pkg/cel" +) + +// ErrorType decodes the error to determine the error type +// that the metrics understand. +func ErrorType(err error) ValidationErrorType { + if err == nil { + return ValidationNoError + } + if errors.Is(err, apiservercel.ErrCompilation) { + return ValidationCompileError + } + if errors.Is(err, apiservercel.ErrOutOfBudget) { + return ValidatingOutOfBudget + } + return ValidatingInvalidError +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/metrics.go index 9f8a941105a..15e6fbe5421 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/metrics.go @@ -29,6 +29,22 @@ const ( metricsSubsystem = "validating_admission_policy" ) +// ValidationErrorType defines different error types that happen to a validation expression +type ValidationErrorType string + +const ( + // ValidationCompileError indicates that the expression fails to compile. + ValidationCompileError ValidationErrorType = "compile_error" + // ValidatingInvalidError indicates that the expression fails due to internal + // errors that are out of the control of the user. + ValidatingInvalidError ValidationErrorType = "invalid_error" + // ValidatingOutOfBudget indicates that the expression fails due to running + // out of cost budget, or the budget cannot be obtained. + ValidatingOutOfBudget ValidationErrorType = "out_of_budget" + // ValidationNoError indicates that the expression returns without an error. + ValidationNoError ValidationErrorType = "no_error" +) + var ( // Metrics provides access to validation admission metrics. Metrics = newValidationAdmissionMetrics() @@ -36,9 +52,8 @@ var ( // ValidatingAdmissionPolicyMetrics aggregates Prometheus metrics related to validation admission control. type ValidatingAdmissionPolicyMetrics struct { - policyCheck *metrics.CounterVec - policyDefinition *metrics.CounterVec - policyLatency *metrics.HistogramVec + policyCheck *metrics.CounterVec + policyLatency *metrics.HistogramVec } func newValidationAdmissionMetrics() *ValidatingAdmissionPolicyMetrics { @@ -47,25 +62,16 @@ func newValidationAdmissionMetrics() *ValidatingAdmissionPolicyMetrics { Namespace: metricsNamespace, Subsystem: metricsSubsystem, Name: "check_total", - Help: "Validation admission policy check total, labeled by policy and further identified by binding, enforcement action taken, and state.", + Help: "Validation admission policy check total, labeled by policy and further identified by binding and enforcement action taken.", StabilityLevel: metrics.ALPHA, }, - []string{"policy", "policy_binding", "enforcement_action", "state"}, - ) - definition := metrics.NewCounterVec(&metrics.CounterOpts{ - Namespace: metricsNamespace, - Subsystem: metricsSubsystem, - Name: "definition_total", - Help: "Validation admission policy count total, labeled by state and enforcement action.", - StabilityLevel: metrics.ALPHA, - }, - []string{"state", "enforcement_action"}, + []string{"policy", "policy_binding", "error_type", "enforcement_action"}, ) latency := metrics.NewHistogramVec(&metrics.HistogramOpts{ Namespace: metricsNamespace, Subsystem: metricsSubsystem, Name: "check_duration_seconds", - Help: "Validation admission latency for individual validation expressions in seconds, labeled by policy and further including binding, state and enforcement action taken.", + Help: "Validation admission latency for individual validation expressions in seconds, labeled by policy and further including binding and enforcement action taken.", // the bucket distribution here is based oo the benchmark suite at // github.com/DangerOnTheRanger/cel-benchmark performed on 16-core Intel Xeon // the lowest bucket was based around the 180ns/op figure for BenchmarkAccess, @@ -77,47 +83,40 @@ func newValidationAdmissionMetrics() *ValidatingAdmissionPolicyMetrics { Buckets: []float64{0.0000005, 0.001, 0.01, 0.1, 1.0}, StabilityLevel: metrics.ALPHA, }, - []string{"policy", "policy_binding", "enforcement_action", "state"}, + []string{"policy", "policy_binding", "error_type", "enforcement_action"}, ) legacyregistry.MustRegister(check) - legacyregistry.MustRegister(definition) legacyregistry.MustRegister(latency) - return &ValidatingAdmissionPolicyMetrics{policyCheck: check, policyDefinition: definition, policyLatency: latency} + return &ValidatingAdmissionPolicyMetrics{policyCheck: check, policyLatency: latency} } // Reset resets all validation admission-related Prometheus metrics. func (m *ValidatingAdmissionPolicyMetrics) Reset() { m.policyCheck.Reset() - m.policyDefinition.Reset() m.policyLatency.Reset() } -// ObserveDefinition observes a policy definition. -func (m *ValidatingAdmissionPolicyMetrics) ObserveDefinition(ctx context.Context, state, enforcementAction string) { - m.policyDefinition.WithContext(ctx).WithLabelValues(state, enforcementAction).Inc() -} - -// ObserveAdmissionWithError observes a policy validation error that was ignored due to failure policy. -func (m *ValidatingAdmissionPolicyMetrics) ObserveAdmissionWithError(ctx context.Context, elapsed time.Duration, policy, binding, state string) { - m.policyCheck.WithContext(ctx).WithLabelValues(policy, binding, "allow", state).Inc() - m.policyLatency.WithContext(ctx).WithLabelValues(policy, binding, "allow", state).Observe(elapsed.Seconds()) +// ObserveAdmission observes a policy validation, with an optional error to indicate the error that may occur but ignored. +func (m *ValidatingAdmissionPolicyMetrics) ObserveAdmission(ctx context.Context, elapsed time.Duration, policy, binding string, errorType ValidationErrorType) { + m.policyCheck.WithContext(ctx).WithLabelValues(policy, binding, string(errorType), "allow").Inc() + m.policyLatency.WithContext(ctx).WithLabelValues(policy, binding, string(errorType), "allow").Observe(elapsed.Seconds()) } // ObserveRejection observes a policy validation error that was at least one of the reasons for a deny. -func (m *ValidatingAdmissionPolicyMetrics) ObserveRejection(ctx context.Context, elapsed time.Duration, policy, binding, state string) { - m.policyCheck.WithContext(ctx).WithLabelValues(policy, binding, "deny", state).Inc() - m.policyLatency.WithContext(ctx).WithLabelValues(policy, binding, "deny", state).Observe(elapsed.Seconds()) +func (m *ValidatingAdmissionPolicyMetrics) ObserveRejection(ctx context.Context, elapsed time.Duration, policy, binding string, errorType ValidationErrorType) { + m.policyCheck.WithContext(ctx).WithLabelValues(policy, binding, string(errorType), "deny").Inc() + m.policyLatency.WithContext(ctx).WithLabelValues(policy, binding, string(errorType), "deny").Observe(elapsed.Seconds()) } // ObserveAudit observes a policy validation audit annotation was published for a validation failure. -func (m *ValidatingAdmissionPolicyMetrics) ObserveAudit(ctx context.Context, elapsed time.Duration, policy, binding, state string) { - m.policyCheck.WithContext(ctx).WithLabelValues(policy, binding, "audit", state).Inc() - m.policyLatency.WithContext(ctx).WithLabelValues(policy, binding, "audit", state).Observe(elapsed.Seconds()) +func (m *ValidatingAdmissionPolicyMetrics) ObserveAudit(ctx context.Context, elapsed time.Duration, policy, binding string, errorType ValidationErrorType) { + m.policyCheck.WithContext(ctx).WithLabelValues(policy, binding, string(errorType), "audit").Inc() + m.policyLatency.WithContext(ctx).WithLabelValues(policy, binding, string(errorType), "audit").Observe(elapsed.Seconds()) } // ObserveWarn observes a policy validation warning was published for a validation failure. -func (m *ValidatingAdmissionPolicyMetrics) ObserveWarn(ctx context.Context, elapsed time.Duration, policy, binding, state string) { - m.policyCheck.WithContext(ctx).WithLabelValues(policy, binding, "warn", state).Inc() - m.policyLatency.WithContext(ctx).WithLabelValues(policy, binding, "warn", state).Observe(elapsed.Seconds()) +func (m *ValidatingAdmissionPolicyMetrics) ObserveWarn(ctx context.Context, elapsed time.Duration, policy, binding string, errorType ValidationErrorType) { + m.policyCheck.WithContext(ctx).WithLabelValues(policy, binding, string(errorType), "warn").Inc() + m.policyLatency.WithContext(ctx).WithLabelValues(policy, binding, string(errorType), "warn").Observe(elapsed.Seconds()) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/metrics_test.go index 7c9e3e75998..ad450001b90 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/metrics_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/metrics/metrics_test.go @@ -31,7 +31,6 @@ type metricsObserver func() func TestNoUtils(t *testing.T) { metrics := []string{ - "apiserver_validating_admission_policy_definition_total", "apiserver_validating_admission_policy_check_total", "apiserver_validating_admission_policy_check_duration_seconds", } @@ -44,53 +43,43 @@ func TestNoUtils(t *testing.T) { { desc: "observe policy admission", want: ` - # HELP apiserver_validating_admission_policy_check_duration_seconds [ALPHA] Validation admission latency for individual validation expressions in seconds, labeled by policy and further including binding, state and enforcement action taken. + # HELP apiserver_validating_admission_policy_check_duration_seconds [ALPHA] Validation admission latency for individual validation expressions in seconds, labeled by policy and further including binding and enforcement action taken. # TYPE apiserver_validating_admission_policy_check_duration_seconds histogram - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="0.0000005"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="0.001"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="0.01"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="0.1"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="1"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="+Inf"} 1 - apiserver_validating_admission_policy_check_duration_seconds_sum{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active"} 10 - apiserver_validating_admission_policy_check_duration_seconds_count{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active"} 1 - # HELP apiserver_validating_admission_policy_check_total [ALPHA] Validation admission policy check total, labeled by policy and further identified by binding, enforcement action taken, and state. + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="0.0000005"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="0.001"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="0.01"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="0.1"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="1"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="+Inf"} 1 + apiserver_validating_admission_policy_check_duration_seconds_sum{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com"} 10 + apiserver_validating_admission_policy_check_duration_seconds_count{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com"} 1 + # HELP apiserver_validating_admission_policy_check_total [ALPHA] Validation admission policy check total, labeled by policy and further identified by binding and enforcement action taken. # TYPE apiserver_validating_admission_policy_check_total counter - apiserver_validating_admission_policy_check_total{enforcement_action="allow",policy="policy.example.com",policy_binding="binding.example.com",state="active"} 1 + apiserver_validating_admission_policy_check_total{enforcement_action="allow",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com"} 1 `, observer: func() { - Metrics.ObserveAdmissionWithError(context.TODO(), time.Duration(10)*time.Second, "policy.example.com", "binding.example.com", "active") + Metrics.ObserveAdmission(context.TODO(), time.Duration(10)*time.Second, "policy.example.com", "binding.example.com", ValidatingInvalidError) }, }, { desc: "observe policy rejection", want: ` - # HELP apiserver_validating_admission_policy_check_duration_seconds [ALPHA] Validation admission latency for individual validation expressions in seconds, labeled by policy and further including binding, state and enforcement action taken. + # HELP apiserver_validating_admission_policy_check_duration_seconds [ALPHA] Validation admission latency for individual validation expressions in seconds, labeled by policy and further including binding and enforcement action taken. # TYPE apiserver_validating_admission_policy_check_duration_seconds histogram - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="0.0000005"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="0.001"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="0.01"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="0.1"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="1"} 0 - apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active",le="+Inf"} 1 - apiserver_validating_admission_policy_check_duration_seconds_sum{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active"} 10 - apiserver_validating_admission_policy_check_duration_seconds_count{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active"} 1 - # HELP apiserver_validating_admission_policy_check_total [ALPHA] Validation admission policy check total, labeled by policy and further identified by binding, enforcement action taken, and state. + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="0.0000005"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="0.001"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="0.01"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="0.1"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="1"} 0 + apiserver_validating_admission_policy_check_duration_seconds_bucket{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com",le="+Inf"} 1 + apiserver_validating_admission_policy_check_duration_seconds_sum{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com"} 10 + apiserver_validating_admission_policy_check_duration_seconds_count{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com"} 1 + # HELP apiserver_validating_admission_policy_check_total [ALPHA] Validation admission policy check total, labeled by policy and further identified by binding and enforcement action taken. # TYPE apiserver_validating_admission_policy_check_total counter - apiserver_validating_admission_policy_check_total{enforcement_action="deny",policy="policy.example.com",policy_binding="binding.example.com",state="active"} 1 + apiserver_validating_admission_policy_check_total{enforcement_action="deny",error_type="invalid_error",policy="policy.example.com",policy_binding="binding.example.com"} 1 `, observer: func() { - Metrics.ObserveRejection(context.TODO(), time.Duration(10)*time.Second, "policy.example.com", "binding.example.com", "active") - }, - }, - { - desc: "observe policy definition", - want: `# HELP apiserver_validating_admission_policy_definition_total [ALPHA] Validation admission policy count total, labeled by state and enforcement action. - # TYPE apiserver_validating_admission_policy_definition_total counter - apiserver_validating_admission_policy_definition_total{enforcement_action="deny",state="active"} 1 - `, - observer: func() { - Metrics.ObserveDefinition(context.TODO(), "active", "deny") + Metrics.ObserveRejection(context.TODO(), time.Duration(10)*time.Second, "policy.example.com", "binding.example.com", ValidatingInvalidError) }, }, } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator.go index da2ea1c10f8..c429ae22fcf 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/validator.go @@ -18,6 +18,7 @@ package validating import ( "context" + "errors" "fmt" "strings" @@ -132,19 +133,14 @@ func (v *validator) Validate(ctx context.Context, matchedResource schema.GroupVe } 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"))) { + } else if errors.Is(err, apiservercel.ErrInternal) || errors.Is(err, apiservercel.ErrOutOfBudget) { decision.Action = policyDecisionActionForError(f) decision.Evaluation = EvalError decision.Message = fmt.Sprintf("failed messageExpression: %s", err) 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 99b3556b544..44d79e60f88 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 @@ -53,6 +53,7 @@ func (f *fakeCelFilter) ForInput(ctx context.Context, versionedAttr *admission.V 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"), + Cause: apiservercel.ErrOutOfBudget, } } if f.throwError { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/errors.go b/staging/src/k8s.io/apiserver/pkg/cel/errors.go index 907ca6ec8f1..d7b052fc9e7 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/errors.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/errors.go @@ -16,11 +16,46 @@ limitations under the License. package cel +import ( + "fmt" + + "github.com/google/cel-go/cel" +) + +// ErrInternal the basic error that occurs when the expression fails to evaluate +// due to internal reasons. Any Error that has the Type of +// ErrorInternal is considered equal to ErrInternal +var ErrInternal = fmt.Errorf("internal") + +// ErrInvalid is the basic error that occurs when the expression fails to +// evaluate but not due to internal reasons. Any Error that has the Type of +// ErrorInvalid is considered equal to ErrInvalid. +var ErrInvalid = fmt.Errorf("invalid") + +// ErrRequired is the basic error that occurs when the expression is required +// but absent. +// Any Error that has the Type of ErrorRequired is considered equal +// to ErrRequired. +var ErrRequired = fmt.Errorf("required") + +// ErrCompilation is the basic error that occurs when the expression fails to +// compile. Any CompilationError wraps ErrCompilation. +// ErrCompilation wraps ErrInvalid +var ErrCompilation = fmt.Errorf("%w: compilation error", ErrInvalid) + +// ErrOutOfBudget is the basic error that occurs when the expression fails due to +// exceeding budget. +var ErrOutOfBudget = fmt.Errorf("out of budget") + // Error is an implementation of the 'error' interface, which represents a // XValidation error. type Error struct { Type ErrorType Detail string + + // Cause is an optional wrapped errors that can be useful to + // programmatically retrieve detailed errors. + Cause error } var _ error = &Error{} @@ -30,7 +65,24 @@ func (v *Error) Error() string { return v.Detail } -// ErrorType is a machine readable value providing more detail about why +func (v *Error) Is(err error) bool { + switch v.Type { + case ErrorTypeRequired: + return err == ErrRequired + case ErrorTypeInvalid: + return err == ErrInvalid + case ErrorTypeInternal: + return err == ErrInternal + } + return false +} + +// Unwrap returns the wrapped Cause. +func (v *Error) Unwrap() error { + return v.Cause +} + +// ErrorType is a machine-readable value providing more detail about why // a XValidation is invalid. type ErrorType string @@ -45,3 +97,28 @@ const ( // to user input. See InternalError(). ErrorTypeInternal ErrorType = "InternalError" ) + +// CompilationError indicates an error during expression compilation. +// It wraps ErrCompilation. +type CompilationError struct { + err *Error + Issues *cel.Issues +} + +// NewCompilationError wraps a cel.Issues to indicate a compilation failure. +func NewCompilationError(issues *cel.Issues) *CompilationError { + return &CompilationError{ + Issues: issues, + err: &Error{ + Type: ErrorTypeInvalid, + Detail: fmt.Sprintf("compilation error: %s", issues), + }} +} + +func (e *CompilationError) Error() string { + return e.err.Error() +} + +func (e *CompilationError) Unwrap() []error { + return []error{e.err, ErrCompilation} +} diff --git a/staging/src/k8s.io/apiserver/pkg/cel/errors_test.go b/staging/src/k8s.io/apiserver/pkg/cel/errors_test.go new file mode 100644 index 00000000000..2822aaab714 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/cel/errors_test.go @@ -0,0 +1,63 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +import ( + "errors" + "testing" + + "github.com/google/cel-go/cel" +) + +func TestOutOfBudgetError(t *testing.T) { + err := &Error{ + Type: ErrorTypeInvalid, + Detail: "expression out of budget", + Cause: ErrOutOfBudget, + } + if !errors.Is(err, ErrOutOfBudget) { + t.Errorf("unexpected %v is not %v", err, ErrOutOfBudget) + } + if !errors.Is(err, ErrInvalid) { + t.Errorf("unexpected %v is not %v", err, ErrInvalid) + } +} + +func TestCompilationError(t *testing.T) { + if !errors.Is(ErrCompilation, ErrInvalid) { + t.Errorf("unexpected %v is not %v", ErrCompilation, ErrInvalid) + } + issues := &cel.Issues{} + err := &Error{ + Type: ErrorTypeInvalid, + Detail: "fake compilation failed", + Cause: NewCompilationError(issues), + } + if !errors.Is(err, ErrCompilation) { + t.Errorf("unexpected %v is not %v", err, ErrCompilation) + } + if !errors.Is(err, ErrInvalid) { + t.Errorf("unexpected %v is not %v", err, ErrInvalid) + } + var compilationErr *CompilationError + if errors.As(err, &compilationErr); compilationErr == nil { + t.Errorf("unexpected %v cannot be fitted into CompilationError", err) + } + if compilationErr.Issues != issues { + t.Errorf("retrieved issues is not the original") + } +}