From ccc1851c88662859173b457c610adff986ece5b3 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Tue, 6 Aug 2024 14:53:25 +0000 Subject: [PATCH] Implement PATCH /v3/service_plans/{guid}/visibility Also - Add visibility_type to the service plan rest resource - Implement filtering service offerings by broke name - Implement filtering service plans by name The above improvements enable certain options of the `cf service-acces` and the `cf enable-service-access` commands fixes #3276 Co-authored-by: Georgi Sabev --- .../fake/cfservice_plan_repository.go | 83 +++++ api/handlers/service_offering_test.go | 8 +- api/handlers/service_plan.go | 26 +- api/handlers/service_plan_test.go | 86 ++++- api/main.go | 4 +- api/payloads/service_offering.go | 9 +- api/payloads/service_offering_test.go | 6 +- api/payloads/service_plan.go | 53 ++- api/payloads/service_plan_test.go | 86 ++++- api/presenter/service_offering.go | 24 +- api/presenter/service_offering_test.go | 8 +- api/presenter/service_plan.go | 35 +- api/presenter/service_plan_test.go | 26 +- .../service_offering_repository.go | 60 +++- .../service_offering_repository_test.go | 62 +++- api/repositories/service_plan_repository.go | 121 +++++-- .../service_plan_repository_test.go | 335 +++++++++++++++--- .../api/v1alpha1/cfservice_plan_types.go | 9 +- .../api/v1alpha1/zz_generated.deepcopy.go | 7 +- .../services/brokers/controller_test.go | 3 +- ...orifi.cloudfoundry.org_cfserviceplans.yaml | 5 + model/services/plans.go | 5 + tests/e2e/service_plans_test.go | 31 +- 23 files changed, 930 insertions(+), 162 deletions(-) diff --git a/api/handlers/fake/cfservice_plan_repository.go b/api/handlers/fake/cfservice_plan_repository.go index 27f1a0a30..d471d0b16 100644 --- a/api/handlers/fake/cfservice_plan_repository.go +++ b/api/handlers/fake/cfservice_plan_repository.go @@ -56,6 +56,21 @@ type CFServicePlanRepository struct { result1 []repositories.ServicePlanRecord result2 error } + UpdatePlanVisibilityStub func(context.Context, authorization.Info, repositories.UpdateServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error) + updatePlanVisibilityMutex sync.RWMutex + updatePlanVisibilityArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.UpdateServicePlanVisibilityMessage + } + updatePlanVisibilityReturns struct { + result1 repositories.ServicePlanRecord + result2 error + } + updatePlanVisibilityReturnsOnCall map[int]struct { + result1 repositories.ServicePlanRecord + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -258,6 +273,72 @@ func (fake *CFServicePlanRepository) ListPlansReturnsOnCall(i int, result1 []rep }{result1, result2} } +func (fake *CFServicePlanRepository) UpdatePlanVisibility(arg1 context.Context, arg2 authorization.Info, arg3 repositories.UpdateServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error) { + fake.updatePlanVisibilityMutex.Lock() + ret, specificReturn := fake.updatePlanVisibilityReturnsOnCall[len(fake.updatePlanVisibilityArgsForCall)] + fake.updatePlanVisibilityArgsForCall = append(fake.updatePlanVisibilityArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.UpdateServicePlanVisibilityMessage + }{arg1, arg2, arg3}) + stub := fake.UpdatePlanVisibilityStub + fakeReturns := fake.updatePlanVisibilityReturns + fake.recordInvocation("UpdatePlanVisibility", []interface{}{arg1, arg2, arg3}) + fake.updatePlanVisibilityMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *CFServicePlanRepository) UpdatePlanVisibilityCallCount() int { + fake.updatePlanVisibilityMutex.RLock() + defer fake.updatePlanVisibilityMutex.RUnlock() + return len(fake.updatePlanVisibilityArgsForCall) +} + +func (fake *CFServicePlanRepository) UpdatePlanVisibilityCalls(stub func(context.Context, authorization.Info, repositories.UpdateServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error)) { + fake.updatePlanVisibilityMutex.Lock() + defer fake.updatePlanVisibilityMutex.Unlock() + fake.UpdatePlanVisibilityStub = stub +} + +func (fake *CFServicePlanRepository) UpdatePlanVisibilityArgsForCall(i int) (context.Context, authorization.Info, repositories.UpdateServicePlanVisibilityMessage) { + fake.updatePlanVisibilityMutex.RLock() + defer fake.updatePlanVisibilityMutex.RUnlock() + argsForCall := fake.updatePlanVisibilityArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServicePlanRepository) UpdatePlanVisibilityReturns(result1 repositories.ServicePlanRecord, result2 error) { + fake.updatePlanVisibilityMutex.Lock() + defer fake.updatePlanVisibilityMutex.Unlock() + fake.UpdatePlanVisibilityStub = nil + fake.updatePlanVisibilityReturns = struct { + result1 repositories.ServicePlanRecord + result2 error + }{result1, result2} +} + +func (fake *CFServicePlanRepository) UpdatePlanVisibilityReturnsOnCall(i int, result1 repositories.ServicePlanRecord, result2 error) { + fake.updatePlanVisibilityMutex.Lock() + defer fake.updatePlanVisibilityMutex.Unlock() + fake.UpdatePlanVisibilityStub = nil + if fake.updatePlanVisibilityReturnsOnCall == nil { + fake.updatePlanVisibilityReturnsOnCall = make(map[int]struct { + result1 repositories.ServicePlanRecord + result2 error + }) + } + fake.updatePlanVisibilityReturnsOnCall[i] = struct { + result1 repositories.ServicePlanRecord + result2 error + }{result1, result2} +} + func (fake *CFServicePlanRepository) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -267,6 +348,8 @@ func (fake *CFServicePlanRepository) Invocations() map[string][][]interface{} { defer fake.getPlanMutex.RUnlock() fake.listPlansMutex.RLock() defer fake.listPlansMutex.RUnlock() + fake.updatePlanVisibilityMutex.RLock() + defer fake.updatePlanVisibilityMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/api/handlers/service_offering_test.go b/api/handlers/service_offering_test.go index f86239124..12065c1d4 100644 --- a/api/handlers/service_offering_test.go +++ b/api/handlers/service_offering_test.go @@ -41,13 +41,7 @@ var _ = Describe("ServiceOffering", func() { CFResource: model.CFResource{ GUID: "offering-guid", }, - Relationships: repositories.ServiceOfferingRelationships{ - ServiceBroker: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: "broker-guid", - }, - }, - }, + ServiceBrokerGUID: "broker-guid", }}, nil) }) diff --git a/api/handlers/service_plan.go b/api/handlers/service_plan.go index 905fb49c7..35f838bc7 100644 --- a/api/handlers/service_plan.go +++ b/api/handlers/service_plan.go @@ -25,6 +25,7 @@ type CFServicePlanRepository interface { GetPlan(context.Context, authorization.Info, string) (repositories.ServicePlanRecord, error) ListPlans(context.Context, authorization.Info, repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error) ApplyPlanVisibility(context.Context, authorization.Info, repositories.ApplyServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error) + UpdatePlanVisibility(context.Context, authorization.Info, repositories.UpdateServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error) } type ServicePlan struct { @@ -79,7 +80,7 @@ func (h *ServicePlan) getPlanVisibility(r *http.Request) (*routing.Response, err func (h *ServicePlan) applyPlanVisibility(r *http.Request) (*routing.Response, error) { authInfo, _ := authorization.InfoFromContext(r.Context()) - logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.get-visibility") + logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.apply-visibility") planGUID := routing.URLParam(r, "guid") logger = logger.WithValues("guid", planGUID) @@ -89,7 +90,7 @@ func (h *ServicePlan) applyPlanVisibility(r *http.Request) (*routing.Response, e return nil, apierrors.LogAndReturn(logger, err, "failed to decode json payload") } - visibility, err := h.servicePlanRepo.ApplyPlanVisibility(r.Context(), authInfo, payload.ToMessage(planGUID)) + visibility, err := h.servicePlanRepo.ApplyPlanVisibility(r.Context(), authInfo, payload.ToApplyMessage(planGUID)) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to apply plan visibility") } @@ -97,6 +98,26 @@ func (h *ServicePlan) applyPlanVisibility(r *http.Request) (*routing.Response, e return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServicePlanVisibility(visibility, h.serverURL)), nil } +func (h *ServicePlan) updatePlanVisibility(r *http.Request) (*routing.Response, error) { + authInfo, _ := authorization.InfoFromContext(r.Context()) + logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.update-visibility") + + planGUID := routing.URLParam(r, "guid") + logger = logger.WithValues("guid", planGUID) + + payload := payloads.ServicePlanVisibility{} + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to decode json payload") + } + + visibility, err := h.servicePlanRepo.UpdatePlanVisibility(r.Context(), authInfo, payload.ToUpdateMessage(planGUID)) + if err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to update plan visibility") + } + + return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServicePlanVisibility(visibility, h.serverURL)), nil +} + func (h *ServicePlan) UnauthenticatedRoutes() []routing.Route { return nil } @@ -106,5 +127,6 @@ func (h *ServicePlan) AuthenticatedRoutes() []routing.Route { {Method: "GET", Pattern: ServicePlansPath, Handler: h.list}, {Method: "GET", Pattern: ServicePlanVisivilityPath, Handler: h.getPlanVisibility}, {Method: "POST", Pattern: ServicePlanVisivilityPath, Handler: h.applyPlanVisibility}, + {Method: "PATCH", Pattern: ServicePlanVisivilityPath, Handler: h.updatePlanVisibility}, } } diff --git a/api/handlers/service_plan_test.go b/api/handlers/service_plan_test.go index 574d396e0..9317aacd0 100644 --- a/api/handlers/service_plan_test.go +++ b/api/handlers/service_plan_test.go @@ -41,13 +41,7 @@ var _ = Describe("ServicePlan", func() { CFResource: model.CFResource{ GUID: "plan-guid", }, - Relationships: repositories.ServicePlanRelationships{ - ServiceOffering: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: "service-offering-guid", - }, - }, - }, + ServiceOfferingGUID: "service-offering-guid", }}, nil) }) @@ -112,7 +106,9 @@ var _ = Describe("ServicePlan", func() { Describe("GET /v3/service_plans/{guid}/visibility", func() { BeforeEach(func() { servicePlanRepo.GetPlanReturns(repositories.ServicePlanRecord{ - VisibilityType: korifiv1alpha1.AdminServicePlanVisibilityType, + Visibility: repositories.PlanVisibility{ + Type: korifiv1alpha1.AdminServicePlanVisibilityType, + }, }, nil) }) @@ -155,7 +151,9 @@ var _ = Describe("ServicePlan", func() { }) servicePlanRepo.ApplyPlanVisibilityReturns(repositories.ServicePlanRecord{ - VisibilityType: korifiv1alpha1.PublicServicePlanVisibilityType, + Visibility: repositories.PlanVisibility{ + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + }, }, nil) }) @@ -177,8 +175,9 @@ var _ = Describe("ServicePlan", func() { _, actualAuthInfo, actualMessage := servicePlanRepo.ApplyPlanVisibilityArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) Expect(actualMessage).To(Equal(repositories.ApplyServicePlanVisibilityMessage{ - PlanGUID: "my-service-plan", - Type: korifiv1alpha1.PublicServicePlanVisibilityType, + PlanGUID: "my-service-plan", + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + Organizations: []string{}, })) Expect(rr).To(HaveHTTPStatus(http.StatusOK)) @@ -209,4 +208,69 @@ var _ = Describe("ServicePlan", func() { }) }) }) + + Describe("PATCH /v3/service_plans/{guid}/visibility", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + }) + + servicePlanRepo.UpdatePlanVisibilityReturns(repositories.ServicePlanRecord{ + Visibility: repositories.PlanVisibility{ + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + }, + }, nil) + }) + + JustBeforeEach(func() { + req, err := http.NewRequestWithContext(ctx, "PATCH", "/v3/service_plans/my-service-plan/visibility", strings.NewReader("the-payload")) + Expect(err).NotTo(HaveOccurred()) + + routerBuilder.Build().ServeHTTP(rr, req) + }) + + It("validates the payload", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + Expect(bodyString(actualReq)).To(Equal("the-payload")) + }) + + It("updates the plan visibility", func() { + Expect(servicePlanRepo.UpdatePlanVisibilityCallCount()).To(Equal(1)) + _, actualAuthInfo, actualMessage := servicePlanRepo.UpdatePlanVisibilityArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualMessage).To(Equal(repositories.UpdateServicePlanVisibilityMessage{ + PlanGUID: "my-service-plan", + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + Organizations: []string{}, + })) + + Expect(rr).To(HaveHTTPStatus(http.StatusOK)) + Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) + + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.type", korifiv1alpha1.PublicServicePlanVisibilityType), + ))) + }) + + When("the payload is invalid", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("invalid-payload")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + + When("updating the visibility fails", func() { + BeforeEach(func() { + servicePlanRepo.UpdatePlanVisibilityReturns(repositories.ServicePlanRecord{}, errors.New("visibility-err")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + }) }) diff --git a/api/main.go b/api/main.go index 6211ae704..33eae6d44 100644 --- a/api/main.go +++ b/api/main.go @@ -227,8 +227,8 @@ func main() { ) metricsRepo := repositories.NewMetricsRepo(userClientFactory) serviceBrokerRepo := repositories.NewServiceBrokerRepo(userClientFactory, cfg.RootNamespace) - serviceOfferingRepo := repositories.NewServiceOfferingRepo(userClientFactory, cfg.RootNamespace) - servicePlanRepo := repositories.NewServicePlanRepo(userClientFactory, cfg.RootNamespace) + serviceOfferingRepo := repositories.NewServiceOfferingRepo(userClientFactory, cfg.RootNamespace, serviceBrokerRepo) + servicePlanRepo := repositories.NewServicePlanRepo(userClientFactory, cfg.RootNamespace, orgRepo) processStats := actions.NewProcessStats(processRepo, appRepo, metricsRepo) manifest := actions.NewManifest( diff --git a/api/payloads/service_offering.go b/api/payloads/service_offering.go index 75579b792..f3031a49d 100644 --- a/api/payloads/service_offering.go +++ b/api/payloads/service_offering.go @@ -9,17 +9,19 @@ import ( ) type ServiceOfferingList struct { - Names string + Names string + BrokerNames string } func (l *ServiceOfferingList) ToMessage() repositories.ListServiceOfferingMessage { return repositories.ListServiceOfferingMessage{ - Names: parse.ArrayParam(l.Names), + Names: parse.ArrayParam(l.Names), + BrokerNames: parse.ArrayParam(l.BrokerNames), } } func (l *ServiceOfferingList) SupportedKeys() []string { - return []string{"names", "page", "per_page"} + return []string{"names", "service_broker_names", "page", "per_page"} } func (l *ServiceOfferingList) IgnoredKeys() []*regexp.Regexp { @@ -28,5 +30,6 @@ func (l *ServiceOfferingList) IgnoredKeys() []*regexp.Regexp { func (l *ServiceOfferingList) DecodeFromURLValues(values url.Values) error { l.Names = values.Get("names") + l.BrokerNames = values.Get("service_broker_names") return nil } diff --git a/api/payloads/service_offering_test.go b/api/payloads/service_offering_test.go index 70eca4c4e..af257dfb8 100644 --- a/api/payloads/service_offering_test.go +++ b/api/payloads/service_offering_test.go @@ -16,14 +16,16 @@ var _ = Describe("ServiceOffering", func() { Expect(*actualServiceOfferingList).To(Equal(expectedServiceOfferingList)) }, Entry("names", "names=b1,b2", payloads.ServiceOfferingList{Names: "b1,b2"}), + Entry("service_broker_names", "service_broker_names=b1,b2", payloads.ServiceOfferingList{BrokerNames: "b1,b2"}), ) Describe("ToMessage", func() { It("converts payload to repository message", func() { - payload := &payloads.ServiceOfferingList{Names: "b1,b2"} + payload := &payloads.ServiceOfferingList{Names: "b1,b2", BrokerNames: "br1,br2"} Expect(payload.ToMessage()).To(Equal(repositories.ListServiceOfferingMessage{ - Names: []string{"b1", "b2"}, + Names: []string{"b1", "b2"}, + BrokerNames: []string{"br1", "br2"}, })) }) }) diff --git a/api/payloads/service_plan.go b/api/payloads/service_plan.go index 46d5be01e..26eead826 100644 --- a/api/payloads/service_plan.go +++ b/api/payloads/service_plan.go @@ -1,25 +1,33 @@ package payloads import ( + "fmt" "net/url" "regexp" "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" + "code.cloudfoundry.org/korifi/model/services" + "github.com/BooleanCat/go-functional/iter" + jellidation "github.com/jellydator/validation" ) type ServicePlanList struct { ServiceOfferingGUIDs string + Names string } func (l *ServicePlanList) ToMessage() repositories.ListServicePlanMessage { return repositories.ListServicePlanMessage{ ServiceOfferingGUIDs: parse.ArrayParam(l.ServiceOfferingGUIDs), + Names: parse.ArrayParam(l.Names), } } func (l *ServicePlanList) SupportedKeys() []string { - return []string{"service_offering_guids", "page", "per_page", "include"} + return []string{"service_offering_guids", "names", "page", "per_page", "include"} } func (l *ServicePlanList) IgnoredKeys() []*regexp.Regexp { @@ -28,16 +36,55 @@ func (l *ServicePlanList) IgnoredKeys() []*regexp.Regexp { func (l *ServicePlanList) DecodeFromURLValues(values url.Values) error { l.ServiceOfferingGUIDs = values.Get("service_offering_guids") + l.Names = values.Get("names") return nil } type ServicePlanVisibility struct { - Type string `json:"type"` + Type string `json:"type"` + Organizations []services.VisibilityOrganization `json:"organizations"` } -func (p *ServicePlanVisibility) ToMessage(planGUID string) repositories.ApplyServicePlanVisibilityMessage { +func (p ServicePlanVisibility) Validate() error { + organizationsRule := jellidation.By(func(value any) error { + orgs, ok := value.([]services.VisibilityOrganization) + if !ok { + return fmt.Errorf("%T is not supported, []services.VisibilityOrganization is expected", value) + } + + if p.Type != korifiv1alpha1.OrganizationServicePlanVisibilityType { + return jellidation.Empty.Validate(orgs) + } + + return jellidation.Required.Validate(orgs) + }) + + return jellidation.ValidateStruct(&p, + jellidation.Field(&p.Type, validation.OneOf( + korifiv1alpha1.AdminServicePlanVisibilityType, + korifiv1alpha1.PublicServicePlanVisibilityType, + korifiv1alpha1.OrganizationServicePlanVisibilityType, + )), + jellidation.Field(&p.Organizations, organizationsRule), + ) +} + +func (p *ServicePlanVisibility) ToApplyMessage(planGUID string) repositories.ApplyServicePlanVisibilityMessage { return repositories.ApplyServicePlanVisibilityMessage{ PlanGUID: planGUID, Type: p.Type, + Organizations: iter.Map(iter.Lift(p.Organizations), func(v services.VisibilityOrganization) string { + return v.GUID + }).Collect(), + } +} + +func (p *ServicePlanVisibility) ToUpdateMessage(planGUID string) repositories.UpdateServicePlanVisibilityMessage { + return repositories.UpdateServicePlanVisibilityMessage{ + PlanGUID: planGUID, + Type: p.Type, + Organizations: iter.Map(iter.Lift(p.Organizations), func(v services.VisibilityOrganization) string { + return v.GUID + }).Collect(), } } diff --git a/api/payloads/service_plan_test.go b/api/payloads/service_plan_test.go index 87e99ab12..dbf3e9ae1 100644 --- a/api/payloads/service_plan_test.go +++ b/api/payloads/service_plan_test.go @@ -4,8 +4,10 @@ import ( "code.cloudfoundry.org/korifi/api/payloads" "code.cloudfoundry.org/korifi/api/repositories" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/model/services" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/types" ) var _ = Describe("ServicePlan", func() { @@ -18,30 +20,98 @@ var _ = Describe("ServicePlan", func() { Expect(*actualServicePlanList).To(Equal(expectedServicePlanList)) }, Entry("service_offering_guids", "service_offering_guids=b1,b2", payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2"}), + Entry("names", "names=b1,b2", payloads.ServicePlanList{Names: "b1,b2"}), ) Describe("ToMessage", func() { It("converts payload to repository message", func() { - payload := &payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2"} + payload := &payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2", Names: "n1,n2"} Expect(payload.ToMessage()).To(Equal(repositories.ListServicePlanMessage{ ServiceOfferingGUIDs: []string{"b1", "b2"}, + Names: []string{"n1", "n2"}, })) }) }) }) Describe("Visibility", func() { + Describe("Validation", func() { + admin := payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.AdminServicePlanVisibilityType, + } + adminWithOrgs := payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.AdminServicePlanVisibilityType, + Organizations: []services.VisibilityOrganization{{ + GUID: "foo", + }}, + } + + public := payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + } + publicWithOrgs := payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + Organizations: []services.VisibilityOrganization{{ + GUID: "foo", + }}, + } + + org := payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []services.VisibilityOrganization{{ + GUID: "foo", + }}, + } + orgNoOrgs := payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + } + + invalidType := payloads.ServicePlanVisibility{Type: "invalid"} + + DescribeTable("Validation", + func(payload payloads.ServicePlanVisibility, validationMatcher types.GomegaMatcher) { + Expect(payload.Validate()).To(validationMatcher) + }, + + Entry("invalid type", invalidType, MatchError(ContainSubstring("type: value must be one of"))), + Entry("admin", admin, Not(HaveOccurred())), + Entry("admin with orgs", adminWithOrgs, MatchError(ContainSubstring("organizations: must be blank"))), + Entry("public", public, Not(HaveOccurred())), + Entry("public with orgs", publicWithOrgs, MatchError(ContainSubstring("organizations: must be blank"))), + Entry("org", org, Not(HaveOccurred())), + Entry("org no orgs", orgNoOrgs, MatchError(ContainSubstring("organizations: cannot be blank"))), + ) + }) + Describe("ToMessage", func() { - It("converts payload to repository message", func() { - payload := &payloads.ServicePlanVisibility{ - Type: korifiv1alpha1.PublicServicePlanVisibilityType, + var payload *payloads.ServicePlanVisibility + + BeforeEach(func() { + payload = &payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []services.VisibilityOrganization{{GUID: "org-guid"}}, } + }) - Expect(payload.ToMessage("plan-guid")).To(Equal(repositories.ApplyServicePlanVisibilityMessage{ - PlanGUID: "plan-guid", - Type: korifiv1alpha1.PublicServicePlanVisibilityType, - })) + Describe("ToApplyMessage", func() { + It("converts payload to repository message", func() { + Expect(payload.ToApplyMessage("plan-guid")).To(Equal(repositories.ApplyServicePlanVisibilityMessage{ + PlanGUID: "plan-guid", + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []string{"org-guid"}, + })) + }) + }) + + Describe("ToUpdateMessage", func() { + It("converts payload to repository message", func() { + Expect(payload.ToUpdateMessage("plan-guid")).To(Equal(repositories.UpdateServicePlanVisibilityMessage{ + PlanGUID: "plan-guid", + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []string{"org-guid"}, + })) + }) }) }) }) diff --git a/api/presenter/service_offering.go b/api/presenter/service_offering.go index 5b70a04f6..817fcb91b 100644 --- a/api/presenter/service_offering.go +++ b/api/presenter/service_offering.go @@ -4,6 +4,8 @@ import ( "net/url" "code.cloudfoundry.org/korifi/api/repositories" + "code.cloudfoundry.org/korifi/model" + "code.cloudfoundry.org/korifi/model/services" ) const ( @@ -18,13 +20,27 @@ type ServiceOfferingLinks struct { } type ServiceOfferingResponse struct { - repositories.ServiceOfferingRecord - Links ServiceOfferingLinks `json:"links"` + services.ServiceOffering + model.CFResource + Relationships ServiceOfferingRelationships `json:"relationships"` + Links ServiceOfferingLinks `json:"links"` +} + +type ServiceOfferingRelationships struct { + ServiceBroker model.ToOneRelationship `json:"service_broker"` } func ForServiceOffering(serviceOffering repositories.ServiceOfferingRecord, baseURL url.URL) ServiceOfferingResponse { return ServiceOfferingResponse{ - ServiceOfferingRecord: serviceOffering, + ServiceOffering: serviceOffering.ServiceOffering, + CFResource: serviceOffering.CFResource, + Relationships: ServiceOfferingRelationships{ + ServiceBroker: model.ToOneRelationship{ + Data: model.Relationship{ + GUID: serviceOffering.ServiceBrokerGUID, + }, + }, + }, Links: ServiceOfferingLinks{ Self: Link{ HRef: buildURL(baseURL).appendPath(serviceOfferingsBase, serviceOffering.GUID).build(), @@ -33,7 +49,7 @@ func ForServiceOffering(serviceOffering repositories.ServiceOfferingRecord, base HRef: buildURL(baseURL).appendPath(servicePlansBase).setQuery("service_offering_guids=" + serviceOffering.GUID).build(), }, ServiceBroker: Link{ - HRef: buildURL(baseURL).appendPath(serviceBrokersBase, serviceOffering.Relationships.ServiceBroker.Data.GUID).build(), + HRef: buildURL(baseURL).appendPath(serviceBrokersBase, serviceOffering.ServiceBrokerGUID).build(), }, }, } diff --git a/api/presenter/service_offering_test.go b/api/presenter/service_offering_test.go index 110a37410..258260973 100644 --- a/api/presenter/service_offering_test.go +++ b/api/presenter/service_offering_test.go @@ -60,13 +60,7 @@ var _ = Describe("Service Offering", func() { }, }, }, - Relationships: repositories.ServiceOfferingRelationships{ - ServiceBroker: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: "broker-guid", - }, - }, - }, + ServiceBrokerGUID: "broker-guid", } }) diff --git a/api/presenter/service_plan.go b/api/presenter/service_plan.go index b1c295ff3..8973fa011 100644 --- a/api/presenter/service_plan.go +++ b/api/presenter/service_plan.go @@ -4,7 +4,8 @@ import ( "net/url" "code.cloudfoundry.org/korifi/api/repositories" - korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/model" + "code.cloudfoundry.org/korifi/model/services" ) type ServicePlanLinks struct { @@ -13,20 +14,36 @@ type ServicePlanLinks struct { Visibility Link `json:"visibility"` } +type ServicePlanRelationships struct { + ServiceOffering model.ToOneRelationship `json:"service_offering"` +} + type ServicePlanResponse struct { - repositories.ServicePlanRecord - Links ServicePlanLinks `json:"links"` + services.ServicePlan + model.CFResource + VisibilityType string `json:"visibility_type"` + Relationships ServicePlanRelationships `json:"relationships"` + Links ServicePlanLinks `json:"links"` } func ForServicePlan(servicePlan repositories.ServicePlanRecord, baseURL url.URL) ServicePlanResponse { return ServicePlanResponse{ - ServicePlanRecord: servicePlan, + ServicePlan: servicePlan.ServicePlan, + CFResource: servicePlan.CFResource, + VisibilityType: servicePlan.Visibility.Type, + Relationships: ServicePlanRelationships{ + ServiceOffering: model.ToOneRelationship{ + Data: model.Relationship{ + GUID: servicePlan.ServiceOfferingGUID, + }, + }, + }, Links: ServicePlanLinks{ Self: Link{ HRef: buildURL(baseURL).appendPath(servicePlansBase, servicePlan.GUID).build(), }, ServiceOffering: Link{ - HRef: buildURL(baseURL).appendPath(serviceOfferingsBase, servicePlan.Relationships.ServiceOffering.Data.GUID).build(), + HRef: buildURL(baseURL).appendPath(serviceOfferingsBase, servicePlan.ServiceOfferingGUID).build(), }, Visibility: Link{ HRef: buildURL(baseURL).appendPath(servicePlansBase, servicePlan.GUID, "visibility").build(), @@ -35,10 +52,14 @@ func ForServicePlan(servicePlan repositories.ServicePlanRecord, baseURL url.URL) } } -type ServicePlanVisibilityResponse korifiv1alpha1.ServicePlanVisibility +type ServicePlanVisibilityResponse struct { + Type string `json:"type"` + Organizations []services.VisibilityOrganization `json:"organizations,omitempty"` +} func ForServicePlanVisibility(plan repositories.ServicePlanRecord, _ url.URL) ServicePlanVisibilityResponse { return ServicePlanVisibilityResponse{ - Type: plan.VisibilityType, + Type: plan.Visibility.Type, + Organizations: plan.Visibility.Organizations, } } diff --git a/api/presenter/service_plan_test.go b/api/presenter/service_plan_test.go index f81ac76da..6a527ba85 100644 --- a/api/presenter/service_plan_test.go +++ b/api/presenter/service_plan_test.go @@ -81,14 +81,10 @@ var _ = Describe("Service Plan", func() { }, }, }, - VisibilityType: "visibility-type", - Relationships: repositories.ServicePlanRelationships{ - ServiceOffering: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: "service-offering-guid", - }, - }, + Visibility: repositories.PlanVisibility{ + Type: "visibility-type", }, + ServiceOfferingGUID: "service-offering-guid", } }) @@ -174,7 +170,13 @@ var _ = Describe("Service Plan", func() { BeforeEach(func() { record = repositories.ServicePlanRecord{ - VisibilityType: "admin", + Visibility: repositories.PlanVisibility{ + Type: "organization", + Organizations: []services.VisibilityOrganization{{ + GUID: "org-guid", + Name: "org-name", + }}, + }, } }) @@ -187,7 +189,13 @@ var _ = Describe("Service Plan", func() { It("returns the expected JSON", func() { Expect(output).To(MatchJSON(`{ - "type": "admin" + "type": "organization", + "organizations": [ + { + "guid": "org-guid", + "name": "org-name" + } + ] }`)) }) }) diff --git a/api/repositories/service_offering_repository.go b/api/repositories/service_offering_repository.go index c7f962064..4f3937e94 100644 --- a/api/repositories/service_offering_repository.go +++ b/api/repositories/service_offering_repository.go @@ -3,6 +3,7 @@ package repositories import ( "context" "fmt" + "slices" "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" @@ -19,33 +20,33 @@ const ServiceOfferingResourceType = "Service Offering" type ServiceOfferingRecord struct { services.ServiceOffering model.CFResource - Relationships ServiceOfferingRelationships `json:"relationships"` -} - -type ServiceOfferingRelationships struct { - ServiceBroker model.ToOneRelationship `json:"service_broker"` + ServiceBrokerGUID string } type ServiceOfferingRepo struct { userClientFactory authorization.UserK8sClientFactory rootNamespace string + brokerRepo *ServiceBrokerRepo } type ListServiceOfferingMessage struct { - Names []string + Names []string + BrokerNames []string } -func (m *ListServiceOfferingMessage) matches(cfServiceOffering korifiv1alpha1.CFServiceOffering) bool { +func (m *ListServiceOfferingMessage) matchesName(cfServiceOffering korifiv1alpha1.CFServiceOffering) bool { return emptyOrContains(m.Names, cfServiceOffering.Spec.Name) } func NewServiceOfferingRepo( userClientFactory authorization.UserK8sClientFactory, rootNamespace string, + brokerRepo *ServiceBrokerRepo, ) *ServiceOfferingRepo { return &ServiceOfferingRepo{ userClientFactory: userClientFactory, rootNamespace: rootNamespace, + brokerRepo: brokerRepo, } } @@ -67,7 +68,42 @@ func (r *ServiceOfferingRepo) ListOfferings(ctx context.Context, authInfo author ) } - return iter.Map(iter.Lift(offeringsList.Items).Filter(message.matches), offeringToRecord).Collect(), nil + filteredByName := iter.Map(iter.Lift(offeringsList.Items).Filter(message.matchesName), offeringToRecord).Collect() + + filteredByBroker := []ServiceOfferingRecord{} + for _, offering := range filteredByName { + matchesBroker, err := r.matchesBroker(ctx, authInfo, offering, message) + if err != nil { + return nil, err + } + if matchesBroker { + filteredByBroker = append(filteredByBroker, offering) + } + } + + return filteredByBroker, nil +} + +func (r *ServiceOfferingRepo) matchesBroker( + ctx context.Context, + authInfo authorization.Info, + offering ServiceOfferingRecord, + message ListServiceOfferingMessage, +) (bool, error) { + if len(message.BrokerNames) == 0 { + return true, nil + } + + brokers, err := r.brokerRepo.ListServiceBrokers(ctx, authInfo, ListServiceBrokerMessage{ + Names: message.BrokerNames, + }) + if err != nil { + return false, fmt.Errorf("failed to list brokers when filtering offerings: %w", err) + } + + return slices.ContainsFunc(brokers, func(b ServiceBrokerRecord) bool { + return b.GUID == offering.ServiceBrokerGUID + }), nil } func offeringToRecord(offering korifiv1alpha1.CFServiceOffering) ServiceOfferingRecord { @@ -81,12 +117,6 @@ func offeringToRecord(offering korifiv1alpha1.CFServiceOffering) ServiceOffering Annotations: offering.Annotations, }, }, - Relationships: ServiceOfferingRelationships{ - ServiceBroker: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: offering.Labels[korifiv1alpha1.RelServiceBrokerLabel], - }, - }, - }, + ServiceBrokerGUID: offering.Labels[korifiv1alpha1.RelServiceBrokerLabel], } } diff --git a/api/repositories/service_offering_repository_test.go b/api/repositories/service_offering_repository_test.go index 1f07b0b87..ca89fd47d 100644 --- a/api/repositories/service_offering_repository_test.go +++ b/api/repositories/service_offering_repository_test.go @@ -3,7 +3,6 @@ package repositories_test import ( "code.cloudfoundry.org/korifi/api/repositories" 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/onsi/gomega/gstruct" @@ -19,12 +18,20 @@ var _ = Describe("ServiceOfferingRepo", func() { var repo *repositories.ServiceOfferingRepo BeforeEach(func() { - repo = repositories.NewServiceOfferingRepo(userClientFactory, rootNamespace) + repo = repositories.NewServiceOfferingRepo( + userClientFactory, + rootNamespace, + repositories.NewServiceBrokerRepo( + userClientFactory, + rootNamespace, + ), + ) }) Describe("List", func() { var ( offeringGUID string + broker *korifiv1alpha1.CFServiceBroker listedOfferings []repositories.ServiceOfferingRecord message repositories.ListServiceOfferingMessage listErr error @@ -32,12 +39,26 @@ var _ = Describe("ServiceOfferingRepo", func() { BeforeEach(func() { offeringGUID = uuid.NewString() + + broker = &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + }, + Spec: korifiv1alpha1.CFServiceBrokerSpec{ + ServiceBroker: services.ServiceBroker{ + Name: uuid.NewString(), + }, + }, + } + Expect(k8sClient.Create(ctx, broker)).To(Succeed()) + Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServiceOffering{ ObjectMeta: metav1.ObjectMeta{ Namespace: rootNamespace, Name: offeringGUID, Labels: map[string]string{ - korifiv1alpha1.RelServiceBrokerLabel: "broker-guid", + korifiv1alpha1.RelServiceBrokerLabel: broker.Name, }, Annotations: map[string]string{ "annotation": "annotation-value", @@ -103,17 +124,11 @@ var _ = Describe("ServiceOfferingRepo", func() { "CreatedAt": Not(BeZero()), "UpdatedAt": BeNil(), "Metadata": MatchAllFields(Fields{ - "Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, "broker-guid"), + "Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, broker.Name), "Annotations": HaveKeyWithValue("annotation", "annotation-value"), }), }), - "Relationships": Equal(repositories.ServiceOfferingRelationships{ - ServiceBroker: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: "broker-guid", - }, - }, - }), + "ServiceBrokerGUID": Equal(broker.Name), }))) }) @@ -143,5 +158,30 @@ var _ = Describe("ServiceOfferingRepo", func() { }))) }) }) + + When("filtering by broker name", func() { + BeforeEach(func() { + Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServiceOffering{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + Labels: map[string]string{ + korifiv1alpha1.RelServiceBrokerLabel: uuid.NewString(), + }, + }, + })).To(Succeed()) + + message.BrokerNames = []string{broker.Spec.Name} + }) + + It("returns the matching offerings", func() { + Expect(listErr).NotTo(HaveOccurred()) + Expect(listedOfferings).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ + "ServiceOffering": MatchFields(IgnoreExtras, Fields{ + "Name": Equal("my-offering"), + }), + }))) + }) + }) }) }) diff --git a/api/repositories/service_plan_repository.go b/api/repositories/service_plan_repository.go index 11fd4e614..5dd653f12 100644 --- a/api/repositories/service_plan_repository.go +++ b/api/repositories/service_plan_repository.go @@ -3,6 +3,7 @@ package repositories import ( "context" "fmt" + "slices" "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" @@ -22,39 +23,68 @@ const ( type ServicePlanRecord struct { services.ServicePlan model.CFResource - VisibilityType string `json:"visibility_type"` - Relationships ServicePlanRelationships `json:"relationships"` + Visibility PlanVisibility + ServiceOfferingGUID string } -type ServicePlanRelationships struct { - ServiceOffering model.ToOneRelationship `json:"service_offering"` +type PlanVisibility struct { + Type string + Organizations []services.VisibilityOrganization } type ServicePlanRepo struct { userClientFactory authorization.UserK8sClientFactory rootNamespace string + orgRepo *OrgRepo } type ListServicePlanMessage struct { ServiceOfferingGUIDs []string + Names []string } func (m *ListServicePlanMessage) matches(cfServicePlan korifiv1alpha1.CFServicePlan) bool { - return emptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel]) + return emptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel]) && + emptyOrContains(m.Names, cfServicePlan.Spec.Name) } type ApplyServicePlanVisibilityMessage struct { - PlanGUID string - Type string + PlanGUID string + Type string + Organizations []string +} + +func (m *ApplyServicePlanVisibilityMessage) apply(cfServicePlan *korifiv1alpha1.CFServicePlan) { + cfServicePlan.Spec.Visibility.Type = m.Type + cfServicePlan.Spec.Visibility.Organizations = uniq(append( + cfServicePlan.Spec.Visibility.Organizations, + m.Organizations..., + )) + if m.Type != korifiv1alpha1.OrganizationServicePlanVisibilityType { + cfServicePlan.Spec.Visibility.Organizations = []string{} + } +} + +type UpdateServicePlanVisibilityMessage struct { + PlanGUID string + Type string + Organizations []string +} + +func (m *UpdateServicePlanVisibilityMessage) apply(cfServicePlan *korifiv1alpha1.CFServicePlan) { + cfServicePlan.Spec.Visibility.Type = m.Type + cfServicePlan.Spec.Visibility.Organizations = uniq(m.Organizations) } func NewServicePlanRepo( userClientFactory authorization.UserK8sClientFactory, rootNamespace string, + orgRepo *OrgRepo, ) *ServicePlanRepo { return &ServicePlanRepo{ userClientFactory: userClientFactory, rootNamespace: rootNamespace, + orgRepo: orgRepo, } } @@ -69,7 +99,18 @@ func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization. return nil, apierrors.FromK8sError(err, ServicePlanResourceType) } - return iter.Map(iter.Lift(cfServicePlans.Items).Filter(message.matches), planToRecord).Collect(), nil + filteredPlans := iter.Lift(cfServicePlans.Items).Filter(message.matches).Collect() + + planRecords := []ServicePlanRecord{} + for _, plan := range filteredPlans { + record, err := r.planToRecord(ctx, authInfo, plan) + if err != nil { + return nil, err + } + planRecords = append(planRecords, record) + } + + return planRecords, nil } func (r *ServicePlanRepo) GetPlan(ctx context.Context, authInfo authorization.Info, planGUID string) (ServicePlanRecord, error) { @@ -89,10 +130,23 @@ func (r *ServicePlanRepo) GetPlan(ctx context.Context, authInfo authorization.In if err != nil { return ServicePlanRecord{}, apierrors.FromK8sError(err, ServicePlanVisibilityResourceType) } - return planToRecord(*cfServicePlan), nil + return r.planToRecord(ctx, authInfo, *cfServicePlan) } func (r *ServicePlanRepo) ApplyPlanVisibility(ctx context.Context, authInfo authorization.Info, message ApplyServicePlanVisibilityMessage) (ServicePlanRecord, error) { + return r.patchServicePlan(ctx, authInfo, message.PlanGUID, message.apply) +} + +func (r *ServicePlanRepo) UpdatePlanVisibility(ctx context.Context, authInfo authorization.Info, message UpdateServicePlanVisibilityMessage) (ServicePlanRecord, error) { + return r.patchServicePlan(ctx, authInfo, message.PlanGUID, message.apply) +} + +func (r *ServicePlanRepo) patchServicePlan( + ctx context.Context, + authInfo authorization.Info, + planGUID string, + patchFunc func(*korifiv1alpha1.CFServicePlan), +) (ServicePlanRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { return ServicePlanRecord{}, fmt.Errorf("failed to build user client: %w", err) @@ -101,20 +155,29 @@ func (r *ServicePlanRepo) ApplyPlanVisibility(ctx context.Context, authInfo auth cfServicePlan := &korifiv1alpha1.CFServicePlan{ ObjectMeta: metav1.ObjectMeta{ Namespace: r.rootNamespace, - Name: message.PlanGUID, + Name: planGUID, }, } if err := PatchResource(ctx, userClient, cfServicePlan, func() { - cfServicePlan.Spec.Visibility.Type = message.Type + patchFunc(cfServicePlan) }); err != nil { return ServicePlanRecord{}, apierrors.FromK8sError(err, ServicePlanVisibilityResourceType) } - return planToRecord(*cfServicePlan), nil + return r.planToRecord(ctx, authInfo, *cfServicePlan) } -func planToRecord(plan korifiv1alpha1.CFServicePlan) ServicePlanRecord { +func (r *ServicePlanRepo) planToRecord(ctx context.Context, authInfo authorization.Info, plan korifiv1alpha1.CFServicePlan) (ServicePlanRecord, error) { + organizations := []services.VisibilityOrganization{} + if plan.Spec.Visibility.Type == korifiv1alpha1.OrganizationServicePlanVisibilityType { + var err error + organizations, err = r.toVisibilityOrganizations(ctx, authInfo, plan.Spec.Visibility.Organizations) + if err != nil { + return ServicePlanRecord{}, err + } + } + return ServicePlanRecord{ ServicePlan: plan.Spec.ServicePlan, CFResource: model.CFResource{ @@ -125,13 +188,31 @@ func planToRecord(plan korifiv1alpha1.CFServicePlan) ServicePlanRecord { Annotations: plan.Annotations, }, }, - VisibilityType: plan.Spec.Visibility.Type, - Relationships: ServicePlanRelationships{ - ServiceOffering: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: plan.Labels[korifiv1alpha1.RelServiceOfferingLabel], - }, - }, + Visibility: PlanVisibility{ + Type: plan.Spec.Visibility.Type, + Organizations: organizations, }, + ServiceOfferingGUID: plan.Labels[korifiv1alpha1.RelServiceOfferingLabel], + }, nil +} + +func (r *ServicePlanRepo) toVisibilityOrganizations(ctx context.Context, authInfo authorization.Info, orgGUIDs []string) ([]services.VisibilityOrganization, error) { + orgs, err := r.orgRepo.ListOrgs(ctx, authInfo, ListOrgsMessage{ + GUIDs: orgGUIDs, + }) + if err != nil { + return nil, fmt.Errorf("failed to list orgs for plan visibility: %w", err) } + + return iter.Map(iter.Lift(orgs), func(o OrgRecord) services.VisibilityOrganization { + return services.VisibilityOrganization{ + GUID: o.GUID, + Name: o.Name, + } + }).Collect(), nil +} + +func uniq(s []string) []string { + slices.Sort(s) + return slices.Compact(s) } diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index a05abf508..a5929c9f9 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -3,10 +3,11 @@ package repositories_test import ( apierrors "code.cloudfoundry.org/korifi/api/errors" "code.cloudfoundry.org/korifi/api/repositories" + "code.cloudfoundry.org/korifi/api/repositories/fakeawaiter" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/model" "code.cloudfoundry.org/korifi/model/services" "code.cloudfoundry.org/korifi/tests/matchers" + "code.cloudfoundry.org/korifi/tools/k8s" . "github.com/onsi/gomega/gstruct" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -24,7 +25,12 @@ var _ = Describe("ServicePlanRepo", func() { ) BeforeEach(func() { - repo = repositories.NewServicePlanRepo(userClientFactory, rootNamespace) + orgRepo := repositories.NewOrgRepo(rootNamespace, k8sClient, userClientFactory, nsPerms, &fakeawaiter.FakeAwaiter[ + *korifiv1alpha1.CFOrg, + korifiv1alpha1.CFOrgList, + *korifiv1alpha1.CFOrgList, + ]{}) + repo = repositories.NewServicePlanRepo(userClientFactory, rootNamespace, orgRepo) planGUID = uuid.NewString() Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServicePlan{ @@ -139,14 +145,11 @@ var _ = Describe("ServicePlanRepo", func() { "Annotations": HaveKeyWithValue("annotation", "annotation-value"), }), }), - "VisibilityType": Equal(korifiv1alpha1.AdminServicePlanVisibilityType), - "Relationships": Equal(repositories.ServicePlanRelationships{ - ServiceOffering: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: "offering-guid", - }, - }, + "Visibility": MatchAllFields(Fields{ + "Type": Equal(korifiv1alpha1.AdminServicePlanVisibilityType), + "Organizations": BeEmpty(), }), + "ServiceOfferingGUID": Equal("offering-guid"), })) }) }) @@ -198,57 +201,307 @@ var _ = Describe("ServicePlanRepo", func() { It("returns matching service plans", func() { Expect(listErr).NotTo(HaveOccurred()) Expect(listedPlans).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ - "Relationships": Equal(repositories.ServicePlanRelationships{ - ServiceOffering: model.ToOneRelationship{ - Data: model.Relationship{ - GUID: "other-offering-guid", - }, + "ServiceOfferingGUID": Equal("other-offering-guid"), + }))) + }) + }) + + When("filtering by names", func() { + BeforeEach(func() { + Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + Labels: map[string]string{ + korifiv1alpha1.RelServiceOfferingLabel: "other-offering-guid", }, - }), + }, + Spec: korifiv1alpha1.CFServicePlanSpec{ + Visibility: korifiv1alpha1.ServicePlanVisibility{ + Type: korifiv1alpha1.AdminServicePlanVisibilityType, + }, + ServicePlan: services.ServicePlan{ + Name: "other-plan", + }, + }, + })).To(Succeed()) + + message.Names = []string{"other-plan"} + }) + + It("returns matching service plans", func() { + Expect(listErr).NotTo(HaveOccurred()) + Expect(listedPlans).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ + "ServiceOfferingGUID": Equal("other-offering-guid"), }))) }) }) }) - Describe("ApplyPlanVisibility", func() { + Describe("PlanVisibility", func() { var ( - plan repositories.ServicePlanRecord - applyErr error + visibilityType string + orgGUIDs []string + cfOrg *korifiv1alpha1.CFOrg + anotherOrg *korifiv1alpha1.CFOrg + plan repositories.ServicePlanRecord + visibilityErr error ) - JustBeforeEach(func() { - plan, applyErr = repo.ApplyPlanVisibility(ctx, authInfo, repositories.ApplyServicePlanVisibilityMessage{ - PlanGUID: planGUID, - Type: korifiv1alpha1.PublicServicePlanVisibilityType, - }) + BeforeEach(func() { + cfOrg = createOrgWithCleanup(ctx, uuid.NewString()) + createRoleBinding(ctx, userName, orgUserRole.Name, cfOrg.Name) + + anotherOrg = createOrgWithCleanup(ctx, uuid.NewString()) + createRoleBinding(ctx, userName, orgUserRole.Name, anotherOrg.Name) + + visibilityType = korifiv1alpha1.OrganizationServicePlanVisibilityType + orgGUIDs = []string{cfOrg.Name} }) - It("returns unauthorized error", func() { - Expect(applyErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) + Describe("ApplyPlanVisibility", func() { + JustBeforeEach(func() { + plan, visibilityErr = repo.ApplyPlanVisibility(ctx, authInfo, repositories.ApplyServicePlanVisibilityMessage{ + PlanGUID: planGUID, + Type: visibilityType, + Organizations: orgGUIDs, + }) + }) + + It("returns unauthorized error", func() { + Expect(visibilityErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) + }) + + When("the user has permissions", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + }) + + It("returns the patched plan visibility", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + Expect(plan.Visibility).To(Equal(repositories.PlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []services.VisibilityOrganization{{ + GUID: cfOrg.Name, + Name: cfOrg.Spec.DisplayName, + }}, + })) + }) + + It("patches the plan visibility in kubernetes", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + + servicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) + + Expect(servicePlan.Spec.Visibility).To(Equal(korifiv1alpha1.ServicePlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []string{cfOrg.Name}, + })) + }) + + When("the plan already has the org visibility type", func() { + BeforeEach(func() { + cfServicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + + Expect(k8s.PatchResource(ctx, k8sClient, cfServicePlan, func() { + cfServicePlan.Spec.Visibility = korifiv1alpha1.ServicePlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []string{anotherOrg.Name}, + } + })).To(Succeed()) + }) + + It("returns plan visibility with merged orgs", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + Expect(plan.Visibility.Organizations).To(ConsistOf( + services.VisibilityOrganization{ + GUID: anotherOrg.Name, + Name: anotherOrg.Spec.DisplayName, + }, + services.VisibilityOrganization{ + GUID: cfOrg.Name, + Name: cfOrg.Spec.DisplayName, + }, + )) + }) + + It("patches the plan visibility in kubernetes by merging org guids", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + + servicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) + + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(anotherOrg.Name, cfOrg.Name)) + }) + + When("the visibility type is set to non-org", func() { + BeforeEach(func() { + visibilityType = korifiv1alpha1.AdminServicePlanVisibilityType + orgGUIDs = []string{} + }) + + It("clears the visibility organizations in kubernetes", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + + servicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) + + Expect(servicePlan.Spec.Visibility.Organizations).To(BeEmpty()) + }) + }) + }) + + When("the visibility org list contains duplicates", func() { + BeforeEach(func() { + orgGUIDs = []string{cfOrg.Name, cfOrg.Name} + }) + + It("deduplicates it", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + + servicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) + + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name)) + }) + }) + }) }) - When("the user has permissions", func() { - BeforeEach(func() { - createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + Describe("UpdatePlanVisibility", func() { + JustBeforeEach(func() { + plan, visibilityErr = repo.UpdatePlanVisibility(ctx, authInfo, repositories.UpdateServicePlanVisibilityMessage{ + PlanGUID: planGUID, + Type: visibilityType, + Organizations: orgGUIDs, + }) }) - It("returns the patched plan visibility", func() { - Expect(applyErr).NotTo(HaveOccurred()) - Expect(plan.VisibilityType).To(Equal(korifiv1alpha1.PublicServicePlanVisibilityType)) + It("returns unauthorized error", func() { + Expect(visibilityErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) }) - It("patches the plan visibility in kubernetes", func() { - Expect(applyErr).NotTo(HaveOccurred()) + When("the user has permissions", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + }) - servicePlan := &korifiv1alpha1.CFServicePlan{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: rootNamespace, - Name: planGUID, - }, - } - Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) + It("returns the patched plan visibility", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + Expect(plan.Visibility).To(Equal(repositories.PlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []services.VisibilityOrganization{{ + GUID: cfOrg.Name, + Name: cfOrg.Spec.DisplayName, + }}, + })) + }) + + It("patches the plan visibility in kubernetes", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + + servicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) + + Expect(servicePlan.Spec.Visibility).To(Equal(korifiv1alpha1.ServicePlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []string{cfOrg.Name}, + })) + }) + + When("the plan already has the org visibility type", func() { + BeforeEach(func() { + anotherOrg := createOrgWithCleanup(ctx, uuid.NewString()) + createRoleBinding(ctx, userName, orgUserRole.Name, anotherOrg.Name) + + cfServicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + + Expect(k8s.PatchResource(ctx, k8sClient, cfServicePlan, func() { + cfServicePlan.Spec.Visibility = korifiv1alpha1.ServicePlanVisibility{ + Type: korifiv1alpha1.OrganizationServicePlanVisibilityType, + Organizations: []string{anotherOrg.Name}, + } + })).To(Succeed()) + }) + + It("returns plan visibility with replaced orgs", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + Expect(plan.Visibility.Organizations).To(ConsistOf(services.VisibilityOrganization{ + GUID: cfOrg.Name, + Name: cfOrg.Spec.DisplayName, + })) + }) + + It("patches the plan visibility in kubernetes by replacing org guids", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + + servicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) + + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name)) + }) + }) + + When("the visibility org list contains duplicates", func() { + BeforeEach(func() { + orgGUIDs = []string{cfOrg.Name, cfOrg.Name} + }) + + It("deduplicates it", func() { + Expect(visibilityErr).NotTo(HaveOccurred()) + + servicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) - Expect(servicePlan.Spec.Visibility.Type).To(Equal(korifiv1alpha1.PublicServicePlanVisibilityType)) + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name)) + }) + }) }) }) }) diff --git a/controllers/api/v1alpha1/cfservice_plan_types.go b/controllers/api/v1alpha1/cfservice_plan_types.go index 94849b9b4..dbd8e56b3 100644 --- a/controllers/api/v1alpha1/cfservice_plan_types.go +++ b/controllers/api/v1alpha1/cfservice_plan_types.go @@ -11,13 +11,16 @@ type CFServicePlanSpec struct { } const ( - AdminServicePlanVisibilityType = "admin" - PublicServicePlanVisibilityType = "public" + AdminServicePlanVisibilityType = "admin" + PublicServicePlanVisibilityType = "public" + OrganizationServicePlanVisibilityType = "organization" ) type ServicePlanVisibility struct { - // +kubebuilder:validation:Enum=admin;public + // +kubebuilder:validation:Enum=admin;public;organization Type string `json:"type"` + // +kubebuilder:validation:Optional + Organizations []string `json:"organizations,omitempty"` } // +kubebuilder:object:root=true diff --git a/controllers/api/v1alpha1/zz_generated.deepcopy.go b/controllers/api/v1alpha1/zz_generated.deepcopy.go index 20a6ea487..e9f1fb879 100644 --- a/controllers/api/v1alpha1/zz_generated.deepcopy.go +++ b/controllers/api/v1alpha1/zz_generated.deepcopy.go @@ -1622,7 +1622,7 @@ func (in *CFServicePlanList) DeepCopyObject() runtime.Object { func (in *CFServicePlanSpec) DeepCopyInto(out *CFServicePlanSpec) { *out = *in in.ServicePlan.DeepCopyInto(&out.ServicePlan) - out.Visibility = in.Visibility + in.Visibility.DeepCopyInto(&out.Visibility) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CFServicePlanSpec. @@ -2103,6 +2103,11 @@ func (in *RunnerInfoStatus) DeepCopy() *RunnerInfoStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServicePlanVisibility) DeepCopyInto(out *ServicePlanVisibility) { *out = *in + if in.Organizations != nil { + in, out := &in.Organizations, &out.Organizations + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServicePlanVisibility. diff --git a/controllers/controllers/services/brokers/controller_test.go b/controllers/controllers/services/brokers/controller_test.go index c1fddd860..a51261c94 100644 --- a/controllers/controllers/services/brokers/controller_test.go +++ b/controllers/controllers/services/brokers/controller_test.go @@ -228,7 +228,8 @@ var _ = Describe("CFServiceBroker", func() { }), }), "Visibility": MatchAllFields(Fields{ - "Type": Equal(korifiv1alpha1.AdminServicePlanVisibilityType), + "Type": Equal(korifiv1alpha1.AdminServicePlanVisibilityType), + "Organizations": BeEmpty(), }), })) }).Should(Succeed()) diff --git a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceplans.yaml b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceplans.yaml index 19f09ff12..716e04f47 100644 --- a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceplans.yaml +++ b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceplans.yaml @@ -113,10 +113,15 @@ spec: type: object visibility: properties: + organizations: + items: + type: string + type: array type: enum: - admin - public + - organization type: string required: - type diff --git a/model/services/plans.go b/model/services/plans.go index 334d47859..e7321d1cb 100644 --- a/model/services/plans.go +++ b/model/services/plans.go @@ -49,3 +49,8 @@ type ServicePlanFeatures struct { PlanUpdateable bool `json:"plan_updateable"` Bindable bool `json:"bindable"` } + +type VisibilityOrganization struct { + GUID string `json:"guid"` + Name string `json:"name"` +} diff --git a/tests/e2e/service_plans_test.go b/tests/e2e/service_plans_test.go index 02692029b..1c91d5e04 100644 --- a/tests/e2e/service_plans_test.go +++ b/tests/e2e/service_plans_test.go @@ -46,7 +46,10 @@ var _ = Describe("Service Plans", func() { }) Describe("Visibility", func() { - var planGUID string + var ( + planGUID string + result planVisibilityResource + ) BeforeEach(func() { plans := resourceList[resource]{} @@ -63,8 +66,6 @@ var _ = Describe("Service Plans", func() { }) Describe("Get Visibility", func() { - var result planVisibilityResource - JustBeforeEach(func() { var err error resp, err = adminClient.R().SetResult(&result).Get(fmt.Sprintf("/v3/service_plans/%s/visibility", planGUID)) @@ -80,8 +81,6 @@ var _ = Describe("Service Plans", func() { }) Describe("Apply Visibility", func() { - var result planVisibilityResource - JustBeforeEach(func() { var err error resp, err = adminClient.R(). @@ -93,6 +92,28 @@ var _ = Describe("Service Plans", func() { Expect(err).NotTo(HaveOccurred()) }) + It("applies the plan visibility", func() { + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusOK), + HaveRestyBody(MatchJSON(`{ + "type": "public" + }`)), + )) + }) + }) + + Describe("Update Visibility", func() { + JustBeforeEach(func() { + var err error + resp, err = adminClient.R(). + SetResult(&result). + SetBody(planVisibilityResource{ + Type: "public", + }). + Patch(fmt.Sprintf("/v3/service_plans/%s/visibility", planGUID)) + Expect(err).NotTo(HaveOccurred()) + }) + It("updates the plan visibility", func() { Expect(resp).To(SatisfyAll( HaveRestyStatusCode(http.StatusOK),