Middleware upgrade (#554)

* Adds root level middleware

* Added todo

* Better way for extensions to be added.

* Bad conflict merge?

api/server/app_listeners.go

@@ -3,12 +3,12 @@ package server
 import (
 	"context"
 
-	"github.com/fnproject/fn/api/extensions"
 	"github.com/fnproject/fn/api/models"
+	"github.com/fnproject/fn/fnext"
 )
 
 // AddAppListener adds a listener that will be notified on App created.
-func (s *Server) AddAppListener(listener extensions.AppListener) {
+func (s *Server) AddAppListener(listener fnext.AppListener) {
 	s.appListeners = append(s.appListeners, listener)
 }
 

api/server/extension_points.go

@@ -6,40 +6,17 @@ import (
 
 	"github.com/fnproject/fn/api"
 	"github.com/fnproject/fn/api/models"
+	"github.com/fnproject/fn/fnext"
 	"github.com/gin-gonic/gin"
 )
 
-type ApiHandlerFunc func(w http.ResponseWriter, r *http.Request)
-
-// ServeHTTP calls f(w, r).
-func (f ApiHandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	f(w, r)
-}
-
-type ApiHandler interface {
-	// Handle(ctx context.Context)
-	ServeHTTP(w http.ResponseWriter, r *http.Request)
-}
-
-type ApiAppHandler interface {
-	// Handle(ctx context.Context)
-	ServeHTTP(w http.ResponseWriter, r *http.Request, app *models.App)
-}
-
-type ApiAppHandlerFunc func(w http.ResponseWriter, r *http.Request, app *models.App)
-
-// ServeHTTP calls f(w, r).
-func (f ApiAppHandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request, app *models.App) {
-	f(w, r, app)
-}
-
-func (s *Server) apiHandlerWrapperFunc(apiHandler ApiHandler) gin.HandlerFunc {
+func (s *Server) apiHandlerWrapperFunc(apiHandler fnext.ApiHandler) gin.HandlerFunc {
 	return func(c *gin.Context) {
 		apiHandler.ServeHTTP(c.Writer, c.Request)
 	}
 }
 
-func (s *Server) apiAppHandlerWrapperFunc(apiHandler ApiAppHandler) gin.HandlerFunc {
+func (s *Server) apiAppHandlerWrapperFunc(apiHandler fnext.ApiAppHandler) gin.HandlerFunc {
 	return func(c *gin.Context) {
 		// get the app
 		appName := c.Param(api.CApp)
@@ -59,21 +36,7 @@ func (s *Server) apiAppHandlerWrapperFunc(apiHandler ApiAppHandler) gin.HandlerF
 	}
 }
 
-// per Route
-
-type ApiRouteHandler interface {
-	// Handle(ctx context.Context)
-	ServeHTTP(w http.ResponseWriter, r *http.Request, app *models.App, route *models.Route)
-}
-
-type ApiRouteHandlerFunc func(w http.ResponseWriter, r *http.Request, app *models.App, route *models.Route)
-
-// ServeHTTP calls f(w, r).
-func (f ApiRouteHandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request, app *models.App, route *models.Route) {
-	f(w, r, app, route)
-}
-
-func (s *Server) apiRouteHandlerWrapperFunc(apiHandler ApiRouteHandler) gin.HandlerFunc {
+func (s *Server) apiRouteHandlerWrapperFunc(apiHandler fnext.ApiRouteHandler) gin.HandlerFunc {
 	return func(c *gin.Context) {
 		context := c.Request.Context()
 		// get the app
@@ -108,7 +71,7 @@ func (s *Server) apiRouteHandlerWrapperFunc(apiHandler ApiRouteHandler) gin.Hand
 }
 
 // AddEndpoint adds an endpoint to /v1/x
-func (s *Server) AddEndpoint(method, path string, handler ApiHandler) {
+func (s *Server) AddEndpoint(method, path string, handler fnext.ApiHandler) {
 	v1 := s.Router.Group("/v1")
 	// v1.GET("/apps/:app/log", logHandler(cfg))
 	v1.Handle(method, path, s.apiHandlerWrapperFunc(handler))
@@ -116,27 +79,27 @@ func (s *Server) AddEndpoint(method, path string, handler ApiHandler) {
 
 // AddEndpoint adds an endpoint to /v1/x
 func (s *Server) AddEndpointFunc(method, path string, handler func(w http.ResponseWriter, r *http.Request)) {
-	s.AddEndpoint(method, path, ApiHandlerFunc(handler))
+	s.AddEndpoint(method, path, fnext.ApiHandlerFunc(handler))
 }
 
 // AddAppEndpoint adds an endpoints to /v1/apps/:app/x
-func (s *Server) AddAppEndpoint(method, path string, handler ApiAppHandler) {
+func (s *Server) AddAppEndpoint(method, path string, handler fnext.ApiAppHandler) {
 	v1 := s.Router.Group("/v1")
 	v1.Handle(method, "/apps/:app"+path, s.apiAppHandlerWrapperFunc(handler))
 }
 
 // AddAppEndpoint adds an endpoints to /v1/apps/:app/x
 func (s *Server) AddAppEndpointFunc(method, path string, handler func(w http.ResponseWriter, r *http.Request, app *models.App)) {
-	s.AddAppEndpoint(method, path, ApiAppHandlerFunc(handler))
+	s.AddAppEndpoint(method, path, fnext.ApiAppHandlerFunc(handler))
 }
 
 // AddRouteEndpoint adds an endpoints to /v1/apps/:app/routes/:route/x
-func (s *Server) AddRouteEndpoint(method, path string, handler ApiRouteHandler) {
+func (s *Server) AddRouteEndpoint(method, path string, handler fnext.ApiRouteHandler) {
 	v1 := s.Router.Group("/v1")
 	v1.Handle(method, "/apps/:app/routes/:route"+path, s.apiRouteHandlerWrapperFunc(handler)) // conflicts with existing wildcard
 }
 
 // AddRouteEndpoint adds an endpoints to /v1/apps/:app/routes/:route/x
 func (s *Server) AddRouteEndpointFunc(method, path string, handler func(w http.ResponseWriter, r *http.Request, app *models.App, route *models.Route)) {
-	s.AddRouteEndpoint(method, path, ApiRouteHandlerFunc(handler))
+	s.AddRouteEndpoint(method, path, fnext.ApiRouteHandlerFunc(handler))
 }

api/server/extensions.go

@@ -0,0 +1,30 @@
+package server
+
+import (
+	"fmt"
+	"log"
+
+	"github.com/fnproject/fn/fnext"
+)
+
+// TODO: Move this into `github.com/fnproject/fn` package after main is moved out of root dir.
+var extensions = map[string]fnext.Extension{}
+
+// RegisterExtension registers the extension so it's available, but does not initialize it or anything
+func RegisterExtension(ext fnext.Extension) {
+	extensions[ext.Name()] = ext
+}
+
+// AddExtensionByName This essentially just makes sure the extensions are ordered properly.
+// It could do some initialization if required too.
+func (s *Server) AddExtensionByName(name string) {
+	fmt.Printf("extensions: %+v\n", extensions)
+	e, ok := extensions[name]
+	if !ok {
+		log.Fatalf("Extension %v not registered.\n", name)
+	}
+	err := e.Setup(s)
+	if err != nil {
+		log.Fatalf("Failed to add extension %v: %v\n", name, err)
+	}
+}

api/server/gin_middlewares.go

@@ -0,0 +1,96 @@
+// This is middleware we're using for the entire server.
+
+package server
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/fnproject/fn/api"
+	"github.com/fnproject/fn/api/common"
+	"github.com/fnproject/fn/api/models"
+	"github.com/gin-contrib/cors"
+	"github.com/gin-gonic/gin"
+	opentracing "github.com/opentracing/opentracing-go"
+	"github.com/opentracing/opentracing-go/ext"
+	"github.com/sirupsen/logrus"
+)
+
+func optionalCorsWrap(r *gin.Engine) {
+	// By default no CORS are allowed unless one
+	// or more Origins are defined by the API_CORS
+	// environment variable.
+	corsStr := getEnv(EnvAPICORS, "")
+	if len(corsStr) > 0 {
+		origins := strings.Split(strings.Replace(corsStr, " ", "", -1), ",")
+
+		corsConfig := cors.DefaultConfig()
+		corsConfig.AllowOrigins = origins
+
+		logrus.Infof("CORS enabled for domains: %s", origins)
+
+		r.Use(cors.New(corsConfig))
+	}
+}
+
+// we should use http grr
+func traceWrap(c *gin.Context) {
+	// try to grab a span from the request if made from another service, ignore err if not
+	wireContext, _ := opentracing.GlobalTracer().Extract(
+		opentracing.HTTPHeaders,
+		opentracing.HTTPHeadersCarrier(c.Request.Header))
+
+	// Create the span referring to the RPC client if available.
+	// If wireContext == nil, a root span will be created.
+	// TODO we should add more tags?
+	serverSpan := opentracing.StartSpan("serve_http", ext.RPCServerOption(wireContext), opentracing.Tag{Key: "path", Value: c.Request.URL.Path})
+	serverSpan.SetBaggageItem("fn_appname", c.Param(api.CApp))
+	serverSpan.SetBaggageItem("fn_path", c.Param(api.CRoute))
+	defer serverSpan.Finish()
+
+	ctx := opentracing.ContextWithSpan(c.Request.Context(), serverSpan)
+	c.Request = c.Request.WithContext(ctx)
+	c.Next()
+}
+
+func panicWrap(c *gin.Context) {
+	defer func(c *gin.Context) {
+		if rec := recover(); rec != nil {
+			err, ok := rec.(error)
+			if !ok {
+				err = fmt.Errorf("fn: %v", rec)
+			}
+			handleErrorResponse(c, err)
+			c.Abort()
+		}
+	}(c)
+	c.Next()
+}
+
+func loggerWrap(c *gin.Context) {
+	ctx, _ := common.LoggerWithFields(c.Request.Context(), extractFields(c))
+
+	if appName := c.Param(api.CApp); appName != "" {
+		c.Set(api.AppName, appName)
+		ctx = context.WithValue(ctx, api.AppName, appName)
+	}
+
+	if routePath := c.Param(api.CRoute); routePath != "" {
+		c.Set(api.Path, routePath)
+		ctx = context.WithValue(ctx, api.Path, routePath)
+	}
+
+	c.Request = c.Request.WithContext(ctx)
+	c.Next()
+}
+
+func appWrap(c *gin.Context) {
+	appName := c.GetString(api.AppName)
+	if appName == "" {
+		handleErrorResponse(c, models.ErrAppsMissingName)
+		c.Abort()
+		return
+	}
+	c.Next()
+}

api/server/listeners.go

@@ -1,8 +1,8 @@
 package server
 
-import "github.com/fnproject/fn/api/extensions"
+import "github.com/fnproject/fn/fnext"
 
 // AddCallListener adds a listener that will be fired before and after a function is executed.
-func (s *Server) AddCallListener(listener extensions.CallListener) {
+func (s *Server) AddCallListener(listener fnext.CallListener) {
 	s.Agent.AddCallListener(listener)
 }

api/server/middleware.go

@@ -2,67 +2,128 @@ package server
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 
 	"github.com/fnproject/fn/api/common"
+	"github.com/fnproject/fn/fnext"
 	"github.com/gin-gonic/gin"
 )
 
-// Middleware just takes a http.Handler and returns one. So the next middle ware must be called
-// within the returned handler or it would be ignored.
-type Middleware interface {
-	Chain(next http.Handler) http.Handler
+type middlewareController struct {
+	// NOTE: I tried to make this work as if it were a normal context, but it just doesn't work right. If someone
+	// does something like context.WithValue, then the return is a new &context.valueCtx{} which can't be cast. So now stuffing it into a value instead.
+	// context.Context
+
+	// separating this out so we can use it and don't have to reimplement context.Context above
+	ginContext     *gin.Context
+	server         *Server
+	functionCalled bool
 }
 
-// MiddlewareFunc is a here to allow a plain function to be a middleware.
-type MiddlewareFunc func(next http.Handler) http.Handler
-
-// Chain used to allow middlewarefuncs to be middleware.
-func (m MiddlewareFunc) Chain(next http.Handler) http.Handler {
-	return m(next)
+// CallFunction bypasses any further gin routing and calls the function directly
+func (c *middlewareController) CallFunction(w http.ResponseWriter, r *http.Request) {
+	c.functionCalled = true
+	ctx := r.Context()
+	ctx = context.WithValue(ctx, fnext.MiddlewareControllerKey, c)
+	r = r.WithContext(ctx)
+	c.ginContext.Request = r
+	c.server.handleFunctionCall(c.ginContext)
+	c.ginContext.Abort()
+}
+func (c *middlewareController) FunctionCalled() bool {
+	return c.functionCalled
 }
 
-func (s *Server) middlewareWrapperFunc(ctx context.Context) gin.HandlerFunc {
+func (s *Server) apiMiddlewareWrapper() gin.HandlerFunc {
 	return func(c *gin.Context) {
-		if len(s.middlewares) > 0 {
-			defer func() {
-				//This is so that if the server errors or panics on a middleware the server will still respond and not send eof to client.
-				err := recover()
-				if err != nil {
-					common.Logger(c.Request.Context()).WithField("MiddleWarePanicRecovery:", err).Errorln("A panic occurred during middleware.")
-					handleErrorResponse(c, ErrInternalServerError)
-				}
-			}()
-			var h http.Handler
-			keepgoing := false
-			h = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				c.Request = c.Request.WithContext(r.Context())
-				keepgoing = true
-			})
-
-			s.chainAndServe(c.Writer, c.Request, h)
-			if !keepgoing {
-				c.Abort()
-			}
-		}
+		s.runMiddleware(c, s.apiMiddlewares)
 	}
 }
 
-func (s *Server) chainAndServe(w http.ResponseWriter, r *http.Request, h http.Handler) {
-	for _, m := range s.middlewares {
-		h = m.Chain(h)
+func (s *Server) rootMiddlewareWrapper() gin.HandlerFunc {
+	return func(c *gin.Context) {
+		s.runMiddleware(c, s.rootMiddlewares)
+	}
+}
+
+// This is basically a single gin middleware that runs a bunch of fn middleware.
+// The final handler will pass it back to gin for further processing.
+func (s *Server) runMiddleware(c *gin.Context, ms []fnext.Middleware) {
+	if len(ms) == 0 {
+		c.Next()
+		return
+	}
+	defer func() {
+		//This is so that if the server errors or panics on a middleware the server will still respond and not send eof to client.
+		err := recover()
+		if err != nil {
+			common.Logger(c.Request.Context()).WithField("MiddleWarePanicRecovery:", err).Errorln("A panic occurred during middleware.")
+			handleErrorResponse(c, ErrInternalServerError)
+		}
+	}()
+
+	ctx := context.WithValue(c.Request.Context(), fnext.MiddlewareControllerKey, s.newMiddlewareController(c))
+	last := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Println("final function called")
+		// check for bypass
+		mctx := fnext.GetMiddlewareController(r.Context())
+		if mctx.FunctionCalled() {
+			fmt.Println("function already called, skipping")
+			c.Abort()
+			return
+		}
+		c.Next()
+	})
+
+	chainAndServe(ms, c.Writer, c.Request.WithContext(ctx), last)
+}
+
+func (s *Server) newMiddlewareController(c *gin.Context) *middlewareController {
+	return &middlewareController{
+		ginContext: c,
+		server:     s,
+	}
+}
+
+// chainAndServe essentially makes a chain of middleware wrapped around each other, then calls ServerHTTP on the end result.
+// then each middleware also calls ServeHTTP within it
+func chainAndServe(ms []fnext.Middleware, w http.ResponseWriter, r *http.Request, last http.Handler) {
+	h := last
+	// These get chained in reverse order so they play out in the right order. Don't ask.
+	for i := len(ms) - 1; i >= 0; i-- {
+		m := ms[i]
+		h = m.Handle(h)
 	}
 	h.ServeHTTP(w, r)
 }
 
-// AddMiddleware add middleware
-func (s *Server) AddMiddleware(m Middleware) {
-	//Prepend to array so that we can do first,second,third,last,third,second,first
-	//and not third,second,first,last,first,second,third
-	s.middlewares = append([]Middleware{m}, s.middlewares...)
+// AddMiddleware DEPRECATED - see AddAPIMiddleware
+func (s *Server) AddMiddleware(m fnext.Middleware) {
+	s.AddAPIMiddleware(m)
 }
 
-// AddMiddlewareFunc add middlewarefunc
-func (s *Server) AddMiddlewareFunc(m MiddlewareFunc) {
-	s.AddMiddleware(m)
+// AddMiddlewareFunc DEPRECATED - see AddAPIMiddlewareFunc
+func (s *Server) AddMiddlewareFunc(m fnext.MiddlewareFunc) {
+	s.AddAPIMiddlewareFunc(m)
+}
+
+// AddAPIMiddleware add middleware
+func (s *Server) AddAPIMiddleware(m fnext.Middleware) {
+	s.apiMiddlewares = append(s.apiMiddlewares, m)
+}
+
+// AddAPIMiddlewareFunc add middlewarefunc
+func (s *Server) AddAPIMiddlewareFunc(m fnext.MiddlewareFunc) {
+	s.AddAPIMiddleware(m)
+}
+
+// AddRootMiddleware add middleware add middleware for end user applications
+func (s *Server) AddRootMiddleware(m fnext.Middleware) {
+	s.rootMiddlewares = append(s.rootMiddlewares, m)
+}
+
+// AddRootMiddlewareFunc add middleware for end user applications
+func (s *Server) AddRootMiddlewareFunc(m fnext.MiddlewareFunc) {
+	s.AddRootMiddleware(m)
 }

api/server/middleware_test.go

@@ -1,39 +1,59 @@
 package server
 
 import (
+	"context"
+	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"strings"
 	"testing"
+
+	"github.com/fnproject/fn/api/datastore"
+	"github.com/fnproject/fn/api/logs"
+	"github.com/fnproject/fn/api/models"
+	"github.com/fnproject/fn/api/mqs"
+	"github.com/fnproject/fn/fnext"
+	"github.com/gin-gonic/gin"
 )
 
+func TestMain(m *testing.M) {
+	// call flag.Parse() here if TestMain uses flags
+	os.Exit(m.Run())
+}
+
 type middleWareStruct struct {
 	name string
 }
 
-func (m *middleWareStruct) Chain(next http.Handler) http.Handler {
+func (m *middleWareStruct) Handle(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Write([]byte(m.name + ","))
 		next.ServeHTTP(w, r)
 	})
 }
 
-func TestMiddleWareChaining(t *testing.T) {
+func TestMiddlewareChaining(t *testing.T) {
 	var lastHandler http.Handler
 	lastHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Write([]byte("last"))
 	})
 
 	s := Server{}
-	s.AddMiddleware(&middleWareStruct{"first"})
-	s.AddMiddleware(&middleWareStruct{"second"})
-	s.AddMiddleware(&middleWareStruct{"third"})
-	s.AddMiddleware(&middleWareStruct{"fourth"})
+	s.AddAPIMiddleware(&middleWareStruct{"first"})
+	s.AddAPIMiddleware(&middleWareStruct{"second"})
+	s.AddAPIMiddleware(&middleWareStruct{"third"})
+	s.AddAPIMiddleware(&middleWareStruct{"fourth"})
+	c := &gin.Context{}
 
 	rec := httptest.NewRecorder()
 	req, _ := http.NewRequest("get", "http://localhost/", nil)
+	ctx := context.WithValue(req.Context(), fnext.MiddlewareControllerKey, s.newMiddlewareController(c))
+	req = req.WithContext(ctx)
+	c.Request = req
 
-	s.chainAndServe(rec, req, lastHandler)
+	chainAndServe(s.apiMiddlewares, rec, req, lastHandler)
 
 	result, err := ioutil.ReadAll(rec.Result().Body)
 	if err != nil {
@@ -41,6 +61,89 @@ func TestMiddleWareChaining(t *testing.T) {
 	}
 
 	if string(result) != "first,second,third,fourth,last" {
-		t.Fatal("You failed to chain correctly.", string(result))
+		t.Fatal("You failed to chain correctly:", string(result))
+	}
+}
+
+func TestRootMiddleware(t *testing.T) {
+
+	ds := datastore.NewMockInit(
+		[]*models.App{
+			{Name: "myapp", Config: models.Config{}},
+			{Name: "myapp2", Config: models.Config{}},
+		},
+		[]*models.Route{
+			{Path: "/", AppName: "myapp", Image: "fnproject/hello", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}},
+			{Path: "/myroute", AppName: "myapp", Image: "fnproject/hello", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}},
+			{Path: "/app2func", AppName: "myapp2", Image: "fnproject/hello", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}},
+				Config: map[string]string{"NAME": "johnny"},
+			},
+		}, nil,
+	)
+
+	rnr, cancelrnr := testRunner(t, ds)
+	defer cancelrnr()
+
+	fnl := logs.NewMock()
+	srv := testServer(ds, &mqs.Mock{}, fnl, rnr)
+	srv.AddRootMiddlewareFunc(func(next http.Handler) http.Handler {
+		// this one will override a call to the API based on a header
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.Header.Get("funcit") != "" {
+				fmt.Fprintf(os.Stderr, "breaker breaker!\n")
+				ctx := r.Context()
+				// TODO: this is a little dicey, should have some functions to set these in case the context keys change or something.
+				ctx = context.WithValue(ctx, "app_name", "myapp2")
+				ctx = context.WithValue(ctx, "path", "/app2func")
+				mctx := fnext.GetMiddlewareController(ctx)
+				mctx.CallFunction(w, r.WithContext(ctx))
+				return
+			}
+			// If any context changes, user should use this: next.ServeHTTP(w, r.WithContext(ctx))
+			next.ServeHTTP(w, r)
+		})
+	})
+	srv.AddRootMiddlewareFunc(func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// fmt.Fprintf(os.Stderr, "middle log\n")
+			next.ServeHTTP(w, r)
+		})
+	})
+	srv.AddRootMiddleware(&middleWareStruct{"middle"})
+
+	for i, test := range []struct {
+		path           string
+		body           string
+		method         string
+		headers        map[string][]string
+		expectedCode   int
+		expectedInBody string
+	}{
+		{"/r/myapp", ``, "GET", map[string][]string{}, http.StatusOK, "middle"},
+		{"/r/myapp/myroute", ``, "GET", map[string][]string{}, http.StatusOK, "middle"},
+		{"/v1/apps", ``, "GET", map[string][]string{"funcit": {"Test"}}, http.StatusOK, "johnny"},
+	} {
+		body := strings.NewReader(test.body)
+		req, err := http.NewRequest(test.method, "http://127.0.0.1:8080"+test.path, body)
+		if err != nil {
+			t.Fatalf("Test: Could not create %s request to %s: %v", test.method, test.path, err)
+		}
+		for k, v := range test.headers {
+			req.Header.Add(k, v[0])
+		}
+		fmt.Println("TESTING:", req.URL.String())
+		_, rec := routerRequest2(t, srv.Router, req)
+		// t.Log("REC: %+v\n", rec)
+
+		result, err := ioutil.ReadAll(rec.Result().Body)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		rbody := string(result)
+		t.Log("rbody:", rbody)
+		if !strings.Contains(rbody, test.expectedInBody) {
+			t.Fatal(i, "middleware didn't work correctly", string(result))
+		}
 	}
 }

api/server/runner.go

@@ -2,9 +2,9 @@ package server
 
 import (
 	"bytes"
+	"errors"
 	"net/http"
 	"path"
-	"strings"
 	"time"
 
 	"github.com/fnproject/fn/api"
@@ -20,24 +20,35 @@ type runnerResponse struct {
 	Error     *models.ErrorBody `json:"error,omitempty"`
 }
 
-func (s *Server) handleRequest(c *gin.Context) {
-	if strings.HasPrefix(c.Request.URL.Path, "/v1") {
-		c.Status(http.StatusNotFound)
+// handleFunctionCall executes the function.
+// Requires the following in the context:
+// * "app_name"
+// * "path"
+func (s *Server) handleFunctionCall(c *gin.Context) {
+	// @treeder: Is this necessary? An app could have this prefix too. Leaving here for review.
+	// if strings.HasPrefix(c.Request.URL.Path, "/v1") {
+	// 	c.Status(http.StatusNotFound)
+	// 	return
+	// }
+
+	ctx := c.Request.Context()
+	var p string
+	r := ctx.Value(api.Path)
+	if r == nil {
+		p = "/"
+	} else {
+		p = r.(string)
+	}
+
+	var a string
+	ai := ctx.Value(api.AppName)
+	if ai == nil {
+		handleErrorResponse(c, errors.New("app name not set"))
 		return
 	}
+	a = ai.(string)
 
-	r, routeExists := c.Get(api.Path)
-	if !routeExists {
-		r = "/"
-	}
-
-	reqRoute := &models.Route{
-		AppName: c.MustGet(api.AppName).(string),
-		Path:    path.Clean(r.(string)),
-	}
-
-	s.serve(c, reqRoute.AppName, reqRoute.Path)
-
+	s.serve(c, a, path.Clean(p))
 }
 
 // convert gin.Params to agent.Params to avoid introducing gin

api/server/server.go

@@ -11,24 +11,19 @@ import (
 	"os"
 	"path"
 	"strconv"
-	"strings"
 	"syscall"
 
-	"github.com/fnproject/fn/api"
 	"github.com/fnproject/fn/api/agent"
-	"github.com/fnproject/fn/api/common"
 	"github.com/fnproject/fn/api/datastore"
 	"github.com/fnproject/fn/api/datastore/cache"
-	"github.com/fnproject/fn/api/extensions"
 	"github.com/fnproject/fn/api/id"
 	"github.com/fnproject/fn/api/logs"
 	"github.com/fnproject/fn/api/models"
 	"github.com/fnproject/fn/api/mqs"
 	"github.com/fnproject/fn/api/version"
-	"github.com/gin-contrib/cors"
+	"github.com/fnproject/fn/fnext"
 	"github.com/gin-gonic/gin"
 	"github.com/opentracing/opentracing-go"
-	"github.com/opentracing/opentracing-go/ext"
 	"github.com/openzipkin/zipkin-go-opentracing"
 	"github.com/sirupsen/logrus"
 )
@@ -58,8 +53,9 @@ type Server struct {
 	MQ        models.MessageQueue
 	LogDB     models.LogStore
 
-	appListeners []extensions.AppListener
-	middlewares  []Middleware
+	appListeners    []fnext.AppListener
+	rootMiddlewares []fnext.Middleware
+	apiMiddlewares  []fnext.Middleware
 }
 
 // NewFromEnv creates a new Functions server based on env vars.
@@ -96,23 +92,6 @@ func NewFromURLs(ctx context.Context, dbURL, mqURL, logstoreURL string, opts ...
 	return New(ctx, ds, mq, logDB, opts...)
 }
 
-func optionalCorsWrap(r *gin.Engine) {
-	// By default no CORS are allowed unless one
-	// or more Origins are defined by the API_CORS
-	// environment variable.
-	corsStr := getEnv(EnvAPICORS, "")
-	if len(corsStr) > 0 {
-		origins := strings.Split(strings.Replace(corsStr, " ", "", -1), ",")
-
-		corsConfig := cors.DefaultConfig()
-		corsConfig.AllowOrigins = origins
-
-		logrus.Infof("CORS enabled for domains: %s", origins)
-
-		r.Use(cors.New(corsConfig))
-	}
-}
-
 // New creates a new Functions server with the passed in datastore, message queue and API URL
 func New(ctx context.Context, ds models.Datastore, mq models.MessageQueue, ls models.LogStore, opts ...ServerOption) *Server {
 	setTracer()
@@ -125,10 +104,10 @@ func New(ctx context.Context, ds models.Datastore, mq models.MessageQueue, ls mo
 		LogDB:     ls,
 	}
 
+	// NOTE: testServer() in tests doesn't use these
 	setMachineID()
 	s.Router.Use(loggerWrap, traceWrap, panicWrap)
 	optionalCorsWrap(s.Router)
-
 	s.bindHandlers(ctx)
 
 	for _, opt := range opts {
@@ -140,26 +119,6 @@ func New(ctx context.Context, ds models.Datastore, mq models.MessageQueue, ls mo
 	return s
 }
 
-// we should use http grr
-func traceWrap(c *gin.Context) {
-	// try to grab a span from the request if made from another service, ignore err if not
-	wireContext, _ := opentracing.GlobalTracer().Extract(
-		opentracing.HTTPHeaders,
-		opentracing.HTTPHeadersCarrier(c.Request.Header))
-
-	// Create the span referring to the RPC client if available.
-	// If wireContext == nil, a root span will be created.
-	// TODO we should add more tags?
-	serverSpan := opentracing.StartSpan("serve_http", ext.RPCServerOption(wireContext), opentracing.Tag{Key: "path", Value: c.Request.URL.Path})
-	serverSpan.SetBaggageItem("fn_appname", c.Param(api.CApp))
-	serverSpan.SetBaggageItem("fn_path", c.Param(api.CRoute))
-	defer serverSpan.Finish()
-
-	ctx := opentracing.ContextWithSpan(c.Request.Context(), serverSpan)
-	c.Request = c.Request.WithContext(ctx)
-	c.Next()
-}
-
 func setTracer() {
 	var (
 		debugMode          = false
@@ -239,51 +198,6 @@ func whoAmI() net.IP {
 	return nil
 }
 
-func panicWrap(c *gin.Context) {
-	defer func(c *gin.Context) {
-		if rec := recover(); rec != nil {
-			err, ok := rec.(error)
-			if !ok {
-				err = fmt.Errorf("fn: %v", rec)
-			}
-			handleErrorResponse(c, err)
-			c.Abort()
-		}
-	}(c)
-	c.Next()
-}
-
-func loggerWrap(c *gin.Context) {
-	ctx, _ := common.LoggerWithFields(c.Request.Context(), extractFields(c))
-
-	if appName := c.Param(api.CApp); appName != "" {
-		c.Set(api.AppName, appName)
-		c.Request = c.Request.WithContext(context.WithValue(c.Request.Context(), api.AppName, appName))
-	}
-
-	if routePath := c.Param(api.CRoute); routePath != "" {
-		c.Set(api.Path, routePath)
-		c.Request = c.Request.WithContext(context.WithValue(c.Request.Context(), api.Path, routePath))
-	}
-
-	c.Request = c.Request.WithContext(ctx)
-	c.Next()
-}
-
-func appWrap(c *gin.Context) {
-	appName := c.GetString(api.AppName)
-	if appName == "" {
-		handleErrorResponse(c, models.ErrAppsMissingName)
-		c.Abort()
-		return
-	}
-	c.Next()
-}
-
-func (s *Server) handleRunnerRequest(c *gin.Context) {
-	s.handleRequest(c)
-}
-
 func extractFields(c *gin.Context) logrus.Fields {
 	fields := logrus.Fields{"action": path.Base(c.HandlerName())}
 	for _, param := range c.Params {
@@ -342,15 +256,18 @@ func (s *Server) startGears(ctx context.Context, cancel context.CancelFunc) {
 
 func (s *Server) bindHandlers(ctx context.Context) {
 	engine := s.Router
+	// now for extendible middleware
+	engine.Use(s.rootMiddlewareWrapper())
 
 	engine.GET("/", handlePing)
 	engine.GET("/version", handleVersion)
+	// TODO: move the following under v1
 	engine.GET("/stats", s.handleStats)
 	engine.GET("/metrics", s.handlePrometheusMetrics)
 
 	{
 		v1 := engine.Group("/v1")
-		v1.Use(s.middlewareWrapperFunc(ctx))
+		v1.Use(s.apiMiddlewareWrapper())
 		v1.GET("/apps", s.handleAppList)
 		v1.POST("/apps", s.handleAppCreate)
 
@@ -379,8 +296,8 @@ func (s *Server) bindHandlers(ctx context.Context) {
 	{
 		runner := engine.Group("/r")
 		runner.Use(appWrap)
-		runner.Any("/:app", s.handleRunnerRequest)
-		runner.Any("/:app/*route", s.handleRunnerRequest)
+		runner.Any("/:app", s.handleFunctionCall)
+		runner.Any("/:app/*route", s.handleFunctionCall)
 	}
 
 	engine.NoRoute(func(c *gin.Context) {

api/server/server_test.go

@@ -44,6 +44,10 @@ func routerRequest(t *testing.T, router *gin.Engine, method, path string, body i
 	if err != nil {
 		t.Fatalf("Test: Could not create %s request to %s: %v", method, path, err)
 	}
+	return routerRequest2(t, router, req)
+}
+
+func routerRequest2(t *testing.T, router *gin.Engine, req *http.Request) (*http.Request, *httptest.ResponseRecorder) {
 
 	rec := httptest.NewRecorder()
 	router.ServeHTTP(rec, req)