From b1e7e6cf463bfdb7d5dcf0724dcae614761fee82 Mon Sep 17 00:00:00 2001 From: Michael Fraenkel Date: Sun, 24 Jul 2016 13:01:38 -0400 Subject: [PATCH] Service names conform to RFC 1035 --- hack/make-rules/test-cmd.sh | 22 +++++++++++----------- hack/testdata/pod-with-large-name.yaml | 2 +- pkg/api/validation/validation.go | 8 ++++---- pkg/api/validation/validation_test.go | 4 ++-- pkg/dns/dns.go | 6 +++--- pkg/kubectl/cmd/expose.go | 6 +++--- pkg/kubectl/cmd/expose_test.go | 8 ++++---- pkg/util/validation/validation.go | 20 ++++++++++---------- pkg/util/validation/validation_test.go | 10 +++++----- 9 files changed, 43 insertions(+), 43 deletions(-) diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index 6721bd30411..d6f65dd9120 100755 --- a/hack/make-rules/test-cmd.sh +++ b/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}:" diff --git a/hack/testdata/pod-with-large-name.yaml b/hack/testdata/pod-with-large-name.yaml index 5fb3c343bfa..8e8f0e07cac 100644 --- a/hack/testdata/pod-with-large-name.yaml +++ b/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: diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index e08bb1e5d60..de2c16c257c 100644 --- a/pkg/api/validation/validation.go +++ b/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. -func NameIsDNS952Label(name string, prefix bool) []string { +// NameIsDNS1035Label is a ValidateNameFunc for names that must be a DNS 952 label. +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. diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index d3fe378ec84..382f02f15cf 100644 --- a/pkg/api/validation/validation_test.go +++ b/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, }, diff --git a/pkg/dns/dns.go b/pkg/dns/dns.go index f9aebd2e65e..4763d8531c2 100644 --- a/pkg/dns/dns.go +++ b/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 { - glog.V(2).Infof("not a federation query: %q is not an RFC 952 label: %q", path[0], errs) + if errs := validation.IsDNS1035Label(path[0]); len(errs) != 0 { + 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 { diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index c4fca0bd7d9..bf456bb9bcf 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/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 @@ -170,8 +170,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 { - name = name[:validation.DNS952LabelMaxLength] + if len(name) > validation.DNS1035LabelMaxLength { + name = name[:validation.DNS1035LabelMaxLength] } params["default-name"] = name diff --git a/pkg/kubectl/cmd/expose_test.go b/pkg/kubectl/cmd/expose_test.go index 2338e0c344a..5febb530d23 100644 --- a/pkg/kubectl/cmd/expose_test.go +++ b/pkg/kubectl/cmd/expose_test.go @@ -230,10 +230,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{ @@ -241,7 +241,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{ { @@ -253,7 +253,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, }, { diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go index 6e6a0270ca4..74096344122 100644 --- a/pkg/util/validation/validation.go +++ b/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 DNS952LabelMaxLength int = 24 +const dns1035LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" +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 -// DNS (RFC 952). -func IsDNS952Label(value string) []string { +// IsDNS1035Label tests for a string that conforms to the definition of a label in +// DNS (RFC 1035). +func IsDNS1035Label(value string) []string { var errs []string - if len(value) > DNS952LabelMaxLength { - errs = append(errs, MaxLenError(DNS952LabelMaxLength)) + if len(value) > DNS1035LabelMaxLength { + errs = append(errs, MaxLenError(DNS1035LabelMaxLength)) } - if !dns952LabelRegexp.MatchString(value) { - errs = append(errs, RegexError(dns952LabelFmt, "my-name", "abc-123")) + if !dns1035LabelRegexp.MatchString(value) { + errs = append(errs, RegexError(dns1035LabelFmt, "my-name", "abc-123")) } return errs } diff --git a/pkg/util/validation/validation_test.go b/pkg/util/validation/validation_test.go index b799469450f..0111817f247 100644 --- a/pkg/util/validation/validation_test.go +++ b/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) } }