Add DELETE for org in a Service Plan Visibility (#3650)

Add DELETE v3/service_plans/:guid/visibility/:org_guid

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},
 	}
 }

api/handlers/service_plan_test.go

@@ -352,6 +352,34 @@ var _ = Describe("ServicePlan", func() {
 		})
 	})
 
+	Describe("DELETE /v3/service_plans/{guid}/visibility/{org-guid}", func() {
+		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 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{

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 {

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,88 @@ var _ = Describe("ServicePlanRepo", func() {
 				})
 			})
 		})
+
+		Describe("DeletePlanVisibility", func() {
+			var deleteMessage repositories.DeleteServicePlanVisibilityMessage
+
+			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())
+
+				deleteMessage = repositories.DeleteServicePlanVisibilityMessage{
+					PlanGUID: planGUID,
+					OrgGUID:  anotherOrg.Name,
+				}
+			})
+
+			JustBeforeEach(func() {
+				visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, deleteMessage)
+			})
+
+			When("the user is not authorized", func() {
+				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() {
+					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(ConsistOf(cfOrg.Name))
+					})
+				})
+
+				When("the plan does not exist", func() {
+					BeforeEach(func() {
+						deleteMessage.PlanGUID = "does-not-exist"
+					})
+
+					It("returns an NotFoundError", func() {
+						Expect(errors.As(visibilityErr, &apierrors.NotFoundError{})).To(BeTrue())
+					})
+				})
+
+				When("the org does not exist", func() {
+					BeforeEach(func() {
+						deleteMessage.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(ConsistOf(cfOrg.Name, anotherOrg.Name))
+					})
+				})
+			})
+		})
 	})
 
 	Describe("Delete", func() {

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) {

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() {
+			BeforeEach(func() {
+				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),
+				))
+			})
+
+			JustBeforeEach(func() {
+				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() {