Add port to ServiceResolvers

pkg/apis/admissionregistration/validation/validation_test.go

@@ -377,6 +377,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 						Service: &admissionregistration.ServiceReference{
 							Namespace: "ns",
 							Name:      "n",
+							Port:      443,
 						},
 						URL: strPtr("example.com/k8s/webhook"),
 					},
@@ -478,6 +479,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 							Namespace: "ns",
 							Name:      "n",
 							Path:      strPtr("foo/"),
+							Port:      443,
 						},
 					},
 				},
@@ -494,6 +496,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 							Namespace: "ns",
 							Name:      "n",
 							Path:      strPtr("/"),
+							Port:      443,
 						},
 					},
 				},
@@ -510,6 +513,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 							Namespace: "ns",
 							Name:      "n",
 							Path:      strPtr("/foo"),
+							Port:      443,
 						},
 					},
 				},
@@ -526,6 +530,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 							Namespace: "ns",
 							Name:      "n",
 							Path:      strPtr("//"),
+							Port:      443,
 						},
 					},
 				},
@@ -542,6 +547,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 							Namespace: "ns",
 							Name:      "n",
 							Path:      strPtr("/foo//bar/"),
+							Port:      443,
 						},
 					},
 				},
@@ -557,6 +563,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 							Namespace: "ns",
 							Name:      "n",
 							Path:      strPtr("/foo/bar//"),
+							Port:      443,
 						},
 					},
 				},
@@ -573,6 +580,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 							Namespace: "ns",
 							Name:      "n",
 							Path:      strPtr("/apis/foo.bar/v1alpha1/--bad"),
+							Port:      443,
 						},
 					},
 				},
@@ -595,7 +603,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 						},
 					},
 				}, true),
-			expectedError: `Invalid value: 0: port must be a valid number between 1 and 65535, inclusive`,
+			expectedError: `Invalid value: 0: port is not valid: must be between 1 and 65535, inclusive`,
 		},
 		{
 			name: "invalid port >65535",
@@ -613,7 +621,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 						},
 					},
 				}, true),
-			expectedError: `Invalid value: 65536: port must be a valid number between 1 and 65535, inclusive`,
+			expectedError: `Invalid value: 65536: port is not valid: must be between 1 and 65535, inclusive`,
 		},
 		{
 			name: "timeout seconds cannot be greater than 30",

pkg/apis/auditregistration/validation/validation_test.go

@@ -155,6 +155,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 					Service: &auditregistration.ServiceReference{
 						Namespace: "ns",
 						Name:      "n",
+						Port:      443,
 					},
 					URL: strPtr("example.com/k8s/webhook"),
 				},
@@ -223,6 +224,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 						Namespace: "ns",
 						Name:      "n",
 						Path:      strPtr("foo/"),
+						Port:      443,
 					},
 				},
 			},
@@ -240,7 +242,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 					},
 				},
 			},
-			expectedError: `Invalid value: 65536: port must be a valid number between 1 and 65535, inclusive`,
+			expectedError: `Invalid value: 65536: port is not valid: must be between 1 and 65535, inclusive`,
 		},
 		{
 			name: "invalid port 0",
@@ -254,7 +256,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 					},
 				},
 			},
-			expectedError: `Invalid value: 0: port must be a valid number between 1 and 65535, inclusive`,
+			expectedError: `Invalid value: 0: port is not valid: must be between 1 and 65535, inclusive`,
 		},
 		{
 			name: "path accepts slash",
@@ -264,6 +266,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 						Namespace: "ns",
 						Name:      "n",
 						Path:      strPtr("/"),
+						Port:      443,
 					},
 				},
 			},
@@ -277,6 +280,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 						Namespace: "ns",
 						Name:      "n",
 						Path:      strPtr("/foo"),
+						Port:      443,
 					},
 				},
 			},
@@ -290,6 +294,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 						Namespace: "ns",
 						Name:      "n",
 						Path:      strPtr("//"),
+						Port:      443,
 					},
 				},
 			},
@@ -303,6 +308,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 						Namespace: "ns",
 						Name:      "n",
 						Path:      strPtr("/foo//bar/"),
+						Port:      443,
 					},
 				},
 			},
