Spec qualified names (label keys) more tightly

It can be a (DNS_SUBDOMAIN/)?label, but we were validating it
incorrectly before.

docs/labels.md

@@ -32,10 +32,10 @@ These are just examples; you are free to develop your own conventions.
 
 ## Syntax and character set
 
-As already mentioned, well formed _labels_ are key value pairs. Valid label keys have two segments - prefix and name - separated by a slash (`/`).  The name segment is required and must be a DNS label: 63 characters or less, all lowercase, beginning and ending with an alphanumeric character (`[a-z0-9A-Z]`), with dashes (`-`) and alphanumerics between.  The prefix and slash are optional.  If specified, the prefix must be a DNS subdomain (a series of DNS labels separated by dots (`.`), not longer than 253 characters in total.
-If the prefix is omitted, the label key is presumed to be private to the user. System components which use labels must specify a prefix.  The `kubernetes.io` prefix is reserved for kubernetes core components.
+_Labels_ are key value pairs. Valid label keys have two segments: an optional prefix and name, separated by a slash (`/`).  The name segment is required and must be 63 characters or less, beginning and ending with an alphanumeric character (`[a-z0-9A-Z]`) with dashes (`-`), underscores (`_`), dots (`.`), and alphanumerics between.  The prefix is optional.  If specified, the prefix must be a DNS subdomain: a series of DNS labels separated by dots (`.`), not longer than 253 characters in total, followed by a slash (`/`).
+If the prefix is omitted, the label key is presumed to be private to the user. System components which use labels must specify a prefix.  The `kubernetes.io/` prefix is reserved for kubernetes core components.
 
-Valid label values must be shorter than 64 characters, accepted characters are (`[-A-Za-z0-9_.]`) but the first character must be  (`[A-Za-z0-9]`).
+Valid label values must be 63 characters or less and must be empty or begin and end with an alphanumeric character (`[a-z0-9A-Z]`) with dashes (`-`), underscores (`_`), dots (`.`), and alphanumerics between.
 
 ## Label selectors
 

pkg/api/validation/validation.go

@@ -40,7 +40,7 @@ func intervalErrorMsg(lo, hi int) string {
 }
 
 var labelValueErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.LabelValueMaxLength, util.LabelValueFmt)
-var qualifiedNameErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.QualifiedNameMaxLength, util.QualifiedNameFmt)
+var qualifiedNameErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s, with an optional DNS prefix", util.QualifiedNameMaxLength, util.QualifiedNameFmt)
 var dnsSubdomainErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123SubdomainMaxLength, util.DNS1123SubdomainFmt)
 var dns1123LabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123LabelMaxLength, util.DNS1123LabelFmt)
 var dns952LabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS952LabelMaxLength, util.DNS952LabelFmt)

pkg/util/validation.go

@@ -19,13 +19,35 @@ package util
 import (
 	"net"
 	"regexp"
+	"strings"
 )
 
 const qnameCharFmt string = "[A-Za-z0-9]"
 const qnameExtCharFmt string = "[-A-Za-z0-9_.]"
-const qnameTokenFmt string = "(" + qnameCharFmt + qnameExtCharFmt + "*)?" + qnameCharFmt
+const QualifiedNameFmt string = "(" + qnameCharFmt + qnameExtCharFmt + "*)?" + qnameCharFmt
+const QualifiedNameMaxLength int = 63
 
-const LabelValueFmt string = "(" + qnameTokenFmt + ")?"
+var qualifiedNameRegexp = regexp.MustCompile("^" + QualifiedNameFmt + "$")
+
+func IsQualifiedName(value string) bool {
+	parts := strings.Split(value, "/")
+	var left, right string
+	switch len(parts) {
+	case 1:
+		left, right = "", parts[0]
+	case 2:
+		left, right = parts[0], parts[1]
+	default:
+		return false
+	}
+
+	if left != "" && !IsDNS1123Subdomain(left) {
+		return false
+	}
+	return right != "" && len(right) <= QualifiedNameMaxLength && qualifiedNameRegexp.MatchString(right)
+}
+
+const LabelValueFmt string = "(" + QualifiedNameFmt + ")?"
 const LabelValueMaxLength int = 63
 
 var labelValueRegexp = regexp.MustCompile("^" + LabelValueFmt + "$")
@@ -34,15 +56,6 @@ func IsValidLabelValue(value string) bool {
 	return (len(value) <= LabelValueMaxLength && labelValueRegexp.MatchString(value))
 }
 
-const QualifiedNameFmt string = "(" + qnameTokenFmt + "/)?" + qnameTokenFmt
-const QualifiedNameMaxLength int = 253
-
-var qualifiedNameRegexp = regexp.MustCompile("^" + QualifiedNameFmt + "$")
-
-func IsQualifiedName(value string) bool {
-	return (len(value) <= QualifiedNameMaxLength && qualifiedNameRegexp.MatchString(value))
-}
-
 const DNS1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
 const DNS1123LabelMaxLength int = 63
 

pkg/util/validation_test.go

@@ -168,24 +168,30 @@ func TestIsQualifiedName(t *testing.T) {
 		"1-num.2-num/3-num",
 		"1234/5678",
 		"1.2.3.4/5678",
-		"UppercaseIsOK123",
+		"Uppercase_Is_OK_123",
+		"example.com/Uppercase_Is_OK_123",
+		strings.Repeat("a", 63),
+		strings.Repeat("a", 253) + "/" + strings.Repeat("b", 63),
 	}
 	for i := range successCases {
 		if !IsQualifiedName(successCases[i]) {
-			t.Errorf("case[%d] expected success", i)
+			t.Errorf("case[%d]: %q: expected success", i, successCases[i])
 		}
 	}
 
 	errorCases := []string{
 		"nospecialchars%^=@",
 		"cantendwithadash-",
+		"-cantstartwithadash-",
 		"only/one/slash",
-		strings.Repeat("a", 254),
-		"-cantstartwithadash",
+		"Example.com/abc",
+		"example_com/abc",
+		strings.Repeat("a", 64),
+		strings.Repeat("a", 254) + "/abc",
 	}
 	for i := range errorCases {
 		if IsQualifiedName(errorCases[i]) {
-			t.Errorf("case[%d] expected failure", i)
+			t.Errorf("case[%d]: %q: expected failure", i, errorCases[i])
 		}
 	}
 }