Review changes for HTTP paths

This reviews the code and fixes up suggestions made by team for
the HTTP paths PR #789.

- Removed feature-flag (this is backwards-compatible, so I see
no value in adding the flag)
- There was a URL transform happening for calls proxied to the
back end, I changed this for the nil-transform - i.e. it does not
change anything in the URL
- Introduced variables to describe the regex indicies used in
the URL trimming.

Tested with Docker Swarm with a ruby-microservice, with
system calls and with function calls using the UI.

Signed-off-by: Alex Ellis (VMware) <alexellis2@gmail.com>

gateway/build.sh

@@ -19,7 +19,9 @@ if [ "$1" ] ; then
   fi
 fi
 
-echo Building openfaas/gateway:$eTAG
+NS=openfaas
+
+echo Building $NS/gateway:$eTAG
 
 GIT_COMMIT_MESSAGE=$(git log -1 --pretty=%B 2>&1 | head -n 1)
 GIT_COMMIT_SHA=$(git rev-list -1 HEAD)
@@ -28,4 +30,4 @@ VERSION=$(git describe --all --exact-match `git rev-parse HEAD` | grep tags | se
 docker build --build-arg https_proxy=$https_proxy --build-arg http_proxy=$http_proxy \
   --build-arg GIT_COMMIT_MESSAGE="$GIT_COMMIT_MESSAGE" --build-arg GIT_COMMIT_SHA=$GIT_COMMIT_SHA \
   --build-arg VERSION=${VERSION:-dev} \
-  -t openfaas/gateway:$eTAG . -f $dockerfile --no-cache
+  -t $NS/gateway:$eTAG . -f $dockerfile --no-cache

gateway/handlers/forwarding_proxy.go

@@ -19,6 +19,14 @@ import (
 // Parse out the service name (group 1) and rest of path (group 2).
 var functionMatcher = regexp.MustCompile("^/?(?:async-)?function/([^/?]+)([^?]*)")
 
+// Indicies and meta-data for functionMatcher regex parts
+const (
+	hasPathCount = 3
+	routeIndex   = 0
+	nameIndex    = 1
+	pathIndex    = 2
+)
+
 // HTTPNotifier notify about HTTP request/response
 type HTTPNotifier interface {
 	Notify(method string, URL string, statusCode int, duration time.Duration)
@@ -29,7 +37,7 @@ type BaseURLResolver interface {
 	Resolve(r *http.Request) string
 }
 
-// RequestURLPathTransformer Transform the incoming URL path for upstream requests
+// URLPathTransformer Transform the incoming URL path for upstream requests
 type URLPathTransformer interface {
 	Transform(r *http.Request) string
 }
@@ -151,8 +159,8 @@ func getServiceName(urlValue string) string {
 		// 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]
+		if len(matches) == hasPathCount {
+			serviceName = matches[nameIndex]
 		}
 	}
 	return strings.Trim(serviceName, "/")
@@ -224,7 +232,7 @@ func (f FunctionPathTruncatingURLPathTransformer) Transform(r *http.Request) str
 		// In the following regex, in the case of a match the r.URL.Path will be at `0`,
 		// the function name at `1` and the rest of the path (the part we are interested in)
 		// at `2`.  For this transformer, all we need to do is confirm it is a function.
-		if 3 == len(parts) {
+		if len(parts) == hasPathCount {
 			ret = "/"
 		}
 	}
@@ -249,8 +257,8 @@ func (f FunctionPrefixTrimmingURLPathTransformer) Transform(r *http.Request) str
 		// rest of the path (the part we are interested in) at `2`.
 		matcher := functionMatcher.Copy()
 		parts := matcher.FindStringSubmatch(ret)
-		if 3 == len(parts) {
-			ret = parts[2]
+		if len(parts) == hasPathCount {
+			ret = parts[pathIndex]
 		}
 	}
 

gateway/handlers/forwarding_proxy_test.go

@@ -78,7 +78,7 @@ func Test_buildUpstreamRequest_HasHostHeaderWhenSet(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	upstream := buildUpstreamRequest(request, "/")
+	upstream := buildUpstreamRequest(request, "/", "/")
 
 	if request.Host != upstream.Host {
 		t.Errorf("Host - want: %s, got: %s", request.Host, upstream.Host)
@@ -95,7 +95,7 @@ func Test_buildUpstreamRequest_HostHeader_Empty_WhenNotSet(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	upstream := buildUpstreamRequest(request, "/")
+	upstream := buildUpstreamRequest(request, "/", "/")
 
 	if request.Host != upstream.Host {
 		t.Errorf("Host - want: %s, got: %s", request.Host, upstream.Host)

gateway/server.go

@@ -74,23 +74,17 @@ func main() {
 		functionURLResolver = urlResolver
 	}
 
-	urlTransformer := handlers.FunctionPathTruncatingURLPathTransformer{}
-	var functionURLTransformer handlers.URLPathTransformer
-
-	if config.PassURLPathsToFunctions {
-		functionURLTransformer = handlers.FunctionPrefixTrimmingURLPathTransformer{}
-	} else {
-		functionURLTransformer = urlTransformer
-	}
+	nilURLTransformer := handlers.TransparentURLPathTransformer{}
+	functionURLTransformer := handlers.FunctionPrefixTrimmingURLPathTransformer{}
 
 	faasHandlers.Proxy = handlers.MakeForwardingProxyHandler(reverseProxy, functionNotifiers, functionURLResolver, functionURLTransformer)
 
-	faasHandlers.RoutelessProxy = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, urlTransformer)
-	faasHandlers.ListFunctions = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, urlTransformer)
-	faasHandlers.DeployFunction = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, urlTransformer)
-	faasHandlers.DeleteFunction = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, urlTransformer)
-	faasHandlers.UpdateFunction = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, urlTransformer)
-	queryFunction := handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, urlTransformer)
+	faasHandlers.RoutelessProxy = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer)
+	faasHandlers.ListFunctions = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer)
+	faasHandlers.DeployFunction = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer)
+	faasHandlers.DeleteFunction = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer)
+	faasHandlers.UpdateFunction = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer)
+	queryFunction := handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer)
 
 	alertHandler := plugin.NewExternalServiceQuery(*config.FunctionsProviderURL)
 	faasHandlers.Alert = handlers.MakeAlertHandler(alertHandler)
