Forbid CEL transition rules on unmergeable CRD subschemas.

Transition rules (i.e. validation rules whose expressions reference
existing state) are not allowed on schemas, or the descendants of
schemas, that are unmergeable according to server-side apply
semantics. Today, this means that only objects with map-type
"granular" (or unspecified) and arrays with list-type "map" support
transition rules on their property/item subschemas.
This commit is contained in:
Ben Luddy 2022-02-08 15:22:01 -05:00
parent f757ab13af
commit fedaa23f24
No known key found for this signature in database
GPG Key ID: E01DF04B82AF03C0
5 changed files with 682 additions and 59 deletions

View File

@ -654,6 +654,10 @@ type specStandardValidator interface {
// insideResourceMeta returns true when validating either TypeMeta or ObjectMeta, from an embedded resource or on the top-level. // insideResourceMeta returns true when validating either TypeMeta or ObjectMeta, from an embedded resource or on the top-level.
insideResourceMeta() bool insideResourceMeta() bool
withInsideResourceMeta() specStandardValidator withInsideResourceMeta() specStandardValidator
// forbidOldSelfValidations returns the path to the first ancestor of the visited path that can't be safely correlated between two revisions of an object, or nil if there is no such path
forbidOldSelfValidations() *field.Path
withForbidOldSelfValidations(path *field.Path) specStandardValidator
} }
// validateCustomResourceDefinitionValidation statically validates // validateCustomResourceDefinitionValidation statically validates
@ -776,9 +780,14 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
for property, jsonSchema := range schema.Properties { for property, jsonSchema := range schema.Properties {
subSsv := ssv subSsv := ssv
// defensively assumes that a future map type is uncorrelatable
if schema.XMapType != nil && (*schema.XMapType != "granular" && *schema.XMapType != "atomic") {
subSsv = subSsv.withForbidOldSelfValidations(fldPath)
}
if (isRoot || schema.XEmbeddedResource) && metaFields.Has(property) { if (isRoot || schema.XEmbeddedResource) && metaFields.Has(property) {
// we recurse into the schema that applies to ObjectMeta. // we recurse into the schema that applies to ObjectMeta.
subSsv = ssv.withInsideResourceMeta() subSsv = subSsv.withInsideResourceMeta()
if isRoot { if isRoot {
subSsv = subSsv.withForbiddenDefaults(fmt.Sprintf("in top-level %s", property)) subSsv = subSsv.withForbiddenDefaults(fmt.Sprintf("in top-level %s", property))
} }
@ -814,10 +823,19 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
} }
if schema.Items != nil { if schema.Items != nil {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), ssv, false, opts)...) subSsv := ssv
// we can only correlate old/new items for "map" and "set" lists, and correlation of
// "set" elements by identity is not supported for cel (x-kubernetes-validations)
// rules. an unset list type defaults to "atomic".
if schema.XListType == nil || *schema.XListType != "map" {
subSsv = subSsv.withForbidOldSelfValidations(fldPath)
}
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), subSsv, false, opts)...)
if len(schema.Items.JSONSchemas) != 0 { if len(schema.Items.JSONSchemas) != 0 {
for i, jsonSchema := range schema.Items.JSONSchemas { for i, jsonSchema := range schema.Items.JSONSchemas {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), ssv, false, opts)...) allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), subSsv, false, opts)...)
} }
} }
} }
@ -940,6 +958,15 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i], cr.Error.Detail)) allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i], cr.Error.Detail))
} }
} }
if cr.TransitionRule {
if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, fmt.Sprintf("oldSelf cannot be used on the uncorrelatable portion of the schema within %v", uncorrelatablePath)))
} else {
// todo: remove when transition rule validation is implemented
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, "validation of rules containing oldSelf is not yet implemented"))
}
}
} }
} }
} }
@ -1010,6 +1037,7 @@ type specStandardValidatorV3 struct {
disallowDefaultsReason string disallowDefaultsReason string
isInsideResourceMeta bool isInsideResourceMeta bool
requireValidPropertyType bool requireValidPropertyType bool
uncorrelatableOldSelfValidationPath *field.Path
} }
func (v *specStandardValidatorV3) withForbiddenDefaults(reason string) specStandardValidator { func (v *specStandardValidatorV3) withForbiddenDefaults(reason string) specStandardValidator {
@ -1029,6 +1057,21 @@ func (v *specStandardValidatorV3) insideResourceMeta() bool {
return v.isInsideResourceMeta return v.isInsideResourceMeta
} }
func (v *specStandardValidatorV3) withForbidOldSelfValidations(path *field.Path) specStandardValidator {
if v.uncorrelatableOldSelfValidationPath != nil {
// oldSelf validations are already forbidden. preserve the highest-level path
// causing oldSelf validations to be forbidden
return v
}
clone := *v
clone.uncorrelatableOldSelfValidationPath = path
return &clone
}
func (v *specStandardValidatorV3) forbidOldSelfValidations() *field.Path {
return v.uncorrelatableOldSelfValidationPath
}
// validate validates against OpenAPI Schema v3. // validate validates against OpenAPI Schema v3.
func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList { func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}

View File

@ -58,6 +58,13 @@ func immutable(path ...string) validationMatch {
func forbidden(path ...string) validationMatch { func forbidden(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeForbidden} return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeForbidden}
} }
func notImplemented(path ...string) validationMatch {
return validationMatch{
path: field.NewPath(path[0], path[1:]...),
errorType: field.ErrorTypeInvalid,
contains: "not yet implemented",
}
}
func (v validationMatch) matches(err *field.Error) bool { func (v validationMatch) matches(err *field.Error) bool {
return err.Type == v.errorType && err.Field == v.path.String() && strings.Contains(err.Error(), v.contains) return err.Type == v.errorType && err.Field == v.path.String() && strings.Contains(err.Error(), v.contains)
@ -7588,6 +7595,373 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
invalid("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[2].message"), invalid("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[2].message"),
}, },
}, },
{
name: "forbid transition rule on element of list of type atomic",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
XListType: strPtr("atomic"),
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
},
},
{
name: "forbid transition rule on element of list defaulting to type atomic",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on list of type atomic",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
XListType: strPtr("atomic"),
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
},
},
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on list defaulting to type atomic",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
},
},
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"),
},
},
{
name: "forbid transition rule on element of list of type set",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
XListType: strPtr("set"),
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on list of type set",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
XListType: strPtr("set"),
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
},
},
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on element of list of type map",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
XListType: strPtr("map"),
XListMapKeys: []string{"name"},
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "object",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
Required: []string{"name"},
Properties: map[string]apiextensions.JSONSchemaProps{
"name": {Type: "string"},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on list of type map",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
XListType: strPtr("map"),
XListMapKeys: []string{"name"},
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Required: []string{"name"},
Properties: map[string]apiextensions.JSONSchemaProps{
"name": {Type: "string"},
},
},
},
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on element of map of type granular",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "object",
XMapType: strPtr("granular"),
Properties: map[string]apiextensions.JSONSchemaProps{
"subfield": {
Type: "string",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].properties[subfield].x-kubernetes-validations[0].rule"),
},
},
{
name: "forbid transition rule on element of map of unrecognized type",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "object",
XMapType: strPtr("future"),
Properties: map[string]apiextensions.JSONSchemaProps{
"subfield": {
Type: "string",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].properties[subfield].x-kubernetes-validations[0].rule"),
unsupported("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-map-type"),
},
},
{
name: "allow transition rule on element of map defaulting to type granular",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"subfield": {
Type: "string",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].properties[subfield].x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on map of type granular",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "object",
XMapType: strPtr("granular"),
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on map defaulting to type granular",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on element of map of type atomic",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "object",
XMapType: strPtr("atomic"),
Properties: map[string]apiextensions.JSONSchemaProps{
"subfield": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].properties[subfield].x-kubernetes-validations[0].rule"),
},
},
{
name: "allow transition rule on map of type atomic",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "object",
XMapType: strPtr("atomic"),
XValidations: apiextensions.ValidationRules{
{Rule: "self == oldSelf"},
},
},
},
},
},
expectedErrors: []validationMatch{
notImplemented("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"),
},
},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {

View File

@ -26,18 +26,30 @@ import (
expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1" expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
"google.golang.org/protobuf/proto" "google.golang.org/protobuf/proto"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/library" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/library"
celmodel "k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model" celmodel "k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model"
) )
// ScopedVarName is the variable name assigned to the locally scoped data element of a CEL valid. const (
const ScopedVarName = "self" // ScopedVarName is the variable name assigned to the locally scoped data element of a CEL validation
// expression.
ScopedVarName = "self"
// OldScopedVarName is the variable name assigned to the existing value of the locally scoped data element of a
// CEL validation expression.
OldScopedVarName = "oldSelf"
)
// CompilationResult represents the cel compilation result for one rule // CompilationResult represents the cel compilation result for one rule
type CompilationResult struct { type CompilationResult struct {
Program cel.Program Program cel.Program
Error *Error Error *Error
// If true, the compiled expression contains a reference to the identifier "oldSelf", and its corresponding rule
// is implicitly a transition rule.
TransitionRule bool
} }
// Compile compiles all the XValidations rules (without recursing into the schema) and returns a slice containing a // Compile compiles all the XValidations rules (without recursing into the schema) and returns a slice containing a
@ -81,6 +93,7 @@ func Compile(s *schema.Structural, isResourceRoot bool) ([]CompilationResult, er
root = rootDecl.MaybeAssignTypeName(scopedTypeName) root = rootDecl.MaybeAssignTypeName(scopedTypeName)
} }
propDecls = append(propDecls, decls.NewVar(ScopedVarName, root.ExprType())) propDecls = append(propDecls, decls.NewVar(ScopedVarName, root.ExprType()))
propDecls = append(propDecls, decls.NewVar(OldScopedVarName, root.ExprType()))
opts = append(opts, cel.Declarations(propDecls...), cel.HomogeneousAggregateLiterals()) opts = append(opts, cel.Declarations(propDecls...), cel.HomogeneousAggregateLiterals())
opts = append(opts, library.ExtensionLibs...) opts = append(opts, library.ExtensionLibs...)
env, err = env.Extend(opts...) env, err = env.Extend(opts...)
@ -91,32 +104,51 @@ func Compile(s *schema.Structural, isResourceRoot bool) ([]CompilationResult, er
// compResults is the return value which saves a list of compilation results in the same order as x-kubernetes-validations rules. // compResults is the return value which saves a list of compilation results in the same order as x-kubernetes-validations rules.
compResults := make([]CompilationResult, len(celRules)) compResults := make([]CompilationResult, len(celRules))
for i, rule := range celRules { for i, rule := range celRules {
var compilationResult CompilationResult compResults[i] = compileRule(rule, env)
if len(strings.TrimSpace(rule.Rule)) == 0 {
// include a compilation result, but leave both program and error nil per documented return semantics of this
// function
} else {
ast, issues := env.Compile(rule.Rule)
if issues != nil {
compilationResult.Error = &Error{ErrorTypeInvalid, "compilation failed: " + issues.String()}
} else if !proto.Equal(ast.ResultType(), decls.Bool) {
compilationResult.Error = &Error{ErrorTypeInvalid, "cel expression must evaluate to a bool"}
} else {
prog, err := env.Program(ast, cel.EvalOptions(cel.OptOptimize))
if err != nil {
compilationResult.Error = &Error{ErrorTypeInvalid, "program instantiation failed: " + err.Error()}
} else {
compilationResult.Program = prog
}
}
}
compResults[i] = compilationResult
} }
return compResults, nil return compResults, nil
} }
func compileRule(rule apiextensions.ValidationRule, env *cel.Env) (compilationResult CompilationResult) {
if len(strings.TrimSpace(rule.Rule)) == 0 {
// include a compilation result, but leave both program and error nil per documented return semantics of this
// function
return
}
ast, issues := env.Compile(rule.Rule)
if issues != nil {
compilationResult.Error = &Error{ErrorTypeInvalid, "compilation failed: " + issues.String()}
return
}
if !proto.Equal(ast.ResultType(), decls.Bool) {
compilationResult.Error = &Error{ErrorTypeInvalid, "cel expression must evaluate to a bool"}
return
}
checkedExpr, err := cel.AstToCheckedExpr(ast)
if err != nil {
// should be impossible since env.Compile returned no issues
compilationResult.Error = &Error{ErrorTypeInternal, "unexpected compilation error: " + err.Error()}
return
}
for _, ref := range checkedExpr.ReferenceMap {
if ref.Name == OldScopedVarName {
compilationResult.TransitionRule = true
break
}
}
prog, err := env.Program(ast, cel.EvalOptions(cel.OptOptimize))
if err != nil {
compilationResult.Error = &Error{ErrorTypeInvalid, "program instantiation failed: " + err.Error()}
return
}
compilationResult.Program = prog
return
}
// generateUniqueSelfTypeName creates a placeholder type name to use in a CEL programs for cases // generateUniqueSelfTypeName creates a placeholder type name to use in a CEL programs for cases
// where we do not wish to expose a stable type name to CEL validator rule authors. For this to effectively prevent // where we do not wish to expose a stable type name to CEL validator rule authors. For this to effectively prevent
// developers from depending on the generated name (i.e. using it in CEL programs), it must be changed each time a // developers from depending on the generated name (i.e. using it in CEL programs), it must be changed each time a

