Skip to content

Commit 296b23c

Browse files
klapkovklapkov
authored
Add DELETE /v3/service_plans/:guid (#3620)
* Add DELETE /v3/service_plans/:guid
1 parent 42c250a commit 296b23c

File tree

9 files changed

+231
-9
lines changed

9 files changed

+231
-9
lines changed

‎api/handlers/fake/cfservice_plan_repository.go‎

Lines changed: 78 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/handlers/service_plan.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ import (
1717
)
1818

1919
const (
20+
ServicePlanPath = "/v3/service_plans/{guid}"
2021
ServicePlansPath = "/v3/service_plans"
21-
ServicePlanVisivilityPath = "/v3/service_plans/{guid}/visibility"
22+
ServicePlanVisivilityPath = ServicePlanPath + "/visibility"
2223
)
2324

2425
//counterfeiter:generate -o fake -fake-name CFServicePlanRepository . CFServicePlanRepository
@@ -27,6 +28,7 @@ type CFServicePlanRepository interface {
2728
ListPlans(context.Context, authorization.Info, repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error)
2829
ApplyPlanVisibility(context.Context, authorization.Info, repositories.ApplyServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error)
2930
UpdatePlanVisibility(context.Context, authorization.Info, repositories.UpdateServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error)
31+
DeletePlan(context.Context, authorization.Info, string) error
3032
}
3133

3234
type ServicePlan struct {
@@ -130,6 +132,18 @@ func (h *ServicePlan) updatePlanVisibility(r *http.Request) (*routing.Response,
130132
return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServicePlanVisibility(visibility, h.serverURL)), nil
131133
}
132134

135+
func (h *ServicePlan) delete(r *http.Request) (*routing.Response, error) {
136+
authInfo, _ := authorization.InfoFromContext(r.Context())
137+
logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.delete")
138+
139+
planGUID := routing.URLParam(r, "guid")
140+
if err := h.servicePlanRepo.DeletePlan(r.Context(), authInfo, planGUID); err != nil {
141+
return nil, apierrors.LogAndReturn(logger, err, "failed to delete plan: %s", planGUID)
142+
}
143+
144+
return routing.NewResponse(http.StatusNoContent), nil
145+
}
146+
133147
func (h *ServicePlan) UnauthenticatedRoutes() []routing.Route {
134148
return nil
135149
}
@@ -140,5 +154,6 @@ func (h *ServicePlan) AuthenticatedRoutes() []routing.Route {
140154
{Method: "GET", Pattern: ServicePlanVisivilityPath, Handler: h.getPlanVisibility},
141155
{Method: "POST", Pattern: ServicePlanVisivilityPath, Handler: h.applyPlanVisibility},
142156
{Method: "PATCH", Pattern: ServicePlanVisivilityPath, Handler: h.updatePlanVisibility},
157+
{Method: "DELETE", Pattern: ServicePlanPath, Handler: h.delete},
143158
}
144159
}

‎api/handlers/service_plan_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"net/http"
66
"strings"
77

8+
apierrors "code.cloudfoundry.org/korifi/api/errors"
89
. "code.cloudfoundry.org/korifi/api/handlers"
910
"code.cloudfoundry.org/korifi/api/handlers/fake"
1011
"code.cloudfoundry.org/korifi/api/payloads"
@@ -350,4 +351,53 @@ var _ = Describe("ServicePlan", func() {
350351
})
351352
})
352353
})
354+
355+
Describe("DELETE /v3/service_plans/:guid", func() {
356+
BeforeEach(func() {
357+
servicePlanRepo.GetPlanReturns(repositories.ServicePlanRecord{
358+
CFResource: model.CFResource{
359+
GUID: "plan-guid",
360+
},
361+
ServiceOfferingGUID: "service-offering-guid",
362+
}, nil)
363+
})
364+
365+
JustBeforeEach(func() {
366+
req, err := http.NewRequestWithContext(ctx, "DELETE", "/v3/service_plans/plan-guid", nil)
367+
Expect(err).NotTo(HaveOccurred())
368+
369+
routerBuilder.Build().ServeHTTP(rr, req)
370+
})
371+
372+
It("deletes the service plan", func() {
373+
Expect(servicePlanRepo.DeletePlanCallCount()).To(Equal(1))
374+
_, actualAuthInfo, actualPlanGUID := servicePlanRepo.DeletePlanArgsForCall(0)
375+
Expect(actualAuthInfo).To(Equal(authInfo))
376+
Expect(actualPlanGUID).To(Equal("plan-guid"))
377+
378+
Expect(rr).Should(HaveHTTPStatus(http.StatusNoContent))
379+
})
380+
381+
When("deleting the service plan fails with not found", func() {
382+
BeforeEach(func() {
383+
servicePlanRepo.DeletePlanReturns(
384+
apierrors.NewNotFoundError(errors.New("not found"), repositories.ServicePlanResourceType),
385+
)
386+
})
387+
388+
It("returns 404 Not Found", func() {
389+
expectNotFoundError("Service Plan")
390+
})
391+
})
392+
393+
When("deleting the service plan fails", func() {
394+
BeforeEach(func() {
395+
servicePlanRepo.DeletePlanReturns(errors.New("boom"))
396+
})
397+
398+
It("returns 500 Internal Server Error", func() {
399+
expectUnknownError()
400+
})
401+
})
402+
})
353403
})

