diff --git a/Gopkg.lock b/Gopkg.lock index 0afd1fdda..386f2958c 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -659,6 +659,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "a7ba96a9cacdff4bbcb3f1824aea4095601292e2ac97987245bb27f9853d887b" + inputs-digest = "476e0b453b4947a75639db11c7a092d3157be8e665eb7a7a2633422f737f5369" solver-name = "gps-cdcl" solver-version = 1 diff --git a/api/agent/agent.go b/api/agent/agent.go index 12c61b1de..020d9108c 100644 --- a/api/agent/agent.go +++ b/api/agent/agent.go @@ -92,6 +92,12 @@ type Agent interface { // Enqueue is to use the agent's sweet sweet client bindings to remotely // queue async tasks and should be removed from Agent interface ASAP. Enqueue(context.Context, *models.Call) error + + // GetAppID is to get the match of an app name to its ID + GetAppID(ctx context.Context, appName string) (string, error) + + // GetAppByID is to get the app by ID + GetAppByID(ctx context.Context, appID string) (*models.App, error) } type agent struct { @@ -151,6 +157,14 @@ func createAgent(da DataAccess, withDocker bool) Agent { return a } +func (a *agent) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + return a.da.GetAppByID(ctx, appID) +} + +func (a *agent) GetAppID(ctx context.Context, appName string) (string, error) { + return a.da.GetAppID(ctx, appName) +} + // TODO shuffle this around somewhere else (maybe) func (a *agent) Enqueue(ctx context.Context, call *models.Call) error { return a.da.Enqueue(ctx, call) @@ -645,7 +659,7 @@ func (a *agent) runHot(ctx context.Context, call *call, tok ResourceToken, state container, closer := NewHotContainer(call, &a.cfg) defer closer() - logger := logrus.WithFields(logrus.Fields{"id": container.id, "app": call.AppName, "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, "route": call.Path, "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.Prepare(ctx, container) @@ -836,10 +850,10 @@ func NewHotContainer(call *call, cfg *AgentConfig) (*container, func()) { // have to be read or *BOTH* blocked consistently. In other words, we cannot block one and continue // reading from the other one without risking head-of-line blocking. stderr.Swap(newLineWriter(&logWriter{ - logrus.WithFields(logrus.Fields{"tag": "stderr", "app_name": call.AppName, "path": call.Path, "image": call.Image, "container_id": id}), + logrus.WithFields(logrus.Fields{"tag": "stderr", "app_id": call.AppID, "path": call.Path, "image": call.Image, "container_id": id}), })) stdout.Swap(newLineWriter(&logWriter{ - logrus.WithFields(logrus.Fields{"tag": "stdout", "app_name": call.AppName, "path": call.Path, "image": call.Image, "container_id": id}), + logrus.WithFields(logrus.Fields{"tag": "stdout", "app_id": call.AppID, "path": call.Path, "image": call.Image, "container_id": id}), })) } diff --git a/api/agent/agent_test.go b/api/agent/agent_test.go index 247fcbf8b..56b60d460 100644 --- a/api/agent/agent_test.go +++ b/api/agent/agent_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/fnproject/fn/api/datastore" + "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/models" "github.com/fnproject/fn/api/mqs" "github.com/sirupsen/logrus" @@ -35,7 +36,7 @@ func init() { func checkExpectedHeaders(t *testing.T, expectedHeaders http.Header, receivedHeaders http.Header) { checkMap := make([]string, 0, len(expectedHeaders)) - for k, _ := range expectedHeaders { + for k := range expectedHeaders { checkMap = append(checkMap, k) } @@ -46,7 +47,7 @@ func checkExpectedHeaders(t *testing.T, expectedHeaders http.Header, receivedHea } } - for i, _ := range checkMap { + for i := range checkMap { if checkMap[i] == k { checkMap = append(checkMap[:i], checkMap[i+1:]...) break @@ -72,15 +73,15 @@ func TestCallConfigurationRequest(t *testing.T) { cfg := models.Config{"APP_VAR": "FOO"} rCfg := models.Config{"ROUTE_VAR": "BAR"} + app := &models.App{Name: appName, Config: cfg} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: appName, Config: cfg}, - }, + []*models.App{app}, []*models.Route{ { + AppID: app.ID, Config: rCfg, Path: path, - AppName: appName, Image: image, Type: typ, Format: format, @@ -112,7 +113,7 @@ func TestCallConfigurationRequest(t *testing.T) { call, err := a.GetCall( WithWriter(w), // XXX (reed): order matters [for now] - FromRequest(appName, path, req), + FromRequest(app, path, req), ) if err != nil { t.Fatal(err) @@ -124,8 +125,8 @@ func TestCallConfigurationRequest(t *testing.T) { if model.ID == "" { t.Fatal("model does not have id, GetCall should assign id") } - if model.AppName != appName { - t.Fatal("app name mismatch", model.AppName, appName) + 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) @@ -191,7 +192,8 @@ func TestCallConfigurationRequest(t *testing.T) { } func TestCallConfigurationModel(t *testing.T) { - appName := "myapp" + app := &models.App{Name: "myapp"} + app.SetDefaults() path := "/" image := "fnproject/fn-test-utils" const timeout = 1 @@ -199,13 +201,13 @@ func TestCallConfigurationModel(t *testing.T) { 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/r/" + app.Name + path payload := "payload" typ := "sync" format := "default" cfg := models.Config{ "FN_FORMAT": format, - "FN_APP_NAME": appName, + "FN_APP_NAME": app.Name, "FN_PATH": path, "FN_MEMORY": strconv.Itoa(memory), "FN_CPUS": CPUs.String(), @@ -215,8 +217,8 @@ func TestCallConfigurationModel(t *testing.T) { } cm := &models.Call{ + AppID: app.ID, Config: cfg, - AppName: appName, Path: path, Image: image, Type: typ, @@ -252,7 +254,8 @@ func TestCallConfigurationModel(t *testing.T) { } func TestAsyncCallHeaders(t *testing.T) { - appName := "myapp" + app := &models.App{Name: "myapp"} + app.SetDefaults() path := "/" image := "fnproject/fn-test-utils" const timeout = 1 @@ -260,7 +263,7 @@ func TestAsyncCallHeaders(t *testing.T) { 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/r/" + app.Name + path payload := "payload" typ := "async" format := "http" @@ -268,7 +271,7 @@ func TestAsyncCallHeaders(t *testing.T) { contentLength := strconv.FormatInt(int64(len(payload)), 10) config := map[string]string{ "FN_FORMAT": format, - "FN_APP_NAME": appName, + "FN_APP_NAME": app.Name, "FN_PATH": path, "FN_MEMORY": strconv.Itoa(memory), "FN_CPUS": CPUs.String(), @@ -279,14 +282,14 @@ func TestAsyncCallHeaders(t *testing.T) { } headers := map[string][]string{ // FromRequest would insert these from original HTTP request - "Content-Type": []string{contentType}, - "Content-Length": []string{contentLength}, + "Content-Type": {contentType}, + "Content-Length": {contentLength}, } cm := &models.Call{ + AppID: app.ID, Config: config, Headers: headers, - AppName: appName, Path: path, Image: image, Type: typ, @@ -389,7 +392,8 @@ func TestLoggerTooBig(t *testing.T) { } func TestSubmitError(t *testing.T) { - appName := "myapp" + app := &models.App{Name: "myapp"} + app.SetDefaults() path := "/" image := "fnproject/fn-test-utils" const timeout = 10 @@ -397,13 +401,13 @@ func TestSubmitError(t *testing.T) { 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/r/" + app.Name + path payload := `{"sleepTime": 0, "isDebug": true, "isCrash": true}` typ := "sync" format := "default" config := map[string]string{ "FN_FORMAT": format, - "FN_APP_NAME": appName, + "FN_APP_NAME": app.Name, "FN_PATH": path, "FN_MEMORY": strconv.Itoa(memory), "FN_CPUS": CPUs.String(), @@ -414,8 +418,8 @@ func TestSubmitError(t *testing.T) { } cm := &models.Call{ + AppID: app.ID, Config: config, - AppName: appName, Path: path, Image: image, Type: typ, @@ -469,15 +473,15 @@ func TestHTTPWithoutContentLengthWorks(t *testing.T) { path := "/hello" url := "http://127.0.0.1:8080/r/" + appName + path + app := &models.App{Name: appName} + app.SetDefaults() // we need to load in app & route so that FromRequest works ds := datastore.NewMockInit( - []*models.App{ - {Name: appName}, - }, + []*models.App{app}, []*models.Route{ { Path: path, - AppName: appName, + AppID: app.ID, Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", // this _is_ the test @@ -503,7 +507,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(appName, path, req), WithWriter(&out)) + callI, err := a.GetCall(FromRequest(app, path, req), WithWriter(&out)) if err != nil { t.Fatal(err) } @@ -538,7 +542,7 @@ func TestHTTPWithoutContentLengthWorks(t *testing.T) { func TestGetCallReturnsResourceImpossibility(t *testing.T) { call := &models.Call{ - AppName: "yo", + AppID: id.New().String(), Path: "/yoyo", Image: "fnproject/fn-test-utils", Type: "sync", @@ -565,6 +569,8 @@ func testCall() *models.Call { appName := "myapp" path := "/" image := "fnproject/fn-test-utils:latest" + app := &models.App{Name: appName} + app.SetDefaults() const timeout = 10 const idleTimeout = 20 const memory = 256 @@ -594,9 +600,9 @@ func testCall() *models.Call { } return &models.Call{ + AppID: app.ID, Config: config, Headers: headers, - AppName: appName, Path: path, Image: image, Type: typ, @@ -637,16 +643,14 @@ 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} // we need to load in app & route so that FromRequest works ds := datastore.NewMockInit( - []*models.App{ - {Name: ca.AppName}, - }, + []*models.App{app}, []*models.Route{ { Path: ca.Path, - AppName: ca.AppName, + AppID: ca.AppID, Image: ca.Image, Type: ca.Type, Format: ca.Format, @@ -676,7 +680,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(ca.AppName, ca.Path, req), WithWriter(&outOne)) + callI, err := a.GetCall(FromRequest(app, ca.Path, req), WithWriter(&outOne)) if err != nil { t.Fatal(err) } @@ -710,7 +714,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(ca.AppName, ca.Path, req), WithWriter(&outTwo)) + callI, err = a.GetCall(FromRequest(app, ca.Path, req), WithWriter(&outTwo)) if err != nil { t.Fatal(err) } @@ -787,16 +791,16 @@ func TestPipesDontMakeSpuriousCalls(t *testing.T) { call.Format = "http" call.IdleTimeout = 60 // keep this bad boy alive call.Timeout = 4 // short - + app := &models.App{Name: "myapp"} + app.SetDefaults() + app.ID = call.AppID // we need to load in app & route so that FromRequest works ds := datastore.NewMockInit( - []*models.App{ - {Name: call.AppName}, - }, + []*models.App{app}, []*models.Route{ { Path: call.Path, - AppName: call.AppName, + AppID: call.AppID, Image: call.Image, Type: call.Type, Format: call.Format, @@ -817,7 +821,7 @@ func TestPipesDontMakeSpuriousCalls(t *testing.T) { } var outOne bytes.Buffer - callI, err := a.GetCall(FromRequest(call.AppName, call.Path, req), WithWriter(&outOne)) + callI, err := a.GetCall(FromRequest(app, call.Path, req), WithWriter(&outOne)) if err != nil { t.Fatal(err) } @@ -842,7 +846,7 @@ func TestPipesDontMakeSpuriousCalls(t *testing.T) { } var outTwo bytes.Buffer - callI, err = a.GetCall(FromRequest(call.AppName, call.Path, req), WithWriter(&outTwo)) + callI, err = a.GetCall(FromRequest(app, call.Path, req), WithWriter(&outTwo)) if err != nil { t.Fatal(err) } diff --git a/api/agent/async.go b/api/agent/async.go index 5f60661a0..43c6771b6 100644 --- a/api/agent/async.go +++ b/api/agent/async.go @@ -88,7 +88,7 @@ func (a *agent) asyncRun(ctx context.Context, model *models.Call) { logrus.Fatal(err) } ctx, err = tag.New(ctx, - tag.Insert(appKey, model.AppName), + tag.Insert(appKey, model.AppID), tag.Insert(pathKey, model.Path), ) if err != nil { diff --git a/api/agent/call.go b/api/agent/call.go index 0d8cffefc..1fc64af80 100644 --- a/api/agent/call.go +++ b/api/agent/call.go @@ -49,14 +49,9 @@ type Param struct { } type Params []Param -func FromRequest(appName, path string, req *http.Request) CallOpt { +func FromRequest(app *models.App, path string, req *http.Request) CallOpt { return func(a *agent, c *call) error { - app, err := a.da.GetApp(req.Context(), appName) - if err != nil { - return err - } - - route, err := a.da.GetRoute(req.Context(), appName, path) + route, err := a.da.GetRoute(req.Context(), app.ID, path) if err != nil { return err } @@ -87,10 +82,9 @@ func FromRequest(appName, path string, req *http.Request) CallOpt { } c.Call = &models.Call{ - ID: id, - AppName: appName, - Path: route.Path, - Image: route.Image, + ID: id, + Path: route.Path, + Image: route.Image, // Delay: 0, Type: route.Type, Format: route.Format, @@ -106,6 +100,7 @@ func FromRequest(appName, path string, req *http.Request) CallOpt { CreatedAt: strfmt.DateTime(time.Now()), URL: reqURL(req), Method: req.Method, + AppID: app.ID, } c.req = req @@ -247,11 +242,11 @@ func (a *agent) GetCall(opts ...CallOpt) (Call, error) { c.ct = a ctx, _ := common.LoggerWithFields(c.req.Context(), - logrus.Fields{"id": c.ID, "app": c.AppName, "route": c.Path}) + logrus.Fields{"id": c.ID, "app_id": c.AppID, "route": c.Path}) c.req = c.req.WithContext(ctx) // setup stderr logger separate (don't inherit ctx vars) - logger := logrus.WithFields(logrus.Fields{"user_log": true, "app_name": c.AppName, "path": c.Path, "image": c.Image, "call_id": c.ID}) + logger := logrus.WithFields(logrus.Fields{"user_log": true, "app_id": c.AppID, "path": c.Path, "image": c.Image, "call_id": c.ID}) c.stderr = setupLogger(logger, a.cfg.MaxLogSize) if c.w == nil { // send STDOUT to logs if no writer given (async...) diff --git a/api/agent/data_access.go b/api/agent/data_access.go index 564f1ef4f..ac8e62507 100644 --- a/api/agent/data_access.go +++ b/api/agent/data_access.go @@ -16,11 +16,13 @@ import ( // but actually operate on the data in different ways (by direct access or by // mediation through an API node). type DataAccess interface { - // GetApp abstracts querying the datastore for an app. - GetApp(ctx context.Context, appName string) (*models.App, error) + GetAppID(ctx context.Context, appName string) (string, error) + + // GetAppByID abstracts querying the datastore for an app. + GetAppByID(ctx context.Context, appID string) (*models.App, error) // GetRoute abstracts querying the datastore for a route within an app. - GetRoute(ctx context.Context, appName string, routePath string) (*models.Route, error) + GetRoute(ctx context.Context, appID string, routePath string) (*models.Route, error) // Enqueue will add a Call to the queue (ultimately forwards to mq.Push). Enqueue(ctx context.Context, mCall *models.Call) error @@ -54,16 +56,20 @@ func NewCachedDataAccess(da DataAccess) DataAccess { return cda } -func routeCacheKey(appname, path string) string { - return "r:" + appname + "\x00" + path +func routeCacheKey(app, path string) string { + return "r:" + app + "\x00" + path } -func appCacheKey(appname string) string { - return "a:" + appname +func appIDCacheKey(appID string) string { + return "a:" + appID } -func (da *CachedDataAccess) GetApp(ctx context.Context, appName string) (*models.App, error) { - key := appCacheKey(appName) +func (da *CachedDataAccess) GetAppID(ctx context.Context, appName string) (string, error) { + return da.DataAccess.GetAppID(ctx, appName) +} + +func (da *CachedDataAccess) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + key := appIDCacheKey(appID) app, ok := da.cache.Get(key) if ok { return app.(*models.App), nil @@ -71,7 +77,7 @@ func (da *CachedDataAccess) GetApp(ctx context.Context, appName string) (*models resp, err := da.singleflight.Do(key, func() (interface{}, error) { - return da.DataAccess.GetApp(ctx, appName) + return da.DataAccess.GetAppByID(ctx, appID) }) if err != nil { @@ -82,8 +88,8 @@ func (da *CachedDataAccess) GetApp(ctx context.Context, appName string) (*models return app.(*models.App), nil } -func (da *CachedDataAccess) GetRoute(ctx context.Context, appName string, routePath string) (*models.Route, error) { - key := routeCacheKey(appName, routePath) +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 @@ -91,7 +97,7 @@ func (da *CachedDataAccess) GetRoute(ctx context.Context, appName string, routeP resp, err := da.singleflight.Do(key, func() (interface{}, error) { - return da.DataAccess.GetRoute(ctx, appName, routePath) + return da.DataAccess.GetRoute(ctx, appID, routePath) }) if err != nil { @@ -117,12 +123,16 @@ func NewDirectDataAccess(ds models.Datastore, ls models.LogStore, mq models.Mess return da } -func (da *directDataAccess) GetApp(ctx context.Context, appName string) (*models.App, error) { - return da.ds.GetApp(ctx, appName) +func (da *directDataAccess) GetAppID(ctx context.Context, appName string) (string, error) { + return da.ds.GetAppID(ctx, appName) } -func (da *directDataAccess) GetRoute(ctx context.Context, appName string, routePath string) (*models.Route, error) { - return da.ds.GetRoute(ctx, appName, routePath) +func (da *directDataAccess) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + return da.ds.GetAppByID(ctx, appID) +} + +func (da *directDataAccess) GetRoute(ctx context.Context, appID string, routePath string) (*models.Route, error) { + return da.ds.GetRoute(ctx, appID, routePath) } func (da *directDataAccess) Enqueue(ctx context.Context, mCall *models.Call) error { @@ -155,7 +165,7 @@ func (da *directDataAccess) Finish(ctx context.Context, mCall *models.Call, stde // note: Not returning err here since the job could have already finished successfully. } - if err := da.ls.InsertLog(ctx, mCall.AppName, mCall.ID, stderr); err != nil { + if err := da.ls.InsertLog(ctx, mCall.AppID, mCall.ID, stderr); err != nil { common.Logger(ctx).WithError(err).Error("error uploading log") // note: Not returning err here since the job could have already finished successfully. } diff --git a/api/agent/hybrid/client.go b/api/agent/hybrid/client.go index 162d018ce..46eda8a28 100644 --- a/api/agent/hybrid/client.go +++ b/api/agent/hybrid/client.go @@ -118,18 +118,29 @@ func (cl *client) Finish(ctx context.Context, c *models.Call, r io.Reader, async return err } -func (cl *client) GetApp(ctx context.Context, appName string) (*models.App, error) { - ctx, span := trace.StartSpan(ctx, "hybrid_client_get_app") +func (cl *client) GetAppID(ctx context.Context, appName string) (string, error) { + ctx, span := trace.StartSpan(ctx, "hybrid_client_get_app_id") defer span.End() var a struct { A models.App `json:"app"` } err := cl.do(ctx, nil, &a, "GET", "apps", appName) + return a.A.ID, err +} + +func (cl *client) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + ctx, span := trace.StartSpan(ctx, "hybrid_client_get_app_id") + defer span.End() + + var a struct { + A models.App `json:"app"` + } + err := cl.do(ctx, nil, &a, "GET", "runner", "apps", appID) return &a.A, err } -func (cl *client) GetRoute(ctx context.Context, appName, route string) (*models.Route, error) { +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() @@ -137,7 +148,7 @@ func (cl *client) GetRoute(ctx context.Context, appName, route string) (*models. var r struct { R models.Route `json:"route"` } - err := cl.do(ctx, nil, &r, "GET", "apps", appName, "routes", strings.TrimPrefix(route, "/")) + err := cl.do(ctx, nil, &r, "GET", "runner", "apps", appID, "routes", strings.TrimPrefix(route, "/")) return &r.R, err } @@ -162,7 +173,7 @@ func (cl *client) do(ctx context.Context, request, result interface{}, method st err = cl.once(ctx, request, result, method, url...) switch err := err.(type) { case nil: - return err + return nil case *httpErr: if err.code < 500 { return err diff --git a/api/agent/hybrid/nop.go b/api/agent/hybrid/nop.go index 9f587bc6f..d5992b2fc 100644 --- a/api/agent/hybrid/nop.go +++ b/api/agent/hybrid/nop.go @@ -17,6 +17,18 @@ func NewNopDataStore() (agent.DataAccess, error) { return &nopDataStore{}, nil } +func (cl *nopDataStore) GetAppID(ctx context.Context, appName string) (string, error) { + ctx, span := trace.StartSpan(ctx, "nop_datastore_get_app_id") + defer span.End() + return "", errors.New("should not call GetAppID on a NOP data store") +} + +func (cl *nopDataStore) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + ctx, span := trace.StartSpan(ctx, "nop_datastore_get_app_by_id") + defer span.End() + return nil, errors.New("should not call GetAppByID on a NOP data store") +} + func (cl *nopDataStore) Enqueue(ctx context.Context, c *models.Call) error { ctx, span := trace.StartSpan(ctx, "nop_datastore_enqueue") defer span.End() @@ -41,12 +53,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) GetApp(ctx context.Context, appName string) (*models.App, error) { - ctx, span := trace.StartSpan(ctx, "nop_datastore_get_app") - defer span.End() - return nil, errors.New("Should not call GetApp on a NOP data store") -} - 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() diff --git a/api/agent/lb_agent.go b/api/agent/lb_agent.go index 77cecc55d..689ba46b4 100644 --- a/api/agent/lb_agent.go +++ b/api/agent/lb_agent.go @@ -112,6 +112,16 @@ func NewLBAgent(da DataAccess, rp pool.RunnerPool, p pool.Placer) (Agent, error) return a, nil } +// GetAppID is to get the match of an app name to its ID +func (a *lbAgent) GetAppID(ctx context.Context, appName string) (string, error) { + return a.delegatedAgent.GetAppID(ctx, appName) +} + +// GetAppByID is to get the app by ID +func (a *lbAgent) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + return a.delegatedAgent.GetAppByID(ctx, appID) +} + // GetCall delegates to the wrapped agent but disables the capacity check as // this agent isn't actually running the call. func (a *lbAgent) GetCall(opts ...CallOpt) (Call, error) { diff --git a/api/agent/pure_runner.go b/api/agent/pure_runner.go index 6c1c35259..73168a912 100644 --- a/api/agent/pure_runner.go +++ b/api/agent/pure_runner.go @@ -250,6 +250,14 @@ type pureRunner struct { capacity CapacityGate } +func (pr *pureRunner) GetAppID(ctx context.Context, appName string) (string, error) { + return pr.a.GetAppID(ctx, appName) +} + +func (pr *pureRunner) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + return pr.a.GetAppByID(ctx, appID) +} + func (pr *pureRunner) GetCall(opts ...CallOpt) (Call, error) { return pr.a.GetCall(opts...) } diff --git a/api/agent/slots.go b/api/agent/slots.go index f3aec0d96..cbe10943b 100644 --- a/api/agent/slots.go +++ b/api/agent/slots.go @@ -284,7 +284,7 @@ func getSlotQueueKey(call *call) string { hash.Reset() defer shapool.Put(hash) - hash.Write(unsafeBytes(call.AppName)) + hash.Write(unsafeBytes(call.AppID)) hash.Write(unsafeBytes("\x00")) hash.Write(unsafeBytes(call.Path)) hash.Write(unsafeBytes("\x00")) diff --git a/api/agent/slots_test.go b/api/agent/slots_test.go index 8ac2a7cda..69d5dd137 100644 --- a/api/agent/slots_test.go +++ b/api/agent/slots_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/models" ) @@ -266,6 +267,7 @@ func TestSlotQueueBasic3(t *testing.T) { func BenchmarkSlotKey(b *testing.B) { appName := "myapp" + appID := id.New().String() path := "/" image := "fnproject/fn-test-utils" const timeout = 1 @@ -290,7 +292,7 @@ func BenchmarkSlotKey(b *testing.B) { cm := &models.Call{ Config: cfg, - AppName: appName, + AppID: appID, Path: path, Image: image, Type: typ, diff --git a/api/const.go b/api/const.go index 31cfa0542..b702a7a47 100644 --- a/api/const.go +++ b/api/const.go @@ -2,9 +2,10 @@ package api // Request context key names const ( - AppName string = "app_name" - Path string = "path" - Call string = "call" + App string = "app_name" + AppID string = "app_id" + Path string = "path" + Call string = "call" // Short forms for API URLs CApp string = "app" CRoute string = "route" diff --git a/api/datastore/internal/datastoretest/test.go b/api/datastore/internal/datastoretest/test.go index 621dcf1a9..c6bf172a7 100644 --- a/api/datastore/internal/datastoretest/test.go +++ b/api/datastore/internal/datastoretest/test.go @@ -32,6 +32,9 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { } }() + testApp.SetDefaults() + testRoute.AppID = testApp.ID + ctx := context.Background() call := new(models.Call) @@ -40,7 +43,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { call.Error = "ya dun goofed" call.StartedAt = strfmt.DateTime(time.Now()) call.CompletedAt = strfmt.DateTime(time.Now()) - call.AppName = testApp.Name + call.AppID = testApp.ID call.Path = testRoute.Path t.Run("call-insert", func(t *testing.T) { @@ -67,7 +70,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { if err != nil { t.Fatalf("Test UpdateCall: unexpected error `%v`", err) } - dbCall, err := ds.GetCall(ctx, call.AppName, call.ID) + dbCall, err := ds.GetCall(ctx, call.AppID, call.ID) if err != nil { t.Fatalf("Test UpdateCall: unexpected error `%v`", err) } @@ -89,8 +92,8 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { if time.Time(dbCall.CompletedAt).Unix() != time.Time(newCall.CompletedAt).Unix() { t.Fatalf("Test GetCall: completed_at mismatch `%v` `%v`", call.CompletedAt, newCall.CompletedAt) } - if dbCall.AppName != newCall.AppName { - t.Fatalf("Test GetCall: app_name mismatch `%v` `%v`", call.AppName, newCall.AppName) + if dbCall.AppID != newCall.AppID { + t.Fatalf("Test GetCall: app_name mismatch `%v` `%v`", call.AppID, newCall.AppID) } if dbCall.Path != newCall.Path { t.Fatalf("Test GetCall: path mismatch `%v` `%v`", call.Path, newCall.Path) @@ -139,7 +142,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { if err != nil { t.Fatalf("Test GetCall: unexpected error `%v`", err) } - newCall, err := ds.GetCall(ctx, call.AppName, call.ID) + newCall, err := ds.GetCall(ctx, call.AppID, call.ID) if err != nil { t.Fatalf("Test GetCall: unexpected error `%v`", err) } @@ -161,8 +164,8 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { if time.Time(call.CompletedAt).Unix() != time.Time(newCall.CompletedAt).Unix() { t.Fatalf("Test GetCall: completed_at mismatch `%v` `%v`", call.CompletedAt, newCall.CompletedAt) } - if call.AppName != newCall.AppName { - t.Fatalf("Test GetCall: app_name mismatch `%v` `%v`", call.AppName, newCall.AppName) + if call.AppID != newCall.AppID { + t.Fatalf("Test GetCall: app_name mismatch `%v` `%v`", call.AppID, newCall.AppID) } if call.Path != newCall.Path { t.Fatalf("Test GetCall: path mismatch `%v` `%v`", call.Path, newCall.Path) @@ -171,7 +174,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Run("calls-get", func(t *testing.T) { ds := dsf(t) - filter := &models.CallFilter{AppName: call.AppName, Path: call.Path, PerPage: 100} + filter := &models.CallFilter{AppID: call.AppID, Path: call.Path, PerPage: 100} call.ID = id.New().String() call.CreatedAt = strfmt.DateTime(time.Now()) err := ds.InsertCall(ctx, call) @@ -238,7 +241,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { } // test that filters actually applied - calls, err = ds.GetCalls(ctx, &models.CallFilter{AppName: "wrongappname", PerPage: 100}) + calls, err = ds.GetCalls(ctx, &models.CallFilter{AppID: "wrongappname", PerPage: 100}) if err != nil { t.Fatalf("Test GetCalls(ctx, filter): unexpected error `%v`", err) } @@ -291,53 +294,56 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { if !inserted.Equals(testApp) { t.Fatalf("Test InsertApp: expected to insert:\n%v\nbut got:\n%v", testApp, inserted) } - - _, err = ds.InsertApp(ctx, testApp) - if err != models.ErrAppsAlreadyExists { - t.Fatalf("Test InsertApp duplicated: expected error `%v`, but it was `%v`", models.ErrAppsAlreadyExists, err) - } + testApp.ID = inserted.ID { // Set a config var - updated, err := ds.UpdateApp(ctx, &models.App{Name: testApp.Name, Config: map[string]string{"TEST": "1"}}) + testApp, err := ds.GetAppByID(ctx, testApp.ID) + if err != nil { + t.Fatal(err.Error()) + } + testApp.Config = map[string]string{"TEST": "1"} + updated, err := ds.UpdateApp(ctx, testApp) if err != nil { t.Fatalf("Test UpdateApp: error when updating app: %v", err) } - expected := &models.App{Name: testApp.Name, Config: map[string]string{"TEST": "1"}} + expected := &models.App{ID: testApp.ID, Name: testApp.Name, Config: map[string]string{"TEST": "1"}} if !updated.Equals(expected) { t.Fatalf("Test UpdateApp: expected updated `%v` but got `%v`", expected, updated) } // Set a different var (without clearing the existing) - updated, err = ds.UpdateApp(ctx, - &models.App{Name: testApp.Name, Config: map[string]string{"OTHER": "TEST"}}) + another := testApp.Clone() + another.Config = map[string]string{"OTHER": "TEST"} + updated, err = ds.UpdateApp(ctx, another) if err != nil { t.Fatalf("Test UpdateApp: error when updating app: %v", err) } - expected = &models.App{Name: testApp.Name, Config: map[string]string{"TEST": "1", "OTHER": "TEST"}} + expected = &models.App{Name: testApp.Name, ID: testApp.ID, Config: map[string]string{"TEST": "1", "OTHER": "TEST"}} if !updated.Equals(expected) { t.Fatalf("Test UpdateApp: expected updated `%v` but got `%v`", expected, updated) } // Delete a var - updated, err = ds.UpdateApp(ctx, - &models.App{Name: testApp.Name, Config: map[string]string{"TEST": ""}}) + dVar := testApp.Clone() + dVar.Config = map[string]string{"TEST": ""} + updated, err = ds.UpdateApp(ctx, dVar) if err != nil { t.Fatalf("Test UpdateApp: error when updating app: %v", err) } - expected = &models.App{Name: testApp.Name, Config: map[string]string{"OTHER": "TEST"}} + expected = &models.App{Name: testApp.Name, ID: testApp.ID, Config: map[string]string{"OTHER": "TEST"}} if !updated.Equals(expected) { t.Fatalf("Test UpdateApp: expected updated `%v` but got `%v`", expected, updated) } } // Testing get app - _, err = ds.GetApp(ctx, "") - if err != models.ErrAppsMissingName { - t.Fatalf("Test GetApp: expected error to be %v, but it was %s", models.ErrAppsMissingName, err) + _, err = ds.GetAppByID(ctx, "") + if err != models.ErrDatastoreEmptyAppID { + t.Fatalf("Test GetApp: expected error to be %v, but it was %s", models.ErrDatastoreEmptyAppID, err) } - app, err := ds.GetApp(ctx, testApp.Name) + app, err := ds.GetAppByID(ctx, testApp.ID) if err != nil { t.Fatalf("Test GetApp: error: %s", err) } @@ -358,14 +364,14 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { } // test pagination stuff (ordering / limits / cursoring) - a2 := *testApp - a3 := *testApp - a2.Name = "Testa" - a3.Name = "Testb" - if _, err = ds.InsertApp(ctx, &a2); err != nil { + a2 := &models.App{Name: "Testa"} + a2.SetDefaults() + a3 := &models.App{Name: "Testb"} + a3.SetDefaults() + if _, err = ds.InsertApp(ctx, a2); err != nil { t.Fatal(err) } - if _, err = ds.InsertApp(ctx, &a3); err != nil { + if _, err = ds.InsertApp(ctx, a3); err != nil { t.Fatal(err) } @@ -391,10 +397,9 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatalf("Test GetApps: expected `app.Name` to be `%s` but it was `%s`", a3.Name, apps[1].Name) } - a4 := *testApp - a4.Name = "Abcdefg" // < /test lexicographically, but not in length - - if _, err = ds.InsertApp(ctx, &a4); err != nil { + a4 := &models.App{Name: "Abcdefg"} + a4.SetDefaults() + if _, err = ds.InsertApp(ctx, a4); err != nil { t.Fatal(err) } @@ -419,29 +424,33 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // Testing app delete err = ds.RemoveApp(ctx, "") - if err != models.ErrAppsMissingName { - t.Fatalf("Test RemoveApp: expected error `%v`, but it was `%v`", models.ErrAppsMissingName, err) + if err != models.ErrDatastoreEmptyAppID { + t.Fatalf("Test RemoveApp: expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyAppID, err) } - err = ds.RemoveApp(ctx, testApp.Name) + testApp, _ := ds.GetAppByID(ctx, testApp.ID) + err = ds.RemoveApp(ctx, testApp.ID) if err != nil { t.Fatalf("Test RemoveApp: error: %s", err) } - app, err = ds.GetApp(ctx, testApp.Name) + app, err = ds.GetAppByID(ctx, testApp.ID) if err != models.ErrAppsNotFound { t.Fatalf("Test GetApp(removed): expected error `%v`, but it was `%v`", models.ErrAppsNotFound, err) } if app != nil { - t.Fatal("Test RemoveApp: failed to remove the app") + t.Log(err.Error()) + t.Fatal("Test RemoveApp: failed to remove the app, app should be gone already") } // Test update inexistent app - _, err = ds.UpdateApp(ctx, &models.App{ + missingApp := &models.App{ Name: testApp.Name, Config: map[string]string{ "TEST": "1", }, - }) + } + missingApp.SetDefaults() + _, err = ds.UpdateApp(ctx, missingApp) if err != models.ErrAppsNotFound { t.Fatalf("Test UpdateApp(inexistent): expected error `%v`, but it was `%v`", models.ErrAppsNotFound, err) } @@ -450,7 +459,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Run("routes", func(t *testing.T) { ds := dsf(t) // Insert app again to test routes - _, err := ds.InsertApp(ctx, testApp) + testApp, err := ds.InsertApp(ctx, testApp) if err != nil && err != models.ErrAppsAlreadyExists { t.Fatal("Test InsertRoute Prep: failed to insert app: ", err) } @@ -462,14 +471,15 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatalf("Test InsertRoute(nil): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyRoute, err) } - copyRoute := *testRoute - copyRoute.AppName = "notreal" - _, err = ds.InsertRoute(ctx, ©Route) + newTestRoute := testRoute.Clone() + newTestRoute.AppID = "notreal" + _, err = ds.InsertRoute(ctx, newTestRoute) if err != models.ErrAppsNotFound { t.Fatalf("Test InsertRoute: expected error `%v`, but it was `%v`", models.ErrAppsNotFound, err) } - _, err = ds.InsertRoute(ctx, testRoute) + testRoute.AppID = testApp.ID + testRoute, err = ds.InsertRoute(ctx, testRoute) if err != nil { t.Fatalf("Test InsertRoute: error when storing new route: %s", err) } @@ -482,17 +492,17 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // Testing get { - _, err = ds.GetRoute(ctx, "a", "") + _, err = ds.GetRoute(ctx, id.New().String(), "") if err != models.ErrRoutesMissingPath { t.Fatalf("Test GetRoute(empty route path): expected error `%v`, but it was `%v`", models.ErrRoutesMissingPath, err) } _, err = ds.GetRoute(ctx, "", "a") - if err != models.ErrAppsMissingName { - t.Fatalf("Test GetRoute(empty app name): expected error `%v`, but it was `%v`", models.ErrAppsMissingName, err) + if err != models.ErrDatastoreEmptyAppID { + t.Fatalf("Test GetRoute(empty app name): expected error `%v`, but it was `%v`", models.ErrRoutesMissingPath, err) } - route, err := ds.GetRoute(ctx, testApp.Name, testRoute.Path) + route, err := ds.GetRoute(ctx, testApp.ID, testRoute.Path) if err != nil { t.Fatalf("Test GetRoute: unexpected error %v", err) } @@ -505,7 +515,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { { // Update some fields, and add 3 configs and 3 headers. updated, err := ds.UpdateRoute(ctx, &models.Route{ - AppName: testRoute.AppName, + AppID: testApp.ID, Path: testRoute.Path, Timeout: 2, Config: map[string]string{ @@ -524,7 +534,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { } expected := &models.Route{ // unchanged - AppName: testRoute.AppName, + AppID: testApp.ID, Path: testRoute.Path, Image: "fnproject/fn-test-utils", Type: "sync", @@ -551,8 +561,8 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // Update a config var, remove another. Add one Header, remove another. updated, err = ds.UpdateRoute(ctx, &models.Route{ - AppName: testRoute.AppName, - Path: testRoute.Path, + AppID: testRoute.AppID, + Path: testRoute.Path, Config: map[string]string{ "FIRST": "first", "SECOND": "", @@ -568,7 +578,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { } expected = &models.Route{ // unchanged - AppName: testRoute.AppName, + AppID: testRoute.AppID, Path: testRoute.Path, Image: "fnproject/fn-test-utils", Type: "sync", @@ -593,7 +603,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { } // Testing list routes - routes, err := ds.GetRoutesByApp(ctx, testApp.Name, &models.RouteFilter{PerPage: 1}) + routes, err := ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 1}) if err != nil { t.Fatalf("Test GetRoutesByApp: unexpected error %v", err) } @@ -606,7 +616,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatalf("Test GetRoutes: expected `app.Name` to be `%s` but it was `%s`", testRoute.Path, routes[0].Path) } - routes, err = ds.GetRoutesByApp(ctx, testApp.Name, &models.RouteFilter{Image: testRoute.Image, PerPage: 1}) + routes, err = ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{Image: testRoute.Image, PerPage: 1}) if err != nil { t.Fatalf("Test GetRoutesByApp: unexpected error %v", err) } @@ -619,7 +629,9 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatalf("Test GetRoutesByApp: expected `route.Path` to be `%s` but it was `%s`", testRoute.Path, routes[0].Path) } - routes, err = ds.GetRoutesByApp(ctx, "notreal", &models.RouteFilter{PerPage: 1}) + nre := &models.App{Name: "notreal"} + nre.SetDefaults() + routes, err = ds.GetRoutesByApp(ctx, nre.ID, &models.RouteFilter{PerPage: 1}) if err != nil { t.Fatalf("Test GetRoutesByApp: error: %s", err) } @@ -629,7 +641,9 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // test pagination stuff r2 := *testRoute + r2.AppID = testApp.ID r3 := *testRoute + r2.AppID = testApp.ID r2.Path = "/testa" r3.Path = "/testb" @@ -640,7 +654,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatal(err) } - routes, err = ds.GetRoutesByApp(ctx, testApp.Name, &models.RouteFilter{PerPage: 1}) + routes, err = ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 1}) if err != nil { t.Fatalf("Test GetRoutesByApp: error: %s", err) } @@ -650,7 +664,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatalf("Test GetRoutesByApp: expected `route.Path` to be `%s` but it was `%s`", testRoute.Path, routes[0].Path) } - routes, err = ds.GetRoutesByApp(ctx, testApp.Name, &models.RouteFilter{PerPage: 2, Cursor: routes[0].Path}) + routes, err = ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 2, Cursor: routes[0].Path}) if err != nil { t.Fatalf("Test GetRoutesByApp: error: %s", err) } @@ -669,7 +683,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatal(err) } - routes, err = ds.GetRoutesByApp(ctx, testApp.Name, &models.RouteFilter{PerPage: 100}) + routes, err = ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 100}) if err != nil { t.Fatalf("Test GetRoutesByApp: error: %s", err) } @@ -684,21 +698,21 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // Testing route delete err = ds.RemoveRoute(ctx, "", "") - if err != models.ErrAppsMissingName { - t.Fatalf("Test RemoveRoute(empty app name): expected error `%v`, but it was `%v`", models.ErrAppsMissingName, err) + if err != models.ErrDatastoreEmptyAppID { + t.Fatalf("Test RemoveRoute(empty app name): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyAppID, err) } - err = ds.RemoveRoute(ctx, "a", "") + err = ds.RemoveRoute(ctx, testApp.ID, "") if err != models.ErrRoutesMissingPath { t.Fatalf("Test RemoveRoute(empty route path): expected error `%v`, but it was `%v`", models.ErrRoutesMissingPath, err) } - err = ds.RemoveRoute(ctx, testRoute.AppName, testRoute.Path) + err = ds.RemoveRoute(ctx, testApp.ID, testRoute.Path) if err != nil { t.Fatalf("Test RemoveApp: unexpected error: %v", err) } - route, err := ds.GetRoute(ctx, testRoute.AppName, testRoute.Path) + route, err := ds.GetRoute(ctx, testApp.ID, testRoute.Path) if err != nil && err != models.ErrRoutesNotFound { t.Fatalf("Test GetRoute: expected error `%v`, but it was `%v`", models.ErrRoutesNotFound, err) } @@ -707,9 +721,9 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { } _, err = ds.UpdateRoute(ctx, &models.Route{ - AppName: testRoute.AppName, - Path: testRoute.Path, - Image: "test", + AppID: testApp.ID, + Path: testRoute.Path, + Image: "test", }) if err != models.ErrRoutesNotFound { t.Fatalf("Test UpdateRoute inexistent: expected error to be `%v`, but it was `%v`", models.ErrRoutesNotFound, err) @@ -722,7 +736,6 @@ var testApp = &models.App{ } var testRoute = &models.Route{ - AppName: testApp.Name, Path: "/test", Image: "fnproject/fn-test-utils", Type: "sync", diff --git a/api/datastore/internal/datastoreutil/metrics.go b/api/datastore/internal/datastoreutil/metrics.go index b03505470..7a62dfcdb 100644 --- a/api/datastore/internal/datastoreutil/metrics.go +++ b/api/datastore/internal/datastoreutil/metrics.go @@ -18,10 +18,16 @@ type metricds struct { ds models.Datastore } -func (m *metricds) GetApp(ctx context.Context, appName string) (*models.App, error) { - ctx, span := trace.StartSpan(ctx, "ds_get_app") +func (m *metricds) GetAppID(ctx context.Context, appName string) (string, error) { + ctx, span := trace.StartSpan(ctx, "ds_get_app_id") defer span.End() - return m.ds.GetApp(ctx, appName) + return m.ds.GetAppID(ctx, appName) +} + +func (m *metricds) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + ctx, span := trace.StartSpan(ctx, "ds_get_app_by_id") + defer span.End() + return m.ds.GetAppByID(ctx, appID) } func (m *metricds) GetApps(ctx context.Context, filter *models.AppFilter) ([]*models.App, error) { @@ -42,22 +48,22 @@ func (m *metricds) UpdateApp(ctx context.Context, app *models.App) (*models.App, return m.ds.UpdateApp(ctx, app) } -func (m *metricds) RemoveApp(ctx context.Context, appName string) error { +func (m *metricds) RemoveApp(ctx context.Context, appID string) error { ctx, span := trace.StartSpan(ctx, "ds_remove_app") defer span.End() - return m.ds.RemoveApp(ctx, appName) + return m.ds.RemoveApp(ctx, appID) } -func (m *metricds) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { +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, appName, routePath) + return m.ds.GetRoute(ctx, appID, routePath) } -func (m *metricds) GetRoutesByApp(ctx context.Context, appName string, filter *models.RouteFilter) (routes []*models.Route, err error) { +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, appName, filter) + return m.ds.GetRoutesByApp(ctx, appID, filter) } func (m *metricds) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { @@ -72,10 +78,10 @@ func (m *metricds) UpdateRoute(ctx context.Context, route *models.Route) (*model return m.ds.UpdateRoute(ctx, route) } -func (m *metricds) RemoveRoute(ctx context.Context, appName, routePath string) error { +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, appName, routePath) + return m.ds.RemoveRoute(ctx, appID, routePath) } func (m *metricds) InsertCall(ctx context.Context, call *models.Call) error { diff --git a/api/datastore/internal/datastoreutil/validator.go b/api/datastore/internal/datastoreutil/validator.go index a7bfe8194..4648dce8a 100644 --- a/api/datastore/internal/datastoreutil/validator.go +++ b/api/datastore/internal/datastoreutil/validator.go @@ -17,12 +17,19 @@ type validator struct { models.Datastore } -// name will never be empty. -func (v *validator) GetApp(ctx context.Context, name string) (app *models.App, err error) { - if name == "" { - return nil, models.ErrAppsMissingName +func (v *validator) GetAppID(ctx context.Context, appName string) (string, error) { + if appName == "" { + return "", models.ErrAppsMissingName } - return v.Datastore.GetApp(ctx, name) + return v.Datastore.GetAppID(ctx, appName) +} + +func (v *validator) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + if appID == "" { + return nil, models.ErrDatastoreEmptyAppID + } + + return v.Datastore.GetAppByID(ctx, appID) } func (v *validator) GetApps(ctx context.Context, appFilter *models.AppFilter) ([]*models.App, error) { @@ -48,40 +55,41 @@ func (v *validator) UpdateApp(ctx context.Context, app *models.App) (*models.App if app == nil { return nil, models.ErrDatastoreEmptyApp } - if app.Name == "" { - return nil, models.ErrAppsMissingName + if app.ID == "" { + return nil, models.ErrDatastoreEmptyAppID } return v.Datastore.UpdateApp(ctx, app) } // name will never be empty. -func (v *validator) RemoveApp(ctx context.Context, name string) error { - if name == "" { - return models.ErrAppsMissingName +func (v *validator) RemoveApp(ctx context.Context, appID string) error { + if appID == "" { + return models.ErrDatastoreEmptyAppID } - return v.Datastore.RemoveApp(ctx, name) + return v.Datastore.RemoveApp(ctx, appID) } // appName and routePath will never be empty. -func (v *validator) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { - if appName == "" { - return nil, models.ErrAppsMissingName +func (v *validator) GetRoute(ctx context.Context, appID, routePath string) (*models.Route, error) { + if appID == "" { + return nil, models.ErrDatastoreEmptyAppID } if routePath == "" { return nil, models.ErrRoutesMissingPath } - return v.Datastore.GetRoute(ctx, appName, routePath) + return v.Datastore.GetRoute(ctx, appID, routePath) } // appName will never be empty -func (v *validator) GetRoutesByApp(ctx context.Context, appName string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { - if appName == "" { - return nil, models.ErrAppsMissingName +func (v *validator) GetRoutesByApp(ctx context.Context, appID string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { + if appID == "" { + return nil, models.ErrDatastoreEmptyAppID } - return v.Datastore.GetRoutesByApp(ctx, appName, routeFilter) + + return v.Datastore.GetRoutesByApp(ctx, appID, routeFilter) } // route will never be nil and route's AppName and Path will never be empty. @@ -103,8 +111,8 @@ func (v *validator) UpdateRoute(ctx context.Context, newroute *models.Route) (*m if newroute == nil { return nil, models.ErrDatastoreEmptyRoute } - if newroute.AppName == "" { - return nil, models.ErrAppsMissingName + if newroute.AppID == "" { + return nil, models.ErrRoutesMissingAppID } if newroute.Path == "" { return nil, models.ErrRoutesMissingPath @@ -113,15 +121,15 @@ func (v *validator) UpdateRoute(ctx context.Context, newroute *models.Route) (*m } // appName and routePath will never be empty. -func (v *validator) RemoveRoute(ctx context.Context, appName, routePath string) error { - if appName == "" { - return models.ErrAppsMissingName +func (v *validator) RemoveRoute(ctx context.Context, appID string, routePath string) error { + if appID == "" { + return models.ErrDatastoreEmptyAppID } if routePath == "" { return models.ErrRoutesMissingPath } - return v.Datastore.RemoveRoute(ctx, appName, routePath) + return v.Datastore.RemoveRoute(ctx, appID, routePath) } // callID will never be empty. diff --git a/api/datastore/mock.go b/api/datastore/mock.go index 8d7bfdf97..1699d2c8f 100644 --- a/api/datastore/mock.go +++ b/api/datastore/mock.go @@ -29,9 +29,19 @@ func NewMockInit(apps []*models.App, routes []*models.Route, calls []*models.Cal return datastoreutil.NewValidator(&mock{apps, routes, calls, make(map[string][]byte), logs.NewMock()}) } -func (m *mock) GetApp(ctx context.Context, appName string) (app *models.App, err error) { +func (m *mock) GetAppID(ctx context.Context, appName string) (string, error) { for _, a := range m.Apps { if a.Name == appName { + return a.ID, nil + } + } + + return "", models.ErrAppsNotFound +} + +func (m *mock) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + for _, a := range m.Apps { + if a.ID == appID { return a, nil } } @@ -63,7 +73,7 @@ func (m *mock) GetApps(ctx context.Context, appFilter *models.AppFilter) ([]*mod } func (m *mock) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { - if a, _ := m.GetApp(ctx, app.Name); a != nil { + if a, _ := m.GetAppByID(ctx, app.ID); a != nil { return nil, models.ErrAppsAlreadyExists } m.Apps = append(m.Apps, app) @@ -71,7 +81,7 @@ func (m *mock) InsertApp(ctx context.Context, app *models.App) (*models.App, err } func (m *mock) UpdateApp(ctx context.Context, app *models.App) (*models.App, error) { - a, err := m.GetApp(ctx, app.Name) + a, err := m.GetAppByID(ctx, app.ID) if err != nil { return nil, err } @@ -80,11 +90,11 @@ func (m *mock) UpdateApp(ctx context.Context, app *models.App) (*models.App, err return a.Clone(), nil } -func (m *mock) RemoveApp(ctx context.Context, appName string) error { - m.batchDeleteCalls(ctx, appName) - m.batchDeleteRoutes(ctx, appName) +func (m *mock) RemoveApp(ctx context.Context, appID string) error { + m.batchDeleteCalls(ctx, appID) + m.batchDeleteRoutes(ctx, appID) for i, a := range m.Apps { - if a.Name == appName { + if a.ID == appID { m.Apps = append(m.Apps[:i], m.Apps[i+1:]...) return nil } @@ -92,9 +102,9 @@ func (m *mock) RemoveApp(ctx context.Context, appName string) error { return models.ErrAppsNotFound } -func (m *mock) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { +func (m *mock) GetRoute(ctx context.Context, appID, routePath string) (*models.Route, error) { for _, r := range m.Routes { - if r.AppName == appName && r.Path == routePath { + if r.AppID == appID && r.Path == routePath { return r, nil } } @@ -107,7 +117,7 @@ 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, appName string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { +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)) @@ -116,7 +126,7 @@ func (m *mock) GetRoutesByApp(ctx context.Context, appName string, routeFilter * break } - if r.AppName == appName && + if r.AppID == appID && //strings.HasPrefix(r.Path, routeFilter.PathPrefix) && // TODO (routeFilter.Image == "" || routeFilter.Image == r.Image) && strings.Compare(routeFilter.Cursor, r.Path) < 0 { @@ -128,11 +138,11 @@ func (m *mock) GetRoutesByApp(ctx context.Context, appName string, routeFilter * } func (m *mock) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - if _, err := m.GetApp(ctx, route.AppName); err != nil { + if _, err := m.GetAppByID(ctx, route.AppID); err != nil { return nil, err } - if r, _ := m.GetRoute(ctx, route.AppName, route.Path); r != nil { + if r, _ := m.GetRoute(ctx, route.AppID, route.Path); r != nil { return nil, models.ErrRoutesAlreadyExists } m.Routes = append(m.Routes, route) @@ -140,7 +150,7 @@ func (m *mock) InsertRoute(ctx context.Context, route *models.Route) (*models.Ro } func (m *mock) UpdateRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - r, err := m.GetRoute(ctx, route.AppName, route.Path) + r, err := m.GetRoute(ctx, route.AppID, route.Path) if err != nil { return nil, err } @@ -154,9 +164,9 @@ func (m *mock) UpdateRoute(ctx context.Context, route *models.Route) (*models.Ro return clone, nil } -func (m *mock) RemoveRoute(ctx context.Context, appName, routePath string) error { +func (m *mock) RemoveRoute(ctx context.Context, appID, routePath string) error { for i, r := range m.Routes { - if r.AppName == appName && r.Path == routePath { + if r.AppID == appID && r.Path == routePath { m.Routes = append(m.Routes[:i], m.Routes[i+1:]...) return nil } @@ -190,7 +200,7 @@ func equivalentCalls(expected *models.Call, actual *models.Call) bool { time.Time(expected.StartedAt).Unix() == time.Time(actual.StartedAt).Unix() && time.Time(expected.CompletedAt).Unix() == time.Time(actual.CompletedAt).Unix() && expected.Status == actual.Status && - expected.AppName == actual.AppName && + expected.AppID == actual.AppID && expected.Path == actual.Path && expected.Error == actual.Error && len(expected.Stats) == len(actual.Stats) @@ -200,7 +210,7 @@ func equivalentCalls(expected *models.Call, actual *models.Call) bool { func (m *mock) UpdateCall(ctx context.Context, from *models.Call, to *models.Call) error { for _, t := range m.Calls { - if t.ID == from.ID && t.AppName == from.AppName { + if t.ID == from.ID && t.AppID == from.AppID { if equivalentCalls(from, t) { *t = *to return nil @@ -211,9 +221,9 @@ func (m *mock) UpdateCall(ctx context.Context, from *models.Call, to *models.Cal return models.ErrCallNotFound } -func (m *mock) GetCall(ctx context.Context, appName, callID string) (*models.Call, error) { +func (m *mock) GetCall(ctx context.Context, appID, callID string) (*models.Call, error) { for _, t := range m.Calls { - if t.ID == callID && t.AppName == appName { + if t.ID == callID && t.AppID == appID { return t, nil } } @@ -238,7 +248,7 @@ func (m *mock) GetCalls(ctx context.Context, filter *models.CallFilter) ([]*mode break } - if (filter.AppName == "" || c.AppName == filter.AppName) && + 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))) && @@ -251,10 +261,10 @@ func (m *mock) GetCalls(ctx context.Context, filter *models.CallFilter) ([]*mode return calls, nil } -func (m *mock) batchDeleteCalls(ctx context.Context, appName string) error { +func (m *mock) batchDeleteCalls(ctx context.Context, appID string) error { newCalls := []*models.Call{} for _, c := range m.Calls { - if c.AppName != appName { + if c.AppID != appID || c.ID != appID { newCalls = append(newCalls, c) } } @@ -262,10 +272,10 @@ func (m *mock) batchDeleteCalls(ctx context.Context, appName string) error { return nil } -func (m *mock) batchDeleteRoutes(ctx context.Context, appName string) error { +func (m *mock) batchDeleteRoutes(ctx context.Context, appID string) error { newRoutes := []*models.Route{} for _, c := range m.Routes { - if c.AppName != appName { + if c.AppID != appID { newRoutes = append(newRoutes, c) } } diff --git a/api/datastore/sql/migratex/migrate.go b/api/datastore/sql/migratex/migrate.go index b717d4f65..659ccdc55 100644 --- a/api/datastore/sql/migratex/migrate.go +++ b/api/datastore/sql/migratex/migrate.go @@ -111,7 +111,8 @@ func migrate(ctx context.Context, db *sqlx.DB, migs []Migration, up bool) error } for _, m := range migs { // skip over migrations we have run - if (up && curVersion < m.Version()) || (!up && curVersion >= m.Version()) { + mVersion := m.Version() + if (up && curVersion < mVersion) || (!up && curVersion >= mVersion) { // do each individually, for large migrations it's better to checkpoint // than to try to do them all in one big go. diff --git a/api/datastore/sql/migrations/10_add_app_id.go b/api/datastore/sql/migrations/10_add_app_id.go new file mode 100644 index 000000000..fe421390d --- /dev/null +++ b/api/datastore/sql/migrations/10_add_app_id.go @@ -0,0 +1,159 @@ +package migrations + +import ( + "context" + "database/sql" + "fmt" + "github.com/fnproject/fn/api/datastore/sql/migratex" + "github.com/fnproject/fn/api/id" + "github.com/fnproject/fn/api/models" + "github.com/jmoiron/sqlx" +) + +func up10(ctx context.Context, tx *sqlx.Tx) error { + addAppIDStatements := []string{ + "ALTER TABLE apps ADD id VARCHAR(256);", + "ALTER TABLE calls ADD app_id VARCHAR(256);", + "ALTER TABLE logs ADD app_id VARCHAR(256);", + "ALTER TABLE routes ADD app_id VARCHAR(256);", + } + for _, statement := range addAppIDStatements { + _, err := tx.ExecContext(ctx, statement) + if err != nil { + return err + } + } + + rows, err := tx.QueryxContext(ctx, "SELECT DISTINCT name FROM apps;") + if err != nil { + return err + } + + res := []*models.App{} + for rows.Next() { + + var app models.App + err := rows.StructScan(&app) + if err != nil { + if err == sql.ErrNoRows { + return nil + } + return err + } + app.ID = id.New().String() + res = append(res, &app) + } + err = rows.Close() + if err != nil { + return err + } + + if err := rows.Err(); err != nil { + return err + } + + // it is required for some reason, can't do this within the rows iteration. + for _, app := range res { + query := tx.Rebind(`UPDATE apps SET id=:id WHERE name=:name`) + _, err = tx.NamedExecContext(ctx, query, app) + if err != nil { + return err + } + + for _, t := range []string{"routes", "calls", "logs"} { + q := fmt.Sprintf(`UPDATE %s SET app_id=:id WHERE app_name=:name;`, t) + _, err = tx.NamedExecContext(ctx, tx.Rebind(q), app) + if err != nil { + return err + } + } + } + + dropAppNameStatements := []string{ + "ALTER TABLE routes DROP COLUMN app_name;", + "ALTER TABLE calls DROP COLUMN app_name;", + "ALTER TABLE logs DROP COLUMN app_name;", + } + for _, statement := range dropAppNameStatements { + _, err := tx.ExecContext(ctx, statement) + if err != nil { + return err + } + } + + return nil +} + +func down10(ctx context.Context, tx *sqlx.Tx) error { + + addAppNameStatements := []string{ + "ALTER TABLE calls ADD app_name VARCHAR(256);", + "ALTER TABLE logs ADD app_name VARCHAR(256);", + "ALTER TABLE routes ADD app_name VARCHAR(256);", + } + for _, statement := range addAppNameStatements { + _, err := tx.ExecContext(ctx, statement) + if err != nil { + return err + } + } + + rows, err := tx.QueryxContext(ctx, "SELECT DISTINCT id, name FROM apps;") + if err != nil { + return err + } + + res := []*models.App{} + for rows.Next() { + var app models.App + err := rows.StructScan(&app) + if err != nil { + if err == sql.ErrNoRows { + return nil + } + return err + } + res = append(res, &app) + } + err = rows.Close() + if err != nil { + return err + } + + if err := rows.Err(); err != nil { + return err + } + + // it is required for some reason, can't do this within the rows iteration. + for _, app := range res { + for _, t := range []string{"routes", "calls", "logs"} { + q := "UPDATE " + t + " SET app_name=:name WHERE app_id=:id;" + _, err = tx.NamedExecContext(ctx, tx.Rebind(q), app) + if err != nil { + return err + } + } + } + + removeAppIDStatements := []string{ + "ALTER TABLE logs DROP COLUMN app_id;", + "ALTER TABLE calls DROP COLUMN app_id;", + "ALTER TABLE routes DROP COLUMN app_id;", + "ALTER TABLE apps DROP COLUMN id;", + } + for _, statement := range removeAppIDStatements { + _, err := tx.ExecContext(ctx, statement) + if err != nil { + return err + } + } + return nil +} + +func init() { + Migrations = append(Migrations, &migratex.MigFields{ + VersionFunc: vfunc(10), + UpFunc: up10, + DownFunc: down10, + }) +} diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 0a34eb81f..9f2118ba4 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -39,7 +39,7 @@ import ( // with migrations (sadly, need complex transaction) var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( - app_name varchar(256) NOT NULL, + app_id varchar(256) NOT NULL, path varchar(256) NOT NULL, image varchar(256) NOT NULL, format varchar(16) NOT NULL, @@ -53,10 +53,11 @@ var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( annotations text NOT NULL, created_at text, updated_at varchar(256), - PRIMARY KEY (app_name, path) + PRIMARY KEY (app_id, path) );`, `CREATE TABLE IF NOT EXISTS apps ( + id varchar(256), name varchar(256) NOT NULL PRIMARY KEY, config text NOT NULL, annotations text NOT NULL, @@ -70,7 +71,7 @@ var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( completed_at varchar(256) NOT NULL, status varchar(256) NOT NULL, id varchar(256) NOT NULL, - app_name varchar(256) NOT NULL, + app_id varchar(256) NOT NULL, path varchar(256) NOT NULL, stats text, error text, @@ -79,14 +80,16 @@ var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( `CREATE TABLE IF NOT EXISTS logs ( id varchar(256) NOT NULL PRIMARY KEY, - app_name varchar(256) NOT NULL, + app_id varchar(256) NOT NULL, log text NOT NULL );`, } const ( - routeSelector = `SELECT app_name, path, image, format, memory, cpus, type, timeout, idle_timeout, headers, config, annotations, created_at, updated_at FROM routes` - callSelector = `SELECT id, created_at, started_at, completed_at, status, app_name, path, stats, error FROM calls` + routeSelector = `SELECT app_id, path, image, format, memory, type, cpus, timeout, idle_timeout, 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` + appIDSelector = `SELECT id, name, config, annotations, created_at, updated_at FROM apps WHERE id=?` + ensureAppSelector = `SELECT id FROM apps WHERE name=?` ) type sqlStore struct { @@ -252,8 +255,25 @@ func (ds *sqlStore) clear() error { }) } +func (ds *sqlStore) GetAppID(ctx context.Context, appName string) (string, error) { + var app models.App + query := ds.db.Rebind(ensureAppSelector) + row := ds.db.QueryRowxContext(ctx, query, appName) + + err := row.StructScan(&app) + if err == sql.ErrNoRows { + return "", models.ErrAppsNotFound + } + if err != nil { + return "", err + } + + return app.ID, nil +} + func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { query := ds.db.Rebind(`INSERT INTO apps ( + id, name, config, annotations, @@ -261,6 +281,7 @@ func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App updated_at ) VALUES ( + :id, :name, :config, :annotations, @@ -290,17 +311,19 @@ func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App } func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models.App, error) { - app := &models.App{Name: newapp.Name} + var app models.App err := ds.Tx(func(tx *sqlx.Tx) error { // NOTE: must query whole object since we're returning app, Update logic // must only modify modifiable fields (as seen here). need to fix brittle.. - query := tx.Rebind(`SELECT name, config, annotations, created_at, updated_at FROM apps WHERE name=?`) - row := tx.QueryRowxContext(ctx, query, app.Name) - err := row.StructScan(app) + query := tx.Rebind(appIDSelector) + row := tx.QueryRowxContext(ctx, query, newapp.ID) + + err := row.StructScan(&app) if err == sql.ErrNoRows { return models.ErrAppsNotFound - } else if err != nil { + } + if err != nil { return err } @@ -329,12 +352,12 @@ func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models. return nil, err } - return app, nil + return &app, nil } -func (ds *sqlStore) RemoveApp(ctx context.Context, appName string) error { +func (ds *sqlStore) RemoveApp(ctx context.Context, appID string) error { return ds.Tx(func(tx *sqlx.Tx) error { - res, err := tx.ExecContext(ctx, tx.Rebind(`DELETE FROM apps WHERE name=?`), appName) + res, err := tx.ExecContext(ctx, tx.Rebind(`DELETE FROM apps WHERE id=?`), appID) if err != nil { return err } @@ -347,33 +370,34 @@ func (ds *sqlStore) RemoveApp(ctx context.Context, appName string) error { } deletes := []string{ - `DELETE FROM logs WHERE app_name=?`, - `DELETE FROM calls WHERE app_name=?`, - `DELETE FROM routes WHERE app_name=?`, + `DELETE FROM logs WHERE app_id=?`, + `DELETE FROM calls WHERE app_id=?`, + `DELETE FROM routes WHERE app_id=?`, } - for _, stmt := range deletes { - _, err := tx.ExecContext(ctx, tx.Rebind(stmt), appName) + _, err := tx.ExecContext(ctx, tx.Rebind(stmt), appID) if err != nil { return err } } + return nil }) } -func (ds *sqlStore) GetApp(ctx context.Context, name string) (*models.App, error) { - query := ds.db.Rebind(`SELECT name, config, annotations, created_at, updated_at FROM apps WHERE name=?`) - row := ds.db.QueryRowxContext(ctx, query, name) +func (ds *sqlStore) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + var app models.App + query := ds.db.Rebind(appIDSelector) + row := ds.db.QueryRowxContext(ctx, query, appID) - var res models.App - err := row.StructScan(&res) + err := row.StructScan(&app) if err == sql.ErrNoRows { return nil, models.ErrAppsNotFound - } else if err != nil { + } + if err != nil { return nil, err } - return &res, nil + return &app, err } // GetApps retrieves an array of apps according to a specific filter. @@ -413,15 +437,15 @@ func (ds *sqlStore) GetApps(ctx context.Context, filter *models.AppFilter) ([]*m func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { err := ds.Tx(func(tx *sqlx.Tx) error { - query := tx.Rebind(`SELECT 1 FROM apps WHERE name=?`) - r := tx.QueryRowContext(ctx, query, route.AppName) + 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_name=? AND path=?`) - same, err := tx.QueryContext(ctx, query, route.AppName, route.Path) + 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 } @@ -431,7 +455,7 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode } query = tx.Rebind(`INSERT INTO routes ( - app_name, + app_id, path, image, format, @@ -447,7 +471,7 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode updated_at ) VALUES ( - :app_name, + :app_id, :path, :image, :format, @@ -474,8 +498,8 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode 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_name=? AND path=?", routeSelector)) - row := tx.QueryRowxContext(ctx, query, newroute.AppName, newroute.Path) + 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 { @@ -502,7 +526,7 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m config = :config, annotations = :annotations, updated_at = :updated_at - WHERE app_name=:app_name AND path=:path;`) + WHERE app_id=:app_id AND path=:path;`) res, err := tx.NamedExecContext(ctx, query, &route) if err != nil { @@ -525,9 +549,9 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m return &route, nil } -func (ds *sqlStore) RemoveRoute(ctx context.Context, appName, routePath string) error { - query := ds.db.Rebind(`DELETE FROM routes WHERE path = ? AND app_name = ?`) - res, err := ds.db.ExecContext(ctx, query, routePath, appName) +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 } @@ -544,10 +568,10 @@ func (ds *sqlStore) RemoveRoute(ctx context.Context, appName, routePath string) return nil } -func (ds *sqlStore) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { - rSelectCondition := "%s WHERE app_name=? AND path=?" +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, appName, routePath) + row := ds.db.QueryRowxContext(ctx, query, appID, routePath) var route models.Route err := row.StructScan(&route) @@ -560,13 +584,13 @@ func (ds *sqlStore) GetRoute(ctx context.Context, appName, routePath string) (*m } // GetRoutesByApp retrieves a route with a specific app name. -func (ds *sqlStore) GetRoutesByApp(ctx context.Context, appName string, filter *models.RouteFilter) ([]*models.Route, error) { +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.AppName = appName + filter.AppID = appID filterQuery, args := buildFilterRouteQuery(filter) query := fmt.Sprintf("%s %s", routeSelector, filterQuery) @@ -617,7 +641,7 @@ func (ds *sqlStore) InsertCall(ctx context.Context, call *models.Call) error { started_at, completed_at, status, - app_name, + app_id, path, stats, error @@ -628,7 +652,7 @@ func (ds *sqlStore) InsertCall(ctx context.Context, call *models.Call) error { :started_at, :completed_at, :status, - :app_name, + :app_id, :path, :stats, :error @@ -646,25 +670,25 @@ func equivalentCalls(expected *models.Call, actual *models.Call) bool { time.Time(expected.StartedAt).Unix() == time.Time(actual.StartedAt).Unix() && time.Time(expected.CompletedAt).Unix() == time.Time(actual.CompletedAt).Unix() && expected.Status == actual.Status && - expected.AppName == actual.AppName && expected.Path == actual.Path && expected.Error == actual.Error && - len(expected.Stats) == len(actual.Stats) + len(expected.Stats) == len(actual.Stats) && + expected.AppID == actual.AppID // TODO: We don't do comparisons of individual Stats. We probably should. return equivalentFields } func (ds *sqlStore) UpdateCall(ctx context.Context, from *models.Call, to *models.Call) error { // Assert that from and to are supposed to be the same call - if from.ID != to.ID || from.AppName != to.AppName { + if from.ID != to.ID || from.AppID != to.AppID { return errors.New("assertion error: 'from' and 'to' calls refer to different app/ID") } // Atomic update err := ds.Tx(func(tx *sqlx.Tx) error { var call models.Call - query := tx.Rebind(fmt.Sprintf(`%s WHERE id=? AND app_name=?`, callSelector)) - row := tx.QueryRowxContext(ctx, query, from.ID, from.AppName) + query := tx.Rebind(fmt.Sprintf(`%s WHERE id=? AND app_id=?`, callSelector)) + row := tx.QueryRowxContext(ctx, query, from.ID, from.AppID) err := row.StructScan(&call) if err == sql.ErrNoRows { @@ -686,11 +710,11 @@ func (ds *sqlStore) UpdateCall(ctx context.Context, from *models.Call, to *model started_at = :started_at, completed_at = :completed_at, status = :status, - app_name = :app_name, + app_id = :app_id, path = :path, stats = :stats, error = :error - WHERE id=:id AND app_name=:app_name;`) + WHERE id=:id AND app_id=:app_id;`) res, err := tx.NamedExecContext(ctx, query, to) if err != nil { @@ -713,10 +737,10 @@ func (ds *sqlStore) UpdateCall(ctx context.Context, from *models.Call, to *model return nil } -func (ds *sqlStore) GetCall(ctx context.Context, appName, callID string) (*models.Call, error) { - query := fmt.Sprintf(`%s WHERE id=? AND app_name=?`, callSelector) +func (ds *sqlStore) GetCall(ctx context.Context, appID, callID string) (*models.Call, error) { + query := fmt.Sprintf(`%s WHERE id=? AND app_id=?`, callSelector) query = ds.db.Rebind(query) - row := ds.db.QueryRowxContext(ctx, query, callID, appName) + row := ds.db.QueryRowxContext(ctx, query, callID, appID) var call models.Call err := row.StructScan(&call) @@ -754,7 +778,7 @@ func (ds *sqlStore) GetCalls(ctx context.Context, filter *models.CallFilter) ([] return res, nil } -func (ds *sqlStore) InsertLog(ctx context.Context, appName, callID string, logR io.Reader) error { +func (ds *sqlStore) InsertLog(ctx context.Context, appID, callID string, logR io.Reader) error { // coerce this into a string for sql var log string if stringer, ok := logR.(fmt.Stringer); ok { @@ -767,14 +791,15 @@ func (ds *sqlStore) InsertLog(ctx context.Context, appName, callID string, logR log = b.String() } - query := ds.db.Rebind(`INSERT INTO logs (id, app_name, log) VALUES (?, ?, ?);`) - _, err := ds.db.ExecContext(ctx, query, callID, appName, log) + query := ds.db.Rebind(`INSERT INTO logs (id, app_id, log) VALUES (?, ?, ?);`) + _, err := ds.db.ExecContext(ctx, query, callID, appID, log) + return err } -func (ds *sqlStore) GetLog(ctx context.Context, appName, callID string) (io.Reader, error) { - query := ds.db.Rebind(`SELECT log FROM logs WHERE id=? AND app_name=?`) - row := ds.db.QueryRowContext(ctx, query, callID, appName) +func (ds *sqlStore) GetLog(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) @@ -806,7 +831,7 @@ func buildFilterRouteQuery(filter *models.RouteFilter) (string, []interface{}) { } } - where("app_name=? ", filter.AppName) + where("app_id=? ", filter.AppID) where("image=?", filter.Image) where("path>?", filter.Cursor) // where("path LIKE ?%", filter.PathPrefix) TODO needs escaping @@ -857,7 +882,6 @@ func buildFilterAppQuery(filter *models.AppFilter) (string, []interface{}, error fmt.Fprintf(&b, ` LIMIT ?`) args = append(args, filter.PerPage) if len(filter.NameIn) > 0 { - // fmt.Println("about to sqlx.in:", b.String(), args) return sqlx.In(b.String(), args...) } return b.String(), args, nil @@ -888,7 +912,7 @@ func buildFilterCallQuery(filter *models.CallFilter) (string, []interface{}) { if !time.Time(filter.FromTime).IsZero() { where("created_at>", filter.FromTime.String()) } - where("app_name=", filter.AppName) + where("app_id=", filter.AppID) where("path=", filter.Path) fmt.Fprintf(&b, ` ORDER BY id DESC`) // TODO assert this is indexed diff --git a/api/logs/log_test.go b/api/logs/log_test.go index 659360d97..81b9e08f7 100644 --- a/api/logs/log_test.go +++ b/api/logs/log_test.go @@ -1,29 +1,11 @@ package logs import ( - "net/url" - "os" - "testing" - - "context" - - "github.com/fnproject/fn/api/datastore/sql" logTesting "github.com/fnproject/fn/api/logs/testing" + "testing" ) -const tmpLogDb = "/tmp/func_test_log.db" - func TestDatastore(t *testing.T) { - os.Remove(tmpLogDb) - ctx := context.Background() - uLog, err := url.Parse("sqlite3://" + tmpLogDb) - if err != nil { - t.Fatalf("failed to parse url: %v", err) - } - - ds, err := sql.New(ctx, uLog) - if err != nil { - t.Fatalf("failed to create sqlite3 datastore: %v", err) - } - logTesting.Test(t, ds) + ds := logTesting.SetupSQLiteDS(t) + logTesting.Test(t, ds, ds) } diff --git a/api/logs/mock.go b/api/logs/mock.go index 5aa99a758..b88b1d1af 100644 --- a/api/logs/mock.go +++ b/api/logs/mock.go @@ -17,13 +17,13 @@ func NewMock() models.LogStore { return &mock{make(map[string][]byte)} } -func (m *mock) InsertLog(ctx context.Context, appName, callID string, callLog io.Reader) error { +func (m *mock) InsertLog(ctx context.Context, appID, callID string, callLog io.Reader) error { bytes, err := ioutil.ReadAll(callLog) m.Logs[callID] = bytes return err } -func (m *mock) GetLog(ctx context.Context, appName, callID string) (io.Reader, error) { +func (m *mock) GetLog(ctx context.Context, appID, callID string) (io.Reader, error) { logEntry, ok := m.Logs[callID] if !ok { return nil, models.ErrCallLogNotFound diff --git a/api/logs/s3/s3.go b/api/logs/s3/s3.go index cfb6f4eb1..2fe660c45 100644 --- a/api/logs/s3/s3.go +++ b/api/logs/s3/s3.go @@ -63,8 +63,7 @@ func createStore(bucketName, endpoint, region, accessKeyID, secretAccessKey stri DisableSSL: aws.Bool(!useSSL), S3ForcePathStyle: aws.Bool(true), } - session := session.Must(session.NewSession(config)) - client := s3.New(session) + client := s3.New(session.Must(session.NewSession(config))) return &store{ client: client, @@ -125,14 +124,13 @@ func path(appName, callID string) string { return appName + "/" + callID } -func (s *store) InsertLog(ctx context.Context, appName, callID string, callLog io.Reader) error { +func (s *store) InsertLog(ctx context.Context, appID, callID string, callLog io.Reader) error { ctx, span := trace.StartSpan(ctx, "s3_insert_log") defer span.End() // wrap original reader in a decorator to keep track of read bytes without buffering cr := &countingReader{r: callLog} - - objectName := path(appName, callID) + objectName := path(appID, callID) params := &s3manager.UploadInput{ Bucket: aws.String(s.bucket), Key: aws.String(objectName), @@ -150,11 +148,11 @@ func (s *store) InsertLog(ctx context.Context, appName, callID string, callLog i return nil } -func (s *store) GetLog(ctx context.Context, appName, callID string) (io.Reader, error) { +func (s *store) GetLog(ctx context.Context, appID, callID string) (io.Reader, error) { ctx, span := trace.StartSpan(ctx, "s3_get_log") defer span.End() - objectName := path(appName, callID) + objectName := path(appID, callID) logrus.WithFields(logrus.Fields{"bucketName": s.bucket, "key": objectName}).Debug("Downloading log") // stream the logs to an in-memory buffer diff --git a/api/logs/s3/s3_test.go b/api/logs/s3/s3_test.go index 1335af104..19116292a 100644 --- a/api/logs/s3/s3_test.go +++ b/api/logs/s3/s3_test.go @@ -24,5 +24,5 @@ func TestS3(t *testing.T) { if err != nil { t.Fatalf("failed to create s3 datastore: %v", err) } - logTesting.Test(t, ls) + logTesting.Test(t, nil, ls) } diff --git a/api/logs/testing/test.go b/api/logs/testing/test.go index 599619177..0c8535a71 100644 --- a/api/logs/testing/test.go +++ b/api/logs/testing/test.go @@ -8,47 +8,88 @@ import ( "testing" "time" + "github.com/fnproject/fn/api/datastore/sql" "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/models" "github.com/go-openapi/strfmt" + "net/url" + "os" ) var testApp = &models.App{ Name: "Test", + ID: id.New().String(), } var testRoute = &models.Route{ - AppName: testApp.Name, - Path: "/test", - Image: "fnproject/fn-test-utils", - Type: "sync", - Format: "http", + Path: "/test", + Image: "fnproject/fn-test-utils", + Type: "sync", + Format: "http", } -func SetupTestCall() *models.Call { +func SetupTestCall(t *testing.T, ctx context.Context, ds models.Datastore) *models.Call { + testApp.SetDefaults() + + _, err := ds.InsertApp(ctx, testApp) + if err != nil { + t.Log(err.Error()) + t.Fatalf("Test InsertLog(ctx, call.ID, logText): unable to insert app, err: `%v`", err) + } + testRoute.AppID = testApp.ID + _, err = ds.InsertRoute(ctx, testRoute) + if err != nil { + t.Log(err.Error()) + t.Fatalf("Test InsertLog(ctx, call.ID, logText): unable to insert app route, err: `%v`", err) + } + var call models.Call + call.AppID = testApp.ID call.CreatedAt = strfmt.DateTime(time.Now()) call.Status = "success" call.StartedAt = strfmt.DateTime(time.Now()) call.CompletedAt = strfmt.DateTime(time.Now()) - call.AppName = testApp.Name + call.AppID = testApp.ID call.Path = testRoute.Path return &call } -func Test(t *testing.T, fnl models.LogStore) { +const tmpLogDb = "/tmp/func_test_log.db" + +func SetupSQLiteDS(t *testing.T) models.Datastore { + os.Remove(tmpLogDb) ctx := context.Background() - call := SetupTestCall() + uLog, err := url.Parse("sqlite3://" + tmpLogDb) + if err != nil { + t.Fatalf("failed to parse url: %v", err) + } + + ds, err := sql.New(ctx, uLog) + if err != nil { + t.Fatalf("failed to create sqlite3 datastore: %v", err) + } + return ds +} + +func Test(t *testing.T, ds models.Datastore, fnl models.LogStore) { + ctx := context.Background() + if ds == nil { + ds = SetupSQLiteDS(t) + } + call := SetupTestCall(t, ctx, ds) t.Run("call-log-insert-get", func(t *testing.T) { call.ID = id.New().String() logText := "test" log := strings.NewReader(logText) - err := fnl.InsertLog(ctx, call.AppName, call.ID, log) + err := fnl.InsertLog(ctx, call.AppID, call.ID, log) if err != nil { t.Fatalf("Test InsertLog(ctx, call.ID, logText): unexpected error during inserting log `%v`", err) } - logEntry, err := fnl.GetLog(ctx, call.AppName, call.ID) + logEntry, err := fnl.GetLog(ctx, testApp.ID, call.ID) + if err != nil { + t.Fatalf("Test InsertLog(ctx, call.ID, logText): unexpected error during log get `%v`", err) + } var b bytes.Buffer io.Copy(&b, logEntry) if !strings.Contains(b.String(), logText) { @@ -59,7 +100,7 @@ func Test(t *testing.T, fnl models.LogStore) { t.Run("call-log-not-found", func(t *testing.T) { call.ID = id.New().String() - _, err := fnl.GetLog(ctx, call.AppName, call.ID) + _, err := fnl.GetLog(ctx, call.AppID, call.ID) if err != models.ErrCallLogNotFound { t.Fatal("GetLog should return not found, but got:", err) } diff --git a/api/models/app.go b/api/models/app.go index 052c01027..8b0d3ad17 100644 --- a/api/models/app.go +++ b/api/models/app.go @@ -4,10 +4,12 @@ import ( "time" "unicode" + "github.com/fnproject/fn/api/id" "github.com/go-openapi/strfmt" ) type App struct { + ID string `json:"id" db:"id"` Name string `json:"name" db:"name"` Config Config `json:"config,omitempty" db:"config"` Annotations Annotations `json:"annotations,omitempty" db:"annotations"` @@ -26,6 +28,9 @@ func (a *App) SetDefaults() { // keeps the json from being nil a.Config = map[string]string{} } + if a.ID == "" { + a.ID = id.New().String() + } } func (a *App) Validate() error { @@ -58,7 +63,7 @@ func (a *App) Clone() *App { clone.Config[k] = v } } - + clone.ID = a.ID return clone } @@ -67,6 +72,7 @@ func (a1 *App) Equals(a2 *App) bool { // the RHS of && won't eval if eq==false so config checking is lazy eq := true + eq = eq && a1.ID == a2.ID eq = eq && a1.Name == a2.Name eq = eq && a1.Config.Equals(a2.Config) eq = eq && a1.Annotations.Equals(a2.Annotations) diff --git a/api/models/call.go b/api/models/call.go index 6a0b4f48a..7f80b65a8 100644 --- a/api/models/call.go +++ b/api/models/call.go @@ -27,12 +27,6 @@ const ( var possibleStatuses = [...]string{"delayed", "queued", "running", "success", "error", "cancelled"} -type CallLog struct { - CallID string `json:"call_id" db:"id"` - Log string `json:"log" db:"log"` - AppName string `json:"app_name" db:"app_name"` -} - // Call is a representation of a specific invocation of a route. type Call struct { // Unique identifier representing a specific call. @@ -75,9 +69,6 @@ type Call struct { // - client_request - Request was cancelled by a client. Status string `json:"status" db:"status"` - // App this call belongs to. - AppName string `json:"app_name" db:"app_name"` - // Path of the route that is responsible for this call Path string `json:"path" db:"path"` @@ -147,11 +138,13 @@ type Call struct { // Error is the reason why the call failed, it is only non-empty if // status is equal to "error". Error string `json:"error,omitempty" db:"error"` + // App this call belongs to. + AppID string `json:"app_id" db:"app_id"` } type CallFilter struct { Path string // match - AppName string // match + AppID string // match FromTime strfmt.DateTime ToTime strfmt.DateTime Cursor string diff --git a/api/models/datastore.go b/api/models/datastore.go index 5b23aa101..533c6cfa8 100644 --- a/api/models/datastore.go +++ b/api/models/datastore.go @@ -7,10 +7,15 @@ import ( ) type Datastore interface { - // GetApp gets an App by name. + // GetAppByID gets an App by ID. + // Returns ErrDatastoreEmptyAppID for empty appID. + // Returns ErrAppsNotFound if no app is found. + GetAppByID(ctx context.Context, appID string) (*App, error) + + // GetAppID gets an app ID by app name, ensures if app exists. // Returns ErrDatastoreEmptyAppName for empty appName. // Returns ErrAppsNotFound if no app is found. - GetApp(ctx context.Context, appName string) (*App, error) + GetAppID(ctx context.Context, appName string) (string, error) // GetApps gets a slice of Apps, optionally filtered by name. // Missing filter or empty name will match all Apps. @@ -28,17 +33,17 @@ type Datastore interface { // RemoveApp removes the App named appName. Returns ErrDatastoreEmptyAppName if appName is empty. // Returns ErrAppsNotFound if an App is not found. - RemoveApp(ctx context.Context, appName string) error + 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, appName, routePath string) (*Route, error) + 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, appName string, filter *RouteFilter) ([]*Route, error) + 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. @@ -49,9 +54,9 @@ type Datastore interface { // ErrDatastoreEmptyAppName or ErrDatastoreEmptyRoutePath for empty AppName or Path. UpdateRoute(ctx context.Context, route *Route) (*Route, error) - // RemoveRoute removes a route. Returns ErrDatastoreEmptyAppName when appName is empty, and + // 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, appName, routePath string) error + RemoveRoute(ctx context.Context, appID, routePath string) error // InsertCall inserts a call into the datastore, it will error if the call already // exists. @@ -63,7 +68,7 @@ type Datastore interface { UpdateCall(ctx context.Context, from *Call, to *Call) error // GetCall returns a call at a certain id and app name. - GetCall(ctx context.Context, appName, callID string) (*Call, error) + GetCall(ctx context.Context, appID, 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. diff --git a/api/models/error.go b/api/models/error.go index 0de55ffb6..77c5411ee 100644 --- a/api/models/error.go +++ b/api/models/error.go @@ -60,6 +60,10 @@ var ( code: http.StatusBadRequest, error: errors.New("Missing app"), } + ErrDatastoreEmptyAppID = err{ + code: http.StatusBadRequest, + error: errors.New("Missing app ID"), + } ErrDatastoreEmptyRoute = err{ code: http.StatusBadRequest, error: errors.New("Missing route"), @@ -112,7 +116,7 @@ var ( code: http.StatusBadRequest, error: errors.New("Invalid route Format"), } - ErrRoutesMissingAppName = err{ + ErrRoutesMissingAppID = err{ code: http.StatusBadRequest, error: errors.New("Missing route AppName"), } diff --git a/api/models/logs.go b/api/models/logs.go index c161b9aea..cd7dad07b 100644 --- a/api/models/logs.go +++ b/api/models/logs.go @@ -8,11 +8,11 @@ import ( type LogStore interface { // InsertLog will insert the log at callID, overwriting if it previously // existed. - InsertLog(ctx context.Context, appName, callID string, callLog io.Reader) error + InsertLog(ctx context.Context, appID, callID string, callLog io.Reader) error // GetLog will return the log at callID, an error will be returned if the log // cannot be found. - GetLog(ctx context.Context, appName, callID string) (io.Reader, error) + GetLog(ctx context.Context, appID, callID string) (io.Reader, error) // TODO we should probably allow deletion of a range of logs (also calls)? // common cases for deletion will be: diff --git a/api/models/route.go b/api/models/route.go index 42f4f985b..f31da82ac 100644 --- a/api/models/route.go +++ b/api/models/route.go @@ -25,7 +25,7 @@ var RouteMaxMemory = uint64(8 * 1024) type Routes []*Route type Route struct { - AppName string `json:"app_name" db:"app_name"` + 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"` @@ -83,8 +83,8 @@ func (r *Route) SetDefaults() { // Validate validates all field values, returning the first error, if any. func (r *Route) Validate() error { - if r.AppName == "" { - return ErrRoutesMissingAppName + if r.AppID == "" { + return ErrRoutesMissingAppID } if r.Path == "" { @@ -164,7 +164,7 @@ func (r1 *Route) Equals(r2 *Route) bool { // the RHS of && won't eval if eq==false so config/headers checking is lazy eq := true - eq = eq && r1.AppName == r2.AppName + eq = eq && r1.AppID == r2.AppID eq = eq && r1.Path == r2.Path eq = eq && r1.Image == r2.Image eq = eq && r1.Memory == r2.Memory @@ -244,7 +244,7 @@ func (r *Route) Update(patch *Route) { type RouteFilter struct { PathPrefix string // this is prefix match TODO - AppName string // this is exact match (important for security) + AppID string // this is exact match (important for security) Image string // this is exact match Cursor string diff --git a/api/models/route_test.go b/api/models/route_test.go index d5e8e59d1..798c512a6 100644 --- a/api/models/route_test.go +++ b/api/models/route_test.go @@ -1,13 +1,14 @@ package models import ( + "github.com/fnproject/fn/api/id" "testing" ) func TestRouteSimple(t *testing.T) { route1 := &Route{ - AppName: "test", + AppID: id.New().String(), Path: "/some", Image: "foo", Memory: 128, @@ -24,7 +25,7 @@ func TestRouteSimple(t *testing.T) { } route2 := &Route{ - AppName: "test", + AppID: id.New().String(), Path: "/some", Image: "foo", Memory: 128, diff --git a/api/server/apps_delete.go b/api/server/apps_delete.go index d73fc670b..6e924e776 100644 --- a/api/server/apps_delete.go +++ b/api/server/apps_delete.go @@ -10,8 +10,7 @@ import ( func (s *Server) handleAppDelete(c *gin.Context) { ctx := c.Request.Context() - appName := c.MustGet(api.AppName).(string) - err := s.datastore.RemoveApp(ctx, appName) + err := s.datastore.RemoveApp(ctx, c.MustGet(api.AppID).(string)) if err != nil { handleErrorResponse(c, err) return diff --git a/api/server/apps_get.go b/api/server/apps_get.go index 1b1c027d7..a08497d2f 100644 --- a/api/server/apps_get.go +++ b/api/server/apps_get.go @@ -7,12 +7,22 @@ import ( "github.com/gin-gonic/gin" ) -func (s *Server) handleAppGet(c *gin.Context) { +func (s *Server) handleAppGetByName(c *gin.Context) { ctx := c.Request.Context() - appName := c.MustGet(api.AppName).(string) - - app, err := s.datastore.GetApp(ctx, appName) + app, err := s.datastore.GetAppByID(ctx, c.MustGet(api.AppID).(string)) + if err != nil { + handleErrorResponse(c, err) + return + } + + c.JSON(http.StatusOK, appResponse{"Successfully loaded app", app}) +} + +func (s *Server) handleAppGetByID(c *gin.Context) { + ctx := c.Request.Context() + + app, err := s.datastore.GetAppByID(ctx, c.Param(api.CApp)) if err != nil { handleErrorResponse(c, err) return diff --git a/api/server/apps_test.go b/api/server/apps_test.go index d2eed2201..710258a71 100644 --- a/api/server/apps_test.go +++ b/api/server/apps_test.go @@ -112,6 +112,13 @@ func TestAppDelete(t *testing.T) { } }() + app := &models.App{ + Name: "myapp", + } + app.SetDefaults() + ds := datastore.NewMockInit( + []*models.App{app}, nil, nil, + ) for i, test := range []struct { ds models.Datastore logDB models.LogStore @@ -121,11 +128,7 @@ func TestAppDelete(t *testing.T) { expectedError error }{ {datastore.NewMock(), logs.NewMock(), "/v1/apps/myapp", "", http.StatusNotFound, nil}, - {datastore.NewMockInit( - []*models.App{{ - Name: "myapp", - }}, nil, nil, - ), logs.NewMock(), "/v1/apps/myapp", "", http.StatusOK, nil}, + {ds, logs.NewMock(), "/v1/apps/myapp", "", http.StatusOK, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr, ServerTypeFull) @@ -270,6 +273,14 @@ func TestAppUpdate(t *testing.T) { } }() + app := &models.App{ + Name: "myapp", + } + app.SetDefaults() + ds := datastore.NewMockInit( + []*models.App{app}, nil, nil, + ) + for i, test := range []struct { mock models.Datastore logDB models.LogStore @@ -279,35 +290,19 @@ func TestAppUpdate(t *testing.T) { expectedError error }{ // errors - {datastore.NewMock(), logs.NewMock(), "/v1/apps/myapp", ``, http.StatusBadRequest, models.ErrInvalidJSON}, + {ds, logs.NewMock(), "/v1/apps/myapp", ``, http.StatusBadRequest, models.ErrInvalidJSON}, // Addresses #380 - {datastore.NewMockInit( - []*models.App{{ - Name: "myapp", - }}, nil, nil, - ), logs.NewMock(), "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusConflict, nil}, + {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusConflict, nil}, // success: add/set MD key - {datastore.NewMockInit( - []*models.App{{ - Name: "myapp", - }}, nil, nil, - ), logs.NewMock(), "/v1/apps/myapp", `{ "app": { "annotations": {"k-0" : "val"} } }`, http.StatusOK, nil}, + {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "annotations": {"k-0" : "val"} } }`, http.StatusOK, nil}, // success - {datastore.NewMockInit( - []*models.App{{ - Name: "myapp", - }}, nil, nil, - ), logs.NewMock(), "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil}, + {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil}, // success - {datastore.NewMockInit( - []*models.App{{ - Name: "myapp", - }}, nil, nil, - ), logs.NewMock(), "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil}, + {ds, logs.NewMock(), "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr, ServerTypeFull) diff --git a/api/server/apps_update.go b/api/server/apps_update.go index def73ffe1..1a5dfcf3e 100644 --- a/api/server/apps_update.go +++ b/api/server/apps_update.go @@ -33,7 +33,8 @@ func (s *Server) handleAppUpdate(c *gin.Context) { return } - wapp.App.Name = c.MustGet(api.AppName).(string) + wapp.App.Name = c.MustGet(api.App).(string) + wapp.App.ID = c.MustGet(api.AppID).(string) app, err := s.datastore.UpdateApp(ctx, wapp.App) if err != nil { diff --git a/api/server/call_get.go b/api/server/call_get.go index 767e13b87..e2b1c0bf3 100644 --- a/api/server/call_get.go +++ b/api/server/call_get.go @@ -10,9 +10,10 @@ import ( func (s *Server) handleCallGet(c *gin.Context) { ctx := c.Request.Context() - appName := c.MustGet(api.AppName).(string) callID := c.Param(api.Call) - callObj, err := s.datastore.GetCall(ctx, appName, callID) + appID := c.MustGet(api.AppID).(string) + + callObj, err := s.datastore.GetCall(ctx, appID, callID) if err != nil { handleErrorResponse(c, err) return diff --git a/api/server/call_list.go b/api/server/call_list.go index e81172dca..0b5b4e285 100644 --- a/api/server/call_list.go +++ b/api/server/call_list.go @@ -13,14 +13,13 @@ import ( func (s *Server) handleCallList(c *gin.Context) { ctx := c.Request.Context() + var err error - appName := c.MustGet(api.AppName).(string) - + appID := c.MustGet(api.AppID).(string) // TODO api.CRoute needs to be escaped probably, since it has '/' a lot - filter := models.CallFilter{AppName: appName, Path: c.Query("path")} + filter := models.CallFilter{AppID: appID, Path: c.Query("path")} filter.Cursor, filter.PerPage = pageParams(c, false) // ids are url safe - var err error filter.FromTime, filter.ToTime, err = timeParams(c) if err != nil { handleErrorResponse(c, err) @@ -29,16 +28,6 @@ func (s *Server) handleCallList(c *gin.Context) { calls, err := s.datastore.GetCalls(ctx, &filter) - if len(calls) == 0 { - // TODO this should be done in front of this handler to even get here... - _, err = s.datastore.GetApp(c, appName) - } - - if err != nil { - handleErrorResponse(c, err) - return - } - var nextCursor string if len(calls) > 0 && len(calls) == filter.PerPage { nextCursor = calls[len(calls)-1].ID diff --git a/api/server/call_logs.go b/api/server/call_logs.go index 2218c488c..5ced30e56 100644 --- a/api/server/call_logs.go +++ b/api/server/call_logs.go @@ -15,28 +15,32 @@ import ( // note: for backward compatibility, will go away later type callLogResponse struct { - Message string `json:"message"` - Log *models.CallLog `json:"log"` + Message string `json:"message"` + Log *CallLog `json:"log"` } -func writeJSON(c *gin.Context, callID, appName string, logReader io.Reader) { +type CallLog struct { + CallID string `json:"call_id" db:"id"` + Log string `json:"log" db:"log"` +} + +func writeJSON(c *gin.Context, callID string, logReader io.Reader) { var b bytes.Buffer b.ReadFrom(logReader) c.JSON(http.StatusOK, callLogResponse{"Successfully loaded log", - &models.CallLog{ - CallID: callID, - AppName: appName, - Log: b.String(), + &CallLog{ + CallID: callID, + Log: b.String(), }}) } func (s *Server) handleCallLogGet(c *gin.Context) { ctx := c.Request.Context() - appName := c.MustGet(api.AppName).(string) + appID := c.MustGet(api.AppID).(string) callID := c.Param(api.Call) - logReader, err := s.logstore.GetLog(ctx, appName, callID) + logReader, err := s.logstore.GetLog(ctx, appID, callID) if err != nil { handleErrorResponse(c, err) return @@ -45,13 +49,13 @@ func (s *Server) handleCallLogGet(c *gin.Context) { mimeTypes, _ := c.Request.Header["Accept"] if len(mimeTypes) == 0 { - writeJSON(c, callID, appName, logReader) + writeJSON(c, callID, logReader) return } for _, mimeType := range mimeTypes { if strings.Contains(mimeType, "application/json") { - writeJSON(c, callID, appName, logReader) + writeJSON(c, callID, logReader) return } if strings.Contains(mimeType, "text/plain") { @@ -60,7 +64,7 @@ func (s *Server) handleCallLogGet(c *gin.Context) { } if strings.Contains(mimeType, "*/*") { - writeJSON(c, callID, appName, logReader) + writeJSON(c, callID, logReader) return } } diff --git a/api/server/calls_test.go b/api/server/calls_test.go index 5bec43eb6..92e50a5d4 100644 --- a/api/server/calls_test.go +++ b/api/server/calls_test.go @@ -19,18 +19,30 @@ import ( func TestCallGet(t *testing.T) { buf := setLogBuffer() + app := &models.App{Name: "myapp"} + app.SetDefaults() call := &models.Call{ - ID: id.New().String(), - AppName: "myapp", - Path: "/thisisatest", + AppID: app.ID, + ID: id.New().String(), + Path: "/thisisatest", + Image: "fnproject/hello", + // Delay: 0, + Type: "sync", + Format: "default", + // Payload: TODO, + Priority: new(int32), // TODO this is crucial, apparently + Timeout: 30, + IdleTimeout: 30, + Memory: 256, + CreatedAt: strfmt.DateTime(time.Now()), + URL: "http://localhost:8080/r/myapp/thisisatest", + Method: "GET", } rnr, cancel := testRunner(t) defer cancel() ds := datastore.NewMockInit( - []*models.App{ - {Name: call.AppName}, - }, + []*models.App{app}, nil, []*models.Call{call}, ) @@ -44,7 +56,8 @@ func TestCallGet(t *testing.T) { expectedError error }{ {"/v1/apps//calls/" + call.ID, "", http.StatusBadRequest, models.ErrAppsMissingName}, - {"/v1/apps/nodawg/calls/" + call.ID, "", http.StatusNotFound, models.ErrCallNotFound}, // TODO a little weird + {"/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}, } { @@ -52,6 +65,7 @@ func TestCallGet(t *testing.T) { 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) } @@ -61,6 +75,8 @@ func TestCallGet(t *testing.T) { if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { t.Log(buf.String()) + t.Log(resp.Error.Message) + t.Log(rec.Body.String()) t.Errorf("Test %d: Expected error message to have `%s`", i, test.expectedError.Error()) } @@ -72,10 +88,25 @@ func TestCallGet(t *testing.T) { func TestCallList(t *testing.T) { buf := setLogBuffer() + app := &models.App{Name: "myapp"} + app.SetDefaults() + call := &models.Call{ - ID: id.New().String(), - AppName: "myapp", - Path: "/thisisatest", + AppID: app.ID, + ID: id.New().String(), + Path: "/thisisatest", + Image: "fnproject/hello", + // Delay: 0, + Type: "sync", + Format: "default", + // Payload: TODO, + Priority: new(int32), // TODO this is crucial, apparently + Timeout: 30, + IdleTimeout: 30, + Memory: 256, + CreatedAt: strfmt.DateTime(time.Now()), + URL: "http://localhost:8080/r/myapp/thisisatest", + Method: "GET", } c2 := *call c3 := *call @@ -89,9 +120,7 @@ func TestCallList(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() ds := datastore.NewMockInit( - []*models.App{ - {Name: call.AppName}, - }, + []*models.App{app}, nil, []*models.Call{call, &c2, &c3}, ) diff --git a/api/server/extension_points.go b/api/server/extension_points.go index 4fa1a7bc8..0dfc352fb 100644 --- a/api/server/extension_points.go +++ b/api/server/extension_points.go @@ -19,8 +19,8 @@ func (s *Server) apiHandlerWrapperFunc(apiHandler fnext.ApiHandler) gin.HandlerF func (s *Server) apiAppHandlerWrapperFunc(apiHandler fnext.ApiAppHandler) gin.HandlerFunc { return func(c *gin.Context) { // get the app - appName := c.Param(api.CApp) - app, err := s.datastore.GetApp(c.Request.Context(), appName) + appID := c.MustGet(api.AppID).(string) + app, err := s.datastore.GetAppByID(c.Request.Context(), appID) if err != nil { handleErrorResponse(c, err) c.Abort() @@ -39,22 +39,9 @@ func (s *Server) apiAppHandlerWrapperFunc(apiHandler fnext.ApiAppHandler) gin.Ha func (s *Server) apiRouteHandlerWrapperFunc(apiHandler fnext.ApiRouteHandler) gin.HandlerFunc { return func(c *gin.Context) { context := c.Request.Context() - // get the app - appName := c.Param(api.CApp) - app, err := s.datastore.GetApp(context, appName) - if err != nil { - handleErrorResponse(c, err) - c.Abort() - return - } - if app == nil { - handleErrorResponse(c, models.ErrAppsNotFound) - c.Abort() - return - } - // get the route TODO + appID := c.MustGet(api.AppID).(string) routePath := "/" + c.Param(api.CRoute) - route, err := s.datastore.GetRoute(context, appName, routePath) + route, err := s.datastore.GetRoute(context, appID, routePath) if err != nil { handleErrorResponse(c, err) c.Abort() @@ -66,6 +53,18 @@ func (s *Server) apiRouteHandlerWrapperFunc(apiHandler fnext.ApiRouteHandler) gi return } + app, err := s.datastore.GetAppByID(context, appID) + if err != nil { + handleErrorResponse(c, err) + c.Abort() + return + } + if app == nil { + handleErrorResponse(c, models.ErrAppsNotFound) + c.Abort() + return + } + apiHandler.ServeHTTP(c.Writer, c.Request, app, route) } } @@ -85,6 +84,7 @@ func (s *Server) AddEndpointFunc(method, path string, handler func(w http.Respon // AddAppEndpoint adds an endpoints to /v1/apps/:app/x func (s *Server) AddAppEndpoint(method, path string, handler fnext.ApiAppHandler) { v1 := s.Router.Group("/v1") + v1.Use(s.checkAppPresenceByName()) v1.Handle(method, "/apps/:app"+path, s.apiAppHandlerWrapperFunc(handler)) } @@ -96,6 +96,7 @@ func (s *Server) AddAppEndpointFunc(method, path string, handler func(w http.Res // 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.apiRouteHandlerWrapperFunc(handler)) // conflicts with existing wildcard } diff --git a/api/server/gin_middlewares.go b/api/server/gin_middlewares.go index 7cd62e730..796c7b210 100644 --- a/api/server/gin_middlewares.go +++ b/api/server/gin_middlewares.go @@ -81,8 +81,8 @@ func loggerWrap(c *gin.Context) { ctx, _ := common.LoggerWithFields(c.Request.Context(), extractFields(c)) if appName := c.Param(api.CApp); appName != "" { - c.Set(api.AppName, appName) - ctx = context.WithValue(ctx, api.AppName, appName) + c.Set(api.App, appName) + ctx = context.WithValue(ctx, api.App, appName) } if routePath := c.Param(api.CRoute); routePath != "" { @@ -94,9 +94,49 @@ func loggerWrap(c *gin.Context) { c.Next() } +func (s *Server) checkAppPresenceByNameAtRunner() gin.HandlerFunc { + return func(c *gin.Context) { + ctx, _ := common.LoggerWithFields(c.Request.Context(), extractFields(c)) + + appName := c.Param(api.CApp) + if appName != "" { + appID, err := s.agent.GetAppID(ctx, appName) + if err != nil { + handleErrorResponse(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)) + + appName := c.MustGet(api.App).(string) + if appName != "" { + appID, err := s.datastore.GetAppID(ctx, appName) + if err != nil { + handleErrorResponse(c, err) + c.Abort() + return + } + c.Set(api.AppID, appID) + } + + c.Request = c.Request.WithContext(ctx) + c.Next() + } +} + func setAppNameInCtx(c *gin.Context) { // add appName to context - appName := c.GetString(api.AppName) + appName := c.GetString(api.App) if appName != "" { c.Request = c.Request.WithContext(context.WithValue(c.Request.Context(), fnext.AppNameKey, appName)) } @@ -104,7 +144,7 @@ func setAppNameInCtx(c *gin.Context) { } func appNameCheck(c *gin.Context) { - appName := c.GetString(api.AppName) + appName := c.GetString(api.App) if appName == "" { handleErrorResponse(c, models.ErrAppsMissingName) c.Abort() diff --git a/api/server/hybrid.go b/api/server/hybrid.go index de04c7c3f..e25465d0b 100644 --- a/api/server/hybrid.go +++ b/api/server/hybrid.go @@ -170,7 +170,7 @@ func (s *Server) handleRunnerFinish(c *gin.Context) { // note: Not returning err here since the job could have already finished successfully. } - if err := s.logstore.InsertLog(ctx, call.AppName, call.ID, strings.NewReader(body.Log)); err != nil { + if err := s.logstore.InsertLog(ctx, call.AppID, call.ID, strings.NewReader(body.Log)); err != nil { common.Logger(ctx).WithError(err).Error("error uploading log") // note: Not returning err here since the job could have already finished successfully. } diff --git a/api/server/middleware.go b/api/server/middleware.go index bccb0773d..82085f071 100644 --- a/api/server/middleware.go +++ b/api/server/middleware.go @@ -4,6 +4,7 @@ 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" @@ -24,9 +25,24 @@ type middlewareController struct { 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 := ctx.Value(api.CApp).(string) + if appName != "" { + appID, err := c.server.datastore.GetAppID(ctx, appName) + if err != nil { + handleErrorResponse(c.ginContext, err) + c.ginContext.Abort() + return + } + c.ginContext.Set(api.AppID, appID) + } + c.server.handleFunctionCall(c.ginContext) c.ginContext.Abort() } diff --git a/api/server/middleware_test.go b/api/server/middleware_test.go index bf1031e06..6a6de597f 100644 --- a/api/server/middleware_test.go +++ b/api/server/middleware_test.go @@ -67,15 +67,16 @@ func TestMiddlewareChaining(t *testing.T) { func TestRootMiddleware(t *testing.T) { + app1 := &models.App{Name: "myapp", Config: models.Config{}} + app1.SetDefaults() + app2 := &models.App{Name: "myapp2", Config: models.Config{}} + app2.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp", Config: models.Config{}}, - {Name: "myapp2", Config: models.Config{}}, - }, + []*models.App{app1, app2}, []*models.Route{ - {Path: "/", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Memory: 128, CPUs: 100, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}}, - {Path: "/myroute", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}}, - {Path: "/app2func", AppName: "myapp2", Image: "fnproject/fn-test-utils", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}, + {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"}, }, }, nil, @@ -93,7 +94,7 @@ func TestRootMiddleware(t *testing.T) { fmt.Fprintf(os.Stderr, "breaker breaker!\n") ctx := r.Context() // TODO: this is a little dicey, should have some functions to set these in case the context keys change or something. - ctx = context.WithValue(ctx, "app_name", "myapp2") + ctx = context.WithValue(ctx, "app", "myapp2") ctx = context.WithValue(ctx, "path", "/app2func") mctx := fnext.GetMiddlewareController(ctx) mctx.CallFunction(w, r.WithContext(ctx)) @@ -105,7 +106,7 @@ func TestRootMiddleware(t *testing.T) { }) srv.AddRootMiddlewareFunc(func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // fmt.Fprintf(os.Stderr, "middle log\n") + fmt.Fprintf(os.Stderr, "middle log\n") next.ServeHTTP(w, r) }) }) @@ -141,7 +142,7 @@ func TestRootMiddleware(t *testing.T) { } rbody := string(result) - t.Log("rbody:", rbody) + t.Logf("Test %v: response body: %v", i, rbody) if !strings.Contains(rbody, test.expectedInBody) { t.Fatal(i, "middleware didn't work correctly", string(result)) } diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go index 159dda997..c7c728671 100644 --- a/api/server/routes_create_update.go +++ b/api/server/routes_create_update.go @@ -22,7 +22,8 @@ import ( update only Patch accepts partial updates / skips validation of zero values. */ -func (s *Server) handleRoutesPostPutPatch(c *gin.Context) { + +func (s *Server) handleRoutesPostPut(c *gin.Context) { ctx := c.Request.Context() method := strings.ToUpper(c.Request.Method) @@ -32,14 +33,36 @@ func (s *Server) handleRoutesPostPutPatch(c *gin.Context) { handleErrorResponse(c, err) return } - if method != http.MethodPatch { - err = s.ensureApp(ctx, &wroute, method) - if err != nil { - handleErrorResponse(c, err) - return - } + appName := c.MustGet(api.App).(string) + + appID, err := s.ensureApp(ctx, appName, method) + if err != nil { + handleErrorResponse(c, err) + return } - resp, err := s.ensureRoute(ctx, method, &wroute) + + resp, err := s.ensureRoute(ctx, appID, &wroute, method) + if err != nil { + handleErrorResponse(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 { + handleErrorResponse(c, err) + return + } + appID := c.MustGet(api.AppID).(string) + + resp, err := s.ensureRoute(ctx, appID, &wroute, method) if err != nil { handleErrorResponse(c, err) return @@ -66,10 +89,11 @@ func (s *Server) changeRoute(ctx context.Context, wroute *models.RouteWrapper) e return nil } -// ensureApp will only execute if it is on put -func (s *Server) ensureRoute(ctx context.Context, method string, wroute *models.RouteWrapper) (routeResponse, error) { +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) @@ -78,7 +102,7 @@ func (s *Server) ensureRoute(ctx context.Context, method string, wroute *models. } return routeResponse{"Route successfully created", wroute.Route}, nil case http.MethodPut: - _, err := s.datastore.GetRoute(ctx, wroute.Route.AppName, wroute.Route.Path) + _, err := s.datastore.GetRoute(ctx, appID, wroute.Route.Path) if err != nil && err == models.ErrRoutesNotFound { err := s.submitRoute(ctx, wroute) if err != nil { @@ -102,19 +126,22 @@ func (s *Server) ensureRoute(ctx context.Context, method string, wroute *models. } // 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, wroute *models.RouteWrapper, method string) error { - app, err := s.datastore.GetApp(ctx, wroute.Route.AppName) +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 app == nil { - // Create a new application - newapp := &models.App{Name: wroute.Route.AppName} - _, err = s.datastore.InsertApp(ctx, newapp) + 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 "", err } + return app.ID, nil } - return nil + + return appID, nil } // bindRoute binds the RouteWrapper to the json from the request. @@ -130,7 +157,6 @@ func bindRoute(c *gin.Context, method string, wroute *models.RouteWrapper) error if wroute.Route == nil { return models.ErrRoutesMissingNew } - wroute.Route.AppName = c.MustGet(api.AppName).(string) if method == http.MethodPut || method == http.MethodPatch { p := path.Clean(c.MustGet(api.Path).(string)) diff --git a/api/server/routes_delete.go b/api/server/routes_delete.go index cebe03c65..4e57d6ac1 100644 --- a/api/server/routes_delete.go +++ b/api/server/routes_delete.go @@ -11,15 +11,15 @@ import ( func (s *Server) handleRouteDelete(c *gin.Context) { ctx := c.Request.Context() - appName := c.MustGet(api.AppName).(string) + appID := c.MustGet(api.AppID).(string) routePath := path.Clean(c.MustGet(api.Path).(string)) - if _, err := s.datastore.GetRoute(ctx, appName, routePath); err != nil { + if _, err := s.datastore.GetRoute(ctx, appID, routePath); err != nil { handleErrorResponse(c, err) return } - if err := s.datastore.RemoveRoute(ctx, appName, routePath); err != nil { + if err := s.datastore.RemoveRoute(ctx, appID, routePath); err != nil { handleErrorResponse(c, err) return } diff --git a/api/server/routes_get.go b/api/server/routes_get.go index f508d56f9..9c026108d 100644 --- a/api/server/routes_get.go +++ b/api/server/routes_get.go @@ -8,12 +8,11 @@ import ( "github.com/gin-gonic/gin" ) -func (s *Server) handleRouteGet(c *gin.Context) { +func routeGet(s *Server, appID string, c *gin.Context) { ctx := c.Request.Context() - appName := c.MustGet(api.AppName).(string) routePath := path.Clean("/" + c.MustGet(api.Path).(string)) - route, err := s.datastore.GetRoute(ctx, appName, routePath) + route, err := s.datastore.GetRoute(ctx, appID, routePath) if err != nil { handleErrorResponse(c, err) return @@ -21,3 +20,11 @@ func (s *Server) handleRouteGet(c *gin.Context) { c.JSON(http.StatusOK, routeResponse{"Successfully loaded route", route}) } + +func (s *Server) handleRouteGetAPI(c *gin.Context) { + routeGet(s, c.MustGet(api.AppID).(string), c) +} + +func (s *Server) handleRouteGetRunner(c *gin.Context) { + routeGet(s, c.Param(api.CApp), c) +} diff --git a/api/server/routes_list.go b/api/server/routes_list.go index e88204785..076f425dd 100644 --- a/api/server/routes_list.go +++ b/api/server/routes_list.go @@ -12,22 +12,12 @@ import ( func (s *Server) handleRouteList(c *gin.Context) { ctx := c.Request.Context() - appName := c.MustGet(api.AppName).(string) - 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, appName, &filter) - - // if there are no routes for the app, check if the app exists to return - // 404 if it does not - // TODO this should be done in front of this handler to even get here... - if err == nil && len(routes) == 0 { - _, err = s.datastore.GetApp(ctx, appName) - } - + routes, err := s.datastore.GetRoutesByApp(ctx, c.MustGet(api.AppID).(string), &filter) if err != nil { handleErrorResponse(c, err) return diff --git a/api/server/routes_test.go b/api/server/routes_test.go index f08ebc828..d3112d952 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -34,6 +34,7 @@ func (test *routeTestCase) run(t *testing.T, i int, buf *bytes.Buffer) { 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) } @@ -97,22 +98,25 @@ func (test *routeTestCase) run(t *testing.T, i int, buf *bytes.Buffer) { func TestRouteCreate(t *testing.T) { buf := setLogBuffer() + a := &models.App{Name: "a"} + a.SetDefaults() + commonDS := datastore.NewMockInit([]*models.App{a}, nil, nil) for i, test := range []routeTestCase{ // errors - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "path": "/myroute", "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesMissingPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesInvalidPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, - {datastore.NewMock(), 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(nil, + {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{ { - AppName: "a", - Path: "/myroute", + AppID: a.ID, + Path: "/myroute", }, }, nil, ), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesAlreadyExists}, @@ -129,21 +133,25 @@ func TestRouteCreate(t *testing.T) { func TestRoutePut(t *testing.T) { buf := setLogBuffer() + a := &models.App{Name: "a"} + a.SetDefaults() + commonDS := datastore.NewMockInit([]*models.App{a}, nil, nil) + for i, test := range []routeTestCase{ // errors (NOTE: this route doesn't exist yet) - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "path": "/myroute", "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "diffRoute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesInvalidType}, - {datastore.NewMock(), 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}, + {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 - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "path": "/myroute", "type": "sync" } }`, http.StatusOK, nil}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/fn-test-utils", "type": "sync" } }`, http.StatusOK, nil}, + {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}, } { test.run(t, i, buf) } @@ -152,8 +160,10 @@ func TestRoutePut(t *testing.T) { func TestRouteDelete(t *testing.T) { buf := setLogBuffer() - routes := []*models.Route{{AppName: "a", Path: "/myroute"}} - apps := []*models.App{{Name: "a", Config: nil}} + a := &models.App{Name: "a"} + a.SetDefaults() + routes := []*models.Route{{AppID: a.ID, Path: "/myroute"}} + commonDS := datastore.NewMockInit([]*models.App{a}, routes, nil) for i, test := range []struct { ds models.Datastore @@ -163,8 +173,8 @@ func TestRouteDelete(t *testing.T) { expectedCode int expectedError error }{ - {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, models.ErrRoutesNotFound}, - {datastore.NewMockInit(apps, routes, nil), logs.NewMock(), "/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, + {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) @@ -172,6 +182,7 @@ func TestRouteDelete(t *testing.T) { 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) } @@ -195,23 +206,23 @@ func TestRouteList(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() + app := &models.App{Name: "myapp"} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp"}, - }, + []*models.App{app}, []*models.Route{ { - AppName: "myapp", - Path: "/myroute", + Path: "/myroute", + AppID: app.ID, }, { - AppName: "myapp", - Path: "/myroute1", + Path: "/myroute1", + AppID: app.ID, }, { - AppName: "myapp", - Path: "/myroute2", - Image: "fnproject/fn-test-utils", + Path: "/myroute2", + Image: "fnproject/fn-test-utils", + AppID: app.ID, }, }, nil, // no calls diff --git a/api/server/runner.go b/api/server/runner.go index 0620a856e..795878005 100644 --- a/api/server/runner.go +++ b/api/server/runner.go @@ -39,18 +39,16 @@ func (s *Server) handleFunctionCall2(c *gin.Context) error { p = r.(string) } - var a string - ai := ctx.Value(api.AppName) - if ai == nil { - err := models.ErrAppsMissingName + appID := c.MustGet(api.AppID).(string) + app, err := s.agent.GetAppByID(ctx, appID) + if err != nil { return err } - a = ai.(string) // 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.serve(c, a, path.Clean(p)) + return s.serve(c, app, path.Clean(p)) } var ( @@ -59,7 +57,7 @@ var ( // TODO it would be nice if we could make this have nothing to do with the gin.Context but meh // TODO make async store an *http.Request? would be sexy until we have different api format... -func (s *Server) serve(c *gin.Context, appName, path string) error { +func (s *Server) serve(c *gin.Context, app *models.App, path string) error { buf := bufPool.Get().(*bytes.Buffer) buf.Reset() writer := syncResponseWriter{ @@ -70,14 +68,18 @@ func (s *Server) serve(c *gin.Context, appName, path string) error { // 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(appName, path, c.Request), + agent.FromRequest(app, path, c.Request), ) if err != nil { return err } - model := call.Model() { // scope this, to disallow ctx use outside of this scope. add id for handleErrorResponse logger ctx, _ := common.LoggerWithFields(c.Request.Context(), logrus.Fields{"id": model.ID}) diff --git a/api/server/runner_async_test.go b/api/server/runner_async_test.go index 18e9e00cc..9ee4fa5b5 100644 --- a/api/server/runner_async_test.go +++ b/api/server/runner_async_test.go @@ -35,16 +35,16 @@ func testRouterAsync(ds models.Datastore, mq models.MessageQueue, rnr agent.Agen func TestRouteRunnerAsyncExecution(t *testing.T) { buf := setLogBuffer() + app := &models.App{Name: "myapp", Config: map[string]string{"app": "true"}} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp", Config: map[string]string{"app": "true"}}, - }, + []*models.App{app}, []*models.Route{ - {Type: "async", Path: "/hot-http", AppName: "myapp", Image: "fnproject/fn-test-utils", Format: "http", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 4, IdleTimeout: 30}, - {Type: "async", Path: "/hot-json", AppName: "myapp", Image: "fnproject/fn-test-utils", Format: "json", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 4, IdleTimeout: 30}, - {Type: "async", Path: "/myroute", AppName: "myapp", Image: "fnproject/fn-test-utils", Config: map[string]string{"test": "true"}, Memory: 128, CPUs: 200, Timeout: 30, IdleTimeout: 30}, - {Type: "async", Path: "/myerror", AppName: "myapp", Image: "fnproject/fn-test-utils", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, - {Type: "async", Path: "/myroute/:param", AppName: "myapp", Image: "fnproject/fn-test-utils", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, + {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}, }, nil, ) mq := &mqs.Mock{} diff --git a/api/server/runner_test.go b/api/server/runner_test.go index f35f8b9cc..1ffc59ce4 100644 --- a/api/server/runner_test.go +++ b/api/server/runner_test.go @@ -41,7 +41,7 @@ func envTweaker(name, value string) func() { } } -func testRunner(t *testing.T, args ...interface{}) (agent.Agent, context.CancelFunc) { +func testRunner(_ *testing.T, args ...interface{}) (agent.Agent, context.CancelFunc) { ds := datastore.NewMock() var mq models.MessageQueue = &mqs.Mock{} for _, a := range args { @@ -58,10 +58,10 @@ func testRunner(t *testing.T, args ...interface{}) (agent.Agent, context.CancelF func TestRouteRunnerGet(t *testing.T) { buf := setLogBuffer() + app := &models.App{Name: "myapp", Config: models.Config{}} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp", Config: models.Config{}}, - }, nil, nil, + []*models.App{app}, nil, nil, ) rnr, cancel := testRunner(t, ds) @@ -102,10 +102,10 @@ func TestRouteRunnerGet(t *testing.T) { func TestRouteRunnerPost(t *testing.T) { buf := setLogBuffer() + app := &models.App{Name: "myapp", Config: models.Config{}} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp", Config: models.Config{}}, - }, nil, nil, + []*models.App{app}, nil, nil, ) rnr, cancel := testRunner(t, ds) @@ -169,13 +169,14 @@ func TestRouteRunnerIOPipes(t *testing.T) { rCfg := map[string]string{"ENABLE_HEADER": "yes", "ENABLE_FOOTER": "yes"} // enable container start/end header/footer rImg := "fnproject/fn-test-utils" + app := &models.App{Name: "zoo"} + app.SetDefaults() + ds := datastore.NewMockInit( - []*models.App{ - {Name: "zoo", Config: models.Config{}}, - }, + []*models.App{app}, []*models.Route{ - {Path: "/json", AppName: "zoo", Image: rImg, Type: "sync", Format: "json", Memory: 64, Timeout: 30, IdleTimeout: 30, Config: rCfg}, - {Path: "/http", AppName: "zoo", Image: rImg, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Config: rCfg}, + {Path: "/json", AppID: app.ID, Image: rImg, Type: "sync", Format: "json", Memory: 64, Timeout: 30, IdleTimeout: 30, Config: rCfg}, + {Path: "/http", AppID: app.ID, Image: rImg, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Config: rCfg}, }, nil, ) @@ -328,23 +329,23 @@ func TestRouteRunnerExecution(t *testing.T) { rImgBs1 := "fnproject/imagethatdoesnotexist" rImgBs2 := "localhost:5000/fnproject/imagethatdoesnotexist" + app := &models.App{Name: "myapp"} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp", Config: models.Config{}}, - }, + []*models.App{app}, []*models.Route{ - {Path: "/", AppName: "myapp", Image: rImg, Type: "sync", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myhot", AppName: "myapp", Image: rImg, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myhotjason", AppName: "myapp", Image: rImg, Type: "sync", Format: "json", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myroute", AppName: "myapp", Image: rImg, Type: "sync", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myerror", AppName: "myapp", Image: rImg, Type: "sync", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/mydne", AppName: "myapp", Image: rImgBs1, Type: "sync", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/mydnehot", AppName: "myapp", Image: rImgBs1, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/mydneregistry", AppName: "myapp", Image: rImgBs2, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/myoom", AppName: "myapp", Image: rImg, Type: "sync", Memory: 8, Timeout: 30, IdleTimeout: 30, Headers: rHdr, Config: rCfg}, - {Path: "/mybigoutputcold", AppName: "myapp", Image: rImg, Type: "sync", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, - {Path: "/mybigoutputhttp", AppName: "myapp", Image: rImg, Type: "sync", Format: "http", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, - {Path: "/mybigoutputjson", AppName: "myapp", Image: rImg, Type: "sync", Format: "json", Memory: 64, Timeout: 10, IdleTimeout: 20, Headers: rHdr, Config: rCfg}, + {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}, }, nil, ) @@ -408,9 +409,9 @@ func TestRouteRunnerExecution(t *testing.T) { {"/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, "function response too large", 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, "function response too large", nil}, + {"/r/myapp/mybigoutputcold", bigoutput, "GET", http.StatusBadGateway, nil, "", nil}, {"/r/myapp/mybigoutputcold", smalloutput, "GET", http.StatusOK, nil, "", nil}, } { trx := fmt.Sprintf("_trx_%d_", i) @@ -531,12 +532,12 @@ func (mock *errorMQ) Code() int { func TestFailedEnqueue(t *testing.T) { buf := setLogBuffer() + app := &models.App{Name: "myapp", Config: models.Config{}} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp", Config: models.Config{}}, - }, + []*models.App{app}, []*models.Route{ - {Path: "/dummy", AppName: "myapp", Image: "dummy/dummy", Type: "async", Memory: 128, Timeout: 30, IdleTimeout: 30}, + {Path: "/dummy", Image: "dummy/dummy", Type: "async", Memory: 128, Timeout: 30, IdleTimeout: 30, AppID: app.ID}, }, nil, ) err := errors.New("Unable to push task to queue") @@ -580,16 +581,16 @@ func TestRouteRunnerTimeout(t *testing.T) { models.RouteMaxMemory = uint64(1024 * 1024 * 1024) // 1024 TB hugeMem := uint64(models.RouteMaxMemory - 1) + app := &models.App{Name: "myapp", Config: models.Config{}} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp", Config: models.Config{}}, - }, + []*models.App{app}, []*models.Route{ - {Path: "/cold", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Memory: 128, Timeout: 4, IdleTimeout: 30}, - {Path: "/hot", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: 128, Timeout: 4, IdleTimeout: 30}, - {Path: "/hot-json", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Format: "json", Memory: 128, Timeout: 4, IdleTimeout: 30}, - {Path: "/bigmem-cold", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Memory: hugeMem, Timeout: 1, IdleTimeout: 30}, - {Path: "/bigmem-hot", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: hugeMem, Timeout: 1, IdleTimeout: 30}, + {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}, }, nil, ) @@ -654,12 +655,12 @@ func TestRouteRunnerTimeout(t *testing.T) { func TestRouteRunnerMinimalConcurrentHotSync(t *testing.T) { buf := setLogBuffer() + app := &models.App{Name: "myapp", Config: models.Config{}} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{ - {Name: "myapp", Config: models.Config{}}, - }, + []*models.App{app}, []*models.Route{ - {Path: "/hot", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: 128, Timeout: 30, IdleTimeout: 5}, + {Path: "/hot", AppID: app.ID, Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: 128, Timeout: 30, IdleTimeout: 5}, }, nil, ) diff --git a/api/server/server.go b/api/server/server.go index af6f0061b..674534b79 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -806,7 +806,7 @@ func (s *Server) startGears(ctx context.Context, cancel context.CancelFunc) { func (s *Server) bindHandlers(ctx context.Context) { engine := s.Router - // now for extendible middleware + // now for extensible middleware engine.Use(s.rootMiddlewareWrapper()) engine.GET("/", handlePing) @@ -822,7 +822,8 @@ func (s *Server) bindHandlers(ctx context.Context) { // Pure runners don't have any route, they have grpc if s.nodeType != ServerTypePureRunner { if s.nodeType != ServerTypeRunner { - v1 := engine.Group("/v1") + clean := engine.Group("/v1") + v1 := clean.Group("") v1.Use(setAppNameInCtx) v1.Use(s.apiMiddlewareWrapper()) v1.GET("/apps", s.handleAppList) @@ -832,39 +833,48 @@ func (s *Server) bindHandlers(ctx context.Context) { apps := v1.Group("/apps/:app") apps.Use(appNameCheck) - apps.GET("", s.handleAppGet) - apps.PATCH("", s.handleAppUpdate) - apps.DELETE("", s.handleAppDelete) + { + withAppCheck := apps.Group("") + withAppCheck.Use(s.checkAppPresenceByName()) + withAppCheck.GET("", s.handleAppGetByName) + withAppCheck.PATCH("", s.handleAppUpdate) + withAppCheck.DELETE("", s.handleAppDelete) + 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.handleCallGet) + withAppCheck.GET("/calls/:call/log", s.handleCallLogGet) + withAppCheck.GET("/calls", s.handleCallList) + } - apps.GET("/routes", s.handleRouteList) - apps.POST("/routes", s.handleRoutesPostPutPatch) - apps.GET("/routes/:route", s.handleRouteGet) - apps.PATCH("/routes/*route", s.handleRoutesPostPutPatch) - apps.PUT("/routes/*route", s.handleRoutesPostPutPatch) - apps.DELETE("/routes/*route", s.handleRouteDelete) - - apps.GET("/calls", s.handleCallList) - - apps.GET("/calls/:call", s.handleCallGet) - apps.GET("/calls/:call/log", s.handleCallLogGet) + apps.POST("/routes", s.handleRoutesPostPut) + apps.PUT("/routes/*route", s.handleRoutesPostPut) } { - runner := v1.Group("/runner") + runner := clean.Group("/runner") runner.PUT("/async", s.handleRunnerEnqueue) runner.GET("/async", s.handleRunnerDequeue) runner.POST("/start", s.handleRunnerStart) runner.POST("/finish", s.handleRunnerFinish) + + appsAPIV2 := runner.Group("/apps/:app") + appsAPIV2.Use(setAppNameInCtx) + appsAPIV2.GET("", s.handleAppGetByID) + appsAPIV2.GET("/routes/:route", s.handleRouteGetRunner) + } } if s.nodeType != ServerTypeAPI { runner := engine.Group("/r") - runner.Use(appNameCheck) + runner.Use(s.checkAppPresenceByNameAtRunner()) runner.Any("/:app", s.handleFunctionCall) runner.Any("/:app/*route", s.handleFunctionCall) } + } engine.NoRoute(func(c *gin.Context) { @@ -880,6 +890,7 @@ func (s *Server) bindHandlers(ctx context.Context) { } handleErrorResponse(c, err) }) + } // implements fnext.ExtServer diff --git a/api/server/server_test.go b/api/server/server_test.go index 98b38847d..a1d405d4d 100644 --- a/api/server/server_test.go +++ b/api/server/server_test.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "io" - "io/ioutil" "net/http" "net/http/httptest" "os" @@ -70,7 +69,7 @@ func routerRequest(t *testing.T, router *gin.Engine, method, path string, body i return routerRequest2(t, router, req) } -func routerRequest2(t *testing.T, router *gin.Engine, req *http.Request) (*http.Request, *httptest.ResponseRecorder) { +func routerRequest2(_ *testing.T, router *gin.Engine, req *http.Request) (*http.Request, *httptest.ResponseRecorder) { rec := httptest.NewRecorder() rec.Body = new(bytes.Buffer) router.ServeHTTP(rec, req) @@ -84,19 +83,13 @@ func newRouterRequest(t *testing.T, method, path string, body io.Reader) (*http. return req, rec } -func getErrorResponse(t *testing.T, rec *httptest.ResponseRecorder) models.Error { - respBody, err := ioutil.ReadAll(rec.Body) - if err != nil { +func getErrorResponse(t *testing.T, rec *httptest.ResponseRecorder) *models.Error { + var err models.Error + decodeErr := json.NewDecoder(rec.Body).Decode(&err) + if decodeErr != nil { t.Error("Test: Expected not empty response body") } - - var errResp models.Error - err = json.Unmarshal(respBody, &errResp) - if err != nil { - t.Error("Test: Expected response body to be a valid models.Error object") - } - - return errResp + return &err } func prepareDB(ctx context.Context, t *testing.T) (models.Datastore, models.LogStore, func()) { @@ -152,6 +145,7 @@ func TestFullStack(t *testing.T) { 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) } @@ -265,13 +259,13 @@ func TestApiNode(t *testing.T) { func TestHybridEndpoints(t *testing.T) { buf := setLogBuffer() + app := &models.App{Name: "myapp"} + app.SetDefaults() ds := datastore.NewMockInit( - []*models.App{{ - Name: "myapp", - }}, + []*models.App{app}, []*models.Route{{ - AppName: "myapp", - Path: "yodawg", + AppID: app.ID, + Path: "yodawg", }}, nil, ) @@ -281,9 +275,9 @@ func TestHybridEndpoints(t *testing.T) { newCallBody := func() string { call := &models.Call{ - ID: id.New().String(), - AppName: "myapp", - Path: "yodawg", + AppID: app.ID, + ID: id.New().String(), + Path: "yodawg", // TODO ? } var b bytes.Buffer diff --git a/docs/swagger.yml b/docs/swagger.yml index d88c3d42a..b158773e8 100644 --- a/docs/swagger.yml +++ b/docs/swagger.yml @@ -470,6 +470,9 @@ 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 @@ -538,6 +541,10 @@ definitions: 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 _." @@ -675,9 +682,9 @@ definitions: type: string description: Call execution error, if status is 'error'. readOnly: true - app_name: + app_id: type: string - description: App name that is assigned to a route that is being executed. + description: App ID that is assigned to a route that is being executed. readOnly: true path: type: string diff --git a/fnext/datastore.go b/fnext/datastore.go index cb57a56ee..4bcac400c 100644 --- a/fnext/datastore.go +++ b/fnext/datastore.go @@ -20,13 +20,13 @@ type extds struct { rl RouteListener } -func (e *extds) GetApp(ctx context.Context, appName string) (*models.App, error) { - err := e.al.BeforeAppGet(ctx, appName) +func (e *extds) GetAppByID(ctx context.Context, appID string) (*models.App, error) { + err := e.al.BeforeAppGet(ctx, appID) if err != nil { return nil, err } - app, err := e.Datastore.GetApp(ctx, appName) + app, err := e.Datastore.GetAppByID(ctx, appID) if err != nil { return nil, err } diff --git a/fnext/listeners.go b/fnext/listeners.go index 0d53d50b0..fd3b3f358 100644 --- a/fnext/listeners.go +++ b/fnext/listeners.go @@ -21,7 +21,7 @@ type AppListener interface { // AfterAppDelete called after deleting App in the database AfterAppDelete(ctx context.Context, app *models.App) error // BeforeAppGet called right before getting an app - BeforeAppGet(ctx context.Context, appName string) error + BeforeAppGet(ctx context.Context, appID string) error // AfterAppGet called after getting app from database AfterAppGet(ctx context.Context, app *models.App) error // BeforeAppsList called right before getting a list of all user's apps. Modify the filter to adjust what gets returned. diff --git a/test/fn-api-tests/exec_test.go b/test/fn-api-tests/exec_test.go index 671d9b537..ef7344001 100644 --- a/test/fn-api-tests/exec_test.go +++ b/test/fn-api-tests/exec_test.go @@ -183,9 +183,6 @@ func TestCanGetAsyncState(t *testing.T) { } callObject := callResponse.Payload.Call - if callObject.AppName != s.AppName { - t.Errorf("Call object app name mismatch.\n\tExpected: %v\n\tActual:%v", s.AppName, callObject.AppName) - } if callObject.ID != callID { t.Errorf("Call object ID mismatch.\n\tExpected: %v\n\tActual:%v", callID, callObject.ID) }