diff --git a/api/agent/agent.go b/api/agent/agent.go index 30e744157..7ffe9be6f 100644 --- a/api/agent/agent.go +++ b/api/agent/agent.go @@ -14,6 +14,8 @@ import ( "sync" "time" + "path/filepath" + "github.com/fnproject/fn/api/agent/drivers" "github.com/fnproject/fn/api/agent/protocol" "github.com/fnproject/fn/api/common" @@ -24,7 +26,6 @@ import ( "github.com/sirupsen/logrus" "go.opencensus.io/stats" "go.opencensus.io/trace" - "path/filepath" ) // TODO we should prob store async calls in db immediately since we're returning id (will 404 until post-execution) @@ -836,8 +837,8 @@ func (a *agent) prepCold(ctx context.Context, call *call, tok ResourceToken, ch logCfg: drivers.LoggerConfig{ URL: strings.TrimSpace(call.SyslogURL), Tags: []drivers.LoggerTag{ - {Name: "app_name", Value: call.AppName}, - {Name: "func_name", Value: call.Path}, + {Name: "app_id", Value: call.AppID}, + {Name: "fn_id", Value: call.FnID}, }, }, stdin: call.req.Body, @@ -902,7 +903,7 @@ func (a *agent) runHot(ctx context.Context, call *call, tok ResourceToken, state close(udsAwait) // XXX(reed): short case first / kill this } - logger := logrus.WithFields(logrus.Fields{"id": container.id, "app_id": call.AppID, "route": call.Path, "image": call.Image, "memory": call.Memory, "cpus": call.CPUs, "format": call.Format, "idle_timeout": call.IdleTimeout}) + logger := logrus.WithFields(logrus.Fields{"id": container.id, "app_id": call.AppID, "fn_id": call.FnID, "image": call.Image, "memory": call.Memory, "cpus": call.CPUs, "format": call.Format, "idle_timeout": call.IdleTimeout}) ctx = common.WithLogger(ctx, logger) cookie, err := a.driver.CreateCookie(ctx, container) @@ -1239,10 +1240,10 @@ func newHotContainer(ctx context.Context, call *call, cfg *Config) (*container, bufs = []*bytes.Buffer{buf1, buf2} soc := &nopCloser{&logWriter{ - logrus.WithFields(logrus.Fields{"tag": "stdout", "app_id": call.AppID, "path": call.Path, "image": call.Image, "container_id": id}), + logrus.WithFields(logrus.Fields{"tag": "stdout", "app_id": call.AppID, "fn_id": call.FnID, "image": call.Image, "container_id": id}), }} sec := &nopCloser{&logWriter{ - logrus.WithFields(logrus.Fields{"tag": "stderr", "app_id": call.AppID, "path": call.Path, "image": call.Image, "container_id": id}), + logrus.WithFields(logrus.Fields{"tag": "stderr", "app_id": call.AppID, "fn_id": call.FnID, "image": call.Image, "container_id": id}), }} stdout.Swap(newLineWriterWithBuffer(buf1, soc)) @@ -1288,8 +1289,8 @@ func newHotContainer(ctx context.Context, call *call, cfg *Config) (*container, logCfg: drivers.LoggerConfig{ URL: strings.TrimSpace(call.SyslogURL), Tags: []drivers.LoggerTag{ - {Name: "app_name", Value: call.AppName}, - {Name: "func_name", Value: call.Path}, + {Name: "app_id", Value: call.AppID}, + {Name: "fn_id", Value: call.FnID}, }, }, stdin: stdin, diff --git a/api/agent/agent_test.go b/api/agent/agent_test.go index 157618d25..a2944e438 100644 --- a/api/agent/agent_test.go +++ b/api/agent/agent_test.go @@ -69,8 +69,6 @@ func checkClose(t *testing.T, a Agent) { } func TestCallConfigurationRequest(t *testing.T) { - appName := "myapp" - path := "/" image := "fnproject/fn-test-utils" const timeout = 1 const idleTimeout = 20 @@ -79,19 +77,19 @@ func TestCallConfigurationRequest(t *testing.T) { format := "default" cfg := models.Config{"APP_VAR": "FOO"} - rCfg := models.Config{"ROUTE_VAR": "BAR"} + rCfg := models.Config{"FN_VAR": "BAR"} - app := &models.App{ID: "app_id", Name: appName, Config: cfg} - route := &models.Route{ - AppID: app.ID, - Config: rCfg, - Path: path, - Image: image, - Type: typ, - Format: format, - Timeout: timeout, - IdleTimeout: idleTimeout, - Memory: memory, + app := &models.App{ID: "app_id", Config: cfg} + fn := &models.Fn{ + ID: "fn_id", + AppID: app.ID, + Config: rCfg, + Image: image, + Format: format, + ResourceConfig: models.ResourceConfig{Timeout: timeout, + IdleTimeout: idleTimeout, + Memory: memory, + }, } ls := logs.NewMock() @@ -102,7 +100,7 @@ func TestCallConfigurationRequest(t *testing.T) { w := httptest.NewRecorder() method := "GET" - url := "http://127.0.0.1:8080/r/" + appName + path + url := "http://127.0.0.1:8080/invoke/" + fn.ID payload := "payload" contentLength := strconv.Itoa(len(payload)) req, err := http.NewRequest(method, url, strings.NewReader(payload)) @@ -113,11 +111,11 @@ func TestCallConfigurationRequest(t *testing.T) { req.Header.Add("MYREALHEADER", "FOOLORD") req.Header.Add("MYREALHEADER", "FOOPEASANT") req.Header.Add("Content-Length", contentLength) - req.Header.Add("FN_PATH", "thewrongroute") // ensures that this doesn't leak out, should be overwritten + req.Header.Add("FN_PATH", "thewrongfn") // ensures that this doesn't leak out, should be overwritten call, err := a.GetCall( WithWriter(w), // XXX (reed): order matters [for now] - FromRequest(app, route, req), + FromHTTPFnRequest(app, fn, req), ) if err != nil { t.Fatal(err) @@ -132,14 +130,11 @@ func TestCallConfigurationRequest(t *testing.T) { if model.AppID != app.ID { t.Fatal("app ID mismatch", model.ID, app.ID) } - if model.Path != path { - t.Fatal("path mismatch", model.Path, path) - } if model.Image != image { t.Fatal("image mismatch", model.Image, image) } if model.Type != "sync" { - t.Fatal("route type mismatch", model.Type) + t.Fatal("fn type mismatch", model.Type) } if model.Priority == nil { t.Fatal("GetCall should make priority non-nil so that async works because for whatever reason some clowns plumbed it all over the mqs even though the user can't specify it gg") @@ -164,13 +159,11 @@ func TestCallConfigurationRequest(t *testing.T) { } expectedConfig := map[string]string{ - "FN_FORMAT": format, - "FN_APP_NAME": appName, - "FN_PATH": path, - "FN_MEMORY": strconv.Itoa(memory), - "FN_TYPE": typ, - "APP_VAR": "FOO", - "ROUTE_VAR": "BAR", + "FN_FORMAT": format, + "FN_MEMORY": strconv.Itoa(memory), + "FN_TYPE": typ, + "APP_VAR": "FOO", + "FN_VAR": "BAR", } for k, v := range expectedConfig { @@ -192,46 +185,38 @@ func TestCallConfigurationRequest(t *testing.T) { checkExpectedHeaders(t, expectedHeaders, model.Headers) - // TODO check response writer for route headers + // TODO check response writer for fn headers } func TestCallConfigurationModel(t *testing.T) { - app := &models.App{Name: "myapp"} + fn := &models.Fn{ID: "fn_id"} - path := "/" image := "fnproject/fn-test-utils" const timeout = 1 const idleTimeout = 20 const memory = 256 - CPUs := models.MilliCPUs(1000) method := "GET" - url := "http://127.0.0.1:8080/r/" + app.Name + path + url := "http://127.0.0.1:8080/invoke/" + fn.ID payload := "payload" typ := "sync" format := "default" cfg := models.Config{ - "FN_FORMAT": format, - "FN_APP_NAME": app.Name, - "FN_PATH": path, - "FN_MEMORY": strconv.Itoa(memory), - "FN_CPUS": CPUs.String(), - "FN_TYPE": typ, - "APP_VAR": "FOO", - "ROUTE_VAR": "BAR", + "FN_FORMAT": format, + "FN_MEMORY": strconv.Itoa(memory), + "FN_TYPE": typ, + "APP_VAR": "FOO", + "FN_VAR": "BAR", } cm := &models.Call{ - AppID: app.ID, - AppName: app.Name, + FnID: fn.ID, Config: cfg, - Path: path, Image: image, Type: typ, Format: format, Timeout: timeout, IdleTimeout: idleTimeout, Memory: memory, - CPUs: CPUs, Payload: payload, URL: url, Method: method, @@ -258,33 +243,10 @@ func TestCallConfigurationModel(t *testing.T) { } } -func TestAsyncCallHeaders(t *testing.T) { - app := &models.App{ID: "app_id", Name: "myapp"} - - path := "/" - image := "fnproject/fn-test-utils" - const timeout = 1 - const idleTimeout = 20 - const memory = 256 - CPUs := models.MilliCPUs(200) - method := "GET" - url := "http://127.0.0.1:8080/r/" + app.Name + path +func TestGetCallFromModelRoundTripACall(t *testing.T) { payload := "payload" - typ := "async" - format := "http" contentType := "suberb_type" contentLength := strconv.FormatInt(int64(len(payload)), 10) - config := map[string]string{ - "FN_FORMAT": format, - "FN_APP_NAME": app.Name, - "FN_PATH": path, - "FN_MEMORY": strconv.Itoa(memory), - "FN_CPUS": CPUs.String(), - "FN_TYPE": typ, - "APP_VAR": "FOO", - "ROUTE_VAR": "BAR", - "DOUBLE_VAR": "BIZ, BAZ", - } headers := map[string][]string{ // FromRequest would insert these from original HTTP request "Content-Type": {contentType}, @@ -292,21 +254,9 @@ func TestAsyncCallHeaders(t *testing.T) { } cm := &models.Call{ - AppID: app.ID, - AppName: app.Name, - Config: config, - Headers: headers, - Path: path, - Image: image, - Type: typ, - Format: format, - Timeout: timeout, - IdleTimeout: idleTimeout, - Memory: memory, - CPUs: CPUs, - Payload: payload, - URL: url, - Method: method, + FnID: "fn_id", + Headers: headers, + Payload: payload, } // FromModel doesn't need a datastore, for now... @@ -409,43 +359,38 @@ func (l testListener) BeforeCall(context.Context, *models.Call) error { } func TestSubmitError(t *testing.T) { - app := &models.App{Name: "myapp"} + app := &models.App{ID: "app_id", Name: "myapp"} + fn := &models.Fn{ID: "fn_id", AppID: app.ID} - path := "/" image := "fnproject/fn-test-utils" const timeout = 10 const idleTimeout = 20 const memory = 256 - CPUs := models.MilliCPUs(200) method := "GET" - url := "http://127.0.0.1:8080/r/" + app.Name + path + url := "http://127.0.0.1:8080/invoke/" + fn.ID payload := `{"sleepTime": 0, "isDebug": true, "isCrash": true}` typ := "sync" format := "default" config := map[string]string{ "FN_FORMAT": format, "FN_APP_NAME": app.Name, - "FN_PATH": path, "FN_MEMORY": strconv.Itoa(memory), - "FN_CPUS": CPUs.String(), "FN_TYPE": typ, "APP_VAR": "FOO", - "ROUTE_VAR": "BAR", + "FN_VAR": "BAR", "DOUBLE_VAR": "BIZ, BAZ", } cm := &models.Call{ AppID: app.ID, - AppName: app.Name, + FnID: fn.ID, Config: config, - Path: path, Image: image, Type: typ, Format: format, Timeout: timeout, IdleTimeout: idleTimeout, Memory: memory, - CPUs: CPUs, Payload: payload, URL: url, Method: method, @@ -497,23 +442,20 @@ type dummyReader struct { func TestHTTPWithoutContentLengthWorks(t *testing.T) { // TODO it may be a good idea to mock out the http server and use a real // response writer with sync, and also test that this works with async + log - - appName := "myapp" - path := "/hello" - url := "http://127.0.0.1:8080/r/" + appName + path - - app := &models.App{ID: "app_id", Name: appName} - route := &models.Route{ - Path: path, - AppID: app.ID, - Image: "fnproject/fn-test-utils", - Type: "sync", - Format: "http", // this _is_ the test - Timeout: 5, - IdleTimeout: 10, - Memory: 128, + app := &models.App{ID: "app_id"} + fn := &models.Fn{ + ID: "fn_id", + Image: "fnproject/fn-test-utils", + Format: "http", // this _is_ the test + ResourceConfig: models.ResourceConfig{ + Timeout: 5, + IdleTimeout: 10, + Memory: 128, + }, } + url := "http://127.0.0.1:8080/invoke/" + fn.ID + ls := logs.NewMock() a := New(NewDirectCallDataAccess(ls, new(mqs.Mock))) defer checkClose(t, a) @@ -530,7 +472,7 @@ func TestHTTPWithoutContentLengthWorks(t *testing.T) { // grab a buffer so we can read what gets written to this guy var out bytes.Buffer - callI, err := a.GetCall(FromRequest(app, route, req), WithWriter(&out)) + callI, err := a.GetCall(FromHTTPFnRequest(app, fn, req), WithWriter(&out)) if err != nil { t.Fatal(err) } @@ -566,8 +508,7 @@ func TestHTTPWithoutContentLengthWorks(t *testing.T) { func TestGetCallReturnsResourceImpossibility(t *testing.T) { call := &models.Call{ AppID: id.New().String(), - AppName: "appName", - Path: "/yoyo", + FnID: id.New().String(), Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", @@ -590,23 +531,22 @@ func TestGetCallReturnsResourceImpossibility(t *testing.T) { // Tmp directory should be RW by default. // func TestTmpFsRW(t *testing.T) { - appName := "myapp" - path := "/hello" - url := "http://127.0.0.1:8080/r/" + appName + path - app := &models.App{ID: "app_id", Name: appName} + app := &models.App{ID: "app_id"} - route := &models.Route{ - Path: path, - AppID: app.ID, - Image: "fnproject/fn-test-utils", - Type: "sync", - Format: "http", // this _is_ the test - Timeout: 5, - IdleTimeout: 10, - Memory: 128, + fn := &models.Fn{ + ID: "fn_id", + AppID: app.ID, + Image: "fnproject/fn-test-utils", + Format: "http", // this _is_ the test + ResourceConfig: models.ResourceConfig{Timeout: 5, + IdleTimeout: 10, + Memory: 128, + }, } + url := "http://127.0.0.1:8080/invoke/" + fn.ID + ls := logs.NewMock() a := New(NewDirectCallDataAccess(ls, new(mqs.Mock))) defer checkClose(t, a) @@ -620,7 +560,7 @@ func TestTmpFsRW(t *testing.T) { } var out bytes.Buffer - callI, err := a.GetCall(FromRequest(app, route, req), WithWriter(&out)) + callI, err := a.GetCall(FromHTTPFnRequest(app, fn, req), WithWriter(&out)) if err != nil { t.Fatal(err) } @@ -682,29 +622,26 @@ func TestTmpFsRW(t *testing.T) { func TestTmpFsSize(t *testing.T) { appName := "myapp" - path := "/hello" - url := "http://127.0.0.1:8080/r/" + appName + path - app := &models.App{ID: "app_id", Name: appName} - route := &models.Route{ - Path: path, - AppID: app.ID, - Image: "fnproject/fn-test-utils", - Type: "sync", - Format: "http", // this _is_ the test - Timeout: 5, - IdleTimeout: 10, - Memory: 64, - TmpFsSize: 1, + fn := &models.Fn{ + ID: "fn_id", + AppID: app.ID, + Image: "fnproject/fn-test-utils", + Format: "http", // this _is_ the test + ResourceConfig: models.ResourceConfig{Timeout: 5, + IdleTimeout: 10, + Memory: 64, + }, } + url := "http://127.0.0.1:8080/invoke/" + fn.ID cfg, err := NewConfig() if err != nil { t.Fatal(err) } - cfg.MaxTmpFsInodes = 1024 + cfg.MaxTmpFsInodes = 1025 ls := logs.NewMock() a := New(NewDirectCallDataAccess(ls, new(mqs.Mock)), WithConfig(cfg)) @@ -713,13 +650,15 @@ func TestTmpFsSize(t *testing.T) { // Here we tell fn-test-utils to read file /proc/mounts and create a /tmp/salsa of 4MB bodOne := `{"readFile":"/proc/mounts", "createFile":"/tmp/salsa", "createFileSize": 4194304, "isDebug": true}` - req, err := http.NewRequest("GET", url, &dummyReader{Reader: strings.NewReader(bodOne)}) + req, err := http.NewRequest("POST", url, &dummyReader{Reader: strings.NewReader(bodOne)}) if err != nil { t.Fatal("unexpected error building request", err) } var out bytes.Buffer - callI, err := a.GetCall(FromRequest(app, route, req), WithWriter(&out)) + callI, err := a.GetCall(FromHTTPFnRequest(app, fn, req), WithWriter(&out)) + + callI.Model().TmpFsSize = 1 if err != nil { t.Fatal(err) } @@ -760,7 +699,7 @@ func TestTmpFsSize(t *testing.T) { opts := tokens[3] // rw tmp dir with size and inode limits applied. - if point == "/tmp" && opts == "rw,nosuid,nodev,noexec,relatime,size=1024k,nr_inodes=1024" { + if point == "/tmp" && opts == "rw,nosuid,nodev,noexec,relatime,size=1024k,nr_inodes=1025" { // good isFound = true } else if point == "/" && strings.HasPrefix(opts, "ro,") { @@ -781,32 +720,26 @@ func TestTmpFsSize(t *testing.T) { // return a model with all fields filled in with fnproject/fn-test-utils:latest image, change as needed func testCall() *models.Call { - appName := "myapp" - path := "/" image := "fnproject/fn-test-utils:latest" - app := &models.App{ID: "app_id", Name: appName} + fn := &models.Fn{ID: "fn_id"} const timeout = 10 const idleTimeout = 20 const memory = 256 - CPUs := models.MilliCPUs(200) method := "GET" - url := "http://127.0.0.1:8080/r/" + appName + path + url := "http://127.0.0.1:8080/invoke/" + fn.ID payload := "payload" typ := "sync" format := "http" contentType := "suberb_type" contentLength := strconv.FormatInt(int64(len(payload)), 10) config := map[string]string{ - "FN_FORMAT": format, - "FN_APP_NAME": appName, - "FN_PATH": path, - "FN_MEMORY": strconv.Itoa(memory), - "FN_CPUS": CPUs.String(), - "FN_TYPE": typ, - "APP_VAR": "FOO", - "ROUTE_VAR": "BAR", - "DOUBLE_VAR": "BIZ, BAZ", + "FN_FORMAT": format, + "FN_MEMORY": strconv.Itoa(memory), + "FN_TYPE": typ, + "APP_VAR": "FOO", + "FN_VAR": "BAR", + "DOUBLE_VAR": "BIZ, BAZ", } headers := map[string][]string{ // FromRequest would insert these from original HTTP request @@ -815,18 +748,15 @@ func testCall() *models.Call { } return &models.Call{ - AppID: app.ID, - AppName: app.Name, + FnID: fn.ID, Config: config, Headers: headers, - Path: path, Image: image, Type: typ, Format: format, Timeout: timeout, IdleTimeout: idleTimeout, Memory: memory, - CPUs: CPUs, Payload: payload, URL: url, Method: method, @@ -859,17 +789,18 @@ func TestPipesAreClear(t *testing.T) { ca.Format = "http" ca.IdleTimeout = 60 // keep this bad boy alive ca.Timeout = 4 // short - app := &models.App{Name: "myapp", ID: ca.AppID} + app := &models.App{ID: ca.AppID} - route := &models.Route{ - Path: ca.Path, - AppID: ca.AppID, - Image: ca.Image, - Type: ca.Type, - Format: ca.Format, - Timeout: ca.Timeout, - IdleTimeout: ca.IdleTimeout, - Memory: ca.Memory, + fn := &models.Fn{ + AppID: ca.AppID, + ID: ca.FnID, + Image: ca.Image, + Format: ca.Format, + ResourceConfig: models.ResourceConfig{ + Timeout: ca.Timeout, + IdleTimeout: ca.IdleTimeout, + Memory: ca.Memory, + }, } ls := logs.NewMock() @@ -892,7 +823,7 @@ func TestPipesAreClear(t *testing.T) { req.Header.Set("Content-Length", fmt.Sprintf("%d", len(bodOne))) var outOne bytes.Buffer - callI, err := a.GetCall(FromRequest(app, route, req), WithWriter(&outOne)) + callI, err := a.GetCall(FromHTTPFnRequest(app, fn, req), WithWriter(&outOne)) if err != nil { t.Fatal(err) } @@ -926,7 +857,7 @@ func TestPipesAreClear(t *testing.T) { req.Header.Set("Content-Length", fmt.Sprintf("%d", len(bodTwo))) var outTwo bytes.Buffer - callI, err = a.GetCall(FromRequest(app, route, req), WithWriter(&outTwo)) + callI, err = a.GetCall(FromHTTPFnRequest(app, fn, req), WithWriter(&outTwo)) if err != nil { t.Fatal(err) } @@ -1007,15 +938,16 @@ func TestPipesDontMakeSpuriousCalls(t *testing.T) { app.ID = call.AppID - route := &models.Route{ - Path: call.Path, - AppID: call.AppID, - Image: call.Image, - Type: call.Type, - Format: call.Format, - Timeout: call.Timeout, - IdleTimeout: call.IdleTimeout, - Memory: call.Memory, + fn := &models.Fn{ + ID: "fn_id", + AppID: call.AppID, + Image: call.Image, + Format: call.Format, + ResourceConfig: models.ResourceConfig{ + Timeout: call.Timeout, + IdleTimeout: call.IdleTimeout, + Memory: call.Memory, + }, } ls := logs.NewMock() @@ -1029,7 +961,7 @@ func TestPipesDontMakeSpuriousCalls(t *testing.T) { } var outOne bytes.Buffer - callI, err := a.GetCall(FromRequest(app, route, req), WithWriter(&outOne)) + callI, err := a.GetCall(FromHTTPFnRequest(app, fn, req), WithWriter(&outOne)) if err != nil { t.Fatal(err) } @@ -1054,7 +986,7 @@ func TestPipesDontMakeSpuriousCalls(t *testing.T) { } var outTwo bytes.Buffer - callI, err = a.GetCall(FromRequest(app, route, req), WithWriter(&outTwo)) + callI, err = a.GetCall(FromHTTPFnRequest(app, fn, req), WithWriter(&outTwo)) if err != nil { t.Fatal(err) } @@ -1100,15 +1032,16 @@ func TestNBIOResourceTracker(t *testing.T) { app.ID = call.AppID - route := &models.Route{ - Path: call.Path, - AppID: call.AppID, - Image: call.Image, - Type: call.Type, - Format: call.Format, - Timeout: call.Timeout, - IdleTimeout: call.IdleTimeout, - Memory: call.Memory, + fn := &models.Fn{ + ID: call.FnID, + AppID: call.AppID, + Image: call.Image, + Format: call.Format, + ResourceConfig: models.ResourceConfig{ + Timeout: call.Timeout, + IdleTimeout: call.IdleTimeout, + Memory: call.Memory, + }, } cfg, err := NewConfig() @@ -1135,7 +1068,7 @@ func TestNBIOResourceTracker(t *testing.T) { } var outOne bytes.Buffer - callI, err := a.GetCall(FromRequest(app, route, req), WithWriter(&outOne)) + callI, err := a.GetCall(FromHTTPFnRequest(app, fn, req), WithWriter(&outOne)) if err != nil { t.Fatal(err) } diff --git a/api/agent/async.go b/api/agent/async.go index 7e7cbe3be..4e86f8d1b 100644 --- a/api/agent/async.go +++ b/api/agent/async.go @@ -83,17 +83,17 @@ func (a *agent) asyncRun(ctx context.Context, model *models.Call) { // since async doesn't come in through the normal request path, // we've gotta add tags here for stats to come out properly. - appKey, err := tag.NewKey("fn_appname") + appKey, err := tag.NewKey("app_id") if err != nil { logrus.Fatal(err) } - pathKey, err := tag.NewKey("fn_path") + fnKey, err := tag.NewKey("fn_id") if err != nil { logrus.Fatal(err) } ctx, err = tag.New(ctx, tag.Insert(appKey, model.AppID), - tag.Insert(pathKey, model.Path), + tag.Insert(fnKey, model.FnID), ) if err != nil { logrus.Fatal(err) diff --git a/api/agent/call.go b/api/agent/call.go index bfd17264b..068505a8c 100644 --- a/api/agent/call.go +++ b/api/agent/call.go @@ -52,88 +52,6 @@ const ( invokePath = "/invoke" ) -// FromRequest initialises a call to a route from an HTTP request -// deprecate with routes -func FromRequest(app *models.App, route *models.Route, req *http.Request) CallOpt { - return func(c *call) error { - ctx := req.Context() - - log := common.Logger(ctx) - // Check whether this is a CloudEvent, if coming in via HTTP router (only way currently), then we'll look for a special header - // Content-Type header: https://github.com/cloudevents/spec/blob/master/http-transport-binding.md#32-structured-content-mode - // Expected Content-Type for a CloudEvent: application/cloudevents+json; charset=UTF-8 - contentType := req.Header.Get("Content-Type") - t, _, err := mime.ParseMediaType(contentType) - if err != nil { - // won't fail here, but log - log.Debugf("Could not parse Content-Type header: %v", err) - } else { - if t == ceMimeType { - c.IsCloudEvent = true - route.Format = models.FormatCloudEvent - } - } - - if route.Format == "" { - route.Format = models.FormatDefault - } - - id := id.New().String() - - // TODO this relies on ordering of opts, but tests make sure it works, probably re-plumb/destroy headers - // TODO async should probably supply an http.ResponseWriter that records the logs, to attach response headers to - if rw, ok := c.w.(http.ResponseWriter); ok { - rw.Header().Add("FN_CALL_ID", id) - for k, vs := range route.Headers { - for _, v := range vs { - // pre-write in these headers to response - rw.Header().Add(k, v) - } - } - } - - // this ensures that there is an image, path, timeouts, memory, etc are valid. - // NOTE: this means assign any changes above into route's fields - err = route.Validate() - if err != nil { - return err - } - - var syslogURL string - if app.SyslogURL != nil { - syslogURL = *app.SyslogURL - } - - c.Call = &models.Call{ - ID: id, - Path: route.Path, - Image: route.Image, - // Delay: 0, - Type: route.Type, - Format: route.Format, - // Payload: TODO, - Priority: new(int32), // TODO this is crucial, apparently - Timeout: route.Timeout, - IdleTimeout: route.IdleTimeout, - TmpFsSize: route.TmpFsSize, - Memory: route.Memory, - CPUs: route.CPUs, - Config: buildConfig(app, route), - Annotations: app.Annotations.MergeChange(route.Annotations), - Headers: req.Header, - CreatedAt: common.DateTime(time.Now()), - URL: reqURL(req), - Method: req.Method, - AppID: app.ID, - AppName: app.Name, - SyslogURL: syslogURL, - } - - c.req = req - return nil - } -} - // Sets up a call from an http trigger request func FromHTTPTriggerRequest(app *models.App, fn *models.Fn, trigger *models.Trigger, req *http.Request) CallOpt { return func(c *call) error { @@ -174,7 +92,6 @@ func FromHTTPTriggerRequest(app *models.App, fn *models.Fn, trigger *models.Trig c.Call = &models.Call{ ID: id, - Path: trigger.Source, Image: fn.Image, // Delay: 0, Type: "sync", @@ -186,7 +103,7 @@ func FromHTTPTriggerRequest(app *models.App, fn *models.Fn, trigger *models.Trig TmpFsSize: 0, // TODO clean up this Memory: fn.Memory, CPUs: 0, // TODO clean up this - Config: buildConfigWithPath(app, fn, trigger.Source), + Config: buildConfig(app, fn, trigger.Source), // TODO - this wasn't really the intention here (that annotations would naturally cascade // but seems to be necessary for some runner behaviour Annotations: app.Annotations.MergeChange(fn.Annotations).MergeChange(trigger.Annotations), @@ -246,7 +163,6 @@ func FromHTTPFnRequest(app *models.App, fn *models.Fn, req *http.Request) CallOp c.Call = &models.Call{ ID: id, - Path: invokePath, Image: fn.Image, // Delay: 0, Type: "sync", @@ -258,7 +174,7 @@ func FromHTTPFnRequest(app *models.App, fn *models.Fn, req *http.Request) CallOp TmpFsSize: 0, // TODO clean up this Memory: fn.Memory, CPUs: 0, // TODO clean up this - Config: buildConfigWithPath(app, fn, invokePath), + Config: buildConfig(app, fn, invokePath), // TODO - this wasn't really the intention here (that annotations would naturally cascade // but seems to be necessary for some runner behaviour Annotations: app.Annotations.MergeChange(fn.Annotations), @@ -277,31 +193,7 @@ func FromHTTPFnRequest(app *models.App, fn *models.Fn, req *http.Request) CallOp } } -func buildConfig(app *models.App, route *models.Route) models.Config { - conf := make(models.Config, 8+len(app.Config)+len(route.Config)) - for k, v := range app.Config { - conf[k] = v - } - for k, v := range route.Config { - conf[k] = v - } - - conf["FN_FORMAT"] = route.Format - conf["FN_APP_NAME"] = app.Name - conf["FN_PATH"] = route.Path - // TODO: might be a good idea to pass in: "FN_BASE_PATH" = fmt.Sprintf("/r/%s", appName) || "/" if using DNS entries per app - conf["FN_MEMORY"] = fmt.Sprintf("%d", route.Memory) - conf["FN_TYPE"] = route.Type - conf["FN_TMPSIZE"] = fmt.Sprintf("%d", route.TmpFsSize) - - CPUs := route.CPUs.String() - if CPUs != "" { - conf["FN_CPUS"] = CPUs - } - return conf -} - -func buildConfigWithPath(app *models.App, fn *models.Fn, path string) models.Config { +func buildConfig(app *models.App, fn *models.Fn, path string) models.Config { conf := make(models.Config, 8+len(app.Config)+len(fn.Config)) for k, v := range app.Config { conf[k] = v @@ -450,7 +342,7 @@ func (a *agent) GetCall(opts ...CallOpt) (Call, error) { func setupCtx(c *call) { ctx, _ := common.LoggerWithFields(c.req.Context(), - logrus.Fields{"id": c.ID, "app_id": c.AppID, "route": c.Path}) + logrus.Fields{"id": c.ID, "app_id": c.AppID, "fn_id": c.FnID}) c.req = c.req.WithContext(ctx) } diff --git a/api/agent/data_access.go b/api/agent/data_access.go index 5536e4ce4..bc661822b 100644 --- a/api/agent/data_access.go +++ b/api/agent/data_access.go @@ -18,8 +18,6 @@ type ReadDataAccess interface { GetAppByID(ctx context.Context, appID string) (*models.App, error) GetTriggerBySource(ctx context.Context, appId string, triggerType, source string) (*models.Trigger, error) GetFnByID(ctx context.Context, fnId string) (*models.Fn, error) - // GetRoute abstracts querying the datastore for a route within an app. - GetRoute(ctx context.Context, appID string, routePath string) (*models.Route, error) } //DequeueDataAccess abstracts an underlying dequeue for async runners @@ -55,7 +53,7 @@ type DataAccess interface { CallHandler } -// CachedDataAccess wraps a DataAccess and caches the results of GetApp and GetRoute. +// CachedDataAccess wraps a DataAccess and caches the results of GetApp. type cachedDataAccess struct { ReadDataAccess @@ -71,10 +69,6 @@ func NewCachedDataAccess(da ReadDataAccess) ReadDataAccess { return cda } -func routeCacheKey(app, path string) string { - return "r:" + app + "\x00" + path -} - func appIDCacheKey(appID string) string { return "a:" + appID } @@ -103,26 +97,6 @@ func (da *cachedDataAccess) GetAppByID(ctx context.Context, appID string) (*mode return app.(*models.App), nil } -func (da *cachedDataAccess) GetRoute(ctx context.Context, appID string, routePath string) (*models.Route, error) { - key := routeCacheKey(appID, routePath) - r, ok := da.cache.Get(key) - if ok { - return r.(*models.Route), nil - } - - resp, err := da.singleflight.Do(key, - func() (interface{}, error) { - return da.ReadDataAccess.GetRoute(ctx, appID, routePath) - }) - - if err != nil { - return nil, err - } - r = resp.(*models.Route) - da.cache.Set(key, r, cache.DefaultExpiration) - return r.(*models.Route), nil -} - type directDataAccess struct { mq models.MessageQueue ls models.LogStore diff --git a/api/agent/drivers/docker/docker.go b/api/agent/drivers/docker/docker.go index c0a47a752..eb8929c02 100644 --- a/api/agent/drivers/docker/docker.go +++ b/api/agent/drivers/docker/docker.go @@ -635,7 +635,7 @@ func (w *waitResult) wait(ctx context.Context) (status string, err error) { return drivers.StatusSuccess, nil case 137: // OOM common.Logger(ctx).Error("docker oom") - err := errors.New("container out of memory, you may want to raise route.memory for this route (default: 128MB)") + err := errors.New("container out of memory, you may want to raise fn.memory for this function (default: 128MB)") return drivers.StatusKilled, models.NewAPIError(http.StatusBadGateway, err) } } diff --git a/api/agent/func_logger.go b/api/agent/func_logger.go index 8e057f26d..cb233995b 100644 --- a/api/agent/func_logger.go +++ b/api/agent/func_logger.go @@ -46,7 +46,7 @@ func setupLogger(ctx context.Context, maxSize uint64, debug bool, c *models.Call if debug { // accumulate all line writers, wrap in same line writer (to re-use buffer) - stderrLogger := common.Logger(ctx).WithFields(logrus.Fields{"user_log": true, "app_id": c.AppID, "path": c.Path, "image": c.Image, "call_id": c.ID}) + stderrLogger := common.Logger(ctx).WithFields(logrus.Fields{"user_log": true, "app_id": c.AppID, "fn_id": c.FnID, "image": c.Image, "call_id": c.ID}) loggo := &nopCloser{&logWriter{stderrLogger}} // we don't need to limit the log writer(s), but we do need it to dispense lines diff --git a/api/agent/hybrid/client.go b/api/agent/hybrid/client.go index 63eaf4685..c95d0b064 100644 --- a/api/agent/hybrid/client.go +++ b/api/agent/hybrid/client.go @@ -146,16 +146,6 @@ func (cl *client) GetAppByID(ctx context.Context, appID string) (*models.App, er return &a, err } -func (cl *client) GetRoute(ctx context.Context, appID, route string) (*models.Route, error) { - ctx, span := trace.StartSpan(ctx, "hybrid_client_get_route") - defer span.End() - - // TODO trim prefix is pretty odd here eh? - var r = models.Route{} - err := cl.do(ctx, nil, &r, "GET", noQuery, "runner", "apps", appID, "routes", strings.TrimPrefix(route, "/")) - return &r, err -} - func (cl *client) GetTriggerBySource(ctx context.Context, appID string, triggerType, source string) (*models.Trigger, error) { ctx, span := trace.StartSpan(ctx, "hybrid_client_get_trigger_by_source") defer span.End() diff --git a/api/agent/hybrid/nop.go b/api/agent/hybrid/nop.go index f20d0bec5..d1f297cfe 100644 --- a/api/agent/hybrid/nop.go +++ b/api/agent/hybrid/nop.go @@ -65,12 +65,6 @@ func (cl *nopDataStore) Finish(ctx context.Context, c *models.Call, r io.Reader, return nil // It's ok to call this method, and it does no operations } -func (cl *nopDataStore) GetRoute(ctx context.Context, appName, route string) (*models.Route, error) { - ctx, span := trace.StartSpan(ctx, "nop_datastore_get_route") - defer span.End() - return nil, errors.New("Should not call GetRoute on a NOP data store") -} - func (cl *nopDataStore) Close() error { return nil } diff --git a/api/agent/protocol/json.go b/api/agent/protocol/json.go index 5c1dd6d37..d6734f461 100644 --- a/api/agent/protocol/json.go +++ b/api/agent/protocol/json.go @@ -126,7 +126,7 @@ func (h *JSONProtocol) Dispatch(ctx context.Context, ci CallInfo, w io.Writer) e p := jout.Protocol for k, v := range p.Headers { for _, vv := range v { - rw.Header().Add(k, vv) // on top of any specified on the route + rw.Header().Add(k, vv) // on top of any specified on the fn } } } diff --git a/api/agent/pure_runner.go b/api/agent/pure_runner.go index ed5108001..8fdd54240 100644 --- a/api/agent/pure_runner.go +++ b/api/agent/pure_runner.go @@ -581,7 +581,7 @@ func (pr *pureRunner) handleTryCall(tc *runner.TryCall, state *callHandle) error // Status image is reserved for internal Status checks. // We need to make sure normal functions calls cannot call it. if pr.status.imageName != "" && c.Image == pr.status.imageName { - err = models.ErrRoutesInvalidImage + err = models.ErrFnsInvalidImage state.enqueueCallResponse(err) return err } @@ -699,7 +699,6 @@ func (pr *pureRunner) runStatusCall(ctx context.Context) *runner.RunnerStatus { // Most of these arguments are baked in. We might want to make this // more configurable. c.ID = id.New().String() - c.Path = "/" c.Image = pr.status.imageName c.Type = "sync" c.Format = "json" diff --git a/api/agent/slots.go b/api/agent/slots.go index 085db61cc..d782edc2d 100644 --- a/api/agent/slots.go +++ b/api/agent/slots.go @@ -274,8 +274,8 @@ func (a *slotQueueMgr) deleteSlotQueue(slots *slotQueue) bool { var shapool = &sync.Pool{New: func() interface{} { return sha256.New() }} -// TODO do better; once we have app+route versions this function -// can be simply app+route names & version +// TODO do better; once we have app+fn versions this function +// can be simply app+fn ids & version func getSlotQueueKey(call *call) string { // return a sha256 hash of a (hopefully) unique string of all the config // values, to make map lookups quicker [than the giant unique string] @@ -288,7 +288,7 @@ func getSlotQueueKey(call *call) string { hash.Write(unsafeBytes("\x00")) hash.Write(unsafeBytes(call.SyslogURL)) hash.Write(unsafeBytes("\x00")) - hash.Write(unsafeBytes(call.Path)) + hash.Write(unsafeBytes(call.FnID)) hash.Write(unsafeBytes("\x00")) hash.Write(unsafeBytes(call.Image)) hash.Write(unsafeBytes("\x00")) diff --git a/api/agent/slots_test.go b/api/agent/slots_test.go index 69d5dd137..b61c92fcd 100644 --- a/api/agent/slots_test.go +++ b/api/agent/slots_test.go @@ -268,32 +268,30 @@ func TestSlotQueueBasic3(t *testing.T) { func BenchmarkSlotKey(b *testing.B) { appName := "myapp" appID := id.New().String() - path := "/" + fnID := id.New().String() image := "fnproject/fn-test-utils" const timeout = 1 const idleTimeout = 20 const memory = 256 CPUs := models.MilliCPUs(1000) method := "GET" - url := "http://127.0.0.1:8080/r/" + appName + path + url := "http://127.0.0.1:8080/invoke/" + fnID payload := "payload" typ := "sync" format := "default" cfg := models.Config{ "FN_FORMAT": format, "FN_APP_NAME": appName, - "FN_PATH": path, "FN_MEMORY": strconv.Itoa(memory), "FN_CPUS": CPUs.String(), "FN_TYPE": typ, "APP_VAR": "FOO", - "ROUTE_VAR": "BAR", } cm := &models.Call{ Config: cfg, AppID: appID, - Path: path, + FnID: fnID, Image: image, Type: typ, Format: format, diff --git a/api/const.go b/api/const.go index e1da65435..dbeba3d82 100644 --- a/api/const.go +++ b/api/const.go @@ -5,19 +5,15 @@ const ( AppName string = "app_name" // AppID is the app id context key AppID string = "app_id" - // Path is a route's path context key - Path string = "path" // ParamAppID is the url path parameter for app id ParamAppID string = "appID" // ParamAppName is the url path parameter for app name ParamAppName string = "appName" - // ParamRouteName is the url path parameter for route name - ParamRouteName string = "route" // ParamTriggerID is the url path parameter for trigger id ParamTriggerID string = "triggerID" // ParamCallID is the url path parameter for call id - ParamCallID string = "call" + ParamCallID string = "callID" // ParamFnID is the url path parameter for fn id ParamFnID string = "fnID" // ParamTriggerSource is the triggers source parameter diff --git a/api/datastore/datastoretest/test.go b/api/datastore/datastoretest/test.go index f865bd085..4b836d5fe 100644 --- a/api/datastore/datastoretest/test.go +++ b/api/datastore/datastoretest/test.go @@ -14,7 +14,6 @@ import ( "testing" "time" - "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/models" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" @@ -39,8 +38,6 @@ type ResourceProvider interface { ValidApp() *models.App // ValidFn returns a valid fn to use for inserts ValidFn(appId string) *models.Fn - // ValidFn returns a valid fn to use for inserts - ValidRoute(appId string) *models.Route // ValidTrigger returns a valid trigger to use for inserts ValidTrigger(appId string, fnId string) *models.Trigger @@ -94,21 +91,6 @@ func (brp *BasicResourceProvider) ValidTrigger(appId, funcId string) *models.Tri return trigger } -// Creates a valid route which always has a sequential named -func (brp *BasicResourceProvider) ValidRoute(appId string) *models.Route { - testRoute := &models.Route{ - AppID: appId, - Path: fmt.Sprintf("/test_%09d", brp.NextID()), - Image: "fnproject/fn-test-utils", - Type: "sync", - Format: "http", - Timeout: models.DefaultTimeout, - IdleTimeout: models.DefaultIdleTimeout, - Memory: models.DefaultMemory, - } - return testRoute -} - func (brp *BasicResourceProvider) ValidFn(appId string) *models.Fn { return &models.Fn{ AppID: appId, @@ -140,15 +122,6 @@ func (h *Harness) GivenAppInDb(app *models.App) *models.App { return a } -func (h *Harness) GivenRouteInDb(rt *models.Route) *models.Route { - r, err := h.ds.InsertRoute(h.ctx, rt) - if err != nil { - h.t.Fatal("failed to create rt", err) - return nil - } - return r -} - func (h *Harness) Cleanup() { for _, appId := range h.appIds { err := h.ds.RemoveApp(h.ctx, appId) @@ -208,12 +181,6 @@ func (f TriggerByName) Len() int { return len(f) } func (f TriggerByName) Swap(i, j int) { f[i], f[j] = f[j], f[i] } func (f TriggerByName) Less(i, j int) bool { return f[i].Name < f[j].Name } -type RouteByPath []*models.Route - -func (f RouteByPath) Len() int { return len(f) } -func (f RouteByPath) Swap(i, j int) { f[i], f[j] = f[j], f[i] } -func (f RouteByPath) Less(i, j int) bool { return f[i].Path < f[j].Path } - func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { ds := dsf(t) @@ -534,343 +501,6 @@ func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { } -func RunRoutesTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { - - ds := dsf(t) - ctx := rp.DefaultCtx() - - t.Run("routes", func(t *testing.T) { - - t.Run("empty val", func(t *testing.T) { - _, err := ds.InsertRoute(ctx, nil) - if err != models.ErrDatastoreEmptyRoute { - t.Fatalf("expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyRoute, err) - } - - }) - - t.Run("Insert with non-existant app ", func(t *testing.T) { - - newTestRoute := rp.ValidRoute("notreal") - _, err := ds.InsertRoute(ctx, newTestRoute) - if err != models.ErrAppsNotFound { - t.Fatalf("expected error `%v`, but it was `%v`", models.ErrAppsNotFound, err) - } - }) - - t.Run("Insert duplicate route fails", func(t *testing.T) { - h := NewHarness(t, ctx, ds) - defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) - testRoute := rp.ValidRoute(testApp.ID) - h.GivenRouteInDb(testRoute) - - _, err := ds.InsertRoute(ctx, testRoute) - if err != models.ErrRoutesAlreadyExists { - t.Fatalf("expected error to be `%v`, but it was `%v`", models.ErrRoutesAlreadyExists, err) - } - }) - - // Testing get - t.Run("get route with empty path", func(t *testing.T) { - _, err := ds.GetRoute(ctx, id.New().String(), "") - if err != models.ErrRoutesMissingPath { - t.Fatalf("expected error `%v`, but it was `%v`", models.ErrRoutesMissingPath, err) - } - }) - - t.Run("get route with empty app id", func(t *testing.T) { - - _, err := ds.GetRoute(ctx, "", "a") - if err != models.ErrRoutesMissingAppID { - t.Fatalf("expected error `%v`, but it was `%v`", models.ErrRoutesMissingAppID, err) - } - }) - t.Run("get valid route", func(t *testing.T) { - h := NewHarness(t, ctx, ds) - defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) - testRoute := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) - - route, err := ds.GetRoute(ctx, testApp.ID, testRoute.Path) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - if !route.Equals(testRoute) { - t.Fatalf("expected to insert:\n%v\nbut got:\n%v", testRoute, *route) - } - }) - - // Testing update - t.Run("update route set headers and config", func(t *testing.T) { - h := NewHarness(t, ctx, ds) - defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) - testRoute := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) - - // Update some fields, and add 3 configs and 3 headers. - updated, err := ds.UpdateRoute(ctx, &models.Route{ - AppID: testApp.ID, - Path: testRoute.Path, - Timeout: 2, - Config: map[string]string{ - "FIRST": "1", - "SECOND": "2", - "THIRD": "3", - }, - Headers: models.Headers{ - "First": []string{"test"}, - "Second": []string{"test", "test"}, - "Third": []string{"test", "test2"}, - }, - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - expected := &models.Route{ - // unchanged - AppID: testApp.ID, - Path: testRoute.Path, - Image: "fnproject/fn-test-utils", - Type: "sync", - Format: "http", - IdleTimeout: testRoute.IdleTimeout, - Memory: testRoute.Memory, - CPUs: testRoute.CPUs, - // updated - Timeout: 2, - Config: map[string]string{ - "FIRST": "1", - "SECOND": "2", - "THIRD": "3", - }, - Headers: models.Headers{ - "First": []string{"test"}, - "Second": []string{"test", "test"}, - "Third": []string{"test", "test2"}, - }, - } - if !updated.Equals(expected) { - t.Fatalf("expected updated `%v` but got `%v`", expected, updated) - } - }) - - t.Run("update route modify headers and config", func(t *testing.T) { - h := NewHarness(t, ctx, ds) - defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) - testRoute := rp.ValidRoute(testApp.ID) - testRoute.Config = map[string]string{ - "FIRST": "1", - "SECOND": "2", - "THIRD": "3", - } - testRoute.Headers = models.Headers{ - "First": []string{"test"}, - "Second": []string{"test", "test"}, - "Third": []string{"test", "test2"}, - } - testRoute.Timeout = 2 - h.GivenRouteInDb(testRoute) - - // Update a config var, remove another. Add one Header, remove another. - updated, err := ds.UpdateRoute(ctx, &models.Route{ - AppID: testRoute.AppID, - Path: testRoute.Path, - Config: map[string]string{ - "FIRST": "first", - "SECOND": "", - "THIRD": "3", - }, - Headers: models.Headers{ - "First": []string{"test2"}, - "Second": nil, - }, - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - expected := &models.Route{ - // unchanged - AppID: testRoute.AppID, - Path: testRoute.Path, - Image: "fnproject/fn-test-utils", - Type: "sync", - Format: "http", - Timeout: 2, - Memory: testRoute.Memory, - CPUs: testRoute.CPUs, - IdleTimeout: testRoute.IdleTimeout, - // updated - Config: map[string]string{ - "FIRST": "first", - "THIRD": "3", - }, - Headers: models.Headers{ - "First": []string{"test2"}, - "Third": []string{"test", "test2"}, - }, - } - if !updated.Equals(expected) { - t.Fatalf("expected updated:\n`%#v`\nbut got:\n`%#v`", expected, updated) - } - }) - - t.Run("simple pagination", func(t *testing.T) { - h := NewHarness(t, ctx, ds) - defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) - testRoute := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) - - // Testing list fns - routes, err := ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 1}) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - if len(routes) == 0 { - t.Fatal("expected result count to be greater than 0") - } - if routes[0] == nil { - t.Fatalf("expected non-nil route") - } else if routes[0].Path != testRoute.Path { - t.Fatalf("expected `app.Name` to be `%s` but it was `%s`", testRoute.Path, routes[0].Path) - } - - routes, err = ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{Image: testRoute.Image, PerPage: 1}) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - if len(routes) == 0 { - t.Fatal("expected result count to be greater than 0") - } - if routes[0] == nil { - t.Fatalf("expected non-nil route") - } else if routes[0].Path != testRoute.Path { - t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", testRoute.Path, routes[0].Path) - } - - }) - - t.Run("pagination on empty app is invalid", func(t *testing.T) { - _, err := ds.GetRoutesByApp(ctx, "", &models.RouteFilter{PerPage: 1}) - if err != models.ErrRoutesMissingAppID { - t.Fatalf("Expecting app ID error, got %s", err) - } - }) - - t.Run("pagination on non-existant app returns no routes", func(t *testing.T) { - routes, err := ds.GetRoutesByApp(ctx, id.New().String(), &models.RouteFilter{PerPage: 1}) - if err != nil { - t.Fatalf("error: %s", err) - } - if len(routes) != 0 { - t.Fatalf("expected result count to be 0 but got %d", len(routes)) - } - }) - - t.Run("pagination on routes return routes in order ", func(t *testing.T) { - h := NewHarness(t, ctx, ds) - defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) - - r1 := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) - r2 := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) - r3 := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) - - gendRoutes := []*models.Route{r1, r2, r3} - sort.Sort(RouteByPath(gendRoutes)) - - routes, err := ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 1}) - if err != nil { - t.Fatalf("error: %s", err) - } - if len(routes) != 1 { - t.Fatalf("expected result count to be 1 but got %d", len(routes)) - } else if routes[0].Path != gendRoutes[0].Path { - t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", gendRoutes[0].Path, routes[0].Path) - } - - routes, err = ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 2, Cursor: routes[0].Path}) - if err != nil { - t.Fatalf("error: %s", err) - } - - if len(routes) != 2 { - t.Fatalf("expected result count to be 2 but got %d", len(routes)) - } else if routes[0].Path != gendRoutes[1].Path { - t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", gendRoutes[1].Path, routes[0].Path) - } else if routes[1].Path != gendRoutes[2].Path { - t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", gendRoutes[2].Path, routes[1].Path) - } - - r4 := rp.ValidRoute(testApp.ID) - r4.Path = "/abcdefg" // < /test lexicographically, but not in length - - h.GivenRouteInDb(r4) - - routes, err = ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 100}) - if err != nil { - t.Fatalf("error: %s", err) - } - if len(routes) != 4 { - t.Fatalf("expected result count to be 4 but got %d", len(routes)) - } else if routes[0].Path != r4.Path { - t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", r4.Path, routes[0].Path) - } - }) - - t.Run("remove route with empty app ID", func(t *testing.T) { - - // Testing route delete - err := ds.RemoveRoute(ctx, "", "") - if err != models.ErrRoutesMissingAppID { - t.Fatalf("expected error `%v`, but it was `%v`", models.ErrRoutesMissingAppID, err) - } - - }) - - t.Run("remove route with empty app path", func(t *testing.T) { - h := NewHarness(t, ctx, ds) - defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) - - err := ds.RemoveRoute(ctx, testApp.ID, "") - if err != models.ErrRoutesMissingPath { - t.Fatalf("expected error `%v`, but it was `%v`", models.ErrRoutesMissingPath, err) - } - }) - - t.Run("remove valid route removes route ", func(t *testing.T) { - h := NewHarness(t, ctx, ds) - defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) - testRoute := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) - - err := ds.RemoveRoute(ctx, testApp.ID, testRoute.Path) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - route, err := ds.GetRoute(ctx, testApp.ID, testRoute.Path) - if err != nil && err != models.ErrRoutesNotFound { - t.Fatalf("expected error `%v`, but it was `%v`", models.ErrRoutesNotFound, err) - } - if route != nil { - t.Fatalf("failed to remove the route: %v", route) - } - - _, err = ds.UpdateRoute(ctx, &models.Route{ - AppID: testApp.ID, - Path: testRoute.Path, - Image: "test", - }) - if err != models.ErrRoutesNotFound { - t.Fatalf("expected error to be `%v`, but it was `%v`", models.ErrRoutesNotFound, err) - } - }) - }) -} - func RunFnsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { ds := dsf(t) @@ -1593,7 +1223,6 @@ func RunAllTests(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { }() RunAppsTest(t, dsf, rp) - RunRoutesTest(t, dsf, rp) RunFnsTest(t, dsf, rp) RunTriggersTest(t, dsf, rp) RunTriggerBySourceTests(t, dsf, rp) diff --git a/api/datastore/internal/datastoreutil/metrics.go b/api/datastore/internal/datastoreutil/metrics.go index 651495241..8d1211aab 100644 --- a/api/datastore/internal/datastoreutil/metrics.go +++ b/api/datastore/internal/datastoreutil/metrics.go @@ -58,36 +58,6 @@ func (m *metricds) RemoveApp(ctx context.Context, appID string) error { return m.ds.RemoveApp(ctx, appID) } -func (m *metricds) GetRoute(ctx context.Context, appID, routePath string) (*models.Route, error) { - ctx, span := trace.StartSpan(ctx, "ds_get_route") - defer span.End() - return m.ds.GetRoute(ctx, appID, routePath) -} - -func (m *metricds) GetRoutesByApp(ctx context.Context, appID string, filter *models.RouteFilter) (routes []*models.Route, err error) { - ctx, span := trace.StartSpan(ctx, "ds_get_routes_by_app") - defer span.End() - return m.ds.GetRoutesByApp(ctx, appID, filter) -} - -func (m *metricds) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - ctx, span := trace.StartSpan(ctx, "ds_insert_route") - defer span.End() - return m.ds.InsertRoute(ctx, route) -} - -func (m *metricds) UpdateRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - ctx, span := trace.StartSpan(ctx, "ds_update_route") - defer span.End() - return m.ds.UpdateRoute(ctx, route) -} - -func (m *metricds) RemoveRoute(ctx context.Context, appID string, routePath string) error { - ctx, span := trace.StartSpan(ctx, "ds_remove_route") - defer span.End() - return m.ds.RemoveRoute(ctx, appID, routePath) -} - func (m *metricds) InsertTrigger(ctx context.Context, trigger *models.Trigger) (*models.Trigger, error) { ctx, span := trace.StartSpan(ctx, "ds_insert_trigger") defer span.End() diff --git a/api/datastore/internal/datastoreutil/validator.go b/api/datastore/internal/datastoreutil/validator.go index c82e7759d..f2da3ad97 100644 --- a/api/datastore/internal/datastoreutil/validator.go +++ b/api/datastore/internal/datastoreutil/validator.go @@ -67,66 +67,6 @@ func (v *validator) RemoveApp(ctx context.Context, appID string) error { return v.Datastore.RemoveApp(ctx, appID) } -// appName and routePath will never be empty. -func (v *validator) GetRoute(ctx context.Context, appID, routePath string) (*models.Route, error) { - if appID == "" { - return nil, models.ErrRoutesMissingAppID - } - if routePath == "" { - return nil, models.ErrRoutesMissingPath - } - - return v.Datastore.GetRoute(ctx, appID, routePath) -} - -// appName will never be empty -func (v *validator) GetRoutesByApp(ctx context.Context, appID string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { - if appID == "" { - return nil, models.ErrRoutesMissingAppID - } - - return v.Datastore.GetRoutesByApp(ctx, appID, routeFilter) -} - -// route will never be nil and route's AppName and Path will never be empty. -func (v *validator) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - if route == nil { - return nil, models.ErrDatastoreEmptyRoute - } - - if err := route.Validate(); err != nil { - return nil, err - } - - return v.Datastore.InsertRoute(ctx, route) -} - -// route will never be nil and route's AppName and Path will never be empty. -func (v *validator) UpdateRoute(ctx context.Context, newroute *models.Route) (*models.Route, error) { - if newroute == nil { - return nil, models.ErrDatastoreEmptyRoute - } - if newroute.AppID == "" { - return nil, models.ErrRoutesMissingAppID - } - if newroute.Path == "" { - return nil, models.ErrRoutesMissingPath - } - return v.Datastore.UpdateRoute(ctx, newroute) -} - -// appName and routePath will never be empty. -func (v *validator) RemoveRoute(ctx context.Context, appID string, routePath string) error { - if appID == "" { - return models.ErrRoutesMissingAppID - } - if routePath == "" { - return models.ErrRoutesMissingPath - } - - return v.Datastore.RemoveRoute(ctx, appID, routePath) -} - func (v *validator) InsertTrigger(ctx context.Context, t *models.Trigger) (*models.Trigger, error) { if t.ID != "" { diff --git a/api/datastore/mock.go b/api/datastore/mock.go index 7d720b669..97e1b3c48 100644 --- a/api/datastore/mock.go +++ b/api/datastore/mock.go @@ -17,7 +17,6 @@ import ( type mock struct { Apps []*models.App - Routes []*models.Route Fns []*models.Fn Triggers []*models.Trigger @@ -47,8 +46,6 @@ func NewMockInit(args ...interface{}) models.Datastore { switch x := a.(type) { case []*models.App: mocker.Apps = x - case []*models.Route: - mocker.Routes = x case []*models.Fn: mocker.Fns = x case []*models.Trigger: @@ -167,8 +164,6 @@ func (m *mock) UpdateApp(ctx context.Context, app *models.App) (*models.App, err } func (m *mock) RemoveApp(ctx context.Context, appID string) error { - m.batchDeleteRoutes(ctx, appID) - for i, a := range m.Apps { if a.ID == appID { var newFns []*models.Fn @@ -198,95 +193,6 @@ func (m *mock) RemoveApp(ctx context.Context, appID string) error { return models.ErrAppsNotFound } -func (m *mock) GetRoute(ctx context.Context, appID, routePath string) (*models.Route, error) { - for _, r := range m.Routes { - if r.AppID == appID && r.Path == routePath { - return r.Clone(), nil - } - } - return nil, models.ErrRoutesNotFound -} - -type sortR []*models.Route - -func (s sortR) Len() int { return len(s) } -func (s sortR) Less(i, j int) bool { return strings.Compare(s[i].Path, s[j].Path) < 0 } -func (s sortR) Swap(i, j int) { s[i], s[j] = s[j], s[i] } - -func (m *mock) GetRoutesByApp(ctx context.Context, appID string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { - // sort them all first for cursoring (this is for testing, n is small & mock is not concurrent..) - sort.Sort(sortR(m.Routes)) - - for _, r := range m.Routes { - if len(routes) == routeFilter.PerPage { - break - } - - if r.AppID == appID && - //strings.HasPrefix(r.Path, routeFilter.PathPrefix) && // TODO - (routeFilter.Image == "" || routeFilter.Image == r.Image) && - strings.Compare(routeFilter.Cursor, r.Path) < 0 { - - routes = append(routes, r.Clone()) - } - } - return -} - -func (m *mock) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - - c := route.Clone() - c.SetDefaults() - c.CreatedAt = common.DateTime(time.Now()) - c.UpdatedAt = c.CreatedAt - - if _, err := m.GetAppByID(ctx, route.AppID); err != nil { - return nil, err - } - - if r, _ := m.GetRoute(ctx, route.AppID, route.Path); r != nil { - return nil, models.ErrRoutesAlreadyExists - } - m.Routes = append(m.Routes, c) - return c.Clone(), nil -} - -func (m *mock) UpdateRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - r, err := m.GetRoute(ctx, route.AppID, route.Path) - if err != nil { - return nil, err - } - clone := r.Clone() - clone.Update(route) - err = clone.Validate() - if err != nil { - return nil, err - } - r.Update(route) // only if validate works (pointer) - return clone, nil -} - -func (m *mock) RemoveRoute(ctx context.Context, appID, routePath string) error { - for i, r := range m.Routes { - if r.AppID == appID && r.Path == routePath { - m.Routes = append(m.Routes[:i], m.Routes[i+1:]...) - return nil - } - } - return models.ErrRoutesNotFound -} - -func (m *mock) batchDeleteRoutes(ctx context.Context, appID string) error { - var newRoutes []*models.Route - for _, c := range m.Routes { - if c.AppID != appID { - newRoutes = append(newRoutes, c) - } - } - m.Routes = newRoutes - return nil -} - func (m *mock) InsertFn(ctx context.Context, fn *models.Fn) (*models.Fn, error) { _, err := m.GetAppByID(ctx, fn.AppID) if err != nil { diff --git a/api/datastore/sql/migrations/20_drop_routes_table.go b/api/datastore/sql/migrations/20_drop_routes_table.go new file mode 100644 index 000000000..e980b5df6 --- /dev/null +++ b/api/datastore/sql/migrations/20_drop_routes_table.go @@ -0,0 +1,45 @@ +package migrations + +import ( + "context" + + "github.com/fnproject/fn/api/datastore/sql/migratex" + "github.com/jmoiron/sqlx" +) + +func up20(ctx context.Context, tx *sqlx.Tx) error { + _, err := tx.ExecContext(ctx, "DROP TABLE routes;") + + return err +} + +func down20(ctx context.Context, tx *sqlx.Tx) error { + _, err := tx.ExecContext(ctx, `CREATE TABLE IF NOT EXISTS routes ( + app_id varchar(256) NOT NULL, + path varchar(256) NOT NULL, + image varchar(256) NOT NULL, + format varchar(16) NOT NULL, + memory int NOT NULL, + cpus int, + timeout int NOT NULL, + idle_timeout int NOT NULL, + tmpfs_size int, + type varchar(16) NOT NULL, + headers text NOT NULL, + config text NOT NULL, + annotations text NOT NULL, + created_at text, + updated_at varchar(256), + PRIMARY KEY (app_id, path) +);`) + + return err +} + +func init() { + Migrations = append(Migrations, &migratex.MigFields{ + VersionFunc: vfunc(20), + UpFunc: up20, + DownFunc: down20, + }) +} diff --git a/api/datastore/sql/migrations/21_drop_cols_path.go b/api/datastore/sql/migrations/21_drop_cols_path.go new file mode 100644 index 000000000..ecbc1e94b --- /dev/null +++ b/api/datastore/sql/migrations/21_drop_cols_path.go @@ -0,0 +1,28 @@ +package migrations + +import ( + "context" + + "github.com/fnproject/fn/api/datastore/sql/migratex" + "github.com/jmoiron/sqlx" +) + +func up21(ctx context.Context, tx *sqlx.Tx) error { + _, err := tx.ExecContext(ctx, "ALTER TABLE calls DROP COLUMN path;") + + return err +} + +func down21(ctx context.Context, tx *sqlx.Tx) error { + _, err := tx.Exec("ALTER TABLE calls ADD path varchar(256);") + + return err +} + +func init() { + Migrations = append(Migrations, &migratex.MigFields{ + VersionFunc: vfunc(21), + UpFunc: up21, + DownFunc: down21, + }) +} diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 97a3acb05..4121f7d79 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -74,7 +74,6 @@ var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( id varchar(256) NOT NULL, app_id varchar(256), fn_id varchar(256), - path varchar(256) NOT NULL, stats text, error text, PRIMARY KEY (id) @@ -118,8 +117,7 @@ var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( } const ( - routeSelector = `SELECT app_id, path, image, format, memory, type, cpus, timeout, idle_timeout, tmpfs_size, headers, config, annotations, created_at, updated_at FROM routes` - callSelector = `SELECT id, created_at, started_at, completed_at, status, app_id, path, stats, error FROM calls` + callSelector = `SELECT id, created_at, started_at, completed_at, status, app_id, fn_id, stats, error FROM calls` appIDSelector = `SELECT id, name, config, annotations, syslog_url, created_at, updated_at FROM apps WHERE id=?` ensureAppSelector = `SELECT id FROM apps WHERE name=?` @@ -324,14 +322,9 @@ func latestVersion(migs []migratex.Migration) int64 { // clear is for tests only, be careful, it deletes all records. func (ds *SQLStore) clear() error { return ds.Tx(func(tx *sqlx.Tx) error { - query := tx.Rebind(`DELETE FROM routes`) - _, err := tx.Exec(query) - if err != nil { - return err - } - query = tx.Rebind(`DELETE FROM calls`) - _, err = tx.Exec(query) + query := tx.Rebind(`DELETE FROM calls`) + _, err := tx.Exec(query) if err != nil { return err } @@ -550,200 +543,6 @@ func (ds *SQLStore) GetApps(ctx context.Context, filter *models.AppFilter) (*mod return res, nil } -func (ds *SQLStore) InsertRoute(ctx context.Context, newRoute *models.Route) (*models.Route, error) { - route := newRoute.Clone() - route.CreatedAt = common.DateTime(time.Now()) - route.UpdatedAt = route.CreatedAt - - err := ds.Tx(func(tx *sqlx.Tx) error { - query := tx.Rebind(`SELECT 1 FROM apps WHERE id=?`) - r := tx.QueryRowContext(ctx, query, route.AppID) - if err := r.Scan(new(int)); err != nil { - if err == sql.ErrNoRows { - return models.ErrAppsNotFound - } - } - query = tx.Rebind(`SELECT 1 FROM routes WHERE app_id=? AND path=?`) - same, err := tx.QueryContext(ctx, query, route.AppID, route.Path) - if err != nil { - return err - } - defer same.Close() - if same.Next() { - return models.ErrRoutesAlreadyExists - } - - query = tx.Rebind(`INSERT INTO routes ( - app_id, - path, - image, - format, - memory, - cpus, - type, - timeout, - idle_timeout, - tmpfs_size, - headers, - config, - annotations, - created_at, - updated_at - ) - VALUES ( - :app_id, - :path, - :image, - :format, - :memory, - :cpus, - :type, - :timeout, - :idle_timeout, - :tmpfs_size, - :headers, - :config, - :annotations, - :created_at, - :updated_at - );`) - - _, err = tx.NamedExecContext(ctx, query, route) - - return err - }) - - return route, err -} - -func (ds *SQLStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*models.Route, error) { - var route models.Route - err := ds.Tx(func(tx *sqlx.Tx) error { - query := tx.Rebind(fmt.Sprintf("%s WHERE app_id=? AND path=?", routeSelector)) - row := tx.QueryRowxContext(ctx, query, newroute.AppID, newroute.Path) - - err := row.StructScan(&route) - if err == sql.ErrNoRows { - return models.ErrRoutesNotFound - } else if err != nil { - return err - } - - route.Update(newroute) - err = route.Validate() - if err != nil { - return err - } - - query = tx.Rebind(`UPDATE routes SET - image = :image, - format = :format, - memory = :memory, - cpus = :cpus, - type = :type, - timeout = :timeout, - idle_timeout = :idle_timeout, - tmpfs_size = :tmpfs_size, - headers = :headers, - config = :config, - annotations = :annotations, - updated_at = :updated_at - WHERE app_id=:app_id AND path=:path;`) - - res, err := tx.NamedExecContext(ctx, query, &route) - if err != nil { - return err - } - - if n, err := res.RowsAffected(); err != nil { - return err - } else if n == 0 { - // inside of the transaction, we are querying for the row, so we know that it exists - return nil - } - - return nil - }) - - if err != nil { - return nil, err - } - return &route, nil -} - -func (ds *SQLStore) RemoveRoute(ctx context.Context, appID string, routePath string) error { - query := ds.db.Rebind(`DELETE FROM routes WHERE path = ? AND app_id = ?`) - res, err := ds.db.ExecContext(ctx, query, routePath, appID) - if err != nil { - return err - } - - n, err := res.RowsAffected() - if err != nil { - return err - } - - if n == 0 { - return models.ErrRoutesNotFound - } - - return nil -} - -func (ds *SQLStore) GetRoute(ctx context.Context, appID, routePath string) (*models.Route, error) { - rSelectCondition := "%s WHERE app_id=? AND path=?" - query := ds.db.Rebind(fmt.Sprintf(rSelectCondition, routeSelector)) - row := ds.db.QueryRowxContext(ctx, query, appID, routePath) - - var route models.Route - err := row.StructScan(&route) - if err == sql.ErrNoRows { - return nil, models.ErrRoutesNotFound - } else if err != nil { - return nil, err - } - return &route, nil -} - -// GetRoutesByApp retrieves a route with a specific app name. -func (ds *SQLStore) GetRoutesByApp(ctx context.Context, appID string, filter *models.RouteFilter) ([]*models.Route, error) { - res := []*models.Route{} - if filter == nil { - filter = new(models.RouteFilter) - } - - filter.AppID = appID - filterQuery, args := buildFilterRouteQuery(filter) - - query := fmt.Sprintf("%s %s", routeSelector, filterQuery) - query = ds.db.Rebind(query) - rows, err := ds.db.QueryxContext(ctx, query, args...) - if err != nil { - if err == sql.ErrNoRows { - return res, nil // no error for empty list - } - return nil, err - } - defer rows.Close() - - for rows.Next() { - var route models.Route - err := rows.StructScan(&route) - if err != nil { - continue - } - res = append(res, &route) - } - - if err := rows.Err(); err != nil { - if err == sql.ErrNoRows { - return res, nil // no error for empty list - } - } - - return res, nil -} - func (ds *SQLStore) InsertFn(ctx context.Context, newFn *models.Fn) (*models.Fn, error) { fn := newFn.Clone() fn.ID = id.New().String() @@ -958,7 +757,6 @@ func (ds *SQLStore) InsertCall(ctx context.Context, call *models.Call) error { status, app_id, fn_id, - path, stats, error ) @@ -970,7 +768,6 @@ func (ds *SQLStore) InsertCall(ctx context.Context, call *models.Call) error { :status, :app_id, :fn_id, - :path, :stats, :error );`) @@ -1078,22 +875,6 @@ func (ds *SQLStore) InsertLog(ctx context.Context, call *models.Call, logR io.Re return err } -func (ds *SQLStore) GetLog1(ctx context.Context, appID, callID string) (io.Reader, error) { - query := ds.db.Rebind(`SELECT log FROM logs WHERE id=? AND app_id=?`) - row := ds.db.QueryRowContext(ctx, query, callID, appID) - - var log string - err := row.Scan(&log) - if err != nil { - if err == sql.ErrNoRows { - return nil, models.ErrCallLogNotFound - } - return nil, err - } - - return strings.NewReader(log), nil -} - func (ds *SQLStore) GetLog(ctx context.Context, fnID, callID string) (io.Reader, error) { query := ds.db.Rebind(`SELECT log FROM logs WHERE id=? AND fn_id=?`) row := ds.db.QueryRowContext(ctx, query, callID, fnID) @@ -1110,25 +891,6 @@ func (ds *SQLStore) GetLog(ctx context.Context, fnID, callID string) (io.Reader, return strings.NewReader(log), nil } -func buildFilterRouteQuery(filter *models.RouteFilter) (string, []interface{}) { - if filter == nil { - return "", nil - } - var b bytes.Buffer - var args []interface{} - - args = where(&b, args, "app_id=? ", filter.AppID) - args = where(&b, args, "image=?", filter.Image) - args = where(&b, args, "path>?", filter.Cursor) - // where("path LIKE ?%", filter.PathPrefix) TODO needs escaping - - fmt.Fprintf(&b, ` ORDER BY path ASC`) // TODO assert this is indexed - fmt.Fprintf(&b, ` LIMIT ?`) - args = append(args, filter.PerPage) - - return b.String(), args -} - func buildFilterAppQuery(filter *models.AppFilter) (string, []interface{}, error) { var args []interface{} if filter == nil { @@ -1168,15 +930,9 @@ func buildFilterCallQuery(filter *models.CallFilter) (string, []interface{}) { if !time.Time(filter.FromTime).IsZero() { args = where(&b, args, "created_at>?", filter.FromTime.String()) } - if filter.AppID != "" { - args = where(&b, args, "app_id=?", filter.AppID) - } if filter.FnID != "" { args = where(&b, args, "fn_id=?", filter.FnID) } - if filter.Path != "" { - args = where(&b, args, "path=?", filter.Path) - } fmt.Fprintf(&b, ` ORDER BY id DESC`) // TODO assert this is indexed fmt.Fprintf(&b, ` LIMIT ?`) diff --git a/api/logs/metrics/metrics.go b/api/logs/metrics/metrics.go index b15dfa63f..f8d417bdc 100644 --- a/api/logs/metrics/metrics.go +++ b/api/logs/metrics/metrics.go @@ -22,24 +22,12 @@ func (m *metricls) InsertCall(ctx context.Context, call *models.Call) error { return m.ls.InsertCall(ctx, call) } -func (m *metricls) GetCall1(ctx context.Context, appName, callID string) (*models.Call, error) { - ctx, span := trace.StartSpan(ctx, "ls_get_call") - defer span.End() - return m.ls.GetCall1(ctx, appName, callID) -} - func (m *metricls) GetCall(ctx context.Context, fnID, callID string) (*models.Call, error) { ctx, span := trace.StartSpan(ctx, "ls_get_call") defer span.End() return m.ls.GetCall(ctx, fnID, callID) } -func (m *metricls) GetCalls1(ctx context.Context, filter *models.CallFilter) ([]*models.Call, error) { - ctx, span := trace.StartSpan(ctx, "ls_get_calls") - defer span.End() - return m.ls.GetCalls1(ctx, filter) -} - func (m *metricls) GetCalls(ctx context.Context, filter *models.CallFilter) (*models.CallList, error) { ctx, span := trace.StartSpan(ctx, "ls_get_calls") defer span.End() diff --git a/api/logs/mock.go b/api/logs/mock.go index c03247797..f6769c90e 100644 --- a/api/logs/mock.go +++ b/api/logs/mock.go @@ -38,14 +38,6 @@ func (m *mock) InsertLog(ctx context.Context, call *models.Call, callLog io.Read return err } -func (m *mock) GetLog1(ctx context.Context, appID, callID string) (io.Reader, error) { - logEntry, ok := m.Logs[callID] - if !ok { - return nil, models.ErrCallLogNotFound - } - return bytes.NewReader(logEntry), nil -} - func (m *mock) GetLog(ctx context.Context, fnID, callID string) (io.Reader, error) { logEntry, ok := m.Logs[callID] if !ok { @@ -59,16 +51,6 @@ func (m *mock) InsertCall(ctx context.Context, call *models.Call) error { return nil } -func (m *mock) GetCall1(ctx context.Context, appID, callID string) (*models.Call, error) { - for _, t := range m.Calls { - if t.ID == callID && t.AppID == appID { - return t, nil - } - } - - return nil, models.ErrCallNotFound -} - func (m *mock) GetCall(ctx context.Context, fnID, callID string) (*models.Call, error) { for _, t := range m.Calls { if t.ID == callID && @@ -86,30 +68,6 @@ func (s sortC) Len() int { return len(s) } func (s sortC) Less(i, j int) bool { return strings.Compare(s[i].ID, s[j].ID) < 0 } func (s sortC) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (m *mock) GetCalls1(ctx context.Context, filter *models.CallFilter) ([]*models.Call, error) { - // sort them all first for cursoring (this is for testing, n is small & mock is not concurrent..) - // calls are in DESC order so use sort.Reverse - sort.Sort(sort.Reverse(sortC(m.Calls))) - - var calls []*models.Call - for _, c := range m.Calls { - if len(calls) == filter.PerPage { - break - } - - if (filter.AppID == "" || c.AppID == filter.AppID) && - (filter.Path == "" || filter.Path == c.Path) && - (time.Time(filter.FromTime).IsZero() || time.Time(filter.FromTime).Before(time.Time(c.CreatedAt))) && - (time.Time(filter.ToTime).IsZero() || time.Time(c.CreatedAt).Before(time.Time(filter.ToTime))) && - (filter.Cursor == "" || strings.Compare(filter.Cursor, c.ID) > 0) { - - calls = append(calls, c) - } - } - - return calls, nil -} - func (m *mock) GetCalls(ctx context.Context, filter *models.CallFilter) (*models.CallList, error) { // sort them all first for cursoring (this is for testing, n is small & mock is not concurrent..) // calls are in DESC order so use sort.Reverse diff --git a/api/logs/s3/s3.go b/api/logs/s3/s3.go index 082f76371..62a606a49 100644 --- a/api/logs/s3/s3.go +++ b/api/logs/s3/s3.go @@ -143,12 +143,7 @@ func (s *store) InsertLog(ctx context.Context, call *models.Call, callLog io.Rea // wrap original reader in a decorator to keep track of read bytes without buffering cr := &countingReader{r: callLog} - objectName := "" - if call.FnID != "" { - objectName = logKey(call.FnID, call.ID) - } else { - objectName = logKey(call.AppID, call.ID) - } + objectName := logKey(call.FnID, call.ID) params := &s3manager.UploadInput{ Bucket: aws.String(s.bucket), @@ -203,7 +198,7 @@ func (s *store) InsertCall(ctx context.Context, call *models.Call) error { objectName := callKey(call.AppID, call.ID) if call.FnID != "" { - objectName = callKey2(call.FnID, call.ID) + objectName = callKey(call.FnID, call.ID) } params := &s3manager.UploadInput{ Bucket: aws.String(s.bucket), @@ -218,48 +213,15 @@ func (s *store) InsertCall(ctx context.Context, call *models.Call) error { return fmt.Errorf("failed to insert call, %v", err) } - // at this point, they can point lookup the log and it will work. now, we can try to upload - // the marker key. if the marker key upload fails, the user will simply not - // see this entry when listing only when specifying a route path. (NOTE: this - // behavior will go away if we stop listing by route -> triggers) - - if call.FnID == "" { - objectName = callMarkerKey(call.AppID, call.Path, call.ID) - params = &s3manager.UploadInput{ - Bucket: aws.String(s.bucket), - Key: aws.String(objectName), - Body: bytes.NewReader([]byte{}), - ContentType: aws.String("text/plain"), - } - - logrus.WithFields(logrus.Fields{"bucketName": s.bucket, "key": objectName}).Debug("Uploading call marker") - _, err = s.uploader.UploadWithContext(ctx, params) - if err != nil { - // XXX(reed): we could just log this? - return fmt.Errorf("failed to write marker key for log, %v", err) - } - } - return nil } -// GetCall1 returns a call at a certain id and app name. -func (s *store) GetCall1(ctx context.Context, appID, callID string) (*models.Call, error) { - ctx, span := trace.StartSpan(ctx, "s3_get_call") - defer span.End() - - objectName := callKey(appID, callID) - logrus.WithFields(logrus.Fields{"bucketName": s.bucket, "key": objectName}).Debug("Downloading call") - - return s.getCallByKey(ctx, objectName) -} - // GetCall returns a call at a certain id func (s *store) GetCall(ctx context.Context, fnID, callID string) (*models.Call, error) { ctx, span := trace.StartSpan(ctx, "s3_get_call") defer span.End() - objectName := callKey2(fnID, callID) + objectName := callKey(fnID, callID) logrus.WithFields(logrus.Fields{"bucketName": s.bucket, "key": objectName}).Debug("Downloading call") return s.getCallByKey(ctx, objectName) @@ -297,25 +259,11 @@ func flipCursor(oid string) string { return id.EncodeDescending(oid) } -func callMarkerKey(app, path, id string) string { - id = flipCursor(id) - // s3 urls use / and are url, we need to encode this since paths have / in them - // NOTE: s3 urls are max of 1024 chars. path is the only non-fixed sized object in here - // but it is fixed to 256 chars in sql (by chance, mostly). further validation may be needed if weirdness ensues. - path = base64.RawURLEncoding.EncodeToString([]byte(path)) - return callMarkerPrefix + app + "/" + path + "/" + id -} - -func callKey(app, id string) string { - id = flipCursor(id) - return callKeyFlipped(app, id) -} - func callKeyFlipped(app, id string) string { return callKeyPrefix + app + "/" + id } -func callKey2(fnID, id string) string { +func callKey(fnID, id string) string { id = flipCursor(id) return callKeyPrefix + fnID + "/" + id } @@ -326,127 +274,6 @@ func logKey(appID, callID string) string { // GetCalls1 returns a list of calls that satisfy the given CallFilter. If no // calls exist, an empty list and a nil error are returned. -// NOTE: this relies on call ids being lexicographically sortable and <= 16 byte -func (s *store) GetCalls1(ctx context.Context, filter *models.CallFilter) ([]*models.Call, error) { - ctx, span := trace.StartSpan(ctx, "s3_get_calls") - defer span.End() - - if filter.AppID == "" { - return nil, errors.New("s3 store does not support listing across all apps") - } - - // NOTE: - // if filter.Path != "" - // find marker from marker keys, start there, list keys, get next marker from there - // else - // use marker for keys - - // NOTE we need marker keys to support (app is REQUIRED): - // 1) quick iteration per path - // 2) sorted by id across all path - // marker key: m : {app} : {path} : {id} - // key: s: {app} : {id} - // - // also s3 api returns sorted in lexicographic order, we need the reverse of this. - - // marker is either a provided marker, or a key we create based on parameters - // that contains app_id, may be a marker key if path is provided, and may - // have a time guesstimate if to time is provided. - - var marker string - - // filter.Cursor is a call id, translate to our key format. if a path is - // provided, we list keys from markers instead. - if filter.Cursor != "" { - marker = callKey(filter.AppID, filter.Cursor) - if filter.Path != "" { - marker = callMarkerKey(filter.AppID, filter.Path, filter.Cursor) - } - } else if t := time.Time(filter.ToTime); !t.IsZero() { - // get a fake id that has the most significant bits set to the to_time (first 48 bits) - fako := id.NewWithTime(t) - //var buf [id.EncodedSize]byte - //fakoId.MarshalTextTo(buf) - //mid := string(buf[:10]) - mid := fako.String() - marker = callKey(filter.AppID, mid) - if filter.Path != "" { - marker = callMarkerKey(filter.AppID, filter.Path, mid) - } - } - - // prefix prevents leaving bounds of app or path marker keys - prefix := callKey(filter.AppID, "") - if filter.Path != "" { - prefix = callMarkerKey(filter.AppID, filter.Path, "") - } - - input := &s3.ListObjectsInput{ - Bucket: aws.String(s.bucket), - MaxKeys: aws.Int64(int64(filter.PerPage)), - Marker: aws.String(marker), - Prefix: aws.String(prefix), - } - - result, err := s.client.ListObjects(input) - if err != nil { - return nil, fmt.Errorf("failed to list logs: %v", err) - } - - calls := make([]*models.Call, 0, len(result.Contents)) - - for _, obj := range result.Contents { - if len(calls) == filter.PerPage { - break - } - - // extract the app and id from the key to lookup the object, this also - // validates we aren't reading strangely keyed objects from the bucket. - var app, id string - if filter.Path != "" { - fields := strings.Split(*obj.Key, "/") - if len(fields) != 4 { - return calls, fmt.Errorf("invalid key in call markers: %v", *obj.Key) - } - app = fields[1] - id = fields[3] - } else { - fields := strings.Split(*obj.Key, "/") - if len(fields) != 3 { - return calls, fmt.Errorf("invalid key in calls: %v", *obj.Key) - } - app = fields[1] - id = fields[2] - } - - // the id here is already reverse encoded, keep it that way. - objectName := callKeyFlipped(app, id) - - // NOTE: s3 doesn't have a way to get multiple objects so just use GetCall - // TODO we should reuse the buffer to decode these - call, err := s.getCallByKey(ctx, objectName) - if err != nil { - common.Logger(ctx).WithError(err).WithFields(logrus.Fields{"app": app, "id": id}).Error("error filling call object") - continue - } - - // ensure: from_time < created_at < to_time - fromTime := time.Time(filter.FromTime).Truncate(time.Millisecond) - if !fromTime.IsZero() && !fromTime.Before(time.Time(call.CreatedAt)) { - // NOTE could break, ids and created_at aren't necessarily in perfect order - continue - } - - toTime := time.Time(filter.ToTime).Truncate(time.Millisecond) - if !toTime.IsZero() && !time.Time(call.CreatedAt).Before(toTime) { - continue - } - - calls = append(calls, call) - } - - return calls, nil -} // GetCalls returns a list of calls that satisfy the given CallFilter. If no // calls exist, an empty list and a nil error are returned. @@ -478,7 +305,7 @@ func (s *store) GetCalls(ctx context.Context, filter *models.CallFilter) (*model // filter.Cursor is a call id, translate to our key format. if a path is // provided, we list keys from markers instead. if filter.Cursor != "" { - key = callKey2(filter.FnID, filter.Cursor) + key = callKey(filter.FnID, filter.Cursor) } else if t := time.Time(filter.ToTime); !t.IsZero() { // get a fake id that has the most significant bits set to the to_time (first 48 bits) fako := id.NewWithTime(t) @@ -490,7 +317,7 @@ func (s *store) GetCalls(ctx context.Context, filter *models.CallFilter) (*model } // prefix prevents leaving bounds of app or path marker keys - prefix := callKey2(filter.FnID, "") + prefix := callKey(filter.FnID, "") input := &s3.ListObjectsInput{ Bucket: aws.String(s.bucket), diff --git a/api/logs/testing/test.go b/api/logs/testing/test.go index fcbce8500..694cf7fdb 100644 --- a/api/logs/testing/test.go +++ b/api/logs/testing/test.go @@ -54,10 +54,10 @@ func Test(t *testing.T, fnl models.LogStore) { } calls, err := fnl.GetCalls(ctx, filter) if err != nil { - t.Fatalf("Test GetCalls2(ctx, filter): one call, unexpected error `%v`", err) + t.Fatalf("Test GetCalls(ctx, filter): one call, unexpected error `%v`", err) } if len(calls.Items) != 1 { - t.Fatalf("Test GetCalls2(ctx, filter): one call, unexpected length 1 != `%v`", len(calls.Items)) + t.Fatalf("Test GetCalls(ctx, filter): one call, unexpected length 1 != `%v`", len(calls.Items)) } c2 := *call @@ -214,8 +214,5 @@ func Test(t *testing.T, fnl models.LogStore) { if call.AppID != newCall.AppID { t.Fatalf("Test GetCall: fn id mismatch `%v` `%v`", call.FnID, newCall.FnID) } - if call.Path != newCall.Path { - t.Fatalf("Test GetCall: path mismatch `%v` `%v`", call.Path, newCall.Path) - } }) } diff --git a/api/logs/validator/validator.go b/api/logs/validator/validator.go index 5c463c2bb..4ec2c5e24 100644 --- a/api/logs/validator/validator.go +++ b/api/logs/validator/validator.go @@ -27,14 +27,14 @@ func (v *validator) InsertLog(ctx context.Context, call *models.Call, callLog io } // callID or appID will never be empty. -func (v *validator) GetLog(ctx context.Context, appID, callID string) (io.Reader, error) { +func (v *validator) GetLog(ctx context.Context, fnID, callID string) (io.Reader, error) { if callID == "" { return nil, models.ErrDatastoreEmptyCallID } - if appID == "" { + if fnID == "" { return nil, models.ErrMissingFnID } - return v.LogStore.GetLog(ctx, appID, callID) + return v.LogStore.GetLog(ctx, fnID, callID) } // callID or appID will never be empty. @@ -42,19 +42,19 @@ func (v *validator) InsertCall(ctx context.Context, call *models.Call) error { if call.ID == "" { return models.ErrDatastoreEmptyCallID } - if call.AppID == "" { - return models.ErrMissingAppID + if call.FnID == "" { + return models.ErrMissingFnID } return v.LogStore.InsertCall(ctx, call) } // callID or appID will never be empty. -func (v *validator) GetCall(ctx context.Context, appID, callID string) (*models.Call, error) { +func (v *validator) GetCall(ctx context.Context, fnID, callID string) (*models.Call, error) { if callID == "" { return nil, models.ErrDatastoreEmptyCallID } - if appID == "" { - return nil, models.ErrMissingAppID + if fnID == "" { + return nil, models.ErrMissingFnID } - return v.LogStore.GetCall(ctx, appID, callID) + return v.LogStore.GetCall(ctx, fnID, callID) } diff --git a/api/models/call.go b/api/models/call.go index 552d78f04..1ee03c7c5 100644 --- a/api/models/call.go +++ b/api/models/call.go @@ -31,7 +31,7 @@ const ( var possibleStatuses = [...]string{"delayed", "queued", "running", "success", "error", "cancelled"} -// Call is a representation of a specific invocation of a route. +// Call is a representation of a specific invocation of a fn. type Call struct { // Unique identifier representing a specific call. ID string `json:"id" db:"id"` @@ -73,9 +73,6 @@ type Call struct { // - client_request - Request was cancelled by a client. Status string `json:"status" db:"status"` - // Path of the route that is responsible for this call - Path string `json:"path" db:"path"` - // Name of Docker image to use. Image string `json:"image,omitempty" db:"-"` @@ -124,7 +121,7 @@ type Call struct { // Config is the set of configuration variables for the call Config Config `json:"config,omitempty" db:"-"` - // Annotations is the set of annotations for the app/route of the call. + // Annotations is the set of annotations for the app/fn of the call. Annotations Annotations `json:"annotations,omitempty" db:"-"` // Headers are headers from the request that created this call @@ -163,8 +160,6 @@ type Call struct { } type CallFilter struct { - Path string // match - AppID string // match FnID string //match FromTime common.DateTime ToTime common.DateTime diff --git a/api/models/datastore.go b/api/models/datastore.go index 4f2eb709f..3f85152d8 100644 --- a/api/models/datastore.go +++ b/api/models/datastore.go @@ -33,29 +33,6 @@ type Datastore interface { // Returns ErrAppsNotFound if an App is not found. RemoveApp(ctx context.Context, appID string) error - // GetRoute looks up a matching Route for appName and the literal request route routePath. - // Returns ErrDatastoreEmptyAppName when appName is empty, and ErrDatastoreEmptyRoutePath when - // routePath is empty. - // Returns ErrRoutesNotFound when no matching route is found. - GetRoute(ctx context.Context, appID, routePath string) (*Route, error) - - // GetRoutesByApp gets a slice of routes for a appName, optionally filtering on filter (filter.AppName is ignored). - // Returns ErrDatastoreEmptyAppName if appName is empty. - GetRoutesByApp(ctx context.Context, appID string, filter *RouteFilter) ([]*Route, error) - - // InsertRoute inserts a route. Returns ErrDatastoreEmptyRoute when route is nil, and ErrDatastoreEmptyAppName - // or ErrDatastoreEmptyRoutePath for empty AppName or Path. - // Returns ErrRoutesAlreadyExists if the exact route.Path already exists - InsertRoute(ctx context.Context, route *Route) (*Route, error) - - // UpdateRoute updates route's Config and Header fields. Returns ErrDatastoreEmptyRoute when route is nil, and - // ErrDatastoreEmptyAppName or ErrDatastoreEmptyRoutePath for empty AppName or Path. - UpdateRoute(ctx context.Context, route *Route) (*Route, error) - - // RemoveRoute removes a route. Returns ErrDatastoreEmptyAppID when appName is empty, and - // ErrDatastoreEmptyRoutePath when routePath is empty. Returns ErrRoutesNotFound when no route exists. - RemoveRoute(ctx context.Context, appID, routePath string) error - // InsertFn inserts a new function if one does not exist, applying any defaults necessary, InsertFn(ctx context.Context, fn *Fn) (*Fn, error) diff --git a/api/models/error.go b/api/models/error.go index b4c91a355..35ba74b32 100644 --- a/api/models/error.go +++ b/api/models/error.go @@ -77,62 +77,10 @@ var ( code: http.StatusBadRequest, error: errors.New("Invalid payload"), } - ErrDatastoreEmptyRoute = err{ - code: http.StatusBadRequest, - error: errors.New("Missing route"), - } - ErrRoutesAlreadyExists = err{ - code: http.StatusConflict, - error: errors.New("Route already exists"), - } - ErrRoutesMissingNew = err{ - code: http.StatusBadRequest, - error: errors.New("Missing new route"), - } - ErrRoutesNotFound = err{ - code: http.StatusNotFound, - error: errors.New("Route not found"), - } - ErrRoutesPathImmutable = err{ - code: http.StatusConflict, - error: errors.New("Could not update - path is immutable"), - } ErrFoundDynamicURL = err{ code: http.StatusBadRequest, error: errors.New("Dynamic URL is not allowed"), } - ErrRoutesInvalidPath = err{ - code: http.StatusBadRequest, - error: errors.New("Invalid route path format"), - } - ErrRoutesInvalidType = err{ - code: http.StatusBadRequest, - error: errors.New("Invalid route Type"), - } - ErrRoutesInvalidFormat = err{ - code: http.StatusBadRequest, - error: errors.New("Invalid route Format"), - } - ErrRoutesMissingAppID = err{ - code: http.StatusBadRequest, - error: errors.New("Missing route AppName"), - } - ErrRoutesMissingImage = err{ - code: http.StatusBadRequest, - error: errors.New("Missing route Image"), - } - ErrRoutesInvalidImage = err{ - code: http.StatusBadRequest, - error: errors.New("Invalid route Image"), - } - ErrRoutesMissingName = err{ - code: http.StatusBadRequest, - error: errors.New("Missing route Name"), - } - ErrRoutesMissingPath = err{ - code: http.StatusBadRequest, - error: errors.New("Missing route Path"), - } ErrPathMalformed = err{ code: http.StatusBadRequest, error: errors.New("Path malformed"), @@ -145,21 +93,9 @@ var ( code: http.StatusBadRequest, error: errors.New("from_time is not an epoch time"), } - ErrRoutesInvalidTimeout = err{ - code: http.StatusBadRequest, - error: fmt.Errorf("timeout value is out of range. Sync should be between 0 and %d, async should be between 0 and %d", MaxSyncTimeout, MaxAsyncTimeout), - } - ErrRoutesInvalidIdleTimeout = err{ - code: http.StatusBadRequest, - error: fmt.Errorf("idle_timeout value is out of range. It should be between 0 and %d", MaxIdleTimeout), - } - ErrRoutesInvalidMemory = err{ - code: http.StatusBadRequest, - error: fmt.Errorf("memory value is out of range. It should be between 0 and %d", RouteMaxMemory), - } ErrInvalidMemory = err{ code: http.StatusBadRequest, - error: fmt.Errorf("memory value is out of range. It should be between 0 and %d", RouteMaxMemory), + error: fmt.Errorf("memory value is out of range. It should be between 0 and %d", MaxMemory), } ErrCallResourceTooBig = err{ code: http.StatusBadRequest, @@ -178,14 +114,6 @@ var ( code: http.StatusNotFound, error: errors.New("Call log not found"), } - ErrInvokeNotSupported = err{ - code: http.StatusBadRequest, - error: errors.New("Invoking routes /r/ is not supported on nodes configured as type API"), - } - ErrAPINotSupported = err{ - code: http.StatusBadRequest, - error: errors.New("Invoking api /v1/ requests is not supported on nodes configured as type Runner"), - } ErrPathNotFound = err{ code: http.StatusNotFound, error: errors.New("Path not found"), diff --git a/api/models/fn.go b/api/models/fn.go index 93b4bd59c..cd300548d 100644 --- a/api/models/fn.go +++ b/api/models/fn.go @@ -18,6 +18,13 @@ var ( MaxTimeout int32 = 300 // 5m MaxIdleTimeout int32 = 3600 // 1h + DefaultTimeout int32 = 30 // seconds + DefaultIdleTimeout int32 = 30 // seconds + DefaultMemory uint64 = 128 // MB + + MaxSyncTimeout = 120 // 2 minutes + MaxAsyncTimeout = 3600 // 1 hour + ErrFnsIDMismatch = err{ code: http.StatusBadRequest, error: errors.New("Fn ID in path does not match that in body"), @@ -50,6 +57,10 @@ var ( code: http.StatusBadRequest, error: errors.New("Missing image on Fn"), } + ErrFnsInvalidImage = err{ + code: http.StatusBadRequest, + error: errors.New("Invalid Fn image"), + } ErrFnsInvalidFormat = err{ code: http.StatusBadRequest, error: errors.New("Invalid format on Fn"), diff --git a/api/models/logs.go b/api/models/logs.go index 3fb2decef..bd5b1f0e2 100644 --- a/api/models/logs.go +++ b/api/models/logs.go @@ -16,7 +16,7 @@ type LogStore interface { // TODO we should probably allow deletion of a range of logs (also calls)? // common cases for deletion will be: - // * route gets nuked + // * fn gets nuked // * app gets nuked // * call+logs getting cleaned up periodically @@ -24,16 +24,9 @@ type LogStore interface { // exists. InsertCall(ctx context.Context, call *Call) error - // GetCall returns a call at a certain id and app name. - GetCall1(ctx context.Context, appId, callID string) (*Call, error) - // GetCall2 returns a call at a certain id GetCall(ctx context.Context, fnID, callID string) (*Call, error) - // GetCalls returns a list of calls that satisfy the given CallFilter. If no - // calls exist, an empty list and a nil error are returned. - GetCalls1(ctx context.Context, filter *CallFilter) ([]*Call, error) - // GetCalls returns a list of calls that satisfy the given CallFilter. If no // calls exist, an empty list and a nil error are returned. GetCalls(ctx context.Context, filter *CallFilter) (*CallList, error) diff --git a/api/models/route.go b/api/models/route.go deleted file mode 100644 index c195d554b..000000000 --- a/api/models/route.go +++ /dev/null @@ -1,249 +0,0 @@ -package models - -import ( - "net/http" - "net/url" - "path" - "strings" - "time" - - "github.com/fnproject/fn/api/common" -) - -const ( - DefaultTimeout = 30 // seconds - DefaultIdleTimeout = 30 // seconds - DefaultMemory = 128 // MB - - MaxSyncTimeout = 120 // 2 minutes - MaxAsyncTimeout = 3600 // 1 hour -) - -var RouteMaxMemory = uint64(8 * 1024) - -type Routes []*Route - -type Route struct { - AppID string `json:"app_id" db:"app_id"` - Path string `json:"path" db:"path"` - Image string `json:"image" db:"image"` - Memory uint64 `json:"memory" db:"memory"` - CPUs MilliCPUs `json:"cpus" db:"cpus"` - Headers Headers `json:"headers,omitempty" db:"headers"` - Type string `json:"type" db:"type"` - Format string `json:"format" db:"format"` - Timeout int32 `json:"timeout" db:"timeout"` - IdleTimeout int32 `json:"idle_timeout" db:"idle_timeout"` - TmpFsSize uint32 `json:"tmpfs_size" db:"tmpfs_size"` - Config Config `json:"config,omitempty" db:"config"` - Annotations Annotations `json:"annotations,omitempty" db:"annotations"` - CreatedAt common.DateTime `json:"created_at,omitempty" db:"created_at"` - UpdatedAt common.DateTime `json:"updated_at,omitempty" db:"updated_at"` -} - -// SetDefaults sets zeroed field to defaults. -func (r *Route) SetDefaults() { - if r.Memory == 0 { - r.Memory = DefaultMemory - } - - if r.Type == TypeNone { - r.Type = TypeSync - } - - if r.Format == "" { - r.Format = FormatDefault - } - - if r.Headers == nil { - r.Headers = Headers(http.Header{}) - } - - if r.Config == nil { - // keeps the json from being nil - r.Config = map[string]string{} - } - - if r.Timeout == 0 { - r.Timeout = DefaultTimeout - } - - if r.IdleTimeout == 0 { - r.IdleTimeout = DefaultIdleTimeout - } - -} - -// Validate validates all field values, returning the first error, if any. -func (r *Route) Validate() error { - if r.AppID == "" { - return ErrRoutesMissingAppID - } - - if r.Path == "" { - return ErrRoutesMissingPath - } - - u, err := url.Parse(r.Path) - if err != nil { - return ErrPathMalformed - } - - if strings.Contains(u.Path, ":") { - return ErrFoundDynamicURL - } - - if !path.IsAbs(u.Path) { - return ErrRoutesInvalidPath - } - - if r.Image == "" { - return ErrRoutesMissingImage - } - - if r.Type != TypeAsync && r.Type != TypeSync { - return ErrRoutesInvalidType - } - - if r.Format != FormatDefault && r.Format != FormatHTTP && r.Format != FormatJSON && r.Format != FormatCloudEvent { - return ErrRoutesInvalidFormat - } - - if r.Timeout <= 0 || - (r.Type == TypeSync && r.Timeout > MaxSyncTimeout) || - (r.Type == TypeAsync && r.Timeout > MaxAsyncTimeout) { - return ErrRoutesInvalidTimeout - } - - if r.IdleTimeout <= 0 || r.IdleTimeout > MaxIdleTimeout { - return ErrRoutesInvalidIdleTimeout - } - - if r.Memory < 1 || r.Memory > RouteMaxMemory { - return ErrRoutesInvalidMemory - } - - err = r.Annotations.Validate() - if err != nil { - return err - } - - return nil -} - -func (r *Route) Clone() *Route { - clone := new(Route) - *clone = *r // shallow copy - - // now deep copy the maps - if r.Config != nil { - clone.Config = make(Config, len(r.Config)) - for k, v := range r.Config { - clone.Config[k] = v - } - } - if r.Headers != nil { - clone.Headers = make(Headers, len(r.Headers)) - for k, v := range r.Headers { - // TODO technically, we need to deep copy this slice... - clone.Headers[k] = v - } - } - return clone -} - -func (r1 *Route) Equals(r2 *Route) bool { - // start off equal, check equivalence of each field. - // the RHS of && won't eval if eq==false so config/headers checking is lazy - - eq := true - eq = eq && r1.AppID == r2.AppID - eq = eq && r1.Path == r2.Path - eq = eq && r1.Image == r2.Image - eq = eq && r1.Memory == r2.Memory - eq = eq && r1.CPUs == r2.CPUs - eq = eq && r1.Headers.Equals(r2.Headers) - eq = eq && r1.Type == r2.Type - eq = eq && r1.Format == r2.Format - eq = eq && r1.Timeout == r2.Timeout - eq = eq && r1.IdleTimeout == r2.IdleTimeout - eq = eq && r1.TmpFsSize == r2.TmpFsSize - eq = eq && r1.Config.Equals(r2.Config) - eq = eq && r1.Annotations.Equals(r2.Annotations) - // NOTE: datastore tests are not very fun to write with timestamp checks, - // and these are not values the user may set so we kind of don't care. - //eq = eq && time.Time(r1.CreatedAt).Equal(time.Time(r2.CreatedAt)) - //eq = eq && time.Time(r2.UpdatedAt).Equal(time.Time(r2.UpdatedAt)) - return eq -} - -// Update updates fields in r with non-zero field values from new, and sets -// updated_at if any of the fields change. 0-length slice Header values, and -// empty-string Config values trigger removal of map entry. -func (r *Route) Update(patch *Route) { - original := r.Clone() - - if patch.Image != "" { - r.Image = patch.Image - } - if patch.Memory != 0 { - r.Memory = patch.Memory - } - if patch.CPUs != 0 { - r.CPUs = patch.CPUs - } - if patch.Type != "" { - r.Type = patch.Type - } - if patch.Timeout != 0 { - r.Timeout = patch.Timeout - } - if patch.IdleTimeout != 0 { - r.IdleTimeout = patch.IdleTimeout - } - if patch.TmpFsSize != 0 { - r.TmpFsSize = patch.TmpFsSize - } - if patch.Format != "" { - r.Format = patch.Format - } - if patch.Headers != nil { - if r.Headers == nil { - r.Headers = Headers(make(http.Header)) - } - for k, v := range patch.Headers { - if len(v) == 0 { - http.Header(r.Headers).Del(k) - } else { - r.Headers[k] = v - } - } - } - if patch.Config != nil { - if r.Config == nil { - r.Config = make(Config) - } - for k, v := range patch.Config { - if v == "" { - delete(r.Config, k) - } else { - r.Config[k] = v - } - } - } - - r.Annotations = r.Annotations.MergeChange(patch.Annotations) - - if !r.Equals(original) { - r.UpdatedAt = common.DateTime(time.Now()) - } -} - -type RouteFilter struct { - PathPrefix string // this is prefix match TODO - AppID string // this is exact match (important for security) - Image string // this is exact match - - Cursor string - PerPage int -} diff --git a/api/models/route_test.go b/api/models/route_test.go deleted file mode 100644 index 3b624f724..000000000 --- a/api/models/route_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package models - -import ( - "github.com/fnproject/fn/api/id" - "testing" -) - -func TestRouteSimple(t *testing.T) { - - route1 := &Route{ - AppID: id.New().String(), - Path: "/some", - Image: "foo", - Memory: 128, - CPUs: 100, - Type: "sync", - Format: "http", - Timeout: 10, - IdleTimeout: 10, - TmpFsSize: 10, - } - - err := route1.Validate() - if err != nil { - t.Fatal("should not have failed, got: ", err) - } - - route2 := &Route{ - AppID: id.New().String(), - Path: "/some", - Image: "foo", - Memory: 128, - CPUs: 100, - Type: "sync", - Format: "nonsense", - Timeout: 10, - IdleTimeout: 10, - } - - err = route2.Validate() - if err == nil { - t.Fatalf("should have failed route: %#v", route2) - } -} diff --git a/api/models/route_wrapper.go b/api/models/route_wrapper.go deleted file mode 100644 index 307e5e262..000000000 --- a/api/models/route_wrapper.go +++ /dev/null @@ -1,12 +0,0 @@ -package models - -type RouteWrapper struct { - Route *Route `json:"route"` -} - -func (m *RouteWrapper) Validate() error { - if m.Route != nil { - return m.Route.Validate() - } - return nil -} diff --git a/api/mqs/mock.go b/api/mqs/mock.go index 5106e425a..2cf526d45 100644 --- a/api/mqs/mock.go +++ b/api/mqs/mock.go @@ -7,10 +7,8 @@ import ( ) type Mock struct { - FakeApp *models.App - Apps []*models.App - FakeRoute *models.Route - Routes []*models.Route + FakeApp *models.App + Apps []*models.App } func (mock *Mock) Push(context.Context, *models.Call) (*models.Call, error) { diff --git a/api/runnerpool/ch_placer.go b/api/runnerpool/ch_placer.go index ac7269ca8..9d394851f 100644 --- a/api/runnerpool/ch_placer.go +++ b/api/runnerpool/ch_placer.go @@ -31,8 +31,7 @@ func (p *chPlacer) PlaceCall(rp RunnerPool, ctx context.Context, call RunnerCall state := NewPlacerTracker(ctx, &p.cfg) defer state.HandleDone() - // The key is just the path in this case - key := call.Model().Path + key := call.Model().FnID sum64 := siphash.Hash(0, 0x4c617279426f6174, []byte(key)) for { diff --git a/api/server/apps_list.go b/api/server/apps_list.go index 6bbab1008..54f64e448 100644 --- a/api/server/apps_list.go +++ b/api/server/apps_list.go @@ -12,7 +12,7 @@ func (s *Server) handleAppList(c *gin.Context) { filter := &models.AppFilter{} - filter.Cursor, filter.PerPage = pageParamsV2(c) + filter.Cursor, filter.PerPage = pageParams(c) filter.Name = c.Query("name") diff --git a/api/server/apps_v1_create.go b/api/server/apps_v1_create.go deleted file mode 100644 index 67b6a9c37..000000000 --- a/api/server/apps_v1_create.go +++ /dev/null @@ -1,39 +0,0 @@ -package server - -import ( - "net/http" - - "github.com/fnproject/fn/api/models" - "github.com/gin-gonic/gin" -) - -//TODO deprecate with V2 -func (s *Server) handleV1AppCreate(c *gin.Context) { - ctx := c.Request.Context() - - var wapp models.AppWrapper - - err := c.BindJSON(&wapp) - if err != nil { - if models.IsAPIError(err) { - handleV1ErrorResponse(c, err) - } else { - handleV1ErrorResponse(c, models.ErrInvalidJSON) - } - return - } - - app := wapp.App - if app == nil { - handleV1ErrorResponse(c, models.ErrAppsMissingNew) - return - } - - app, err = s.datastore.InsertApp(ctx, app) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, appResponse{"App successfully created", app}) -} diff --git a/api/server/apps_v1_delete.go b/api/server/apps_v1_delete.go deleted file mode 100644 index f3a57ef9e..000000000 --- a/api/server/apps_v1_delete.go +++ /dev/null @@ -1,21 +0,0 @@ -package server - -import ( - "net/http" - - "github.com/fnproject/fn/api" - "github.com/gin-gonic/gin" -) - -// TODO: Deprecate with v1 -func (s *Server) handleV1AppDelete(c *gin.Context) { - ctx := c.Request.Context() - - err := s.datastore.RemoveApp(ctx, c.MustGet(api.AppID).(string)) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, gin.H{"message": "App deleted"}) -} diff --git a/api/server/apps_v1_get.go b/api/server/apps_v1_get.go deleted file mode 100644 index bf2e004b7..000000000 --- a/api/server/apps_v1_get.go +++ /dev/null @@ -1,23 +0,0 @@ -package server - -import ( - "net/http" - - "github.com/fnproject/fn/api" - "github.com/gin-gonic/gin" -) - -// TODO: Deprecate with V1 API -func (s *Server) handleV1AppGetByIdOrName(c *gin.Context) { - ctx := c.Request.Context() - - param := c.MustGet(api.AppID).(string) - - app, err := s.datastore.GetAppByID(ctx, param) - - if err != nil { - handleV1ErrorResponse(c, err) - return - } - c.JSON(http.StatusOK, appResponse{"Successfully loaded app", app}) -} diff --git a/api/server/apps_v1_list.go b/api/server/apps_v1_list.go deleted file mode 100644 index bc6e9e1ad..000000000 --- a/api/server/apps_v1_list.go +++ /dev/null @@ -1,35 +0,0 @@ -package server - -import ( - "encoding/base64" - "net/http" - - "github.com/fnproject/fn/api/models" - "github.com/gin-gonic/gin" -) - -// TODO: Deprecate with V1 API -func (s *Server) handleV1AppList(c *gin.Context) { - ctx := c.Request.Context() - - filter := &models.AppFilter{} - filter.Cursor, filter.PerPage = pageParamsV2(c) - - apps, err := s.datastore.GetApps(ctx, filter) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - var nextCursor string - if len(apps.Items) > 0 && len(apps.Items) == filter.PerPage { - last := []byte(apps.Items[len(apps.Items)-1].Name) - nextCursor = base64.RawURLEncoding.EncodeToString(last) - } - - c.JSON(http.StatusOK, appsV1Response{ - Message: "Successfully listed applications", - NextCursor: nextCursor, - Apps: apps.Items, - }) -} diff --git a/api/server/apps_v1_test.go b/api/server/apps_v1_test.go deleted file mode 100644 index 6c2fa41f8..000000000 --- a/api/server/apps_v1_test.go +++ /dev/null @@ -1,342 +0,0 @@ -package server - -import ( - "bytes" - "encoding/base64" - "encoding/json" - "errors" - "net/http" - "strings" - "testing" - "time" - - "github.com/fnproject/fn/api/datastore" - "github.com/fnproject/fn/api/logs" - "github.com/fnproject/fn/api/models" - "github.com/fnproject/fn/api/mqs" -) - -func TestV1AppCreate(t *testing.T) { - buf := setLogBuffer() - defer func() { - if t.Failed() { - t.Log(buf.String()) - } - }() - - for i, test := range []struct { - mock models.Datastore - logDB models.LogStore - path string - body string - expectedCode int - expectedError error - }{ - // errors - {datastore.NewMock(), logs.NewMock(), "/v1/apps", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{}`, http.StatusBadRequest, models.ErrAppsMissingNew}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "name": "Test" }`, http.StatusBadRequest, models.ErrAppsMissingNew}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "" } }`, http.StatusBadRequest, models.ErrMissingName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "1234567890123456789012345678901" } }`, http.StatusBadRequest, models.ErrAppsTooLongName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "app", "annotations" : { "":"val" }}}`, http.StatusBadRequest, models.ErrInvalidAnnotationKey}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "app", "annotations" : { "key":"" }}}`, http.StatusBadRequest, models.ErrInvalidAnnotationValue}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "app", "syslog_url":"yo"}}`, http.StatusBadRequest, errors.New(`invalid syslog url: "yo"`)}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "app", "syslog_url":"yo://sup.com:1"}}`, http.StatusBadRequest, errors.New(`invalid syslog url: "yo://sup.com:1"`)}, - // success - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "teste" } }`, http.StatusOK, nil}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "teste" , "annotations": {"k1":"v1", "k2":[]}}}`, http.StatusOK, nil}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "teste", "syslog_url":"tcp://example.com:443" } }`, http.StatusOK, nil}, - } { - rnr, cancel := testRunner(t) - srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr, ServerTypeFull) - router := srv.Router - - body := bytes.NewBuffer([]byte(test.body)) - _, rec := routerRequest(t, router, "POST", test.path, body) - - if rec.Code != test.expectedCode { - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Errorf("Test %d: Expected error message to have `%s` but got `%s`", - i, test.expectedError.Error(), resp.Error.Message) - } - } - - if test.expectedCode == http.StatusOK { - var awrap models.AppWrapper - err := json.NewDecoder(rec.Body).Decode(&awrap) - if err != nil { - t.Log(buf.String()) - t.Errorf("Test %d: error decoding body for 'ok' json, it was a lie: %v", i, err) - } - - app := awrap.App - - // IsZero() doesn't really work, this ensures it's not unset as long as we're not in 1970 - if time.Time(app.CreatedAt).Before(time.Now().Add(-1 * time.Hour)) { - t.Log(buf.String()) - t.Errorf("Test %d: expected created_at to be set on app, it wasn't: %s", i, app.CreatedAt) - } - if !(time.Time(app.CreatedAt)).Equal(time.Time(app.UpdatedAt)) { - t.Log(buf.String()) - t.Errorf("Test %d: expected updated_at to be set and same as created at, it wasn't: %s %s", i, app.CreatedAt, app.UpdatedAt) - } - } - - cancel() - } -} - -func TestV1AppDelete(t *testing.T) { - buf := setLogBuffer() - defer func() { - if t.Failed() { - t.Log(buf.String()) - } - }() - - app := &models.App{ - Name: "myapp", - ID: "appId", - } - ds := datastore.NewMockInit([]*models.App{app}) - for i, test := range []struct { - ds models.Datastore - logDB models.LogStore - path string - body string - expectedCode int - expectedError error - }{ - {datastore.NewMock(), logs.NewMock(), "/v1/apps/myapp", "", http.StatusNotFound, nil}, - {ds, logs.NewMock(), "/v1/apps/myapp", "", http.StatusOK, nil}, - } { - rnr, cancel := testRunner(t) - srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr, ServerTypeFull) - - _, rec := routerRequest(t, srv.Router, "DELETE", test.path, nil) - - if rec.Code != test.expectedCode { - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Errorf("Test %d: Expected error message to have `%s`", - i, test.expectedError.Error()) - } - } - cancel() - } -} - -func TestV1AppList(t *testing.T) { - buf := setLogBuffer() - defer func() { - if t.Failed() { - t.Log(buf.String()) - } - }() - - rnr, cancel := testRunner(t) - defer cancel() - ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp"}, - {Name: "myapp2"}, - {Name: "myapp3"}, - }, - ) - fnl := logs.NewMock() - srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) - - a1b := base64.RawURLEncoding.EncodeToString([]byte("myapp")) - a2b := base64.RawURLEncoding.EncodeToString([]byte("myapp2")) - a3b := base64.RawURLEncoding.EncodeToString([]byte("myapp3")) - - for i, test := range []struct { - path string - body string - expectedCode int - expectedError error - expectedLen int - nextCursor string - }{ - {"/v1/apps?per_page", "", http.StatusOK, nil, 3, ""}, - {"/v1/apps?per_page=1", "", http.StatusOK, nil, 1, a1b}, - {"/v1/apps?per_page=1&cursor=" + a1b, "", http.StatusOK, nil, 1, a2b}, - {"/v1/apps?per_page=1&cursor=" + a2b, "", http.StatusOK, nil, 1, a3b}, - {"/v1/apps?per_page=100&cursor=" + a2b, "", http.StatusOK, nil, 1, ""}, // cursor is empty if per_page > len(results) - {"/v1/apps?per_page=1&cursor=" + a3b, "", http.StatusOK, nil, 0, ""}, // cursor could point to empty page - } { - _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) - - if rec.Code != test.expectedCode { - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Errorf("Test %d: Expected error message to have `%s`", - i, test.expectedError.Error()) - } - } else { - // normal path - - var resp appsV1Response - err := json.NewDecoder(rec.Body).Decode(&resp) - if err != nil { - t.Errorf("Test %d: Expected response body to be a valid json object. err: %v", i, err) - } - if len(resp.Apps) != test.expectedLen { - t.Errorf("Test %d: Expected apps length to be %d, but got %d", i, test.expectedLen, len(resp.Apps)) - } - if resp.NextCursor != test.nextCursor { - t.Errorf("Test %d: Expected next_cursor to be %s, but got %s", i, test.nextCursor, resp.NextCursor) - } - } - } -} - -func TestV1AppGet(t *testing.T) { - buf := setLogBuffer() - defer func() { - if t.Failed() { - t.Log(buf.String()) - } - }() - - rnr, cancel := testRunner(t) - defer cancel() - ds := datastore.NewMock() - fnl := logs.NewMock() - srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) - - for i, test := range []struct { - path string - body string - expectedCode int - expectedError error - }{ - {"/v1/apps/myapp", "", http.StatusNotFound, nil}, - } { - _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) - - if rec.Code != test.expectedCode { - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Errorf("Test %d: Expected error message to have `%s`", - i, test.expectedError.Error()) - } - } - } -} - -func TestV1AppUpdate(t *testing.T) { - buf := setLogBuffer() - defer func() { - if t.Failed() { - t.Log(buf.String()) - } - }() - - app := &models.App{ - Name: "myapp", - ID: "app_id", - } - ds := datastore.NewMockInit([]*models.App{app}) - - for i, test := range []struct { - mock models.Datastore - logDB models.LogStore - path string - body string - expectedCode int - expectedError error - }{ - // errors - {ds, logs.NewMock(), "/v1/apps/myapp", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - - // Addresses #380 - {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusConflict, nil}, - - // success: add/set MD key - {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "annotations": {"k-0" : "val"} } }`, http.StatusOK, nil}, - - // success - {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil}, - - // success - {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil}, - - // success - {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "syslog_url":"tcp://example.com:443" } }`, http.StatusOK, nil}, - } { - rnr, cancel := testRunner(t) - srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr, ServerTypeFull) - - body := bytes.NewBuffer([]byte(test.body)) - _, rec := routerRequest(t, srv.Router, "PATCH", test.path, body) - - if rec.Code != test.expectedCode { - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Errorf("Test %d: Expected error message to have `%s` but was `%s`", - i, test.expectedError.Error(), resp.Error.Message) - } - } - - if test.expectedCode == http.StatusOK { - var awrap models.AppWrapper - err := json.NewDecoder(rec.Body).Decode(&awrap) - if err != nil { - t.Log(buf.String()) - t.Errorf("Test %d: error decoding body for 'ok' json, it was a lie: %v", i, err) - } - - app := awrap.App - // IsZero() doesn't really work, this ensures it's not unset as long as we're not in 1970 - if time.Time(app.UpdatedAt).Before(time.Now().Add(-1 * time.Hour)) { - t.Log(buf.String()) - t.Errorf("Test %d: expected updated_at to be set on app, it wasn't: %s", i, app.UpdatedAt) - } - - // this isn't perfect, since a PATCH could succeed without updating any - // fields (among other reasons), but just don't make a test for that or - // special case (the body or smth) to ignore it here! - // this is a decent approximation that the timestamp gets changed - if (time.Time(app.UpdatedAt)).Equal(time.Time(app.CreatedAt)) { - t.Log(buf.String()) - t.Errorf("Test %d: expected updated_at to not be the same as created at, it wasn't: %s %s", i, app.CreatedAt, app.UpdatedAt) - } - } - - cancel() - } -} diff --git a/api/server/apps_v1_update.go b/api/server/apps_v1_update.go deleted file mode 100644 index 53211238d..000000000 --- a/api/server/apps_v1_update.go +++ /dev/null @@ -1,47 +0,0 @@ -package server - -import ( - "net/http" - - "github.com/fnproject/fn/api" - "github.com/fnproject/fn/api/models" - "github.com/gin-gonic/gin" -) - -// TODO: Deprecate with V1 API -func (s *Server) handleV1AppUpdate(c *gin.Context) { - ctx := c.Request.Context() - - wapp := models.AppWrapper{} - - err := c.BindJSON(&wapp) - if err != nil { - if models.IsAPIError(err) { - handleV1ErrorResponse(c, err) - } else { - handleV1ErrorResponse(c, models.ErrInvalidJSON) - } - return - } - - if wapp.App == nil { - handleV1ErrorResponse(c, models.ErrAppsMissingNew) - return - } - - if wapp.App.Name != "" { - handleV1ErrorResponse(c, models.ErrAppsNameImmutable) - return - } - - wapp.App.Name = c.MustGet(api.AppName).(string) - wapp.App.ID = c.MustGet(api.AppID).(string) - - app, err := s.datastore.UpdateApp(ctx, wapp.App) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, appResponse{"AppName successfully updated", app}) -} diff --git a/api/server/call_get.go b/api/server/call_get.go index 31f1bdfdd..f9f02fd7e 100644 --- a/api/server/call_get.go +++ b/api/server/call_get.go @@ -4,35 +4,36 @@ import ( "net/http" "github.com/fnproject/fn/api" + "github.com/fnproject/fn/api/models" "github.com/gin-gonic/gin" ) -func (s *Server) handleCallGet1(c *gin.Context) { - ctx := c.Request.Context() - - callID := c.Param(api.ParamCallID) - appID := c.MustGet(api.AppID).(string) - - callObj, err := s.logstore.GetCall1(ctx, appID, callID) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, callResponse{"Successfully loaded call", callObj}) -} - func (s *Server) handleCallGet(c *gin.Context) { ctx := c.Request.Context() fnID := c.Param(api.ParamFnID) - callID := c.Param(api.ParamCallID) - callObj, err := s.logstore.GetCall(ctx, fnID, callID) - if err != nil { - handleV1ErrorResponse(c, err) + if fnID == "" { + handleErrorResponse(c, models.ErrFnsMissingID) return } - c.JSON(http.StatusOK, callResponse{"Successfully loaded call", callObj}) + _, err := s.datastore.GetFnByID(ctx, c.Param(api.ParamFnID)) + if err != nil { + handleErrorResponse(c, err) + return + } + + callID := c.Param(api.ParamCallID) + if callID == "" { + handleErrorResponse(c, models.ErrDatastoreEmptyCallID) + } + + callObj, err := s.logstore.GetCall(ctx, fnID, callID) + if err != nil { + handleErrorResponse(c, err) + return + } + + c.JSON(http.StatusOK, callObj) } diff --git a/api/server/call_list.go b/api/server/call_list.go index db3956aed..57ab9ec3f 100644 --- a/api/server/call_list.go +++ b/api/server/call_list.go @@ -11,48 +11,29 @@ import ( "github.com/gin-gonic/gin" ) -func (s *Server) handleCallList1(c *gin.Context) { - ctx := c.Request.Context() - var err error - - appID := c.MustGet(api.AppID).(string) - // TODO api.ParamRouteName needs to be escaped probably, since it has '/' a lot - filter := models.CallFilter{AppID: appID, Path: c.Query("path")} - filter.Cursor, filter.PerPage = pageParams(c, false) // ids are url safe - - filter.FromTime, filter.ToTime, err = timeParams(c) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - calls, err := s.logstore.GetCalls1(ctx, &filter) - - var nextCursor string - if len(calls) > 0 && len(calls) == filter.PerPage { - nextCursor = calls[len(calls)-1].ID - // don't base64, IDs are url safe - } - - c.JSON(http.StatusOK, callsResponse{ - Message: "Successfully listed calls", - NextCursor: nextCursor, - Calls: calls, - }) -} - func (s *Server) handleCallList(c *gin.Context) { ctx := c.Request.Context() var err error - fnID := c.MustGet(api.ParamFnID).(string) - // TODO api.ParamRouteName needs to be escaped probably, since it has '/' a lot + fnID := c.Param(api.ParamFnID) + + if fnID == "" { + handleErrorResponse(c, models.ErrFnsMissingID) + return + } + + _, err = s.datastore.GetFnByID(ctx, c.Param(api.ParamFnID)) + if err != nil { + handleErrorResponse(c, err) + return + } + filter := models.CallFilter{FnID: fnID} - filter.Cursor, filter.PerPage = pageParams(c, false) // ids are url safe + filter.Cursor, filter.PerPage = pageParams(c) filter.FromTime, filter.ToTime, err = timeParams(c) if err != nil { - handleV1ErrorResponse(c, err) + handleErrorResponse(c, err) return } diff --git a/api/server/call_logs.go b/api/server/call_logs.go index cd5045708..d29c61017 100644 --- a/api/server/call_logs.go +++ b/api/server/call_logs.go @@ -34,46 +34,6 @@ func writeJSON(c *gin.Context, callID string, logReader io.Reader) { }}) } -func (s *Server) handleCallLogGet1(c *gin.Context) { - ctx := c.Request.Context() - - appID := c.MustGet(api.AppID).(string) - callID := c.Param(api.ParamCallID) - - logReader, err := s.logstore.GetLog(ctx, appID, callID) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - mimeTypes, _ := c.Request.Header["Accept"] - - if len(mimeTypes) == 0 { - writeJSON(c, callID, logReader) - return - } - - for _, mimeType := range mimeTypes { - if strings.Contains(mimeType, "application/json") { - writeJSON(c, callID, logReader) - return - } - if strings.Contains(mimeType, "text/plain") { - io.Copy(c.Writer, logReader) - return - - } - if strings.Contains(mimeType, "*/*") { - writeJSON(c, callID, logReader) - return - } - } - - // if we've reached this point it means that Fn didn't recognize Accepted content type - handleV1ErrorResponse(c, models.NewAPIError(http.StatusNotAcceptable, - errors.New("unable to respond within acceptable response content types"))) -} - func (s *Server) handleCallLogGet(c *gin.Context) { ctx := c.Request.Context() @@ -82,7 +42,7 @@ func (s *Server) handleCallLogGet(c *gin.Context) { logReader, err := s.logstore.GetLog(ctx, fnID, callID) if err != nil { - handleV1ErrorResponse(c, err) + handleErrorResponse(c, err) return } @@ -110,6 +70,6 @@ func (s *Server) handleCallLogGet(c *gin.Context) { } // if we've reached this point it means that Fn didn't recognize Accepted content type - handleV1ErrorResponse(c, models.NewAPIError(http.StatusNotAcceptable, + handleErrorResponse(c, models.NewAPIError(http.StatusNotAcceptable, errors.New("unable to respond within acceptable response content types"))) } diff --git a/api/server/calls_test.go b/api/server/calls_test.go index 1638462c5..aaff97348 100644 --- a/api/server/calls_test.go +++ b/api/server/calls_test.go @@ -1,6 +1,7 @@ package server import ( + "encoding/base64" "encoding/json" "fmt" "net/http" @@ -24,11 +25,10 @@ func TestCallGet(t *testing.T) { } }() - app := &models.App{Name: "myapp", ID: "app_id"} + fn := &models.Fn{Name: "myfn", ID: "fn_id"} call := &models.Call{ - AppID: app.ID, + FnID: fn.ID, ID: id.New().String(), - Path: "/thisisatest", Image: "fnproject/hello", // Delay: 0, Type: "sync", @@ -46,7 +46,7 @@ func TestCallGet(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() ds := datastore.NewMockInit( - []*models.App{app}, + []*models.Fn{fn}, ) fnl := logs.NewMock([]*models.Call{call}) srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) @@ -57,11 +57,11 @@ func TestCallGet(t *testing.T) { expectedCode int expectedError error }{ - {"/v1/apps//calls/" + call.ID, "", http.StatusBadRequest, models.ErrAppsMissingName}, - {"/v1/apps/nodawg/calls/" + call.ID, "", http.StatusNotFound, models.ErrAppsNotFound}, - {"/v1/apps/myapp/calls/" + id.New().String(), "", http.StatusNotFound, models.ErrCallNotFound}, - {"/v1/apps/myapp/calls/" + call.ID[:3], "", http.StatusNotFound, models.ErrCallNotFound}, - {"/v1/apps/myapp/calls/" + call.ID, "", http.StatusOK, nil}, + {"/v2/fns//calls/" + call.ID, "", http.StatusBadRequest, models.ErrFnsMissingID}, + {"/v2/fns/missing_fn/calls/" + call.ID, "", http.StatusNotFound, models.ErrFnsNotFound}, + {"/v2/fns/fn_id/calls/" + id.New().String(), "", http.StatusNotFound, models.ErrCallNotFound}, + {"/v2/fns/fn_id/calls/" + call.ID[:3], "", http.StatusNotFound, models.ErrCallNotFound}, + {"/v2/fns/fn_id/calls/" + call.ID, "", http.StatusOK, nil}, } { _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) @@ -72,10 +72,10 @@ func TestCallGet(t *testing.T) { } if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) + resp := getErrorResponse(t, rec) - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Log(resp.Error.Message) + if !strings.Contains(resp.Message, test.expectedError.Error()) { + t.Log(resp.Message) t.Log(rec.Body.String()) t.Errorf("Test %d: Expected error message to have `%s`", i, test.expectedError.Error()) @@ -93,12 +93,11 @@ func TestCallList(t *testing.T) { } }() - app := &models.App{Name: "myapp", ID: "app_id"} + fn := &models.Fn{ID: "fn_id"} call := &models.Call{ - AppID: app.ID, + FnID: fn.ID, ID: id.New().String(), - Path: "/thisisatest", Image: "fnproject/hello", // Delay: 0, Type: "sync", @@ -116,15 +115,17 @@ func TestCallList(t *testing.T) { c3 := *call c2.CreatedAt = common.DateTime(time.Now().Add(100 * time.Second)) c2.ID = id.New().String() - c2.Path = "test2" c3.CreatedAt = common.DateTime(time.Now().Add(200 * time.Second)) c3.ID = id.New().String() - c3.Path = "/test3" + + encodedC1ID := base64.RawURLEncoding.EncodeToString([]byte(call.ID)) + encodedC2ID := base64.RawURLEncoding.EncodeToString([]byte(c2.ID)) + encodedC3ID := base64.RawURLEncoding.EncodeToString([]byte(c3.ID)) rnr, cancel := testRunner(t) defer cancel() ds := datastore.NewMockInit( - []*models.App{app}, + []*models.Fn{fn}, ) fnl := logs.NewMock([]*models.Call{call, &c2, &c3}) srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) @@ -143,20 +144,20 @@ func TestCallList(t *testing.T) { expectedLen int nextCursor string }{ - {"/v1/apps//calls", "", http.StatusBadRequest, models.ErrAppsMissingName, 0, ""}, - {"/v1/apps/nodawg/calls", "", http.StatusNotFound, models.ErrAppsNotFound, 0, ""}, - {"/v1/apps/myapp/calls", "", http.StatusOK, nil, 3, ""}, - {"/v1/apps/myapp/calls?per_page=1", "", http.StatusOK, nil, 1, c3.ID}, - {"/v1/apps/myapp/calls?per_page=1&cursor=" + c3.ID, "", http.StatusOK, nil, 1, c2.ID}, - {"/v1/apps/myapp/calls?per_page=1&cursor=" + c2.ID, "", http.StatusOK, nil, 1, call.ID}, - {"/v1/apps/myapp/calls?per_page=100&cursor=" + c2.ID, "", http.StatusOK, nil, 1, ""}, // cursor is empty if per_page > len(results) - {"/v1/apps/myapp/calls?per_page=1&cursor=" + call.ID, "", http.StatusOK, nil, 0, ""}, // cursor could point to empty page - {"/v1/apps/myapp/calls?" + rangeTest, "", http.StatusOK, nil, 1, ""}, - {"/v1/apps/myapp/calls?from_time=xyz", "", http.StatusBadRequest, models.ErrInvalidFromTime, 0, ""}, - {"/v1/apps/myapp/calls?to_time=xyz", "", http.StatusBadRequest, models.ErrInvalidToTime, 0, ""}, + {"/v2/fns//calls", "", http.StatusBadRequest, models.ErrFnsMissingID, 0, ""}, + {"/v2/fns/nodawg/calls", "", http.StatusNotFound, models.ErrFnsNotFound, 0, ""}, + {"/v2/fns/fn_id/calls", "", http.StatusOK, nil, 3, ""}, + {"/v2/fns/fn_id/calls?per_page=1", "", http.StatusOK, nil, 1, encodedC3ID}, + {"/v2/fns/fn_id/calls?per_page=1&cursor=" + encodedC3ID, "", http.StatusOK, nil, 1, encodedC2ID}, + {"/v2/fns/fn_id/calls?per_page=1&cursor=" + encodedC2ID, "", http.StatusOK, nil, 1, encodedC1ID}, + {"/v2/fns/fn_id/calls?per_page=100&cursor=" + encodedC2ID, "", http.StatusOK, nil, 1, ""}, // cursor is empty if per_page > len(results) + {"/v2/fns/fn_id/calls?per_page=1&cursor=" + encodedC1ID, "", http.StatusOK, nil, 0, ""}, // cursor could point to empty page + {"/v2/fns/fn_id/calls?" + rangeTest, "", http.StatusOK, nil, 1, ""}, + {"/v2/fns/fn_id/calls?from_time=xyz", "", http.StatusBadRequest, models.ErrInvalidFromTime, 0, ""}, + {"/v2/fns/fn_id/calls?to_time=xyz", "", http.StatusBadRequest, models.ErrInvalidToTime, 0, ""}, - // TODO path isn't url safe w/ '/', so this is weird. hack in for tests - {"/v1/apps/myapp/calls?path=test2", "", http.StatusOK, nil, 1, ""}, + // // TODO path isn't url safe w/ '/', so this is weird. hack in for tests + // {"/v2/fns/fn_id/calls?path=test2", "", http.StatusOK, nil, 1, ""}, } { _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) @@ -166,22 +167,22 @@ func TestCallList(t *testing.T) { } if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) + resp := getErrorResponse(t, rec) - if resp.Error == nil || !strings.Contains(resp.Error.Message, test.expectedError.Error()) { + if resp.Message == "" || !strings.Contains(resp.Message, test.expectedError.Error()) { t.Errorf("Test %d: Expected error message to have `%s`, got: `%s`", - i, test.expectedError.Error(), resp.Error) + i, test.expectedError.Error(), resp.Message) } } else { // normal path - var resp callsResponse + var resp models.CallList err := json.NewDecoder(rec.Body).Decode(&resp) if err != nil { t.Errorf("Test %d: Expected response body to be a valid json object. err: %v", i, err) } - if len(resp.Calls) != test.expectedLen { - t.Fatalf("Test %d: Expected apps length to be %d, but got %d", i, test.expectedLen, len(resp.Calls)) + if len(resp.Items) != test.expectedLen { + t.Fatalf("Test %d: Expected calls length to be %d, but got %d", i, test.expectedLen, len(resp.Items)) } if resp.NextCursor != test.nextCursor { t.Errorf("Test %d: Expected next_cursor to be %s, but got %s", i, test.nextCursor, resp.NextCursor) diff --git a/api/server/error_response.go b/api/server/error_response.go index 4d82ecf50..b18061718 100644 --- a/api/server/error_response.go +++ b/api/server/error_response.go @@ -16,61 +16,14 @@ import ( // ErrInternalServerError returned when something exceptional happens. var ErrInternalServerError = errors.New("internal server error") -func simpleV1Error(err error) *models.ErrorWrapper { - return &models.ErrorWrapper{Error: &models.Error{Message: err.Error()}} -} - func simpleError(err error) *models.Error { return &models.Error{Message: err.Error()} } -// Legacy this is the old wrapped error -// TODO delete me ! -func handleV1ErrorResponse(ctx *gin.Context, err error) { - log := common.Logger(ctx) - - w := ctx.Writer - - if ctx.Err() == context.Canceled { - log.Info("client context cancelled") - w.WriteHeader(models.ErrClientCancel.Code()) - return - } - - var statuscode int - if e, ok := err.(models.APIError); ok { - if e.Code() >= 500 { - log.WithFields(logrus.Fields{"code": e.Code()}).WithError(e).Error("api error") - } - if err == models.ErrCallTimeoutServerBusy { - // TODO: Determine a better delay value here (perhaps ask Agent). For now 15 secs with - // the hopes that fnlb will land this on a better server immediately. - w.Header().Set("Retry-After", "15") - } - statuscode = e.Code() - } else { - log.WithError(err).WithFields(logrus.Fields{"stack": string(debug.Stack())}).Error("internal server error") - statuscode = http.StatusInternalServerError - err = ErrInternalServerError - } - writeV1Error(ctx, w, statuscode, err) -} - func handleErrorResponse(c *gin.Context, err error) { HandleErrorResponse(c.Request.Context(), c.Writer, err) } -// WriteError easy way to do standard error response, but can set statuscode and error message easier than handleV1ErrorResponse -func writeV1Error(ctx context.Context, w http.ResponseWriter, statuscode int, err error) { - log := common.Logger(ctx) - w.Header().Set("Content-Type", "application/json; charset=utf-8") - w.WriteHeader(statuscode) - err = json.NewEncoder(w).Encode(simpleV1Error(err)) - if err != nil { - log.WithError(err).Errorln("error encoding error json") - } -} - // HandleErrorResponse used to handle response errors in the same way. func HandleErrorResponse(ctx context.Context, w http.ResponseWriter, err error) { log := common.Logger(ctx) diff --git a/api/server/extension_points.go b/api/server/extension_points.go index 178053343..6200fd1af 100644 --- a/api/server/extension_points.go +++ b/api/server/extension_points.go @@ -23,12 +23,12 @@ func (s *Server) apiAppHandlerWrapperFn(apiHandler fnext.APIAppHandler) gin.Hand appID := c.MustGet(api.AppID).(string) app, err := s.datastore.GetAppByID(c.Request.Context(), appID) if err != nil { - handleV1ErrorResponse(c, err) + handleErrorResponse(c, err) c.Abort() return } if app == nil { - handleV1ErrorResponse(c, models.ErrAppsNotFound) + handleErrorResponse(c, models.ErrAppsNotFound) c.Abort() return } @@ -37,39 +37,6 @@ func (s *Server) apiAppHandlerWrapperFn(apiHandler fnext.APIAppHandler) gin.Hand } } -func (s *Server) apiRouteHandlerWrapperFn(apiHandler fnext.APIRouteHandler) gin.HandlerFunc { - return func(c *gin.Context) { - context := c.Request.Context() - appID := c.MustGet(api.AppID).(string) - routePath := "/" + c.Param(api.ParamRouteName) - route, err := s.datastore.GetRoute(context, appID, routePath) - if err != nil { - handleV1ErrorResponse(c, err) - c.Abort() - return - } - if route == nil { - handleV1ErrorResponse(c, models.ErrRoutesNotFound) - c.Abort() - return - } - - app, err := s.datastore.GetAppByID(context, appID) - if err != nil { - handleV1ErrorResponse(c, err) - c.Abort() - return - } - if app == nil { - handleV1ErrorResponse(c, models.ErrAppsNotFound) - c.Abort() - return - } - - apiHandler.ServeHTTP(c.Writer, c.Request, app, route) - } -} - // AddEndpoint adds an endpoint to /v1/x func (s *Server) AddEndpoint(method, path string, handler fnext.APIHandler) { v1 := s.Router.Group("/v1") @@ -93,15 +60,3 @@ func (s *Server) AddAppEndpoint(method, path string, handler fnext.APIAppHandler func (s *Server) AddAppEndpointFunc(method, path string, handler func(w http.ResponseWriter, r *http.Request, app *models.App)) { 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 fnext.APIRouteHandler) { - v1 := s.Router.Group("/v1") - v1.Use(s.checkAppPresenceByName()) - v1.Handle(method, "/apps/:app/routes/:route"+path, s.apiRouteHandlerWrapperFn(handler)) // conflicts with existing wildcard -} - -// AddRouteEndpointFunc 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, fnext.APIRouteHandlerFunc(handler)) -} diff --git a/api/server/fns_list.go b/api/server/fns_list.go index c610d21af..f191a9af0 100644 --- a/api/server/fns_list.go +++ b/api/server/fns_list.go @@ -12,7 +12,7 @@ func (s *Server) handleFnList(c *gin.Context) { ctx := c.Request.Context() var filter models.FnFilter - filter.Cursor, filter.PerPage = pageParamsV2(c) + filter.Cursor, filter.PerPage = pageParams(c) filter.AppID = c.Query("app_id") filter.Name = c.Query("name") diff --git a/api/server/gin_middlewares.go b/api/server/gin_middlewares.go index 3686ecfb4..fcde68693 100644 --- a/api/server/gin_middlewares.go +++ b/api/server/gin_middlewares.go @@ -7,10 +7,12 @@ import ( "fmt" "strings" + "strconv" + "time" + "github.com/fnproject/fn/api" "github.com/fnproject/fn/api/common" "github.com/fnproject/fn/api/models" - "github.com/fnproject/fn/fnext" "github.com/gin-contrib/cors" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" @@ -18,8 +20,6 @@ import ( "go.opencensus.io/stats/view" "go.opencensus.io/tag" "go.opencensus.io/trace" - "strconv" - "time" ) var ( @@ -67,13 +67,18 @@ func traceWrap(c *gin.Context) { if err != nil { logrus.Fatal(err) } - pathKey, err := tag.NewKey("fn_path") + appIDKey, err := tag.NewKey("fn_app_id") + if err != nil { + logrus.Fatal(err) + } + fnKey, err := tag.NewKey("fn_fn_id") if err != nil { logrus.Fatal(err) } ctx, err := tag.New(c.Request.Context(), tag.Insert(appKey, c.Param(api.ParamAppName)), - tag.Insert(pathKey, c.Param(api.ParamRouteName)), + tag.Insert(appIDKey, c.Param(api.ParamAppID)), + tag.Insert(fnKey, c.Param(api.ParamFnID)), ) if err != nil { logrus.Fatal(err) @@ -169,7 +174,7 @@ func panicWrap(c *gin.Context) { if !ok { err = fmt.Errorf("fn: %v", rec) } - handleV1ErrorResponse(c, err) + handleErrorResponse(c, err) c.Abort() } }(c) @@ -184,27 +189,41 @@ func loggerWrap(c *gin.Context) { ctx = ContextWithApp(ctx, appName) } - if routePath := c.Param(api.ParamRouteName); routePath != "" { - c.Set(api.Path, routePath) - ctx = ContextWithPath(ctx, routePath) + if appID := c.Param(api.ParamAppID); appID != "" { + c.Set(api.ParamAppID, appID) + ctx = ContextWithAppID(ctx, appID) + } + + if fnID := c.Param(api.ParamFnID); fnID != "" { + c.Set(api.ParamFnID, fnID) + ctx = ContextWithFnID(ctx, fnID) } c.Request = c.Request.WithContext(ctx) c.Next() } -type ctxPathKey string +type ctxFnIDKey string -// ContextWithPath sets the routePath value on a context, it may be retrieved -// using PathFromContext. -// TODO this is also used as a gin.Key -- stop one of these two things. -func ContextWithPath(ctx context.Context, routePath string) context.Context { - return context.WithValue(ctx, ctxPathKey(api.Path), routePath) +func ContextWithFnID(ctx context.Context, fnID string) context.Context { + return context.WithValue(ctx, ctxFnIDKey(api.ParamFnID), fnID) } -// PathFromContext returns the path from a context, if set. -func PathFromContext(ctx context.Context) string { - r, _ := ctx.Value(ctxPathKey(api.Path)).(string) +// FnIDFromContext returns the app from a context, if set. +func FnIDFromContext(ctx context.Context) string { + r, _ := ctx.Value(ctxFnIDKey(api.ParamFnID)).(string) + return r +} + +type ctxAppIDKey string + +func ContextWithAppID(ctx context.Context, appID string) context.Context { + return context.WithValue(ctx, ctxAppIDKey(api.ParamAppID), appID) +} + +// AppIDFromContext returns the app from a context, if set. +func AppIDFromContext(ctx context.Context) string { + r, _ := ctx.Value(ctxAppIDKey(api.ParamAppID)).(string) return r } @@ -223,26 +242,6 @@ func AppFromContext(ctx context.Context) string { return r } -func (s *Server) checkAppPresenceByNameAtLB() gin.HandlerFunc { - return func(c *gin.Context) { - ctx, _ := common.LoggerWithFields(c.Request.Context(), extractFields(c)) - - appName := c.Param(api.ParamAppName) - if appName != "" { - appID, err := s.lbReadAccess.GetAppID(ctx, appName) - if err != nil { - handleV1ErrorResponse(c, err) - c.Abort() - return - } - c.Set(api.AppID, appID) - } - - c.Request = c.Request.WithContext(ctx) - c.Next() - } -} - func (s *Server) checkAppPresenceByName() gin.HandlerFunc { return func(c *gin.Context) { ctx, _ := common.LoggerWithFields(c.Request.Context(), extractFields(c)) @@ -251,7 +250,7 @@ func (s *Server) checkAppPresenceByName() gin.HandlerFunc { if appName != "" { appID, err := s.datastore.GetAppID(ctx, appName) if err != nil { - handleV1ErrorResponse(c, err) + handleErrorResponse(c, err) c.Abort() return } @@ -263,15 +262,6 @@ func (s *Server) checkAppPresenceByName() gin.HandlerFunc { } } -func setAppNameInCtx(c *gin.Context) { - // add appName to context - appName := c.GetString(api.AppName) - if appName != "" { - c.Request = c.Request.WithContext(context.WithValue(c.Request.Context(), fnext.AppNameKey, appName)) - } - c.Next() -} - func setAppIDInCtx(c *gin.Context) { // add appName to context appID := c.Param(api.ParamAppID) @@ -283,10 +273,10 @@ func setAppIDInCtx(c *gin.Context) { c.Next() } -func appNameCheck(c *gin.Context) { - appName := c.GetString(api.AppName) - if appName == "" { - handleV1ErrorResponse(c, models.ErrAppsMissingName) +func appIDCheck(c *gin.Context) { + appID := c.GetString(api.ParamAppID) + if appID == "" { + handleErrorResponse(c, models.ErrAppsMissingID) c.Abort() return } diff --git a/api/server/hybrid.go b/api/server/hybrid.go index 1dd961af1..e9af2ece0 100644 --- a/api/server/hybrid.go +++ b/api/server/hybrid.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "net/http" - "path" "github.com/fnproject/fn/api" "github.com/fnproject/fn/api/common" @@ -189,20 +188,6 @@ func (s *Server) handleRunnerFinish(c *gin.Context) { c.String(http.StatusNoContent, "") } -// This is a sort of interim route that is V2 API style but due for deprectation -func (s *Server) handleRunnerGetRoute(c *gin.Context) { - ctx := c.Request.Context() - - routePath := path.Clean("/" + c.MustGet(api.Path).(string)) - route, err := s.datastore.GetRoute(ctx, c.MustGet(api.AppID).(string), routePath) - if err != nil { - handleErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, route) -} - func (s *Server) handleRunnerGetTriggerBySource(c *gin.Context) { ctx := c.Request.Context() diff --git a/api/server/hybrid_test.go b/api/server/hybrid_test.go index e1fe105d2..e8e815b6e 100644 --- a/api/server/hybrid_test.go +++ b/api/server/hybrid_test.go @@ -3,25 +3,24 @@ package server import ( "bytes" "encoding/json" + "net/http" + "strings" + "testing" + "github.com/fnproject/fn/api/datastore" "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/logs" "github.com/fnproject/fn/api/models" "github.com/fnproject/fn/api/mqs" - "net/http" - "strings" - "testing" ) func TestHybridEndpoints(t *testing.T) { buf := setLogBuffer() app := &models.App{ID: "app_id", Name: "myapp"} + fn := &models.Fn{ID: "fn_id", AppID: app.ID} ds := datastore.NewMockInit( []*models.App{app}, - []*models.Route{{ - AppID: app.ID, - Path: "yodawg", - }}, + []*models.Fn{fn}, ) logDB := logs.NewMock() @@ -30,10 +29,8 @@ func TestHybridEndpoints(t *testing.T) { newCallBody := func() string { call := &models.Call{ - AppID: app.ID, - ID: id.New().String(), - Path: "yodawg", - // TODO ? + FnID: fn.ID, + ID: id.New().String(), } var b bytes.Buffer json.NewEncoder(&b).Encode(&call) diff --git a/api/server/middleware.go b/api/server/middleware.go index 5e00c989a..81d0a4dfe 100644 --- a/api/server/middleware.go +++ b/api/server/middleware.go @@ -4,7 +4,6 @@ import ( "context" "net/http" - "github.com/fnproject/fn/api" "github.com/fnproject/fn/api/common" "github.com/fnproject/fn/fnext" "github.com/gin-gonic/gin" @@ -16,38 +15,8 @@ type middlewareController struct { // 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 -} - -// 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 - - // since we added middleware that checks the app ID - // we need to ensure that we set it as soon as possible - appName := AppFromContext(ctx) - if appName != "" { - appID, err := c.server.datastore.GetAppID(ctx, appName) - if err != nil { - handleV1ErrorResponse(c.ginContext, err) - c.ginContext.Abort() - return - } - c.ginContext.Set(api.AppID, appID) - } - - c.server.handleV1FunctionCall(c.ginContext) - c.ginContext.Abort() -} -func (c *middlewareController) FunctionCalled() bool { - return c.functionCalled + ginContext *gin.Context + server *Server } func (s *Server) apiMiddlewareWrapper() gin.HandlerFunc { @@ -78,7 +47,7 @@ func (s *Server) runMiddleware(c *gin.Context, ms []fnext.Middleware) { err := recover() if err != nil { common.Logger(c.Request.Context()).WithField("MiddleWarePanicRecovery:", err).Errorln("A panic occurred during middleware.") - handleV1ErrorResponse(c, ErrInternalServerError) + handleErrorResponse(c, ErrInternalServerError) } }() @@ -86,13 +55,6 @@ func (s *Server) runMiddleware(c *gin.Context, ms []fnext.Middleware) { last := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // fmt.Println("final handler called") ctx := r.Context() - mctx := fnext.GetMiddlewareController(ctx) - // check for bypass - if mctx.FunctionCalled() { - // fmt.Println("func already called, skipping") - c.Abort() - return - } c.Request = r.WithContext(ctx) c.Next() }) diff --git a/api/server/middleware_test.go b/api/server/middleware_test.go index 35917df52..8a5143b6a 100644 --- a/api/server/middleware_test.go +++ b/api/server/middleware_test.go @@ -11,6 +11,7 @@ import ( "testing" "fmt" + "github.com/fnproject/fn/api/datastore" "github.com/fnproject/fn/api/logs" "github.com/fnproject/fn/api/models" @@ -78,12 +79,10 @@ func TestRootMiddleware(t *testing.T) { app2 := &models.App{ID: "app_id_2", Name: "myapp2", Config: models.Config{}} ds := datastore.NewMockInit( []*models.App{app1, app2}, - []*models.Route{ - {Path: "/", AppID: app1.ID, Image: "fnproject/hello", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}}, - {Path: "/myroute", AppID: app1.ID, Image: "fnproject/hello", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}}, - {Path: "/app2func", AppID: app2.ID, Image: "fnproject/hello", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}, - Config: map[string]string{"NAME": "johnny"}, - }, + []*models.Fn{ + {ID: "fn_id1", AppID: app1.ID, Image: "fnproject/hello"}, + {ID: "fn_id2", AppID: app1.ID, Image: "fnproject/hello"}, + {ID: "fn_id3", AppID: app2.ID, Image: "fnproject/hello"}, }, ) @@ -97,11 +96,7 @@ func TestRootMiddleware(t *testing.T) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Header.Get("funcit") != "" { t.Log("breaker breaker!") - ctx := r.Context() - ctx = ContextWithApp(ctx, "myapp2") - ctx = ContextWithPath(ctx, "/app2func") - mctx := fnext.GetMiddlewareController(ctx) - mctx.CallFunction(w, r.WithContext(ctx)) + w.Write([]byte("Rerooted")) return } // If any context changes, user should use this: next.ServeHTTP(w, r.WithContext(ctx)) @@ -132,9 +127,9 @@ func TestRootMiddleware(t *testing.T) { expectedCode int expectedInBody string }{ - {"/r/myapp", `{"isDebug": true}`, "GET", map[string][]string{}, http.StatusOK, "middle"}, - {"/r/myapp/myroute", `{"isDebug": true}`, "GET", map[string][]string{}, http.StatusOK, "middle"}, - {"/v1/apps", `{"isDebug": true}`, "GET", map[string][]string{"funcit": {"Test"}}, http.StatusOK, "johnny"}, + {"/invoke/fn_id1", `{"isDebug": true}`, "POST", map[string][]string{}, http.StatusOK, "middle"}, + {"/v2/apps/app_id_1/fns/fn_id1", `{"isDebug": true}`, "POST", map[string][]string{}, http.StatusOK, "middle"}, + {"/v2/apps", `{"isDebug": true}`, "POST", map[string][]string{"funcit": {"Test"}}, http.StatusOK, "Rerooted"}, } { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { body := strings.NewReader(test.body) @@ -163,7 +158,7 @@ func TestRootMiddleware(t *testing.T) { } - req, err := http.NewRequest("POST", "http://127.0.0.1:8080/v1/apps", strings.NewReader("{\"app\": {\"name\": \"myapp3\"}}")) + req, err := http.NewRequest("POST", "http://127.0.0.1:8080/v2/apps", strings.NewReader("{\"name\": \"myapp3\"}")) if err != nil { t.Fatalf("Test: Could not create create app request") } diff --git a/api/server/route_listeners.go b/api/server/route_listeners.go deleted file mode 100644 index 8abe268bd..000000000 --- a/api/server/route_listeners.go +++ /dev/null @@ -1,78 +0,0 @@ -package server - -import ( - "context" - - "github.com/fnproject/fn/api/models" - "github.com/fnproject/fn/fnext" -) - -type routeListeners []fnext.RouteListener - -var _ fnext.RouteListener = new(routeListeners) - -// AddRouteListener adds a route listener extension to the set of listeners -// to be called around each route operation. -func (s *Server) AddRouteListener(listener fnext.RouteListener) { - *s.routeListeners = append(*s.routeListeners, listener) -} - -func (a *routeListeners) BeforeRouteCreate(ctx context.Context, route *models.Route) error { - for _, l := range *a { - err := l.BeforeRouteCreate(ctx, route) - if err != nil { - return err - } - } - return nil -} - -func (a *routeListeners) AfterRouteCreate(ctx context.Context, route *models.Route) error { - for _, l := range *a { - err := l.AfterRouteCreate(ctx, route) - if err != nil { - return err - } - } - return nil -} - -func (a *routeListeners) BeforeRouteUpdate(ctx context.Context, route *models.Route) error { - for _, l := range *a { - err := l.BeforeRouteUpdate(ctx, route) - if err != nil { - return err - } - } - return nil -} - -func (a *routeListeners) AfterRouteUpdate(ctx context.Context, route *models.Route) error { - for _, l := range *a { - err := l.AfterRouteUpdate(ctx, route) - if err != nil { - return err - } - } - return nil -} - -func (a *routeListeners) BeforeRouteDelete(ctx context.Context, appId string, routePath string) error { - for _, l := range *a { - err := l.BeforeRouteDelete(ctx, appId, routePath) - if err != nil { - return err - } - } - return nil -} - -func (a *routeListeners) AfterRouteDelete(ctx context.Context, appId string, routePath string) error { - for _, l := range *a { - err := l.AfterRouteDelete(ctx, appId, routePath) - if err != nil { - return err - } - } - return nil -} diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go deleted file mode 100644 index 4fe3721f7..000000000 --- a/api/server/routes_create_update.go +++ /dev/null @@ -1,178 +0,0 @@ -package server - -import ( - "context" - "net/http" - "path" - "strings" - - "github.com/fnproject/fn/api" - "github.com/fnproject/fn/api/models" - "github.com/gin-gonic/gin" -) - -/* handleRouteCreateOrUpdate is used to handle POST PUT and PATCH for routes. - Post will only create route if its not there and create app if its not. - create only - Post does not skip validation of zero values - Put will create app if its not there and if route is there update if not it will create new route. - update if exists or create if not exists - Put does not skip validation of zero values - Patch will not create app if it does not exist since the route needs to exist as well... - update only - Patch accepts partial updates / skips validation of zero values. -*/ - -func (s *Server) handleRoutesPostPut(c *gin.Context) { - ctx := c.Request.Context() - method := strings.ToUpper(c.Request.Method) - - var wroute models.RouteWrapper - err := bindRoute(c, method, &wroute) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - appName := c.MustGet(api.AppName).(string) - - appID, err := s.ensureApp(ctx, appName, method) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - resp, err := s.ensureRoute(ctx, appID, &wroute, method) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, resp) -} - -func (s *Server) handleRoutesPatch(c *gin.Context) { - ctx := c.Request.Context() - method := strings.ToUpper(c.Request.Method) - - var wroute models.RouteWrapper - err := bindRoute(c, method, &wroute) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - appID := c.MustGet(api.AppID).(string) - - resp, err := s.ensureRoute(ctx, appID, &wroute, method) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, resp) -} - -func (s *Server) submitRoute(ctx context.Context, wroute *models.RouteWrapper) error { - if wroute.Route != nil { - wroute.Route.SetDefaults() - } - r, err := s.datastore.InsertRoute(ctx, wroute.Route) - if err != nil { - return err - } - wroute.Route = r - return nil -} - -func (s *Server) changeRoute(ctx context.Context, wroute *models.RouteWrapper) error { - r, err := s.datastore.UpdateRoute(ctx, wroute.Route) - if err != nil { - return err - } - wroute.Route = r - return nil -} - -func (s *Server) ensureRoute(ctx context.Context, appID string, wroute *models.RouteWrapper, method string) (routeResponse, error) { - bad := new(routeResponse) - - wroute.Route.AppID = appID - - switch method { - case http.MethodPost: - err := s.submitRoute(ctx, wroute) - if err != nil { - return *bad, err - } - return routeResponse{"Route successfully created", wroute.Route}, nil - case http.MethodPut: - _, err := s.datastore.GetRoute(ctx, appID, wroute.Route.Path) - if err != nil && err == models.ErrRoutesNotFound { - err := s.submitRoute(ctx, wroute) - if err != nil { - return *bad, err - } - return routeResponse{"Route successfully created", wroute.Route}, nil - } - err = s.changeRoute(ctx, wroute) - if err != nil { - return *bad, err - } - return routeResponse{"Route successfully updated", wroute.Route}, nil - case http.MethodPatch: - err := s.changeRoute(ctx, wroute) - if err != nil { - return *bad, err - } - return routeResponse{"Route successfully updated", wroute.Route}, nil - } - return *bad, nil -} - -// ensureApp will only execute if it is on post or put. Patch is not allowed to create apps. -func (s *Server) ensureApp(ctx context.Context, appName string, method string) (string, error) { - appID, err := s.datastore.GetAppID(ctx, appName) - if err != nil && err != models.ErrAppsNotFound { - return "", err - } else if appID == "" { - // Create a new application assuming that /:app/ is an app name - newapp := &models.App{Name: appName} - - app, err := s.datastore.InsertApp(ctx, newapp) - if err != nil { - return "", err - } - return app.ID, nil - } - - return appID, nil -} - -// bindRoute binds the RouteWrapper to the json from the request. -func bindRoute(c *gin.Context, method string, wroute *models.RouteWrapper) error { - err := c.BindJSON(wroute) - if err != nil { - if models.IsAPIError(err) { - return err - } - return models.ErrInvalidJSON - } - - if wroute.Route == nil { - return models.ErrRoutesMissingNew - } - - if method == http.MethodPut || method == http.MethodPatch { - p := path.Clean(c.MustGet(api.Path).(string)) - - if wroute.Route.Path != "" && wroute.Route.Path != p { - return models.ErrRoutesPathImmutable - } - wroute.Route.Path = p - } - if method == http.MethodPost { - if wroute.Route.Path == "" { - return models.ErrRoutesMissingPath - } - } - return nil -} diff --git a/api/server/routes_delete.go b/api/server/routes_delete.go deleted file mode 100644 index f3239285d..000000000 --- a/api/server/routes_delete.go +++ /dev/null @@ -1,28 +0,0 @@ -package server - -import ( - "net/http" - "path" - - "github.com/fnproject/fn/api" - "github.com/gin-gonic/gin" -) - -func (s *Server) handleRouteDelete(c *gin.Context) { - ctx := c.Request.Context() - - appID := c.MustGet(api.AppID).(string) - routePath := path.Clean(c.MustGet(api.Path).(string)) - - if _, err := s.datastore.GetRoute(ctx, appID, routePath); err != nil { - handleV1ErrorResponse(c, err) - return - } - - if err := s.datastore.RemoveRoute(ctx, appID, routePath); err != nil { - handleV1ErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, gin.H{"message": "Route deleted"}) -} diff --git a/api/server/routes_get.go b/api/server/routes_get.go deleted file mode 100644 index a32949bb7..000000000 --- a/api/server/routes_get.go +++ /dev/null @@ -1,21 +0,0 @@ -package server - -import ( - "github.com/fnproject/fn/api" - "github.com/gin-gonic/gin" - "net/http" - "path" -) - -func (s *Server) handleRouteGetAPI(c *gin.Context) { - ctx := c.Request.Context() - - routePath := path.Clean("/" + c.MustGet(api.Path).(string)) - route, err := s.datastore.GetRoute(ctx, c.MustGet(api.AppID).(string), routePath) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - c.JSON(http.StatusOK, routeResponse{"Successfully loaded route", route}) -} diff --git a/api/server/routes_list.go b/api/server/routes_list.go deleted file mode 100644 index deb6948e3..000000000 --- a/api/server/routes_list.go +++ /dev/null @@ -1,37 +0,0 @@ -package server - -import ( - "encoding/base64" - "net/http" - - "github.com/fnproject/fn/api" - "github.com/fnproject/fn/api/models" - "github.com/gin-gonic/gin" -) - -func (s *Server) handleRouteList(c *gin.Context) { - ctx := c.Request.Context() - - var filter models.RouteFilter - filter.Image = c.Query("image") - // filter.PathPrefix = c.Query("path_prefix") TODO not hooked up - filter.Cursor, filter.PerPage = pageParams(c, true) - - routes, err := s.datastore.GetRoutesByApp(ctx, c.MustGet(api.AppID).(string), &filter) - if err != nil { - handleV1ErrorResponse(c, err) - return - } - - var nextCursor string - if len(routes) > 0 && len(routes) == filter.PerPage { - last := []byte(routes[len(routes)-1].Path) - nextCursor = base64.RawURLEncoding.EncodeToString(last) - } - - c.JSON(http.StatusOK, routesResponse{ - Message: "Successfully listed routes", - NextCursor: nextCursor, - Routes: routes, - }) -} diff --git a/api/server/routes_test.go b/api/server/routes_test.go deleted file mode 100644 index 00b4768fa..000000000 --- a/api/server/routes_test.go +++ /dev/null @@ -1,358 +0,0 @@ -package server - -import ( - "bytes" - "encoding/base64" - "encoding/json" - "net/http" - "strings" - "testing" - "time" - - "fmt" - "github.com/fnproject/fn/api/datastore" - "github.com/fnproject/fn/api/logs" - "github.com/fnproject/fn/api/models" - "github.com/fnproject/fn/api/mqs" -) - -type routeTestCase struct { - ds models.Datastore - logDB models.LogStore - method string - path string - body string - expectedCode int - expectedError error -} - -func (test *routeTestCase) run(t *testing.T, i int, buf *bytes.Buffer) { - rnr, cancel := testRunner(t) - srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr, ServerTypeFull) - - body := bytes.NewBuffer([]byte(test.body)) - _, rec := routerRequest(t, srv.Router, test.method, test.path, body) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Log(rec.Body.String()) - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - if resp.Error == nil { - t.Log(buf.String()) - t.Errorf("Test %d: Expected error message to have `%s`, but it was nil", - i, test.expectedError) - } else if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Log(buf.String()) - t.Errorf("Test %d: Expected error message to have `%s`, but it was `%s`", - i, test.expectedError, resp.Error.Message) - } - } - - if test.expectedCode == http.StatusOK { - var rwrap models.RouteWrapper - err := json.NewDecoder(rec.Body).Decode(&rwrap) - if err != nil { - t.Log(buf.String()) - t.Errorf("Test %d: error decoding body for 'ok' json, it was a lie: %v", i, err) - } - - route := rwrap.Route - if test.method == http.MethodPost { - // IsZero() doesn't really work, this ensures it's not unset as long as we're not in 1970 - if time.Time(route.CreatedAt).Before(time.Now().Add(-1 * time.Hour)) { - t.Log(buf.String()) - t.Errorf("Test %d: expected created_at to be set on route, it wasn't: %s", i, route.CreatedAt) - } - if !(time.Time(route.CreatedAt)).Equal(time.Time(route.UpdatedAt)) { - t.Log(buf.String()) - t.Errorf("Test %d: expected updated_at to be set and same as created at, it wasn't: %s %s", i, route.CreatedAt, route.UpdatedAt) - } - } - - if test.method == http.MethodPatch { - // IsZero() doesn't really work, this ensures it's not unset as long as we're not in 1970 - if time.Time(route.UpdatedAt).Before(time.Now().Add(-1 * time.Hour)) { - t.Log(buf.String()) - t.Errorf("Test %d: expected updated_at to be set on route, it wasn't: %s", i, route.UpdatedAt) - } - - // this isn't perfect, since a PATCH could succeed without updating any - // fields (among other reasons), but just don't make a test for that or - // special case (the body or smth) to ignore it here! - // this is a decent approximation that the timestamp gets changed - if (time.Time(route.UpdatedAt)).Equal(time.Time(route.CreatedAt)) { - t.Log(buf.String()) - t.Errorf("Test %d: expected updated_at to not be the same as created at, it wasn't: %s %s", i, route.CreatedAt, route.UpdatedAt) - } - } - } - - cancel() - buf.Reset() -} - -func TestRouteCreate(t *testing.T) { - buf := setLogBuffer() - - a := &models.App{Name: "a", ID: "app_id"} - commonDS := datastore.NewMockInit([]*models.App{a}) - for i, test := range []routeTestCase{ - // errors - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "path": "/myroute", "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesMissingPath}, - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingImage}, - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingPath}, - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesInvalidPath}, - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, - {commonDS, logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync", "cpus": "-100" } }`, http.StatusBadRequest, models.ErrInvalidCPUs}, - {datastore.NewMockInit([]*models.App{a}, - []*models.Route{ - { - AppID: a.ID, - Path: "/myroute", - }, - }, - ), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesAlreadyExists}, - - // success - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusOK, nil}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync", "cpus": "100m" } }`, http.StatusOK, nil}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync", "cpus": "0.2" } }`, http.StatusOK, nil}, - } { - test.run(t, i, buf) - } -} - -func TestRoutePut(t *testing.T) { - buf := setLogBuffer() - - a := &models.App{Name: "a", ID: "app_id"} - commonDS := datastore.NewMockInit([]*models.App{a}) - - for i, test := range []routeTestCase{ - // errors (NOTE: this route doesn't exist yet) - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "path": "/myroute", "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingImage}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingImage}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "diffRoute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesInvalidType}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "format": "invalid-format", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesInvalidFormat}, - - // success - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusOK, nil}, - {commonDS, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusOK, nil}, - } { - t.Run(fmt.Sprintf("case %d", i), - func(t *testing.T) { - test.run(t, i, buf) - }) - } -} - -func TestRouteDelete(t *testing.T) { - buf := setLogBuffer() - - a := &models.App{Name: "a", ID: "app_id"} - routes := []*models.Route{{AppID: a.ID, Path: "/myroute"}} - commonDS := datastore.NewMockInit([]*models.App{a}, routes) - - for i, test := range []struct { - ds models.Datastore - logDB models.LogStore - path string - body string - expectedCode int - expectedError error - }{ - {commonDS, logs.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, models.ErrRoutesNotFound}, - {commonDS, logs.NewMock(), "/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, - } { - rnr, cancel := testRunner(t) - srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr, ServerTypeFull) - _, rec := routerRequest(t, srv.Router, "DELETE", test.path, nil) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Log(rec.Body.String()) - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Log(buf.String()) - t.Errorf("Test %d: Expected error message to have `%s`", - i, test.expectedError.Error()) - } - } - cancel() - } -} - -func TestRouteList(t *testing.T) { - buf := setLogBuffer() - - rnr, cancel := testRunner(t) - defer cancel() - - app := &models.App{Name: "myapp", ID: "app_id"} - ds := datastore.NewMockInit( - []*models.App{app}, - []*models.Route{ - { - Path: "/myroute", - AppID: app.ID, - }, - { - Path: "/myroute1", - AppID: app.ID, - }, - { - Path: "/myroute2", - Image: "fnproject/fn-test-utils", - AppID: app.ID, - }, - }, - ) - fnl := logs.NewMock() - - r1b := base64.RawURLEncoding.EncodeToString([]byte("/myroute")) - r2b := base64.RawURLEncoding.EncodeToString([]byte("/myroute1")) - r3b := base64.RawURLEncoding.EncodeToString([]byte("/myroute2")) - - srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) - - for i, test := range []struct { - path string - body string - - expectedCode int - expectedError error - expectedLen int - nextCursor string - }{ - {"/v1/apps//routes", "", http.StatusBadRequest, models.ErrAppsMissingName, 0, ""}, - {"/v1/apps/a/routes", "", http.StatusNotFound, models.ErrAppsNotFound, 0, ""}, - {"/v1/apps/myapp/routes", "", http.StatusOK, nil, 3, ""}, - {"/v1/apps/myapp/routes?per_page=1", "", http.StatusOK, nil, 1, r1b}, - {"/v1/apps/myapp/routes?per_page=1&cursor=" + r1b, "", http.StatusOK, nil, 1, r2b}, - {"/v1/apps/myapp/routes?per_page=1&cursor=" + r2b, "", http.StatusOK, nil, 1, r3b}, - {"/v1/apps/myapp/routes?per_page=100&cursor=" + r2b, "", http.StatusOK, nil, 1, ""}, // cursor is empty if per_page > len(results) - {"/v1/apps/myapp/routes?per_page=1&cursor=" + r3b, "", http.StatusOK, nil, 0, ""}, // cursor could point to empty page - {"/v1/apps/myapp/routes?image=fnproject/fn-test-utils", "", http.StatusOK, nil, 1, ""}, - } { - _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Log(buf.String()) - t.Errorf("Test %d: Expected error message to have `%s`", - i, test.expectedError.Error()) - } - } else { - // normal path - - var resp routesResponse - err := json.NewDecoder(rec.Body).Decode(&resp) - if err != nil { - t.Errorf("Test %d: Expected response body to be a valid json object. err: %v", i, err) - } - if len(resp.Routes) != test.expectedLen { - t.Errorf("Test %d: Expected route length to be %d, but got %d", i, test.expectedLen, len(resp.Routes)) - } - if resp.NextCursor != test.nextCursor { - t.Errorf("Test %d: Expected next_cursor to be %s, but got %s", i, test.nextCursor, resp.NextCursor) - } - } - } -} - -func TestRouteGet(t *testing.T) { - buf := setLogBuffer() - - rnr, cancel := testRunner(t) - defer cancel() - - ds := datastore.NewMock() - fnl := logs.NewMock() - - srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) - - for i, test := range []struct { - path string - body string - expectedCode int - expectedError error - }{ - {"/v1/apps/a/routes/myroute", "", http.StatusNotFound, nil}, - } { - _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Log(buf.String()) - t.Errorf("Test %d: Expected error message to have `%s`", - i, test.expectedError.Error()) - } - } - } -} - -func TestRouteUpdate(t *testing.T) { - buf := setLogBuffer() - ds := datastore.NewMockInit() - - for i, test := range []routeTestCase{ - // success - {ds, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "fnproject/yodawg" } }`, http.StatusOK, nil}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "fnproject/fn-test-utils" } }`, http.StatusOK, nil}, - - // errors (after success, so route exists) - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesInvalidType}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesInvalidFormat}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "timeout": 121 } }`, http.StatusBadRequest, models.ErrRoutesInvalidTimeout}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "async", "timeout": 3601 } }`, http.StatusBadRequest, models.ErrRoutesInvalidTimeout}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "async", "timeout": 121, "idle_timeout": 240 } }`, http.StatusOK, nil}, // should work if async - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "idle_timeout": 3601 } }`, http.StatusBadRequest, models.ErrRoutesInvalidIdleTimeout}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "memory": 100000000000000 } }`, http.StatusBadRequest, models.ErrRoutesInvalidMemory}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "cpus": "foo" } }`, http.StatusBadRequest, models.ErrInvalidCPUs}, - // TODO this should be correct, waiting for patch to come in - //{ds, logs.NewMock(), http.MethodPatch, "/v1/apps/b/routes/myroute/dont", `{ "route": {} }`, http.StatusNotFound, models.ErrAppsNotFound}, - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/dont", `{ "route": {} }`, http.StatusNotFound, models.ErrRoutesNotFound}, - - // Addresses #381 - {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, - } { - test.run(t, i, buf) - } -} diff --git a/api/server/runner.go b/api/server/runner.go deleted file mode 100644 index 0a00e823d..000000000 --- a/api/server/runner.go +++ /dev/null @@ -1,164 +0,0 @@ -package server - -import ( - "bytes" - "io" - "net/http" - "path" - "strconv" - "sync" - "time" - - "github.com/fnproject/fn/api" - "github.com/fnproject/fn/api/agent" - "github.com/fnproject/fn/api/common" - "github.com/fnproject/fn/api/models" - "github.com/gin-gonic/gin" - "github.com/sirupsen/logrus" -) - -// handleV1FunctionCall executes the function, for router handlers -func (s *Server) handleV1FunctionCall(c *gin.Context) { - err := s.handleFunctionCall2(c) - if err != nil { - handleV1ErrorResponse(c, err) - } -} - -// handleFunctionCall2 executes the function and returns an error -// Requires the following in the context: -// * "app" -// * "path" -func (s *Server) handleFunctionCall2(c *gin.Context) error { - ctx := c.Request.Context() - var p string - r := PathFromContext(ctx) - if r == "" { - p = "/" - } else { - p = r - } - - appID := c.MustGet(api.AppID).(string) - app, err := s.lbReadAccess.GetAppByID(ctx, appID) - if err != nil { - return err - } - - routePath := path.Clean(p) - route, err := s.lbReadAccess.GetRoute(ctx, appID, routePath) - if err != nil { - return err - } - // gin sets this to 404 on NoRoute, so we'll just ensure it's 200 by default. - c.Status(200) // this doesn't write the header yet - - return s.ServeRoute(c, app, route) -} - -var ( - bufPool = &sync.Pool{New: func() interface{} { return new(bytes.Buffer) }} -) - -// ServeRoute serves an HTTP route for a given app -// This is exported to allow extensions to plugin their own route handling -func (s *Server) ServeRoute(c *gin.Context, app *models.App, route *models.Route) error { - buf := bufPool.Get().(*bytes.Buffer) - buf.Reset() - writer := syncResponseWriter{ - Buffer: buf, - headers: c.Writer.Header(), // copy ref - } - defer bufPool.Put(buf) // TODO need to ensure this is safe with Dispatch? - - // GetCall can mod headers, assign an id, look up the route/app (cached), - // strip params, etc. - // this should happen ASAP to turn app name to app ID - - // GetCall can mod headers, assign an id, look up the route/app (cached), - // strip params, etc. - - call, err := s.agent.GetCall( - agent.WithWriter(&writer), // XXX (reed): order matters [for now] - agent.FromRequest(app, route, c.Request), - ) - if err != nil { - return err - } - model := call.Model() - { // scope this, to disallow ctx use outside of this scope. add id for handleV1ErrorResponse logger - ctx, _ := common.LoggerWithFields(c.Request.Context(), logrus.Fields{"id": model.ID}) - c.Request = c.Request.WithContext(ctx) - } - - if model.Type == "async" { - // TODO we should push this into GetCall somehow (CallOpt maybe) or maybe agent.Queue(Call) ? - if c.Request.ContentLength > 0 { - buf.Grow(int(c.Request.ContentLength)) - } - _, err := buf.ReadFrom(c.Request.Body) - if err != nil { - return models.ErrInvalidPayload - } - model.Payload = buf.String() - - err = s.lbEnqueue.Enqueue(c.Request.Context(), model) - if err != nil { - return err - } - - c.JSON(http.StatusAccepted, map[string]string{"call_id": model.ID}) - return nil - } - - err = s.agent.Submit(call) - if err != nil { - // NOTE if they cancel the request then it will stop the call (kind of cool), - // we could filter that error out here too as right now it yells a little - if err == models.ErrCallTimeoutServerBusy || err == models.ErrCallTimeout { - // TODO maneuver - // add this, since it means that start may not have been called [and it's relevant] - c.Writer.Header().Add("XXX-FXLB-WAIT", time.Now().Sub(time.Time(model.CreatedAt)).String()) - } - return err - } - - // if they don't set a content-type - detect it - if writer.Header().Get("Content-Type") == "" { - // see http.DetectContentType, the go server is supposed to do this for us but doesn't appear to? - var contentType string - jsonPrefix := [1]byte{'{'} // stack allocated - if bytes.HasPrefix(buf.Bytes(), jsonPrefix[:]) { - // try to detect json, since DetectContentType isn't a hipster. - contentType = "application/json; charset=utf-8" - } else { - contentType = http.DetectContentType(buf.Bytes()) - } - writer.Header().Set("Content-Type", contentType) - } - - writer.Header().Set("Content-Length", strconv.Itoa(int(buf.Len()))) - - if writer.status > 0 { - c.Writer.WriteHeader(writer.status) - } - io.Copy(c.Writer, &writer) - - return nil -} - -var _ http.ResponseWriter = new(syncResponseWriter) - -// implements http.ResponseWriter -// this little guy buffers responses from user containers and lets them still -// set headers and such without us risking writing partial output [as much, the -// server could still die while we're copying the buffer]. this lets us set -// content length and content type nicely, as a bonus. it is sad, yes. -type syncResponseWriter struct { - headers http.Header - status int - *bytes.Buffer -} - -func (s *syncResponseWriter) Header() http.Header { return s.headers } -func (s *syncResponseWriter) WriteHeader(code int) { s.status = code } diff --git a/api/server/runner_async_test.go b/api/server/runner_async_test.go deleted file mode 100644 index 6692e29f1..000000000 --- a/api/server/runner_async_test.go +++ /dev/null @@ -1,105 +0,0 @@ -package server - -import ( - "bytes" - "context" - "net/http" - "testing" - - "github.com/fnproject/fn/api/agent" - "github.com/fnproject/fn/api/datastore" - "github.com/fnproject/fn/api/models" - "github.com/fnproject/fn/api/mqs" - "github.com/gin-gonic/gin" -) - -func testRouterAsync(ds models.Datastore, mq models.MessageQueue, rnr agent.Agent) *gin.Engine { - ctx := context.Background() - engine := gin.New() - s := &Server{ - agent: rnr, - Router: engine, - AdminRouter: engine, - datastore: ds, - lbReadAccess: ds, - lbEnqueue: agent.NewDirectEnqueueAccess(mq), - mq: mq, - nodeType: ServerTypeFull, - } - - r := s.Router - r.Use(gin.Logger()) - - s.Router.Use(loggerWrap) - s.bindHandlers(ctx) - return r -} - -func TestRouteRunnerAsyncExecution(t *testing.T) { - buf := setLogBuffer() - - app := &models.App{ID: "app_id", Name: "myapp", Config: map[string]string{"app": "true"}} - ds := datastore.NewMockInit( - []*models.App{app}, - []*models.Route{ - {Type: "async", Path: "/hot-http", AppID: app.ID, Image: "fnproject/fn-test-utils", Format: "http", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 4, IdleTimeout: 30}, - {Type: "async", Path: "/hot-json", AppID: app.ID, Image: "fnproject/fn-test-utils", Format: "json", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 4, IdleTimeout: 30}, - {Type: "async", Path: "/myroute", AppID: app.ID, Image: "fnproject/hello", Config: map[string]string{"test": "true"}, Memory: 128, CPUs: 200, Timeout: 30, IdleTimeout: 30}, - {Type: "async", Path: "/myerror", AppID: app.ID, Image: "fnproject/error", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, - {Type: "async", Path: "/myroute/:param", AppID: app.ID, Image: "fnproject/hello", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, - }, - ) - mq := &mqs.Mock{} - - for i, test := range []struct { - path string - body string - headers map[string][]string - expectedCode int - expectedEnv map[string]string - }{ - {"/r/myapp/myroute", `{"isDebug": true}`, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, - {"/r/myapp/hot-http", `{"isDebug": true}`, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, - {"/r/myapp/hot-json", `{"isDebug": true}`, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, - // FIXME: this just hangs - //{"/r/myapp/myroute/1", ``, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, - {"/r/myapp/myerror", `{"isDebug": true, "isCrash": true}`, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, - {"/r/myapp/myroute", `{"echoContent": "test","isDebug": true}`, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, - { - "/r/myapp/myroute", - `{"isDebug": true}`, - map[string][]string{"X-Function": []string{"test"}}, - http.StatusAccepted, - map[string]string{ - "TEST": "true", - "APP": "true", - "HEADER_X_FUNCTION": "test", - }, - }, - } { - body := bytes.NewBuffer([]byte(test.body)) - - t.Log("About to start router") - rnr, cancel := testRunner(t, ds) - router := testRouterAsync(ds, mq, rnr) - - t.Log("making requests") - req, rec := newRouterRequest(t, "POST", test.path, body) - for name, value := range test.headers { - req.Header.Set(name, value[0]) - } - t.Log("About to start router2") - router.ServeHTTP(rec, req) - t.Log("after servehttp") - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - // TODO can test body and headers in the actual mq message w/ an agent that doesn't dequeue? - // this just makes sure tasks are submitted (ok)... - - cancel() - } -} diff --git a/api/server/runner_fninvoke.go b/api/server/runner_fninvoke.go index 758840adc..162a44eae 100644 --- a/api/server/runner_fninvoke.go +++ b/api/server/runner_fninvoke.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "strconv" + "sync" "time" "github.com/fnproject/fn/api" @@ -15,6 +16,26 @@ import ( "github.com/sirupsen/logrus" ) +var ( + bufPool = &sync.Pool{New: func() interface{} { return new(bytes.Buffer) }} +) + +var _ http.ResponseWriter = new(syncResponseWriter) + +// implements http.ResponseWriter +// this little guy buffers responses from user containers and lets them still +// set headers and such without us risking writing partial output [as much, the +// server could still die while we're copying the buffer]. this lets us set +// content length and content type nicely, as a bonus. it is sad, yes. +type syncResponseWriter struct { + headers http.Header + status int + *bytes.Buffer +} + +func (s *syncResponseWriter) Header() http.Header { return s.headers } +func (s *syncResponseWriter) WriteHeader(code int) { s.status = code } + // handleFnInvokeCall executes the function, for router handlers func (s *Server) handleFnInvokeCall(c *gin.Context) { fnID := c.Param(api.ParamFnID) diff --git a/api/server/runner_fninvoke_test.go b/api/server/runner_fninvoke_test.go index 9232d953d..3238a8e9b 100644 --- a/api/server/runner_fninvoke_test.go +++ b/api/server/runner_fninvoke_test.go @@ -1,6 +1,8 @@ package server import ( + "crypto/rand" + "encoding/base64" "fmt" "io/ioutil" "net/http" @@ -163,7 +165,7 @@ func TestFnInvokeRunnerExecution(t *testing.T) { rnr, cancelrnr := testRunner(t, ds, ls) defer cancelrnr() - srv := testServer(ds, &mqs.Mock{}, ls, rnr, ServerTypeFull) + srv := testServer(ds, &mqs.Mock{}, ls, rnr, ServerTypeFull, LimitRequestBody(32256)) expHeaders := map[string][]string{"Content-Type": {"application/json; charset=utf-8"}} expCTHeaders := map[string][]string{"Content-Type": {"foo/bar"}} @@ -182,6 +184,10 @@ func TestFnInvokeRunnerExecution(t *testing.T) { // sleep between logs and with debug enabled, fn-test-utils will log header/footer below: multiLog := `{"echoContent": "_TRX_ID_", "sleepTime": 1000, "isDebug": true}` + //over sized request + var bigbufa [32257]byte + rand.Read(bigbufa[:]) + bigbuf := base64.StdEncoding.EncodeToString(bigbufa[:]) // this will be > bigbufa, but json compatible bigoutput := `{"echoContent": "_TRX_ID_", "isDebug": true, "trailerRepeat": 1000}` // 1000 trailers to exceed 2K smalloutput := `{"echoContent": "_TRX_ID_", "isDebug": true, "trailerRepeat": 1}` // 1 trailer < 2K @@ -222,6 +228,7 @@ func TestFnInvokeRunnerExecution(t *testing.T) { {"/invoke/http_fn_id", smalloutput, "POST", http.StatusOK, nil, "", nil}, {"/invoke/default_fn_id", bigoutput, "POST", http.StatusBadGateway, nil, "", nil}, {"/invoke/default_fn_id", smalloutput, "POST", http.StatusOK, nil, "", nil}, + {"/invoke/http_fn_id", bigbuf, "POST", http.StatusRequestEntityTooLarge, nil, "", nil}, } callIds := make([]string, len(testCases)) @@ -294,8 +301,8 @@ func TestInvokeRunnerTimeout(t *testing.T) { } }() - models.RouteMaxMemory = uint64(1024 * 1024 * 1024) // 1024 TB - hugeMem := uint64(models.RouteMaxMemory - 1) + models.MaxMemory = uint64(1024 * 1024 * 1024) // 1024 TB + hugeMem := uint64(models.MaxMemory - 1) app := &models.App{ID: "app_id", Name: "myapp", Config: models.Config{}} coldFn := &models.Fn{ID: "cold", Name: "cold", AppID: app.ID, Format: "", Image: "fnproject/fn-test-utils", ResourceConfig: models.ResourceConfig{Memory: 128, Timeout: 4, IdleTimeout: 30}} diff --git a/api/server/runner_httptrigger_test.go b/api/server/runner_httptrigger_test.go index 2f0ffe1bc..01d8896b7 100644 --- a/api/server/runner_httptrigger_test.go +++ b/api/server/runner_httptrigger_test.go @@ -9,7 +9,6 @@ import ( "testing" "context" - "errors" "os" "github.com/fnproject/fn/api/agent" @@ -88,52 +87,6 @@ func checkLogs(t *testing.T, tnum int, ds models.LogStore, callID string, expect return true } -// implement models.MQ and models.APIError -type errorMQ struct { - error - code int -} - -func (mock *errorMQ) Push(context.Context, *models.Call) (*models.Call, error) { return nil, mock } -func (mock *errorMQ) Reserve(context.Context) (*models.Call, error) { return nil, mock } -func (mock *errorMQ) Delete(context.Context, *models.Call) error { return mock } -func (mock *errorMQ) Code() int { return mock.code } -func (mock *errorMQ) Close() error { return nil } -func TestFailedEnqueue(t *testing.T) { - buf := setLogBuffer() - app := &models.App{ID: "app_id", Name: "myapp", Config: models.Config{}} - ds := datastore.NewMockInit( - []*models.App{app}, - []*models.Route{ - {Path: "/dummy", Image: "dummy/dummy", Type: "async", Memory: 128, Timeout: 30, IdleTimeout: 30, AppID: app.ID}, - }, - ) - err := errors.New("Unable to push task to queue") - mq := &errorMQ{err, http.StatusInternalServerError} - fnl := logs.NewMock() - rnr, cancelrnr := testRunner(t, ds, mq, fnl) - defer cancelrnr() - - srv := testServer(ds, mq, fnl, rnr, ServerTypeFull) - for i, test := range []struct { - path string - body string - method string - expectedCode int - expectedHeaders map[string][]string - }{ - {"/r/myapp/dummy", ``, "POST", http.StatusInternalServerError, nil}, - } { - body := strings.NewReader(test.body) - _, rec := routerRequest(t, srv.Router, test.method, test.path, body) - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) - } - } -} - func TestTriggerRunnerGet(t *testing.T) { buf := setLogBuffer() app := &models.App{ID: "app_id", Name: "myapp", Config: models.Config{}} @@ -475,8 +428,8 @@ func TestTriggerRunnerTimeout(t *testing.T) { } }() - models.RouteMaxMemory = uint64(1024 * 1024 * 1024) // 1024 TB - hugeMem := uint64(models.RouteMaxMemory - 1) + models.MaxMemory = uint64(1024 * 1024 * 1024) // 1024 TB + hugeMem := uint64(models.MaxMemory - 1) app := &models.App{ID: "app_id", Name: "myapp", Config: models.Config{}} coldFn := &models.Fn{ID: "cold", Name: "cold", AppID: app.ID, Format: "", Image: "fnproject/fn-test-utils", ResourceConfig: models.ResourceConfig{Memory: 128, Timeout: 4, IdleTimeout: 30}} diff --git a/api/server/runner_test.go b/api/server/runner_test.go deleted file mode 100644 index dda1c4755..000000000 --- a/api/server/runner_test.go +++ /dev/null @@ -1,490 +0,0 @@ -package server - -import ( - "bytes" - "fmt" - "io/ioutil" - "net/http" - "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" -) - -// TODO Deprecate with Routes - -func TestRouteRunnerGet(t *testing.T) { - buf := setLogBuffer() - app := &models.App{ID: "app_id", Name: "myapp", Config: models.Config{}} - ds := datastore.NewMockInit( - []*models.App{app}, - ) - - rnr, cancel := testRunner(t, ds) - defer cancel() - logDB := logs.NewMock() - srv := testServer(ds, &mqs.Mock{}, logDB, rnr, ServerTypeFull) - - for i, test := range []struct { - path string - body string - expectedCode int - expectedError error - }{ - {"/route", "", http.StatusNotFound, nil}, - {"/r/app/route", "", http.StatusNotFound, models.ErrAppsNotFound}, - {"/r/myapp/route", "", http.StatusNotFound, models.ErrRoutesNotFound}, - } { - _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Errorf("Test %d: Expected status code for path %s to be %d but was %d", - i, test.path, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { - t.Log(buf.String()) - t.Errorf("Test %d: Expected error message to have `%s`, but got `%s`", - i, test.expectedError.Error(), resp.Error.Message) - } - } - } -} - -func TestRouteRunnerPost(t *testing.T) { - buf := setLogBuffer() - - app := &models.App{ID: "app_id", Name: "myapp", Config: models.Config{}} - ds := datastore.NewMockInit( - []*models.App{app}, - ) - - rnr, cancel := testRunner(t, ds) - defer cancel() - - fnl := logs.NewMock() - srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) - - for i, test := range []struct { - path string - body string - expectedCode int - expectedError error - }{ - {"/route", `{ "payload": "" }`, http.StatusNotFound, nil}, - {"/r/app/route", `{ "payload": "" }`, http.StatusNotFound, models.ErrAppsNotFound}, - {"/r/myapp/route", `{ "payload": "" }`, http.StatusNotFound, models.ErrRoutesNotFound}, - } { - body := bytes.NewBuffer([]byte(test.body)) - _, rec := routerRequest(t, srv.Router, "POST", test.path, body) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Errorf("Test %d: Expected status code for path %s to be %d but was %d", - i, test.path, test.expectedCode, rec.Code) - } - - if test.expectedError != nil { - resp := getV1ErrorResponse(t, rec) - respMsg := resp.Error.Message - expMsg := test.expectedError.Error() - if respMsg != expMsg && !strings.Contains(respMsg, expMsg) { - t.Log(buf.String()) - t.Errorf("Test %d: Expected error message to have `%s`", - i, test.expectedError.Error()) - } - } - } -} - -func TestRouteRunnerExecEmptyBody(t *testing.T) { - buf := setLogBuffer() - isFailure := false - - defer func() { - if isFailure { - t.Log(buf.String()) - } - }() - - rCfg := map[string]string{"ENABLE_HEADER": "yes", "ENABLE_FOOTER": "yes"} // enable container start/end header/footer - rHdr := map[string][]string{"X-Function": {"Test"}} - rImg := "fnproject/fn-test-utils" - - app := &models.App{ID: "app_id", Name: "soup"} - ds := datastore.NewMockInit( - []*models.App{app}, - []*models.Route{ - {Path: "/cold", AppID: app.ID, Image: rImg, Type: "sync", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, - {Path: "/hothttp", AppID: app.ID, Image: rImg, Type: "sync", Format: "http", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, - {Path: "/hotjson", AppID: app.ID, Image: rImg, Type: "sync", Format: "json", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, - }, - ) - ls := logs.NewMock() - - rnr, cancelrnr := testRunner(t, ds, ls) - defer cancelrnr() - - srv := testServer(ds, &mqs.Mock{}, ls, rnr, ServerTypeFull) - - expHeaders := map[string][]string{"X-Function": {"Test"}} - emptyBody := `{"echoContent": "_TRX_ID_", "isDebug": true, "isEmptyBody": true}` - - // Test hot cases twice to rule out hot-containers corrupting next request. - testCases := []struct { - path string - }{ - {"/r/soup/cold/"}, - {"/r/soup/hothttp"}, - {"/r/soup/hothttp"}, - {"/r/soup/hotjson"}, - {"/r/soup/hotjson"}, - } - - for i, test := range testCases { - trx := fmt.Sprintf("_trx_%d_", i) - body := strings.NewReader(strings.Replace(emptyBody, "_TRX_ID_", trx, 1)) - _, rec := routerRequest(t, srv.Router, "GET", test.path, body) - respBytes, _ := ioutil.ReadAll(rec.Body) - respBody := string(respBytes) - maxBody := len(respBody) - if maxBody > 1024 { - maxBody = 1024 - } - - if rec.Code != http.StatusOK { - isFailure = true - t.Errorf("Test %d: Expected status code to be %d but was %d. body: %s", - i, http.StatusOK, rec.Code, respBody[:maxBody]) - } else if len(respBytes) != 0 { - isFailure = true - t.Errorf("Test %d: Expected empty body but got %d. body: %s", - i, len(respBytes), respBody[:maxBody]) - } - - for name, header := range expHeaders { - if header[0] != rec.Header().Get(name) { - isFailure = true - t.Errorf("Test %d: Expected header `%s` to be %s but was %s. body: %s", - i, name, header[0], rec.Header().Get(name), respBody) - } - } - } -} - -func TestRouteRunnerExecution(t *testing.T) { - buf := setLogBuffer() - isFailure := false - tweaker := envTweaker("FN_MAX_RESPONSE_SIZE", "2048") - defer tweaker() - - // Log once after we are done, flow of events are important (hot/cold containers, idle timeout, etc.) - // for figuring out why things failed. - defer func() { - if isFailure { - t.Log(buf.String()) - } - }() - - rCfg := map[string]string{"ENABLE_HEADER": "yes", "ENABLE_FOOTER": "yes"} // enable container start/end header/footer - rHdr := map[string][]string{"X-Function": {"Test"}} - rImg := "fnproject/fn-test-utils" - rImgBs1 := "fnproject/imagethatdoesnotexist" - rImgBs2 := "localhost:5050/fnproject/imagethatdoesnotexist" - - app := &models.App{ID: "app_id", Name: "myapp"} - ds := datastore.NewMockInit( - []*models.App{app}, - []*models.Route{ - {Path: "/", AppID: app.ID, Image: rImg, Type: "sync", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myhot", AppID: app.ID, Image: rImg, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myhotjason", AppID: app.ID, Image: rImg, Type: "sync", Format: "json", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myroute", AppID: app.ID, Image: rImg, Type: "sync", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myerror", AppID: app.ID, Image: rImg, Type: "sync", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/mydne", AppID: app.ID, Image: rImgBs1, Type: "sync", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/mydnehot", AppID: app.ID, Image: rImgBs1, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/mydneregistry", AppID: app.ID, Image: rImgBs2, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myoom", AppID: app.ID, Image: rImg, Type: "sync", Memory: 8, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/mybigoutputcold", AppID: app.ID, Image: rImg, Type: "sync", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, - {Path: "/mybigoutputhttp", AppID: app.ID, Image: rImg, Type: "sync", Format: "http", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, - {Path: "/mybigoutputjson", AppID: app.ID, Image: rImg, Type: "sync", Format: "json", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, - }, - ) - ls := logs.NewMock() - - rnr, cancelrnr := testRunner(t, ds, ls) - defer cancelrnr() - - srv := testServer(ds, &mqs.Mock{}, ls, rnr, ServerTypeFull) - - expHeaders := map[string][]string{"X-Function": {"Test"}, "Content-Type": {"application/json; charset=utf-8"}} - expCTHeaders := map[string][]string{"X-Function": {"Test"}, "Content-Type": {"foo/bar"}} - - // Checking for EndOfLogs currently depends on scheduling of go-routines (in docker/containerd) that process stderr & stdout. - // Therefore, not testing for EndOfLogs for hot containers (which has complex I/O processing) anymore. - multiLogExpectCold := []string{"BeginOfLogs", "EndOfLogs"} - multiLogExpectHot := []string{"BeginOfLogs" /*, "EndOfLogs" */} - - crasher := `{"echoContent": "_TRX_ID_", "isDebug": true, "isCrash": true}` // crash container - oomer := `{"echoContent": "_TRX_ID_", "isDebug": true, "allocateMemory": 12000000}` // ask for 12MB - badHot := `{"echoContent": "_TRX_ID_", "invalidResponse": true, "isDebug": true}` // write a not json/http as output - ok := `{"echoContent": "_TRX_ID_", "isDebug": true}` // good response / ok - respTypeLie := `{"echoContent": "_TRX_ID_", "responseContentType": "foo/bar", "isDebug": true}` // Content-Type: foo/bar - respTypeJason := `{"echoContent": "_TRX_ID_", "jasonContentType": "foo/bar", "isDebug": true}` // Content-Type: foo/bar - - // sleep between logs and with debug enabled, fn-test-utils will log header/footer below: - multiLog := `{"echoContent": "_TRX_ID_", "sleepTime": 1000, "isDebug": true}` - bigoutput := `{"echoContent": "_TRX_ID_", "isDebug": true, "trailerRepeat": 1000}` // 1000 trailers to exceed 2K - smalloutput := `{"echoContent": "_TRX_ID_", "isDebug": true, "trailerRepeat": 1}` // 1 trailer < 2K - - testCases := []struct { - path string - body string - method string - expectedCode int - expectedHeaders map[string][]string - expectedErrSubStr string - expectedLogsSubStr []string - }{ - {"/r/myapp/", ok, "GET", http.StatusOK, expHeaders, "", nil}, - - {"/r/myapp/myhot", badHot, "GET", http.StatusBadGateway, expHeaders, "invalid http response", nil}, - // hot container now back to normal: - {"/r/myapp/myhot", ok, "GET", http.StatusOK, expHeaders, "", nil}, - - {"/r/myapp/myhotjason", badHot, "GET", http.StatusBadGateway, expHeaders, "invalid json response", nil}, - // hot container now back to normal: - {"/r/myapp/myhotjason", ok, "GET", http.StatusOK, expHeaders, "", nil}, - - {"/r/myapp/myhot", respTypeLie, "GET", http.StatusOK, expCTHeaders, "", nil}, - {"/r/myapp/myhotjason", respTypeLie, "GET", http.StatusOK, expCTHeaders, "", nil}, - {"/r/myapp/myhotjason", respTypeJason, "GET", http.StatusOK, expCTHeaders, "", nil}, - - {"/r/myapp/myroute", ok, "GET", http.StatusOK, expHeaders, "", nil}, - {"/r/myapp/myerror", crasher, "GET", http.StatusBadGateway, expHeaders, "container exit code 2", nil}, - {"/r/myapp/mydne", ``, "GET", http.StatusNotFound, nil, "pull access denied", nil}, - {"/r/myapp/mydnehot", ``, "GET", http.StatusNotFound, nil, "pull access denied", nil}, - // hit a registry that doesn't exist, make sure the real error body gets plumbed out - {"/r/myapp/mydneregistry", ``, "GET", http.StatusInternalServerError, nil, "connection refused", nil}, - - {"/r/myapp/myoom", oomer, "GET", http.StatusBadGateway, nil, "container out of memory", nil}, - {"/r/myapp/myhot", multiLog, "GET", http.StatusOK, nil, "", multiLogExpectHot}, - {"/r/myapp/", multiLog, "GET", http.StatusOK, nil, "", multiLogExpectCold}, - {"/r/myapp/mybigoutputjson", bigoutput, "GET", http.StatusBadGateway, nil, "function response too large", nil}, - {"/r/myapp/mybigoutputjson", smalloutput, "GET", http.StatusOK, nil, "", nil}, - {"/r/myapp/mybigoutputhttp", bigoutput, "GET", http.StatusBadGateway, nil, "", nil}, - {"/r/myapp/mybigoutputhttp", smalloutput, "GET", http.StatusOK, nil, "", nil}, - {"/r/myapp/mybigoutputcold", bigoutput, "GET", http.StatusBadGateway, nil, "", nil}, - {"/r/myapp/mybigoutputcold", smalloutput, "GET", http.StatusOK, nil, "", nil}, - } - - callIds := make([]string, len(testCases)) - - for i, test := range testCases { - trx := fmt.Sprintf("_trx_%d_", i) - body := strings.NewReader(strings.Replace(test.body, "_TRX_ID_", trx, 1)) - _, rec := routerRequest(t, srv.Router, test.method, test.path, body) - respBytes, _ := ioutil.ReadAll(rec.Body) - respBody := string(respBytes) - maxBody := len(respBody) - if maxBody > 1024 { - maxBody = 1024 - } - - callIds[i] = rec.Header().Get("Fn_call_id") - - if rec.Code != test.expectedCode { - isFailure = true - t.Errorf("Test %d: Expected status code to be %d but was %d. body: %s", - i, test.expectedCode, rec.Code, respBody[:maxBody]) - } - - if rec.Code == http.StatusOK && !strings.Contains(respBody, trx) { - isFailure = true - t.Errorf("Test %d: Expected response to include %s but got body: %s", - i, trx, respBody[:maxBody]) - - } - - if test.expectedErrSubStr != "" && !strings.Contains(respBody, test.expectedErrSubStr) { - isFailure = true - t.Errorf("Test %d: Expected response to include %s but got body: %s", - i, test.expectedErrSubStr, respBody[:maxBody]) - - } - - if test.expectedHeaders != nil { - for name, header := range test.expectedHeaders { - if header[0] != rec.Header().Get(name) { - isFailure = true - t.Errorf("Test %d: Expected header `%s` to be %s but was %s. body: %s", - i, name, header[0], rec.Header().Get(name), respBody) - } - } - } - } - - for i, test := range testCases { - if test.expectedLogsSubStr != nil { - if !checkLogs(t, i, ls, callIds[i], test.expectedLogsSubStr) { - isFailure = true - } - } - } -} - -func TestRouteRunnerTimeout(t *testing.T) { - buf := setLogBuffer() - isFailure := false - - // Log once after we are done, flow of events are important (hot/cold containers, idle timeout, etc.) - // for figuring out why things failed. - defer func() { - if isFailure { - t.Log(buf.String()) - } - }() - - models.RouteMaxMemory = uint64(1024 * 1024 * 1024) // 1024 TB - hugeMem := uint64(models.RouteMaxMemory - 1) - - app := &models.App{ID: "app_id", Name: "myapp", Config: models.Config{}} - ds := datastore.NewMockInit( - []*models.App{app}, - []*models.Route{ - {Path: "/cold", Image: "fnproject/fn-test-utils", Type: "sync", Memory: 128, Timeout: 4, IdleTimeout: 30, AppID: app.ID}, - {Path: "/hot", Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: 128, Timeout: 4, IdleTimeout: 30, AppID: app.ID}, - {Path: "/hot-json", Image: "fnproject/fn-test-utils", Type: "sync", Format: "json", Memory: 128, Timeout: 4, IdleTimeout: 30, AppID: app.ID}, - {Path: "/bigmem-cold", Image: "fnproject/fn-test-utils", Type: "sync", Memory: hugeMem, Timeout: 1, IdleTimeout: 30, AppID: app.ID}, - {Path: "/bigmem-hot", Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: hugeMem, Timeout: 1, IdleTimeout: 30, AppID: app.ID}, - }, - ) - - fnl := logs.NewMock() - rnr, cancelrnr := testRunner(t, ds, fnl) - defer cancelrnr() - - srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) - - for i, test := range []struct { - path string - body string - method string - expectedCode int - expectedHeaders map[string][]string - }{ - {"/r/myapp/cold", `{"echoContent": "_TRX_ID_", "sleepTime": 0, "isDebug": true}`, "POST", http.StatusOK, nil}, - {"/r/myapp/cold", `{"echoContent": "_TRX_ID_", "sleepTime": 5000, "isDebug": true}`, "POST", http.StatusGatewayTimeout, nil}, - {"/r/myapp/hot", `{"echoContent": "_TRX_ID_", "sleepTime": 5000, "isDebug": true}`, "POST", http.StatusGatewayTimeout, nil}, - {"/r/myapp/hot", `{"echoContent": "_TRX_ID_", "sleepTime": 0, "isDebug": true}`, "POST", http.StatusOK, nil}, - {"/r/myapp/hot-json", `{"echoContent": "_TRX_ID_", "sleepTime": 5000, "isDebug": true}`, "POST", http.StatusGatewayTimeout, nil}, - {"/r/myapp/hot-json", `{"echoContent": "_TRX_ID_", "sleepTime": 0, "isDebug": true}`, "POST", http.StatusOK, nil}, - {"/r/myapp/bigmem-cold", `{"echoContent": "_TRX_ID_", "sleepTime": 0, "isDebug": true}`, "POST", http.StatusBadRequest, nil}, - {"/r/myapp/bigmem-hot", `{"echoContent": "_TRX_ID_", "sleepTime": 0, "isDebug": true}`, "POST", http.StatusBadRequest, nil}, - } { - trx := fmt.Sprintf("_trx_%d_", i) - body := strings.NewReader(strings.Replace(test.body, "_TRX_ID_", trx, 1)) - _, rec := routerRequest(t, srv.Router, test.method, test.path, body) - respBytes, _ := ioutil.ReadAll(rec.Body) - respBody := string(respBytes) - maxBody := len(respBody) - if maxBody > 1024 { - maxBody = 1024 - } - - if rec.Code != test.expectedCode { - isFailure = true - t.Errorf("Test %d: Expected status code to be %d but was %d body: %#v", - i, test.expectedCode, rec.Code, respBody[:maxBody]) - } - - if rec.Code == http.StatusOK && !strings.Contains(respBody, trx) { - isFailure = true - t.Errorf("Test %d: Expected response to include %s but got body: %s", - i, trx, respBody[:maxBody]) - - } - - if test.expectedHeaders != nil { - for name, header := range test.expectedHeaders { - if header[0] != rec.Header().Get(name) { - isFailure = true - t.Errorf("Test %d: Expected header `%s` to be %s but was %s body: %#v", - i, name, header[0], rec.Header().Get(name), respBody[:maxBody]) - } - } - } - } -} - -// Minimal test that checks the possibility of invoking concurrent hot sync functions. -func TestRouteRunnerMinimalConcurrentHotSync(t *testing.T) { - buf := setLogBuffer() - - app := &models.App{ID: "app_id", Name: "myapp", Config: models.Config{}} - ds := datastore.NewMockInit( - []*models.App{app}, - []*models.Route{ - {Path: "/hot", AppID: app.ID, Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: 128, Timeout: 30, IdleTimeout: 5}, - }, - ) - - fnl := logs.NewMock() - rnr, cancelrnr := testRunner(t, ds, fnl) - defer cancelrnr() - - srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) - - for i, test := range []struct { - path string - body string - method string - expectedCode int - expectedHeaders map[string][]string - }{ - {"/r/myapp/hot", `{"sleepTime": 100, "isDebug": true}`, "POST", http.StatusOK, nil}, - } { - errs := make(chan error) - numCalls := 4 - for k := 0; k < numCalls; k++ { - go func() { - body := strings.NewReader(test.body) - _, rec := routerRequest(t, srv.Router, test.method, test.path, body) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - errs <- fmt.Errorf("Test %d: Expected status code to be %d but was %d body: %#v", - i, test.expectedCode, rec.Code, rec.Body.String()) - return - } - - if test.expectedHeaders == nil { - errs <- nil - return - } - for name, header := range test.expectedHeaders { - if header[0] != rec.Header().Get(name) { - t.Log(buf.String()) - errs <- fmt.Errorf("Test %d: Expected header `%s` to be %s but was %s body: %#v", - i, name, header[0], rec.Header().Get(name), rec.Body.String()) - return - } - } - errs <- nil - }() - } - for k := 0; k < numCalls; k++ { - err := <-errs - if err != nil { - t.Errorf("%v", err) - } - } - } -} diff --git a/api/server/server.go b/api/server/server.go index 1334e7587..e20c47faa 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -5,7 +5,6 @@ import ( "bytes" "context" "crypto/tls" - "encoding/base64" "errors" "fmt" "io" @@ -204,7 +203,6 @@ type Server struct { noFnInvokeEndpoint bool noCallEndpoints bool appListeners *appListeners - routeListeners *routeListeners fnListeners *fnListeners triggerListeners *triggerListeners rootMiddlewares []fnext.Middleware @@ -703,13 +701,12 @@ func New(ctx context.Context, opts ...Option) *Server { s.bindHandlers(ctx) s.appListeners = new(appListeners) - s.routeListeners = new(routeListeners) s.fnListeners = new(fnListeners) s.triggerListeners = new(triggerListeners) // TODO it's not clear that this is always correct as the read store won't get wrapping s.datastore = datastore.Wrap(s.datastore) - s.datastore = fnext.NewDatastore(s.datastore, s.appListeners, s.routeListeners, s.fnListeners, s.triggerListeners) + s.datastore = fnext.NewDatastore(s.datastore, s.appListeners, s.fnListeners, s.triggerListeners) s.logstore = logs.Wrap(s.logstore) return s @@ -1086,38 +1083,6 @@ func (s *Server) bindHandlers(ctx context.Context) { switch s.nodeType { case ServerTypeFull, ServerTypeAPI: - clean := engine.Group("/v1") - v1 := clean.Group("") - v1.Use(setAppNameInCtx) - v1.Use(s.apiMiddlewareWrapper()) - v1.GET("/apps", s.handleV1AppList) - v1.POST("/apps", s.handleV1AppCreate) - - { - apps := v1.Group("/apps/:appName") - apps.Use(appNameCheck) - - { - withAppCheck := apps.Group("") - withAppCheck.Use(s.checkAppPresenceByName()) - - withAppCheck.GET("", s.handleV1AppGetByIdOrName) - withAppCheck.PATCH("", s.handleV1AppUpdate) - withAppCheck.DELETE("", s.handleV1AppDelete) - - withAppCheck.GET("/routes", s.handleRouteList) - withAppCheck.GET("/routes/:route", s.handleRouteGetAPI) - withAppCheck.PATCH("/routes/*route", s.handleRoutesPatch) - withAppCheck.DELETE("/routes/*route", s.handleRouteDelete) - withAppCheck.GET("/calls/:call", s.handleCallGet1) - withAppCheck.GET("/calls/:call/log", s.handleCallLogGet1) - withAppCheck.GET("/calls", s.handleCallList1) - } - - apps.POST("/routes", s.handleRoutesPostPut) - apps.PUT("/routes/*route", s.handleRoutesPostPut) - } - cleanv2 := engine.Group("/v2") v2 := cleanv2.Group("") v2.Use(s.apiMiddlewareWrapper()) @@ -1165,7 +1130,6 @@ func (s *Server) bindHandlers(ctx context.Context) { runnerAppAPI.Use(setAppIDInCtx) // Both of these are somewhat odd - // Deprecate, remove with routes - runnerAppAPI.GET("/routes/*route", s.handleRunnerGetRoute) runnerAppAPI.GET("/triggerBySource/:triggerType/*triggerSource", s.handleRunnerGetTriggerBySource) } } @@ -1176,33 +1140,19 @@ func (s *Server) bindHandlers(ctx context.Context) { lbTriggerGroup := engine.Group("/t") lbTriggerGroup.Any("/:appName", s.handleHTTPTriggerCall) lbTriggerGroup.Any("/:appName/*triggerSource", s.handleHTTPTriggerCall) - - // TODO Deprecate with routes - lbRouteGroup := engine.Group("/r") - lbRouteGroup.Use(s.checkAppPresenceByNameAtLB()) - lbRouteGroup.Any("/:appName", s.handleV1FunctionCall) - lbRouteGroup.Any("/:appName/*route", s.handleV1FunctionCall) } if !s.noFnInvokeEndpoint { lbFnInvokeGroup := engine.Group("/invoke") lbFnInvokeGroup.POST("/:fnID", s.handleFnInvokeCall) } - } engine.NoRoute(func(c *gin.Context) { var err error - switch { - case s.nodeType == ServerTypeAPI && strings.HasPrefix(c.Request.URL.Path, "/r/"): - err = models.ErrInvokeNotSupported - case s.nodeType == ServerTypeRunner && strings.HasPrefix(c.Request.URL.Path, "/v1/"): - err = models.ErrAPINotSupported - default: - var e models.APIError = models.ErrPathNotFound - err = models.NewAPIError(e.Code(), fmt.Errorf("%v: %s", e.Error(), c.Request.URL.Path)) - } - handleV1ErrorResponse(c, err) + var e models.APIError = models.ErrPathNotFound + err = models.NewAPIError(e.Code(), fmt.Errorf("%v: %s", e.Error(), c.Request.URL.Path)) + handleErrorResponse(c, err) }) } @@ -1217,26 +1167,7 @@ func (s *Server) Agent() agent.Agent { return s.agent } -// returns the unescaped ?cursor and ?perPage values -// pageParams clamps 0 < ?perPage <= 100 and defaults to 30 if 0 -// ignores parsing errors and falls back to defaults. -func pageParams(c *gin.Context, base64d bool) (cursor string, perPage int) { - cursor = c.Query("cursor") - if base64d { - cbytes, _ := base64.RawURLEncoding.DecodeString(cursor) - cursor = string(cbytes) - } - - perPage, _ = strconv.Atoi(c.Query("per_page")) - if perPage > 100 { - perPage = 100 - } else if perPage <= 0 { - perPage = 30 - } - return cursor, perPage -} - -func pageParamsV2(c *gin.Context) (cursor string, perPage int) { +func pageParams(c *gin.Context) (cursor string, perPage int) { cursor = c.Query("cursor") perPage, _ = strconv.Atoi(c.Query("per_page")) @@ -1247,37 +1178,3 @@ func pageParamsV2(c *gin.Context) (cursor string, perPage int) { } return cursor, perPage } - -type appResponse struct { - Message string `json:"message"` - App *models.App `json:"app"` -} - -//TODO deprecate with V1 -type appsV1Response struct { - Message string `json:"message"` - NextCursor string `json:"next_cursor"` - Apps []*models.App `json:"apps"` -} - -type routeResponse struct { - Message string `json:"message"` - Route *models.Route `json:"route"` -} - -type routesResponse struct { - Message string `json:"message"` - NextCursor string `json:"next_cursor"` - Routes []*models.Route `json:"routes"` -} - -type callResponse struct { - Message string `json:"message"` - Call *models.Call `json:"call"` -} - -type callsResponse struct { - Message string `json:"message"` - NextCursor string `json:"next_cursor"` - Calls []*models.Call `json:"calls"` -} diff --git a/api/server/server_options.go b/api/server/server_options.go index 24feead40..d212106c3 100644 --- a/api/server/server_options.go +++ b/api/server/server_options.go @@ -64,7 +64,7 @@ func limitRequestBody(max int64) func(c *gin.Context) { if cl > max { // try to deny this quickly, instead of just letting it get lopped off - handleV1ErrorResponse(c, errTooBig{cl, max}) + handleErrorResponse(c, errTooBig{cl, max}) c.Abort() return } diff --git a/api/server/server_test.go b/api/server/server_test.go index 14688051f..78821ace0 100644 --- a/api/server/server_test.go +++ b/api/server/server_test.go @@ -3,32 +3,20 @@ package server import ( "bytes" "context" - "crypto/rand" - "encoding/base64" "encoding/json" - "errors" "io" "net/http" "net/http/httptest" - "net/url" - "os" "strconv" - "strings" "testing" "github.com/fnproject/fn/api/agent" _ "github.com/fnproject/fn/api/agent/drivers/docker" - "github.com/fnproject/fn/api/datastore" - "github.com/fnproject/fn/api/datastore/sql" _ "github.com/fnproject/fn/api/datastore/sql/sqlite" - "github.com/fnproject/fn/api/logs" "github.com/fnproject/fn/api/models" - "github.com/fnproject/fn/api/mqs" "github.com/gin-gonic/gin" ) -var tmpDatastoreTests = "/tmp/func_test_datastore.db" - func testServer(ds models.Datastore, mq models.MessageQueue, logDB models.LogStore, rnr agent.Agent, nodeType NodeType, opts ...Option) *Server { return New(context.Background(), append(opts, WithLogLevel(getEnv(EnvLogLevel, DefaultLogLevel)), @@ -91,15 +79,6 @@ func newRouterRequest(t *testing.T, method, path string, body io.Reader) (*http. return req, rec } -func getV1ErrorResponse(t *testing.T, rec *httptest.ResponseRecorder) *models.ErrorWrapper { - var err models.ErrorWrapper - decodeErr := json.NewDecoder(rec.Body).Decode(&err) - if decodeErr != nil { - t.Error("Test: Expected not empty response body") - } - return &err -} - func getErrorResponse(t *testing.T, rec *httptest.ResponseRecorder) *models.Error { var err models.Error decodeErr := json.NewDecoder(rec.Body).Decode(&err) @@ -108,202 +87,3 @@ func getErrorResponse(t *testing.T, rec *httptest.ResponseRecorder) *models.Erro } return &err } - -func prepareDB(ctx context.Context, t *testing.T) (models.Datastore, models.LogStore, func()) { - os.Remove(tmpDatastoreTests) - uri, err := url.Parse("sqlite3://" + tmpDatastoreTests) - if err != nil { - t.Fatal(err) - } - ss, err := sql.New(ctx, uri) - if err != nil { - t.Fatalf("Error when creating datastore: %s", err) - } - logDB := logs.Wrap(ss) - ds := datastore.Wrap(ss) - - return ds, logDB, func() { - os.Remove(tmpDatastoreTests) - } -} - -func TestFullStack(t *testing.T) { - ctx := context.Background() - buf := setLogBuffer() - ds, logDB, close := prepareDB(ctx, t) - defer close() - - rnr, rnrcancel := testRunner(t, ds) - defer rnrcancel() - - srv := testServer(ds, &mqs.Mock{}, logDB, rnr, ServerTypeFull, LimitRequestBody(32256)) - - var bigbufa [32257]byte - rand.Read(bigbufa[:]) - bigbuf := base64.StdEncoding.EncodeToString(bigbufa[:]) // this will be > bigbufa, but json compatible - toobigerr := errors.New("Content-Length too large for this server") - gatewayerr := errors.New("container exit code") - - for _, test := range []struct { - name string - method string - path string - body string - expectedCode int - expectedCacheSize int // TODO kill me - expectedError error - }{ - {"create my app", "POST", "/v1/apps", `{ "app": { "name": "myapp" } }`, http.StatusOK, 0, nil}, - {"list apps", "GET", "/v1/apps", ``, http.StatusOK, 0, nil}, - {"get app", "GET", "/v1/apps/myapp", ``, http.StatusOK, 0, nil}, - // NOTE: cache is lazy, loads when a request comes in for the route, not when added - {"add myroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusOK, 0, nil}, - {"add myroute2", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute2", "path": "/myroute2", "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusOK, 0, nil}, - {"get myroute", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusOK, 0, nil}, - {"get myroute2", "GET", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 0, nil}, - {"get all routes", "GET", "/v1/apps/myapp/routes", ``, http.StatusOK, 0, nil}, - {"execute myroute", "POST", "/r/myapp/myroute", `{ "echoContent": "Teste" }`, http.StatusOK, 1, nil}, - - // fails - {"execute myroute2", "POST", "/r/myapp/myroute2", `{"sleepTime": 0, "isDebug": true, "isCrash": true}`, http.StatusBadGateway, 2, gatewayerr}, - {"request body too large", "POST", "/r/myapp/myroute", bigbuf, http.StatusRequestEntityTooLarge, 0, toobigerr}, - - {"get myroute2", "GET", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 2, nil}, - {"delete myroute", "DELETE", "/v1/apps/myapp/routes/myroute", ``, http.StatusOK, 1, nil}, - {"delete myroute2", "DELETE", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 0, nil}, - {"delete app (success)", "DELETE", "/v1/apps/myapp", ``, http.StatusOK, 0, nil}, - - {"get deleted app", "GET", "/v1/apps/myapp", ``, http.StatusNotFound, 0, models.ErrAppsNotFound}, - {"get deleteds route on deleted app", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusNotFound, 0, models.ErrAppsNotFound}, - } { - t.Run(test.name, func(t *testing.T) { - _, rec := routerRequest(t, srv.Router, test.method, test.path, bytes.NewBuffer([]byte(test.body))) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Log(rec.Body.String()) - t.Errorf("Test \"%s\": Expected status code to be %d but was %d", - test.name, test.expectedCode, rec.Code) - } - - if rec.Code > 300 && test.expectedError == nil { - t.Log(buf.String()) - t.Error("got error when not expected error", rec.Body.String()) - } else if test.expectedError != nil { - if !strings.Contains(rec.Body.String(), test.expectedError.Error()) { - t.Log(buf.String()) - t.Errorf("Test %s: Expected error message to have `%s`, but got `%s`", - test.name, test.expectedError.Error(), rec.Body.String()) - } - } - }) - } -} - -func TestRunnerNode(t *testing.T) { - ctx := context.Background() - buf := setLogBuffer() - ds, logDB, close := prepareDB(ctx, t) - defer close() - - rnr, rnrcancel := testRunner(t, ds) - defer rnrcancel() - - // Add route with an API server using the same DB - { - apiServer := testServer(ds, &mqs.Mock{}, logDB, nil, ServerTypeAPI) - _, rec := routerRequest(t, apiServer.Router, "POST", "/v1/apps/myapp/routes", bytes.NewBuffer([]byte(`{ "route": { "name": "myroute", "path": "/myroute", "image": "fnproject/fn-test-utils", "type": "sync" } }`))) - if rec.Code != http.StatusOK { - t.Errorf("Expected status code 200 when creating sync route, but got %d", rec.Code) - } - _, rec = routerRequest(t, apiServer.Router, "POST", "/v1/apps/myapp/routes", bytes.NewBuffer([]byte(`{ "route": { "name": "myasyncroute", "path": "/myasyncroute", "image": "fnproject/fn-test-utils", "type": "async" } }`))) - if rec.Code != http.StatusOK { - t.Errorf("Expected status code 200 when creating async route, but got %d", rec.Code) - } - } - - srv := testServer(ds, &mqs.Mock{}, logDB, rnr, ServerTypeRunner) - - for _, test := range []struct { - name string - method string - path string - body string - expectedCode int - expectedCacheSize int // TODO kill me - }{ - // Support sync and async API calls - {"execute sync route succeeds", "POST", "/r/myapp/myroute", `{ "echoContent": "Teste" }`, http.StatusOK, 1}, - {"execute async route succeeds", "POST", "/r/myapp/myasyncroute", `{ "echoContent": "Teste" }`, http.StatusAccepted, 1}, - - // All other API functions should not be available on runner nodes - {"create app not found", "POST", "/v1/apps", `{ "app": { "name": "myapp" } }`, http.StatusBadRequest, 0}, - {"list apps not found", "GET", "/v1/apps", ``, http.StatusBadRequest, 0}, - {"get app not found", "GET", "/v1/apps/myapp", ``, http.StatusBadRequest, 0}, - - {"add route not found", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusBadRequest, 0}, - {"get route not found", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusBadRequest, 0}, - {"get all routes not found", "GET", "/v1/apps/myapp/routes", ``, http.StatusBadRequest, 0}, - {"delete app not found", "DELETE", "/v1/apps/myapp", ``, http.StatusBadRequest, 0}, - } { - t.Run(test.name, func(t *testing.T) { - _, rec := routerRequest(t, srv.Router, test.method, test.path, bytes.NewBuffer([]byte(test.body))) - - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Errorf("Test \"%s\": Expected status code to be %d but was %d", - test.name, test.expectedCode, rec.Code) - } - }) - - } -} - -func TestApiNode(t *testing.T) { - ctx := context.Background() - buf := setLogBuffer() - ds, logDB, close := prepareDB(ctx, t) - defer close() - - srv := testServer(ds, &mqs.Mock{}, logDB, nil, ServerTypeAPI) - - for _, test := range []struct { - name string - method string - path string - body string - expectedCode int - expectedCacheSize int // TODO kill me - }{ - // All routes should be supported - {"create my app", "POST", "/v1/apps", `{ "app": { "name": "myapp" } }`, http.StatusOK, 0}, - {"list apps", "GET", "/v1/apps", ``, http.StatusOK, 0}, - {"get app", "GET", "/v1/apps/myapp", ``, http.StatusOK, 0}, - - {"add myroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusOK, 0}, - {"add myroute2", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute2", "path": "/myroute2", "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusOK, 0}, - {"add myasyncroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myasyncroute", "path": "/myasyncroute", "image": "fnproject/fn-test-utils", "type": "async" } }`, http.StatusOK, 0}, - {"get myroute", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusOK, 0}, - {"get myroute2", "GET", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 0}, - {"get all routes", "GET", "/v1/apps/myapp/routes", ``, http.StatusOK, 0}, - - // Don't support calling sync or async - {"execute myroute", "POST", "/r/myapp/myroute", `{ "echoContent": "Teste" }`, http.StatusBadRequest, 1}, - {"execute myroute2", "POST", "/r/myapp/myroute2", `{ "echoContent": "Teste" }`, http.StatusBadRequest, 2}, - {"execute myasyncroute", "POST", "/r/myapp/myasyncroute", `{ "echoContent": "Teste" }`, http.StatusBadRequest, 1}, - - {"get myroute2", "GET", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 2}, - {"delete myroute", "DELETE", "/v1/apps/myapp/routes/myroute", ``, http.StatusOK, 1}, - {"delete myroute2", "DELETE", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 0}, - {"delete app (success)", "DELETE", "/v1/apps/myapp", ``, http.StatusOK, 0}, - {"get deleted app", "GET", "/v1/apps/myapp", ``, http.StatusNotFound, 0}, - {"get deleted route on deleted app", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusNotFound, 0}, - } { - _, rec := routerRequest(t, srv.Router, test.method, test.path, bytes.NewBuffer([]byte(test.body))) - if rec.Code != test.expectedCode { - t.Log(buf.String()) - t.Errorf("Test \"%s\": Expected status code to be %d but was %d", - test.name, test.expectedCode, rec.Code) - } - } -} diff --git a/api/server/trigger_list.go b/api/server/trigger_list.go index 22eca2b8e..0f85a4bf4 100644 --- a/api/server/trigger_list.go +++ b/api/server/trigger_list.go @@ -13,7 +13,7 @@ func (s *Server) handleTriggerList(c *gin.Context) { ctx := c.Request.Context() filter := &models.TriggerFilter{} - filter.Cursor, filter.PerPage = pageParamsV2(c) + filter.Cursor, filter.PerPage = pageParams(c) filter.AppID = c.Query("app_id") diff --git a/docs/swagger.yml b/docs/swagger.yml deleted file mode 100644 index 2b139f746..000000000 --- a/docs/swagger.yml +++ /dev/null @@ -1,773 +0,0 @@ -swagger: '2.0' -info: - title: fn - description: The open source serverless platform. - version: "0.2.5" -# the domain of the service -host: "127.0.0.1:8080" -# array of all schemes that your API supports -schemes: - - https - - http -# will be prefixed to all paths -basePath: /v1 -consumes: - - application/json -produces: - - application/json -paths: - /apps: - get: - summary: "Get all app names." - description: "Get a list of all the apps in the system, returned in alphabetical order." - tags: - - Apps - parameters: - - name: cursor - description: Cursor from previous response.next_cursor to begin results after, if any. - required: false - type: string - in: query - - name: per_page - description: Number of results to return, defaults to 30. Max of 100. - required: false - type: integer - in: query - responses: - 200: - description: List of apps. - schema: - $ref: '#/definitions/AppsWrapper' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - post: - summary: "Post new app" - description: "Insert a new app" - tags: - - Apps - parameters: - - name: body - in: body - description: App to post. - required: true - schema: - $ref: '#/definitions/AppWrapper' - responses: - 200: - description: App details and stats. - schema: - $ref: '#/definitions/AppWrapper' - 400: - description: Parameters are missing or invalid. - schema: - $ref: '#/definitions/Error' - 409: - description: App already exists. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - - - /apps/{app}: - delete: - summary: "Delete an app." - description: "Delete an app." - tags: - - Apps - parameters: - - name: app - in: path - description: Name of the app. - required: true - type: string - responses: - 200: - description: Apps successfully deleted. - 404: - description: App does not exist. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - get: - summary: "Get information for a app." - description: "This gives more details about a app, such as statistics." - tags: - - Apps - parameters: - - name: app - in: path - description: name of the app. - required: true - type: string - responses: - 200: - description: App details and stats. - schema: - $ref: '#/definitions/AppWrapper' - 404: - description: App does not exist. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - patch: - summary: "Updates an app." - description: "You can set app level settings here. " - tags: - - Apps - parameters: - - name: app - in: path - description: name of the app. - required: true - type: string - - name: body - in: body - description: App to post. - required: true - schema: - $ref: '#/definitions/AppWrapper' - responses: - 200: - description: App details and stats. - schema: - $ref: '#/definitions/AppWrapper' - 400: - description: Parameters are missing or invalid. - schema: - $ref: '#/definitions/Error' - 404: - description: App does not exist. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - - /apps/{app}/routes: - post: - summary: Create new Route - description: Create a new route in an app, if app doesn't exists, it creates the app. Post does not skip validation of zero values. - tags: - - Routes - parameters: - - name: app - in: path - description: name of the app. - required: true - type: string - - name: body - in: body - description: One route to post. - required: true - schema: - $ref: '#/definitions/RouteWrapper' - responses: - 200: - description: Route created - schema: - $ref: '#/definitions/RouteWrapper' - 400: - description: Invalid route due to parameters being missing or invalid. - schema: - $ref: '#/definitions/Error' - 409: - description: Route already exists. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - - get: - summary: Get route list by app name. - description: This will list routes for a particular app, returned in alphabetical order. - tags: - - Routes - parameters: - - name: app - in: path - description: Name of app for this set of routes. - required: true - type: string - - name: image - description: Route image to match, exact. - required: false - type: string - in: query - - name: cursor - description: Cursor from previous response.next_cursor to begin results after, if any. - required: false - type: string - in: query - - name: per_page - description: Number of results to return, defaults to 30. Max of 100. - required: false - type: integer - in: query - responses: - 200: - description: Route information - schema: - $ref: '#/definitions/RoutesWrapper' - 404: - description: App does not exist. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - - /apps/{app}/routes/{route}: - put: - summary: Create a Route if it does not exist. Update if it does. Will also create app if it does not exist. Put does not skip validation of zero values - description: Update or Create a route - tags: - - Routes - parameters: - - name: app - in: path - description: name of the app. - required: true - type: string - - name: route - in: path - description: route path. - required: true - type: string - - name: body - in: body - description: One route to post. - required: true - schema: - $ref: '#/definitions/RouteWrapper' - responses: - 200: - description: Route created or updated - schema: - $ref: '#/definitions/RouteWrapper' - 400: - description: Invalid route due to parameters being missing or invalid. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - patch: - summary: Update a Route, Fails if the route or app does not exist. Accepts partial updates / skips validation of zero values. - description: Update a route - tags: - - Routes - parameters: - - name: app - in: path - description: name of the app. - required: true - type: string - - name: route - in: path - description: route path. - required: true - type: string - - name: body - in: body - description: One route to post. - required: true - schema: - $ref: '#/definitions/RouteWrapper' - responses: - 200: - description: Route updated - schema: - $ref: '#/definitions/RouteWrapper' - 400: - description: Invalid route due to parameters being missing or invalid. - schema: - $ref: '#/definitions/Error' - 404: - description: App / Route does not exist. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - get: - summary: Gets route by name - description: Gets a route by name. - tags: - - Routes - parameters: - - name: app - in: path - description: Name of app for this set of routes. - required: true - type: string - - name: route - in: path - description: Route name - required: true - type: string - responses: - 200: - description: Route information - schema: - $ref: '#/definitions/RouteWrapper' - 404: - description: Route does not exist. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - - delete: - summary: Deletes the route - tags: - - Routes - description: Deletes the route. - parameters: - - name: app - in: path - description: Name of app for this set of routes. - required: true - type: string - - name: route - in: path - description: Route name - required: true - type: string - responses: - 200: - description: Route successfully deleted. - 404: - description: Route does not exist. - schema: - $ref: '#/definitions/Error' - default: - description: Unexpected error - schema: - $ref: '#/definitions/Error' - - /apps/{app}/calls/{call}/log: - get: - summary: Get call logs - description: Get call logs - tags: - - Call - - Log - parameters: - - name: app - description: App Name - required: true - type: string - in: path - - name: call - description: Call ID. - required: true - type: string - in: path - responses: - 200: - description: Log found - schema: - $ref: '#/definitions/LogWrapper' - 404: - description: Log not found. - schema: - $ref: '#/definitions/Error' - - /apps/{app}/calls/{call}: - get: - summary: Get call information - description: Get call information - tags: - - Call - parameters: - - name: app - description: app name - required: true - type: string - in: path - - name: call - description: Call ID. - required: true - type: string - in: path - responses: - 200: - description: Call found - schema: - $ref: '#/definitions/CallWrapper' - 404: - description: Call not found. - schema: - $ref: '#/definitions/Error' - - /apps/{app}/calls: - get: - summary: Get app-bound calls. - description: Get app-bound calls can filter to route-bound calls, results returned in created_at, descending order (newest first). - tags: - - Call - parameters: - - name: app - description: App name. - required: true - type: string - in: path - - name: path - description: Route path to match, exact. - required: false - type: string - in: query - - name: cursor - description: Cursor from previous response.next_cursor to begin results after, if any. - required: false - type: string - in: query - - name: per_page - description: Number of results to return, defaults to 30. Max of 100. - required: false - type: integer - in: query - - name: from_time - description: Unix timestamp in seconds, of call.created_at to begin the results at, default 0. - required: false - type: integer - in: query - - name: to_time - description: Unix timestamp in seconds, of call.created_at to end the results at, defaults to latest. - required: false - type: integer - in: query - responses: - 200: - description: Calls found - schema: - $ref: '#/definitions/CallsWrapper' - 404: - description: Calls not found. - schema: - $ref: '#/definitions/Error' - -definitions: - Route: - type: object - properties: - app_id: - type: string - description: App ID - path: - type: string - description: URL path that will be matched to this route - readOnly: true - image: - description: Name of Docker image to use in this route. You should include the image tag, which should be a version number, to be more accurate. Can be overridden on a per route basis with route.image. - type: string - headers: - type: object - description: Map of http headers that will be sent with the response - additionalProperties: - type: array - items: - type: string - memory: - type: integer - format: uint64 - description: Max usable memory for this route (MiB). - cpus: - type: string - description: Max usable CPU cores for this route. Value in MilliCPUs (eg. 500m) or as floating-point (eg. 0.5) - type: - enum: - - sync - - async - description: Route type - type: string - format: - enum: - - default - - http - - json - description: Payload format sent into function. - type: string - config: - type: object - description: Route configuration - overrides application configuration - additionalProperties: - type: string - timeout: - type: integer - default: 30 - format: int32 - description: Timeout for executions of this route. Value in Seconds - idle_timeout: - type: integer - default: 30 - format: int32 - description: Hot functions idle timeout before termination. Value in Seconds - tmpfs_size: - type: integer - format: uint32 - description: Tmpfs size (MiB). - annotations: - type: object - description: Route annotations - this is a map of annotations attached to this route, keys must not exceed 128 bytes and must consist of non-whitespace printable ascii characters, and the seralized representation of individual values must not exeed 512 bytes - additionalProperties: - type: object - created_at: - type: string - format: date-time - description: Time when route was created. Always in UTC. - readOnly: true - updated_at: - type: string - format: date-time - description: Most recent time that route was updated. Always in UTC. - readOnly: true - - App: - type: object - properties: - id: - type: string - description: App ID - readOnly: true - name: - type: string - description: "Name of this app. Must be different than the image name. Can ony contain alphanumeric, -, and _." - readOnly: true - config: - type: object - description: Application function configuration, applied to all routes. - additionalProperties: - type: string - annotations: - type: object - description: Application annotations - this is a map of annotations attached to this app, keys must not exceed 128 bytes and must consist of non-whitespace printable ascii characters, and the seralized representation of individual values must not exeed 512 bytes - additionalProperties: - type: object - syslog_url: - type: string - description: A comma separated list of syslog urls to send all function logs to. supports tls, udp or tcp. e.g. tls://logs.papertrailapp.com:1 - created_at: - type: string - format: date-time - description: Time when app was created. Always in UTC. - readOnly: true - updated_at: - type: string - format: date-time - description: Most recent time that app was updated. Always in UTC. - readOnly: true - - RoutesWrapper: - type: object - required: - - routes - properties: - next_cursor: - type: string - description: cursor to send with subsequent request to receive the next page, if non-empty - readOnly: true - routes: - type: array - items: - $ref: '#/definitions/Route' - error: - $ref: '#/definitions/ErrorBody' - - RouteWrapper: - type: object - required: - - route - properties: - message: - type: string - error: - $ref: '#/definitions/ErrorBody' - route: - $ref: '#/definitions/Route' - - AppsWrapper: - type: object - required: - - apps - properties: - next_cursor: - type: string - description: cursor to send with subsequent request to receive the next page, if non-empty - readOnly: true - apps: - type: array - items: - $ref: '#/definitions/App' - error: - $ref: '#/definitions/ErrorBody' - - AppWrapper: - type: object - required: - - app - properties: - app: - $ref: '#/definitions/App' - error: - $ref: '#/definitions/ErrorBody' - - CallsWrapper: - type: object - required: - - calls - properties: - next_cursor: - type: string - description: cursor to send with subsequent request to receive the next page, if non-empty - readOnly: true - calls: - type: array - items: - $ref: '#/definitions/Call' - error: - $ref: '#/definitions/ErrorBody' - - CallWrapper: - type: object - required: - - call - properties: - call: - $ref: '#/definitions/Call' - description: "Call object." - - LogWrapper: - type: object - required: - - log - properties: - log: - $ref: '#/definitions/Log' - description: "Call log entry." - - Log: - type: object - properties: - call_id: - type: string - description: Call UUID ID - log: - type: string # maybe bytes, long logs wouldn't fit into string type - - Call: - type: object - properties: - id: - type: string - description: Call UUID ID. - readOnly: true - status: - type: string - description: Call execution status. - readOnly: true - error: - type: string - description: Call execution error, if status is 'error'. - readOnly: true - app_id: - type: string - description: App ID that is assigned to a route that is being executed. - readOnly: true - path: - type: string - description: App route that is being executed. - readOnly: true - created_at: - type: string - format: date-time - description: Time when call was submitted. Always in UTC. - readOnly: true - started_at: - type: string - format: date-time - description: Time when call started execution. Always in UTC. - readOnly: true - completed_at: - type: string - format: date-time - description: Time when call completed, whether it was successul or failed. Always in UTC. - readOnly: true - stats: - type: array - items: - $ref: '#/definitions/Stat' - description: A histogram of stats for a call, each is a snapshot of a calls state at the timestamp. - readOnly: true - - Stat: - type: object - properties: - timestamp: - type: string - format: date-time - metrics: - type: object - properties: - net_rx: - type: integer - format: int64 - net_tx: - type: integer - format: int64 - mem_limit: - type: integer - format: int64 - mem_usage: - type: integer - format: int64 - disk_read: - type: integer - format: int64 - disk_write: - type: integer - format: int64 - cpu_user: - type: integer - format: int64 - cpu_total: - type: integer - format: int64 - cpu_kernel: - type: integer - format: int64 - - ErrorBody: - type: object - properties: - message: - type: string - readOnly: true - fields: - type: string - readOnly: true - - Error: - type: object - properties: - error: - $ref: '#/definitions/ErrorBody' diff --git a/docs/swagger_v2.yml b/docs/swagger_v2.yml index e6918b1a0..d6a919633 100644 --- a/docs/swagger_v2.yml +++ b/docs/swagger_v2.yml @@ -433,6 +433,8 @@ paths: description: "Calls not found" schema: $ref: '#/definitions/Error' + 410: + description: Server does not support this operation. /fns/{fnID}/calls/{callID}: get: @@ -452,6 +454,8 @@ paths: description: Call not found. schema: $ref: '#/definitions/Error' + 410: + description: Server does not support this operation. /fns/{fnID}/calls/{callID}/log: get: @@ -473,6 +477,8 @@ paths: description: Log not found. schema: $ref: '#/definitions/Error' + 410: + description: Server does not support this operation. definitions: App: diff --git a/examples/extensions/main.go b/examples/extensions/main.go index 328b22c56..a0efba01e 100644 --- a/examples/extensions/main.go +++ b/examples/extensions/main.go @@ -29,14 +29,6 @@ func main() { fmt.Println("custom4Handler called") fmt.Fprintf(w, "Hello app %v func, %q", app.Name, html.EscapeString(r.URL.Path)) }) - // the following will be at /v1/apps/:app_name/routes/:route_name/custom5 - // and /v1/apps/:app_name/routes/:route_name/custom6 - funcServer.AddRouteEndpoint("GET", "/custom5", &custom5Handler{}) - funcServer.AddRouteEndpointFunc("GET", "/custom6", func(w http.ResponseWriter, r *http.Request, app *models.App, route *models.Route) { - // fmt.Fprintf(w, "Hello, %q", html.EscapeString(r.URL.Path)) - fmt.Println("custom6Handler called") - fmt.Fprintf(w, "Hello app %v, route %v, request %q", app.Name, route.Path, html.EscapeString(r.URL.Path)) - }) funcServer.Start(ctx) } @@ -53,10 +45,3 @@ func (h *custom3Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, app * fmt.Println("custom3Handler called") fmt.Fprintf(w, "Hello app %v, %q", app.Name, html.EscapeString(r.URL.Path)) } - -type custom5Handler struct{} - -func (h *custom5Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, app *models.App, route *models.Route) { - fmt.Println("custom5Handler called") - fmt.Fprintf(w, "Hello! app %v, route %v, request %q", app.Name, route.Path, html.EscapeString(r.URL.Path)) -} diff --git a/fnext/api.go b/fnext/api.go index 18a0a33d4..43732af6c 100644 --- a/fnext/api.go +++ b/fnext/api.go @@ -34,18 +34,3 @@ type APIAppHandlerFunc func(w http.ResponseWriter, r *http.Request, app *models. func (f APIAppHandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request, app *models.App) { f(w, r, app) } - -// APIRouteHandler may be used to add an http endpoint on the versioned route of fn API, -// at /:version/apps/:app/routes/:route -type APIRouteHandler interface { - // Handle(ctx context.Context) - ServeHTTP(w http.ResponseWriter, r *http.Request, app *models.App, route *models.Route) -} - -// APIRouteHandlerFunc is a convenience for getting an APIRouteHandler. -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) -} diff --git a/fnext/datastore.go b/fnext/datastore.go index fafc59ade..cd8782de1 100644 --- a/fnext/datastore.go +++ b/fnext/datastore.go @@ -8,11 +8,10 @@ import ( // NewDatastore returns a Datastore that wraps the provided Datastore, calling any relevant // listeners around any of the Datastore methods. -func NewDatastore(ds models.Datastore, al AppListener, rl RouteListener, fl FnListener, tl TriggerListener) models.Datastore { +func NewDatastore(ds models.Datastore, al AppListener, fl FnListener, tl TriggerListener) models.Datastore { return &extds{ Datastore: ds, al: al, - rl: rl, fl: fl, tl: tl, } @@ -21,7 +20,6 @@ func NewDatastore(ds models.Datastore, al AppListener, rl RouteListener, fl FnLi type extds struct { models.Datastore al AppListener - rl RouteListener fl FnListener tl TriggerListener } @@ -147,48 +145,6 @@ func (e *extds) GetApps(ctx context.Context, filter *models.AppFilter) (*models. return apps, err } -func (e *extds) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - err := e.rl.BeforeRouteCreate(ctx, route) - if err != nil { - return nil, err - } - - route, err = e.Datastore.InsertRoute(ctx, route) - if err != nil { - return nil, err - } - - err = e.rl.AfterRouteCreate(ctx, route) - return route, err -} - -func (e *extds) UpdateRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - err := e.rl.BeforeRouteUpdate(ctx, route) - if err != nil { - return nil, err - } - - route, err = e.Datastore.UpdateRoute(ctx, route) - if err != nil { - return nil, err - } - - err = e.rl.AfterRouteUpdate(ctx, route) - return route, err -} - -func (e *extds) RemoveRoute(ctx context.Context, appId string, routePath string) error { - err := e.rl.BeforeRouteDelete(ctx, appId, routePath) - if err != nil { - return err - } - err = e.Datastore.RemoveRoute(ctx, appId, routePath) - if err != nil { - return err - } - return e.rl.AfterRouteDelete(ctx, appId, routePath) -} - func (e *extds) InsertFn(ctx context.Context, fn *models.Fn) (*models.Fn, error) { err := e.fl.BeforeFnCreate(ctx, fn) if err != nil { diff --git a/fnext/listeners.go b/fnext/listeners.go index e446c558c..5cbb70f93 100644 --- a/fnext/listeners.go +++ b/fnext/listeners.go @@ -41,22 +41,6 @@ type AppListener interface { // } } -// RouteListener is an interface used to inject custom code at key points in the route lifecycle. -type RouteListener interface { - // BeforeRouteCreate called before route created in the datastore - BeforeRouteCreate(ctx context.Context, route *models.Route) error - // AfterRouteCreate called after route create in the datastore - AfterRouteCreate(ctx context.Context, route *models.Route) error - // BeforeRouteUpdate called before route update in datastore - BeforeRouteUpdate(ctx context.Context, route *models.Route) error - // AfterRouteUpdate called after route updated in datastore - AfterRouteUpdate(ctx context.Context, route *models.Route) error - // BeforeRouteDelete called before route deleted from the datastore - BeforeRouteDelete(ctx context.Context, appId string, routePath string) error - // AfterRouteDelete called after route deleted from the datastore - AfterRouteDelete(ctx context.Context, appId string, routePath string) error -} - // FnListener enables callbacks around Fn events type FnListener interface { // BeforeFnCreate called before fn created in the datastore diff --git a/fnext/middleware.go b/fnext/middleware.go index 6c9c667e1..11e2b6cd4 100644 --- a/fnext/middleware.go +++ b/fnext/middleware.go @@ -1,29 +1,9 @@ package fnext import ( - "context" "net/http" ) -// MiddlewareController allows a bit more flow control to the middleware, since we multiple paths a request can go down. -// 1) Could be routed towards the API -// 2) Could be routed towards a function -type MiddlewareController interface { - - // CallFunction skips any API routing and goes down the function path - CallFunction(w http.ResponseWriter, r *http.Request) - - // If function has already been called - FunctionCalled() bool -} - -// GetMiddlewareController returns MiddlewareController from context. -func GetMiddlewareController(ctx context.Context) MiddlewareController { - // return ctx.(MiddlewareContext) - v := ctx.Value(MiddlewareControllerKey) - return v.(MiddlewareController) -} - // 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 { diff --git a/fnext/setup.go b/fnext/setup.go index 429439d72..4475d9e3b 100644 --- a/fnext/setup.go +++ b/fnext/setup.go @@ -38,10 +38,6 @@ type ExtServer interface { AddAppEndpoint(method, path string, handler APIAppHandler) // AddAppEndpoint adds an endpoints to /v1/apps/:app/x AddAppEndpointFunc(method, path string, handler func(w http.ResponseWriter, r *http.Request, app *models.App)) - // AddRouteEndpoint adds an endpoints to /v1/apps/:app/routes/:route/x - AddRouteEndpoint(method, path string, handler APIRouteHandler) - // AddRouteEndpoint adds an endpoints to /v1/apps/:app/routes/:route/x - AddRouteEndpointFunc(method, path string, handler func(w http.ResponseWriter, r *http.Request, app *models.App, route *models.Route)) // Datastore returns the Datastore Fn is using Datastore() models.Datastore diff --git a/test.sh b/test.sh index 4f2d80a0e..4f7152769 100755 --- a/test.sh +++ b/test.sh @@ -23,5 +23,4 @@ go vet $(go list ./... | grep -v vendor) remove_containers ${CONTEXT} -docker run -v `pwd`:/go/src/github.com/fnproject/fn --rm fnproject/swagger:0.0.1 /go/src/github.com/fnproject/fn/docs/swagger.yml docker run -v `pwd`:/go/src/github.com/fnproject/fn --rm fnproject/swagger:0.0.1 /go/src/github.com/fnproject/fn/docs/swagger_v2.yml diff --git a/test/fn-system-tests/exec_route_test.go b/test/fn-system-tests/exec_fn_test.go similarity index 66% rename from test/fn-system-tests/exec_route_test.go rename to test/fn-system-tests/exec_fn_test.go index cf89450da..bc2f621c2 100644 --- a/test/fn-system-tests/exec_route_test.go +++ b/test/fn-system-tests/exec_fn_test.go @@ -3,9 +3,7 @@ package tests import ( "bytes" "context" - "encoding/json" "fmt" - "io" "net/http" "net/url" "path" @@ -13,6 +11,7 @@ import ( "testing" "time" + "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/models" ) @@ -21,7 +20,20 @@ import ( func TestCanExecuteFunction(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() - rt := ensureRoute(t) + + app := &models.App{Name: id.New().String()} + app = ensureApp(t, app) + + fn := &models.Fn{ + AppID: app.ID, + Name: id.New().String(), + Image: image, + Format: format, + ResourceConfig: models.ResourceConfig{ + Memory: memory, + }, + } + fn = ensureFn(t, fn) lb, err := LB() if err != nil { @@ -31,13 +43,13 @@ func TestCanExecuteFunction(t *testing.T) { Scheme: "http", Host: lb, } - u.Path = path.Join(u.Path, "r", appName, rt.Path) + u.Path = path.Join(u.Path, "invoke", fn.ID) body := `{"echoContent": "HelloWorld", "sleepTime": 0, "isDebug": true}` content := bytes.NewBuffer([]byte(body)) output := &bytes.Buffer{} - resp, err := callFN(ctx, u.String(), content, output, "POST") + resp, err := callFN(ctx, u.String(), content, output) if err != nil { t.Fatalf("Got unexpected error: %v", err) } @@ -68,7 +80,20 @@ func TestCanExecuteFunction(t *testing.T) { func TestCanExecuteBigOutput(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() - rt := ensureRoute(t) + + app := &models.App{Name: id.New().String()} + app = ensureApp(t, app) + + fn := &models.Fn{ + AppID: app.ID, + Name: id.New().String(), + Image: image, + Format: format, + ResourceConfig: models.ResourceConfig{ + Memory: memory, + }, + } + fn = ensureFn(t, fn) lb, err := LB() if err != nil { @@ -78,14 +103,14 @@ func TestCanExecuteBigOutput(t *testing.T) { Scheme: "http", Host: lb, } - u.Path = path.Join(u.Path, "r", appName, rt.Path) + u.Path = path.Join(u.Path, "invoke", fn.ID) // Approx 5.3MB output body := `{"echoContent": "HelloWorld", "sleepTime": 0, "isDebug": true, "trailerRepeat": 410000}` content := bytes.NewBuffer([]byte(body)) output := &bytes.Buffer{} - resp, err := callFN(ctx, u.String(), content, output, "POST") + resp, err := callFN(ctx, u.String(), content, output) if err != nil { t.Fatalf("Got unexpected error: %v", err) } @@ -105,7 +130,20 @@ func TestCanExecuteBigOutput(t *testing.T) { func TestCanExecuteTooBigOutput(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() - rt := ensureRoute(t) + + app := &models.App{Name: id.New().String()} + app = ensureApp(t, app) + + fn := &models.Fn{ + AppID: app.ID, + Name: id.New().String(), + Image: image, + Format: format, + ResourceConfig: models.ResourceConfig{ + Memory: memory, + }, + } + fn = ensureFn(t, fn) lb, err := LB() if err != nil { @@ -115,19 +153,19 @@ func TestCanExecuteTooBigOutput(t *testing.T) { Scheme: "http", Host: lb, } - u.Path = path.Join(u.Path, "r", appName, rt.Path) + u.Path = path.Join(u.Path, "invoke", fn.ID) // > 6MB output body := `{"echoContent": "HelloWorld", "sleepTime": 0, "isDebug": true, "trailerRepeat": 600000}` content := bytes.NewBuffer([]byte(body)) output := &bytes.Buffer{} - resp, err := callFN(ctx, u.String(), content, output, "POST") + resp, err := callFN(ctx, u.String(), content, output) if err != nil { t.Fatalf("Got unexpected error: %v", err) } - exp := "{\"error\":{\"message\":\"function response too large\"}}\n" + exp := "{\"message\":\"function response too large\"}\n" actual := output.String() if !strings.Contains(exp, actual) || len(exp) != len(actual) { @@ -142,7 +180,20 @@ func TestCanExecuteTooBigOutput(t *testing.T) { func TestCanExecuteEmptyOutput(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() - rt := ensureRoute(t) + + app := &models.App{Name: id.New().String()} + app = ensureApp(t, app) + + fn := &models.Fn{ + AppID: app.ID, + Name: id.New().String(), + Image: image, + Format: format, + ResourceConfig: models.ResourceConfig{ + Memory: memory, + }, + } + fn = ensureFn(t, fn) lb, err := LB() if err != nil { @@ -152,14 +203,14 @@ func TestCanExecuteEmptyOutput(t *testing.T) { Scheme: "http", Host: lb, } - u.Path = path.Join(u.Path, "r", appName, rt.Path) + u.Path = path.Join(u.Path, "invoke", fn.ID) // empty body output body := `{"sleepTime": 0, "isDebug": true, "isEmptyBody": true}` content := bytes.NewBuffer([]byte(body)) output := &bytes.Buffer{} - resp, err := callFN(ctx, u.String(), content, output, "POST") + resp, err := callFN(ctx, u.String(), content, output) if err != nil { t.Fatalf("Got unexpected error: %v", err) } @@ -178,7 +229,20 @@ func TestCanExecuteEmptyOutput(t *testing.T) { func TestBasicConcurrentExecution(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() - rt := ensureRoute(t) + + app := &models.App{Name: id.New().String()} + app = ensureApp(t, app) + + fn := &models.Fn{ + AppID: app.ID, + Name: id.New().String(), + Image: image, + Format: format, + ResourceConfig: models.ResourceConfig{ + Memory: memory, + }, + } + fn = ensureFn(t, fn) lb, err := LB() if err != nil { @@ -188,7 +252,7 @@ func TestBasicConcurrentExecution(t *testing.T) { Scheme: "http", Host: lb, } - u.Path = path.Join(u.Path, "r", appName, rt.Path) + u.Path = path.Join(u.Path, "invoke", fn.ID) results := make(chan error) concurrentFuncs := 10 @@ -197,7 +261,7 @@ func TestBasicConcurrentExecution(t *testing.T) { body := `{"echoContent": "HelloWorld", "sleepTime": 0, "isDebug": true}` content := bytes.NewBuffer([]byte(body)) output := &bytes.Buffer{} - resp, err := callFN(ctx, u.String(), content, output, "POST") + resp, err := callFN(ctx, u.String(), content, output) if err != nil { results <- fmt.Errorf("Got unexpected error: %v", err) return @@ -222,87 +286,4 @@ func TestBasicConcurrentExecution(t *testing.T) { t.Fatalf("Error in basic concurrency execution test: %v", err) } } - -} - -func callFN(ctx context.Context, u string, content io.Reader, output io.Writer, method string) (*http.Response, error) { - if method == "" { - if content == nil { - method = "GET" - } else { - method = "POST" - } - } - - req, err := http.NewRequest(method, u, content) - if err != nil { - return nil, fmt.Errorf("error running route: %s", err) - } - req.Header.Set("Content-Type", "application/json") - req = req.WithContext(ctx) - - resp, err := http.DefaultClient.Do(req) - if err != nil { - return nil, fmt.Errorf("error running route: %s", err) - } - - io.Copy(output, resp.Body) - - return resp, nil -} - -func ensureRoute(t *testing.T, rts ...*models.Route) *models.Route { - var rt *models.Route - if len(rts) > 0 { - rt = rts[0] - } else { - rt = &models.Route{ - Path: routeName + "yabbadabbadoo", - Image: image, - Format: format, - Memory: memory, - Type: typ, - } - } - var wrapped struct { - Route *models.Route `json:"route"` - } - - wrapped.Route = rt - - var buf bytes.Buffer - err := json.NewEncoder(&buf).Encode(wrapped) - if err != nil { - t.Fatal("error encoding body", err) - } - - urlStr := host() + "/v1/apps/" + appName + "/routes" + rt.Path - u, err := url.Parse(urlStr) - if err != nil { - t.Fatal("error creating url", urlStr, err) - } - - req, err := http.NewRequest("PUT", u.String(), &buf) - if err != nil { - t.Fatal("error creating request", err) - } - - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatal("error creating route", err) - } - - buf.Reset() - io.Copy(&buf, resp.Body) - if resp.StatusCode != 200 { - t.Fatal("error creating/updating app or otherwise ensuring it exists:", resp.StatusCode, buf.String()) - } - - wrapped.Route = nil - err = json.NewDecoder(&buf).Decode(&wrapped) - if err != nil { - t.Fatal("error decoding response") - } - - return wrapped.Route } diff --git a/test/fn-system-tests/exec_runner_status_test.go b/test/fn-system-tests/exec_runner_status_test.go index 2e6ee61c7..8fae2f447 100644 --- a/test/fn-system-tests/exec_runner_status_test.go +++ b/test/fn-system-tests/exec_runner_status_test.go @@ -3,6 +3,7 @@ package tests import ( "bytes" "context" + "fmt" "io" "net/http" "net/url" @@ -10,10 +11,31 @@ import ( "testing" "time" + "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/models" "github.com/fnproject/fn/api/runnerpool" ) +func callFN(ctx context.Context, u string, content io.Reader, output io.Writer) (*http.Response, error) { + method := "POST" + + req, err := http.NewRequest(method, u, content) + if err != nil { + return nil, fmt.Errorf("error running fn: %s", err) + } + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(ctx) + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, fmt.Errorf("error running fn: %s", err) + } + + io.Copy(output, resp.Body) + + return resp, nil +} + // We should not be able to invoke a StatusImage func TestCannotExecuteStatusImage(t *testing.T) { if StatusImage == "" { @@ -23,15 +45,19 @@ func TestCannotExecuteStatusImage(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - rt := &models.Route{ - Path: routeName + "yogurt", + app := &models.App{Name: id.New().String()} + app = ensureApp(t, app) + + fn := &models.Fn{ + AppID: app.ID, + Name: id.New().String(), Image: StatusImage, Format: format, - Memory: memory, - Type: typ, + ResourceConfig: models.ResourceConfig{ + Memory: memory, + }, } - - rt = ensureRoute(t, rt) + fn = ensureFn(t, fn) lb, err := LB() if err != nil { @@ -41,12 +67,12 @@ func TestCannotExecuteStatusImage(t *testing.T) { Scheme: "http", Host: lb, } - u.Path = path.Join(u.Path, "r", appName, rt.Path) + u.Path = path.Join(u.Path, "invoke", fn.ID) content := bytes.NewBuffer([]byte(`status`)) output := &bytes.Buffer{} - resp, err := callFN(ctx, u.String(), content, output, "POST") + resp, err := callFN(ctx, u.String(), content, output) if err != nil { t.Fatalf("Got unexpected error: %v", err) }