Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore service port validation compatibility with 1.0/1.1 #21680

Merged
merged 1 commit into from
Feb 24, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/swagger-spec/v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -17772,7 +17772,7 @@
},
"targetPort": {
"type": "string",
"description": "Number or name of the port to access on the pods targeted by the service. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. 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). Defaults to the service port. More info: http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service"
"description": "Number or name of the port to access on the pods targeted by the service. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. 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 the 'port' field is used (an identity map). This field is ignored for services with clusterIP=None, and should be omitted or set equal to the 'port' field. More info: http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service"
},
"nodePort": {
"type": "integer",
Expand Down
2 changes: 1 addition & 1 deletion docs/api-reference/v1/definitions.html
Original file line number Diff line number Diff line change
Expand Up @@ -5513,7 +5513,7 @@ <h3 id="_v1_serviceport">v1.ServicePort</h3>
</tr>
<tr>
<td class="tableblock halign-left valign-top"><p class="tableblock">targetPort</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">Number or name of the port to access on the pods targeted by the service. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. If this is a string, it will be looked up as a named port in the target Pod&#8217;s container ports. If this is not specified, the value of Port is used (an identity map). Defaults to the service port. More info: <a href="http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service">http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service</a></p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">Number or name of the port to access on the pods targeted by the service. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. If this is a string, it will be looked up as a named port in the target Pod&#8217;s container ports. If this is not specified, the value of the <em>port</em> field is used (an identity map). This field is ignored for services with clusterIP=None, and should be omitted or set equal to the <em>port</em> field. More info: <a href="http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service">http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service</a></p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">string</p></td>
<td class="tableblock halign-left valign-top"></td>
Expand Down
6 changes: 4 additions & 2 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1526,8 +1526,10 @@ type ServicePort struct {

// Optional: The target port on pods selected by this service. 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 default value
// is the sames as the Port field (an identity map).
// Pod's container ports. If this is not specified, the value
// of the 'port' field is used (an identity map).
// This field is ignored for services with clusterIP=None, and should be
// omitted or set equal to the 'port' field.
TargetPort intstr.IntOrString `json:"targetPort"`

// The port on each node on which this service is exposed.
Expand Down
5 changes: 3 additions & 2 deletions pkg/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1848,8 +1848,9 @@ type ServicePort struct {
// Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME.
// 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).
// Defaults to the service port.
// of the 'port' field is used (an identity map).
// This field is ignored for services with clusterIP=None, and should be
// omitted or set equal to the 'port' field.
// More info: http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service
TargetPort intstr.IntOrString `json:"targetPort,omitempty"`

Expand Down
2 changes: 1 addition & 1 deletion pkg/api/v1/types_swagger_doc_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -1500,7 +1500,7 @@ var map_ServicePort = map[string]string{
"name": "The name of this port within the service. This must be a DNS_LABEL. All ports within a ServiceSpec must have unique names. This maps to the 'Name' field in EndpointPort objects. Optional if only one ServicePort is defined on this service.",
"protocol": "The IP protocol for this port. Supports \"TCP\" and \"UDP\". Default is TCP.",
"port": "The port that will be exposed by this service.",
"targetPort": "Number or name of the port to access on the pods targeted by the service. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. 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). Defaults to the service port. More info: http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service",
"targetPort": "Number or name of the port to access on the pods targeted by the service. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. 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 the 'port' field is used (an identity map). This field is ignored for services with clusterIP=None, and should be omitted or set equal to the 'port' field. More info: http://releases.k8s.io/HEAD/docs/user-guide/services.md#defining-a-service",
"nodePort": "The port on each node on which this service is exposed when type=NodePort or LoadBalancer. Usually assigned by the system. If specified, it will be allocated to the service if unused or else creation of the service will fail. Default is to auto-allocate a port if the ServiceType of this Service requires one. More info: http://releases.k8s.io/HEAD/docs/user-guide/services.md#type--nodeport",
}

Expand Down
13 changes: 8 additions & 5 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -1770,11 +1770,14 @@ func validateServicePort(sp *api.ServicePort, requireName, isHeadlessService boo
allErrs = append(allErrs, field.Invalid(fldPath.Child("targetPort"), sp.TargetPort, PortNameErrorMsg))
}

if isHeadlessService {
if sp.TargetPort.Type == intstr.String || (sp.TargetPort.Type == intstr.Int && sp.Port != sp.TargetPort.IntValue()) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), sp.Port, "must be equal to targetPort when clusterIP = None"))
}
}
// in the v1 API, targetPorts on headless services were tolerated.
// once we have version-specific validation, we can reject this on newer API versions, but until then, we have to tolerate it for compatibility.
//
// if isHeadlessService {
// if sp.TargetPort.Type == intstr.String || (sp.TargetPort.Type == intstr.Int && sp.Port != sp.TargetPort.IntValue()) {
// allErrs = append(allErrs, field.Invalid(fldPath.Child("targetPort"), sp.TargetPort, "must be equal to the value of 'port' when clusterIP = None"))
// }
// }

return allErrs
}
Expand Down
14 changes: 10 additions & 4 deletions pkg/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2620,22 +2620,28 @@ func TestValidateService(t *testing.T) {
numErrs: 0,
},
{
name: "invalid port headless",
name: "invalid port headless 1",
tweakSvc: func(s *api.Service) {
s.Spec.Ports[0].Port = 11722
s.Spec.Ports[0].TargetPort = intstr.FromInt(11721)
s.Spec.ClusterIP = api.ClusterIPNone
},
numErrs: 1,
// in the v1 API, targetPorts on headless services were tolerated.
// once we have version-specific validation, we can reject this on newer API versions, but until then, we have to tolerate it for compatibility.
// numErrs: 1,
numErrs: 0,
},
{
name: "invalid port headless",
name: "invalid port headless 2",
tweakSvc: func(s *api.Service) {
s.Spec.Ports[0].Port = 11722
s.Spec.Ports[0].TargetPort = intstr.FromString("target")
s.Spec.ClusterIP = api.ClusterIPNone
},
numErrs: 1,
// in the v1 API, targetPorts on headless services were tolerated.
// once we have version-specific validation, we can reject this on newer API versions, but until then, we have to tolerate it for compatibility.
// numErrs: 1,
numErrs: 0,
},
{
name: "invalid publicIPs localhost",
Expand Down