Skip to content

K
k3s
Commit 4e046a8d authored by k8s-merge-robot's avatar k8s-merge-robot Committed by GitHub
Browse files
Options

Merge pull request #24802 from thockin/validation_pt8-10

Automatic merge from submit-queue Make validators return error strings Part of the larger validation PR, broken out for easier review and merge. Builds on previous PRs in the series. This is a group of smaller commits, so I don't waster precious PR numbers.
parents d07328dc 37786e0e
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 162 additions and 95 deletions
pkg/api/validation/validation_test.go
View file @ 4e046a8d
......@@ -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"}},
......@@ -1155,7 +1155,7 @@ func TestValidateEnv(t *testing.T) {
{
name: "name not a C identifier",
envs: []api.EnvVar{{Name: "a.b.c"}},
expectedError: `[0].name: Invalid value: "a.b.c": must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName"`,
expectedError: `[0].name: Invalid value: "a.b.c": must match the regex`,
},
{
name: "value and valueFrom specified",
......@@ -3489,13 +3489,20 @@ func TestValidateService(t *testing.T) {
numErrs: 1,
},
{
name: "invalid publicIPs",
name: "invalid publicIPs unspecified",
tweakSvc: func(s *api.Service) {
s.Spec.ExternalIPs = []string{"0.0.0.0"}
},
numErrs: 1,
},
{
name: "invalid publicIPs loopback",
tweakSvc: func(s *api.Service) {
s.Spec.ExternalIPs = []string{"127.0.0.1"}
},
numErrs: 1,
},
{
name: "invalid publicIPs host",
tweakSvc: func(s *api.Service) {
s.Spec.ExternalIPs = []string{"myhost.mydomain"}
......@@ -5968,7 +5975,7 @@ func TestValidateEndpoints(t *testing.T) {
},
},
errorType: "FieldValueInvalid",
errorDetail: PortRangeErrorMsg,
errorDetail: "between",
},
"Invalid protocol": {
endpoints: api.Endpoints{
......@@ -6006,7 +6013,7 @@ func TestValidateEndpoints(t *testing.T) {
},
},
errorType: "FieldValueInvalid",
errorDetail: PortRangeErrorMsg,
errorDetail: "between",
},
"Port missing protocol": {
endpoints: api.Endpoints{
......
pkg/apis/extensions/validation/validation.go
View file @ 4e046a8d
......@@ -149,8 +149,8 @@ func ValidatePositiveIntOrPercent(intOrPercent intstr.IntOrString, fldPath *fiel
allErrs := field.ErrorList{}
switch intOrPercent.Type {
case intstr.String:
if !validation.IsValidPercent(intOrPercent.StrVal) {
allErrs = append(allErrs, field.Invalid(fldPath, intOrPercent, "must be an integer or percentage (e.g '5%%')"))
for _, msg := range validation.IsValidPercent(intOrPercent.StrVal) {
allErrs = append(allErrs, field.Invalid(fldPath, intOrPercent, msg))
}
case intstr.Int:
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(intOrPercent.IntValue()), fldPath)...)
......@@ -161,7 +161,10 @@ func ValidatePositiveIntOrPercent(intOrPercent intstr.IntOrString, fldPath *fiel
}
func getPercentValue(intOrStringValue intstr.IntOrString) (int, bool) {
if intOrStringValue.Type != intstr.String || !validation.IsValidPercent(intOrStringValue.StrVal) {
if intOrStringValue.Type != intstr.String {
return 0, false
}
if len(validation.IsValidPercent(intOrStringValue.StrVal)) != 0 {
return 0, false
}
value, _ := strconv.Atoi(intOrStringValue.StrVal[:len(intOrStringValue.StrVal)-1])
......@@ -425,16 +428,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
}
......
pkg/apis/extensions/validation/validation_test.go
View file @ 4e046a8d
......@@ -632,7 +632,7 @@ func TestValidateDeployment(t *testing.T) {
MaxSurge: intstr.FromString("20Percent"),
},
}
errorCases["must be an integer or percentage"] = invalidMaxSurgeDeployment
errorCases["must match the regex"] = invalidMaxSurgeDeployment
// MaxSurge and MaxUnavailable cannot both be zero.
invalidRollingUpdateDeployment := validDeployment()
......
pkg/kubectl/run.go
View file @ 4e046a8d
......@@ -835,7 +835,10 @@ func parseEnvs(envArray []string) ([]api.EnvVar, error) {
}
name := env[:pos]
value := env[pos+1:]
if len(name) == 0 || !validation.IsCIdentifier(name) || len(value) == 0 {
if len(name) == 0 || len(value) == 0 {
return nil, fmt.Errorf("invalid env: %v", env)
}
if len(validation.IsCIdentifier(name)) != 0 {
return nil, fmt.Errorf("invalid env: %v", env)
}
envVar := api.EnvVar{Name: name, Value: value}
......@@ -853,7 +856,7 @@ func parseV1Envs(envArray []string) ([]v1.EnvVar, error) {
}
name := env[:pos]
value := env[pos+1:]
if len(name) == 0 || !validation.IsCIdentifier(name) || len(value) == 0 {
if len(name) == 0 || len(validation.IsCIdentifier(name)) != 0 || len(value) == 0 {
return nil, fmt.Errorf("invalid env: %v", env)
}
envVar := v1.EnvVar{Name: name, Value: value}
......
pkg/util/validation/validation.go
View file @ 4e046a8d
......@@ -145,13 +145,19 @@ var cIdentifierRegexp = regexp.MustCompile("^" + CIdentifierFmt + "$")
// IsCIdentifier tests for a string that conforms the definition of an identifier
// in C. This checks the format, but not the length.
func IsCIdentifier(value string) bool {
return cIdentifierRegexp.MatchString(value)
func IsCIdentifier(value string) []string {
if !cIdentifierRegexp.MatchString(value) {
return []string{RegexError(CIdentifierFmt, "my_name", "MY_NAME", "MyName")}
}
return nil
}
// 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 1 <= port && port <= 65535 {
return nil
}
return []string{InclusiveRangeError(1, 65535)}
}
// Now in libcontainer UID/GID limits is 0 ~ 1<<31 - 1
......@@ -163,67 +169,82 @@ const (
maxGroupID = math.MaxInt32
)
// IsValidGroupId tests that the argument is a valid gids.
func IsValidGroupId(gid int64) bool {
return minGroupID <= gid && gid <= maxGroupID
// IsValidGroupId tests that the argument is a valid Unix GID.
func IsValidGroupId(gid int64) []string {
if minGroupID <= gid && gid <= maxGroupID {
return nil
}
return []string{InclusiveRangeError(minGroupID, maxGroupID)}
}
// IsValidUserId tests that the argument is a valid uids.
func IsValidUserId(uid int64) bool {
return minUserID <= uid && uid <= maxUserID
// IsValidUserId tests that the argument is a valid Unix UID.
func IsValidUserId(uid int64) []string {
if minUserID <= uid && uid <= maxUserID {
return nil
}
return []string{InclusiveRangeError(minUserID, maxUserID)}
}
const doubleHyphensFmt string = ".*(--).*"
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
var portNameCharsetRegex = regexp.MustCompile("^[-a-z0-9]+$")
var portNameOneLetterRegexp = regexp.MustCompile("[a-z]")
// 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 !portNameCharsetRegex.MatchString(port) {
errs = append(errs, "must contain only alpha-numeric characters (a-z, 0-9), and hyphens (-)")
}
if !portNameOneLetterRegexp.MatchString(port) {
errs = append(errs, "must contain at least one letter (a-z)")
}
if doubleHyphensRegexp.MatchString(port) {
return false
if strings.Contains(port, "--") {
errs = append(errs, "must not contain consecutive hyphens")
}
if identifierNoHyphensBeginEndRegexp.MatchString(port) && atLeastOneLetterRegexp.MatchString(port) {
return true
if len(port) > 0 && (port[0] == '-' || port[len(port)-1] == '-') {
errs = append(errs, "must not begin or end with a hyphen")
}
return false
return errs
}
// IsValidIP tests that the argument is a valid IP address.
func IsValidIP(value string) bool {
return net.ParseIP(value) != nil
func IsValidIP(value string) []string {
if net.ParseIP(value) == nil {
return []string{"must be a valid IP address, (e.g. 10.9.8.7)"}
}
return nil
}
const percentFmt string = "[0-9]+%"
var percentRegexp = regexp.MustCompile("^" + percentFmt + "$")
func IsValidPercent(percent string) bool {
return percentRegexp.MatchString(percent)
func IsValidPercent(percent string) []string {
if !percentRegexp.MatchString(percent) {
return []string{RegexError(percentFmt, "1%", "93%")}
}
return nil
}
const HTTPHeaderNameFmt string = "[-A-Za-z0-9]+"
const httpHeaderNameFmt string = "[-A-Za-z0-9]+"
var httpHeaderNameRegexp = regexp.MustCompile("^" + HTTPHeaderNameFmt + "$")
var httpHeaderNameRegexp = regexp.MustCompile("^" + httpHeaderNameFmt + "$")
// IsHTTPHeaderName checks that a string conforms to the Go HTTP library's
// definition of a valid header field name (a stricter subset than RFC7230).
func IsHTTPHeaderName(value string) bool {
return httpHeaderNameRegexp.MatchString(value)
func IsHTTPHeaderName(value string) []string {
if !httpHeaderNameRegexp.MatchString(value) {
return []string{RegexError(httpHeaderNameFmt, "X-Header-Name")}
}
return nil
}
// MaxLenError returns a string explanation of a "string too long" validation
......@@ -260,3 +281,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)
}
pkg/util/validation/validation_test.go
View file @ 4e046a8d
......@@ -119,8 +119,8 @@ func TestIsCIdentifier(t *testing.T) {
"A", "AB", "AbC", "A1", "_A", "A_", "A_B", "A_1", "A__1__2__B", "__123_ABC",
}
for _, val := range goodValues {
if !IsCIdentifier(val) {
t.Errorf("expected true for '%s'", val)
if msgs := IsCIdentifier(val); len(msgs) != 0 {
t.Errorf("expected true for '%s': %v", val, msgs)
}
}
......@@ -132,7 +132,7 @@ func TestIsCIdentifier(t *testing.T) {
"#a#",
}
for _, val := range badValues {
if IsCIdentifier(val) {
if msgs := IsCIdentifier(val); len(msgs) == 0 {
t.Errorf("expected false for '%s'", val)
}
}
......@@ -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)
}
}
}
......@@ -157,14 +157,14 @@ func TestIsValidPortNum(t *testing.T) {
func TestIsValidGroupId(t *testing.T) {
goodValues := []int64{0, 1, 1000, 65535, 2147483647}
for _, val := range goodValues {
if !IsValidGroupId(val) {
t.Errorf("expected true for '%d'", val)
if msgs := IsValidGroupId(val); len(msgs) != 0 {
t.Errorf("expected true for '%d': %v", val, msgs)
}
}
badValues := []int64{-1, -1003, 2147483648, 4147483647}
for _, val := range badValues {
if IsValidGroupId(val) {
if msgs := IsValidGroupId(val); len(msgs) == 0 {
t.Errorf("expected false for '%d'", val)
}
}
......@@ -173,14 +173,14 @@ func TestIsValidGroupId(t *testing.T) {
func TestIsValidUserId(t *testing.T) {
goodValues := []int64{0, 1, 1000, 65535, 2147483647}
for _, val := range goodValues {
if !IsValidUserId(val) {
t.Errorf("expected true for '%d'", val)
if msgs := IsValidUserId(val); len(msgs) != 0 {
t.Errorf("expected true for '%d': %v", val, msgs)
}
}
badValues := []int64{-1, -1003, 2147483648, 4147483647}
for _, val := range badValues {
if IsValidUserId(val) {
if msgs := IsValidUserId(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)
}
}
......@@ -293,8 +293,8 @@ func TestIsValidIP(t *testing.T) {
"0.0.0.0",
}
for _, val := range goodValues {
if !IsValidIP(val) {
t.Errorf("expected true for %q", val)
if msgs := IsValidIP(val); len(msgs) != 0 {
t.Errorf("expected true for %q: %v", val, msgs)
}
}
......@@ -306,7 +306,7 @@ func TestIsValidIP(t *testing.T) {
"a",
}
for _, val := range badValues {
if IsValidIP(val) {
if msgs := IsValidIP(val); len(msgs) == 0 {
t.Errorf("expected false for %q", val)
}
}
......@@ -321,8 +321,8 @@ func TestIsHTTPHeaderName(t *testing.T) {
"A", "AB", "AbC", "A1", "-A", "A-", "A-B", "A-1", "A--1--2--B", "--123-ABC",
}
for _, val := range goodValues {
if !IsHTTPHeaderName(val) {
t.Errorf("expected true for '%s'", val)
if msgs := IsHTTPHeaderName(val); len(msgs) != 0 {
t.Errorf("expected true for '%s': %v", val, msgs)
}
}
......@@ -333,8 +333,44 @@ func TestIsHTTPHeaderName(t *testing.T) {
"?", "@", "{",
}
for _, val := range badValues {
if IsHTTPHeaderName(val) {
if msgs := IsHTTPHeaderName(val); len(msgs) == 0 {
t.Errorf("expected false for '%s'", val)
}
}
}
func TestIsValidPercent(t *testing.T) {
goodValues := []string{
"0%",
"00000%",
"1%",
"01%",
"99%",
"100%",
"101%",
}
for _, val := range goodValues {
if msgs := IsValidPercent(val); len(msgs) != 0 {
t.Errorf("expected true for %q: %v", val, msgs)
}
}
badValues := []string{
"",
"0",
"100",
"0.0%",
"99.9%",
"hundred",
" 1%",
"1% ",
"-0%",
"-1%",
"+1%",
}
for _, val := range badValues {
if msgs := IsValidPercent(val); len(msgs) == 0 {
t.Errorf("expected false for %q", val)
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment