From ebcf86cd0cca4bfacde5819976a1b9da3026d4da Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Wed, 7 Aug 2024 11:09:20 +0000 Subject: [PATCH] Include broker fields when listing offerings fixes #3413 Co-authored-by: Georgi Sabev --- api/handlers/package_test.go | 6 +- api/handlers/service_broker_test.go | 5 +- api/handlers/service_offering.go | 53 ++++++++++++- api/handlers/service_offering_test.go | 68 +++++++++++++++++ api/main.go | 1 + api/payloads/package_test.go | 8 +- api/payloads/parse/arrays.go | 2 +- api/payloads/parse/arrays_test.go | 6 +- api/payloads/service_offering.go | 18 ++++- api/payloads/service_offering_test.go | 10 +++ api/presenter/service_binding.go | 17 +++-- api/presenter/shared.go | 28 +++++-- api/presenter/shared_test.go | 75 +++++++++++++++++-- api/repositories/app_repository.go | 7 +- api/repositories/domain_repository.go | 3 +- api/repositories/droplet_repository.go | 3 +- api/repositories/org_repository.go | 5 +- api/repositories/package_repository.go | 5 +- api/repositories/process_repository.go | 5 +- api/repositories/route_repository.go | 9 ++- .../service_binding_repository.go | 5 +- api/repositories/service_broker_repository.go | 9 +-- .../service_broker_repository_test.go | 16 ++++ .../service_instance_repository.go | 6 +- .../service_offering_repository.go | 3 +- api/repositories/service_plan_repository.go | 15 ++-- api/repositories/shared.go | 9 --- api/repositories/space_repository.go | 7 +- api/repositories/task_repository.go | 5 +- model/included.go | 6 ++ tools/collections.go | 19 +++++ tools/collections_test.go | 26 +++++++ 32 files changed, 365 insertions(+), 95 deletions(-) create mode 100644 model/included.go create mode 100644 tools/collections.go create mode 100644 tools/collections_test.go diff --git a/api/handlers/package_test.go b/api/handlers/package_test.go index 84d0c842c..1b6f114a1 100644 --- a/api/handlers/package_test.go +++ b/api/handlers/package_test.go @@ -191,9 +191,7 @@ var _ = Describe("Package", func() { It("calls the package repository with expected arguments", func() { _, _, message := packageRepo.ListPackagesArgsForCall(0) Expect(message).To(Equal(repositories.ListPackagesMessage{ - GUIDs: []string{}, AppGUIDs: []string{appGUID}, - States: []string{}, })) Expect(rr).To(HaveHTTPStatus(http.StatusOK)) @@ -252,9 +250,7 @@ var _ = Describe("Package", func() { It("calls repository ListPackage with the correct message object", func() { _, _, message := packageRepo.ListPackagesArgsForCall(0) Expect(message).To(Equal(repositories.ListPackagesMessage{ - GUIDs: []string{}, - AppGUIDs: []string{}, - States: []string{"READY", "AWAITING_UPLOAD"}, + States: []string{"READY", "AWAITING_UPLOAD"}, })) Expect(rr).To(HaveHTTPStatus(http.StatusOK)) }) diff --git a/api/handlers/service_broker_test.go b/api/handlers/service_broker_test.go index 86ac2f08d..2b49a3058 100644 --- a/api/handlers/service_broker_test.go +++ b/api/handlers/service_broker_test.go @@ -14,7 +14,6 @@ import ( "code.cloudfoundry.org/korifi/model/services" . "code.cloudfoundry.org/korifi/tests/matchers" "code.cloudfoundry.org/korifi/tools" - . "github.com/onsi/gomega/gstruct" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -120,9 +119,7 @@ var _ = Describe("ServiceBroker", func() { Expect(serviceBrokerRepo.ListServiceBrokersCallCount()).To(Equal(1)) _, actualAuthInfo, actualListMsg := serviceBrokerRepo.ListServiceBrokersArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) - Expect(actualListMsg).To(MatchAllFields(Fields{ - "Names": BeEmpty(), - })) + Expect(actualListMsg).To(Equal(repositories.ListServiceBrokerMessage{})) Expect(rr).Should(HaveHTTPStatus(http.StatusOK)) Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) diff --git a/api/handlers/service_offering.go b/api/handlers/service_offering.go index 4c8544532..a4cde23d5 100644 --- a/api/handlers/service_offering.go +++ b/api/handlers/service_offering.go @@ -5,6 +5,7 @@ import ( "context" "net/http" "net/url" + "slices" "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" @@ -12,6 +13,9 @@ import ( "code.cloudfoundry.org/korifi/api/presenter" "code.cloudfoundry.org/korifi/api/repositories" "code.cloudfoundry.org/korifi/api/routing" + "code.cloudfoundry.org/korifi/model" + "code.cloudfoundry.org/korifi/tools" + "github.com/BooleanCat/go-functional/iter" "github.com/go-logr/logr" ) @@ -28,17 +32,20 @@ type ServiceOffering struct { serverURL url.URL requestValidator RequestValidator serviceOfferingRepo CFServiceOfferingRepository + serviceBrokerRepo CFServiceBrokerRepository } func NewServiceOffering( serverURL url.URL, requestValidator RequestValidator, serviceOfferingRepo CFServiceOfferingRepository, + serviceBrokerRepo CFServiceBrokerRepository, ) *ServiceOffering { return &ServiceOffering{ serverURL: serverURL, requestValidator: requestValidator, serviceOfferingRepo: serviceOfferingRepo, + serviceBrokerRepo: serviceBrokerRepo, } } @@ -53,10 +60,52 @@ func (h *ServiceOffering) list(r *http.Request) (*routing.Response, error) { serviceOfferingList, err := h.serviceOfferingRepo.ListOfferings(r.Context(), authInfo, payload.ToMessage()) if err != nil { - return nil, apierrors.LogAndReturn(logger, err, "Failed to list service offerings") + return nil, apierrors.LogAndReturn(logger, err, "failed to list service offerings") } - return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceOffering, serviceOfferingList, h.serverURL, *r.URL)), nil + brokerIncludes, err := h.getBrokerIncludes(r.Context(), authInfo, serviceOfferingList, payload.IncludeBrokerFields) + if err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to get broker includes") + } + + return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceOffering, serviceOfferingList, h.serverURL, *r.URL, brokerIncludes...)), nil +} + +func (h *ServiceOffering) getBrokerIncludes( + ctx context.Context, + authInfo authorization.Info, + serviceOfferings []repositories.ServiceOfferingRecord, + brokerFields []string, +) ([]model.IncludedResource, error) { + if len(brokerFields) == 0 { + return nil, nil + } + + brokerGUIDs := iter.Map(iter.Lift(serviceOfferings), func(o repositories.ServiceOfferingRecord) string { + return o.ServiceBrokerGUID + }).Collect() + + brokers, err := h.serviceBrokerRepo.ListServiceBrokers(ctx, authInfo, repositories.ListServiceBrokerMessage{ + GUIDs: tools.Uniq(brokerGUIDs), + }) + if err != nil { + return nil, err + } + + return iter.Map(iter.Lift(brokers), func(b repositories.ServiceBrokerRecord) model.IncludedResource { + resource := map[string]string{} + if slices.Contains(brokerFields, "guid") { + resource["guid"] = b.GUID + } + if slices.Contains(brokerFields, "name") { + resource["name"] = b.Name + } + + return model.IncludedResource{ + Type: "service_brokers", + Resource: resource, + } + }).Collect(), nil } func (h *ServiceOffering) UnauthenticatedRoutes() []routing.Route { diff --git a/api/handlers/service_offering_test.go b/api/handlers/service_offering_test.go index 12065c1d4..88598cfbb 100644 --- a/api/handlers/service_offering_test.go +++ b/api/handlers/service_offering_test.go @@ -20,16 +20,19 @@ var _ = Describe("ServiceOffering", func() { var ( requestValidator *fake.RequestValidator serviceOfferingRepo *fake.CFServiceOfferingRepository + serviceBrokerRepo *fake.CFServiceBrokerRepository ) BeforeEach(func() { requestValidator = new(fake.RequestValidator) serviceOfferingRepo = new(fake.CFServiceOfferingRepository) + serviceBrokerRepo = new(fake.CFServiceBrokerRepository) apiHandler := NewServiceOffering( *serverURL, requestValidator, serviceOfferingRepo, + serviceBrokerRepo, ) routerBuilder.LoadRoutes(apiHandler) }) @@ -83,6 +86,71 @@ var _ = Describe("ServiceOffering", func() { }) }) + Describe("include broker fields", func() { + BeforeEach(func() { + serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerRecord{{ + ServiceBroker: services.ServiceBroker{ + Name: "broker-name", + }, + CFResource: model.CFResource{ + GUID: "broker-guid", + }, + }}, nil) + + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceOfferingList{ + IncludeBrokerFields: []string{"foo"}, + }) + }) + + It("lists the brokers", func() { + Expect(serviceBrokerRepo.ListServiceBrokersCallCount()).To(Equal(1)) + _, _, actualListMessage := serviceBrokerRepo.ListServiceBrokersArgsForCall(0) + Expect(actualListMessage).To(Equal(repositories.ListServiceBrokerMessage{ + GUIDs: []string{"broker-guid"}, + })) + }) + + When("listing brokers fails", func() { + BeforeEach(func() { + serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerRecord{}, errors.New("list-broker-err")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + + Describe("broker name", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceOfferingList{ + IncludeBrokerFields: []string{"name"}, + }) + }) + + It("includes broker fields in the response", func() { + Expect(rr).Should(HaveHTTPStatus(http.StatusOK)) + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.included.service_brokers[0].name", "broker-name"), + ))) + }) + }) + + Describe("broker guid", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceOfferingList{ + IncludeBrokerFields: []string{"guid"}, + }) + }) + + It("includes broker fields in the response", func() { + Expect(rr).Should(HaveHTTPStatus(http.StatusOK)) + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.included.service_brokers[0].guid", "broker-guid"), + ))) + }) + }) + }) + When("the request is invalid", func() { BeforeEach(func() { requestValidator.DecodeAndValidateURLValuesReturns(errors.New("invalid-request")) diff --git a/api/main.go b/api/main.go index 33eae6d44..e967ea7aa 100644 --- a/api/main.go +++ b/api/main.go @@ -425,6 +425,7 @@ func main() { *serverURL, requestValidator, serviceOfferingRepo, + serviceBrokerRepo, ), handlers.NewServicePlan( *serverURL, diff --git a/api/payloads/package_test.go b/api/payloads/package_test.go index 4258863bf..5f9c2f51b 100644 --- a/api/payloads/package_test.go +++ b/api/payloads/package_test.go @@ -319,10 +319,10 @@ var _ = Describe("PackageList", func() { Expect(actualListPackagesMessage).To(Equal(expectedListPackagesMessage)) }, - Entry("guids", payloads.PackageList{GUIDs: "g1,g2", AppGUIDs: "", States: ""}, repositories.ListPackagesMessage{GUIDs: []string{"g1", "g2"}, AppGUIDs: []string{}, States: []string{}}), - Entry("app_guids", payloads.PackageList{GUIDs: "", AppGUIDs: "ag1,ag2", States: ""}, repositories.ListPackagesMessage{GUIDs: []string{}, AppGUIDs: []string{"ag1", "ag2"}, States: []string{}}), - Entry("states", payloads.PackageList{GUIDs: "", AppGUIDs: "", States: "s1,s2"}, repositories.ListPackagesMessage{GUIDs: []string{}, AppGUIDs: []string{}, States: []string{"s1", "s2"}}), - Entry("empty", payloads.PackageList{}, repositories.ListPackagesMessage{GUIDs: []string{}, AppGUIDs: []string{}, States: []string{}}), + Entry("guids", payloads.PackageList{GUIDs: "g1,g2"}, repositories.ListPackagesMessage{GUIDs: []string{"g1", "g2"}}), + Entry("app_guids", payloads.PackageList{AppGUIDs: "ag1,ag2"}, repositories.ListPackagesMessage{AppGUIDs: []string{"ag1", "ag2"}}), + Entry("states", payloads.PackageList{States: "s1,s2"}, repositories.ListPackagesMessage{States: []string{"s1", "s2"}}), + Entry("empty", payloads.PackageList{}, repositories.ListPackagesMessage{}), ) DescribeTable("invalid query", diff --git a/api/payloads/parse/arrays.go b/api/payloads/parse/arrays.go index 4e337156f..28947ff23 100644 --- a/api/payloads/parse/arrays.go +++ b/api/payloads/parse/arrays.go @@ -4,7 +4,7 @@ import "strings" func ArrayParam(arrayParam string) []string { if arrayParam == "" { - return []string{} + return nil } elements := strings.Split(arrayParam, ",") diff --git a/api/payloads/parse/arrays_test.go b/api/payloads/parse/arrays_test.go index 95041a0fc..c67d8c683 100644 --- a/api/payloads/parse/arrays_test.go +++ b/api/payloads/parse/arrays_test.go @@ -7,9 +7,9 @@ import ( ) var _ = Describe("ParseArrayParam", func() { - When("a nil value is specified", func() { - It("returns an empty array", func() { - Expect(ArrayParam("")).To(Equal([]string{})) + When("an empty string is specified", func() { + It("returns nil", func() { + Expect(ArrayParam("")).To(BeNil()) }) }) diff --git a/api/payloads/service_offering.go b/api/payloads/service_offering.go index f3031a49d..a4dfb2774 100644 --- a/api/payloads/service_offering.go +++ b/api/payloads/service_offering.go @@ -5,12 +5,21 @@ import ( "regexp" "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 ServiceOfferingList struct { - Names string - BrokerNames string + Names string + BrokerNames string + IncludeBrokerFields []string +} + +func (l ServiceOfferingList) Validate() error { + return jellidation.ValidateStruct(&l, + jellidation.Field(&l.IncludeBrokerFields, jellidation.Each(validation.OneOf("guid", "name"))), + ) } func (l *ServiceOfferingList) ToMessage() repositories.ListServiceOfferingMessage { @@ -21,15 +30,16 @@ func (l *ServiceOfferingList) ToMessage() repositories.ListServiceOfferingMessag } func (l *ServiceOfferingList) SupportedKeys() []string { - return []string{"names", "service_broker_names", "page", "per_page"} + return []string{"names", "service_broker_names", "fields[service_broker]", "page", "per_page"} } func (l *ServiceOfferingList) IgnoredKeys() []*regexp.Regexp { - return []*regexp.Regexp{regexp.MustCompile(`fields\[.+\]`)} + return nil } func (l *ServiceOfferingList) DecodeFromURLValues(values url.Values) error { l.Names = values.Get("names") l.BrokerNames = values.Get("service_broker_names") + l.IncludeBrokerFields = parse.ArrayParam(values.Get("fields[service_broker]")) return nil } diff --git a/api/payloads/service_offering_test.go b/api/payloads/service_offering_test.go index af257dfb8..881e673fb 100644 --- a/api/payloads/service_offering_test.go +++ b/api/payloads/service_offering_test.go @@ -5,6 +5,7 @@ import ( "code.cloudfoundry.org/korifi/api/repositories" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/types" ) var _ = Describe("ServiceOffering", func() { @@ -17,6 +18,15 @@ var _ = Describe("ServiceOffering", func() { }, Entry("names", "names=b1,b2", payloads.ServiceOfferingList{Names: "b1,b2"}), Entry("service_broker_names", "service_broker_names=b1,b2", payloads.ServiceOfferingList{BrokerNames: "b1,b2"}), + Entry("fields[service_broker]", "fields[service_broker]=guid,name", payloads.ServiceOfferingList{IncludeBrokerFields: []string{"guid", "name"}}), + ) + + DescribeTable("invalid query", + func(query string, errMatcher types.GomegaMatcher) { + _, decodeErr := decodeQuery[payloads.ServiceOfferingList](query) + Expect(decodeErr).To(errMatcher) + }, + Entry("invalid service broker field", "fields[service_broker]=foo", MatchError(ContainSubstring("value must be one of: guid, name"))), ) Describe("ToMessage", func() { diff --git a/api/presenter/service_binding.go b/api/presenter/service_binding.go index c953dd0e1..46ad4a328 100644 --- a/api/presenter/service_binding.go +++ b/api/presenter/service_binding.go @@ -4,6 +4,8 @@ import ( "net/url" "code.cloudfoundry.org/korifi/api/repositories" + "code.cloudfoundry.org/korifi/model" + "github.com/BooleanCat/go-functional/iter" ) const ( @@ -77,13 +79,12 @@ func ForServiceBinding(record repositories.ServiceBindingRecord, baseURL url.URL } func ForServiceBindingList(serviceBindingRecords []repositories.ServiceBindingRecord, appRecords []repositories.AppRecord, baseURL, requestURL url.URL) ListResponse[ServiceBindingResponse] { - ret := ForList(ForServiceBinding, serviceBindingRecords, baseURL, requestURL) - if len(appRecords) > 0 { - appData := IncludedData{} - for _, appRecord := range appRecords { - appData.Apps = append(appData.Apps, ForApp(appRecord, baseURL)) + includedApps := iter.Map(iter.Lift(appRecords), func(app repositories.AppRecord) model.IncludedResource { + return model.IncludedResource{ + Type: "apps", + Resource: ForApp(app, baseURL), } - ret.Included = &appData - } - return ret + }).Collect() + + return ForList(ForServiceBinding, serviceBindingRecords, baseURL, requestURL, includedApps...) } diff --git a/api/presenter/shared.go b/api/presenter/shared.go index cf7a73a33..de27bca9d 100644 --- a/api/presenter/shared.go +++ b/api/presenter/shared.go @@ -4,6 +4,8 @@ import ( "net/url" "path" "time" + + "code.cloudfoundry.org/korifi/model" ) type Lifecycle struct { @@ -37,9 +39,9 @@ type Link struct { } type ListResponse[T any] struct { - PaginationData PaginationData `json:"pagination"` - Resources []T `json:"resources"` - Included *IncludedData `json:"included,omitempty"` + PaginationData PaginationData `json:"pagination"` + Resources []T `json:"resources"` + Included map[string][]any `json:"included,omitempty"` } type PaginationData struct { @@ -51,17 +53,13 @@ type PaginationData struct { Previous *int `json:"previous"` } -type IncludedData struct { - Apps []interface{} `json:"apps"` -} - type PageRef struct { HREF string `json:"href"` } type itemPresenter[T, S any] func(T, url.URL) S -func ForList[T, S any](itemPresenter itemPresenter[T, S], resources []T, baseURL, requestURL url.URL) ListResponse[S] { +func ForList[T, S any](itemPresenter itemPresenter[T, S], resources []T, baseURL, requestURL url.URL, includes ...model.IncludedResource) ListResponse[S] { presenters := []S{} for _, resource := range resources { presenters = append(presenters, itemPresenter(resource, baseURL)) @@ -78,7 +76,21 @@ func ForList[T, S any](itemPresenter itemPresenter[T, S], resources []T, baseURL }, }, Resources: presenters, + Included: includedResources(includes...), + } +} + +func includedResources(includes ...model.IncludedResource) map[string][]any { + resources := map[string][]any{} + for _, include := range includes { + if resources[include.Type] == nil { + resources[include.Type] = []any{} + } + + resources[include.Type] = append(resources[include.Type], include.Resource) } + + return resources } type buildURL url.URL diff --git a/api/presenter/shared_test.go b/api/presenter/shared_test.go index 53b925f13..52fa96fc2 100644 --- a/api/presenter/shared_test.go +++ b/api/presenter/shared_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/gomega" "code.cloudfoundry.org/korifi/api/presenter" + "code.cloudfoundry.org/korifi/model" ) type ( @@ -31,10 +32,11 @@ func forRecord(r record, u url.URL) presentedRecord { var _ = Describe("Shared", func() { Describe("ForList", func() { var ( - records []record - baseURL *url.URL - requestURL *url.URL - output []byte + records []record + includedResources []model.IncludedResource + baseURL *url.URL + requestURL *url.URL + output []byte ) BeforeEach(func() { @@ -46,10 +48,11 @@ var _ = Describe("Shared", func() { Expect(err).NotTo(HaveOccurred()) records = []record{{N: 42}, {N: 43}} + includedResources = []model.IncludedResource{} }) JustBeforeEach(func() { - response := presenter.ForList(forRecord, records, *baseURL, *requestURL) + response := presenter.ForList(forRecord, records, *baseURL, *requestURL, includedResources...) var err error output, err = json.Marshal(response) Expect(err).NotTo(HaveOccurred()) @@ -82,6 +85,68 @@ var _ = Describe("Shared", func() { }`)) }) + When("included resources are provided", func() { + BeforeEach(func() { + includedResources = []model.IncludedResource{ + { + Type: "typeA", + Resource: map[string]string{"a": "A"}, + }, + { + Type: "typeA", + Resource: map[string]string{"a1": "A1"}, + }, + { + Type: "typeB", + Resource: map[string]string{"b": "B"}, + }, + } + }) + + It("returns the expected json", func() { + Expect(output).To(MatchJSON(`{ + "pagination": { + "total_results": 2, + "total_pages": 1, + "first": { + "href": "https://api.example.org/v3/records?foo=bar" + }, + "last": { + "href": "https://api.example.org/v3/records?foo=bar" + }, + "next": null, + "previous": null + }, + "resources": [ + { + "m": 42, + "u": "https://api.example.org" + }, + { + "m": 43, + "u": "https://api.example.org" + } + ], + "included": { + "typeA": [ + { + "a": "A" + }, + { + "a1": "A1" + } + ], + "typeB": [ + { + "b": "B" + } + ] + } + } + `)) + }) + }) + When("records are empty", func() { BeforeEach(func() { records = nil diff --git a/api/repositories/app_repository.go b/api/repositories/app_repository.go index 6eee6b5a6..f440d0905 100644 --- a/api/repositories/app_repository.go +++ b/api/repositories/app_repository.go @@ -13,6 +13,7 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/env" "code.cloudfoundry.org/korifi/controllers/webhooks/validation" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" @@ -182,12 +183,12 @@ type ListAppsMessage struct { } func (m *ListAppsMessage) matchesNamespace(ns string) bool { - return emptyOrContains(m.SpaceGUIDs, ns) + return tools.EmptyOrContains(m.SpaceGUIDs, ns) } func (m *ListAppsMessage) matches(cfApp korifiv1alpha1.CFApp) bool { - return emptyOrContains(m.Names, cfApp.Spec.DisplayName) && - emptyOrContains(m.Guids, cfApp.Name) + return tools.EmptyOrContains(m.Names, cfApp.Spec.DisplayName) && + tools.EmptyOrContains(m.Guids, cfApp.Name) } type byName []AppRecord diff --git a/api/repositories/domain_repository.go b/api/repositories/domain_repository.go index 72efb0ccf..5931a73e5 100644 --- a/api/repositories/domain_repository.go +++ b/api/repositories/domain_repository.go @@ -9,6 +9,7 @@ import ( "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" "github.com/google/uuid" @@ -70,7 +71,7 @@ type ListDomainsMessage struct { } func (m *ListDomainsMessage) matches(d korifiv1alpha1.CFDomain) bool { - return emptyOrContains(m.Names, d.Spec.Name) + return tools.EmptyOrContains(m.Names, d.Spec.Name) } func (r *DomainRepo) GetDomain(ctx context.Context, authInfo authorization.Info, domainGUID string) (DomainRecord, error) { diff --git a/api/repositories/droplet_repository.go b/api/repositories/droplet_repository.go index c9e871d1b..46273ecde 100644 --- a/api/repositories/droplet_repository.go +++ b/api/repositories/droplet_repository.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" @@ -64,7 +65,7 @@ type ListDropletsMessage struct { } func (m *ListDropletsMessage) matches(b korifiv1alpha1.CFBuild) bool { - return emptyOrContains(m.PackageGUIDs, b.Spec.PackageRef.Name) && + return tools.EmptyOrContains(m.PackageGUIDs, b.Spec.PackageRef.Name) && meta.IsStatusConditionFalse(b.Status.Conditions, StagingConditionType) && meta.IsStatusConditionTrue(b.Status.Conditions, SucceededConditionType) } diff --git a/api/repositories/org_repository.go b/api/repositories/org_repository.go index 9013c2d1d..5e7001510 100644 --- a/api/repositories/org_repository.go +++ b/api/repositories/org_repository.go @@ -8,6 +8,7 @@ import ( "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" @@ -35,8 +36,8 @@ type ListOrgsMessage struct { } func (m *ListOrgsMessage) matches(org korifiv1alpha1.CFOrg) bool { - return emptyOrContains(m.GUIDs, org.Name) && - emptyOrContains(m.Names, org.Spec.DisplayName) && + return tools.EmptyOrContains(m.GUIDs, org.Name) && + tools.EmptyOrContains(m.Names, org.Spec.DisplayName) && meta.IsStatusConditionTrue(org.Status.Conditions, korifiv1alpha1.StatusConditionReady) } diff --git a/api/repositories/package_repository.go b/api/repositories/package_repository.go index b45821141..6a92e3a07 100644 --- a/api/repositories/package_repository.go +++ b/api/repositories/package_repository.go @@ -10,6 +10,7 @@ import ( apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/packages" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/dockercfg" "code.cloudfoundry.org/korifi/tools/k8s" @@ -88,8 +89,8 @@ type ListPackagesMessage struct { } func (m *ListPackagesMessage) matches(p korifiv1alpha1.CFPackage) bool { - return emptyOrContains(m.GUIDs, p.Name) && - emptyOrContains(m.AppGUIDs, p.Spec.AppRef.Name) && + return tools.EmptyOrContains(m.GUIDs, p.Name) && + tools.EmptyOrContains(m.AppGUIDs, p.Spec.AppRef.Name) && m.matchesState(p) } diff --git a/api/repositories/process_repository.go b/api/repositories/process_repository.go index 22f21a039..853749778 100644 --- a/api/repositories/process_repository.go +++ b/api/repositories/process_repository.go @@ -8,6 +8,7 @@ import ( "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" @@ -110,8 +111,8 @@ type ListProcessesMessage struct { } func (m *ListProcessesMessage) matches(process korifiv1alpha1.CFProcess) bool { - return emptyOrContains(m.AppGUIDs, process.Spec.AppRef.Name) && - emptyOrContains(m.ProcessTypes, process.Spec.ProcessType) + return tools.EmptyOrContains(m.AppGUIDs, process.Spec.AppRef.Name) && + tools.EmptyOrContains(m.ProcessTypes, process.Spec.ProcessType) } func (m *ListProcessesMessage) matchesNamespace(ns string) bool { diff --git a/api/repositories/route_repository.go b/api/repositories/route_repository.go index 0bfee7515..7bce6658e 100644 --- a/api/repositories/route_repository.go +++ b/api/repositories/route_repository.go @@ -9,6 +9,7 @@ import ( "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" @@ -102,14 +103,14 @@ type ListRoutesMessage struct { } func (m *ListRoutesMessage) matches(r korifiv1alpha1.CFRoute) bool { - return emptyOrContains(m.DomainGUIDs, r.Spec.DomainRef.Name) && - emptyOrContains(m.Hosts, r.Spec.Host) && - emptyOrContains(m.Paths, r.Spec.Path) && + return tools.EmptyOrContains(m.DomainGUIDs, r.Spec.DomainRef.Name) && + tools.EmptyOrContains(m.Hosts, r.Spec.Host) && + tools.EmptyOrContains(m.Paths, r.Spec.Path) && m.matchesApp(r) } func (m *ListRoutesMessage) matchesNamespace(ns string) bool { - return emptyOrContains(m.SpaceGUIDs, ns) + return tools.EmptyOrContains(m.SpaceGUIDs, ns) } func (m *ListRoutesMessage) matchesApp(r korifiv1alpha1.CFRoute) bool { diff --git a/api/repositories/service_binding_repository.go b/api/repositories/service_binding_repository.go index c6ed43405..bdcc00675 100644 --- a/api/repositories/service_binding_repository.go +++ b/api/repositories/service_binding_repository.go @@ -13,6 +13,7 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/webhooks/services/bindings" "code.cloudfoundry.org/korifi/controllers/webhooks/validation" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" @@ -90,8 +91,8 @@ type ListServiceBindingsMessage struct { } func (m *ListServiceBindingsMessage) matches(serviceBinding korifiv1alpha1.CFServiceBinding) bool { - return emptyOrContains(m.ServiceInstanceGUIDs, serviceBinding.Spec.Service.Name) && - emptyOrContains(m.AppGUIDs, serviceBinding.Spec.AppRef.Name) + return tools.EmptyOrContains(m.ServiceInstanceGUIDs, serviceBinding.Spec.Service.Name) && + tools.EmptyOrContains(m.AppGUIDs, serviceBinding.Spec.AppRef.Name) } func (m CreateServiceBindingMessage) toCFServiceBinding() *korifiv1alpha1.CFServiceBinding { diff --git a/api/repositories/service_broker_repository.go b/api/repositories/service_broker_repository.go index f9eb273c6..7ad769083 100644 --- a/api/repositories/service_broker_repository.go +++ b/api/repositories/service_broker_repository.go @@ -3,7 +3,6 @@ package repositories import ( "context" "fmt" - "slices" "time" "code.cloudfoundry.org/korifi/api/authorization" @@ -32,14 +31,12 @@ type CreateServiceBrokerMessage struct { type ListServiceBrokerMessage struct { Names []string + GUIDs []string } func (l ListServiceBrokerMessage) matches(b korifiv1alpha1.CFServiceBroker) bool { - if len(l.Names) == 0 { - return true - } - - return slices.Contains(l.Names, b.Spec.Name) + return tools.EmptyOrContains(l.Names, b.Spec.Name) && + tools.EmptyOrContains(l.GUIDs, b.Name) } type UpdateServiceBrokerMessage struct { diff --git a/api/repositories/service_broker_repository_test.go b/api/repositories/service_broker_repository_test.go index 01b83ae3f..53ab6461c 100644 --- a/api/repositories/service_broker_repository_test.go +++ b/api/repositories/service_broker_repository_test.go @@ -333,6 +333,22 @@ var _ = Describe("ServiceBrokerRepo", func() { }) }) + When("a guid filter is applied", func() { + BeforeEach(func() { + message.GUIDs = []string{"broker-2"} + }) + + It("only returns the matching brokers", func() { + Expect(brokers).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "CFResource": MatchFields(IgnoreExtras, Fields{ + "GUID": Equal("broker-2"), + }), + }), + )) + }) + }) + When("a nonexistent name filter is applied", func() { BeforeEach(func() { message.Names = []string{"no-such-broker"} diff --git a/api/repositories/service_instance_repository.go b/api/repositories/service_instance_repository.go index 60dc41325..180511490 100644 --- a/api/repositories/service_instance_repository.go +++ b/api/repositories/service_instance_repository.go @@ -90,12 +90,12 @@ type ListServiceInstanceMessage struct { } func (m *ListServiceInstanceMessage) matches(serviceInstance korifiv1alpha1.CFServiceInstance) bool { - return emptyOrContains(m.Names, serviceInstance.Spec.DisplayName) && - emptyOrContains(m.GUIDs, serviceInstance.Name) + return tools.EmptyOrContains(m.Names, serviceInstance.Spec.DisplayName) && + tools.EmptyOrContains(m.GUIDs, serviceInstance.Name) } func (m *ListServiceInstanceMessage) matchesNamespace(ns string) bool { - return emptyOrContains(m.SpaceGUIDs, ns) + return tools.EmptyOrContains(m.SpaceGUIDs, ns) } type DeleteServiceInstanceMessage struct { diff --git a/api/repositories/service_offering_repository.go b/api/repositories/service_offering_repository.go index 4f3937e94..08ac63919 100644 --- a/api/repositories/service_offering_repository.go +++ b/api/repositories/service_offering_repository.go @@ -10,6 +10,7 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/model" "code.cloudfoundry.org/korifi/model/services" + "code.cloudfoundry.org/korifi/tools" "github.com/BooleanCat/go-functional/iter" k8serrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" @@ -35,7 +36,7 @@ type ListServiceOfferingMessage struct { } func (m *ListServiceOfferingMessage) matchesName(cfServiceOffering korifiv1alpha1.CFServiceOffering) bool { - return emptyOrContains(m.Names, cfServiceOffering.Spec.Name) + return tools.EmptyOrContains(m.Names, cfServiceOffering.Spec.Name) } func NewServiceOfferingRepo( diff --git a/api/repositories/service_plan_repository.go b/api/repositories/service_plan_repository.go index 5dd653f12..076bf133a 100644 --- a/api/repositories/service_plan_repository.go +++ b/api/repositories/service_plan_repository.go @@ -3,13 +3,13 @@ package repositories import ( "context" "fmt" - "slices" "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/model" "code.cloudfoundry.org/korifi/model/services" + "code.cloudfoundry.org/korifi/tools" "github.com/BooleanCat/go-functional/iter" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -44,8 +44,8 @@ type ListServicePlanMessage struct { } func (m *ListServicePlanMessage) matches(cfServicePlan korifiv1alpha1.CFServicePlan) bool { - return emptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel]) && - emptyOrContains(m.Names, cfServicePlan.Spec.Name) + return tools.EmptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel]) && + tools.EmptyOrContains(m.Names, cfServicePlan.Spec.Name) } type ApplyServicePlanVisibilityMessage struct { @@ -56,7 +56,7 @@ type ApplyServicePlanVisibilityMessage struct { func (m *ApplyServicePlanVisibilityMessage) apply(cfServicePlan *korifiv1alpha1.CFServicePlan) { cfServicePlan.Spec.Visibility.Type = m.Type - cfServicePlan.Spec.Visibility.Organizations = uniq(append( + cfServicePlan.Spec.Visibility.Organizations = tools.Uniq(append( cfServicePlan.Spec.Visibility.Organizations, m.Organizations..., )) @@ -73,7 +73,7 @@ type UpdateServicePlanVisibilityMessage struct { func (m *UpdateServicePlanVisibilityMessage) apply(cfServicePlan *korifiv1alpha1.CFServicePlan) { cfServicePlan.Spec.Visibility.Type = m.Type - cfServicePlan.Spec.Visibility.Organizations = uniq(m.Organizations) + cfServicePlan.Spec.Visibility.Organizations = tools.Uniq(m.Organizations) } func NewServicePlanRepo( @@ -211,8 +211,3 @@ func (r *ServicePlanRepo) toVisibilityOrganizations(ctx context.Context, authInf } }).Collect(), nil } - -func uniq(s []string) []string { - slices.Sort(s) - return slices.Compact(s) -} diff --git a/api/repositories/shared.go b/api/repositories/shared.go index ff938b7af..d4cc9bd59 100644 --- a/api/repositories/shared.go +++ b/api/repositories/shared.go @@ -3,7 +3,6 @@ package repositories import ( "context" "fmt" - "slices" "time" "code.cloudfoundry.org/korifi/api/authorization" @@ -72,14 +71,6 @@ func getLabelOrAnnotation(mapObj map[string]string, key string) string { return mapObj[key] } -func emptyOrContains[S ~[]E, E comparable](elements S, e E) bool { - if len(elements) == 0 { - return true - } - - return slices.Contains(elements, e) -} - func authorizedSpaceNamespaces(ctx context.Context, authInfo authorization.Info, namespacePermissions *authorization.NamespacePermissions) (*iter.LiftIter[string], error) { nsList, err := namespacePermissions.GetAuthorizedSpaceNamespaces(ctx, authInfo) if err != nil { diff --git a/api/repositories/space_repository.go b/api/repositories/space_repository.go index 3cf917da8..a5a615be7 100644 --- a/api/repositories/space_repository.go +++ b/api/repositories/space_repository.go @@ -8,6 +8,7 @@ import ( "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" @@ -36,12 +37,12 @@ type ListSpacesMessage struct { func (m *ListSpacesMessage) matches(space korifiv1alpha1.CFSpace) bool { return meta.IsStatusConditionTrue(space.Status.Conditions, korifiv1alpha1.StatusConditionReady) && - emptyOrContains(m.GUIDs, space.Name) && - emptyOrContains(m.Names, space.Spec.DisplayName) + tools.EmptyOrContains(m.GUIDs, space.Name) && + tools.EmptyOrContains(m.Names, space.Spec.DisplayName) } func (m *ListSpacesMessage) matchesNamespace(ns string) bool { - return emptyOrContains(m.OrganizationGUIDs, ns) + return tools.EmptyOrContains(m.OrganizationGUIDs, ns) } type DeleteSpaceMessage struct { diff --git a/api/repositories/task_repository.go b/api/repositories/task_repository.go index 21eb78cd3..8286e6736 100644 --- a/api/repositories/task_repository.go +++ b/api/repositories/task_repository.go @@ -9,6 +9,7 @@ import ( apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/tasks" + "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/BooleanCat/go-functional/iter" "github.com/google/uuid" @@ -61,8 +62,8 @@ type ListTaskMessage struct { } func (m *ListTaskMessage) matches(task korifiv1alpha1.CFTask) bool { - return emptyOrContains(m.SequenceIDs, task.Status.SequenceID) && - emptyOrContains(m.AppGUIDs, task.Spec.AppRef.Name) + return tools.EmptyOrContains(m.SequenceIDs, task.Status.SequenceID) && + tools.EmptyOrContains(m.AppGUIDs, task.Spec.AppRef.Name) } type PatchTaskMetadataMessage struct { diff --git a/model/included.go b/model/included.go new file mode 100644 index 000000000..a7f9b94a4 --- /dev/null +++ b/model/included.go @@ -0,0 +1,6 @@ +package model + +type IncludedResource struct { + Type string + Resource any +} diff --git a/tools/collections.go b/tools/collections.go new file mode 100644 index 000000000..42924960f --- /dev/null +++ b/tools/collections.go @@ -0,0 +1,19 @@ +package tools + +import ( + "cmp" + "slices" +) + +func Uniq[S ~[]E, E cmp.Ordered](elements S) S { + slices.Sort(elements) + return slices.Compact(elements) +} + +func EmptyOrContains[S ~[]E, E comparable](elements S, e E) bool { + if len(elements) == 0 { + return true + } + + return slices.Contains(elements, e) +} diff --git a/tools/collections_test.go b/tools/collections_test.go new file mode 100644 index 000000000..66d45dc89 --- /dev/null +++ b/tools/collections_test.go @@ -0,0 +1,26 @@ +package tools_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/onsi/gomega/types" + + "code.cloudfoundry.org/korifi/tools" +) + +var _ = Describe("Collections", func() { + Describe("Uniq", func() { + It("deduplicates the slice", func() { + Expect(tools.Uniq([]string{"b", "a", "b", "a"})).To(ConsistOf("a", "b")) + }) + }) + + DescribeTable("EmptyOrContains", + func(slice []string, match types.GomegaMatcher) { + Expect(tools.EmptyOrContains(slice, "needle")).To(match) + }, + Entry("empty slice", []string{}, BeTrue()), + Entry("contains element", []string{"nail", "needle", "pin"}, BeTrue()), + Entry("does not contain element", []string{"nail", "pin"}, BeFalse()), + ) +})