Add an error matcher, convert 2 tests

I fixed up the TestValidateEndpointsCreate path to show the matcher
instead of manual origin checking.

I picked TestValidateTopologySpreadConstraints because it was the last
failing test on my screen when I changed on of the commonly hard-coded
error strings. I fixed exactly those validation errors that were needed
to make this test pass.  Some of the Origin values can be debated.

The `field/testing.Matcher` interface allows tests to configure the
criteria by which they want to match expected and actual errors.  The
hope is that everyone will use Origin for Invalid errors.

There's some collateral impact for tests which use exact-comparisons and
don't expect origins.  These are all candidates for using the matcher.
This commit is contained in:
Tim Hockin 2025-02-23 22:53:29 -08:00
parent 6b7e38f018
commit c8111709e5
No known key found for this signature in database
8 changed files with 361 additions and 107 deletions

View File

@ -523,7 +523,7 @@ func TestValidateStatefulSet(t *testing.T) {
}, },
} }
cmpOpts := []cmp.Option{cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail"), cmpopts.SortSlices(func(a, b *field.Error) bool { return a.Error() < b.Error() })} cmpOpts := []cmp.Option{cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail", "Origin"), cmpopts.SortSlices(func(a, b *field.Error) bool { return a.Error() < b.Error() })}
for _, testCase := range append(successCases, errorCases...) { for _, testCase := range append(successCases, errorCases...) {
name := testCase.name name := testCase.name
var testTitle string var testTitle string
@ -936,7 +936,7 @@ func TestValidateStatefulSetUpdate(t *testing.T) {
} }
cmpOpts := []cmp.Option{ cmpOpts := []cmp.Option{
cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail"), cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail", "Origin"),
cmpopts.SortSlices(func(a, b *field.Error) bool { return a.Error() < b.Error() }), cmpopts.SortSlices(func(a, b *field.Error) bool { return a.Error() < b.Error() }),
cmpopts.EquateEmpty(), cmpopts.EquateEmpty(),
} }

View File

