Skip to content

Commit

Permalink
refactor: rename TransitionRule to UsesOldSelf
Browse files Browse the repository at this point in the history
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

Kubernetes-commit: 18adc309337f91e2a8c5deb29acc5e90d618f668
  • Loading branch information
alexzielenski authored and k8s-publishing-bot committed Oct 24, 2023
1 parent 915de7b commit 2c1c704
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pkg/apis/apiextensions/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
}
}
}
if cr.TransitionRule {
if cr.UsesOldSelf {
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)))
}
Expand Down
7 changes: 3 additions & 4 deletions pkg/apiserver/schema/cel/compilation.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@ const (
type CompilationResult struct {
Program cel.Program
Error *apiservercel.Error
// If true, the compiled expression contains a reference to the identifier "oldSelf", and its corresponding rule
// is implicitly a transition rule.
TransitionRule bool
// If true, the compiled expression contains a reference to the identifier "oldSelf".
UsesOldSelf bool
// Represents the worst-case cost of the compiled expression in terms of CEL's cost units, as used by cel.EstimateCost.
MaxCost uint64
// MaxCardinality represents the worse case number of times this validation rule could be invoked if contained under an
Expand Down Expand Up @@ -190,7 +189,7 @@ func compileRule(s *schema.Structural, rule apiextensions.ValidationRule, envSet
}
for _, ref := range checkedExpr.ReferenceMap {
if ref.Name == OldScopedVarName {
compilationResult.TransitionRule = true
compilationResult.UsesOldSelf = true
break
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/schema/cel/compilation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func transitionRule(t bool) validationMatcher {
}

func (v transitionRuleMatcher) matches(cr CompilationResult) bool {
return cr.TransitionRule == bool(v)
return cr.UsesOldSelf == bool(v)
}

func (v transitionRuleMatcher) String() string {
Expand Down
6 changes: 3 additions & 3 deletions pkg/apiserver/schema/cel/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
var activationFactory func(*schema.Structural, interface{}, interface{}) interpreter.Activation = validationActivationWithoutOldSelf
for _, rule := range compiledRules {
if rule.TransitionRule {
if rule.UsesOldSelf {
activationFactory = validationActivationWithOldSelf
break
}
Expand Down Expand Up @@ -300,7 +300,7 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
// rule is empty
continue
}
if compiled.TransitionRule && oldObj == nil {
if compiled.UsesOldSelf && oldObj == nil {
// transition rules are evaluated only if there is a comparable existing value
continue
}
Expand Down Expand Up @@ -344,7 +344,7 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
}

addErr := func(e *field.Error) {
if !compiled.TransitionRule && correlation.shouldRatchetError() {
if !compiled.UsesOldSelf && correlation.shouldRatchetError() {
warning.AddWarning(ctx, "", e.Error())
} else {
errs = append(errs, e)
Expand Down

0 comments on commit 2c1c704

Please sign in to comment.