From fe54e1f38666977600adda9aa97a81e7c5d31ac7 Mon Sep 17 00:00:00 2001 From: Rob Scott Date: Mon, 28 Oct 2019 15:22:01 -0700 Subject: [PATCH] Fixing EndpointSlice port validation This updates EndpointSlice port validation to mirror the validation already in use for Service and Endpoint ports. This is required to ensure all valid Service ports can be mapped directly to EndpointSlice ports. --- api/openapi-spec/swagger.json | 2 +- pkg/apis/discovery/types.go | 9 +++---- pkg/apis/discovery/validation/validation.go | 8 ++---- .../discovery/validation/validation_test.go | 27 +++++++++++++++++++ .../api/discovery/v1alpha1/generated.proto | 9 +++---- .../k8s.io/api/discovery/v1alpha1/types.go | 9 +++---- .../v1alpha1/types_swagger_doc_generated.go | 2 +- 7 files changed, 43 insertions(+), 23 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 6c0a05b96ce..b2509dbb63c 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -12071,7 +12071,7 @@ "description": "EndpointPort represents a Port used by an EndpointSlice", "properties": { "name": { - "description": "The name of this port. All ports in an EndpointSlice must have a unique name. If the EndpointSlice is dervied from a Kubernetes service, this corresponds to the Service.ports[].name. Name must either be an empty string or pass IANA_SVC_NAME validation: * must be no more than 15 characters long * may contain only [-a-z0-9] * must contain at least one letter [a-z] * it must not start or end with a hyphen, nor contain adjacent hyphens Default is empty string.", + "description": "The name of this port. All ports in an EndpointSlice must have a unique name. If the EndpointSlice is dervied from a Kubernetes service, this corresponds to the Service.ports[].name. Name must either be an empty string or pass DNS_LABEL validation: * must be no more than 63 characters long. * must consist of lower case alphanumeric characters or '-'. * must start and end with an alphanumeric character. Default is empty string.", "type": "string" }, "port": { diff --git a/pkg/apis/discovery/types.go b/pkg/apis/discovery/types.go index 2d4c573ce66..cb5c91f8146 100644 --- a/pkg/apis/discovery/types.go +++ b/pkg/apis/discovery/types.go @@ -118,11 +118,10 @@ type EndpointPort struct { // The name of this port. All ports in an EndpointSlice must have a unique // name. If the EndpointSlice is dervied from a Kubernetes service, this // corresponds to the Service.ports[].name. - // Name must either be an empty string or pass IANA_SVC_NAME validation: - // * must be no more than 15 characters long - // * may contain only [-a-z0-9] - // * must contain at least one letter [a-z] - // * it must not start or end with a hyphen, nor contain adjacent hyphens + // Name must either be an empty string or pass DNS_LABEL validation: + // * must be no more than 63 characters long. + // * must consist of lower case alphanumeric characters or '-'. + // * must start and end with an alphanumeric character. Name *string // The IP protocol for this port. // Must be UDP, TCP, or SCTP. diff --git a/pkg/apis/discovery/validation/validation.go b/pkg/apis/discovery/validation/validation.go index 96d9038b7bb..cc9d6b5957b 100644 --- a/pkg/apis/discovery/validation/validation.go +++ b/pkg/apis/discovery/validation/validation.go @@ -109,9 +109,7 @@ func validateEndpoints(endpoints []discovery.Endpoint, addrType discovery.Addres allErrs = append(allErrs, metavalidation.ValidateLabels(endpoint.Topology, topologyPath)...) if endpoint.Hostname != nil { - for _, msg := range validation.IsDNS1123Label(*endpoint.Hostname) { - allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), *endpoint.Hostname, msg)) - } + allErrs = append(allErrs, apivalidation.ValidateDNS1123Label(*endpoint.Hostname, idxPath.Child("hostname"))...) } } @@ -131,9 +129,7 @@ func validatePorts(endpointPorts []discovery.EndpointPort, fldPath *field.Path) idxPath := fldPath.Index(i) if len(*endpointPort.Name) > 0 { - for _, msg := range validation.IsValidPortName(*endpointPort.Name) { - allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), endpointPort.Name, msg)) - } + allErrs = append(allErrs, apivalidation.ValidateDNS1123Label(*endpointPort.Name, idxPath.Child("name"))...) } if portNames.Has(*endpointPort.Name) { diff --git a/pkg/apis/discovery/validation/validation_test.go b/pkg/apis/discovery/validation/validation_test.go index 907dd7d2134..4faf29a40ec 100644 --- a/pkg/apis/discovery/validation/validation_test.go +++ b/pkg/apis/discovery/validation/validation_test.go @@ -18,6 +18,7 @@ package validation import ( "fmt" + "strings" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -104,6 +105,20 @@ func TestValidateEndpointSlice(t *testing.T) { }}, }, }, + "long-port-name": { + expectedErrors: 0, + endpointSlice: &discovery.EndpointSlice{ + ObjectMeta: standardMeta, + AddressType: addressTypePtr(discovery.AddressTypeIP), + Ports: []discovery.EndpointPort{{ + Name: utilpointer.StringPtr(strings.Repeat("a", 63)), + Protocol: protocolPtr(api.ProtocolTCP), + }}, + Endpoints: []discovery.Endpoint{{ + Addresses: generateIPAddresses(1), + }}, + }, + }, "empty-ports-and-endpoints": { expectedErrors: 0, endpointSlice: &discovery.EndpointSlice{ @@ -200,6 +215,18 @@ func TestValidateEndpointSlice(t *testing.T) { Endpoints: []discovery.Endpoint{}, }, }, + "bad-port-name-length": { + expectedErrors: 1, + endpointSlice: &discovery.EndpointSlice{ + ObjectMeta: standardMeta, + AddressType: addressTypePtr(discovery.AddressTypeIP), + Ports: []discovery.EndpointPort{{ + Name: utilpointer.StringPtr(strings.Repeat("a", 64)), + Protocol: protocolPtr(api.ProtocolTCP), + }}, + Endpoints: []discovery.Endpoint{}, + }, + }, "invalid-port-protocol": { expectedErrors: 1, endpointSlice: &discovery.EndpointSlice{ diff --git a/staging/src/k8s.io/api/discovery/v1alpha1/generated.proto b/staging/src/k8s.io/api/discovery/v1alpha1/generated.proto index 47282cd786f..53fcad3f260 100644 --- a/staging/src/k8s.io/api/discovery/v1alpha1/generated.proto +++ b/staging/src/k8s.io/api/discovery/v1alpha1/generated.proto @@ -88,11 +88,10 @@ message EndpointPort { // The name of this port. All ports in an EndpointSlice must have a unique // name. If the EndpointSlice is dervied from a Kubernetes service, this // corresponds to the Service.ports[].name. - // Name must either be an empty string or pass IANA_SVC_NAME validation: - // * must be no more than 15 characters long - // * may contain only [-a-z0-9] - // * must contain at least one letter [a-z] - // * it must not start or end with a hyphen, nor contain adjacent hyphens + // Name must either be an empty string or pass DNS_LABEL validation: + // * must be no more than 63 characters long. + // * must consist of lower case alphanumeric characters or '-'. + // * must start and end with an alphanumeric character. // Default is empty string. optional string name = 1; diff --git a/staging/src/k8s.io/api/discovery/v1alpha1/types.go b/staging/src/k8s.io/api/discovery/v1alpha1/types.go index 638c9bf517d..be08b33317b 100644 --- a/staging/src/k8s.io/api/discovery/v1alpha1/types.go +++ b/staging/src/k8s.io/api/discovery/v1alpha1/types.go @@ -121,11 +121,10 @@ type EndpointPort struct { // The name of this port. All ports in an EndpointSlice must have a unique // name. If the EndpointSlice is dervied from a Kubernetes service, this // corresponds to the Service.ports[].name. - // Name must either be an empty string or pass IANA_SVC_NAME validation: - // * must be no more than 15 characters long - // * may contain only [-a-z0-9] - // * must contain at least one letter [a-z] - // * it must not start or end with a hyphen, nor contain adjacent hyphens + // Name must either be an empty string or pass DNS_LABEL validation: + // * must be no more than 63 characters long. + // * must consist of lower case alphanumeric characters or '-'. + // * must start and end with an alphanumeric character. // Default is empty string. Name *string `json:"name,omitempty" protobuf:"bytes,1,name=name"` // The IP protocol for this port. diff --git a/staging/src/k8s.io/api/discovery/v1alpha1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/discovery/v1alpha1/types_swagger_doc_generated.go index 85f80397349..908e7070a76 100644 --- a/staging/src/k8s.io/api/discovery/v1alpha1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/discovery/v1alpha1/types_swagger_doc_generated.go @@ -51,7 +51,7 @@ func (EndpointConditions) SwaggerDoc() map[string]string { var map_EndpointPort = map[string]string{ "": "EndpointPort represents a Port used by an EndpointSlice", - "name": "The name of this port. All ports in an EndpointSlice must have a unique name. If the EndpointSlice is dervied from a Kubernetes service, this corresponds to the Service.ports[].name. Name must either be an empty string or pass IANA_SVC_NAME validation: * must be no more than 15 characters long * may contain only [-a-z0-9] * must contain at least one letter [a-z] * it must not start or end with a hyphen, nor contain adjacent hyphens Default is empty string.", + "name": "The name of this port. All ports in an EndpointSlice must have a unique name. If the EndpointSlice is dervied from a Kubernetes service, this corresponds to the Service.ports[].name. Name must either be an empty string or pass DNS_LABEL validation: * must be no more than 63 characters long. * must consist of lower case alphanumeric characters or '-'. * must start and end with an alphanumeric character. Default is empty string.", "protocol": "The IP protocol for this port. Must be UDP, TCP, or SCTP. Default is TCP.", "port": "The port number of the endpoint. If this is not specified, ports are not restricted and must be interpreted in the context of the specific consumer.", }