@ -48,7 +48,7 @@ var (
timeZoneEmptySpace = " " timeZoneEmptySpace = " "
) )
var ignoreErrValueDetail = cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail") var ignoreErrValueDetail = cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail", "Origin")
func getValidManualSelector() *metav1.LabelSelector { func getValidManualSelector() *metav1.LabelSelector {
return &metav1.LabelSelector{ return &metav1.LabelSelector{

View File

@ -350,6 +350,15 @@ func ValidateNonnegativeQuantity(value resource.Quantity, fldPath *field.Path) f
return allErrs return allErrs
} }
// Validates that given value is positive.
func ValidatePositiveField(value int64, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if value <= 0 {
allErrs = append(allErrs, field.Invalid(fldPath, value, isNotPositiveErrorMsg).WithOrigin("minimum"))
}
return allErrs
}
// Validates that a Quantity is positive // Validates that a Quantity is positive
func ValidatePositiveQuantityValue(value resource.Quantity, fldPath *field.Path) field.ErrorList { func ValidatePositiveQuantityValue(value resource.Quantity, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
@ -7905,8 +7914,8 @@ func validateTopologySpreadConstraints(constraints []core.TopologySpreadConstrai
for i, constraint := range constraints { for i, constraint := range constraints {
subFldPath := fldPath.Index(i) subFldPath := fldPath.Index(i)
if err := ValidateMaxSkew(subFldPath.Child("maxSkew"), constraint.MaxSkew); err != nil { if errs := ValidatePositiveField(int64(constraint.MaxSkew), subFldPath.Child("maxSkew")); len(errs) > 0 {
allErrs = append(allErrs, err) allErrs = append(allErrs, errs...)
} }
if err := ValidateTopologyKey(subFldPath.Child("topologyKey"), constraint.TopologyKey); err != nil { if err := ValidateTopologyKey(subFldPath.Child("topologyKey"), constraint.TopologyKey); err != nil {
allErrs = append(allErrs, err) allErrs = append(allErrs, err)
@ -7934,26 +7943,16 @@ func validateTopologySpreadConstraints(constraints []core.TopologySpreadConstrai
return allErrs return allErrs
} }
// ValidateMaxSkew tests that the argument is a valid MaxSkew.
func ValidateMaxSkew(fldPath *field.Path, maxSkew int32) *field.Error {
if maxSkew <= 0 {
return field.Invalid(fldPath, maxSkew, isNotPositiveErrorMsg)
}
return nil
}
// validateMinDomains tests that the argument is a valid MinDomains. // validateMinDomains tests that the argument is a valid MinDomains.
func validateMinDomains(fldPath *field.Path, minDomains *int32, action core.UnsatisfiableConstraintAction) field.ErrorList { func validateMinDomains(fldPath *field.Path, minDomains *int32, action core.UnsatisfiableConstraintAction) field.ErrorList {
if minDomains == nil { if minDomains == nil {
return nil return nil
} }
var allErrs field.ErrorList var allErrs field.ErrorList
if *minDomains <= 0 { allErrs = append(allErrs, ValidatePositiveField(int64(*minDomains), fldPath)...)
allErrs = append(allErrs, field.Invalid(fldPath, minDomains, isNotPositiveErrorMsg))
}
// When MinDomains is non-nil, whenUnsatisfiable must be DoNotSchedule. // When MinDomains is non-nil, whenUnsatisfiable must be DoNotSchedule.
if action != core.DoNotSchedule { if action != core.DoNotSchedule {
allErrs = append(allErrs, field.Invalid(fldPath, minDomains, fmt.Sprintf("can only use minDomains if whenUnsatisfiable=%s, not %s", core.DoNotSchedule, action))) allErrs = append(allErrs, field.Invalid(fldPath, minDomains, fmt.Sprintf("can only use minDomains if whenUnsatisfiable=%s, not %s", core.DoNotSchedule, action)).WithOrigin("dependsOn"))
} }
return allErrs return allErrs
} }
@ -8077,7 +8076,7 @@ func validateMatchLabelKeysInTopologySpread(fldPath *field.Path, matchLabelKeys
for i, key := range matchLabelKeys { for i, key := range matchLabelKeys {
allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(key, fldPath.Index(i))...) allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(key, fldPath.Index(i))...)
if labelSelectorKeys.Has(key) { if labelSelectorKeys.Has(key) {
allErrs = append(allErrs, field.Invalid(fldPath.Index(i), key, "exists in both matchLabelKeys and labelSelector")) allErrs = append(allErrs, field.Invalid(fldPath.Index(i), key, "exists in both matchLabelKeys and labelSelector").WithOrigin("overlappingKeys"))
} }
} }

View File

@ -38,6 +38,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/validation/field"
fldtest "k8s.io/apimachinery/pkg/util/validation/field/testing"
"k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/version"
utilfeature "k8s.io/apiserver/pkg/util/feature" utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/component-base/featuregate" "k8s.io/component-base/featuregate"
@ -20718,24 +20719,31 @@ func TestValidateEndpointsCreate(t *testing.T) {
errorCases := map[string]struct { errorCases := map[string]struct {
endpoints core.Endpoints endpoints core.Endpoints
errorType field.ErrorType expectedErrs field.ErrorList
errorOrigin string
}{ }{
"missing namespace": { "missing namespace": {
endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "mysvc"}}, endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "mysvc"}},
errorType: "FieldValueRequired", expectedErrs: field.ErrorList{
field.Required(field.NewPath("metadata.namespace"), ""),
},
}, },
"missing name": { "missing name": {
endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Namespace: "namespace"}}, endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Namespace: "namespace"}},
errorType: "FieldValueRequired", expectedErrs: field.ErrorList{
field.Required(field.NewPath("metadata.name"), ""),
},
}, },
"invalid namespace": { "invalid namespace": {
endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "no@#invalid.;chars\"allowed"}}, endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "no@#invalid.;chars\"allowed"}},
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
field.Invalid(field.NewPath("metadata.namespace"), nil, ""),
},
}, },
"invalid name": { "invalid name": {
endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "-_Invliad^&Characters", Namespace: "namespace"}}, endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "-_Invliad^&Characters", Namespace: "namespace"}},
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
field.Invalid(field.NewPath("metadata.name"), nil, ""),
},
}, },
"empty addresses": { "empty addresses": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20744,7 +20752,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}}, Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueRequired", expectedErrs: field.ErrorList{
field.Required(field.NewPath("subsets[0]"), ""),
},
}, },
"invalid IP": { "invalid IP": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20754,8 +20764,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}}, Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
errorOrigin: "format=ip-sloppy", field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=ip-sloppy"),
},
}, },
"Multiple ports, one without name": { "Multiple ports, one without name": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20765,7 +20776,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Port: 8675, Protocol: "TCP"}, {Name: "b", Port: 309, Protocol: "TCP"}}, Ports: []core.EndpointPort{{Port: 8675, Protocol: "TCP"}, {Name: "b", Port: 309, Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueRequired", expectedErrs: field.ErrorList{
field.Required(field.NewPath("subsets[0].ports[0].name"), ""),
},
}, },
"Invalid port number": { "Invalid port number": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20775,8 +20788,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "a", Port: 66000, Protocol: "TCP"}}, Ports: []core.EndpointPort{{Name: "a", Port: 66000, Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
errorOrigin: "portNum", field.Invalid(field.NewPath("subsets[0].ports[0].port"), nil, "").WithOrigin("portNum"),
},
}, },
"Invalid protocol": { "Invalid protocol": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20786,7 +20800,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "Protocol"}}, Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "Protocol"}},
}}, }},
}, },
errorType: "FieldValueNotSupported", expectedErrs: field.ErrorList{
field.NotSupported[string](field.NewPath("subsets[0].ports[0].protocol"), nil, nil),
},
}, },
"Address missing IP": { "Address missing IP": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20796,8 +20812,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}}, Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
errorOrigin: "format=ip-sloppy", field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=ip-sloppy"),
},
}, },
"Port missing number": { "Port missing number": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20807,8 +20824,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "a", Protocol: "TCP"}}, Ports: []core.EndpointPort{{Name: "a", Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
errorOrigin: "portNum", field.Invalid(field.NewPath("subsets[0].ports[0].port"), nil, "").WithOrigin("portNum"),
},
}, },
"Port missing protocol": { "Port missing protocol": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20818,7 +20836,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "a", Port: 93}}, Ports: []core.EndpointPort{{Name: "a", Port: 93}},
}}, }},
}, },
errorType: "FieldValueRequired", expectedErrs: field.ErrorList{
field.Required(field.NewPath("subsets[0].ports[0].protocol"), ""),
},
}, },
"Address is loopback": { "Address is loopback": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20828,8 +20848,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}}, Ports: []core.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
errorOrigin: "format=non-special-ip", field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=non-special-ip"),
},
}, },
"Address is link-local": { "Address is link-local": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20839,8 +20860,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}}, Ports: []core.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
errorOrigin: "format=non-special-ip", field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=non-special-ip"),
},
}, },
"Address is link-local multicast": { "Address is link-local multicast": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20850,8 +20872,9 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}}, Ports: []core.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}},
}}, }},
}, },
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
errorOrigin: "format=non-special-ip", field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=non-special-ip"),
},
}, },
"Invalid AppProtocol": { "Invalid AppProtocol": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20861,16 +20884,18 @@ func TestValidateEndpointsCreate(t *testing.T) {
Ports: []core.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP", AppProtocol: utilpointer.String("lots-of[invalid]-{chars}")}}, Ports: []core.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP", AppProtocol: utilpointer.String("lots-of[invalid]-{chars}")}},
}}, }},
}, },
errorType: "FieldValueInvalid", expectedErrs: field.ErrorList{
errorOrigin: "format=qualified-name", field.Invalid(field.NewPath("subsets[0].ports[0].appProtocol"), nil, "").WithOrigin("format=qualified-name"),
},
}, },
} }
for k, v := range errorCases { for k, v := range errorCases {
t.Run(k, func(t *testing.T) { t.Run(k, func(t *testing.T) {
if errs := ValidateEndpointsCreate(&v.endpoints); len(errs) == 0 || errs[0].Type != v.errorType || errs[0].Origin != v.errorOrigin { errs := ValidateEndpointsCreate(&v.endpoints)
t.Errorf("Expected error type %s with origin %q, got %#v", v.errorType, v.errorOrigin, errs[0]) // TODO: set .RequireOriginWhenInvalid() once metadata is done
} matcher := fldtest.Match().ByType().ByField().ByOrigin()
fldtest.MatchErrors(t, v.expectedErrs, errs, matcher)
}) })
} }
} }
@ -22630,8 +22655,8 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
testCases := []struct { testCases := []struct {
name string name string
constraints []core.TopologySpreadConstraint constraints []core.TopologySpreadConstraint
wantFieldErrors field.ErrorList
opts PodValidationOptions opts PodValidationOptions
wantFieldErrors field.ErrorList
}{{ }{{
name: "all required fields ok", name: "all required fields ok",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22640,19 +22665,23 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
WhenUnsatisfiable: core.DoNotSchedule, WhenUnsatisfiable: core.DoNotSchedule,
MinDomains: utilpointer.Int32(3), MinDomains: utilpointer.Int32(3),
}}, }},
wantFieldErrors: field.ErrorList{}, wantFieldErrors: nil,
}, { }, {
name: "missing MaxSkew", name: "missing MaxSkew",
constraints: []core.TopologySpreadConstraint{ constraints: []core.TopologySpreadConstraint{
{TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule}, {TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule},
}, },
wantFieldErrors: []*field.Error{field.Invalid(fieldPathMaxSkew, int32(0), isNotPositiveErrorMsg)}, wantFieldErrors: field.ErrorList{
field.Invalid(fieldPathMaxSkew, nil, "").WithOrigin("minimum"),
},
}, { }, {
name: "negative MaxSkew", name: "negative MaxSkew",
constraints: []core.TopologySpreadConstraint{ constraints: []core.TopologySpreadConstraint{
{MaxSkew: -1, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule}, {MaxSkew: -1, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule},
}, },
wantFieldErrors: []*field.Error{field.Invalid(fieldPathMaxSkew, int32(-1), isNotPositiveErrorMsg)}, wantFieldErrors: field.ErrorList{
field.Invalid(fieldPathMaxSkew, nil, "").WithOrigin("minimum"),
},
}, { }, {
name: "can use MinDomains with ScheduleAnyway, when MinDomains = nil", name: "can use MinDomains with ScheduleAnyway, when MinDomains = nil",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22661,7 +22690,7 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
WhenUnsatisfiable: core.ScheduleAnyway, WhenUnsatisfiable: core.ScheduleAnyway,
MinDomains: nil, MinDomains: nil,
}}, }},
wantFieldErrors: field.ErrorList{}, wantFieldErrors: nil,
}, { }, {
name: "negative minDomains is invalid", name: "negative minDomains is invalid",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22670,7 +22699,9 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
WhenUnsatisfiable: core.DoNotSchedule, WhenUnsatisfiable: core.DoNotSchedule,
MinDomains: utilpointer.Int32(-1), MinDomains: utilpointer.Int32(-1),
}}, }},
wantFieldErrors: []*field.Error{field.Invalid(fieldPathMinDomains, utilpointer.Int32(-1), isNotPositiveErrorMsg)}, wantFieldErrors: field.ErrorList{
field.Invalid(fieldPathMinDomains, nil, "").WithOrigin("minimum"),
},
}, { }, {
name: "cannot use non-nil MinDomains with ScheduleAnyway", name: "cannot use non-nil MinDomains with ScheduleAnyway",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22679,7 +22710,9 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
WhenUnsatisfiable: core.ScheduleAnyway, WhenUnsatisfiable: core.ScheduleAnyway,
MinDomains: utilpointer.Int32(10), MinDomains: utilpointer.Int32(10),
}}, }},
wantFieldErrors: []*field.Error{field.Invalid(fieldPathMinDomains, utilpointer.Int32(10), fmt.Sprintf("can only use minDomains if whenUnsatisfiable=%s, not %s", string(core.DoNotSchedule), string(core.ScheduleAnyway)))}, wantFieldErrors: field.ErrorList{
field.Invalid(fieldPathMinDomains, nil, "").WithOrigin("dependsOn"),
},
}, { }, {
name: "use negative MinDomains with ScheduleAnyway(invalid)", name: "use negative MinDomains with ScheduleAnyway(invalid)",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22688,50 +22721,58 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
WhenUnsatisfiable: core.ScheduleAnyway, WhenUnsatisfiable: core.ScheduleAnyway,
MinDomains: utilpointer.Int32(-1), MinDomains: utilpointer.Int32(-1),
}}, }},
wantFieldErrors: []*field.Error{ wantFieldErrors: field.ErrorList{
field.Invalid(fieldPathMinDomains, utilpointer.Int32(-1), isNotPositiveErrorMsg), field.Invalid(fieldPathMinDomains, nil, "").WithOrigin("minimum"),
field.Invalid(fieldPathMinDomains, utilpointer.Int32(-1), fmt.Sprintf("can only use minDomains if whenUnsatisfiable=%s, not %s", string(core.DoNotSchedule), string(core.ScheduleAnyway))), field.Invalid(fieldPathMinDomains, nil, "").WithOrigin("dependsOn"),
}, },
}, { }, {
name: "missing TopologyKey", name: "missing TopologyKey",
constraints: []core.TopologySpreadConstraint{ constraints: []core.TopologySpreadConstraint{
{MaxSkew: 1, WhenUnsatisfiable: core.DoNotSchedule}, {MaxSkew: 1, WhenUnsatisfiable: core.DoNotSchedule},
}, },
wantFieldErrors: []*field.Error{field.Required(fieldPathTopologyKey, "can not be empty")}, wantFieldErrors: field.ErrorList{
field.Required(fieldPathTopologyKey, ""),
},
}, { }, {
name: "missing scheduling mode", name: "missing scheduling mode",
constraints: []core.TopologySpreadConstraint{ constraints: []core.TopologySpreadConstraint{
{MaxSkew: 1, TopologyKey: "k8s.io/zone"}, {MaxSkew: 1, TopologyKey: "k8s.io/zone"},
}, },
wantFieldErrors: []*field.Error{field.NotSupported(fieldPathWhenUnsatisfiable, core.UnsatisfiableConstraintAction(""), sets.List(supportedScheduleActions))}, wantFieldErrors: field.ErrorList{
field.NotSupported[core.UnsatisfiableConstraintAction](fieldPathWhenUnsatisfiable, nil, nil),
},
}, { }, {
name: "unsupported scheduling mode", name: "unsupported scheduling mode",
constraints: []core.TopologySpreadConstraint{ constraints: []core.TopologySpreadConstraint{
{MaxSkew: 1, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.UnsatisfiableConstraintAction("N/A")}, {MaxSkew: 1, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.UnsatisfiableConstraintAction("N/A")},
}, },
wantFieldErrors: []*field.Error{field.NotSupported(fieldPathWhenUnsatisfiable, core.UnsatisfiableConstraintAction("N/A"), sets.List(supportedScheduleActions))}, wantFieldErrors: field.ErrorList{
field.NotSupported[core.UnsatisfiableConstraintAction](fieldPathWhenUnsatisfiable, nil, nil),
},
}, { }, {
name: "multiple constraints ok with all required fields", name: "multiple constraints ok with all required fields",
constraints: []core.TopologySpreadConstraint{ constraints: []core.TopologySpreadConstraint{
{MaxSkew: 1, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule}, {MaxSkew: 1, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule},
{MaxSkew: 2, TopologyKey: "k8s.io/node", WhenUnsatisfiable: core.ScheduleAnyway}, {MaxSkew: 2, TopologyKey: "k8s.io/node", WhenUnsatisfiable: core.ScheduleAnyway},
}, },
wantFieldErrors: field.ErrorList{}, wantFieldErrors: nil,
}, { }, {
name: "multiple constraints missing TopologyKey on partial ones", name: "multiple constraints missing TopologyKey on partial ones",
constraints: []core.TopologySpreadConstraint{ constraints: []core.TopologySpreadConstraint{
{MaxSkew: 1, WhenUnsatisfiable: core.ScheduleAnyway}, {MaxSkew: 1, WhenUnsatisfiable: core.ScheduleAnyway},
{MaxSkew: 2, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule}, {MaxSkew: 2, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule},
}, },
wantFieldErrors: []*field.Error{field.Required(fieldPathTopologyKey, "can not be empty")}, wantFieldErrors: field.ErrorList{
field.Required(fieldPathTopologyKey, ""),
},
}, { }, {
name: "duplicate constraints", name: "duplicate constraints",
constraints: []core.TopologySpreadConstraint{ constraints: []core.TopologySpreadConstraint{
{MaxSkew: 1, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule}, {MaxSkew: 1, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule},
{MaxSkew: 2, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule}, {MaxSkew: 2, TopologyKey: "k8s.io/zone", WhenUnsatisfiable: core.DoNotSchedule},
}, },
wantFieldErrors: []*field.Error{ wantFieldErrors: field.ErrorList{
field.Duplicate(fieldPathTopologyKeyAndWhenUnsatisfiable, fmt.Sprintf("{%v, %v}", "k8s.io/zone", core.DoNotSchedule)), field.Duplicate(fieldPathTopologyKeyAndWhenUnsatisfiable, nil),
}, },
}, { }, {
name: "supported policy name set on NodeAffinityPolicy and NodeTaintsPolicy", name: "supported policy name set on NodeAffinityPolicy and NodeTaintsPolicy",
@ -22742,7 +22783,7 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
NodeAffinityPolicy: &honor, NodeAffinityPolicy: &honor,
NodeTaintsPolicy: &ignore, NodeTaintsPolicy: &ignore,
}}, }},
wantFieldErrors: []*field.Error{}, wantFieldErrors: nil,
}, { }, {
name: "unsupported policy name set on NodeAffinityPolicy", name: "unsupported policy name set on NodeAffinityPolicy",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22752,8 +22793,8 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
NodeAffinityPolicy: &unknown, NodeAffinityPolicy: &unknown,
NodeTaintsPolicy: &ignore, NodeTaintsPolicy: &ignore,
}}, }},
wantFieldErrors: []*field.Error{ wantFieldErrors: field.ErrorList{
field.NotSupported(nodeAffinityField, &unknown, sets.List(supportedPodTopologySpreadNodePolicies)), field.NotSupported[core.NodeInclusionPolicy](nodeAffinityField, nil, nil),
}, },
}, { }, {
name: "unsupported policy name set on NodeTaintsPolicy", name: "unsupported policy name set on NodeTaintsPolicy",
@ -22764,8 +22805,8 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
NodeAffinityPolicy: &honor, NodeAffinityPolicy: &honor,
NodeTaintsPolicy: &unknown, NodeTaintsPolicy: &unknown,
}}, }},
wantFieldErrors: []*field.Error{ wantFieldErrors: field.ErrorList{
field.NotSupported(nodeTaintsField, &unknown, sets.List(supportedPodTopologySpreadNodePolicies)), field.NotSupported[core.NodeInclusionPolicy](nodeTaintsField, nil, nil),
}, },
}, { }, {
name: "key in MatchLabelKeys isn't correctly defined", name: "key in MatchLabelKeys isn't correctly defined",
@ -22776,7 +22817,9 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
WhenUnsatisfiable: core.DoNotSchedule, WhenUnsatisfiable: core.DoNotSchedule,
MatchLabelKeys: []string{"/simple"}, MatchLabelKeys: []string{"/simple"},
}}, }},
wantFieldErrors: field.ErrorList{field.Invalid(fieldPathMatchLabelKeys.Index(0), "/simple", "prefix part must be non-empty")}, wantFieldErrors: field.ErrorList{
field.Invalid(fieldPathMatchLabelKeys.Index(0), nil, "").WithOrigin("labelKey"),
},
}, { }, {
name: "key exists in both matchLabelKeys and labelSelector", name: "key exists in both matchLabelKeys and labelSelector",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22794,7 +22837,9 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
}, },
}, },
}}, }},
wantFieldErrors: field.ErrorList{field.Invalid(fieldPathMatchLabelKeys.Index(0), "foo", "exists in both matchLabelKeys and labelSelector")}, wantFieldErrors: field.ErrorList{
field.Invalid(fieldPathMatchLabelKeys.Index(0), nil, "").WithOrigin("overlappingKeys"),
},
}, { }, {
name: "key in MatchLabelKeys is forbidden to be specified when labelSelector is not set", name: "key in MatchLabelKeys is forbidden to be specified when labelSelector is not set",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22803,7 +22848,9 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
WhenUnsatisfiable: core.DoNotSchedule, WhenUnsatisfiable: core.DoNotSchedule,
MatchLabelKeys: []string{"foo"}, MatchLabelKeys: []string{"foo"},
}}, }},
wantFieldErrors: field.ErrorList{field.Forbidden(fieldPathMatchLabelKeys, "must not be specified when labelSelector is not set")}, wantFieldErrors: field.ErrorList{
field.Forbidden(fieldPathMatchLabelKeys, ""),
},
}, { }, {
name: "invalid matchLabels set on labelSelector when AllowInvalidTopologySpreadConstraintLabelSelector is false", name: "invalid matchLabels set on labelSelector when AllowInvalidTopologySpreadConstraintLabelSelector is false",
constraints: []core.TopologySpreadConstraint{{ constraints: []core.TopologySpreadConstraint{{
@ -22813,7 +22860,9 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
MinDomains: nil, MinDomains: nil,
LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "foo"}}, LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "foo"}},
}}, }},
wantFieldErrors: []*field.Error{field.Invalid(labelSelectorField.Child("matchLabels"), "NoUppercaseOrSpecialCharsLike=Equals", "name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')")}, wantFieldErrors: field.ErrorList{
field.Invalid(labelSelectorField.Child("matchLabels"), nil, "").WithOrigin("labelKey"),
},
opts: PodValidationOptions{AllowInvalidTopologySpreadConstraintLabelSelector: false}, opts: PodValidationOptions{AllowInvalidTopologySpreadConstraintLabelSelector: false},
}, { }, {
name: "invalid matchLabels set on labelSelector when AllowInvalidTopologySpreadConstraintLabelSelector is true", name: "invalid matchLabels set on labelSelector when AllowInvalidTopologySpreadConstraintLabelSelector is true",
@ -22824,7 +22873,7 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
MinDomains: nil, MinDomains: nil,
LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "foo"}}, LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "foo"}},
}}, }},
wantFieldErrors: []*field.Error{}, wantFieldErrors: nil,
opts: PodValidationOptions{AllowInvalidTopologySpreadConstraintLabelSelector: true}, opts: PodValidationOptions{AllowInvalidTopologySpreadConstraintLabelSelector: true},
}, { }, {
name: "valid matchLabels set on labelSelector when AllowInvalidTopologySpreadConstraintLabelSelector is false", name: "valid matchLabels set on labelSelector when AllowInvalidTopologySpreadConstraintLabelSelector is false",
@ -22835,17 +22884,15 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
MinDomains: nil, MinDomains: nil,
LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "foo"}}, LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "foo"}},
}}, }},
wantFieldErrors: []*field.Error{}, wantFieldErrors: nil,
opts: PodValidationOptions{AllowInvalidTopologySpreadConstraintLabelSelector: false}, opts: PodValidationOptions{AllowInvalidTopologySpreadConstraintLabelSelector: false},
}, }}
}
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
errs := validateTopologySpreadConstraints(tc.constraints, fieldPath, tc.opts) errs := validateTopologySpreadConstraints(tc.constraints, fieldPath, tc.opts)
if diff := cmp.Diff(tc.wantFieldErrors, errs); diff != "" { matcher := fldtest.Match().ByType().ByField().ByOrigin().RequireOriginWhenInvalid()
t.Errorf("unexpected field errors (-want, +got):\n%s", diff) fldtest.MatchErrors(t, tc.wantFieldErrors, errs, matcher)
}
}) })
} }
} }

