From 28dd19f0b8e661e410e91357c1dcc6f54f79cc1f Mon Sep 17 00:00:00 2001 From: Georgi Sabev Date: Tue, 16 Jul 2024 15:13:59 +0000 Subject: [PATCH] Implement `DELETE /v3/service_brokers/guid` fixes #3268 Co-authored-by: Danail Branekov --- .../fake/cfservice_broker_repository.go | 161 +++++++++++++++++ api/handlers/job.go | 1 + api/handlers/service_broker.go | 24 +++ api/handlers/service_broker_test.go | 60 +++++++ api/main.go | 13 +- api/payloads/service_broker.go | 2 +- api/presenter/job.go | 1 + api/repositories/service_broker_repository.go | 57 ++++++ .../service_broker_repository_test.go | 162 ++++++++++++++++++ .../korifi/controllers/cf_roles/cf_admin.yaml | 1 + tests/e2e/e2e_suite_test.go | 25 +-- tests/e2e/service_brokers_test.go | 75 ++++---- 12 files changed, 526 insertions(+), 56 deletions(-) diff --git a/api/handlers/fake/cfservice_broker_repository.go b/api/handlers/fake/cfservice_broker_repository.go index 8e482cc7a..7c7871819 100644 --- a/api/handlers/fake/cfservice_broker_repository.go +++ b/api/handlers/fake/cfservice_broker_repository.go @@ -26,6 +26,34 @@ type CFServiceBrokerRepository struct { result1 repositories.ServiceBrokerResource result2 error } + DeleteServiceBrokerStub func(context.Context, authorization.Info, string) error + deleteServiceBrokerMutex sync.RWMutex + deleteServiceBrokerArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 string + } + deleteServiceBrokerReturns struct { + result1 error + } + deleteServiceBrokerReturnsOnCall map[int]struct { + result1 error + } + GetServiceBrokerStub func(context.Context, authorization.Info, string) (repositories.ServiceBrokerResource, error) + getServiceBrokerMutex sync.RWMutex + getServiceBrokerArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 string + } + getServiceBrokerReturns struct { + result1 repositories.ServiceBrokerResource + result2 error + } + getServiceBrokerReturnsOnCall map[int]struct { + result1 repositories.ServiceBrokerResource + result2 error + } ListServiceBrokersStub func(context.Context, authorization.Info, repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerResource, error) listServiceBrokersMutex sync.RWMutex listServiceBrokersArgsForCall []struct { @@ -111,6 +139,135 @@ func (fake *CFServiceBrokerRepository) CreateServiceBrokerReturnsOnCall(i int, r }{result1, result2} } +func (fake *CFServiceBrokerRepository) DeleteServiceBroker(arg1 context.Context, arg2 authorization.Info, arg3 string) error { + fake.deleteServiceBrokerMutex.Lock() + ret, specificReturn := fake.deleteServiceBrokerReturnsOnCall[len(fake.deleteServiceBrokerArgsForCall)] + fake.deleteServiceBrokerArgsForCall = append(fake.deleteServiceBrokerArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 string + }{arg1, arg2, arg3}) + stub := fake.DeleteServiceBrokerStub + fakeReturns := fake.deleteServiceBrokerReturns + fake.recordInvocation("DeleteServiceBroker", []interface{}{arg1, arg2, arg3}) + fake.deleteServiceBrokerMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *CFServiceBrokerRepository) DeleteServiceBrokerCallCount() int { + fake.deleteServiceBrokerMutex.RLock() + defer fake.deleteServiceBrokerMutex.RUnlock() + return len(fake.deleteServiceBrokerArgsForCall) +} + +func (fake *CFServiceBrokerRepository) DeleteServiceBrokerCalls(stub func(context.Context, authorization.Info, string) error) { + fake.deleteServiceBrokerMutex.Lock() + defer fake.deleteServiceBrokerMutex.Unlock() + fake.DeleteServiceBrokerStub = stub +} + +func (fake *CFServiceBrokerRepository) DeleteServiceBrokerArgsForCall(i int) (context.Context, authorization.Info, string) { + fake.deleteServiceBrokerMutex.RLock() + defer fake.deleteServiceBrokerMutex.RUnlock() + argsForCall := fake.deleteServiceBrokerArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServiceBrokerRepository) DeleteServiceBrokerReturns(result1 error) { + fake.deleteServiceBrokerMutex.Lock() + defer fake.deleteServiceBrokerMutex.Unlock() + fake.DeleteServiceBrokerStub = nil + fake.deleteServiceBrokerReturns = struct { + result1 error + }{result1} +} + +func (fake *CFServiceBrokerRepository) DeleteServiceBrokerReturnsOnCall(i int, result1 error) { + fake.deleteServiceBrokerMutex.Lock() + defer fake.deleteServiceBrokerMutex.Unlock() + fake.DeleteServiceBrokerStub = nil + if fake.deleteServiceBrokerReturnsOnCall == nil { + fake.deleteServiceBrokerReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.deleteServiceBrokerReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *CFServiceBrokerRepository) GetServiceBroker(arg1 context.Context, arg2 authorization.Info, arg3 string) (repositories.ServiceBrokerResource, error) { + fake.getServiceBrokerMutex.Lock() + ret, specificReturn := fake.getServiceBrokerReturnsOnCall[len(fake.getServiceBrokerArgsForCall)] + fake.getServiceBrokerArgsForCall = append(fake.getServiceBrokerArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 string + }{arg1, arg2, arg3}) + stub := fake.GetServiceBrokerStub + fakeReturns := fake.getServiceBrokerReturns + fake.recordInvocation("GetServiceBroker", []interface{}{arg1, arg2, arg3}) + fake.getServiceBrokerMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *CFServiceBrokerRepository) GetServiceBrokerCallCount() int { + fake.getServiceBrokerMutex.RLock() + defer fake.getServiceBrokerMutex.RUnlock() + return len(fake.getServiceBrokerArgsForCall) +} + +func (fake *CFServiceBrokerRepository) GetServiceBrokerCalls(stub func(context.Context, authorization.Info, string) (repositories.ServiceBrokerResource, error)) { + fake.getServiceBrokerMutex.Lock() + defer fake.getServiceBrokerMutex.Unlock() + fake.GetServiceBrokerStub = stub +} + +func (fake *CFServiceBrokerRepository) GetServiceBrokerArgsForCall(i int) (context.Context, authorization.Info, string) { + fake.getServiceBrokerMutex.RLock() + defer fake.getServiceBrokerMutex.RUnlock() + argsForCall := fake.getServiceBrokerArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServiceBrokerRepository) GetServiceBrokerReturns(result1 repositories.ServiceBrokerResource, result2 error) { + fake.getServiceBrokerMutex.Lock() + defer fake.getServiceBrokerMutex.Unlock() + fake.GetServiceBrokerStub = nil + fake.getServiceBrokerReturns = struct { + result1 repositories.ServiceBrokerResource + result2 error + }{result1, result2} +} + +func (fake *CFServiceBrokerRepository) GetServiceBrokerReturnsOnCall(i int, result1 repositories.ServiceBrokerResource, result2 error) { + fake.getServiceBrokerMutex.Lock() + defer fake.getServiceBrokerMutex.Unlock() + fake.GetServiceBrokerStub = nil + if fake.getServiceBrokerReturnsOnCall == nil { + fake.getServiceBrokerReturnsOnCall = make(map[int]struct { + result1 repositories.ServiceBrokerResource + result2 error + }) + } + fake.getServiceBrokerReturnsOnCall[i] = struct { + result1 repositories.ServiceBrokerResource + result2 error + }{result1, result2} +} + func (fake *CFServiceBrokerRepository) ListServiceBrokers(arg1 context.Context, arg2 authorization.Info, arg3 repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerResource, error) { fake.listServiceBrokersMutex.Lock() ret, specificReturn := fake.listServiceBrokersReturnsOnCall[len(fake.listServiceBrokersArgsForCall)] @@ -182,6 +339,10 @@ func (fake *CFServiceBrokerRepository) Invocations() map[string][][]interface{} defer fake.invocationsMutex.RUnlock() fake.createServiceBrokerMutex.RLock() defer fake.createServiceBrokerMutex.RUnlock() + fake.deleteServiceBrokerMutex.RLock() + defer fake.deleteServiceBrokerMutex.RUnlock() + fake.getServiceBrokerMutex.RLock() + defer fake.getServiceBrokerMutex.RUnlock() fake.listServiceBrokersMutex.RLock() defer fake.listServiceBrokersMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} diff --git a/api/handlers/job.go b/api/handlers/job.go index b1254993f..2ea5f3fbb 100644 --- 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 index 7c5370e2e..cf4bf6acd 100644 --- 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 index 8ad95ce8c..dff4fa587 100644 --- 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 index 7a680e128..523aa54d0 100644 --- 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 index f54046ec0..2a3af7834 100644 --- 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 index 876b99f54..14ba76774 100644 --- 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 ( diff --git a/api/repositories/service_broker_repository.go b/api/repositories/service_broker_repository.go index c4623ab8e..5fb85ea37 100644 --- a/api/repositories/service_broker_repository.go +++ b/api/repositories/service_broker_repository.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "slices" + "time" "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" @@ -169,3 +170,59 @@ func (r *ServiceBrokerRepo) ListServiceBrokers(ctx context.Context, authInfo aut return iter.Map(brokers, toServiceBrokerResource).Collect(), nil } + +func (r *ServiceBrokerRepo) GetServiceBroker(ctx context.Context, authInfo authorization.Info, guid string) (ServiceBrokerResource, error) { + serviceBroker, err := r.getServiceBroker(ctx, authInfo, guid) + if err != nil { + return ServiceBrokerResource{}, err + } + return toServiceBrokerResource(*serviceBroker), nil +} + +func (r *ServiceBrokerRepo) getServiceBroker(ctx context.Context, authInfo authorization.Info, guid string) (*korifiv1alpha1.CFServiceBroker, error) { + userClient, err := r.userClientFactory.BuildClient(authInfo) + if err != nil { + return nil, fmt.Errorf("failed to build user client: %w", err) + } + + serviceBroker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.rootNamespace, + Name: guid, + }, + } + + if err := userClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker); err != nil { + return nil, apierrors.FromK8sError(err, ServiceBrokerResourceType) + } + + return serviceBroker, nil +} + +func (r *ServiceBrokerRepo) DeleteServiceBroker(ctx context.Context, authInfo authorization.Info, guid string) error { + userClient, err := r.userClientFactory.BuildClient(authInfo) + if err != nil { + return fmt.Errorf("failed to build user client: %w", err) + } + + serviceBroker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Name: guid, + Namespace: r.rootNamespace, + }, + } + + return apierrors.FromK8sError( + userClient.Delete(ctx, serviceBroker), + ServiceBrokerResourceType, + ) +} + +func (r *ServiceBrokerRepo) GetDeletedAt(ctx context.Context, authInfo authorization.Info, guid string) (*time.Time, error) { + serviceBroker, err := r.getServiceBroker(ctx, authInfo, guid) + if err != nil { + return nil, err + } + + return golangTime(serviceBroker.GetDeletionTimestamp()), nil +} diff --git a/api/repositories/service_broker_repository_test.go b/api/repositories/service_broker_repository_test.go index 15b9f7326..eb28173b1 100644 --- a/api/repositories/service_broker_repository_test.go +++ b/api/repositories/service_broker_repository_test.go @@ -1,11 +1,14 @@ package repositories_test import ( + "time" + 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" "code.cloudfoundry.org/korifi/tools/k8s" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -334,4 +337,163 @@ var _ = Describe("ServiceBrokerRepo", func() { }) }) }) + + Describe("GetServiceBroker", func() { + var ( + serviceBroker repositories.ServiceBrokerResource + getErr error + ) + + BeforeEach(func() { + Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: "broker-1", + Labels: map[string]string{ + "broker-label": "broker-label-value", + }, + Annotations: map[string]string{ + "broker-annotation": "broker-annotation-value", + }, + }, + Spec: korifiv1alpha1.CFServiceBrokerSpec{ + ServiceBroker: services.ServiceBroker{ + Name: "first-broker", + URL: "https://first.broker", + }, + }, + })).To(Succeed()) + }) + + JustBeforeEach(func() { + serviceBroker, getErr = repo.GetServiceBroker(ctx, authInfo, "broker-1") + }) + + It("returns a forbidden error", func() { + Expect(getErr).To(SatisfyAll( + BeAssignableToTypeOf(apierrors.ForbiddenError{}), + MatchError(ContainSubstring(`cfservicebrokers.korifi.cloudfoundry.org "broker-1" is forbidden`)), + )) + }) + + When("the user can get CFServiceBrokers", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + }) + + It("returns the broker", func() { + Expect(getErr).NotTo(HaveOccurred()) + Expect(serviceBroker).To(MatchAllFields(Fields{ + "ServiceBroker": MatchAllFields(Fields{ + "Name": Equal("first-broker"), + "URL": Equal("https://first.broker"), + }), + "CFResource": MatchFields(IgnoreExtras, Fields{ + "GUID": Equal("broker-1"), + "CreatedAt": Not(BeZero()), + "Metadata": MatchAllFields(Fields{ + "Labels": HaveKeyWithValue("broker-label", "broker-label-value"), + "Annotations": HaveKeyWithValue("broker-annotation", "broker-annotation-value"), + }), + }), + })) + }) + }) + }) + + Describe("DeleteServiceBroker", func() { + var ( + deleteErr error + brokerGUID string + ) + + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + + brokerGUID = uuid.NewString() + Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: brokerGUID, + }, + })).To(Succeed()) + }) + + JustBeforeEach(func() { + deleteErr = repo.DeleteServiceBroker(ctx, authInfo, brokerGUID) + }) + + It("deletes the CFServiceBroker resource", func() { + Expect(deleteErr).NotTo(HaveOccurred()) + + brokers := &korifiv1alpha1.CFServiceBrokerList{} + Expect(k8sClient.List(ctx, brokers, client.InNamespace(rootNamespace))).To(Succeed()) + Expect(brokers.Items).To(BeEmpty()) + }) + + When("the broker doesn't exist", func() { + BeforeEach(func() { + brokerGUID = "no-such-broker" + }) + + It("errors", func() { + Expect(deleteErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.NotFoundError{})) + }) + }) + }) + + Describe("GetDeletedAt", func() { + var ( + broker *korifiv1alpha1.CFServiceBroker + deletedAt *time.Time + getErr error + ) + + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + + broker = &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + }, + } + + Expect(k8sClient.Create(ctx, broker)).To(Succeed()) + }) + + JustBeforeEach(func() { + deletedAt, getErr = repo.GetDeletedAt(ctx, authInfo, broker.Name) + }) + + It("returns nil", func() { + Expect(getErr).NotTo(HaveOccurred()) + Expect(deletedAt).To(BeNil()) + }) + + When("the broker is being deleted", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, k8sClient, broker, func() { + broker.Finalizers = append(broker.Finalizers, "kubernetes") + })).To(Succeed()) + + Expect(k8sClient.Delete(ctx, broker)).To(Succeed()) + }) + + It("returns the deletion time", func() { + Expect(getErr).NotTo(HaveOccurred()) + Expect(deletedAt).To(PointTo(BeTemporally("~", time.Now(), time.Minute))) + }) + }) + + When("the broker isn't found", func() { + BeforeEach(func() { + Expect(k8sClient.Delete(ctx, broker)).To(Succeed()) + }) + + It("errors", func() { + Expect(getErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.NotFoundError{})) + }) + }) + }) }) diff --git a/helm/korifi/controllers/cf_roles/cf_admin.yaml b/helm/korifi/controllers/cf_roles/cf_admin.yaml index d06f11dd2..bf0e4707c 100644 --- a/helm/korifi/controllers/cf_roles/cf_admin.yaml +++ b/helm/korifi/controllers/cf_roles/cf_admin.yaml @@ -195,6 +195,7 @@ rules: - create - get - list + - delete - apiGroups: - korifi.cloudfoundry.org diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index c26029361..2f5b41db4 100644 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -383,7 +383,7 @@ var _ = SynchronizedBeforeSuite(func() []byte { var _ = SynchronizedAfterSuite(func() { }, func() { deleteOrg(commonTestOrgGUID) - cleanupBrokers() + cleanupBroker(serviceBrokerGUID) serviceAccountFactory.DeleteServiceAccount(adminServiceAccount) }) @@ -1203,27 +1203,28 @@ func createBroker(brokerURL string) string { return brokerGUID } -func cleanupBrokers() { +func cleanupBroker(brokerGUID string) { + GinkgoHelper() + + Expect(brokerGUID).NotTo(BeEmpty()) + _, err := adminClient.R(). + Delete("/v3/service_brokers/" + brokerGUID) + Expect(err).NotTo(HaveOccurred()) + + ctx := context.Background() + config, err := controllerruntime.GetConfig() Expect(err).NotTo(HaveOccurred()) k8sClient, err := client.New(config, client.Options{Scheme: scheme.Scheme}) Expect(err).NotTo(HaveOccurred()) - ctx := context.Background() - Expect(k8sClient.Delete(ctx, &korifiv1alpha1.CFServiceBroker{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: rootNamespace, - Name: serviceBrokerGUID, - }, - })).To(Succeed()) - Expect(k8sClient.DeleteAllOf( ctx, &korifiv1alpha1.CFServiceOffering{}, client.InNamespace(rootNamespace), client.MatchingLabels{ - korifiv1alpha1.RelServiceBrokerLabel: serviceBrokerGUID, + korifiv1alpha1.RelServiceBrokerLabel: brokerGUID, }, )).To(Succeed()) @@ -1232,7 +1233,7 @@ func cleanupBrokers() { &korifiv1alpha1.CFServicePlan{}, client.InNamespace(rootNamespace), client.MatchingLabels{ - korifiv1alpha1.RelServiceBrokerLabel: serviceBrokerGUID, + korifiv1alpha1.RelServiceBrokerLabel: brokerGUID, }, )).To(Succeed()) } diff --git a/tests/e2e/service_brokers_test.go b/tests/e2e/service_brokers_test.go index 2b816de85..964ed8216 100644 --- a/tests/e2e/service_brokers_test.go +++ b/tests/e2e/service_brokers_test.go @@ -1,21 +1,14 @@ package e2e_test import ( - "context" "net/http" "strings" - korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "github.com/go-resty/resty/v2" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - controllerruntime "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("Service Brokers", func() { @@ -25,10 +18,6 @@ var _ = Describe("Service Brokers", func() { ) Describe("Create", func() { - BeforeEach(func() { - Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) - }) - JustBeforeEach(func() { resp, err = adminClient.R(). SetBody(serviceBrokerResource{ @@ -54,32 +43,6 @@ var _ = Describe("Service Brokers", func() { HaveRestyHeaderWithValue("Location", ContainSubstring("/v3/jobs/service_broker.create~")), )) - locationSplit := strings.Split(resp.Header().Get("Location"), "~") - DeferCleanup(func() { - // Temporarily delete the broker created by the test via the k8s client - // Once the API supports broker deletion, we could get rid of this - if len(locationSplit) != 2 { - return - } - - var config *rest.Config - config, err = controllerruntime.GetConfig() - Expect(err).NotTo(HaveOccurred()) - - var k8sClient client.Client - k8sClient, err = client.New(config, client.Options{Scheme: scheme.Scheme}) - Expect(err).NotTo(HaveOccurred()) - - broker := &korifiv1alpha1.CFServiceBroker{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: rootNamespace, - Name: locationSplit[1], - }, - } - - Expect(k8sClient.Delete(context.Background(), broker)).To(Succeed()) - }) - jobURL := resp.Header().Get("Location") Eventually(func(g Gomega) { resp, err = adminClient.R().Get(jobURL) @@ -87,6 +50,12 @@ var _ = Describe("Service Brokers", func() { jobRespBody := string(resp.Body()) g.Expect(jobRespBody).To(ContainSubstring("COMPLETE")) }).Should(Succeed()) + + jobURLSplit := strings.Split(jobURL, "~") + Expect(jobURLSplit).To(HaveLen(2)) + DeferCleanup(func() { + cleanupBroker(jobURLSplit[1]) + }) }) }) @@ -105,4 +74,36 @@ var _ = Describe("Service Brokers", func() { }))) }) }) + + Describe("Delete", func() { + var brokerGUID string + + BeforeEach(func() { + brokerGUID = createBroker(serviceBrokerURL) + }) + + AfterEach(func() { + cleanupBroker(brokerGUID) + }) + + JustBeforeEach(func() { + resp, err = adminClient.R().Delete("/v3/service_brokers/" + brokerGUID) + Expect(err).NotTo(HaveOccurred()) + }) + + It("succeeds with a job redirect", func() { + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusAccepted), + HaveRestyHeaderWithValue("Location", ContainSubstring("/v3/jobs/service_broker.delete~")), + )) + + jobURL := resp.Header().Get("Location") + Eventually(func(g Gomega) { + resp, err = adminClient.R().Get(jobURL) + g.Expect(err).NotTo(HaveOccurred()) + jobRespBody := string(resp.Body()) + g.Expect(jobRespBody).To(ContainSubstring("COMPLETE")) + }).Should(Succeed()) + }) + }) })