‎api/repositories/service_plan_repository.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,26 @@ func (r *ServicePlanRepo) UpdatePlanVisibility(ctx context.Context, authInfo aut
155155
return r.patchServicePlan(ctx, authInfo, message.PlanGUID, message.apply)
156156
}
157157

158+
func (r *ServicePlanRepo) DeletePlan(ctx context.Context, authInfo authorization.Info, planGUID string) error {
159+
userClient, err := r.userClientFactory.BuildClient(authInfo)
160+
if err != nil {
161+
return fmt.Errorf("failed to build user client: %w", err)
162+
}
163+
164+
cfServicePlan := &korifiv1alpha1.CFServicePlan{
165+
ObjectMeta: metav1.ObjectMeta{
166+
Namespace: r.rootNamespace,
167+
Name: planGUID,
168+
},
169+
}
170+
171+
if err := userClient.Delete(ctx, cfServicePlan); err != nil {
172+
return apierrors.FromK8sError(err, ServicePlanResourceType)
173+
}
174+
175+
return nil
176+
}
177+
158178
func (r *ServicePlanRepo) patchServicePlan(
159179
ctx context.Context,
160180
authInfo authorization.Info,

‎api/repositories/service_plan_repository_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package repositories_test
22

33
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
48
apierrors "code.cloudfoundry.org/korifi/api/errors"
59
"code.cloudfoundry.org/korifi/api/repositories"
610
"code.cloudfoundry.org/korifi/api/repositories/fakeawaiter"
@@ -10,8 +14,10 @@ import (
1014
"code.cloudfoundry.org/korifi/tools"
1115
"code.cloudfoundry.org/korifi/tools/k8s"
1216
. "github.com/onsi/gomega/gstruct"
17+
k8serrors "k8s.io/apimachinery/pkg/api/errors"
1318
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1419
"k8s.io/apimachinery/pkg/runtime"
20+
"k8s.io/apimachinery/pkg/types"
1521
"sigs.k8s.io/controller-runtime/pkg/client"
1622

1723
"github.com/google/uuid"
@@ -601,4 +607,35 @@ var _ = Describe("ServicePlanRepo", func() {
601607
})
602608
})
603609
})
610+
611+
Describe("Delete", func() {
612+
var deleteErr error
613+
614+
JustBeforeEach(func() {
615+
createRoleBinding(ctx, userName, adminRole.Name, rootNamespace)
616+
deleteErr = repo.DeletePlan(ctx, authInfo, planGUID)
617+
})
618+
619+
It("deletes the service plan", func() {
620+
Expect(deleteErr).ToNot(HaveOccurred())
621+
622+
namespacedName := types.NamespacedName{
623+
Name: planGUID,
624+
Namespace: rootNamespace,
625+
}
626+
627+
err := k8sClient.Get(context.Background(), namespacedName, &korifiv1alpha1.CFServicePlan{})
628+
Expect(k8serrors.IsNotFound(err)).To(BeTrue(), fmt.Sprintf("error: %+v", err))
629+
})
630+
631+
When("the service plan does not exist", func() {
632+
BeforeEach(func() {
633+
planGUID = "does-not-exist"
634+
})
635+
636+
It("returns a not found error", func() {
637+
Expect(errors.As(deleteErr, &apierrors.NotFoundError{})).To(BeTrue())
638+
})
639+
})
640+
})
604641
})

‎controllers/controllers/services/brokers/controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ func (r *Reconciler) secretToServiceBroker(ctx context.Context, o client.Object)
102102

