From 8ac1631fba9ac13bb72b217ffacd41c561482d33 Mon Sep 17 00:00:00 2001 From: Konstantin Lapkov Date: Tue, 3 Dec 2024 13:52:36 +0200 Subject: [PATCH 1/3] Add DELETE v3/service_plans/:guid/visibility/:org_guid --- .../fake/cfservice_plan_repository.go | 78 ++++++++++++++++ api/handlers/service_plan.go | 33 +++++-- api/handlers/service_plan_test.go | 42 +++++++++ api/repositories/service_plan_repository.go | 21 +++++ .../service_plan_repository_test.go | 93 ++++++++++++++++++- 5 files changed, 260 insertions(+), 7 deletions(-) diff --git a/api/handlers/fake/cfservice_plan_repository.go b/api/handlers/fake/cfservice_plan_repository.go index e1b3033a1..cbe0523f6 100644 --- a/api/handlers/fake/cfservice_plan_repository.go +++ b/api/handlers/fake/cfservice_plan_repository.go @@ -39,6 +39,19 @@ type CFServicePlanRepository struct { deletePlanReturnsOnCall map[int]struct { result1 error } + DeletePlanVisibilityStub func(context.Context, authorization.Info, repositories.DeleteServicePlanVisibilityMessage) error + deletePlanVisibilityMutex sync.RWMutex + deletePlanVisibilityArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.DeleteServicePlanVisibilityMessage + } + deletePlanVisibilityReturns struct { + result1 error + } + deletePlanVisibilityReturnsOnCall map[int]struct { + result1 error + } GetPlanStub func(context.Context, authorization.Info, string) (repositories.ServicePlanRecord, error) getPlanMutex sync.RWMutex getPlanArgsForCall []struct { @@ -217,6 +230,69 @@ func (fake *CFServicePlanRepository) DeletePlanReturnsOnCall(i int, result1 erro }{result1} } +func (fake *CFServicePlanRepository) DeletePlanVisibility(arg1 context.Context, arg2 authorization.Info, arg3 repositories.DeleteServicePlanVisibilityMessage) error { + fake.deletePlanVisibilityMutex.Lock() + ret, specificReturn := fake.deletePlanVisibilityReturnsOnCall[len(fake.deletePlanVisibilityArgsForCall)] + fake.deletePlanVisibilityArgsForCall = append(fake.deletePlanVisibilityArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.DeleteServicePlanVisibilityMessage + }{arg1, arg2, arg3}) + stub := fake.DeletePlanVisibilityStub + fakeReturns := fake.deletePlanVisibilityReturns + fake.recordInvocation("DeletePlanVisibility", []interface{}{arg1, arg2, arg3}) + fake.deletePlanVisibilityMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *CFServicePlanRepository) DeletePlanVisibilityCallCount() int { + fake.deletePlanVisibilityMutex.RLock() + defer fake.deletePlanVisibilityMutex.RUnlock() + return len(fake.deletePlanVisibilityArgsForCall) +} + +func (fake *CFServicePlanRepository) DeletePlanVisibilityCalls(stub func(context.Context, authorization.Info, repositories.DeleteServicePlanVisibilityMessage) error) { + fake.deletePlanVisibilityMutex.Lock() + defer fake.deletePlanVisibilityMutex.Unlock() + fake.DeletePlanVisibilityStub = stub +} + +func (fake *CFServicePlanRepository) DeletePlanVisibilityArgsForCall(i int) (context.Context, authorization.Info, repositories.DeleteServicePlanVisibilityMessage) { + fake.deletePlanVisibilityMutex.RLock() + defer fake.deletePlanVisibilityMutex.RUnlock() + argsForCall := fake.deletePlanVisibilityArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServicePlanRepository) DeletePlanVisibilityReturns(result1 error) { + fake.deletePlanVisibilityMutex.Lock() + defer fake.deletePlanVisibilityMutex.Unlock() + fake.DeletePlanVisibilityStub = nil + fake.deletePlanVisibilityReturns = struct { + result1 error + }{result1} +} + +func (fake *CFServicePlanRepository) DeletePlanVisibilityReturnsOnCall(i int, result1 error) { + fake.deletePlanVisibilityMutex.Lock() + defer fake.deletePlanVisibilityMutex.Unlock() + fake.DeletePlanVisibilityStub = nil + if fake.deletePlanVisibilityReturnsOnCall == nil { + fake.deletePlanVisibilityReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.deletePlanVisibilityReturnsOnCall[i] = struct { + result1 error + }{result1} +} + 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)] @@ -422,6 +498,8 @@ func (fake *CFServicePlanRepository) Invocations() map[string][][]interface{} { defer fake.applyPlanVisibilityMutex.RUnlock() fake.deletePlanMutex.RLock() defer fake.deletePlanMutex.RUnlock() + fake.deletePlanVisibilityMutex.RLock() + defer fake.deletePlanVisibilityMutex.RUnlock() fake.getPlanMutex.RLock() defer fake.getPlanMutex.RUnlock() fake.listPlansMutex.RLock() diff --git a/api/handlers/service_plan.go b/api/handlers/service_plan.go index 144ed08f6..f6721cca2 100644 --- a/api/handlers/service_plan.go +++ b/api/handlers/service_plan.go @@ -17,9 +17,10 @@ import ( ) const ( - ServicePlanPath = "/v3/service_plans/{guid}" - ServicePlansPath = "/v3/service_plans" - ServicePlanVisivilityPath = ServicePlanPath + "/visibility" + ServicePlanPath = "/v3/service_plans/{guid}" + ServicePlansPath = "/v3/service_plans" + ServicePlanVisibilityPath = "/v3/service_plans/{guid}/visibility" + ServicePlanVisibilityOrgPath = "/v3/service_plans/{guid}/visibility/{org-guid}" ) //counterfeiter:generate -o fake -fake-name CFServicePlanRepository . CFServicePlanRepository @@ -28,6 +29,7 @@ type CFServicePlanRepository interface { 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) + DeletePlanVisibility(context.Context, authorization.Info, repositories.DeleteServicePlanVisibilityMessage) error DeletePlan(context.Context, authorization.Info, string) error } @@ -132,6 +134,24 @@ func (h *ServicePlan) updatePlanVisibility(r *http.Request) (*routing.Response, return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServicePlanVisibility(visibility, h.serverURL)), nil } +func (h *ServicePlan) deletePlanVisibility(r *http.Request) (*routing.Response, error) { + authInfo, _ := authorization.InfoFromContext(r.Context()) + logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.delete-visibility") + + planGUID := routing.URLParam(r, "guid") + orgGUID := routing.URLParam(r, "org-guid") + logger = logger.WithValues("guid", planGUID) + + if err := h.servicePlanRepo.DeletePlanVisibility(r.Context(), authInfo, repositories.DeleteServicePlanVisibilityMessage{ + PlanGUID: planGUID, + OrgGUID: orgGUID, + }); err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to delete org: %s for plan visibility", orgGUID) + } + + return routing.NewResponse(http.StatusNoContent), nil +} + func (h *ServicePlan) delete(r *http.Request) (*routing.Response, error) { authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.delete") @@ -151,9 +171,10 @@ func (h *ServicePlan) UnauthenticatedRoutes() []routing.Route { 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}, - {Method: "PATCH", Pattern: ServicePlanVisivilityPath, Handler: h.updatePlanVisibility}, + {Method: "GET", Pattern: ServicePlanVisibilityPath, Handler: h.getPlanVisibility}, + {Method: "POST", Pattern: ServicePlanVisibilityPath, Handler: h.applyPlanVisibility}, + {Method: "PATCH", Pattern: ServicePlanVisibilityPath, Handler: h.updatePlanVisibility}, + {Method: "DELETE", Pattern: ServicePlanVisibilityOrgPath, Handler: h.deletePlanVisibility}, {Method: "DELETE", Pattern: ServicePlanPath, Handler: h.delete}, } } diff --git a/api/handlers/service_plan_test.go b/api/handlers/service_plan_test.go index 75f2e7c7f..ffb93e0ec 100644 --- a/api/handlers/service_plan_test.go +++ b/api/handlers/service_plan_test.go @@ -352,6 +352,48 @@ var _ = Describe("ServicePlan", func() { }) }) + Describe("DELETE /v3/service_plans/{guid}/visibility/{org-guid}", func() { + BeforeEach(func() { + servicePlanRepo.DeletePlanVisibilityReturns(nil) + }) + + JustBeforeEach(func() { + req, err := http.NewRequestWithContext(ctx, "DELETE", "/v3/service_plans/my-service-plan/visibility/org-guid", nil) + Expect(err).NotTo(HaveOccurred()) + + routerBuilder.Build().ServeHTTP(rr, req) + }) + + It("deletes the service plan visibility", func() { + Expect(servicePlanRepo.DeletePlanVisibilityCallCount()).To(Equal(1)) + _, actualAuthInfo, actualMessage := servicePlanRepo.DeletePlanVisibilityArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualMessage.PlanGUID).To(Equal("my-service-plan")) + Expect(actualMessage.OrgGUID).To(Equal("org-guid")) + Expect(rr).To(HaveHTTPStatus(http.StatusNoContent)) + }) + + When("deleting the visibility fails with not found", func() { + BeforeEach(func() { + servicePlanRepo.DeletePlanVisibilityReturns(apierrors.NewNotFoundError(nil, repositories.ServicePlanVisibilityResourceType)) + }) + + It("returns 404 Not Found", func() { + expectNotFoundError("Service Plan Visibility") + }) + }) + + When("deleting the visibility fails with an error", func() { + BeforeEach(func() { + servicePlanRepo.DeletePlanVisibilityReturns(errors.New("visibility-err")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + }) + Describe("DELETE /v3/service_plans/:guid", func() { BeforeEach(func() { servicePlanRepo.GetPlanReturns(repositories.ServicePlanRecord{ diff --git a/api/repositories/service_plan_repository.go b/api/repositories/service_plan_repository.go index 7d3047116..13e66bbbd 100644 --- a/api/repositories/service_plan_repository.go +++ b/api/repositories/service_plan_repository.go @@ -99,6 +99,19 @@ func (m *UpdateServicePlanVisibilityMessage) apply(cfServicePlan *korifiv1alpha1 cfServicePlan.Spec.Visibility.Organizations = tools.Uniq(m.Organizations) } +type DeleteServicePlanVisibilityMessage struct { + PlanGUID string + OrgGUID string +} + +func (m *DeleteServicePlanVisibilityMessage) apply(cfServicePlan *korifiv1alpha1.CFServicePlan) { + for i, org := range cfServicePlan.Spec.Visibility.Organizations { + if org == m.OrgGUID { + cfServicePlan.Spec.Visibility.Organizations = append(cfServicePlan.Spec.Visibility.Organizations[:i], cfServicePlan.Spec.Visibility.Organizations[i+1:]...) + } + } +} + func NewServicePlanRepo( userClientFactory authorization.UserClientFactory, rootNamespace string, @@ -155,6 +168,14 @@ func (r *ServicePlanRepo) UpdatePlanVisibility(ctx context.Context, authInfo aut return r.patchServicePlan(ctx, authInfo, message.PlanGUID, message.apply) } +func (r *ServicePlanRepo) DeletePlanVisibility(ctx context.Context, authInfo authorization.Info, message DeleteServicePlanVisibilityMessage) error { + if _, err := r.patchServicePlan(ctx, authInfo, message.PlanGUID, message.apply); err != nil { + return err + } + + return nil +} + func (r *ServicePlanRepo) DeletePlan(ctx context.Context, authInfo authorization.Info, planGUID string) error { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index ff7f17f62..dec3ba4af 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -544,7 +544,7 @@ var _ = Describe("ServicePlanRepo", func() { When("the plan already has the org visibility type", func() { BeforeEach(func() { - anotherOrg := createOrgWithCleanup(ctx, uuid.NewString()) + anotherOrg = createOrgWithCleanup(ctx, uuid.NewString()) createRoleBinding(ctx, userName, orgUserRole.Name, anotherOrg.Name) cfServicePlan := &korifiv1alpha1.CFServicePlan{ @@ -606,6 +606,97 @@ var _ = Describe("ServicePlanRepo", func() { }) }) }) + + Describe("DeletePlanVisibility", func() { + BeforeEach(func() { + cfServicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: planGUID, + }, + } + Expect(k8s.PatchResource(ctx, k8sClient, cfServicePlan, func() { + cfServicePlan.Spec.Visibility.Type = visibilityType + cfServicePlan.Spec.Visibility.Organizations = []string{cfOrg.Name, anotherOrg.Name} + })).To(Succeed()) + }) + + When("the user is not authorized", func() { + BeforeEach(func() { + visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ + PlanGUID: planGUID, + OrgGUID: anotherOrg.Name, + }) + }) + + It("returns unauthorized error", func() { + Expect(visibilityErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) + }) + }) + + When("The user has persmissions", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + }) + + When("the plan and org visibility exist", func() { + BeforeEach(func() { + visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ + PlanGUID: planGUID, + OrgGUID: anotherOrg.Name, + }) + }) + + It("deletes the plan visibility in kubernetes", func() { + Expect(visibilityErr).ToNot(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(Equal([]string{cfOrg.Name})) + }) + }) + + When("the plan does not exist", func() { + JustBeforeEach(func() { + visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ + PlanGUID: "does-not-exist", + OrgGUID: anotherOrg.Name, + }) + }) + + It("returns an NotFoundError", func() { + Expect(errors.As(visibilityErr, &apierrors.NotFoundError{})).To(BeTrue()) + }) + }) + + When("the org does not exist", func() { + BeforeEach(func() { + visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ + PlanGUID: planGUID, + OrgGUID: "does-not-exist", + }) + }) + + It("does not change the visibility orgs", func() { + Expect(visibilityErr).ToNot(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(Equal([]string{cfOrg.Name, anotherOrg.Name})) + }) + }) + }) + }) }) Describe("Delete", func() { From f161ec29679ed94b692a6f8d63ab8a111e5df2e7 Mon Sep 17 00:00:00 2001 From: Konstantin Lapkov Date: Tue, 10 Dec 2024 09:16:24 +0200 Subject: [PATCH 2/3] Address comments --- api/handlers/service_plan_test.go | 15 --------- .../service_plan_repository_test.go | 25 ++++++--------- tests/e2e/e2e_suite_test.go | 4 ++- tests/e2e/service_plans_test.go | 31 +++++++++++++++++++ 4 files changed, 43 insertions(+), 32 deletions(-) diff --git a/api/handlers/service_plan_test.go b/api/handlers/service_plan_test.go index ffb93e0ec..4d9eccb26 100644 --- a/api/handlers/service_plan_test.go +++ b/api/handlers/service_plan_test.go @@ -5,7 +5,6 @@ import ( "net/http" "strings" - apierrors "code.cloudfoundry.org/korifi/api/errors" . "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/handlers/fake" "code.cloudfoundry.org/korifi/api/payloads" @@ -353,10 +352,6 @@ var _ = Describe("ServicePlan", func() { }) Describe("DELETE /v3/service_plans/{guid}/visibility/{org-guid}", func() { - BeforeEach(func() { - servicePlanRepo.DeletePlanVisibilityReturns(nil) - }) - JustBeforeEach(func() { req, err := http.NewRequestWithContext(ctx, "DELETE", "/v3/service_plans/my-service-plan/visibility/org-guid", nil) Expect(err).NotTo(HaveOccurred()) @@ -373,16 +368,6 @@ var _ = Describe("ServicePlan", func() { Expect(rr).To(HaveHTTPStatus(http.StatusNoContent)) }) - When("deleting the visibility fails with not found", func() { - BeforeEach(func() { - servicePlanRepo.DeletePlanVisibilityReturns(apierrors.NewNotFoundError(nil, repositories.ServicePlanVisibilityResourceType)) - }) - - It("returns 404 Not Found", func() { - expectNotFoundError("Service Plan Visibility") - }) - }) - When("deleting the visibility fails with an error", func() { BeforeEach(func() { servicePlanRepo.DeletePlanVisibilityReturns(errors.New("visibility-err")) diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index dec3ba4af..6dc8bb316 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -621,14 +621,14 @@ var _ = Describe("ServicePlanRepo", func() { })).To(Succeed()) }) - When("the user is not authorized", func() { - BeforeEach(func() { - visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ - PlanGUID: planGUID, - OrgGUID: anotherOrg.Name, - }) + JustBeforeEach(func() { + visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ + PlanGUID: planGUID, + OrgGUID: anotherOrg.Name, }) + }) + When("the user is not authorized", func() { It("returns unauthorized error", func() { Expect(visibilityErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) }) @@ -640,13 +640,6 @@ var _ = Describe("ServicePlanRepo", func() { }) When("the plan and org visibility exist", func() { - BeforeEach(func() { - visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ - PlanGUID: planGUID, - OrgGUID: anotherOrg.Name, - }) - }) - It("deletes the plan visibility in kubernetes", func() { Expect(visibilityErr).ToNot(HaveOccurred()) @@ -657,7 +650,7 @@ var _ = Describe("ServicePlanRepo", func() { }, } Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) - Expect(servicePlan.Spec.Visibility.Organizations).To(Equal([]string{cfOrg.Name})) + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name)) }) }) @@ -675,7 +668,7 @@ var _ = Describe("ServicePlanRepo", func() { }) When("the org does not exist", func() { - BeforeEach(func() { + JustBeforeEach(func() { visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ PlanGUID: planGUID, OrgGUID: "does-not-exist", @@ -692,7 +685,7 @@ var _ = Describe("ServicePlanRepo", func() { }, } Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) - Expect(servicePlan.Spec.Visibility.Organizations).To(Equal([]string{cfOrg.Name, anotherOrg.Name})) + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name)) }) }) }) diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index cd23ac3c3..65465e543 100644 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "code.cloudfoundry.org/korifi/model/services" "code.cloudfoundry.org/korifi/tests/helpers" "code.cloudfoundry.org/korifi/tests/helpers/fail_handler" @@ -293,7 +294,8 @@ type cfErr struct { } type planVisibilityResource struct { - Type string `json:"type"` + Type string `json:"type"` + Organizations []services.VisibilityOrganization `json:"organizations"` } func TestE2E(t *testing.T) { diff --git a/tests/e2e/service_plans_test.go b/tests/e2e/service_plans_test.go index 9ff2d064e..45e282aca 100644 --- a/tests/e2e/service_plans_test.go +++ b/tests/e2e/service_plans_test.go @@ -5,6 +5,7 @@ import ( "net/http" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/model/services" "code.cloudfoundry.org/korifi/tests/helpers/broker" "github.com/BooleanCat/go-functional/v2/it/itx" "github.com/go-resty/resty/v2" @@ -121,6 +122,36 @@ var _ = Describe("Service Plans", func() { )) }) }) + + Describe("Delete Visibility", func() { + JustBeforeEach(func() { + var err error + + resp, err = adminClient.R(). + SetBody(planVisibilityResource{ + Type: "organization", + Organizations: []services.VisibilityOrganization{{ + GUID: "org-guid", + }}, + }). + Post(fmt.Sprintf("/v3/service_plans/%s/visibility", planGUID)) + Expect(err).NotTo(HaveOccurred()) + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusOK), + )) + + resp, err = adminClient.R(). + SetResult(&result). + Delete(fmt.Sprintf("/v3/service_plans/%s/visibility/%s", planGUID, "org-guid")) + Expect(err).NotTo(HaveOccurred()) + }) + + It("deletes the plan visibility", func() { + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusNoContent), + )) + }) + }) }) Describe("Delete", func() { From c7711327211c3495ffd9c5f829436f1e811b052e Mon Sep 17 00:00:00 2001 From: Konstantin Lapkov Date: Thu, 12 Dec 2024 16:38:28 +0200 Subject: [PATCH 3/3] Some more test changes --- api/handlers/service_plan_test.go | 1 + .../service_plan_repository_test.go | 28 +++++++++---------- tests/e2e/service_plans_test.go | 6 ++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/api/handlers/service_plan_test.go b/api/handlers/service_plan_test.go index 4d9eccb26..9d9091efd 100644 --- a/api/handlers/service_plan_test.go +++ b/api/handlers/service_plan_test.go @@ -5,6 +5,7 @@ import ( "net/http" "strings" + apierrors "code.cloudfoundry.org/korifi/api/errors" . "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/handlers/fake" "code.cloudfoundry.org/korifi/api/payloads" diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index 6dc8bb316..d263400a6 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -608,6 +608,8 @@ var _ = Describe("ServicePlanRepo", func() { }) Describe("DeletePlanVisibility", func() { + var deleteMessage repositories.DeleteServicePlanVisibilityMessage + BeforeEach(func() { cfServicePlan := &korifiv1alpha1.CFServicePlan{ ObjectMeta: metav1.ObjectMeta{ @@ -619,13 +621,15 @@ var _ = Describe("ServicePlanRepo", func() { cfServicePlan.Spec.Visibility.Type = visibilityType cfServicePlan.Spec.Visibility.Organizations = []string{cfOrg.Name, anotherOrg.Name} })).To(Succeed()) - }) - JustBeforeEach(func() { - visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ + deleteMessage = repositories.DeleteServicePlanVisibilityMessage{ PlanGUID: planGUID, OrgGUID: anotherOrg.Name, - }) + } + }) + + JustBeforeEach(func() { + visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, deleteMessage) }) When("the user is not authorized", func() { @@ -655,11 +659,8 @@ var _ = Describe("ServicePlanRepo", func() { }) When("the plan does not exist", func() { - JustBeforeEach(func() { - visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ - PlanGUID: "does-not-exist", - OrgGUID: anotherOrg.Name, - }) + BeforeEach(func() { + deleteMessage.PlanGUID = "does-not-exist" }) It("returns an NotFoundError", func() { @@ -668,11 +669,8 @@ var _ = Describe("ServicePlanRepo", func() { }) When("the org does not exist", func() { - JustBeforeEach(func() { - visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ - PlanGUID: planGUID, - OrgGUID: "does-not-exist", - }) + BeforeEach(func() { + deleteMessage.OrgGUID = "does-not-exist" }) It("does not change the visibility orgs", func() { @@ -685,7 +683,7 @@ var _ = Describe("ServicePlanRepo", func() { }, } Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) - Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name)) + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name, anotherOrg.Name)) }) }) }) diff --git a/tests/e2e/service_plans_test.go b/tests/e2e/service_plans_test.go index 45e282aca..d514926f0 100644 --- a/tests/e2e/service_plans_test.go +++ b/tests/e2e/service_plans_test.go @@ -124,9 +124,7 @@ var _ = Describe("Service Plans", func() { }) Describe("Delete Visibility", func() { - JustBeforeEach(func() { - var err error - + BeforeEach(func() { resp, err = adminClient.R(). SetBody(planVisibilityResource{ Type: "organization", @@ -139,7 +137,9 @@ var _ = Describe("Service Plans", func() { Expect(resp).To(SatisfyAll( HaveRestyStatusCode(http.StatusOK), )) + }) + JustBeforeEach(func() { resp, err = adminClient.R(). SetResult(&result). Delete(fmt.Sprintf("/v3/service_plans/%s/visibility/%s", planGUID, "org-guid"))