From 7870b87c38c11f6dd886d72578da801d239859ae Mon Sep 17 00:00:00 2001 From: Thomas E Lackey Date: Tue, 24 Jul 2018 23:37:24 -0500 Subject: [PATCH] Implement proposal 716, passing full paths through the Gateway and fwatchdog. Previously, only the query string of the URL was passed through the Gateway. With this change, the entire path requested by the client is passed through as well as the query string. While fwatchdog already supported passing the path through, in practice this would not happen since the Gateway would have swallowed it before forwarding the request to the watchdog. With this change, the path portion after the function name is added to the Http_Path environment variable, provided that cgiHeaders are enabled. This is similar to the of-watchdog equivalent. Signed-off-by: Thomas E Lackey --- gateway/handlers/forwarding_proxy.go | 36 +++++++++++++++-- gateway/handlers/forwarding_proxy_test.go | 48 ++++++++++++++++++++++- gateway/server.go | 2 + 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/gateway/handlers/forwarding_proxy.go b/gateway/handlers/forwarding_proxy.go index 3ec96b90..63db766d 100644 --- a/gateway/handlers/forwarding_proxy.go +++ b/gateway/handlers/forwarding_proxy.go @@ -6,6 +6,7 @@ import ( "io" "log" "net/http" + "regexp" "strconv" "strings" "time" @@ -15,6 +16,9 @@ import ( "github.com/prometheus/client_golang/prometheus" ) +// Parse out the service name (group 1) and rest of path (group 2). +var functionMatcher = regexp.MustCompile("^/?function/([^/?]+)([^?]*)") + // HTTPNotifier notify about HTTP request/response type HTTPNotifier interface { Notify(method string, URL string, statusCode int, duration time.Duration) @@ -46,7 +50,24 @@ func MakeForwardingProxyHandler(proxy *types.HTTPClientReverseProxy, notifiers [ } } -func buildUpstreamRequest(r *http.Request, url string) *http.Request { +func buildUpstreamRequest(r *http.Request, baseURL string, requestURL string) *http.Request { + url := baseURL + + if requestURL != "" { + // When forwarding to a function, since the `/function/xyz` portion + // of a path like `/function/xyz/rest/of/path` is only used or needed + // by the Gateway, we want to trim it down to `/rest/of/path` for the + // upstream request. In the following regex, in the case of a match + // the requestURL will be at `0`, the function name at `1` and the + // rest of the path (the part we are interested in) at `2`. + matcher := functionMatcher.Copy() + parts := matcher.FindStringSubmatch(requestURL) + if 3 == len(parts) { + url += parts[2] + } else { + url += requestURL + } + } if len(r.URL.RawQuery) > 0 { url = fmt.Sprintf("%s?%s", url, r.URL.RawQuery) @@ -69,7 +90,7 @@ func buildUpstreamRequest(r *http.Request, url string) *http.Request { func forwardRequest(w http.ResponseWriter, r *http.Request, proxyClient *http.Client, baseURL string, requestURL string, timeout time.Duration) (int, error) { - upstreamReq := buildUpstreamRequest(r, baseURL+requestURL) + upstreamReq := buildUpstreamRequest(r, baseURL, requestURL) if upstreamReq.Body != nil { defer upstreamReq.Body.Close() } @@ -134,7 +155,16 @@ func getServiceName(urlValue string) string { var serviceName string forward := "/function/" if strings.HasPrefix(urlValue, forward) { - serviceName = urlValue[len(forward):] + // With a path like `/function/xyz/rest/of/path?q=a`, the service + // name we wish to locate is just the `xyz` portion. With a postive + // match on the regex below, it will return a three-element slice. + // The item at index `0` is the same as `urlValue`, at `1` + // will be the service name we need, and at `2` the rest of the path. + matcher := functionMatcher.Copy() + matches := matcher.FindStringSubmatch(urlValue) + if 3 == len(matches) { + serviceName = matches[1] + } } return strings.Trim(serviceName, "/") } diff --git a/gateway/handlers/forwarding_proxy_test.go b/gateway/handlers/forwarding_proxy_test.go index 4c314cfd..e973a5c8 100644 --- a/gateway/handlers/forwarding_proxy_test.go +++ b/gateway/handlers/forwarding_proxy_test.go @@ -20,7 +20,7 @@ func Test_buildUpstreamRequest_Body_Method_Query(t *testing.T) { t.Fail() } - upstream := buildUpstreamRequest(request, "/") + upstream := buildUpstreamRequest(request, "/", "") if request.Method != upstream.Method { t.Errorf("Method - want: %s, got: %s", request.Method, upstream.Method) @@ -49,7 +49,7 @@ func Test_buildUpstreamRequest_Body_Method_Query(t *testing.T) { func Test_buildUpstreamRequest_NoBody_GetMethod_NoQuery(t *testing.T) { request, _ := http.NewRequest(http.MethodGet, "/", nil) - upstream := buildUpstreamRequest(request, "/") + upstream := buildUpstreamRequest(request, "/", "") if request.Method != upstream.Method { t.Errorf("Method - want: %s, got: %s", request.Method, upstream.Method) @@ -150,3 +150,47 @@ func Test_getServiceName(t *testing.T) { }) } } + +func Test_buildUpstreamRequest_Body_Method_Query_Path(t *testing.T) { + srcBytes := []byte("hello world") + path := "/my/deep/path" + + reader := bytes.NewReader(srcBytes) + request, _ := http.NewRequest(http.MethodPost, "/function/xyz"+path+"?code=1", reader) + request.Header.Set("X-Source", "unit-test") + + if request.URL.RawQuery != "code=1" { + t.Errorf("Query - want: %s, got: %s", "code=1", request.URL.RawQuery) + t.Fail() + } + + upstream := buildUpstreamRequest(request, "http://xyz:8080", request.URL.Path) + + if request.Method != upstream.Method { + t.Errorf("Method - want: %s, got: %s", request.Method, upstream.Method) + t.Fail() + } + + upstreamBytes, _ := ioutil.ReadAll(upstream.Body) + + if string(upstreamBytes) != string(srcBytes) { + t.Errorf("Body - want: %s, got: %s", string(upstreamBytes), string(srcBytes)) + t.Fail() + } + + if request.Header.Get("X-Source") != upstream.Header.Get("X-Source") { + t.Errorf("Header X-Source - want: %s, got: %s", request.Header.Get("X-Source"), upstream.Header.Get("X-Source")) + t.Fail() + } + + if request.URL.RawQuery != upstream.URL.RawQuery { + t.Errorf("URL.RawQuery - want: %s, got: %s", request.URL.RawQuery, upstream.URL.RawQuery) + t.Fail() + } + + if path != upstream.URL.Path { + t.Errorf("URL.Path - want: %s, got: %s", path, upstream.URL.Path) + t.Fail() + } + +} diff --git a/gateway/server.go b/gateway/server.go index af5f297b..40c066a5 100644 --- a/gateway/server.go +++ b/gateway/server.go @@ -135,6 +135,7 @@ func main() { // r.StrictSlash(false) // This didn't work, so register routes twice. r.HandleFunc("/function/{name:[-a-zA-Z_0-9]+}", functionProxy) r.HandleFunc("/function/{name:[-a-zA-Z_0-9]+}/", functionProxy) + r.HandleFunc("/function/{name:[-a-zA-Z_0-9]+}/{params:.*}", faasHandlers.Proxy) r.HandleFunc("/system/info", handlers.MakeInfoHandler(handlers.MakeForwardingProxyHandler( reverseProxy, forwardingNotifiers, urlResolver))).Methods(http.MethodGet) @@ -151,6 +152,7 @@ func main() { if faasHandlers.QueuedProxy != nil { r.HandleFunc("/async-function/{name:[-a-zA-Z_0-9]+}/", faasHandlers.QueuedProxy).Methods(http.MethodPost) r.HandleFunc("/async-function/{name:[-a-zA-Z_0-9]+}", faasHandlers.QueuedProxy).Methods(http.MethodPost) + r.HandleFunc("/async-function/{name:[-a-zA-Z_0-9]+}/{params:.*}", faasHandlers.QueuedProxy).Methods(http.MethodPost) r.HandleFunc("/system/async-report", faasHandlers.AsyncReport) }