From 4b13faa34643a95c24e833e6bc14c2f09a00d392 Mon Sep 17 00:00:00 2001 From: Salvatore Dario Minonne Date: Fri, 12 Jun 2015 18:33:11 +0200 Subject: [PATCH] To add validation for service ports when defined as string (fixing issue #9734) --- api/swagger-spec/v1.json | 8 +- api/swagger-spec/v1beta3.json | 8 +- contrib/mesos/pkg/executor/executor_test.go | 2 +- pkg/api/rest/update_test.go | 3 +- pkg/api/types.go | 8 +- pkg/api/v1/types.go | 16 ++- pkg/api/v1beta3/types.go | 16 ++- pkg/api/validation/validation.go | 22 ++-- pkg/api/validation/validation_test.go | 45 ++++---- pkg/master/controller.go | 2 +- pkg/registry/service/rest_test.go | 111 +++++++++++++------- pkg/util/validation.go | 30 ++++++ pkg/util/validation_test.go | 16 +++ 13 files changed, 193 insertions(+), 94 deletions(-) diff --git a/api/swagger-spec/v1.json b/api/swagger-spec/v1.json index 35391f1bf7b..09afc083601 100644 --- a/api/swagger-spec/v1.json +++ b/api/swagger-spec/v1.json @@ -12387,7 +12387,7 @@ "properties": { "name": { "type": "string", - "description": "name for the port that can be referred to by services; must be a DNS_LABEL and unique without the pod" + "description": "name for the port that can be referred to by services; must be a IANA_SVC_NAME and unique within the pod" }, "hostPort": { "type": "integer", @@ -12529,7 +12529,7 @@ }, "port": { "type": "string", - "description": "number or name of the port to access on the container" + "description": "number or name of the port to access on the container; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME" }, "host": { "type": "string", @@ -12545,7 +12545,7 @@ "properties": { "port": { "type": "string", - "description": "number of name of the port to access on the container" + "description": "number of name of the port to access on the container; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME" } } }, @@ -13264,7 +13264,7 @@ }, "targetPort": { "type": "string", - "description": "the port to access on the pods targeted by the service; defaults to the service port" + "description": "number or name of the port to access on the pods targeted by the service; defaults to the service port; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME" }, "nodePort": { "type": "integer", diff --git a/api/swagger-spec/v1beta3.json b/api/swagger-spec/v1beta3.json index 42c02827882..6e8826ffa28 100644 --- a/api/swagger-spec/v1beta3.json +++ b/api/swagger-spec/v1beta3.json @@ -12389,7 +12389,7 @@ "properties": { "name": { "type": "string", - "description": "name for the port that can be referred to by services; must be a DNS_LABEL and unique without the pod" + "description": "name for the port that can be referred to by services; must be a IANA_SVC_NAME and unique within the pod" }, "hostPort": { "type": "integer", @@ -12531,7 +12531,7 @@ }, "port": { "type": "string", - "description": "number or name of the port to access on the container" + "description": "number or name of the port to access on the container; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME" }, "host": { "type": "string", @@ -12547,7 +12547,7 @@ "properties": { "port": { "type": "string", - "description": "number of name of the port to access on the container" + "description": "number or name of the port to access on the container; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME" } } }, @@ -13270,7 +13270,7 @@ }, "targetPort": { "type": "string", - "description": "the port to access on the pods targeted by the service; defaults to the service port" + "description": "number or name of the port to access on the pods targeted by the service; defaults to the service port; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME" }, "nodePort": { "type": "integer", diff --git a/contrib/mesos/pkg/executor/executor_test.go b/contrib/mesos/pkg/executor/executor_test.go index 2678665616c..27cc816728a 100644 --- a/contrib/mesos/pkg/executor/executor_test.go +++ b/contrib/mesos/pkg/executor/executor_test.go @@ -451,7 +451,7 @@ func TestExecutorStaticPods(t *testing.T) { "enabled": true, "type": "http", "initialDelaySeconds": 30, - "httpGet": { "path": "/", "port": "80" } + "httpGet": { "path": "/", "port": 80 } } }] } diff --git a/pkg/api/rest/update_test.go b/pkg/api/rest/update_test.go index 6c85eb87f80..a401efccdcb 100644 --- a/pkg/api/rest/update_test.go +++ b/pkg/api/rest/update_test.go @@ -21,6 +21,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) func makeValidService() api.Service { @@ -36,7 +37,7 @@ func makeValidService() api.Service { Selector: map[string]string{"key": "val"}, SessionAffinity: "None", Type: api.ServiceTypeClusterIP, - Ports: []api.ServicePort{{Name: "p", Protocol: "TCP", Port: 8675}}, + Ports: []api.ServicePort{{Name: "p", Protocol: "TCP", Port: 8675, TargetPort: util.NewIntOrStringFromInt(8675)}}, }, } } diff --git a/pkg/api/types.go b/pkg/api/types.go index 81e4c897217..18c0f621c65 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -47,6 +47,12 @@ import ( // [a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)* // or more simply: // DNS_LABEL(\.DNS_LABEL)* +// +// IANA_SVC_NAME: This is a string, no more than 15 characters long, that +// conforms to the definition of IANA service name in RFC 6335. +// 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. // TypeMeta describes an individual object in an API response or request // with strings representing the type of the object and its API schema version. @@ -545,7 +551,7 @@ type RBDVolumeSource struct { // ContainerPort represents a network port in a single container type ContainerPort struct { - // Optional: If specified, this must be a DNS_LABEL. Each named port + // Optional: If specified, this must be a IANA_SVC_NAME Each named port // in a pod must have a unique name. Name string `json:"name,omitempty"` // Optional: If specified, this must be a valid port number, 0 < x < 65536. diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index e39456d7054..d04df6e9a0a 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -45,6 +45,12 @@ import ( // [a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)* // or more simply: // DNS_LABEL(\.DNS_LABEL)* +// +// IANA_SVC_NAME: This is a string, no more than 15 characters long, that +// conforms to the definition of IANA service name in RFC 6335. +// 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. // TypeMeta describes an individual object in an API response or request // with strings representing the type of the object and its API schema version. @@ -555,9 +561,9 @@ type ISCSIVolumeSource struct { // ContainerPort represents a network port in a single container. type ContainerPort struct { - // Optional: If specified, this must be a DNS_LABEL. Each named port + // Optional: If specified, this must be a IANA_SVC_NAME Each named port // in a pod must have a unique name. - Name string `json:"name,omitempty" description:"name for the port that can be referred to by services; must be a DNS_LABEL and unique without the pod"` + Name string `json:"name,omitempty" description:"name for the port that can be referred to by services; must be a IANA_SVC_NAME and unique within the pod"` // Optional: If specified, this must be a valid port number, 0 < x < 65536. // If HostNetwork is specified, this must match ContainerPort. HostPort int `json:"hostPort,omitempty" description:"number of port to expose on the host; most containers do not need this"` @@ -615,7 +621,7 @@ type HTTPGetAction struct { // Optional: Path to access on the HTTP server. Path string `json:"path,omitempty" description:"path to access on the HTTP server"` // Required: Name or number of the port to access on the container. - Port util.IntOrString `json:"port" description:"number or name of the port to access on the container"` + Port util.IntOrString `json:"port" description:"number or name of the port to access on the container; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME"` // Optional: Host name to connect to, defaults to the pod IP. Host string `json:"host,omitempty" description:"hostname to connect to; defaults to pod IP"` } @@ -623,7 +629,7 @@ type HTTPGetAction struct { // TCPSocketAction describes an action based on opening a socket type TCPSocketAction struct { // Required: Port to connect to. - Port util.IntOrString `json:"port" description:"number of name of the port to access on the container"` + Port util.IntOrString `json:"port" description:"number of name of the port to access on the container; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME"` } // ExecAction describes a "run in container" action. @@ -1132,7 +1138,7 @@ type ServicePort struct { // If this is a string, it will be looked up as a named port in the // target Pod's container ports. If this is not specified, the value // of Port is used (an identity map). - TargetPort util.IntOrString `json:"targetPort,omitempty" description:"the port to access on the pods targeted by the service; defaults to the service port"` + TargetPort util.IntOrString `json:"targetPort,omitempty" description:"number or name of the port to access on the pods targeted by the service; defaults to the service port; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME"` // The port on each node on which this service is exposed. // Default is to auto-allocate a port if the ServiceType of this Service requires one. diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 63323dad7df..68fdd99272c 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -45,6 +45,12 @@ import ( // [a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)* // or more simply: // DNS_LABEL(\.DNS_LABEL)* +// +// IANA_SVC_NAME: This is a string, no more than 15 characters long, that +// conforms to the definition of IANA service name in RFC 6335. +// 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. // TypeMeta describes an individual object in an API response or request // with strings representing the type of the object and its API schema version. @@ -555,9 +561,9 @@ type ISCSIVolumeSource struct { // ContainerPort represents a network port in a single container. type ContainerPort struct { - // Optional: If specified, this must be a DNS_LABEL. Each named port + // Optional: If specified, this must be a IANA_SVC_NAME. Each named port // in a pod must have a unique name. - Name string `json:"name,omitempty" description:"name for the port that can be referred to by services; must be a DNS_LABEL and unique without the pod"` + Name string `json:"name,omitempty" description:"name for the port that can be referred to by services; must be a IANA_SVC_NAME and unique within the pod"` // Optional: If specified, this must be a valid port number, 0 < x < 65536. // If HostNetwork is specified, this must match ContainerPort. HostPort int `json:"hostPort,omitempty" description:"number of port to expose on the host; most containers do not need this"` @@ -615,7 +621,7 @@ type HTTPGetAction struct { // Optional: Path to access on the HTTP server. Path string `json:"path,omitempty" description:"path to access on the HTTP server"` // Required: Name or number of the port to access on the container. - Port util.IntOrString `json:"port" description:"number or name of the port to access on the container"` + Port util.IntOrString `json:"port" description:"number or name of the port to access on the container; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME"` // Optional: Host name to connect to, defaults to the pod IP. Host string `json:"host,omitempty" description:"hostname to connect to; defaults to pod IP"` } @@ -623,7 +629,7 @@ type HTTPGetAction struct { // TCPSocketAction describes an action based on opening a socket type TCPSocketAction struct { // Required: Port to connect to. - Port util.IntOrString `json:"port" description:"number of name of the port to access on the container"` + Port util.IntOrString `json:"port" description:"number or name of the port to access on the container; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME"` } // ExecAction describes a "run in container" action. @@ -1138,7 +1144,7 @@ type ServicePort struct { // If this is a string, it will be looked up as a named port in the // target Pod's container ports. If this is not specified, the value // of Port is used (an identity map). - TargetPort util.IntOrString `json:"targetPort,omitempty" description:"the port to access on the pods targeted by the service; defaults to the service port"` + TargetPort util.IntOrString `json:"targetPort,omitempty" description:"number or name of the port to access on the pods targeted by the service; defaults to the service port; number must be in the range 1 to 65535; name must be a IANA_SVC_NAME"` // The port on each node on which this service is exposed. // Default is to auto-allocate a port if the ServiceType of this Service requires one. diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 6278597f4ba..b018e29183f 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -49,6 +49,7 @@ var dns1123LabelErrorMsg string = fmt.Sprintf(`must be a DNS label (at most %d c var dns952LabelErrorMsg string = fmt.Sprintf(`must be a DNS 952 label (at most %d characters, matching regex %s): e.g. "my-name"`, util.DNS952LabelMaxLength, util.DNS952LabelFmt) var pdPartitionErrorMsg string = intervalErrorMsg(0, 255) var portRangeErrorMsg string = intervalErrorMsg(0, 65536) +var portNameErrorMsg string = fmt.Sprintf(`must be a IANA_SVC_NAME (at most 15 characters, matching regex %s and it must containts at least one letter [a-z], hypens cannot be adjacent to other hyphens): e.g. "http"`, util.IdentifierNoHyphensBeginEndFmt) const totalAnnotationSizeLimitB int = 64 * (1 << 10) // 64 kB @@ -594,8 +595,8 @@ func validatePorts(ports []api.ContainerPort) errs.ValidationErrorList { for i, port := range ports { pErrs := errs.ValidationErrorList{} if len(port.Name) > 0 { - if len(port.Name) > util.DNS1123LabelMaxLength || !util.IsDNS1123Label(port.Name) { - pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, dns1123LabelErrorMsg)) + if !util.IsValidPortName(port.Name) { + pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, portNameErrorMsg)) } else if allNames.Has(port.Name) { pErrs = append(pErrs, errs.NewFieldDuplicate("name", port.Name)) } else { @@ -759,8 +760,8 @@ func validateHTTPGetAction(http *api.HTTPGetAction) errs.ValidationErrorList { } if http.Port.Kind == util.IntstrInt && !util.IsValidPortNum(http.Port.IntVal) { allErrors = append(allErrors, errs.NewFieldInvalid("port", http.Port, portRangeErrorMsg)) - } else if http.Port.Kind == util.IntstrString && len(http.Port.StrVal) == 0 { - allErrors = append(allErrors, errs.NewFieldRequired("port")) + } else if http.Port.Kind == util.IntstrString && !util.IsValidPortName(http.Port.StrVal) { + allErrors = append(allErrors, errs.NewFieldInvalid("port", http.Port.StrVal, portNameErrorMsg)) } return allErrors } @@ -769,8 +770,8 @@ func validateTCPSocketAction(tcp *api.TCPSocketAction) errs.ValidationErrorList allErrors := errs.ValidationErrorList{} if tcp.Port.Kind == util.IntstrInt && !util.IsValidPortNum(tcp.Port.IntVal) { allErrors = append(allErrors, errs.NewFieldInvalid("port", tcp.Port, portRangeErrorMsg)) - } else if tcp.Port.Kind == util.IntstrString && len(tcp.Port.StrVal) == 0 { - allErrors = append(allErrors, errs.NewFieldRequired("port")) + } else if tcp.Port.Kind == util.IntstrString && !util.IsValidPortName(tcp.Port.StrVal) { + allErrors = append(allErrors, errs.NewFieldInvalid("port", tcp.Port.StrVal, portNameErrorMsg)) } return allErrors } @@ -1150,10 +1151,11 @@ func validateServicePort(sp *api.ServicePort, requireName bool, allNames *util.S allErrs = append(allErrs, errs.NewFieldNotSupported("protocol", sp.Protocol)) } - if sp.TargetPort != util.NewIntOrStringFromInt(0) && sp.TargetPort != util.NewIntOrStringFromString("") { - if sp.TargetPort.Kind == util.IntstrInt && !util.IsValidPortNum(sp.TargetPort.IntVal) { - allErrs = append(allErrs, errs.NewFieldInvalid("targetPort", sp.TargetPort, portRangeErrorMsg)) - } + if sp.TargetPort.Kind == util.IntstrInt && !util.IsValidPortNum(sp.TargetPort.IntVal) { + allErrs = append(allErrs, errs.NewFieldInvalid("targetPort", sp.TargetPort, portRangeErrorMsg)) + } + if sp.TargetPort.Kind == util.IntstrString && !util.IsValidPortName(sp.TargetPort.StrVal) { + allErrs = append(allErrs, errs.NewFieldInvalid("targetPort", sp.TargetPort, portNameErrorMsg)) } return allErrs diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 9ba9a0f14f1..5256daaa7ea 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -512,8 +512,9 @@ func TestValidatePorts(t *testing.T) { F string D string }{ - "name > 63 characters": {[]api.ContainerPort{{Name: strings.Repeat("a", 64), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dns1123LabelErrorMsg}, - "name not a DNS label": {[]api.ContainerPort{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dns1123LabelErrorMsg}, + "name > 15 characters": {[]api.ContainerPort{{Name: strings.Repeat("a", 16), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", portNameErrorMsg}, + "name not a IANA svc name ": {[]api.ContainerPort{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", portNameErrorMsg}, + "name not a IANA svc name (i.e. a number)": {[]api.ContainerPort{{Name: "80", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", portNameErrorMsg}, "name not unique": {[]api.ContainerPort{ {Name: "abc", ContainerPort: 80, Protocol: "TCP"}, {Name: "abc", ContainerPort: 81, Protocol: "TCP"}, @@ -1435,7 +1436,7 @@ func makeValidService() api.Service { Selector: map[string]string{"key": "val"}, SessionAffinity: "None", Type: api.ServiceTypeClusterIP, - Ports: []api.ServicePort{{Name: "p", Protocol: "TCP", Port: 8675}}, + Ports: []api.ServicePort{{Name: "p", Protocol: "TCP", Port: 8675, TargetPort: util.NewIntOrStringFromInt(8675)}}, }, } } @@ -1554,7 +1555,7 @@ func TestValidateService(t *testing.T) { { name: "empty port[1] name", tweakSvc: func(s *api.Service) { - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "", Protocol: "TCP", Port: 12345}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "", Protocol: "TCP", Port: 12345, TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 1, }, @@ -1562,7 +1563,7 @@ func TestValidateService(t *testing.T) { name: "empty multi-port port[0] name", tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Name = "" - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Protocol: "TCP", Port: 12345}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Protocol: "TCP", Port: 12345, TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 1, }, @@ -1640,7 +1641,7 @@ func TestValidateService(t *testing.T) { name: "dup port name", tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Name = "p" - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Port: 12345, Protocol: "TCP"}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Port: 12345, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 1, }, @@ -1656,7 +1657,7 @@ func TestValidateService(t *testing.T) { name: "invalid load balancer protocol 2", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeLoadBalancer - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "UDP"}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "UDP", TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 1, }, @@ -1715,7 +1716,7 @@ func TestValidateService(t *testing.T) { name: "valid type loadbalancer 2 ports", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeLoadBalancer - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP"}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 0, }, @@ -1723,7 +1724,7 @@ func TestValidateService(t *testing.T) { name: "valid external load balancer 2 ports", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeLoadBalancer - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP"}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 0, }, @@ -1731,8 +1732,8 @@ func TestValidateService(t *testing.T) { name: "duplicate nodeports", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeNodePort - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1}) - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "r", Port: 2, Protocol: "TCP", NodePort: 1}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: util.NewIntOrStringFromInt(1)}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "r", Port: 2, Protocol: "TCP", NodePort: 1, TargetPort: util.NewIntOrStringFromInt(2)}) }, numErrs: 1, }, @@ -1740,8 +1741,8 @@ func TestValidateService(t *testing.T) { name: "duplicate nodeports (different protocols)", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeNodePort - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1}) - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "r", Port: 2, Protocol: "UDP", NodePort: 1}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: util.NewIntOrStringFromInt(1)}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "r", Port: 2, Protocol: "UDP", NodePort: 1, TargetPort: util.NewIntOrStringFromInt(2)}) }, numErrs: 0, }, @@ -1770,7 +1771,7 @@ func TestValidateService(t *testing.T) { name: "valid type loadbalancer 2 ports", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeLoadBalancer - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP"}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 0, }, @@ -1778,7 +1779,7 @@ func TestValidateService(t *testing.T) { name: "valid type loadbalancer with NodePort", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeLoadBalancer - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", NodePort: 12345}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", NodePort: 12345, TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 0, }, @@ -1786,7 +1787,7 @@ func TestValidateService(t *testing.T) { name: "valid type=NodePort service with NodePort", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeNodePort - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", NodePort: 12345}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", NodePort: 12345, TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 0, }, @@ -1794,7 +1795,7 @@ func TestValidateService(t *testing.T) { name: "valid type=NodePort service without NodePort", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeNodePort - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP"}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 0, }, @@ -1802,7 +1803,7 @@ func TestValidateService(t *testing.T) { name: "valid cluster service without NodePort", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeClusterIP - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP"}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 0, }, @@ -1810,7 +1811,7 @@ func TestValidateService(t *testing.T) { name: "invalid cluster service with NodePort", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeClusterIP - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", NodePort: 12345}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", NodePort: 12345, TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 1, }, @@ -1818,8 +1819,8 @@ func TestValidateService(t *testing.T) { name: "invalid public service with duplicate NodePort", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeNodePort - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p1", Port: 1, Protocol: "TCP", NodePort: 1}) - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p2", Port: 2, Protocol: "TCP", NodePort: 1}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p1", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: util.NewIntOrStringFromInt(1)}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p2", Port: 2, Protocol: "TCP", NodePort: 1, TargetPort: util.NewIntOrStringFromInt(2)}) }, numErrs: 1, }, @@ -1827,7 +1828,7 @@ func TestValidateService(t *testing.T) { name: "valid type=LoadBalancer", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeLoadBalancer - s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP"}) + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(12345)}) }, numErrs: 0, }, diff --git a/pkg/master/controller.go b/pkg/master/controller.go index 60d60721c9f..a5f02a54215 100644 --- a/pkg/master/controller.go +++ b/pkg/master/controller.go @@ -155,7 +155,7 @@ func (c *Controller) CreateMasterServiceIfNeeded(serviceName string, serviceIP n Labels: map[string]string{"provider": "kubernetes", "component": "apiserver"}, }, Spec: api.ServiceSpec{ - Ports: []api.ServicePort{{Port: servicePort, Protocol: api.ProtocolTCP}}, + Ports: []api.ServicePort{{Port: servicePort, Protocol: api.ProtocolTCP, TargetPort: util.NewIntOrStringFromInt(servicePort)}}, // maintained by this code, not by the pod selector Selector: nil, ClusterIP: serviceIP.String(), diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index 5cf064be0b8..ec349e1b895 100644 --- a/pkg/registry/service/rest_test.go +++ b/pkg/registry/service/rest_test.go @@ -76,8 +76,9 @@ func TestServiceRegistryCreate(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -118,8 +119,9 @@ func TestServiceStorageValidatesCreate(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, }, @@ -134,6 +136,18 @@ func TestServiceStorageValidatesCreate(t *testing.T) { }}, }, }, + "missing targetPort": { + ObjectMeta: api.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeClusterIP, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, + }, + }, } ctx := api.NewDefaultContext() for _, failureCase := range failureCases { @@ -155,8 +169,9 @@ func TestServiceRegistryUpdate(t *testing.T) { Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz1"}, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, }) @@ -173,8 +188,9 @@ func TestServiceRegistryUpdate(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, }) @@ -217,8 +233,9 @@ func TestServiceStorageValidatesUpdate(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, }, @@ -229,8 +246,9 @@ func TestServiceStorageValidatesUpdate(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, }, @@ -256,8 +274,9 @@ func TestServiceRegistryExternalService(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeLoadBalancer, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -330,8 +349,9 @@ func TestServiceRegistryUpdateExternalService(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -366,13 +386,15 @@ func TestServiceRegistryUpdateMultiPortExternalService(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeLoadBalancer, Ports: []api.ServicePort{{ - Name: "p", - Port: 6502, - Protocol: api.ProtocolTCP, + Name: "p", + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }, { - Name: "q", - Port: 8086, - Protocol: api.ProtocolTCP, + Name: "q", + Port: 8086, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(8086), }}, }, } @@ -506,8 +528,9 @@ func TestServiceRegistryIPAllocation(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -528,8 +551,9 @@ func TestServiceRegistryIPAllocation(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }} ctx = api.NewDefaultContext() @@ -558,8 +582,9 @@ func TestServiceRegistryIPAllocation(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -584,8 +609,9 @@ func TestServiceRegistryIPReallocation(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -611,8 +637,9 @@ func TestServiceRegistryIPReallocation(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -637,8 +664,9 @@ func TestServiceRegistryIPUpdate(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -681,8 +709,9 @@ func TestServiceRegistryIPLoadBalancer(t *testing.T) { SessionAffinity: api.ServiceAffinityNone, Type: api.ServiceTypeLoadBalancer, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, } @@ -754,8 +783,9 @@ func TestCreate(t *testing.T) { SessionAffinity: "None", Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, }, @@ -771,8 +801,9 @@ func TestCreate(t *testing.T) { SessionAffinity: "None", Type: api.ServiceTypeClusterIP, Ports: []api.ServicePort{{ - Port: 6502, - Protocol: api.ProtocolTCP, + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(6502), }}, }, }, diff --git a/pkg/util/validation.go b/pkg/util/validation.go index dd8905eb5da..66010f5c8f0 100644 --- a/pkg/util/validation.go +++ b/pkg/util/validation.go @@ -105,6 +105,36 @@ func IsValidPortNum(port int) bool { return 0 < port && port < 65536 } +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 + } + if doubleHyphensRegexp.MatchString(port) { + return false + } + if identifierNoHyphensBeginEndRegexp.MatchString(port) && atLeastOneLetterRegexp.MatchString(port) { + return true + } + return false +} + // IsValidIPv4 tests that the argument is a valid IPv4 address. func IsValidIPv4(value string) bool { return net.ParseIP(value) != nil && net.ParseIP(value).To4() != nil diff --git a/pkg/util/validation_test.go b/pkg/util/validation_test.go index c29f2ddd26f..b14565f020a 100644 --- a/pkg/util/validation_test.go +++ b/pkg/util/validation_test.go @@ -154,6 +154,22 @@ func TestIsValidPortNum(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 '%d'", val) + } + } + + badValues := []string{"longerthan15characters", "", "12345", "1-2-3-4", "-begin", "end-", "two--hyphens", "1-2", "whois++"} + for _, val := range badValues { + if IsValidPortName(val) { + t.Errorf("expected false for '%d'", val) + } + } +} + func TestIsQualifiedName(t *testing.T) { successCases := []string{ "simple",