From caba9e0ec63476e2330138c98bb9c64aa2b2dcd8 Mon Sep 17 00:00:00 2001 From: Reed Allman Date: Wed, 20 Sep 2017 23:10:15 -0700 Subject: [PATCH] more strict configuration of routes * idle_timeout max of 1h * timeout max of 120s for sync, 1h for async * max memory of 8GB * do full route validation before call invocation * ensure that idle_timeout >= timeout we are now doing validation of updating route inside of the database transaction, which is what we should have been doing all along really. we need this behavior to ensure that the idle timeout is longer than the timeout, among other benefits (like not updating the most recent version of the existing struct and overwriting previous updates, yay). since we have this, we can get rid of the weird skipZero behavior on validate too and validate the real deal holyfield. validating the route before making the call is handy so that we don't do weird things like run a func that wants to use 300GB of RAM and run for 3 weeks. closes #192 closes #344 closes #162 --- .circleci/config.yml | 5 +- Makefile | 3 + api/agent/call.go | 15 ++-- api/datastore/internal/datastoretest/test.go | 43 +++++++----- api/datastore/mock.go | 10 ++- api/datastore/sql/sql.go | 4 ++ api/models/app.go | 6 +- api/models/error.go | 44 +++++++----- api/models/route.go | 73 +++++++++++--------- api/models/route_wrapper.go | 10 +-- api/server/apps_test.go | 8 +-- api/server/calls_test.go | 4 +- api/server/routes_create_update.go | 14 +--- api/server/routes_test.go | 63 ++++++++--------- api/server/runner_async_test.go | 6 +- api/server/runner_test.go | 18 ++--- api/server/server.go | 2 +- test.sh | 3 - 18 files changed, 175 insertions(+), 156 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a06d2266e..558d4e05e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -32,7 +32,10 @@ jobs: - run: docker version # login here for tests - run: docker login -u $DOCKER_USER -p $DOCKER_PASS - - run: ./test.sh + - run: make docker-build + - run: make install + - run: make test + # TODO these should be inside test.sh file ? - run: ./api_test.sh mysql 4 - run: ./api_test.sh postgres 4 - run: ./api_test.sh sqlite 4 diff --git a/Makefile b/Makefile index 97145f484..5ae5448dd 100644 --- a/Makefile +++ b/Makefile @@ -10,6 +10,9 @@ dep-up: build: go build -o functions +install: + go install + test: ./test.sh diff --git a/api/agent/call.go b/api/agent/call.go index 14821e813..07a69e8d9 100644 --- a/api/agent/call.go +++ b/api/agent/call.go @@ -146,6 +146,13 @@ func FromRequest(appName, path string, req *http.Request) CallOpt { } } + // this ensures that there is an image, path, timeouts, memory, etc are valid. + // NOTE: this means assign any changes above into route's fields + err = route.Validate() + if err != nil { + return err + } + c.Call = &models.Call{ ID: id, AppName: appName, @@ -166,14 +173,6 @@ func FromRequest(appName, path string, req *http.Request) CallOpt { Method: req.Method, } - // TODO if these made it to here we have a problemo. error instead? - if c.Timeout <= 0 { - c.Timeout = models.DefaultRouteTimeout - } - if c.IdleTimeout <= 0 { - c.IdleTimeout = models.DefaultIdleTimeout - } - c.req = req return nil } diff --git a/api/datastore/internal/datastoretest/test.go b/api/datastore/internal/datastoretest/test.go index 0868b18ff..271dc88d8 100644 --- a/api/datastore/internal/datastoretest/test.go +++ b/api/datastore/internal/datastoretest/test.go @@ -449,7 +449,7 @@ func Test(t *testing.T, dsf func() models.Datastore) { updated, err := ds.UpdateRoute(ctx, &models.Route{ AppName: testRoute.AppName, Path: testRoute.Path, - Timeout: 100, + Timeout: 2, Config: map[string]string{ "FIRST": "1", "SECOND": "2", @@ -467,13 +467,15 @@ func Test(t *testing.T, dsf func() models.Datastore) { } expected := &models.Route{ // unchanged - AppName: testRoute.AppName, - Path: testRoute.Path, - Image: "fnproject/hello", - Type: "sync", - Format: "http", + AppName: testRoute.AppName, + Path: testRoute.Path, + Image: "fnproject/hello", + Type: "sync", + Format: "http", + IdleTimeout: testRoute.IdleTimeout, + Memory: testRoute.Memory, // updated - Timeout: 100, + Timeout: 2, Config: map[string]string{ "FIRST": "1", "SECOND": "2", @@ -510,12 +512,14 @@ func Test(t *testing.T, dsf func() models.Datastore) { } expected = &models.Route{ // unchanged - AppName: testRoute.AppName, - Path: testRoute.Path, - Image: "fnproject/hello", - Type: "sync", - Format: "http", - Timeout: 100, + AppName: testRoute.AppName, + Path: testRoute.Path, + Image: "fnproject/hello", + Type: "sync", + Format: "http", + Timeout: 2, + Memory: testRoute.Memory, + IdleTimeout: testRoute.IdleTimeout, // updated Config: map[string]string{ "FIRST": "first", @@ -682,9 +686,12 @@ var testApp = &models.App{ } var testRoute = &models.Route{ - AppName: testApp.Name, - Path: "/test", - Image: "fnproject/hello", - Type: "sync", - Format: "http", + AppName: testApp.Name, + Path: "/test", + Image: "fnproject/hello", + Type: "sync", + Format: "http", + Timeout: models.DefaultTimeout, + IdleTimeout: models.DefaultIdleTimeout, + Memory: models.DefaultMemory, } diff --git a/api/datastore/mock.go b/api/datastore/mock.go index 420338a57..0add9108a 100644 --- a/api/datastore/mock.go +++ b/api/datastore/mock.go @@ -144,8 +144,14 @@ func (m *mock) UpdateRoute(ctx context.Context, route *models.Route) (*models.Ro if err != nil { return nil, err } - r.Update(route) - return r.Clone(), nil + clone := r.Clone() + clone.Update(route) + err = clone.Validate() + if err != nil { + return nil, err + } + r.Update(route) // only if validate works (pointer) + return clone, nil } func (m *mock) RemoveRoute(ctx context.Context, appName, routePath string) error { diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index b1d6bae51..434177514 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -335,6 +335,10 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m } route.Update(newroute) + err = route.Validate() + if err != nil { + return err + } query = tx.Rebind(`UPDATE routes SET image = :image, diff --git a/api/models/app.go b/api/models/app.go index 7f3adcf2c..5ae81eefb 100644 --- a/api/models/app.go +++ b/api/models/app.go @@ -7,14 +7,14 @@ type App struct { func (a *App) Validate() error { if a.Name == "" { - return ErrAppsValidationMissingName + return ErrAppsMissingName } if len(a.Name) > maxAppName { - return ErrAppsValidationTooLongName + return ErrAppsTooLongName } for _, c := range a.Name { if (c < '0' || '9' < c) && (c < 'A' || 'Z' > c) && (c < 'a' || 'z' < c) && c != '_' && c != '-' { - return ErrAppsValidationInvalidName + return ErrAppsInvalidName } } return nil diff --git a/api/models/error.go b/api/models/error.go index 6cb64d27c..504cdb70d 100644 --- a/api/models/error.go +++ b/api/models/error.go @@ -20,15 +20,15 @@ var ( code: http.StatusGatewayTimeout, error: errors.New("Timed out"), } - ErrAppsValidationMissingName = err{ + ErrAppsMissingName = err{ code: http.StatusBadRequest, error: errors.New("Missing app name"), } - ErrAppsValidationTooLongName = err{ + ErrAppsTooLongName = err{ code: http.StatusBadRequest, error: fmt.Errorf("App name must be %v characters or less", maxAppName), } - ErrAppsValidationInvalidName = err{ + ErrAppsInvalidName = err{ code: http.StatusBadRequest, error: errors.New("Invalid app name"), } @@ -42,7 +42,7 @@ var ( } ErrAppsNameImmutable = err{ code: http.StatusConflict, - error: errors.New("Could not update app - name is immutable"), + error: errors.New("Could not update - name is immutable"), } ErrAppsNotFound = err{ code: http.StatusNotFound, @@ -94,41 +94,41 @@ var ( } ErrRoutesPathImmutable = err{ code: http.StatusConflict, - error: errors.New("Could not update route - path is immutable"), + error: errors.New("Could not update - path is immutable"), } ErrFoundDynamicURL = err{ code: http.StatusBadRequest, error: errors.New("Dynamic URL is not allowed"), } - ErrInvalidPath = err{ + ErrRoutesInvalidPath = err{ code: http.StatusBadRequest, - error: errors.New("Invalid Path format"), + error: errors.New("Invalid route path format"), } - ErrInvalidType = err{ + ErrRoutesInvalidType = err{ code: http.StatusBadRequest, error: errors.New("Invalid route Type"), } - ErrInvalidFormat = err{ + ErrRoutesInvalidFormat = err{ code: http.StatusBadRequest, error: errors.New("Invalid route Format"), } - ErrMissingAppName = err{ + ErrRoutesMissingAppName = err{ code: http.StatusBadRequest, error: errors.New("Missing route AppName"), } - ErrMissingImage = err{ + ErrRoutesMissingImage = err{ code: http.StatusBadRequest, error: errors.New("Missing route Image"), } - ErrMissingName = err{ + ErrRoutesMissingName = err{ code: http.StatusBadRequest, error: errors.New("Missing route Name"), } - ErrMissingPath = err{ + ErrRoutesMissingPath = err{ code: http.StatusBadRequest, error: errors.New("Missing route Path"), } - ErrMissingType = err{ + ErrRoutesMissingType = err{ code: http.StatusBadRequest, error: errors.New("Missing route Type"), } @@ -144,13 +144,21 @@ var ( code: http.StatusBadRequest, error: errors.New("from_time is not an epoch time"), } - ErrNegativeTimeout = err{ + ErrRoutesInvalidTimeout = err{ code: http.StatusBadRequest, - error: errors.New("Negative timeout"), + error: fmt.Errorf("timeout value is too large or small. 0 < timeout < max. async max: %d sync max: %d", MaxAsyncTimeout, MaxSyncTimeout), } - ErrNegativeIdleTimeout = err{ + ErrRoutesInvalidIdleTimeout = err{ code: http.StatusBadRequest, - error: errors.New("Negative idle timeout"), + error: fmt.Errorf("idle_timeout value is too large or small. 0 < timeout < %d", MaxIdleTimeout), + } + ErrRoutesInvalidMemory = err{ + code: http.StatusBadRequest, + error: fmt.Errorf("memory value is invalid. 0 < memory < %d", MaxMemory), + } + ErrRoutesTimeoutLongerThanIdle = err{ + code: http.StatusBadRequest, + error: errors.New("timeout must be less than idle_timeout"), } ErrCallNotFound = err{ code: http.StatusNotFound, diff --git a/api/models/route.go b/api/models/route.go index 8b6cd10e1..5633fd07a 100644 --- a/api/models/route.go +++ b/api/models/route.go @@ -8,8 +8,14 @@ import ( ) const ( - DefaultRouteTimeout = 30 // seconds - DefaultIdleTimeout = 30 // seconds + DefaultTimeout = 30 // seconds + DefaultIdleTimeout = 30 // seconds + DefaultMemory = 128 // MB + + MaxSyncTimeout = 120 // 2 minutes + MaxAsyncTimeout = 3600 // 1 hour + MaxIdleTimeout = MaxAsyncTimeout + MaxMemory = 1024 * 8 // 8GB TODO should probably be a var of machine max? ) type Routes []*Route @@ -30,7 +36,7 @@ type Route struct { // SetDefaults sets zeroed field to defaults. func (r *Route) SetDefaults() { if r.Memory == 0 { - r.Memory = 128 + r.Memory = DefaultMemory } if r.Type == TypeNone { @@ -50,7 +56,7 @@ func (r *Route) SetDefaults() { } if r.Timeout == 0 { - r.Timeout = DefaultRouteTimeout + r.Timeout = DefaultTimeout } if r.IdleTimeout == 0 { @@ -58,24 +64,15 @@ func (r *Route) SetDefaults() { } } -// Validate validates field values, skipping zeroed fields if skipZero is true. -// it returns the first error, if any. -func (r *Route) Validate(skipZero bool) error { - if !skipZero { - if r.AppName == "" { - return ErrMissingAppName - } - - if r.Path == "" { - return ErrMissingPath - } - - if r.Image == "" { - return ErrMissingImage - } +// Validate validates all field values, returning the first error, if any. +func (r *Route) Validate() error { + if r.AppName == "" { + return ErrRoutesMissingAppName } - if !skipZero || r.Path != "" { + if r.Path == "" { + return ErrRoutesMissingPath + } else { u, err := url.Parse(r.Path) if err != nil { return ErrPathMalformed @@ -86,28 +83,38 @@ func (r *Route) Validate(skipZero bool) error { } if !path.IsAbs(u.Path) { - return ErrInvalidPath + return ErrRoutesInvalidPath } } - if !skipZero || r.Type != "" { - if r.Type != TypeAsync && r.Type != TypeSync { - return ErrInvalidType - } + if r.Image == "" { + return ErrRoutesMissingImage } - if !skipZero || r.Format != "" { - if r.Format != FormatDefault && r.Format != FormatHTTP { - return ErrInvalidFormat - } + if r.Type != TypeAsync && r.Type != TypeSync { + return ErrRoutesInvalidType } - if r.Timeout < 0 { - return ErrNegativeTimeout + if r.Format != FormatDefault && r.Format != FormatHTTP { + return ErrRoutesInvalidFormat } - if r.IdleTimeout < 0 { - return ErrNegativeIdleTimeout + if r.Timeout <= 0 || + (r.Type == TypeSync && r.Timeout > MaxSyncTimeout) || + (r.Type == TypeAsync && r.Timeout > MaxAsyncTimeout) { + return ErrRoutesInvalidTimeout + } + + if r.IdleTimeout <= 0 || r.IdleTimeout > MaxIdleTimeout { + return ErrRoutesInvalidIdleTimeout + } + + if r.Timeout > r.IdleTimeout { + return ErrRoutesTimeoutLongerThanIdle + } + + if r.Memory < 1 || r.Memory > MaxMemory { + return ErrRoutesInvalidMemory } return nil diff --git a/api/models/route_wrapper.go b/api/models/route_wrapper.go index dad82dab0..307e5e262 100644 --- a/api/models/route_wrapper.go +++ b/api/models/route_wrapper.go @@ -4,15 +4,9 @@ type RouteWrapper struct { Route *Route `json:"route"` } -func (m *RouteWrapper) Validate(skipZero bool) error { return m.validateRoute(skipZero) } - -func (m *RouteWrapper) validateRoute(skipZero bool) error { - +func (m *RouteWrapper) Validate() error { if m.Route != nil { - if err := m.Route.Validate(skipZero); err != nil { - return err - } + return m.Route.Validate() } - return nil } diff --git a/api/server/apps_test.go b/api/server/apps_test.go index a8b6834ad..42cd0a530 100644 --- a/api/server/apps_test.go +++ b/api/server/apps_test.go @@ -41,10 +41,10 @@ func TestAppCreate(t *testing.T) { {datastore.NewMock(), logs.NewMock(), "/v1/apps", ``, http.StatusBadRequest, models.ErrInvalidJSON}, {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{}`, http.StatusBadRequest, models.ErrAppsMissingNew}, {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "name": "Test" }`, http.StatusBadRequest, models.ErrAppsMissingNew}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "" } }`, http.StatusBadRequest, models.ErrAppsValidationMissingName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "1234567890123456789012345678901" } }`, http.StatusBadRequest, models.ErrAppsValidationTooLongName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "" } }`, http.StatusBadRequest, models.ErrAppsMissingName}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "1234567890123456789012345678901" } }`, http.StatusBadRequest, models.ErrAppsTooLongName}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, // success {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "teste" } }`, http.StatusOK, nil}, diff --git a/api/server/calls_test.go b/api/server/calls_test.go index 53228e1fb..f45a022f1 100644 --- a/api/server/calls_test.go +++ b/api/server/calls_test.go @@ -57,7 +57,7 @@ func TestCallGet(t *testing.T) { expectedCode int expectedError error }{ - {"/v1/apps//calls/" + call.ID, "", http.StatusBadRequest, models.ErrMissingAppName}, + {"/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/myapp/calls/" + call.ID[:3], "", http.StatusNotFound, models.ErrCallNotFound}, {"/v1/apps/myapp/calls/" + call.ID, "", http.StatusOK, nil}, @@ -140,7 +140,7 @@ func TestCallList(t *testing.T) { expectedLen int nextCursor string }{ - {"/v1/apps//calls", "", http.StatusBadRequest, models.ErrMissingAppName, 0, ""}, + {"/v1/apps//calls", "", http.StatusBadRequest, models.ErrAppsMissingName, 0, ""}, {"/v1/apps/nodawg/calls", "", http.StatusNotFound, models.ErrAppsNotFound, 0, ""}, {"/v1/apps/myapp/calls", "", http.StatusOK, nil, 3, ""}, {"/v1/apps/myapp/calls?per_page=1", "", http.StatusOK, nil, 1, c3.ID}, diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go index 5e65b4dd9..b09e596dd 100644 --- a/api/server/routes_create_update.go +++ b/api/server/routes_create_update.go @@ -49,7 +49,8 @@ func (s *Server) handleRoutesPostPutPatch(c *gin.Context) { } func (s *Server) submitRoute(ctx context.Context, wroute *models.RouteWrapper) error { - err := s.setDefaultsAndValidate(wroute) + wroute.Route.SetDefaults() + err := wroute.Route.Validate() if err != nil { return err } @@ -62,10 +63,6 @@ func (s *Server) submitRoute(ctx context.Context, wroute *models.RouteWrapper) e } func (s *Server) changeRoute(ctx context.Context, wroute *models.RouteWrapper) error { - err := wroute.Validate(true) - if err != nil { - return err - } r, err := s.Datastore.UpdateRoute(ctx, wroute.Route) if err != nil { return err @@ -163,13 +160,8 @@ func bindRoute(c *gin.Context, method string, wroute *models.RouteWrapper) error } if method == http.MethodPost { if wroute.Route.Path == "" { - return models.ErrMissingPath + return models.ErrRoutesMissingPath } } return nil } - -func (s *Server) setDefaultsAndValidate(wroute *models.RouteWrapper) error { - wroute.Route.SetDefaults() - return wroute.Validate(false) -} diff --git a/api/server/routes_test.go b/api/server/routes_test.go index 4964fd18e..414980571 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -61,11 +61,11 @@ func TestRouteCreate(t *testing.T) { {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.ErrMissingPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/hello", "type": "sync" } }`, http.StatusBadRequest, models.ErrMissingPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/hello", "path": "myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrInvalidPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "fnproject/hello", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, + {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/hello", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesMissingPath}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "fnproject/hello", "path": "myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesInvalidPath}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "fnproject/hello", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, {datastore.NewMockInit(nil, []*models.Route{ { @@ -89,13 +89,13 @@ func TestRoutePut(t *testing.T) { // 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.ErrMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrMissingImage}, + {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/hello", "path": "myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/hello", "path": "diffRoute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "fnproject/hello", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/hello", "path": "/myroute", "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrInvalidType}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/hello", "path": "/myroute", "format": "invalid-format", "type": "sync" } }`, http.StatusBadRequest, models.ErrInvalidFormat}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "fnproject/hello", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsInvalidName}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/hello", "path": "/myroute", "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesInvalidType}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "fnproject/hello", "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/hello", "path": "/myroute", "type": "sync" } }`, http.StatusOK, nil}, @@ -189,7 +189,7 @@ func TestRouteList(t *testing.T) { expectedLen int nextCursor string }{ - {"/v1/apps//routes", "", http.StatusBadRequest, models.ErrMissingAppName, 0, ""}, + {"/v1/apps//routes", "", http.StatusBadRequest, models.ErrAppsMissingName, 0, ""}, {"/v1/apps/a/routes", "", http.StatusNotFound, models.ErrAppsNotFound, 0, ""}, {"/v1/apps/myapp/routes", "", http.StatusOK, nil, 3, ""}, {"/v1/apps/myapp/routes?per_page=1", "", http.StatusOK, nil, 1, r1b}, @@ -274,33 +274,30 @@ func TestRouteGet(t *testing.T) { func TestRouteUpdate(t *testing.T) { buf := setLogBuffer() + ds := datastore.NewMockInit(nil, nil, nil) for i, test := range []routeTestCase{ - // errors - {datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrInvalidType}, - {datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrInvalidFormat}, - // success - {datastore.NewMockInit(nil, - []*models.Route{ - { - AppName: "a", - Path: "/myroute/do", - }, - }, nil, - ), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "fnproject/hello" } }`, http.StatusOK, nil}, + {ds, logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "fnproject/yodawg" } }`, http.StatusOK, nil}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "fnproject/hello" } }`, http.StatusOK, nil}, + + // errors (after success, so route exists) + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesInvalidType}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesInvalidFormat}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "timeout": 121 } }`, http.StatusBadRequest, models.ErrRoutesInvalidTimeout}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "async", "timeout": 3601 } }`, http.StatusBadRequest, models.ErrRoutesInvalidTimeout}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "async", "timeout": 121, "idle_timeout": 240 } }`, http.StatusOK, nil}, // should work if async + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "idle_timeout": 3601 } }`, http.StatusBadRequest, models.ErrRoutesInvalidIdleTimeout}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "timeout": 241 } }`, http.StatusBadRequest, models.ErrRoutesTimeoutLongerThanIdle}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "memory": 100000000000000 } }`, http.StatusBadRequest, models.ErrRoutesInvalidMemory}, + // TODO this should be correct, waiting for patch to come in + //{ds, logs.NewMock(), http.MethodPatch, "/v1/apps/b/routes/myroute/dont", `{ "route": {} }`, http.StatusNotFound, models.ErrAppsNotFound}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/dont", `{ "route": {} }`, http.StatusNotFound, models.ErrRoutesNotFound}, // Addresses #381 - {datastore.NewMockInit(nil, - []*models.Route{ - { - AppName: "a", - Path: "/myroute/do", - }, - }, nil, - ), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, + {ds, logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, } { test.run(t, i, buf) } diff --git a/api/server/runner_async_test.go b/api/server/runner_async_test.go index 6fd908085..7da951ffd 100644 --- a/api/server/runner_async_test.go +++ b/api/server/runner_async_test.go @@ -39,9 +39,9 @@ func TestRouteRunnerAsyncExecution(t *testing.T) { {Name: "myapp", Config: map[string]string{"app": "true"}}, }, []*models.Route{ - {Type: "async", Path: "/myroute", AppName: "myapp", Image: "fnproject/hello", Config: map[string]string{"test": "true"}}, - {Type: "async", Path: "/myerror", AppName: "myapp", Image: "fnproject/error", Config: map[string]string{"test": "true"}}, - {Type: "async", Path: "/myroute/:param", AppName: "myapp", Image: "fnproject/hello", Config: map[string]string{"test": "true"}}, + {Type: "async", Path: "/myroute", AppName: "myapp", Image: "fnproject/hello", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, + {Type: "async", Path: "/myerror", AppName: "myapp", Image: "fnproject/error", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, + {Type: "async", Path: "/myroute/:param", AppName: "myapp", 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 3fcfcee64..a31819f97 100644 --- a/api/server/runner_test.go +++ b/api/server/runner_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "io/ioutil" "net/http" "strings" "testing" @@ -128,11 +129,11 @@ func TestRouteRunnerExecution(t *testing.T) { {Name: "myapp", Config: models.Config{}}, }, []*models.Route{ - {Path: "/", AppName: "myapp", Image: "fnproject/hello", Headers: map[string][]string{"X-Function": {"Test"}}}, - {Path: "/myroute", AppName: "myapp", Image: "fnproject/hello", Headers: map[string][]string{"X-Function": {"Test"}}}, - {Path: "/myerror", AppName: "myapp", Image: "fnproject/error", Headers: map[string][]string{"X-Function": {"Test"}}}, - {Path: "/mydne", AppName: "myapp", Image: "fnproject/imagethatdoesnotexist"}, - {Path: "/mydnehot", AppName: "myapp", Image: "fnproject/imagethatdoesnotexist", Format: "http"}, + {Path: "/", AppName: "myapp", Image: "fnproject/hello", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}}, + {Path: "/myroute", AppName: "myapp", Image: "fnproject/hello", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}}, + {Path: "/myerror", AppName: "myapp", Image: "fnproject/error", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30, Headers: map[string][]string{"X-Function": {"Test"}}}, + {Path: "/mydne", AppName: "myapp", Image: "fnproject/imagethatdoesnotexist", Type: "sync", Memory: 128, Timeout: 30, IdleTimeout: 30}, + {Path: "/mydnehot", AppName: "myapp", Image: "fnproject/imagethatdoesnotexist", Type: "sync", Format: "http", Memory: 128, Timeout: 30, IdleTimeout: 30}, }, nil, ) @@ -165,8 +166,9 @@ func TestRouteRunnerExecution(t *testing.T) { if rec.Code != test.expectedCode { t.Log(buf.String()) - t.Errorf("Test %d: Expected status code to be %d but was %d", - i, test.expectedCode, rec.Code) + bod, _ := ioutil.ReadAll(rec.Body) + t.Errorf("Test %d: Expected status code to be %d but was %d. body: %s", + i, test.expectedCode, rec.Code, string(bod)) } if test.expectedHeaders == nil { @@ -200,7 +202,7 @@ func TestFailedEnqueue(t *testing.T) { {Name: "myapp", Config: models.Config{}}, }, []*models.Route{ - {Path: "/dummy", AppName: "myapp", Image: "dummy/dummy", Type: "async"}, + {Path: "/dummy", AppName: "myapp", Image: "dummy/dummy", Type: "async", Memory: 128, Timeout: 30, IdleTimeout: 30}, }, nil, ) err := errors.New("Unable to push task to queue") diff --git a/api/server/server.go b/api/server/server.go index 395364c91..dc15185f3 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -215,7 +215,7 @@ func loggerWrap(c *gin.Context) { func appWrap(c *gin.Context) { appName := c.GetString(api.AppName) if appName == "" { - handleErrorResponse(c, models.ErrMissingAppName) + handleErrorResponse(c, models.ErrAppsMissingName) c.Abort() return } diff --git a/test.sh b/test.sh index fe63f6e51..c4e598699 100755 --- a/test.sh +++ b/test.sh @@ -2,9 +2,6 @@ set -ex -make build -make docker-build - docker rm -fv func-postgres-test || echo No prev test db container docker run --name func-postgres-test -p 15432:5432 -d postgres docker rm -fv func-mysql-test || echo No prev mysql test db container