Merge pull request #130355 from yongruilin/validation_origin

validation: Add Origin field to field.Error for more precise error tracking
This commit is contained in:
Kubernetes Prow Robot 2025-02-28 00:04:23 -08:00 committed by GitHub
commit 803e9d6495
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 239 additions and 136 deletions

View File

@ -133,7 +133,7 @@ func ValidateAnnotations(annotations map[string]string, fldPath *field.Path) fie
func ValidateDNS1123Label(value string, fldPath *field.Path) field.ErrorList { func ValidateDNS1123Label(value string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for _, msg := range validation.IsDNS1123Label(value) { for _, msg := range validation.IsDNS1123Label(value) {
allErrs = append(allErrs, field.Invalid(fldPath, value, msg)) allErrs = append(allErrs, field.Invalid(fldPath, value, msg).WithOrigin("format=dns-label"))
} }
return allErrs return allErrs
} }
@ -142,7 +142,7 @@ func ValidateDNS1123Label(value string, fldPath *field.Path) field.ErrorList {
func ValidateQualifiedName(value string, fldPath *field.Path) field.ErrorList { func ValidateQualifiedName(value string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for _, msg := range validation.IsQualifiedName(value) { for _, msg := range validation.IsQualifiedName(value) {
allErrs = append(allErrs, field.Invalid(fldPath, value, msg)) allErrs = append(allErrs, field.Invalid(fldPath, value, msg).WithOrigin("format=qualified-name"))
} }
return allErrs return allErrs
} }
@ -6266,7 +6266,7 @@ func ValidateReplicationControllerSpec(spec, oldSpec *core.ReplicationController
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...) allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...)
allErrs = append(allErrs, ValidateNonEmptySelector(spec.Selector, fldPath.Child("selector"))...) allErrs = append(allErrs, ValidateNonEmptySelector(spec.Selector, fldPath.Child("selector"))...)
allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.Replicas), fldPath.Child("replicas"))...) allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.Replicas), fldPath.Child("replicas")).WithOrigin("minimum")...)
allErrs = append(allErrs, ValidatePodTemplateSpecForRC(spec.Template, spec.Selector, spec.Replicas, fldPath.Child("template"), opts)...) allErrs = append(allErrs, ValidatePodTemplateSpecForRC(spec.Template, spec.Selector, spec.Replicas, fldPath.Child("template"), opts)...)
return allErrs return allErrs
} }
@ -7466,7 +7466,7 @@ func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path)
// During endpoint update, verify that NodeName is a DNS subdomain and transition rules allow the update // During endpoint update, verify that NodeName is a DNS subdomain and transition rules allow the update
if address.NodeName != nil { if address.NodeName != nil {
for _, msg := range ValidateNodeName(*address.NodeName, false) { for _, msg := range ValidateNodeName(*address.NodeName, false) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), *address.NodeName, msg)) allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), *address.NodeName, msg).WithOrigin("format=dns-label"))
} }
} }
allErrs = append(allErrs, ValidateNonSpecialIP(address.IP, fldPath.Child("ip"))...) allErrs = append(allErrs, ValidateNonSpecialIP(address.IP, fldPath.Child("ip"))...)
@ -7485,20 +7485,20 @@ func ValidateNonSpecialIP(ipAddress string, fldPath *field.Path) field.ErrorList
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
ip := netutils.ParseIPSloppy(ipAddress) ip := netutils.ParseIPSloppy(ipAddress)
if ip == nil { if ip == nil {
allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "must be a valid IP address")) allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "must be a valid IP address").WithOrigin("format=ip-sloppy"))
return allErrs return allErrs
} }
if ip.IsUnspecified() { if ip.IsUnspecified() {
allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, fmt.Sprintf("may not be unspecified (%v)", ipAddress))) allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, fmt.Sprintf("may not be unspecified (%v)", ipAddress)).WithOrigin("format=non-special-ip"))
} }
if ip.IsLoopback() { if ip.IsLoopback() {
allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the loopback range (127.0.0.0/8, ::1/128)")) allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the loopback range (127.0.0.0/8, ::1/128)").WithOrigin("format=non-special-ip"))
} }
if ip.IsLinkLocalUnicast() { if ip.IsLinkLocalUnicast() {
allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the link-local range (169.254.0.0/16, fe80::/10)")) allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the link-local range (169.254.0.0/16, fe80::/10)").WithOrigin("format=non-special-ip"))
} }
if ip.IsLinkLocalMulticast() { if ip.IsLinkLocalMulticast() {
allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the link-local multicast range (224.0.0.0/24, ff02::/10)")) allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the link-local multicast range (224.0.0.0/24, ff02::/10)").WithOrigin("format=non-special-ip"))
} }
return allErrs return allErrs
} }
@ -7511,7 +7511,7 @@ func validateEndpointPort(port *core.EndpointPort, requireName bool, fldPath *fi
allErrs = append(allErrs, ValidateDNS1123Label(port.Name, fldPath.Child("name"))...) allErrs = append(allErrs, ValidateDNS1123Label(port.Name, fldPath.Child("name"))...)
} }
for _, msg := range validation.IsValidPortNum(int(port.Port)) { for _, msg := range validation.IsValidPortNum(int(port.Port)) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), port.Port, msg)) allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), port.Port, msg).WithOrigin("portNum"))
} }
if len(port.Protocol) == 0 { if len(port.Protocol) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("protocol"), "")) allErrs = append(allErrs, field.Required(fldPath.Child("protocol"), ""))

