From 02f7dc55d162fcde7377b1cb0c5e2f9e8974548e Mon Sep 17 00:00:00 2001 From: yongruilin Date: Fri, 21 Feb 2025 20:20:47 +0000 Subject: [PATCH] feat: Add Origin field to Error and related methods This change introducing a new field in Error. It would be used in testing to compare the expected errors without matching the detail strings. Co-authored-by: Tim Hockin --- .../pkg/util/validation/field/errors.go | 48 +++++++++++++++---- .../pkg/util/validation/field/errors_test.go | 38 +++++++++++++++ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go index f1634bc0df8..c51c6832a35 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go @@ -33,6 +33,20 @@ type Error struct { Field string BadValue interface{} Detail string + + // Origin uniquely identifies where this error was generated from. It is used in testing to + // compare expected errors against actual errors without relying on exact detail string matching. + // This allows tests to verify the correct validation logic triggered the error + // regardless of how the error message might be formatted or localized. + // + // The value should be either: + // - A simple camelCase identifier (e.g., "maximum", "maxItems") + // - A structured format using "format=" for validation errors related to specific formats + // (e.g., "format=dns-label", "format=qualified-name") + // + // Origin should be set in the most deeply nested validation function that + // can still identify the unique source of the error. + Origin string } var _ error = &Error{} @@ -96,6 +110,12 @@ func (v *Error) ErrorBody() string { return s } +// WithOrigin adds origin information to the FieldError +func (v *Error) WithOrigin(o string) *Error { + v.Origin = o + return v +} + // ErrorType is a machine readable value providing more detail about why // a field is invalid. These values are expected to match 1-1 with // CauseType in api/types.go. @@ -169,32 +189,32 @@ func (t ErrorType) String() string { // TypeInvalid returns a *Error indicating "type is invalid" func TypeInvalid(field *Path, value interface{}, detail string) *Error { - return &Error{ErrorTypeTypeInvalid, field.String(), value, detail} + return &Error{ErrorTypeTypeInvalid, field.String(), value, detail, ""} } // NotFound returns a *Error indicating "value not found". This is // used to report failure to find a requested value (e.g. looking up an ID). func NotFound(field *Path, value interface{}) *Error { - return &Error{ErrorTypeNotFound, field.String(), value, ""} + return &Error{ErrorTypeNotFound, field.String(), value, "", ""} } // Required returns a *Error indicating "value required". This is used // to report required values that are not provided (e.g. empty strings, null // values, or empty arrays). func Required(field *Path, detail string) *Error { - return &Error{ErrorTypeRequired, field.String(), "", detail} + return &Error{ErrorTypeRequired, field.String(), "", detail, ""} } // Duplicate returns a *Error indicating "duplicate value". This is // used to report collisions of values that must be unique (e.g. names or IDs). func Duplicate(field *Path, value interface{}) *Error { - return &Error{ErrorTypeDuplicate, field.String(), value, ""} + return &Error{ErrorTypeDuplicate, field.String(), value, "", ""} } // Invalid returns a *Error indicating "invalid value". This is used // to report malformed values (e.g. failed regex match, too long, out of bounds). func Invalid(field *Path, value interface{}, detail string) *Error { - return &Error{ErrorTypeInvalid, field.String(), value, detail} + return &Error{ErrorTypeInvalid, field.String(), value, detail, ""} } // NotSupported returns a *Error indicating "unsupported value". @@ -209,7 +229,7 @@ func NotSupported[T ~string](field *Path, value interface{}, validValues []T) *E } detail = "supported values: " + strings.Join(quotedValues, ", ") } - return &Error{ErrorTypeNotSupported, field.String(), value, detail} + return &Error{ErrorTypeNotSupported, field.String(), value, detail, ""} } // Forbidden returns a *Error indicating "forbidden". This is used to @@ -217,7 +237,7 @@ func NotSupported[T ~string](field *Path, value interface{}, validValues []T) *E // some conditions, but which are not permitted by current conditions (e.g. // security policy). func Forbidden(field *Path, detail string) *Error { - return &Error{ErrorTypeForbidden, field.String(), "", detail} + return &Error{ErrorTypeForbidden, field.String(), "", detail, ""} } // TooLong returns a *Error indicating "too long". This is used to report that @@ -231,7 +251,7 @@ func TooLong(field *Path, value interface{}, maxLength int) *Error { } else { msg = "value is too long" } - return &Error{ErrorTypeTooLong, field.String(), "", msg} + return &Error{ErrorTypeTooLong, field.String(), "", msg, ""} } // TooLongMaxLength returns a *Error indicating "too long". @@ -259,14 +279,14 @@ func TooMany(field *Path, actualQuantity, maxQuantity int) *Error { actual = omitValue } - return &Error{ErrorTypeTooMany, field.String(), actual, msg} + return &Error{ErrorTypeTooMany, field.String(), actual, msg, ""} } // InternalError returns a *Error indicating "internal error". This is used // to signal that an error was found that was not directly related to user // input. The err argument must be non-nil. func InternalError(field *Path, err error) *Error { - return &Error{ErrorTypeInternal, field.String(), nil, err.Error()} + return &Error{ErrorTypeInternal, field.String(), nil, err.Error(), ""} } // ErrorList holds a set of Errors. It is plausible that we might one day have @@ -285,6 +305,14 @@ func NewErrorTypeMatcher(t ErrorType) utilerrors.Matcher { } } +// WithOrigin sets the origin for all errors in the list and returns the updated list. +func (list ErrorList) WithOrigin(origin string) ErrorList { + for _, err := range list { + err.Origin = origin + } + return list +} + // ToAggregate converts the ErrorList into an errors.Aggregate. func (list ErrorList) ToAggregate() utilerrors.Aggregate { if len(list) == 0 { diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors_test.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors_test.go index a2f9763b8ef..7c4c650edec 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors_test.go @@ -173,3 +173,41 @@ func TestNotSupported(t *testing.T) { t.Errorf("Expected: %s\n, but got: %s\n", expected, notSupported.ErrorBody()) } } + +func TestErrorOrigin(t *testing.T) { + err := Invalid(NewPath("field"), "value", "detail") + + // Test WithOrigin + newErr := err.WithOrigin("origin1") + if newErr.Origin != "origin1" { + t.Errorf("Expected Origin to be 'origin1', got '%s'", newErr.Origin) + } + if err.Origin != "origin1" { + t.Errorf("Expected Origin to be 'origin1', got '%s'", err.Origin) + } +} + +func TestErrorListOrigin(t *testing.T) { + // Create an ErrorList with multiple errors + list := ErrorList{ + Invalid(NewPath("field1"), "value1", "detail1"), + Invalid(NewPath("field2"), "value2", "detail2"), + Required(NewPath("field3"), "detail3"), + } + + // Test WithOrigin + newList := list.WithOrigin("origin1") + // Check that WithOrigin returns the modified list + for i, err := range newList { + if err.Origin != "origin1" { + t.Errorf("Error %d: Expected Origin to be 'origin2', got '%s'", i, err.Origin) + } + } + + // Check that the original list was also modified (WithOrigin modifies and returns the same list) + for i, err := range list { + if err.Origin != "origin1" { + t.Errorf("Error %d: Expected original list Origin to be 'origin2', got '%s'", i, err.Origin) + } + } +}