From 5459664324a23f21de3d61fe03ae545761dea4b4 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Mon, 5 Aug 2024 12:42:30 +0000 Subject: [PATCH] Implement POST /v3/service_plans/{guid}/visibility - For now `public` is the only allowd value other than `admin`, which is the default - Bonus: Introduce the `visibility_type` field in the service plan resource. This makes `cf service-access` display the plan visibility correctly fixes #3275 Co-authored-by: Georgi Sabev --- .../fake/cfservice_plan_repository.go | 171 +++++++++++---- api/handlers/service_plan.go | 26 ++- api/handlers/service_plan_test.go | 73 ++++++- api/payloads/service_plan.go | 11 + api/payloads/service_plan_test.go | 46 ++-- api/presenter/service_plan.go | 8 +- api/presenter/service_plan_test.go | 11 +- api/repositories/service_plan_repository.go | 57 +++-- .../service_plan_repository_test.go | 203 ++++++++++-------- .../api/v1alpha1/cfservice_plan_types.go | 7 +- .../services/brokers/controller.go | 7 +- .../services/brokers/controller_test.go | 51 +++++ .../korifi/controllers/cf_roles/cf_admin.yaml | 1 + ...orifi.cloudfoundry.org_cfserviceplans.yaml | 1 + tests/e2e/e2e_suite_test.go | 17 +- tests/e2e/service_brokers_test.go | 17 +- tests/e2e/service_offerings_test.go | 15 +- tests/e2e/service_plans_test.go | 71 ++++-- 18 files changed, 585 insertions(+), 208 deletions(-) diff --git a/api/handlers/fake/cfservice_plan_repository.go b/api/handlers/fake/cfservice_plan_repository.go index e1faa8a20..27f1a0a30 100644 --- a/api/handlers/fake/cfservice_plan_repository.go +++ b/api/handlers/fake/cfservice_plan_repository.go @@ -11,19 +11,34 @@ import ( ) type CFServicePlanRepository struct { - GetPlanVisibilityStub func(context.Context, authorization.Info, string) (repositories.ServicePlanVisibilityRecord, error) - getPlanVisibilityMutex sync.RWMutex - getPlanVisibilityArgsForCall []struct { + ApplyPlanVisibilityStub func(context.Context, authorization.Info, repositories.ApplyServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error) + applyPlanVisibilityMutex sync.RWMutex + applyPlanVisibilityArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.ApplyServicePlanVisibilityMessage + } + applyPlanVisibilityReturns struct { + result1 repositories.ServicePlanRecord + result2 error + } + applyPlanVisibilityReturnsOnCall map[int]struct { + result1 repositories.ServicePlanRecord + result2 error + } + GetPlanStub func(context.Context, authorization.Info, string) (repositories.ServicePlanRecord, error) + getPlanMutex sync.RWMutex + getPlanArgsForCall []struct { arg1 context.Context arg2 authorization.Info arg3 string } - getPlanVisibilityReturns struct { - result1 repositories.ServicePlanVisibilityRecord + getPlanReturns struct { + result1 repositories.ServicePlanRecord result2 error } - getPlanVisibilityReturnsOnCall map[int]struct { - result1 repositories.ServicePlanVisibilityRecord + getPlanReturnsOnCall map[int]struct { + result1 repositories.ServicePlanRecord result2 error } ListPlansStub func(context.Context, authorization.Info, repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error) @@ -45,18 +60,84 @@ type CFServicePlanRepository struct { invocationsMutex sync.RWMutex } -func (fake *CFServicePlanRepository) GetPlanVisibility(arg1 context.Context, arg2 authorization.Info, arg3 string) (repositories.ServicePlanVisibilityRecord, error) { - fake.getPlanVisibilityMutex.Lock() - ret, specificReturn := fake.getPlanVisibilityReturnsOnCall[len(fake.getPlanVisibilityArgsForCall)] - fake.getPlanVisibilityArgsForCall = append(fake.getPlanVisibilityArgsForCall, struct { +func (fake *CFServicePlanRepository) ApplyPlanVisibility(arg1 context.Context, arg2 authorization.Info, arg3 repositories.ApplyServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error) { + fake.applyPlanVisibilityMutex.Lock() + ret, specificReturn := fake.applyPlanVisibilityReturnsOnCall[len(fake.applyPlanVisibilityArgsForCall)] + fake.applyPlanVisibilityArgsForCall = append(fake.applyPlanVisibilityArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.ApplyServicePlanVisibilityMessage + }{arg1, arg2, arg3}) + stub := fake.ApplyPlanVisibilityStub + fakeReturns := fake.applyPlanVisibilityReturns + fake.recordInvocation("ApplyPlanVisibility", []interface{}{arg1, arg2, arg3}) + fake.applyPlanVisibilityMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *CFServicePlanRepository) ApplyPlanVisibilityCallCount() int { + fake.applyPlanVisibilityMutex.RLock() + defer fake.applyPlanVisibilityMutex.RUnlock() + return len(fake.applyPlanVisibilityArgsForCall) +} + +func (fake *CFServicePlanRepository) ApplyPlanVisibilityCalls(stub func(context.Context, authorization.Info, repositories.ApplyServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error)) { + fake.applyPlanVisibilityMutex.Lock() + defer fake.applyPlanVisibilityMutex.Unlock() + fake.ApplyPlanVisibilityStub = stub +} + +func (fake *CFServicePlanRepository) ApplyPlanVisibilityArgsForCall(i int) (context.Context, authorization.Info, repositories.ApplyServicePlanVisibilityMessage) { + fake.applyPlanVisibilityMutex.RLock() + defer fake.applyPlanVisibilityMutex.RUnlock() + argsForCall := fake.applyPlanVisibilityArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServicePlanRepository) ApplyPlanVisibilityReturns(result1 repositories.ServicePlanRecord, result2 error) { + fake.applyPlanVisibilityMutex.Lock() + defer fake.applyPlanVisibilityMutex.Unlock() + fake.ApplyPlanVisibilityStub = nil + fake.applyPlanVisibilityReturns = struct { + result1 repositories.ServicePlanRecord + result2 error + }{result1, result2} +} + +func (fake *CFServicePlanRepository) ApplyPlanVisibilityReturnsOnCall(i int, result1 repositories.ServicePlanRecord, result2 error) { + fake.applyPlanVisibilityMutex.Lock() + defer fake.applyPlanVisibilityMutex.Unlock() + fake.ApplyPlanVisibilityStub = nil + if fake.applyPlanVisibilityReturnsOnCall == nil { + fake.applyPlanVisibilityReturnsOnCall = make(map[int]struct { + result1 repositories.ServicePlanRecord + result2 error + }) + } + fake.applyPlanVisibilityReturnsOnCall[i] = struct { + result1 repositories.ServicePlanRecord + result2 error + }{result1, result2} +} + +func (fake *CFServicePlanRepository) GetPlan(arg1 context.Context, arg2 authorization.Info, arg3 string) (repositories.ServicePlanRecord, error) { + fake.getPlanMutex.Lock() + ret, specificReturn := fake.getPlanReturnsOnCall[len(fake.getPlanArgsForCall)] + fake.getPlanArgsForCall = append(fake.getPlanArgsForCall, struct { arg1 context.Context arg2 authorization.Info arg3 string }{arg1, arg2, arg3}) - stub := fake.GetPlanVisibilityStub - fakeReturns := fake.getPlanVisibilityReturns - fake.recordInvocation("GetPlanVisibility", []interface{}{arg1, arg2, arg3}) - fake.getPlanVisibilityMutex.Unlock() + stub := fake.GetPlanStub + fakeReturns := fake.getPlanReturns + fake.recordInvocation("GetPlan", []interface{}{arg1, arg2, arg3}) + fake.getPlanMutex.Unlock() if stub != nil { return stub(arg1, arg2, arg3) } @@ -66,47 +147,47 @@ func (fake *CFServicePlanRepository) GetPlanVisibility(arg1 context.Context, arg return fakeReturns.result1, fakeReturns.result2 } -func (fake *CFServicePlanRepository) GetPlanVisibilityCallCount() int { - fake.getPlanVisibilityMutex.RLock() - defer fake.getPlanVisibilityMutex.RUnlock() - return len(fake.getPlanVisibilityArgsForCall) +func (fake *CFServicePlanRepository) GetPlanCallCount() int { + fake.getPlanMutex.RLock() + defer fake.getPlanMutex.RUnlock() + return len(fake.getPlanArgsForCall) } -func (fake *CFServicePlanRepository) GetPlanVisibilityCalls(stub func(context.Context, authorization.Info, string) (repositories.ServicePlanVisibilityRecord, error)) { - fake.getPlanVisibilityMutex.Lock() - defer fake.getPlanVisibilityMutex.Unlock() - fake.GetPlanVisibilityStub = stub +func (fake *CFServicePlanRepository) GetPlanCalls(stub func(context.Context, authorization.Info, string) (repositories.ServicePlanRecord, error)) { + fake.getPlanMutex.Lock() + defer fake.getPlanMutex.Unlock() + fake.GetPlanStub = stub } -func (fake *CFServicePlanRepository) GetPlanVisibilityArgsForCall(i int) (context.Context, authorization.Info, string) { - fake.getPlanVisibilityMutex.RLock() - defer fake.getPlanVisibilityMutex.RUnlock() - argsForCall := fake.getPlanVisibilityArgsForCall[i] +func (fake *CFServicePlanRepository) GetPlanArgsForCall(i int) (context.Context, authorization.Info, string) { + fake.getPlanMutex.RLock() + defer fake.getPlanMutex.RUnlock() + argsForCall := fake.getPlanArgsForCall[i] return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *CFServicePlanRepository) GetPlanVisibilityReturns(result1 repositories.ServicePlanVisibilityRecord, result2 error) { - fake.getPlanVisibilityMutex.Lock() - defer fake.getPlanVisibilityMutex.Unlock() - fake.GetPlanVisibilityStub = nil - fake.getPlanVisibilityReturns = struct { - result1 repositories.ServicePlanVisibilityRecord +func (fake *CFServicePlanRepository) GetPlanReturns(result1 repositories.ServicePlanRecord, result2 error) { + fake.getPlanMutex.Lock() + defer fake.getPlanMutex.Unlock() + fake.GetPlanStub = nil + fake.getPlanReturns = struct { + result1 repositories.ServicePlanRecord result2 error }{result1, result2} } -func (fake *CFServicePlanRepository) GetPlanVisibilityReturnsOnCall(i int, result1 repositories.ServicePlanVisibilityRecord, result2 error) { - fake.getPlanVisibilityMutex.Lock() - defer fake.getPlanVisibilityMutex.Unlock() - fake.GetPlanVisibilityStub = nil - if fake.getPlanVisibilityReturnsOnCall == nil { - fake.getPlanVisibilityReturnsOnCall = make(map[int]struct { - result1 repositories.ServicePlanVisibilityRecord +func (fake *CFServicePlanRepository) GetPlanReturnsOnCall(i int, result1 repositories.ServicePlanRecord, result2 error) { + fake.getPlanMutex.Lock() + defer fake.getPlanMutex.Unlock() + fake.GetPlanStub = nil + if fake.getPlanReturnsOnCall == nil { + fake.getPlanReturnsOnCall = make(map[int]struct { + result1 repositories.ServicePlanRecord result2 error }) } - fake.getPlanVisibilityReturnsOnCall[i] = struct { - result1 repositories.ServicePlanVisibilityRecord + fake.getPlanReturnsOnCall[i] = struct { + result1 repositories.ServicePlanRecord result2 error }{result1, result2} } @@ -180,8 +261,10 @@ func (fake *CFServicePlanRepository) ListPlansReturnsOnCall(i int, result1 []rep func (fake *CFServicePlanRepository) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() - fake.getPlanVisibilityMutex.RLock() - defer fake.getPlanVisibilityMutex.RUnlock() + fake.applyPlanVisibilityMutex.RLock() + defer fake.applyPlanVisibilityMutex.RUnlock() + fake.getPlanMutex.RLock() + defer fake.getPlanMutex.RUnlock() fake.listPlansMutex.RLock() defer fake.listPlansMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} diff --git a/api/handlers/service_plan.go b/api/handlers/service_plan.go index a3542d39f..905fb49c7 100644 --- a/api/handlers/service_plan.go +++ b/api/handlers/service_plan.go @@ -22,8 +22,9 @@ const ( //counterfeiter:generate -o fake -fake-name CFServicePlanRepository . CFServicePlanRepository type CFServicePlanRepository interface { + GetPlan(context.Context, authorization.Info, string) (repositories.ServicePlanRecord, error) ListPlans(context.Context, authorization.Info, repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error) - GetPlanVisibility(context.Context, authorization.Info, string) (repositories.ServicePlanVisibilityRecord, error) + ApplyPlanVisibility(context.Context, authorization.Info, repositories.ApplyServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error) } type ServicePlan struct { @@ -68,11 +69,31 @@ func (h *ServicePlan) getPlanVisibility(r *http.Request) (*routing.Response, err planGUID := routing.URLParam(r, "guid") logger = logger.WithValues("guid", planGUID) - visibility, err := h.servicePlanRepo.GetPlanVisibility(r.Context(), authInfo, planGUID) + plan, err := h.servicePlanRepo.GetPlan(r.Context(), authInfo, planGUID) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to get plan visibility") } + return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServicePlanVisibility(plan, h.serverURL)), nil +} + +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") + + 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.ApplyPlanVisibility(r.Context(), authInfo, payload.ToMessage(planGUID)) + if err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to apply plan visibility") + } + return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServicePlanVisibility(visibility, h.serverURL)), nil } @@ -84,5 +105,6 @@ func (h *ServicePlan) AuthenticatedRoutes() []routing.Route { return []routing.Route{ {Method: "GET", Pattern: ServicePlansPath, Handler: h.list}, {Method: "GET", Pattern: ServicePlanVisivilityPath, Handler: h.getPlanVisibility}, + {Method: "POST", Pattern: ServicePlanVisivilityPath, Handler: h.applyPlanVisibility}, } } diff --git a/api/handlers/service_plan_test.go b/api/handlers/service_plan_test.go index da8bf4ac1..574d396e0 100644 --- a/api/handlers/service_plan_test.go +++ b/api/handlers/service_plan_test.go @@ -3,6 +3,7 @@ package handlers_test import ( "errors" "net/http" + "strings" . "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/handlers/fake" @@ -110,8 +111,8 @@ var _ = Describe("ServicePlan", func() { Describe("GET /v3/service_plans/{guid}/visibility", func() { BeforeEach(func() { - servicePlanRepo.GetPlanVisibilityReturns(repositories.ServicePlanVisibilityRecord{ - Type: korifiv1alpha1.AdminServicePlanVisibilityType, + servicePlanRepo.GetPlanReturns(repositories.ServicePlanRecord{ + VisibilityType: korifiv1alpha1.AdminServicePlanVisibilityType, }, nil) }) @@ -123,8 +124,8 @@ var _ = Describe("ServicePlan", func() { }) It("returns the plan visibility", func() { - Expect(servicePlanRepo.GetPlanVisibilityCallCount()).To(Equal(1)) - _, actualAuthInfo, actualPlanID := servicePlanRepo.GetPlanVisibilityArgsForCall(0) + Expect(servicePlanRepo.GetPlanCallCount()).To(Equal(1)) + _, actualAuthInfo, actualPlanID := servicePlanRepo.GetPlanArgsForCall(0) Expect(actualPlanID).To(Equal("my-service-plan")) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -138,7 +139,69 @@ var _ = Describe("ServicePlan", func() { When("getting the visibility fails", func() { BeforeEach(func() { - servicePlanRepo.GetPlanVisibilityReturns(repositories.ServicePlanVisibilityRecord{}, errors.New("visibility-err")) + servicePlanRepo.GetPlanReturns(repositories.ServicePlanRecord{}, errors.New("visibility-err")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + }) + + Describe("POST /v3/service_plans/{guid}/visibility", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + }) + + servicePlanRepo.ApplyPlanVisibilityReturns(repositories.ServicePlanRecord{ + VisibilityType: korifiv1alpha1.PublicServicePlanVisibilityType, + }, nil) + }) + + JustBeforeEach(func() { + req, err := http.NewRequestWithContext(ctx, "POST", "/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.ApplyPlanVisibilityCallCount()).To(Equal(1)) + _, actualAuthInfo, actualMessage := servicePlanRepo.ApplyPlanVisibilityArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualMessage).To(Equal(repositories.ApplyServicePlanVisibilityMessage{ + PlanGUID: "my-service-plan", + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + })) + + 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.ApplyPlanVisibilityReturns(repositories.ServicePlanRecord{}, errors.New("visibility-err")) }) It("returns an error", func() { diff --git a/api/payloads/service_plan.go b/api/payloads/service_plan.go index e19039fbe..46d5be01e 100644 --- a/api/payloads/service_plan.go +++ b/api/payloads/service_plan.go @@ -30,3 +30,14 @@ func (l *ServicePlanList) DecodeFromURLValues(values url.Values) error { l.ServiceOfferingGUIDs = values.Get("service_offering_guids") return nil } + +type ServicePlanVisibility struct { + Type string `json:"type"` +} + +func (p *ServicePlanVisibility) ToMessage(planGUID string) repositories.ApplyServicePlanVisibilityMessage { + return repositories.ApplyServicePlanVisibilityMessage{ + PlanGUID: planGUID, + Type: p.Type, + } +} diff --git a/api/payloads/service_plan_test.go b/api/payloads/service_plan_test.go index d2d50a44f..87e99ab12 100644 --- a/api/payloads/service_plan_test.go +++ b/api/payloads/service_plan_test.go @@ -3,28 +3,46 @@ package payloads_test import ( "code.cloudfoundry.org/korifi/api/payloads" "code.cloudfoundry.org/korifi/api/repositories" + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) var _ = Describe("ServicePlan", func() { - DescribeTable("valid query", - func(query string, expectedServicePlanList payloads.ServicePlanList) { - actualServicePlanList, decodeErr := decodeQuery[payloads.ServicePlanList](query) + Describe("List", func() { + DescribeTable("valid query", + func(query string, expectedServicePlanList payloads.ServicePlanList) { + actualServicePlanList, decodeErr := decodeQuery[payloads.ServicePlanList](query) - Expect(decodeErr).NotTo(HaveOccurred()) - Expect(*actualServicePlanList).To(Equal(expectedServicePlanList)) - }, - Entry("service_offering_guids", "service_offering_guids=b1,b2", payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2"}), - ) + Expect(decodeErr).NotTo(HaveOccurred()) + Expect(*actualServicePlanList).To(Equal(expectedServicePlanList)) + }, + Entry("service_offering_guids", "service_offering_guids=b1,b2", payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2"}), + ) - Describe("ToMessage", func() { - It("converts payload to repository message", func() { - payload := &payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2"} + Describe("ToMessage", func() { + It("converts payload to repository message", func() { + payload := &payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2"} - Expect(payload.ToMessage()).To(Equal(repositories.ListServicePlanMessage{ - ServiceOfferingGUIDs: []string{"b1", "b2"}, - })) + Expect(payload.ToMessage()).To(Equal(repositories.ListServicePlanMessage{ + ServiceOfferingGUIDs: []string{"b1", "b2"}, + })) + }) + }) + }) + + Describe("Visibility", func() { + Describe("ToMessage", func() { + It("converts payload to repository message", func() { + payload := &payloads.ServicePlanVisibility{ + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + } + + Expect(payload.ToMessage("plan-guid")).To(Equal(repositories.ApplyServicePlanVisibilityMessage{ + PlanGUID: "plan-guid", + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + })) + }) }) }) }) diff --git a/api/presenter/service_plan.go b/api/presenter/service_plan.go index e154e577f..b1c295ff3 100644 --- a/api/presenter/service_plan.go +++ b/api/presenter/service_plan.go @@ -10,6 +10,7 @@ import ( type ServicePlanLinks struct { Self Link `json:"self"` ServiceOffering Link `json:"service_offering"` + Visibility Link `json:"visibility"` } type ServicePlanResponse struct { @@ -27,14 +28,17 @@ func ForServicePlan(servicePlan repositories.ServicePlanRecord, baseURL url.URL) ServiceOffering: Link{ HRef: buildURL(baseURL).appendPath(serviceOfferingsBase, servicePlan.Relationships.ServiceOffering.Data.GUID).build(), }, + Visibility: Link{ + HRef: buildURL(baseURL).appendPath(servicePlansBase, servicePlan.GUID, "visibility").build(), + }, }, } } type ServicePlanVisibilityResponse korifiv1alpha1.ServicePlanVisibility -func ForServicePlanVisibility(visibility repositories.ServicePlanVisibilityRecord, _ url.URL) ServicePlanVisibilityResponse { +func ForServicePlanVisibility(plan repositories.ServicePlanRecord, _ url.URL) ServicePlanVisibilityResponse { return ServicePlanVisibilityResponse{ - Type: visibility.Type, + Type: plan.VisibilityType, } } diff --git a/api/presenter/service_plan_test.go b/api/presenter/service_plan_test.go index ddc2c6a63..f81ac76da 100644 --- a/api/presenter/service_plan_test.go +++ b/api/presenter/service_plan_test.go @@ -81,6 +81,7 @@ var _ = Describe("Service Plan", func() { }, }, }, + VisibilityType: "visibility-type", Relationships: repositories.ServicePlanRelationships{ ServiceOffering: model.ToOneRelationship{ Data: model.Relationship{ @@ -135,6 +136,7 @@ var _ = Describe("Service Plan", func() { } }, "guid": "resource-guid", + "visibility_type": "visibility-type", "created_at": "1970-01-01T00:00:01Z", "updated_at": "1970-01-01T00:00:02Z", "metadata": { @@ -158,6 +160,9 @@ var _ = Describe("Service Plan", func() { }, "service_offering": { "href": "https://api.example.org/v3/service_offerings/service-offering-guid" + }, + "visibility": { + "href": "https://api.example.org/v3/service_plans/resource-guid/visibility" } } }`)) @@ -165,11 +170,11 @@ var _ = Describe("Service Plan", func() { }) Describe("ForServicePlanVisibility", func() { - var record repositories.ServicePlanVisibilityRecord + var record repositories.ServicePlanRecord BeforeEach(func() { - record = repositories.ServicePlanVisibilityRecord{ - Type: "admin", + record = repositories.ServicePlanRecord{ + VisibilityType: "admin", } }) diff --git a/api/repositories/service_plan_repository.go b/api/repositories/service_plan_repository.go index 529779513..11fd4e614 100644 --- a/api/repositories/service_plan_repository.go +++ b/api/repositories/service_plan_repository.go @@ -22,11 +22,10 @@ const ( type ServicePlanRecord struct { services.ServicePlan model.CFResource - Relationships ServicePlanRelationships `json:"relationships"` + VisibilityType string `json:"visibility_type"` + Relationships ServicePlanRelationships `json:"relationships"` } -type ServicePlanVisibilityRecord korifiv1alpha1.ServicePlanVisibility - type ServicePlanRelationships struct { ServiceOffering model.ToOneRelationship `json:"service_offering"` } @@ -36,6 +35,19 @@ type ServicePlanRepo struct { rootNamespace string } +type ListServicePlanMessage struct { + ServiceOfferingGUIDs []string +} + +func (m *ListServicePlanMessage) matches(cfServicePlan korifiv1alpha1.CFServicePlan) bool { + return emptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel]) +} + +type ApplyServicePlanVisibilityMessage struct { + PlanGUID string + Type string +} + func NewServicePlanRepo( userClientFactory authorization.UserK8sClientFactory, rootNamespace string, @@ -46,14 +58,6 @@ func NewServicePlanRepo( } } -type ListServicePlanMessage struct { - ServiceOfferingGUIDs []string -} - -func (m *ListServicePlanMessage) matches(cfServicePlan korifiv1alpha1.CFServicePlan) bool { - return emptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel]) -} - func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization.Info, message ListServicePlanMessage) ([]ServicePlanRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { @@ -68,10 +72,10 @@ func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization. return iter.Map(iter.Lift(cfServicePlans.Items).Filter(message.matches), planToRecord).Collect(), nil } -func (r *ServicePlanRepo) GetPlanVisibility(ctx context.Context, authInfo authorization.Info, planGUID string) (ServicePlanVisibilityRecord, error) { +func (r *ServicePlanRepo) GetPlan(ctx context.Context, authInfo authorization.Info, planGUID string) (ServicePlanRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { - return ServicePlanVisibilityRecord{}, fmt.Errorf("failed to build user client: %w", err) + return ServicePlanRecord{}, fmt.Errorf("failed to build user client: %w", err) } cfServicePlan := &korifiv1alpha1.CFServicePlan{ @@ -83,9 +87,31 @@ func (r *ServicePlanRepo) GetPlanVisibility(ctx context.Context, authInfo author err = userClient.Get(ctx, client.ObjectKeyFromObject(cfServicePlan), cfServicePlan) if err != nil { - return ServicePlanVisibilityRecord{}, apierrors.FromK8sError(err, ServicePlanVisibilityResourceType) + return ServicePlanRecord{}, apierrors.FromK8sError(err, ServicePlanVisibilityResourceType) + } + return planToRecord(*cfServicePlan), nil +} + +func (r *ServicePlanRepo) ApplyPlanVisibility(ctx context.Context, authInfo authorization.Info, message ApplyServicePlanVisibilityMessage) (ServicePlanRecord, error) { + userClient, err := r.userClientFactory.BuildClient(authInfo) + if err != nil { + return ServicePlanRecord{}, fmt.Errorf("failed to build user client: %w", err) + } + + cfServicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.rootNamespace, + Name: message.PlanGUID, + }, + } + + if err := PatchResource(ctx, userClient, cfServicePlan, func() { + cfServicePlan.Spec.Visibility.Type = message.Type + }); err != nil { + return ServicePlanRecord{}, apierrors.FromK8sError(err, ServicePlanVisibilityResourceType) } - return ServicePlanVisibilityRecord(cfServicePlan.Spec.Visibility), nil + + return planToRecord(*cfServicePlan), nil } func planToRecord(plan korifiv1alpha1.CFServicePlan) ServicePlanRecord { @@ -99,6 +125,7 @@ func planToRecord(plan korifiv1alpha1.CFServicePlan) ServicePlanRecord { Annotations: plan.Annotations, }, }, + VisibilityType: plan.Spec.Visibility.Type, Relationships: ServicePlanRelationships{ ServiceOffering: model.ToOneRelationship{ Data: model.Relationship{ diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index 497d78f87..a05abf508 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -1,13 +1,16 @@ package repositories_test import ( + apierrors "code.cloudfoundry.org/korifi/api/errors" "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/tests/matchers" . "github.com/onsi/gomega/gstruct" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" @@ -15,86 +18,81 @@ import ( ) var _ = Describe("ServicePlanRepo", func() { - var repo *repositories.ServicePlanRepo + var ( + repo *repositories.ServicePlanRepo + planGUID string + ) BeforeEach(func() { repo = repositories.NewServicePlanRepo(userClientFactory, rootNamespace) - }) - - Describe("List", func() { - var ( - planGUID string - listedPlans []repositories.ServicePlanRecord - message repositories.ListServicePlanMessage - listErr error - ) - BeforeEach(func() { - planGUID = uuid.NewString() - Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServicePlan{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: rootNamespace, - Name: planGUID, - Labels: map[string]string{ - korifiv1alpha1.RelServiceOfferingLabel: "offering-guid", - }, - Annotations: map[string]string{ - "annotation": "annotation-value", - }, + planGUID = uuid.NewString() + Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + Labels: map[string]string{ + korifiv1alpha1.RelServiceOfferingLabel: "offering-guid", }, - Spec: korifiv1alpha1.CFServicePlanSpec{ - ServicePlan: services.ServicePlan{ - Name: "my-service-plan", - Free: true, - Description: "service plan description", - BrokerCatalog: services.ServicePlanBrokerCatalog{ - ID: "broker-plan-guid", - Metadata: &runtime.RawExtension{ - Raw: []byte(`{"foo":"bar"}`), - }, - Features: services.ServicePlanFeatures{ - PlanUpdateable: true, - Bindable: true, - }, + Annotations: map[string]string{ + "annotation": "annotation-value", + }, + }, + Spec: korifiv1alpha1.CFServicePlanSpec{ + ServicePlan: services.ServicePlan{ + Name: "my-service-plan", + Free: true, + Description: "service plan description", + BrokerCatalog: services.ServicePlanBrokerCatalog{ + ID: "broker-plan-guid", + Metadata: &runtime.RawExtension{ + Raw: []byte(`{"foo":"bar"}`), + }, + Features: services.ServicePlanFeatures{ + PlanUpdateable: true, + Bindable: true, }, - Schemas: services.ServicePlanSchemas{ - ServiceInstance: services.ServiceInstanceSchema{ - Create: services.InputParameterSchema{ - Parameters: &runtime.RawExtension{ - Raw: []byte(`{"create-param":"create-value"}`), - }, + }, + Schemas: services.ServicePlanSchemas{ + ServiceInstance: services.ServiceInstanceSchema{ + Create: services.InputParameterSchema{ + Parameters: &runtime.RawExtension{ + Raw: []byte(`{"create-param":"create-value"}`), }, - Update: services.InputParameterSchema{ - Parameters: &runtime.RawExtension{ - Raw: []byte(`{"update-param":"update-value"}`), - }, + }, + Update: services.InputParameterSchema{ + Parameters: &runtime.RawExtension{ + Raw: []byte(`{"update-param":"update-value"}`), }, }, - ServiceBinding: services.ServiceBindingSchema{ - Create: services.InputParameterSchema{ - Parameters: &runtime.RawExtension{ - Raw: []byte(`{"binding-create-param":"binding-create-value"}`), - }, + }, + ServiceBinding: services.ServiceBindingSchema{ + Create: services.InputParameterSchema{ + Parameters: &runtime.RawExtension{ + Raw: []byte(`{"binding-create-param":"binding-create-value"}`), }, }, }, }, - Visibility: korifiv1alpha1.ServicePlanVisibility{ - Type: korifiv1alpha1.AdminServicePlanVisibilityType, - }, }, - })).To(Succeed()) + Visibility: korifiv1alpha1.ServicePlanVisibility{ + Type: korifiv1alpha1.AdminServicePlanVisibilityType, + }, + }, + })).To(Succeed()) + }) - message = repositories.ListServicePlanMessage{} - }) + Describe("Get", func() { + var plan repositories.ServicePlanRecord JustBeforeEach(func() { - listedPlans, listErr = repo.ListPlans(ctx, authInfo, message) + var err error + plan, err = repo.GetPlan(ctx, authInfo, planGUID) + Expect(err).NotTo(HaveOccurred()) }) - It("lists service offerings", func() { - Expect(listErr).NotTo(HaveOccurred()) - Expect(listedPlans).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ + It("returns the plan", func() { + Expect(plan).To(MatchFields(IgnoreExtras, Fields{ "ServicePlan": MatchFields(IgnoreExtras, Fields{ "Name": Equal("my-service-plan"), "Description": Equal("service plan description"), @@ -141,6 +139,7 @@ var _ = Describe("ServicePlanRepo", func() { "Annotations": HaveKeyWithValue("annotation", "annotation-value"), }), }), + "VisibilityType": Equal(korifiv1alpha1.AdminServicePlanVisibilityType), "Relationships": Equal(repositories.ServicePlanRelationships{ ServiceOffering: model.ToOneRelationship{ Data: model.Relationship{ @@ -148,6 +147,31 @@ var _ = Describe("ServicePlanRepo", func() { }, }, }), + })) + }) + }) + + Describe("List", func() { + var ( + listedPlans []repositories.ServicePlanRecord + message repositories.ListServicePlanMessage + listErr error + ) + + BeforeEach(func() { + message = repositories.ListServicePlanMessage{} + }) + + JustBeforeEach(func() { + listedPlans, listErr = repo.ListPlans(ctx, authInfo, message) + }) + + It("lists service offerings", func() { + Expect(listErr).NotTo(HaveOccurred()) + Expect(listedPlans).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ + "CFResource": MatchFields(IgnoreExtras, Fields{ + "GUID": Equal(planGUID), + }), }))) }) @@ -186,37 +210,46 @@ var _ = Describe("ServicePlanRepo", func() { }) }) - Describe("GetPlanVisibility", func() { + Describe("ApplyPlanVisibility", func() { var ( - planGUID string - visibility repositories.ServicePlanVisibilityRecord + plan repositories.ServicePlanRecord + applyErr error ) - BeforeEach(func() { - planGUID = uuid.NewString() - Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServicePlan{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: rootNamespace, - Name: planGUID, - }, - Spec: korifiv1alpha1.CFServicePlanSpec{ - Visibility: korifiv1alpha1.ServicePlanVisibility{ - Type: korifiv1alpha1.AdminServicePlanVisibilityType, - }, - }, - })).To(Succeed()) + JustBeforeEach(func() { + plan, applyErr = repo.ApplyPlanVisibility(ctx, authInfo, repositories.ApplyServicePlanVisibilityMessage{ + PlanGUID: planGUID, + Type: korifiv1alpha1.PublicServicePlanVisibilityType, + }) }) - JustBeforeEach(func() { - var err error - visibility, err = repo.GetPlanVisibility(ctx, authInfo, planGUID) - Expect(err).NotTo(HaveOccurred()) + It("returns unauthorized error", func() { + Expect(applyErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) }) - It("returns the plan visibility", func() { - Expect(visibility).To(Equal(repositories.ServicePlanVisibilityRecord{ - Type: korifiv1alpha1.AdminServicePlanVisibilityType, - })) + When("the user has permissions", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + }) + + It("returns the patched plan visibility", func() { + Expect(applyErr).NotTo(HaveOccurred()) + Expect(plan.VisibilityType).To(Equal(korifiv1alpha1.PublicServicePlanVisibilityType)) + }) + + It("patches the plan visibility in kubernetes", func() { + Expect(applyErr).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)) + }) }) }) }) diff --git a/controllers/api/v1alpha1/cfservice_plan_types.go b/controllers/api/v1alpha1/cfservice_plan_types.go index eb64771ee..94849b9b4 100644 --- a/controllers/api/v1alpha1/cfservice_plan_types.go +++ b/controllers/api/v1alpha1/cfservice_plan_types.go @@ -10,10 +10,13 @@ type CFServicePlanSpec struct { Visibility ServicePlanVisibility `json:"visibility"` } -const AdminServicePlanVisibilityType = "admin" +const ( + AdminServicePlanVisibilityType = "admin" + PublicServicePlanVisibilityType = "public" +) type ServicePlanVisibility struct { - // +kubebuilder:validation:Enum=admin + // +kubebuilder:validation:Enum=admin;public Type string `json:"type"` } diff --git a/controllers/controllers/services/brokers/controller.go b/controllers/controllers/services/brokers/controller.go index 1e86b7ab0..e801e89f2 100644 --- a/controllers/controllers/services/brokers/controller.go +++ b/controllers/controllers/services/brokers/controller.go @@ -233,6 +233,11 @@ func (r *Reconciler) reconcileCatalogPlan(ctx context.Context, serviceOffering * return fmt.Errorf("failed to marshal service plan %q metadata: %w", catalogPlan.ID, err) } + visibilityType := korifiv1alpha1.AdminServicePlanVisibilityType + if servicePlan.Spec.Visibility.Type != "" { + visibilityType = servicePlan.Spec.Visibility.Type + } + servicePlan.Spec = korifiv1alpha1.CFServicePlanSpec{ ServicePlan: services.ServicePlan{ Name: catalogPlan.Name, @@ -251,7 +256,7 @@ func (r *Reconciler) reconcileCatalogPlan(ctx context.Context, serviceOffering * Schemas: catalogPlan.Schemas, }, Visibility: korifiv1alpha1.ServicePlanVisibility{ - Type: korifiv1alpha1.AdminServicePlanVisibilityType, + Type: visibilityType, }, } diff --git a/controllers/controllers/services/brokers/controller_test.go b/controllers/controllers/services/brokers/controller_test.go index 04f3fb3f6..c1fddd860 100644 --- a/controllers/controllers/services/brokers/controller_test.go +++ b/controllers/controllers/services/brokers/controller_test.go @@ -130,6 +130,7 @@ var _ = Describe("CFServiceBroker", func() { ))) }).Should(Succeed()) }) + It("sets the ObservedGeneration status field", func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) @@ -233,6 +234,56 @@ var _ = Describe("CFServiceBroker", func() { }).Should(Succeed()) }) + When("the plan visibility is updated", func() { + var planGUID string + + BeforeEach(func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) + g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + + plans := &korifiv1alpha1.CFServicePlanList{} + Expect(adminClient.List(ctx, plans, client.InNamespace(serviceBroker.Namespace))).To(Succeed()) + Expect(plans.Items).To(HaveLen(1)) + + planGUID = plans.Items[0].Name + + plan := &plans.Items[0] + Expect(k8s.PatchResource(ctx, adminClient, plan, func() { + plan.Spec.Visibility.Type = korifiv1alpha1.PublicServicePlanVisibilityType + })).To(Succeed()) + }) + + When("the controller reconciles the broker", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, serviceBroker, func() { + serviceBroker.Spec.Name = uuid.NewString() + })).To(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) + g.Expect(meta.IsStatusConditionTrue(serviceBroker.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) + g.Expect(serviceBroker.Generation).To(Equal(serviceBroker.Status.ObservedGeneration)) + }).Should(Succeed()) + }) + + It("keeps the updated value", func() { + plan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: serviceBroker.Namespace, + Name: planGUID, + }, + } + Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(plan), plan)).To(Succeed()) + Expect(plan.Spec.Visibility.Type).To(Equal(korifiv1alpha1.PublicServicePlanVisibilityType)) + }) + }) + }) + When("getting the catalog fails", func() { BeforeEach(func() { Expect(k8s.PatchResource(ctx, adminClient, serviceBroker, func() { diff --git a/helm/korifi/controllers/cf_roles/cf_admin.yaml b/helm/korifi/controllers/cf_roles/cf_admin.yaml index a70c2013a..6eaf7c7e7 100644 --- a/helm/korifi/controllers/cf_roles/cf_admin.yaml +++ b/helm/korifi/controllers/cf_roles/cf_admin.yaml @@ -206,6 +206,7 @@ rules: verbs: - list - get + - patch - apiGroups: - rbac.authorization.k8s.io diff --git a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceplans.yaml b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceplans.yaml index f2a94ceee..19f09ff12 100644 --- a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceplans.yaml +++ b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceplans.yaml @@ -116,6 +116,7 @@ spec: type: enum: - admin + - public type: string required: - type diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index c397f5642..543fc1c9c 100644 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -52,7 +52,6 @@ var ( commonTestOrgName string defaultAppBitsFile string multiProcessAppBitsFile string - serviceBrokerGUID string serviceBrokerURL string ) @@ -327,7 +326,6 @@ type sharedSetupData struct { CommonOrgName string `json:"commonOrgName"` CommonOrgGUID string `json:"commonOrgGuid"` DefaultAppBitsFile string `json:"defaultAppBitsFile"` - ServiceBrokerGUID string `json:"service_broker_guid"` ServiceBrokerURL string `json:"service_broker_url"` MultiProcessAppBitsFile string `json:"multiProcessAppBitsFile"` AdminServiceAccount string `json:"admin_service_account"` @@ -345,17 +343,15 @@ var _ = SynchronizedBeforeSuite(func() []byte { commonTestOrgName = generateGUID("common-test-org") commonTestOrgGUID = createOrg(commonTestOrgName) - brokerGUID, brokerURL := createSampleBroker(helpers.ZipDirectory( - helpers.GetDefaultedEnvVar("DEFAULT_SERVICE_BROKER_BITS_PATH", "../assets/sample-broker"), - )) sharedData := sharedSetupData{ CommonOrgName: commonTestOrgName, CommonOrgGUID: commonTestOrgGUID, DefaultAppBitsFile: helpers.ZipDirectory( helpers.GetDefaultedEnvVar("DEFAULT_APP_BITS_PATH", "../assets/dorifi"), ), - ServiceBrokerGUID: brokerGUID, - ServiceBrokerURL: brokerURL, + ServiceBrokerURL: pushSampleBroker(helpers.ZipDirectory( + helpers.GetDefaultedEnvVar("DEFAULT_SERVICE_BROKER_BITS_PATH", "../assets/sample-broker"), + )), MultiProcessAppBitsFile: helpers.ZipDirectory("../assets/multi-process"), AdminServiceAccount: adminServiceAccount, AdminServiceAccountToken: adminServiceAccountToken, @@ -375,7 +371,6 @@ var _ = SynchronizedBeforeSuite(func() []byte { commonTestOrgGUID = sharedSetup.CommonOrgGUID commonTestOrgName = sharedSetup.CommonOrgName defaultAppBitsFile = sharedSetup.DefaultAppBitsFile - serviceBrokerGUID = sharedSetup.ServiceBrokerGUID serviceBrokerURL = sharedSetup.ServiceBrokerURL multiProcessAppBitsFile = sharedSetup.MultiProcessAppBitsFile adminServiceAccount = sharedSetup.AdminServiceAccount @@ -387,7 +382,6 @@ var _ = SynchronizedBeforeSuite(func() []byte { var _ = SynchronizedAfterSuite(func() { }, func() { deleteOrg(commonTestOrgGUID) - cleanupBroker(serviceBrokerGUID) serviceAccountFactory.DeleteServiceAccount(adminServiceAccount) }) @@ -1144,13 +1138,12 @@ func printAllRoleBindings(config *rest.Config) { fmt.Fprint(GinkgoWriter, "\n\n========== Expected 404 debug log (end) ==========\n\n") } -func createSampleBroker(brokerBitsFile string) (string, string) { +func pushSampleBroker(brokerBitsFile string) string { spaceGUID := createSpace(uuid.NewString(), commonTestOrgGUID) brokerAppGUID, _ := pushTestApp(spaceGUID, brokerBitsFile) body := curlApp(brokerAppGUID, "") Expect(body).To(ContainSubstring("Hi, I'm the sample broker!")) - brokerURL := getBrokerInClusterURL(brokerAppGUID) - return createBroker(brokerURL), brokerURL + return getBrokerInClusterURL(brokerAppGUID) } func getBrokerInClusterURL(brokerAppGUID string) string { diff --git a/tests/e2e/service_brokers_test.go b/tests/e2e/service_brokers_test.go index 58699f37d..4aa5bb86a 100644 --- a/tests/e2e/service_brokers_test.go +++ b/tests/e2e/service_brokers_test.go @@ -61,7 +61,18 @@ var _ = Describe("Service Brokers", func() { }) Describe("List", func() { - var result resourceList[resource] + var ( + result resourceList[resource] + brokerGUID string + ) + + BeforeEach(func() { + brokerGUID = createBroker(serviceBrokerURL) + }) + + AfterEach(func() { + cleanupBroker(brokerGUID) + }) JustBeforeEach(func() { resp, err = adminClient.R().SetResult(&result).Get("/v3/service_brokers") @@ -71,7 +82,7 @@ var _ = Describe("Service Brokers", func() { It("returns a list of brokers", func() { Expect(resp).To(HaveRestyStatusCode(http.StatusOK)) Expect(result.Resources).To(ContainElement(MatchFields(IgnoreExtras, Fields{ - "GUID": Equal(serviceBrokerGUID), + "GUID": Equal(brokerGUID), }))) }) }) @@ -127,7 +138,7 @@ var _ = Describe("Service Brokers", func() { Expect(resp).To(HaveRestyStatusCode(http.StatusOK)) Expect(servicePlans.Resources).To(ContainElement(MatchFields(IgnoreExtras, Fields{ "Metadata": PointTo(MatchFields(IgnoreExtras, Fields{ - "Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, serviceBrokerGUID), + "Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, brokerGUID), })), }))) }) diff --git a/tests/e2e/service_offerings_test.go b/tests/e2e/service_offerings_test.go index b251f9933..6089159e2 100644 --- a/tests/e2e/service_offerings_test.go +++ b/tests/e2e/service_offerings_test.go @@ -10,7 +10,18 @@ import ( ) var _ = Describe("Service Offerings", func() { - var resp *resty.Response + var ( + resp *resty.Response + brokerGUID string + ) + + BeforeEach(func() { + brokerGUID = createBroker(serviceBrokerURL) + }) + + AfterEach(func() { + cleanupBroker(brokerGUID) + }) Describe("List", func() { var result resourceList[resource] @@ -26,7 +37,7 @@ var _ = Describe("Service Offerings", func() { Expect(result.Resources).To(ContainElement(MatchFields(IgnoreExtras, Fields{ "Relationships": HaveKeyWithValue("service_broker", relationship{ Data: resource{ - GUID: serviceBrokerGUID, + GUID: brokerGUID, }, }), }))) diff --git a/tests/e2e/service_plans_test.go b/tests/e2e/service_plans_test.go index 8cc8334e9..02692029b 100644 --- a/tests/e2e/service_plans_test.go +++ b/tests/e2e/service_plans_test.go @@ -13,7 +13,18 @@ import ( ) var _ = Describe("Service Plans", func() { - var resp *resty.Response + var ( + brokerGUID string + resp *resty.Response + ) + + BeforeEach(func() { + brokerGUID = createBroker(serviceBrokerURL) + }) + + AfterEach(func() { + cleanupBroker(brokerGUID) + }) Describe("List", func() { var result resourceList[resource] @@ -28,44 +39,68 @@ var _ = Describe("Service Plans", func() { Expect(resp).To(HaveRestyStatusCode(http.StatusOK)) Expect(result.Resources).To(ContainElement(MatchFields(IgnoreExtras, Fields{ "Metadata": PointTo(MatchFields(IgnoreExtras, Fields{ - "Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, serviceBrokerGUID), + "Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, brokerGUID), })), }))) }) }) - Describe("Get Visibility", func() { - var ( - result planVisibilityResource - planGUID string - ) + Describe("Visibility", func() { + var planGUID string BeforeEach(func() { plans := resourceList[resource]{} - listResp, err := adminClient.R().SetResult(&plans).Get("/v3/service_plans") Expect(err).NotTo(HaveOccurred()) Expect(listResp).To(HaveRestyStatusCode(http.StatusOK)) brokerPlans := iter.Lift(plans.Resources).Filter(func(r resource) bool { - return r.Metadata.Labels[korifiv1alpha1.RelServiceBrokerLabel] == serviceBrokerGUID + return r.Metadata.Labels[korifiv1alpha1.RelServiceBrokerLabel] == brokerGUID }).Collect() Expect(brokerPlans).NotTo(BeEmpty()) planGUID = brokerPlans[0].GUID }) - JustBeforeEach(func() { - var err error - resp, err = adminClient.R().SetResult(&result).Get(fmt.Sprintf("/v3/service_plans/%s/visibility", planGUID)) - Expect(err).NotTo(HaveOccurred()) + 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)) + Expect(err).NotTo(HaveOccurred()) + }) + + It("returns the service plan visibility", func() { + Expect(resp).To(HaveRestyStatusCode(http.StatusOK)) + Expect(result).To(Equal(planVisibilityResource{ + Type: korifiv1alpha1.AdminServicePlanVisibilityType, + })) + }) }) - It("returns the service plan visibility", func() { - Expect(resp).To(HaveRestyStatusCode(http.StatusOK)) - Expect(result).To(Equal(planVisibilityResource{ - Type: korifiv1alpha1.AdminServicePlanVisibilityType, - })) + Describe("Apply Visibility", func() { + var result planVisibilityResource + + JustBeforeEach(func() { + var err error + resp, err = adminClient.R(). + SetResult(&result). + SetBody(planVisibilityResource{ + Type: "public", + }). + Post(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), + HaveRestyBody(MatchJSON(`{ + "type": "public" + }`)), + )) + }) }) }) })