ratcheting-cel: use Optional[T] for oldSelf when optionalOldSelf is true

This commit is contained in:
Alexander Zielenski 2023-11-03 15:22:07 -07:00
parent 5edb27aa38
commit eef1515815
5 changed files with 616 additions and 16 deletions

View File

@ -23,6 +23,7 @@ import (
"github.com/google/cel-go/cel"
"github.com/google/cel-go/checker"
"github.com/google/cel-go/common/types"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
@ -126,7 +127,7 @@ func Compile(s *schema.Structural, declType *apiservercel.DeclType, perCallLimit
}
celRules := s.Extensions.XValidations
envSet, err := prepareEnvSet(baseEnvSet, declType)
oldSelfEnvSet, optionalOldSelfEnvSet, err := prepareEnvSet(baseEnvSet, declType)
if err != nil {
return nil, err
}
@ -135,15 +136,20 @@ func Compile(s *schema.Structural, declType *apiservercel.DeclType, perCallLimit
compResults := make([]CompilationResult, len(celRules))
maxCardinality := maxCardinality(declType.MinSerializedSize)
for i, rule := range celRules {
compResults[i] = compileRule(s, rule, envSet, envLoader, estimator, maxCardinality, perCallLimit)
ruleEnvSet := oldSelfEnvSet
if rule.OptionalOldSelf != nil && *rule.OptionalOldSelf {
ruleEnvSet = optionalOldSelfEnvSet
}
compResults[i] = compileRule(s, rule, ruleEnvSet, envLoader, estimator, maxCardinality, perCallLimit)
}
return compResults, nil
}
func prepareEnvSet(baseEnvSet *environment.EnvSet, declType *apiservercel.DeclType) (*environment.EnvSet, error) {
func prepareEnvSet(baseEnvSet *environment.EnvSet, declType *apiservercel.DeclType) (oldSelfEnvSet *environment.EnvSet, optionalOldSelfEnvSet *environment.EnvSet, err error) {
scopedType := declType.MaybeAssignTypeName(generateUniqueSelfTypeName())
return baseEnvSet.Extend(
oldSelfEnvSet, err = baseEnvSet.Extend(
environment.VersionedOptions{
// Feature epoch was actually 1.23, but we artificially set it to 1.0 because these
// options should always be present.
@ -162,6 +168,34 @@ func prepareEnvSet(baseEnvSet *environment.EnvSet, declType *apiservercel.DeclTy
},
},
)
if err != nil {
return nil, nil, err
}
optionalOldSelfEnvSet, err = baseEnvSet.Extend(
environment.VersionedOptions{
// Feature epoch was actually 1.23, but we artificially set it to 1.0 because these
// options should always be present.
IntroducedVersion: version.MajorMinor(1, 0),
EnvOptions: []cel.EnvOption{
cel.Variable(ScopedVarName, scopedType.CelType()),
},
DeclTypes: []*apiservercel.DeclType{
scopedType,
},
},
environment.VersionedOptions{
IntroducedVersion: version.MajorMinor(1, 24),
EnvOptions: []cel.EnvOption{
cel.Variable(OldScopedVarName, types.NewOptionalType(scopedType.CelType())),
},
},
)
if err != nil {
return nil, nil, err
}
return oldSelfEnvSet, optionalOldSelfEnvSet, nil
}
func compileRule(s *schema.Structural, rule apiextensions.ValidationRule, envSet *environment.EnvSet, envLoader EnvLoader, estimator *library.CostEstimator, maxCardinality uint64, perCallLimit uint64) (compilationResult CompilationResult) {

View File

@ -29,10 +29,14 @@ import (
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
"k8s.io/apimachinery/pkg/util/version"
celconfig "k8s.io/apiserver/pkg/apis/cel"
"k8s.io/apiserver/pkg/cel"
"k8s.io/apiserver/pkg/cel/environment"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/utils/ptr"
)
const (
@ -151,12 +155,99 @@ func (v transitionRuleMatcher) String() string {
}
func TestCelCompilation(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CRDValidationRatcheting, true)()
cases := []struct {
name string
input schema.Structural
expectedResults []validationMatcher
unmodified bool
}{
{
name: "optional primitive transition rule type checking",
input: schema.Structural{
Generic: schema.Generic{
Type: "integer",
},
Extensions: schema.Extensions{
XValidations: apiextensions.ValidationRules{
{Rule: "self >= oldSelf.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self >= oldSelf.orValue(1)", OptionalOldSelf: ptr.To(true)},
{Rule: "oldSelf.hasValue() ? self >= oldSelf.value() : true", OptionalOldSelf: ptr.To(true)},
{Rule: "self >= oldSelf", OptionalOldSelf: ptr.To(true)},
{Rule: "self >= oldSelf.orValue('')", OptionalOldSelf: ptr.To(true)},
},
},
},
expectedResults: []validationMatcher{
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(invalidError("optional")),
matchesAll(invalidError("orValue")),
},
},
{
name: "optional complex transition rule type checking",
input: schema.Structural{
Generic: schema.Generic{
Type: "object",
},
Properties: map[string]schema.Structural{
"i": {Generic: schema.Generic{Type: "integer"}},
"b": {Generic: schema.Generic{Type: "boolean"}},
"s": {Generic: schema.Generic{Type: "string"}},
"a": {
Generic: schema.Generic{Type: "array"},
Items: &schema.Structural{Generic: schema.Generic{Type: "integer"}},
},
"o": {
Generic: schema.Generic{Type: "object"},
Properties: map[string]schema.Structural{
"i": {Generic: schema.Generic{Type: "integer"}},
"b": {Generic: schema.Generic{Type: "boolean"}},
"s": {Generic: schema.Generic{Type: "string"}},
"a": {
Generic: schema.Generic{Type: "array"},
Items: &schema.Structural{Generic: schema.Generic{Type: "integer"}},
},
"o": {
Generic: schema.Generic{Type: "object"},
},
},
},
},
Extensions: schema.Extensions{
XValidations: apiextensions.ValidationRules{
{Rule: "self.i >= oldSelf.i.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self.s == oldSelf.s.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self.b == oldSelf.b.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self.o == oldSelf.o.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self.o.i >= oldSelf.o.i.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self.o.s == oldSelf.o.s.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self.o.b == oldSelf.o.b.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self.o.o == oldSelf.o.o.value()", OptionalOldSelf: ptr.To(true)},
{Rule: "self.o.i >= oldSelf.o.i.orValue(1)", OptionalOldSelf: ptr.To(true)},
{Rule: "oldSelf.hasValue() ? self.o.i >= oldSelf.o.i.value() : true", OptionalOldSelf: ptr.To(true)},
{Rule: "self.o.i >= oldSelf.o.i", OptionalOldSelf: ptr.To(true)},
{Rule: "self.o.i >= oldSelf.o.s.orValue(0)", OptionalOldSelf: ptr.To(true)},
},
},
},
expectedResults: []validationMatcher{
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(noError(), transitionRule(true)),
matchesAll(invalidError("optional")),
matchesAll(invalidError("orValue")),
},
},
{
name: "valid object",
input: schema.Structural{

View File

@ -32,15 +32,18 @@ import (
"github.com/google/cel-go/interpreter"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model"
"k8s.io/apiextensions-apiserver/pkg/features"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/cel"
"k8s.io/apiserver/pkg/cel/common"
"k8s.io/apiserver/pkg/cel/environment"
"k8s.io/apiserver/pkg/cel/metrics"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/apiserver/pkg/warning"
celconfig "k8s.io/apiserver/pkg/apis/cel"
@ -65,9 +68,10 @@ type Validator struct {
// custom resource being validated, or the root of an XEmbeddedResource object.
isResourceRoot bool
// celActivationFactory produces an Activation, which resolves identifiers (e.g. self and
// oldSelf) to CEL values.
celActivationFactory func(sts *schema.Structural, obj, oldObj interface{}) interpreter.Activation
// celActivationFactory produces a Activations, which resolve identifiers
// (e.g. self and oldSelf) to CEL values. One activation must be produced
// for each of the cases when oldSelf is optional and non-optional.
celActivationFactory func(sts *schema.Structural, obj, oldObj interface{}) (activation interpreter.Activation, optionalOldSelfActivation interpreter.Activation)
}
// NewValidator returns compiles all the CEL programs defined in x-kubernetes-validations extensions
@ -122,7 +126,7 @@ func validator(s *schema.Structural, isResourceRoot bool, declType *cel.DeclType
additionalPropertiesValidator = validator(s.AdditionalProperties.Structural, s.AdditionalProperties.Structural.XEmbeddedResource, declType.ElemType, perCallLimit)
}
if len(compiledRules) > 0 || err != nil || itemsValidator != nil || additionalPropertiesValidator != nil || len(propertiesValidators) > 0 {
var activationFactory func(*schema.Structural, interface{}, interface{}) interpreter.Activation = validationActivationWithoutOldSelf
activationFactory := validationActivationWithoutOldSelf
for _, rule := range compiledRules {
if rule.UsesOldSelf {
activationFactory = validationActivationWithOldSelf
@ -289,7 +293,7 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
if s.isResourceRoot {
sts = model.WithTypeAndObjectMeta(sts)
}
activation := s.celActivationFactory(sts, obj, oldObj)
activation, optionalOldSelfActivation := s.celActivationFactory(sts, obj, oldObj)
for i, compiled := range s.compiledRules {
rule := sts.XValidations[i]
if compiled.Error != nil {
@ -300,11 +304,29 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
// rule is empty
continue
}
// If ratcheting is enabled, allow rule with oldSelf to evaluate
// when `optionalOldSelf` is set to true
optionalOldSelfRule := ptr.Deref(rule.OptionalOldSelf, false)
if compiled.UsesOldSelf && oldObj == nil {
// transition rules are evaluated only if there is a comparable existing value
continue
// But if the rule uses optional oldSelf and gate is enabled we allow
// the rule to be evaluated
if !utilfeature.DefaultFeatureGate.Enabled(features.CRDValidationRatcheting) {
continue
}
if !optionalOldSelfRule {
continue
}
}
evalResult, evalDetails, err := compiled.Program.ContextEval(ctx, activation)
ruleActivation := activation
if optionalOldSelfRule {
ruleActivation = optionalOldSelfActivation
}
evalResult, evalDetails, err := compiled.Program.ContextEval(ctx, ruleActivation)
if evalDetails == nil {
errs = append(errs, field.InternalError(fldPath, fmt.Errorf("runtime cost could not be calculated for validation rule: %v, no further validation rules will be run", ruleErrorString(rule))))
return errs, -1
@ -622,21 +644,31 @@ type validationActivation struct {
hasOldSelf bool
}
func validationActivationWithOldSelf(sts *schema.Structural, obj, oldObj interface{}) interpreter.Activation {
func validationActivationWithOldSelf(sts *schema.Structural, obj, oldObj interface{}) (activation interpreter.Activation, optionalOldSelfActivation interpreter.Activation) {
va := &validationActivation{
self: UnstructuredToVal(obj, sts),
}
optionalVA := &validationActivation{
self: va.self,
hasOldSelf: true, // this means the oldSelf variable is defined for CEL to reference, not that it has a value
oldSelf: types.OptionalNone,
}
if oldObj != nil {
va.oldSelf = UnstructuredToVal(oldObj, sts) // +k8s:verify-mutation:reason=clone
va.hasOldSelf = true // +k8s:verify-mutation:reason=clone
va.hasOldSelf = true
optionalVA.oldSelf = types.OptionalOf(va.oldSelf) // +k8s:verify-mutation:reason=clone
}
return va
return va, optionalVA
}
func validationActivationWithoutOldSelf(sts *schema.Structural, obj, _ interface{}) interpreter.Activation {
return &validationActivation{
func validationActivationWithoutOldSelf(sts *schema.Structural, obj, _ interface{}) (interpreter.Activation, interpreter.Activation) {
res := &validationActivation{
self: UnstructuredToVal(obj, sts),
}
return res, res
}
func (a *validationActivation) ResolveName(name string) (interface{}, bool) {

View File

@ -31,16 +31,20 @@ import (
"github.com/stretchr/testify/require"
"k8s.io/klog/v2"
"k8s.io/kube-openapi/pkg/validation/strfmt"
"k8s.io/utils/ptr"
apiextensionsinternal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apimachinery/pkg/util/yaml"
celconfig "k8s.io/apiserver/pkg/apis/cel"
"k8s.io/apiserver/pkg/cel/common"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/apiserver/pkg/warning"
featuregatetesting "k8s.io/component-base/featuregate/testing"
)
// TestValidationExpressions tests CEL integration with custom resource values and OpenAPIv3.
@ -3618,6 +3622,401 @@ func TestRatcheting(t *testing.T) {
}
}
// Runs transition rule cases with OptionalOldSelf set to true on the schema
func TestOptionalOldSelf(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CRDValidationRatcheting, true)()
tests := []struct {
name string
schema *schema.Structural
obj interface{}
oldObj interface{}
errors []string // strings that error message must contain
}{
{
name: "allow new value if old value is null",
obj: map[string]interface{}{
"foo": "bar",
},
schema: withRulePtr(objectTypePtr(map[string]schema.Structural{
"foo": stringType,
}), "self.foo == 'not bar' || !oldSelf.hasValue()"),
},
{
name: "block new value if old value is not null",
obj: map[string]interface{}{
"foo": "invalid",
},
oldObj: map[string]interface{}{
"foo": "bar",
},
schema: withRulePtr(objectTypePtr(map[string]schema.Structural{
"foo": stringType,
}), "self.foo == 'valid' || !oldSelf.hasValue()"),
errors: []string{"failed rule"},
},
{
name: "allow invalid new value if old value is also invalid",
obj: map[string]interface{}{
"foo": "invalid again",
},
oldObj: map[string]interface{}{
"foo": "invalid",
},
schema: withRulePtr(objectTypePtr(map[string]schema.Structural{
"foo": stringType,
}), "self.foo == 'valid' || (oldSelf.hasValue() && oldSelf.value().foo != 'valid')"),
},
{
name: "allow invalid new value if old value is also invalid with chained optionals",
obj: map[string]interface{}{
"foo": "invalid again",
},
oldObj: map[string]interface{}{
"foo": "invalid",
},
schema: withRulePtr(objectTypePtr(map[string]schema.Structural{
"foo": stringType,
}), "self.foo == 'valid' || oldSelf.foo.orValue('') != 'valid'"),
},
{
name: "block invalid new value if old value is valid",
obj: map[string]interface{}{
"foo": "invalid",
},
oldObj: map[string]interface{}{
"foo": "valid",
},
schema: withRulePtr(objectTypePtr(map[string]schema.Structural{
"foo": stringType,
}), "self.foo == 'valid' || (oldSelf.hasValue() && oldSelf.value().foo != 'valid')"),
errors: []string{"failed rule"},
},
{
name: "create: new min or allow higher than oldValue",
obj: 10,
schema: cloneWithRule(&integerType, "self >= 10 || (oldSelf.hasValue() && oldSelf.value() <= self)"),
},
{
name: "block create: new min or allow higher than oldValue",
obj: 9,
// Can't use != null because type is integer and no overload
// workaround by comparing type, but kinda hacky
schema: cloneWithRule(&integerType, "self >= 10 || (oldSelf.hasValue() && oldSelf.value() <= self)"),
errors: []string{"failed rule"},
},
{
name: "update: new min or allow higher than oldValue",
obj: 10,
oldObj: 5,
schema: cloneWithRule(&integerType, "self >= 10 || (oldSelf.hasValue() && oldSelf.value() <= self)"),
},
{
name: "ratchet update: new min or allow higher than oldValue",
obj: 9,
oldObj: 5,
schema: cloneWithRule(&integerType, "self >= 10 || (oldSelf.hasValue() && oldSelf.value() <= self)"),
},
{
name: "ratchet noop update: new min or allow higher than oldValue",
obj: 5,
oldObj: 5,
schema: cloneWithRule(&integerType, "self >= 10 || (oldSelf.hasValue() && oldSelf.value() <= self)"),
},
{
name: "block update: new min or allow higher than oldValue",
obj: 4,
oldObj: 5,
schema: cloneWithRule(&integerType, "self >= 10 || (oldSelf.hasValue() && oldSelf.value() <= self)"),
errors: []string{"failed rule"},
},
}
for _, tt := range tests {
tt := tt
tp := true
for i := range tt.schema.XValidations {
tt.schema.XValidations[i].OptionalOldSelf = &tp
}
t.Run(tt.name, func(t *testing.T) {
// t.Parallel()
ctx := context.TODO()
celValidator := validator(tt.schema, true, model.SchemaDeclType(tt.schema, false), celconfig.PerCallLimit)
if celValidator == nil {
t.Fatal("expected non nil validator")
}
errs, _ := celValidator.Validate(ctx, field.NewPath("root"), tt.schema, tt.obj, tt.oldObj, math.MaxInt)
unmatched := map[string]struct{}{}
for _, e := range tt.errors {
unmatched[e] = struct{}{}
}
for _, err := range errs {
if err.Type != field.ErrorTypeInvalid {
t.Errorf("expected only ErrorTypeInvalid errors, but got: %v", err)
continue
}
matched := false
for expected := range unmatched {
if strings.Contains(err.Error(), expected) {
delete(unmatched, expected)
matched = true
break
}
}
if !matched {
t.Errorf("expected error to contain one of %v, but got: %v", unmatched, err)
}
}
if len(unmatched) > 0 {
t.Errorf("expected errors %v", unmatched)
}
})
}
}
// Shows that type(oldSelf) == null_type works for all supported OpenAPI types
// both when oldSelf is null and when it is not null
func TestOptionalOldSelfCheckForNull(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CRDValidationRatcheting, true)()
tests := []struct {
name string
schema schema.Structural
obj interface{}
oldObj interface{}
}{
{
name: "object",
obj: map[string]interface{}{
"foo": "bar",
},
oldObj: map[string]interface{}{
"foo": "baz",
},
schema: withRule(objectType(map[string]schema.Structural{
"foo": stringType,
}), `!oldSelf.hasValue() || self.foo == "bar"`),
},
{
name: "object - conditional field",
obj: map[string]interface{}{
"foo": "bar",
},
oldObj: map[string]interface{}{
"foo": "baz",
},
schema: withRule(objectType(map[string]schema.Structural{
"foo": stringType,
}), `self.foo != "bar" || oldSelf.?foo.orValue("baz") == "baz"`),
},
{
name: "string",
obj: "bar",
oldObj: "baz",
schema: withRule(stringType, `
!oldSelf.hasValue() || self == "bar"
`),
},
{
name: "integer",
obj: 1,
oldObj: 2,
schema: withRule(integerType, `
!oldSelf.hasValue() || self == 1
`),
},
{
name: "number",
obj: 1.1,
oldObj: 2.2,
schema: withRule(numberType, `
!oldSelf.hasValue() || self == 1.1
`),
},
{
name: "boolean",
obj: true,
oldObj: false,
schema: withRule(booleanType, `
!oldSelf.hasValue() || self == true
`),
},
{
name: "array",
obj: []interface{}{"bar"},
oldObj: []interface{}{"baz"},
schema: withRule(arrayType("", nil, &stringSchema), `
!oldSelf.hasValue() || self[0] == "bar"
`),
},
{
name: "array - conditional index",
obj: []interface{}{},
oldObj: []interface{}{
"baz",
},
schema: withRule(arrayType("", nil, &stringSchema), `
self.size() > 0 || oldSelf[?0].orValue("baz") == "baz"
`),
},
{
name: "set-array",
obj: []interface{}{"bar"},
oldObj: []interface{}{"baz"},
schema: withRule(arrayType("set", nil, &stringSchema), `
!oldSelf.hasValue() || self[0] == "bar"
`),
},
{
name: "map-array",
obj: []interface{}{map[string]interface{}{
"key": "foo",
"value": "bar",
}},
oldObj: []interface{}{map[string]interface{}{
"key": "foo",
"value": "baz",
}},
schema: withRule(arrayType("map", []string{"key"}, objectTypePtr(map[string]schema.Structural{
"key": stringType,
"value": stringType,
})), `
!oldSelf.hasValue() || self[0].value == "bar"
`),
},
}
for _, tt := range tests {
tt := tt
tp := true
for i := range tt.schema.XValidations {
tt.schema.XValidations[i].OptionalOldSelf = &tp
}
t.Run(tt.name, func(t *testing.T) {
ctx := context.TODO()
celValidator := validator(&tt.schema, false, model.SchemaDeclType(&tt.schema, false), celconfig.PerCallLimit)
if celValidator == nil {
t.Fatal("expected non nil validator")
}
t.Run("null old", func(t *testing.T) {
errs, _ := celValidator.Validate(ctx, field.NewPath("root"), &tt.schema, tt.obj, nil, math.MaxInt)
if len(errs) != 0 {
t.Errorf("expected no errors, but got: %v", errs)
}
})
t.Run("non-null old", func(t *testing.T) {
errs, _ := celValidator.Validate(ctx, field.NewPath("root"), &tt.schema, tt.obj, tt.oldObj, math.MaxInt)
if len(errs) != 0 {
t.Errorf("expected no errors, but got: %v", errs)
}
})
})
}
}
// Show that we cant just use oldSelf as if it was unwrapped
func TestOptionalOldSelfIsOptionalType(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CRDValidationRatcheting, true)()
cases := []struct {
name string
schema schema.Structural
obj interface{}
errors []string
}{
{
name: "forbid direct usage of optional integer",
schema: withRule(integerType, `
oldSelf + self > 5
`),
obj: 5,
errors: []string{"no matching overload for '_+_' applied to '(optional(int), int)"},
},
{
name: "forbid direct usage of optional string",
schema: withRule(stringType, `
oldSelf == "foo"
`),
obj: "bar",
errors: []string{"no matching overload for '_==_' applied to '(optional(string), string)"},
},
{
name: "forbid direct usage of optional array",
schema: withRule(arrayType("", nil, &stringSchema), `
oldSelf.all(x, x == x)
`),
obj: []interface{}{"bar"},
errors: []string{"expression of type 'optional(list(string))' cannot be range of a comprehension"},
},
{
name: "forbid direct usage of optional array element",
schema: withRule(arrayType("", nil, &stringSchema), `
oldSelf[0] == "foo"
`),
obj: []interface{}{"bar"},
errors: []string{"found no matching overload for '_==_' applied to '(optional(string), string)"},
},
{
name: "forbid direct usage of optional struct",
schema: withRule(arrayType("map", []string{"key"}, objectTypePtr(map[string]schema.Structural{
"key": stringType,
"value": stringType,
})), `oldSelf.key == "foo"`),
obj: []interface{}{map[string]interface{}{
"key": "bar",
"value": "baz",
}},
errors: []string{"does not support field selection"},
},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
ctx := context.TODO()
for i := range tt.schema.XValidations {
tt.schema.XValidations[i].OptionalOldSelf = ptr.To(true)
}
celValidator := validator(&tt.schema, false, model.SchemaDeclType(&tt.schema, false), celconfig.PerCallLimit)
if celValidator == nil {
t.Fatal("expected non nil validator")
}
errs, _ := celValidator.Validate(ctx, field.NewPath("root"), &tt.schema, tt.obj, tt.obj, math.MaxInt)
unmatched := map[string]struct{}{}
for _, e := range tt.errors {
unmatched[e] = struct{}{}
}
for _, err := range errs {
if err.Type != field.ErrorTypeInvalid {
t.Errorf("expected only ErrorTypeInvalid errors, but got: %v", err)
continue
}
matched := false
for expected := range unmatched {
if strings.Contains(err.Error(), expected) {
delete(unmatched, expected)
matched = true
break
}
}
if !matched {
t.Errorf("expected error to contain one of %v, but got: %v", unmatched, err)
}
}
if len(unmatched) > 0 {
t.Errorf("expected errors %v", unmatched)
}
})
}
}
func genString(n int, c rune) string {
b := strings.Builder{}
for i := 0; i < n; i++ {

View File

@ -1340,6 +1340,50 @@ func TestRatchetingFunctionality(t *testing.T) {
}}},
},
},
{
Name: "CEL Optional OldSelf",
Operations: []ratchetingTestOperation{
updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensionsv1.JSONSchemaProps{
"field": {
Type: "string",
XValidations: []apiextensionsv1.ValidationRule{
{
Rule: "!oldSelf.hasValue()",
Message: "oldSelf must be null",
OptionalOldSelf: ptr(true),
},
},
},
},
}},
applyPatchOperation{
"create instance passes since oldself is null",
myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{
"field": "value",
}},
expectError{
applyPatchOperation{
"update field fails, since oldself is not null",
myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{
"field": "value2",
},
},
},
expectError{
applyPatchOperation{
"noop update field fails, since oldself is not null and transition rules are not ratcheted",
myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{
"field": "value",
},
},
},
},
},
// Features that should not ratchet
{
Name: "AllOf_should_not_ratchet",