From 91ba8c37d037510baed6e152dbd14cdf2367cf91 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Tue, 23 Jan 2018 10:45:29 -0500
Subject: [PATCH] Return ServiceUnavailable error consistently from proxy

---
 .../pkg/apiserver/handler_proxy.go            |  3 +-
 .../pkg/apiserver/handler_proxy_test.go       | 38 ++++++++++++++++---
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go
index f9cd5930fbc..7d3d603b5d7 100644
--- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go
+++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go
@@ -120,7 +120,8 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	location.Scheme = "https"
 	rloc, err := r.serviceResolver.ResolveEndpoint(handlingInfo.serviceNamespace, handlingInfo.serviceName)
 	if err != nil {
-		http.Error(w, fmt.Sprintf("missing route (%s)", err.Error()), http.StatusInternalServerError)
+		glog.Errorf("error resolving %s/%s: %v", handlingInfo.serviceNamespace, handlingInfo.serviceName, err)
+		http.Error(w, "service unavailable", http.StatusServiceUnavailable)
 		return
 	}
 	location.Host = rloc.Host
diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go
index 5927e27afc4..b994cd18f88 100644
--- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go
+++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go
@@ -18,6 +18,7 @@ package apiserver
 
 import (
 	"crypto/tls"
+	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -84,13 +85,11 @@ func (*fakeRequestContextMapper) Update(req *http.Request, context genericapireq
 
 type mockedRouter struct {
 	destinationHost string
+	err             error
 }
 
 func (r *mockedRouter) ResolveEndpoint(namespace, name string) (*url.URL, error) {
-	return &url.URL{
-		Scheme: "https",
-		Host:   r.destinationHost,
-	}, nil
+	return &url.URL{Scheme: "https", Host: r.destinationHost}, r.err
 }
 
 func TestProxyHandler(t *testing.T) {
@@ -109,6 +108,8 @@ func TestProxyHandler(t *testing.T) {
 		path       string
 		apiService *apiregistration.APIService
 
+		serviceResolver ServiceResolver
+
 		expectedStatusCode int
 		expectedBody       string
 		expectedCalled     bool
@@ -220,6 +221,29 @@ func TestProxyHandler(t *testing.T) {
 			},
 			expectedStatusCode: http.StatusServiceUnavailable,
 		},
+		"service unresolveable": {
+			user: &user.DefaultInfo{
+				Name:   "username",
+				Groups: []string{"one", "two"},
+			},
+			path:            "/request/path",
+			serviceResolver: &mockedRouter{err: fmt.Errorf("unresolveable")},
+			apiService: &apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"},
+				Spec: apiregistration.APIServiceSpec{
+					Service:  &apiregistration.ServiceReference{Name: "bad-service", Namespace: "test-ns"},
+					Group:    "foo",
+					Version:  "v1",
+					CABundle: testCACrt,
+				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
+					},
+				},
+			},
+			expectedStatusCode: http.StatusServiceUnavailable,
+		},
 		"fail on bad serving cert": {
 			user: &user.DefaultInfo{
 				Name:   "username",
@@ -247,9 +271,13 @@ func TestProxyHandler(t *testing.T) {
 		target.Reset()
 
 		func() {
+			serviceResolver := tc.serviceResolver
+			if serviceResolver == nil {
+				serviceResolver = &mockedRouter{destinationHost: targetServer.Listener.Addr().String()}
+			}
 			handler := &proxyHandler{
 				localDelegate:   http.NewServeMux(),
-				serviceResolver: &mockedRouter{destinationHost: targetServer.Listener.Addr().String()},
+				serviceResolver: serviceResolver,
 				proxyTransport:  &http.Transport{},
 			}
 			handler.contextMapper = &fakeRequestContextMapper{user: tc.user}