Merge pull request #134368 from aaron-prindle/error-matcher-path-normalization

feat: add path normalization support to error matcher
2026-02-22 15:19:12 +00:00 · 2025-10-03 17:00:58 -07:00
parent ab7e45228f 03da38a802
commit 054af8e91b
2 changed files with 200 additions and 9 deletions
--- 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()

--- 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(),