diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 0bde466ae69..2cfdb8984e9 100644 --- a/pkg/apiserver/apiserver.go +++ b/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 } diff --git a/pkg/apiserver/errors.go b/pkg/apiserver/errors.go index 14a11501d1f..6b6cffee5b0 100644 --- a/pkg/apiserver/errors.go +++ b/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) +} diff --git a/pkg/apiserver/minionproxy.go b/pkg/apiserver/minionproxy.go index 87e3fc4bbab..b85e0ccc8ef 100644 --- a/pkg/apiserver/minionproxy.go +++ b/pkg/apiserver/minionproxy.go @@ -31,14 +31,8 @@ import ( "github.com/golang/glog" ) -func (server *APIServer) handleProxyMinion(w http.ResponseWriter, req *http.Request) { - minionPrefix := "/proxy/minion/" - if !strings.HasPrefix(req.URL.Path, minionPrefix) { - notFound(w, req) - return - } - - path := req.URL.Path[len(minionPrefix):] +func handleProxyMinion(w http.ResponseWriter, req *http.Request) { + path := strings.TrimLeft(req.URL.Path, "/") 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, "/") - minionHost := path[:idx] + parts := strings.SplitN(path, "/", 2) + 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") { - message := fmt.Sprintf("Failed to connect to minion:%s", req.URL.Host) - resp = &http.Response{ - StatusCode: http.StatusServiceUnavailable, - Body: ioutil.NopCloser(strings.NewReader(message)), + if err != nil { + if strings.Contains(err.Error(), "connection refused") { + message := fmt.Sprintf("Failed to connect to minion:%s", req.URL.Host) + resp = &http.Response{ + 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") { diff --git a/pkg/apiserver/minionproxy_test.go b/pkg/apiserver/minionproxy_test.go index e7bed930f63..c58990c59eb 100644 --- a/pkg/apiserver/minionproxy_test.go +++ b/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) + } +}