From 519dd6887dc275f77d5be32a1f711ff71117c87d Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 26 Dec 2023 21:11:15 -0500 Subject: [PATCH] Make validation.IsValidIP return a field.ErrorList for consistency --- pkg/apis/core/validation/validation.go | 24 ++++++------------- .../pkg/util/validation/validation.go | 7 +++--- .../pkg/util/validation/validation_test.go | 16 ++++++------- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index a6f7fef3012..fde26bf97ee 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4027,9 +4027,7 @@ func validatePodIPs(pod *core.Pod) field.ErrorList { // all PodIPs must be valid IPs for i, podIP := range pod.Status.PodIPs { - for _, msg := range validation.IsValidIP(podIP.IP) { - allErrs = append(allErrs, field.Invalid(podIPsField.Index(i), podIP.IP, msg)) - } + allErrs = append(allErrs, validation.IsValidIP(podIPsField.Index(i), podIP.IP)...) } // if we have more than one Pod.PodIP then @@ -4081,9 +4079,7 @@ func validateHostIPs(pod *core.Pod) field.ErrorList { // all HostPs must be valid IPs for i, hostIP := range pod.Status.HostIPs { - for _, msg := range validation.IsValidIP(hostIP.IP) { - allErrs = append(allErrs, field.Invalid(hostIPsField.Index(i), hostIP.IP, msg)) - } + allErrs = append(allErrs, validation.IsValidIP(hostIPsField.Index(i), hostIP.IP)...) } // if we have more than one Pod.HostIP then @@ -5399,10 +5395,8 @@ func ValidateService(service *core.Service) field.ErrorList { ipPath := specPath.Child("externalIPs") for i, ip := range service.Spec.ExternalIPs { idxPath := ipPath.Index(i) - if msgs := validation.IsValidIP(ip); len(msgs) != 0 { - for i := range msgs { - allErrs = append(allErrs, field.Invalid(idxPath, ip, msgs[i])) - } + if errs := validation.IsValidIP(idxPath, ip); len(errs) != 0 { + allErrs = append(allErrs, errs...) } else { allErrs = append(allErrs, ValidateNonSpecialIP(ip, idxPath)...) } @@ -6914,9 +6908,7 @@ func validateEndpointSubsets(subsets []core.EndpointSubset, fldPath *field.Path) func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - for _, msg := range validation.IsValidIP(address.IP) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), address.IP, msg)) - } + allErrs = append(allErrs, validation.IsValidIP(fldPath.Child("ip"), address.IP)...) if len(address.Hostname) > 0 { allErrs = append(allErrs, ValidateDNS1123Label(address.Hostname, fldPath.Child("hostname"))...) } @@ -7618,11 +7610,9 @@ func ValidateServiceClusterIPsRelatedFields(service *core.Service) field.ErrorLi } // is it valid ip? - errorMessages := validation.IsValidIP(clusterIP) + errorMessages := validation.IsValidIP(clusterIPsField.Index(i), clusterIP) hasInvalidIPs = (len(errorMessages) != 0) || hasInvalidIPs - for _, msg := range errorMessages { - allErrs = append(allErrs, field.Invalid(clusterIPsField.Index(i), clusterIP, msg)) - } + allErrs = append(allErrs, errorMessages...) } // max two diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go index 6f8a2e26bdb..0eb33e68aaf 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go @@ -350,11 +350,12 @@ func IsValidPortName(port string) []string { } // IsValidIP tests that the argument is a valid IP address. -func IsValidIP(value string) []string { +func IsValidIP(fldPath *field.Path, value string) field.ErrorList { + var allErrors field.ErrorList if netutils.ParseIPSloppy(value) == nil { - return []string{"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)")) } - return nil + return allErrors } // IsValidIPv4Address tests that the argument is a valid IPv4 address. diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go index aeda7bf5662..2ce11d560f7 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go @@ -458,20 +458,20 @@ func TestIsValidIP(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - msgs := IsValidIP(tc.in) + errs := IsValidIP(field.NewPath(""), tc.in) if tc.err == "" { - if len(msgs) != 0 { - t.Errorf("expected %q to be valid but got: %v", tc.in, msgs) + if len(errs) != 0 { + t.Errorf("expected %q to be valid but got: %v", tc.in, errs) } } else { - if len(msgs) != 1 { - t.Errorf("expected %q to have 1 error but got: %v", tc.in, msgs) - } else if !strings.Contains(msgs[0], tc.err) { - t.Errorf("expected error for %q to contain %q but got: %q", tc.in, tc.err, msgs[0]) + if len(errs) != 1 { + t.Errorf("expected %q to have 1 error but got: %v", tc.in, errs) + } else if !strings.Contains(errs[0].Detail, tc.err) { + t.Errorf("expected error for %q to contain %q but got: %q", tc.in, tc.err, errs[0].Detail) } } - errs := IsValidIPv4Address(field.NewPath(""), tc.in) + errs = IsValidIPv4Address(field.NewPath(""), tc.in) if tc.family == 4 { if len(errs) != 0 { t.Errorf("expected %q to pass IsValidIPv4Address but got: %v", tc.in, errs)