Merge pull request #29523 from fraenkel/service_names_rfc1035

Automatic merge from submit-queue

Allow service names up to 63 characters (RFC 1035)

fixes #3752

hack/make-rules/test-cmd.sh

@@ -605,7 +605,7 @@ runTests() {
   kubectl label pods valid-pod record-change=true --record=true "${kube_flags[@]}"
   # Post-condition: valid-pod has record annotation
   kube::test::get_object_assert 'pod valid-pod' "{{range$annotations_field}}{{.}}:{{end}}" ".*--record=true.*"
-  
+
   ### Do not record label change
   # Command
   kubectl label pods valid-pod no-record-change=true --record=false "${kube_flags[@]}"
@@ -618,7 +618,7 @@ runTests() {
   # Post-condition: valid-pod's record annotation contains new change
   kube::test::get_object_assert 'pod valid-pod' "{{range$annotations_field}}{{.}}:{{end}}" ".*new-record-change=true.*"
 
-    
+
   ### Delete POD by label
   # Pre-condition: valid-pod POD exists
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
@@ -1765,15 +1765,15 @@ __EOF__
 
   ### Try to generate a service with invalid name (exceeding maximum valid size)
   # Pre-condition: use --name flag
-  output_message=$(! kubectl expose -f hack/testdata/pod-with-large-name.yaml --name=invalid-large-service-name --port=8081 2>&1 "${kube_flags[@]}")
+  output_message=$(! kubectl expose -f hack/testdata/pod-with-large-name.yaml --name=invalid-large-service-name-that-has-more-than-sixty-three-characters --port=8081 2>&1 "${kube_flags[@]}")
   # Post-condition: should fail due to invalid name
   kube::test::if_has_string "${output_message}" 'metadata.name: Invalid value'
   # Pre-condition: default run without --name flag; should succeed by truncating the inherited name
   output_message=$(kubectl expose -f hack/testdata/pod-with-large-name.yaml --port=8081 2>&1 "${kube_flags[@]}")
   # Post-condition: inherited name from pod has been truncated
-  kube::test::if_has_string "${output_message}" '\"kubernetes-serve-hostnam\" exposed'
+  kube::test::if_has_string "${output_message}" '\"kubernetes-serve-hostname-testing-sixty-three-characters-in-len\" exposed'
   # Clean-up
-  kubectl delete svc kubernetes-serve-hostnam "${kube_flags[@]}"
+  kubectl delete svc kubernetes-serve-hostname-testing-sixty-three-characters-in-len "${kube_flags[@]}"
 
   ### Expose multiport object as a new service
   # Pre-condition: don't use --port flag
@@ -1893,7 +1893,7 @@ __EOF__
   # Clean up
   kubectl delete deployment nginx "${kube_flags[@]}"
 
-  ### Set image of a deployment 
+  ### Set image of a deployment
   # Pre-condition: no deployment exists
   kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" ''
   # Create a deployment
@@ -1901,13 +1901,13 @@ __EOF__
   kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" 'nginx-deployment:'
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_DEPLOYMENT_R1}:"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_second_image_field}}:{{end}}" "${IMAGE_PERL}:"
-  # Set the deployment's image 
+  # Set the deployment's image
   kubectl set image deployment nginx-deployment nginx="${IMAGE_DEPLOYMENT_R2}" "${kube_flags[@]}"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_DEPLOYMENT_R2}:"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_second_image_field}}:{{end}}" "${IMAGE_PERL}:"
-  # Set non-existing container should fail 
+  # Set non-existing container should fail
   ! kubectl set image deployment nginx-deployment redis=redis "${kube_flags[@]}"
-  # Set image of deployments without specifying name 
+  # Set image of deployments without specifying name
   kubectl set image deployments --all nginx="${IMAGE_DEPLOYMENT_R1}" "${kube_flags[@]}"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_DEPLOYMENT_R1}:"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_second_image_field}}:{{end}}" "${IMAGE_PERL}:"
@@ -1915,11 +1915,11 @@ __EOF__
   kubectl set image -f hack/testdata/deployment-multicontainer.yaml nginx="${IMAGE_DEPLOYMENT_R2}" "${kube_flags[@]}"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_DEPLOYMENT_R2}:"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_second_image_field}}:{{end}}" "${IMAGE_PERL}:"
-  # Set image of a local file without talking to the server 
+  # Set image of a local file without talking to the server
   kubectl set image -f hack/testdata/deployment-multicontainer.yaml nginx="${IMAGE_DEPLOYMENT_R1}" "${kube_flags[@]}" --local -o yaml
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_DEPLOYMENT_R2}:"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_second_image_field}}:{{end}}" "${IMAGE_PERL}:"
-  # Set image of all containers of the deployment 
+  # Set image of all containers of the deployment
   kubectl set image deployment nginx-deployment "*"="${IMAGE_DEPLOYMENT_R1}" "${kube_flags[@]}"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_DEPLOYMENT_R1}:"
   kube::test::get_object_assert deployment "{{range.items}}{{$deployment_second_image_field}}:{{end}}" "${IMAGE_DEPLOYMENT_R1}:"

