From 00c30b354df7afc15258ecdbdd629555d200de5a Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Mon, 7 Jul 2025 17:15:41 -0500 Subject: [PATCH] Reduce nested conditionals around validation rule errors --- .../pkg/apiserver/schema/cel/validation.go | 69 ++++++++++--------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go index 575fd5e2e9a..d441bbdc471 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go @@ -451,42 +451,49 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path } continue } - if evalResult != types.True { - currentFldPath := fldPath - if len(compiled.NormalizedRuleFieldPath) > 0 { - currentFldPath = currentFldPath.Child(compiled.NormalizedRuleFieldPath) - } - addErr := func(e *field.Error) { - if !compiled.UsesOldSelf && correlation.shouldRatchetError() { - warning.AddWarning(ctx, "", e.Error()) - } else { - errs = append(errs, e) - } - } + if evalResult == types.True { + continue + } - if compiled.MessageExpression != nil { - messageExpression, newRemainingBudget, msgErr := evalMessageExpression(ctx, compiled.MessageExpression, rule.MessageExpression, activation, remainingBudget) - if msgErr != nil { - if msgErr.Type == cel.ErrorTypeInternal { - addErr(field.InternalError(currentFldPath, msgErr)) - return errs, -1 - } else if msgErr.Type == cel.ErrorTypeInvalid { - addErr(field.Invalid(currentFldPath, sts.Type, msgErr.Error())) - return errs, -1 - } else { - klog.V(2).ErrorS(msgErr, "messageExpression evaluation failed") - addErr(fieldErrorForReason(currentFldPath, sts.Type, ruleMessageOrDefault(rule), rule.Reason)) - remainingBudget = newRemainingBudget - } - } else { - addErr(fieldErrorForReason(currentFldPath, sts.Type, messageExpression, rule.Reason)) - remainingBudget = newRemainingBudget - } + // Prepare a field error describing why the expression evaluated to False. + // Its detail may come from another expression that might fail to evaluate or exceed the budget. + + currentFldPath := fldPath + if len(compiled.NormalizedRuleFieldPath) > 0 { + currentFldPath = currentFldPath.Child(compiled.NormalizedRuleFieldPath) + } + + addErr := func(e *field.Error) { + if !compiled.UsesOldSelf && correlation.shouldRatchetError() { + warning.AddWarning(ctx, "", e.Error()) } else { - addErr(fieldErrorForReason(currentFldPath, sts.Type, ruleMessageOrDefault(rule), rule.Reason)) + errs = append(errs, e) } } + + detail, ok := "", false + if compiled.MessageExpression != nil { + messageExpression, newRemainingBudget, msgErr := evalMessageExpression(ctx, compiled.MessageExpression, rule.MessageExpression, activation, remainingBudget) + if msgErr == nil { + detail, ok = messageExpression, true + remainingBudget = newRemainingBudget + } else if msgErr.Type == cel.ErrorTypeInternal { + addErr(field.InternalError(currentFldPath, msgErr)) + return errs, -1 + } else if msgErr.Type == cel.ErrorTypeInvalid { + addErr(field.Invalid(currentFldPath, sts.Type, msgErr.Error())) + return errs, -1 + } else { + klog.V(2).ErrorS(msgErr, "messageExpression evaluation failed") + remainingBudget = newRemainingBudget + } + } + if !ok { + detail = ruleMessageOrDefault(rule) + } + + addErr(fieldErrorForReason(currentFldPath, sts.Type, detail, rule.Reason)) } return errs, remainingBudget }