View File

@ -17,6 +17,7 @@ limitations under the License.
package cel package cel
import ( import (
"fmt"
"strings" "strings"
"testing" "testing"
@ -24,24 +25,106 @@ import (
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
) )
type validationMatch struct { type validationMatcher interface {
matches(cr CompilationResult) bool
String() string
}
type allMatcher []validationMatcher
func matchesAll(matchers ...validationMatcher) validationMatcher {
return allMatcher(matchers)
}
func (m allMatcher) matches(cr CompilationResult) bool {
for _, each := range m {
if !each.matches(cr) {
return false
}
}
return true
}
func (m allMatcher) String() string {
if len(m) == 0 {
return "any result"
}
var b strings.Builder
for i, each := range m {
b.WriteString(each.String())
if i < len(m)-1 {
b.WriteString(" and ")
}
}
return b.String()
}
type fnMatcher struct {
fn func(CompilationResult) bool
msg string
}
func (m fnMatcher) matches(cr CompilationResult) bool {
return m.fn(cr)
}
func (m fnMatcher) String() string {
return m.msg
}
type errorMatcher struct {
errorType ErrorType errorType ErrorType
contains string contains string
} }
func invalidError(contains string) validationMatch { func invalidError(contains string) validationMatcher {
return validationMatch{errorType: ErrorTypeInvalid, contains: contains} return errorMatcher{errorType: ErrorTypeInvalid, contains: contains}
} }
func (v validationMatch) matches(err *Error) bool { func (v errorMatcher) matches(cr CompilationResult) bool {
return err.Type == v.errorType && strings.Contains(err.Error(), v.contains) return cr.Error != nil && cr.Error.Type == v.errorType && strings.Contains(cr.Error.Error(), v.contains)
}
func (v errorMatcher) String() string {
return fmt.Sprintf("has error of type %q containing string %q", v.errorType, v.contains)
}
type noErrorMatcher struct{}
func noError() validationMatcher {
return noErrorMatcher{}
}
func (noErrorMatcher) matches(cr CompilationResult) bool {
return cr.Error == nil
}
func (noErrorMatcher) String() string {
return "no error"
}
type transitionRuleMatcher bool
func transitionRule(t bool) validationMatcher {
return transitionRuleMatcher(t)
}
func (v transitionRuleMatcher) matches(cr CompilationResult) bool {
return cr.TransitionRule == bool(v)
}
func (v transitionRuleMatcher) String() string {
if v {
return "is a transition rule"
}
return "is not a transition rule"
} }
func TestCelCompilation(t *testing.T) { func TestCelCompilation(t *testing.T) {
cases := []struct { cases := []struct {
name string name string
input schema.Structural input schema.Structural
expectedErrors []validationMatch expectedResults []validationMatcher
}{ }{
{ {
name: "valid object", name: "valid object",
@ -70,6 +153,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid for string", name: "valid for string",
@ -86,6 +172,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid for byte", name: "valid for byte",
@ -105,6 +194,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid for boolean", name: "valid for boolean",
@ -121,6 +213,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid for integer", name: "valid for integer",
@ -137,6 +232,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid for number", name: "valid for number",
@ -153,6 +251,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid nested object of object", name: "valid nested object of object",
@ -186,6 +287,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid nested object of array", name: "valid nested object of array",
@ -219,6 +323,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid nested array of array", name: "valid nested array of array",
@ -250,6 +357,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid nested array of object", name: "valid nested array of object",
@ -288,6 +398,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "valid map", name: "valid map",
@ -313,6 +426,9 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
},
}, },
{ {
name: "invalid checking for number", name: "invalid checking for number",
@ -329,7 +445,7 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedErrors: []validationMatch{ expectedResults: []validationMatcher{
invalidError("compilation failed"), invalidError("compilation failed"),
}, },
}, },
@ -348,7 +464,7 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedErrors: []validationMatch{ expectedResults: []validationMatcher{
invalidError("compilation failed"), invalidError("compilation failed"),
}, },
}, },
@ -400,6 +516,11 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedResults: []validationMatcher{
noError(),
noError(),
noError(),
},
}, },
{ {
name: "invalid for escaping", name: "invalid for escaping",
@ -449,12 +570,69 @@ func TestCelCompilation(t *testing.T) {
}, },
}, },
}, },
expectedErrors: []validationMatch{ expectedResults: []validationMatcher{
invalidError("undefined field 'namespace'"), invalidError("undefined field 'namespace'"),
invalidError("undefined field 'if'"), invalidError("undefined field 'if'"),
invalidError("found no matching overload"), invalidError("found no matching overload"),
}, },
}, },
{
name: "transition rule identified",
input: schema.Structural{
Generic: schema.Generic{
Type: "integer",
},
Extensions: schema.Extensions{
XValidations: apiextensions.ValidationRules{
{Rule: "self > 0"},
{Rule: "self >= oldSelf"},
},
},
},
expectedResults: []validationMatcher{
matchesAll(noError(), transitionRule(false)),
matchesAll(noError(), transitionRule(true)),
},
},
{
name: "whitespace-only rule",
input: schema.Structural{
Generic: schema.Generic{
Type: "object",
},
Extensions: schema.Extensions{
XValidations: apiextensions.ValidationRules{
{Rule: " \t"},
},
},
},
expectedResults: []validationMatcher{
matchesAll(
noError(),
fnMatcher{
msg: "program is nil",
fn: func(cr CompilationResult) bool {
return cr.Program == nil
},
}),
},
},
{
name: "expression must evaluate to bool",
input: schema.Structural{
Generic: schema.Generic{
Type: "object",
},
Extensions: schema.Extensions{
XValidations: apiextensions.ValidationRules{
{Rule: "42"},
},
},
},
expectedResults: []validationMatcher{
invalidError("must evaluate to a bool"),
},
},
} }
for _, tt := range cases { for _, tt := range cases {
@ -464,26 +642,17 @@ func TestCelCompilation(t *testing.T) {
t.Errorf("Expected no error, but got: %v", err) t.Errorf("Expected no error, but got: %v", err)
} }
seenErrs := make([]bool, len(compilationResults)) if len(compilationResults) != len(tt.input.XValidations) {
t.Fatalf("compilation did not produce one result per rule")
for _, expectedError := range tt.expectedErrors {
found := false
for i, result := range compilationResults {
if expectedError.matches(result.Error) && !seenErrs[i] {
found = true
seenErrs[i] = true
break
}
} }
if !found { if len(compilationResults) != len(tt.expectedResults) {
t.Errorf("expected error: %v", expectedError) t.Fatalf("one test expectation per rule is required")
}
} }
for i, seen := range seenErrs { for i, expectedResult := range tt.expectedResults {
if !seen && compilationResults[i].Error != nil { if !expectedResult.matches(compilationResults[i]) {
t.Errorf("unexpected error: %v", compilationResults[i].Error) t.Errorf("result %d does not match expectation: %v", i+1, expectedResult)
} }
} }
}) })

View File

@ -135,6 +135,11 @@ func (s *Validator) validateExpressions(fldPath *field.Path, sts *schema.Structu
// rule is empty // rule is empty
continue continue
} }
if compiled.TransitionRule {
// transition rules are evaluated only if there is a comparable existing value
errs = append(errs, field.InternalError(fldPath, fmt.Errorf("oldSelf validation not implemented")))
continue // todo: wire oldObj parameter
}
evalResult, _, err := compiled.Program.Eval(activation) evalResult, _, err := compiled.Program.Eval(activation)
if err != nil { if err != nil {
// see types.Err for list of well defined error types // see types.Err for list of well defined error types