refactor: rename TransitionRule to UsesOldSelf

not all rules that use OldSelf are transition rules, and this flag was used to check for oldSelf usage anyway, not specifically whether the rule was a transition rule
This commit is contained in:
Alexander Zielenski 2023-10-24 11:39:46 -07:00
parent 974735854b
commit 18adc30933
4 changed files with 8 additions and 9 deletions

View File

@ -1182,7 +1182,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
} }
} }
} }
if cr.TransitionRule { if cr.UsesOldSelf {
if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil { if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil {
allErrs.CELErrors = append(allErrs.CELErrors, 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))) allErrs.CELErrors = append(allErrs.CELErrors, 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)))
} }

View File

@ -48,9 +48,8 @@ const (
type CompilationResult struct { type CompilationResult struct {
Program cel.Program Program cel.Program
Error *apiservercel.Error Error *apiservercel.Error
// If true, the compiled expression contains a reference to the identifier "oldSelf", and its corresponding rule // If true, the compiled expression contains a reference to the identifier "oldSelf".
// is implicitly a transition rule. UsesOldSelf bool
TransitionRule bool
// Represents the worst-case cost of the compiled expression in terms of CEL's cost units, as used by cel.EstimateCost. // Represents the worst-case cost of the compiled expression in terms of CEL's cost units, as used by cel.EstimateCost.
MaxCost uint64 MaxCost uint64
// MaxCardinality represents the worse case number of times this validation rule could be invoked if contained under an // MaxCardinality represents the worse case number of times this validation rule could be invoked if contained under an
@ -190,7 +189,7 @@ func compileRule(s *schema.Structural, rule apiextensions.ValidationRule, envSet
} }
for _, ref := range checkedExpr.ReferenceMap { for _, ref := range checkedExpr.ReferenceMap {
if ref.Name == OldScopedVarName { if ref.Name == OldScopedVarName {
compilationResult.TransitionRule = true compilationResult.UsesOldSelf = true
break break
} }
} }

View File

@ -140,7 +140,7 @@ func transitionRule(t bool) validationMatcher {
} }
func (v transitionRuleMatcher) matches(cr CompilationResult) bool { func (v transitionRuleMatcher) matches(cr CompilationResult) bool {
return cr.TransitionRule == bool(v) return cr.UsesOldSelf == bool(v)
} }
func (v transitionRuleMatcher) String() string { func (v transitionRuleMatcher) String() string {

View File

@ -124,7 +124,7 @@ func validator(s *schema.Structural, isResourceRoot bool, declType *cel.DeclType
if len(compiledRules) > 0 || err != nil || itemsValidator != nil || additionalPropertiesValidator != nil || len(propertiesValidators) > 0 { if len(compiledRules) > 0 || err != nil || itemsValidator != nil || additionalPropertiesValidator != nil || len(propertiesValidators) > 0 {
var activationFactory func(*schema.Structural, interface{}, interface{}) interpreter.Activation = validationActivationWithoutOldSelf var activationFactory func(*schema.Structural, interface{}, interface{}) interpreter.Activation = validationActivationWithoutOldSelf
for _, rule := range compiledRules { for _, rule := range compiledRules {
if rule.TransitionRule { if rule.UsesOldSelf {
activationFactory = validationActivationWithOldSelf activationFactory = validationActivationWithOldSelf
break break
} }
@ -300,7 +300,7 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
// rule is empty // rule is empty
continue continue
} }
if compiled.TransitionRule && oldObj == nil { if compiled.UsesOldSelf && oldObj == nil {
// transition rules are evaluated only if there is a comparable existing value // transition rules are evaluated only if there is a comparable existing value
continue continue
} }
@ -344,7 +344,7 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
} }
addErr := func(e *field.Error) { addErr := func(e *field.Error) {
if !compiled.TransitionRule && correlation.shouldRatchetError() { if !compiled.UsesOldSelf && correlation.shouldRatchetError() {
warning.AddWarning(ctx, "", e.Error()) warning.AddWarning(ctx, "", e.Error())
} else { } else {
errs = append(errs, e) errs = append(errs, e)