diff --git a/api/models/app.go b/api/models/app.go index 3ba982146..4bb8e4e90 100644 --- a/api/models/app.go +++ b/api/models/app.go @@ -87,5 +87,6 @@ func (a *App) UpdateConfig(patch Config) { } type AppFilter struct { + // An SQL LIKE query. Empty does not filter. Name string } diff --git a/api/models/route.go b/api/models/route.go index 7ce6ee726..372bb131c 100644 --- a/api/models/route.go +++ b/api/models/route.go @@ -43,63 +43,34 @@ type Route struct { } var ( - ErrRoutesValidationFoundDynamicURL = errors.New("Dynamic URL is not allowed") - ErrRoutesValidationInvalidPath = errors.New("Invalid Path format") - ErrRoutesValidationInvalidType = errors.New("Invalid route Type") - ErrRoutesValidationInvalidFormat = errors.New("Invalid route Format") - ErrRoutesValidationMissingAppName = errors.New("Missing route AppName") - ErrRoutesValidationMissingImage = errors.New("Missing route Image") - ErrRoutesValidationMissingName = errors.New("Missing route Name") - ErrRoutesValidationMissingPath = errors.New("Missing route Path") - ErrRoutesValidationMissingType = errors.New("Missing route Type") - ErrRoutesValidationPathMalformed = errors.New("Path malformed") - ErrRoutesValidationNegativeTimeout = errors.New("Negative timeout") + ErrRoutesValidationFoundDynamicURL = errors.New("Dynamic URL is not allowed") + ErrRoutesValidationInvalidPath = errors.New("Invalid Path format") + ErrRoutesValidationInvalidType = errors.New("Invalid route Type") + ErrRoutesValidationInvalidFormat = errors.New("Invalid route Format") + ErrRoutesValidationMissingAppName = errors.New("Missing route AppName") + ErrRoutesValidationMissingImage = errors.New("Missing route Image") + ErrRoutesValidationMissingName = errors.New("Missing route Name") + ErrRoutesValidationMissingPath = errors.New("Missing route Path") + ErrRoutesValidationMissingType = errors.New("Missing route Type") + ErrRoutesValidationPathMalformed = errors.New("Path malformed") + ErrRoutesValidationNegativeTimeout = errors.New("Negative timeout") + ErrRoutesValidationNegativeMaxConcurrency = errors.New("Negative MaxConcurrency") ) -func (r *Route) Validate() error { - var res []error - +// SetDefaults sets zeroed field to defaults. +func (r *Route) SetDefaults() { if r.Memory == 0 { r.Memory = 128 } - if r.AppName == "" { - res = append(res, ErrRoutesValidationMissingAppName) - } - - if r.Path == "" { - res = append(res, ErrRoutesValidationMissingPath) - } - - u, err := url.Parse(r.Path) - if err != nil { - res = append(res, ErrRoutesValidationPathMalformed) - } - - if strings.Contains(u.Path, ":") { - res = append(res, ErrRoutesValidationFoundDynamicURL) - } - - if !path.IsAbs(u.Path) { - res = append(res, ErrRoutesValidationInvalidPath) - } - if r.Type == TypeNone { r.Type = TypeSync } - if r.Type != TypeAsync && r.Type != TypeSync { - res = append(res, ErrRoutesValidationInvalidType) - } - if r.Format == "" { r.Format = FormatDefault } - if r.Format != FormatDefault && r.Format != FormatHTTP { - res = append(res, ErrRoutesValidationInvalidFormat) - } - if r.MaxConcurrency == 0 { r.MaxConcurrency = 1 } @@ -114,7 +85,59 @@ func (r *Route) Validate() error { if r.Timeout == 0 { r.Timeout = defaultRouteTimeout - } else if r.Timeout < 0 { + } +} + +// Validate validates field values, skipping zeroed fields if skipZero is true. +func (r *Route) Validate(skipZero bool) error { + var res []error + + if !skipZero { + if r.AppName == "" { + res = append(res, ErrRoutesValidationMissingAppName) + } + + if r.Image == "" { + res = append(res, ErrRoutesValidationMissingImage) + } + + if r.Path == "" { + res = append(res, ErrRoutesValidationMissingPath) + } + } + + if !skipZero || r.Path != "" { + u, err := url.Parse(r.Path) + if err != nil { + res = append(res, ErrRoutesValidationPathMalformed) + } + + if strings.Contains(u.Path, ":") { + res = append(res, ErrRoutesValidationFoundDynamicURL) + } + + if !path.IsAbs(u.Path) { + res = append(res, ErrRoutesValidationInvalidPath) + } + } + + if !skipZero || r.Type != "" { + if r.Type != TypeAsync && r.Type != TypeSync { + res = append(res, ErrRoutesValidationInvalidType) + } + } + + if !skipZero || r.Format != "" { + if r.Format != FormatDefault && r.Format != FormatHTTP { + res = append(res, ErrRoutesValidationInvalidFormat) + } + } + + if r.MaxConcurrency < 0 { + res = append(res, ErrRoutesValidationNegativeMaxConcurrency) + } + + if r.Timeout < 0 { res = append(res, ErrRoutesValidationNegativeTimeout) } @@ -182,6 +205,7 @@ func (r *Route) Update(new *Route) { } } +//TODO are these sql LIKE queries? or strict matches? type RouteFilter struct { Path string AppName string diff --git a/api/models/route_wrapper.go b/api/models/route_wrapper.go index 0d7e21bdc..4420bac8a 100644 --- a/api/models/route_wrapper.go +++ b/api/models/route_wrapper.go @@ -6,10 +6,10 @@ type RouteWrapper struct { Route *Route `json:"route"` } -func (m *RouteWrapper) Validate() error { +func (m *RouteWrapper) Validate(skipZero bool) error { var res []error - if err := m.validateRoute(); err != nil { + if err := m.validateRoute(skipZero); err != nil { res = append(res, err) } @@ -19,10 +19,10 @@ func (m *RouteWrapper) Validate() error { return nil } -func (m *RouteWrapper) validateRoute() error { +func (m *RouteWrapper) validateRoute(skipZero bool) error { if m.Route != nil { - if err := m.Route.Validate(); err != nil { + if err := m.Route.Validate(skipZero); err != nil { return err } } diff --git a/api/server/apps_delete.go b/api/server/apps_delete.go index 3f592a863..7d6f17c22 100644 --- a/api/server/apps_delete.go +++ b/api/server/apps_delete.go @@ -22,7 +22,7 @@ func (s *Server) handleAppDelete(c *gin.Context) { c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) return } - + //TODO allow this? #528 if len(routes) > 0 { log.WithError(err).Debug(models.ErrDeleteAppsWithRoutes) c.JSON(http.StatusBadRequest, simpleError(models.ErrDeleteAppsWithRoutes)) diff --git a/api/server/routes_create.go b/api/server/routes_create.go index fccef0b65..f287498ff 100644 --- a/api/server/routes_create.go +++ b/api/server/routes_create.go @@ -31,18 +31,14 @@ func (s *Server) handleRouteCreate(c *gin.Context) { wroute.Route.AppName = c.MustGet(api.AppName).(string) - if err := wroute.Validate(); err != nil { + wroute.Route.SetDefaults() + + if err := wroute.Validate(false); err != nil { 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, // }) diff --git a/api/server/routes_test.go b/api/server/routes_test.go index 8f306eeb8..911fbe3b3 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -50,11 +50,14 @@ func TestRouteCreate(t *testing.T) { if test.expectedError != nil { resp := getErrorResponse(t, rec) - - if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { + if resp.Error == nil { + t.Log(buf.String()) + t.Errorf("Test %d: Expected error message to have `%s`, but it was nil", + i, test.expectedError) + } else if !strings.Contains(resp.Error.Message, test.expectedError.Error()) { t.Log(buf.String()) t.Errorf("Test %d: Expected error message to have `%s`, but it was `%s`", - i, test.expectedError.Error(), resp.Error.Message) + i, test.expectedError, resp.Error.Message) } } cancel() @@ -193,6 +196,8 @@ func TestRouteUpdate(t *testing.T) { // errors {datastore.NewMock(), "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON}, {datastore.NewMock(), "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {datastore.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, nil}, + {datastore.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, nil}, // success {datastore.NewMockInit(nil, @@ -223,8 +228,8 @@ func TestRouteUpdate(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) + t.Errorf("Test %d: Expected status code to be %d but was %d: %s", + i, test.expectedCode, rec.Code, rec.Body.String()) } if test.expectedError != nil { diff --git a/api/server/routes_update.go b/api/server/routes_update.go index 81e3fed2f..79507f273 100644 --- a/api/server/routes_update.go +++ b/api/server/routes_update.go @@ -39,6 +39,12 @@ func (s *Server) handleRouteUpdate(c *gin.Context) { wroute.Route.AppName = c.MustGet(api.AppName).(string) wroute.Route.Path = path.Clean(c.MustGet(api.Path).(string)) + if err := wroute.Validate(true); err != nil { + log.WithError(err).Debug(models.ErrRoutesUpdate) + c.JSON(http.StatusBadRequest, simpleError(err)) + return + } + if wroute.Route.Image != "" { // err = s.Runner.EnsureImageExists(ctx, &task.Config{ // Image: wroute.Route.Image,