@@ -110,7 +104,7 @@ func main() {
 	faasHandlers.ListFunctions = metrics.AddMetricsHandler(faasHandlers.ListFunctions, prometheusQuery)
 	faasHandlers.Proxy = handlers.MakeCallIDMiddleware(faasHandlers.Proxy)
 
-	faasHandlers.ScaleFunction = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, urlTransformer)
+	faasHandlers.ScaleFunction = handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer)
 
 	if credentials != nil {
 		faasHandlers.UpdateFunction =
@@ -123,7 +117,6 @@ func main() {
 			handlers.DecorateWithBasicAuth(faasHandlers.ListFunctions, credentials)
 		faasHandlers.ScaleFunction =
 			handlers.DecorateWithBasicAuth(faasHandlers.ScaleFunction, credentials)
-
 	}
 
 	r := mux.NewRouter()
@@ -147,7 +140,7 @@ func main() {
 	r.HandleFunc("/function/{name:[-a-zA-Z_0-9]+}/{params:.*}", functionProxy)
 
 	r.HandleFunc("/system/info", handlers.MakeInfoHandler(handlers.MakeForwardingProxyHandler(
-		reverseProxy, forwardingNotifiers, urlResolver, urlTransformer))).Methods(http.MethodGet)
+		reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer))).Methods(http.MethodGet)
 
 	r.HandleFunc("/system/alert", faasHandlers.Alert)
 
@@ -181,7 +174,7 @@ func main() {
 
 	metricsHandler := metrics.PrometheusHandler()
 	r.Handle("/metrics", metricsHandler)
-	r.HandleFunc("/healthz", handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, urlTransformer)).Methods(http.MethodGet)
+	r.HandleFunc("/healthz", handlers.MakeForwardingProxyHandler(reverseProxy, forwardingNotifiers, urlResolver, nilURLTransformer)).Methods(http.MethodGet)
 
 	r.Handle("/", http.RedirectHandler("/ui/", http.StatusMovedPermanently)).Methods(http.MethodGet)
 

gateway/types/readconfig.go

@@ -105,8 +105,6 @@ func (ReadConfig) Read(hasEnv HasEnv) GatewayConfig {
 	cfg.DirectFunctions = parseBoolValue(hasEnv.Getenv("direct_functions"))
 	cfg.DirectFunctionsSuffix = hasEnv.Getenv("direct_functions_suffix")
 
-	cfg.PassURLPathsToFunctions = parseBoolValue(hasEnv.Getenv("pass_url_path_to_functions"))
-
 	cfg.UseBasicAuth = parseBoolValue(hasEnv.Getenv("basic_auth"))
 
 	secretPath := hasEnv.Getenv("secret_mount_path")
@@ -152,10 +150,6 @@ type GatewayConfig struct {
 	// If set this will be used to resolve functions directly
 	DirectFunctionsSuffix string
 
-	// If set to true, the requested path will be passed along to the function, minus the "/function/xyz"
-	// prefix, else the path will be truncated to "/" regardless of what the client sends.
-	PassURLPathsToFunctions bool
-
 	// If set, reads secrets from file-system for enabling basic auth.
 	UseBasicAuth bool