diff --git a/api/server/apps_create.go b/api/server/apps_create.go index 24034531b..6b40d128c 100644 --- a/api/server/apps_create.go +++ b/api/server/apps_create.go @@ -36,24 +36,28 @@ func (s *Server) handleAppCreate(c *gin.Context) { err = s.FireBeforeAppCreate(ctx, wapp.App) if err != nil { - log.WithError(err).Errorln(models.ErrAppsCreate) + log.WithError(err).Error(models.ErrAppsCreate) c.JSON(http.StatusInternalServerError, simpleError(err)) return } app, err := s.Datastore.InsertApp(ctx, wapp.App) - if err != nil { - log.WithError(err).Errorln(models.ErrAppsCreate) - c.JSON(http.StatusInternalServerError, simpleError(err)) + if err == models.ErrAppsAlreadyExists { + log.WithError(err).Debug(models.ErrAppsCreate) + c.JSON(http.StatusConflict, simpleError(err)) + return + } else if err != nil { + log.WithError(err).Error(models.ErrAppsCreate) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } err = s.FireAfterAppCreate(ctx, wapp.App) if err != nil { - log.WithError(err).Errorln(models.ErrAppsCreate) + log.WithError(err).Error(models.ErrAppsCreate) c.JSON(http.StatusInternalServerError, simpleError(err)) return } - c.JSON(http.StatusCreated, appResponse{"App successfully created", app}) + c.JSON(http.StatusOK, appResponse{"App successfully created", app}) } diff --git a/api/server/apps_delete.go b/api/server/apps_delete.go index c25d033dc..5061ec523 100644 --- a/api/server/apps_delete.go +++ b/api/server/apps_delete.go @@ -17,8 +17,8 @@ func (s *Server) handleAppDelete(c *gin.Context) { routes, err := s.Datastore.GetRoutesByApp(ctx, app.Name, &models.RouteFilter{}) if err != nil { - log.WithError(err).Debug(models.ErrAppsRemoving) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsRemoving)) + log.WithError(err).Error(models.ErrAppsRemoving) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } @@ -30,25 +30,26 @@ func (s *Server) handleAppDelete(c *gin.Context) { err = s.FireBeforeAppDelete(ctx, app) if err != nil { - log.WithError(err).Errorln(models.ErrAppsRemoving) - c.JSON(http.StatusInternalServerError, simpleError(err)) + log.WithError(err).Error(models.ErrAppsRemoving) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } - if err = s.Datastore.RemoveApp(ctx, app.Name); err != nil { + err = s.Datastore.RemoveApp(ctx, app.Name) + if err == models.ErrAppsNotFound { log.WithError(err).Debug(models.ErrAppsRemoving) - if err == models.ErrAppsNotFound { - c.JSON(http.StatusNotFound, simpleError(models.ErrAppsNotFound)) - } else { - c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsRemoving)) - } + c.JSON(http.StatusNotFound, simpleError(err)) + return + } else if err != nil { + log.WithError(err).Error(models.ErrAppsRemoving) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } err = s.FireAfterAppDelete(ctx, app) if err != nil { - log.WithError(err).Errorln(models.ErrAppsRemoving) - c.JSON(http.StatusInternalServerError, simpleError(err)) + log.WithError(err).Error(models.ErrAppsRemoving) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } diff --git a/api/server/apps_list.go b/api/server/apps_list.go index 0845ada8c..46a7d839a 100644 --- a/api/server/apps_list.go +++ b/api/server/apps_list.go @@ -17,8 +17,8 @@ func (s *Server) handleAppList(c *gin.Context) { apps, err := s.Datastore.GetApps(ctx, filter) if err != nil { - log.WithError(err).Debug(models.ErrAppsList) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsList)) + log.WithError(err).Error(models.ErrAppsList) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } diff --git a/api/server/apps_test.go b/api/server/apps_test.go index d542aba1d..1d71952aa 100644 --- a/api/server/apps_test.go +++ b/api/server/apps_test.go @@ -55,7 +55,7 @@ func TestAppCreate(t *testing.T) { {&datastore.Mock{}, "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, // success - {&datastore.Mock{}, "/v1/apps", `{ "app": { "name": "teste" } }`, http.StatusCreated, nil}, + {&datastore.Mock{}, "/v1/apps", `{ "app": { "name": "teste" } }`, http.StatusOK, nil}, } { rnr, cancel := testRunner(t) router := testRouter(test.mock, &mqs.Mock{}, rnr, tasks) @@ -226,7 +226,7 @@ func TestAppUpdate(t *testing.T) { Apps: []*models.App{{ Name: "myapp", }}, - }, "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusForbidden, nil}, + }, "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusBadRequest, nil}, } { rnr, cancel := testRunner(t) router := testRouter(test.mock, &mqs.Mock{}, rnr, tasks) diff --git a/api/server/apps_update.go b/api/server/apps_update.go index 53c837420..670043a23 100644 --- a/api/server/apps_update.go +++ b/api/server/apps_update.go @@ -30,7 +30,7 @@ func (s *Server) handleAppUpdate(c *gin.Context) { if wapp.App.Name != "" { log.Debug(models.ErrAppsNameImmutable) - c.JSON(http.StatusForbidden, simpleError(models.ErrAppsNameImmutable)) + c.JSON(http.StatusBadRequest, simpleError(models.ErrAppsNameImmutable)) return } @@ -38,25 +38,28 @@ func (s *Server) handleAppUpdate(c *gin.Context) { err = s.FireAfterAppUpdate(ctx, wapp.App) if err != nil { - log.WithError(err).Errorln(models.ErrAppsUpdate) - c.JSON(http.StatusInternalServerError, simpleError(err)) + log.WithError(err).Error(models.ErrAppsUpdate) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } app, err := s.Datastore.UpdateApp(ctx, wapp.App) - if err != nil { + if err == models.ErrAppsNotFound { log.WithError(err).Debug(models.ErrAppsUpdate) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsUpdate)) + c.JSON(http.StatusNotFound, simpleError(err)) + return + } else if err != nil { + log.WithError(err).Error(models.ErrAppsUpdate) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } err = s.FireAfterAppUpdate(ctx, wapp.App) if err != nil { - log.WithError(err).Errorln(models.ErrAppsUpdate) - c.JSON(http.StatusInternalServerError, simpleError(err)) + log.WithError(err).Error(models.ErrAppsUpdate) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } - // Nothing to update right now in apps c.JSON(http.StatusOK, appResponse{"App successfully updated", app}) } diff --git a/api/server/routes_create.go b/api/server/routes_create.go index 83c7d5f92..5ccd172e0 100644 --- a/api/server/routes_create.go +++ b/api/server/routes_create.go @@ -18,13 +18,13 @@ func (s *Server) handleRouteCreate(c *gin.Context) { err := c.BindJSON(&wroute) if err != nil { - log.WithError(err).Error(models.ErrInvalidJSON) + log.WithError(err).Debug(models.ErrInvalidJSON) c.JSON(http.StatusBadRequest, simpleError(models.ErrInvalidJSON)) return } if wroute.Route == nil { - log.WithError(err).Error(models.ErrInvalidJSON) + log.WithError(err).Debug(models.ErrInvalidJSON) c.JSON(http.StatusBadRequest, simpleError(models.ErrRoutesMissingNew)) return } @@ -32,20 +32,22 @@ func (s *Server) handleRouteCreate(c *gin.Context) { wroute.Route.AppName = ctx.Value("appName").(string) if err := wroute.Validate(); err != nil { - log.Error(err) - c.JSON(http.StatusInternalServerError, simpleError(err)) + log.WithError(err).Debug(models.ErrRoutesCreate) + c.JSON(http.StatusBadRequest, simpleError(err)) return } if wroute.Route.Image == "" { + log.WithError(models.ErrRoutesValidationMissingImage).Debug(models.ErrRoutesCreate) c.JSON(http.StatusBadRequest, simpleError(models.ErrRoutesValidationMissingImage)) return } + err = s.Runner.EnsureImageExists(ctx, &task.Config{ Image: wroute.Route.Image, }) if err != nil { - c.JSON(http.StatusInternalServerError, simpleError(models.ErrUsableImage)) + c.JSON(http.StatusBadRequest, simpleError(models.ErrUsableImage)) return } @@ -65,33 +67,37 @@ func (s *Server) handleRouteCreate(c *gin.Context) { err = s.FireBeforeAppCreate(ctx, newapp) if err != nil { - log.WithError(err).Errorln(models.ErrAppsCreate) - c.JSON(http.StatusInternalServerError, simpleError(err)) + log.WithError(err).Error(models.ErrAppsCreate) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } _, err = s.Datastore.InsertApp(ctx, newapp) if err != nil { - log.WithError(err).Error(models.ErrAppsCreate) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsCreate)) + log.WithError(err).Error(models.ErrRoutesCreate) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } err = s.FireAfterAppCreate(ctx, newapp) if err != nil { - log.WithError(err).Errorln(models.ErrAppsCreate) - c.JSON(http.StatusInternalServerError, simpleError(err)) + log.WithError(err).Error(models.ErrRoutesCreate) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } } route, err := s.Datastore.InsertRoute(ctx, wroute.Route) - if err != nil { + if err == models.ErrRoutesAlreadyExists { + log.WithError(err).Debug(models.ErrRoutesCreate) + c.JSON(http.StatusConflict, simpleError(models.ErrRoutesAlreadyExists)) + return + } else if err != nil { log.WithError(err).Error(models.ErrRoutesCreate) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesCreate)) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } - c.JSON(http.StatusCreated, routeResponse{"Route successfully created", route}) + c.JSON(http.StatusOK, routeResponse{"Route successfully created", route}) } diff --git a/api/server/routes_delete.go b/api/server/routes_delete.go index fc7ff9dfc..281ad3c16 100644 --- a/api/server/routes_delete.go +++ b/api/server/routes_delete.go @@ -17,18 +17,12 @@ func (s *Server) handleRouteDelete(c *gin.Context) { appName := ctx.Value("appName").(string) routePath := path.Clean(ctx.Value("routePath").(string)) - route, err := s.Datastore.GetRoute(ctx, appName, routePath) - if err != nil || route == nil { - log.Error(models.ErrRoutesNotFound) - c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound)) - return - } - if err := s.Datastore.RemoveRoute(ctx, appName, routePath); err != nil { - log.WithError(err).Debug(models.ErrRoutesRemoving) if err == models.ErrRoutesNotFound { + log.WithError(err).Debug(models.ErrRoutesRemoving) c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound)) } else { + log.WithError(err).Error(models.ErrRoutesRemoving) c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesRemoving)) } return diff --git a/api/server/routes_get.go b/api/server/routes_get.go index 4af8577e2..503ad63bf 100644 --- a/api/server/routes_get.go +++ b/api/server/routes_get.go @@ -5,7 +5,6 @@ import ( "net/http" "path" - "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" "github.com/iron-io/functions/api/models" "github.com/iron-io/runner/common" @@ -20,16 +19,14 @@ func (s *Server) handleRouteGet(c *gin.Context) { route, err := s.Datastore.GetRoute(ctx, appName, routePath) if err != nil && err != models.ErrRoutesNotFound { - log.WithError(err).Error(models.ErrRoutesGet) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesGet)) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } else if route == nil { + log.Debug(models.ErrRoutesNotFound) c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound)) return } - log.WithFields(logrus.Fields{"route": route}).Debug("Got route") - c.JSON(http.StatusOK, routeResponse{"Successfully loaded route", route}) } diff --git a/api/server/routes_list.go b/api/server/routes_list.go index a53c94eb7..d62d1a020 100644 --- a/api/server/routes_list.go +++ b/api/server/routes_list.go @@ -4,7 +4,6 @@ import ( "context" "net/http" - "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" "github.com/iron-io/functions/api/models" "github.com/iron-io/runner/common" @@ -28,13 +27,15 @@ func (s *Server) handleRouteList(c *gin.Context) { routes, err = s.Datastore.GetRoutes(ctx, filter) } - if err != nil { + if err == models.ErrAppsNotFound { + log.WithError(err).Debug(models.ErrRoutesGet) + c.JSON(http.StatusNotFound, simpleError(err)) + return + } else if err != nil { log.WithError(err).Error(models.ErrRoutesGet) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesGet)) + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } - log.WithFields(logrus.Fields{"routes": routes}).Debug("Got routes") - c.JSON(http.StatusOK, routesResponse{"Sucessfully listed routes", routes}) } diff --git a/api/server/routes_test.go b/api/server/routes_test.go index d4d8a0e05..f4700a107 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -27,14 +27,14 @@ func TestRouteCreate(t *testing.T) { {&datastore.Mock{}, "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON}, {&datastore.Mock{}, "/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, {&datastore.Mock{}, "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { } }`, http.StatusInternalServerError, models.ErrRoutesValidationMissingPath}, + {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello" } }`, http.StatusInternalServerError, models.ErrRoutesValidationMissingPath}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello", "path": "myroute" } }`, http.StatusInternalServerError, models.ErrRoutesValidationInvalidPath}, + {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, + {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, {&datastore.Mock{}, "/v1/apps/$/routes", `{ "route": { "image": "iron/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, // success - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello", "path": "/myroute" } }`, http.StatusCreated, nil}, + {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello", "path": "/myroute" } }`, http.StatusOK, nil}, } { rnr, cancel := testRunner(t) router := testRouter(test.mock, &mqs.Mock{}, rnr, tasks) @@ -212,7 +212,7 @@ func TestRouteUpdate(t *testing.T) { Path: "/myroute/do", }, }, - }, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusForbidden, nil}, + }, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusBadRequest, nil}, } { rnr, cancel := testRunner(t) router := testRouter(test.ds, &mqs.Mock{}, rnr, tasks) diff --git a/api/server/routes_update.go b/api/server/routes_update.go index ae8fb6c30..31aea0799 100644 --- a/api/server/routes_update.go +++ b/api/server/routes_update.go @@ -25,14 +25,14 @@ func (s *Server) handleRouteUpdate(c *gin.Context) { } if wroute.Route == nil { - log.WithError(err).Error(models.ErrInvalidJSON) + log.Debug(models.ErrRoutesMissingNew) c.JSON(http.StatusBadRequest, simpleError(models.ErrRoutesMissingNew)) return } if wroute.Route.Path != "" { log.Debug(models.ErrRoutesPathImmutable) - c.JSON(http.StatusForbidden, simpleError(models.ErrRoutesPathImmutable)) + c.JSON(http.StatusBadRequest, simpleError(models.ErrRoutesPathImmutable)) return } @@ -44,14 +44,19 @@ func (s *Server) handleRouteUpdate(c *gin.Context) { Image: wroute.Route.Image, }) if err != nil { - c.JSON(http.StatusInternalServerError, simpleError(models.ErrUsableImage)) + log.WithError(err).Debug(models.ErrRoutesUpdate) + c.JSON(http.StatusBadRequest, simpleError(models.ErrUsableImage)) return } } route, err := s.Datastore.UpdateRoute(ctx, wroute.Route) - if err != nil { + if err == models.ErrRoutesNotFound { log.WithError(err).Debug(models.ErrRoutesUpdate) + c.JSON(http.StatusNotFound, simpleError(err)) + return + } else if err != nil { + log.WithError(err).Error(models.ErrRoutesUpdate) c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesUpdate)) return } diff --git a/api/server/server_test.go b/api/server/server_test.go index 25b02aa63..1545c1126 100644 --- a/api/server/server_test.go +++ b/api/server/server_test.go @@ -104,11 +104,11 @@ func TestFullStack(t *testing.T) { body string expectedCode int }{ - {"create my app", "POST", "/v1/apps", `{ "app": { "name": "myapp" } }`, http.StatusCreated}, + {"create my app", "POST", "/v1/apps", `{ "app": { "name": "myapp" } }`, http.StatusOK}, {"list apps", "GET", "/v1/apps", ``, http.StatusOK}, {"get app", "GET", "/v1/apps/myapp", ``, http.StatusOK}, - {"add myroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "iron/hello" } }`, http.StatusCreated}, - {"add myroute2", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute2", "path": "/myroute2", "image": "iron/error" } }`, http.StatusCreated}, + {"add myroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "iron/hello" } }`, http.StatusOK}, + {"add myroute2", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute2", "path": "/myroute2", "image": "iron/error" } }`, http.StatusOK}, {"get myroute", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusOK}, {"get myroute2", "GET", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK}, {"get all routes", "GET", "/v1/apps/myapp/routes", ``, http.StatusOK}, diff --git a/docs/swagger.yml b/docs/swagger.yml index ef39a6413..ddbaf481c 100644 --- a/docs/swagger.yml +++ b/docs/swagger.yml @@ -6,7 +6,7 @@ swagger: '2.0' info: title: IronFunctions description: The open source serverless platform. - version: "0.1.25" + version: "0.1.26" # the domain of the service host: "127.0.0.1:8080" # array of all schemes that your API supports @@ -66,8 +66,8 @@ paths: description: Parameters are missing or invalid. schema: $ref: '#/definitions/Error' - 500: - description: Could not accept app due to internal error. + 409: + description: App already exists. schema: $ref: '#/definitions/Error' default: @@ -149,8 +149,8 @@ paths: description: Parameters are missing or invalid. schema: $ref: '#/definitions/Error' - 500: - description: Could not accept app due to internal error. + 404: + description: App does not exist. schema: $ref: '#/definitions/Error' default: @@ -161,7 +161,7 @@ paths: /apps/{app}/routes: post: summary: Create new Route - description: Create a new route + description: Create a new route in an app, if app doesn't exists, it creates the app tags: - Routes parameters: @@ -177,16 +177,16 @@ paths: schema: $ref: '#/definitions/RouteWrapper' responses: - 201: + 200: description: Route created schema: $ref: '#/definitions/RouteWrapper' 400: - description: One or more of the routes were invalid due to parameters being missing or invalid. + description: Invalid route due to parameters being missing or invalid. schema: $ref: '#/definitions/Error' - 500: - description: Could not accept routes due to internal error. + 409: + description: Route already exists. schema: $ref: '#/definitions/Error' default: @@ -210,6 +210,10 @@ paths: description: Route information schema: $ref: '#/definitions/RoutesWrapper' + 404: + description: App does not exist. + schema: + $ref: '#/definitions/Error' default: description: Unexpected error schema: @@ -239,16 +243,16 @@ paths: schema: $ref: '#/definitions/RouteWrapper' responses: - 201: + 200: description: Route updated schema: $ref: '#/definitions/RouteWrapper' 400: - description: One or more of the routes were invalid due to parameters being missing or invalid. + description: Invalid route due to parameters being missing or invalid. schema: $ref: '#/definitions/Error' - 500: - description: Could not accept routes due to internal error. + 404: + description: App does not exist. schema: $ref: '#/definitions/Error' default: