aggregation: restore cluster-ip routing tests

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

@@ -19,10 +19,14 @@ go_test(
     tags = ["automanaged"],
     deps = [
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
         "//vendor/k8s.io/client-go/listers/core/v1:go_default_library",
         "//vendor/k8s.io/client-go/pkg/api/v1:go_default_library",
+        "//vendor/k8s.io/client-go/tools/cache:go_default_library",
+        "//vendor/k8s.io/kube-aggregator/pkg/apis/apiregistration:go_default_library",
     ],
 )
 

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

@@ -17,103 +17,215 @@ limitations under the License.
 package proxy
 
 import (
+	"net/url"
 	"testing"
 
-	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/intstr"
-	listersv1 "k8s.io/client-go/listers/core/v1"
+	v1listers "k8s.io/client-go/listers/core/v1"
 	"k8s.io/client-go/pkg/api/v1"
-	"net/http"
+	"k8s.io/client-go/tools/cache"
+
+	"k8s.io/kube-aggregator/pkg/apis/apiregistration"
 )
 
-type serviceListerMock struct {
+func TestResolve(t *testing.T) {
-	services []*v1.Service
+	endpoints := []*v1.Endpoints{{
-	err      error
+		ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
-}
+		Subsets: []v1.EndpointSubset{{
+			Addresses: []v1.EndpointAddress{{Hostname: "dummy-host", IP: "127.0.0.1"}},
+			Ports:     []v1.EndpointPort{{Port: 443}},
+		}},
+	}}
 
-func (s *serviceListerMock) List(selector labels.Selector) (ret []*v1.Service, err error) {
+	type expectation struct {
-	return s.services, err
+		url   string
-}
+		error bool
-
-func (s *serviceListerMock) Services(namespace string) listersv1.ServiceNamespaceLister {
-	return nil
-}
-
-func (s *serviceListerMock) GetPodServices(pod *v1.Pod) ([]*v1.Service, error) {
-	return nil, nil
-}
-
-type endpointsListerMock struct {
-	endpoints []*v1.Endpoints
-	err       error
-}
-
-func (e *endpointsListerMock) List(selector labels.Selector) (ret []*v1.Endpoints, err error) {
-	return e.endpoints, e.err
-}
-
-func (e *endpointsListerMock) Endpoints(namespace string) listersv1.EndpointsNamespaceLister {
-	return endpointsNamespaceListMock{
-		endpoints: e.endpoints,
-		err:       e.err,
-	}
-}
-
-type endpointsNamespaceListMock struct {
-	endpoints []*v1.Endpoints
-	err       error
-}
-
-func (e endpointsNamespaceListMock) List(selector labels.Selector) (ret []*v1.Endpoints, err error) {
-	return e.endpoints, e.err
-}
-
-func (e endpointsNamespaceListMock) Get(name string) (*v1.Endpoints, error) {
-	if len(e.endpoints) == 0 {
-		return nil, e.err
-	}
-	return e.endpoints[0], e.err
-}
-
-func TestNoEndpointNoPort(t *testing.T) {
-	services := &serviceListerMock{}
-	endpoints := &endpointsListerMock{err: errors.NewNotFound(v1.Resource("endpoints"), "dummy-svc")}
-	url, err := ResolveEndpoint(services, endpoints, "dummy-ns", "dummy-svc")
-	if url != nil {
-		t.Error("Should not have gotten back an URL")
-	}
-	if err == nil {
-		t.Error("Should have gotten an error")
-	}
-	se, ok := err.(*errors.StatusError)
-	if !ok {
-		t.Error("Should have gotten a status error not %T", err)
-	}
-	if se.ErrStatus.Code != http.StatusNotFound {
-		t.Error("Should have gotten a http 404 not %d", se.ErrStatus.Code)
-	}
-}
-
-func TestOneEndpointNoPort(t *testing.T) {
-	services := &serviceListerMock{}
-	address := v1.EndpointAddress{Hostname: "dummy-host", IP: "127.0.0.1"}
-	addresses := []v1.EndpointAddress{address}
-	port := v1.EndpointPort{Port: 443}
-	ports := []v1.EndpointPort{port}
-	endpoint := v1.EndpointSubset{Addresses: addresses, Ports: ports}
-	subsets := []v1.EndpointSubset{endpoint}
-	one := &v1.Endpoints{Subsets: subsets}
-	slice := []*v1.Endpoints{one}
-	endpoints := &endpointsListerMock{endpoints: slice}
-	url, err := ResolveEndpoint(services, endpoints, "dummy-ns", "dummy-svc")
-	if err != nil {
-		t.Errorf("Should not have gotten error %v", err)
-	}
-	if url == nil {
-		t.Error("Should not have gotten back an URL")
-	}
-	if url.Host != "127.0.0.1:443" {
-		t.Error("Should have gotten back a host of dummy-host not %s", url.Host)
 	}
 
+	tests := []struct {
+		name       string
+		services   []*v1.Service
+		endpoints  []*v1.Endpoints
+		apiService *apiregistration.APIService
+
+		clusterMode  expectation
+		endpointMode expectation
+	}{
+		{
+			name: "cluster ip without ports",
+			services: []*v1.Service{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
+					Spec: v1.ServiceSpec{
+						Type:      v1.ServiceTypeClusterIP,
+						ClusterIP: "hit",
+					},
+				},
+			},
+			endpoints: endpoints, // TODO: do we have endpoints without ports?
+			apiService: &apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
+				Spec: apiregistration.APIServiceSpec{
+					Service: &apiregistration.ServiceReference{
+						Namespace: "one",
+						Name:      "alfa",
+					},
+				},
+			},
+
+			clusterMode:  expectation{url: "https://hit"}, // TODO: this should be an error as well
+			endpointMode: expectation{error: true},
+		},
+		{
+			name: "cluster ip",
+			services: []*v1.Service{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
+					Spec: v1.ServiceSpec{
+						Type:      v1.ServiceTypeClusterIP,
+						ClusterIP: "hit",
+					},
+				},
+			},
+			endpoints: endpoints,
+			apiService: &apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
+				Spec: apiregistration.APIServiceSpec{
+					Service: &apiregistration.ServiceReference{
+						Namespace: "one",
+						Name:      "alfa",
+					},
+				},
+			},
+
+			clusterMode:  expectation{url: "https://hit"},
+			endpointMode: expectation{url: "https://127.0.0.1"},
+		},
+		{
+			name: "cluster ip without endpoints",
+			services: []*v1.Service{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
+					Spec: v1.ServiceSpec{
+						Type:      v1.ServiceTypeClusterIP,
+						ClusterIP: "hit",
+					},
+				},
+			},
+			endpoints: nil,
+			apiService: &apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
+				Spec: apiregistration.APIServiceSpec{
+					Service: &apiregistration.ServiceReference{
+						Namespace: "one",
+						Name:      "alfa",
+					},
+				},
+			},
+
+			clusterMode:  expectation{url: "https://hit"},
+			endpointMode: expectation{error: true},
+		},
+		{
+			name: "loadbalancer",
+			services: []*v1.Service{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
+					Spec: v1.ServiceSpec{
+						Type:      v1.ServiceTypeLoadBalancer,
+						ClusterIP: "lb",
+					},
+				},
+			},
+			endpoints: nil,
+			apiService: &apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
+				Spec: apiregistration.APIServiceSpec{
+					Service: &apiregistration.ServiceReference{
+						Namespace: "one",
+						Name:      "alfa",
+					},
+				},
+			},
+
+			clusterMode:  expectation{url: "https://lb"},
+			endpointMode: expectation{error: true},
+		},
+		{
+			name: "node port",
+			services: []*v1.Service{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
+					Spec: v1.ServiceSpec{
+						Type:      v1.ServiceTypeNodePort,
+						ClusterIP: "np",
+					},
+				},
+			},
+			endpoints: nil,
+			apiService: &apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
+				Spec: apiregistration.APIServiceSpec{
+					Service: &apiregistration.ServiceReference{
+						Namespace: "one",
+						Name:      "alfa",
+					},
+				},
+			},
+
+			clusterMode:  expectation{url: "https://np"},
+			endpointMode: expectation{error: true},
+		},
+		{
+			name:      "missing service",
+			services:  nil,
+			endpoints: nil,
+			apiService: &apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
+				Spec: apiregistration.APIServiceSpec{
+					Service: &apiregistration.ServiceReference{
+						Namespace: "one",
+						Name:      "alfa",
+					},
+				},
+			},
+
+			clusterMode:  expectation{url: "https://alfa.one.svc"}, // defaulting to 443 due to https:// prefix
+			endpointMode: expectation{error: true},
+		},
+	}
+
+	for _, test := range tests {
+		serviceCache := cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
+		serviceLister := v1listers.NewServiceLister(serviceCache)
+		for i := range test.services {
+			serviceCache.Add(test.services[i])
+		}
+
+		endpointCache := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
+		endpointLister := v1listers.NewEndpointsLister(endpointCache)
+		for i := range test.endpoints {
+			endpointCache.Add(test.endpoints[i])
+		}
+
+		check := func(mode string, expected expectation, url *url.URL, err error) {
+			switch {
+			case err == nil && expected.error:
+				t.Errorf("%s in %s mode expected error, got none", test.name, mode)
+			case err != nil && expected.error:
+				// ignore
+			case err != nil:
+				t.Errorf("%s in %s mode unexpected error: %v", test.name, mode, err)
+			case url.String() != expected.url:
+				t.Errorf("%s in %s mode expected url %q, got %q", test.name, mode, expected.url, url.String())
+			}
+		}
+
+		clusterURL, err := ResolveCluster(serviceLister, "one", "alfa")
+		check("cluster", test.clusterMode, clusterURL, err)
+
+		endpointURL, err := ResolveEndpoint(serviceLister, endpointLister, "one", "alfa")
+		check("endpoint", test.endpointMode, endpointURL, err)
+	}
 }

