diff --git a/api/handlers/app_test.go b/api/handlers/app_test.go index 93efe6ec1..fa5d0b28d 100644 --- a/api/handlers/app_test.go +++ b/api/handlers/app_test.go @@ -140,8 +140,8 @@ var _ = Describe("App", func() { BeforeEach(func() { payload = &payloads.AppCreate{ Name: appName, - Relationships: payloads.AppRelationships{ - Space: payloads.Relationship{ + Relationships: &payloads.AppRelationships{ + Space: &payloads.Relationship{ Data: &payloads.RelationshipData{ GUID: spaceGUID, }, diff --git a/api/handlers/process.go b/api/handlers/process.go index ae24d6556..99b478012 100644 --- a/api/handlers/process.go +++ b/api/handlers/process.go @@ -44,20 +44,20 @@ type Process struct { serverURL url.URL processRepo CFProcessRepository processStats ProcessStats - decoderValidator *DecoderValidator + requestValidator RequestValidator } func NewProcess( serverURL url.URL, processRepo CFProcessRepository, processStatsFetcher ProcessStats, - decoderValidator *DecoderValidator, + requestValidator RequestValidator, ) *Process { return &Process{ serverURL: serverURL, processRepo: processRepo, processStats: processStatsFetcher, - decoderValidator: decoderValidator, + requestValidator: requestValidator, } } @@ -110,7 +110,7 @@ func (h *Process) scale(r *http.Request) (*routing.Response, error) { processGUID := routing.URLParam(r, "guid") var payload payloads.ProcessScale - if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } @@ -149,12 +149,8 @@ func (h *Process) list(r *http.Request) (*routing.Response, error) { //nolint:du authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.process.list") - if err := r.ParseForm(); err != nil { - return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters") - } - processListFilter := new(payloads.ProcessList) - err := payloads.Decode(processListFilter, r.Form) + err := h.requestValidator.DecodeAndValidateURLValues(r, processListFilter) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters") } @@ -174,7 +170,7 @@ func (h *Process) update(r *http.Request) (*routing.Response, error) { processGUID := routing.URLParam(r, "guid") var payload payloads.ProcessPatch - if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode json payload") } diff --git a/api/handlers/process_test.go b/api/handlers/process_test.go index e673a912a..efdc5ed2a 100644 --- a/api/handlers/process_test.go +++ b/api/handlers/process_test.go @@ -2,14 +2,15 @@ package handlers_test import ( "errors" + "io" "net/http" - "net/http/httptest" "strings" "code.cloudfoundry.org/korifi/api/actions" apierrors "code.cloudfoundry.org/korifi/api/errors" . "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/handlers/fake" + "code.cloudfoundry.org/korifi/api/payloads" "code.cloudfoundry.org/korifi/api/repositories" . "code.cloudfoundry.org/korifi/tests/matchers" "code.cloudfoundry.org/korifi/tools" @@ -20,21 +21,21 @@ import ( var _ = Describe("Process", func() { var ( - processRepo *fake.CFProcessRepository - processStats *fake.ProcessStats + processRepo *fake.CFProcessRepository + processStats *fake.ProcessStats + requestValidator *fake.RequestValidator ) BeforeEach(func() { processRepo = new(fake.CFProcessRepository) processStats = new(fake.ProcessStats) - decoderValidator, err := NewDefaultDecoderValidator() - Expect(err).NotTo(HaveOccurred()) + requestValidator = new(fake.RequestValidator) apiHandler := NewProcess( *serverURL, processRepo, processStats, - decoderValidator, + requestValidator, ) routerBuilder.LoadRoutes(apiHandler) }) @@ -151,6 +152,12 @@ var _ = Describe("Process", func() { "memory_in_mb": 512, "disk_in_mb": 256 }` + + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidateJSONPayloadStub(&payloads.ProcessScale{ + Instances: tools.PtrTo(3), + MemoryMB: tools.PtrTo[int64](512), + DiskMB: tools.PtrTo[int64](256), + }) }) JustBeforeEach(func() { @@ -160,6 +167,12 @@ var _ = Describe("Process", func() { }) It("scales the process", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + req, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + reqBytes, err := io.ReadAll(req.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(reqBytes)).To(Equal(requestBody)) + Expect(processRepo.GetProcessCallCount()).To(Equal(1)) _, actualAuthInfo, actualProcessGUID := processRepo.GetProcessArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -190,11 +203,11 @@ var _ = Describe("Process", func() { When("the request JSON is invalid", func() { BeforeEach(func() { - requestBody = `}` + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("boom")) }) - It("has the expected error response body", func() { - expectBadRequestError() + It("returns an error", func() { + expectUnknownError() }) }) @@ -227,24 +240,6 @@ var _ = Describe("Process", func() { expectUnknownError() }) }) - - When("validating scale parameters", func() { - DescribeTable("returns a validation decision", - func(requestBody string, status int) { - tableTestRecorder := httptest.NewRecorder() - req, err := http.NewRequestWithContext(ctx, "POST", "/v3/processes/process-guid/actions/scale", strings.NewReader(requestBody)) - Expect(err).NotTo(HaveOccurred()) - routerBuilder.Build().ServeHTTP(tableTestRecorder, req) - Expect(tableTestRecorder.Code).To(Equal(status)) - }, - Entry("instances is negative", `{"instances":-1}`, http.StatusUnprocessableEntity), - Entry("memory is not a positive integer", `{"memory_in_mb":0}`, http.StatusUnprocessableEntity), - Entry("disk is not a positive integer", `{"disk_in_mb":0}`, http.StatusUnprocessableEntity), - Entry("instances is zero", `{"instances":0}`, http.StatusOK), - Entry("memory is a positive integer", `{"memory_in_mb":1024}`, http.StatusOK), - Entry("disk is a positive integer", `{"disk_in_mb":1024}`, http.StatusOK), - ) - }) }) Describe("the GET /v3/processes//stats endpoint", func() { @@ -308,10 +303,8 @@ var _ = Describe("Process", func() { }) Describe("the GET /v3/processes endpoint", func() { - var queryString string - BeforeEach(func() { - queryString = "" + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ProcessList{}) processRepo.ListProcessesReturns([]repositories.ProcessRecord{ { GUID: "process-guid", @@ -320,12 +313,16 @@ var _ = Describe("Process", func() { }) JustBeforeEach(func() { - req, err := http.NewRequestWithContext(ctx, "GET", "/v3/processes"+queryString, nil) + req, err := http.NewRequestWithContext(ctx, "GET", "/v3/processes", nil) Expect(err).NotTo(HaveOccurred()) routerBuilder.Build().ServeHTTP(rr, req) }) It("returns the processes", func() { + Expect(requestValidator.DecodeAndValidateURLValuesCallCount()).To(Equal(1)) + req, _ := requestValidator.DecodeAndValidateURLValuesArgsForCall(0) + Expect(req.URL.String()).To(HaveSuffix("/v3/processes")) + Expect(rr).To(HaveHTTPStatus(http.StatusOK)) Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) Expect(rr).To(HaveHTTPBody(SatisfyAll( @@ -335,9 +332,11 @@ var _ = Describe("Process", func() { ))) }) - When("Query Parameters are provided", func() { + When("app_guids query parameter is provided", func() { BeforeEach(func() { - queryString = "?app_guids=my-app-guid" + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ProcessList{ + AppGUIDs: "my-app-guid", + }) }) It("invokes process repository with correct args", func() { @@ -349,13 +348,13 @@ var _ = Describe("Process", func() { }) }) - When("invalid query parameters are provided", func() { + When("the request body is invalid", func() { BeforeEach(func() { - queryString = "?foo=my-app-guid" + requestValidator.DecodeAndValidateURLValuesReturns(errors.New("boo")) }) - It("returns an Unknown key error", func() { - expectUnknownKeyError("The query parameter is invalid: Valid parameters are: .*") + It("returns an error", func() { + expectUnknownError() }) }) @@ -397,6 +396,22 @@ var _ = Describe("Process", func() { processRepo.PatchProcessReturns(repositories.ProcessRecord{ GUID: "process-guid", }, nil) + + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidateJSONPayloadStub(&payloads.ProcessPatch{ + Metadata: &payloads.MetadataPatch{ + Labels: map[string]*string{ + "foo": tools.PtrTo("value1"), + }, + }, + HealthCheck: &payloads.HealthCheck{ + Type: tools.PtrTo("port"), + Data: &payloads.Data{ + Timeout: tools.PtrTo[int64](5), + Endpoint: tools.PtrTo("http://myapp.com/health"), + InvocationTimeout: tools.PtrTo[int64](2), + }, + }, + }) }) JustBeforeEach(func() { @@ -406,6 +421,12 @@ var _ = Describe("Process", func() { }) It("updates the process", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + req, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + reqBytes, err := io.ReadAll(req.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(reqBytes)).To(Equal(requestBody)) + Expect(processRepo.PatchProcessCallCount()).To(Equal(1)) _, actualAuthInfo, actualMsg := processRepo.PatchProcessArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -426,25 +447,11 @@ var _ = Describe("Process", func() { When("the request body is invalid json", func() { BeforeEach(func() { - requestBody = `{` + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("boom")) }) - It("return an request malformed error", func() { - expectBadRequestError() - }) - }) - - When("the request body is invalid with an unknown field", func() { - BeforeEach(func() { - requestBody = `{ - "health_check": { - "endpoint": "my-endpoint" - } - }` - }) - - It("return an request malformed error", func() { - expectUnprocessableEntityError("invalid request body: json: unknown field \"endpoint\"") + It("return an error", func() { + expectUnknownError() }) }) diff --git a/api/handlers/role.go b/api/handlers/role.go index 1a55b8ad2..13011a895 100644 --- a/api/handlers/role.go +++ b/api/handlers/role.go @@ -21,22 +21,6 @@ const ( RolePath = RolesPath + "/{guid}" ) -type RoleName string - -const ( - RoleAdmin RoleName = "admin" - RoleAdminReadOnly RoleName = "admin_read_only" - RoleGlobalAuditor RoleName = "global_auditor" - RoleOrganizationAuditor RoleName = "organization_auditor" - RoleOrganizationBillingManager RoleName = "organization_billing_manager" - RoleOrganizationManager RoleName = "organization_manager" - RoleOrganizationUser RoleName = "organization_user" - RoleSpaceAuditor RoleName = "space_auditor" - RoleSpaceDeveloper RoleName = "space_developer" - RoleSpaceManager RoleName = "space_manager" - RoleSpaceSupporter RoleName = "space_supporter" -) - //counterfeiter:generate -o fake -fake-name CFRoleRepository . CFRoleRepository type CFRoleRepository interface { @@ -49,14 +33,14 @@ type CFRoleRepository interface { type Role struct { apiBaseURL url.URL roleRepo CFRoleRepository - decoderValidator RequestValidator + requestValidator RequestValidator } -func NewRole(apiBaseURL url.URL, roleRepo CFRoleRepository, decoderValidator RequestValidator) *Role { +func NewRole(apiBaseURL url.URL, roleRepo CFRoleRepository, requestValidator RequestValidator) *Role { return &Role{ apiBaseURL: apiBaseURL, roleRepo: roleRepo, - decoderValidator: decoderValidator, + requestValidator: requestValidator, } } @@ -65,7 +49,7 @@ func (h *Role) create(r *http.Request) (*routing.Response, error) { logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.role.create") var payload payloads.RoleCreate - if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } @@ -84,12 +68,8 @@ func (h *Role) list(r *http.Request) (*routing.Response, error) { authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.role.list") - if err := r.ParseForm(); err != nil { - return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters") - } - - roleListFilter := new(payloads.RoleListFilter) - err := payloads.Decode(roleListFilter, r.Form) + roleListFilter := new(payloads.RoleList) + err := h.requestValidator.DecodeAndValidateURLValues(r, roleListFilter) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters") } @@ -101,14 +81,14 @@ func (h *Role) list(r *http.Request) (*routing.Response, error) { filteredRoles := filterRoles(roleListFilter, roles) - if err := h.sortList(filteredRoles, r.FormValue("order_by")); err != nil { + if err := h.sortList(filteredRoles, roleListFilter.OrderBy); err != nil { return nil, apierrors.LogAndReturn(logger, err, "unable to parse order by request") } return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForRole, filteredRoles, h.apiBaseURL, *r.URL)), nil } -func filterRoles(roleListFilter *payloads.RoleListFilter, roles []repositories.RoleRecord) []repositories.RoleRecord { +func filterRoles(roleListFilter *payloads.RoleList, roles []repositories.RoleRecord) []repositories.RoleRecord { var filteredRoles []repositories.RoleRecord for _, role := range roles { if match(roleListFilter.GUIDs, role.GUID) && diff --git a/api/handlers/role_test.go b/api/handlers/role_test.go index c50d50864..bebf128ff 100644 --- a/api/handlers/role_test.go +++ b/api/handlers/role_test.go @@ -2,6 +2,7 @@ package handlers_test import ( "errors" + "io" "net/http" "net/http/httptest" "strings" @@ -45,7 +46,7 @@ var _ = Describe("Role", func() { roleCreate = &payloads.RoleCreate{ Type: "space_developer", Relationships: payloads.RoleRelationships{ - User: &payloads.UserRelationship{ + User: payloads.UserRelationship{ Data: payloads.UserRelationshipData{ Username: "my-user", }, @@ -62,12 +63,18 @@ var _ = Describe("Role", func() { }) JustBeforeEach(func() { - req, err := http.NewRequestWithContext(ctx, "POST", rolesBase, strings.NewReader("")) + req, err := http.NewRequestWithContext(ctx, "POST", rolesBase, strings.NewReader("payload-data")) Expect(err).NotTo(HaveOccurred()) routerBuilder.Build().ServeHTTP(rr, req) }) It("creates the role", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + req, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + bodyBytes, err := io.ReadAll(req.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(bodyBytes)).To(Equal("payload-data")) + Expect(roleRepo.CreateRoleCallCount()).To(Equal(1)) _, actualAuthInfo, roleMessage := roleRepo.CreateRoleArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -153,23 +160,25 @@ var _ = Describe("Role", func() { }) Describe("List roles", func() { - var query string - BeforeEach(func() { - query = "" roleRepo.ListRolesReturns([]repositories.RoleRecord{ {GUID: "role-1"}, {GUID: "role-2"}, }, nil) + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.RoleList{}) }) JustBeforeEach(func() { - req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+query, nil) + req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?foo=bar", nil) Expect(err).NotTo(HaveOccurred()) routerBuilder.Build().ServeHTTP(rr, req) }) It("lists roles", func() { + Expect(requestValidator.DecodeAndValidateURLValuesCallCount()).To(Equal(1)) + req, _ := requestValidator.DecodeAndValidateURLValuesArgsForCall(0) + Expect(req.URL.String()).To(HaveSuffix(rolesBase + "?foo=bar")) + Expect(roleRepo.ListRolesCallCount()).To(Equal(1)) _, actualAuthInfo := roleRepo.ListRolesArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -178,7 +187,7 @@ var _ = Describe("Role", func() { Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) Expect(rr).To(HaveHTTPBody(SatisfyAll( MatchJSONPath("$.pagination.total_results", BeEquivalentTo(2)), - MatchJSONPath("$.pagination.first.href", "https://api.example.org/v3/roles"), + MatchJSONPath("$.pagination.first.href", "https://api.example.org/v3/roles?foo=bar"), MatchJSONPath("$.resources", HaveLen(2)), MatchJSONPath("$.resources[0].guid", "role-1"), MatchJSONPath("$.resources[0].links.self.href", "https://api.example.org/v3/roles/role-1"), @@ -186,17 +195,6 @@ var _ = Describe("Role", func() { ))) }) - When("include is specified", func() { - BeforeEach(func() { - query = "?include=user" - }) - - It("does not fail but has no effect on the result", func() { - Expect(rr).To(HaveHTTPStatus(http.StatusOK)) - Expect(rr).To(HaveHTTPBody(MatchJSONPath("$.pagination.total_results", BeEquivalentTo(2)))) - }) - }) - Describe("filtering and ordering", func() { BeforeEach(func() { roleRepo.ListRolesReturns([]repositories.RoleRecord{ @@ -207,31 +205,35 @@ var _ = Describe("Role", func() { }, nil) }) - DescribeTable("filtering", func(filter string, expectedGUIDs ...any) { - req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?"+filter, nil) + DescribeTable("filtering", func(filter payloads.RoleList, expectedGUIDs ...any) { + req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?foo=bar", nil) Expect(err).NotTo(HaveOccurred()) + + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&filter) rr = httptest.NewRecorder() routerBuilder.Build().ServeHTTP(rr, req) Expect(rr).To(HaveHTTPStatus(http.StatusOK)) Expect(rr).To(HaveHTTPBody(MatchJSONPath("$.resources[*].guid", expectedGUIDs))) }, - Entry("no filter", "", "1", "2", "3", "4"), - Entry("guids1", "guids=4", "4"), - Entry("guids2", "guids=1,3", "1", "3"), - Entry("types1", "types=a", "1"), - Entry("types2", "types=b,c", "2", "3", "4"), - Entry("space_guids1", "space_guids=space1", "1"), - Entry("space_guids2", "space_guids=space1,space2", "1", "2"), - Entry("organization_guids1", "organization_guids=org1", "3"), - Entry("organization_guids2", "organization_guids=org1,org2", "3", "4"), - Entry("user_guids1", "user_guids=user1", "1", "2", "3"), - Entry("user_guids2", "user_guids=user1,user2", "1", "2", "3", "4"), + Entry("no filter", payloads.RoleList{}, "1", "2", "3", "4"), + Entry("guids1", payloads.RoleList{GUIDs: map[string]bool{"4": true}}, "4"), + Entry("guids2", payloads.RoleList{GUIDs: map[string]bool{"1": true, "3": true}}, "1", "3"), + Entry("types1", payloads.RoleList{Types: map[string]bool{"a": true}}, "1"), + Entry("types2", payloads.RoleList{Types: map[string]bool{"b": true, "c": true}}, "2", "3", "4"), + Entry("space_guids1", payloads.RoleList{SpaceGUIDs: map[string]bool{"space1": true}}, "1"), + Entry("space_guids2", payloads.RoleList{SpaceGUIDs: map[string]bool{"space1": true, "space2": true}}, "1", "2"), + Entry("organization_guids1", payloads.RoleList{OrgGUIDs: map[string]bool{"org1": true}}, "3"), + Entry("organization_guids2", payloads.RoleList{OrgGUIDs: map[string]bool{"org1": true, "org2": true}}, "3", "4"), + Entry("user_guids1", payloads.RoleList{UserGUIDs: map[string]bool{"user1": true}}, "1", "2", "3"), + Entry("user_guids2", payloads.RoleList{UserGUIDs: map[string]bool{"user1": true, "user2": true}}, "1", "2", "3", "4"), ) DescribeTable("ordering", func(order string, expectedGUIDs ...any) { - req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?order_by="+order, nil) + req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?order_by=not-used-in-test", nil) Expect(err).NotTo(HaveOccurred()) + + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.RoleList{OrderBy: order}) rr = httptest.NewRecorder() routerBuilder.Build().ServeHTTP(rr, req) @@ -245,13 +247,13 @@ var _ = Describe("Role", func() { ) }) - When("order_by is not a valid field", func() { + When("decoding the url values fails", func() { BeforeEach(func() { - query = "?order_by=not_valid" + requestValidator.DecodeAndValidateURLValuesReturns(errors.New("boom")) }) - It("returns an Unknown key error", func() { - expectUnknownKeyError("The query parameter is invalid: Order by can only be: .*") + It("returns an Unknown error", func() { + expectUnknownError() }) }) diff --git a/api/handlers/route.go b/api/handlers/route.go index a6746c4ff..7c90123ca 100644 --- a/api/handlers/route.go +++ b/api/handlers/route.go @@ -41,7 +41,7 @@ type Route struct { domainRepo CFDomainRepository appRepo CFAppRepository spaceRepo SpaceRepository - decoderValidator *DecoderValidator + requestValidator RequestValidator } func NewRoute( @@ -50,7 +50,7 @@ func NewRoute( domainRepo CFDomainRepository, appRepo CFAppRepository, spaceRepo SpaceRepository, - decoderValidator *DecoderValidator, + requestValidator RequestValidator, ) *Route { return &Route{ serverURL: serverURL, @@ -58,7 +58,7 @@ func NewRoute( domainRepo: domainRepo, appRepo: appRepo, spaceRepo: spaceRepo, - decoderValidator: decoderValidator, + requestValidator: requestValidator, } } @@ -80,13 +80,8 @@ func (h *Route) list(r *http.Request) (*routing.Response, error) { authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.route.list") - if err := r.ParseForm(); err != nil { - return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters") - } - routeListFilter := new(payloads.RouteList) - err := payloads.Decode(routeListFilter, r.Form) - if err != nil { + if err := h.requestValidator.DecodeAndValidateURLValues(r, routeListFilter); err != nil { return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters") } @@ -117,7 +112,7 @@ func (h *Route) create(r *http.Request) (*routing.Response, error) { logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.route.create") var payload payloads.RouteCreate - if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } @@ -166,8 +161,8 @@ func (h *Route) insertDestinations(r *http.Request) (*routing.Response, error) { authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.route.insert-destinations") - var destinationCreatePayload payloads.DestinationListCreate - if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &destinationCreatePayload); err != nil { + var destinationCreatePayload payloads.RouteDestinationCreate + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &destinationCreatePayload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } @@ -236,23 +231,6 @@ func (h *Route) delete(r *http.Request) (*routing.Response, error) { return routing.NewResponse(http.StatusAccepted).WithHeader("Location", presenter.JobURLForRedirects(routeGUID, presenter.RouteDeleteOperation, h.serverURL)), nil } -func (h *Route) UnauthenticatedRoutes() []routing.Route { - return nil -} - -func (h *Route) AuthenticatedRoutes() []routing.Route { - return []routing.Route{ - {Method: "GET", Pattern: RoutePath, Handler: h.get}, - {Method: "GET", Pattern: RoutesPath, Handler: h.list}, - {Method: "GET", Pattern: RouteDestinationsPath, Handler: h.listDestinations}, - {Method: "POST", Pattern: RoutesPath, Handler: h.create}, - {Method: "DELETE", Pattern: RoutePath, Handler: h.delete}, - {Method: "POST", Pattern: RouteDestinationsPath, Handler: h.insertDestinations}, - {Method: "DELETE", Pattern: RouteDestinationPath, Handler: h.deleteDestination}, - {Method: "PATCH", Pattern: RoutePath, Handler: h.update}, - } -} - // Fetch Route and compose related Domain information within func (h *Route) lookupRouteAndDomain(ctx context.Context, logger logr.Logger, authInfo authorization.Info, routeGUID string) (repositories.RouteRecord, error) { route, err := h.routeRepo.GetRoute(ctx, authInfo, routeGUID) @@ -305,7 +283,7 @@ func (h *Route) update(r *http.Request) (*routing.Response, error) { } var payload payloads.RoutePatch - if err = h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + if err = h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } @@ -315,3 +293,20 @@ func (h *Route) update(r *http.Request) (*routing.Response, error) { } return routing.NewResponse(http.StatusOK).WithBody(presenter.ForRoute(route, h.serverURL)), nil } + +func (h *Route) UnauthenticatedRoutes() []routing.Route { + return nil +} + +func (h *Route) AuthenticatedRoutes() []routing.Route { + return []routing.Route{ + {Method: "GET", Pattern: RoutePath, Handler: h.get}, + {Method: "GET", Pattern: RoutesPath, Handler: h.list}, + {Method: "GET", Pattern: RouteDestinationsPath, Handler: h.listDestinations}, + {Method: "POST", Pattern: RoutesPath, Handler: h.create}, + {Method: "DELETE", Pattern: RoutePath, Handler: h.delete}, + {Method: "POST", Pattern: RouteDestinationsPath, Handler: h.insertDestinations}, + {Method: "DELETE", Pattern: RouteDestinationPath, Handler: h.deleteDestination}, + {Method: "PATCH", Pattern: RoutePath, Handler: h.update}, + } +} diff --git a/api/handlers/route_test.go b/api/handlers/route_test.go index ed205f803..593a53213 100644 --- a/api/handlers/route_test.go +++ b/api/handlers/route_test.go @@ -3,15 +3,17 @@ package handlers_test import ( "errors" "fmt" + "io" "net/http" - "regexp" "strings" apierrors "code.cloudfoundry.org/korifi/api/errors" . "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/handlers/fake" + "code.cloudfoundry.org/korifi/api/payloads" "code.cloudfoundry.org/korifi/api/repositories" . "code.cloudfoundry.org/korifi/tests/matchers" + "code.cloudfoundry.org/korifi/tools" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -20,10 +22,11 @@ import ( var _ = Describe("Route", func() { var ( - routeRepo *fake.CFRouteRepository - domainRepo *fake.CFDomainRepository - appRepo *fake.CFAppRepository - spaceRepo *fake.SpaceRepository + routeRepo *fake.CFRouteRepository + domainRepo *fake.CFDomainRepository + appRepo *fake.CFAppRepository + spaceRepo *fake.SpaceRepository + requestValidator *fake.RequestValidator requestMethod string requestPath string @@ -66,8 +69,7 @@ var _ = Describe("Route", func() { Name: "test-space-guid", }, nil) - decoderValidator, err := NewDefaultDecoderValidator() - Expect(err).NotTo(HaveOccurred()) + requestValidator = new(fake.RequestValidator) apiHandler := NewRoute( *serverURL, @@ -75,7 +77,7 @@ var _ = Describe("Route", func() { domainRepo, appRepo, spaceRepo, - decoderValidator, + requestValidator, ) routerBuilder.LoadRoutes(apiHandler) }) @@ -166,11 +168,17 @@ var _ = Describe("Route", func() { }, nil) requestMethod = http.MethodGet - requestPath = "/v3/routes" + requestPath = "/v3/routes?foo=bar" requestBody = "" + + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.RouteList{}) }) It("returns the routes list", func() { + Expect(requestValidator.DecodeAndValidateURLValuesCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateURLValuesArgsForCall(0) + Expect(actualReq.URL.String()).To(HaveSuffix("/v3/routes?foo=bar")) + Expect(routeRepo.ListRoutesCallCount()).To(Equal(1)) _, actualAuthInfo, _ := routeRepo.ListRoutesArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -184,6 +192,7 @@ var _ = Describe("Route", func() { Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) Expect(rr).To(HaveHTTPBody(SatisfyAll( MatchJSONPath("$.pagination.total_results", BeEquivalentTo(2)), + MatchJSONPath("$.pagination.first.href", "https://api.example.org/v3/routes?foo=bar"), MatchJSONPath("$.resources[0].guid", "test-route-guid"), MatchJSONPath("$.resources[0].url", "test-route-host.example.org/some_path"), MatchJSONPath("$.resources[1].guid", "other-test-route-guid"), @@ -193,34 +202,24 @@ var _ = Describe("Route", func() { When("query parameters are provided", func() { BeforeEach(func() { - requestPath += "?app_guids=my-app-guid&" + - "space_guids=my-space-guid&" + - "domain_guids=my-domain-guid&" + - "hosts=my-host&" + - "paths=/some/path" + payload := &payloads.RouteList{ + AppGUIDs: "a1,a2", + SpaceGUIDs: "s1,s2", + DomainGUIDs: "d1,d2", + Hosts: "h1,h2", + Paths: "p1,p2", + } + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(payload) }) It("filters routes by that", func() { Expect(routeRepo.ListRoutesCallCount()).To(Equal(1)) _, _, message := routeRepo.ListRoutesArgsForCall(0) - Expect(message.AppGUIDs).To(ConsistOf("my-app-guid")) - Expect(message.SpaceGUIDs).To(ConsistOf("my-space-guid")) - Expect(message.DomainGUIDs).To(ConsistOf("my-domain-guid")) - Expect(message.Hosts).To(ConsistOf("my-host")) - Expect(message.Paths).To(ConsistOf("/some/path")) - - Expect(rr).To(HaveHTTPStatus(http.StatusOK)) - Expect(rr).To(HaveHTTPBody( - MatchJSONPath( - "$.pagination.first.href", - "https://api.example.org/v3/routes?"+ - "app_guids=my-app-guid&"+ - "space_guids=my-space-guid&"+ - "domain_guids=my-domain-guid&"+ - "hosts=my-host&"+ - "paths=/some/path", - ), - )) + Expect(message.AppGUIDs).To(ConsistOf("a1", "a2")) + Expect(message.SpaceGUIDs).To(ConsistOf("s1", "s2")) + Expect(message.DomainGUIDs).To(ConsistOf("d1", "d2")) + Expect(message.Hosts).To(ConsistOf("h1", "h2")) + Expect(message.Paths).To(ConsistOf("p1", "p2")) }) }) @@ -246,11 +245,11 @@ var _ = Describe("Route", func() { When("invalid query parameters are provided", func() { BeforeEach(func() { - requestPath += "?foo=my-app-guid" + requestValidator.DecodeAndValidateURLValuesReturns(errors.New("boom!")) }) - It("returns an Unknown key error", func() { - expectUnknownKeyError("The query parameter is invalid: Valid parameters are: .*") + It("returns an error", func() { + expectUnknownError() }) }) }) @@ -273,33 +272,34 @@ var _ = Describe("Route", func() { UpdatedAt: "update-time", }, nil) - requestBody = `{ - "host": "test-route-host", - "path": "/test-route-path", - "relationships": { - "domain": { - "data": { - "guid": "test-domain-guid" - } + requestBody = `doesn't matter` + + payload := payloads.RouteCreate{ + Host: "test-route-host", + Path: "/test-route-path", + Relationships: &payloads.RouteRelationships{ + Domain: payloads.Relationship{ + Data: &payloads.RelationshipData{GUID: "test-domain-guid"}, }, - "space": { - "data": { - "guid": "test-space-guid" - } - } - }, - "metadata": { - "labels": { - "label-key": "label-val" + Space: payloads.Relationship{ + Data: &payloads.RelationshipData{GUID: "test-space-guid"}, }, - "annotations": { - "annotation-key": "annotation-val" - } - } - }` + }, + Metadata: payloads.Metadata{ + Labels: map[string]string{"label-key": "label-val"}, + Annotations: map[string]string{"annotation-key": "annotation-val"}, + }, + } + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidateJSONPayloadStub(&payload) }) It("creates the route", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + bodyBytes, err := io.ReadAll(actualReq.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(bodyBytes)).To(Equal(requestBody)) + Expect(spaceRepo.GetSpaceCallCount()).To(Equal(1)) _, actualAuthInfo, actualSpaceGUID := spaceRepo.GetSpaceArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -330,78 +330,11 @@ var _ = Describe("Route", func() { When("the request body is invalid JSON", func() { BeforeEach(func() { - requestBody = `{` - }) - - It("returns a message parse erorr", func() { - expectBadRequestError() - }) - }) - - When("the request body includes an unknown description field", func() { - BeforeEach(func() { - requestBody = `{"description" : "Invalid Request"}` - }) - - It("returns an error", func() { - expectUnprocessableEntityError(`invalid request body: json: unknown field "description"`) - }) - }) - - When("the host is not a string", func() { - BeforeEach(func() { - requestBody = `{ - "host": 12345, - "relationships": { - "space": { - "data": { - "guid": "2f35885d-0c9d-4423-83ad-fd05066f8576" - } - } - } - }` - }) - - It("returns an error", func() { - expectUnprocessableEntityError("Host must be a string") - }) - }) - - When("the request body is missing the domain relationship", func() { - BeforeEach(func() { - requestBody = `{ - "host": "test-route-host", - "relationships": { - "space": { - "data": { - "guid": "0c78dd5d-c723-4f2e-b168-df3c3e1d0806" - } - } - } - }` + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("boom")) }) It("returns an error", func() { - expectUnprocessableEntityError("Data is a required field") - }) - }) - - When("the request body is missing the space relationship", func() { - BeforeEach(func() { - requestBody = `{ - "host": "test-route-host", - "relationships": { - "domain": { - "data": { - "guid": "0b78dd5d-c723-4f2e-b168-df3c3e1d0806" - } - } - } - }` - }) - - It("returns an error", func() { - expectUnprocessableEntityError("Data is a required field") + expectUnknownError() }) }) @@ -490,29 +423,30 @@ var _ = Describe("Route", func() { }, }, nil) - requestBody = `{ - "metadata": { - "labels": { - "env": "production", - "foo.example.com/my-identifier": "aruba" - }, - "annotations": { - "hello": "there", - "foo.example.com/lorem-ipsum": "Lorem ipsum." - } - } - }` + requestBody = `doesn't matter` + + payload := payloads.RoutePatch{ + Metadata: payloads.MetadataPatch{ + Annotations: map[string]*string{"a": tools.PtrTo("av")}, + Labels: map[string]*string{"l": tools.PtrTo("lv")}, + }, + } + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidateJSONPayloadStub(&payload) }) It("patches the route", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + bodyBytes, err := io.ReadAll(actualReq.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(bodyBytes)).To(Equal(requestBody)) + Expect(routeRepo.PatchRouteMetadataCallCount()).To(Equal(1)) _, _, msg := routeRepo.PatchRouteMetadataArgsForCall(0) Expect(msg.RouteGUID).To(Equal("test-route-guid")) Expect(msg.SpaceGUID).To(Equal(spaceGUID)) - Expect(msg.Annotations).To(HaveKeyWithValue("hello", PointTo(Equal("there")))) - Expect(msg.Annotations).To(HaveKeyWithValue("foo.example.com/lorem-ipsum", PointTo(Equal("Lorem ipsum.")))) - Expect(msg.Labels).To(HaveKeyWithValue("env", PointTo(Equal("production")))) - Expect(msg.Labels).To(HaveKeyWithValue("foo.example.com/my-identifier", PointTo(Equal("aruba")))) + Expect(msg.Annotations).To(HaveKeyWithValue("a", PointTo(Equal("av")))) + Expect(msg.Labels).To(HaveKeyWithValue("l", PointTo(Equal("lv")))) Expect(rr).To(HaveHTTPStatus(http.StatusOK)) Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) @@ -554,71 +488,13 @@ var _ = Describe("Route", func() { }) }) - When("a label is invalid", func() { - When("the prefix is cloudfoundry.org", func() { - BeforeEach(func() { - requestBody = `{ - "metadata": { - "labels": { - "cloudfoundry.org/test": "production" - } - } - }` - }) - - It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`) - }) - }) - - When("the prefix is a subdomain of cloudfoundry.org", func() { - BeforeEach(func() { - requestBody = `{ - "metadata": { - "labels": { - "korifi.cloudfoundry.org/test": "production" - } - } - }` - }) - - It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`) - }) - }) - }) - - When("an annotation is invalid", func() { - When("the prefix is cloudfoundry.org", func() { - BeforeEach(func() { - requestBody = `{ - "metadata": { - "annotations": { - "cloudfoundry.org/test": "there" - } - } - }` - }) - - It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`) - }) - - When("the prefix is a subdomain of cloudfoundry.org", func() { - BeforeEach(func() { - requestBody = `{ - "metadata": { - "annotations": { - "korifi.cloudfoundry.org/test": "there" - } - } - }` - }) - - It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`) - }) - }) + When("the request is invalid", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("boom")) + }) + + It("returns an error", func() { + expectUnknownError() }) }) }) @@ -693,28 +569,37 @@ var _ = Describe("Route", func() { requestMethod = http.MethodPost requestPath = "/v3/routes/test-route-guid/destinations" - requestBody = `{ - "destinations": [ + requestBody = `doesn't matter` + + payload := payloads.RouteDestinationCreate{ + Destinations: []payloads.RouteDestination{ { - "app": { - "guid": "app-1-guid" - } + App: payloads.AppResource{ + GUID: "app-1-guid", + }, }, { - "app": { - "guid": "app-2-guid", - "process": { - "type": "queue" - } + App: payloads.AppResource{ + GUID: "app-2-guid", + Process: &payloads.DestinationAppProcess{ + Type: "queue", + }, }, - "port": 1234, - "protocol": "http1" - } - ] - }` + Port: tools.PtrTo(1234), + Protocol: tools.PtrTo("http1"), + }, + }, + } + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidateJSONPayloadStub(&payload) }) It("adds the destinations to the route", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + bodyBytes, err := io.ReadAll(actualReq.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(bodyBytes)).To(Equal(requestBody)) + Expect(routeRepo.GetRouteCallCount()).To(Equal(1)) _, actualAuthInfo, actualRouteGUID := routeRepo.GetRouteArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -797,86 +682,13 @@ var _ = Describe("Route", func() { }) }) - When("JSON is invalid", func() { + When("request is invalid", func() { BeforeEach(func() { - requestBody = `{` + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("boom")) }) It("returns an error", func() { - expectBadRequestError() - }) - }) - - When("app is missing", func() { - BeforeEach(func() { - requestBody = `{ - "destinations": [ - { "port": 9000, "protocol": "http1" } - ] - }` - }) - - It("returns an error and doesn't add the destinations", func() { - Expect(routeRepo.AddDestinationsToRouteCallCount()).To(Equal(0)) - expectUnprocessableEntityError("App is a required field") - }) - }) - - When("app GUID is missing", func() { - BeforeEach(func() { - requestBody = `{ - "destinations": [ - { "app": {}, "port": 9000, "protocol": "http1" } - ] - }` - }) - - It("returns an error and doesn't add the destinations", func() { - Expect(routeRepo.AddDestinationsToRouteCallCount()).To(Equal(0)) - expectUnprocessableEntityError("GUID is a required field") - }) - }) - - When("process type is missing", func() { - BeforeEach(func() { - requestBody = `{ - "destinations": [ - { - "app": { - "guid": "01856e12-8ee8-11e9-98a5-bb397dbc818f", - "process": {} - }, - "port": 9000, - "protocol": "http1" - } - ] - }` - }) - - It("returns an error and doesn't add the destinations", func() { - Expect(routeRepo.AddDestinationsToRouteCallCount()).To(Equal(0)) - expectUnprocessableEntityError("Type is a required field") - }) - }) - - When("destination protocol is not http1", func() { - BeforeEach(func() { - requestBody = `{ - "destinations": [ - { - "app": { - "guid": "01856e12-8ee8-11e9-98a5-bb397dbc818f" - }, - "port": 9000, - "protocol": "http" - } - ] - }` - }) - - It("returns an error and doesn't add the destinations", func() { - expectUnprocessableEntityError(regexp.QuoteMeta("Protocol must be one of [http1]")) - Expect(routeRepo.AddDestinationsToRouteCallCount()).To(Equal(0)) + expectUnknownError() }) }) }) diff --git a/api/handlers/service_binding.go b/api/handlers/service_binding.go index 62e6a5c56..6383aab37 100644 --- a/api/handlers/service_binding.go +++ b/api/handlers/service_binding.go @@ -26,7 +26,7 @@ type ServiceBinding struct { serviceBindingRepo CFServiceBindingRepository serviceInstanceRepo CFServiceInstanceRepository serverURL url.URL - decoderValidator *DecoderValidator + requestValidator RequestValidator } //counterfeiter:generate -o fake -fake-name CFServiceBindingRepository . CFServiceBindingRepository @@ -38,13 +38,13 @@ type CFServiceBindingRepository interface { UpdateServiceBinding(context.Context, authorization.Info, repositories.UpdateServiceBindingMessage) (repositories.ServiceBindingRecord, error) } -func NewServiceBinding(serverURL url.URL, serviceBindingRepo CFServiceBindingRepository, appRepo CFAppRepository, serviceInstanceRepo CFServiceInstanceRepository, decoderValidator *DecoderValidator) *ServiceBinding { +func NewServiceBinding(serverURL url.URL, serviceBindingRepo CFServiceBindingRepository, appRepo CFAppRepository, serviceInstanceRepo CFServiceInstanceRepository, requestValidator RequestValidator) *ServiceBinding { return &ServiceBinding{ appRepo: appRepo, serviceInstanceRepo: serviceInstanceRepo, serviceBindingRepo: serviceBindingRepo, serverURL: serverURL, - decoderValidator: decoderValidator, + requestValidator: requestValidator, } } @@ -53,7 +53,7 @@ func (h *ServiceBinding) create(r *http.Request) (*routing.Response, error) { logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-binding.create") var payload payloads.ServiceBindingCreate - if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } @@ -102,13 +102,8 @@ func (h *ServiceBinding) list(r *http.Request) (*routing.Response, error) { authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-binding.list") - if err := r.ParseForm(); err != nil { - return nil, apierrors.LogAndReturn(logger, apierrors.NewUnprocessableEntityError(err, "unable to parse query"), "Unable to parse request query parameters") - } - listFilter := new(payloads.ServiceBindingList) - err := payloads.Decode(listFilter, r.Form) - if err != nil { + if err := h.requestValidator.DecodeAndValidateURLValues(r, listFilter); err != nil { return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters") } @@ -141,7 +136,7 @@ func (h *ServiceBinding) update(r *http.Request) (*routing.Response, error) { // serviceBindingGUID := routing.URLParam(r, "guid") var payload payloads.ServiceBindingUpdate - if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } diff --git a/api/handlers/service_binding_test.go b/api/handlers/service_binding_test.go index 71772f527..548216170 100644 --- a/api/handlers/service_binding_test.go +++ b/api/handlers/service_binding_test.go @@ -2,13 +2,14 @@ package handlers_test import ( "errors" + "io" "net/http" - "regexp" "strings" apierrors "code.cloudfoundry.org/korifi/api/errors" . "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/handlers/fake" + "code.cloudfoundry.org/korifi/api/payloads" "code.cloudfoundry.org/korifi/api/repositories" . "code.cloudfoundry.org/korifi/tests/matchers" "code.cloudfoundry.org/korifi/tools" @@ -26,6 +27,7 @@ var _ = Describe("ServiceBinding", func() { serviceBindingRepo *fake.CFServiceBindingRepository appRepo *fake.CFAppRepository serviceInstanceRepo *fake.CFServiceInstanceRepository + requestValidator *fake.RequestValidator ) BeforeEach(func() { @@ -46,15 +48,14 @@ var _ = Describe("ServiceBinding", func() { SpaceGUID: "space-guid", }, nil) - decoderValidator, err := NewDefaultDecoderValidator() - Expect(err).NotTo(HaveOccurred()) + requestValidator = new(fake.RequestValidator) apiHandler := NewServiceBinding( *serverURL, serviceBindingRepo, appRepo, serviceInstanceRepo, - decoderValidator, + requestValidator, ) routerBuilder.LoadRoutes(apiHandler) }) @@ -67,31 +68,42 @@ var _ = Describe("ServiceBinding", func() { }) Describe("POST /v3/service_credential_bindings", func() { + var payload payloads.ServiceBindingCreate + BeforeEach(func() { requestMethod = http.MethodPost requestPath = "/v3/service_credential_bindings" - requestBody = `{ - "type": "app", - "relationships": { - "app": { - "data": { - "guid": "app-guid" - } - }, - "service_instance": { - "data": { - "guid": "service-instance-guid" - } - } - } - }` + requestBody = "doesn't matter" serviceBindingRepo.CreateServiceBindingReturns(repositories.ServiceBindingRecord{ GUID: "service-binding-guid", }, nil) + + payload = payloads.ServiceBindingCreate{ + Relationships: &payloads.ServiceBindingRelationships{ + App: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "app-guid", + }, + }, + ServiceInstance: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "service-instance-guid", + }, + }, + }, + Type: "app", + } + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidateJSONPayloadStub(&payload) }) It("creates a service binding", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + bodyBytes, err := io.ReadAll(actualReq.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(bodyBytes)).To(Equal(requestBody)) + Expect(appRepo.GetAppCallCount()).To(Equal(1)) _, actualAuthInfo, actualAppGUID := appRepo.GetAppArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -119,88 +131,11 @@ var _ = Describe("ServiceBinding", func() { When("the request body is invalid json", func() { BeforeEach(func() { - requestBody = "{" + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("boom")) }) - It("returns an error and doesn't create the service binding", func() { - expectBadRequestError() - Expect(serviceBindingRepo.CreateServiceBindingCallCount()).To(Equal(0)) - }) - }) - - When(`the type is "key"`, func() { - BeforeEach(func() { - requestBody = `{ - "type": "key", - "relationships": { - "app": { - "data": { - "guid": "app-guid" - } - }, - "service_instance": { - "data": { - "guid": "service-instance-guid" - } - } - } - }` - }) - - It("returns an error and doesn't create the ServiceBinding", func() { - expectUnprocessableEntityError(regexp.QuoteMeta("Type must be one of [app]")) - Expect(serviceBindingRepo.CreateServiceBindingCallCount()).To(Equal(0)) - }) - }) - - When("all relationships are missing", func() { - BeforeEach(func() { - requestBody = `{ "type": "app" }` - }) - - It("returns an error and doesn't create the ServiceBinding", func() { - expectUnprocessableEntityError(`Relationships is a required field`) - Expect(serviceBindingRepo.CreateServiceBindingCallCount()).To(Equal(0)) - }) - }) - - When("the app relationship is missing", func() { - BeforeEach(func() { - requestBody = `{ - "type": "app", - "relationships": { - "service_instance": { - "data": { - "guid": "service-instance-guid" - } - } - } - }` - }) - - It("returns an error and doesn't create the ServiceBinding", func() { - expectUnprocessableEntityError(`App is a required field`) - Expect(serviceBindingRepo.CreateServiceBindingCallCount()).To(Equal(0)) - }) - }) - - When("the service_instance relationship is missing", func() { - BeforeEach(func() { - requestBody = `{ - "type": "app", - "relationships": { - "app": { - "data": { - "guid": "app-guid" - } - } - } - }` - }) - - It("returns an error and doesn't create the ServiceBinding", func() { - expectUnprocessableEntityError("ServiceInstance is a required field") - Expect(serviceBindingRepo.CreateServiceBindingCallCount()).To(Equal(0)) + It("returns an error", func() { + expectUnknownError() }) }) @@ -290,20 +225,35 @@ var _ = Describe("ServiceBinding", func() { BeforeEach(func() { requestMethod = http.MethodGet requestBody = "" - requestPath = "/v3/service_credential_bindings" + requestPath = "/v3/service_credential_bindings?foo=bar" serviceBindingRepo.ListServiceBindingsReturns([]repositories.ServiceBindingRecord{ {GUID: "service-binding-guid", AppGUID: "app-guid"}, }, nil) appRepo.ListAppsReturns([]repositories.AppRecord{{Name: "some-app-name"}}, nil) + + payload := payloads.ServiceBindingList{ + AppGUIDs: "a1,a2", + ServiceInstanceGUIDs: "s1,s2", + } + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payload) }) It("returns the list of ServiceBindings", func() { + Expect(requestValidator.DecodeAndValidateURLValuesCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateURLValuesArgsForCall(0) + Expect(actualReq.URL.String()).To(HaveSuffix(requestPath)) + + Expect(serviceBindingRepo.ListServiceBindingsCallCount()).To(Equal(1)) + _, _, message := serviceBindingRepo.ListServiceBindingsArgsForCall(0) + Expect(message.AppGUIDs).To(ConsistOf([]string{"a1", "a2"})) + Expect(message.ServiceInstanceGUIDs).To(ConsistOf([]string{"s1", "s2"})) + Expect(rr).To(HaveHTTPStatus(http.StatusOK)) Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) Expect(rr).To(HaveHTTPBody(SatisfyAll( MatchJSONPath("$.pagination.total_results", BeEquivalentTo(1)), - MatchJSONPath("$.pagination.first.href", "https://api.example.org/v3/service_credential_bindings"), + MatchJSONPath("$.pagination.first.href", "https://api.example.org/v3/service_credential_bindings?foo=bar"), MatchJSONPath("$.resources[0].guid", "service-binding-guid"), ))) }) @@ -320,7 +270,10 @@ var _ = Describe("ServiceBinding", func() { When("an include=app query parameter is specified", func() { BeforeEach(func() { - requestPath += "?include=app" + payload := payloads.ServiceBindingList{ + Include: "app", + } + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payload) }) It("includes app data in the response", func() { @@ -332,47 +285,13 @@ var _ = Describe("ServiceBinding", func() { }) }) - When("an app_guids query parameter is provided", func() { - BeforeEach(func() { - requestPath += "?app_guids=1,2,3" - }) - - It("passes the list of app GUIDs to the repository", func() { - Expect(serviceBindingRepo.ListServiceBindingsCallCount()).To(Equal(1)) - _, _, message := serviceBindingRepo.ListServiceBindingsArgsForCall(0) - Expect(message.AppGUIDs).To(ConsistOf([]string{"1", "2", "3"})) - }) - }) - - When("a service_instance_guids query parameter is provided", func() { - BeforeEach(func() { - requestPath += "?service_instance_guids=1,2,3" - }) - - It("passes the list of service instance GUIDs to the repository", func() { - Expect(serviceBindingRepo.ListServiceBindingsCallCount()).To(Equal(1)) - _, _, message := serviceBindingRepo.ListServiceBindingsArgsForCall(0) - Expect(message.ServiceInstanceGUIDs).To(ConsistOf([]string{"1", "2", "3"})) - }) - }) - - When("a type query parameter is provided", func() { - BeforeEach(func() { - requestPath += "?type=app" - }) - - It("returns success", func() { - Expect(rr).To(HaveHTTPStatus(http.StatusOK)) - }) - }) - - When("invalid query parameters are provided", func() { + When("decoding URL params fails", func() { BeforeEach(func() { - requestPath += "?foo=bar" + requestValidator.DecodeAndValidateURLValuesReturns(errors.New("boom")) }) - It("returns an Unknown key error", func() { - expectUnknownKeyError("The query parameter is invalid: Valid parameters are: .*") + It("returns an error", func() { + expectUnknownError() }) }) }) @@ -397,19 +316,28 @@ var _ = Describe("ServiceBinding", func() { BeforeEach(func() { requestMethod = "PATCH" requestPath = "/v3/service_credential_bindings/service-binding-guid" - requestBody = `{ - "metadata": { - "labels": { "foo": "bar" }, - "annotations": { "bar": "baz" } - } - }` + requestBody = "doesn't matter" serviceBindingRepo.UpdateServiceBindingReturns(repositories.ServiceBindingRecord{ GUID: "service-binding-guid", }, nil) + + payload := payloads.ServiceBindingUpdate{ + Metadata: payloads.MetadataPatch{ + Labels: map[string]*string{"foo": tools.PtrTo("bar")}, + Annotations: map[string]*string{"bar": tools.PtrTo("baz")}, + }, + } + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidateJSONPayloadStub(&payload) }) It("updates the service binding", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + bodyBytes, err := io.ReadAll(actualReq.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(bodyBytes)).To(Equal(requestBody)) + Expect(rr).To(HaveHTTPStatus(http.StatusOK)) Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) Expect(rr).To(HaveHTTPBody(SatisfyAll( @@ -431,71 +359,11 @@ var _ = Describe("ServiceBinding", func() { When("the payload cannot be decoded", func() { BeforeEach(func() { - requestBody = `{"one":"two"}` + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("boom")) }) It("returns an error", func() { - expectUnprocessableEntityError("invalid request body: json: unknown field \"one\"") - }) - }) - - When("a label is invalid", func() { - When("the prefix is cloudfoundry.org", func() { - BeforeEach(func() { - requestBody = `{ - "metadata": { - "labels": { "cloudfoundry.org/test": "production" } - } - }` - }) - - It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`) - }) - }) - - When("the prefix is a subdomain of cloudfoundry.org", func() { - BeforeEach(func() { - requestBody = `{ - "metadata": { - "labels": { "korifi.cloudfoundry.org/test": "production" } - } - }` - }) - - It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`) - }) - }) - }) - - When("an annotation is invalid", func() { - When("the prefix is cloudfoundry.org", func() { - BeforeEach(func() { - requestBody = `{ - "metadata": { - "annotations": { "cloudfoundry.org/test": "production" } - } - }` - }) - - It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`) - }) - }) - - When("the prefix is a subdomain of cloudfoundry.org", func() { - BeforeEach(func() { - requestBody = `{ - "metadata": { - "annotations": { "korifi.cloudfoundry.org/test": "production" } - } - }` - }) - - It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`) - }) + expectUnknownError() }) }) diff --git a/api/handlers/validator.go b/api/handlers/validator.go index ccb46a1ac..26819bfc2 100644 --- a/api/handlers/validator.go +++ b/api/handlers/validator.go @@ -11,7 +11,6 @@ import ( "strings" apierrors "code.cloudfoundry.org/korifi/api/errors" - "code.cloudfoundry.org/korifi/api/payloads" "github.com/go-playground/locales/en" ut "github.com/go-playground/universal-translator" @@ -227,28 +226,6 @@ func wireValidator() (*validator.Validate, ut.Translator, error) { return nil, nil, err } - v.RegisterStructValidation(checkRoleTypeAndOrgSpace, payloads.RoleCreate{}) - - err = v.RegisterTranslation("cannot_have_both_org_and_space_set", trans, func(ut ut.Translator) error { - return ut.Add("cannot_have_both_org_and_space_set", "Cannot pass both 'organization' and 'space' in a create role request", false) - }, func(ut ut.Translator, fe validator.FieldError) string { - t, _ := ut.T("cannot_have_both_org_and_space_set", fe.Field()) - return t - }) - if err != nil { - return nil, nil, err - } - - err = v.RegisterTranslation("valid_role", trans, func(ut ut.Translator) error { - return ut.Add("valid_role", "{0} is not a valid role", false) - }, func(ut ut.Translator, fe validator.FieldError) string { - t, _ := ut.T("valid_role", fmt.Sprintf("%v", fe.Value())) - return t - }) - if err != nil { - return nil, nil, err - } - return v, trans, nil } @@ -265,43 +242,6 @@ func registerDefaultTranslator(v *validator.Validate) (ut.Translator, error) { return trans, nil } -func checkRoleTypeAndOrgSpace(sl validator.StructLevel) { - roleCreate := sl.Current().Interface().(payloads.RoleCreate) - - if roleCreate.Relationships.Organization != nil && roleCreate.Relationships.Space != nil { - sl.ReportError(roleCreate.Relationships.Organization, "relationships.organization", "Organization", "cannot_have_both_org_and_space_set", "") - } - - roleType := RoleName(roleCreate.Type) - - switch roleType { - case RoleSpaceManager: - fallthrough - case RoleSpaceAuditor: - fallthrough - case RoleSpaceDeveloper: - fallthrough - case RoleSpaceSupporter: - if roleCreate.Relationships.Space == nil { - sl.ReportError(roleCreate.Relationships.Space, "relationships.space", "Space", "required", "") - } - case RoleOrganizationUser: - fallthrough - case RoleOrganizationAuditor: - fallthrough - case RoleOrganizationManager: - fallthrough - case RoleOrganizationBillingManager: - if roleCreate.Relationships.Organization == nil { - sl.ReportError(roleCreate.Relationships.Organization, "relationships.organization", "Organization", "required", "") - } - - case RoleName(""): - default: - sl.ReportError(roleCreate.Type, "type", "Role type", "valid_role", "") - } -} - func serviceInstanceTagLength(fl validator.FieldLevel) bool { tags, ok := fl.Field().Interface().([]string) if !ok { diff --git a/api/payloads/app.go b/api/payloads/app.go index a7f276c80..7253b8fa1 100644 --- a/api/payloads/app.go +++ b/api/payloads/app.go @@ -22,7 +22,7 @@ var DefaultLifecycleConfig = config.DefaultLifecycleConfig{ type AppCreate struct { Name string `json:"name"` EnvironmentVariables map[string]string `json:"environment_variables"` - Relationships AppRelationships `json:"relationships"` + Relationships *AppRelationships `json:"relationships"` Lifecycle *Lifecycle `json:"lifecycle"` Metadata Metadata `json:"metadata"` } @@ -30,19 +30,19 @@ type AppCreate struct { func (c AppCreate) Validate() error { return validation.ValidateStruct(&c, validation.Field(&c.Name, payload_validation.StrictlyRequired), - validation.Field(&c.Relationships, payload_validation.StrictlyRequired), + validation.Field(&c.Relationships, validation.NotNil), validation.Field(&c.Lifecycle), validation.Field(&c.Metadata), ) } type AppRelationships struct { - Space Relationship `json:"space"` + Space *Relationship `json:"space"` } func (r AppRelationships) Validate() error { return validation.ValidateStruct(&r, - validation.Field(&r.Space, payload_validation.StrictlyRequired), + validation.Field(&r.Space, validation.NotNil), ) } diff --git a/api/payloads/app_test.go b/api/payloads/app_test.go index e4c619ab5..e446231e1 100644 --- a/api/payloads/app_test.go +++ b/api/payloads/app_test.go @@ -40,8 +40,8 @@ var _ = Describe("App payload validation", func() { BeforeEach(func() { payload = payloads.AppCreate{ Name: "my-app", - Relationships: payloads.AppRelationships{ - Space: payloads.Relationship{ + Relationships: &payloads.AppRelationships{ + Space: &payloads.Relationship{ Data: &payloads.RelationshipData{ GUID: "app-guid", }, @@ -83,11 +83,21 @@ var _ = Describe("App payload validation", func() { When("relationships are not set", func() { BeforeEach(func() { - payload.Relationships = payloads.AppRelationships{} + payload.Relationships = nil }) It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(validatorErr, "relationships cannot be blank") + expectUnprocessableEntityError(validatorErr, "relationships is required") + }) + }) + + When("relationships space is not set", func() { + BeforeEach(func() { + payload.Relationships.Space = nil + }) + + It("returns an unprocessable entity error", func() { + expectUnprocessableEntityError(validatorErr, "relationships.space is required") }) }) diff --git a/api/payloads/deployment.go b/api/payloads/deployment.go index 61264bf72..1aaa57c09 100644 --- a/api/payloads/deployment.go +++ b/api/payloads/deployment.go @@ -9,12 +9,12 @@ type DropletGUID struct { } type DeploymentCreate struct { - Droplet DropletGUID `json:"droplet"` - Relationships DeploymentRelationships `json:"relationships" validate:"required"` + Droplet DropletGUID `json:"droplet"` + Relationships *DeploymentRelationships `json:"relationships" validate:"required"` } type DeploymentRelationships struct { - App Relationship `json:"app" validate:"required"` + App *Relationship `json:"app" validate:"required"` } func (c *DeploymentCreate) ToMessage() repositories.CreateDeploymentMessage { diff --git a/api/payloads/deployment_test.go b/api/payloads/deployment_test.go index d6a1a1cc4..300cd2146 100644 --- a/api/payloads/deployment_test.go +++ b/api/payloads/deployment_test.go @@ -17,8 +17,8 @@ var _ = Describe("DeploymentCreate", func() { Droplet: payloads.DropletGUID{ Guid: "the-droplet", }, - Relationships: payloads.DeploymentRelationships{ - App: payloads.Relationship{ + Relationships: &payloads.DeploymentRelationships{ + App: &payloads.Relationship{ Data: &payloads.RelationshipData{ GUID: "the-app", }, @@ -57,13 +57,23 @@ var _ = Describe("DeploymentCreate", func() { }) }) - When("the app relationship is not specified", func() { + When("the relationship is not specified", func() { BeforeEach(func() { - createDeployment.Relationships = payloads.DeploymentRelationships{} + createDeployment.Relationships = nil }) - It("says app data is required", func() { - expectUnprocessableEntityError(validatorErr, "Data is a required field") + It("says relationships is required", func() { + expectUnprocessableEntityError(validatorErr, "Relationships is a required field") + }) + }) + + When("the relationship app is not specified", func() { + BeforeEach(func() { + createDeployment.Relationships.App = nil + }) + + It("says app is required", func() { + expectUnprocessableEntityError(validatorErr, "App is a required field") }) }) diff --git a/api/payloads/destination.go b/api/payloads/destination.go deleted file mode 100644 index f6e96744f..000000000 --- a/api/payloads/destination.go +++ /dev/null @@ -1,58 +0,0 @@ -package payloads - -import ( - "code.cloudfoundry.org/korifi/api/repositories" - korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" -) - -type DestinationListCreate struct { - Destinations []Destination `json:"destinations" validate:"required,dive"` -} - -type Destination struct { - App *AppResource `json:"app" validate:"required"` - Port *int `json:"port"` - Protocol *string `json:"protocol" validate:"omitempty,oneof=http1"` -} - -type AppResource struct { - GUID string `json:"guid" validate:"required"` - Process *DestinationAppProcess `json:"process"` -} - -type DestinationAppProcess struct { - Type string `json:"type" validate:"required"` -} - -func (dc DestinationListCreate) ToMessage(routeRecord repositories.RouteRecord) repositories.AddDestinationsToRouteMessage { - addDestinations := make([]repositories.DestinationMessage, 0, len(dc.Destinations)) - for _, destination := range dc.Destinations { - processType := korifiv1alpha1.ProcessTypeWeb - if destination.App.Process != nil { - processType = destination.App.Process.Type - } - - port := 8080 - if destination.Port != nil { - port = *destination.Port - } - - protocol := "http1" - if destination.Protocol != nil { - protocol = *destination.Protocol - } - - addDestinations = append(addDestinations, repositories.DestinationMessage{ - AppGUID: destination.App.GUID, - ProcessType: processType, - Port: port, - Protocol: protocol, - }) - } - return repositories.AddDestinationsToRouteMessage{ - RouteGUID: routeRecord.GUID, - SpaceGUID: routeRecord.SpaceGUID, - ExistingDestinations: routeRecord.Destinations, - NewDestinations: addDestinations, - } -} diff --git a/api/payloads/domain_test.go b/api/payloads/domain_test.go index 786766a8f..1b1fb610c 100644 --- a/api/payloads/domain_test.go +++ b/api/payloads/domain_test.go @@ -76,7 +76,7 @@ var _ = Describe("DomainCreate", func() { }) It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "data cannot be blank") + expectUnprocessableEntityError(validatorErr, "data is required") }) }) }) diff --git a/api/payloads/lifecycle.go b/api/payloads/lifecycle.go index b936d622c..4402f363b 100644 --- a/api/payloads/lifecycle.go +++ b/api/payloads/lifecycle.go @@ -1,30 +1,28 @@ package payloads import ( - payload_validation "code.cloudfoundry.org/korifi/api/payloads/validation" "github.com/jellydator/validation" ) type Lifecycle struct { - Type string `json:"type" validate:"required"` - Data LifecycleData `json:"data" validate:"required"` + Type string `json:"type"` + Data *LifecycleData `json:"data"` } func (l Lifecycle) Validate() error { return validation.ValidateStruct(&l, - validation.Field(&l.Type, payload_validation.StrictlyRequired), - validation.Field(&l.Data, payload_validation.StrictlyRequired), + validation.Field(&l.Type, validation.Required), + validation.Field(&l.Data, validation.NotNil), ) } type LifecycleData struct { - Buildpacks []string `json:"buildpacks" validate:"required"` - Stack string `json:"stack" validate:"required"` + Buildpacks []string `json:"buildpacks"` + Stack string `json:"stack"` } func (d LifecycleData) Validate() error { return validation.ValidateStruct(&d, - validation.Field(&d.Buildpacks, payload_validation.StrictlyRequired), - validation.Field(&d.Stack, payload_validation.StrictlyRequired), + validation.Field(&d.Stack, validation.Required), ) } diff --git a/api/payloads/lifecycle_test.go b/api/payloads/lifecycle_test.go index 400449b50..bd967aa44 100644 --- a/api/payloads/lifecycle_test.go +++ b/api/payloads/lifecycle_test.go @@ -22,7 +22,7 @@ var _ = Describe("Lifecycle", func() { payload = payloads.Lifecycle{ Type: "buildpack", - Data: payloads.LifecycleData{ + Data: &payloads.LifecycleData{ Buildpacks: []string{"foo", "bar"}, Stack: "baz", }, @@ -50,23 +50,23 @@ var _ = Describe("Lifecycle", func() { }) }) - When("stack is not set", func() { + When("lifecycle data is not set", func() { BeforeEach(func() { - payload.Data.Stack = "" + payload.Data = nil }) It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "data.stack cannot be blank") + expectUnprocessableEntityError(validatorErr, "data is required") }) }) - When("buildpacks are not set", func() { + When("stack is not set", func() { BeforeEach(func() { - payload.Data.Buildpacks = nil + payload.Data.Stack = "" }) It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "data.buildpacks cannot be blank") + expectUnprocessableEntityError(validatorErr, "data.stack cannot be blank") }) }) }) diff --git a/api/payloads/process.go b/api/payloads/process.go index 05eaf7842..5e0b35e30 100644 --- a/api/payloads/process.go +++ b/api/payloads/process.go @@ -5,12 +5,21 @@ import ( "code.cloudfoundry.org/korifi/api/payloads/parse" "code.cloudfoundry.org/korifi/api/repositories" + "github.com/jellydator/validation" ) type ProcessScale struct { - Instances *int `json:"instances" validate:"omitempty,gte=0"` - MemoryMB *int64 `json:"memory_in_mb" validate:"omitempty,gt=0"` - DiskMB *int64 `json:"disk_in_mb" validate:"omitempty,gt=0"` + Instances *int `json:"instances"` + MemoryMB *int64 `json:"memory_in_mb"` + DiskMB *int64 `json:"disk_in_mb"` +} + +func (p ProcessScale) Validate() error { + return validation.ValidateStruct(&p, + validation.Field(&p.Instances, validation.Min(0).Error("must be 0 or greater")), + validation.Field(&p.MemoryMB, validation.Min(1).Error("must be greater than 0")), + validation.Field(&p.DiskMB, validation.Min(1).Error("must be greater than 0")), + ) } type ProcessPatch struct { diff --git a/api/payloads/process_test.go b/api/payloads/process_test.go index cc40854c4..ea4e54250 100644 --- a/api/payloads/process_test.go +++ b/api/payloads/process_test.go @@ -70,7 +70,7 @@ var _ = Describe("Process payload validation", func() { }) It("returns an error", func() { - expectUnprocessableEntityError(validatorErr, "Instances must be 0 or greater") + expectUnprocessableEntityError(validatorErr, "instances must be 0 or greater") }) }) @@ -80,7 +80,7 @@ var _ = Describe("Process payload validation", func() { }) It("returns an error", func() { - expectUnprocessableEntityError(validatorErr, "MemoryMB must be greater than 0") + expectUnprocessableEntityError(validatorErr, "memory_in_mb must be greater than 0") }) }) @@ -90,7 +90,7 @@ var _ = Describe("Process payload validation", func() { }) It("returns an error", func() { - expectUnprocessableEntityError(validatorErr, "DiskMB must be greater than 0") + expectUnprocessableEntityError(validatorErr, "disk_in_mb must be greater than 0") }) }) }) diff --git a/api/payloads/relationship.go b/api/payloads/relationship.go index d2205cb2d..d1ce811b3 100644 --- a/api/payloads/relationship.go +++ b/api/payloads/relationship.go @@ -1,7 +1,6 @@ package payloads import ( - payload_validation "code.cloudfoundry.org/korifi/api/payloads/validation" "github.com/jellydator/validation" ) @@ -10,7 +9,9 @@ type Relationship struct { } func (r Relationship) Validate() error { - return validation.ValidateStruct(&r, validation.Field(&r.Data, payload_validation.StrictlyRequired)) + return validation.ValidateStruct(&r, + validation.Field(&r.Data, validation.NotNil), + ) } type RelationshipData struct { @@ -18,5 +19,7 @@ type RelationshipData struct { } func (r RelationshipData) Validate() error { - return validation.ValidateStruct(&r, validation.Field(&r.GUID, payload_validation.StrictlyRequired)) + return validation.ValidateStruct(&r, + validation.Field(&r.GUID, validation.Required), + ) } diff --git a/api/payloads/relationship_test.go b/api/payloads/relationship_test.go index 4dc7cf25b..f676c8cf4 100644 --- a/api/payloads/relationship_test.go +++ b/api/payloads/relationship_test.go @@ -38,7 +38,7 @@ var _ = Describe("Relationship", func() { }) It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "data cannot be blank") + expectUnprocessableEntityError(validatorErr, "data is required") }) }) diff --git a/api/payloads/role.go b/api/payloads/role.go index 909a30ca2..35553bfca 100644 --- a/api/payloads/role.go +++ b/api/payloads/role.go @@ -1,44 +1,53 @@ package payloads import ( + "context" "net/url" "strings" "code.cloudfoundry.org/korifi/api/authorization" + "code.cloudfoundry.org/korifi/api/payloads/validation" + jellidation "github.com/jellydator/validation" "code.cloudfoundry.org/korifi/api/repositories" rbacv1 "k8s.io/api/rbac/v1" ) -type ( - RoleCreate struct { - Type string `json:"type" validate:"required"` - Relationships RoleRelationships `json:"relationships" validate:"required"` - } +const ( + RoleAdmin = "admin" + RoleAdminReadOnly = "admin_read_only" + RoleGlobalAuditor = "global_auditor" + RoleOrganizationAuditor = "organization_auditor" + RoleOrganizationBillingManager = "organization_billing_manager" + RoleOrganizationManager = "organization_manager" + RoleOrganizationUser = "organization_user" + RoleSpaceAuditor = "space_auditor" + RoleSpaceDeveloper = "space_developer" + RoleSpaceManager = "space_manager" + RoleSpaceSupporter = "space_supporter" +) - UserRelationship struct { - Data UserRelationshipData `json:"data" validate:"required"` - } +type RoleCreate struct { + Type string `json:"type"` + Relationships RoleRelationships `json:"relationships"` +} - UserRelationshipData struct { - Username string `json:"username" validate:"required_without=GUID"` - GUID string `json:"guid" validate:"required_without=Username"` - } +type ctxType string - RoleRelationships struct { - User *UserRelationship `json:"user" validate:"required"` - Space *Relationship `json:"space"` - Organization *Relationship `json:"organization"` - } +const typeKey ctxType = "type" - RoleListFilter struct { - GUIDs map[string]bool - Types map[string]bool - SpaceGUIDs map[string]bool - OrgGUIDs map[string]bool - UserGUIDs map[string]bool - } -) +func (p RoleCreate) Validate() error { + ctx := context.WithValue(context.Background(), typeKey, p.Type) + + return jellidation.ValidateStructWithContext(ctx, &p, + jellidation.Field(&p.Type, + jellidation.Required, + validation.OneOf(RoleSpaceManager, RoleSpaceAuditor, RoleSpaceDeveloper, RoleSpaceSupporter, + RoleOrganizationUser, RoleOrganizationAuditor, RoleOrganizationManager, RoleOrganizationBillingManager), + ), + jellidation.Field(&p.Relationships, validation.StrictlyRequired), + ) +} func (p RoleCreate) ToMessage() repositories.CreateRoleMessage { record := repositories.CreateRoleMessage{ @@ -70,25 +79,84 @@ func (p RoleCreate) ToMessage() repositories.CreateRoleMessage { return record } -func (r *RoleListFilter) SupportedKeys() []string { +type RoleRelationships struct { + User UserRelationship `json:"user"` + Space *Relationship `json:"space"` + Organization *Relationship `json:"organization"` +} + +func (r RoleRelationships) ValidateWithContext(ctx context.Context) error { + roleType := ctx.Value(typeKey) + + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.User, validation.StrictlyRequired), + + jellidation.Field(&r.Space, + jellidation.When(r.Organization != nil && r.Organization.Data != nil && r.Organization.Data.GUID != "", + jellidation.Nil.Error("cannot pass both 'organization' and 'space' in a create role request"))), + + jellidation.Field(&r.Space, + jellidation.When( + roleType == RoleSpaceAuditor || roleType == RoleSpaceDeveloper || + roleType == RoleSpaceManager || roleType == RoleSpaceSupporter, + jellidation.NotNil, + validation.StrictlyRequired, + )), + + jellidation.Field(&r.Organization, + jellidation.When( + roleType == RoleOrganizationAuditor || roleType == RoleOrganizationBillingManager || + roleType == RoleOrganizationManager || roleType == RoleOrganizationUser, + jellidation.NotNil, + validation.StrictlyRequired, + )), + ) +} + +type UserRelationship struct { + Data UserRelationshipData `json:"data"` +} + +type UserRelationshipData struct { + Username string `json:"username"` + GUID string `json:"guid"` +} + +type RoleList struct { + GUIDs map[string]bool + Types map[string]bool + SpaceGUIDs map[string]bool + OrgGUIDs map[string]bool + UserGUIDs map[string]bool + OrderBy string +} + +func (r RoleList) SupportedKeys() []string { return []string{"guids", "types", "space_guids", "organization_guids", "user_guids", "order_by", "include", "per_page", "page"} } -func (r *RoleListFilter) DecodeFromURLValues(values url.Values) error { +func (r *RoleList) DecodeFromURLValues(values url.Values) error { r.GUIDs = commaSepToSet(values.Get("guids")) r.Types = commaSepToSet(values.Get("types")) r.SpaceGUIDs = commaSepToSet(values.Get("space_guids")) r.OrgGUIDs = commaSepToSet(values.Get("organization_guids")) r.UserGUIDs = commaSepToSet(values.Get("user_guids")) + r.OrderBy = values.Get("order_by") return nil } +func (r RoleList) Validate() error { + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.OrderBy, validation.OneOf("created_at", "updated_at", "-created_at", "-updated_at")), + ) +} + func commaSepToSet(in string) map[string]bool { - out := map[string]bool{} if in == "" { - return out + return nil } + out := map[string]bool{} for _, s := range strings.Split(in, ",") { out[s] = true } diff --git a/api/payloads/role_test.go b/api/payloads/role_test.go index dcf75c3ce..1783d0a92 100644 --- a/api/payloads/role_test.go +++ b/api/payloads/role_test.go @@ -8,7 +8,6 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "code.cloudfoundry.org/korifi/api/errors" - "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/payloads" . "github.com/onsi/ginkgo/v2" @@ -29,7 +28,7 @@ var _ = Describe("RoleCreate", func() { createPayload = payloads.RoleCreate{ Type: "space_manager", Relationships: payloads.RoleRelationships{ - User: &payloads.UserRelationship{ + User: payloads.UserRelationship{ Data: payloads.UserRelationshipData{ Username: "cf-service-account", }, @@ -53,33 +52,15 @@ var _ = Describe("RoleCreate", func() { Expect(roleCreate).To(PointTo(Equal(createPayload))) }) - When("the user name is missing", func() { - BeforeEach(func() { - createPayload.Relationships.User = nil - }) - - It("fails", func() { - Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("User is a required field")) - }) - }) - When("the user name and GUID are missing", func() { BeforeEach(func() { - createPayload.Relationships.User = &payloads.UserRelationship{ - Data: payloads.UserRelationshipData{ - Username: "", - GUID: "", - }, - } + createPayload.Relationships.User.Data.GUID = "" + createPayload.Relationships.User.Data.Username = "" }) It("fails", func() { Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(SatisfyAll( - ContainSubstring("Field validation for 'GUID' failed"), - ContainSubstring("Field validation for 'Username' failed"), - )) + Expect(apiError.Detail()).To(ContainSubstring("user cannot be blank")) }) }) @@ -90,29 +71,7 @@ var _ = Describe("RoleCreate", func() { It("fails", func() { Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("Type is a required field")) - }) - }) - - When("a space role is missing a space relationship", func() { - BeforeEach(func() { - createPayload.Relationships.Space = nil - }) - - It("fails", func() { - Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("relationships.space is a required field")) - }) - }) - - When("an org role is missing an org relationship", func() { - BeforeEach(func() { - createPayload.Type = "organization_manager" - }) - - It("fails", func() { - Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("relationships.organization is a required field")) + Expect(apiError.Detail()).To(ContainSubstring("type cannot be blank")) }) }) @@ -127,7 +86,7 @@ var _ = Describe("RoleCreate", func() { It("fails", func() { Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("Cannot pass both 'organization' and 'space' in a create role request")) + Expect(apiError.Detail()).To(ContainSubstring("cannot pass both 'organization' and 'space' in a create role request")) }) }) @@ -198,23 +157,53 @@ var _ = DescribeTable("Role org / space combination validation", } }, - Entry("org auditor w org", string(handlers.RoleOrganizationAuditor), "organization", true, ""), - Entry("org auditor w space", string(handlers.RoleOrganizationAuditor), "space", false, "relationships.organization is a required field"), - Entry("org billing manager w org", string(handlers.RoleOrganizationBillingManager), "organization", true, ""), - Entry("org billing manager w space", string(handlers.RoleOrganizationBillingManager), "space", false, "relationships.organization is a required field"), - Entry("org manager w org", string(handlers.RoleOrganizationManager), "organization", true, ""), - Entry("org manager w space", string(handlers.RoleOrganizationManager), "space", false, "relationships.organization is a required field"), - Entry("org user w org", string(handlers.RoleOrganizationUser), "organization", true, ""), - Entry("org user w space", string(handlers.RoleOrganizationUser), "space", false, "relationships.organization is a required field"), - - Entry("space auditor w org", string(handlers.RoleSpaceAuditor), "organization", false, "relationships.space is a required field"), - Entry("space auditor w space", string(handlers.RoleSpaceAuditor), "space", true, ""), - Entry("space developer w org", string(handlers.RoleSpaceDeveloper), "organization", false, "relationships.space is a required field"), - Entry("space developer w space", string(handlers.RoleSpaceDeveloper), "space", true, ""), - Entry("space manager w org", string(handlers.RoleSpaceManager), "organization", false, "relationships.space is a required field"), - Entry("space manager w space", string(handlers.RoleSpaceManager), "space", true, ""), - Entry("space supporter w org", string(handlers.RoleSpaceSupporter), "organization", false, "relationships.space is a required field"), - Entry("space supporter w space", string(handlers.RoleSpaceSupporter), "space", true, ""), - - Entry("invalid role name", "does-not-exist", "organization", false, "does-not-exist is not a valid role"), + Entry("org auditor w org", payloads.RoleOrganizationAuditor, "organization", true, ""), + Entry("org auditor w space", payloads.RoleOrganizationAuditor, "space", false, "relationships.organization is required"), + Entry("org billing manager w org", payloads.RoleOrganizationBillingManager, "organization", true, ""), + Entry("org billing manager w space", payloads.RoleOrganizationBillingManager, "space", false, "relationships.organization is required"), + Entry("org manager w org", payloads.RoleOrganizationManager, "organization", true, ""), + Entry("org manager w space", payloads.RoleOrganizationManager, "space", false, "relationships.organization is required"), + Entry("org user w org", payloads.RoleOrganizationUser, "organization", true, ""), + Entry("org user w space", payloads.RoleOrganizationUser, "space", false, "relationships.organization is required"), + + Entry("space auditor w org", payloads.RoleSpaceAuditor, "organization", false, "relationships.space is required"), + Entry("space auditor w space", payloads.RoleSpaceAuditor, "space", true, ""), + Entry("space developer w org", payloads.RoleSpaceDeveloper, "organization", false, "relationships.space is required"), + Entry("space developer w space", payloads.RoleSpaceDeveloper, "space", true, ""), + Entry("space manager w org", payloads.RoleSpaceManager, "organization", false, "relationships.space is required"), + Entry("space manager w space", payloads.RoleSpaceManager, "space", true, ""), + Entry("space supporter w org", payloads.RoleSpaceSupporter, "organization", false, "relationships.space is required"), + Entry("space supporter w space", payloads.RoleSpaceSupporter, "space", true, ""), + + Entry("invalid role name", "does-not-exist", "organization", false, "type value must be one of"), ) + +var _ = Describe("role list", func() { + DescribeTable("valid query", + func(query string, expectedRoleListQueryParameters payloads.RoleList) { + actualRoleListQueryParameters, decodeErr := decodeQuery[payloads.RoleList](query) + + Expect(decodeErr).NotTo(HaveOccurred()) + Expect(*actualRoleListQueryParameters).To(Equal(expectedRoleListQueryParameters)) + }, + + Entry("guids", "guids=g1,g2", payloads.RoleList{GUIDs: map[string]bool{"g1": true, "g2": true}}), + Entry("types", "types=g1,g2", payloads.RoleList{Types: map[string]bool{"g1": true, "g2": true}}), + Entry("space_guids", "space_guids=g1,g2", payloads.RoleList{SpaceGUIDs: map[string]bool{"g1": true, "g2": true}}), + Entry("organization_guids", "organization_guids=g1,g2", payloads.RoleList{OrgGUIDs: map[string]bool{"g1": true, "g2": true}}), + Entry("user_guids", "user_guids=g1,g2", payloads.RoleList{UserGUIDs: map[string]bool{"g1": true, "g2": true}}), + Entry("order_by1", "order_by=created_at", payloads.RoleList{OrderBy: "created_at"}), + Entry("order_by2", "order_by=-created_at", payloads.RoleList{OrderBy: "-created_at"}), + Entry("order_by3", "order_by=updated_at", payloads.RoleList{OrderBy: "updated_at"}), + Entry("order_by4", "order_by=-updated_at", payloads.RoleList{OrderBy: "-updated_at"}), + Entry("include", "include=foo", payloads.RoleList{}), + ) + + DescribeTable("invalid query", + func(query string, expectedErrMsg string) { + _, decodeErr := decodeQuery[payloads.RoleList](query) + Expect(decodeErr).To(MatchError(ContainSubstring(expectedErrMsg))) + }, + Entry("invalid order_by", "order_by=foo", "value must be one of"), + ) +}) diff --git a/api/payloads/route.go b/api/payloads/route.go index 827554aeb..a9c2394a9 100644 --- a/api/payloads/route.go +++ b/api/payloads/route.go @@ -4,19 +4,25 @@ import ( "net/url" "code.cloudfoundry.org/korifi/api/payloads/parse" + "code.cloudfoundry.org/korifi/api/payloads/validation" "code.cloudfoundry.org/korifi/api/repositories" + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + jellidation "github.com/jellydator/validation" ) type RouteCreate struct { - Host string `json:"host" validate:"required"` // TODO: Not required when we support private domains - Path string `json:"path"` - Relationships RouteRelationships `json:"relationships" validate:"required"` - Metadata Metadata `json:"metadata"` + Host string `json:"host"` + Path string `json:"path"` + Relationships *RouteRelationships `json:"relationships"` + Metadata Metadata `json:"metadata"` } -type RouteRelationships struct { - Domain Relationship `json:"domain" validate:"required"` - Space Relationship `json:"space" validate:"required"` +func (p RouteCreate) Validate() error { + return jellidation.ValidateStruct(&p, + jellidation.Field(&p.Host, jellidation.Required), + jellidation.Field(&p.Relationships, jellidation.NotNil), + jellidation.Field(&p.Metadata), + ) } func (p RouteCreate) ToMessage(domainNamespace, domainName string) repositories.CreateRouteMessage { @@ -32,6 +38,18 @@ func (p RouteCreate) ToMessage(domainNamespace, domainName string) repositories. } } +type RouteRelationships struct { + Domain Relationship `json:"domain"` + Space Relationship `json:"space"` +} + +func (r RouteRelationships) Validate() error { + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.Domain, validation.StrictlyRequired), + jellidation.Field(&r.Space, validation.StrictlyRequired), + ) +} + type RouteList struct { AppGUIDs string SpaceGUIDs string @@ -40,7 +58,7 @@ type RouteList struct { Paths string } -func (p *RouteList) ToMessage() repositories.ListRoutesMessage { +func (p RouteList) ToMessage() repositories.ListRoutesMessage { return repositories.ListRoutesMessage{ AppGUIDs: parse.ArrayParam(p.AppGUIDs), SpaceGUIDs: parse.ArrayParam(p.SpaceGUIDs), @@ -50,7 +68,7 @@ func (p *RouteList) ToMessage() repositories.ListRoutesMessage { } } -func (p *RouteList) SupportedKeys() []string { +func (p RouteList) SupportedKeys() []string { return []string{"app_guids", "space_guids", "domain_guids", "hosts", "paths", "per_page", "page"} } @@ -67,13 +85,97 @@ type RoutePatch struct { Metadata MetadataPatch `json:"metadata"` } -func (a *RoutePatch) ToMessage(routeGUID, spaceGUID string) repositories.PatchRouteMetadataMessage { +func (p RoutePatch) Validate() error { + return jellidation.ValidateStruct(&p, + jellidation.Field(&p.Metadata), + ) +} + +func (p RoutePatch) ToMessage(routeGUID, spaceGUID string) repositories.PatchRouteMetadataMessage { return repositories.PatchRouteMetadataMessage{ RouteGUID: routeGUID, SpaceGUID: spaceGUID, MetadataPatch: repositories.MetadataPatch{ - Annotations: a.Metadata.Annotations, - Labels: a.Metadata.Labels, + Annotations: p.Metadata.Annotations, + Labels: p.Metadata.Labels, }, } } + +type RouteDestinationCreate struct { + Destinations []RouteDestination `json:"destinations"` +} + +func (r RouteDestinationCreate) Validate() error { + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.Destinations), + ) +} + +type RouteDestination struct { + App AppResource `json:"app"` + Port *int `json:"port"` + Protocol *string `json:"protocol"` +} + +func (r RouteDestination) Validate() error { + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.App), + jellidation.Field(&r.Protocol, validation.OneOf("http1")), + ) +} + +type AppResource struct { + GUID string `json:"guid"` + Process *DestinationAppProcess `json:"process"` +} + +func (a AppResource) Validate() error { + return jellidation.ValidateStruct(&a, + jellidation.Field(&a.GUID, jellidation.Required), + jellidation.Field(&a.Process), + ) +} + +type DestinationAppProcess struct { + Type string `json:"type"` +} + +func (p DestinationAppProcess) Validate() error { + return jellidation.ValidateStruct(&p, + jellidation.Field(&p.Type, jellidation.Required), + ) +} + +func (dc RouteDestinationCreate) ToMessage(routeRecord repositories.RouteRecord) repositories.AddDestinationsToRouteMessage { + addDestinations := make([]repositories.DestinationMessage, 0, len(dc.Destinations)) + for _, destination := range dc.Destinations { + processType := korifiv1alpha1.ProcessTypeWeb + if destination.App.Process != nil { + processType = destination.App.Process.Type + } + + port := 8080 + if destination.Port != nil { + port = *destination.Port + } + + protocol := "http1" + if destination.Protocol != nil { + protocol = *destination.Protocol + } + + addDestinations = append(addDestinations, repositories.DestinationMessage{ + AppGUID: destination.App.GUID, + ProcessType: processType, + Port: port, + Protocol: protocol, + }) + } + return repositories.AddDestinationsToRouteMessage{ + RouteGUID: routeRecord.GUID, + SpaceGUID: routeRecord.SpaceGUID, + ExistingDestinations: routeRecord.Destinations, + NewDestinations: addDestinations, + } +} diff --git a/api/payloads/route_test.go b/api/payloads/route_test.go index 7dad4f7c5..cb76d9001 100644 --- a/api/payloads/route_test.go +++ b/api/payloads/route_test.go @@ -3,20 +3,35 @@ package payloads_test import ( "net/http" + "code.cloudfoundry.org/korifi/api/errors" "code.cloudfoundry.org/korifi/api/payloads" + "code.cloudfoundry.org/korifi/tools" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/gstruct" ) var _ = Describe("RouteList", func() { Describe("decode from url values", func() { - It("succeeds", func() { - routeList := payloads.RouteList{} - req, err := http.NewRequest("GET", "http://foo.com/bar?app_guids=app_guid&space_guids=space_guid&domain_guids=domain_guid&hosts=host&paths=path", nil) - Expect(err).NotTo(HaveOccurred()) - err = validator.DecodeAndValidateURLValues(req, &routeList) + var ( + routeList payloads.RouteList + decodeErr error + params string + ) + BeforeEach(func() { + routeList = payloads.RouteList{} + params = "app_guids=app_guid&space_guids=space_guid&domain_guids=domain_guid&hosts=host&paths=path" + }) + + JustBeforeEach(func() { + req, err := http.NewRequest("GET", "http://foo.com/bar?"+params, nil) Expect(err).NotTo(HaveOccurred()) + decodeErr = validator.DecodeAndValidateURLValues(req, &routeList) + }) + + It("succeeds", func() { + Expect(decodeErr).NotTo(HaveOccurred()) Expect(routeList).To(Equal(payloads.RouteList{ AppGUIDs: "app_guid", SpaceGUIDs: "space_guid", @@ -25,5 +40,228 @@ var _ = Describe("RouteList", func() { Paths: "path", })) }) + + When("it contains an invalid key", func() { + BeforeEach(func() { + params = "foo=bar" + }) + + It("fails", func() { + Expect(decodeErr).To(MatchError("unsupported query parameter: foo")) + }) + }) + }) +}) + +var _ = Describe("RouteCreate", func() { + var ( + createPayload payloads.RouteCreate + routeCreate *payloads.RouteCreate + validatorErr error + apiError errors.ApiError + ) + + BeforeEach(func() { + routeCreate = new(payloads.RouteCreate) + createPayload = payloads.RouteCreate{ + Host: "h1", + Path: "p1", + Relationships: &payloads.RouteRelationships{ + Domain: payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "d1", + }, + }, + Space: payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "s1", + }, + }, + }, + Metadata: payloads.Metadata{ + Annotations: map[string]string{"a": "av"}, + Labels: map[string]string{"l": "lv"}, + }, + } + }) + + JustBeforeEach(func() { + validatorErr = validator.DecodeAndValidateJSONPayload(createRequest(createPayload), routeCreate) + apiError, _ = validatorErr.(errors.ApiError) + }) + + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(routeCreate).To(gstruct.PointTo(Equal(createPayload))) + }) + + When("host is blank", func() { + BeforeEach(func() { + createPayload.Host = "" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("host cannot be blank")) + }) + }) + + When("relationships is empty", func() { + BeforeEach(func() { + createPayload.Relationships = nil + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("relationships is required")) + }) + }) + + When("domain guid is blank", func() { + BeforeEach(func() { + createPayload.Relationships.Domain.Data.GUID = "" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("relationships.domain.data.guid cannot be blank")) + }) + }) + + When("space guid is blank", func() { + BeforeEach(func() { + createPayload.Relationships.Space.Data.GUID = "" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("relationships.space.data.guid cannot be blank")) + }) + }) + + When("metadata uses the cloudfoundry domain", func() { + BeforeEach(func() { + createPayload.Metadata.Labels["foo.cloudfoundry.org/bar"] = "baz" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("cannot use the cloudfoundry.org domain")) + }) + }) +}) + +var _ = Describe("RoutePatch", func() { + var ( + patchPayload payloads.RoutePatch + routePatch *payloads.RoutePatch + validatorErr error + apiError errors.ApiError + ) + + BeforeEach(func() { + routePatch = new(payloads.RoutePatch) + patchPayload = payloads.RoutePatch{ + Metadata: payloads.MetadataPatch{ + Annotations: map[string]*string{"a": tools.PtrTo("av")}, + Labels: map[string]*string{"l": tools.PtrTo("lv")}, + }, + } + }) + + JustBeforeEach(func() { + validatorErr = validator.DecodeAndValidateJSONPayload(createRequest(patchPayload), routePatch) + apiError, _ = validatorErr.(errors.ApiError) + }) + + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(routePatch).To(gstruct.PointTo(Equal(patchPayload))) + }) + + When("metadata uses the cloudfoundry domain", func() { + BeforeEach(func() { + patchPayload.Metadata.Labels["foo.cloudfoundry.org/bar"] = tools.PtrTo("baz") + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("cannot use the cloudfoundry.org domain")) + }) + }) +}) + +var _ = Describe("Add destination", func() { + var ( + addPayload payloads.RouteDestinationCreate + destinationAdd *payloads.RouteDestinationCreate + validatorErr error + apiError errors.ApiError + ) + + BeforeEach(func() { + destinationAdd = new(payloads.RouteDestinationCreate) + addPayload = payloads.RouteDestinationCreate{ + Destinations: []payloads.RouteDestination{ + { + App: payloads.AppResource{ + GUID: "app-1-guid", + }, + }, + { + App: payloads.AppResource{ + GUID: "app-2-guid", + Process: &payloads.DestinationAppProcess{ + Type: "queue", + }, + }, + Port: tools.PtrTo(1234), + Protocol: tools.PtrTo("http1"), + }, + }, + } + }) + + JustBeforeEach(func() { + validatorErr = validator.DecodeAndValidateJSONPayload(createRequest(addPayload), destinationAdd) + apiError, _ = validatorErr.(errors.ApiError) + }) + + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(destinationAdd).To(gstruct.PointTo(Equal(addPayload))) + }) + + When("app guid is empty", func() { + BeforeEach(func() { + addPayload.Destinations[0].App.GUID = "" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("guid cannot be blank")) + }) + }) + + When("process type is empty", func() { + BeforeEach(func() { + addPayload.Destinations[1].App.Process.Type = "" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("type cannot be blank")) + }) + }) + + When("protocol is not http1", func() { + BeforeEach(func() { + addPayload.Destinations[1].Protocol = tools.PtrTo("http") + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("value must be one of: http1")) + }) }) }) diff --git a/api/payloads/service_binding.go b/api/payloads/service_binding.go index 1bb615481..9a4356941 100644 --- a/api/payloads/service_binding.go +++ b/api/payloads/service_binding.go @@ -4,7 +4,9 @@ import ( "net/url" "code.cloudfoundry.org/korifi/api/payloads/parse" + "code.cloudfoundry.org/korifi/api/payloads/validation" "code.cloudfoundry.org/korifi/api/repositories" + jellidation "github.com/jellydator/validation" ) type ServiceBindingCreate struct { @@ -13,11 +15,6 @@ type ServiceBindingCreate struct { Name *string `json:"name"` } -type ServiceBindingRelationships struct { - App *Relationship `json:"app" validate:"required"` - ServiceInstance *Relationship `json:"service_instance" validate:"required"` -} - func (p ServiceBindingCreate) ToMessage(spaceGUID string) repositories.CreateServiceBindingMessage { return repositories.CreateServiceBindingMessage{ Name: p.Name, @@ -27,6 +24,25 @@ func (p ServiceBindingCreate) ToMessage(spaceGUID string) repositories.CreateSer } } +func (p ServiceBindingCreate) Validate() error { + return jellidation.ValidateStruct(&p, + jellidation.Field(&p.Type, validation.OneOf("app")), + jellidation.Field(&p.Relationships, jellidation.NotNil), + ) +} + +type ServiceBindingRelationships struct { + App *Relationship `json:"app"` + ServiceInstance *Relationship `json:"service_instance"` +} + +func (r ServiceBindingRelationships) Validate() error { + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.App, jellidation.NotNil), + jellidation.Field(&r.ServiceInstance, jellidation.NotNil), + ) +} + type ServiceBindingList struct { AppGUIDs string ServiceInstanceGUIDs string @@ -55,6 +71,12 @@ type ServiceBindingUpdate struct { Metadata MetadataPatch `json:"metadata"` } +func (u ServiceBindingUpdate) Validate() error { + return jellidation.ValidateStruct(&u, + jellidation.Field(&u.Metadata), + ) +} + func (c *ServiceBindingUpdate) ToMessage(serviceBindingGUID string) repositories.UpdateServiceBindingMessage { return repositories.UpdateServiceBindingMessage{ GUID: serviceBindingGUID, diff --git a/api/payloads/service_binding_test.go b/api/payloads/service_binding_test.go index 7b5518748..2e197a207 100644 --- a/api/payloads/service_binding_test.go +++ b/api/payloads/service_binding_test.go @@ -3,9 +3,12 @@ package payloads_test import ( "net/http" + "code.cloudfoundry.org/korifi/api/errors" "code.cloudfoundry.org/korifi/api/payloads" + "code.cloudfoundry.org/korifi/tools" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/gstruct" ) var _ = Describe("ServiceBindingList", func() { @@ -25,3 +28,147 @@ var _ = Describe("ServiceBindingList", func() { }) }) }) + +var _ = Describe("ServiceBindingCreate", func() { + var ( + createPayload payloads.ServiceBindingCreate + serviceBindingCreate *payloads.ServiceBindingCreate + validatorErr error + apiError errors.ApiError + ) + + BeforeEach(func() { + serviceBindingCreate = new(payloads.ServiceBindingCreate) + createPayload = payloads.ServiceBindingCreate{ + Relationships: &payloads.ServiceBindingRelationships{ + App: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "app-guid", + }, + }, + ServiceInstance: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "service-instance-guid", + }, + }, + }, + Type: "app", + } + }) + + JustBeforeEach(func() { + validatorErr = validator.DecodeAndValidateJSONPayload(createRequest(createPayload), serviceBindingCreate) + apiError, _ = validatorErr.(errors.ApiError) + }) + + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(serviceBindingCreate).To(gstruct.PointTo(Equal(createPayload))) + }) + + When(`the type is "key"`, func() { + BeforeEach(func() { + createPayload.Type = "key" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("type value must be one of: app")) + }) + }) + + When("all relationships are missing", func() { + BeforeEach(func() { + createPayload.Relationships = nil + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("relationships is required")) + }) + }) + + When("app relationship is missing", func() { + BeforeEach(func() { + createPayload.Relationships.App = nil + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("relationships.app is required")) + }) + }) + + When("the app GUID is blank", func() { + BeforeEach(func() { + createPayload.Relationships.App.Data.GUID = "" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("app.data.guid cannot be blank")) + }) + }) + + When("service instance relationship is missing", func() { + BeforeEach(func() { + createPayload.Relationships.ServiceInstance = nil + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("relationships.service_instance is required")) + }) + }) + + When("the service instance GUID is blank", func() { + BeforeEach(func() { + createPayload.Relationships.ServiceInstance.Data.GUID = "" + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("relationships.service_instance.data.guid cannot be blank")) + }) + }) +}) + +var _ = Describe("ServiceBindingUpdate", func() { + var ( + patchPayload payloads.ServiceBindingUpdate + serviceBindingPatch *payloads.ServiceBindingUpdate + validatorErr error + apiError errors.ApiError + ) + + BeforeEach(func() { + serviceBindingPatch = new(payloads.ServiceBindingUpdate) + patchPayload = payloads.ServiceBindingUpdate{ + Metadata: payloads.MetadataPatch{ + Annotations: map[string]*string{"a": tools.PtrTo("av")}, + Labels: map[string]*string{"l": tools.PtrTo("lv")}, + }, + } + }) + + JustBeforeEach(func() { + validatorErr = validator.DecodeAndValidateJSONPayload(createRequest(patchPayload), serviceBindingPatch) + apiError, _ = validatorErr.(errors.ApiError) + }) + + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(serviceBindingPatch).To(gstruct.PointTo(Equal(patchPayload))) + }) + + When("metadata uses the cloudfoundry domain", func() { + BeforeEach(func() { + patchPayload.Metadata.Labels["foo.cloudfoundry.org/bar"] = tools.PtrTo("baz") + }) + + It("fails", func() { + Expect(apiError).To(HaveOccurred()) + Expect(apiError.Detail()).To(ContainSubstring("cannot use the cloudfoundry.org domain")) + }) + }) +}) diff --git a/api/payloads/validation/rules.go b/api/payloads/validation/rules.go index 3ee11138b..04c558cf6 100644 --- a/api/payloads/validation/rules.go +++ b/api/payloads/validation/rules.go @@ -26,7 +26,7 @@ type strictlyRequiredRule struct { validation.RequiredRule } -// We wrap tje original validation.RequiredRule in order to workaround +// We wrap the original validation.RequiredRule in order to workaround // incorrect zero type check: // https://github.com/jellydator/validation/blob/44595f5c48dd0da8bbeff0f56ceaa581631e55b1/util.go#L151-L156 func (r strictlyRequiredRule) Validate(value interface{}) error {