103103
//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebrokers,verbs=get;list;watch;create;update;patch;delete
104104
//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebrokers/status,verbs=get;update;patch
105-
//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfserviceofferings,verbs=get;list;watch;create;update;patch
106-
//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfserviceplans,verbs=get;list;watch;create;update;patch
105+
//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfserviceofferings,verbs=get;list;watch;create;update;patch;delete
106+
//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfserviceplans,verbs=get;list;watch;create;update;patch;delete
107107

108108
func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBroker *korifiv1alpha1.CFServiceBroker) (ctrl.Result, error) {
109109
log := logr.FromContextOrDiscard(ctx).WithValues("broker-id", cfServiceBroker.Name)

‎helm/korifi/controllers/cf_roles/cf_admin.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ rules:
207207
- list
208208
- get
209209
- patch
210+
- delete
210211

211212
- apiGroups:
212213
- rbac.authorization.k8s.io

‎helm/korifi/controllers/role.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ rules:
110110
- cfservicebindings
111111
- cfservicebrokers
112112
- cfserviceinstances
113+
- cfserviceofferings
114+
- cfserviceplans
113115
- cfspaces
114116
- cftasks
115117
verbs:

‎tests/e2e/service_plans_test.go‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ var _ = Describe("Service Plans", func() {
1717
var (
1818
brokerGUID string
1919
resp *resty.Response
20+
err error
2021
)
2122

2223
BeforeEach(func() {
@@ -31,7 +32,6 @@ var _ = Describe("Service Plans", func() {
3132
var result resourceList[resource]
3233

3334
JustBeforeEach(func() {
34-
var err error
3535
resp, err = adminClient.R().SetResult(&result).Get("/v3/service_plans")
3636
Expect(err).NotTo(HaveOccurred())
3737
})
@@ -54,9 +54,9 @@ var _ = Describe("Service Plans", func() {
5454

5555
BeforeEach(func() {
5656
plans := resourceList[resource]{}
57-
listResp, err := adminClient.R().SetResult(&plans).Get("/v3/service_plans")
57+
resp, err = adminClient.R().SetResult(&plans).Get("/v3/service_plans")
5858
Expect(err).NotTo(HaveOccurred())
59-
Expect(listResp).To(HaveRestyStatusCode(http.StatusOK))
59+
Expect(resp).To(HaveRestyStatusCode(http.StatusOK))
6060

6161
brokerPlans := itx.FromSlice(plans.Resources).Filter(func(r resource) bool {
6262
return r.Metadata.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] == brokerGUID
@@ -68,7 +68,6 @@ var _ = Describe("Service Plans", func() {
6868

6969
Describe("Get Visibility", func() {
7070
JustBeforeEach(func() {
71-
var err error
7271
resp, err = adminClient.R().SetResult(&result).Get(fmt.Sprintf("/v3/service_plans/%s/visibility", planGUID))
7372
Expect(err).NotTo(HaveOccurred())
7473
})
@@ -83,7 +82,6 @@ var _ = Describe("Service Plans", func() {
8382

8483
Describe("Apply Visibility", func() {
8584
JustBeforeEach(func() {
86-
var err error
8785
resp, err = adminClient.R().
8886
SetResult(&result).
8987
SetBody(planVisibilityResource{
@@ -105,7 +103,6 @@ var _ = Describe("Service Plans", func() {
105103

106104
Describe("Update Visibility", func() {
107105
JustBeforeEach(func() {
108-
var err error
109106
resp, err = adminClient.R().
110107
SetResult(&result).
111108
SetBody(planVisibilityResource{
@@ -125,4 +122,26 @@ var _ = Describe("Service Plans", func() {
125122
})
126123
})
127124
})
125+
126+
Describe("Delete", func() {
127+
var servicePlanGUID string
128+
129+
BeforeEach(func() {
130+
plans := resourceList[resource]{}
131+
resp, err = adminClient.R().SetResult(&plans).Get("/v3/service_plans?service_broker_guids=" + brokerGUID)
132+
Expect(err).ToNot(HaveOccurred())
133+
134+
servicePlanGUID = plans.Resources[0].GUID
135+
resp, err = adminClient.R().Delete("/v3/service_plans/" + servicePlanGUID)
136+
})
137+
138+
It("deletes the service plan", func() {
139+
Expect(err).ToNot(HaveOccurred())
140+
Expect(resp).To(HaveRestyStatusCode(http.StatusNoContent))
141+
142+
resp, err = adminClient.R().Get("/v3/service_plans/" + servicePlanGUID)
143+
Expect(err).ToNot(HaveOccurred())
144+
Expect(resp).To(HaveRestyStatusCode(http.StatusNotFound))
145+
})
146+
})
128147
})

0 commit comments

Comments
 (0)