staging/src/k8s.io/kube-aggregator/pkg/apiserver/BUILD

@@ -25,8 +25,6 @@ go_test(
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
-        "//vendor/k8s.io/client-go/listers/core/v1:go_default_library",
-        "//vendor/k8s.io/client-go/pkg/api/v1:go_default_library",
         "//vendor/k8s.io/client-go/tools/cache:go_default_library",
         "//vendor/k8s.io/kube-aggregator/pkg/apis/apiregistration:go_default_library",
         "//vendor/k8s.io/kube-aggregator/pkg/client/listers/apiregistration/internalversion:go_default_library",

staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiservice_controller.go

@@ -106,28 +106,6 @@ func (c *APIServiceRegistrationController) sync(key string) error {
 	return nil
 }
 
-func (c *APIServiceRegistrationController) getDestinationHost(apiService *apiregistration.APIService) string {
-	if apiService.Spec.Service == nil {
-		return ""
-	}
-
-	destinationHost := apiService.Spec.Service.Name + "." + apiService.Spec.Service.Namespace + ".svc"
-	service, err := c.serviceLister.Services(apiService.Spec.Service.Namespace).Get(apiService.Spec.Service.Name)
-	if err != nil {
-		return destinationHost
-	}
-	switch {
-	// use IP from a clusterIP for these service types
-	case service.Spec.Type == v1.ServiceTypeClusterIP,
-		service.Spec.Type == v1.ServiceTypeNodePort,
-		service.Spec.Type == v1.ServiceTypeLoadBalancer:
-		return service.Spec.ClusterIP
-	}
-
-	// return the normal DNS name by default
-	return destinationHost
-}
-
 func (c *APIServiceRegistrationController) Run(stopCh <-chan struct{}) {
 	defer utilruntime.HandleCrash()
 	defer c.queue.ShutDown()

staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiservice_controller_test.go

@@ -1,139 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package apiserver
-
-import (
-	"testing"
-
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	v1listers "k8s.io/client-go/listers/core/v1"
-	"k8s.io/client-go/pkg/api/v1"
-	"k8s.io/client-go/tools/cache"
-
-	"k8s.io/kube-aggregator/pkg/apis/apiregistration"
-)
-
-func TestGetDestinationHost(t *testing.T) {
-	tests := []struct {
-		name       string
-		services   []*v1.Service
-		apiService *apiregistration.APIService
-
-		expected string
-	}{
-		{
-			name: "cluster ip",
-			services: []*v1.Service{
-				{
-					ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
-					Spec: v1.ServiceSpec{
-						Type:      v1.ServiceTypeClusterIP,
-						ClusterIP: "hit",
-					},
-				},
-			},
-			apiService: &apiregistration.APIService{
-				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
-				Spec: apiregistration.APIServiceSpec{
-					Service: &apiregistration.ServiceReference{
-						Namespace: "one",
-						Name:      "alfa",
-					},
-				},
-			},
-
-			expected: "hit",
-		},
-		{
-			name: "loadbalancer",
-			services: []*v1.Service{
-				{
-					ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
-					Spec: v1.ServiceSpec{
-						Type:      v1.ServiceTypeLoadBalancer,
-						ClusterIP: "lb",
-					},
-				},
-			},
-			apiService: &apiregistration.APIService{
-				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
-				Spec: apiregistration.APIServiceSpec{
-					Service: &apiregistration.ServiceReference{
-						Namespace: "one",
-						Name:      "alfa",
-					},
-				},
-			},
-
-			expected: "lb",
-		},
-		{
-			name: "node port",
-			services: []*v1.Service{
-				{
-					ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alfa"},
-					Spec: v1.ServiceSpec{
-						Type:      v1.ServiceTypeNodePort,
-						ClusterIP: "np",
-					},
-				},
-			},
-			apiService: &apiregistration.APIService{
-				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
-				Spec: apiregistration.APIServiceSpec{
-					Service: &apiregistration.ServiceReference{
-						Namespace: "one",
-						Name:      "alfa",
-					},
-				},
-			},
-
-			expected: "np",
-		},
-		{
-			name: "missing service",
-			apiService: &apiregistration.APIService{
-				ObjectMeta: metav1.ObjectMeta{Name: "v1."},
-				Spec: apiregistration.APIServiceSpec{
-					Service: &apiregistration.ServiceReference{
-						Namespace: "one",
-						Name:      "alfa",
-					},
-				},
-			},
-
-			expected: "alfa.one.svc",
-		},
-	}
-
-	for _, test := range tests {
-		serviceCache := cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
-		serviceLister := v1listers.NewServiceLister(serviceCache)
-		c := &APIServiceRegistrationController{
-			serviceLister: serviceLister,
-		}
-		for i := range test.services {
-			serviceCache.Add(test.services[i])
-		}
-
-		actual := c.getDestinationHost(test.apiService)
-		if actual != test.expected {
-			t.Errorf("%s expected %v, got %v", test.name, test.expected, actual)
-		}
-
-	}
-}