View File

@ -9183,7 +9183,7 @@ func TestValidateContainers(t *testing.T) {
t.Fatal("expected error but received none") t.Fatal("expected error but received none")
} }
if diff := cmp.Diff(tc.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail")); diff != "" { if diff := cmp.Diff(tc.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail", "Origin")); diff != "" {
t.Errorf("unexpected diff in errors (-want, +got):\n%s", diff) t.Errorf("unexpected diff in errors (-want, +got):\n%s", diff)
t.Errorf("INFO: all errors:\n%s", prettyErrorList(errs)) t.Errorf("INFO: all errors:\n%s", prettyErrorList(errs))
} }
@ -16791,144 +16791,179 @@ func TestValidateReplicationController(t *testing.T) {
} }
} }
errorCases := map[string]core.ReplicationController{ errorCases := map[string]struct {
rc core.ReplicationController
expectedOrigin []string
}{
"zero-length ID": { "zero-length ID": {
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault}, rc: core.ReplicationController{
Spec: core.ReplicationControllerSpec{ ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
Selector: validSelector, Spec: core.ReplicationControllerSpec{
Template: &validPodTemplate.Template, Selector: validSelector,
Template: &validPodTemplate.Template,
},
}, },
}, },
"missing-namespace": { "missing-namespace": {
ObjectMeta: metav1.ObjectMeta{Name: "abc-123"}, rc: core.ReplicationController{
Spec: core.ReplicationControllerSpec{ ObjectMeta: metav1.ObjectMeta{Name: "abc-123"},
Selector: validSelector, Spec: core.ReplicationControllerSpec{
Template: &validPodTemplate.Template, Selector: validSelector,
Template: &validPodTemplate.Template,
},
}, },
}, },
"empty selector": { "empty selector": {
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault}, rc: core.ReplicationController{
Spec: core.ReplicationControllerSpec{ ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Template: &validPodTemplate.Template, Spec: core.ReplicationControllerSpec{
Template: &validPodTemplate.Template,
},
}, },
}, },
"selector_doesnt_match": { "selector_doesnt_match": {
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault}, rc: core.ReplicationController{
Spec: core.ReplicationControllerSpec{ ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Selector: map[string]string{"foo": "bar"}, Spec: core.ReplicationControllerSpec{
Template: &validPodTemplate.Template, Selector: map[string]string{"foo": "bar"},
Template: &validPodTemplate.Template,
},
}, },
}, },
"invalid manifest": { "invalid manifest": {
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault}, rc: core.ReplicationController{
Spec: core.ReplicationControllerSpec{ ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Selector: validSelector, Spec: core.ReplicationControllerSpec{
Selector: validSelector,
},
}, },
}, },
"read-write persistent disk with > 1 pod": { "read-write persistent disk with > 1 pod": {
ObjectMeta: metav1.ObjectMeta{Name: "abc"}, rc: core.ReplicationController{
Spec: core.ReplicationControllerSpec{ ObjectMeta: metav1.ObjectMeta{Name: "abc"},
Replicas: 2, Spec: core.ReplicationControllerSpec{
Selector: validSelector, Replicas: 2,
Template: &readWriteVolumePodTemplate.Template, Selector: validSelector,
Template: &readWriteVolumePodTemplate.Template,
},
}, },
}, },
"negative_replicas": { "negative_replicas": {
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault}, rc: core.ReplicationController{
Spec: core.ReplicationControllerSpec{ ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Replicas: -1, Spec: core.ReplicationControllerSpec{
Selector: validSelector, Replicas: -1,
Selector: validSelector,
},
},
expectedOrigin: []string{
"minimum",
}, },
}, },
"invalid_label": { "invalid_label": {
ObjectMeta: metav1.ObjectMeta{ rc: core.ReplicationController{
Name: "abc-123", ObjectMeta: metav1.ObjectMeta{
Namespace: metav1.NamespaceDefault, Name: "abc-123",
Labels: map[string]string{ Namespace: metav1.NamespaceDefault,
"NoUppercaseOrSpecialCharsLike=Equals": "bar", Labels: map[string]string{
"NoUppercaseOrSpecialCharsLike=Equals": "bar",
},
},
Spec: core.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Template,
}, },
},
Spec: core.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Template,
}, },
}, },
"invalid_label 2": { "invalid_label 2": {
ObjectMeta: metav1.ObjectMeta{ rc: core.ReplicationController{
Name: "abc-123", ObjectMeta: metav1.ObjectMeta{
Namespace: metav1.NamespaceDefault, Name: "abc-123",
Labels: map[string]string{ Namespace: metav1.NamespaceDefault,
"NoUppercaseOrSpecialCharsLike=Equals": "bar", Labels: map[string]string{
"NoUppercaseOrSpecialCharsLike=Equals": "bar",
},
},
Spec: core.ReplicationControllerSpec{
Template: &invalidPodTemplate.Template,
}, },
},
Spec: core.ReplicationControllerSpec{
Template: &invalidPodTemplate.Template,
}, },
}, },
"invalid_annotation": { "invalid_annotation": {
ObjectMeta: metav1.ObjectMeta{ rc: core.ReplicationController{
Name: "abc-123", ObjectMeta: metav1.ObjectMeta{
Namespace: metav1.NamespaceDefault, Name: "abc-123",
Annotations: map[string]string{ Namespace: metav1.NamespaceDefault,
"NoUppercaseOrSpecialCharsLike=Equals": "bar", Annotations: map[string]string{
"NoUppercaseOrSpecialCharsLike=Equals": "bar",
},
},
Spec: core.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Template,
}, },
},
Spec: core.ReplicationControllerSpec{
Selector: validSelector,
Template: &validPodTemplate.Template,
}, },
}, },
"invalid restart policy 1": { "invalid restart policy 1": {
ObjectMeta: metav1.ObjectMeta{ rc: core.ReplicationController{
Name: "abc-123", ObjectMeta: metav1.ObjectMeta{
Namespace: metav1.NamespaceDefault, Name: "abc-123",
}, Namespace: metav1.NamespaceDefault,
Spec: core.ReplicationControllerSpec{ },
Selector: validSelector, Spec: core.ReplicationControllerSpec{
Template: &core.PodTemplateSpec{ Selector: validSelector,
Spec: podtest.MakePodSpec(podtest.SetRestartPolicy(core.RestartPolicyOnFailure)), Template: &core.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ Spec: podtest.MakePodSpec(podtest.SetRestartPolicy(core.RestartPolicyOnFailure)),
Labels: validSelector, ObjectMeta: metav1.ObjectMeta{
Labels: validSelector,
},
}, },
}, },
}, },
}, },
"invalid restart policy 2": { "invalid restart policy 2": {
ObjectMeta: metav1.ObjectMeta{ rc: core.ReplicationController{
Name: "abc-123", ObjectMeta: metav1.ObjectMeta{
Namespace: metav1.NamespaceDefault, Name: "abc-123",
}, Namespace: metav1.NamespaceDefault,
Spec: core.ReplicationControllerSpec{ },
Selector: validSelector, Spec: core.ReplicationControllerSpec{
Template: &core.PodTemplateSpec{ Selector: validSelector,
Spec: podtest.MakePodSpec(podtest.SetRestartPolicy(core.RestartPolicyNever)), Template: &core.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ Spec: podtest.MakePodSpec(podtest.SetRestartPolicy(core.RestartPolicyNever)),
Labels: validSelector, ObjectMeta: metav1.ObjectMeta{
Labels: validSelector,
},
}, },
}, },
}, },
}, },
"template may not contain ephemeral containers": { "template may not contain ephemeral containers": {
ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault}, rc: core.ReplicationController{
Spec: core.ReplicationControllerSpec{ ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault},
Replicas: 1, Spec: core.ReplicationControllerSpec{
Selector: validSelector, Replicas: 1,
Template: &core.PodTemplateSpec{ Selector: validSelector,
ObjectMeta: metav1.ObjectMeta{ Template: &core.PodTemplateSpec{
Labels: validSelector, ObjectMeta: metav1.ObjectMeta{
Labels: validSelector,
},
Spec: podtest.MakePodSpec(
podtest.SetEphemeralContainers(core.EphemeralContainer{EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}),
),
}, },
Spec: podtest.MakePodSpec(
podtest.SetEphemeralContainers(core.EphemeralContainer{EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}),
),
}, },
}, },
}, },
} }
for k, v := range errorCases { for k, v := range errorCases {
errs := ValidateReplicationController(&v, PodValidationOptions{}) errs := ValidateReplicationController(&v.rc, PodValidationOptions{})
if len(errs) == 0 { if len(errs) == 0 {
t.Errorf("expected failure for %s", k) t.Errorf("expected failure for %s", k)
} }
expectedOrigins := sets.NewString(v.expectedOrigin...)
for i := range errs { for i := range errs {
field := errs[i].Field field := errs[i].Field
if !strings.HasPrefix(field, "spec.template.") && if !strings.HasPrefix(field, "spec.template.") &&
@ -16944,6 +16979,16 @@ func TestValidateReplicationController(t *testing.T) {
field != "status.replicas" { field != "status.replicas" {
t.Errorf("%s: missing prefix for: %v", k, errs[i]) t.Errorf("%s: missing prefix for: %v", k, errs[i])
} }
if len(v.expectedOrigin) > 0 && errs[i].Origin != "" {
if !expectedOrigins.Has(errs[i].Origin) {
t.Errorf("%s: unexpected origin for: %v, expected one of %v", k, errs[i].Origin, v.expectedOrigin)
}
expectedOrigins.Delete(errs[i].Origin)
}
}
if len(expectedOrigins) > 0 {
t.Errorf("%s: missing errors with origin: %v", k, expectedOrigins.List())
} }
} }
} }
@ -20674,7 +20719,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
errorCases := map[string]struct { errorCases := map[string]struct {
endpoints core.Endpoints endpoints core.Endpoints
errorType field.ErrorType errorType field.ErrorType
errorDetail string errorOrigin string
}{ }{
"missing namespace": { "missing namespace": {
endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "mysvc"}}, endpoints: core.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "mysvc"}},
@ -20685,14 +20730,12 @@ func TestValidateEndpointsCreate(t *testing.T) {
errorType: "FieldValueRequired", errorType: "FieldValueRequired",
}, },
"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", errorType: "FieldValueInvalid",
errorDetail: dnsLabelErrMsg,
}, },
"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", errorType: "FieldValueInvalid",
errorDetail: dnsSubdomainLabelErrMsg,
}, },
"empty addresses": { "empty addresses": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20712,7 +20755,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}}, }},
}, },
errorType: "FieldValueInvalid", errorType: "FieldValueInvalid",
errorDetail: "must be a valid IP address", errorOrigin: "format=ip-sloppy",
}, },
"Multiple ports, one without name": { "Multiple ports, one without name": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20733,7 +20776,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}}, }},
}, },
errorType: "FieldValueInvalid", errorType: "FieldValueInvalid",
errorDetail: "between", errorOrigin: "portNum",
}, },
"Invalid protocol": { "Invalid protocol": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20754,7 +20797,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}}, }},
}, },
errorType: "FieldValueInvalid", errorType: "FieldValueInvalid",
errorDetail: "must be a valid IP address", errorOrigin: "format=ip-sloppy",
}, },
"Port missing number": { "Port missing number": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20765,7 +20808,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}}, }},
}, },
errorType: "FieldValueInvalid", errorType: "FieldValueInvalid",
errorDetail: "between", errorOrigin: "portNum",
}, },
"Port missing protocol": { "Port missing protocol": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20786,7 +20829,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}}, }},
}, },
errorType: "FieldValueInvalid", errorType: "FieldValueInvalid",
errorDetail: "loopback", errorOrigin: "format=non-special-ip",
}, },
"Address is link-local": { "Address is link-local": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20797,7 +20840,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}}, }},
}, },
errorType: "FieldValueInvalid", errorType: "FieldValueInvalid",
errorDetail: "link-local", errorOrigin: "format=non-special-ip",
}, },
"Address is link-local multicast": { "Address is link-local multicast": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20808,7 +20851,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}}, }},
}, },
errorType: "FieldValueInvalid", errorType: "FieldValueInvalid",
errorDetail: "link-local multicast", errorOrigin: "format=non-special-ip",
}, },
"Invalid AppProtocol": { "Invalid AppProtocol": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
@ -20819,14 +20862,14 @@ func TestValidateEndpointsCreate(t *testing.T) {
}}, }},
}, },
errorType: "FieldValueInvalid", errorType: "FieldValueInvalid",
errorDetail: "name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character", errorOrigin: "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 || !strings.Contains(errs[0].Detail, v.errorDetail) { if errs := ValidateEndpointsCreate(&v.endpoints); len(errs) == 0 || errs[0].Type != v.errorType || errs[0].Origin != v.errorOrigin {
t.Errorf("Expected error type %s with detail %q, got %v", v.errorType, v.errorDetail, errs) t.Errorf("Expected error type %s with origin %q, got %#v", v.errorType, v.errorOrigin, errs[0])
} }
}) })
} }
@ -21190,7 +21233,7 @@ func TestValidateSchedulingGates(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
errs := validateSchedulingGates(tt.schedulingGates, fieldPath) errs := validateSchedulingGates(tt.schedulingGates, fieldPath)
if diff := cmp.Diff(tt.wantFieldErrors, errs); diff != "" { if diff := cmp.Diff(tt.wantFieldErrors, errs, cmpopts.IgnoreFields(field.Error{}, "Detail", "Origin")); diff != "" {
t.Errorf("unexpected field errors (-want, +got):\n%s", diff) t.Errorf("unexpected field errors (-want, +got):\n%s", diff)
} }
}) })

