From d11bafb868989d9ec8bda29158e22caa788535b3 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Tue, 15 Aug 2017 20:39:21 +0300 Subject: [PATCH] Fix route update procedure API impact: We need to drop default value for type because it brought this type of bugs. Starting this patch users should specify route type through CLI or func.yml Closes: #222 --- api/server/routes_create_update.go | 4 +++- api/server/routes_test.go | 34 +++++++++++++++--------------- api/server/server_test.go | 4 ++-- test/fn-api-tests/routes_test.go | 12 +++++++++++ 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go index c13898031..ea163e6a6 100644 --- a/api/server/routes_create_update.go +++ b/api/server/routes_create_update.go @@ -110,7 +110,9 @@ func (s *Server) bindAndValidate(c *gin.Context, method string, wroute *models.R wroute.Route.Path = p } - wroute.Route.SetDefaults() + if method != http.MethodPatch { + wroute.Route.SetDefaults() + } return wroute.Validate(method == http.MethodPatch) } diff --git a/api/server/routes_test.go b/api/server/routes_test.go index e448cdec1..e1cba37ea 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -57,13 +57,13 @@ func TestRouteCreate(t *testing.T) { 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", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {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.ErrRoutesValidationMissingPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, {datastore.NewMockInit(nil, []*models.Route{ { @@ -71,10 +71,10 @@ func TestRouteCreate(t *testing.T) { Path: "/myroute", }, }, nil, nil, - ), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusConflict, models.ErrRoutesAlreadyExists}, + ), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesAlreadyExists}, // success - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusOK, nil}, } { test.run(t, i, buf) } @@ -86,18 +86,18 @@ func TestRoutePut(t *testing.T) { 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" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "diffRoute" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, + {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.ErrRoutesValidationMissingImage}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "diffRoute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidType}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "format": "invalid-format", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat}, // success - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello" } }`, http.StatusOK, nil}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusOK, nil}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "type": "sync" } }`, http.StatusOK, nil}, } { test.run(t, i, buf) test.ds = datastore.NewMockInit(nil, diff --git a/api/server/server_test.go b/api/server/server_test.go index 3b95e7fd9..2e962af3c 100644 --- a/api/server/server_test.go +++ b/api/server/server_test.go @@ -116,8 +116,8 @@ func TestFullStack(t *testing.T) { {"list apps", "GET", "/v1/apps", ``, http.StatusOK, 0}, {"get app", "GET", "/v1/apps/myapp", ``, http.StatusOK, 0}, // NOTE: cache is lazy, loads when a request comes in for the route, not when added - {"add myroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "funcy/hello" } }`, http.StatusOK, 0}, - {"add myroute2", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute2", "path": "/myroute2", "image": "funcy/error" } }`, http.StatusOK, 0}, + {"add myroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "funcy/hello", "type": "sync" } }`, http.StatusOK, 0}, + {"add myroute2", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute2", "path": "/myroute2", "image": "funcy/error", "type": "sync" } }`, http.StatusOK, 0}, {"get myroute", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusOK, 0}, {"get myroute2", "GET", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 0}, {"get all routes", "GET", "/v1/apps/myapp/routes", ``, http.StatusOK, 0}, diff --git a/test/fn-api-tests/routes_test.go b/test/fn-api-tests/routes_test.go index 19b5b1446..527c7c403 100644 --- a/test/fn-api-tests/routes_test.go +++ b/test/fn-api-tests/routes_test.go @@ -12,6 +12,18 @@ func TestRoutes(t *testing.T) { newRouteType := "sync" newRoutePath := id.New().String() + t.Run("create-route-with-empty-type", func(t *testing.T) { + t.Parallel() + s := SetupDefaultSuite() + CreateApp(t, s.Context, s.Client, s.AppName, map[string]string{}) + _, err := createRoute(s.Context, s.Client, s.AppName, s.RoutePath, s.Image, "", + s.RouteConfig, s.RouteHeaders) + if err == nil { + t.Errorf("Should fail with Invalid route Type.") + } + DeleteApp(t, s.Context, s.Client, s.AppName) + }) + t.Run("create-route", func(t *testing.T) { t.Parallel() s := SetupDefaultSuite()