From 748e71d9d5e1d7382337de185a91f8f9949c7499 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Thu, 5 Sep 2024 15:32:42 +0000 Subject: [PATCH] Implement creation of managed service instances fixes #3288 fixes #3439 Co-authored-by: Yusmen Zabanov Co-authored-by: Georgi Sabev Co-authored-by: Yusmen Zabanov --- .../fake/cfservice_instance_repository.go | 165 ++++-- api/handlers/job.go | 26 +- api/handlers/service_instance.go | 35 +- api/handlers/service_instance_test.go | 120 +++- api/main.go | 5 +- api/payloads/service_instance.go | 41 +- api/payloads/service_instance_test.go | 253 ++++++--- api/payloads/service_plan.go | 14 +- api/payloads/service_plan_test.go | 3 + api/presenter/job.go | 21 +- api/presenter/service_offering_test.go | 2 +- .../service_instance_repository.go | 110 +++- .../service_instance_repository_test.go | 307 +++++++--- .../service_offering_repository_test.go | 2 +- api/repositories/service_plan_repository.go | 2 + .../service_plan_repository_test.go | 17 + .../api/v1alpha1/cfserviceinstance_types.go | 10 + .../api/v1alpha1/zz_generated.deepcopy.go | 5 + .../services/brokers/controller.go | 61 +- .../services/brokers/controller_test.go | 384 ++++++------- .../services/brokers/fake/broker_client.go | 279 ++++++++++ .../brokers/fake/broker_client_factory.go | 120 ++++ .../services/brokers/osbapi/client.go | 197 ------- .../services/brokers/osbapi/client_test.go | 198 ------- .../controllers/services/brokers/package.go | 3 + .../services/brokers/suite_test.go | 37 +- .../services/credentials/credentials.go | 18 +- .../services/credentials/credentials_test.go | 3 +- .../services/instances/controller_test.go | 280 ---------- .../services/instances/managed/controller.go | 313 +++++++++++ .../instances/managed/controller_test.go | 523 ++++++++++++++++++ .../instances/managed/fake/broker_client.go | 279 ++++++++++ .../managed/fake/broker_client_factory.go | 120 ++++ .../services/instances/managed/package.go | 3 + .../services/instances/managed/suite_test.go | 119 ++++ .../instances/{ => upsi}/controller.go | 20 +- .../instances/upsi/controller_test.go | 308 +++++++++++ .../instances/{ => upsi}/suite_test.go | 12 +- .../controllers/services/osbapi/client.go | 196 +++++++ .../services/osbapi/client_test.go | 301 ++++++++++ .../services/osbapi/clientfactory.go | 74 +++ .../services/osbapi/clientfactory_test.go | 160 ++++++ .../{brokers => }/osbapi/suite_test.go | 13 +- .../controllers/services/osbapi/types.go | 74 +++ .../workloads/env/vcap_services_builder.go | 5 +- controllers/main.go | 22 +- controllers/webhooks/finalizer/suite_test.go | 2 + controllers/webhooks/finalizer/webhook.go | 28 +- .../webhooks/finalizer/webhook_test.go | 23 + .../webhooks/services/brokers/suite_test.go | 2 +- .../webhooks/services/instances/validator.go | 7 + .../services/instances/validator_test.go | 15 + ...i.cloudfoundry.org_cfserviceinstances.yaml | 5 + helm/korifi/controllers/manifests.yaml | 1 + helm/korifi/controllers/role.yaml | 10 +- model/services/brokers.go | 11 + model/services/offerings.go | 2 +- tests/assets/sample-broker-golang/go.mod | 2 +- tests/assets/sample-broker-golang/main.go | 26 +- tests/e2e/service_instances_test.go | 110 +++- tests/helpers/broker/broker.go | 26 +- tests/helpers/test_env.go | 5 + tools/k8s/condition_builder.go | 6 + tools/k8s/reconcile.go | 5 + tools/k8s/reconcile_test.go | 23 + 65 files changed, 4293 insertions(+), 1276 deletions(-) create mode 100644 controllers/controllers/services/brokers/fake/broker_client.go create mode 100644 controllers/controllers/services/brokers/fake/broker_client_factory.go delete mode 100644 controllers/controllers/services/brokers/osbapi/client.go delete mode 100644 controllers/controllers/services/brokers/osbapi/client_test.go create mode 100644 controllers/controllers/services/brokers/package.go delete mode 100644 controllers/controllers/services/instances/controller_test.go create mode 100644 controllers/controllers/services/instances/managed/controller.go create mode 100644 controllers/controllers/services/instances/managed/controller_test.go create mode 100644 controllers/controllers/services/instances/managed/fake/broker_client.go create mode 100644 controllers/controllers/services/instances/managed/fake/broker_client_factory.go create mode 100644 controllers/controllers/services/instances/managed/package.go create mode 100644 controllers/controllers/services/instances/managed/suite_test.go rename controllers/controllers/services/instances/{ => upsi}/controller.go (90%) create mode 100644 controllers/controllers/services/instances/upsi/controller_test.go rename controllers/controllers/services/instances/{ => upsi}/suite_test.go (89%) create mode 100644 controllers/controllers/services/osbapi/client.go create mode 100644 controllers/controllers/services/osbapi/client_test.go create mode 100644 controllers/controllers/services/osbapi/clientfactory.go create mode 100644 controllers/controllers/services/osbapi/clientfactory_test.go rename controllers/controllers/services/{brokers => }/osbapi/suite_test.go (81%) create mode 100644 controllers/controllers/services/osbapi/types.go diff --git a/api/handlers/fake/cfservice_instance_repository.go b/api/handlers/fake/cfservice_instance_repository.go index 79aeb1d6f..736b0efb1 100644 --- a/api/handlers/fake/cfservice_instance_repository.go +++ b/api/handlers/fake/cfservice_instance_repository.go @@ -11,18 +11,33 @@ import ( ) type CFServiceInstanceRepository struct { - CreateServiceInstanceStub func(context.Context, authorization.Info, repositories.CreateServiceInstanceMessage) (repositories.ServiceInstanceRecord, error) - createServiceInstanceMutex sync.RWMutex - createServiceInstanceArgsForCall []struct { + CreateManagedServiceInstanceStub func(context.Context, authorization.Info, repositories.CreateManagedSIMessage) (repositories.ServiceInstanceRecord, error) + createManagedServiceInstanceMutex sync.RWMutex + createManagedServiceInstanceArgsForCall []struct { arg1 context.Context arg2 authorization.Info - arg3 repositories.CreateServiceInstanceMessage + arg3 repositories.CreateManagedSIMessage } - createServiceInstanceReturns struct { + createManagedServiceInstanceReturns struct { result1 repositories.ServiceInstanceRecord result2 error } - createServiceInstanceReturnsOnCall map[int]struct { + createManagedServiceInstanceReturnsOnCall map[int]struct { + result1 repositories.ServiceInstanceRecord + result2 error + } + CreateUserProvidedServiceInstanceStub func(context.Context, authorization.Info, repositories.CreateUPSIMessage) (repositories.ServiceInstanceRecord, error) + createUserProvidedServiceInstanceMutex sync.RWMutex + createUserProvidedServiceInstanceArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.CreateUPSIMessage + } + createUserProvidedServiceInstanceReturns struct { + result1 repositories.ServiceInstanceRecord + result2 error + } + createUserProvidedServiceInstanceReturnsOnCall map[int]struct { result1 repositories.ServiceInstanceRecord result2 error } @@ -88,18 +103,84 @@ type CFServiceInstanceRepository struct { invocationsMutex sync.RWMutex } -func (fake *CFServiceInstanceRepository) CreateServiceInstance(arg1 context.Context, arg2 authorization.Info, arg3 repositories.CreateServiceInstanceMessage) (repositories.ServiceInstanceRecord, error) { - fake.createServiceInstanceMutex.Lock() - ret, specificReturn := fake.createServiceInstanceReturnsOnCall[len(fake.createServiceInstanceArgsForCall)] - fake.createServiceInstanceArgsForCall = append(fake.createServiceInstanceArgsForCall, struct { +func (fake *CFServiceInstanceRepository) CreateManagedServiceInstance(arg1 context.Context, arg2 authorization.Info, arg3 repositories.CreateManagedSIMessage) (repositories.ServiceInstanceRecord, error) { + fake.createManagedServiceInstanceMutex.Lock() + ret, specificReturn := fake.createManagedServiceInstanceReturnsOnCall[len(fake.createManagedServiceInstanceArgsForCall)] + fake.createManagedServiceInstanceArgsForCall = append(fake.createManagedServiceInstanceArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.CreateManagedSIMessage + }{arg1, arg2, arg3}) + stub := fake.CreateManagedServiceInstanceStub + fakeReturns := fake.createManagedServiceInstanceReturns + fake.recordInvocation("CreateManagedServiceInstance", []interface{}{arg1, arg2, arg3}) + fake.createManagedServiceInstanceMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *CFServiceInstanceRepository) CreateManagedServiceInstanceCallCount() int { + fake.createManagedServiceInstanceMutex.RLock() + defer fake.createManagedServiceInstanceMutex.RUnlock() + return len(fake.createManagedServiceInstanceArgsForCall) +} + +func (fake *CFServiceInstanceRepository) CreateManagedServiceInstanceCalls(stub func(context.Context, authorization.Info, repositories.CreateManagedSIMessage) (repositories.ServiceInstanceRecord, error)) { + fake.createManagedServiceInstanceMutex.Lock() + defer fake.createManagedServiceInstanceMutex.Unlock() + fake.CreateManagedServiceInstanceStub = stub +} + +func (fake *CFServiceInstanceRepository) CreateManagedServiceInstanceArgsForCall(i int) (context.Context, authorization.Info, repositories.CreateManagedSIMessage) { + fake.createManagedServiceInstanceMutex.RLock() + defer fake.createManagedServiceInstanceMutex.RUnlock() + argsForCall := fake.createManagedServiceInstanceArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServiceInstanceRepository) CreateManagedServiceInstanceReturns(result1 repositories.ServiceInstanceRecord, result2 error) { + fake.createManagedServiceInstanceMutex.Lock() + defer fake.createManagedServiceInstanceMutex.Unlock() + fake.CreateManagedServiceInstanceStub = nil + fake.createManagedServiceInstanceReturns = struct { + result1 repositories.ServiceInstanceRecord + result2 error + }{result1, result2} +} + +func (fake *CFServiceInstanceRepository) CreateManagedServiceInstanceReturnsOnCall(i int, result1 repositories.ServiceInstanceRecord, result2 error) { + fake.createManagedServiceInstanceMutex.Lock() + defer fake.createManagedServiceInstanceMutex.Unlock() + fake.CreateManagedServiceInstanceStub = nil + if fake.createManagedServiceInstanceReturnsOnCall == nil { + fake.createManagedServiceInstanceReturnsOnCall = make(map[int]struct { + result1 repositories.ServiceInstanceRecord + result2 error + }) + } + fake.createManagedServiceInstanceReturnsOnCall[i] = struct { + result1 repositories.ServiceInstanceRecord + result2 error + }{result1, result2} +} + +func (fake *CFServiceInstanceRepository) CreateUserProvidedServiceInstance(arg1 context.Context, arg2 authorization.Info, arg3 repositories.CreateUPSIMessage) (repositories.ServiceInstanceRecord, error) { + fake.createUserProvidedServiceInstanceMutex.Lock() + ret, specificReturn := fake.createUserProvidedServiceInstanceReturnsOnCall[len(fake.createUserProvidedServiceInstanceArgsForCall)] + fake.createUserProvidedServiceInstanceArgsForCall = append(fake.createUserProvidedServiceInstanceArgsForCall, struct { arg1 context.Context arg2 authorization.Info - arg3 repositories.CreateServiceInstanceMessage + arg3 repositories.CreateUPSIMessage }{arg1, arg2, arg3}) - stub := fake.CreateServiceInstanceStub - fakeReturns := fake.createServiceInstanceReturns - fake.recordInvocation("CreateServiceInstance", []interface{}{arg1, arg2, arg3}) - fake.createServiceInstanceMutex.Unlock() + stub := fake.CreateUserProvidedServiceInstanceStub + fakeReturns := fake.createUserProvidedServiceInstanceReturns + fake.recordInvocation("CreateUserProvidedServiceInstance", []interface{}{arg1, arg2, arg3}) + fake.createUserProvidedServiceInstanceMutex.Unlock() if stub != nil { return stub(arg1, arg2, arg3) } @@ -109,46 +190,46 @@ func (fake *CFServiceInstanceRepository) CreateServiceInstance(arg1 context.Cont return fakeReturns.result1, fakeReturns.result2 } -func (fake *CFServiceInstanceRepository) CreateServiceInstanceCallCount() int { - fake.createServiceInstanceMutex.RLock() - defer fake.createServiceInstanceMutex.RUnlock() - return len(fake.createServiceInstanceArgsForCall) +func (fake *CFServiceInstanceRepository) CreateUserProvidedServiceInstanceCallCount() int { + fake.createUserProvidedServiceInstanceMutex.RLock() + defer fake.createUserProvidedServiceInstanceMutex.RUnlock() + return len(fake.createUserProvidedServiceInstanceArgsForCall) } -func (fake *CFServiceInstanceRepository) CreateServiceInstanceCalls(stub func(context.Context, authorization.Info, repositories.CreateServiceInstanceMessage) (repositories.ServiceInstanceRecord, error)) { - fake.createServiceInstanceMutex.Lock() - defer fake.createServiceInstanceMutex.Unlock() - fake.CreateServiceInstanceStub = stub +func (fake *CFServiceInstanceRepository) CreateUserProvidedServiceInstanceCalls(stub func(context.Context, authorization.Info, repositories.CreateUPSIMessage) (repositories.ServiceInstanceRecord, error)) { + fake.createUserProvidedServiceInstanceMutex.Lock() + defer fake.createUserProvidedServiceInstanceMutex.Unlock() + fake.CreateUserProvidedServiceInstanceStub = stub } -func (fake *CFServiceInstanceRepository) CreateServiceInstanceArgsForCall(i int) (context.Context, authorization.Info, repositories.CreateServiceInstanceMessage) { - fake.createServiceInstanceMutex.RLock() - defer fake.createServiceInstanceMutex.RUnlock() - argsForCall := fake.createServiceInstanceArgsForCall[i] +func (fake *CFServiceInstanceRepository) CreateUserProvidedServiceInstanceArgsForCall(i int) (context.Context, authorization.Info, repositories.CreateUPSIMessage) { + fake.createUserProvidedServiceInstanceMutex.RLock() + defer fake.createUserProvidedServiceInstanceMutex.RUnlock() + argsForCall := fake.createUserProvidedServiceInstanceArgsForCall[i] return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *CFServiceInstanceRepository) CreateServiceInstanceReturns(result1 repositories.ServiceInstanceRecord, result2 error) { - fake.createServiceInstanceMutex.Lock() - defer fake.createServiceInstanceMutex.Unlock() - fake.CreateServiceInstanceStub = nil - fake.createServiceInstanceReturns = struct { +func (fake *CFServiceInstanceRepository) CreateUserProvidedServiceInstanceReturns(result1 repositories.ServiceInstanceRecord, result2 error) { + fake.createUserProvidedServiceInstanceMutex.Lock() + defer fake.createUserProvidedServiceInstanceMutex.Unlock() + fake.CreateUserProvidedServiceInstanceStub = nil + fake.createUserProvidedServiceInstanceReturns = struct { result1 repositories.ServiceInstanceRecord result2 error }{result1, result2} } -func (fake *CFServiceInstanceRepository) CreateServiceInstanceReturnsOnCall(i int, result1 repositories.ServiceInstanceRecord, result2 error) { - fake.createServiceInstanceMutex.Lock() - defer fake.createServiceInstanceMutex.Unlock() - fake.CreateServiceInstanceStub = nil - if fake.createServiceInstanceReturnsOnCall == nil { - fake.createServiceInstanceReturnsOnCall = make(map[int]struct { +func (fake *CFServiceInstanceRepository) CreateUserProvidedServiceInstanceReturnsOnCall(i int, result1 repositories.ServiceInstanceRecord, result2 error) { + fake.createUserProvidedServiceInstanceMutex.Lock() + defer fake.createUserProvidedServiceInstanceMutex.Unlock() + fake.CreateUserProvidedServiceInstanceStub = nil + if fake.createUserProvidedServiceInstanceReturnsOnCall == nil { + fake.createUserProvidedServiceInstanceReturnsOnCall = make(map[int]struct { result1 repositories.ServiceInstanceRecord result2 error }) } - fake.createServiceInstanceReturnsOnCall[i] = struct { + fake.createUserProvidedServiceInstanceReturnsOnCall[i] = struct { result1 repositories.ServiceInstanceRecord result2 error }{result1, result2} @@ -418,8 +499,10 @@ func (fake *CFServiceInstanceRepository) PatchServiceInstanceReturnsOnCall(i int func (fake *CFServiceInstanceRepository) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() - fake.createServiceInstanceMutex.RLock() - defer fake.createServiceInstanceMutex.RUnlock() + fake.createManagedServiceInstanceMutex.RLock() + defer fake.createManagedServiceInstanceMutex.RUnlock() + fake.createUserProvidedServiceInstanceMutex.RLock() + defer fake.createUserProvidedServiceInstanceMutex.RUnlock() fake.deleteServiceInstanceMutex.RLock() defer fake.deleteServiceInstanceMutex.RUnlock() fake.getServiceInstanceMutex.RLock() diff --git a/api/handlers/job.go b/api/handlers/job.go index 517f6fb19..97a23cfc7 100644 --- a/api/handlers/job.go +++ b/api/handlers/job.go @@ -17,19 +17,19 @@ import ( ) const ( - JobPath = "/v3/jobs/{guid}" - syncSpaceJobType = "space.apply_manifest" - AppDeleteJobType = "app.delete" - OrgDeleteJobType = "org.delete" - RouteDeleteJobType = "route.delete" - SpaceDeleteJobType = "space.delete" - DomainDeleteJobType = "domain.delete" - RoleDeleteJobType = "role.delete" - ServiceBrokerCreateJobType = "service_broker.create" - ServiceBrokerUpdateJobType = "service_broker.update" - ServiceBrokerDeleteJobType = "service_broker.delete" - - JobTimeoutDuration = 120.0 + JobPath = "/v3/jobs/{guid}" + syncSpaceJobType = "space.apply_manifest" + AppDeleteJobType = "app.delete" + OrgDeleteJobType = "org.delete" + RouteDeleteJobType = "route.delete" + SpaceDeleteJobType = "space.delete" + DomainDeleteJobType = "domain.delete" + RoleDeleteJobType = "role.delete" + ServiceBrokerCreateJobType = "service_broker.create" + ServiceBrokerUpdateJobType = "service_broker.update" + ServiceBrokerDeleteJobType = "service_broker.delete" + ManagedServiceInstanceCreateJobType = "managed_service_instance.create" + JobTimeoutDuration = 120.0 ) const JobResourceType = "Job" diff --git a/api/handlers/service_instance.go b/api/handlers/service_instance.go index d0ba24c5c..b19841d23 100644 --- a/api/handlers/service_instance.go +++ b/api/handlers/service_instance.go @@ -27,7 +27,8 @@ const ( //counterfeiter:generate -o fake -fake-name CFServiceInstanceRepository . CFServiceInstanceRepository type CFServiceInstanceRepository interface { - CreateServiceInstance(context.Context, authorization.Info, repositories.CreateServiceInstanceMessage) (repositories.ServiceInstanceRecord, error) + CreateUserProvidedServiceInstance(context.Context, authorization.Info, repositories.CreateUPSIMessage) (repositories.ServiceInstanceRecord, error) + CreateManagedServiceInstance(context.Context, authorization.Info, repositories.CreateManagedSIMessage) (repositories.ServiceInstanceRecord, error) PatchServiceInstance(context.Context, authorization.Info, repositories.PatchServiceInstanceMessage) (repositories.ServiceInstanceRecord, error) ListServiceInstances(context.Context, authorization.Info, repositories.ListServiceInstanceMessage) ([]repositories.ServiceInstanceRecord, error) GetServiceInstance(context.Context, authorization.Info, string) (repositories.ServiceInstanceRecord, error) @@ -82,9 +83,37 @@ func (h *ServiceInstance) create(r *http.Request) (*routing.Response, error) { ) } - serviceInstanceRecord, err := h.serviceInstanceRepo.CreateServiceInstance(r.Context(), authInfo, payload.ToServiceInstanceCreateMessage()) + if payload.Type == "managed" { + return h.createManagedServiceInstance(r.Context(), logger, authInfo, payload) + } + + return h.createUserProvidedServiceInstance(r.Context(), logger, authInfo, payload) +} + +func (h *ServiceInstance) createManagedServiceInstance( + ctx context.Context, + logger logr.Logger, + authInfo authorization.Info, + payload payloads.ServiceInstanceCreate, +) (*routing.Response, error) { + serviceInstanceRecord, err := h.serviceInstanceRepo.CreateManagedServiceInstance(ctx, authInfo, payload.ToManagedSICreateMessage()) + if err != nil { + return nil, apierrors.LogAndReturn(logger, err, "Failed to create managed service instance", "Service Instance Name", payload.Name) + } + + return routing.NewResponse(http.StatusAccepted). + WithHeader("Location", presenter.JobURLForRedirects(serviceInstanceRecord.GUID, presenter.ManagedServiceInstanceCreateOperation, h.serverURL)), nil +} + +func (h *ServiceInstance) createUserProvidedServiceInstance( + ctx context.Context, + logger logr.Logger, + authInfo authorization.Info, + payload payloads.ServiceInstanceCreate, +) (*routing.Response, error) { + serviceInstanceRecord, err := h.serviceInstanceRepo.CreateUserProvidedServiceInstance(ctx, authInfo, payload.ToUPSICreateMessage()) if err != nil { - return nil, apierrors.LogAndReturn(logger, err, "Failed to create service instance", "Service Instance Name", serviceInstanceRecord.Name) + return nil, apierrors.LogAndReturn(logger, err, "Failed to create user provided service instance", "Service Instance Name", payload.Name) } return routing.NewResponse(http.StatusCreated).WithBody(presenter.ForServiceInstance(serviceInstanceRecord, h.serverURL)), nil diff --git a/api/handlers/service_instance_test.go b/api/handlers/service_instance_test.go index 86f8b4c43..c4118daf8 100644 --- a/api/handlers/service_instance_test.go +++ b/api/handlers/service_instance_test.go @@ -79,9 +79,6 @@ var _ = Describe("ServiceInstance", func() { reqMethod = http.MethodPost requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServiceInstanceCreate{ - Name: "service-instance-name", - Type: "user-provided", - Tags: []string{"foo", "bar"}, Relationships: &payloads.ServiceInstanceRelationships{ Space: &payloads.Relationship{ Data: &payloads.RelationshipData{ @@ -89,33 +86,13 @@ var _ = Describe("ServiceInstance", func() { }, }, }, - Metadata: payloads.Metadata{}, }) - - serviceInstanceRepo.CreateServiceInstanceReturns(repositories.ServiceInstanceRecord{GUID: "service-instance-guid"}, nil) }) - It("creates a CFServiceInstance", func() { + It("validates the request", func() { Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) Expect(bodyString(actualReq)).To(Equal("the-json-body")) - - Expect(serviceInstanceRepo.CreateServiceInstanceCallCount()).To(Equal(1)) - _, actualAuthInfo, actualCreate := serviceInstanceRepo.CreateServiceInstanceArgsForCall(0) - Expect(actualAuthInfo).To(Equal(authInfo)) - Expect(actualCreate).To(Equal(repositories.CreateServiceInstanceMessage{ - Name: "service-instance-name", - SpaceGUID: "space-guid", - Type: "user-provided", - Tags: []string{"foo", "bar"}, - })) - - Expect(rr).To(HaveHTTPStatus(http.StatusCreated)) - Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) - Expect(rr).To(HaveHTTPBody(SatisfyAll( - MatchJSONPath("$.guid", "service-instance-guid"), - MatchJSONPath("$.links.self.href", "https://api.example.org/v3/service_instances/service-instance-guid"), - ))) }) When("the request body is not valid", func() { @@ -154,13 +131,100 @@ var _ = Describe("ServiceInstance", func() { }) }) - When("creating the service instance fails", func() { + When("creating a user provided serivce instance", func() { BeforeEach(func() { - serviceInstanceRepo.CreateServiceInstanceReturns(repositories.ServiceInstanceRecord{}, errors.New("space-instance-creation-failed")) + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServiceInstanceCreate{ + Name: "service-instance-name", + Type: "user-provided", + Relationships: &payloads.ServiceInstanceRelationships{ + Space: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "space-guid", + }, + }, + }, + }) + + serviceInstanceRepo.CreateUserProvidedServiceInstanceReturns(repositories.ServiceInstanceRecord{GUID: "service-instance-guid"}, nil) }) - It("returns unknown error", func() { - expectUnknownError() + It("creates a user provided service instance with the repository", func() { + Expect(serviceInstanceRepo.CreateUserProvidedServiceInstanceCallCount()).To(Equal(1)) + _, actualAuthInfo, actualCreate := serviceInstanceRepo.CreateUserProvidedServiceInstanceArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualCreate).To(Equal(repositories.CreateUPSIMessage{ + Name: "service-instance-name", + SpaceGUID: "space-guid", + })) + }) + + When("creating the service instance fails", func() { + BeforeEach(func() { + serviceInstanceRepo.CreateUserProvidedServiceInstanceReturns(repositories.ServiceInstanceRecord{}, errors.New("space-instance-creation-failed")) + }) + + It("returns unknown error", func() { + expectUnknownError() + }) + }) + + It("returns HTTP 201 Created response", func() { + Expect(rr).To(HaveHTTPStatus(http.StatusCreated)) + Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.guid", "service-instance-guid"), + MatchJSONPath("$.links.self.href", "https://api.example.org/v3/service_instances/service-instance-guid"), + ))) + }) + }) + + When("the service instance is managed", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServiceInstanceCreate{ + Name: "service-instance-name", + Type: "managed", + Relationships: &payloads.ServiceInstanceRelationships{ + Space: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "space-guid", + }, + }, + ServicePlan: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "plan-guid", + }, + }, + }, + }) + + serviceInstanceRepo.CreateManagedServiceInstanceReturns(repositories.ServiceInstanceRecord{GUID: "service-instance-guid"}, nil) + }) + + It("creates a managed service instance with the repository", func() { + Expect(serviceInstanceRepo.CreateManagedServiceInstanceCallCount()).To(Equal(1)) + _, actualAuthInfo, actualCreate := serviceInstanceRepo.CreateManagedServiceInstanceArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualCreate).To(Equal(repositories.CreateManagedSIMessage{ + Name: "service-instance-name", + SpaceGUID: "space-guid", + PlanGUID: "plan-guid", + })) + }) + + When("creating the managed service instance fails", func() { + BeforeEach(func() { + serviceInstanceRepo.CreateManagedServiceInstanceReturns(repositories.ServiceInstanceRecord{}, errors.New("create-managed-err")) + }) + + It("returns unknown error", func() { + expectUnknownError() + }) + }) + + It("returns HTTP 202 Accepted response", func() { + Expect(rr).To(HaveHTTPStatus(http.StatusAccepted)) + Expect(rr).To(HaveHTTPHeaderWithValue("Location", + ContainSubstring("/v3/jobs/managed_service_instance.create~service-instance-guid"))) }) }) }) diff --git a/api/main.go b/api/main.go index 8e6d0bfaf..07b5a27ef 100644 --- a/api/main.go +++ b/api/main.go @@ -362,8 +362,9 @@ func main() { handlers.ServiceBrokerDeleteJobType: serviceBrokerRepo, }, map[string]handlers.StateRepository{ - handlers.ServiceBrokerCreateJobType: serviceBrokerRepo, - handlers.ServiceBrokerUpdateJobType: serviceBrokerRepo, + handlers.ServiceBrokerCreateJobType: serviceBrokerRepo, + handlers.ServiceBrokerUpdateJobType: serviceBrokerRepo, + handlers.ManagedServiceInstanceCreateJobType: serviceInstanceRepo, }, 500*time.Millisecond, ), diff --git a/api/payloads/service_instance.go b/api/payloads/service_instance.go index 221eaf568..a66dde974 100644 --- a/api/payloads/service_instance.go +++ b/api/payloads/service_instance.go @@ -20,6 +20,7 @@ type ServiceInstanceCreate struct { Type string `json:"type"` Tags []string `json:"tags"` Credentials map[string]any `json:"credentials"` + Parameters map[string]any `json:"parameters"` Relationships *ServiceInstanceRelationships `json:"relationships"` Metadata Metadata `json:"metadata"` } @@ -46,32 +47,58 @@ func validateTagLength(tags any) error { func (c ServiceInstanceCreate) Validate() error { return jellidation.ValidateStruct(&c, jellidation.Field(&c.Name, jellidation.Required), - jellidation.Field(&c.Type, jellidation.Required, validation.OneOf("user-provided")), + jellidation.Field(&c.Type, jellidation.Required, validation.OneOf("user-provided", "managed")), jellidation.Field(&c.Tags, jellidation.By(validateTagLength)), - jellidation.Field(&c.Relationships, jellidation.NotNil), + jellidation.Field(&c.Relationships, jellidation.NotNil, jellidation.By(func(r any) error { + rel := r.(*ServiceInstanceRelationships) + if c.Type == "user-provided" { + return rel.ValidateUserProvidedRelationships() + } + + return rel.ValidateManagedRelationships() + })), jellidation.Field(&c.Metadata), ) } -func (p ServiceInstanceCreate) ToServiceInstanceCreateMessage() repositories.CreateServiceInstanceMessage { - return repositories.CreateServiceInstanceMessage{ +func (p ServiceInstanceCreate) ToUPSICreateMessage() repositories.CreateUPSIMessage { + return repositories.CreateUPSIMessage{ Name: p.Name, SpaceGUID: p.Relationships.Space.Data.GUID, Credentials: p.Credentials, - Type: p.Type, Tags: p.Tags, Labels: p.Metadata.Labels, Annotations: p.Metadata.Annotations, } } +func (p ServiceInstanceCreate) ToManagedSICreateMessage() repositories.CreateManagedSIMessage { + return repositories.CreateManagedSIMessage{ + Name: p.Name, + SpaceGUID: p.Relationships.Space.Data.GUID, + PlanGUID: p.Relationships.ServicePlan.Data.GUID, + Tags: p.Tags, + Labels: p.Metadata.Labels, + Annotations: p.Metadata.Annotations, + Parameters: p.Parameters, + } +} + type ServiceInstanceRelationships struct { - Space *Relationship `json:"space"` + Space *Relationship `json:"space"` + ServicePlan *Relationship `json:"service_plan"` +} + +func (r ServiceInstanceRelationships) ValidateUserProvidedRelationships() error { + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.Space, jellidation.NotNil), + ) } -func (r ServiceInstanceRelationships) Validate() error { +func (r ServiceInstanceRelationships) ValidateManagedRelationships() error { return jellidation.ValidateStruct(&r, jellidation.Field(&r.Space, jellidation.NotNil), + jellidation.Field(&r.ServicePlan, jellidation.NotNil), ) } diff --git a/api/payloads/service_instance_test.go b/api/payloads/service_instance_test.go index b9dd68722..f99ceb8ae 100644 --- a/api/payloads/service_instance_test.go +++ b/api/payloads/service_instance_test.go @@ -96,110 +96,172 @@ var _ = Describe("ServiceInstanceCreate", func() { BeforeEach(func() { serviceInstanceCreate = new(payloads.ServiceInstanceCreate) - createPayload = payloads.ServiceInstanceCreate{ - Name: "service-instance-name", - Type: "user-provided", - Tags: []string{"foo", "bar"}, - Credentials: map[string]any{ - "username": "bob", - "password": "float", - "object": map[string]any{ - "a": "b", - }, - }, - Relationships: &payloads.ServiceInstanceRelationships{ - Space: &payloads.Relationship{ - Data: &payloads.RelationshipData{ - GUID: "space-guid", - }, - }, - }, - Metadata: payloads.Metadata{ - Annotations: map[string]string{"ann1": "val_ann1"}, - Labels: map[string]string{"lab1": "val_lab1"}, - }, - } - }) - - JustBeforeEach(func() { - validatorErr = validator.DecodeAndValidateJSONPayload(createJSONRequest(createPayload), serviceInstanceCreate) - }) - - It("succeeds", func() { - Expect(validatorErr).NotTo(HaveOccurred()) - Expect(serviceInstanceCreate).To(PointTo(Equal(createPayload))) }) - When("name is not set", func() { + Describe("Validation", func() { BeforeEach(func() { - createPayload.Name = "" + createPayload = payloads.ServiceInstanceCreate{ + Name: "service-instance-name", + Type: "user-provided", + Tags: []string{"foo", "bar"}, + Credentials: map[string]any{ + "username": "bob", + "password": "float", + "object": map[string]any{ + "a": "b", + }, + }, + Relationships: &payloads.ServiceInstanceRelationships{ + Space: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "space-guid", + }, + }, + }, + Metadata: payloads.Metadata{ + Annotations: map[string]string{"ann1": "val_ann1"}, + Labels: map[string]string{"lab1": "val_lab1"}, + }, + } }) - It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "name cannot be blank") + JustBeforeEach(func() { + validatorErr = validator.DecodeAndValidateJSONPayload(createJSONRequest(createPayload), serviceInstanceCreate) }) - }) - When("type is not set", func() { - BeforeEach(func() { - createPayload.Type = "" + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(serviceInstanceCreate).To(PointTo(Equal(createPayload))) }) - It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "type cannot be blank") + When("name is not set", func() { + BeforeEach(func() { + createPayload.Name = "" + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "name cannot be blank") + }) }) - }) - When("type is invalid", func() { - BeforeEach(func() { - createPayload.Type = "service-instance-type" + When("type is not set", func() { + BeforeEach(func() { + createPayload.Type = "" + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "type cannot be blank") + }) }) - It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "type value must be one of: user-provided") + When("type is invalid", func() { + BeforeEach(func() { + createPayload.Type = "service-instance-type" + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "type value must be one of: user-provided") + }) }) - }) - When("space relationship data is not set", func() { - BeforeEach(func() { - createPayload.Relationships.Space.Data = nil + When("space relationship data is not set", func() { + BeforeEach(func() { + createPayload.Relationships.Space.Data = nil + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "data is required") + }) }) - It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "data is required") + When("tags length is too long", func() { + BeforeEach(func() { + longString := strings.Repeat("a", 2048) + createPayload.Tags = append(createPayload.Tags, longString) + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "combined length of tags cannot exceed") + }) }) - }) - When("tags length is too long", func() { - BeforeEach(func() { - longString := strings.Repeat("a", 2048) - createPayload.Tags = append(createPayload.Tags, longString) + When("metadata is invalid", func() { + BeforeEach(func() { + createPayload.Metadata = payloads.Metadata{ + Labels: map[string]string{ + "foo.cloudfoundry.org/bar": "jim", + }, + } + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "label/annotation key cannot use the cloudfoundry.org domain") + }) }) - It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "combined length of tags cannot exceed") + When("the instance type is managed", func() { + BeforeEach(func() { + createPayload.Type = "managed" + createPayload.Credentials = nil + createPayload.Relationships.ServicePlan = &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "plan_guid", + }, + } + }) + + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(serviceInstanceCreate).To(PointTo(Equal(createPayload))) + }) + + When("plan relationship is not set", func() { + BeforeEach(func() { + createPayload.Relationships.ServicePlan = nil + }) + + It("return an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "relationships.service_plan is required") + }) + }) }) }) - When("metadata is invalid", func() { + Describe("ToUPSICreateMessage()", func() { + var msg repositories.CreateUPSIMessage + BeforeEach(func() { - createPayload.Metadata = payloads.Metadata{ - Labels: map[string]string{ - "foo.cloudfoundry.org/bar": "jim", + createPayload = payloads.ServiceInstanceCreate{ + Name: "service-instance-name", + Type: "user-provided", + Tags: []string{"foo", "bar"}, + Credentials: map[string]any{ + "username": "bob", + "password": "float", + "object": map[string]any{ + "a": "b", + }, + }, + Relationships: &payloads.ServiceInstanceRelationships{ + Space: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "space-guid", + }, + }, + }, + Metadata: payloads.Metadata{ + Annotations: map[string]string{"ann1": "val_ann1"}, + Labels: map[string]string{"lab1": "val_lab1"}, }, } }) - It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "label/annotation key cannot use the cloudfoundry.org domain") + JustBeforeEach(func() { + msg = createPayload.ToUPSICreateMessage() }) - }) - Context("ToServiceInstanceCreateMessage()", func() { It("converts to repo message correctly", func() { - msg := serviceInstanceCreate.ToServiceInstanceCreateMessage() Expect(msg.Name).To(Equal("service-instance-name")) - Expect(msg.Type).To(Equal("user-provided")) Expect(msg.SpaceGUID).To(Equal("space-guid")) Expect(msg.Tags).To(ConsistOf("foo", "bar")) Expect(msg.Annotations).To(HaveLen(1)) @@ -215,6 +277,55 @@ var _ = Describe("ServiceInstanceCreate", func() { })) }) }) + + Describe("ToManagedSICreateMessage()", func() { + var msg repositories.CreateManagedSIMessage + + BeforeEach(func() { + createPayload = payloads.ServiceInstanceCreate{ + Name: "service-instance-name", + Type: "managed", + Tags: []string{"foo", "bar"}, + Parameters: map[string]any{ + "param1": "param1-value", + }, + Relationships: &payloads.ServiceInstanceRelationships{ + Space: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "space-guid", + }, + }, + ServicePlan: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "plan-guid", + }, + }, + }, + Metadata: payloads.Metadata{ + Annotations: map[string]string{"ann1": "val_ann1"}, + Labels: map[string]string{"lab1": "val_lab1"}, + }, + } + }) + + JustBeforeEach(func() { + msg = createPayload.ToManagedSICreateMessage() + }) + + It("converts to repo message correctly", func() { + Expect(msg).To(Equal(repositories.CreateManagedSIMessage{ + Name: "service-instance-name", + SpaceGUID: "space-guid", + PlanGUID: "plan-guid", + Parameters: map[string]any{ + "param1": "param1-value", + }, + Tags: []string{"foo", "bar"}, + Labels: map[string]string{"lab1": "val_lab1"}, + Annotations: map[string]string{"ann1": "val_ann1"}, + })) + }) + }) }) var _ = Describe("ServiceInstancePatch custom unmarshalling", func() { diff --git a/api/payloads/service_plan.go b/api/payloads/service_plan.go index c96077fee..7f512fec5 100644 --- a/api/payloads/service_plan.go +++ b/api/payloads/service_plan.go @@ -21,6 +21,7 @@ import ( type ServicePlanList struct { ServiceOfferingGUIDs string + BrokerGUIDs string BrokerNames string Names string ServiceOfferingNames string @@ -64,12 +65,22 @@ func (l *ServicePlanList) ToMessage() repositories.ListServicePlanMessage { Names: parse.ArrayParam(l.Names), ServiceOfferingNames: parse.ArrayParam(l.ServiceOfferingNames), BrokerNames: parse.ArrayParam(l.BrokerNames), + BrokerGUIDs: parse.ArrayParam(l.BrokerGUIDs), Available: l.Available, } } func (l *ServicePlanList) SupportedKeys() []string { - return []string{"service_offering_guids", "names", "available", "fields[service_offering.service_broker]", "service_broker_names", "include", "service_offering_names"} + return []string{ + "service_offering_guids", + "names", + "available", + "fields[service_offering.service_broker]", + "service_broker_names", + "service_broker_guids", + "include", + "service_offering_names", + } } func (l *ServicePlanList) IgnoredKeys() []*regexp.Regexp { @@ -85,6 +96,7 @@ func (l *ServicePlanList) DecodeFromURLValues(values url.Values) error { l.Names = values.Get("names") l.ServiceOfferingNames = values.Get("service_offering_names") l.BrokerNames = values.Get("service_broker_names") + l.BrokerGUIDs = values.Get("service_broker_guids") available, err := parseBool(values.Get("available")) if err != nil { diff --git a/api/payloads/service_plan_test.go b/api/payloads/service_plan_test.go index 7b4c017db..a7481b7dc 100644 --- a/api/payloads/service_plan_test.go +++ b/api/payloads/service_plan_test.go @@ -28,6 +28,7 @@ var _ = Describe("ServicePlan", func() { Entry("available", "available=true", payloads.ServicePlanList{Available: tools.PtrTo(true)}), Entry("not available", "available=false", payloads.ServicePlanList{Available: tools.PtrTo(false)}), Entry("broker names", "service_broker_names=b1,b2", payloads.ServicePlanList{BrokerNames: "b1,b2"}), + Entry("broker guids", "service_broker_guids=b1,b2", payloads.ServicePlanList{BrokerGUIDs: "b1,b2"}), Entry("include", "include=service_offering&include=space.organization", payloads.ServicePlanList{ IncludeResourceRules: []params.IncludeResourceRule{ { @@ -62,6 +63,7 @@ var _ = Describe("ServicePlan", func() { It("converts payload to repository message", func() { payload := payloads.ServicePlanList{ ServiceOfferingGUIDs: "b1,b2", + BrokerGUIDs: "b1-guid,b2-guid", BrokerNames: "br1,br2", Names: "n1,n2", ServiceOfferingNames: "so1,so2", @@ -70,6 +72,7 @@ var _ = Describe("ServicePlan", func() { Expect(payload.ToMessage()).To(Equal(repositories.ListServicePlanMessage{ ServiceOfferingGUIDs: []string{"b1", "b2"}, BrokerNames: []string{"br1", "br2"}, + BrokerGUIDs: []string{"b1-guid", "b2-guid"}, Names: []string{"n1", "n2"}, ServiceOfferingNames: []string{"so1", "so2"}, Available: tools.PtrTo(true), diff --git a/api/presenter/job.go b/api/presenter/job.go index 8b7209d84..b387333a6 100644 --- a/api/presenter/job.go +++ b/api/presenter/job.go @@ -16,16 +16,17 @@ const ( StateFailed = "FAILED" StateProcessing = "PROCESSING" - AppDeleteOperation = "app.delete" - OrgDeleteOperation = "org.delete" - RouteDeleteOperation = "route.delete" - SpaceApplyManifestOperation = "space.apply_manifest" - SpaceDeleteOperation = "space.delete" - DomainDeleteOperation = "domain.delete" - RoleDeleteOperation = "role.delete" - ServiceBrokerCreateOperation = "service_broker.create" - ServiceBrokerDeleteOperation = "service_broker.delete" - ServiceBrokerUpdateOperation = "service_broker.update" + AppDeleteOperation = "app.delete" + OrgDeleteOperation = "org.delete" + RouteDeleteOperation = "route.delete" + SpaceApplyManifestOperation = "space.apply_manifest" + SpaceDeleteOperation = "space.delete" + DomainDeleteOperation = "domain.delete" + RoleDeleteOperation = "role.delete" + ServiceBrokerCreateOperation = "service_broker.create" + ServiceBrokerDeleteOperation = "service_broker.delete" + ServiceBrokerUpdateOperation = "service_broker.update" + ManagedServiceInstanceCreateOperation = "managed_service_instance.create" ) var ( diff --git a/api/presenter/service_offering_test.go b/api/presenter/service_offering_test.go index 258260973..dfc7935d8 100644 --- a/api/presenter/service_offering_test.go +++ b/api/presenter/service_offering_test.go @@ -34,7 +34,7 @@ var _ = Describe("Service Offering", func() { Requires: []string{"r1"}, DocumentationURL: tools.PtrTo("https://doc.url"), BrokerCatalog: services.ServiceBrokerCatalog{ - Id: "catalog-id", + ID: "catalog-id", Metadata: &runtime.RawExtension{ Raw: []byte(`{"foo": "bar"}`), }, diff --git a/api/repositories/service_instance_repository.go b/api/repositories/service_instance_repository.go index d05d595c2..92e9f62e8 100644 --- a/api/repositories/service_instance_repository.go +++ b/api/repositories/service_instance_repository.go @@ -2,6 +2,7 @@ package repositories import ( "context" + "encoding/json" "errors" "fmt" "slices" @@ -10,6 +11,7 @@ import ( "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/model" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" @@ -18,8 +20,10 @@ import ( "github.com/google/uuid" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -55,11 +59,20 @@ func NewServiceInstanceRepo( } } -type CreateServiceInstanceMessage struct { +type CreateUPSIMessage struct { Name string SpaceGUID string Credentials map[string]any - Type string + Tags []string + Labels map[string]string + Annotations map[string]string +} + +type CreateManagedSIMessage struct { + Name string + SpaceGUID string + PlanGUID string + Parameters map[string]any Tags []string Labels map[string]string Annotations map[string]string @@ -117,6 +130,7 @@ type ServiceInstanceRecord struct { Annotations map[string]string CreatedAt time.Time UpdatedAt *time.Time + Ready bool } func (r ServiceInstanceRecord) Relationships() map[string]string { @@ -130,13 +144,27 @@ func (r ServiceInstanceRecord) Relationships() map[string]string { return relationships } -func (r *ServiceInstanceRepo) CreateServiceInstance(ctx context.Context, authInfo authorization.Info, message CreateServiceInstanceMessage) (ServiceInstanceRecord, error) { +func (r *ServiceInstanceRepo) CreateUserProvidedServiceInstance(ctx context.Context, authInfo authorization.Info, message CreateUPSIMessage) (ServiceInstanceRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { return ServiceInstanceRecord{}, fmt.Errorf("failed to build user client: %w", err) } - cfServiceInstance := message.toCFServiceInstance() + guid := uuid.NewString() + cfServiceInstance := &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: guid, + Namespace: message.SpaceGUID, + Labels: message.Labels, + Annotations: message.Annotations, + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + DisplayName: message.Name, + SecretName: guid, + Type: korifiv1alpha1.UserProvidedType, + Tags: message.Tags, + }, + } err = userClient.Create(ctx, cfServiceInstance) if err != nil { return ServiceInstanceRecord{}, apierrors.FromK8sError(err, ServiceInstanceResourceType) @@ -150,6 +178,42 @@ func (r *ServiceInstanceRepo) CreateServiceInstance(ctx context.Context, authInf return cfServiceInstanceToRecord(*cfServiceInstance), nil } +func (r *ServiceInstanceRepo) CreateManagedServiceInstance(ctx context.Context, authInfo authorization.Info, message CreateManagedSIMessage) (ServiceInstanceRecord, error) { + userClient, err := r.userClientFactory.BuildClient(authInfo) + if err != nil { + return ServiceInstanceRecord{}, fmt.Errorf("failed to build user client: %w", err) + } + + parameterBytes, err := json.Marshal(message.Parameters) + if err != nil { + return ServiceInstanceRecord{}, fmt.Errorf("failed to marshal parameters: %w", err) + } + + cfServiceInstance := &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: message.SpaceGUID, + Labels: message.Labels, + Annotations: message.Annotations, + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + DisplayName: message.Name, + Type: korifiv1alpha1.ManagedType, + PlanGUID: message.PlanGUID, + Tags: message.Tags, + Parameters: &runtime.RawExtension{ + Raw: parameterBytes, + }, + }, + } + err = userClient.Create(ctx, cfServiceInstance) + if err != nil { + return ServiceInstanceRecord{}, apierrors.FromK8sError(err, ServiceInstanceResourceType) + } + + return cfServiceInstanceToRecord(*cfServiceInstance), nil +} + func (r *ServiceInstanceRepo) PatchServiceInstance(ctx context.Context, authInfo authorization.Info, message PatchServiceInstanceMessage) (ServiceInstanceRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { @@ -324,22 +388,21 @@ func (r *ServiceInstanceRepo) DeleteServiceInstance(ctx context.Context, authInf return nil } -func (m CreateServiceInstanceMessage) toCFServiceInstance() *korifiv1alpha1.CFServiceInstance { - guid := uuid.NewString() - return &korifiv1alpha1.CFServiceInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: guid, - Namespace: m.SpaceGUID, - Labels: m.Labels, - Annotations: m.Annotations, - }, - Spec: korifiv1alpha1.CFServiceInstanceSpec{ - DisplayName: m.Name, - SecretName: guid, - Type: korifiv1alpha1.InstanceType(m.Type), - Tags: m.Tags, - }, +func (r ServiceInstanceRecord) GetResourceType() string { + return ServiceInstanceResourceType +} + +func (r *ServiceInstanceRepo) GetState(ctx context.Context, authInfo authorization.Info, guid string) (model.CFResourceState, error) { + instanceRecord, err := r.GetServiceInstance(ctx, authInfo, guid) + if err != nil { + return model.CFResourceStateUnknown, err } + + if instanceRecord.Ready { + return model.CFResourceStateReady, nil + } + + return model.CFResourceStateUnknown, nil } func cfServiceInstanceToRecord(cfServiceInstance korifiv1alpha1.CFServiceInstance) ServiceInstanceRecord { @@ -355,5 +418,14 @@ func cfServiceInstanceToRecord(cfServiceInstance korifiv1alpha1.CFServiceInstanc Annotations: cfServiceInstance.Annotations, CreatedAt: cfServiceInstance.CreationTimestamp.Time, UpdatedAt: getLastUpdatedTime(&cfServiceInstance), + Ready: isReady(cfServiceInstance), } } + +func isReady(cfServiceInstance korifiv1alpha1.CFServiceInstance) bool { + if cfServiceInstance.Generation != cfServiceInstance.Status.ObservedGeneration { + return false + } + + return meta.IsStatusConditionTrue(cfServiceInstance.Status.Conditions, korifiv1alpha1.StatusConditionReady) +} diff --git a/api/repositories/service_instance_repository_test.go b/api/repositories/service_instance_repository_test.go index 48f0db16c..1bb89230a 100644 --- a/api/repositories/service_instance_repository_test.go +++ b/api/repositories/service_instance_repository_test.go @@ -11,6 +11,7 @@ import ( "code.cloudfoundry.org/korifi/api/repositories" "code.cloudfoundry.org/korifi/api/repositories/fakeawaiter" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/model" "code.cloudfoundry.org/korifi/tests/matchers" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" @@ -22,13 +23,13 @@ import ( . "github.com/onsi/gomega/gstruct" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) var _ = Describe("ServiceInstanceRepository", func() { var ( - testCtx context.Context serviceInstanceRepo *repositories.ServiceInstanceRepo conditionAwaiter *fakeawaiter.FakeAwaiter[ *korifiv1alpha1.CFServiceInstance, @@ -43,7 +44,6 @@ var _ = Describe("ServiceInstanceRepository", func() { ) BeforeEach(func() { - testCtx = context.Background() conditionAwaiter = &fakeawaiter.FakeAwaiter[ *korifiv1alpha1.CFServiceInstance, korifiv1alpha1.CFServiceInstance, @@ -52,72 +52,246 @@ var _ = Describe("ServiceInstanceRepository", func() { ]{} serviceInstanceRepo = repositories.NewServiceInstanceRepo(namespaceRetriever, userClientFactory, nsPerms, conditionAwaiter) - org = createOrgWithCleanup(testCtx, prefixedGUID("org")) - space = createSpaceWithCleanup(testCtx, org.Name, prefixedGUID("space1")) - serviceInstanceName = prefixedGUID("service-instance") + org = createOrgWithCleanup(ctx, uuid.NewString()) + space = createSpaceWithCleanup(ctx, org.Name, uuid.NewString()) + serviceInstanceName = uuid.NewString() }) - Describe("CreateServiceInstance", func() { + Describe("CreateUserProvidedServiceInstance", func() { var ( - serviceInstanceCreateMessage repositories.CreateServiceInstanceMessage - serviceInstanceTags []string - serviceInstanceCredentials map[string]any - - createdServiceInstanceRecord repositories.ServiceInstanceRecord + serviceInstanceCreateMessage repositories.CreateUPSIMessage + record repositories.ServiceInstanceRecord createErr error ) BeforeEach(func() { - serviceInstanceTags = []string{"foo", "bar"} - serviceInstanceCredentials = map[string]any{ - "object": map[string]any{"a": "b"}, + serviceInstanceCreateMessage = repositories.CreateUPSIMessage{ + Name: serviceInstanceName, + SpaceGUID: space.Name, + Credentials: map[string]any{ + "object": map[string]any{"a": "b"}, + }, + Tags: []string{"foo", "bar"}, } - - serviceInstanceCreateMessage = initializeServiceInstanceCreateMessage(serviceInstanceName, space.Name, serviceInstanceTags, serviceInstanceCredentials) }) JustBeforeEach(func() { - createdServiceInstanceRecord, createErr = serviceInstanceRepo.CreateServiceInstance(testCtx, authInfo, serviceInstanceCreateMessage) + record, createErr = serviceInstanceRepo.CreateUserProvidedServiceInstance(ctx, authInfo, serviceInstanceCreateMessage) }) - When("user has permissions to create ServiceInstances", func() { - var createdSecret *corev1.Secret + It("returns a Forbidden error", func() { + Expect(createErr).To(BeAssignableToTypeOf(apierrors.ForbiddenError{})) + }) + When("user has permissions to create ServiceInstances", func() { BeforeEach(func() { - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) }) - JustBeforeEach(func() { + It("returns a service instance record", func() { Expect(createErr).NotTo(HaveOccurred()) - secretLookupKey := types.NamespacedName{Name: createdServiceInstanceRecord.SecretName, Namespace: createdServiceInstanceRecord.SpaceGUID} - createdSecret = &corev1.Secret{} - Expect(k8sClient.Get(context.Background(), secretLookupKey, createdSecret)).To(Succeed()) + Expect(record.GUID).NotTo(BeEmpty()) + Expect(record.SpaceGUID).To(Equal(space.Name)) + Expect(record.Name).To(Equal(serviceInstanceName)) + Expect(record.Type).To(Equal("user-provided")) + Expect(record.Tags).To(ConsistOf([]string{"foo", "bar"})) + Expect(record.SecretName).NotTo(BeEmpty()) + Expect(record.Relationships()).To(Equal(map[string]string{ + "space": space.Name, + })) + + Expect(record.CreatedAt).To(BeTemporally("~", time.Now(), timeCheckThreshold)) + Expect(record.UpdatedAt).To(PointTo(BeTemporally("~", time.Now(), timeCheckThreshold))) }) - It("creates a new ServiceInstance CR", func() { - Expect(createdServiceInstanceRecord.GUID).To(MatchRegexp("^[-0-9a-f]{36}$"), "record GUID was not a 36 character guid") - Expect(createdServiceInstanceRecord.SpaceGUID).To(Equal(space.Name), "SpaceGUID in record did not match input") - Expect(createdServiceInstanceRecord.Name).To(Equal(serviceInstanceName), "Name in record did not match input") - Expect(createdServiceInstanceRecord.Type).To(Equal("user-provided"), "Type in record did not match input") - Expect(createdServiceInstanceRecord.Tags).To(ConsistOf([]string{"foo", "bar"}), "Tags in record did not match input") + It("creates a CFServiceInstance resource", func() { + Expect(createErr).NotTo(HaveOccurred()) + + cfServiceInstance := &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: record.SpaceGUID, + Name: record.GUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceInstance), cfServiceInstance)).To(Succeed()) - Expect(createdServiceInstanceRecord.CreatedAt).To(BeTemporally("~", time.Now(), timeCheckThreshold)) - Expect(createdServiceInstanceRecord.UpdatedAt).To(PointTo(BeTemporally("~", time.Now(), timeCheckThreshold))) + Expect(cfServiceInstance.Spec.DisplayName).To(Equal(serviceInstanceName)) + Expect(cfServiceInstance.Spec.SecretName).NotTo(BeEmpty()) + Expect(cfServiceInstance.Spec.Type).To(BeEquivalentTo(korifiv1alpha1.UserProvidedType)) + Expect(cfServiceInstance.Spec.Tags).To(ConsistOf("foo", "bar")) }) It("creates the credentials secret", func() { - Expect(createdSecret.Type).To(Equal(corev1.SecretTypeOpaque)) - Expect(createdSecret.Data).To(MatchAllKeys(Keys{tools.CredentialsSecretKey: Not(BeEmpty())})) + Expect(createErr).NotTo(HaveOccurred()) + + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: record.SpaceGUID, + Name: record.SecretName, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed()) + + Expect(credentialsSecret.Type).To(Equal(corev1.SecretTypeOpaque)) + Expect(credentialsSecret.Data).To(MatchAllKeys(Keys{tools.CredentialsSecretKey: Not(BeEmpty())})) credentials := map[string]any{} - Expect(json.Unmarshal(createdSecret.Data[tools.CredentialsSecretKey], &credentials)).To(Succeed()) - Expect(credentials).To(Equal(serviceInstanceCredentials)) + Expect(json.Unmarshal(credentialsSecret.Data[tools.CredentialsSecretKey], &credentials)).To(Succeed()) + Expect(credentials).To(Equal(map[string]any{ + "object": map[string]any{"a": "b"}, + })) + }) + }) + }) + + Describe("CreateManagedServiceInstance", func() { + var ( + serviceInstanceCreateMessage repositories.CreateManagedSIMessage + record repositories.ServiceInstanceRecord + createErr error + ) + + BeforeEach(func() { + serviceInstanceCreateMessage = repositories.CreateManagedSIMessage{ + Name: serviceInstanceName, + SpaceGUID: space.Name, + PlanGUID: "plan-guid", + Tags: []string{"foo", "bar"}, + Parameters: map[string]any{ + "p1": map[string]any{ + "p11": "v11", + }, + }, + } + }) + + JustBeforeEach(func() { + record, createErr = serviceInstanceRepo.CreateManagedServiceInstance(ctx, authInfo, serviceInstanceCreateMessage) + }) + + It("returns a Forbidden error", func() { + Expect(createErr).To(BeAssignableToTypeOf(apierrors.ForbiddenError{})) + }) + + When("user has permissions to create ServiceInstances", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) + }) + + It("returns a service instance record", func() { + Expect(createErr).NotTo(HaveOccurred()) + + Expect(record.GUID).NotTo(BeEmpty()) + Expect(record.SpaceGUID).To(Equal(space.Name)) + Expect(record.Name).To(Equal(serviceInstanceName)) + Expect(record.Type).To(Equal("managed")) + Expect(record.Tags).To(ConsistOf([]string{"foo", "bar"})) + Expect(record.SecretName).To(BeEmpty()) + Expect(record.Relationships()).To(Equal(map[string]string{ + "service_plan": "plan-guid", + "space": space.Name, + })) + Expect(record.CreatedAt).To(BeTemporally("~", time.Now(), timeCheckThreshold)) + Expect(record.UpdatedAt).To(PointTo(BeTemporally("~", time.Now(), timeCheckThreshold))) }) + + It("creates a CFServiceInstance resource", func() { + Expect(createErr).NotTo(HaveOccurred()) + + cfServiceInstance := &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: record.SpaceGUID, + Name: record.GUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceInstance), cfServiceInstance)).To(Succeed()) + + Expect(cfServiceInstance.Spec.DisplayName).To(Equal(serviceInstanceName)) + Expect(cfServiceInstance.Spec.SecretName).To(BeEmpty()) + Expect(cfServiceInstance.Spec.Type).To(BeEquivalentTo(korifiv1alpha1.ManagedType)) + Expect(cfServiceInstance.Spec.Tags).To(ConsistOf("foo", "bar")) + Expect(cfServiceInstance.Spec.PlanGUID).To(Equal("plan-guid")) + Expect(cfServiceInstance.Spec.Parameters).NotTo(BeNil()) + + actualParams := map[string]any{} + Expect(json.Unmarshal(cfServiceInstance.Spec.Parameters.Raw, &actualParams)).To(Succeed()) + Expect(actualParams).To(Equal(map[string]any{ + "p1": map[string]any{ + "p11": "v11", + }, + })) + }) + }) + }) + + Describe("GetState", func() { + var ( + cfServiceInstance *korifiv1alpha1.CFServiceInstance + state model.CFResourceState + stateErr error + ) + BeforeEach(func() { + cfServiceInstance = &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: space.Name, + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + Type: "managed", + }, + } + + Expect(k8sClient.Create(ctx, cfServiceInstance)).To(Succeed()) + }) + + JustBeforeEach(func() { + state, stateErr = serviceInstanceRepo.GetState(ctx, authInfo, cfServiceInstance.Name) + }) + + It("returns a forbidden error", func() { + Expect(stateErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) }) - When("user does not have permissions to create ServiceInstances", func() { - It("returns a Forbidden error", func() { - Expect(createErr).To(BeAssignableToTypeOf(apierrors.ForbiddenError{})) + When("the user can get CFServiceInstance", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, cfServiceInstance.Namespace) + }) + + It("returns unknown state", func() { + Expect(stateErr).NotTo(HaveOccurred()) + Expect(state).To(Equal(model.CFResourceStateUnknown)) + }) + + When("the service instance is ready", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, k8sClient, cfServiceInstance, func() { + meta.SetStatusCondition(&cfServiceInstance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.StatusConditionReady, + Status: metav1.ConditionTrue, + Message: "Ready", + Reason: "Ready", + }) + cfServiceInstance.Status.ObservedGeneration = cfServiceInstance.Generation + })).To(Succeed()) + }) + + It("returns ready state", func() { + Expect(stateErr).NotTo(HaveOccurred()) + Expect(state).To(Equal(model.CFResourceStateReady)) + }) + + When("the ready status is stale ", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, k8sClient, cfServiceInstance, func() { + cfServiceInstance.Status.ObservedGeneration = -1 + })).To(Succeed()) + }) + + It("returns unknown state", func() { + Expect(stateErr).NotTo(HaveOccurred()) + Expect(state).To(Equal(model.CFResourceStateUnknown)) + }) + }) }) }) }) @@ -165,13 +339,13 @@ var _ = Describe("ServiceInstanceRepository", func() { }) JustBeforeEach(func() { - serviceInstanceRecord, err = serviceInstanceRepo.PatchServiceInstance(testCtx, authInfo, patchMessage) + serviceInstanceRecord, err = serviceInstanceRepo.PatchServiceInstance(ctx, authInfo, patchMessage) }) When("authorized in the space", func() { BeforeEach(func() { - createRoleBinding(testCtx, userName, orgUserRole.Name, org.Name) - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space.Name) + createRoleBinding(ctx, userName, orgUserRole.Name, org.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) }) It("returns the updated record", func() { @@ -329,7 +503,7 @@ var _ = Describe("ServiceInstanceRepository", func() { ) BeforeEach(func() { - space2 = createSpaceWithCleanup(testCtx, org.Name, prefixedGUID("space2")) + space2 = createSpaceWithCleanup(ctx, org.Name, prefixedGUID("space2")) cfServiceInstance1 = &korifiv1alpha1.CFServiceInstance{ ObjectMeta: metav1.ObjectMeta{ @@ -367,7 +541,7 @@ var _ = Describe("ServiceInstanceRepository", func() { } Expect(k8sClient.Create(ctx, cfServiceInstance3)).To(Succeed()) - space3 := createSpaceWithCleanup(testCtx, org.Name, prefixedGUID("space3")) + space3 := createSpaceWithCleanup(ctx, org.Name, prefixedGUID("space3")) Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServiceInstance{ ObjectMeta: metav1.ObjectMeta{ Namespace: space3.Name, @@ -381,9 +555,10 @@ var _ = Describe("ServiceInstanceRepository", func() { nonCFNamespace := uuid.NewString() Expect(k8sClient.Create( - testCtx, + ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: nonCFNamespace}}, )).To(Succeed()) + Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServiceInstance{ ObjectMeta: metav1.ObjectMeta{ Namespace: nonCFNamespace, @@ -399,7 +574,7 @@ var _ = Describe("ServiceInstanceRepository", func() { }) JustBeforeEach(func() { - serviceInstanceList, listErr = serviceInstanceRepo.ListServiceInstances(testCtx, authInfo, filters) + serviceInstanceList, listErr = serviceInstanceRepo.ListServiceInstances(ctx, authInfo, filters) }) It("returns an empty list of ServiceInstanceRecord", func() { @@ -409,8 +584,8 @@ var _ = Describe("ServiceInstanceRepository", func() { When("user is allowed to list service instances ", func() { BeforeEach(func() { - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space.Name) - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space2.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space2.Name) }) It("returns the service instances from the allowed namespaces", func() { @@ -534,15 +709,15 @@ var _ = Describe("ServiceInstanceRepository", func() { ) BeforeEach(func() { - space2 = createSpaceWithCleanup(testCtx, org.Name, prefixedGUID("space2")) + space2 = createSpaceWithCleanup(ctx, org.Name, prefixedGUID("space2")) - serviceInstance = createServiceInstanceCR(testCtx, k8sClient, prefixedGUID("service-instance"), space.Name, "the-service-instance", prefixedGUID("secret")) - createServiceInstanceCR(testCtx, k8sClient, prefixedGUID("service-instance"), space2.Name, "some-other-service-instance", prefixedGUID("secret")) + serviceInstance = createServiceInstanceCR(ctx, k8sClient, prefixedGUID("service-instance"), space.Name, "the-service-instance", prefixedGUID("secret")) + createServiceInstanceCR(ctx, k8sClient, prefixedGUID("service-instance"), space2.Name, "some-other-service-instance", prefixedGUID("secret")) getGUID = serviceInstance.Name }) JustBeforeEach(func() { - record, getErr = serviceInstanceRepo.GetServiceInstance(testCtx, authInfo, getGUID) + record, getErr = serviceInstanceRepo.GetServiceInstance(ctx, authInfo, getGUID) }) When("there are no permissions on service instances", func() { @@ -553,8 +728,8 @@ var _ = Describe("ServiceInstanceRepository", func() { When("the user has permissions to get the service instance", func() { BeforeEach(func() { - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space.Name) - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space2.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space2.Name) }) It("returns the correct service instance", func() { @@ -604,9 +779,9 @@ var _ = Describe("ServiceInstanceRepository", func() { When("more than one service instance with the same guid exists", func() { BeforeEach(func() { - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space.Name) - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space2.Name) - createServiceInstanceCR(testCtx, k8sClient, getGUID, space2.Name, "the-service-instance", prefixedGUID("secret")) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space2.Name) + createServiceInstanceCR(ctx, k8sClient, getGUID, space2.Name, "the-service-instance", prefixedGUID("secret")) }) It("returns a error", func() { @@ -617,7 +792,7 @@ var _ = Describe("ServiceInstanceRepository", func() { When("the context has expired", func() { BeforeEach(func() { var cancel context.CancelFunc - testCtx, cancel = context.WithCancel(testCtx) + ctx, cancel = context.WithCancel(ctx) cancel() }) @@ -635,7 +810,7 @@ var _ = Describe("ServiceInstanceRepository", func() { ) BeforeEach(func() { - serviceInstance = createServiceInstanceCR(testCtx, k8sClient, prefixedGUID("service-instance"), space.Name, "the-service-instance", prefixedGUID("secret")) + serviceInstance = createServiceInstanceCR(ctx, k8sClient, prefixedGUID("service-instance"), space.Name, "the-service-instance", prefixedGUID("secret")) deleteMessage = repositories.DeleteServiceInstanceMessage{ GUID: serviceInstance.Name, @@ -644,12 +819,12 @@ var _ = Describe("ServiceInstanceRepository", func() { }) JustBeforeEach(func() { - deleteErr = serviceInstanceRepo.DeleteServiceInstance(testCtx, authInfo, deleteMessage) + deleteErr = serviceInstanceRepo.DeleteServiceInstance(ctx, authInfo, deleteMessage) }) When("the user has permissions to delete service instances", func() { BeforeEach(func() { - createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) }) It("deletes the service instance", func() { @@ -682,13 +857,3 @@ var _ = Describe("ServiceInstanceRepository", func() { }) }) }) - -func initializeServiceInstanceCreateMessage(serviceInstanceName string, spaceGUID string, tags []string, credentials map[string]any) repositories.CreateServiceInstanceMessage { - return repositories.CreateServiceInstanceMessage{ - Name: serviceInstanceName, - SpaceGUID: spaceGUID, - Type: "user-provided", - Credentials: credentials, - Tags: tags, - } -} diff --git a/api/repositories/service_offering_repository_test.go b/api/repositories/service_offering_repository_test.go index c3fe6c889..ecfbe8690 100644 --- a/api/repositories/service_offering_repository_test.go +++ b/api/repositories/service_offering_repository_test.go @@ -75,7 +75,7 @@ var _ = Describe("ServiceOfferingRepo", func() { Requires: []string{"r1"}, DocumentationURL: tools.PtrTo("https://my.offering.com"), BrokerCatalog: services.ServiceBrokerCatalog{ - Id: "offering-catalog-guid", + ID: "offering-catalog-guid", Metadata: &runtime.RawExtension{ Raw: []byte(`{"offering-md": "offering-md-value"}`), }, diff --git a/api/repositories/service_plan_repository.go b/api/repositories/service_plan_repository.go index 5508bf8e0..e67dc6796 100644 --- a/api/repositories/service_plan_repository.go +++ b/api/repositories/service_plan_repository.go @@ -53,6 +53,7 @@ type ListServicePlanMessage struct { ServiceOfferingGUIDs []string ServiceOfferingNames []string BrokerNames []string + BrokerGUIDs []string Available *bool } @@ -61,6 +62,7 @@ func (m *ListServicePlanMessage) matches(cfServicePlan korifiv1alpha1.CFServiceP tools.EmptyOrContains(m.GUIDs, cfServicePlan.Name) && tools.EmptyOrContains(m.Names, cfServicePlan.Spec.Name) && tools.EmptyOrContains(m.BrokerNames, cfServicePlan.Labels[korifiv1alpha1.RelServiceBrokerNameLabel]) && + tools.EmptyOrContains(m.BrokerGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel]) && tools.EmptyOrContains(m.ServiceOfferingNames, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingNameLabel]) && tools.NilOrEquals(m.Available, isAvailable(cfServicePlan)) } diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index c6b28e4a7..abf1da1d4 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -43,6 +43,7 @@ var _ = Describe("ServicePlanRepo", func() { korifiv1alpha1.RelServiceOfferingGUIDLabel: "offering-guid", korifiv1alpha1.RelServiceBrokerNameLabel: "broker-name", korifiv1alpha1.RelServiceOfferingNameLabel: "offering-name", + korifiv1alpha1.RelServiceBrokerGUIDLabel: "broker-guid", }, Annotations: map[string]string{ "annotation": "annotation-value", @@ -195,6 +196,7 @@ var _ = Describe("ServicePlanRepo", func() { korifiv1alpha1.RelServiceOfferingGUIDLabel: "other-offering-guid", korifiv1alpha1.RelServiceBrokerNameLabel: "other-broker-name", korifiv1alpha1.RelServiceOfferingNameLabel: "other-offering-name", + korifiv1alpha1.RelServiceBrokerGUIDLabel: "other-broker-guid", }, }, Spec: korifiv1alpha1.CFServicePlanSpec{ @@ -301,6 +303,21 @@ var _ = Describe("ServicePlanRepo", func() { }) }) + When("filtering by broker guid", func() { + BeforeEach(func() { + message.BrokerGUIDs = []string{"other-broker-guid"} + }) + + It("returns matching service plans", func() { + Expect(listErr).NotTo(HaveOccurred()) + Expect(listedPlans).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ + "CFResource": MatchFields(IgnoreExtras, Fields{ + "GUID": Equal(otherPlanGUID), + }), + }))) + }) + }) + When("filtering by service offering name", func() { BeforeEach(func() { message.ServiceOfferingNames = []string{"other-offering-name"} diff --git a/controllers/api/v1alpha1/cfserviceinstance_types.go b/controllers/api/v1alpha1/cfserviceinstance_types.go index b66c7805e..beb4ecc49 100644 --- a/controllers/api/v1alpha1/cfserviceinstance_types.go +++ b/controllers/api/v1alpha1/cfserviceinstance_types.go @@ -21,11 +21,17 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtime "k8s.io/apimachinery/pkg/runtime" ) const ( UserProvidedType = "user-provided" ManagedType = "managed" + + CFManagedServiceInstanceFinalizerName = "managed.cfServiceInstance.korifi.cloudfoundry.org" + + ProvisionRequestedCondition = "ProvisionRequested" + ProvisioningFailedCondition = "ProvisioningFailed" ) // CFServiceInstanceSpec defines the desired state of CFServiceInstance @@ -48,6 +54,8 @@ type CFServiceInstanceSpec struct { Tags []string `json:"tags,omitempty"` PlanGUID string `json:"plan_guid"` + + Parameters *runtime.RawExtension `json:"parameters,omitempty"` } // InstanceType defines the type of the Service Instance @@ -71,6 +79,8 @@ type CFServiceInstanceStatus struct { // This will ensure that interested contollers are notified on instance credentials change //+kubebuilder:validation:Optional CredentialsObservedVersion string `json:"credentialsObservedVersion,omitempty"` + + ProvisionOperation string `json:"provisionOperation,omitempty"` } //+kubebuilder:object:root=true diff --git a/controllers/api/v1alpha1/zz_generated.deepcopy.go b/controllers/api/v1alpha1/zz_generated.deepcopy.go index e9f1fb879..db18adc48 100644 --- a/controllers/api/v1alpha1/zz_generated.deepcopy.go +++ b/controllers/api/v1alpha1/zz_generated.deepcopy.go @@ -1451,6 +1451,11 @@ func (in *CFServiceInstanceSpec) DeepCopyInto(out *CFServiceInstanceSpec) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.Parameters != nil { + in, out := &in.Parameters, &out.Parameters + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CFServiceInstanceSpec. diff --git a/controllers/controllers/services/brokers/controller.go b/controllers/controllers/services/brokers/controller.go index 1199cef94..5185be6de 100644 --- a/controllers/controllers/services/brokers/controller.go +++ b/controllers/controllers/services/brokers/controller.go @@ -23,7 +23,7 @@ import ( "time" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers/osbapi" + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/model/services" "code.cloudfoundry.org/korifi/tools" @@ -43,25 +43,33 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -type CatalogClient interface { - GetCatalog(context.Context, *korifiv1alpha1.CFServiceBroker) (*osbapi.Catalog, error) -} +//counterfeiter:generate -o fake -fake-name BrokerClient code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi.BrokerClient + +//counterfeiter:generate -o fake -fake-name BrokerClientFactory code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi.BrokerClientFactory type Reconciler struct { - k8sClient client.Client - catalogClient CatalogClient - scheme *runtime.Scheme - log logr.Logger + k8sClient client.Client + osbapiClientFactory osbapi.BrokerClientFactory + scheme *runtime.Scheme + log logr.Logger } func NewReconciler( client client.Client, - catalogClient CatalogClient, + osbapiClientFactory osbapi.BrokerClientFactory, scheme *runtime.Scheme, log logr.Logger, ) *k8s.PatchingReconciler[korifiv1alpha1.CFServiceBroker, *korifiv1alpha1.CFServiceBroker] { - serviceInstanceReconciler := Reconciler{k8sClient: client, catalogClient: catalogClient, scheme: scheme, log: log} - return k8s.NewPatchingReconciler(log, client, &serviceInstanceReconciler) + return k8s.NewPatchingReconciler( + log, + client, + &Reconciler{ + k8sClient: client, + osbapiClientFactory: osbapiClientFactory, + scheme: scheme, + log: log, + }, + ) } func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder { @@ -122,14 +130,15 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBroker *kor return ctrl.Result{}, notReadyErr } - if err = r.validateCredentials(credentialsSecret); err != nil { - return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("SecretInvalid") - } - log.V(1).Info("credentials secret", "name", credentialsSecret.Name, "version", credentialsSecret.ResourceVersion) cfServiceBroker.Status.CredentialsObservedVersion = credentialsSecret.ResourceVersion - catalog, err := r.catalogClient.GetCatalog(ctx, cfServiceBroker) + osbapiClient, err := r.osbapiClientFactory.CreateClient(ctx, cfServiceBroker) + if err != nil { + return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("OSBAPIClientCreationFailed") + } + + catalog, err := osbapiClient.GetCatalog(ctx) if err != nil { log.Error(err, "failed to get catalog from broker", "broker", cfServiceBroker.Name) return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("GetCatalogFailed") @@ -144,23 +153,7 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBroker *kor return ctrl.Result{}, nil } -func (r *Reconciler) validateCredentials(credentialsSecret *corev1.Secret) error { - creds := map[string]any{} - err := json.Unmarshal(credentialsSecret.Data[tools.CredentialsSecretKey], &creds) - if err != nil { - return fmt.Errorf("invalid credentials secret %q: %w", credentialsSecret.Name, err) - } - - for _, k := range []string{korifiv1alpha1.UsernameCredentialsKey, korifiv1alpha1.PasswordCredentialsKey} { - if _, ok := creds[k]; !ok { - return fmt.Errorf("broker credentials secret %q does not specify %q", credentialsSecret.Name, k) - } - } - - return nil -} - -func (r *Reconciler) reconcileCatalog(ctx context.Context, cfServiceBroker *korifiv1alpha1.CFServiceBroker, catalog *osbapi.Catalog) error { +func (r *Reconciler) reconcileCatalog(ctx context.Context, cfServiceBroker *korifiv1alpha1.CFServiceBroker, catalog osbapi.Catalog) error { for _, service := range catalog.Services { err := r.reconcileCatalogService(ctx, cfServiceBroker, service) if err != nil { @@ -266,7 +259,7 @@ func toSpecServiceOffering(catalogService osbapi.Service) (services.ServiceOffer Tags: catalogService.Tags, Requires: catalogService.Requires, BrokerCatalog: services.ServiceBrokerCatalog{ - Id: catalogService.ID, + ID: catalogService.ID, Features: catalogService.BrokerCatalogFeatures, }, } diff --git a/controllers/controllers/services/brokers/controller_test.go b/controllers/controllers/services/brokers/controller_test.go index 98ca64101..9e2c0d34e 100644 --- a/controllers/controllers/services/brokers/controller_test.go +++ b/controllers/controllers/services/brokers/controller_test.go @@ -1,15 +1,19 @@ package brokers_test import ( + "errors" + "slices" + + "github.com/BooleanCat/go-functional/v2/it" + "github.com/BooleanCat/go-functional/v2/it/itx" "github.com/google/uuid" "sigs.k8s.io/controller-runtime/pkg/client" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers/osbapi" + "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers/fake" + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" "code.cloudfoundry.org/korifi/model/services" - "code.cloudfoundry.org/korifi/tests/helpers/broker" . "code.cloudfoundry.org/korifi/tests/matchers" - "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -22,20 +26,15 @@ import ( var _ = Describe("CFServiceBroker", func() { var ( - testNamespace string - serviceBroker *korifiv1alpha1.CFServiceBroker - credentialsSecret *corev1.Secret + brokerClient *fake.BrokerClient + brokerSecret *corev1.Secret + serviceBroker *korifiv1alpha1.CFServiceBroker ) BeforeEach(func() { - testNamespace = uuid.NewString() - Expect(adminClient.Create(ctx, &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: testNamespace, - }, - })).To(Succeed()) - - brokerServer := broker.NewServer().WithCatalog(&osbapi.Catalog{ + brokerClient = new(fake.BrokerClient) + brokerClientFactory.CreateClientReturns(brokerClient, nil) + brokerClient.GetCatalogReturns(osbapi.Catalog{ Services: []osbapi.Service{{ ID: "service-id", Name: "service-name", @@ -87,35 +86,28 @@ var _ = Describe("CFServiceBroker", func() { }, }}, }}, - }).Start() - - DeferCleanup(func() { - brokerServer.Stop() - }) + }, nil) - credentialsSecret = &corev1.Secret{ + brokerSecret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, Name: uuid.NewString(), - Namespace: testNamespace, - }, - Data: map[string][]byte{ - tools.CredentialsSecretKey: []byte(`{"username": "broker-user", "password": "broker-password"}`), }, } - Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed()) + Expect(adminClient.Create(ctx, brokerSecret)).To(Succeed()) serviceBroker = &korifiv1alpha1.CFServiceBroker{ ObjectMeta: metav1.ObjectMeta{ - Namespace: testNamespace, + Namespace: rootNamespace, Name: uuid.NewString(), }, Spec: korifiv1alpha1.CFServiceBrokerSpec{ ServiceBroker: services.ServiceBroker{ Name: "my-service-broker", - URL: brokerServer.URL(), + URL: "some-url", }, Credentials: corev1.LocalObjectReference{ - Name: credentialsSecret.Name, + Name: brokerSecret.Name, }, }, } @@ -142,7 +134,10 @@ var _ = Describe("CFServiceBroker", func() { It("creates CFServiceOfferings to reflect the catalog offerings", func() { Eventually(func(g Gomega) { offerings := &korifiv1alpha1.CFServiceOfferingList{} - g.Expect(adminClient.List(ctx, offerings, client.InNamespace(serviceBroker.Namespace))).To(Succeed()) + g.Expect(adminClient.List(ctx, offerings, + client.InNamespace(serviceBroker.Namespace), + client.MatchingLabels{korifiv1alpha1.RelServiceBrokerGUIDLabel: serviceBroker.Name}, + )).To(Succeed()) g.Expect(offerings.Items).To(HaveLen(1)) offering := offerings.Items[0] @@ -158,7 +153,7 @@ var _ = Describe("CFServiceBroker", func() { "Requires": ConsistOf("r1"), "DocumentationURL": PointTo(Equal("https://doc.url")), "BrokerCatalog": MatchAllFields(Fields{ - "Id": Equal("service-id"), + "ID": Equal("service-id"), "Features": MatchAllFields(Fields{ "PlanUpdateable": BeTrue(), "Bindable": BeTrue(), @@ -181,11 +176,17 @@ var _ = Describe("CFServiceBroker", func() { It("creates CFServicePlans to reflect catalog plans", func() { Eventually(func(g Gomega) { offerings := &korifiv1alpha1.CFServiceOfferingList{} - g.Expect(adminClient.List(ctx, offerings, client.InNamespace(serviceBroker.Namespace))).To(Succeed()) + g.Expect(adminClient.List(ctx, offerings, + client.InNamespace(serviceBroker.Namespace), + client.MatchingLabels{korifiv1alpha1.RelServiceBrokerGUIDLabel: serviceBroker.Name}, + )).To(Succeed()) g.Expect(offerings.Items).To(HaveLen(1)) plans := &korifiv1alpha1.CFServicePlanList{} - g.Expect(adminClient.List(ctx, plans, client.InNamespace(serviceBroker.Namespace))).To(Succeed()) + g.Expect(adminClient.List(ctx, plans, + client.InNamespace(serviceBroker.Namespace), + client.MatchingLabels{korifiv1alpha1.RelServiceBrokerGUIDLabel: serviceBroker.Name}, + )).To(Succeed()) g.Expect(plans.Items).To(HaveLen(1)) plan := plans.Items[0] @@ -244,7 +245,7 @@ var _ = Describe("CFServiceBroker", func() { When("the plan visibility is updated", func() { var planGUID string - BeforeEach(func() { + JustBeforeEach(func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( @@ -254,58 +255,35 @@ var _ = Describe("CFServiceBroker", func() { }).Should(Succeed()) plans := &korifiv1alpha1.CFServicePlanList{} - Expect(adminClient.List(ctx, plans, client.InNamespace(serviceBroker.Namespace))).To(Succeed()) + Expect(adminClient.List(ctx, plans, + client.InNamespace(serviceBroker.Namespace), + client.MatchingLabels{korifiv1alpha1.RelServiceBrokerGUIDLabel: serviceBroker.Name}, + )).To(Succeed()) Expect(plans.Items).To(HaveLen(1)) - planGUID = plans.Items[0].Name - plan := &plans.Items[0] + planGUID = plan.Name + Expect(k8s.PatchResource(ctx, adminClient, plan, func() { plan.Spec.Visibility.Type = korifiv1alpha1.PublicServicePlanVisibilityType })).To(Succeed()) - }) - When("the controller reconciles the broker", func() { - BeforeEach(func() { - Expect(k8s.PatchResource(ctx, adminClient, serviceBroker, func() { - serviceBroker.Spec.Name = uuid.NewString() - })).To(Succeed()) - - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(serviceBroker.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) - g.Expect(serviceBroker.Generation).To(Equal(serviceBroker.Status.ObservedGeneration)) - }).Should(Succeed()) - }) - - It("keeps the updated value", func() { - plan := &korifiv1alpha1.CFServicePlan{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: serviceBroker.Namespace, - Name: planGUID, - }, - } - Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(plan), plan)).To(Succeed()) - Expect(plan.Spec.Visibility.Type).To(Equal(korifiv1alpha1.PublicServicePlanVisibilityType)) - }) - }) - }) - - When("getting the catalog fails", func() { - BeforeEach(func() { Expect(k8s.PatchResource(ctx, adminClient, serviceBroker, func() { - serviceBroker.Spec.URL = "https://must.not.exist" + serviceBroker.Spec.Name = uuid.NewString() })).To(Succeed()) }) - It("sets the ready condition to False", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("GetCatalogFailed")), - ))) + It("keeps the updated value", func() { + plan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: serviceBroker.Namespace, + Name: planGUID, + }, + } + + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(plan), plan)).To(Succeed()) + g.Expect(plan.Spec.Visibility.Type).To(Equal(korifiv1alpha1.PublicServicePlanVisibilityType)) }).Should(Succeed()) }) }) @@ -316,15 +294,12 @@ var _ = Describe("CFServiceBroker", func() { BeforeEach(func() { anotherServiceBroker = &korifiv1alpha1.CFServiceBroker{ ObjectMeta: metav1.ObjectMeta{ - Namespace: testNamespace, + Namespace: rootNamespace, Name: uuid.NewString(), }, Spec: korifiv1alpha1.CFServiceBrokerSpec{ - ServiceBroker: services.ServiceBroker{ - URL: serviceBroker.Spec.URL, - }, Credentials: corev1.LocalObjectReference{ - Name: credentialsSecret.Name, + Name: brokerSecret.Name, }, }, } @@ -334,177 +309,156 @@ var _ = Describe("CFServiceBroker", func() { It("creates an offering per broker", func() { Eventually(func(g Gomega) { offerings := &korifiv1alpha1.CFServiceOfferingList{} - g.Expect(adminClient.List(ctx, offerings, client.InNamespace(testNamespace))).To(Succeed()) - g.Expect(offerings.Items).To(HaveLen(2)) - - brokerGUIDs := []string{ - offerings.Items[0].Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel], - offerings.Items[1].Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel], - } - g.Expect(brokerGUIDs).To(ConsistOf(serviceBroker.Name, anotherServiceBroker.Name)) - - g.Expect(offerings.Items[0].Spec.BrokerCatalog.Id).To(Equal("service-id")) - g.Expect(offerings.Items[1].Spec.BrokerCatalog.Id).To(Equal("service-id")) + g.Expect(adminClient.List(ctx, offerings, client.InNamespace(rootNamespace))).To(Succeed()) + + brokerOfferings := slices.Collect(it.Filter(itx.FromSlice(offerings.Items), func(o korifiv1alpha1.CFServiceOffering) bool { + return o.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] == serviceBroker.Name + })) + g.Expect(brokerOfferings).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "Spec": MatchFields(IgnoreExtras, Fields{ + "ServiceOffering": MatchFields(IgnoreExtras, Fields{ + "BrokerCatalog": MatchFields(IgnoreExtras, Fields{ + "ID": Equal("service-id"), + }), + }), + }), + }), + )) + + anotherBrokerOfferings := slices.Collect(it.Filter(itx.FromSlice(offerings.Items), func(o korifiv1alpha1.CFServiceOffering) bool { + return o.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] == anotherServiceBroker.Name + })) + g.Expect(anotherBrokerOfferings).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "Spec": MatchFields(IgnoreExtras, Fields{ + "ServiceOffering": MatchFields(IgnoreExtras, Fields{ + "BrokerCatalog": MatchFields(IgnoreExtras, Fields{ + "ID": Equal("service-id"), + }), + }), + }), + }), + )) }).Should(Succeed()) }) It("creates a plan per broker", func() { Eventually(func(g Gomega) { plans := &korifiv1alpha1.CFServicePlanList{} - g.Expect(adminClient.List(ctx, plans, client.InNamespace(testNamespace))).To(Succeed()) - g.Expect(plans.Items).To(HaveLen(2)) - - brokerGUIDs := []string{ - plans.Items[0].Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel], - plans.Items[1].Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel], - } - g.Expect(brokerGUIDs).To(ConsistOf(serviceBroker.Name, anotherServiceBroker.Name)) - - g.Expect(plans.Items[0].Spec.BrokerCatalog.ID).To(Equal("plan-id")) - g.Expect(plans.Items[1].Spec.BrokerCatalog.ID).To(Equal("plan-id")) + g.Expect(adminClient.List(ctx, plans, client.InNamespace(serviceBroker.Namespace))).To(Succeed()) + + brokerPlans := slices.Collect(it.Filter(itx.FromSlice(plans.Items), func(o korifiv1alpha1.CFServicePlan) bool { + return o.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] == serviceBroker.Name + })) + g.Expect(brokerPlans).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "Spec": MatchFields(IgnoreExtras, Fields{ + "ServicePlan": MatchFields(IgnoreExtras, Fields{ + "BrokerCatalog": MatchFields(IgnoreExtras, Fields{ + "ID": Equal("plan-id"), + }), + }), + }), + }), + )) + + anotherBrokerPlans := slices.Collect(it.Filter(itx.FromSlice(plans.Items), func(o korifiv1alpha1.CFServicePlan) bool { + return o.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] == anotherServiceBroker.Name + })) + g.Expect(anotherBrokerPlans).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "Spec": MatchFields(IgnoreExtras, Fields{ + "ServicePlan": MatchFields(IgnoreExtras, Fields{ + "BrokerCatalog": MatchFields(IgnoreExtras, Fields{ + "ID": Equal("plan-id"), + }), + }), + }), + }), + )) }).Should(Succeed()) }) }) - Describe("credentials secret", func() { - var credentialsSecret *corev1.Secret + It("sets the credentials secret observed version", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) + g.Expect(serviceBroker.Status.CredentialsObservedVersion).NotTo(BeEmpty()) + }).Should(Succeed()) + }) + When("the credentials secret does not exist", func() { BeforeEach(func() { - credentialsSecret = &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: uuid.NewString(), - Namespace: testNamespace, - }, - Data: map[string][]byte{ - tools.CredentialsSecretKey: []byte(`{"username": "broker-user", "password": "broker-password"}`), - }, - } - Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed()) - Expect(k8s.PatchResource(ctx, adminClient, serviceBroker, func() { - serviceBroker.Spec.Credentials.Name = credentialsSecret.Name - })).To(Succeed()) + Expect(adminClient.Delete(ctx, brokerSecret)).To(Succeed()) }) - It("sets the Ready condition to true", func() { + It("sets the Ready condition to false", func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionTrue)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("CredentialsSecretNotAvailable")), ))) }).Should(Succeed()) }) + }) + + When("the credentials secret is updated", func() { + var credentialsObservedVersion string - It("sets the credentials secret observed version", func() { + JustBeforeEach(func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(serviceBroker.Status.CredentialsObservedVersion).NotTo(BeEmpty()) + g.Expect(meta.IsStatusConditionTrue(serviceBroker.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) + credentialsObservedVersion = serviceBroker.Status.CredentialsObservedVersion + + Expect(k8s.Patch(ctx, adminClient, brokerSecret, func() { + brokerSecret.StringData = map[string]string{"f": "b"} + })).To(Succeed()) }) - When("the credentials secret data does not have the credentials key", func() { - BeforeEach(func() { - Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { - credentialsSecret.Data = map[string][]byte{} - })).To(Succeed()) - }) - - It("sets the ready condition to false", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("SecretInvalid")), - ))) - }).Should(Succeed()) - }) + It("triggers the reconciler", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) + g.Expect(serviceBroker.Status.CredentialsObservedVersion).NotTo(Equal(credentialsObservedVersion)) + }).Should(Succeed()) }) + }) - When("the credentials secret data does not have username", func() { - BeforeEach(func() { - Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { - credentialsSecret.Data = map[string][]byte{ - tools.CredentialsSecretKey: []byte(`{ "password": "broker-password"}`), - } - })).To(Succeed()) - }) - - It("sets the ready condition to false", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("SecretInvalid")), - ))) - }).Should(Succeed()) - }) + When("osbapi client creation fails", func() { + BeforeEach(func() { + brokerClientFactory.CreateClientReturns(nil, errors.New("factory-err")) }) - When("the credentials secret data does not have password", func() { - BeforeEach(func() { - Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { - credentialsSecret.Data = map[string][]byte{ - tools.CredentialsSecretKey: []byte(`{ "username": "broker-username"}`), - } - })).To(Succeed()) - }) - - It("sets the ready condition to false", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("SecretInvalid")), - ))) - }).Should(Succeed()) - }) + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) + g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("OSBAPIClientCreationFailed")), + ))) + }).Should(Succeed()) }) + }) - When("the credentials secret does not exist", func() { - BeforeEach(func() { - Expect(adminClient.Delete(ctx, credentialsSecret)).To(Succeed()) - }) - - It("sets the Ready condition to false", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("CredentialsSecretNotAvailable")), - ))) - }).Should(Succeed()) - }) + When("getting the catalog fails", func() { + BeforeEach(func() { + brokerClient.GetCatalogReturns(osbapi.Catalog{}, errors.New("get-catalog-err")) }) - When("the credentials secret is reconciled", func() { - var credentialsObservedVersion string - - BeforeEach(func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(serviceBroker.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) - }).Should(Succeed()) - credentialsObservedVersion = serviceBroker.Status.CredentialsObservedVersion - }) - - When("the credentials secret changes", func() { - BeforeEach(func() { - Expect(k8s.Patch(ctx, adminClient, credentialsSecret, func() { - credentialsSecret.StringData = map[string]string{"f": "b"} - })).To(Succeed()) - }) - - It("updates the credentials secret observed version", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) - g.Expect(serviceBroker.Status.CredentialsObservedVersion).NotTo(Equal(credentialsObservedVersion)) - }).Should(Succeed()) - }) - }) + It("sets the ready condition to False", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker)).To(Succeed()) + g.Expect(serviceBroker.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("GetCatalogFailed")), + ))) + }).Should(Succeed()) }) }) }) diff --git a/controllers/controllers/services/brokers/fake/broker_client.go b/controllers/controllers/services/brokers/fake/broker_client.go new file mode 100644 index 000000000..a90be2e68 --- /dev/null +++ b/controllers/controllers/services/brokers/fake/broker_client.go @@ -0,0 +1,279 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package fake + +import ( + "context" + "sync" + + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" +) + +type BrokerClient struct { + GetCatalogStub func(context.Context) (osbapi.Catalog, error) + getCatalogMutex sync.RWMutex + getCatalogArgsForCall []struct { + arg1 context.Context + } + getCatalogReturns struct { + result1 osbapi.Catalog + result2 error + } + getCatalogReturnsOnCall map[int]struct { + result1 osbapi.Catalog + result2 error + } + GetServiceInstanceLastOperationStub func(context.Context, osbapi.GetLastOperationPayload) (osbapi.LastOperationResponse, error) + getServiceInstanceLastOperationMutex sync.RWMutex + getServiceInstanceLastOperationArgsForCall []struct { + arg1 context.Context + arg2 osbapi.GetLastOperationPayload + } + getServiceInstanceLastOperationReturns struct { + result1 osbapi.LastOperationResponse + result2 error + } + getServiceInstanceLastOperationReturnsOnCall map[int]struct { + result1 osbapi.LastOperationResponse + result2 error + } + ProvisionStub func(context.Context, osbapi.InstanceProvisionPayload) (osbapi.ProvisionServiceInstanceResponse, error) + provisionMutex sync.RWMutex + provisionArgsForCall []struct { + arg1 context.Context + arg2 osbapi.InstanceProvisionPayload + } + provisionReturns struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + } + provisionReturnsOnCall map[int]struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *BrokerClient) GetCatalog(arg1 context.Context) (osbapi.Catalog, error) { + fake.getCatalogMutex.Lock() + ret, specificReturn := fake.getCatalogReturnsOnCall[len(fake.getCatalogArgsForCall)] + fake.getCatalogArgsForCall = append(fake.getCatalogArgsForCall, struct { + arg1 context.Context + }{arg1}) + stub := fake.GetCatalogStub + fakeReturns := fake.getCatalogReturns + fake.recordInvocation("GetCatalog", []interface{}{arg1}) + fake.getCatalogMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClient) GetCatalogCallCount() int { + fake.getCatalogMutex.RLock() + defer fake.getCatalogMutex.RUnlock() + return len(fake.getCatalogArgsForCall) +} + +func (fake *BrokerClient) GetCatalogCalls(stub func(context.Context) (osbapi.Catalog, error)) { + fake.getCatalogMutex.Lock() + defer fake.getCatalogMutex.Unlock() + fake.GetCatalogStub = stub +} + +func (fake *BrokerClient) GetCatalogArgsForCall(i int) context.Context { + fake.getCatalogMutex.RLock() + defer fake.getCatalogMutex.RUnlock() + argsForCall := fake.getCatalogArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *BrokerClient) GetCatalogReturns(result1 osbapi.Catalog, result2 error) { + fake.getCatalogMutex.Lock() + defer fake.getCatalogMutex.Unlock() + fake.GetCatalogStub = nil + fake.getCatalogReturns = struct { + result1 osbapi.Catalog + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) GetCatalogReturnsOnCall(i int, result1 osbapi.Catalog, result2 error) { + fake.getCatalogMutex.Lock() + defer fake.getCatalogMutex.Unlock() + fake.GetCatalogStub = nil + if fake.getCatalogReturnsOnCall == nil { + fake.getCatalogReturnsOnCall = make(map[int]struct { + result1 osbapi.Catalog + result2 error + }) + } + fake.getCatalogReturnsOnCall[i] = struct { + result1 osbapi.Catalog + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) GetServiceInstanceLastOperation(arg1 context.Context, arg2 osbapi.GetLastOperationPayload) (osbapi.LastOperationResponse, error) { + fake.getServiceInstanceLastOperationMutex.Lock() + ret, specificReturn := fake.getServiceInstanceLastOperationReturnsOnCall[len(fake.getServiceInstanceLastOperationArgsForCall)] + fake.getServiceInstanceLastOperationArgsForCall = append(fake.getServiceInstanceLastOperationArgsForCall, struct { + arg1 context.Context + arg2 osbapi.GetLastOperationPayload + }{arg1, arg2}) + stub := fake.GetServiceInstanceLastOperationStub + fakeReturns := fake.getServiceInstanceLastOperationReturns + fake.recordInvocation("GetServiceInstanceLastOperation", []interface{}{arg1, arg2}) + fake.getServiceInstanceLastOperationMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationCallCount() int { + fake.getServiceInstanceLastOperationMutex.RLock() + defer fake.getServiceInstanceLastOperationMutex.RUnlock() + return len(fake.getServiceInstanceLastOperationArgsForCall) +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationCalls(stub func(context.Context, osbapi.GetLastOperationPayload) (osbapi.LastOperationResponse, error)) { + fake.getServiceInstanceLastOperationMutex.Lock() + defer fake.getServiceInstanceLastOperationMutex.Unlock() + fake.GetServiceInstanceLastOperationStub = stub +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationArgsForCall(i int) (context.Context, osbapi.GetLastOperationPayload) { + fake.getServiceInstanceLastOperationMutex.RLock() + defer fake.getServiceInstanceLastOperationMutex.RUnlock() + argsForCall := fake.getServiceInstanceLastOperationArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationReturns(result1 osbapi.LastOperationResponse, result2 error) { + fake.getServiceInstanceLastOperationMutex.Lock() + defer fake.getServiceInstanceLastOperationMutex.Unlock() + fake.GetServiceInstanceLastOperationStub = nil + fake.getServiceInstanceLastOperationReturns = struct { + result1 osbapi.LastOperationResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationReturnsOnCall(i int, result1 osbapi.LastOperationResponse, result2 error) { + fake.getServiceInstanceLastOperationMutex.Lock() + defer fake.getServiceInstanceLastOperationMutex.Unlock() + fake.GetServiceInstanceLastOperationStub = nil + if fake.getServiceInstanceLastOperationReturnsOnCall == nil { + fake.getServiceInstanceLastOperationReturnsOnCall = make(map[int]struct { + result1 osbapi.LastOperationResponse + result2 error + }) + } + fake.getServiceInstanceLastOperationReturnsOnCall[i] = struct { + result1 osbapi.LastOperationResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) Provision(arg1 context.Context, arg2 osbapi.InstanceProvisionPayload) (osbapi.ProvisionServiceInstanceResponse, error) { + fake.provisionMutex.Lock() + ret, specificReturn := fake.provisionReturnsOnCall[len(fake.provisionArgsForCall)] + fake.provisionArgsForCall = append(fake.provisionArgsForCall, struct { + arg1 context.Context + arg2 osbapi.InstanceProvisionPayload + }{arg1, arg2}) + stub := fake.ProvisionStub + fakeReturns := fake.provisionReturns + fake.recordInvocation("Provision", []interface{}{arg1, arg2}) + fake.provisionMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClient) ProvisionCallCount() int { + fake.provisionMutex.RLock() + defer fake.provisionMutex.RUnlock() + return len(fake.provisionArgsForCall) +} + +func (fake *BrokerClient) ProvisionCalls(stub func(context.Context, osbapi.InstanceProvisionPayload) (osbapi.ProvisionServiceInstanceResponse, error)) { + fake.provisionMutex.Lock() + defer fake.provisionMutex.Unlock() + fake.ProvisionStub = stub +} + +func (fake *BrokerClient) ProvisionArgsForCall(i int) (context.Context, osbapi.InstanceProvisionPayload) { + fake.provisionMutex.RLock() + defer fake.provisionMutex.RUnlock() + argsForCall := fake.provisionArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *BrokerClient) ProvisionReturns(result1 osbapi.ProvisionServiceInstanceResponse, result2 error) { + fake.provisionMutex.Lock() + defer fake.provisionMutex.Unlock() + fake.ProvisionStub = nil + fake.provisionReturns = struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) ProvisionReturnsOnCall(i int, result1 osbapi.ProvisionServiceInstanceResponse, result2 error) { + fake.provisionMutex.Lock() + defer fake.provisionMutex.Unlock() + fake.ProvisionStub = nil + if fake.provisionReturnsOnCall == nil { + fake.provisionReturnsOnCall = make(map[int]struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + }) + } + fake.provisionReturnsOnCall[i] = struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.getCatalogMutex.RLock() + defer fake.getCatalogMutex.RUnlock() + fake.getServiceInstanceLastOperationMutex.RLock() + defer fake.getServiceInstanceLastOperationMutex.RUnlock() + fake.provisionMutex.RLock() + defer fake.provisionMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *BrokerClient) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ osbapi.BrokerClient = new(BrokerClient) diff --git a/controllers/controllers/services/brokers/fake/broker_client_factory.go b/controllers/controllers/services/brokers/fake/broker_client_factory.go new file mode 100644 index 000000000..fba51a140 --- /dev/null +++ b/controllers/controllers/services/brokers/fake/broker_client_factory.go @@ -0,0 +1,120 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package fake + +import ( + "context" + "sync" + + "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" +) + +type BrokerClientFactory struct { + CreateClientStub func(context.Context, *v1alpha1.CFServiceBroker) (osbapi.BrokerClient, error) + createClientMutex sync.RWMutex + createClientArgsForCall []struct { + arg1 context.Context + arg2 *v1alpha1.CFServiceBroker + } + createClientReturns struct { + result1 osbapi.BrokerClient + result2 error + } + createClientReturnsOnCall map[int]struct { + result1 osbapi.BrokerClient + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *BrokerClientFactory) CreateClient(arg1 context.Context, arg2 *v1alpha1.CFServiceBroker) (osbapi.BrokerClient, error) { + fake.createClientMutex.Lock() + ret, specificReturn := fake.createClientReturnsOnCall[len(fake.createClientArgsForCall)] + fake.createClientArgsForCall = append(fake.createClientArgsForCall, struct { + arg1 context.Context + arg2 *v1alpha1.CFServiceBroker + }{arg1, arg2}) + stub := fake.CreateClientStub + fakeReturns := fake.createClientReturns + fake.recordInvocation("CreateClient", []interface{}{arg1, arg2}) + fake.createClientMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClientFactory) CreateClientCallCount() int { + fake.createClientMutex.RLock() + defer fake.createClientMutex.RUnlock() + return len(fake.createClientArgsForCall) +} + +func (fake *BrokerClientFactory) CreateClientCalls(stub func(context.Context, *v1alpha1.CFServiceBroker) (osbapi.BrokerClient, error)) { + fake.createClientMutex.Lock() + defer fake.createClientMutex.Unlock() + fake.CreateClientStub = stub +} + +func (fake *BrokerClientFactory) CreateClientArgsForCall(i int) (context.Context, *v1alpha1.CFServiceBroker) { + fake.createClientMutex.RLock() + defer fake.createClientMutex.RUnlock() + argsForCall := fake.createClientArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *BrokerClientFactory) CreateClientReturns(result1 osbapi.BrokerClient, result2 error) { + fake.createClientMutex.Lock() + defer fake.createClientMutex.Unlock() + fake.CreateClientStub = nil + fake.createClientReturns = struct { + result1 osbapi.BrokerClient + result2 error + }{result1, result2} +} + +func (fake *BrokerClientFactory) CreateClientReturnsOnCall(i int, result1 osbapi.BrokerClient, result2 error) { + fake.createClientMutex.Lock() + defer fake.createClientMutex.Unlock() + fake.CreateClientStub = nil + if fake.createClientReturnsOnCall == nil { + fake.createClientReturnsOnCall = make(map[int]struct { + result1 osbapi.BrokerClient + result2 error + }) + } + fake.createClientReturnsOnCall[i] = struct { + result1 osbapi.BrokerClient + result2 error + }{result1, result2} +} + +func (fake *BrokerClientFactory) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.createClientMutex.RLock() + defer fake.createClientMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *BrokerClientFactory) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ osbapi.BrokerClientFactory = new(BrokerClientFactory) diff --git a/controllers/controllers/services/brokers/osbapi/client.go b/controllers/controllers/services/brokers/osbapi/client.go deleted file mode 100644 index a9aee03c3..000000000 --- a/controllers/controllers/services/brokers/osbapi/client.go +++ /dev/null @@ -1,197 +0,0 @@ -package osbapi - -import ( - "bytes" - "context" - "crypto/tls" - "encoding/base64" - "encoding/json" - "fmt" - "io" - "net/http" - "net/url" - - korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/services/credentials" - "code.cloudfoundry.org/korifi/model/services" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -const osbapiVersion = "2.17" - -type Catalog struct { - Services []Service `json:"services"` -} - -type Service struct { - services.BrokerCatalogFeatures `json:",inline"` - ID string `json:"id"` - Name string `json:"name"` - Description string `json:"description"` - Tags []string `json:"tags"` - Requires []string `json:"requires"` - Metadata map[string]any `json:"metadata"` - DashboardClient struct { - Id string `json:"id"` - Secret string `json:"secret"` - RedirectUri string `json:"redirect_url"` - } `json:"dashboard_client"` - Plans []Plan `json:"plans"` -} - -type Plan struct { - ID string `json:"id"` - Name string `json:"name"` - Description string `json:"description"` - Metadata map[string]any `json:"metadata"` - Free bool `json:"free"` - Bindable bool `json:"bindable"` - BindingRotatable bool `json:"binding_rotatable"` - PlanUpdateable bool `json:"plan_updateable"` - Schemas services.ServicePlanSchemas `json:"schemas"` -} - -type Client struct { - k8sClient client.Client - insecure bool -} - -func NewClient(k8sClient client.Client, insecure bool) *Client { - return &Client{k8sClient: k8sClient, insecure: insecure} -} - -func (c *Client) GetCatalog(ctx context.Context, broker *korifiv1alpha1.CFServiceBroker) (*Catalog, error) { - _, resp, err := c.newBrokerRequester().forBroker(broker).sendRequest(ctx, "/v2/catalog", http.MethodGet, nil) - if err != nil { - return nil, fmt.Errorf("get catalog request failed: %w", err) - } - - catalog := &Catalog{} - err = json.Unmarshal(resp, catalog) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal catalog: %w", err) - } - - return catalog, nil -} - -func payloadToReader(payload map[string]any) (io.Reader, error) { - if len(payload) == 0 { - return nil, nil - } - - payloadBytes, err := json.Marshal(payload) - if err != nil { - return nil, fmt.Errorf("failed to marshal payload: %w", err) - } - - return bytes.NewBuffer(payloadBytes), nil -} - -type brokerRequester struct { - k8sClient client.Client - broker *korifiv1alpha1.CFServiceBroker - insecure bool -} - -func (c *Client) newBrokerRequester() *brokerRequester { - return &brokerRequester{k8sClient: c.k8sClient, insecure: c.insecure} -} - -func (r *brokerRequester) forBroker(broker *korifiv1alpha1.CFServiceBroker) *brokerRequester { - r.broker = broker - return r -} - -func (r *brokerRequester) sendRequest(ctx context.Context, requestPath string, method string, payload map[string]any) (int, []byte, error) { - requestUrl, err := url.JoinPath(r.broker.Spec.URL, requestPath) - if err != nil { - return 0, nil, fmt.Errorf("failed to build broker requestUrl for path %q: %w", requestPath, err) - } - - payloadReader, err := payloadToReader(payload) - if err != nil { - return 0, nil, fmt.Errorf("failed create payload reader: %w", err) - } - - req, err := http.NewRequest(method, requestUrl, payloadReader) - if err != nil { - return 0, nil, fmt.Errorf("failed to create new HTTP request: %w", err) - } - req.Header.Add("X-Broker-API-Version", osbapiVersion) - - authHeader, err := r.buildAuthorizationHeaderValue(ctx) - if err != nil { - return 0, nil, fmt.Errorf("failed to build Authorization request header value: %w", err) - } - req.Header.Add("Authorization", authHeader) - - client := &http.Client{Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: r.insecure}, //#nosec G402 - }} - resp, err := client.Do(req) - if err != nil { - return 0, nil, fmt.Errorf("failed to execute HTTP request: %w", err) - } - defer resp.Body.Close() - - respBody, err := io.ReadAll(resp.Body) - if err != nil { - return 0, nil, fmt.Errorf("failed to read body: %w", err) - } - - if resp.StatusCode > 299 { - return resp.StatusCode, respBody, fmt.Errorf("request returned non-OK status %d: %s", resp.StatusCode, string(respBody)) - } - - return resp.StatusCode, respBody, nil -} - -func (r *brokerRequester) buildAuthorizationHeaderValue(ctx context.Context) (string, error) { - userName, password, err := r.getCredentials(ctx) - if err != nil { - return "", fmt.Errorf("failed to get credentials: %w", err) - } - authPlain := fmt.Sprintf("%s:%s", userName, password) - auth := base64.StdEncoding.EncodeToString([]byte(authPlain)) - return "Basic " + auth, nil -} - -func (r *brokerRequester) getCredentials(ctx context.Context) (string, string, error) { - credsSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: r.broker.Namespace, - Name: r.broker.Spec.Credentials.Name, - }, - } - - err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(credsSecret), credsSecret) - if err != nil { - return "", "", fmt.Errorf("failed to get credentials secret %q: %v", credsSecret.Name, err) - } - - creds, err := credentials.GetCredentials(credsSecret) - if err != nil { - return "", "", fmt.Errorf("failed to get credentials from the credentials secret %q: %v", credsSecret.Name, err) - } - - username, ok := creds[korifiv1alpha1.UsernameCredentialsKey].(string) - if !ok { - return "", "", fmt.Errorf("credentials secret %q does not have a string under the %q key", - credsSecret.Name, - korifiv1alpha1.UsernameCredentialsKey, - ) - } - - password, ok := creds[korifiv1alpha1.PasswordCredentialsKey].(string) - if !ok { - return "", "", fmt.Errorf("credentials secret %q does not have a string under the %q key", - credsSecret.Name, - korifiv1alpha1.PasswordCredentialsKey, - ) - } - - return username, password, nil -} diff --git a/controllers/controllers/services/brokers/osbapi/client_test.go b/controllers/controllers/services/brokers/osbapi/client_test.go deleted file mode 100644 index e2924c45f..000000000 --- a/controllers/controllers/services/brokers/osbapi/client_test.go +++ /dev/null @@ -1,198 +0,0 @@ -package osbapi_test - -import ( - "encoding/base64" - "net/http" - "strconv" - - korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers/osbapi" - "code.cloudfoundry.org/korifi/model/services" - "code.cloudfoundry.org/korifi/tests/helpers" - "code.cloudfoundry.org/korifi/tests/helpers/broker" - "code.cloudfoundry.org/korifi/tools" - "github.com/google/uuid" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -var _ = Describe("OSBAPI Client", func() { - var ( - testNamespace string - brokerClient *osbapi.Client - serviceBroker *korifiv1alpha1.CFServiceBroker - brokerServer *broker.BrokerServer - ) - - BeforeEach(func() { - testNamespace = uuid.NewString() - Expect(adminClient.Create(ctx, &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: testNamespace, - }, - })).To(Succeed()) - - brokerServer = broker.NewServer() - - brokerClient = osbapi.NewClient(adminClient, true) - - creds := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testNamespace, - Name: uuid.NewString(), - }, - Data: map[string][]byte{ - tools.CredentialsSecretKey: []byte(`{"username":"broker-user","password":"broker-password"}`), - }, - } - helpers.EnsureCreate(adminClient, creds) - - serviceBroker = &korifiv1alpha1.CFServiceBroker{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testNamespace, - Name: uuid.NewString(), - }, - Spec: korifiv1alpha1.CFServiceBrokerSpec{ - ServiceBroker: services.ServiceBroker{ - Name: uuid.NewString(), - }, - Credentials: corev1.LocalObjectReference{Name: creds.Name}, - }, - } - helpers.EnsureCreate(adminClient, serviceBroker) - }) - - JustBeforeEach(func() { - brokerServer.Start() - DeferCleanup(func() { - brokerServer.Stop() - }) - - helpers.EnsurePatch(adminClient, serviceBroker, func(b *korifiv1alpha1.CFServiceBroker) { - b.Spec.URL = brokerServer.URL() - }) - }) - - Describe("GetCatalog", func() { - var ( - catalog *osbapi.Catalog - getCatalogErr error - ) - - BeforeEach(func() { - brokerServer.WithCatalog(&osbapi.Catalog{ - Services: []osbapi.Service{{ - ID: "123456", - Name: "test-service", - Description: "test service description", - BrokerCatalogFeatures: services.BrokerCatalogFeatures{ - Bindable: true, - }, - }}, - }) - }) - - JustBeforeEach(func() { - catalog, getCatalogErr = brokerClient.GetCatalog(ctx, serviceBroker) - }) - - It("gets the catalog", func() { - Expect(getCatalogErr).NotTo(HaveOccurred()) - Expect(catalog).To(PointTo(Equal(osbapi.Catalog{ - Services: []osbapi.Service{{ - ID: "123456", - Name: "test-service", - Description: "test service description", - BrokerCatalogFeatures: services.BrokerCatalogFeatures{ - Bindable: true, - }, - }}, - }))) - }) - - It("sends broker credentials in the Authorization request header", func() { - Expect(brokerServer.ServedRequests()).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ - "Header": HaveKeyWithValue( - "Authorization", ConsistOf("Basic "+base64.StdEncoding.EncodeToString([]byte("broker-user:broker-password"))), - ), - })))) - }) - - It("sends OSBAPI version request header", func() { - Expect(brokerServer.ServedRequests()).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ - "Header": HaveKeyWithValue( - "X-Broker-Api-Version", ConsistOf("2.17"), - ), - })))) - }) - - When("the client does not trust insecure brokers", func() { - BeforeEach(func() { - brokerClient = osbapi.NewClient(adminClient, false) - }) - - It("returns an error", func() { - Expect(getCatalogErr).To(MatchError(ContainSubstring("failed to verify certificate"))) - }) - }) - - When("getting the catalog fails", func() { - BeforeEach(func() { - brokerServer = broker.NewServer().WithHandler("/v2/catalog", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusTeapot) - })) - }) - - It("returns an error", func() { - Expect(getCatalogErr).To(MatchError(ContainSubstring(strconv.Itoa(http.StatusTeapot)))) - }) - }) - - When("the catalog response cannot be unmarshalled", func() { - BeforeEach(func() { - brokerServer = broker.NewServer().WithHandler("/v2/catalog", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte("hello")) - })) - }) - - It("returns an error", func() { - Expect(getCatalogErr).To(MatchError(ContainSubstring("failed to unmarshal catalog"))) - }) - }) - - When("broker credentials secret does not exist", func() { - BeforeEach(func() { - helpers.EnsurePatch(adminClient, serviceBroker, func(b *korifiv1alpha1.CFServiceBroker) { - b.Spec.Credentials.Name = "i-do-not-exist" - }) - }) - - It("returns an error", func() { - Expect(getCatalogErr).To(MatchError(ContainSubstring("failed to get credentials secret"))) - }) - }) - - When("broker credentials secret is invalid", func() { - BeforeEach(func() { - creds := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testNamespace, - Name: serviceBroker.Spec.Credentials.Name, - }, - } - Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(creds), creds)).To(Succeed()) - helpers.EnsurePatch(adminClient, creds, func(s *corev1.Secret) { - s.Data = map[string][]byte{"foo": []byte("bar")} - }) - }) - - It("returns an error", func() { - Expect(getCatalogErr).To(MatchError(ContainSubstring("data of secret"))) - }) - }) - }) -}) diff --git a/controllers/controllers/services/brokers/package.go b/controllers/controllers/services/brokers/package.go new file mode 100644 index 000000000..26c14322c --- /dev/null +++ b/controllers/controllers/services/brokers/package.go @@ -0,0 +1,3 @@ +package brokers + +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate diff --git a/controllers/controllers/services/brokers/suite_test.go b/controllers/controllers/services/brokers/suite_test.go index 97899de5b..7137412d0 100644 --- a/controllers/controllers/services/brokers/suite_test.go +++ b/controllers/controllers/services/brokers/suite_test.go @@ -24,30 +24,38 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers" - "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers/osbapi" + "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers/fake" "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/tests/helpers" + "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/manager" ) var ( ctx context.Context + k8sManager manager.Manager stopManager context.CancelFunc stopClientCache context.CancelFunc testEnv *envtest.Environment adminClient client.Client + rootNamespace string + + brokerClientFactory *fake.BrokerClientFactory ) func TestAPIs(t *testing.T) { - SetDefaultEventuallyTimeout(30 * time.Second) + SetDefaultEventuallyTimeout(10 * time.Second) SetDefaultEventuallyPollingInterval(250 * time.Millisecond) RegisterFailHandler(Fail) @@ -70,25 +78,40 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) +}) + +var _ = AfterSuite(func() { + Expect(testEnv.Stop()).To(Succeed()) +}) - k8sManager := helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml")) +var _ = BeforeEach(func() { + k8sManager = helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml")) Expect(shared.SetupIndexWithManager(k8sManager)).To(Succeed()) adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config) - err = (brokers.NewReconciler( + rootNamespace = uuid.NewString() + Expect(adminClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: rootNamespace, + }, + })).To(Succeed()) + + brokerClientFactory = new(fake.BrokerClientFactory) + err := (brokers.NewReconciler( k8sManager.GetClient(), - osbapi.NewClient(k8sManager.GetClient(), true), + brokerClientFactory, k8sManager.GetScheme(), ctrl.Log.WithName("controllers").WithName("CFServiceBroker"), )).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) +}) +var _ = JustBeforeEach(func() { stopManager = helpers.StartK8sManager(k8sManager) }) -var _ = AfterSuite(func() { +var _ = AfterEach(func() { stopClientCache() stopManager() - Expect(testEnv.Stop()).To(Succeed()) }) diff --git a/controllers/controllers/services/credentials/credentials.go b/controllers/controllers/services/credentials/credentials.go index 602120972..e9c41f91a 100644 --- a/controllers/controllers/services/credentials/credentials.go +++ b/controllers/controllers/services/credentials/credentials.go @@ -6,13 +6,15 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/tools" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" ) const ServiceBindingSecretTypePrefix = "servicebinding.io/" func GetBindingSecretType(credentialsSecret *corev1.Secret) (corev1.SecretType, error) { - credentials, err := GetCredentials(credentialsSecret) + credentials := map[string]any{} + err := GetCredentials(credentialsSecret, &credentials) if err != nil { return "", err } @@ -26,7 +28,8 @@ func GetBindingSecretType(credentialsSecret *corev1.Secret) (corev1.SecretType, } func GetServiceBindingIOSecretData(credentialsSecret *corev1.Secret) (map[string][]byte, error) { - credentials, err := GetCredentials(credentialsSecret) + credentials := map[string]any{} + err := GetCredentials(credentialsSecret, &credentials) if err != nil { return nil, err } @@ -54,20 +57,15 @@ func toBytes(value any) ([]byte, error) { return json.Marshal(value) } -func GetCredentials(credentialsSecret *corev1.Secret) (map[string]any, error) { +func GetCredentials(credentialsSecret *corev1.Secret, credentialsObject any) error { credentials, ok := credentialsSecret.Data[tools.CredentialsSecretKey] if !ok { - return nil, fmt.Errorf( + return fmt.Errorf( "data of secret %q does not contain the %q key", credentialsSecret.Name, tools.CredentialsSecretKey, ) } - credentialsObject := map[string]any{} - err := json.Unmarshal(credentials, &credentialsObject) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal secret data: %w", err) - } - return credentialsObject, nil + return errors.Wrap(json.Unmarshal(credentials, credentialsObject), "failed to unmarshal secret data") } diff --git a/controllers/controllers/services/credentials/credentials_test.go b/controllers/controllers/services/credentials/credentials_test.go index 3c5556546..aeafd7cb1 100644 --- a/controllers/controllers/services/credentials/credentials_test.go +++ b/controllers/controllers/services/credentials/credentials_test.go @@ -29,7 +29,8 @@ var _ = Describe("Credentials", func() { var creds map[string]any JustBeforeEach(func() { - creds, err = credentials.GetCredentials(credentialsSecret) + creds = map[string]any{} + err = credentials.GetCredentials(credentialsSecret, &creds) }) It("returns the credentials object", func() { diff --git a/controllers/controllers/services/instances/controller_test.go b/controllers/controllers/services/instances/controller_test.go deleted file mode 100644 index 30058a863..000000000 --- a/controllers/controllers/services/instances/controller_test.go +++ /dev/null @@ -1,280 +0,0 @@ -package instances_test - -import ( - "encoding/json" - - "github.com/google/uuid" - . "github.com/onsi/gomega/gstruct" - "sigs.k8s.io/controller-runtime/pkg/client" - - korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - . "code.cloudfoundry.org/korifi/tests/matchers" - "code.cloudfoundry.org/korifi/tools" - "code.cloudfoundry.org/korifi/tools/k8s" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -var _ = Describe("CFServiceInstance", func() { - var ( - testNamespace string - instance *korifiv1alpha1.CFServiceInstance - ) - - BeforeEach(func() { - testNamespace = uuid.NewString() - Expect(adminClient.Create(ctx, &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: testNamespace, - }, - })).To(Succeed()) - - instance = &korifiv1alpha1.CFServiceInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: uuid.NewString(), - Namespace: testNamespace, - }, - Spec: korifiv1alpha1.CFServiceInstanceSpec{ - DisplayName: "service-instance-name", - Type: "user-provided", - Tags: []string{}, - }, - } - Expect(adminClient.Create(ctx, instance)).To(Succeed()) - }) - - It("sets the ObservedGeneration status field", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.ObservedGeneration).To(Equal(instance.Generation)) - }).Should(Succeed()) - }) - - It("sets the CredentialsSecretAvailable condition to false", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("CredentialsSecretNotAvailable")), - ))) - }).Should(Succeed()) - }) - - When("the credentials secret gets created", func() { - var credentialsSecret *corev1.Secret - - BeforeEach(func() { - credentialsSecret = &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: uuid.NewString(), - Namespace: testNamespace, - }, - Data: map[string][]byte{ - tools.CredentialsSecretKey: []byte(`{"foo": "bar"}`), - }, - } - Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed()) - - Expect(k8s.PatchResource(ctx, adminClient, instance, func() { - instance.Spec.SecretName = credentialsSecret.Name - })).To(Succeed()) - }) - - It("sets the CredentialsSecretAvailable condition to true", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionTrue)), - ))) - }).Should(Succeed()) - }) - - It("sets the instance credentials secret name and observed version", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Credentials.Name).To(Equal(instance.Spec.SecretName)) - g.Expect(instance.Status.CredentialsObservedVersion).NotTo(BeEmpty()) - }).Should(Succeed()) - }) - - When("the credentials secret is invalid", func() { - BeforeEach(func() { - Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { - credentialsSecret.Data = map[string][]byte{} - })).To(Succeed()) - }) - - It("sets credentials secret available condition to false", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("SecretInvalid")), - ))) - }).Should(Succeed()) - }) - }) - - When("the credentials secret changes", func() { - var secretVersion string - - BeforeEach(func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionTrue)), - ))) - secretVersion = instance.Status.CredentialsObservedVersion - }).Should(Succeed()) - - Expect(k8s.Patch(ctx, adminClient, credentialsSecret, func() { - credentialsSecret.StringData = map[string]string{"f": "b"} - })).To(Succeed()) - }) - - It("updates the credentials secret observed version", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.CredentialsObservedVersion).NotTo(Equal(secretVersion)) - }).Should(Succeed()) - }) - }) - - When("the credentials secret gets deleted", func() { - var lastObservedVersion string - - BeforeEach(func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionTrue)), - ))) - lastObservedVersion = instance.Status.CredentialsObservedVersion - }).Should(Succeed()) - - Expect(adminClient.Delete(ctx, credentialsSecret)).To(Succeed()) - }) - - It("does not change the instance credentials secret name and observed version", func() { - Consistently(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Credentials.Name).To(Equal(credentialsSecret.Name)) - g.Expect(instance.Status.CredentialsObservedVersion).To(Equal(lastObservedVersion)) - }).Should(Succeed()) - }) - }) - }) - - When("the instance credentials secret is in the 'legacy' format", func() { - var credentialsSecret *corev1.Secret - - getMigratedSecret := func() *corev1.Secret { - migratedSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: instance.Name + "-migrated", - Namespace: testNamespace, - }, - } - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(migratedSecret), migratedSecret)).To(Succeed()) - }).Should(Succeed()) - - return migratedSecret - } - - JustBeforeEach(func() { - credentialsSecret = &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: uuid.NewString(), - Namespace: testNamespace, - }, - Type: corev1.SecretType("servicebinding.io/legacy"), - StringData: map[string]string{ - "foo": "bar", - }, - } - Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed()) - - Expect(k8s.PatchResource(ctx, adminClient, instance, func() { - instance.Spec.SecretName = credentialsSecret.Name - })).To(Succeed()) - }) - - It("creates a derived secret in the new format", func() { - Eventually(func(g Gomega) { - migratedSecret := getMigratedSecret() - g.Expect(migratedSecret.Type).To(Equal(corev1.SecretTypeOpaque)) - g.Expect(migratedSecret.Data).To(MatchAllKeys(Keys{ - tools.CredentialsSecretKey: Not(BeEmpty()), - })) - - credentials := map[string]any{} - g.Expect(json.Unmarshal(migratedSecret.Data[tools.CredentialsSecretKey], &credentials)).To(Succeed()) - g.Expect(credentials).To(MatchAllKeys(Keys{ - "foo": Equal("bar"), - })) - }).Should(Succeed()) - }) - - It("sets an owner reference from the service instance to the migrated secret", func() { - Eventually(func(g Gomega) { - migratedSecret := getMigratedSecret() - g.Expect(migratedSecret.OwnerReferences).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ - "Kind": Equal("CFServiceInstance"), - "Name": Equal(instance.Name), - }))) - }).Should(Succeed()) - }) - - It("sets the instance credentials secret name and observed version to the migrated secret name and version", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Credentials.Name).To(Equal(instance.Name + "-migrated")) - g.Expect(instance.Status.CredentialsObservedVersion).To(Equal(getMigratedSecret().ResourceVersion)) - }).Should(Succeed()) - }) - - It("does not change the original credentials secret", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Credentials.Name).NotTo(BeEmpty()) - - g.Expect(instance.Spec.SecretName).To(Equal(credentialsSecret.Name)) - - previousCredentialsVersion := credentialsSecret.ResourceVersion - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed()) - g.Expect(credentialsSecret.ResourceVersion).To(Equal(previousCredentialsVersion)) - }).Should(Succeed()) - }) - - When("legacy secret cannot be migrated", func() { - BeforeEach(func() { - Expect(adminClient.Create(ctx, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: instance.Name + "-migrated", - Namespace: instance.Namespace, - }, - Type: corev1.SecretType("will-clash-with-migrated-secret-type"), - })).To(Succeed()) - }) - - It("sets the CredentialSecretAvailable condition to false", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) - g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("FailedReconcilingCredentialsSecret")), - ))) - }).Should(Succeed()) - }) - }) - }) -}) diff --git a/controllers/controllers/services/instances/managed/controller.go b/controllers/controllers/services/instances/managed/controller.go new file mode 100644 index 000000000..aef680e6e --- /dev/null +++ b/controllers/controllers/services/instances/managed/controller.go @@ -0,0 +1,313 @@ +/* +Copyright 2021. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package managed + +import ( + "context" + "encoding/json" + "fmt" + "time" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" + "code.cloudfoundry.org/korifi/tools/k8s" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + corev1 "k8s.io/api/core/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +//counterfeiter:generate -o fake -fake-name BrokerClient code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi.BrokerClient + +//counterfeiter:generate -o fake -fake-name BrokerClientFactory code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi.BrokerClientFactory + +type Reconciler struct { + k8sClient client.Client + osbapiClientFactory osbapi.BrokerClientFactory + scheme *runtime.Scheme + rootNamespace string + log logr.Logger +} + +func NewReconciler( + client client.Client, + brokerClientFactory osbapi.BrokerClientFactory, + scheme *runtime.Scheme, + rootNamespace string, + log logr.Logger, +) *k8s.PatchingReconciler[korifiv1alpha1.CFServiceInstance, *korifiv1alpha1.CFServiceInstance] { + return k8s.NewPatchingReconciler(log, client, &Reconciler{ + k8sClient: client, + osbapiClientFactory: brokerClientFactory, + scheme: scheme, + rootNamespace: rootNamespace, + log: log, + }) +} + +func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder { + return ctrl.NewControllerManagedBy(mgr). + For(&korifiv1alpha1.CFServiceInstance{}). + Named("managed-cfserviceinstance"). + WithEventFilter(predicate.NewPredicateFuncs(r.isManaged)) +} + +func (r *Reconciler) isManaged(object client.Object) bool { + serviceInstance, ok := object.(*korifiv1alpha1.CFServiceInstance) + if !ok { + return true + } + + return serviceInstance.Spec.Type == korifiv1alpha1.ManagedType +} + +//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfserviceinstances,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfserviceinstances/status,verbs=get;update;atch +//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfserviceinstances/finalizers,verbs=update + +func (r *Reconciler) ReconcileResource(ctx context.Context, serviceInstance *korifiv1alpha1.CFServiceInstance) (ctrl.Result, error) { + log := logr.FromContextOrDiscard(ctx) + + serviceInstance.Status.ObservedGeneration = serviceInstance.Generation + log.V(1).Info("set observed generation", "generation", serviceInstance.Status.ObservedGeneration) + + if !serviceInstance.GetDeletionTimestamp().IsZero() { + return r.finalizeCFServiceInstance(ctx, serviceInstance) + } + + if isReady(serviceInstance) { + return ctrl.Result{}, nil + } + + if isFailed(serviceInstance) { + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisioningFailed").WithNoRequeue() + } + + servicePlan, err := r.getServicePlan(ctx, serviceInstance.Spec.PlanGUID) + if err != nil { + log.Error(err, "failed to get service plan") + return ctrl.Result{}, err + } + + serviceBroker, err := r.getServiceBroker(ctx, servicePlan.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel]) + if err != nil { + log.Error(err, "failed to get service broker") + return ctrl.Result{}, err + } + + serviceOffering, err := r.getServiceOffering(ctx, servicePlan.Labels[korifiv1alpha1.RelServiceOfferingGUIDLabel]) + if err != nil { + log.Error(err, "failed to get service offering") + return ctrl.Result{}, err + } + + osbapiClient, err := r.osbapiClientFactory.CreateClient(ctx, serviceBroker) + if err != nil { + log.Error(err, "failed to create broker client", "broker", serviceBroker.Name) + return ctrl.Result{}, fmt.Errorf("failed to create client for broker %q: %w", serviceBroker.Name, err) + } + + if !isProvisionRequested(serviceInstance) { + return r.provisionServiceInstance(ctx, osbapiClient, serviceInstance, servicePlan, serviceOffering) + } + + lastOpResponse, err := osbapiClient.GetServiceInstanceLastOperation(ctx, osbapi.GetLastOperationPayload{ + ID: serviceInstance.Name, + GetLastOperationRequest: osbapi.GetLastOperationRequest{ + ServiceId: serviceOffering.Spec.BrokerCatalog.ID, + PlanID: servicePlan.Spec.BrokerCatalog.ID, + Operation: serviceInstance.Status.ProvisionOperation, + }, + }) + if err != nil { + log.Error(err, "getting service instance last operation failed") + return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("GetLastOperationFailed") + } + + if lastOpResponse.State == "in progress" { + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisionInProgress").WithRequeue() + } + + if lastOpResponse.State == "failed" { + meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.ProvisioningFailedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: serviceInstance.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "ProvisionFailed", + Message: lastOpResponse.Description, + }) + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisionFailed") + } + + return ctrl.Result{}, nil +} + +func (r *Reconciler) provisionServiceInstance( + ctx context.Context, + osbapiClient osbapi.BrokerClient, + serviceInstance *korifiv1alpha1.CFServiceInstance, + servicePlan *korifiv1alpha1.CFServicePlan, + serviceOffering *korifiv1alpha1.CFServiceOffering, +) (ctrl.Result, error) { + log := logr.FromContextOrDiscard(ctx).WithName("provision-service-instance") + + parametersMap, err := getServiceInstanceParameters(serviceInstance) + if err != nil { + log.Error(err, "failed to get service instance parameters") + return ctrl.Result{}, fmt.Errorf("failed to get service instance parameters: %w", err) + } + + namespace, err := r.getNamespace(ctx, serviceInstance.Namespace) + if err != nil { + log.Error(err, "failed to get namespace") + return ctrl.Result{}, err + } + + var provisionResponse osbapi.ProvisionServiceInstanceResponse + provisionResponse, err = osbapiClient.Provision(ctx, osbapi.InstanceProvisionPayload{ + InstanceID: serviceInstance.Name, + InstanceProvisionRequest: osbapi.InstanceProvisionRequest{ + ServiceId: serviceOffering.Spec.BrokerCatalog.ID, + PlanID: servicePlan.Spec.BrokerCatalog.ID, + SpaceGUID: namespace.Labels[korifiv1alpha1.SpaceGUIDKey], + OrgGUID: namespace.Labels[korifiv1alpha1.OrgGUIDKey], + Parameters: parametersMap, + }, + }) + if err != nil { + log.Error(err, "failed to provision service") + return ctrl.Result{}, fmt.Errorf("failed to provision service: %w", err) + } + + serviceInstance.Status.ProvisionOperation = provisionResponse.Operation + meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.ProvisionRequestedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: serviceInstance.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "ProvisionRequested", + }) + + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisionRequested").WithRequeue() +} + +func getServiceInstanceParameters(serviceInstance *korifiv1alpha1.CFServiceInstance) (map[string]any, error) { + if serviceInstance.Spec.Parameters == nil { + return nil, nil + } + + parametersMap := map[string]any{} + err := json.Unmarshal(serviceInstance.Spec.Parameters.Raw, ¶metersMap) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal parameters: %w", err) + } + + return parametersMap, nil +} + +func (r *Reconciler) finalizeCFServiceInstance(ctx context.Context, cfServiceInstance *korifiv1alpha1.CFServiceInstance) (ctrl.Result, error) { + log := logr.FromContextOrDiscard(ctx).WithName("finalizeCFServiceInstance") + + if !controllerutil.ContainsFinalizer(cfServiceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) { + return ctrl.Result{}, nil + } + + if controllerutil.RemoveFinalizer(cfServiceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) { + log.V(1).Info("finalizer removed") + } + + return ctrl.Result{}, nil +} + +func (r *Reconciler) getNamespace(ctx context.Context, namespaceName string) (*corev1.Namespace, error) { + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + } + + err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(namespace), namespace) + if err != nil { + return nil, fmt.Errorf("failed to get namespace %q: %w", namespaceName, err) + } + return namespace, nil +} + +func (r *Reconciler) getServiceOffering(ctx context.Context, offeringGUID string) (*korifiv1alpha1.CFServiceOffering, error) { + serviceOffering := &korifiv1alpha1.CFServiceOffering{ + ObjectMeta: metav1.ObjectMeta{ + Name: offeringGUID, + Namespace: r.rootNamespace, + }, + } + err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(serviceOffering), serviceOffering) + if err != nil { + return nil, fmt.Errorf("failed to get service offering %q: %w", offeringGUID, err) + } + + return serviceOffering, nil +} + +func (r *Reconciler) getServicePlan(ctx context.Context, planGUID string) (*korifiv1alpha1.CFServicePlan, error) { + servicePlan := &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Name: planGUID, + Namespace: r.rootNamespace, + }, + } + err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan) + if err != nil { + return nil, fmt.Errorf("failed to get service plan %q: %w", planGUID, err) + } + return servicePlan, nil +} + +func (r *Reconciler) getServiceBroker(ctx context.Context, brokerGUID string) (*korifiv1alpha1.CFServiceBroker, error) { + serviceBroker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Name: brokerGUID, + Namespace: r.rootNamespace, + }, + } + err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(serviceBroker), serviceBroker) + if err != nil { + return nil, fmt.Errorf("failed to get service broker %q: %w", brokerGUID, err) + } + + return serviceBroker, nil +} + +func isProvisionRequested(instance *korifiv1alpha1.CFServiceInstance) bool { + return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.ProvisionRequestedCondition) +} + +func isFailed(instance *korifiv1alpha1.CFServiceInstance) bool { + return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.ProvisioningFailedCondition) +} + +func isReady(instance *korifiv1alpha1.CFServiceInstance) bool { + return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.StatusConditionReady) +} diff --git a/controllers/controllers/services/instances/managed/controller_test.go b/controllers/controllers/services/instances/managed/controller_test.go new file mode 100644 index 000000000..e5cb99450 --- /dev/null +++ b/controllers/controllers/services/instances/managed/controller_test.go @@ -0,0 +1,523 @@ +package managed_test + +import ( + "encoding/json" + "errors" + + "github.com/google/uuid" + "sigs.k8s.io/controller-runtime/pkg/client" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/instances/managed/fake" + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" + "code.cloudfoundry.org/korifi/model/services" + . "code.cloudfoundry.org/korifi/tests/matchers" + "code.cloudfoundry.org/korifi/tools/k8s" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +var _ = Describe("CFServiceInstance", func() { + var ( + brokerClient *fake.BrokerClient + instance *korifiv1alpha1.CFServiceInstance + serviceBroker *korifiv1alpha1.CFServiceBroker + servicePlan *korifiv1alpha1.CFServicePlan + ) + + BeforeEach(func() { + brokerClient = new(fake.BrokerClient) + brokerClientFactory.CreateClientReturns(brokerClient, nil) + + brokerClient.ProvisionReturns(osbapi.ProvisionServiceInstanceResponse{ + Operation: "operation-1", + }, nil) + + brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{ + State: "succeeded", + }, nil) + + serviceBroker = &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + }, + Spec: korifiv1alpha1.CFServiceBrokerSpec{ + ServiceBroker: services.ServiceBroker{ + Name: "my-service-broker", + }, + Credentials: corev1.LocalObjectReference{ + Name: "my-broker-secret", + }, + }, + } + Expect(adminClient.Create(ctx, serviceBroker)).To(Succeed()) + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Labels: map[string]string{ + korifiv1alpha1.SpaceGUIDKey: "space-guid", + korifiv1alpha1.OrgGUIDKey: "org-guid", + }, + }, + } + Expect(adminClient.Create(ctx, namespace)).To(Succeed()) + + serviceOffering := &korifiv1alpha1.CFServiceOffering{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: rootNamespace, + Labels: map[string]string{ + korifiv1alpha1.RelServiceBrokerGUIDLabel: serviceBroker.Name, + }, + }, + Spec: korifiv1alpha1.CFServiceOfferingSpec{ + ServiceOffering: services.ServiceOffering{ + BrokerCatalog: services.ServiceBrokerCatalog{ + ID: "service-offering-id", + }, + }, + }, + } + Expect(adminClient.Create(ctx, serviceOffering)).To(Succeed()) + + servicePlan = &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: rootNamespace, + Labels: map[string]string{ + korifiv1alpha1.RelServiceBrokerGUIDLabel: serviceBroker.Name, + korifiv1alpha1.RelServiceOfferingGUIDLabel: serviceOffering.Name, + }, + }, + Spec: korifiv1alpha1.CFServicePlanSpec{ + Visibility: korifiv1alpha1.ServicePlanVisibility{ + Type: "public", + }, + ServicePlan: services.ServicePlan{ + BrokerCatalog: services.ServicePlanBrokerCatalog{ + ID: "service-plan-id", + }, + }, + }, + } + Expect(adminClient.Create(ctx, servicePlan)).To(Succeed()) + + params := map[string]string{ + "param-key": "param-value", + } + paramBytes, err := json.Marshal(params) + Expect(err).NotTo(HaveOccurred()) + + instance = &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: namespace.Name, + Finalizers: []string{ + korifiv1alpha1.CFManagedServiceInstanceFinalizerName, + }, + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + DisplayName: "service-instance-name", + Type: korifiv1alpha1.ManagedType, + PlanGUID: servicePlan.Name, + Parameters: &runtime.RawExtension{ + Raw: paramBytes, + }, + }, + } + Expect(adminClient.Create(ctx, instance)).To(Succeed()) + }) + + It("sets the ObservedGeneration status field", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.ObservedGeneration).To(Equal(instance.Generation)) + }).Should(Succeed()) + }) + + It("sets the Ready condition to True", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }) + }) + + It("provisions the service", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.ProvisionCallCount()).To(Equal(1)) + _, payload := brokerClient.ProvisionArgsForCall(0) + g.Expect(payload).To(Equal(osbapi.InstanceProvisionPayload{ + InstanceID: instance.Name, + InstanceProvisionRequest: osbapi.InstanceProvisionRequest{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id", + SpaceGUID: "space-guid", + OrgGUID: "org-guid", + Parameters: map[string]any{ + "param-key": "param-value", + }, + }, + })) + + g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(BeNumerically(">", 0)) + _, lastOp := brokerClient.GetServiceInstanceLastOperationArgsForCall(brokerClient.GetServiceInstanceLastOperationCallCount() - 1) + g.Expect(lastOp).To(Equal(osbapi.GetLastOperationPayload{ + ID: instance.Name, + GetLastOperationRequest: osbapi.GetLastOperationRequest{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id", + Operation: "operation-1", + }, + })) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.ProvisionRequestedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + g.Expect(instance.Status.ProvisionOperation).To(Equal("operation-1")) + }).Should(Succeed()) + }) + + When("the service instance parameters are not set", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, instance, func() { + instance.Spec.Parameters = nil + })).To(Succeed()) + }) + + It("requests provisioning without parameters", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.ProvisionCallCount()).To(Equal(1)) + _, payload := brokerClient.ProvisionArgsForCall(0) + g.Expect(payload.Parameters).To(BeEmpty()) + }).Should(Succeed()) + }) + }) + + When("the service plan does not exist", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, instance, func() { + instance.Spec.PlanGUID = "i-do-not-exist" + })).To(Succeed()) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + }) + + When("the service broker does not exist", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, servicePlan, func() { + servicePlan.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] = "invalid" + })).To(Succeed()) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + }) + + When("the service offering does not exist", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, servicePlan, func() { + servicePlan.Labels[korifiv1alpha1.RelServiceOfferingGUIDLabel] = "invalid" + })).To(Succeed()) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + }) + + When("service provisioning fails", func() { + BeforeEach(func() { + brokerClient.ProvisionReturns(osbapi.ProvisionServiceInstanceResponse{}, errors.New("provision-failed")) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + }) + + When("getting service last operation fails", func() { + BeforeEach(func() { + brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{}, errors.New("get-last-op-failed")) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + }) + + When("the last operation is in progress", func() { + BeforeEach(func() { + brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{ + State: "in progress", + }, nil) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + + It("keeps checking last operation", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(BeNumerically(">", 1)) + _, actualLastOpPayload := brokerClient.GetServiceInstanceLastOperationArgsForCall(1) + g.Expect(actualLastOpPayload).To(Equal(osbapi.GetLastOperationPayload{ + ID: instance.Name, + GetLastOperationRequest: osbapi.GetLastOperationRequest{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id", + Operation: "operation-1", + }, + })) + }).Should(Succeed()) + }) + + It("sets the ProvisionRequested condition", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.ProvisionRequestedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.ProvisionRequestedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + }) + + When("the last operation is failed", func() { + BeforeEach(func() { + brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{ + State: "failed", + }, nil) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + + It("sets the failed condition", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.ProvisioningFailedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + }) + + When("the instance has become ready", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, instance, func() { + meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.StatusConditionReady, + Status: metav1.ConditionTrue, + Reason: "ready", + }) + })).To(Succeed()) + }) + + It("does not request provision", func() { + Consistently(func(g Gomega) { + g.Expect(brokerClient.ProvisionCallCount()).To(Equal(0)) + }).Should(Succeed()) + }) + + It("does not check last operation", func() { + Consistently(func(g Gomega) { + g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(Equal(0)) + }).Should(Succeed()) + }) + + It("remains ready", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + }) + + When("the instance provisioning has failed", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, instance, func() { + meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.ProvisioningFailedCondition, + Status: metav1.ConditionTrue, + Reason: "ProvisionFailed", + }) + })).To(Succeed()) + }) + + It("does not request provision", func() { + Consistently(func(g Gomega) { + g.Expect(brokerClient.ProvisionCallCount()).To(Equal(0)) + }).Should(Succeed()) + }) + + It("does not check last operation", func() { + Consistently(func(g Gomega) { + g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(Equal(0)) + }).Should(Succeed()) + }) + + It("remains not ready", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + + It("remains failed", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.ProvisioningFailedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + }) + + When("the provisioninig has been requested", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, instance, func() { + meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.ProvisionRequestedCondition, + Status: metav1.ConditionTrue, + Reason: "ProvisionRequested", + }) + instance.Status.ProvisionOperation = "operation-1" + })).To(Succeed()) + }) + + It("does not request provisioning again", func() { + Consistently(func(g Gomega) { + g.Expect(brokerClient.ProvisionCallCount()).To(Equal(0)) + }).Should(Succeed()) + }) + + It("checks the provisioning status", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(Equal(1)) + _, actualLastOpPayload := brokerClient.GetServiceInstanceLastOperationArgsForCall(0) + g.Expect(actualLastOpPayload).To(Equal(osbapi.GetLastOperationPayload{ + ID: instance.Name, + GetLastOperationRequest: osbapi.GetLastOperationRequest{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id", + Operation: "operation-1", + }, + })) + }).Should(Succeed()) + + Consistently(func(g Gomega) { + g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(Equal(1)) + }).Should(Succeed()) + }) + }) + + When("the instance is deleted", func() { + JustBeforeEach(func() { + Expect(adminClient.Delete(ctx, instance)).To(Succeed()) + }) + + It("deletes the instance", func() { + Eventually(func(g Gomega) { + err := adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance) + g.Expect(k8serrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + }) + }) + + When("the service instance is user-provided", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, instance, func() { + instance.Spec.Type = korifiv1alpha1.UserProvidedType + })).To(Succeed()) + }) + + It("does not reconcile it", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status).To(BeZero()) + }).Should(Succeed()) + }) + }) +}) diff --git a/controllers/controllers/services/instances/managed/fake/broker_client.go b/controllers/controllers/services/instances/managed/fake/broker_client.go new file mode 100644 index 000000000..a90be2e68 --- /dev/null +++ b/controllers/controllers/services/instances/managed/fake/broker_client.go @@ -0,0 +1,279 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package fake + +import ( + "context" + "sync" + + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" +) + +type BrokerClient struct { + GetCatalogStub func(context.Context) (osbapi.Catalog, error) + getCatalogMutex sync.RWMutex + getCatalogArgsForCall []struct { + arg1 context.Context + } + getCatalogReturns struct { + result1 osbapi.Catalog + result2 error + } + getCatalogReturnsOnCall map[int]struct { + result1 osbapi.Catalog + result2 error + } + GetServiceInstanceLastOperationStub func(context.Context, osbapi.GetLastOperationPayload) (osbapi.LastOperationResponse, error) + getServiceInstanceLastOperationMutex sync.RWMutex + getServiceInstanceLastOperationArgsForCall []struct { + arg1 context.Context + arg2 osbapi.GetLastOperationPayload + } + getServiceInstanceLastOperationReturns struct { + result1 osbapi.LastOperationResponse + result2 error + } + getServiceInstanceLastOperationReturnsOnCall map[int]struct { + result1 osbapi.LastOperationResponse + result2 error + } + ProvisionStub func(context.Context, osbapi.InstanceProvisionPayload) (osbapi.ProvisionServiceInstanceResponse, error) + provisionMutex sync.RWMutex + provisionArgsForCall []struct { + arg1 context.Context + arg2 osbapi.InstanceProvisionPayload + } + provisionReturns struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + } + provisionReturnsOnCall map[int]struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *BrokerClient) GetCatalog(arg1 context.Context) (osbapi.Catalog, error) { + fake.getCatalogMutex.Lock() + ret, specificReturn := fake.getCatalogReturnsOnCall[len(fake.getCatalogArgsForCall)] + fake.getCatalogArgsForCall = append(fake.getCatalogArgsForCall, struct { + arg1 context.Context + }{arg1}) + stub := fake.GetCatalogStub + fakeReturns := fake.getCatalogReturns + fake.recordInvocation("GetCatalog", []interface{}{arg1}) + fake.getCatalogMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClient) GetCatalogCallCount() int { + fake.getCatalogMutex.RLock() + defer fake.getCatalogMutex.RUnlock() + return len(fake.getCatalogArgsForCall) +} + +func (fake *BrokerClient) GetCatalogCalls(stub func(context.Context) (osbapi.Catalog, error)) { + fake.getCatalogMutex.Lock() + defer fake.getCatalogMutex.Unlock() + fake.GetCatalogStub = stub +} + +func (fake *BrokerClient) GetCatalogArgsForCall(i int) context.Context { + fake.getCatalogMutex.RLock() + defer fake.getCatalogMutex.RUnlock() + argsForCall := fake.getCatalogArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *BrokerClient) GetCatalogReturns(result1 osbapi.Catalog, result2 error) { + fake.getCatalogMutex.Lock() + defer fake.getCatalogMutex.Unlock() + fake.GetCatalogStub = nil + fake.getCatalogReturns = struct { + result1 osbapi.Catalog + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) GetCatalogReturnsOnCall(i int, result1 osbapi.Catalog, result2 error) { + fake.getCatalogMutex.Lock() + defer fake.getCatalogMutex.Unlock() + fake.GetCatalogStub = nil + if fake.getCatalogReturnsOnCall == nil { + fake.getCatalogReturnsOnCall = make(map[int]struct { + result1 osbapi.Catalog + result2 error + }) + } + fake.getCatalogReturnsOnCall[i] = struct { + result1 osbapi.Catalog + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) GetServiceInstanceLastOperation(arg1 context.Context, arg2 osbapi.GetLastOperationPayload) (osbapi.LastOperationResponse, error) { + fake.getServiceInstanceLastOperationMutex.Lock() + ret, specificReturn := fake.getServiceInstanceLastOperationReturnsOnCall[len(fake.getServiceInstanceLastOperationArgsForCall)] + fake.getServiceInstanceLastOperationArgsForCall = append(fake.getServiceInstanceLastOperationArgsForCall, struct { + arg1 context.Context + arg2 osbapi.GetLastOperationPayload + }{arg1, arg2}) + stub := fake.GetServiceInstanceLastOperationStub + fakeReturns := fake.getServiceInstanceLastOperationReturns + fake.recordInvocation("GetServiceInstanceLastOperation", []interface{}{arg1, arg2}) + fake.getServiceInstanceLastOperationMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationCallCount() int { + fake.getServiceInstanceLastOperationMutex.RLock() + defer fake.getServiceInstanceLastOperationMutex.RUnlock() + return len(fake.getServiceInstanceLastOperationArgsForCall) +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationCalls(stub func(context.Context, osbapi.GetLastOperationPayload) (osbapi.LastOperationResponse, error)) { + fake.getServiceInstanceLastOperationMutex.Lock() + defer fake.getServiceInstanceLastOperationMutex.Unlock() + fake.GetServiceInstanceLastOperationStub = stub +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationArgsForCall(i int) (context.Context, osbapi.GetLastOperationPayload) { + fake.getServiceInstanceLastOperationMutex.RLock() + defer fake.getServiceInstanceLastOperationMutex.RUnlock() + argsForCall := fake.getServiceInstanceLastOperationArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationReturns(result1 osbapi.LastOperationResponse, result2 error) { + fake.getServiceInstanceLastOperationMutex.Lock() + defer fake.getServiceInstanceLastOperationMutex.Unlock() + fake.GetServiceInstanceLastOperationStub = nil + fake.getServiceInstanceLastOperationReturns = struct { + result1 osbapi.LastOperationResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) GetServiceInstanceLastOperationReturnsOnCall(i int, result1 osbapi.LastOperationResponse, result2 error) { + fake.getServiceInstanceLastOperationMutex.Lock() + defer fake.getServiceInstanceLastOperationMutex.Unlock() + fake.GetServiceInstanceLastOperationStub = nil + if fake.getServiceInstanceLastOperationReturnsOnCall == nil { + fake.getServiceInstanceLastOperationReturnsOnCall = make(map[int]struct { + result1 osbapi.LastOperationResponse + result2 error + }) + } + fake.getServiceInstanceLastOperationReturnsOnCall[i] = struct { + result1 osbapi.LastOperationResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) Provision(arg1 context.Context, arg2 osbapi.InstanceProvisionPayload) (osbapi.ProvisionServiceInstanceResponse, error) { + fake.provisionMutex.Lock() + ret, specificReturn := fake.provisionReturnsOnCall[len(fake.provisionArgsForCall)] + fake.provisionArgsForCall = append(fake.provisionArgsForCall, struct { + arg1 context.Context + arg2 osbapi.InstanceProvisionPayload + }{arg1, arg2}) + stub := fake.ProvisionStub + fakeReturns := fake.provisionReturns + fake.recordInvocation("Provision", []interface{}{arg1, arg2}) + fake.provisionMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClient) ProvisionCallCount() int { + fake.provisionMutex.RLock() + defer fake.provisionMutex.RUnlock() + return len(fake.provisionArgsForCall) +} + +func (fake *BrokerClient) ProvisionCalls(stub func(context.Context, osbapi.InstanceProvisionPayload) (osbapi.ProvisionServiceInstanceResponse, error)) { + fake.provisionMutex.Lock() + defer fake.provisionMutex.Unlock() + fake.ProvisionStub = stub +} + +func (fake *BrokerClient) ProvisionArgsForCall(i int) (context.Context, osbapi.InstanceProvisionPayload) { + fake.provisionMutex.RLock() + defer fake.provisionMutex.RUnlock() + argsForCall := fake.provisionArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *BrokerClient) ProvisionReturns(result1 osbapi.ProvisionServiceInstanceResponse, result2 error) { + fake.provisionMutex.Lock() + defer fake.provisionMutex.Unlock() + fake.ProvisionStub = nil + fake.provisionReturns = struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) ProvisionReturnsOnCall(i int, result1 osbapi.ProvisionServiceInstanceResponse, result2 error) { + fake.provisionMutex.Lock() + defer fake.provisionMutex.Unlock() + fake.ProvisionStub = nil + if fake.provisionReturnsOnCall == nil { + fake.provisionReturnsOnCall = make(map[int]struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + }) + } + fake.provisionReturnsOnCall[i] = struct { + result1 osbapi.ProvisionServiceInstanceResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.getCatalogMutex.RLock() + defer fake.getCatalogMutex.RUnlock() + fake.getServiceInstanceLastOperationMutex.RLock() + defer fake.getServiceInstanceLastOperationMutex.RUnlock() + fake.provisionMutex.RLock() + defer fake.provisionMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *BrokerClient) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ osbapi.BrokerClient = new(BrokerClient) diff --git a/controllers/controllers/services/instances/managed/fake/broker_client_factory.go b/controllers/controllers/services/instances/managed/fake/broker_client_factory.go new file mode 100644 index 000000000..fba51a140 --- /dev/null +++ b/controllers/controllers/services/instances/managed/fake/broker_client_factory.go @@ -0,0 +1,120 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package fake + +import ( + "context" + "sync" + + "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" +) + +type BrokerClientFactory struct { + CreateClientStub func(context.Context, *v1alpha1.CFServiceBroker) (osbapi.BrokerClient, error) + createClientMutex sync.RWMutex + createClientArgsForCall []struct { + arg1 context.Context + arg2 *v1alpha1.CFServiceBroker + } + createClientReturns struct { + result1 osbapi.BrokerClient + result2 error + } + createClientReturnsOnCall map[int]struct { + result1 osbapi.BrokerClient + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *BrokerClientFactory) CreateClient(arg1 context.Context, arg2 *v1alpha1.CFServiceBroker) (osbapi.BrokerClient, error) { + fake.createClientMutex.Lock() + ret, specificReturn := fake.createClientReturnsOnCall[len(fake.createClientArgsForCall)] + fake.createClientArgsForCall = append(fake.createClientArgsForCall, struct { + arg1 context.Context + arg2 *v1alpha1.CFServiceBroker + }{arg1, arg2}) + stub := fake.CreateClientStub + fakeReturns := fake.createClientReturns + fake.recordInvocation("CreateClient", []interface{}{arg1, arg2}) + fake.createClientMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClientFactory) CreateClientCallCount() int { + fake.createClientMutex.RLock() + defer fake.createClientMutex.RUnlock() + return len(fake.createClientArgsForCall) +} + +func (fake *BrokerClientFactory) CreateClientCalls(stub func(context.Context, *v1alpha1.CFServiceBroker) (osbapi.BrokerClient, error)) { + fake.createClientMutex.Lock() + defer fake.createClientMutex.Unlock() + fake.CreateClientStub = stub +} + +func (fake *BrokerClientFactory) CreateClientArgsForCall(i int) (context.Context, *v1alpha1.CFServiceBroker) { + fake.createClientMutex.RLock() + defer fake.createClientMutex.RUnlock() + argsForCall := fake.createClientArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *BrokerClientFactory) CreateClientReturns(result1 osbapi.BrokerClient, result2 error) { + fake.createClientMutex.Lock() + defer fake.createClientMutex.Unlock() + fake.CreateClientStub = nil + fake.createClientReturns = struct { + result1 osbapi.BrokerClient + result2 error + }{result1, result2} +} + +func (fake *BrokerClientFactory) CreateClientReturnsOnCall(i int, result1 osbapi.BrokerClient, result2 error) { + fake.createClientMutex.Lock() + defer fake.createClientMutex.Unlock() + fake.CreateClientStub = nil + if fake.createClientReturnsOnCall == nil { + fake.createClientReturnsOnCall = make(map[int]struct { + result1 osbapi.BrokerClient + result2 error + }) + } + fake.createClientReturnsOnCall[i] = struct { + result1 osbapi.BrokerClient + result2 error + }{result1, result2} +} + +func (fake *BrokerClientFactory) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.createClientMutex.RLock() + defer fake.createClientMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *BrokerClientFactory) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ osbapi.BrokerClientFactory = new(BrokerClientFactory) diff --git a/controllers/controllers/services/instances/managed/package.go b/controllers/controllers/services/instances/managed/package.go new file mode 100644 index 000000000..25ba6cc0c --- /dev/null +++ b/controllers/controllers/services/instances/managed/package.go @@ -0,0 +1,3 @@ +package managed + +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate diff --git a/controllers/controllers/services/instances/managed/suite_test.go b/controllers/controllers/services/instances/managed/suite_test.go new file mode 100644 index 000000000..6e99ac03d --- /dev/null +++ b/controllers/controllers/services/instances/managed/suite_test.go @@ -0,0 +1,119 @@ +/* +Copyright 2021. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package managed_test + +import ( + "context" + "path/filepath" + "testing" + "time" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/instances/managed" + "code.cloudfoundry.org/korifi/controllers/controllers/services/instances/managed/fake" + "code.cloudfoundry.org/korifi/controllers/controllers/shared" + "code.cloudfoundry.org/korifi/tests/helpers" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/manager" +) + +var ( + ctx context.Context + stopManager context.CancelFunc + stopClientCache context.CancelFunc + testEnv *envtest.Environment + adminClient client.Client + k8sManager manager.Manager + rootNamespace string + + brokerClientFactory *fake.BrokerClientFactory +) + +func TestAPIs(t *testing.T) { + SetDefaultEventuallyTimeout(30 * time.Second) + SetDefaultEventuallyPollingInterval(250 * time.Millisecond) + + RegisterFailHandler(Fail) + RunSpecs(t, "Services Instance Controller Integration Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + + ctx, stopManager = context.WithCancel(context.TODO()) + + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join("..", "..", "..", "..", "..", "helm", "korifi", "controllers", "crds"), + }, + ErrorIfCRDPathMissing: true, + } + + _, err := testEnv.Start() + Expect(err).NotTo(HaveOccurred()) + + Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) +}) + +var _ = AfterSuite(func() { + Expect(testEnv.Stop()).To(Succeed()) +}) + +var _ = BeforeEach(func() { + k8sManager = helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml")) + Expect(shared.SetupIndexWithManager(k8sManager)).To(Succeed()) + + adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config) + + rootNamespace = uuid.NewString() + Expect(adminClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: rootNamespace, + }, + })).To(Succeed()) + + brokerClientFactory = new(fake.BrokerClientFactory) + + err := (managed.NewReconciler( + k8sManager.GetClient(), + brokerClientFactory, + k8sManager.GetScheme(), + rootNamespace, + ctrl.Log.WithName("controllers").WithName("ManagedCFServiceInstance"), + )).SetupWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) +}) + +var _ = JustBeforeEach(func() { + stopManager = helpers.StartK8sManager(k8sManager) +}) + +var _ = AfterEach(func() { + stopClientCache() + stopManager() +}) diff --git a/controllers/controllers/services/instances/controller.go b/controllers/controllers/services/instances/upsi/controller.go similarity index 90% rename from controllers/controllers/services/instances/controller.go rename to controllers/controllers/services/instances/upsi/controller.go index bb2cede4a..82a183eb9 100644 --- a/controllers/controllers/services/instances/controller.go +++ b/controllers/controllers/services/instances/upsi/controller.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package instances +package upsi import ( "context" @@ -40,6 +40,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -55,18 +56,29 @@ func NewReconciler( log logr.Logger, ) *k8s.PatchingReconciler[korifiv1alpha1.CFServiceInstance, *korifiv1alpha1.CFServiceInstance] { serviceInstanceReconciler := Reconciler{k8sClient: client, scheme: scheme, log: log} - return k8s.NewPatchingReconciler[korifiv1alpha1.CFServiceInstance, *korifiv1alpha1.CFServiceInstance](log, client, &serviceInstanceReconciler) + return k8s.NewPatchingReconciler(log, client, &serviceInstanceReconciler) } func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder { return ctrl.NewControllerManagedBy(mgr). For(&korifiv1alpha1.CFServiceInstance{}). + Named("user-provided-cfserviceinstance"). + WithEventFilter(predicate.NewPredicateFuncs(r.isUPSI)). Watches( &corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.secretToServiceInstance), ) } +func (r *Reconciler) isUPSI(object client.Object) bool { + serviceInstance, ok := object.(*korifiv1alpha1.CFServiceInstance) + if !ok { + return true + } + + return serviceInstance.Spec.Type == korifiv1alpha1.UserProvidedType +} + func (r *Reconciler) secretToServiceInstance(ctx context.Context, o client.Object) []reconcile.Request { serviceInstances := korifiv1alpha1.CFServiceInstanceList{} if err := r.k8sClient.List(ctx, &serviceInstances, @@ -120,7 +132,7 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceInstance *k return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("FailedReconcilingCredentialsSecret") } - if err = r.validateCredentials(ctx, credentialsSecret); err != nil { + if err = r.validateCredentials(credentialsSecret); err != nil { return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("SecretInvalid") } @@ -171,7 +183,7 @@ func (r *Reconciler) reconcileCredentials(ctx context.Context, credentialsSecret return migratedSecret, nil } -func (r *Reconciler) validateCredentials(ctx context.Context, credentialsSecret *corev1.Secret) error { +func (r *Reconciler) validateCredentials(credentialsSecret *corev1.Secret) error { return errors.Wrapf( json.Unmarshal(credentialsSecret.Data[tools.CredentialsSecretKey], &map[string]any{}), "invalid credentials secret %q", diff --git a/controllers/controllers/services/instances/upsi/controller_test.go b/controllers/controllers/services/instances/upsi/controller_test.go new file mode 100644 index 000000000..0aa9cfa50 --- /dev/null +++ b/controllers/controllers/services/instances/upsi/controller_test.go @@ -0,0 +1,308 @@ +package upsi_test + +import ( + "encoding/json" + + "github.com/google/uuid" + . "github.com/onsi/gomega/gstruct" + "sigs.k8s.io/controller-runtime/pkg/client" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + . "code.cloudfoundry.org/korifi/tests/matchers" + "code.cloudfoundry.org/korifi/tools" + "code.cloudfoundry.org/korifi/tools/k8s" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("CFServiceInstance", func() { + var ( + testNamespace string + instance *korifiv1alpha1.CFServiceInstance + ) + + BeforeEach(func() { + testNamespace = uuid.NewString() + Expect(adminClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: testNamespace, + }, + })).To(Succeed()) + }) + + When("the service instance is user-provided", func() { + BeforeEach(func() { + instance = &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: testNamespace, + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + DisplayName: "service-instance-name", + Type: korifiv1alpha1.UserProvidedType, + Tags: []string{}, + }, + } + Expect(adminClient.Create(ctx, instance)).To(Succeed()) + }) + + It("sets the ObservedGeneration status field", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.ObservedGeneration).To(Equal(instance.Generation)) + }).Should(Succeed()) + }) + + It("sets the CredentialsSecretAvailable condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("CredentialsSecretNotAvailable")), + ))) + }).Should(Succeed()) + }) + + When("the credentials secret gets created", func() { + var credentialsSecret *corev1.Secret + + BeforeEach(func() { + credentialsSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: testNamespace, + }, + Data: map[string][]byte{ + tools.CredentialsSecretKey: []byte(`{"foo": "bar"}`), + }, + } + Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed()) + + Expect(k8s.PatchResource(ctx, adminClient, instance, func() { + instance.Spec.SecretName = credentialsSecret.Name + })).To(Succeed()) + }) + + It("sets the CredentialsSecretAvailable condition to true", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + + It("sets the instance credentials secret name and observed version", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Credentials.Name).To(Equal(instance.Spec.SecretName)) + g.Expect(instance.Status.CredentialsObservedVersion).NotTo(BeEmpty()) + }).Should(Succeed()) + }) + + When("the credentials secret is invalid", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.Data = map[string][]byte{} + })).To(Succeed()) + }) + + It("sets credentials secret available condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("SecretInvalid")), + ))) + }).Should(Succeed()) + }) + }) + + When("the credentials secret changes", func() { + var secretVersion string + + BeforeEach(func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + secretVersion = instance.Status.CredentialsObservedVersion + }).Should(Succeed()) + + Expect(k8s.Patch(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.StringData = map[string]string{"f": "b"} + })).To(Succeed()) + }) + + It("updates the credentials secret observed version", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.CredentialsObservedVersion).NotTo(Equal(secretVersion)) + }).Should(Succeed()) + }) + }) + + When("the credentials secret gets deleted", func() { + var lastObservedVersion string + + BeforeEach(func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + lastObservedVersion = instance.Status.CredentialsObservedVersion + }).Should(Succeed()) + + Expect(adminClient.Delete(ctx, credentialsSecret)).To(Succeed()) + }) + + It("does not change the instance credentials secret name and observed version", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Credentials.Name).To(Equal(credentialsSecret.Name)) + g.Expect(instance.Status.CredentialsObservedVersion).To(Equal(lastObservedVersion)) + }).Should(Succeed()) + }) + }) + }) + + When("the instance credentials secret is in the 'legacy' format", func() { + var credentialsSecret *corev1.Secret + + getMigratedSecret := func() *corev1.Secret { + migratedSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: instance.Name + "-migrated", + Namespace: testNamespace, + }, + } + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(migratedSecret), migratedSecret)).To(Succeed()) + }).Should(Succeed()) + + return migratedSecret + } + + JustBeforeEach(func() { + credentialsSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: testNamespace, + }, + Type: corev1.SecretType("servicebinding.io/legacy"), + StringData: map[string]string{ + "foo": "bar", + }, + } + Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed()) + + Expect(k8s.PatchResource(ctx, adminClient, instance, func() { + instance.Spec.SecretName = credentialsSecret.Name + })).To(Succeed()) + }) + + It("creates a derived secret in the new format", func() { + Eventually(func(g Gomega) { + migratedSecret := getMigratedSecret() + g.Expect(migratedSecret.Type).To(Equal(corev1.SecretTypeOpaque)) + g.Expect(migratedSecret.Data).To(MatchAllKeys(Keys{ + tools.CredentialsSecretKey: Not(BeEmpty()), + })) + + credentials := map[string]any{} + g.Expect(json.Unmarshal(migratedSecret.Data[tools.CredentialsSecretKey], &credentials)).To(Succeed()) + g.Expect(credentials).To(MatchAllKeys(Keys{ + "foo": Equal("bar"), + })) + }).Should(Succeed()) + }) + + It("sets an owner reference from the service instance to the migrated secret", func() { + Eventually(func(g Gomega) { + migratedSecret := getMigratedSecret() + g.Expect(migratedSecret.OwnerReferences).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ + "Kind": Equal("CFServiceInstance"), + "Name": Equal(instance.Name), + }))) + }).Should(Succeed()) + }) + + It("sets the instance credentials secret name and observed version to the migrated secret name and version", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Credentials.Name).To(Equal(instance.Name + "-migrated")) + g.Expect(instance.Status.CredentialsObservedVersion).To(Equal(getMigratedSecret().ResourceVersion)) + }).Should(Succeed()) + }) + + It("does not change the original credentials secret", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Credentials.Name).NotTo(BeEmpty()) + + g.Expect(instance.Spec.SecretName).To(Equal(credentialsSecret.Name)) + + previousCredentialsVersion := credentialsSecret.ResourceVersion + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed()) + g.Expect(credentialsSecret.ResourceVersion).To(Equal(previousCredentialsVersion)) + }).Should(Succeed()) + }) + + When("legacy secret cannot be migrated", func() { + BeforeEach(func() { + Expect(adminClient.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: instance.Name + "-migrated", + Namespace: instance.Namespace, + }, + Type: corev1.SecretType("will-clash-with-migrated-secret-type"), + })).To(Succeed()) + }) + + It("sets the CredentialSecretAvailable condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("FailedReconcilingCredentialsSecret")), + ))) + }).Should(Succeed()) + }) + }) + }) + }) + + When("the service instance is managed", func() { + BeforeEach(func() { + instance = &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: testNamespace, + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + DisplayName: "service-instance-name", + Type: korifiv1alpha1.ManagedType, + Tags: []string{}, + }, + } + Expect(adminClient.Create(ctx, instance)).To(Succeed()) + }) + + It("does not reconcile it", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status).To(BeZero()) + }).Should(Succeed()) + }) + }) +}) diff --git a/controllers/controllers/services/instances/suite_test.go b/controllers/controllers/services/instances/upsi/suite_test.go similarity index 89% rename from controllers/controllers/services/instances/suite_test.go rename to controllers/controllers/services/instances/upsi/suite_test.go index f0f30ef38..245251b2b 100644 --- a/controllers/controllers/services/instances/suite_test.go +++ b/controllers/controllers/services/instances/upsi/suite_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package instances_test +package upsi_test import ( "context" @@ -23,7 +23,7 @@ import ( "time" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/services/instances" + "code.cloudfoundry.org/korifi/controllers/controllers/services/instances/upsi" "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/tests/helpers" @@ -50,7 +50,7 @@ func TestAPIs(t *testing.T) { SetDefaultEventuallyPollingInterval(250 * time.Millisecond) RegisterFailHandler(Fail) - RunSpecs(t, "Services Instance Controller Integration Suite") + RunSpecs(t, "User-Provided Services Instance Controller Integration Suite") } var _ = BeforeSuite(func() { @@ -60,7 +60,7 @@ var _ = BeforeSuite(func() { testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{ - filepath.Join("..", "..", "..", "..", "helm", "korifi", "controllers", "crds"), + filepath.Join("..", "..", "..", "..", "..", "helm", "korifi", "controllers", "crds"), }, ErrorIfCRDPathMissing: true, } @@ -75,10 +75,10 @@ var _ = BeforeSuite(func() { adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config) - err = (instances.NewReconciler( + err = (upsi.NewReconciler( k8sManager.GetClient(), k8sManager.GetScheme(), - ctrl.Log.WithName("controllers").WithName("CFServiceInstance"), + ctrl.Log.WithName("controllers").WithName("UPSICFServiceInstance"), )).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/controllers/controllers/services/osbapi/client.go b/controllers/controllers/services/osbapi/client.go new file mode 100644 index 000000000..dc949be42 --- /dev/null +++ b/controllers/controllers/services/osbapi/client.go @@ -0,0 +1,196 @@ +package osbapi + +import ( + "bytes" + "context" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" +) + +const osbapiVersion = "2.17" + +type GoneError struct{} + +func (g GoneError) Error() string { + return "The operation resource is gone" +} + +type Client struct { + broker Broker + httpClient *http.Client +} + +func NewClient(broker Broker, httpClient *http.Client) *Client { + return &Client{ + broker: broker, + httpClient: httpClient, + } +} + +func (c *Client) GetCatalog(ctx context.Context) (Catalog, error) { + statusCode, resp, err := c.newBrokerRequester(). + forBroker(c.broker). + sendRequest(ctx, "/v2/catalog", http.MethodGet, nil) + if err != nil { + return Catalog{}, fmt.Errorf("get catalog request failed: %w", err) + } + + if statusCode > 300 { + return Catalog{}, fmt.Errorf("getting service catalog request failed with status code: %d", statusCode) + } + + var catalog Catalog + err = json.Unmarshal(resp, &catalog) + if err != nil { + return Catalog{}, fmt.Errorf("failed to unmarshal catalog: %w", err) + } + + return catalog, nil +} + +func (c *Client) Provision(ctx context.Context, payload InstanceProvisionPayload) (ProvisionServiceInstanceResponse, error) { + statusCode, respBytes, err := c.newBrokerRequester(). + forBroker(c.broker). + async(). + sendRequest( + ctx, + "/v2/service_instances/"+payload.InstanceID, + http.MethodPut, + payload.InstanceProvisionRequest, + ) + if err != nil { + return ProvisionServiceInstanceResponse{}, fmt.Errorf("provision request failed: %w", err) + } + + if statusCode >= 300 { + return ProvisionServiceInstanceResponse{}, fmt.Errorf("provision request failed with status code: %d", statusCode) + } + + var response ProvisionServiceInstanceResponse + err = json.Unmarshal(respBytes, &response) + if err != nil { + return ProvisionServiceInstanceResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) + } + + return response, nil +} + +func (c *Client) GetServiceInstanceLastOperation(ctx context.Context, payload GetLastOperationPayload) (LastOperationResponse, error) { + statusCode, respBytes, err := c.newBrokerRequester(). + forBroker(c.broker). + sendRequest( + ctx, + "/v2/service_instances/"+payload.ID+"/last_operation", + http.MethodGet, + payload.GetLastOperationRequest, + ) + if err != nil { + return LastOperationResponse{}, fmt.Errorf("getting last operation request failed: %w", err) + } + + if statusCode == http.StatusGone { + return LastOperationResponse{}, GoneError{} + } + + if statusCode != http.StatusOK { + return LastOperationResponse{}, fmt.Errorf("getting last operation request failed with code: %d", statusCode) + } + + var response LastOperationResponse + err = json.Unmarshal(respBytes, &response) + if err != nil { + return LastOperationResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) + } + + return response, nil +} + +func payloadToReader(payload any) (io.Reader, error) { + if payload == nil { + return nil, nil + } + + payloadBytes, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("failed to marshal payload: %w", err) + } + + return bytes.NewBuffer(payloadBytes), nil +} + +type brokerRequester struct { + broker Broker + acceptsIncomplete bool + httpClient *http.Client +} + +func (c *Client) newBrokerRequester() *brokerRequester { + return &brokerRequester{httpClient: c.httpClient} +} + +func (r *brokerRequester) forBroker(broker Broker) *brokerRequester { + r.broker = broker + return r +} + +func (r *brokerRequester) async() *brokerRequester { + r.acceptsIncomplete = true + return r +} + +func (r *brokerRequester) sendRequest(ctx context.Context, requestPath string, method string, payload any) (int, []byte, error) { + requestUrl, err := url.JoinPath(r.broker.URL, requestPath) + if err != nil { + return 0, nil, fmt.Errorf("failed to build broker requestUrl for path %q: %w", requestPath, err) + } + + parsedURL, err := url.Parse(requestUrl) + if err != nil { + panic(err) + } + + payloadReader, err := payloadToReader(payload) + if err != nil { + return 0, nil, fmt.Errorf("failed create payload reader: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, method, parsedURL.String(), payloadReader) + if err != nil { + return 0, nil, fmt.Errorf("failed to create new HTTP request: %w", err) + } + req.Header.Add("X-Broker-API-Version", osbapiVersion) + if r.acceptsIncomplete { + queryValues := req.URL.Query() + queryValues.Add("accepts_incomplete", "true") + req.URL.RawQuery = queryValues.Encode() + } + + authHeader, err := r.buildAuthorizationHeaderValue() + if err != nil { + return 0, nil, fmt.Errorf("failed to build Authorization request header value: %w", err) + } + req.Header.Add("Authorization", authHeader) + + resp, err := r.httpClient.Do(req) + if err != nil { + return 0, nil, fmt.Errorf("failed to execute HTTP request: %w", err) + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return 0, nil, fmt.Errorf("failed to read body: %w", err) + } + + return resp.StatusCode, respBody, nil +} + +func (r *brokerRequester) buildAuthorizationHeaderValue() (string, error) { + authPlain := fmt.Sprintf("%s:%s", r.broker.Username, r.broker.Password) + auth := base64.StdEncoding.EncodeToString([]byte(authPlain)) + return "Basic " + auth, nil +} diff --git a/controllers/controllers/services/osbapi/client_test.go b/controllers/controllers/services/osbapi/client_test.go new file mode 100644 index 000000000..4f408f2b8 --- /dev/null +++ b/controllers/controllers/services/osbapi/client_test.go @@ -0,0 +1,301 @@ +package osbapi_test + +import ( + "crypto/tls" + "encoding/base64" + "encoding/json" + "io" + "net/http" + "strconv" + + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" + "code.cloudfoundry.org/korifi/model/services" + "code.cloudfoundry.org/korifi/tests/helpers/broker" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" +) + +var _ = Describe("OSBAPI Client", func() { + var ( + brokerClient *osbapi.Client + brokerServer *broker.BrokerServer + ) + + BeforeEach(func() { + brokerServer = broker.NewServer() + }) + + JustBeforeEach(func() { + brokerServer.Start() + DeferCleanup(func() { + brokerServer.Stop() + }) + + brokerClient = osbapi.NewClient(osbapi.Broker{ + URL: brokerServer.URL(), + Username: "broker-user", + Password: "broker-password", + }, &http.Client{Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //#nosec G402 + }}) + }) + + Describe("GetCatalog", func() { + var ( + catalog osbapi.Catalog + getCatalogErr error + ) + + BeforeEach(func() { + brokerServer.WithResponse( + "/v2/catalog", + map[string]any{ + "services": []map[string]any{ + { + "id": "123456", + "name": "test-service", + "description": "test service description", + "bindable": true, + }, + }, + }, + http.StatusOK, + ) + }) + + JustBeforeEach(func() { + catalog, getCatalogErr = brokerClient.GetCatalog(ctx) + }) + + It("gets the catalog", func() { + Expect(getCatalogErr).NotTo(HaveOccurred()) + Expect(catalog).To(Equal(osbapi.Catalog{ + Services: []osbapi.Service{{ + ID: "123456", + Name: "test-service", + Description: "test service description", + BrokerCatalogFeatures: services.BrokerCatalogFeatures{ + Bindable: true, + }, + }}, + })) + }) + + It("sends a sync request", func() { + servedRequests := brokerServer.ServedRequests() + Expect(servedRequests).To(HaveLen(1)) + Expect(servedRequests[0].Method).To(Equal(http.MethodGet)) + Expect(servedRequests[0].URL.Query().Get("accepts_incomplete")).To(BeEmpty()) + }) + + It("sends broker credentials in the Authorization request header", func() { + Expect(brokerServer.ServedRequests()).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Header": HaveKeyWithValue( + "Authorization", ConsistOf("Basic "+base64.StdEncoding.EncodeToString([]byte("broker-user:broker-password"))), + ), + })))) + }) + + It("sends OSBAPI version request header", func() { + Expect(brokerServer.ServedRequests()).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Header": HaveKeyWithValue( + "X-Broker-Api-Version", ConsistOf("2.17"), + ), + })))) + }) + + When("getting the catalog fails", func() { + BeforeEach(func() { + brokerServer = broker.NewServer().WithHandler("/v2/catalog", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusTeapot) + })) + }) + + It("returns an error", func() { + Expect(getCatalogErr).To(MatchError(ContainSubstring(strconv.Itoa(http.StatusTeapot)))) + }) + }) + + When("the catalog response cannot be unmarshalled", func() { + BeforeEach(func() { + brokerServer = broker.NewServer().WithHandler("/v2/catalog", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("hello")) + })) + }) + + It("returns an error", func() { + Expect(getCatalogErr).To(MatchError(ContainSubstring("failed to unmarshal catalog"))) + }) + }) + }) + + Describe("Instances", func() { + Describe("Provision", func() { + var ( + provisionResp osbapi.ProvisionServiceInstanceResponse + provisionErr error + ) + + BeforeEach(func() { + brokerServer.WithResponse( + "/v2/service_instances/{id}", + map[string]any{ + "operation": "provision_op1", + }, + http.StatusCreated, + ) + }) + + JustBeforeEach(func() { + provisionResp, provisionErr = brokerClient.Provision(ctx, osbapi.InstanceProvisionPayload{ + InstanceID: "my-service-instance", + InstanceProvisionRequest: osbapi.InstanceProvisionRequest{ + ServiceId: "service-guid", + PlanID: "plan-guid", + SpaceGUID: "space-guid", + OrgGUID: "org-guid", + Parameters: map[string]any{ + "foo": "bar", + }, + }, + }) + }) + + It("provisions the service", func() { + Expect(provisionErr).NotTo(HaveOccurred()) + Expect(provisionResp).To(Equal(osbapi.ProvisionServiceInstanceResponse{ + Operation: "provision_op1", + })) + }) + + It("sends async provision request to broker", func() { + Expect(provisionErr).NotTo(HaveOccurred()) + requests := brokerServer.ServedRequests() + + Expect(requests).To(HaveLen(1)) + + Expect(requests[0].Method).To(Equal(http.MethodPut)) + Expect(requests[0].URL.Path).To(Equal("/v2/service_instances/my-service-instance")) + + Expect(requests[0].URL.Query().Get("accepts_incomplete")).To(Equal("true")) + }) + + It("sends correct request body", func() { + Expect(provisionErr).NotTo(HaveOccurred()) + requests := brokerServer.ServedRequests() + + Expect(requests).To(HaveLen(1)) + + requestBytes, err := io.ReadAll(requests[0].Body) + Expect(err).NotTo(HaveOccurred()) + requestBody := map[string]any{} + Expect(json.Unmarshal(requestBytes, &requestBody)).To(Succeed()) + + Expect(requestBody).To(MatchAllKeys(Keys{ + "service_id": Equal("service-guid"), + "plan_id": Equal("plan-guid"), + "space_guid": Equal("space-guid"), + "organization_guid": Equal("org-guid"), + "parameters": MatchAllKeys(Keys{ + "foo": Equal("bar"), + }), + })) + }) + + When("the provision request fails", func() { + BeforeEach(func() { + brokerServer = broker.NewServer().WithHandler("/v2/service_instances/{id}", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusTeapot) + })) + }) + + It("returns an error", func() { + Expect(provisionErr).To(MatchError(ContainSubstring("provision request failed"))) + }) + }) + }) + + Describe("GetLastOperation", func() { + var ( + lastOpResp osbapi.LastOperationResponse + lastOpErr error + ) + + BeforeEach(func() { + brokerServer.WithResponse( + "/v2/service_instances/{id}/last_operation", + map[string]any{ + "state": "in-progress", + "description": "provisioning", + }, + http.StatusOK, + ) + }) + + JustBeforeEach(func() { + lastOpResp, lastOpErr = brokerClient.GetServiceInstanceLastOperation(ctx, osbapi.GetLastOperationPayload{ + ID: "my-service-instance", + GetLastOperationRequest: osbapi.GetLastOperationRequest{ + ServiceId: "service-guid", + PlanID: "plan-guid", + Operation: "op-guid", + }, + }) + }) + + It("gets the last operation", func() { + Expect(lastOpErr).NotTo(HaveOccurred()) + Expect(lastOpResp).To(Equal(osbapi.LastOperationResponse{ + State: "in-progress", + Description: "provisioning", + })) + }) + + It("sends correct request to broker", func() { + Expect(lastOpErr).NotTo(HaveOccurred()) + requests := brokerServer.ServedRequests() + + Expect(requests).To(HaveLen(1)) + + Expect(requests[0].Method).To(Equal(http.MethodGet)) + Expect(requests[0].URL.Path).To(Equal("/v2/service_instances/my-service-instance/last_operation")) + + requestBytes, err := io.ReadAll(requests[0].Body) + Expect(err).NotTo(HaveOccurred()) + requestBody := map[string]any{} + Expect(json.Unmarshal(requestBytes, &requestBody)).To(Succeed()) + + Expect(requestBody).To(MatchAllKeys(Keys{ + "service_id": Equal("service-guid"), + "plan_id": Equal("plan-guid"), + "operation": Equal("op-guid"), + })) + }) + + When("getting the last operation request fails", func() { + BeforeEach(func() { + brokerServer = broker.NewServer().WithHandler("/v2/service_instances/{id}/last_operation", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusTeapot) + })) + }) + + It("returns an error", func() { + Expect(lastOpErr).To(MatchError(ContainSubstring("getting last operation request failed"))) + }) + }) + + When("getting the last operation request fails with 410 Gone", func() { + BeforeEach(func() { + brokerServer = broker.NewServer().WithHandler("/v2/service_instances/{id}/last_operation", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusGone) + })) + }) + + It("returns a gone error", func() { + Expect(lastOpErr).To(BeAssignableToTypeOf(osbapi.GoneError{})) + }) + }) + }) + }) +}) diff --git a/controllers/controllers/services/osbapi/clientfactory.go b/controllers/controllers/services/osbapi/clientfactory.go new file mode 100644 index 000000000..31bf28b94 --- /dev/null +++ b/controllers/controllers/services/osbapi/clientfactory.go @@ -0,0 +1,74 @@ +package osbapi + +import ( + "context" + "crypto/tls" + "encoding/json" + "fmt" + "net/http" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/model/services" + "code.cloudfoundry.org/korifi/tools" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type BrokerClient interface { + Provision(context.Context, InstanceProvisionPayload) (ProvisionServiceInstanceResponse, error) + GetServiceInstanceLastOperation(context.Context, GetLastOperationPayload) (LastOperationResponse, error) + GetCatalog(context.Context) (Catalog, error) +} + +type BrokerClientFactory interface { + CreateClient(context.Context, *korifiv1alpha1.CFServiceBroker) (BrokerClient, error) +} + +type ClientFactory struct { + k8sClient client.Client + trustInsecureBrokers bool +} + +func NewClientFactory(k8sClient client.Client, trustInsecureBrokers bool) *ClientFactory { + return &ClientFactory{ + k8sClient: k8sClient, + trustInsecureBrokers: trustInsecureBrokers, + } +} + +func (f *ClientFactory) CreateClient(ctx context.Context, cfServiceBroker *korifiv1alpha1.CFServiceBroker) (BrokerClient, error) { + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cfServiceBroker.Namespace, + Name: cfServiceBroker.Spec.Credentials.Name, + }, + } + + err := f.k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret) + if err != nil { + return nil, err + } + + creds := services.BrokerCredentials{} + err = json.Unmarshal(credentialsSecret.Data[tools.CredentialsSecretKey], &creds) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal broker credentials secret: %w", err) + } + + err = creds.Validate() + if err != nil { + return nil, fmt.Errorf("invalid broker credentials: %w", err) + } + + return NewClient( + Broker{ + URL: cfServiceBroker.Spec.URL, + Username: creds.Username, + Password: creds.Password, + }, + &http.Client{Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: f.trustInsecureBrokers}, //#nosec G402 + }}, + ), nil +} diff --git a/controllers/controllers/services/osbapi/clientfactory_test.go b/controllers/controllers/services/osbapi/clientfactory_test.go new file mode 100644 index 000000000..2d0447e58 --- /dev/null +++ b/controllers/controllers/services/osbapi/clientfactory_test.go @@ -0,0 +1,160 @@ +package osbapi_test + +import ( + "net/http" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" + "code.cloudfoundry.org/korifi/model/services" + "code.cloudfoundry.org/korifi/tests/helpers/broker" + "code.cloudfoundry.org/korifi/tools" + "code.cloudfoundry.org/korifi/tools/k8s" + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("ClientFactory", func() { + var ( + brokerServer *broker.BrokerServer + cfServiceBroker *korifiv1alpha1.CFServiceBroker + factory *osbapi.ClientFactory + osbapiClient osbapi.BrokerClient + createClientErr error + ) + + BeforeEach(func() { + brokerServer = broker.NewServer().WithResponse( + "/v2/catalog", + map[string]any{ + "services": []map[string]any{ + { + "name": "test-service", + }, + }, + }, + http.StatusOK, + ).Start() + + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + }, + Data: map[string][]byte{ + tools.CredentialsSecretKey: []byte(`{"username": "broker-user", "password": "broker-password"}`), + }, + } + Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed()) + + cfServiceBroker = &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + }, + Spec: korifiv1alpha1.CFServiceBrokerSpec{ + ServiceBroker: services.ServiceBroker{ + Name: uuid.NewString(), + URL: brokerServer.URL(), + }, + Credentials: corev1.LocalObjectReference{ + Name: credentialsSecret.Name, + }, + }, + } + Expect(adminClient.Create(ctx, cfServiceBroker)).To(Succeed()) + + factory = osbapi.NewClientFactory(adminClient, true) + }) + + JustBeforeEach(func() { + osbapiClient, createClientErr = factory.CreateClient(ctx, cfServiceBroker) + }) + + It("creates a osbapi client", func() { + Expect(createClientErr).NotTo(HaveOccurred()) + + catalog, err := osbapiClient.GetCatalog(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(catalog.Services).NotTo(BeEmpty()) + }) + + When("the credentials secret cannot be found", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, cfServiceBroker, func() { + cfServiceBroker.Spec.Credentials.Name = "i-do-not-exist" + })).To(Succeed()) + }) + + It("returns an error", func() { + Expect(createClientErr).To(MatchError(ContainSubstring("not found"))) + }) + }) + + When("the broker credentials are invalid", func() { + var credentialsSecret *corev1.Secret + + BeforeEach(func() { + credentialsSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: cfServiceBroker.Spec.Credentials.Name, + }, + } + Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed()) + Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.Data = map[string][]byte{ + "foo": []byte("bar"), + } + })).To(Succeed()) + }) + + It("returns an error", func() { + Expect(createClientErr).To(MatchError(ContainSubstring("failed to unmarshal broker credentials secret"))) + }) + + When("username is not set", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.Data = map[string][]byte{ + tools.CredentialsSecretKey: []byte(`{"password": "my-password"}`), + } + })).To(Succeed()) + }) + + It("returns an error", func() { + Expect(createClientErr).To(MatchError(ContainSubstring("username: cannot be blank"))) + }) + }) + + When("password is not set", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.Data = map[string][]byte{ + tools.CredentialsSecretKey: []byte(`{"username": "my-user"}`), + } + })).To(Succeed()) + }) + + It("returns an error", func() { + Expect(createClientErr).To(MatchError(ContainSubstring("password: cannot be blank"))) + }) + }) + }) + + When("the client does not trust insecure brokers", func() { + BeforeEach(func() { + factory = osbapi.NewClientFactory(adminClient, false) + }) + + It("creates a client that does not trust insecure brokers", func() { + Expect(createClientErr).NotTo(HaveOccurred()) + + _, createClientErr = osbapiClient.GetCatalog(ctx) + Expect(createClientErr).To(MatchError(ContainSubstring("failed to verify certificate"))) + }) + }) +}) diff --git a/controllers/controllers/services/brokers/osbapi/suite_test.go b/controllers/controllers/services/osbapi/suite_test.go similarity index 81% rename from controllers/controllers/services/brokers/osbapi/suite_test.go rename to controllers/controllers/services/osbapi/suite_test.go index ee1ae9c24..55d0e98c0 100644 --- a/controllers/controllers/services/brokers/osbapi/suite_test.go +++ b/controllers/controllers/services/osbapi/suite_test.go @@ -9,8 +9,11 @@ import ( "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/tests/helpers" + "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -24,6 +27,7 @@ var ( stopClientCache context.CancelFunc testEnv *envtest.Environment adminClient client.Client + rootNamespace string ) func TestOSBAPI(t *testing.T) { @@ -38,7 +42,7 @@ var _ = BeforeSuite(func() { testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{ - filepath.Join("..", "..", "..", "..", "..", "helm", "korifi", "controllers", "crds"), + filepath.Join("..", "..", "..", "..", "helm", "korifi", "controllers", "crds"), }, ErrorIfCRDPathMissing: true, } @@ -54,6 +58,13 @@ var _ = BeforeSuite(func() { adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config) stopManager = helpers.StartK8sManager(k8sManager) + + rootNamespace = uuid.NewString() + Expect(adminClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: rootNamespace, + }, + })).To(Succeed()) }) var _ = AfterSuite(func() { diff --git a/controllers/controllers/services/osbapi/types.go b/controllers/controllers/services/osbapi/types.go new file mode 100644 index 000000000..10244d98f --- /dev/null +++ b/controllers/controllers/services/osbapi/types.go @@ -0,0 +1,74 @@ +package osbapi + +import "code.cloudfoundry.org/korifi/model/services" + +type Broker struct { + URL string + Username string + Password string +} + +type Catalog struct { + Services []Service `json:"services"` +} + +type Service struct { + services.BrokerCatalogFeatures `json:",inline"` + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Tags []string `json:"tags"` + Requires []string `json:"requires"` + Metadata map[string]any `json:"metadata"` + DashboardClient struct { + Id string `json:"id"` + Secret string `json:"secret"` + RedirectUri string `json:"redirect_url"` + } `json:"dashboard_client"` + Plans []Plan `json:"plans"` +} + +type InstanceProvisionPayload struct { + InstanceID string + InstanceProvisionRequest +} + +type InstanceProvisionRequest struct { + ServiceId string `json:"service_id"` + PlanID string `json:"plan_id"` + SpaceGUID string `json:"space_guid"` + OrgGUID string `json:"organization_guid"` + Parameters map[string]any `json:"parameters"` +} + +type GetLastOperationPayload struct { + ID string + GetLastOperationRequest +} + +type GetLastOperationRequest struct { + ServiceId string `json:"service_id"` + PlanID string `json:"plan_id"` + Operation string `json:"operation"` +} + +type Plan struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Metadata map[string]any `json:"metadata"` + Free bool `json:"free"` + Bindable bool `json:"bindable"` + BindingRotatable bool `json:"binding_rotatable"` + PlanUpdateable bool `json:"plan_updateable"` + Schemas services.ServicePlanSchemas `json:"schemas"` +} + +type ProvisionServiceInstanceResponse struct { + Operation string `json:"operation"` +} + +type LastOperationResponse struct { + State string `json:"state"` + Description string `json:"description"` +} diff --git a/controllers/controllers/workloads/env/vcap_services_builder.go b/controllers/controllers/workloads/env/vcap_services_builder.go index b32fd308a..4cf3c374a 100644 --- a/controllers/controllers/workloads/env/vcap_services_builder.go +++ b/controllers/controllers/workloads/env/vcap_services_builder.go @@ -124,7 +124,8 @@ func fromServiceBinding( tags = []string{} } - credentials, err := credentials.GetCredentials(credentialsSecret) + creds := map[string]any{} + err := credentials.GetCredentials(credentialsSecret, &creds) if err != nil { return ServiceDetails{}, fmt.Errorf("failed to get credentials for service binding %q: %w", serviceBinding.Name, err) } @@ -137,7 +138,7 @@ func fromServiceBinding( InstanceName: serviceInstance.Spec.DisplayName, BindingGUID: serviceBinding.Name, BindingName: bindingName, - Credentials: credentials, + Credentials: creds, SyslogDrainURL: nil, VolumeMounts: []string{}, }, nil diff --git a/controllers/main.go b/controllers/main.go index d734b16bc..be2b683f1 100644 --- a/controllers/main.go +++ b/controllers/main.go @@ -31,8 +31,9 @@ import ( "code.cloudfoundry.org/korifi/controllers/controllers/networking/routes" "code.cloudfoundry.org/korifi/controllers/controllers/services/bindings" "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers" - "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers/osbapi" - "code.cloudfoundry.org/korifi/controllers/controllers/services/instances" + "code.cloudfoundry.org/korifi/controllers/controllers/services/instances/managed" + "code.cloudfoundry.org/korifi/controllers/controllers/services/instances/upsi" + "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/apps" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/build/buildpack" @@ -228,12 +229,12 @@ func main() { os.Exit(1) } - if err = (instances.NewReconciler( + if err = (upsi.NewReconciler( mgr.GetClient(), mgr.GetScheme(), controllersLog, )).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "CFServiceInstance") + setupLog.Error(err, "unable to create controller", "controller", "UPSICFServiceInstance") os.Exit(1) } @@ -306,13 +307,24 @@ func main() { if controllerConfig.ExperimentalManagedServicesEnabled { if err = brokers.NewReconciler( mgr.GetClient(), - osbapi.NewClient(mgr.GetClient(), controllerConfig.TrustInsecureServiceBrokers), + osbapi.NewClientFactory(mgr.GetClient(), controllerConfig.TrustInsecureServiceBrokers), mgr.GetScheme(), controllersLog, ).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CFServiceBroker") os.Exit(1) } + + if err = managed.NewReconciler( + mgr.GetClient(), + osbapi.NewClientFactory(mgr.GetClient(), controllerConfig.TrustInsecureServiceBrokers), + mgr.GetScheme(), + controllerConfig.CFRootNamespace, + controllersLog, + ).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ManagedCFServiceInstance") + os.Exit(1) + } } //+kubebuilder:scaffold:builder diff --git a/controllers/webhooks/finalizer/suite_test.go b/controllers/webhooks/finalizer/suite_test.go index ea4b041d4..62d6b2cc6 100644 --- a/controllers/webhooks/finalizer/suite_test.go +++ b/controllers/webhooks/finalizer/suite_test.go @@ -12,6 +12,7 @@ import ( "code.cloudfoundry.org/korifi/controllers/webhooks/finalizer" "code.cloudfoundry.org/korifi/controllers/webhooks/networking/domains" "code.cloudfoundry.org/korifi/controllers/webhooks/networking/routes" + "code.cloudfoundry.org/korifi/controllers/webhooks/services/instances" "code.cloudfoundry.org/korifi/controllers/webhooks/validation" "code.cloudfoundry.org/korifi/controllers/webhooks/version" "code.cloudfoundry.org/korifi/controllers/webhooks/workloads/apps" @@ -109,6 +110,7 @@ var _ = BeforeSuite(func() { uncachedClient, ).SetupWebhookWithManager(k8sManager)).To(Succeed()) Expect(packages.NewValidator().SetupWebhookWithManager(k8sManager)).To(Succeed()) + Expect(instances.NewValidator(validation.NewDuplicateValidator(coordination.NewNameRegistry(uncachedClient, instances.ServiceInstanceEntityType))).SetupWebhookWithManager(k8sManager)).To(Succeed()) stopManager = helpers.StartK8sManager(k8sManager) diff --git a/controllers/webhooks/finalizer/webhook.go b/controllers/webhooks/finalizer/webhook.go index ddb31f1ac..22479099b 100644 --- a/controllers/webhooks/finalizer/webhook.go +++ b/controllers/webhooks/finalizer/webhook.go @@ -1,12 +1,14 @@ package finalizer -//+kubebuilder:webhook:path=/mutate-korifi-cloudfoundry-org-v1alpha1-controllers-finalizer,mutating=true,failurePolicy=fail,sideEffects=None,groups=korifi.cloudfoundry.org,resources=cfapps;cfspaces;cfpackages;cforgs;cfroutes;cfdomains,verbs=create,versions=v1alpha1,name=mcffinalizer.korifi.cloudfoundry.org,admissionReviewVersions={v1,v1beta1} +//+kubebuilder:webhook:path=/mutate-korifi-cloudfoundry-org-v1alpha1-controllers-finalizer,mutating=true,failurePolicy=fail,sideEffects=None,groups=korifi.cloudfoundry.org,resources=cfapps;cfspaces;cfpackages;cforgs;cfroutes;cfdomains;cfserviceinstances,verbs=create,versions=v1alpha1,name=mcffinalizer.korifi.cloudfoundry.org,admissionReviewVersions={v1,v1beta1} import ( "context" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/tools/k8s" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) @@ -18,11 +20,12 @@ type ControllersFinalizerWebhook struct { func NewControllersFinalizerWebhook() *ControllersFinalizerWebhook { return &ControllersFinalizerWebhook{ delegate: k8s.NewFinalizerWebhook(map[string]k8s.FinalizerDescriptor{ - "CFApp": {FinalizerName: korifiv1alpha1.CFAppFinalizerName, SetPolicy: k8s.Always}, - "CFSpace": {FinalizerName: korifiv1alpha1.CFSpaceFinalizerName, SetPolicy: k8s.Always}, - "CFPackage": {FinalizerName: korifiv1alpha1.CFPackageFinalizerName, SetPolicy: k8s.Always}, - "CFOrg": {FinalizerName: korifiv1alpha1.CFOrgFinalizerName, SetPolicy: k8s.Always}, - "CFDomain": {FinalizerName: korifiv1alpha1.CFDomainFinalizerName, SetPolicy: k8s.Always}, + "CFApp": {FinalizerName: korifiv1alpha1.CFAppFinalizerName, SetPolicy: k8s.Always}, + "CFSpace": {FinalizerName: korifiv1alpha1.CFSpaceFinalizerName, SetPolicy: k8s.Always}, + "CFPackage": {FinalizerName: korifiv1alpha1.CFPackageFinalizerName, SetPolicy: k8s.Always}, + "CFOrg": {FinalizerName: korifiv1alpha1.CFOrgFinalizerName, SetPolicy: k8s.Always}, + "CFDomain": {FinalizerName: korifiv1alpha1.CFDomainFinalizerName, SetPolicy: k8s.Always}, + "CFServiceInstance": {FinalizerName: korifiv1alpha1.CFManagedServiceInstanceFinalizerName, SetPolicy: isManagedServiceInstance}, }), } } @@ -37,3 +40,16 @@ func (r *ControllersFinalizerWebhook) SetupWebhookWithManager(mgr ctrl.Manager) func (r *ControllersFinalizerWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { return r.delegate.Handle(ctx, req) } + +func isManagedServiceInstance(object unstructured.Unstructured) bool { + l := ctrl.Log.WithName("isManagedServiceInstance") + cfServiceInstance := &korifiv1alpha1.CFServiceInstance{} + err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, cfServiceInstance) + if err != nil { + l.Error(err, "failed to convert to CFServiceInstnace from unstructured", "unstructured", object.Object) + return true + } + + l.Info("CFServiceInstance converted", "cfserviceinstance", cfServiceInstance) + return cfServiceInstance.Spec.Type == korifiv1alpha1.ManagedType +} diff --git a/controllers/webhooks/finalizer/webhook_test.go b/controllers/webhooks/finalizer/webhook_test.go index 752351e66..0ec656b4b 100644 --- a/controllers/webhooks/finalizer/webhook_test.go +++ b/controllers/webhooks/finalizer/webhook_test.go @@ -109,5 +109,28 @@ var _ = Describe("Controllers Finalizers Webhook", func() { }, }, ), + Entry("managed CF service instance", + &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-org-" + uuid.NewString(), + Name: uuid.NewString(), + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + Type: korifiv1alpha1.ManagedType, + }, + }, + korifiv1alpha1.CFManagedServiceInstanceFinalizerName, + ), + Entry("user-provided CF service instance", + &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-org-" + uuid.NewString(), + Name: uuid.NewString(), + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + Type: korifiv1alpha1.UserProvidedType, + }, + }, + ), ) }) diff --git a/controllers/webhooks/services/brokers/suite_test.go b/controllers/webhooks/services/brokers/suite_test.go index 8dbc5b348..88a85979e 100644 --- a/controllers/webhooks/services/brokers/suite_test.go +++ b/controllers/webhooks/services/brokers/suite_test.go @@ -11,7 +11,7 @@ import ( ) func TestServicesValidatingWebhooks(t *testing.T) { - SetDefaultEventuallyTimeout(10 * time.Second) + SetDefaultEventuallyTimeout(30 * time.Second) SetDefaultEventuallyPollingInterval(250 * time.Millisecond) RegisterFailHandler(Fail) diff --git a/controllers/webhooks/services/instances/validator.go b/controllers/webhooks/services/instances/validator.go index bb23daae6..7a578f4d0 100644 --- a/controllers/webhooks/services/instances/validator.go +++ b/controllers/webhooks/services/instances/validator.go @@ -7,6 +7,7 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/webhooks" + validationwebhook "code.cloudfoundry.org/korifi/controllers/webhooks/validation" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -66,6 +67,12 @@ func (v *Validator) ValidateUpdate(ctx context.Context, oldObj, obj runtime.Obje return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a CFServiceInstance but got a %T", oldObj)) } + if serviceInstance.Spec.Type != oldServiceInstance.Spec.Type { + return nil, validationwebhook.ValidationError{ + Type: validationwebhook.ImmutableFieldErrorType, + Message: fmt.Sprintf(validationwebhook.ImmutableFieldErrorMessageTemplate, "CFServiceInstance.Spec.Type"), + }.ExportJSONError() + } return nil, v.duplicateValidator.ValidateUpdate(ctx, cfserviceinstancelog, serviceInstance.Namespace, oldServiceInstance, serviceInstance) } diff --git a/controllers/webhooks/services/instances/validator_test.go b/controllers/webhooks/services/instances/validator_test.go index 3c432f398..6b9b5d9fb 100644 --- a/controllers/webhooks/services/instances/validator_test.go +++ b/controllers/webhooks/services/instances/validator_test.go @@ -8,6 +8,8 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/webhooks/fake" "code.cloudfoundry.org/korifi/controllers/webhooks/services/instances" + "code.cloudfoundry.org/korifi/controllers/webhooks/validation" + "code.cloudfoundry.org/korifi/tests/matchers" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" @@ -124,6 +126,19 @@ var _ = Describe("CFServiceInstanceValidatingWebhook", func() { Expect(retErr).To(MatchError("foo")) }) }) + + When("the type is being updated", func() { + BeforeEach(func() { + updatedServiceInstance.Spec.Type = korifiv1alpha1.ManagedType + }) + + It("returns an error", func() { + Expect(retErr).To(matchers.BeValidationError( + validation.ImmutableFieldErrorType, + Equal("'CFServiceInstance.Spec.Type' field is immutable"), + )) + }) + }) }) Describe("ValidateDelete", func() { diff --git a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml index d0baed617..714327abb 100644 --- a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml +++ b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml @@ -50,6 +50,9 @@ spec: description: The mutable, user-friendly name of the service instance. Unlike metadata.name, the user can change this field type: string + parameters: + type: object + x-kubernetes-preserve-unknown-fields: true plan_guid: type: string secretName: @@ -164,6 +167,8 @@ spec: the CFServiceInstance that has been reconciled format: int64 type: integer + provisionOperation: + type: string type: object type: object served: true diff --git a/helm/korifi/controllers/manifests.yaml b/helm/korifi/controllers/manifests.yaml index a9b426b0e..063bdcd8d 100644 --- a/helm/korifi/controllers/manifests.yaml +++ b/helm/korifi/controllers/manifests.yaml @@ -135,6 +135,7 @@ webhooks: - cforgs - cfroutes - cfdomains + - cfserviceinstances sideEffects: None - admissionReviewVersions: - v1 diff --git a/helm/korifi/controllers/role.yaml b/helm/korifi/controllers/role.yaml index 3acd84554..22e45c152 100644 --- a/helm/korifi/controllers/role.yaml +++ b/helm/korifi/controllers/role.yaml @@ -157,7 +157,6 @@ rules: - cfroutes/status - cfservicebindings/status - cfservicebrokers/status - - cfserviceinstances/status - cfspaces/status - cftasks/status verbs: @@ -195,6 +194,15 @@ rules: - list - patch - watch +- apiGroups: + - korifi.cloudfoundry.org + resources: + - cfserviceinstances/status + verbs: + - atch + - get + - patch + - update - apiGroups: - korifi.cloudfoundry.org resources: diff --git a/model/services/brokers.go b/model/services/brokers.go index c8f55eea9..f29d50e7c 100644 --- a/model/services/brokers.go +++ b/model/services/brokers.go @@ -1,5 +1,9 @@ package services +import ( + jellidation "github.com/jellydator/validation" +) + // +kubebuilder:object:generate=true type ServiceBroker struct { Name string `json:"name"` @@ -10,3 +14,10 @@ type BrokerCredentials struct { Username string `json:"username"` Password string `json:"password"` } + +func (c BrokerCredentials) Validate() error { + return jellidation.ValidateStruct(&c, + jellidation.Field(&c.Username, jellidation.Required), + jellidation.Field(&c.Password, jellidation.Required), + ) +} diff --git a/model/services/offerings.go b/model/services/offerings.go index c05373759..f443ce04b 100644 --- a/model/services/offerings.go +++ b/model/services/offerings.go @@ -15,7 +15,7 @@ type ServiceOffering struct { // +kubebuilder:object:generate=true type ServiceBrokerCatalog struct { - Id string `json:"id"` + ID string `json:"id"` // +kubebuilder:validation:Optional Metadata *runtime.RawExtension `json:"metadata"` Features BrokerCatalogFeatures `json:"features"` diff --git a/tests/assets/sample-broker-golang/go.mod b/tests/assets/sample-broker-golang/go.mod index bfc35e42c..a4642cee3 100644 --- a/tests/assets/sample-broker-golang/go.mod +++ b/tests/assets/sample-broker-golang/go.mod @@ -1,3 +1,3 @@ module sample-broker -go 1.19 // Support installations with a slightly older version of kpack +go 1.23 diff --git a/tests/assets/sample-broker-golang/main.go b/tests/assets/sample-broker-golang/main.go index 3c854ee97..9ba360665 100644 --- a/tests/assets/sample-broker-golang/main.go +++ b/tests/assets/sample-broker-golang/main.go @@ -18,8 +18,10 @@ const ( ) func main() { - http.HandleFunc("/", helloWorldHandler) - http.HandleFunc("/v2/catalog", getCatalogHandler) + http.HandleFunc("GET /", helloWorldHandler) + http.HandleFunc("GET /v2/catalog", getCatalogHandler) + http.HandleFunc("PUT /v2/service_instances/{id}", provisionServiceInstanceHandler) + http.HandleFunc("GET /v2/service_instances/{id}/last_operation", serviceInstanceLastOperationHandler) port := os.Getenv("PORT") if port == "" { @@ -65,6 +67,26 @@ func getCatalogHandler(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, string(catalogBytes)) } +func provisionServiceInstanceHandler(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 + } + + fmt.Fprintf(w, `{"operation":"provision-%s"}`, r.PathValue("id")) +} + +func serviceInstanceLastOperationHandler(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 + } + + fmt.Fprint(w, `{"state":"succeeded"}`) +} + func checkCredentials(_ http.ResponseWriter, r *http.Request) (int, error) { authHeader := r.Header.Get("Authorization") if len(authHeader) == 0 { diff --git a/tests/e2e/service_instances_test.go b/tests/e2e/service_instances_test.go index d7490a27d..d5b6562b5 100644 --- a/tests/e2e/service_instances_test.go +++ b/tests/e2e/service_instances_test.go @@ -29,46 +29,100 @@ var _ = Describe("Service Instances", func() { }) Describe("Create", func() { - When("the user has permissions to create service instances", func() { - var instanceName string + var ( + instanceName string + createPayload serviceInstanceResource + result serviceInstanceResource + ) - BeforeEach(func() { - instanceName = generateGUID("service-instance") - }) + BeforeEach(func() { + instanceName = generateGUID("service-instance") + createPayload = serviceInstanceResource{} + }) + + JustBeforeEach(func() { + httpResp, httpError = adminClient.R(). + SetBody(createPayload). + SetResult(&result). + Post("/v3/service_instances") + }) - JustBeforeEach(func() { - httpResp, httpError = adminClient.R(). - SetBody(serviceInstanceResource{ - resource: resource{ - Name: instanceName, - Relationships: relationships{ - "space": { - Data: resource{ - GUID: spaceGUID, - }, + When("creating a user-provided service instance", func() { + BeforeEach(func() { + createPayload = serviceInstanceResource{ + resource: resource{ + Name: instanceName, + Relationships: relationships{ + "space": { + Data: resource{ + GUID: spaceGUID, }, }, }, - Credentials: map[string]any{ - "object": map[string]any{"a": "b"}, - }, - InstanceType: "user-provided", - Tags: []string{"some", "tags"}, - }).Post("/v3/service_instances") + }, + Credentials: map[string]any{ + "object": map[string]any{"a": "b"}, + }, + InstanceType: "user-provided", + } }) It("succeeds", func() { Expect(httpError).NotTo(HaveOccurred()) Expect(httpResp).To(HaveRestyStatusCode(http.StatusCreated)) + Expect(result.Name).To(Equal(instanceName)) + Expect(result.InstanceType).To(Equal("user-provided")) + }) + }) - serviceInstances := listServiceInstances(instanceName) - Expect(serviceInstances.Resources).To(HaveLen(1)) + When("creating a managed service instance", func() { + BeforeEach(func() { + brokerGUID := createBroker(serviceBrokerURL) + DeferCleanup(func() { + cleanupBroker(brokerGUID) + }) + + var plansResp resourceList[resource] + catalogResp, err := adminClient.R().SetResult(&plansResp).Get("/v3/service_plans?service_broker_guids=" + brokerGUID) + Expect(err).NotTo(HaveOccurred()) + Expect(catalogResp).To(HaveRestyStatusCode(http.StatusOK)) + Expect(plansResp.Resources).NotTo(BeEmpty()) + + createPayload = serviceInstanceResource{ + resource: resource{ + Name: instanceName, + Relationships: relationships{ + "space": { + Data: resource{ + GUID: spaceGUID, + }, + }, + "service_plan": { + Data: resource{ + GUID: plansResp.Resources[0].GUID, + }, + }, + }, + }, + InstanceType: "managed", + } + }) - serviceInstance := serviceInstances.Resources[0] - Expect(serviceInstance.Name).To(Equal(instanceName)) - Expect(serviceInstance.Relationships["space"].Data.GUID).To(Equal(spaceGUID)) - Expect(serviceInstance.Tags).To(ConsistOf("some", "tags")) - Expect(serviceInstance.InstanceType).To(Equal("user-provided")) + It("succeeds with a job redirect", func() { + Expect(httpError).NotTo(HaveOccurred()) + Expect(httpResp).To(HaveRestyStatusCode(http.StatusAccepted)) + + Expect(httpResp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusAccepted), + HaveRestyHeaderWithValue("Location", ContainSubstring("/v3/jobs/managed_service_instance.create~")), + )) + + jobURL := httpResp.Header().Get("Location") + Eventually(func(g Gomega) { + jobResp, err := adminClient.R().Get(jobURL) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(string(jobResp.Body())).To(ContainSubstring("COMPLETE")) + }).Should(Succeed()) }) }) }) diff --git a/tests/helpers/broker/broker.go b/tests/helpers/broker/broker.go index 2781f831b..bf5a2de95 100644 --- a/tests/helpers/broker/broker.go +++ b/tests/helpers/broker/broker.go @@ -1,12 +1,13 @@ package broker import ( + "bytes" + "context" "encoding/json" + "io" "net/http" "net/http/httptest" - "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers/osbapi" - . "github.com/onsi/ginkgo/v2" //lint:ignore ST1001 this is a test file . "github.com/onsi/gomega" //lint:ignore ST1001 this is a test file ) @@ -24,21 +25,30 @@ func NewServer() *BrokerServer { } } -func (b *BrokerServer) WithCatalog(catalog *osbapi.Catalog) *BrokerServer { +func (b *BrokerServer) WithResponse(pattern string, response map[string]any, statusCode int) *BrokerServer { GinkgoHelper() - catalogBytes, err := json.Marshal(catalog) + respBytes, err := json.Marshal(response) Expect(err).NotTo(HaveOccurred()) - return b.WithHandler("/v2/catalog", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write(catalogBytes) + return b.WithHandler(pattern, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(respBytes) + w.WriteHeader(statusCode) })) } func (b *BrokerServer) WithHandler(pattern string, handler http.Handler) *BrokerServer { b.mux.Handle(pattern, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - b.requests = append(b.requests, r) - handler.ServeHTTP(w, r) + bodyBytes, err := io.ReadAll(r.Body) + Expect(err).NotTo(HaveOccurred()) + + recordedRequest := r.Clone(context.Background()) + recordedRequest.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) + b.requests = append(b.requests, recordedRequest) + + executedRequest := r.Clone(r.Context()) + executedRequest.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) + handler.ServeHTTP(w, executedRequest) })) return b diff --git a/tests/helpers/test_env.go b/tests/helpers/test_env.go index a0f6e1af1..9224e4fdf 100644 --- a/tests/helpers/test_env.go +++ b/tests/helpers/test_env.go @@ -6,6 +6,7 @@ import ( "path/filepath" "runtime" + "code.cloudfoundry.org/korifi/tools" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" //lint:ignore ST1001 this is a test file . "github.com/onsi/gomega" //lint:ignore ST1001 this is a test file @@ -17,6 +18,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/config" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" @@ -55,6 +57,9 @@ func NewUncachedClient(cfg *rest.Config) client.Client { func NewK8sManager(testEnv *envtest.Environment, managerRolePath string) manager.Manager { k8sManager, err := ctrl.NewManager(SetupTestEnvUser(testEnv, managerRolePath), ctrl.Options{ + Controller: config.Controller{ + SkipNameValidation: tools.PtrTo(true), + }, Scheme: scheme.Scheme, WebhookServer: webhook.NewServer(webhook.Options{ Host: testEnv.WebhookInstallOptions.LocalServingHost, diff --git a/tools/k8s/condition_builder.go b/tools/k8s/condition_builder.go index 0dd08cfcb..bd15dfbff 100644 --- a/tools/k8s/condition_builder.go +++ b/tools/k8s/condition_builder.go @@ -78,6 +78,7 @@ type NotReadyError struct { reason string message string requeueAfter *time.Duration + requeue bool noRequeue bool } @@ -97,6 +98,11 @@ func (e NotReadyError) WithCause(cause error) NotReadyError { return e } +func (e NotReadyError) WithRequeue() NotReadyError { + e.requeue = true + return e +} + func (e NotReadyError) WithRequeueAfter(duration time.Duration) NotReadyError { e.requeueAfter = tools.PtrTo(duration) return e diff --git a/tools/k8s/reconcile.go b/tools/k8s/reconcile.go index d8da8d8e9..c7fb6d43b 100644 --- a/tools/k8s/reconcile.go +++ b/tools/k8s/reconcile.go @@ -86,6 +86,11 @@ func (r *PatchingReconciler[T, PT]) Reconcile(ctx context.Context, req ctrl.Requ result = ctrl.Result{RequeueAfter: *notReadyErr.requeueAfter} delegateErr = nil } + + if notReadyErr.requeue { + result = ctrl.Result{Requeue: true} + delegateErr = nil + } } }) if err != nil { diff --git a/tools/k8s/reconcile_test.go b/tools/k8s/reconcile_test.go index 6b75aac22..5bfcaed6c 100644 --- a/tools/k8s/reconcile_test.go +++ b/tools/k8s/reconcile_test.go @@ -262,6 +262,29 @@ var _ = Describe("Reconcile", func() { }) }) + When("requeue is specified", func() { + BeforeEach(func() { + objectReconciler.reconcileResourceError = k8s.NewNotReadyError().WithRequeue() + }) + + It("sets the ready condition to false", func() { + Expect(fakeStatusWriter.PatchCallCount()).To(Equal(1)) + _, updatedObject, _, _ := fakeStatusWriter.PatchArgsForCall(0) + updatedOrg, ok := updatedObject.(*korifiv1alpha1.CFOrg) + Expect(ok).To(BeTrue()) + + Expect(updatedOrg.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }) + + It("requeues the reconcile event", func() { + Expect(result).To(Equal(ctrl.Result{Requeue: true})) + Expect(err).NotTo(HaveOccurred()) + }) + }) + When("requeueAfter is specified", func() { BeforeEach(func() { objectReconciler.reconcileResourceError = k8s.NewNotReadyError().WithRequeueAfter(time.Minute)