View File

@ -19,9 +19,12 @@ package validation
import ( import (
"testing" "testing"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/dynamic-resource-allocation/api"
) )
// assertFailures compares the expected against the actual errors. // assertFailures compares the expected against the actual errors.
@ -31,22 +34,13 @@ import (
// is informative. // is informative.
func assertFailures(tb testing.TB, want, got field.ErrorList) bool { func assertFailures(tb testing.TB, want, got field.ErrorList) bool {
tb.Helper() tb.Helper()
if !assert.Equal(tb, want, got) { if diff := cmp.Diff(want, got, cmpopts.IgnoreFields(field.Error{}, "Origin"), cmp.AllowUnexported(api.UniqueString{})); diff != "" {
logFailures(tb, "Wanted failures", want) tb.Errorf("unexpected field errors (-want, +got):\n%s", diff)
logFailures(tb, "Got failures", got)
return false return false
} }
return true return true
} }
func logFailures(tb testing.TB, header string, errs field.ErrorList) {
tb.Helper()
tb.Logf("%s:\n", header)
for _, err := range errs {
tb.Logf("- %s\n", err)
}
}
func TestTruncateIfTooLong(t *testing.T) { func TestTruncateIfTooLong(t *testing.T) {
for name, tc := range map[string]struct { for name, tc := range map[string]struct {
str string str string

View File

@ -33,6 +33,20 @@ type Error struct {
Field string Field string
BadValue interface{} BadValue interface{}
Detail string 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=<dash-style-identifier>" 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{} var _ error = &Error{}
@ -96,6 +110,12 @@ func (v *Error) ErrorBody() string {
return s 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 // ErrorType is a machine readable value providing more detail about why
// a field is invalid. These values are expected to match 1-1 with // a field is invalid. These values are expected to match 1-1 with
// CauseType in api/types.go. // CauseType in api/types.go.
@ -169,32 +189,32 @@ func (t ErrorType) String() string {
// TypeInvalid returns a *Error indicating "type is invalid" // TypeInvalid returns a *Error indicating "type is invalid"
func TypeInvalid(field *Path, value interface{}, detail string) *Error { 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 // 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). // used to report failure to find a requested value (e.g. looking up an ID).
func NotFound(field *Path, value interface{}) *Error { 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 // Required returns a *Error indicating "value required". This is used
// to report required values that are not provided (e.g. empty strings, null // to report required values that are not provided (e.g. empty strings, null
// values, or empty arrays). // values, or empty arrays).
func Required(field *Path, detail string) *Error { 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 // Duplicate returns a *Error indicating "duplicate value". This is
// used to report collisions of values that must be unique (e.g. names or IDs). // used to report collisions of values that must be unique (e.g. names or IDs).
func Duplicate(field *Path, value interface{}) *Error { 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 // Invalid returns a *Error indicating "invalid value". This is used
// to report malformed values (e.g. failed regex match, too long, out of bounds). // to report malformed values (e.g. failed regex match, too long, out of bounds).
func Invalid(field *Path, value interface{}, detail string) *Error { 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". // 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, ", ") 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 // 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. // some conditions, but which are not permitted by current conditions (e.g.
// security policy). // security policy).
func Forbidden(field *Path, detail string) *Error { 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 // 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 { } else {
msg = "value is too long" msg = "value is too long"
} }
return &Error{ErrorTypeTooLong, field.String(), "<value omitted>", msg} return &Error{ErrorTypeTooLong, field.String(), "<value omitted>", msg, ""}
} }
// TooLongMaxLength returns a *Error indicating "too long". // TooLongMaxLength returns a *Error indicating "too long".
@ -259,14 +279,14 @@ func TooMany(field *Path, actualQuantity, maxQuantity int) *Error {
actual = omitValue 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 // InternalError returns a *Error indicating "internal error". This is used
// to signal that an error was found that was not directly related to user // to signal that an error was found that was not directly related to user
// input. The err argument must be non-nil. // input. The err argument must be non-nil.
func InternalError(field *Path, err error) *Error { 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 // 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. // ToAggregate converts the ErrorList into an errors.Aggregate.
func (list ErrorList) ToAggregate() utilerrors.Aggregate { func (list ErrorList) ToAggregate() utilerrors.Aggregate {
if len(list) == 0 { if len(list) == 0 {

View File

@ -173,3 +173,41 @@ func TestNotSupported(t *testing.T) {
t.Errorf("Expected: %s\n, but got: %s\n", expected, notSupported.ErrorBody()) 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)
}
}
}

View File

@ -373,7 +373,7 @@ func IsValidPortName(port string) []string {
func IsValidIP(fldPath *field.Path, value string) field.ErrorList { func IsValidIP(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList var allErrors field.ErrorList
if netutils.ParseIPSloppy(value) == nil { if netutils.ParseIPSloppy(value) == nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IP address, (e.g. 10.9.8.7 or 2001:db8::ffff)")) allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IP address, (e.g. 10.9.8.7 or 2001:db8::ffff)").WithOrigin("format=ip-sloppy"))
} }
return allErrors return allErrors
} }