View File

@ -21,6 +21,8 @@ import (
"testing" "testing"
"time" "time"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -517,7 +519,10 @@ func TestValidateClientConnectionConfiguration(t *testing.T) {
} { } {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
errs := validateClientConnectionConfiguration(testCase.ccc, newPath) errs := validateClientConnectionConfiguration(testCase.ccc, newPath)
assert.Equal(t, testCase.expectedErrs, errs, "did not get expected validation errors") cmpOpts := []cmp.Option{cmpopts.IgnoreFields(field.Error{}, "Origin"), cmpopts.SortSlices(func(a, b *field.Error) bool { return a.Error() < b.Error() })}
if diff := cmp.Diff(testCase.expectedErrs, errs, cmpOpts...); diff != "" {
t.Errorf("did not get expected validation errors (-want,+got):\n%s", diff)
}
}) })
} }
} }

View File

@ -104,7 +104,7 @@ func ValidateLabelSelectorRequirement(sr metav1.LabelSelectorRequirement, opts L
func ValidateLabelName(labelName string, fldPath *field.Path) field.ErrorList { func ValidateLabelName(labelName string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for _, msg := range validation.IsQualifiedName(labelName) { for _, msg := range validation.IsQualifiedName(labelName) {
allErrs = append(allErrs, field.Invalid(fldPath, labelName, msg)) allErrs = append(allErrs, field.Invalid(fldPath, labelName, msg).WithOrigin("labelKey"))
} }
return allErrs return allErrs
} }

View File

@ -52,8 +52,8 @@ type Error struct {
var _ error = &Error{} var _ error = &Error{}
// Error implements the error interface. // Error implements the error interface.
func (v *Error) Error() string { func (e *Error) Error() string {
return fmt.Sprintf("%s: %s", v.Field, v.ErrorBody()) return fmt.Sprintf("%s: %s", e.Field, e.ErrorBody())
} }
type OmitValueType struct{} type OmitValueType struct{}
@ -62,21 +62,21 @@ var omitValue = OmitValueType{}
// ErrorBody returns the error message without the field name. This is useful // ErrorBody returns the error message without the field name. This is useful
// for building nice-looking higher-level error reporting. // for building nice-looking higher-level error reporting.
func (v *Error) ErrorBody() string { func (e *Error) ErrorBody() string {
var s string var s string
switch { switch {
case v.Type == ErrorTypeRequired: case e.Type == ErrorTypeRequired:
s = v.Type.String() s = e.Type.String()
case v.Type == ErrorTypeForbidden: case e.Type == ErrorTypeForbidden:
s = v.Type.String() s = e.Type.String()
case v.Type == ErrorTypeTooLong: case e.Type == ErrorTypeTooLong:
s = v.Type.String() s = e.Type.String()
case v.Type == ErrorTypeInternal: case e.Type == ErrorTypeInternal:
s = v.Type.String() s = e.Type.String()
case v.BadValue == omitValue: case e.BadValue == omitValue:
s = v.Type.String() s = e.Type.String()
default: default:
value := v.BadValue value := e.BadValue
valueType := reflect.TypeOf(value) valueType := reflect.TypeOf(value)
if value == nil || valueType == nil { if value == nil || valueType == nil {
value = "null" value = "null"
@ -90,30 +90,30 @@ func (v *Error) ErrorBody() string {
switch t := value.(type) { switch t := value.(type) {
case int64, int32, float64, float32, bool: case int64, int32, float64, float32, bool:
// use simple printer for simple types // use simple printer for simple types
s = fmt.Sprintf("%s: %v", v.Type, value) s = fmt.Sprintf("%s: %v", e.Type, value)
case string: case string:
s = fmt.Sprintf("%s: %q", v.Type, t) s = fmt.Sprintf("%s: %q", e.Type, t)
case fmt.Stringer: case fmt.Stringer:
// anything that defines String() is better than raw struct // anything that defines String() is better than raw struct
s = fmt.Sprintf("%s: %s", v.Type, t.String()) s = fmt.Sprintf("%s: %s", e.Type, t.String())
default: default:
// fallback to raw struct // fallback to raw struct
// TODO: internal types have panic guards against json.Marshalling to prevent // TODO: internal types have panic guards against json.Marshalling to prevent
// accidental use of internal types in external serialized form. For now, use // accidental use of internal types in external serialized form. For now, use
// %#v, although it would be better to show a more expressive output in the future // %#v, although it would be better to show a more expressive output in the future
s = fmt.Sprintf("%s: %#v", v.Type, value) s = fmt.Sprintf("%s: %#v", e.Type, value)
} }
} }
if len(v.Detail) != 0 { if len(e.Detail) != 0 {
s += fmt.Sprintf(": %s", v.Detail) s += fmt.Sprintf(": %s", e.Detail)
} }
return s return s
} }
// WithOrigin adds origin information to the FieldError // WithOrigin adds origin information to the FieldError
func (v *Error) WithOrigin(o string) *Error { func (e *Error) WithOrigin(o string) *Error {
v.Origin = o e.Origin = o
return v return e
} }
// ErrorType is a machine readable value providing more detail about why // ErrorType is a machine readable value providing more detail about why

View File

@ -0,0 +1,203 @@
/*
Copyright 2025 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package testing
import (
"fmt"
"reflect"
"regexp"
"strings"
"testing"
field "k8s.io/apimachinery/pkg/util/validation/field"
)
// MatchErrors compares two ErrorLists with the specified matcher, and fails
// the test if they don't match. If a given "want" error matches multiple "got"
// errors, they will all be consumed. This might be OK (e.g. if there are
// multiple errors on the same field from the same origin) or it might be an
// insufficiently specific matcher, so these will be logged.
func MatchErrors(t *testing.T, want, got field.ErrorList, matcher *Matcher) {
t.Helper()
remaining := got
for _, w := range want {
tmp := make(field.ErrorList, 0, len(remaining))
n := 0
for _, g := range remaining {
if matcher.Matches(w, g) {
n++
} else {
tmp = append(tmp, g)
}
}
if n == 0 {
t.Errorf("expected an error matching:\n%s", matcher.Render(w))
} else if n > 1 {
// This is not necessarily and error, but it's worth logging in
// case it's not what the test author intended.
t.Logf("multiple errors matched:\n%s", matcher.Render(w))
}
remaining = tmp
}
if len(remaining) > 0 {
for _, e := range remaining {
t.Errorf("unmatched error:\n%s", Match().Exactly().Render(e))
}
}
}
// Match returns a new Matcher.
func Match() *Matcher {
return &Matcher{}
}
// Matcher is a helper for comparing field.Error objects.
type Matcher struct {
matchType bool
matchField bool
matchValue bool
matchOrigin bool
matchDetail func(want, got string) bool
requireOriginWhenInvalid bool
}
// Matches returns true if the two field.Error objects match according to the
// configured criteria.
func (m *Matcher) Matches(want, got *field.Error) bool {
if m.matchType && want.Type != got.Type {
return false
}
if m.matchField && want.Field != got.Field {
return false
}
if m.matchValue && !reflect.DeepEqual(want.BadValue, got.BadValue) {
return false
}
if m.matchOrigin {
if want.Origin != got.Origin {
return false
}
if m.requireOriginWhenInvalid && want.Type == field.ErrorTypeInvalid {
if want.Origin == "" || got.Origin == "" {
return false
}
}
}
if m.matchDetail != nil && !m.matchDetail(want.Detail, got.Detail) {
return false
}
return true
}
// Render returns a string representation of the specified Error object,
// according to the criteria configured in the Matcher.
func (m *Matcher) Render(e *field.Error) string {
buf := strings.Builder{}
comma := func() {
if buf.Len() > 0 {
buf.WriteString(", ")
}
}
if m.matchType {
comma()
buf.WriteString(fmt.Sprintf("Type=%q", e.Type))
}
if m.matchField {
comma()
buf.WriteString(fmt.Sprintf("Field=%q", e.Field))
}
if m.matchValue {
comma()
buf.WriteString(fmt.Sprintf("Value=%v", e.BadValue))
}
if m.matchOrigin || m.requireOriginWhenInvalid && e.Type == field.ErrorTypeInvalid {
comma()
buf.WriteString(fmt.Sprintf("Origin=%q", e.Origin))
}
if m.matchDetail != nil {
comma()
buf.WriteString(fmt.Sprintf("Detail=%q", e.Detail))
}
return "{" + buf.String() + "}"
}
// Exactly configures the matcher to match all fields exactly.
func (m *Matcher) Exactly() *Matcher {
return m.ByType().ByField().ByValue().ByOrigin().ByDetailExact()
}
// Exactly configures the matcher to match errors by type.
func (m *Matcher) ByType() *Matcher {
m.matchType = true
return m
}
// ByField configures the matcher to match errors by field path.
func (m *Matcher) ByField() *Matcher {
m.matchField = true
return m
}
// ByValue configures the matcher to match errors by the errant value.
func (m *Matcher) ByValue() *Matcher {
m.matchValue = true
return m
}
// ByOrigin configures the matcher to match errors by the origin.
func (m *Matcher) ByOrigin() *Matcher {
m.matchOrigin = true
return m
}
// RequireOriginWhenInvalid configures the matcher to require the Origin field
// to be set when the Type is Invalid and the matcher is matching by Origin.
func (m *Matcher) RequireOriginWhenInvalid() *Matcher {
m.requireOriginWhenInvalid = true
return m
}
// ByDetailExact configures the matcher to match errors by the exact detail
// string.
func (m *Matcher) ByDetailExact() *Matcher {
m.matchDetail = func(want, got string) bool {
return got == want
}
return m
}
// ByDetailSubstring configures the matcher to match errors by a substring of
// the detail string.
func (m *Matcher) ByDetailSubstring() *Matcher {
m.matchDetail = func(want, got string) bool {
return strings.Contains(got, want)
}
return m
}
// ByDetailRegexp configures the matcher to match errors by a regular
// expression of the detail string, where the "want" string is assumed to be a
// valid regular expression.
func (m *Matcher) ByDetailRegexp() *Matcher {
m.matchDetail = func(want, got string) bool {
return regexp.MustCompile(want).MatchString(got)
}
return m
}