Merge pull request #38090 from xingzhou/kube-37654

Automatic merge from submit-queue (batch tested with PRs 38920, 38090)

Improve error message for name/label validation.

Instead of just providing regex in name/label validation error output, we need to add the naming rules of the name/label, which is more end-user readable.

Fixed #37654

pkg/api/validation/validation_test.go

@@ -36,6 +36,17 @@ import (
 	"k8s.io/kubernetes/pkg/util/validation/field"
 )
 
+const (
+	dnsLabelErrMsg          = "a valid DNS (RFC 1123) label must consist of"
+	dnsSubdomainLabelErrMsg = "a valid DNS (RFC 1123) subdomain"
+	labelErrMsg             = "a valid label must be an empty string or consist of"
+	lowerCaseLabelErrMsg    = "a valid label must consist of"
+	maxLengthErrMsg         = "must be no more than"
+	namePartErrMsg          = "name part must consist of"
+	nameErrMsg              = "a qualified name must consist of"
+	idErrMsg                = "a valid C identifier must"
+)
+
 func expectPrefix(t *testing.T, prefix string, errs field.ErrorList) {
 	for i := range errs {
 		if f, p := errs[i].Field, prefix; !strings.HasPrefix(f, p) {
@@ -458,10 +469,10 @@ func TestValidateAnnotations(t *testing.T) {
 		annotations map[string]string
 		expect      string
 	}{
-		{map[string]string{"nospecialchars^=@": "bar"}, "must match the regex"},
+		{map[string]string{"nospecialchars^=@": "bar"}, namePartErrMsg},
-		{map[string]string{"cantendwithadash-": "bar"}, "must match the regex"},
+		{map[string]string{"cantendwithadash-": "bar"}, namePartErrMsg},
-		{map[string]string{"only/one/slash": "bar"}, "must match the regex"},
+		{map[string]string{"only/one/slash": "bar"}, nameErrMsg},
-		{map[string]string{strings.Repeat("a", 254): "bar"}, "must be no more than"},
+		{map[string]string{strings.Repeat("a", 254): "bar"}, maxLengthErrMsg},
 	}
 	for i := range nameErrorCases {
 		errs := ValidateAnnotations(nameErrorCases[i].annotations, field.NewPath("field"))
@@ -1152,7 +1163,7 @@ func TestValidateVolumes(t *testing.T) {
 			},
 			errtype:   field.ErrorTypeInvalid,
 			errfield:  "name",
-			errdetail: "must match the regex",
+			errdetail: dnsLabelErrMsg,
 		},
 		// More than one source field specified.
 		{
@@ -2530,7 +2541,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 match the regex`,
+			expectedError: `[0].name: Invalid value: "a.b.c": ` + idErrMsg,
 		},
 		{
 			name: "value and valueFrom specified",
@@ -7288,11 +7299,11 @@ func TestValidateLimitRange(t *testing.T) {
 		},
 		"invalid-name": {
 			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "^Invalid", Namespace: "foo"}, Spec: api.LimitRangeSpec{}},
-			"must match the regex",
+			dnsSubdomainLabelErrMsg,
 		},
 		"invalid-namespace": {
 			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: api.LimitRangeSpec{}},
-			"must match the regex",
+			dnsLabelErrMsg,
 		},
 		"duplicate-limit-type": {
 			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: api.LimitRangeSpec{
@@ -7634,11 +7645,11 @@ func TestValidateResourceQuota(t *testing.T) {
 		},
 		"invalid Name": {
 			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "^Invalid", Namespace: "foo"}, Spec: spec},
-			"must match the regex",
+			dnsSubdomainLabelErrMsg,
 		},
 		"invalid Namespace": {
 			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: spec},
-			"must match the regex",
+			dnsLabelErrMsg,
 		},
 		"negative-limits": {
 			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: negativeSpec},
@@ -8232,12 +8243,12 @@ func TestValidateEndpoints(t *testing.T) {
 		"invalid namespace": {
 			endpoints:   api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "no@#invalid.;chars\"allowed"}},
 			errorType:   "FieldValueInvalid",
-			errorDetail: "must match the regex",
+			errorDetail: dnsLabelErrMsg,
 		},
 		"invalid name": {
 			endpoints:   api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "-_Invliad^&Characters", Namespace: "namespace"}},
 			errorType:   "FieldValueInvalid",
-			errorDetail: "must match the regex",
+			errorDetail: dnsSubdomainLabelErrMsg,
 		},
 		"empty addresses": {
 			endpoints: api.Endpoints{

pkg/apis/extensions/validation/validation_test.go

@@ -750,7 +750,7 @@ func TestValidateDeployment(t *testing.T) {
 			MaxSurge: intstr.FromString("20Percent"),
 		},
 	}
-	errorCases["must match the regex"] = invalidMaxSurgeDeployment
+	errorCases["a valid percent string must be"] = invalidMaxSurgeDeployment
 
 	// MaxSurge and MaxUnavailable cannot both be zero.
 	invalidRollingUpdateDeployment := validDeployment()

pkg/apis/meta/v1/validation/validation_test.go

@@ -47,14 +47,19 @@ func TestValidateLabels(t *testing.T) {
 		}
 	}
 
+	namePartErrMsg := "name part must consist of"
+	nameErrMsg := "a qualified name must consist of"
+	labelErrMsg := "a valid label must be an empty string or consist of"
+	maxLengthErrMsg := "must be no more than"
+
 	labelNameErrorCases := []struct {
 		labels map[string]string
 		expect string
 	}{
-		{map[string]string{"nospecialchars^=@": "bar"}, "must match the regex"},
+		{map[string]string{"nospecialchars^=@": "bar"}, namePartErrMsg},
-		{map[string]string{"cantendwithadash-": "bar"}, "must match the regex"},
+		{map[string]string{"cantendwithadash-": "bar"}, namePartErrMsg},
-		{map[string]string{"only/one/slash": "bar"}, "must match the regex"},
+		{map[string]string{"only/one/slash": "bar"}, nameErrMsg},
-		{map[string]string{strings.Repeat("a", 254): "bar"}, "must be no more than"},
+		{map[string]string{strings.Repeat("a", 254): "bar"}, maxLengthErrMsg},
 	}
 	for i := range labelNameErrorCases {
 		errs := ValidateLabels(labelNameErrorCases[i].labels, field.NewPath("field"))
@@ -71,10 +76,10 @@ func TestValidateLabels(t *testing.T) {
 		labels map[string]string
 		expect string
 	}{
-		{map[string]string{"toolongvalue": strings.Repeat("a", 64)}, "must be no more than"},
+		{map[string]string{"toolongvalue": strings.Repeat("a", 64)}, maxLengthErrMsg},
-		{map[string]string{"backslashesinvalue": "some\\bad\\value"}, "must match the regex"},
+		{map[string]string{"backslashesinvalue": "some\\bad\\value"}, labelErrMsg},
-		{map[string]string{"nocommasallowed": "bad,value"}, "must match the regex"},
+		{map[string]string{"nocommasallowed": "bad,value"}, labelErrMsg},
-		{map[string]string{"strangecharsinvalue": "?#$notsogood"}, "must match the regex"},
+		{map[string]string{"strangecharsinvalue": "?#$notsogood"}, labelErrMsg},
 	}
 	for i := range labelValueErrorCases {
 		errs := ValidateLabels(labelValueErrorCases[i].labels, field.NewPath("field"))

pkg/util/validation/validation.go

@@ -27,6 +27,7 @@ import (
 const qnameCharFmt string = "[A-Za-z0-9]"
 const qnameExtCharFmt string = "[-A-Za-z0-9_.]"
 const qualifiedNameFmt string = "(" + qnameCharFmt + qnameExtCharFmt + "*)?" + qnameCharFmt
+const qualifiedNameErrMsg string = "must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character"
 const qualifiedNameMaxLength int = 63
 
 var qualifiedNameRegexp = regexp.MustCompile("^" + qualifiedNameFmt + "$")
@@ -51,8 +52,8 @@ func IsQualifiedName(value string) []string {
 			errs = append(errs, prefixEach(msgs, "prefix part ")...)
 		}
 	default:
-		return append(errs, RegexError(qualifiedNameFmt, "MyName", "my.name", "123-abc")+
+		return append(errs, "a qualified name "+RegexError(qualifiedNameErrMsg, qualifiedNameFmt, "MyName", "my.name", "123-abc")+
-			" with an optional DNS subdomain prefix and '/' (e.g. 'example.com/MyName'")
+			" with an optional DNS subdomain prefix and '/' (e.g. 'example.com/MyName')")
 	}
 
 	if len(name) == 0 {
@@ -61,12 +62,13 @@ func IsQualifiedName(value string) []string {
 		errs = append(errs, "name part "+MaxLenError(qualifiedNameMaxLength))
 	}
 	if !qualifiedNameRegexp.MatchString(name) {
-		errs = append(errs, "name part "+RegexError(qualifiedNameFmt, "MyName", "my.name", "123-abc"))
+		errs = append(errs, "name part "+RegexError(qualifiedNameErrMsg, qualifiedNameFmt, "MyName", "my.name", "123-abc"))
 	}
 	return errs
 }
 
 const labelValueFmt string = "(" + qualifiedNameFmt + ")?"
+const labelValueErrMsg string = "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character"
 const LabelValueMaxLength int = 63
 
 var labelValueRegexp = regexp.MustCompile("^" + labelValueFmt + "$")
@@ -80,12 +82,13 @@ func IsValidLabelValue(value string) []string {
 		errs = append(errs, MaxLenError(LabelValueMaxLength))
 	}
 	if !labelValueRegexp.MatchString(value) {
-		errs = append(errs, RegexError(labelValueFmt, "MyValue", "my_value", "12345"))
+		errs = append(errs, RegexError(labelValueErrMsg, labelValueFmt, "MyValue", "my_value", "12345"))
 	}
 	return errs
 }
 
 const dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
+const dns1123LabelErrMsg string = "a valid DNS (RFC 1123) label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character"
 const DNS1123LabelMaxLength int = 63
 
 var dns1123LabelRegexp = regexp.MustCompile("^" + dns1123LabelFmt + "$")
@@ -98,12 +101,13 @@ func IsDNS1123Label(value string) []string {
 		errs = append(errs, MaxLenError(DNS1123LabelMaxLength))
 	}
 	if !dns1123LabelRegexp.MatchString(value) {
-		errs = append(errs, RegexError(dns1123LabelFmt, "my-name", "123-abc"))
+		errs = append(errs, RegexError(dns1123LabelErrMsg, dns1123LabelFmt, "my-name", "123-abc"))
 	}
 	return errs
 }
 
 const dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*"
+const dns1123SubdomainErrorMsg string = "a valid DNS (RFC 1123) subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
 const DNS1123SubdomainMaxLength int = 253
 
 var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$")
@@ -116,12 +120,13 @@ func IsDNS1123Subdomain(value string) []string {
 		errs = append(errs, MaxLenError(DNS1123SubdomainMaxLength))
 	}
 	if !dns1123SubdomainRegexp.MatchString(value) {
-		errs = append(errs, RegexError(dns1123SubdomainFmt, "example.com"))
+		errs = append(errs, RegexError(dns1123SubdomainErrorMsg, dns1123SubdomainFmt, "example.com"))
 	}
 	return errs
 }
 
 const dns1035LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
+const dns1035LabelErrMsg string = "a valid DNS (RFC 1035) label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character"
 const DNS1035LabelMaxLength int = 63
 
 var dns1035LabelRegexp = regexp.MustCompile("^" + dns1035LabelFmt + "$")
@@ -134,7 +139,7 @@ func IsDNS1035Label(value string) []string {
 		errs = append(errs, MaxLenError(DNS1035LabelMaxLength))
 	}
 	if !dns1035LabelRegexp.MatchString(value) {
-		errs = append(errs, RegexError(dns1035LabelFmt, "my-name", "abc-123"))
+		errs = append(errs, RegexError(dns1035LabelErrMsg, dns1035LabelFmt, "my-name", "abc-123"))
 	}
 	return errs
 }
@@ -144,6 +149,7 @@ func IsDNS1035Label(value string) []string {
 // - valid: *.bar.com, *.foo.bar.com
 // - invalid: *.*.bar.com, *.foo.*.com, *bar.com, f*.bar.com, *
 const wildcardDNS1123SubdomainFmt = "\\*\\." + dns1123SubdomainFmt
+const wildcardDNS1123SubdomainErrMsg = "a valid wildcard DNS (RFC 1123) subdomain must start with '*.', followed by a valid DNS subdomain, which must consist of lower case alphanumeric characters, '-' or '.' and end with an alphanumeric character"
 
 // IsWildcardDNS1123Subdomain tests for a string that conforms to the definition of a
 // wildcard subdomain in DNS (RFC 1034 section 4.3.3).
@@ -155,12 +161,13 @@ func IsWildcardDNS1123Subdomain(value string) []string {
 		errs = append(errs, MaxLenError(DNS1123SubdomainMaxLength))
 	}
 	if !wildcardDNS1123SubdomainRegexp.MatchString(value) {
-		errs = append(errs, RegexError(wildcardDNS1123SubdomainFmt, "*.example.com"))
+		errs = append(errs, RegexError(wildcardDNS1123SubdomainErrMsg, wildcardDNS1123SubdomainFmt, "*.example.com"))
 	}
 	return errs
 }
 
 const cIdentifierFmt string = "[A-Za-z_][A-Za-z0-9_]*"
+const identifierErrMsg string = "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_'"
 
 var cIdentifierRegexp = regexp.MustCompile("^" + cIdentifierFmt + "$")
 
@@ -168,7 +175,7 @@ var cIdentifierRegexp = regexp.MustCompile("^" + cIdentifierFmt + "$")
 // in C. This checks the format, but not the length.
 func IsCIdentifier(value string) []string {
 	if !cIdentifierRegexp.MatchString(value) {
-		return []string{RegexError(cIdentifierFmt, "my_name", "MY_NAME", "MyName")}
+		return []string{RegexError(identifierErrMsg, cIdentifierFmt, "my_name", "MY_NAME", "MyName")}
 	}
 	return nil
 }
@@ -245,17 +252,19 @@ func IsValidIP(value string) []string {
 }
 
 const percentFmt string = "[0-9]+%"
+const percentErrMsg string = "a valid percent string must be a numeric string followed by an ending '%'"
 
 var percentRegexp = regexp.MustCompile("^" + percentFmt + "$")
 
 func IsValidPercent(percent string) []string {
 	if !percentRegexp.MatchString(percent) {
-		return []string{RegexError(percentFmt, "1%", "93%")}
+		return []string{RegexError(percentErrMsg, percentFmt, "1%", "93%")}
 	}
 	return nil
 }
 
 const httpHeaderNameFmt string = "[-A-Za-z0-9]+"
+const httpHeaderNameErrMsg string = "a valid HTTP header must consist of alphanumeric characters or '-'"
 
 var httpHeaderNameRegexp = regexp.MustCompile("^" + httpHeaderNameFmt + "$")
 
@@ -263,12 +272,13 @@ var httpHeaderNameRegexp = regexp.MustCompile("^" + httpHeaderNameFmt + "$")
 // definition of a valid header field name (a stricter subset than RFC7230).
 func IsHTTPHeaderName(value string) []string {
 	if !httpHeaderNameRegexp.MatchString(value) {
-		return []string{RegexError(httpHeaderNameFmt, "X-Header-Name")}
+		return []string{RegexError(httpHeaderNameErrMsg, httpHeaderNameFmt, "X-Header-Name")}
 	}
 	return nil
 }
 
 const configMapKeyFmt = `[-._a-zA-Z0-9]+`
+const configMapKeyErrMsg string = "a valid config key must consist of alphanumeric characters, '-', '_' or '.'"
 
 var configMapKeyRegexp = regexp.MustCompile("^" + configMapKeyFmt + "$")
 
@@ -279,7 +289,7 @@ func IsConfigMapKey(value string) []string {
 		errs = append(errs, MaxLenError(DNS1123SubdomainMaxLength))
 	}
 	if !configMapKeyRegexp.MatchString(value) {
-		errs = append(errs, RegexError(configMapKeyFmt, "key.name", "KEY_NAME", "key-name"))
+		errs = append(errs, RegexError(configMapKeyErrMsg, configMapKeyFmt, "key.name", "KEY_NAME", "key-name"))
 	}
 	if value == "." {
 		errs = append(errs, `must not be '.'`)
@@ -298,19 +308,19 @@ func MaxLenError(length int) string {
 }
 
 // RegexError returns a string explanation of a regex validation failure.
-func RegexError(fmt string, examples ...string) string {
+func RegexError(msg string, fmt string, examples ...string) string {
-	s := "must match the regex " + fmt
 	if len(examples) == 0 {
-		return s
+		return msg + " (regex used for validation is '" + fmt + "')"
 	}
-	s += " (e.g. "
+	msg += " (e.g. "
 	for i := range examples {
 		if i > 0 {
-			s += " or "
+			msg += " or "
 		}
-		s += "'" + examples[i] + "'"
+		msg += "'" + examples[i] + "', "
 	}
-	return s + ")"
+	msg += "regex used for validation is '" + fmt + "')"
+	return msg
 }
 
 // EmptyError returns a string explanation of a "must not be empty" validation