hack/testdata/pod-with-large-name.yaml

@@ -2,7 +2,7 @@
 apiVersion: v1
 kind: Pod
 metadata:
-  name: kubernetes-serve-hostname
+  name: kubernetes-serve-hostname-testing-sixty-three-characters-in-length
   labels:
     name: kubernetes-serve-hostname
 spec:

pkg/api/validation/validation.go

@@ -204,7 +204,7 @@ var ValidateReplicationControllerName = NameIsDNSSubdomain
 // ValidateServiceName can be used to check whether the given service name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-var ValidateServiceName = NameIsDNS952Label
+var ValidateServiceName = NameIsDNS1035Label
 
 // ValidateNodeName can be used to check whether the given node name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
@@ -258,12 +258,12 @@ func NameIsDNSLabel(name string, prefix bool) []string {
 	return validation.IsDNS1123Label(name)
 }
 
-// NameIsDNS952Label is a ValidateNameFunc for names that must be a DNS 952 label.
+// NameIsDNS1035Label is a ValidateNameFunc for names that must be a DNS 952 label.
-func NameIsDNS952Label(name string, prefix bool) []string {
+func NameIsDNS1035Label(name string, prefix bool) []string {
 	if prefix {
 		name = maskTrailingDash(name)
 	}
-	return validation.IsDNS952Label(name)
+	return validation.IsDNS1035Label(name)
 }
 
 // Validates that given value is not negative.

pkg/api/validation/validation_test.go

@@ -3449,7 +3449,7 @@ func TestValidateService(t *testing.T) {
 		{
 			name: "too long name",
 			tweakSvc: func(s *api.Service) {
-				s.Name = strings.Repeat("a", 25)
+				s.Name = strings.Repeat("a", 64)
 			},
 			numErrs: 1,
 		},
@@ -3463,7 +3463,7 @@ func TestValidateService(t *testing.T) {
 		{
 			name: "too long generateName",
 			tweakSvc: func(s *api.Service) {
-				s.GenerateName = strings.Repeat("a", 25)
+				s.GenerateName = strings.Repeat("a", 64)
 			},
 			numErrs: 1,
 		},

pkg/dns/dns.go

@@ -665,7 +665,7 @@ func getSkyMsg(ip string, port int) (*skymsg.Service, string) {
 // The conjunction of the following conditions forms the test for the federated service query
 // pattern:
 //   1. `path` has exactly 4+len(domainPath) segments: mysvc.myns.myfederation.svc.domain.path.
-//   2. Service name component must be a valid RFC 952 name.
+//   2. Service name component must be a valid RFC 1035 name.
 //   3. Namespace component must be a valid RFC 1123 name.
 //   4. Federation component must also be a valid RFC 1123 name.
 //   5. Fourth segment is exactly "svc"
@@ -679,8 +679,8 @@ func (kd *KubeDNS) isFederationQuery(path []string) bool {
 		glog.V(2).Infof("not a federation query: len(%q) != 4+len(%q)", path, kd.domainPath)
 		return false
 	}
-	if errs := validation.IsDNS952Label(path[0]); len(errs) != 0 {
+	if errs := validation.IsDNS1035Label(path[0]); len(errs) != 0 {
-		glog.V(2).Infof("not a federation query: %q is not an RFC 952 label: %q", path[0], errs)
+		glog.V(2).Infof("not a federation query: %q is not an RFC 1035 label: %q", path[0], errs)
 		return false
 	}
 	if errs := validation.IsDNS1123Label(path[1]); len(errs) != 0 {

pkg/kubectl/cmd/expose.go

@@ -52,7 +52,7 @@ var (
 		for that resource as the selector for a new service on the specified port. A deployment or replica set
 		will be exposed as a service only if its selector is convertible to a selector that service supports,
 		i.e. when the selector contains only the matchLabels component. Note that if no port is specified via
-		--port and the exposed resource has multiple ports, all will be re-used by the new service. Also if no 
+		--port and the exposed resource has multiple ports, all will be re-used by the new service. Also if no
 		labels are specified, the new service will re-use the labels from the resource it exposes.
 
 		Possible resources include (case insensitive): `) + expose_resources
@@ -171,8 +171,8 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str
 
 		params := kubectl.MakeParams(cmd, names)
 		name := info.Name
-		if len(name) > validation.DNS952LabelMaxLength {
+		if len(name) > validation.DNS1035LabelMaxLength {
-			name = name[:validation.DNS952LabelMaxLength]
+			name = name[:validation.DNS1035LabelMaxLength]
 		}
 		params["default-name"] = name
 

pkg/kubectl/cmd/expose_test.go

@@ -294,10 +294,10 @@ func TestRunExposeService(t *testing.T) {
 		},
 		{
 			name: "truncate-name",
-			args: []string{"pod", "a-name-that-is-toooo-big-for-a-service"},
+			args: []string{"pod", "a-name-that-is-toooo-big-for-a-service-because-it-can-only-handle-63-characters"},
 			ns:   "test",
 			calls: map[string]string{
-				"GET":  "/namespaces/test/pods/a-name-that-is-toooo-big-for-a-service",
+				"GET":  "/namespaces/test/pods/a-name-that-is-toooo-big-for-a-service-because-it-can-only-handle-63-characters",
 				"POST": "/namespaces/test/services",
 			},
 			input: &api.Pod{
@@ -305,7 +305,7 @@ func TestRunExposeService(t *testing.T) {
 			},
 			flags: map[string]string{"selector": "svc=frompod", "port": "90", "labels": "svc=frompod", "generator": "service/v2"},
 			output: &api.Service{
-				ObjectMeta: api.ObjectMeta{Name: "a-name-that-is-toooo-big", Namespace: "", Labels: map[string]string{"svc": "frompod"}},
+				ObjectMeta: api.ObjectMeta{Name: "a-name-that-is-toooo-big-for-a-service-because-it-can-only-handle-63-characters", Namespace: "", Labels: map[string]string{"svc": "frompod"}},
 				Spec: api.ServiceSpec{
 					Ports: []api.ServicePort{
 						{
@@ -317,7 +317,7 @@ func TestRunExposeService(t *testing.T) {
 					Selector: map[string]string{"svc": "frompod"},
 				},
 			},
-			expected: "service \"a-name-that-is-toooo-big\" exposed",
+			expected: "service \"a-name-that-is-toooo-big-for-a-service-because-it-can-only-hand\" exposed",
 			status:   200,
 		},
 		{

pkg/util/validation/validation.go

@@ -121,20 +121,20 @@ func IsDNS1123Subdomain(value string) []string {
 	return errs
 }
 
-const dns952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
+const dns1035LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
-const DNS952LabelMaxLength int = 24
+const DNS1035LabelMaxLength int = 63
 
-var dns952LabelRegexp = regexp.MustCompile("^" + dns952LabelFmt + "$")
+var dns1035LabelRegexp = regexp.MustCompile("^" + dns1035LabelFmt + "$")
 
-// IsDNS952Label tests for a string that conforms to the definition of a label in
+// IsDNS1035Label tests for a string that conforms to the definition of a label in
-// DNS (RFC 952).
+// DNS (RFC 1035).
-func IsDNS952Label(value string) []string {
+func IsDNS1035Label(value string) []string {
 	var errs []string
-	if len(value) > DNS952LabelMaxLength {
+	if len(value) > DNS1035LabelMaxLength {
-		errs = append(errs, MaxLenError(DNS952LabelMaxLength))
+		errs = append(errs, MaxLenError(DNS1035LabelMaxLength))
 	}
-	if !dns952LabelRegexp.MatchString(value) {
+	if !dns1035LabelRegexp.MatchString(value) {
-		errs = append(errs, RegexError(dns952LabelFmt, "my-name", "abc-123"))
+		errs = append(errs, RegexError(dns1035LabelFmt, "my-name", "abc-123"))
 	}
 	return errs
 }

pkg/util/validation/validation_test.go

@@ -86,13 +86,13 @@ func TestIsDNS1123Subdomain(t *testing.T) {
 	}
 }
 
-func TestIsDNS952Label(t *testing.T) {
+func TestIsDNS1035Label(t *testing.T) {
 	goodValues := []string{
 		"a", "ab", "abc", "a1", "a-1", "a--1--2--b",
-		strings.Repeat("a", 24),
+		strings.Repeat("a", 63),
 	}
 	for _, val := range goodValues {
-		if msgs := IsDNS952Label(val); len(msgs) != 0 {
+		if msgs := IsDNS1035Label(val); len(msgs) != 0 {
 			t.Errorf("expected true for '%s': %v", val, msgs)
 		}
 	}
@@ -104,10 +104,10 @@ func TestIsDNS952Label(t *testing.T) {
 		"_", "a_", "_a", "a_b", "1_", "_1", "1_2",
 		".", "a.", ".a", "a.b", "1.", ".1", "1.2",
 		" ", "a ", " a", "a b", "1 ", " 1", "1 2",
-		strings.Repeat("a", 25),
+		strings.Repeat("a", 64),
 	}
 	for _, val := range badValues {
-		if msgs := IsDNS952Label(val); len(msgs) == 0 {
+		if msgs := IsDNS1035Label(val); len(msgs) == 0 {
 			t.Errorf("expected false for '%s'", val)
 		}
 	}