From 14bece550fc683966b4bf62e2d2fc893921182fc Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Mon, 4 Jan 2016 08:33:26 -0800 Subject: [PATCH] Make IsValidPortNum/Name return error strings --- pkg/api/validation/validation.go | 86 ++++++++++---------- pkg/api/validation/validation_test.go | 22 ++--- pkg/apis/extensions/validation/validation.go | 11 +-- pkg/util/validation/validation.go | 61 ++++++++------ pkg/util/validation/validation_test.go | 16 ++-- 5 files changed, 99 insertions(+), 97 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 6b6b75a90b9..eeb014497e8 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -53,14 +53,8 @@ const isInvalidQuotaResource string = `must be a standard resource for quota` const fieldImmutableErrorMsg string = `field is immutable` const isNotIntegerErrorMsg string = `must be an integer` -func InclusiveRangeErrorMsg(lo, hi int) string { - return fmt.Sprintf(`must be between %d and %d, inclusive`, lo, hi) -} - -var pdPartitionErrorMsg string = InclusiveRangeErrorMsg(1, 255) -var PortRangeErrorMsg string = InclusiveRangeErrorMsg(1, 65535) -var IdRangeErrorMsg string = InclusiveRangeErrorMsg(0, math.MaxInt32) -var PortNameErrorMsg string = fmt.Sprintf(`must be an IANA_SVC_NAME (at most 15 characters, matching regex %s, it must contain at least one letter [a-z], and hyphens cannot be adjacent to other hyphens): e.g. "http"`, validation.IdentifierNoHyphensBeginEndFmt) +var pdPartitionErrorMsg string = validation.InclusiveRangeError(1, 255) +var IdRangeErrorMsg string = validation.InclusiveRangeError(0, math.MaxInt32) const totalAnnotationSizeLimitB int = 256 * (1 << 10) // 256 kB @@ -609,7 +603,7 @@ func validateISCSIVolumeSource(iscsi *api.ISCSIVolumeSource, fldPath *field.Path allErrs = append(allErrs, field.Required(fldPath.Child("iqn"), "")) } if iscsi.Lun < 0 || iscsi.Lun > 255 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("lun"), iscsi.Lun, InclusiveRangeErrorMsg(0, 255))) + allErrs = append(allErrs, field.Invalid(fldPath.Child("lun"), iscsi.Lun, validation.InclusiveRangeError(0, 255))) } return allErrs } @@ -624,7 +618,7 @@ func validateFCVolumeSource(fc *api.FCVolumeSource, fldPath *field.Path) field.E allErrs = append(allErrs, field.Required(fldPath.Child("lun"), "")) } else { if *fc.Lun < 0 || *fc.Lun > 255 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("lun"), fc.Lun, InclusiveRangeErrorMsg(0, 255))) + allErrs = append(allErrs, field.Invalid(fldPath.Child("lun"), fc.Lun, validation.InclusiveRangeError(0, 255))) } } return allErrs @@ -1054,8 +1048,10 @@ func validateContainerPorts(ports []api.ContainerPort, fldPath *field.Path) fiel for i, port := range ports { idxPath := fldPath.Index(i) if len(port.Name) > 0 { - if !validation.IsValidPortName(port.Name) { - allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), port.Name, PortNameErrorMsg)) + if msgs := validation.IsValidPortName(port.Name); len(msgs) != 0 { + for i = range msgs { + allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), port.Name, msgs[i])) + } } else if allNames.Has(port.Name) { allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), port.Name)) } else { @@ -1063,12 +1059,16 @@ func validateContainerPorts(ports []api.ContainerPort, fldPath *field.Path) fiel } } if port.ContainerPort == 0 { - allErrs = append(allErrs, field.Invalid(idxPath.Child("containerPort"), port.ContainerPort, PortRangeErrorMsg)) - } else if !validation.IsValidPortNum(int(port.ContainerPort)) { - allErrs = append(allErrs, field.Invalid(idxPath.Child("containerPort"), port.ContainerPort, PortRangeErrorMsg)) + allErrs = append(allErrs, field.Required(idxPath.Child("containerPort"), "")) + } else { + for _, msg := range validation.IsValidPortNum(int(port.ContainerPort)) { + allErrs = append(allErrs, field.Invalid(idxPath.Child("containerPort"), port.ContainerPort, msg)) + } } - if port.HostPort != 0 && !validation.IsValidPortNum(int(port.HostPort)) { - allErrs = append(allErrs, field.Invalid(idxPath.Child("hostPort"), port.HostPort, PortRangeErrorMsg)) + if port.HostPort != 0 { + for _, msg := range validation.IsValidPortNum(int(port.HostPort)) { + allErrs = append(allErrs, field.Invalid(idxPath.Child("hostPort"), port.HostPort, msg)) + } } if len(port.Protocol) == 0 { allErrs = append(allErrs, field.Required(idxPath.Child("protocol"), "")) @@ -1304,19 +1304,16 @@ func validateExecAction(exec *api.ExecAction, fldPath *field.Path) field.ErrorLi return allErrors } +var supportedHTTPSchemes = sets.NewString(string(api.URISchemeHTTP), string(api.URISchemeHTTPS)) + func validateHTTPGetAction(http *api.HTTPGetAction, fldPath *field.Path) field.ErrorList { allErrors := field.ErrorList{} if len(http.Path) == 0 { allErrors = append(allErrors, field.Required(fldPath.Child("path"), "")) } - if http.Port.Type == intstr.Int && !validation.IsValidPortNum(http.Port.IntValue()) { - allErrors = append(allErrors, field.Invalid(fldPath.Child("port"), http.Port, PortRangeErrorMsg)) - } else if http.Port.Type == intstr.String && !validation.IsValidPortName(http.Port.StrVal) { - allErrors = append(allErrors, field.Invalid(fldPath.Child("port"), http.Port.StrVal, PortNameErrorMsg)) - } - supportedSchemes := sets.NewString(string(api.URISchemeHTTP), string(api.URISchemeHTTPS)) - if !supportedSchemes.Has(string(http.Scheme)) { - allErrors = append(allErrors, field.Invalid(fldPath.Child("scheme"), http.Scheme, fmt.Sprintf("must be one of %v", supportedSchemes.List()))) + allErrors = append(allErrors, ValidatePortNumOrName(http.Port, fldPath.Child("port"))...) + if !supportedHTTPSchemes.Has(string(http.Scheme)) { + allErrors = append(allErrors, field.NotSupported(fldPath.Child("scheme"), http.Scheme, supportedHTTPSchemes.List())) } for _, header := range http.HTTPHeaders { if !validation.IsHTTPHeaderName(header.Name) { @@ -1326,14 +1323,24 @@ func validateHTTPGetAction(http *api.HTTPGetAction, fldPath *field.Path) field.E return allErrors } -func validateTCPSocketAction(tcp *api.TCPSocketAction, fldPath *field.Path) field.ErrorList { - allErrors := field.ErrorList{} - if tcp.Port.Type == intstr.Int && !validation.IsValidPortNum(tcp.Port.IntValue()) { - allErrors = append(allErrors, field.Invalid(fldPath.Child("port"), tcp.Port, PortRangeErrorMsg)) - } else if tcp.Port.Type == intstr.String && !validation.IsValidPortName(tcp.Port.StrVal) { - allErrors = append(allErrors, field.Invalid(fldPath.Child("port"), tcp.Port.StrVal, PortNameErrorMsg)) +func ValidatePortNumOrName(port intstr.IntOrString, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if port.Type == intstr.Int { + for _, msg := range validation.IsValidPortNum(port.IntValue()) { + allErrs = append(allErrs, field.Invalid(fldPath, port.IntValue(), msg)) + } + } else if port.Type == intstr.String { + for _, msg := range validation.IsValidPortName(port.StrVal) { + allErrs = append(allErrs, field.Invalid(fldPath, port.StrVal, msg)) + } + } else { + allErrs = append(allErrs, field.InternalError(fldPath, fmt.Errorf("unknown type: %v", port.Type))) } - return allErrors + return allErrs +} + +func validateTCPSocketAction(tcp *api.TCPSocketAction, fldPath *field.Path) field.ErrorList { + return ValidatePortNumOrName(tcp.Port, fldPath.Child("port")) } func validateHandler(handler *api.Handler, fldPath *field.Path) field.ErrorList { @@ -2162,8 +2169,8 @@ func validateServicePort(sp *api.ServicePort, requireName, isHeadlessService boo } } - if !validation.IsValidPortNum(int(sp.Port)) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), sp.Port, PortRangeErrorMsg)) + for _, msg := range validation.IsValidPortNum(int(sp.Port)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), sp.Port, msg)) } if len(sp.Protocol) == 0 { @@ -2172,12 +2179,7 @@ func validateServicePort(sp *api.ServicePort, requireName, isHeadlessService boo allErrs = append(allErrs, field.NotSupported(fldPath.Child("protocol"), sp.Protocol, supportedPortProtocols.List())) } - if sp.TargetPort.Type == intstr.Int && !validation.IsValidPortNum(sp.TargetPort.IntValue()) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("targetPort"), sp.TargetPort, PortRangeErrorMsg)) - } - if sp.TargetPort.Type == intstr.String && !validation.IsValidPortName(sp.TargetPort.StrVal) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("targetPort"), sp.TargetPort, PortNameErrorMsg)) - } + allErrs = append(allErrs, ValidatePortNumOrName(sp.TargetPort, fldPath.Child("targetPort"))...) // in the v1 API, targetPorts on headless services were tolerated. // once we have version-specific validation, we can reject this on newer API versions, but until then, we have to tolerate it for compatibility. @@ -3075,8 +3077,8 @@ func validateEndpointPort(port *api.EndpointPort, requireName bool, fldPath *fie allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), port.Name, msg)) } } - if !validation.IsValidPortNum(int(port.Port)) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), port.Port, PortRangeErrorMsg)) + for _, msg := range validation.IsValidPortNum(int(port.Port)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), port.Port, msg)) } if len(port.Protocol) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("protocol"), "")) diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 25c2f931875..68ae8fbcd7f 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1030,17 +1030,17 @@ func TestValidatePorts(t *testing.T) { "name > 15 characters": { []api.ContainerPort{{Name: strings.Repeat("a", 16), ContainerPort: 80, Protocol: "TCP"}}, field.ErrorTypeInvalid, - "name", PortNameErrorMsg, + "name", "15", }, - "name not a IANA svc name ": { + "name contains invalid characters": { []api.ContainerPort{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, field.ErrorTypeInvalid, - "name", PortNameErrorMsg, + "name", "alpha-numeric", }, - "name not a IANA svc name (i.e. a number)": { + "name is a number": { []api.ContainerPort{{Name: "80", ContainerPort: 80, Protocol: "TCP"}}, field.ErrorTypeInvalid, - "name", PortNameErrorMsg, + "name", "at least one letter", }, "name not unique": { []api.ContainerPort{ @@ -1052,18 +1052,18 @@ func TestValidatePorts(t *testing.T) { }, "zero container port": { []api.ContainerPort{{ContainerPort: 0, Protocol: "TCP"}}, - field.ErrorTypeInvalid, - "containerPort", PortRangeErrorMsg, + field.ErrorTypeRequired, + "containerPort", "", }, "invalid container port": { []api.ContainerPort{{ContainerPort: 65536, Protocol: "TCP"}}, field.ErrorTypeInvalid, - "containerPort", PortRangeErrorMsg, + "containerPort", "between", }, "invalid host port": { []api.ContainerPort{{ContainerPort: 80, HostPort: 65536, Protocol: "TCP"}}, field.ErrorTypeInvalid, - "hostPort", PortRangeErrorMsg, + "hostPort", "between", }, "invalid protocol case": { []api.ContainerPort{{ContainerPort: 80, Protocol: "tcp"}}, @@ -5968,7 +5968,7 @@ func TestValidateEndpoints(t *testing.T) { }, }, errorType: "FieldValueInvalid", - errorDetail: PortRangeErrorMsg, + errorDetail: "between", }, "Invalid protocol": { endpoints: api.Endpoints{ @@ -6006,7 +6006,7 @@ func TestValidateEndpoints(t *testing.T) { }, }, errorType: "FieldValueInvalid", - errorDetail: PortRangeErrorMsg, + errorDetail: "between", }, "Port missing protocol": { endpoints: api.Endpoints{ diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index c061a03f391..d5e25665394 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -425,16 +425,7 @@ func validateIngressBackend(backend *extensions.IngressBackend, fldPath *field.P allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceName"), backend.ServiceName, msg)) } } - if backend.ServicePort.Type == intstr.String { - for _, msg := range validation.IsDNS1123Label(backend.ServicePort.StrVal) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("servicePort"), backend.ServicePort.StrVal, msg)) - } - if !validation.IsValidPortName(backend.ServicePort.StrVal) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("servicePort"), backend.ServicePort.StrVal, apivalidation.PortNameErrorMsg)) - } - } else if !validation.IsValidPortNum(backend.ServicePort.IntValue()) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("servicePort"), backend.ServicePort, apivalidation.PortRangeErrorMsg)) - } + allErrs = append(allErrs, apivalidation.ValidatePortNumOrName(backend.ServicePort, fldPath.Child("servicePort"))...) return allErrs } diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go index bdb99b1e322..30a1cd16e30 100644 --- a/pkg/util/validation/validation.go +++ b/pkg/util/validation/validation.go @@ -153,8 +153,11 @@ func IsCIdentifier(value string) []string { } // IsValidPortNum tests that the argument is a valid, non-zero port number. -func IsValidPortNum(port int) bool { - return 0 < port && port < 65536 +func IsValidPortNum(port int) []string { + if port < 1 || port > 65535 { + return []string{InclusiveRangeError(1, 65535)} + } + return nil } // Now in libcontainer UID/GID limits is 0 ~ 1<<31 - 1 @@ -176,34 +179,34 @@ func IsValidUserId(uid int64) bool { return minUserID <= uid && uid <= maxUserID } -const doubleHyphensFmt string = ".*(--).*" +var portNameCharsetRegex = regexp.MustCompile("^[-a-z0-9]+$") +var portNameOneLetterRegexp = regexp.MustCompile("[a-z]") -var doubleHyphensRegexp = regexp.MustCompile("^" + doubleHyphensFmt + "$") - -const IdentifierNoHyphensBeginEndFmt string = "[a-z0-9]([a-z0-9-]*[a-z0-9])*" - -var identifierNoHyphensBeginEndRegexp = regexp.MustCompile("^" + IdentifierNoHyphensBeginEndFmt + "$") - -const atLeastOneLetterFmt string = ".*[a-z].*" - -var atLeastOneLetterRegexp = regexp.MustCompile("^" + atLeastOneLetterFmt + "$") - -// IsValidPortName check that the argument is valid syntax. It must be non empty and no more than 15 characters long -// It must contains at least one letter [a-z] and it must contains only [a-z0-9-]. -// Hypens ('-') cannot be leading or trailing character of the string and cannot be adjacent to other hyphens. -// Although RFC 6335 allows upper and lower case characters but case is ignored for comparison purposes: (HTTP -// and http denote the same service). -func IsValidPortName(port string) bool { - if len(port) < 1 || len(port) > 15 { - return false +// IsValidPortName check that the argument is valid syntax. It must be +// non-empty and no more than 15 characters long. It may contain only [-a-z0-9] +// and must contain at least one letter [a-z]. It must not start or end with a +// hyphen, nor contain adjacent hyphens. +// +// Note: We only allow lower-case characters, even though RFC 6335 is case +// insensitive. +func IsValidPortName(port string) []string { + var errs []string + if len(port) > 15 { + errs = append(errs, MaxLenError(15)) } - if doubleHyphensRegexp.MatchString(port) { - return false + if !portNameCharsetRegex.MatchString(port) { + errs = append(errs, "must contain only alpha-numeric characters (a-z, 0-9), and hyphens (-)") } - if identifierNoHyphensBeginEndRegexp.MatchString(port) && atLeastOneLetterRegexp.MatchString(port) { - return true + if !portNameOneLetterRegexp.MatchString(port) { + errs = append(errs, "must contain at least one letter (a-z)") } - return false + if strings.Contains(port, "--") { + errs = append(errs, "must not contain consecutive hyphens") + } + if len(port) > 0 && (port[0] == '-' || port[len(port)-1] == '-') { + errs = append(errs, "must not begin or end with a hyphen") + } + return errs } // IsValidIP tests that the argument is a valid IP address. @@ -263,3 +266,9 @@ func prefixEach(msgs []string, prefix string) []string { } return msgs } + +// InclusiveRangeError returns a string explanation of a numeric "must be +// between" validation failure. +func InclusiveRangeError(lo, hi int) string { + return fmt.Sprintf(`must be between %d and %d, inclusive`, lo, hi) +} diff --git a/pkg/util/validation/validation_test.go b/pkg/util/validation/validation_test.go index 6b248351238..75974231e07 100644 --- a/pkg/util/validation/validation_test.go +++ b/pkg/util/validation/validation_test.go @@ -141,15 +141,15 @@ func TestIsCIdentifier(t *testing.T) { func TestIsValidPortNum(t *testing.T) { goodValues := []int{1, 2, 1000, 16384, 32768, 65535} for _, val := range goodValues { - if !IsValidPortNum(val) { - t.Errorf("expected true for '%d'", val) + if msgs := IsValidPortNum(val); len(msgs) != 0 { + t.Errorf("expected true for %d, got %v", val, msgs) } } badValues := []int{0, -1, 65536, 100000} for _, val := range badValues { - if IsValidPortNum(val) { - t.Errorf("expected false for '%d'", val) + if msgs := IsValidPortNum(val); len(msgs) == 0 { + t.Errorf("expected false for %d", val) } } } @@ -189,14 +189,14 @@ func TestIsValidUserId(t *testing.T) { func TestIsValidPortName(t *testing.T) { goodValues := []string{"telnet", "re-mail-ck", "pop3", "a", "a-1", "1-a", "a-1-b-2-c", "1-a-2-b-3"} for _, val := range goodValues { - if !IsValidPortName(val) { - t.Errorf("expected true for %q", val) + if msgs := IsValidPortName(val); len(msgs) != 0 { + t.Errorf("expected true for %q: %v", val, msgs) } } - badValues := []string{"longerthan15characters", "", "12345", "1-2-3-4", "-begin", "end-", "two--hyphens", "1-2", "whois++"} + badValues := []string{"longerthan15characters", "", strings.Repeat("a", 16), "12345", "1-2-3-4", "-begin", "end-", "two--hyphens", "whois++"} for _, val := range badValues { - if IsValidPortName(val) { + if msgs := IsValidPortName(val); len(msgs) == 0 { t.Errorf("expected false for %q", val) } }