Implement DELETE /v3/service_brokers/guid

fixes #3268 Co-authored-by: Danail Branekov <danailster@gmail.com>
cloudfoundry · Jul 18, 2024 · 28dd19f · 28dd19f
1 parent 55a890b
commit 28dd19f
Show file tree

Hide file tree

Showing 12 changed files with 526 additions and 56 deletions.
diff --git a/api/handlers/fake/cfservice_broker_repository.go b/api/handlers/fake/cfservice_broker_repository.go
diff --git a/api/handlers/job.go b/api/handlers/job.go
@@ -26,6 +26,7 @@ const (
 	DomainDeleteJobType        = "domain.delete"
 	RoleDeleteJobType          = "role.delete"
 	ServiceBrokerCreateJobType = "service_broker.create"
+	ServiceBrokerDeleteJobType = "service_broker.delete"
 
 	JobTimeoutDuration = 120.0
 )

diff --git a/api/handlers/service_broker.go b/api/handlers/service_broker.go
@@ -16,12 +16,15 @@ import (
 
 const (
 	ServiceBrokersPath = "/v3/service_brokers"
+	ServiceBrokerPath  = ServiceBrokersPath + "/{guid}"
 )
 
 //counterfeiter:generate -o fake -fake-name CFServiceBrokerRepository . CFServiceBrokerRepository
 type CFServiceBrokerRepository interface {
 	CreateServiceBroker(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error)
 	ListServiceBrokers(context.Context, authorization.Info, repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerResource, error)
+	GetServiceBroker(context.Context, authorization.Info, string) (repositories.ServiceBrokerResource, error)
+	DeleteServiceBroker(context.Context, authorization.Info, string) error
 }
 
 type ServiceBroker struct {
@@ -76,6 +79,26 @@ func (h *ServiceBroker) list(r *http.Request) (*routing.Response, error) {
 	return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceBroker, brokers, h.serverURL, *r.URL)), nil
 }
 
+func (h *ServiceBroker) delete(r *http.Request) (*routing.Response, error) {
+	authInfo, _ := authorization.InfoFromContext(r.Context())
+	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-broker.delete")
+
+	guid := routing.URLParam(r, "guid")
+
+	_, err := h.serviceBrokerRepo.GetServiceBroker(r.Context(), authInfo, guid)
+	if err != nil {
+		return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "failed to get service broker")
+	}
+
+	err = h.serviceBrokerRepo.DeleteServiceBroker(r.Context(), authInfo, guid)
+	if err != nil {
+		return nil, apierrors.LogAndReturn(logger, err, "error when deleting service broker", "guid", guid)
+	}
+
+	return routing.NewResponse(http.StatusAccepted).
+		WithHeader("Location", presenter.JobURLForRedirects(guid, presenter.ServiceBrokerDeleteOperation, h.serverURL)), nil
+}
+
 func (h *ServiceBroker) UnauthenticatedRoutes() []routing.Route {
 	return nil
 }
@@ -84,5 +107,6 @@ func (h *ServiceBroker) AuthenticatedRoutes() []routing.Route {
 	return []routing.Route{
 		{Method: "POST", Pattern: ServiceBrokersPath, Handler: h.create},
 		{Method: "GET", Pattern: ServiceBrokersPath, Handler: h.list},
+		{Method: "DELETE", Pattern: ServiceBrokerPath, Handler: h.delete},
 	}
 }
diff --git a/api/handlers/service_broker_test.go b/api/handlers/service_broker_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"
@@ -167,4 +168,63 @@ var _ = Describe("ServiceBroker", func() {
 			})
 		})
 	})
