errors improvement.

This commit is contained in:
Jiahui Feng 2024-04-23 16:54:47 -07:00 committed by Cici Huang
parent 1904a56e30
commit b846c39047
6 changed files with 151 additions and 9 deletions

View File

@ -163,11 +163,12 @@ type variableDeclEnvs map[OptionalVariableDeclarations]*environment.EnvSet
// CompileCELExpression returns a compiled CEL expression. // 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. // 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 { 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, errors ...error) CompilationResult {
return CompilationResult{ return CompilationResult{
Error: &apiservercel.Error{ Error: &apiservercel.Error{
Type: errType, Type: errType,
Detail: errorString, Detail: errorString,
Errors: errors,
}, },
ExpressionAccessor: expressionAccessor, ExpressionAccessor: expressionAccessor,
} }
@ -180,7 +181,7 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor, op
ast, issues := env.Compile(expressionAccessor.GetExpression()) ast, issues := env.Compile(expressionAccessor.GetExpression())
if issues != nil { if issues != nil {
return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid) return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid, apiservercel.NewCompilationError(issues))
} }
found := false found := false
returnTypes := expressionAccessor.ReturnTypes() returnTypes := expressionAccessor.ReturnTypes()

View File

@ -192,6 +192,7 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione
evaluation.Error = &cel.Error{ evaluation.Error = &cel.Error{
Type: cel.ErrorTypeInvalid, Type: cel.ErrorTypeInvalid,
Detail: fmt.Sprintf("compilation error: %v", compilationResult.Error), Detail: fmt.Sprintf("compilation error: %v", compilationResult.Error),
Errors: []error{compilationResult.Error},
} }
continue continue
} }
@ -211,6 +212,7 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione
return nil, -1, &cel.Error{ return nil, -1, &cel.Error{
Type: cel.ErrorTypeInvalid, Type: cel.ErrorTypeInvalid,
Detail: fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run"), Detail: fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run"),
Errors: []error{cel.ErrOutOfBudget},
} }
} }
remainingBudget -= compositionCost remainingBudget -= compositionCost
@ -228,12 +230,14 @@ func (f *filter) ForInput(ctx context.Context, versionedAttr *admission.Versione
return nil, -1, &cel.Error{ return nil, -1, &cel.Error{
Type: cel.ErrorTypeInvalid, Type: cel.ErrorTypeInvalid,
Detail: fmt.Sprintf("runtime cost could not be calculated for expression: %v, no further expression will be run", compilationResult.ExpressionAccessor.GetExpression()), Detail: fmt.Sprintf("runtime cost could not be calculated for expression: %v, no further expression will be run", compilationResult.ExpressionAccessor.GetExpression()),
Errors: []error{cel.ErrOutOfBudget},
} }
} else { } else {
if *rtCost > math.MaxInt64 || int64(*rtCost) > remainingBudget { if *rtCost > math.MaxInt64 || int64(*rtCost) > remainingBudget {
return nil, -1, &cel.Error{ return nil, -1, &cel.Error{
Type: cel.ErrorTypeInvalid, Type: cel.ErrorTypeInvalid,
Detail: fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run"), Detail: fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run"),
Errors: []error{cel.ErrOutOfBudget},
} }
} }
remainingBudget -= int64(*rtCost) remainingBudget -= int64(*rtCost)

View File

@ -18,6 +18,7 @@ package validating
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"strings" "strings"
@ -132,19 +133,14 @@ func (v *validator) Validate(ctx context.Context, matchedResource schema.GroupVe
} }
var messageResult *cel.EvaluationResult var messageResult *cel.EvaluationResult
var messageError *apiservercel.Error
if len(messageResults) > i { if len(messageResults) > i {
messageResult = &messageResults[i] messageResult = &messageResults[i]
} }
messageError, _ = err.(*apiservercel.Error)
if evalResult.Error != nil { if evalResult.Error != nil {
decision.Action = policyDecisionActionForError(f) decision.Action = policyDecisionActionForError(f)
decision.Evaluation = EvalError decision.Evaluation = EvalError
decision.Message = evalResult.Error.Error() decision.Message = evalResult.Error.Error()
} else if messageError != nil && } else if errors.Is(err, apiservercel.ErrInternal) || errors.Is(err, apiservercel.ErrOutOfBudget) {
(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.Action = policyDecisionActionForError(f)
decision.Evaluation = EvalError decision.Evaluation = EvalError
decision.Message = fmt.Sprintf("failed messageExpression: %s", err) decision.Message = fmt.Sprintf("failed messageExpression: %s", err)

View File

@ -53,6 +53,7 @@ func (f *fakeCelFilter) ForInput(ctx context.Context, versionedAttr *admission.V
return nil, -1, &apiservercel.Error{ return nil, -1, &apiservercel.Error{
Type: apiservercel.ErrorTypeInvalid, Type: apiservercel.ErrorTypeInvalid,
Detail: fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run"), Detail: fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run"),
Errors: []error{apiservercel.ErrOutOfBudget},
} }
} }
if f.throwError { if f.throwError {

View File

@ -16,11 +16,46 @@ limitations under the License.
package cel 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 // Error is an implementation of the 'error' interface, which represents a
// XValidation error. // XValidation error.
type Error struct { type Error struct {
Type ErrorType Type ErrorType
Detail string Detail string
// Errors are optional wrapped errors that can be useful to
// programmatically retrieve detailed errors.
Errors []error
} }
var _ error = &Error{} var _ error = &Error{}
@ -30,7 +65,24 @@ func (v *Error) Error() string {
return v.Detail 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 wrapped errors.
func (v *Error) Unwrap() []error {
return v.Errors
}
// ErrorType is a machine-readable value providing more detail about why
// a XValidation is invalid. // a XValidation is invalid.
type ErrorType string type ErrorType string
@ -45,3 +97,28 @@ const (
// to user input. See InternalError(). // to user input. See InternalError().
ErrorTypeInternal ErrorType = "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}
}

View File

@ -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",
Errors: []error{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",
Errors: []error{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")
}
}