@@ -315,6 +321,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 						Namespace: "ns",
 						Name:      "n",
 						Path:      strPtr("/foo/bar//"),
+						Port:      443,
 					},
 				},
 			},
@@ -328,6 +335,7 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 						Namespace: "ns",
 						Name:      "n",
 						Path:      strPtr("/apis/foo.bar/v1alpha1/--bad"),
+						Port:      443,
 					},
 				},
 			},

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go

@@ -75,6 +75,7 @@ func webhookClientConfigForCRD(crd *internal.CustomResourceDefinition) *webhook.
 		ret.Service = &webhook.ClientConfigService{
 			Name:      apiConfig.Service.Name,
 			Namespace: apiConfig.Service.Namespace,
+			Port:      apiConfig.Service.Port,
 		}
 		if apiConfig.Service.Path != nil {
 			ret.Service.Path = *apiConfig.Service.Path

staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options/options.go

@@ -129,6 +129,6 @@ type serviceResolver struct {
 	services v1.ServiceLister
 }
 
-func (r *serviceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) {
-	return proxy.ResolveCluster(r.services, namespace, name)
+func (r *serviceResolver) ResolveEndpoint(namespace, name string, port int32) (*url.URL, error) {
+	return proxy.ResolveCluster(r.services, namespace, name, port)
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/initializer/initializer.go

@@ -32,7 +32,7 @@ type WantsServiceResolver interface {
 // ServiceResolver knows how to convert a service reference into an actual
 // location.
 type ServiceResolver interface {
-	ResolveEndpoint(namespace, name string) (*url.URL, error)
+	ResolveEndpoint(namespace, name string, port int32) (*url.URL, error)
 }
 
 // WantsAuthenticationInfoResolverWrapper defines a function that wraps the standard AuthenticationInfoResolver

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/initializer/initializer_test.go

@@ -30,7 +30,7 @@ func (doNothingAdmission) Handles(o admission.Operation) bool { return false }
 
 type fakeServiceResolver struct{}
 
-func (f *fakeServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) {
+func (f *fakeServiceResolver) ResolveEndpoint(namespace, name string, port int32) (*url.URL, error) {
 	return nil, nil
 }
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/service_resolver.go

@@ -33,7 +33,7 @@ func NewServiceResolver(base url.URL) webhook.ServiceResolver {
 	return &serviceResolver{base}
 }
 
-func (f serviceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) {
+func (f serviceResolver) ResolveEndpoint(namespace, name string, port int32) (*url.URL, error) {
 	if namespace == "failResolve" {
 		return nil, fmt.Errorf("couldn't resolve service location")
 	}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go

@@ -34,6 +34,11 @@ func HookClientConfigForWebhook(w *v1beta1.Webhook) webhook.ClientConfig {
 			Name:      w.ClientConfig.Service.Name,
 			Namespace: w.ClientConfig.Service.Namespace,
 		}
+		if w.ClientConfig.Service.Port != nil {
+			ret.Service.Port = *w.ClientConfig.Service.Port
+		} else {
+			ret.Service.Port = 443
+		}
 		if w.ClientConfig.Service.Path != nil {
 			ret.Service.Path = *w.ClientConfig.Service.Path
 		}

staging/src/k8s.io/apiserver/pkg/audit/util/conversion.go

@@ -35,6 +35,12 @@ func HookClientConfigForSink(a *v1alpha1.AuditSink) webhook.ClientConfig {
 			Name:      c.Service.Name,
 			Namespace: c.Service.Namespace,
 		}
+		if c.Service.Port != nil {
+			ret.Service.Port = *c.Service.Port
+		} else {
+			ret.Service.Port = 443
+		}
+
 		if c.Service.Path != nil {
 			ret.Service.Path = *c.Service.Path
 		}

staging/src/k8s.io/apiserver/pkg/util/proxy/proxy.go

@@ -26,28 +26,25 @@ import (
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	listersv1 "k8s.io/client-go/listers/core/v1"
-
-	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 // findServicePort finds the service port by name or numerically.
-func findServicePort(svc *v1.Service, port intstr.IntOrString) (*v1.ServicePort, error) {
+func findServicePort(svc *v1.Service, port int32) (*v1.ServicePort, error) {
 	for _, svcPort := range svc.Spec.Ports {
-		if (port.Type == intstr.Int && int32(svcPort.Port) == port.IntVal) || (port.Type == intstr.String && svcPort.Name == port.StrVal) {
+		if svcPort.Port == port {
 			return &svcPort, nil
 		}
 	}
-	return nil, errors.NewServiceUnavailable(fmt.Sprintf("no service port %q found for service %q", port.String(), svc.Name))
+	return nil, errors.NewServiceUnavailable(fmt.Sprintf("no service port %q found for service %q", port, svc.Name))
 }
 
 // ResourceLocation returns a URL to which one can send traffic for the specified service.
-func ResolveEndpoint(services listersv1.ServiceLister, endpoints listersv1.EndpointsLister, namespace, id string) (*url.URL, error) {
+func ResolveEndpoint(services listersv1.ServiceLister, endpoints listersv1.EndpointsLister, namespace, id string, port int32) (*url.URL, error) {
 	svc, err := services.Services(namespace).Get(id)
 	if err != nil {
 		return nil, err
 	}
 
-	port := intstr.FromInt(443)
 	svcPort, err := findServicePort(svc, port)
 	if err != nil {
 		return nil, err
@@ -92,14 +89,12 @@ func ResolveEndpoint(services listersv1.ServiceLister, endpoints listersv1.Endpo
 	return nil, errors.NewServiceUnavailable(fmt.Sprintf("no endpoints available for service %q", id))
 }
 
-func ResolveCluster(services listersv1.ServiceLister, namespace, id string) (*url.URL, error) {
+func ResolveCluster(services listersv1.ServiceLister, namespace, id string, port int32) (*url.URL, error) {
 	svc, err := services.Services(namespace).Get(id)
 	if err != nil {
 		return nil, err
 	}
 
-	port := intstr.FromInt(443)
-
 	switch {
 	case svc.Spec.Type == v1.ServiceTypeClusterIP && svc.Spec.ClusterIP == v1.ClusterIPNone:
 		return nil, fmt.Errorf(`cannot route to service with ClusterIP "None"`)
@@ -114,12 +109,9 @@ func ResolveCluster(services listersv1.ServiceLister, namespace, id string) (*ur
 			Host:   net.JoinHostPort(svc.Spec.ClusterIP, fmt.Sprintf("%d", svcPort.Port)),
 		}, nil
 	case svc.Spec.Type == v1.ServiceTypeExternalName:
-		if port.Type != intstr.Int {
-			return nil, fmt.Errorf("named ports not supported")
-		}
 		return &url.URL{
 			Scheme: "https",
-			Host:   net.JoinHostPort(svc.Spec.ExternalName, port.String()),
+			Host:   net.JoinHostPort(svc.Spec.ExternalName, fmt.Sprintf("%d", port)),
 		}, nil
 	default:
 		return nil, fmt.Errorf("unsupported service type %q", svc.Spec.Type)

staging/src/k8s.io/apiserver/pkg/util/proxy/proxy_test.go

@@ -234,10 +234,10 @@ func TestResolve(t *testing.T) {
 			}
 		}
 
-		clusterURL, err := ResolveCluster(serviceLister, "one", "alfa")
+		clusterURL, err := ResolveCluster(serviceLister, "one", "alfa", 443)
 		check("cluster", test.clusterMode, clusterURL, err)
 
-		endpointURL, err := ResolveEndpoint(serviceLister, endpointLister, "one", "alfa")
+		endpointURL, err := ResolveEndpoint(serviceLister, endpointLister, "one", "alfa", 443)
 		check("endpoint", test.endpointMode, endpointURL, err)
 	}
 }

staging/src/k8s.io/apiserver/pkg/util/webhook/client.go

@@ -49,6 +49,7 @@ type ClientConfigService struct {
 	Name      string
 	Namespace string
 	Path      string
+	Port      int32
 }
 
 // ClientManager builds REST clients to talk to webhooks. It caches the clients
@@ -164,7 +165,11 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
 		}
 		cfg.Dial = func(ctx context.Context, network, addr string) (net.Conn, error) {
 			if addr == host {
-				u, err := cm.serviceResolver.ResolveEndpoint(cc.Service.Namespace, cc.Service.Name)
+				port := cc.Service.Port
+				if port == 0 {
+					port = 443
+				}
+				u, err := cm.serviceResolver.ResolveEndpoint(cc.Service.Namespace, cc.Service.Name, port)
 				if err != nil {
 					return nil, err
 				}

staging/src/k8s.io/apiserver/pkg/util/webhook/serviceresolver.go

@@ -24,7 +24,7 @@ import (
 
 // ServiceResolver knows how to convert a service reference into an actual location.
 type ServiceResolver interface {
-	ResolveEndpoint(namespace, name string) (*url.URL, error)
+	ResolveEndpoint(namespace, name string, port int32) (*url.URL, error)
 }
 
 type defaultServiceResolver struct{}
@@ -35,12 +35,13 @@ func NewDefaultServiceResolver() ServiceResolver {
 }
 
 // ResolveEndpoint constructs a service URL from a given namespace and name
-// note that the name and namespace are required and by default all created addresses use HTTPS scheme.
+// note that the name, namespace, and port are required and by default all
+// created addresses use HTTPS scheme.
 // for example:
 //  name=ross namespace=andromeda resolves to https://ross.andromeda.svc:443
-func (sr defaultServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) {
-	if len(name) == 0 || len(namespace) == 0 {
-		return nil, errors.New("cannot resolve an empty service name or namespace")
+func (sr defaultServiceResolver) ResolveEndpoint(namespace, name string, port int32) (*url.URL, error) {
+	if len(name) == 0 || len(namespace) == 0 || port == 0 {
+		return nil, errors.New("cannot resolve an empty service name or namespace or port")
 	}
-	return &url.URL{Scheme: "https", Host: fmt.Sprintf("%s.%s.svc:443", name, namespace)}, nil
+	return &url.URL{Scheme: "https", Host: fmt.Sprintf("%s.%s.svc:%d", name, namespace, port)}, nil
 }

staging/src/k8s.io/apiserver/pkg/util/webhook/serviceresolver_test.go

@@ -25,22 +25,25 @@ func TestDefaultServiceResolver(t *testing.T) {
 	scenarios := []struct {
 		serviceName      string
 		serviceNamespace string
+		port             int32
 		expectedOutput   string
 		expectError      bool
 	}{
 		// scenario 1: a service name along with a namespace resolves
-		{serviceName: "ross", serviceNamespace: "andromeda", expectedOutput: "https://ross.andromeda.svc:443"},
+		{serviceName: "ross", serviceNamespace: "andromeda", port: 443, expectedOutput: "https://ross.andromeda.svc:443"},
 		// scenario 2: a service name without a namespace does not resolve
 		{serviceName: "ross", expectError: true},
 		// scenario 3: cannot resolve an empty service name
 		{serviceNamespace: "andromeda", expectError: true},
+		// scenario 1: a service name along with a namespace and different port resolves
+		{serviceName: "ross", serviceNamespace: "andromeda", port: 1002, expectedOutput: "https://ross.andromeda.svc:1002"},
 	}
 
 	// act
 	for index, scenario := range scenarios {
 		t.Run(fmt.Sprintf("scenario %d", index), func(t *testing.T) {
 			target := defaultServiceResolver{}
-			serviceURL, err := target.ResolveEndpoint(scenario.serviceNamespace, scenario.serviceName)
+			serviceURL, err := target.ResolveEndpoint(scenario.serviceNamespace, scenario.serviceName, scenario.port)
 
 			if err != nil && !scenario.expectError {
 				t.Errorf("unexpected error has occurred = %v", err)

staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go

@@ -63,7 +63,7 @@ func ValidateWebhookService(fldPath *field.Path, namespace, name string, path *s
 	}
 
 	if errs := validation.IsValidPortNum(int(port)); errs != nil {
-		allErrors = append(allErrors, field.Invalid(fldPath.Child("port"), port, "port is not valid:"+strings.Join(errs, ",")))
+		allErrors = append(allErrors, field.Invalid(fldPath.Child("port"), port, "port is not valid: "+strings.Join(errs, ", ")))
 	}
 
 	if path == nil {