+
+	Describe("DELETE /v3/service_brokers/guid", func() {
+		BeforeEach(func() {
+			serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerResource{
+				CFResource: model.CFResource{
+					GUID: "broker-guid",
+				},
+			}, nil)
+
+			var err error
+			req, err = http.NewRequestWithContext(ctx, "DELETE", "/v3/service_brokers/broker-guid", nil)
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("deletes the service broker", func() {
+			Expect(serviceBrokerRepo.GetServiceBrokerCallCount()).To(Equal(1))
+			_, actualAuthInfo, actualBrokerGUID := serviceBrokerRepo.GetServiceBrokerArgsForCall(0)
+			Expect(actualAuthInfo).To(Equal(authInfo))
+			Expect(actualBrokerGUID).To(Equal("broker-guid"))
+
+			Expect(serviceBrokerRepo.DeleteServiceBrokerCallCount()).To(Equal(1))
+			_, actualAuthInfo, actualBrokerGUID = serviceBrokerRepo.DeleteServiceBrokerArgsForCall(0)
+			Expect(actualAuthInfo).To(Equal(authInfo))
+			Expect(actualBrokerGUID).To(Equal("broker-guid"))
+
+			Expect(rr).To(HaveHTTPStatus(http.StatusAccepted))
+			Expect(rr).To(HaveHTTPHeaderWithValue("Location", "https://api.example.org/v3/jobs/service_broker.delete~broker-guid"))
+		})
+
+		When("getting the service broker is not allowed", func() {
+			BeforeEach(func() {
+				serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerResource{}, apierrors.NewForbiddenError(nil, repositories.ServiceBrokerResourceType))
+			})
+
+			It("returns a not found error", func() {
+				expectNotFoundError(repositories.ServiceBrokerResourceType)
+			})
+		})
+
+		When("getting the service broker fails", func() {
+			BeforeEach(func() {
+				serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerResource{}, errors.New("get-broker-err"))
+			})
+
+			It("returns an error", func() {
+				expectUnknownError()
+			})
+		})
+
+		When("getting the service broker fails", func() {
+			BeforeEach(func() {
+				serviceBrokerRepo.DeleteServiceBrokerReturns(errors.New("delete-broker-err"))
+			})
+
+			It("returns an error", func() {
+				expectUnknownError()
+			})
+		})
+	})
 })
diff --git a/api/main.go b/api/main.go
@@ -343,12 +343,13 @@ func main() {
 		handlers.NewJob(
 			*serverURL,
 			map[string]handlers.DeletionRepository{
-				handlers.OrgDeleteJobType:    orgRepo,
-				handlers.SpaceDeleteJobType:  spaceRepo,
-				handlers.AppDeleteJobType:    appRepo,
-				handlers.RouteDeleteJobType:  routeRepo,
-				handlers.DomainDeleteJobType: domainRepo,
-				handlers.RoleDeleteJobType:   roleRepo,
+				handlers.OrgDeleteJobType:           orgRepo,
+				handlers.SpaceDeleteJobType:         spaceRepo,
+				handlers.AppDeleteJobType:           appRepo,
+				handlers.RouteDeleteJobType:         routeRepo,
+				handlers.DomainDeleteJobType:        domainRepo,
+				handlers.RoleDeleteJobType:          roleRepo,
+				handlers.ServiceBrokerDeleteJobType: serviceBrokerRepo,
 			},
 			map[string]handlers.StateRepository{
 				handlers.ServiceBrokerCreateJobType: serviceBrokerRepo,

diff --git a/api/payloads/service_broker.go b/api/payloads/service_broker.go
@@ -54,7 +54,7 @@ func (b *ServiceBrokerList) DecodeFromURLValues(values url.Values) error {
 }
 
 func (b *ServiceBrokerList) SupportedKeys() []string {
-	return []string{"names"}
+	return []string{"names", "page", "per_page"}
 }
 
 func (b *ServiceBrokerList) ToMessage() repositories.ListServiceBrokerMessage {

diff --git a/api/presenter/job.go b/api/presenter/job.go
@@ -24,6 +24,7 @@ const (
 	DomainDeleteOperation        = "domain.delete"
 	RoleDeleteOperation          = "role.delete"
 	ServiceBrokerCreateOperation = "service_broker.create"
+	ServiceBrokerDeleteOperation = "service_broker.delete"
 )
 
 var (