From 88d0158972ecf32a24dffb48dca22ec98d3c70d6 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Tue, 30 Jul 2024 12:24:11 +0000 Subject: [PATCH] Implement `PATCH /v3/service_brokers/:guid` fixes #3269 Co-authored-by: Georgi Sabev --- .../fake/cfservice_broker_repository.go | 143 +++++++++--- .../fake/cfservice_offering_repository.go | 20 +- .../fake/cfservice_plan_repository.go | 20 +- api/handlers/job.go | 5 +- api/handlers/job_test.go | 10 +- api/handlers/service_broker.go | 40 +++- api/handlers/service_broker_test.go | 128 ++++++++++- api/handlers/service_offering.go | 2 +- api/handlers/service_offering_test.go | 2 +- api/handlers/service_plan.go | 2 +- api/handlers/service_plan_test.go | 2 +- api/main.go | 1 + api/payloads/service_broker.go | 39 +++- api/payloads/service_broker_test.go | 99 ++++++++- api/presenter/job.go | 1 + api/presenter/service_broker.go | 10 +- api/presenter/service_broker_test.go | 8 +- api/presenter/service_offering.go | 12 +- api/presenter/service_offering_test.go | 8 +- api/presenter/service_plan.go | 16 +- api/presenter/service_plan_test.go | 8 +- api/repositories/patch.go | 27 +++ api/repositories/service_broker_repository.go | 106 +++++++-- .../service_broker_repository_test.go | 208 +++++++++++++++--- .../service_offering_repository.go | 16 +- .../service_offering_repository_test.go | 2 +- api/repositories/service_plan_repository.go | 10 +- .../service_plan_repository_test.go | 2 +- .../korifi/controllers/cf_roles/cf_admin.yaml | 1 + model/cfresource.go | 20 +- tests/assets/sample-broker-golang/main.go | 53 ++++- tests/e2e/droplets_test.go | 2 +- tests/e2e/e2e_suite_test.go | 23 +- tests/e2e/service_brokers_test.go | 58 +++++ 34 files changed, 895 insertions(+), 209 deletions(-) create mode 100644 api/repositories/patch.go diff --git a/api/handlers/fake/cfservice_broker_repository.go b/api/handlers/fake/cfservice_broker_repository.go index 7c7871819..05fb2abcb 100644 --- a/api/handlers/fake/cfservice_broker_repository.go +++ b/api/handlers/fake/cfservice_broker_repository.go @@ -11,7 +11,7 @@ import ( ) type CFServiceBrokerRepository struct { - CreateServiceBrokerStub func(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error) + CreateServiceBrokerStub func(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerRecord, error) createServiceBrokerMutex sync.RWMutex createServiceBrokerArgsForCall []struct { arg1 context.Context @@ -19,11 +19,11 @@ type CFServiceBrokerRepository struct { arg3 repositories.CreateServiceBrokerMessage } createServiceBrokerReturns struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error } createServiceBrokerReturnsOnCall map[int]struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error } DeleteServiceBrokerStub func(context.Context, authorization.Info, string) error @@ -39,7 +39,7 @@ type CFServiceBrokerRepository struct { deleteServiceBrokerReturnsOnCall map[int]struct { result1 error } - GetServiceBrokerStub func(context.Context, authorization.Info, string) (repositories.ServiceBrokerResource, error) + GetServiceBrokerStub func(context.Context, authorization.Info, string) (repositories.ServiceBrokerRecord, error) getServiceBrokerMutex sync.RWMutex getServiceBrokerArgsForCall []struct { arg1 context.Context @@ -47,14 +47,14 @@ type CFServiceBrokerRepository struct { arg3 string } getServiceBrokerReturns struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error } getServiceBrokerReturnsOnCall map[int]struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error } - ListServiceBrokersStub func(context.Context, authorization.Info, repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerResource, error) + ListServiceBrokersStub func(context.Context, authorization.Info, repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerRecord, error) listServiceBrokersMutex sync.RWMutex listServiceBrokersArgsForCall []struct { arg1 context.Context @@ -62,18 +62,33 @@ type CFServiceBrokerRepository struct { arg3 repositories.ListServiceBrokerMessage } listServiceBrokersReturns struct { - result1 []repositories.ServiceBrokerResource + result1 []repositories.ServiceBrokerRecord result2 error } listServiceBrokersReturnsOnCall map[int]struct { - result1 []repositories.ServiceBrokerResource + result1 []repositories.ServiceBrokerRecord + result2 error + } + UpdateServiceBrokerStub func(context.Context, authorization.Info, repositories.UpdateServiceBrokerMessage) (repositories.ServiceBrokerRecord, error) + updateServiceBrokerMutex sync.RWMutex + updateServiceBrokerArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.UpdateServiceBrokerMessage + } + updateServiceBrokerReturns struct { + result1 repositories.ServiceBrokerRecord + result2 error + } + updateServiceBrokerReturnsOnCall map[int]struct { + result1 repositories.ServiceBrokerRecord result2 error } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *CFServiceBrokerRepository) CreateServiceBroker(arg1 context.Context, arg2 authorization.Info, arg3 repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error) { +func (fake *CFServiceBrokerRepository) CreateServiceBroker(arg1 context.Context, arg2 authorization.Info, arg3 repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerRecord, error) { fake.createServiceBrokerMutex.Lock() ret, specificReturn := fake.createServiceBrokerReturnsOnCall[len(fake.createServiceBrokerArgsForCall)] fake.createServiceBrokerArgsForCall = append(fake.createServiceBrokerArgsForCall, struct { @@ -100,7 +115,7 @@ func (fake *CFServiceBrokerRepository) CreateServiceBrokerCallCount() int { return len(fake.createServiceBrokerArgsForCall) } -func (fake *CFServiceBrokerRepository) CreateServiceBrokerCalls(stub func(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error)) { +func (fake *CFServiceBrokerRepository) CreateServiceBrokerCalls(stub func(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerRecord, error)) { fake.createServiceBrokerMutex.Lock() defer fake.createServiceBrokerMutex.Unlock() fake.CreateServiceBrokerStub = stub @@ -113,28 +128,28 @@ func (fake *CFServiceBrokerRepository) CreateServiceBrokerArgsForCall(i int) (co return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *CFServiceBrokerRepository) CreateServiceBrokerReturns(result1 repositories.ServiceBrokerResource, result2 error) { +func (fake *CFServiceBrokerRepository) CreateServiceBrokerReturns(result1 repositories.ServiceBrokerRecord, result2 error) { fake.createServiceBrokerMutex.Lock() defer fake.createServiceBrokerMutex.Unlock() fake.CreateServiceBrokerStub = nil fake.createServiceBrokerReturns = struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error }{result1, result2} } -func (fake *CFServiceBrokerRepository) CreateServiceBrokerReturnsOnCall(i int, result1 repositories.ServiceBrokerResource, result2 error) { +func (fake *CFServiceBrokerRepository) CreateServiceBrokerReturnsOnCall(i int, result1 repositories.ServiceBrokerRecord, result2 error) { fake.createServiceBrokerMutex.Lock() defer fake.createServiceBrokerMutex.Unlock() fake.CreateServiceBrokerStub = nil if fake.createServiceBrokerReturnsOnCall == nil { fake.createServiceBrokerReturnsOnCall = make(map[int]struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error }) } fake.createServiceBrokerReturnsOnCall[i] = struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error }{result1, result2} } @@ -202,7 +217,7 @@ func (fake *CFServiceBrokerRepository) DeleteServiceBrokerReturnsOnCall(i int, r }{result1} } -func (fake *CFServiceBrokerRepository) GetServiceBroker(arg1 context.Context, arg2 authorization.Info, arg3 string) (repositories.ServiceBrokerResource, error) { +func (fake *CFServiceBrokerRepository) GetServiceBroker(arg1 context.Context, arg2 authorization.Info, arg3 string) (repositories.ServiceBrokerRecord, error) { fake.getServiceBrokerMutex.Lock() ret, specificReturn := fake.getServiceBrokerReturnsOnCall[len(fake.getServiceBrokerArgsForCall)] fake.getServiceBrokerArgsForCall = append(fake.getServiceBrokerArgsForCall, struct { @@ -229,7 +244,7 @@ func (fake *CFServiceBrokerRepository) GetServiceBrokerCallCount() int { return len(fake.getServiceBrokerArgsForCall) } -func (fake *CFServiceBrokerRepository) GetServiceBrokerCalls(stub func(context.Context, authorization.Info, string) (repositories.ServiceBrokerResource, error)) { +func (fake *CFServiceBrokerRepository) GetServiceBrokerCalls(stub func(context.Context, authorization.Info, string) (repositories.ServiceBrokerRecord, error)) { fake.getServiceBrokerMutex.Lock() defer fake.getServiceBrokerMutex.Unlock() fake.GetServiceBrokerStub = stub @@ -242,33 +257,33 @@ func (fake *CFServiceBrokerRepository) GetServiceBrokerArgsForCall(i int) (conte return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *CFServiceBrokerRepository) GetServiceBrokerReturns(result1 repositories.ServiceBrokerResource, result2 error) { +func (fake *CFServiceBrokerRepository) GetServiceBrokerReturns(result1 repositories.ServiceBrokerRecord, result2 error) { fake.getServiceBrokerMutex.Lock() defer fake.getServiceBrokerMutex.Unlock() fake.GetServiceBrokerStub = nil fake.getServiceBrokerReturns = struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error }{result1, result2} } -func (fake *CFServiceBrokerRepository) GetServiceBrokerReturnsOnCall(i int, result1 repositories.ServiceBrokerResource, result2 error) { +func (fake *CFServiceBrokerRepository) GetServiceBrokerReturnsOnCall(i int, result1 repositories.ServiceBrokerRecord, 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 + result1 repositories.ServiceBrokerRecord result2 error }) } fake.getServiceBrokerReturnsOnCall[i] = struct { - result1 repositories.ServiceBrokerResource + result1 repositories.ServiceBrokerRecord result2 error }{result1, result2} } -func (fake *CFServiceBrokerRepository) ListServiceBrokers(arg1 context.Context, arg2 authorization.Info, arg3 repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerResource, error) { +func (fake *CFServiceBrokerRepository) ListServiceBrokers(arg1 context.Context, arg2 authorization.Info, arg3 repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerRecord, error) { fake.listServiceBrokersMutex.Lock() ret, specificReturn := fake.listServiceBrokersReturnsOnCall[len(fake.listServiceBrokersArgsForCall)] fake.listServiceBrokersArgsForCall = append(fake.listServiceBrokersArgsForCall, struct { @@ -295,7 +310,7 @@ func (fake *CFServiceBrokerRepository) ListServiceBrokersCallCount() int { return len(fake.listServiceBrokersArgsForCall) } -func (fake *CFServiceBrokerRepository) ListServiceBrokersCalls(stub func(context.Context, authorization.Info, repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerResource, error)) { +func (fake *CFServiceBrokerRepository) ListServiceBrokersCalls(stub func(context.Context, authorization.Info, repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerRecord, error)) { fake.listServiceBrokersMutex.Lock() defer fake.listServiceBrokersMutex.Unlock() fake.ListServiceBrokersStub = stub @@ -308,28 +323,94 @@ func (fake *CFServiceBrokerRepository) ListServiceBrokersArgsForCall(i int) (con return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *CFServiceBrokerRepository) ListServiceBrokersReturns(result1 []repositories.ServiceBrokerResource, result2 error) { +func (fake *CFServiceBrokerRepository) ListServiceBrokersReturns(result1 []repositories.ServiceBrokerRecord, result2 error) { fake.listServiceBrokersMutex.Lock() defer fake.listServiceBrokersMutex.Unlock() fake.ListServiceBrokersStub = nil fake.listServiceBrokersReturns = struct { - result1 []repositories.ServiceBrokerResource + result1 []repositories.ServiceBrokerRecord result2 error }{result1, result2} } -func (fake *CFServiceBrokerRepository) ListServiceBrokersReturnsOnCall(i int, result1 []repositories.ServiceBrokerResource, result2 error) { +func (fake *CFServiceBrokerRepository) ListServiceBrokersReturnsOnCall(i int, result1 []repositories.ServiceBrokerRecord, result2 error) { fake.listServiceBrokersMutex.Lock() defer fake.listServiceBrokersMutex.Unlock() fake.ListServiceBrokersStub = nil if fake.listServiceBrokersReturnsOnCall == nil { fake.listServiceBrokersReturnsOnCall = make(map[int]struct { - result1 []repositories.ServiceBrokerResource + result1 []repositories.ServiceBrokerRecord result2 error }) } fake.listServiceBrokersReturnsOnCall[i] = struct { - result1 []repositories.ServiceBrokerResource + result1 []repositories.ServiceBrokerRecord + result2 error + }{result1, result2} +} + +func (fake *CFServiceBrokerRepository) UpdateServiceBroker(arg1 context.Context, arg2 authorization.Info, arg3 repositories.UpdateServiceBrokerMessage) (repositories.ServiceBrokerRecord, error) { + fake.updateServiceBrokerMutex.Lock() + ret, specificReturn := fake.updateServiceBrokerReturnsOnCall[len(fake.updateServiceBrokerArgsForCall)] + fake.updateServiceBrokerArgsForCall = append(fake.updateServiceBrokerArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.UpdateServiceBrokerMessage + }{arg1, arg2, arg3}) + stub := fake.UpdateServiceBrokerStub + fakeReturns := fake.updateServiceBrokerReturns + fake.recordInvocation("UpdateServiceBroker", []interface{}{arg1, arg2, arg3}) + fake.updateServiceBrokerMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *CFServiceBrokerRepository) UpdateServiceBrokerCallCount() int { + fake.updateServiceBrokerMutex.RLock() + defer fake.updateServiceBrokerMutex.RUnlock() + return len(fake.updateServiceBrokerArgsForCall) +} + +func (fake *CFServiceBrokerRepository) UpdateServiceBrokerCalls(stub func(context.Context, authorization.Info, repositories.UpdateServiceBrokerMessage) (repositories.ServiceBrokerRecord, error)) { + fake.updateServiceBrokerMutex.Lock() + defer fake.updateServiceBrokerMutex.Unlock() + fake.UpdateServiceBrokerStub = stub +} + +func (fake *CFServiceBrokerRepository) UpdateServiceBrokerArgsForCall(i int) (context.Context, authorization.Info, repositories.UpdateServiceBrokerMessage) { + fake.updateServiceBrokerMutex.RLock() + defer fake.updateServiceBrokerMutex.RUnlock() + argsForCall := fake.updateServiceBrokerArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServiceBrokerRepository) UpdateServiceBrokerReturns(result1 repositories.ServiceBrokerRecord, result2 error) { + fake.updateServiceBrokerMutex.Lock() + defer fake.updateServiceBrokerMutex.Unlock() + fake.UpdateServiceBrokerStub = nil + fake.updateServiceBrokerReturns = struct { + result1 repositories.ServiceBrokerRecord + result2 error + }{result1, result2} +} + +func (fake *CFServiceBrokerRepository) UpdateServiceBrokerReturnsOnCall(i int, result1 repositories.ServiceBrokerRecord, result2 error) { + fake.updateServiceBrokerMutex.Lock() + defer fake.updateServiceBrokerMutex.Unlock() + fake.UpdateServiceBrokerStub = nil + if fake.updateServiceBrokerReturnsOnCall == nil { + fake.updateServiceBrokerReturnsOnCall = make(map[int]struct { + result1 repositories.ServiceBrokerRecord + result2 error + }) + } + fake.updateServiceBrokerReturnsOnCall[i] = struct { + result1 repositories.ServiceBrokerRecord result2 error }{result1, result2} } @@ -345,6 +426,8 @@ func (fake *CFServiceBrokerRepository) Invocations() map[string][][]interface{} defer fake.getServiceBrokerMutex.RUnlock() fake.listServiceBrokersMutex.RLock() defer fake.listServiceBrokersMutex.RUnlock() + fake.updateServiceBrokerMutex.RLock() + defer fake.updateServiceBrokerMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/api/handlers/fake/cfservice_offering_repository.go b/api/handlers/fake/cfservice_offering_repository.go index 2bd2a99a4..0105af293 100644 --- a/api/handlers/fake/cfservice_offering_repository.go +++ b/api/handlers/fake/cfservice_offering_repository.go @@ -11,25 +11,25 @@ import ( ) type CFServiceOfferingRepository struct { - ListOfferingsStub func(context.Context, authorization.Info) ([]repositories.ServiceOfferingResource, error) + ListOfferingsStub func(context.Context, authorization.Info) ([]repositories.ServiceOfferingRecord, error) listOfferingsMutex sync.RWMutex listOfferingsArgsForCall []struct { arg1 context.Context arg2 authorization.Info } listOfferingsReturns struct { - result1 []repositories.ServiceOfferingResource + result1 []repositories.ServiceOfferingRecord result2 error } listOfferingsReturnsOnCall map[int]struct { - result1 []repositories.ServiceOfferingResource + result1 []repositories.ServiceOfferingRecord result2 error } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *CFServiceOfferingRepository) ListOfferings(arg1 context.Context, arg2 authorization.Info) ([]repositories.ServiceOfferingResource, error) { +func (fake *CFServiceOfferingRepository) ListOfferings(arg1 context.Context, arg2 authorization.Info) ([]repositories.ServiceOfferingRecord, error) { fake.listOfferingsMutex.Lock() ret, specificReturn := fake.listOfferingsReturnsOnCall[len(fake.listOfferingsArgsForCall)] fake.listOfferingsArgsForCall = append(fake.listOfferingsArgsForCall, struct { @@ -55,7 +55,7 @@ func (fake *CFServiceOfferingRepository) ListOfferingsCallCount() int { return len(fake.listOfferingsArgsForCall) } -func (fake *CFServiceOfferingRepository) ListOfferingsCalls(stub func(context.Context, authorization.Info) ([]repositories.ServiceOfferingResource, error)) { +func (fake *CFServiceOfferingRepository) ListOfferingsCalls(stub func(context.Context, authorization.Info) ([]repositories.ServiceOfferingRecord, error)) { fake.listOfferingsMutex.Lock() defer fake.listOfferingsMutex.Unlock() fake.ListOfferingsStub = stub @@ -68,28 +68,28 @@ func (fake *CFServiceOfferingRepository) ListOfferingsArgsForCall(i int) (contex return argsForCall.arg1, argsForCall.arg2 } -func (fake *CFServiceOfferingRepository) ListOfferingsReturns(result1 []repositories.ServiceOfferingResource, result2 error) { +func (fake *CFServiceOfferingRepository) ListOfferingsReturns(result1 []repositories.ServiceOfferingRecord, result2 error) { fake.listOfferingsMutex.Lock() defer fake.listOfferingsMutex.Unlock() fake.ListOfferingsStub = nil fake.listOfferingsReturns = struct { - result1 []repositories.ServiceOfferingResource + result1 []repositories.ServiceOfferingRecord result2 error }{result1, result2} } -func (fake *CFServiceOfferingRepository) ListOfferingsReturnsOnCall(i int, result1 []repositories.ServiceOfferingResource, result2 error) { +func (fake *CFServiceOfferingRepository) ListOfferingsReturnsOnCall(i int, result1 []repositories.ServiceOfferingRecord, result2 error) { fake.listOfferingsMutex.Lock() defer fake.listOfferingsMutex.Unlock() fake.ListOfferingsStub = nil if fake.listOfferingsReturnsOnCall == nil { fake.listOfferingsReturnsOnCall = make(map[int]struct { - result1 []repositories.ServiceOfferingResource + result1 []repositories.ServiceOfferingRecord result2 error }) } fake.listOfferingsReturnsOnCall[i] = struct { - result1 []repositories.ServiceOfferingResource + result1 []repositories.ServiceOfferingRecord result2 error }{result1, result2} } diff --git a/api/handlers/fake/cfservice_plan_repository.go b/api/handlers/fake/cfservice_plan_repository.go index 246a4a562..c73e5d1cb 100644 --- a/api/handlers/fake/cfservice_plan_repository.go +++ b/api/handlers/fake/cfservice_plan_repository.go @@ -11,25 +11,25 @@ import ( ) type CFServicePlanRepository struct { - ListPlansStub func(context.Context, authorization.Info) ([]repositories.ServicePlanResource, error) + ListPlansStub func(context.Context, authorization.Info) ([]repositories.ServicePlanRecord, error) listPlansMutex sync.RWMutex listPlansArgsForCall []struct { arg1 context.Context arg2 authorization.Info } listPlansReturns struct { - result1 []repositories.ServicePlanResource + result1 []repositories.ServicePlanRecord result2 error } listPlansReturnsOnCall map[int]struct { - result1 []repositories.ServicePlanResource + result1 []repositories.ServicePlanRecord result2 error } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *CFServicePlanRepository) ListPlans(arg1 context.Context, arg2 authorization.Info) ([]repositories.ServicePlanResource, error) { +func (fake *CFServicePlanRepository) ListPlans(arg1 context.Context, arg2 authorization.Info) ([]repositories.ServicePlanRecord, error) { fake.listPlansMutex.Lock() ret, specificReturn := fake.listPlansReturnsOnCall[len(fake.listPlansArgsForCall)] fake.listPlansArgsForCall = append(fake.listPlansArgsForCall, struct { @@ -55,7 +55,7 @@ func (fake *CFServicePlanRepository) ListPlansCallCount() int { return len(fake.listPlansArgsForCall) } -func (fake *CFServicePlanRepository) ListPlansCalls(stub func(context.Context, authorization.Info) ([]repositories.ServicePlanResource, error)) { +func (fake *CFServicePlanRepository) ListPlansCalls(stub func(context.Context, authorization.Info) ([]repositories.ServicePlanRecord, error)) { fake.listPlansMutex.Lock() defer fake.listPlansMutex.Unlock() fake.ListPlansStub = stub @@ -68,28 +68,28 @@ func (fake *CFServicePlanRepository) ListPlansArgsForCall(i int) (context.Contex return argsForCall.arg1, argsForCall.arg2 } -func (fake *CFServicePlanRepository) ListPlansReturns(result1 []repositories.ServicePlanResource, result2 error) { +func (fake *CFServicePlanRepository) ListPlansReturns(result1 []repositories.ServicePlanRecord, result2 error) { fake.listPlansMutex.Lock() defer fake.listPlansMutex.Unlock() fake.ListPlansStub = nil fake.listPlansReturns = struct { - result1 []repositories.ServicePlanResource + result1 []repositories.ServicePlanRecord result2 error }{result1, result2} } -func (fake *CFServicePlanRepository) ListPlansReturnsOnCall(i int, result1 []repositories.ServicePlanResource, result2 error) { +func (fake *CFServicePlanRepository) ListPlansReturnsOnCall(i int, result1 []repositories.ServicePlanRecord, result2 error) { fake.listPlansMutex.Lock() defer fake.listPlansMutex.Unlock() fake.ListPlansStub = nil if fake.listPlansReturnsOnCall == nil { fake.listPlansReturnsOnCall = make(map[int]struct { - result1 []repositories.ServicePlanResource + result1 []repositories.ServicePlanRecord result2 error }) } fake.listPlansReturnsOnCall[i] = struct { - result1 []repositories.ServicePlanResource + result1 []repositories.ServicePlanRecord result2 error }{result1, result2} } diff --git a/api/handlers/job.go b/api/handlers/job.go index 2ea5f3fbb..517f6fb19 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" + ServiceBrokerUpdateJobType = "service_broker.update" ServiceBrokerDeleteJobType = "service_broker.delete" JobTimeoutDuration = 120.0 @@ -182,8 +183,8 @@ func (h *Job) handleStateJob(ctx context.Context, repository StateRepository, jo ) } - switch state.Status { - case model.CFResourceStatusReady: + switch state { + case model.CFResourceStateReady: return presenter.ForJob(job, []presenter.JobResponseError{}, presenter.StateComplete, diff --git a/api/handlers/job_test.go b/api/handlers/job_test.go index 0509c4c66..81e46efc1 100644 --- a/api/handlers/job_test.go +++ b/api/handlers/job_test.go @@ -155,7 +155,7 @@ var _ = Describe("Job", func() { BeforeEach(func() { stateRepo = new(fake.StateRepository) - stateRepo.GetStateReturns(model.CFResourceState{}, nil) + stateRepo.GetStateReturns(model.CFResourceStateUnknown, nil) stateRepos["testing.state"] = stateRepo jobGUID = "testing.state~my-resource-guid" @@ -179,9 +179,7 @@ var _ = Describe("Job", func() { When("the resource state is Ready", func() { BeforeEach(func() { - stateRepo.GetStateReturns(model.CFResourceState{ - Status: model.CFResourceStatusReady, - }, nil) + stateRepo.GetStateReturns(model.CFResourceStateReady, nil) }) It("returns a complete status", func() { @@ -194,7 +192,7 @@ var _ = Describe("Job", func() { When("the user does not have permission to see the resource", func() { BeforeEach(func() { - stateRepo.GetStateReturns(model.CFResourceState{}, fmt.Errorf("wrapped err: %w", apierrors.NewForbiddenError(nil, "foo"))) + stateRepo.GetStateReturns(model.CFResourceStateUnknown, fmt.Errorf("wrapped err: %w", apierrors.NewForbiddenError(nil, "foo"))) }) It("returns a complete status", func() { @@ -207,7 +205,7 @@ var _ = Describe("Job", func() { When("getting the state fails", func() { BeforeEach(func() { - stateRepo.GetStateReturns(model.CFResourceState{}, errors.New("get-state-error")) + stateRepo.GetStateReturns(model.CFResourceStateUnknown, errors.New("get-state-error")) }) It("returns an error", func() { diff --git a/api/handlers/service_broker.go b/api/handlers/service_broker.go index cf4bf6acd..a124520f7 100644 --- a/api/handlers/service_broker.go +++ b/api/handlers/service_broker.go @@ -16,15 +16,16 @@ import ( const ( ServiceBrokersPath = "/v3/service_brokers" - ServiceBrokerPath = ServiceBrokersPath + "/{guid}" + ServiceBrokerPath = "/v3/service_brokers/{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) + CreateServiceBroker(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerRecord, error) + ListServiceBrokers(context.Context, authorization.Info, repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerRecord, error) + GetServiceBroker(context.Context, authorization.Info, string) (repositories.ServiceBrokerRecord, error) DeleteServiceBroker(context.Context, authorization.Info, string) error + UpdateServiceBroker(context.Context, authorization.Info, repositories.UpdateServiceBrokerMessage) (repositories.ServiceBrokerRecord, error) } type ServiceBroker struct { @@ -54,7 +55,7 @@ func (h *ServiceBroker) create(r *http.Request) (*routing.Response, error) { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } - broker, err := h.serviceBrokerRepo.CreateServiceBroker(r.Context(), authInfo, payload.ToCreateServiceBrokerMessage()) + broker, err := h.serviceBrokerRepo.CreateServiceBroker(r.Context(), authInfo, payload.ToMessage()) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to create service broker") } @@ -99,6 +100,34 @@ func (h *ServiceBroker) delete(r *http.Request) (*routing.Response, error) { WithHeader("Location", presenter.JobURLForRedirects(guid, presenter.ServiceBrokerDeleteOperation, h.serverURL)), nil } +func (h *ServiceBroker) update(r *http.Request) (*routing.Response, error) { + authInfo, _ := authorization.InfoFromContext(r.Context()) + logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-broker.update") + + guid := routing.URLParam(r, "guid") + payload := payloads.ServiceBrokerUpdate{} + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") + } + + _, err := h.serviceBrokerRepo.GetServiceBroker(r.Context(), authInfo, guid) + if err != nil { + return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "failed to get service broker") + } + + broker, err := h.serviceBrokerRepo.UpdateServiceBroker(r.Context(), authInfo, payload.ToMessage(guid)) + if err != nil { + return nil, apierrors.LogAndReturn(logger, err, "error when deleting service broker", "guid", guid) + } + + if payload.IsAsyncRequest() { + return routing.NewResponse(http.StatusAccepted). + WithHeader("Location", presenter.JobURLForRedirects(broker.GUID, presenter.ServiceBrokerUpdateOperation, h.serverURL)), nil + } + + return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServiceBroker(broker, h.serverURL)), nil +} + func (h *ServiceBroker) UnauthenticatedRoutes() []routing.Route { return nil } @@ -108,5 +137,6 @@ func (h *ServiceBroker) AuthenticatedRoutes() []routing.Route { {Method: "POST", Pattern: ServiceBrokersPath, Handler: h.create}, {Method: "GET", Pattern: ServiceBrokersPath, Handler: h.list}, {Method: "DELETE", Pattern: ServiceBrokerPath, Handler: h.delete}, + {Method: "PATCH", Pattern: ServiceBrokerPath, Handler: h.update}, } } diff --git a/api/handlers/service_broker_test.go b/api/handlers/service_broker_test.go index dff4fa587..86ac2f08d 100644 --- a/api/handlers/service_broker_test.go +++ b/api/handlers/service_broker_test.go @@ -13,6 +13,7 @@ import ( "code.cloudfoundry.org/korifi/model" "code.cloudfoundry.org/korifi/model/services" . "code.cloudfoundry.org/korifi/tests/matchers" + "code.cloudfoundry.org/korifi/tools" . "github.com/onsi/gomega/gstruct" . "github.com/onsi/ginkgo/v2" @@ -55,7 +56,7 @@ var _ = Describe("ServiceBroker", func() { } requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payload) - serviceBrokerRepo.CreateServiceBrokerReturns(repositories.ServiceBrokerResource{ + serviceBrokerRepo.CreateServiceBrokerReturns(repositories.ServiceBrokerRecord{ CFResource: model.CFResource{ GUID: "service-broker-guid", }, @@ -91,7 +92,7 @@ var _ = Describe("ServiceBroker", func() { When("creating the service broker fails", func() { BeforeEach(func() { - serviceBrokerRepo.CreateServiceBrokerReturns(repositories.ServiceBrokerResource{}, errors.New("create-service-broker-error")) + serviceBrokerRepo.CreateServiceBrokerReturns(repositories.ServiceBrokerRecord{}, errors.New("create-service-broker-error")) }) It("returns an error", func() { @@ -102,7 +103,7 @@ var _ = Describe("ServiceBroker", func() { Describe("GET /v3/service_brokers", func() { BeforeEach(func() { - serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerResource{ + serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerRecord{ { CFResource: model.CFResource{ GUID: "broker-guid", @@ -171,7 +172,7 @@ var _ = Describe("ServiceBroker", func() { Describe("DELETE /v3/service_brokers/guid", func() { BeforeEach(func() { - serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerResource{ + serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerRecord{ CFResource: model.CFResource{ GUID: "broker-guid", }, @@ -199,7 +200,7 @@ var _ = Describe("ServiceBroker", func() { When("getting the service broker is not allowed", func() { BeforeEach(func() { - serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerResource{}, apierrors.NewForbiddenError(nil, repositories.ServiceBrokerResourceType)) + serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerRecord{}, apierrors.NewForbiddenError(nil, repositories.ServiceBrokerResourceType)) }) It("returns a not found error", func() { @@ -209,7 +210,7 @@ var _ = Describe("ServiceBroker", func() { When("getting the service broker fails", func() { BeforeEach(func() { - serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerResource{}, errors.New("get-broker-err")) + serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerRecord{}, errors.New("get-broker-err")) }) It("returns an error", func() { @@ -227,4 +228,119 @@ var _ = Describe("ServiceBroker", func() { }) }) }) + + Describe("PATCH /v3/service_brokers", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServiceBrokerUpdate{ + Name: tools.PtrTo("new-name"), + }) + + serviceBrokerRepo.UpdateServiceBrokerReturns(repositories.ServiceBrokerRecord{ + CFResource: model.CFResource{ + GUID: "service-broker-guid", + }, + }, nil) + + var err error + req, err = http.NewRequestWithContext(ctx, "PATCH", "/v3/service_brokers/service-broker-guid", strings.NewReader("the-json-body")) + Expect(err).NotTo(HaveOccurred()) + }) + + It("updates the service broker", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + Expect(bodyString(actualReq)).To(Equal("the-json-body")) + + Expect(serviceBrokerRepo.UpdateServiceBrokerCallCount()).To(Equal(1)) + _, actualAuthInfo, actualUpdateMesage := serviceBrokerRepo.UpdateServiceBrokerArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualUpdateMesage).To(Equal(repositories.UpdateServiceBrokerMessage{ + GUID: "service-broker-guid", + Name: tools.PtrTo("new-name"), + })) + + Expect(rr).To(HaveHTTPStatus(http.StatusAccepted)) + Expect(rr).To(HaveHTTPHeaderWithValue("Location", "https://api.example.org/v3/jobs/service_broker.update~service-broker-guid")) + }) + + When("only metadata is updated", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServiceBrokerUpdate{ + Metadata: payloads.MetadataPatch{ + Labels: map[string]*string{ + "foo": tools.PtrTo("bar"), + }, + }, + }) + serviceBrokerRepo.UpdateServiceBrokerReturns(repositories.ServiceBrokerRecord{ + CFResource: model.CFResource{ + GUID: "service-broker-guid", + Metadata: model.Metadata{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, nil) + }) + + It("returns OK response", func() { + Expect(rr).To(HaveHTTPStatus(http.StatusOK)) + Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.guid", "service-broker-guid"), + MatchJSONPath("$.metadata.labels.foo", "bar"), + MatchJSONPath("$.links.self.href", "https://api.example.org/v3/service_brokers/service-broker-guid"), + ))) + }) + }) + + When("the user doesn't have permission to get the broker", func() { + BeforeEach(func() { + serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerRecord{}, apierrors.NewForbiddenError(nil, repositories.ServiceBrokerResourceType)) + }) + + It("returns a not found error", func() { + expectNotFoundError(repositories.ServiceBrokerResourceType) + }) + + It("does not call update", func() { + Expect(serviceBrokerRepo.UpdateServiceBrokerCallCount()).To(Equal(0)) + }) + }) + + When("fetching the broker errors", func() { + BeforeEach(func() { + serviceBrokerRepo.GetServiceBrokerReturns(repositories.ServiceBrokerRecord{}, errors.New("get-broker-err")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + + It("does not call update", func() { + Expect(serviceBrokerRepo.UpdateServiceBrokerCallCount()).To(Equal(0)) + }) + }) + + When("updating the broker fails", func() { + BeforeEach(func() { + serviceBrokerRepo.UpdateServiceBrokerReturns(repositories.ServiceBrokerRecord{}, errors.New("update-err")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + + When("the request body is invalid", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadReturns(apierrors.NewUnprocessableEntityError(errors.New("validation-err"), "validation error")) + }) + + It("returns an unprocessable entity error", func() { + expectUnprocessableEntityError("validation error") + }) + }) + }) }) diff --git a/api/handlers/service_offering.go b/api/handlers/service_offering.go index 6a497d068..e3f942e0a 100644 --- a/api/handlers/service_offering.go +++ b/api/handlers/service_offering.go @@ -20,7 +20,7 @@ const ( //counterfeiter:generate -o fake -fake-name CFServiceOfferingRepository . CFServiceOfferingRepository type CFServiceOfferingRepository interface { - ListOfferings(context.Context, authorization.Info) ([]repositories.ServiceOfferingResource, error) + ListOfferings(context.Context, authorization.Info) ([]repositories.ServiceOfferingRecord, error) } type ServiceOffering struct { diff --git a/api/handlers/service_offering_test.go b/api/handlers/service_offering_test.go index 64c8a7049..e1a82bfe1 100644 --- a/api/handlers/service_offering_test.go +++ b/api/handlers/service_offering_test.go @@ -30,7 +30,7 @@ var _ = Describe("ServiceOffering", func() { Describe("GET /v3/service_offerings", func() { BeforeEach(func() { - serviceOfferingRepo.ListOfferingsReturns([]repositories.ServiceOfferingResource{{ + serviceOfferingRepo.ListOfferingsReturns([]repositories.ServiceOfferingRecord{{ ServiceOffering: services.ServiceOffering{}, CFResource: model.CFResource{ GUID: "offering-guid", diff --git a/api/handlers/service_plan.go b/api/handlers/service_plan.go index 05dc3caf4..df017af92 100644 --- a/api/handlers/service_plan.go +++ b/api/handlers/service_plan.go @@ -20,7 +20,7 @@ const ( //counterfeiter:generate -o fake -fake-name CFServicePlanRepository . CFServicePlanRepository type CFServicePlanRepository interface { - ListPlans(context.Context, authorization.Info) ([]repositories.ServicePlanResource, error) + ListPlans(context.Context, authorization.Info) ([]repositories.ServicePlanRecord, error) } type ServicePlan struct { diff --git a/api/handlers/service_plan_test.go b/api/handlers/service_plan_test.go index c09ed6acb..181bd0aa4 100644 --- a/api/handlers/service_plan_test.go +++ b/api/handlers/service_plan_test.go @@ -29,7 +29,7 @@ var _ = Describe("ServicePlan", func() { Describe("GET /v3/service_plans", func() { BeforeEach(func() { - servicePlanRepo.ListPlansReturns([]repositories.ServicePlanResource{{ + servicePlanRepo.ListPlansReturns([]repositories.ServicePlanRecord{{ CFResource: model.CFResource{ GUID: "plan-guid", }, diff --git a/api/main.go b/api/main.go index 523aa54d0..652416b55 100644 --- a/api/main.go +++ b/api/main.go @@ -353,6 +353,7 @@ func main() { }, map[string]handlers.StateRepository{ handlers.ServiceBrokerCreateJobType: serviceBrokerRepo, + handlers.ServiceBrokerUpdateJobType: serviceBrokerRepo, }, 500*time.Millisecond, ), diff --git a/api/payloads/service_broker.go b/api/payloads/service_broker.go index 2a3af7834..3c17ccd3b 100644 --- a/api/payloads/service_broker.go +++ b/api/payloads/service_broker.go @@ -36,7 +36,7 @@ func (c ServiceBrokerCreate) Validate() error { ) } -func (c ServiceBrokerCreate) ToCreateServiceBrokerMessage() repositories.CreateServiceBrokerMessage { +func (c ServiceBrokerCreate) ToMessage() repositories.CreateServiceBrokerMessage { return repositories.CreateServiceBrokerMessage{ Broker: c.ServiceBroker, Metadata: c.Metadata, @@ -62,3 +62,40 @@ func (b *ServiceBrokerList) ToMessage() repositories.ListServiceBrokerMessage { Names: parse.ArrayParam(b.Names), } } + +type ServiceBrokerUpdate struct { + Name *string `json:"name"` + URL *string `json:"url"` + Authentication *BrokerAuthentication `json:"authentication"` + Metadata MetadataPatch `json:"metadata"` +} + +func (c ServiceBrokerUpdate) Validate() error { + return jellidation.ValidateStruct(&c, + jellidation.Field(&c.Name), + jellidation.Field(&c.URL), + jellidation.Field(&c.Authentication), + ) +} + +func (b *ServiceBrokerUpdate) IsAsyncRequest() bool { + return b.Name != nil || b.URL != nil || b.Authentication != nil +} + +func (b *ServiceBrokerUpdate) ToMessage(brokerGUID string) repositories.UpdateServiceBrokerMessage { + message := repositories.UpdateServiceBrokerMessage{ + GUID: brokerGUID, + Name: b.Name, + URL: b.URL, + MetadataPatch: repositories.MetadataPatch(b.Metadata), + } + + if b.Authentication != nil { + message.Credentials = &services.BrokerCredentials{ + Username: b.Authentication.Credentials.Username, + Password: b.Authentication.Credentials.Password, + } + } + + return message +} diff --git a/api/payloads/service_broker_test.go b/api/payloads/service_broker_test.go index 2a5c7a92d..fc17bdb84 100644 --- a/api/payloads/service_broker_test.go +++ b/api/payloads/service_broker_test.go @@ -7,6 +7,7 @@ import ( "code.cloudfoundry.org/korifi/api/repositories" "code.cloudfoundry.org/korifi/model" "code.cloudfoundry.org/korifi/model/services" + "code.cloudfoundry.org/korifi/tools" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" @@ -93,9 +94,9 @@ var _ = Describe("ServiceBrokerCreate", func() { }) }) - Describe("ToServiceBrokerCreateMessage()", func() { + Describe("ToMessage()", func() { It("converts to repo message correctly", func() { - msg := serviceBrokerCreate.ToCreateServiceBrokerMessage() + msg := serviceBrokerCreate.ToMessage() Expect(msg).To(Equal(repositories.CreateServiceBrokerMessage{ Metadata: model.Metadata{ Labels: map[string]string{"label": "label-value"}, @@ -142,3 +143,97 @@ var _ = Describe("ServiceBrokerList", func() { }) }) }) + +var _ = Describe("ServiceBrokerUpdate", func() { + var updatePayload payloads.ServiceBrokerUpdate + + BeforeEach(func() { + updatePayload = payloads.ServiceBrokerUpdate{ + Name: tools.PtrTo("my-broker"), + URL: tools.PtrTo("my-broker-url"), + Metadata: payloads.MetadataPatch{ + Labels: map[string]*string{ + "foo": tools.PtrTo("bar"), + }, + Annotations: map[string]*string{ + "baz": tools.PtrTo("qux"), + }, + }, + } + }) + + Describe("validate", func() { + var ( + validatorErr error + decodedRequest payloads.ServiceBrokerUpdate + ) + + JustBeforeEach(func() { + validatorErr = validator.DecodeAndValidateJSONPayload(createJSONRequest(updatePayload), &decodedRequest) + }) + + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(decodedRequest).To(Equal(updatePayload)) + }) + + When("authentication type is invalid", func() { + BeforeEach(func() { + updatePayload.Authentication = &payloads.BrokerAuthentication{ + Type: "invalid-auth", + } + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "type value must be one of: basic") + }) + }) + }) + + DescribeTable("IsAsyncRequest", + func(payload payloads.ServiceBrokerUpdate, expectedIsAsync bool) { + Expect(payload.IsAsyncRequest()).To(Equal(expectedIsAsync)) + }, + Entry("name", payloads.ServiceBrokerUpdate{Name: tools.PtrTo("foo")}, true), + Entry("url", payloads.ServiceBrokerUpdate{URL: tools.PtrTo("foo")}, true), + Entry("authentication", payloads.ServiceBrokerUpdate{Authentication: &payloads.BrokerAuthentication{}}, true), + Entry("metadata", payloads.ServiceBrokerUpdate{}, false), + ) + + Describe("ToMessage", func() { + It("converts to repo message", func() { + Expect(updatePayload.ToMessage("broker-guid")).To(Equal(repositories.UpdateServiceBrokerMessage{ + GUID: "broker-guid", + Name: tools.PtrTo("my-broker"), + URL: tools.PtrTo("my-broker-url"), + MetadataPatch: repositories.MetadataPatch{ + Labels: map[string]*string{ + "foo": tools.PtrTo("bar"), + }, + Annotations: map[string]*string{ + "baz": tools.PtrTo("qux"), + }, + }, + })) + }) + + When("the message has cretentials", func() { + BeforeEach(func() { + updatePayload.Authentication = &payloads.BrokerAuthentication{ + Credentials: services.BrokerCredentials{ + Username: "user", + Password: "pass", + }, + Type: "basic", + } + }) + + It("converts to repo message with credentials", func() { + Expect(updatePayload.ToMessage("broker-guid").Credentials).To(Equal(&services.BrokerCredentials{ + Username: "user", + Password: "pass", + })) + }) + }) + }) +}) diff --git a/api/presenter/job.go b/api/presenter/job.go index 14ba76774..8b7209d84 100644 --- a/api/presenter/job.go +++ b/api/presenter/job.go @@ -25,6 +25,7 @@ const ( RoleDeleteOperation = "role.delete" ServiceBrokerCreateOperation = "service_broker.create" ServiceBrokerDeleteOperation = "service_broker.delete" + ServiceBrokerUpdateOperation = "service_broker.update" ) var ( diff --git a/api/presenter/service_broker.go b/api/presenter/service_broker.go index c1dea2621..dac9342d5 100644 --- a/api/presenter/service_broker.go +++ b/api/presenter/service_broker.go @@ -16,19 +16,19 @@ type ServiceBrokerLinks struct { } type ServiceBrokerResponse struct { - repositories.ServiceBrokerResource + repositories.ServiceBrokerRecord Links ServiceBrokerLinks `json:"links"` } -func ForServiceBroker(serviceBrokerResource repositories.ServiceBrokerResource, baseURL url.URL) ServiceBrokerResponse { +func ForServiceBroker(serviceBrokerRecord repositories.ServiceBrokerRecord, baseURL url.URL) ServiceBrokerResponse { return ServiceBrokerResponse{ - serviceBrokerResource, + serviceBrokerRecord, ServiceBrokerLinks{ Self: Link{ - HRef: buildURL(baseURL).appendPath(serviceBrokersBase, serviceBrokerResource.GUID).build(), + HRef: buildURL(baseURL).appendPath(serviceBrokersBase, serviceBrokerRecord.GUID).build(), }, ServiceOfferings: Link{ - HRef: buildURL(baseURL).appendPath(serviceOfferingsBase).setQuery("service_broker_guids=" + serviceBrokerResource.GUID).build(), + HRef: buildURL(baseURL).appendPath(serviceOfferingsBase).setQuery("service_broker_guids=" + serviceBrokerRecord.GUID).build(), }, }, } diff --git a/api/presenter/service_broker_test.go b/api/presenter/service_broker_test.go index f60160d7b..5180fca67 100644 --- a/api/presenter/service_broker_test.go +++ b/api/presenter/service_broker_test.go @@ -18,14 +18,14 @@ var _ = Describe("Service Broker", func() { var ( baseURL *url.URL output []byte - record repositories.ServiceBrokerResource + record repositories.ServiceBrokerRecord ) BeforeEach(func() { var err error baseURL, err = url.Parse("https://api.example.org") Expect(err).NotTo(HaveOccurred()) - record = repositories.ServiceBrokerResource{ + record = repositories.ServiceBrokerRecord{ ServiceBroker: services.ServiceBroker{ Name: "my-broker", URL: "https://my.broker", @@ -68,10 +68,6 @@ var _ = Describe("Service Broker", func() { "annotation": "broker-annotation" } }, - "state": { - "Status": 0, - "Details": "" - }, "links": { "self": { "href": "https://api.example.org/v3/service_brokers/resource-guid" diff --git a/api/presenter/service_offering.go b/api/presenter/service_offering.go index 5439ffe1d..5b70a04f6 100644 --- a/api/presenter/service_offering.go +++ b/api/presenter/service_offering.go @@ -18,22 +18,22 @@ type ServiceOfferingLinks struct { } type ServiceOfferingResponse struct { - repositories.ServiceOfferingResource + repositories.ServiceOfferingRecord Links ServiceOfferingLinks `json:"links"` } -func ForServiceOffering(serviceOfferingResource repositories.ServiceOfferingResource, baseURL url.URL) ServiceOfferingResponse { +func ForServiceOffering(serviceOffering repositories.ServiceOfferingRecord, baseURL url.URL) ServiceOfferingResponse { return ServiceOfferingResponse{ - ServiceOfferingResource: serviceOfferingResource, + ServiceOfferingRecord: serviceOffering, Links: ServiceOfferingLinks{ Self: Link{ - HRef: buildURL(baseURL).appendPath(serviceOfferingsBase, serviceOfferingResource.GUID).build(), + HRef: buildURL(baseURL).appendPath(serviceOfferingsBase, serviceOffering.GUID).build(), }, ServicePlans: Link{ - HRef: buildURL(baseURL).appendPath(servicePlansBase).setQuery("service_offering_guids=" + serviceOfferingResource.GUID).build(), + HRef: buildURL(baseURL).appendPath(servicePlansBase).setQuery("service_offering_guids=" + serviceOffering.GUID).build(), }, ServiceBroker: Link{ - HRef: buildURL(baseURL).appendPath(serviceBrokersBase, serviceOfferingResource.Relationships.ServiceBroker.Data.GUID).build(), + HRef: buildURL(baseURL).appendPath(serviceBrokersBase, serviceOffering.Relationships.ServiceBroker.Data.GUID).build(), }, }, } diff --git a/api/presenter/service_offering_test.go b/api/presenter/service_offering_test.go index c92bc0614..110a37410 100644 --- a/api/presenter/service_offering_test.go +++ b/api/presenter/service_offering_test.go @@ -19,14 +19,14 @@ var _ = Describe("Service Offering", func() { var ( baseURL *url.URL output []byte - record repositories.ServiceOfferingResource + record repositories.ServiceOfferingRecord ) BeforeEach(func() { var err error baseURL, err = url.Parse("https://api.example.org") Expect(err).NotTo(HaveOccurred()) - record = repositories.ServiceOfferingResource{ + record = repositories.ServiceOfferingRecord{ ServiceOffering: services.ServiceOffering{ Name: "offering-name", Description: "offering description", @@ -104,10 +104,6 @@ var _ = Describe("Service Offering", func() { "guid": "resource-guid", "created_at": "1970-01-01T00:00:01Z", "updated_at": "1970-01-01T00:00:02Z", - "state": { - "Status": 0, - "Details": "" - }, "metadata": { "labels": { "label": "label-foo" diff --git a/api/presenter/service_plan.go b/api/presenter/service_plan.go index e28d90f84..4120c7ee4 100644 --- a/api/presenter/service_plan.go +++ b/api/presenter/service_plan.go @@ -12,26 +12,20 @@ type ServicePlanLinks struct { } type ServicePlanResponse struct { - repositories.ServicePlanResource + repositories.ServicePlanRecord Links ServicePlanLinks `json:"links"` } -func ForServicePlan(servicePlanResource repositories.ServicePlanResource, baseURL url.URL) ServicePlanResponse { +func ForServicePlan(servicePlan repositories.ServicePlanRecord, baseURL url.URL) ServicePlanResponse { return ServicePlanResponse{ - ServicePlanResource: servicePlanResource, + ServicePlanRecord: servicePlan, Links: ServicePlanLinks{ Self: Link{ - HRef: buildURL(baseURL).appendPath(servicePlansBase, servicePlanResource.GUID).build(), + HRef: buildURL(baseURL).appendPath(servicePlansBase, servicePlan.GUID).build(), }, ServiceOffering: Link{ - HRef: buildURL(baseURL).appendPath(serviceOfferingsBase, servicePlanResource.Relationships.ServiceOffering.Data.GUID).build(), + HRef: buildURL(baseURL).appendPath(serviceOfferingsBase, servicePlan.Relationships.ServiceOffering.Data.GUID).build(), }, }, } } - -func ForServicePlanList(servicePlanResourceList []repositories.ServicePlanResource, baseURL, requestURL url.URL) ListResponse[ServicePlanResponse] { - return ForList(func(servicePlanResource repositories.ServicePlanResource, baseURL url.URL) ServicePlanResponse { - return ForServicePlan(servicePlanResource, baseURL) - }, servicePlanResourceList, baseURL, requestURL) -} diff --git a/api/presenter/service_plan_test.go b/api/presenter/service_plan_test.go index 24803462a..5621862df 100644 --- a/api/presenter/service_plan_test.go +++ b/api/presenter/service_plan_test.go @@ -19,14 +19,14 @@ var _ = Describe("Service Plan", func() { var ( baseURL *url.URL output []byte - record repositories.ServicePlanResource + record repositories.ServicePlanRecord ) BeforeEach(func() { var err error baseURL, err = url.Parse("https://api.example.org") Expect(err).NotTo(HaveOccurred()) - record = repositories.ServicePlanResource{ + record = repositories.ServicePlanRecord{ ServicePlan: services.ServicePlan{ BrokerServicePlan: services.BrokerServicePlan{ Name: "my-service-plan", @@ -134,10 +134,6 @@ var _ = Describe("Service Plan", func() { "guid": "resource-guid", "created_at": "1970-01-01T00:00:01Z", "updated_at": "1970-01-01T00:00:02Z", - "state": { - "Status": 0, - "Details": "" - }, "metadata": { "labels": { "label": "label-foo" diff --git a/api/repositories/patch.go b/api/repositories/patch.go new file mode 100644 index 000000000..eb19337e8 --- /dev/null +++ b/api/repositories/patch.go @@ -0,0 +1,27 @@ +package repositories + +import ( + "context" + "fmt" + + "code.cloudfoundry.org/korifi/tools/k8s" + "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func PatchResource[T any, PT k8s.ObjectWithDeepCopy[T]]( + ctx context.Context, + k8sClient client.Client, + obj PT, + modify func(), +) error { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(obj), obj) + if err != nil { + return fmt.Errorf("failed to get %T %v: %w", obj, client.ObjectKeyFromObject(obj), err) + } + + return errors.Wrapf( + k8s.PatchResource(ctx, k8sClient, obj, modify), + "failed to patch %T %v", obj, client.ObjectKeyFromObject(obj), + ) +} diff --git a/api/repositories/service_broker_repository.go b/api/repositories/service_broker_repository.go index ac5ef45b7..f9eb273c6 100644 --- a/api/repositories/service_broker_repository.go +++ b/api/repositories/service_broker_repository.go @@ -42,12 +42,32 @@ func (l ListServiceBrokerMessage) matches(b korifiv1alpha1.CFServiceBroker) bool return slices.Contains(l.Names, b.Spec.Name) } +type UpdateServiceBrokerMessage struct { + GUID string + Name *string + URL *string + Credentials *services.BrokerCredentials + MetadataPatch MetadataPatch +} + +func (m UpdateServiceBrokerMessage) apply(broker *korifiv1alpha1.CFServiceBroker) { + if m.Name != nil { + broker.Spec.Name = *m.Name + } + + if m.URL != nil { + broker.Spec.URL = *m.URL + } + + m.MetadataPatch.Apply(broker) +} + type ServiceBrokerRepo struct { userClientFactory authorization.UserK8sClientFactory rootNamespace string } -type ServiceBrokerResource struct { +type ServiceBrokerRecord struct { services.ServiceBroker model.CFResource } @@ -62,15 +82,15 @@ func NewServiceBrokerRepo( } } -func (r *ServiceBrokerRepo) CreateServiceBroker(ctx context.Context, authInfo authorization.Info, message CreateServiceBrokerMessage) (ServiceBrokerResource, error) { +func (r *ServiceBrokerRepo) CreateServiceBroker(ctx context.Context, authInfo authorization.Info, message CreateServiceBrokerMessage) (ServiceBrokerRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { - return ServiceBrokerResource{}, fmt.Errorf("failed to build user client: %w", err) + return ServiceBrokerRecord{}, fmt.Errorf("failed to build user client: %w", err) } credsSecretData, err := tools.ToCredentialsSecretData(message.Credentials) if err != nil { - return ServiceBrokerResource{}, fmt.Errorf("failed to create credentials secret data: %w", err) + return ServiceBrokerRecord{}, fmt.Errorf("failed to create credentials secret data: %w", err) } credentialsSecretName := uuid.NewString() @@ -89,7 +109,7 @@ func (r *ServiceBrokerRepo) CreateServiceBroker(ctx context.Context, authInfo au }, } if err = userClient.Create(ctx, cfServiceBroker); err != nil { - return ServiceBrokerResource{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + return ServiceBrokerRecord{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) } credentialsSecret := &corev1.Secret{ @@ -101,18 +121,18 @@ func (r *ServiceBrokerRepo) CreateServiceBroker(ctx context.Context, authInfo au } err = controllerutil.SetOwnerReference(cfServiceBroker, credentialsSecret, scheme.Scheme) if err != nil { - return ServiceBrokerResource{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + return ServiceBrokerRecord{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) } if err = userClient.Create(ctx, credentialsSecret); err != nil { - return ServiceBrokerResource{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + return ServiceBrokerRecord{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) } - return toServiceBrokerResource(*cfServiceBroker), nil + return toServiceBrokerRecord(*cfServiceBroker), nil } -func toServiceBrokerResource(cfServiceBroker korifiv1alpha1.CFServiceBroker) ServiceBrokerResource { - return ServiceBrokerResource{ +func toServiceBrokerRecord(cfServiceBroker korifiv1alpha1.CFServiceBroker) ServiceBrokerRecord { + return ServiceBrokerRecord{ ServiceBroker: cfServiceBroker.Spec.ServiceBroker, CFResource: model.CFResource{ GUID: cfServiceBroker.Name, @@ -128,7 +148,7 @@ func toServiceBrokerResource(cfServiceBroker korifiv1alpha1.CFServiceBroker) Ser func (r *ServiceBrokerRepo) GetState(ctx context.Context, authInfo authorization.Info, brokerGUID string) (model.CFResourceState, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { - return model.CFResourceState{}, fmt.Errorf("failed to build user client: %w", err) + return model.CFResourceStateUnknown, fmt.Errorf("failed to build user client: %w", err) } cfServiceBroker := &korifiv1alpha1.CFServiceBroker{ @@ -139,19 +159,21 @@ func (r *ServiceBrokerRepo) GetState(ctx context.Context, authInfo authorization } if err = userClient.Get(ctx, client.ObjectKeyFromObject(cfServiceBroker), cfServiceBroker); err != nil { - return model.CFResourceState{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + return model.CFResourceStateUnknown, apierrors.FromK8sError(err, ServiceBrokerResourceType) + } + + if cfServiceBroker.Generation != cfServiceBroker.Status.ObservedGeneration { + return model.CFResourceStateUnknown, nil } if meta.IsStatusConditionTrue(cfServiceBroker.Status.Conditions, korifiv1alpha1.StatusConditionReady) { - return model.CFResourceState{ - Status: model.CFResourceStatusReady, - }, nil + return model.CFResourceStateReady, nil } - return model.CFResourceState{}, nil + return model.CFResourceStateUnknown, nil } -func (r *ServiceBrokerRepo) ListServiceBrokers(ctx context.Context, authInfo authorization.Info, message ListServiceBrokerMessage) ([]ServiceBrokerResource, error) { +func (r *ServiceBrokerRepo) ListServiceBrokers(ctx context.Context, authInfo authorization.Info, message ListServiceBrokerMessage) ([]ServiceBrokerRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { return nil, fmt.Errorf("failed to build user client: %w", err) @@ -167,15 +189,15 @@ func (r *ServiceBrokerRepo) ListServiceBrokers(ctx context.Context, authInfo aut } brokers := iter.Lift(brokersList.Items).Filter(message.matches) - return iter.Map(brokers, toServiceBrokerResource).Collect(), nil + return iter.Map(brokers, toServiceBrokerRecord).Collect(), nil } -func (r *ServiceBrokerRepo) GetServiceBroker(ctx context.Context, authInfo authorization.Info, guid string) (ServiceBrokerResource, error) { +func (r *ServiceBrokerRepo) GetServiceBroker(ctx context.Context, authInfo authorization.Info, guid string) (ServiceBrokerRecord, error) { serviceBroker, err := r.getServiceBroker(ctx, authInfo, guid) if err != nil { - return ServiceBrokerResource{}, err + return ServiceBrokerRecord{}, err } - return toServiceBrokerResource(*serviceBroker), nil + return toServiceBrokerRecord(*serviceBroker), nil } func (r *ServiceBrokerRepo) getServiceBroker(ctx context.Context, authInfo authorization.Info, guid string) (*korifiv1alpha1.CFServiceBroker, error) { @@ -198,6 +220,48 @@ func (r *ServiceBrokerRepo) getServiceBroker(ctx context.Context, authInfo autho return serviceBroker, nil } +func (r *ServiceBrokerRepo) UpdateServiceBroker(ctx context.Context, authInfo authorization.Info, message UpdateServiceBrokerMessage) (ServiceBrokerRecord, error) { + userClient, err := r.userClientFactory.BuildClient(authInfo) + if err != nil { + return ServiceBrokerRecord{}, fmt.Errorf("failed to build user client: %w", err) + } + + cfServiceBroker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Name: message.GUID, + Namespace: r.rootNamespace, + }, + } + + if err = PatchResource(ctx, userClient, cfServiceBroker, func() { + message.apply(cfServiceBroker) + }); err != nil { + return ServiceBrokerRecord{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + } + + if message.Credentials != nil { + credsSecretData, err := tools.ToCredentialsSecretData(message.Credentials) + if err != nil { + return ServiceBrokerRecord{}, fmt.Errorf("failed to marshal credentials secret data for service broker: %w", err) + } + + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.rootNamespace, + Name: cfServiceBroker.Spec.Credentials.Name, + }, + } + + if err := PatchResource(ctx, userClient, credentialsSecret, func() { + credentialsSecret.Data = credsSecretData + }); err != nil { + return ServiceBrokerRecord{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + } + } + + return toServiceBrokerRecord(*cfServiceBroker), nil +} + func (r *ServiceBrokerRepo) DeleteServiceBroker(ctx context.Context, authInfo authorization.Info, guid string) error { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { diff --git a/api/repositories/service_broker_repository_test.go b/api/repositories/service_broker_repository_test.go index 84fb14227..01b83ae3f 100644 --- a/api/repositories/service_broker_repository_test.go +++ b/api/repositories/service_broker_repository_test.go @@ -8,7 +8,6 @@ import ( 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" "code.cloudfoundry.org/korifi/tools/k8s" corev1 "k8s.io/api/core/v1" @@ -16,6 +15,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + . "code.cloudfoundry.org/korifi/tests/matchers" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -31,9 +31,9 @@ var _ = Describe("ServiceBrokerRepo", func() { Describe("Create", func() { var ( - createMsg repositories.CreateServiceBrokerMessage - broker repositories.ServiceBrokerResource - createErr error + createMsg repositories.CreateServiceBrokerMessage + brokerRecord repositories.ServiceBrokerRecord + createErr error ) BeforeEach(func() { @@ -58,7 +58,7 @@ var _ = Describe("ServiceBrokerRepo", func() { }) JustBeforeEach(func() { - broker, createErr = repo.CreateServiceBroker(ctx, authInfo, createMsg) + brokerRecord, createErr = repo.CreateServiceBroker(ctx, authInfo, createMsg) }) It("returns a forbidden error", func() { @@ -79,16 +79,13 @@ var _ = Describe("ServiceBrokerRepo", func() { createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) }) - JustBeforeEach(func() { + It("returns a ServiceBrokerRecord", func() { Expect(createErr).NotTo(HaveOccurred()) - }) - - It("returns a ServiceBrokerResource", func() { - Expect(broker.ServiceBroker).To(Equal(services.ServiceBroker{ + Expect(brokerRecord.ServiceBroker).To(Equal(services.ServiceBroker{ Name: "my-broker", URL: "https://my.broker.com", })) - Expect(broker.Metadata).To(Equal(model.Metadata{ + Expect(brokerRecord.Metadata).To(Equal(model.Metadata{ Labels: map[string]string{ "label": "label-value", }, @@ -96,15 +93,16 @@ var _ = Describe("ServiceBrokerRepo", func() { "annotation": "annotation-value", }, })) - Expect(broker.CFResource.GUID).NotTo(BeEmpty()) - Expect(broker.CFResource.CreatedAt).NotTo(BeZero()) + Expect(brokerRecord.CFResource.GUID).NotTo(BeEmpty()) + Expect(brokerRecord.CFResource.CreatedAt).NotTo(BeZero()) }) It("creates a CFServiceBroker resource in Kubernetes", func() { + Expect(createErr).NotTo(HaveOccurred()) cfServiceBroker := &korifiv1alpha1.CFServiceBroker{ ObjectMeta: metav1.ObjectMeta{ Namespace: rootNamespace, - Name: broker.GUID, + Name: brokerRecord.GUID, }, } Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceBroker), cfServiceBroker)).To(Succeed()) @@ -116,10 +114,11 @@ var _ = Describe("ServiceBrokerRepo", func() { }) It("stores broker credentials in a k8s secret", func() { + Expect(createErr).NotTo(HaveOccurred()) cfServiceBroker := &korifiv1alpha1.CFServiceBroker{ ObjectMeta: metav1.ObjectMeta{ Namespace: rootNamespace, - Name: broker.GUID, + Name: brokerRecord.GUID, }, } Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceBroker), cfServiceBroker)).To(Succeed()) @@ -148,25 +147,23 @@ var _ = Describe("ServiceBrokerRepo", func() { Describe("GetState", func() { var ( - brokerGUID string cfServiceBroker *korifiv1alpha1.CFServiceBroker state model.CFResourceState getStateErr error ) BeforeEach(func() { - brokerGUID = uuid.NewString() cfServiceBroker = &korifiv1alpha1.CFServiceBroker{ ObjectMeta: metav1.ObjectMeta{ Namespace: rootNamespace, - Name: brokerGUID, + Name: uuid.NewString(), }, } Expect(k8sClient.Create(ctx, cfServiceBroker)).To(Succeed()) }) JustBeforeEach(func() { - state, getStateErr = repo.GetState(ctx, authInfo, brokerGUID) + state, getStateErr = repo.GetState(ctx, authInfo, cfServiceBroker.Name) }) It("returns a forbidden error", func() { @@ -184,9 +181,7 @@ var _ = Describe("ServiceBrokerRepo", func() { It("returns unknown state", func() { Expect(getStateErr).NotTo(HaveOccurred()) - Expect(state).To(Equal(model.CFResourceState{ - Status: model.CFResourceStatusUnknown, - })) + Expect(state).To(Equal(model.CFResourceStateUnknown)) }) When("the broker is ready", func() { @@ -198,14 +193,26 @@ var _ = Describe("ServiceBrokerRepo", func() { Message: "Ready", Reason: "Ready", }) + cfServiceBroker.Status.ObservedGeneration = cfServiceBroker.Generation })).To(Succeed()) }) It("returns ready state", func() { Expect(getStateErr).NotTo(HaveOccurred()) - Expect(state).To(Equal(model.CFResourceState{ - Status: model.CFResourceStatusReady, - })) + Expect(state).To(Equal(model.CFResourceStateReady)) + }) + + When("the ready status is stale ", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, k8sClient, cfServiceBroker, func() { + cfServiceBroker.Status.ObservedGeneration = -1 + })).To(Succeed()) + }) + + It("returns unknown state", func() { + Expect(getStateErr).NotTo(HaveOccurred()) + Expect(state).To(Equal(model.CFResourceStateUnknown)) + }) }) }) @@ -223,9 +230,7 @@ var _ = Describe("ServiceBrokerRepo", func() { It("returns unknown state", func() { Expect(getStateErr).NotTo(HaveOccurred()) - Expect(state).To(Equal(model.CFResourceState{ - Status: model.CFResourceStatusUnknown, - })) + Expect(state).To(Equal(model.CFResourceStateUnknown)) }) }) }) @@ -233,7 +238,7 @@ var _ = Describe("ServiceBrokerRepo", func() { Describe("ListServiceBrokers", func() { var ( - brokers []repositories.ServiceBrokerResource + brokers []repositories.ServiceBrokerRecord message repositories.ListServiceBrokerMessage ) @@ -341,7 +346,7 @@ var _ = Describe("ServiceBrokerRepo", func() { Describe("GetServiceBroker", func() { var ( - serviceBroker repositories.ServiceBrokerResource + serviceBroker repositories.ServiceBrokerRecord getErr error ) @@ -402,6 +407,145 @@ var _ = Describe("ServiceBrokerRepo", func() { }) }) + Describe("UpdateServiceBroker", func() { + var ( + cfServiceBroker *korifiv1alpha1.CFServiceBroker + brokerRecord repositories.ServiceBrokerRecord + updateMessage repositories.UpdateServiceBrokerMessage + updateErr error + ) + + BeforeEach(func() { + credentialsData, err := tools.ToCredentialsSecretData(map[string]string{ + "username": "user", + "password": "pass", + }) + Expect(err).NotTo(HaveOccurred()) + + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: rootNamespace, + }, + Data: credentialsData, + } + Expect(k8sClient.Create(ctx, credentialsSecret)).To(Succeed()) + + cfServiceBroker = &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + }, + Spec: korifiv1alpha1.CFServiceBrokerSpec{ + ServiceBroker: services.ServiceBroker{ + Name: "my-broker", + URL: "https://my.broker", + }, + Credentials: corev1.LocalObjectReference{ + Name: credentialsSecret.Name, + }, + }, + } + Expect(k8sClient.Create(ctx, cfServiceBroker)).To(Succeed()) + + updateMessage = repositories.UpdateServiceBrokerMessage{ + GUID: cfServiceBroker.Name, + Name: tools.PtrTo("your-broker"), + URL: tools.PtrTo("https://your.broker"), + Credentials: &services.BrokerCredentials{ + Username: "another-user", + Password: "another-pass", + }, + MetadataPatch: repositories.MetadataPatch{ + Labels: map[string]*string{ + "foo": tools.PtrTo("bar"), + }, + Annotations: map[string]*string{ + "baz": tools.PtrTo("qux"), + }, + }, + } + }) + + JustBeforeEach(func() { + brokerRecord, updateErr = repo.UpdateServiceBroker(ctx, authInfo, updateMessage) + }) + + It("returns a forbidden error", func() { + Expect(updateErr).To(WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) + }) + + When("the user has permissions to update brokers", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + }) + + It("returns an updated ServiceBrokerRecord", func() { + Expect(updateErr).NotTo(HaveOccurred()) + Expect(brokerRecord.ServiceBroker).To(Equal(services.ServiceBroker{ + Name: "your-broker", + URL: "https://your.broker", + })) + + Expect(brokerRecord.Metadata.Labels).To(HaveKeyWithValue("foo", "bar")) + Expect(brokerRecord.Metadata.Annotations).To(HaveKeyWithValue("baz", "qux")) + }) + + It("updates the CFServiceBroker resource in Kubernetes", func() { + Expect(updateErr).NotTo(HaveOccurred()) + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceBroker), cfServiceBroker)).To(Succeed()) + + Expect(cfServiceBroker.Spec.ServiceBroker).To(Equal(services.ServiceBroker{ + Name: "your-broker", + URL: "https://your.broker", + })) + + Expect(cfServiceBroker.Labels).To(HaveKeyWithValue("foo", "bar")) + Expect(cfServiceBroker.Annotations).To(HaveKeyWithValue("baz", "qux")) + }) + + It("updates the service borker credentials secret", func() { + Expect(updateErr).NotTo(HaveOccurred()) + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: cfServiceBroker.Spec.Credentials.Name, + Namespace: rootNamespace, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed()) + Expect(credentialsSecret.Data).To(SatisfyAll( + HaveLen(1), + HaveKeyWithValue(tools.CredentialsSecretKey, + MatchJSON(`{"username" : "another-user", "password": "another-pass"}`), + ), + )) + }) + + When("credentials are not updated", func() { + BeforeEach(func() { + updateMessage.Credentials = nil + }) + + It("does not update the credentials secret", func() { + Expect(updateErr).NotTo(HaveOccurred()) + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: cfServiceBroker.Spec.Credentials.Name, + Namespace: rootNamespace, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed()) + Expect(credentialsSecret.Data).To(SatisfyAll( + HaveLen(1), + HaveKeyWithValue(tools.CredentialsSecretKey, + MatchJSON(`{"username" : "user", "password": "pass"}`), + ), + )) + }) + }) + }) + }) + Describe("DeleteServiceBroker", func() { var ( deleteErr error @@ -438,7 +582,7 @@ var _ = Describe("ServiceBrokerRepo", func() { }) It("errors", func() { - Expect(deleteErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.NotFoundError{})) + Expect(deleteErr).To(WrapErrorAssignableToTypeOf(apierrors.NotFoundError{})) }) }) }) @@ -493,7 +637,7 @@ var _ = Describe("ServiceBrokerRepo", func() { }) It("errors", func() { - Expect(getErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.NotFoundError{})) + Expect(getErr).To(WrapErrorAssignableToTypeOf(apierrors.NotFoundError{})) }) }) }) diff --git a/api/repositories/service_offering_repository.go b/api/repositories/service_offering_repository.go index a60b5975e..616945820 100644 --- a/api/repositories/service_offering_repository.go +++ b/api/repositories/service_offering_repository.go @@ -16,7 +16,7 @@ import ( const ServiceOfferingResourceType = "Service Offering" -type ServiceOfferingResource struct { +type ServiceOfferingRecord struct { services.ServiceOffering model.CFResource Relationships ServiceOfferingRelationships `json:"relationships"` @@ -41,29 +41,29 @@ func NewServiceOfferingRepo( } } -func (r *ServiceOfferingRepo) ListOfferings(ctx context.Context, authInfo authorization.Info) ([]ServiceOfferingResource, error) { +func (r *ServiceOfferingRepo) ListOfferings(ctx context.Context, authInfo authorization.Info) ([]ServiceOfferingRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { - return []ServiceOfferingResource{}, fmt.Errorf("failed to build user client: %w", err) + return []ServiceOfferingRecord{}, fmt.Errorf("failed to build user client: %w", err) } offeringsList := &korifiv1alpha1.CFServiceOfferingList{} err = userClient.List(ctx, offeringsList, client.InNamespace(r.rootNamespace)) if err != nil { if k8serrors.IsForbidden(err) { - return []ServiceOfferingResource{}, nil + return []ServiceOfferingRecord{}, nil } - return []ServiceOfferingResource{}, fmt.Errorf("failed to list service offerings: %w", + return []ServiceOfferingRecord{}, fmt.Errorf("failed to list service offerings: %w", apierrors.FromK8sError(err, ServiceOfferingResourceType), ) } - return iter.Map(iter.Lift(offeringsList.Items), offeringToResource).Collect(), nil + return iter.Map(iter.Lift(offeringsList.Items), offeringToRecord).Collect(), nil } -func offeringToResource(offering korifiv1alpha1.CFServiceOffering) ServiceOfferingResource { - return ServiceOfferingResource{ +func offeringToRecord(offering korifiv1alpha1.CFServiceOffering) ServiceOfferingRecord { + return ServiceOfferingRecord{ ServiceOffering: offering.Spec.ServiceOffering, CFResource: model.CFResource{ GUID: offering.Name, diff --git a/api/repositories/service_offering_repository_test.go b/api/repositories/service_offering_repository_test.go index f7e11c08f..1356af696 100644 --- a/api/repositories/service_offering_repository_test.go +++ b/api/repositories/service_offering_repository_test.go @@ -25,7 +25,7 @@ var _ = Describe("ServiceOfferingRepo", func() { Describe("List", func() { var ( offeringGUID string - listedOfferings []repositories.ServiceOfferingResource + listedOfferings []repositories.ServiceOfferingRecord listErr error ) diff --git a/api/repositories/service_plan_repository.go b/api/repositories/service_plan_repository.go index 3478a4db1..c91681222 100644 --- a/api/repositories/service_plan_repository.go +++ b/api/repositories/service_plan_repository.go @@ -15,7 +15,7 @@ import ( const ServicePlanResourceType = "Service Plan" -type ServicePlanResource struct { +type ServicePlanRecord struct { services.ServicePlan model.CFResource Relationships ServicePlanRelationships `json:"relationships"` @@ -40,7 +40,7 @@ func NewServicePlanRepo( } } -func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization.Info) ([]ServicePlanResource, error) { +func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization.Info) ([]ServicePlanRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { return nil, fmt.Errorf("failed to build user client: %w", err) @@ -51,11 +51,11 @@ func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization. return nil, apierrors.FromK8sError(err, ServicePlanResourceType) } - return iter.Map(iter.Lift(cfServicePlans.Items), planToResource).Collect(), nil + return iter.Map(iter.Lift(cfServicePlans.Items), planToRecord).Collect(), nil } -func planToResource(plan korifiv1alpha1.CFServicePlan) ServicePlanResource { - return ServicePlanResource{ +func planToRecord(plan korifiv1alpha1.CFServicePlan) ServicePlanRecord { + return ServicePlanRecord{ ServicePlan: plan.Spec.ServicePlan, CFResource: model.CFResource{ GUID: plan.Name, diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index 295c70521..c2fc668f9 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -24,7 +24,7 @@ var _ = Describe("ServicePlanRepo", func() { Describe("List", func() { var ( planGUID string - listedPlans []repositories.ServicePlanResource + listedPlans []repositories.ServicePlanRecord listErr error ) diff --git a/helm/korifi/controllers/cf_roles/cf_admin.yaml b/helm/korifi/controllers/cf_roles/cf_admin.yaml index bf0e4707c..2fd587276 100644 --- a/helm/korifi/controllers/cf_roles/cf_admin.yaml +++ b/helm/korifi/controllers/cf_roles/cf_admin.yaml @@ -196,6 +196,7 @@ rules: - get - list - delete + - patch - apiGroups: - korifi.cloudfoundry.org diff --git a/model/cfresource.go b/model/cfresource.go index 7d54c68fb..6d2365d49 100644 --- a/model/cfresource.go +++ b/model/cfresource.go @@ -2,22 +2,16 @@ package model import "time" -type CFResourceStatus int +type CFResourceState int const ( - CFResourceStatusUnknown CFResourceStatus = iota - CFResourceStatusReady + CFResourceStateUnknown CFResourceState = iota + CFResourceStateReady ) -type CFResourceState struct { - Status CFResourceStatus - Details string -} - type CFResource struct { - GUID string `json:"guid"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt *time.Time `json:"updated_at"` - Metadata Metadata `json:"metadata"` - State CFResourceState `json:"state"` + GUID string `json:"guid"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt *time.Time `json:"updated_at"` + Metadata Metadata `json:"metadata"` } diff --git a/tests/assets/sample-broker-golang/main.go b/tests/assets/sample-broker-golang/main.go index dd33c8b41..3c854ee97 100644 --- a/tests/assets/sample-broker-golang/main.go +++ b/tests/assets/sample-broker-golang/main.go @@ -1,14 +1,22 @@ package main import ( + "encoding/base64" "encoding/json" + "errors" "fmt" "net/http" "os" + "strings" "sample-broker/osbapi" ) +const ( + hardcodedUserName = "broker-user" + hardcodedPassword = "broker-password" +) + func main() { http.HandleFunc("/", helloWorldHandler) http.HandleFunc("/v2/catalog", getCatalogHandler) @@ -25,7 +33,13 @@ func helloWorldHandler(w http.ResponseWriter, _ *http.Request) { fmt.Fprintln(w, "Hi, I'm the sample broker!") } -func getCatalogHandler(w http.ResponseWriter, _ *http.Request) { +func getCatalogHandler(w http.ResponseWriter, r *http.Request) { + if status, err := checkCredentials(w, r); err != nil { + w.WriteHeader(status) + fmt.Fprintf(w, "Credentials check failed: %v", err) + return + } + catalog := osbapi.Catalog{ Services: []osbapi.Service{{ Name: "sample-service", @@ -43,10 +57,45 @@ func getCatalogHandler(w http.ResponseWriter, _ *http.Request) { catalogBytes, err := json.Marshal(catalog) if err != nil { - fmt.Fprintf(w, "Failed to marshal catalog: %v", err) w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintf(w, "Failed to marshal catalog: %v", err) return } fmt.Fprintln(w, string(catalogBytes)) } + +func checkCredentials(_ http.ResponseWriter, r *http.Request) (int, error) { + authHeader := r.Header.Get("Authorization") + if len(authHeader) == 0 { + return http.StatusUnauthorized, errors.New("Authorization request header not specified") + } + + headerSplit := strings.Split(authHeader, " ") + if len(headerSplit) != 2 { + return http.StatusUnauthorized, errors.New("Could not parse Authorization request header") + } + + if headerSplit[0] != "Basic" { + return http.StatusUnauthorized, errors.New("Unsupported Authorization request header scheme. Only 'Basic' is supported") + } + + credBytes, err := base64.StdEncoding.DecodeString(headerSplit[1]) + if err != nil { + return http.StatusUnauthorized, errors.New("Failed to decode Authorization request header") + } + + creds := strings.Split(string(credBytes), ":") + if len(creds) != 2 { + return http.StatusUnauthorized, errors.New("Failed to extract user credentials from Authorization request header") + } + + username := creds[0] + password := creds[1] + + if username != hardcodedUserName || password != hardcodedPassword { + return http.StatusForbidden, fmt.Errorf("Incorrect credentials: user %q, password %q. Use %q as username and %q as password", username, password, hardcodedUserName, hardcodedPassword) + } + + return -1, nil +} diff --git a/tests/e2e/droplets_test.go b/tests/e2e/droplets_test.go index 39ec79698..f3f9337e8 100644 --- a/tests/e2e/droplets_test.go +++ b/tests/e2e/droplets_test.go @@ -51,7 +51,7 @@ var _ = Describe("Droplets", func() { Type: "docker", }, }) - pkgGUID := createPackage(appGUID, packageResource{ + pkgGUID := createPackage(packageResource{ typedResource: typedResource{ Type: "docker", resource: resource{ diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index 2f5b41db4..4dd11b7c9 100644 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -784,7 +784,7 @@ func getServiceBindingsForApp(appGUID string) []resource { func createBitsPackage(appGUID string) string { GinkgoHelper() - return createPackage(appGUID, packageResource{ + return createPackage(packageResource{ typedResource: typedResource{ Type: "bits", resource: resource{ @@ -796,7 +796,7 @@ func createBitsPackage(appGUID string) string { }) } -func createPackage(appGUID string, pkg packageResource) string { +func createPackage(pkg packageResource) string { var result resource resp, err := adminClient.R(). SetBody(pkg). @@ -1166,7 +1166,7 @@ func getBrokerInClusterURL(brokerAppGUID string) string { return fmt.Sprintf("http://%s.%s:8080", brokerService.Name, brokerService.Namespace) } -func createBroker(brokerURL string) string { +func createBrokerAsync(brokerURL, username, password string) (string, string) { resp, err := adminClient.R(). SetBody(serviceBrokerResource{ resource: resource{ @@ -1176,8 +1176,8 @@ func createBroker(brokerURL string) string { Authentication: serviceBrokerAuthenticationResource{ Type: "basic", Credentials: map[string]any{ - "username": "broker-user", - "password": "broker-password", + "username": username, + "password": password, }, }, }). @@ -1190,16 +1190,21 @@ func createBroker(brokerURL string) string { )) jobURL := resp.Header().Get("Location") + jobURLSplit := strings.Split(jobURL, "~") + Expect(jobURLSplit).To(HaveLen(2)) + brokerGUID := jobURLSplit[1] + return brokerGUID, jobURL +} + +func createBroker(brokerURL string) string { + brokerGUID, jobURL := createBrokerAsync(brokerURL, "broker-user", "broker-password") Eventually(func(g Gomega) { - resp, err = adminClient.R().Get(jobURL) + resp, err := adminClient.R().Get(jobURL) g.Expect(err).NotTo(HaveOccurred()) jobRespBody := string(resp.Body()) g.Expect(jobRespBody).To(ContainSubstring("COMPLETE")) }).Should(Succeed()) - jobURLSplit := strings.Split(jobURL, "~") - Expect(jobURLSplit).To(HaveLen(2)) - brokerGUID := jobURLSplit[1] return brokerGUID } diff --git a/tests/e2e/service_brokers_test.go b/tests/e2e/service_brokers_test.go index 964ed8216..58699f37d 100644 --- a/tests/e2e/service_brokers_test.go +++ b/tests/e2e/service_brokers_test.go @@ -4,6 +4,7 @@ import ( "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" @@ -75,6 +76,63 @@ var _ = Describe("Service Brokers", func() { }) }) + Describe("Update", func() { + var brokerGUID string + + BeforeEach(func() { + var jobURL string + brokerGUID, jobURL = createBrokerAsync(serviceBrokerURL, "incorrect-username", "incorrect-password") + Eventually(func(g Gomega) { + jobResp, jobErr := adminClient.R().Get(jobURL) + g.Expect(jobErr).NotTo(HaveOccurred()) + jobRespBody := string(jobResp.Body()) + g.Expect(jobRespBody).To(ContainSubstring("PROCESSING")) + }).Should(Succeed()) + }) + + AfterEach(func() { + cleanupBroker(brokerGUID) + }) + + JustBeforeEach(func() { + resp, err = adminClient.R().SetBody(map[string]any{ + "authentication": map[string]any{ + "type": "basic", + "credentials": map[string]any{ + "username": "broker-user", + "password": "broker-password", + }, + }, + }).Patch("/v3/service_brokers/" + brokerGUID) + Expect(err).NotTo(HaveOccurred()) + }) + + It("succeeds with job redirect", func() { + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusAccepted), + HaveRestyHeaderWithValue("Location", ContainSubstring("/v3/jobs/service_broker.update~")), + )) + + jobURL := resp.Header().Get("Location") + Eventually(func(g Gomega) { + jobResp, jobErr := adminClient.R().Get(jobURL) + g.Expect(jobErr).NotTo(HaveOccurred()) + jobRespBody := string(jobResp.Body()) + g.Expect(jobRespBody).To(ContainSubstring("COMPLETE")) + }).Should(Succeed()) + + var servicePlans resourceList[resource] + resp, err = adminClient.R().SetResult(&servicePlans).Get("/v3/service_plans") + Expect(err).NotTo(HaveOccurred()) + Expect(resp).To(HaveRestyStatusCode(http.StatusOK)) + Expect(servicePlans.Resources).To(ContainElement(MatchFields(IgnoreExtras, Fields{ + "Metadata": PointTo(MatchFields(IgnoreExtras, Fields{ + "Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, serviceBrokerGUID), + })), + }))) + }) + }) + Describe("Delete", func() { var brokerGUID string