From 2d56547524a14829f080893ce4f5d58fe8b91cfb Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 23 Feb 2015 16:24:10 -0800 Subject: [PATCH 1/3] refactor proxy upgrade path --- pkg/apiserver/proxy.go | 123 ++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 57 deletions(-) diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go index bfea64eea93..06a6e8caf8d 100644 --- a/pkg/apiserver/proxy.go +++ b/pkg/apiserver/proxy.go @@ -182,64 +182,73 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { // TODO convert this entire proxy to an UpgradeAwareProxy similar to // https://github.com/openshift/origin/blob/master/pkg/util/httpproxy/upgradeawareproxy.go. // That proxy needs to be modified to support multiple backends, not just 1. - connectionHeader := strings.ToLower(req.Header.Get(httpstream.HeaderConnection)) - if strings.Contains(connectionHeader, strings.ToLower(httpstream.HeaderUpgrade)) && len(req.Header.Get(httpstream.HeaderUpgrade)) > 0 { - //TODO support TLS? Doesn't look like proxyTransport does anything special ... - dialAddr := netutil.CanonicalAddr(destURL) - backendConn, err := net.Dial("tcp", dialAddr) - if err != nil { - status := errToAPIStatus(err) - writeJSON(status.Code, r.codec, status, w) - return - } - defer backendConn.Close() - - // TODO should we use _ (a bufio.ReadWriter) instead of requestHijackedConn - // when copying between the client and the backend? Docker doesn't when they - // hijack, just for reference... - requestHijackedConn, _, err := w.(http.Hijacker).Hijack() - if err != nil { - status := errToAPIStatus(err) - writeJSON(status.Code, r.codec, status, w) - return - } - defer requestHijackedConn.Close() - - if err = newReq.Write(backendConn); err != nil { - status := errToAPIStatus(err) - writeJSON(status.Code, r.codec, status, w) - return - } - - done := make(chan struct{}, 2) - - go func() { - _, err := io.Copy(backendConn, requestHijackedConn) - if err != nil && !strings.Contains(err.Error(), "use of closed network connection") { - glog.Errorf("Error proxying data from client to backend: %v", err) - } - done <- struct{}{} - }() - - go func() { - _, err := io.Copy(requestHijackedConn, backendConn) - if err != nil && !strings.Contains(err.Error(), "use of closed network connection") { - glog.Errorf("Error proxying data from backend to client: %v", err) - } - done <- struct{}{} - }() - - <-done - } else { - proxy := httputil.NewSingleHostReverseProxy(&url.URL{Scheme: "http", Host: destURL.Host}) - proxy.Transport = &proxyTransport{ - proxyScheme: req.URL.Scheme, - proxyHost: req.URL.Host, - proxyPathPrepend: path.Join(r.prefix, "ns", namespace, resource, id), - } - proxy.FlushInterval = 200 * time.Millisecond - proxy.ServeHTTP(w, newReq) + if r.tryUpgrade(w, req, newReq, destURL) { + return } + + proxy := httputil.NewSingleHostReverseProxy(&url.URL{Scheme: "http", Host: destURL.Host}) + proxy.Transport = &proxyTransport{ + proxyScheme: req.URL.Scheme, + proxyHost: req.URL.Host, + proxyPathPrepend: path.Join(r.prefix, "ns", namespace, resource, id), + } + proxy.FlushInterval = 200 * time.Millisecond + proxy.ServeHTTP(w, newReq) +} + +// tryUpgrade returns true if the request was handled. +func (r *ProxyHandler) tryUpgrade(w http.ResponseWriter, req, newReq *http.Request, destURL *url.URL) bool { + connectionHeader := strings.ToLower(req.Header.Get(httpstream.HeaderConnection)) + if !strings.Contains(connectionHeader, strings.ToLower(httpstream.HeaderUpgrade)) || len(req.Header.Get(httpstream.HeaderUpgrade)) == 0 { + return false + } + //TODO support TLS? Doesn't look like proxyTransport does anything special ... + dialAddr := netutil.CanonicalAddr(destURL) + backendConn, err := net.Dial("tcp", dialAddr) + if err != nil { + status := errToAPIStatus(err) + writeJSON(status.Code, r.codec, status, w) + return true + } + defer backendConn.Close() + + // TODO should we use _ (a bufio.ReadWriter) instead of requestHijackedConn + // when copying between the client and the backend? Docker doesn't when they + // hijack, just for reference... + requestHijackedConn, _, err := w.(http.Hijacker).Hijack() + if err != nil { + status := errToAPIStatus(err) + writeJSON(status.Code, r.codec, status, w) + return true + } + defer requestHijackedConn.Close() + + if err = newReq.Write(backendConn); err != nil { + status := errToAPIStatus(err) + writeJSON(status.Code, r.codec, status, w) + return true + } + + done := make(chan struct{}, 2) + + go func() { + _, err := io.Copy(backendConn, requestHijackedConn) + if err != nil && !strings.Contains(err.Error(), "use of closed network connection") { + glog.Errorf("Error proxying data from client to backend: %v", err) + } + done <- struct{}{} + }() + + go func() { + _, err := io.Copy(requestHijackedConn, backendConn) + if err != nil && !strings.Contains(err.Error(), "use of closed network connection") { + glog.Errorf("Error proxying data from backend to client: %v", err) + } + done <- struct{}{} + }() + + <-done + return true } type proxyTransport struct { From 78d05e5307496df7b3f93f7f88864bb703d626db Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 23 Feb 2015 17:17:39 -0800 Subject: [PATCH 2/3] add URL path generation function --- pkg/apiserver/handlers.go | 25 ++++++++++++++++++++++--- pkg/apiserver/handlers_test.go | 4 ++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/pkg/apiserver/handlers.go b/pkg/apiserver/handlers.go index 04d05b2d714..a85e2f0ecbf 100644 --- a/pkg/apiserver/handlers.go +++ b/pkg/apiserver/handlers.go @@ -19,6 +19,7 @@ package apiserver import ( "fmt" "net/http" + "path" "regexp" "runtime/debug" "strings" @@ -214,8 +215,22 @@ type APIRequestInfo struct { Kind string // Name is empty for some verbs, but if the request directly indicates a name (not in body content) then this field is filled in. Name string - // Parts are the path parts for the request relative to /{resource}/{name} + // Parts are the path parts for the request, always starting with /{resource}/{name} Parts []string + // Raw is the unparsed form of everything other than parts. + // Raw + Parts = complete URL path + Raw []string +} + +// URLPath returns the URL path for this request, including /{resource}/{name} if present but nothing +// following that. +func (info APIRequestInfo) URLPath() string { + p := info.Parts + if n := len(p); n > 2 { + // Only take resource and name + p = p[:2] + } + return path.Join("/", path.Join(info.Raw...), path.Join(p...)) } type APIRequestInfoResolver struct { @@ -247,9 +262,11 @@ type APIRequestInfoResolver struct { // /api/{version}/* // /api/{version}/* func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIRequestInfo, error) { - requestInfo := APIRequestInfo{} + requestInfo := APIRequestInfo{ + Raw: splitPath(req.URL.Path), + } - currentParts := splitPath(req.URL.Path) + currentParts := requestInfo.Raw if len(currentParts) < 1 { return requestInfo, fmt.Errorf("Unable to determine kind and namespace from an empty URL path") } @@ -322,6 +339,8 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques // parsing successful, so we now know the proper value for .Parts requestInfo.Parts = currentParts + // Raw should have everything not in Parts + requestInfo.Raw = requestInfo.Raw[:len(requestInfo.Raw)-len(currentParts)] // if there's another part remaining after the kind, then that's the resource name if len(requestInfo.Parts) >= 2 { diff --git a/pkg/apiserver/handlers_test.go b/pkg/apiserver/handlers_test.go index 59509c9699f..8ff475b24c6 100644 --- a/pkg/apiserver/handlers_test.go +++ b/pkg/apiserver/handlers_test.go @@ -20,6 +20,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "strings" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -142,6 +143,9 @@ func TestGetAPIRequestInfo(t *testing.T) { if !reflect.DeepEqual(successCase.expectedParts, apiRequestInfo.Parts) { t.Errorf("Unexpected parts for url: %s, expected: %v, actual: %v", successCase.url, successCase.expectedParts, apiRequestInfo.Parts) } + if e, a := strings.Split(successCase.url, "?")[0], apiRequestInfo.URLPath(); e != a { + t.Errorf("Expected %v, got %v", e, a) + } } errorCases := map[string]string{ From ec58e6d78ea2e32de9640ac593d0e0bbfdd118ad Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 23 Feb 2015 17:19:44 -0800 Subject: [PATCH 3/3] use correct path for proxy replacements --- pkg/apiserver/proxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go index 06a6e8caf8d..09a4c7b9151 100644 --- a/pkg/apiserver/proxy.go +++ b/pkg/apiserver/proxy.go @@ -190,7 +190,7 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { proxy.Transport = &proxyTransport{ proxyScheme: req.URL.Scheme, proxyHost: req.URL.Host, - proxyPathPrepend: path.Join(r.prefix, "ns", namespace, resource, id), + proxyPathPrepend: requestInfo.URLPath(), } proxy.FlushInterval = 200 * time.Millisecond proxy.ServeHTTP(w, newReq)