From 03da38a80231badc4ec68b6309a445871a8f1bcc Mon Sep 17 00:00:00 2001 From: Aaron Prindle Date: Thu, 2 Oct 2025 00:46:23 +0000 Subject: [PATCH] feat: add path normalization support to error matcher --- .../util/validation/field/error_matcher.go | 87 +++++++++++-- .../validation/field/error_matcher_test.go | 122 ++++++++++++++++++ 2 files changed, 200 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher.go index c6638e10ca2..f0264e50c7d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher.go @@ -23,6 +23,13 @@ import ( "strings" ) +// NormalizationRule holds a pre-compiled regular expression and its replacement string +// for normalizing field paths. +type NormalizationRule struct { + Regexp *regexp.Regexp + Replacement string +} + // ErrorMatcher is a helper for comparing Error objects. type ErrorMatcher struct { // TODO(thockin): consider whether type is ever NOT required, maybe just @@ -37,17 +44,32 @@ type ErrorMatcher struct { matchOrigin bool matchDetail func(want, got string) bool requireOriginWhenInvalid bool + // normalizationRules holds the pre-compiled regex patterns for path normalization. + normalizationRules []NormalizationRule } // Matches returns true if the two Error objects match according to the -// configured criteria. +// configured criteria. When field normalization is configured, only the +// "got" error's field path is normalized (to bring older API versions up +// to the internal/latest format), while "want" is assumed to already be +// in the canonical internal API format. func (m ErrorMatcher) Matches(want, got *Error) bool { if m.matchType && want.Type != got.Type { return false } - if m.matchField && want.Field != got.Field { - return false + if m.matchField { + // Try direct match first (common case) + if want.Field != got.Field { + // Fields don't match, try normalization if rules are configured. + // Only normalize "got" - it may be from an older API version that + // needs to be brought up to the internal/latest format that "want" + // is already in. + if want.Field != m.normalizePath(got.Field) { + return false + } + } } + if m.matchValue && !reflect.DeepEqual(want.BadValue, got.BadValue) { return false } @@ -67,6 +89,18 @@ func (m ErrorMatcher) Matches(want, got *Error) bool { return true } +// normalizePath applies configured path normalization rules. +func (m ErrorMatcher) normalizePath(path string) string { + for _, rule := range m.normalizationRules { + normalized := rule.Regexp.ReplaceAllString(path, rule.Replacement) + if normalized != path { + // Only apply the first matching rule. + return normalized + } + } + return path +} + // Render returns a string representation of the specified Error object, // according to the criteria configured in the ErrorMatcher. func (m ErrorMatcher) Render(e *Error) string { @@ -84,7 +118,11 @@ func (m ErrorMatcher) Render(e *Error) string { } if m.matchField { comma() - buf.WriteString(fmt.Sprintf("Field=%q", e.Field)) + if normalized := m.normalizePath(e.Field); normalized != e.Field { + buf.WriteString(fmt.Sprintf("Field=%q (aka %q)", normalized, e.Field)) + } else { + buf.WriteString(fmt.Sprintf("Field=%q", e.Field)) + } } if m.matchValue { comma() @@ -125,11 +163,39 @@ func (m ErrorMatcher) ByType() ErrorMatcher { } // ByField returns a derived ErrorMatcher which also matches by field path. +// If you need to mutate the field path (e.g. to normalize across versions), +// see ByFieldNormalized. func (m ErrorMatcher) ByField() ErrorMatcher { m.matchField = true return m } +// ByFieldNormalized returns a derived ErrorMatcher which also matches by field path +// after applying normalization rules to the actual (got) error's field path. +// This allows matching field paths from older API versions against the canonical +// internal API format. +// +// The normalization rules are applied ONLY to the "got" error's field path, bringing +// older API version field paths up to the latest/internal format. The "want" error +// is assumed to always be in the internal API format (latest). +// +// The rules slice holds pre-compiled regular expressions and their replacement strings. +// +// Example: +// +// rules := []NormalizationRule{ +// { +// Regexp: regexp.MustCompile(`spec\.devices\.requests\[(\d+)\]\.allocationMode`), +// Replacement: "spec.devices.requests[$1].exactly.allocationMode", +// }, +// } +// matcher := ErrorMatcher{}.ByFieldNormalized(rules) +func (m ErrorMatcher) ByFieldNormalized(rules []NormalizationRule) ErrorMatcher { + m.matchField = true + m.normalizationRules = rules + return m +} + // ByValue returns a derived ErrorMatcher which also matches by the errant // value. func (m ErrorMatcher) ByValue() ErrorMatcher { @@ -194,11 +260,14 @@ type TestIntf interface { } // Test compares two ErrorLists by the criteria configured in this matcher, and -// fails the test if they don't match. If matching by origin is enabled and the -// error has a non-empty origin, a given "want" error can match multiple -// "got" errors, and they will all be consumed. The only exception to this is -// if the matcher got multiple identical (in every way, even those not being -// matched on) errors, which is likely to indicate a bug. +// fails the test if they don't match. The "want" errors are expected to be in +// the internal API format (latest), while "got" errors may be from any API version +// and will be normalized if field normalization rules are configured. +// +// If matching by origin is enabled and the error has a non-empty origin, a given +// "want" error can match multiple "got" errors, and they will all be consumed. +// The only exception to this is if the matcher got multiple identical (in every way, +// even those not being matched on) errors, which is likely to indicate a bug. func (m ErrorMatcher) Test(tb TestIntf, want, got ErrorList) { tb.Helper() diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher_test.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher_test.go index 748b30f7843..45a477ce2a2 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher_test.go @@ -18,6 +18,7 @@ package field import ( "fmt" + "regexp" "strings" "testing" ) @@ -63,6 +64,67 @@ func TestErrorMatcher_Matches(t *testing.T) { wantedErr: baseErr, actualErr: &Error{Field: "other"}, matches: false, + }, { + name: "ByFieldNormalized: older API to latest", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + wantedErr: func() *Error { + e := baseErr() + e.Field = "f[0].x.a" + return e + }, + actualErr: &Error{Field: "f[0].a"}, + matches: true, + }, { + name: "ByFieldNormalized: both latest format", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + wantedErr: func() *Error { + e := baseErr() + e.Field = "f[0].x.a" + return e + }, + actualErr: &Error{Field: "f[0].x.a"}, + matches: true, + }, { + name: "ByFieldNormalized: different index", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + wantedErr: func() *Error { + e := baseErr() + e.Field = "f[0].x.a" + return e + }, + actualErr: &Error{Field: "f[1].a"}, + matches: false, + }, { + name: "ByFieldNormalized: multiple patterns", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.b`), Replacement: "f[$1].x.b"}, + }), + wantedErr: func() *Error { + e := baseErr() + e.Field = "f[2].x.b" + return e + }, + actualErr: &Error{Field: "f[2].b"}, + matches: true, + }, { + name: "ByFieldNormalized: no normalization needed", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + wantedErr: func() *Error { + e := baseErr() + e.Field = "other.field" + return e + }, + actualErr: &Error{Field: "other.field"}, + matches: true, }, { name: "ByValue: match", matcher: ErrorMatcher{}.ByValue(), @@ -308,6 +370,42 @@ func TestErrorMatcher_Test(t *testing.T) { want: ErrorList{Invalid(NewPath("f1"), nil, "")}, got: ErrorList{Invalid(NewPath("f2"), "v", "d")}, expectedErrors: []string{"expected an error matching:", "unmatched error:"}, + }, { + name: "with normalization: older API to latest", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + want: ErrorList{Invalid(NewPath("f").Index(0).Child("x", "a"), nil, "")}, + got: ErrorList{Invalid(NewPath("f").Index(0).Child("a"), "v", "d")}, + }, { + name: "with normalization: both latest", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + want: ErrorList{Invalid(NewPath("f").Index(0).Child("x", "a"), nil, "")}, + got: ErrorList{Invalid(NewPath("f").Index(0).Child("x", "a"), "v", "d")}, + }, { + name: "with normalization: multiple", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.b`), Replacement: "f[$1].x.b"}, + }), + want: ErrorList{ + Invalid(NewPath("f").Index(0).Child("x", "a"), nil, ""), + Invalid(NewPath("f").Index(1).Child("x", "b"), nil, ""), + }, + got: ErrorList{ + Invalid(NewPath("f").Index(0).Child("a"), "v1", "d1"), + Invalid(NewPath("f").Index(1).Child("b"), "v2", "d2"), + }, + }, { + name: "with normalization: no match", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + want: ErrorList{Invalid(NewPath("f").Index(0).Child("x", "a"), nil, "")}, + got: ErrorList{Invalid(NewPath("f").Index(1).Child("a"), "v", "d")}, + expectedErrors: []string{"expected an error matching:", "unmatched error:"}, }, { name: "with origin: single match", matcher: ErrorMatcher{}.ByField().ByOrigin(), @@ -410,6 +508,30 @@ func TestErrorMatcher_Render(t *testing.T) { err: Invalid(NewPath("field"), "value", "detail").WithOrigin("origin"), expected: `{Type="Invalid value", Field="field", Value="value", Origin="origin", Detail="detail"}`, }, + { + name: "with normalization: normalized", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + err: Invalid(NewPath("f").Index(0).Child("a"), "value", "detail"), + expected: `{Field="f[0].x.a" (aka "f[0].a")}`, + }, + { + name: "with normalization: no normalization", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + err: Invalid(NewPath("other", "field"), "value", "detail"), + expected: `{Field="other.field"}`, + }, + { + name: "with normalization: already normalized", + matcher: ErrorMatcher{}.ByFieldNormalized([]NormalizationRule{ + {Regexp: regexp.MustCompile(`f\[(\d+)\]\.a`), Replacement: "f[$1].x.a"}, + }), + err: Invalid(NewPath("f").Index(0).Child("x", "a"), "value", "detail"), + expected: `{Field="f[0].x.a"}`, + }, { name: "requireOriginWhenInvalid with origin", matcher: ErrorMatcher{}.ByOrigin().RequireOriginWhenInvalid(),