Fix proxy error condition and simplify method

Add new minion proxy test case.

pkg/apiserver/apiserver.go

@@ -75,7 +75,7 @@ func New(storage map[string]RESTStorage, prefix string) *APIServer {
 	s.mux.HandleFunc(s.operationPrefix()+"/", s.handleOperation)
 
 	// Proxy minion requests
-	s.mux.HandleFunc("/proxy/minion/", s.handleProxyMinion)
+	s.mux.Handle("/proxy/minion/", http.StripPrefix("/proxy/minion", http.HandlerFunc(handleProxyMinion)))
 
 	return s
 }

pkg/apiserver/errors.go

@@ -70,3 +70,9 @@ func notFound(w http.ResponseWriter, req *http.Request) {
 	w.WriteHeader(http.StatusNotFound)
 	fmt.Fprintf(w, "Not Found: %#v", req.RequestURI)
 }
+
+// badGatewayError renders a simple bad gateway error
+func badGatewayError(w http.ResponseWriter, req *http.Request) {
+	w.WriteHeader(http.StatusBadGateway)
+	fmt.Fprintf(w, "Bad Gateway: %#v", req.RequestURI)
+}

pkg/apiserver/minionproxy.go

@@ -31,14 +31,8 @@ import (
 	"github.com/golang/glog"
 )
 
-func (server *APIServer) handleProxyMinion(w http.ResponseWriter, req *http.Request) {
+func handleProxyMinion(w http.ResponseWriter, req *http.Request) {
-	minionPrefix := "/proxy/minion/"
+	path := strings.TrimLeft(req.URL.Path, "/")
-	if !strings.HasPrefix(req.URL.Path, minionPrefix) {
-		notFound(w, req)
-		return
-	}
-
-	path := req.URL.Path[len(minionPrefix):]
 	rawQuery := req.URL.RawQuery
 
 	// Expect path as: ${minion}/${query_to_minion}
@@ -51,15 +45,19 @@ func (server *APIServer) handleProxyMinion(w http.ResponseWriter, req *http.Requ
 	//
 	// To query logs on a minion, path string can be:
 	// ${minion}/logs/
-	idx := strings.Index(path, "/")
+	parts := strings.SplitN(path, "/", 2)
-	minionHost := path[:idx]
+	if len(parts) != 2 {
+		badGatewayError(w, req)
+		return
+	}
+	minionHost := parts[0]
 	_, port, _ := net.SplitHostPort(minionHost)
 	if port == "" {
 		// Couldn't retrieve port information
 		// TODO: Retrieve port info from a common object
 		minionHost += ":10250"
 	}
-	minionPath := path[idx:]
+	minionPath := "/" + parts[1]
 
 	minionURL := &url.URL{
 		Scheme: "http",
@@ -80,13 +78,16 @@ type minionTransport struct{}
 func (t *minionTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	resp, err := http.DefaultTransport.RoundTrip(req)
 
-	if err != nil && strings.Contains(err.Error(), "connection refused") {
+	if err != nil {
-		message := fmt.Sprintf("Failed to connect to minion:%s", req.URL.Host)
+		if strings.Contains(err.Error(), "connection refused") {
-		resp = &http.Response{
+			message := fmt.Sprintf("Failed to connect to minion:%s", req.URL.Host)
-			StatusCode: http.StatusServiceUnavailable,
+			resp = &http.Response{
-			Body:       ioutil.NopCloser(strings.NewReader(message)),
+				StatusCode: http.StatusServiceUnavailable,
+				Body:       ioutil.NopCloser(strings.NewReader(message)),
+			}
+			return resp, nil
 		}
-		return resp, nil
+		return nil, err
 	}
 
 	if strings.Contains(resp.Header.Get("Content-Type"), "text/plain") {

pkg/apiserver/minionproxy_test.go

@@ -17,8 +17,11 @@ limitations under the License.
 package apiserver
 
 import (
+	"bufio"
+	"fmt"
 	"io/ioutil"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"strings"
 	"testing"
@@ -72,3 +75,70 @@ func TestMinionTransport(t *testing.T) {
 		t.Errorf("Received wrong content: %s", string(body))
 	}
 }
+
+func TestMinionProxy(t *testing.T) {
+	proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte(req.URL.Path))
+	}))
+	server := httptest.NewServer(http.HandlerFunc(handleProxyMinion))
+	//client := http.Client{}
+	proxy, _ := url.Parse(proxyServer.URL)
+
+	testCases := map[string]string{
+		fmt.Sprintf("/%s/", proxy.Host):     "/",
+		fmt.Sprintf("/%s/test", proxy.Host): "/test",
+	}
+
+	for value, expected := range testCases {
+		resp, err := http.Get(fmt.Sprintf("%s%s", server.URL, value))
+		if err != nil {
+			t.Errorf("unexpected error for %s: %v", value, err)
+			continue
+		}
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("expected successful request for %s: %#v", value, resp)
+			continue
+		}
+		defer resp.Body.Close()
+		actual, _ := bufio.NewReader(resp.Body).ReadString('\n')
+		if actual != expected {
+			t.Errorf("expected %s to become %s, got %s", value, expected, actual)
+		}
+	}
+
+	failureCases := map[string]string{
+		"": "",
+		fmt.Sprintf("/%s", proxy.Host): "/",
+	}
+
+	for value, _ := range failureCases {
+		resp, err := http.Get(fmt.Sprintf("%s%s", server.URL, value))
+		if err != nil {
+			t.Errorf("unexpected error for %s: %v", value, err)
+			continue
+		}
+		if resp.StatusCode != http.StatusBadGateway {
+			t.Errorf("expected bad gateway response for %s: %#v", value, resp)
+		}
+	}
+}
+
+func TestApiServerMinionProxy(t *testing.T) {
+	proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte(req.URL.Path))
+	}))
+	server := httptest.NewServer(New(nil, "/prefix"))
+	proxy, _ := url.Parse(proxyServer.URL)
+	resp, err := http.Get(fmt.Sprintf("%s/proxy/minion/%s%s", server.URL, proxy.Host, "/test"))
+	if err != nil {
+		t.Fatalf("unexpected error %v", err)
+	}
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("expected successful request, got %#v", resp)
+	}
+	defer resp.Body.Close()
+	actual, _ := bufio.NewReader(resp.Body).ReadString('\n')
+	if actual != "/test" {
+		t.Errorf("unexpected response body %s", actual)
+	}
+}