diff --git a/api/handlers/fake/cfservice_instance_repository.go b/api/handlers/fake/cfservice_instance_repository.go index 85dab06a4..9cf21f066 100644 --- a/api/handlers/fake/cfservice_instance_repository.go +++ b/api/handlers/fake/cfservice_instance_repository.go @@ -101,18 +101,33 @@ type CFServiceInstanceRepository struct { result1 repositories.ListResult[repositories.ServiceInstanceRecord] result2 error } - PatchServiceInstanceStub func(context.Context, authorization.Info, repositories.PatchServiceInstanceMessage) (repositories.ServiceInstanceRecord, error) - patchServiceInstanceMutex sync.RWMutex - patchServiceInstanceArgsForCall []struct { + PatchManagedServiceInstanceStub func(context.Context, authorization.Info, repositories.PatchManagedSIMessage) (repositories.ServiceInstanceRecord, error) + patchManagedServiceInstanceMutex sync.RWMutex + patchManagedServiceInstanceArgsForCall []struct { arg1 context.Context arg2 authorization.Info - arg3 repositories.PatchServiceInstanceMessage + arg3 repositories.PatchManagedSIMessage } - patchServiceInstanceReturns struct { + patchManagedServiceInstanceReturns struct { result1 repositories.ServiceInstanceRecord result2 error } - patchServiceInstanceReturnsOnCall map[int]struct { + patchManagedServiceInstanceReturnsOnCall map[int]struct { + result1 repositories.ServiceInstanceRecord + result2 error + } + PatchUserProvidedServiceInstanceStub func(context.Context, authorization.Info, repositories.PatchUPSIMessage) (repositories.ServiceInstanceRecord, error) + patchUserProvidedServiceInstanceMutex sync.RWMutex + patchUserProvidedServiceInstanceArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.PatchUPSIMessage + } + patchUserProvidedServiceInstanceReturns struct { + result1 repositories.ServiceInstanceRecord + result2 error + } + patchUserProvidedServiceInstanceReturnsOnCall map[int]struct { result1 repositories.ServiceInstanceRecord result2 error } @@ -516,18 +531,84 @@ func (fake *CFServiceInstanceRepository) ListServiceInstancesReturnsOnCall(i int }{result1, result2} } -func (fake *CFServiceInstanceRepository) PatchServiceInstance(arg1 context.Context, arg2 authorization.Info, arg3 repositories.PatchServiceInstanceMessage) (repositories.ServiceInstanceRecord, error) { - fake.patchServiceInstanceMutex.Lock() - ret, specificReturn := fake.patchServiceInstanceReturnsOnCall[len(fake.patchServiceInstanceArgsForCall)] - fake.patchServiceInstanceArgsForCall = append(fake.patchServiceInstanceArgsForCall, struct { +func (fake *CFServiceInstanceRepository) PatchManagedServiceInstance(arg1 context.Context, arg2 authorization.Info, arg3 repositories.PatchManagedSIMessage) (repositories.ServiceInstanceRecord, error) { + fake.patchManagedServiceInstanceMutex.Lock() + ret, specificReturn := fake.patchManagedServiceInstanceReturnsOnCall[len(fake.patchManagedServiceInstanceArgsForCall)] + fake.patchManagedServiceInstanceArgsForCall = append(fake.patchManagedServiceInstanceArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.PatchManagedSIMessage + }{arg1, arg2, arg3}) + stub := fake.PatchManagedServiceInstanceStub + fakeReturns := fake.patchManagedServiceInstanceReturns + fake.recordInvocation("PatchManagedServiceInstance", []interface{}{arg1, arg2, arg3}) + fake.patchManagedServiceInstanceMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *CFServiceInstanceRepository) PatchManagedServiceInstanceCallCount() int { + fake.patchManagedServiceInstanceMutex.RLock() + defer fake.patchManagedServiceInstanceMutex.RUnlock() + return len(fake.patchManagedServiceInstanceArgsForCall) +} + +func (fake *CFServiceInstanceRepository) PatchManagedServiceInstanceCalls(stub func(context.Context, authorization.Info, repositories.PatchManagedSIMessage) (repositories.ServiceInstanceRecord, error)) { + fake.patchManagedServiceInstanceMutex.Lock() + defer fake.patchManagedServiceInstanceMutex.Unlock() + fake.PatchManagedServiceInstanceStub = stub +} + +func (fake *CFServiceInstanceRepository) PatchManagedServiceInstanceArgsForCall(i int) (context.Context, authorization.Info, repositories.PatchManagedSIMessage) { + fake.patchManagedServiceInstanceMutex.RLock() + defer fake.patchManagedServiceInstanceMutex.RUnlock() + argsForCall := fake.patchManagedServiceInstanceArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServiceInstanceRepository) PatchManagedServiceInstanceReturns(result1 repositories.ServiceInstanceRecord, result2 error) { + fake.patchManagedServiceInstanceMutex.Lock() + defer fake.patchManagedServiceInstanceMutex.Unlock() + fake.PatchManagedServiceInstanceStub = nil + fake.patchManagedServiceInstanceReturns = struct { + result1 repositories.ServiceInstanceRecord + result2 error + }{result1, result2} +} + +func (fake *CFServiceInstanceRepository) PatchManagedServiceInstanceReturnsOnCall(i int, result1 repositories.ServiceInstanceRecord, result2 error) { + fake.patchManagedServiceInstanceMutex.Lock() + defer fake.patchManagedServiceInstanceMutex.Unlock() + fake.PatchManagedServiceInstanceStub = nil + if fake.patchManagedServiceInstanceReturnsOnCall == nil { + fake.patchManagedServiceInstanceReturnsOnCall = make(map[int]struct { + result1 repositories.ServiceInstanceRecord + result2 error + }) + } + fake.patchManagedServiceInstanceReturnsOnCall[i] = struct { + result1 repositories.ServiceInstanceRecord + result2 error + }{result1, result2} +} + +func (fake *CFServiceInstanceRepository) PatchUserProvidedServiceInstance(arg1 context.Context, arg2 authorization.Info, arg3 repositories.PatchUPSIMessage) (repositories.ServiceInstanceRecord, error) { + fake.patchUserProvidedServiceInstanceMutex.Lock() + ret, specificReturn := fake.patchUserProvidedServiceInstanceReturnsOnCall[len(fake.patchUserProvidedServiceInstanceArgsForCall)] + fake.patchUserProvidedServiceInstanceArgsForCall = append(fake.patchUserProvidedServiceInstanceArgsForCall, struct { arg1 context.Context arg2 authorization.Info - arg3 repositories.PatchServiceInstanceMessage + arg3 repositories.PatchUPSIMessage }{arg1, arg2, arg3}) - stub := fake.PatchServiceInstanceStub - fakeReturns := fake.patchServiceInstanceReturns - fake.recordInvocation("PatchServiceInstance", []interface{}{arg1, arg2, arg3}) - fake.patchServiceInstanceMutex.Unlock() + stub := fake.PatchUserProvidedServiceInstanceStub + fakeReturns := fake.patchUserProvidedServiceInstanceReturns + fake.recordInvocation("PatchUserProvidedServiceInstance", []interface{}{arg1, arg2, arg3}) + fake.patchUserProvidedServiceInstanceMutex.Unlock() if stub != nil { return stub(arg1, arg2, arg3) } @@ -537,46 +618,46 @@ func (fake *CFServiceInstanceRepository) PatchServiceInstance(arg1 context.Conte return fakeReturns.result1, fakeReturns.result2 } -func (fake *CFServiceInstanceRepository) PatchServiceInstanceCallCount() int { - fake.patchServiceInstanceMutex.RLock() - defer fake.patchServiceInstanceMutex.RUnlock() - return len(fake.patchServiceInstanceArgsForCall) +func (fake *CFServiceInstanceRepository) PatchUserProvidedServiceInstanceCallCount() int { + fake.patchUserProvidedServiceInstanceMutex.RLock() + defer fake.patchUserProvidedServiceInstanceMutex.RUnlock() + return len(fake.patchUserProvidedServiceInstanceArgsForCall) } -func (fake *CFServiceInstanceRepository) PatchServiceInstanceCalls(stub func(context.Context, authorization.Info, repositories.PatchServiceInstanceMessage) (repositories.ServiceInstanceRecord, error)) { - fake.patchServiceInstanceMutex.Lock() - defer fake.patchServiceInstanceMutex.Unlock() - fake.PatchServiceInstanceStub = stub +func (fake *CFServiceInstanceRepository) PatchUserProvidedServiceInstanceCalls(stub func(context.Context, authorization.Info, repositories.PatchUPSIMessage) (repositories.ServiceInstanceRecord, error)) { + fake.patchUserProvidedServiceInstanceMutex.Lock() + defer fake.patchUserProvidedServiceInstanceMutex.Unlock() + fake.PatchUserProvidedServiceInstanceStub = stub } -func (fake *CFServiceInstanceRepository) PatchServiceInstanceArgsForCall(i int) (context.Context, authorization.Info, repositories.PatchServiceInstanceMessage) { - fake.patchServiceInstanceMutex.RLock() - defer fake.patchServiceInstanceMutex.RUnlock() - argsForCall := fake.patchServiceInstanceArgsForCall[i] +func (fake *CFServiceInstanceRepository) PatchUserProvidedServiceInstanceArgsForCall(i int) (context.Context, authorization.Info, repositories.PatchUPSIMessage) { + fake.patchUserProvidedServiceInstanceMutex.RLock() + defer fake.patchUserProvidedServiceInstanceMutex.RUnlock() + argsForCall := fake.patchUserProvidedServiceInstanceArgsForCall[i] return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *CFServiceInstanceRepository) PatchServiceInstanceReturns(result1 repositories.ServiceInstanceRecord, result2 error) { - fake.patchServiceInstanceMutex.Lock() - defer fake.patchServiceInstanceMutex.Unlock() - fake.PatchServiceInstanceStub = nil - fake.patchServiceInstanceReturns = struct { +func (fake *CFServiceInstanceRepository) PatchUserProvidedServiceInstanceReturns(result1 repositories.ServiceInstanceRecord, result2 error) { + fake.patchUserProvidedServiceInstanceMutex.Lock() + defer fake.patchUserProvidedServiceInstanceMutex.Unlock() + fake.PatchUserProvidedServiceInstanceStub = nil + fake.patchUserProvidedServiceInstanceReturns = struct { result1 repositories.ServiceInstanceRecord result2 error }{result1, result2} } -func (fake *CFServiceInstanceRepository) PatchServiceInstanceReturnsOnCall(i int, result1 repositories.ServiceInstanceRecord, result2 error) { - fake.patchServiceInstanceMutex.Lock() - defer fake.patchServiceInstanceMutex.Unlock() - fake.PatchServiceInstanceStub = nil - if fake.patchServiceInstanceReturnsOnCall == nil { - fake.patchServiceInstanceReturnsOnCall = make(map[int]struct { +func (fake *CFServiceInstanceRepository) PatchUserProvidedServiceInstanceReturnsOnCall(i int, result1 repositories.ServiceInstanceRecord, result2 error) { + fake.patchUserProvidedServiceInstanceMutex.Lock() + defer fake.patchUserProvidedServiceInstanceMutex.Unlock() + fake.PatchUserProvidedServiceInstanceStub = nil + if fake.patchUserProvidedServiceInstanceReturnsOnCall == nil { + fake.patchUserProvidedServiceInstanceReturnsOnCall = make(map[int]struct { result1 repositories.ServiceInstanceRecord result2 error }) } - fake.patchServiceInstanceReturnsOnCall[i] = struct { + fake.patchUserProvidedServiceInstanceReturnsOnCall[i] = struct { result1 repositories.ServiceInstanceRecord result2 error }{result1, result2} diff --git a/api/handlers/job.go b/api/handlers/job.go index 17271f92f..1b762a540 100644 --- a/api/handlers/job.go +++ b/api/handlers/job.go @@ -32,6 +32,7 @@ const ( ServiceBrokerDeleteJobType = "service_broker.delete" ManagedServiceInstanceDeleteJobType = "managed_service_instance.delete" ManagedServiceInstanceCreateJobType = "managed_service_instance.create" + ManagedServiceInstanceUpdateJobType = "managed_service_instance.update" ManagedServiceBindingCreateJobType = "managed_service_binding.create" ManagedServiceBindingDeleteJobType = "managed_service_binding.delete" JobTimeoutDuration = 120.0 diff --git a/api/handlers/service_instance.go b/api/handlers/service_instance.go index 0a57e89d1..7222fed6f 100644 --- a/api/handlers/service_instance.go +++ b/api/handlers/service_instance.go @@ -30,7 +30,8 @@ const ( type CFServiceInstanceRepository interface { 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) + PatchUserProvidedServiceInstance(context.Context, authorization.Info, repositories.PatchUPSIMessage) (repositories.ServiceInstanceRecord, error) + PatchManagedServiceInstance(context.Context, authorization.Info, repositories.PatchManagedSIMessage) (repositories.ServiceInstanceRecord, error) ListServiceInstances(context.Context, authorization.Info, repositories.ListServiceInstanceMessage) (repositories.ListResult[repositories.ServiceInstanceRecord], error) GetServiceInstance(context.Context, authorization.Info, string) (repositories.ServiceInstanceRecord, error) GetServiceInstanceCredentials(context.Context, authorization.Info, string) (map[string]any, error) @@ -185,10 +186,21 @@ func (h *ServiceInstance) patch(r *http.Request) (*routing.Response, error) { return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "failed to get service instance") } - patchMessage := payload.ToServiceInstancePatchMessage(serviceInstance.SpaceGUID, serviceInstance.GUID) - serviceInstance, err = h.serviceInstanceRepo.PatchServiceInstance(r.Context(), authInfo, patchMessage) + if serviceInstance.Type == korifiv1alpha1.ManagedType { + patchMessage := payload.ToManagedSIPatchMessage(serviceInstance.SpaceGUID, serviceInstance.GUID) + serviceInstance, err = h.serviceInstanceRepo.PatchManagedServiceInstance(r.Context(), authInfo, patchMessage) + if err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to patch managed service instance") + } + + return routing.NewResponse(http.StatusAccepted). + WithHeader("Location", presenter.JobURLForRedirects(serviceInstance.GUID, presenter.ManagedServiceInstanceUpdateOperation, h.serverURL)), nil + } + + patchMessage := payload.ToUPSIPatchMessage(serviceInstance.SpaceGUID, serviceInstance.GUID) + serviceInstance, err = h.serviceInstanceRepo.PatchUserProvidedServiceInstance(r.Context(), authInfo, patchMessage) if err != nil { - return nil, apierrors.LogAndReturn(logger, err, "failed to patch service instance") + return nil, apierrors.LogAndReturn(logger, err, "failed to patch user provided service instance") } return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServiceInstance(serviceInstance, h.serverURL)), nil diff --git a/api/handlers/service_instance_test.go b/api/handlers/service_instance_test.go index 431e3f057..650361b3b 100644 --- a/api/handlers/service_instance_test.go +++ b/api/handlers/service_instance_test.go @@ -846,7 +846,7 @@ var _ = Describe("ServiceInstance", func() { }, }) - serviceInstanceRepo.PatchServiceInstanceReturns(repositories.ServiceInstanceRecord{ + serviceInstanceRepo.PatchUserProvidedServiceInstanceReturns(repositories.ServiceInstanceRecord{ Name: "new-name", GUID: "service-instance-guid", }, nil) @@ -865,10 +865,10 @@ var _ = Describe("ServiceInstance", func() { Expect(actualAuthInfo).To(Equal(authInfo)) Expect(actualGUID).To(Equal("service-instance-guid")) - Expect(serviceInstanceRepo.PatchServiceInstanceCallCount()).To(Equal(1)) - _, actualAuthInfo, patchMessage := serviceInstanceRepo.PatchServiceInstanceArgsForCall(0) + Expect(serviceInstanceRepo.PatchUserProvidedServiceInstanceCallCount()).To(Equal(1)) + _, actualAuthInfo, patchMessage := serviceInstanceRepo.PatchUserProvidedServiceInstanceArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) - Expect(patchMessage).To(Equal(repositories.PatchServiceInstanceMessage{ + Expect(patchMessage).To(Equal(repositories.PatchUPSIMessage{ GUID: "service-instance-guid", SpaceGUID: "space-guid", Name: tools.PtrTo("new-name"), @@ -927,13 +927,78 @@ var _ = Describe("ServiceInstance", func() { When("patching the service instances fails", func() { BeforeEach(func() { - serviceInstanceRepo.PatchServiceInstanceReturns(repositories.ServiceInstanceRecord{}, errors.New("oops")) + serviceInstanceRepo.PatchUserProvidedServiceInstanceReturns(repositories.ServiceInstanceRecord{}, errors.New("oops")) }) It("returns the error", func() { expectUnknownError() }) }) + When("updating a managed service instance", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServiceInstancePatch{ + Name: tools.PtrTo("new-name"), + Tags: &[]string{"alice", "bob"}, + Credentials: &map[string]any{"foo": "bar"}, + Relationships: &payloads.ServiceInstanceRelationships{ + ServicePlan: &payloads.Relationship{ + Data: &payloads.RelationshipData{ + GUID: "plan-guid", + }, + }, + }, + Metadata: payloads.MetadataPatch{ + Annotations: map[string]*string{"ann2": tools.PtrTo("ann_val2")}, + Labels: map[string]*string{"lab2": tools.PtrTo("lab_val2")}, + }, + }) + + serviceInstanceRepo.GetServiceInstanceReturns(repositories.ServiceInstanceRecord{ + GUID: "service-instance-guid", + SpaceGUID: "space-guid", + Type: korifiv1alpha1.ManagedType, + }, nil) + serviceInstanceRepo.PatchManagedServiceInstanceReturns(repositories.ServiceInstanceRecord{ + Name: "new-name", + GUID: "service-instance-guid", + }, nil) + }) + + It("patches the managed service instance", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + Expect(bodyString(actualReq)).To(Equal("the-json-body")) + + Expect(serviceInstanceRepo.PatchManagedServiceInstanceCallCount()).To(Equal(1)) + _, actualAuthInfo, patchMessage := serviceInstanceRepo.PatchManagedServiceInstanceArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(patchMessage).To(Equal(repositories.PatchManagedSIMessage{ + GUID: "service-instance-guid", + SpaceGUID: "space-guid", + PlanGUID: tools.PtrTo("plan-guid"), + Name: tools.PtrTo("new-name"), + Tags: &[]string{"alice", "bob"}, + MetadataPatch: repositories.MetadataPatch{ + Annotations: map[string]*string{"ann2": tools.PtrTo("ann_val2")}, + Labels: map[string]*string{"lab2": tools.PtrTo("lab_val2")}, + }, + })) + }) + It("returns HTTP 202 Accepted response", func() { + Expect(rr).To(HaveHTTPStatus(http.StatusAccepted)) + Expect(rr).To(HaveHTTPHeaderWithValue("Location", + ContainSubstring("/v3/jobs/managed_service_instance.update~service-instance-guid"))) + }) + When("patching the managed service instances fails", func() { + BeforeEach(func() { + serviceInstanceRepo.PatchManagedServiceInstanceReturns(repositories.ServiceInstanceRecord{}, errors.New("oops")) + }) + + It("returns the error", func() { + expectUnknownError() + }) + }) + }) }) Describe("DELETE /v3/service_instances/:guid", func() { diff --git a/api/main.go b/api/main.go index d95b1413e..b218eae8d 100644 --- a/api/main.go +++ b/api/main.go @@ -421,6 +421,7 @@ func main() { handlers.ServiceBrokerCreateJobType: serviceBrokerRepo, handlers.ServiceBrokerUpdateJobType: serviceBrokerRepo, handlers.ManagedServiceInstanceCreateJobType: serviceInstanceRepo, + handlers.ManagedServiceInstanceUpdateJobType: serviceInstanceRepo, handlers.ManagedServiceBindingCreateJobType: serviceBindingRepo, }, routeRepo, diff --git a/api/payloads/service_instance.go b/api/payloads/service_instance.go index 9f92158e3..9022b7a15 100644 --- a/api/payloads/service_instance.go +++ b/api/payloads/service_instance.go @@ -171,10 +171,11 @@ func (g *ServiceInstanceGet) DecodeFromURLValues(values url.Values) error { } type ServiceInstancePatch struct { - Name *string `json:"name,omitempty"` - Tags *[]string `json:"tags,omitempty"` - Credentials *map[string]any `json:"credentials,omitempty"` - Metadata MetadataPatch `json:"metadata"` + Name *string `json:"name,omitempty"` + Tags *[]string `json:"tags,omitempty"` + Credentials *map[string]any `json:"credentials,omitempty"` + Relationships *ServiceInstanceRelationships `json:"relationships,omitempty"` + Metadata MetadataPatch `json:"metadata"` } func (p ServiceInstancePatch) Validate() error { @@ -183,8 +184,8 @@ func (p ServiceInstancePatch) Validate() error { ) } -func (p ServiceInstancePatch) ToServiceInstancePatchMessage(spaceGUID, appGUID string) repositories.PatchServiceInstanceMessage { - return repositories.PatchServiceInstanceMessage{ +func (p ServiceInstancePatch) ToUPSIPatchMessage(spaceGUID, appGUID string) repositories.PatchUPSIMessage { + return repositories.PatchUPSIMessage{ SpaceGUID: spaceGUID, GUID: appGUID, Name: p.Name, @@ -197,6 +198,24 @@ func (p ServiceInstancePatch) ToServiceInstancePatchMessage(spaceGUID, appGUID s } } +func (p ServiceInstancePatch) ToManagedSIPatchMessage(spaceGUID, appGUID string) repositories.PatchManagedSIMessage { + var planGUID *string + if p.Relationships.ServicePlan != nil { + planGUID = &p.Relationships.ServicePlan.Data.GUID + } + return repositories.PatchManagedSIMessage{ + SpaceGUID: spaceGUID, + PlanGUID: planGUID, + GUID: appGUID, + Name: p.Name, + Tags: p.Tags, + MetadataPatch: repositories.MetadataPatch{ + Labels: p.Metadata.Labels, + Annotations: p.Metadata.Annotations, + }, + } +} + func (p *ServiceInstancePatch) UnmarshalJSON(data []byte) error { type alias ServiceInstancePatch diff --git a/api/payloads/service_instance_test.go b/api/payloads/service_instance_test.go index 589274008..0863f019e 100644 --- a/api/payloads/service_instance_test.go +++ b/api/payloads/service_instance_test.go @@ -533,7 +533,7 @@ var _ = Describe("ServiceInstancePatch", func() { Context("ToServiceInstancePatchMessage", func() { It("converts to repo message correctly", func() { - msg := serviceInstancePatch.ToServiceInstancePatchMessage("space-guid", "app-guid") + msg := serviceInstancePatch.ToUPSIPatchMessage("space-guid", "app-guid") Expect(msg.SpaceGUID).To(Equal("space-guid")) Expect(msg.GUID).To(Equal("app-guid")) Expect(msg.Name).To(PointTo(Equal("service-instance-name"))) diff --git a/api/presenter/job.go b/api/presenter/job.go index d65e1c925..b7fd7fdd9 100644 --- a/api/presenter/job.go +++ b/api/presenter/job.go @@ -31,6 +31,7 @@ const ( ManagedServiceInstanceResourceType = "managed_service_instance" ManagedServiceBindingResourceType = "managed_service_binding" ManagedServiceInstanceCreateOperation = ManagedServiceInstanceResourceType + ".create" + ManagedServiceInstanceUpdateOperation = ManagedServiceInstanceResourceType + ".update" ManagedServiceInstanceDeleteOperation = ManagedServiceInstanceResourceType + ".delete" ManagedServiceBindingCreateOperation = ManagedServiceBindingResourceType + ".create" ManagedServiceBindingDeleteOperation = ManagedServiceBindingResourceType + ".delete" diff --git a/api/repositories/repositories_suite_test.go b/api/repositories/repositories_suite_test.go index 610e4d78d..2ac313c50 100644 --- a/api/repositories/repositories_suite_test.go +++ b/api/repositories/repositories_suite_test.go @@ -366,7 +366,7 @@ func createAppCR(ctx context.Context, k8sClient client.Client, appName, appGUID, return toReturn } -func createServiceInstanceCR(ctx context.Context, k8sClient client.Client, serviceInstanceGUID, spaceGUID, name, secretName string) *korifiv1alpha1.CFServiceInstance { +func createUserProvidedServiceInstanceCR(ctx context.Context, k8sClient client.Client, serviceInstanceGUID, spaceGUID, name, secretName string) *korifiv1alpha1.CFServiceInstance { serviceInstance := &korifiv1alpha1.CFServiceInstance{ ObjectMeta: metav1.ObjectMeta{ Name: serviceInstanceGUID, diff --git a/api/repositories/service_instance_repository.go b/api/repositories/service_instance_repository.go index 3d1f62eee..f75d1a1f9 100644 --- a/api/repositories/service_instance_repository.go +++ b/api/repositories/service_instance_repository.go @@ -68,7 +68,7 @@ type CreateManagedSIMessage struct { Annotations map[string]string } -type PatchServiceInstanceMessage struct { +type PatchUPSIMessage struct { GUID string SpaceGUID string Name *string @@ -77,7 +77,16 @@ type PatchServiceInstanceMessage struct { MetadataPatch } -func (p PatchServiceInstanceMessage) Apply(cfServiceInstance *korifiv1alpha1.CFServiceInstance) { +type PatchManagedSIMessage struct { + GUID string + SpaceGUID string + PlanGUID *string + Name *string + Tags *[]string + MetadataPatch +} + +func (p PatchUPSIMessage) Apply(cfServiceInstance *korifiv1alpha1.CFServiceInstance) { if p.Name != nil { cfServiceInstance.Spec.DisplayName = *p.Name } @@ -87,6 +96,19 @@ func (p PatchServiceInstanceMessage) Apply(cfServiceInstance *korifiv1alpha1.CFS p.MetadataPatch.Apply(cfServiceInstance) } +func (p PatchManagedSIMessage) Apply(cfServiceInstance *korifiv1alpha1.CFServiceInstance) { + if p.Name != nil { + cfServiceInstance.Spec.DisplayName = *p.Name + } + if p.Tags != nil { + cfServiceInstance.Spec.Tags = *p.Tags + } + if p.PlanGUID != nil { + cfServiceInstance.Spec.PlanGUID = *p.PlanGUID + } + p.MetadataPatch.Apply(cfServiceInstance) +} + type ListServiceInstanceMessage struct { Names []string SpaceGUIDs []string @@ -274,7 +296,7 @@ func (r *ServiceInstanceRepo) servicePlanVisible(ctx context.Context, planGUID s return slices.Contains(servicePlan.Spec.Visibility.Organizations, space.Namespace), nil } -func (r *ServiceInstanceRepo) PatchServiceInstance(ctx context.Context, authInfo authorization.Info, message PatchServiceInstanceMessage) (ServiceInstanceRecord, error) { +func (r *ServiceInstanceRepo) PatchUserProvidedServiceInstance(ctx context.Context, authInfo authorization.Info, message PatchUPSIMessage) (ServiceInstanceRecord, error) { cfServiceInstance := &korifiv1alpha1.CFServiceInstance{ ObjectMeta: metav1.ObjectMeta{ Namespace: message.SpaceGUID, @@ -307,6 +329,28 @@ func (r *ServiceInstanceRepo) PatchServiceInstance(ctx context.Context, authInfo return cfServiceInstanceToRecord(*cfServiceInstance), nil } +func (r *ServiceInstanceRepo) PatchManagedServiceInstance(ctx context.Context, authInfo authorization.Info, message PatchManagedSIMessage) (ServiceInstanceRecord, error) { + cfServiceInstance := &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: message.SpaceGUID, + Name: message.GUID, + }, + } + if err := r.klient.Get(ctx, cfServiceInstance); err != nil { + return ServiceInstanceRecord{}, apierrors.FromK8sError(err, ServiceInstanceResourceType) + } + + err := r.klient.Patch(ctx, cfServiceInstance, func() error { + message.Apply(cfServiceInstance) + return nil + }) + if err != nil { + return ServiceInstanceRecord{}, apierrors.FromK8sError(err, ServiceInstanceResourceType) + } + + return cfServiceInstanceToRecord(*cfServiceInstance), nil +} + func (r *ServiceInstanceRepo) migrateLegacyCredentials(ctx context.Context, cfServiceInstance *korifiv1alpha1.CFServiceInstance) (*korifiv1alpha1.CFServiceInstance, error) { cfServiceInstance, err := r.awaiter.AwaitCondition(ctx, r.klient, cfServiceInstance, korifiv1alpha1.StatusConditionReady) if err != nil { diff --git a/api/repositories/service_instance_repository_test.go b/api/repositories/service_instance_repository_test.go index 35401f69f..d8132f707 100644 --- a/api/repositories/service_instance_repository_test.go +++ b/api/repositories/service_instance_repository_test.go @@ -499,19 +499,19 @@ var _ = Describe("ServiceInstanceRepository", func() { }) }) - Describe("PatchServiceInstance", func() { + Describe("PatchUserProvidedServiceInstance", func() { var ( cfServiceInstance *korifiv1alpha1.CFServiceInstance secret *corev1.Secret serviceInstanceRecord repositories.ServiceInstanceRecord - patchMessage repositories.PatchServiceInstanceMessage + patchMessage repositories.PatchUPSIMessage err error ) BeforeEach(func() { serviceInstanceGUID := uuid.NewString() secretName := uuid.NewString() - cfServiceInstance = createServiceInstanceCR(ctx, k8sClient, serviceInstanceGUID, space.Name, serviceInstanceName, secretName) + cfServiceInstance = createUserProvidedServiceInstanceCR(ctx, k8sClient, serviceInstanceGUID, space.Name, serviceInstanceName, secretName) conditionAwaiter.AwaitConditionReturns(cfServiceInstance, nil) Expect(k8s.Patch(ctx, k8sClient, cfServiceInstance, func() { cfServiceInstance.Status.Credentials.Name = secretName @@ -528,7 +528,7 @@ var _ = Describe("ServiceInstanceRepository", func() { } Expect(k8sClient.Create(ctx, secret)).To(Succeed()) - patchMessage = repositories.PatchServiceInstanceMessage{ + patchMessage = repositories.PatchUPSIMessage{ GUID: cfServiceInstance.Name, SpaceGUID: space.Name, Name: tools.PtrTo("new-name"), @@ -542,7 +542,7 @@ var _ = Describe("ServiceInstanceRepository", func() { }) JustBeforeEach(func() { - serviceInstanceRecord, err = serviceInstanceRepo.PatchServiceInstance(ctx, authInfo, patchMessage) + serviceInstanceRecord, err = serviceInstanceRepo.PatchUserProvidedServiceInstance(ctx, authInfo, patchMessage) }) When("authorized in the space", func() { @@ -688,6 +688,139 @@ var _ = Describe("ServiceInstanceRepository", func() { }) }) + Describe("PatchManagedServiceInstance", func() { + var ( + cfServiceInstance *korifiv1alpha1.CFServiceInstance + serviceInstanceGUID string + serviceInstanceRecord repositories.ServiceInstanceRecord + patchMessage repositories.PatchManagedSIMessage + err error + ) + BeforeEach(func() { + serviceInstanceGUID = uuid.NewString() + cfServiceInstance = &korifiv1alpha1.CFServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceInstanceGUID, + Namespace: space.Name, + Labels: map[string]string{"a-label": "a-label-value"}, + Annotations: map[string]string{"an-annotation": "an-annotation-value"}, + }, + Spec: korifiv1alpha1.CFServiceInstanceSpec{ + DisplayName: serviceInstanceName, + Type: "managed", + Tags: []string{"database", "mysql"}, + PlanGUID: "plan-guid", + }, + } + + Expect(k8sClient.Create(ctx, cfServiceInstance)).To(Succeed()) + + patchMessage = repositories.PatchManagedSIMessage{ + GUID: cfServiceInstance.Name, + SpaceGUID: space.Name, + Name: tools.PtrTo("new-name"), + PlanGUID: tools.PtrTo("new-plan-guid"), + Tags: &[]string{"new"}, + MetadataPatch: repositories.MetadataPatch{ + Labels: map[string]*string{"new-label": tools.PtrTo("new-label-value")}, + Annotations: map[string]*string{"new-annotation": tools.PtrTo("new-annotation-value")}, + }, + } + }) + + JustBeforeEach(func() { + serviceInstanceRecord, err = serviceInstanceRepo.PatchManagedServiceInstance(ctx, authInfo, patchMessage) + }) + + It("returns a Forbidden error", func() { + Expect(err).To(BeAssignableToTypeOf(apierrors.ForbiddenError{})) + }) + + When("authorized in the space", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, orgUserRole.Name, org.Name) + createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) + }) + + It("returns the updated record", func() { + Expect(err).NotTo(HaveOccurred()) + Expect(serviceInstanceRecord.Name).To(Equal("new-name")) + Expect(serviceInstanceRecord.Tags).To(ConsistOf("new")) + Expect(serviceInstanceRecord.Labels).To(HaveKeyWithValue("a-label", "a-label-value")) + Expect(serviceInstanceRecord.Labels).To(HaveKeyWithValue("new-label", "new-label-value")) + Expect(serviceInstanceRecord.Annotations).To(HaveLen(2)) + Expect(serviceInstanceRecord.Annotations).To(HaveKeyWithValue("an-annotation", "an-annotation-value")) + Expect(serviceInstanceRecord.Annotations).To(HaveKeyWithValue("new-annotation", "new-annotation-value")) + Expect(serviceInstanceRecord.Relationships()).To(HaveKeyWithValue("service_plan", "new-plan-guid")) + }) + + It("updates the service instance", func() { + Expect(err).NotTo(HaveOccurred()) + serviceInstance := new(korifiv1alpha1.CFServiceInstance) + + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceInstance), serviceInstance)).To(Succeed()) + g.Expect(serviceInstance.Spec.DisplayName).To(Equal("new-name")) + g.Expect(serviceInstance.Spec.Tags).To(ConsistOf("new")) + g.Expect(serviceInstance.Spec.PlanGUID).To(Equal("new-plan-guid")) + g.Expect(serviceInstance.Labels).To(HaveKeyWithValue("a-label", "a-label-value")) + g.Expect(serviceInstance.Labels).To(HaveKeyWithValue("new-label", "new-label-value")) + g.Expect(serviceInstance.Annotations).To(HaveLen(2)) + g.Expect(serviceInstance.Annotations).To(HaveKeyWithValue("an-annotation", "an-annotation-value")) + g.Expect(serviceInstance.Annotations).To(HaveKeyWithValue("new-annotation", "new-annotation-value")) + }).Should(Succeed()) + }) + + When("tags is an empty list", func() { + BeforeEach(func() { + patchMessage.Tags = &[]string{} + }) + + It("clears the tags", func() { + Expect(err).NotTo(HaveOccurred()) + serviceInstance := new(korifiv1alpha1.CFServiceInstance) + + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceInstance), serviceInstance)).To(Succeed()) + g.Expect(serviceInstance.Spec.Tags).To(BeEmpty()) + }).Should(Succeed()) + }) + }) + + When("tags is nil", func() { + BeforeEach(func() { + patchMessage.Tags = nil + }) + + It("preserves the tags", func() { + Expect(err).NotTo(HaveOccurred()) + serviceInstance := new(korifiv1alpha1.CFServiceInstance) + + Consistently(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceInstance), serviceInstance)).To(Succeed()) + g.Expect(serviceInstance.Spec.Tags).To(ConsistOf("database", "mysql")) + }).Should(Succeed()) + }) + }) + + When("the plan GUID is nil", func() { + BeforeEach(func() { + patchMessage.PlanGUID = nil + }) + + It("preserves the plan GUID", func() { + Expect(err).NotTo(HaveOccurred()) + serviceInstance := new(korifiv1alpha1.CFServiceInstance) + + Consistently(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceInstance), serviceInstance)).To(Succeed()) + g.Expect(serviceInstance.Spec.PlanGUID).To(Equal("plan-guid")) + }).Should(Succeed()) + }) + }) + }) + }) + Describe("ListServiceInstances", func() { var ( cfServiceInstance1 *korifiv1alpha1.CFServiceInstance @@ -815,8 +948,8 @@ var _ = Describe("ServiceInstanceRepository", func() { BeforeEach(func() { space2 = createSpaceWithCleanup(ctx, org.Name, prefixedGUID("space2")) - 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")) + serviceInstance = createUserProvidedServiceInstanceCR(ctx, k8sClient, prefixedGUID("service-instance"), space.Name, "the-service-instance", prefixedGUID("secret")) + createUserProvidedServiceInstanceCR(ctx, k8sClient, prefixedGUID("service-instance"), space2.Name, "some-other-service-instance", prefixedGUID("secret")) getGUID = serviceInstance.Name }) @@ -902,7 +1035,7 @@ var _ = Describe("ServiceInstanceRepository", func() { BeforeEach(func() { 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")) + createUserProvidedServiceInstanceCR(ctx, k8sClient, getGUID, space2.Name, "the-service-instance", prefixedGUID("secret")) }) It("returns a error", func() { @@ -1013,7 +1146,7 @@ var _ = Describe("ServiceInstanceRepository", func() { ) BeforeEach(func() { - serviceInstance = createServiceInstanceCR(ctx, k8sClient, prefixedGUID("service-instance"), space.Name, "the-service-instance", prefixedGUID("secret")) + serviceInstance = createUserProvidedServiceInstanceCR(ctx, k8sClient, prefixedGUID("service-instance"), space.Name, "the-service-instance", prefixedGUID("secret")) deleteMessage = repositories.DeleteServiceInstanceMessage{ GUID: serviceInstance.Name, diff --git a/controllers/api/v1alpha1/cfserviceinstance_types.go b/controllers/api/v1alpha1/cfserviceinstance_types.go index 020a65eb1..e45add546 100644 --- a/controllers/api/v1alpha1/cfserviceinstance_types.go +++ b/controllers/api/v1alpha1/cfserviceinstance_types.go @@ -33,6 +33,7 @@ const ( DeprovisionWithoutBrokerAnnotation = "korifi.cloudfoundry.org/deprovision-without-broker" ProvisioningFailedCondition = "ProvisioningFailed" + UpdateFailedCondition = "UpdateFailed" DeprovisioningFailedCondition = "DeprovisioningFailed" ) @@ -90,6 +91,14 @@ type CFServiceInstanceStatus struct { //+kubebuilder:validation:Optional MaintenanceInfo MaintenanceInfo `json:"maintenanceInfo"` + // The plan guid of the service instance. Only makes seense for managed service instances + //+kubebuilder:validation:Optional + PlanGUID string `json:"planGuid"` + + // True if the service instance has been successfully provisioned. Only makes seense for managed service instances + //+kubebuilder:validation:Optional + Provisioned bool `json:"provisioned"` + // True if there is an upgrade available for for the service instance (i.e. the plan has a new version). Only makes seense for managed service instances //+kubebuilder:validation:Optional UpgradeAvailable bool `json:"upgradeAvailable"` diff --git a/controllers/controllers/services/instances/managed/controller.go b/controllers/controllers/services/instances/managed/controller.go index 3b73f6403..2bb631b78 100644 --- a/controllers/controllers/services/instances/managed/controller.go +++ b/controllers/controllers/services/instances/managed/controller.go @@ -115,6 +115,26 @@ func (r *Reconciler) isManaged(object client.Object) bool { return serviceInstance.Spec.Type == korifiv1alpha1.ManagedType } +func needsUpdate(instance *korifiv1alpha1.CFServiceInstance) bool { + return instance.Spec.PlanGUID != instance.Status.PlanGUID +} + +func (r *Reconciler) ensurePlanVisible(ctx context.Context, serviceInstance *korifiv1alpha1.CFServiceInstance, servicePlan *korifiv1alpha1.CFServicePlan) error { + log := logr.FromContextOrDiscard(ctx) + + planVisible, err := r.isServicePlanVisible(ctx, serviceInstance, servicePlan) + if err != nil { + log.Error(err, "failed to check service plan visibility") + return err + } + + if !planVisible { + return k8s.NewNotReadyError().WithMessage("The service plan is disabled").WithReason("InvalidServicePlan").WithNoRequeue() + } + + return nil +} + //+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;patch //+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfserviceinstances/finalizers,verbs=update @@ -143,27 +163,46 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, serviceInstance *kor serviceInstance.Status.UpgradeAvailable = serviceInstance.Status.MaintenanceInfo.Version != serviceInstanceAssets.ServicePlan.Spec.MaintenanceInfo.Version - if isReady(serviceInstance) { + if isProvisioningFailed(serviceInstance) { + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ServiceInstanceFailed").WithNoRequeue() + } + + if serviceInstance.Spec.ServiceLabel == nil { + serviceInstance.Spec.ServiceLabel = tools.PtrTo(serviceInstanceAssets.ServiceOffering.Spec.Name) return ctrl.Result{}, nil } - if isFailed(serviceInstance) { - return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisioningFailed").WithNoRequeue() + if !serviceInstance.Status.Provisioned { + return r.reconcileProvisionedServiceInstance(ctx, serviceInstance, serviceInstanceAssets, osbapiClient) } - planVisible, err := r.isServicePlanVisible(ctx, serviceInstance, serviceInstanceAssets.ServicePlan) - if err != nil { - log.Error(err, "failed to check service plan visibility") - return ctrl.Result{}, err + // Backfill status.PlanGUID for legacy provisioned instances without it (one-time migration) + if serviceInstance.Status.PlanGUID == "" { + serviceInstance.Status.PlanGUID = serviceInstance.Spec.PlanGUID + return ctrl.Result{}, nil } - if !planVisible { - return ctrl.Result{}, - k8s.NewNotReadyError().WithMessage("The service plan is disabled").WithReason("InvalidServicePlan").WithNoRequeue() + if !needsUpdate(serviceInstance) { + return ctrl.Result{}, nil } - if serviceInstance.Spec.ServiceLabel == nil { - serviceInstance.Spec.ServiceLabel = tools.PtrTo(serviceInstanceAssets.ServiceOffering.Spec.Name) + if isUpdateFailed(serviceInstance) { + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("UpdateFailed").WithNoRequeue() + } + + return r.reconcileUpdatedServiceInstance(ctx, serviceInstance, serviceInstanceAssets, osbapiClient) +} + +func (r *Reconciler) reconcileProvisionedServiceInstance( //nolint:dupl + ctx context.Context, + serviceInstance *korifiv1alpha1.CFServiceInstance, + serviceInstanceAssets osbapi.ServiceInstanceAssets, + osbapiClient osbapi.BrokerClient, +) (ctrl.Result, error) { + log := logr.FromContextOrDiscard(ctx) + + if err := r.ensurePlanVisible(ctx, serviceInstance, serviceInstanceAssets.ServicePlan); err != nil { + return ctrl.Result{}, err } provisionResponse, err := r.provisionServiceInstance(ctx, serviceInstance, serviceInstanceAssets, osbapiClient) @@ -177,14 +216,50 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, serviceInstance *kor if err != nil { return ctrl.Result{}, err } - return r.processProvisionOperation(serviceInstance, lastOpResponse) + return r.processProvisionOperation(serviceInstance, lastOpResponse, serviceInstanceAssets) } + serviceInstance.Status.Provisioned = true + + serviceInstance.Status.PlanGUID = serviceInstance.Spec.PlanGUID serviceInstance.Status.MaintenanceInfo = serviceInstanceAssets.ServicePlan.Spec.MaintenanceInfo serviceInstance.Status.LastOperation.State = "succeeded" return ctrl.Result{}, nil } +func (r *Reconciler) reconcileUpdatedServiceInstance( //nolint:dupl + ctx context.Context, + serviceInstance *korifiv1alpha1.CFServiceInstance, + serviceInstanceAssets osbapi.ServiceInstanceAssets, + osbapiClient osbapi.BrokerClient, +) (ctrl.Result, error) { + log := logr.FromContextOrDiscard(ctx) + + if err := r.ensurePlanVisible(ctx, serviceInstance, serviceInstanceAssets.ServicePlan); err != nil { + return ctrl.Result{}, err + } + + updateResponse, err := r.updateServiceInstance(ctx, serviceInstance, serviceInstanceAssets, osbapiClient) + if err != nil { + log.Error(err, "failed to update service instance") + return ctrl.Result{}, fmt.Errorf("failed to update service instance: %w", err) + } + if updateResponse.IsAsync { + lastOpResponse, err := r.pollLastOperation(ctx, serviceInstance, serviceInstanceAssets, osbapiClient, updateResponse.Operation) + if err != nil { + return ctrl.Result{}, err + } + return r.processUpdateOperation(serviceInstance, lastOpResponse, serviceInstanceAssets) + } + + serviceInstance.Status.LastOperation.State = "succeeded" + if serviceInstance.Spec.PlanGUID != serviceInstance.Status.PlanGUID { + serviceInstance.Status.PlanGUID = serviceInstance.Spec.PlanGUID + serviceInstance.Status.MaintenanceInfo = serviceInstanceAssets.ServicePlan.Spec.MaintenanceInfo + } + return ctrl.Result{}, nil +} + func (r *Reconciler) provisionServiceInstance( ctx context.Context, serviceInstance *korifiv1alpha1.CFServiceInstance, @@ -247,8 +322,12 @@ func (r *Reconciler) provisionServiceInstance( func (r *Reconciler) processProvisionOperation( serviceInstance *korifiv1alpha1.CFServiceInstance, lastOpResponse osbapi.LastOperationResponse, + assets osbapi.ServiceInstanceAssets, ) (ctrl.Result, error) { if lastOpResponse.State == "succeeded" { + serviceInstance.Status.Provisioned = true + serviceInstance.Status.PlanGUID = serviceInstance.Spec.PlanGUID + serviceInstance.Status.MaintenanceInfo = assets.ServicePlan.Spec.MaintenanceInfo return ctrl.Result{}, nil } @@ -267,6 +346,80 @@ func (r *Reconciler) processProvisionOperation( return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisionInProgress").WithRequeue() } +func (r *Reconciler) updateServiceInstance( + ctx context.Context, + serviceInstance *korifiv1alpha1.CFServiceInstance, + assets osbapi.ServiceInstanceAssets, + osbapiClient osbapi.BrokerClient, +) (osbapi.UpdateResponse, error) { + log := logr.FromContextOrDiscard(ctx).WithName("update-service-instance") + + serviceInstance.Status.LastOperation = korifiv1alpha1.LastOperation{ + Type: "update", + State: "in progress", + } + + var updateResponse osbapi.UpdateResponse + var err error + updateResponse, err = osbapiClient.Update(ctx, osbapi.UpdatePayload{ + InstanceID: serviceInstance.Name, + UpdateRequest: osbapi.UpdateRequest{ + ServiceId: assets.ServiceOffering.Spec.BrokerCatalog.ID, + PlanID: assets.ServicePlan.Spec.BrokerCatalog.ID, + }, + }) + if err != nil { + log.Error(err, "failed to update service") + + if osbapi.IsUnrecoveralbeError(err) { + serviceInstance.Status.LastOperation.State = "failed" + meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.UpdateFailedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: serviceInstance.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "UpdateFailed", + Message: err.Error(), + }) + return osbapi.UpdateResponse{}, + k8s.NewNotReadyError().WithReason("UpdateFailed").WithNoRequeue() + + } + + return osbapi.UpdateResponse{}, err + } + + return updateResponse, nil +} + +func (r *Reconciler) processUpdateOperation( + serviceInstance *korifiv1alpha1.CFServiceInstance, + lastOpResponse osbapi.LastOperationResponse, + assets osbapi.ServiceInstanceAssets, +) (ctrl.Result, error) { + if lastOpResponse.State == "succeeded" { + if serviceInstance.Spec.PlanGUID != serviceInstance.Status.PlanGUID { + serviceInstance.Status.PlanGUID = serviceInstance.Spec.PlanGUID + serviceInstance.Status.MaintenanceInfo = assets.ServicePlan.Spec.MaintenanceInfo + } + return ctrl.Result{}, nil + } + + if lastOpResponse.State == "failed" { + meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.UpdateFailedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: serviceInstance.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "UpdateFailed", + Message: lastOpResponse.Description, + }) + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("UpdateFailed") + } + + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("UpdateInProgress").WithRequeue() +} + func (r *Reconciler) finalize( ctx context.Context, serviceInstance *korifiv1alpha1.CFServiceInstance, @@ -471,10 +624,14 @@ func (r *Reconciler) getNamespace(ctx context.Context, namespaceName string) (*c return namespace, nil } -func isFailed(instance *korifiv1alpha1.CFServiceInstance) bool { - return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.ProvisioningFailedCondition) +func isUpdateFailed(instance *korifiv1alpha1.CFServiceInstance) bool { + cond := meta.FindStatusCondition(instance.Status.Conditions, korifiv1alpha1.UpdateFailedCondition) + if cond == nil { + return false + } + return cond.Status == metav1.ConditionTrue && cond.ObservedGeneration == instance.Generation } -func isReady(instance *korifiv1alpha1.CFServiceInstance) bool { - return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.StatusConditionReady) +func isProvisioningFailed(instance *korifiv1alpha1.CFServiceInstance) bool { + return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.ProvisioningFailedCondition) } diff --git a/controllers/controllers/services/instances/managed/controller_test.go b/controllers/controllers/services/instances/managed/controller_test.go index a04f2e713..4fefd2d0a 100644 --- a/controllers/controllers/services/instances/managed/controller_test.go +++ b/controllers/controllers/services/instances/managed/controller_test.go @@ -3,6 +3,7 @@ package managed_test import ( "errors" "net/http" + "time" "github.com/google/uuid" "sigs.k8s.io/controller-runtime/pkg/client" @@ -25,10 +26,11 @@ import ( var _ = Describe("CFServiceInstance", func() { var ( - brokerClient *fake.BrokerClient - instance *korifiv1alpha1.CFServiceInstance - serviceBroker *korifiv1alpha1.CFServiceBroker - servicePlan *korifiv1alpha1.CFServicePlan + brokerClient *fake.BrokerClient + instance *korifiv1alpha1.CFServiceInstance + serviceBroker *korifiv1alpha1.CFServiceBroker + serviceOffering *korifiv1alpha1.CFServiceOffering + servicePlan *korifiv1alpha1.CFServicePlan ) BeforeEach(func() { @@ -62,7 +64,7 @@ var _ = Describe("CFServiceInstance", func() { } Expect(adminClient.Create(ctx, namespace)).To(Succeed()) - serviceOffering := &korifiv1alpha1.CFServiceOffering{ + serviceOffering = &korifiv1alpha1.CFServiceOffering{ ObjectMeta: metav1.ObjectMeta{ Name: uuid.NewString(), Namespace: rootNamespace, @@ -116,7 +118,6 @@ var _ = Describe("CFServiceInstance", func() { PlanGUID: servicePlan.Name, }, } - Expect(adminClient.Create(ctx, instance)).To(Succeed()) }) @@ -127,6 +128,13 @@ var _ = Describe("CFServiceInstance", func() { }).Should(Succeed()) }) + It("sets the Provisioned status field", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Provisioned).To(BeTrue()) + }).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()) @@ -535,6 +543,8 @@ var _ = Describe("CFServiceInstance", func() { When("the instance has become ready", func() { BeforeEach(func() { Expect(k8s.Patch(ctx, adminClient, instance, func() { + instance.Status.Provisioned = true + instance.Status.PlanGUID = instance.Spec.PlanGUID instance.Status.MaintenanceInfo.Version = "1.2.3" meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ Type: korifiv1alpha1.StatusConditionReady, @@ -753,6 +763,330 @@ var _ = Describe("CFServiceInstance", func() { }) }) + Describe("service instance update", func() { + var servicePlan2 *korifiv1alpha1.CFServicePlan + + BeforeEach(func() { + brokerClient.UpdateReturns(osbapi.UpdateResponse{}, nil) + + servicePlan2 = &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", + }, + BrokerCatalog: korifiv1alpha1.ServicePlanBrokerCatalog{ + ID: "service-plan-id-2", + }, + MaintenanceInfo: korifiv1alpha1.MaintenanceInfo{ + Version: "1.2.3", + }, + }, + } + Expect(adminClient.Create(ctx, servicePlan2)).To(Succeed()) + }) + JustBeforeEach(func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.Provisioned).To(BeTrue()) + }).Should(Succeed()) + + Expect(k8s.Patch(ctx, adminClient, instance, func() { + if instance.ObjectMeta.Labels == nil { + instance.ObjectMeta.Labels = map[string]string{} + } + instance.ObjectMeta.Labels["test-label"] = "test-value" + })).To(Succeed()) + }) + + It("does not perform broker update", func() { + Consistently(func(g Gomega) { + g.Expect(brokerClient.UpdateCallCount()).To(BeZero()) + }).WithTimeout(2 * time.Second).Should(Succeed()) + }) + + When("the plan is updated", func() { + JustBeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, instance, func() { + instance.Spec.PlanGUID = servicePlan2.Name + })).To(Succeed()) + }) + + It("updates the PlanGUID status field", 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)), + ))) + g.Expect(instance.Status.Provisioned).To(BeTrue()) + g.Expect(instance.Status.PlanGUID).To(Equal(servicePlan2.Name)) + }).Should(Succeed()) + }) + + It("sends an update request to the broker", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.UpdateCallCount()).To(BeNumerically(">=", 1)) + _, actualUpdatePayload := brokerClient.UpdateArgsForCall(brokerClient.UpdateCallCount() - 1) + g.Expect(actualUpdatePayload).To(Equal(osbapi.UpdatePayload{ + InstanceID: instance.Name, + UpdateRequest: osbapi.UpdateRequest{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id-2", + }, + })) + }).Should(Succeed()) + }) + + When("the new plan is not visible", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, servicePlan2, func() { + servicePlan2.Spec.Visibility = korifiv1alpha1.ServicePlanVisibility{ + Type: korifiv1alpha1.AdminServicePlanVisibilityType, + } + })).To(Succeed()) + }) + + It("fails the instance with InvalidServicePlan", 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("InvalidServicePlan")), + HasMessage(Equal("The service plan is disabled")), + ))) + }).Should(Succeed()) + }) + + It("does not send an update request to the broker", 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("InvalidServicePlan")), + ))) + }).Should(Succeed()) + + Consistently(func(g Gomega) { + g.Expect(brokerClient.UpdateCallCount()).To(BeZero()) + }).WithTimeout(time.Second).Should(Succeed()) + }) + }) + + When("service update fails with recoverable error", func() { + BeforeEach(func() { + brokerClient.UpdateReturns(osbapi.UpdateResponse{}, errors.New("update-failed")) + }) + + It("keeps trying to update the instance", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.UpdateCallCount()).To(BeNumerically(">", 1)) + _, updatePayload := brokerClient.UpdateArgsForCall(brokerClient.UpdateCallCount() - 1) + g.Expect(updatePayload).To(Equal(osbapi.UpdatePayload{ + InstanceID: instance.Name, + UpdateRequest: osbapi.UpdateRequest{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id-2", + }, + })) + }).Should(Succeed()) + }) + + It("sets in progress state in instance last operation", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(brokerClient.UpdateCallCount()).To(BeNumerically(">=", 1)) + g.Expect(instance.Status.LastOperation).To(Equal(korifiv1alpha1.LastOperation{ + Type: "update", + State: "in progress", + })) + }).Should(Succeed()) + }) + }) + + When("service update fails with unrecoverable error", func() { + BeforeEach(func() { + brokerClient.UpdateReturns(osbapi.UpdateResponse{}, osbapi.UnrecoverableError{Status: http.StatusBadRequest}) + }) + + It("fails the instance", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + g.Expect(instance.Status.Conditions).To(ContainElements( + SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ), + SatisfyAll( + HasType(Equal(korifiv1alpha1.UpdateFailedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + HasReason(Equal("UpdateFailed")), + HasMessage(ContainSubstring("The server responded with status: 400")), + ), + )) + }).Should(Succeed()) + }) + + It("sets failed state in instance last operation", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(brokerClient.UpdateCallCount()).To(BeNumerically(">=", 1)) + g.Expect(instance.Status.LastOperation).To(Equal(korifiv1alpha1.LastOperation{ + Type: "update", + State: "failed", + })) + }).Should(Succeed()) + + Consistently(func(g Gomega) { + g.Expect(brokerClient.UpdateCallCount()).To(Equal(1)) + }).WithTimeout(time.Second).Should(Succeed()) + }) + }) + + When("the update is asynchronous", func() { + BeforeEach(func() { + brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{ + State: "in-progress-or-whatever", + }, nil) + + brokerClient.UpdateReturns(osbapi.UpdateResponse{ + IsAsync: true, + Operation: "operation-1", + }, nil) + }) + + It("set sets 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)), + HasReason(Equal("UpdateInProgress")), + ))) + }).Should(Succeed()) + }) + + It("sets in progress state in instance last operation", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(brokerClient.UpdateCallCount()).To(BeNumerically(">=", 1)) + g.Expect(instance.Status.LastOperation).To(Equal(korifiv1alpha1.LastOperation{ + Type: "update", + State: "in progress", + })) + }).Should(Succeed()) + }) + + It("continuously checks the last operation", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(BeNumerically(">", 1)) + _, lastOp := brokerClient.GetServiceInstanceLastOperationArgsForCall(brokerClient.GetServiceInstanceLastOperationCallCount() - 1) + g.Expect(lastOp).To(Equal(osbapi.GetInstanceLastOperationRequest{ + InstanceID: instance.Name, + GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id-2", + Operation: "operation-1", + }, + })) + }).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 succeeded", func() { + BeforeEach(func() { + brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{ + State: "succeeded", + }, nil) + }) + + 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)), + ))) + }).Should(Succeed()) + }) + }) + + When("the last operation is failed", func() { + BeforeEach(func() { + brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{ + State: "failed", + Description: "update-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.UpdateFailedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + HasReason(Equal("UpdateFailed")), + HasMessage(Equal("update-failed")), + ))) + }).Should(Succeed()) + }) + + It("sets failed state in instance last operation", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(brokerClient.UpdateCallCount()).To(BeNumerically(">=", 1)) + g.Expect(instance.Status.LastOperation).To(Equal(korifiv1alpha1.LastOperation{ + Type: "update", + State: "failed", + Description: "update-failed", + })) + }).Should(Succeed()) + }) + }) + }) + }) + }) + When("the service instance is user-provided", func() { BeforeEach(func() { Expect(k8s.PatchResource(ctx, adminClient, instance, func() { diff --git a/controllers/controllers/services/osbapi/client.go b/controllers/controllers/services/osbapi/client.go index 357f5366b..52c448f96 100644 --- a/controllers/controllers/services/osbapi/client.go +++ b/controllers/controllers/services/osbapi/client.go @@ -110,6 +110,40 @@ func (c *Client) Provision(ctx context.Context, payload ProvisionPayload) (Provi return response, nil } +func (c *Client) Update(ctx context.Context, payload UpdatePayload) (UpdateResponse, error) { + statusCode, respBytes, err := c.newBrokerRequester(). + forBroker(c.broker). + async(). + sendRequest( + ctx, + "/v2/service_instances/"+payload.InstanceID, + http.MethodPatch, + nil, + payload.UpdateRequest, + ) + if err != nil { + return UpdateResponse{}, fmt.Errorf("update request failed: %w", err) + } + if statusCode == http.StatusBadRequest || statusCode == http.StatusUnprocessableEntity { + return UpdateResponse{}, UnrecoverableError{Status: statusCode} + } + + if statusCode >= 300 { + return UpdateResponse{}, fmt.Errorf("update request failed with status code: %d", statusCode) + } + + response := UpdateResponse{ + IsAsync: statusCode == http.StatusAccepted, + } + + err = json.Unmarshal(respBytes, &response) + if err != nil { + return UpdateResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) + } + + return response, nil +} + func (c *Client) Deprovision(ctx context.Context, payload DeprovisionPayload) (ProvisionResponse, error) { statusCode, respBytes, err := c.newBrokerRequester(). forBroker(c.broker). diff --git a/controllers/controllers/services/osbapi/client_test.go b/controllers/controllers/services/osbapi/client_test.go index a27dd7210..a1f846698 100644 --- a/controllers/controllers/services/osbapi/client_test.go +++ b/controllers/controllers/services/osbapi/client_test.go @@ -236,6 +236,99 @@ var _ = Describe("OSBAPI Client", func() { }) }) + Describe("Update", func() { + var ( + updateResp osbapi.UpdateResponse + updateErr error + ) + + BeforeEach(func() { + brokerServer = brokerServer.WithResponse( + "/v2/service_instances/{id}", + nil, + http.StatusOK, + ) + }) + + JustBeforeEach(func() { + updateResp, updateErr = brokerClient.Update(ctx, osbapi.UpdatePayload{ + InstanceID: "my-service-instance", + UpdateRequest: osbapi.UpdateRequest{ + ServiceId: "service-guid", + PlanID: "plan-guid", + }, + }) + }) + It("updates the service synchronously", func() { + Expect(updateErr).NotTo(HaveOccurred()) + Expect(updateResp).To(Equal(osbapi.UpdateResponse{})) + }) + It("sends async update request to broker", func() { + Expect(updateErr).NotTo(HaveOccurred()) + requests := brokerServer.ServedRequests() + + Expect(requests).To(HaveLen(1)) + + Expect(requests[0].Method).To(Equal(http.MethodPatch)) + Expect(requests[0].URL.Path).To(Equal("/v2/service_instances/my-service-instance")) + + Expect(requests[0].URL.Query()).To(BeEquivalentTo(map[string][]string{ + "accepts_incomplete": {"true"}, + })) + }) + When("the broker accepts the update request", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse( + "/v2/service_instances/{id}", + map[string]any{ + "operation": "update_op1", + }, + http.StatusAccepted, + ) + }) + + It("updatres the service asynchronously", func() { + Expect(updateErr).NotTo(HaveOccurred()) + Expect(updateResp).To(Equal(osbapi.UpdateResponse{ + IsAsync: true, + Operation: "update_op1", + })) + }) + }) + When("the update request fails with 400 BadRequest error", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse("/v2/service_instances/{id}", nil, http.StatusBadRequest) + }) + + It("returns an unrecoverable error", func() { + Expect(updateErr).To(Equal(osbapi.UnrecoverableError{Status: http.StatusBadRequest})) + }) + }) + + When("the provision request fails with 422 Unprocessable entity error", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse("/v2/service_instances/{id}", nil, http.StatusUnprocessableEntity) + }) + + It("returns an unrecoverable error", func() { + Expect(updateErr).To(Equal(osbapi.UnrecoverableError{Status: http.StatusUnprocessableEntity})) + }) + }) + When("the update request fails", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse( + "/v2/service_instances/{id}", + nil, + http.StatusTeapot, + ) + }) + + It("returns an error", func() { + Expect(updateErr).To(MatchError(ContainSubstring("update request failed"))) + }) + }) + }) + Describe("Deprovision", func() { var ( deprovisionResp osbapi.ProvisionResponse diff --git a/controllers/controllers/services/osbapi/clientfactory.go b/controllers/controllers/services/osbapi/clientfactory.go index b829cd4dc..be8df1a4f 100644 --- a/controllers/controllers/services/osbapi/clientfactory.go +++ b/controllers/controllers/services/osbapi/clientfactory.go @@ -17,6 +17,7 @@ import ( //counterfeiter:generate -o fake -fake-name BrokerClient code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi.BrokerClient type BrokerClient interface { Provision(context.Context, ProvisionPayload) (ProvisionResponse, error) + Update(context.Context, UpdatePayload) (UpdateResponse, error) Deprovision(context.Context, DeprovisionPayload) (ProvisionResponse, error) GetServiceInstanceLastOperation(context.Context, GetInstanceLastOperationRequest) (LastOperationResponse, error) GetCatalog(context.Context) (Catalog, error) diff --git a/controllers/controllers/services/osbapi/fake/broker_client.go b/controllers/controllers/services/osbapi/fake/broker_client.go index 0c145a277..045b21f94 100644 --- a/controllers/controllers/services/osbapi/fake/broker_client.go +++ b/controllers/controllers/services/osbapi/fake/broker_client.go @@ -120,6 +120,20 @@ type BrokerClient struct { result1 osbapi.UnbindResponse result2 error } + UpdateStub func(context.Context, osbapi.UpdatePayload) (osbapi.UpdateResponse, error) + updateMutex sync.RWMutex + updateArgsForCall []struct { + arg1 context.Context + arg2 osbapi.UpdatePayload + } + updateReturns struct { + result1 osbapi.UpdateResponse + result2 error + } + updateReturnsOnCall map[int]struct { + result1 osbapi.UpdateResponse + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -643,6 +657,71 @@ func (fake *BrokerClient) UnbindReturnsOnCall(i int, result1 osbapi.UnbindRespon }{result1, result2} } +func (fake *BrokerClient) Update(arg1 context.Context, arg2 osbapi.UpdatePayload) (osbapi.UpdateResponse, error) { + fake.updateMutex.Lock() + ret, specificReturn := fake.updateReturnsOnCall[len(fake.updateArgsForCall)] + fake.updateArgsForCall = append(fake.updateArgsForCall, struct { + arg1 context.Context + arg2 osbapi.UpdatePayload + }{arg1, arg2}) + stub := fake.UpdateStub + fakeReturns := fake.updateReturns + fake.recordInvocation("Update", []interface{}{arg1, arg2}) + fake.updateMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *BrokerClient) UpdateCallCount() int { + fake.updateMutex.RLock() + defer fake.updateMutex.RUnlock() + return len(fake.updateArgsForCall) +} + +func (fake *BrokerClient) UpdateCalls(stub func(context.Context, osbapi.UpdatePayload) (osbapi.UpdateResponse, error)) { + fake.updateMutex.Lock() + defer fake.updateMutex.Unlock() + fake.UpdateStub = stub +} + +func (fake *BrokerClient) UpdateArgsForCall(i int) (context.Context, osbapi.UpdatePayload) { + fake.updateMutex.RLock() + defer fake.updateMutex.RUnlock() + argsForCall := fake.updateArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *BrokerClient) UpdateReturns(result1 osbapi.UpdateResponse, result2 error) { + fake.updateMutex.Lock() + defer fake.updateMutex.Unlock() + fake.UpdateStub = nil + fake.updateReturns = struct { + result1 osbapi.UpdateResponse + result2 error + }{result1, result2} +} + +func (fake *BrokerClient) UpdateReturnsOnCall(i int, result1 osbapi.UpdateResponse, result2 error) { + fake.updateMutex.Lock() + defer fake.updateMutex.Unlock() + fake.UpdateStub = nil + if fake.updateReturnsOnCall == nil { + fake.updateReturnsOnCall = make(map[int]struct { + result1 osbapi.UpdateResponse + result2 error + }) + } + fake.updateReturnsOnCall[i] = struct { + result1 osbapi.UpdateResponse + result2 error + }{result1, result2} +} + func (fake *BrokerClient) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() diff --git a/controllers/controllers/services/osbapi/types.go b/controllers/controllers/services/osbapi/types.go index 524798769..95e5bcdb3 100644 --- a/controllers/controllers/services/osbapi/types.go +++ b/controllers/controllers/services/osbapi/types.go @@ -83,6 +83,21 @@ type ProvisionResponse struct { Operation string `json:"operation,omitempty"` } +type UpdatePayload struct { + InstanceID string + UpdateRequest +} + +type UpdateRequest struct { + ServiceId string `json:"service_id"` + PlanID string `json:"plan_id,omitempty"` +} + +type UpdateResponse struct { + IsAsync bool + Operation string `json:"operation,omitempty"` +} + type GetBindingRequest struct { InstanceID string BindingID string diff --git a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml index 74f43a8c6..b6a2a8e94 100644 --- a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml +++ b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml @@ -217,6 +217,14 @@ spec: the CFServiceInstance that has been reconciled format: int64 type: integer + planGuid: + description: The plan guid of the service instance. Only makes seense + for managed service instances + type: string + provisioned: + description: True if the service instance has been successfully provisioned. + Only makes seense for managed service instances + type: boolean upgradeAvailable: description: True if there is an upgrade available for for the service instance (i.e. the plan has a new version). Only makes seense for diff --git a/tests/assets/sample-broker-golang/main.go b/tests/assets/sample-broker-golang/main.go index 1a28185fb..85250ab03 100644 --- a/tests/assets/sample-broker-golang/main.go +++ b/tests/assets/sample-broker-golang/main.go @@ -27,6 +27,7 @@ func main() { http.HandleFunc("GET /v2/catalog", getCatalogHandler) http.HandleFunc("PUT /v2/service_instances/{id}", provisionServiceInstanceHandler) + http.HandleFunc("PATCH /v2/service_instances/{id}", updateServiceInstanceHandler) http.HandleFunc("DELETE /v2/service_instances/{id}", deprovisionServiceInstanceHandler) http.HandleFunc("GET /v2/service_instances/{id}/last_operation", getLastOperationHandler) @@ -62,16 +63,28 @@ func getCatalogHandler(w http.ResponseWriter, r *http.Request) { Name: "sample-service", Id: "edfd6e50-aa59-4688-b5bf-b21e2ab27cdb", Description: "A sample service that does nothing", - Plans: []osbapi.Plan{{ - Id: "ebf1c1df-fefb-479b-9231-ddf700a37b58", - Name: "sample", - Description: "Sample plan", - Free: true, - Bindable: true, - MaintenanceInfo: osbapi.MaintenanceInfo{ - Version: "1.2.3", + Plans: []osbapi.Plan{ + { + Id: "ebf1c1df-fefb-479b-9231-ddf700a37b58", + Name: "sample", + Description: "Sample plan", + Free: true, + Bindable: true, + MaintenanceInfo: osbapi.MaintenanceInfo{ + Version: "1.2.3", + }, }, - }}, + { + Id: "003c9135-2d40-426d-9099-d4c1b807cec1", + Name: "sample-2", + Description: "Sample plan 2", + Free: true, + Bindable: true, + MaintenanceInfo: osbapi.MaintenanceInfo{ + Version: "1.2.3", + }, + }, + }, }}, } @@ -96,6 +109,17 @@ func provisionServiceInstanceHandler(w http.ResponseWriter, r *http.Request) { asyncOperation(w, fmt.Sprintf("provision-%s", r.PathValue("id")), "{}") } +func updateServiceInstanceHandler(w http.ResponseWriter, r *http.Request) { + logRequest(r) + + if status, err := checkCredentials(w, r); err != nil { + respond(w, status, fmt.Sprintf("Credentials check failed: %v", err)) + return + } + + asyncOperation(w, fmt.Sprintf("update-%s", r.PathValue("id")), "{}") +} + func deprovisionServiceInstanceHandler(w http.ResponseWriter, r *http.Request) { logRequest(r) diff --git a/tests/e2e/apps_test.go b/tests/e2e/apps_test.go index b4afb5463..9dc821432 100644 --- a/tests/e2e/apps_test.go +++ b/tests/e2e/apps_test.go @@ -670,14 +670,14 @@ var _ = Describe("Apps", func() { "foo": "bar", "baz": "qux", } - serviceInstanceGUID = createServiceInstance(space1GUID, generateGUID("service-instance"), credentials) + serviceInstanceGUID = createUPServiceInstance(space1GUID, generateGUID("service-instance"), credentials) bindingGUID = createUPSIServiceBinding(appGUID, serviceInstanceGUID, "") moreCredentials := map[string]string{ "hello": "there", "secret": "stuff", } - secondServiceInstanceGUID = createServiceInstance(space1GUID, generateGUID("service-instance"), moreCredentials) + secondServiceInstanceGUID = createUPServiceInstance(space1GUID, generateGUID("service-instance"), moreCredentials) bindingName = "custom-named-binding" namedBindingGUID = createUPSIServiceBinding(appGUID, secondServiceInstanceGUID, bindingName) @@ -761,10 +761,10 @@ var _ = Describe("Apps", func() { "foo": "var", }) instanceName = generateGUID("service-instance") - instanceGUID = createServiceInstance(space1GUID, instanceName, nil) + instanceGUID = createUPServiceInstance(space1GUID, instanceName, nil) bindingGUID = createUPSIServiceBinding(appGUID, instanceGUID, "") instanceName2 = generateGUID("service-instance") - instanceGUID2 = createServiceInstance(space1GUID, instanceName2, nil) + instanceGUID2 = createUPServiceInstance(space1GUID, instanceName2, nil) bindingGUID2 = createUPSIServiceBinding(appGUID, instanceGUID2, "") }) diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index 040555d6c..3591242e5 100644 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -734,7 +734,7 @@ func getProcess(appGUID, processType string) processResource { return process } -func createServiceInstance(spaceGUID, name string, credentials map[string]string) string { +func createUPServiceInstance(spaceGUID, name string, credentials map[string]string) string { GinkgoHelper() var serviceInstance typedResource @@ -757,6 +757,46 @@ func createServiceInstance(spaceGUID, name string, credentials map[string]string return serviceInstance.GUID } +func createManagedServiceInstance(brokerGUID, spaceGUID string, name string) string { + GinkgoHelper() + + var serviceInstance typedResource + 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()) + + resp, err := adminClient.R(). + SetBody(typedResource{ + Type: "managed", + resource: resource{ + Name: name, + Relationships: relationships{ + "space": { + Data: resource{GUID: spaceGUID}, + }, + "service_plan": { + Data: resource{GUID: plansResp.Resources[0].GUID}, + }, + }, + }, + }). + SetResult(&serviceInstance). + Post("/v3/service_instances") + + Expect(err).NotTo(HaveOccurred()) + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusAccepted), + HaveRestyHeaderWithValue("Location", ContainSubstring("/v3/jobs/managed_service_instance.create~")), + )) + jobURL := resp.Header().Get("Location") + expectJobCompletes(resp) + + return strings.Split(jobURL, "~")[1] +} + func listServiceInstances(names ...string) resourceList[serviceInstanceResource] { GinkgoHelper() diff --git a/tests/e2e/service_bindings_test.go b/tests/e2e/service_bindings_test.go index 08f116daa..c5f14102b 100644 --- a/tests/e2e/service_bindings_test.go +++ b/tests/e2e/service_bindings_test.go @@ -24,7 +24,7 @@ var _ = Describe("Service Bindings", func() { BeforeEach(func() { spaceGUID = createSpace(generateGUID("space1"), commonTestOrgGUID) appGUID = createBuildpackApp(spaceGUID, generateGUID("app")) - upsiGUID = createServiceInstance(spaceGUID, generateGUID("service-instance"), nil) + upsiGUID = createUPServiceInstance(spaceGUID, generateGUID("service-instance"), nil) }) Describe("POST /v3/service_credential_bindings/{guid}", func() { @@ -60,7 +60,7 @@ var _ = Describe("Service Bindings", func() { BeforeEach(func() { brokerGUID = createBroker(serviceBrokerURL) - instanceGUID = createManagedServiceInstance(brokerGUID, spaceGUID) + instanceGUID = createManagedServiceInstance(brokerGUID, spaceGUID, generateGUID("managed-service-instance")) }) AfterEach(func() { @@ -121,7 +121,7 @@ var _ = Describe("Service Bindings", func() { BeforeEach(func() { brokerGUID = createBroker(serviceBrokerURL) - instanceGUID := createManagedServiceInstance(brokerGUID, spaceGUID) + instanceGUID := createManagedServiceInstance(brokerGUID, spaceGUID, generateGUID("managed-service-instance")) bindingGUID = createManagedServiceBinding(appGUID, instanceGUID, "") }) @@ -150,7 +150,7 @@ var _ = Describe("Service Bindings", func() { BeforeEach(func() { bindingGUID = createUPSIServiceBinding(appGUID, upsiGUID, "") - anotherInstanceGUID = createServiceInstance(spaceGUID, generateGUID("another-service-instance"), nil) + anotherInstanceGUID = createUPServiceInstance(spaceGUID, generateGUID("another-service-instance"), nil) anotherBindingGUID = createUPSIServiceBinding(appGUID, anotherInstanceGUID, "") result = resourceListWithInclusion{} @@ -178,7 +178,7 @@ var _ = Describe("Service Bindings", func() { "foo": "val1", "bar": "val2", } - upsiGUID = createServiceInstance(spaceGUID, uuid.NewString(), credentials) + upsiGUID = createUPServiceInstance(spaceGUID, uuid.NewString(), credentials) bindingGUID = createUPSIServiceBinding(appGUID, upsiGUID, "") result = credentialsResponse{} }) diff --git a/tests/e2e/service_instances_test.go b/tests/e2e/service_instances_test.go index 9305100ee..243ba8eee 100644 --- a/tests/e2e/service_instances_test.go +++ b/tests/e2e/service_instances_test.go @@ -2,11 +2,9 @@ package e2e_test import ( "net/http" - "strings" "code.cloudfoundry.org/korifi/tests/helpers/broker" "github.com/go-resty/resty/v2" - "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" @@ -24,9 +22,9 @@ var _ = Describe("Service Instances", func() { BeforeEach(func() { spaceGUID = createSpace(generateGUID("space1"), commonTestOrgGUID) - upsiName = generateGUID("service-instance") - upsiWithCredsGUID = generateGUID("service-instance-creds") - upsiGUID = createServiceInstance(spaceGUID, upsiName, nil) + upsiName = generateGUID("upsi-service-instance") + upsiWithCredsGUID = generateGUID("upsi-service-instance-creds") + upsiGUID = createUPServiceInstance(spaceGUID, upsiName, nil) }) AfterEach(func() { @@ -53,7 +51,7 @@ var _ = Describe("Service Instances", func() { var result map[string]any BeforeEach(func() { - upsiWithCredsGUID = createServiceInstance(spaceGUID, generateGUID("service-instance2"), map[string]string{"a": "b"}) + upsiWithCredsGUID = createUPServiceInstance(spaceGUID, generateGUID("service-instance2"), map[string]string{"a": "b"}) }) JustBeforeEach(func() { @@ -165,21 +163,35 @@ var _ = Describe("Service Instances", func() { }) Describe("Update", func() { + var ( + updateRequestBody serviceInstanceResource + serviceInstanceGUID string + brokerGUID string + plansResp resourceList[resource] + result serviceInstanceResource + ) + + BeforeEach(func() { + serviceInstanceGUID = upsiGUID + updateRequestBody = serviceInstanceResource{ + resource: resource{ + Name: "new-instance-name", + Metadata: &metadata{ + Labels: map[string]string{"a-label": "a-label-value"}, + Annotations: map[string]string{"an-annotation": "an-annotation-value"}, + }, + }, + Credentials: map[string]any{ + "object-new": map[string]any{"new-a": "new-b"}, + }, + Tags: []string{"some", "tags"}, + } + }) + JustBeforeEach(func() { httpResp, httpError = adminClient.R(). - SetBody(serviceInstanceResource{ - resource: resource{ - Name: "new-instance-name", - Metadata: &metadata{ - Labels: map[string]string{"a-label": "a-label-value"}, - Annotations: map[string]string{"an-annotation": "an-annotation-value"}, - }, - }, - Credentials: map[string]any{ - "object-new": map[string]any{"new-a": "new-b"}, - }, - Tags: []string{"some", "tags"}, - }).Patch("/v3/service_instances/" + upsiGUID) + SetBody(updateRequestBody). + Patch("/v3/service_instances/" + serviceInstanceGUID) }) It("succeeds", func() { @@ -195,6 +207,72 @@ var _ = Describe("Service Instances", func() { Expect(serviceInstance.Metadata.Annotations).To(HaveKeyWithValue("an-annotation", "an-annotation-value")) Expect(serviceInstance.Tags).To(ConsistOf("some", "tags")) }) + + When("updating a managed service instance", func() { + BeforeEach(func() { + brokerGUID = createBroker(serviceBrokerURL) + + 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).To(HaveLen(2)) + + serviceInstanceGUID = createManagedServiceInstance(brokerGUID, spaceGUID, generateGUID("managed-service-instance")) + + updateRequestBody = serviceInstanceResource{ + resource: resource{ + Name: "new-managed-instance-name", + Metadata: &metadata{ + Labels: map[string]string{"a-label": "a-label-value"}, + Annotations: map[string]string{"an-annotation": "an-annotation-value"}, + }, + Relationships: relationships{ + "service_plan": { + Data: resource{GUID: plansResp.Resources[1].GUID}, + }, + }, + }, + Tags: []string{"some", "tags"}, + } + }) + + AfterEach(func() { + broker.NewDeleter(rootNamespace).ForBrokerGUID(brokerGUID).Delete() + }) + + 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.update~")), + )) + expectJobCompletes(httpResp) + }) + It("updates a managed service", func() { + Expect(httpError).NotTo(HaveOccurred()) + Expect(httpResp).To(HaveRestyStatusCode(http.StatusAccepted)) + + expectJobCompletes(httpResp) + + serviceInstances := listServiceInstances("new-managed-instance-name") + Expect(serviceInstances.Resources).To(HaveLen(1)) + + serviceInstance := serviceInstances.Resources[0] + Expect(serviceInstance.Name).To(Equal("new-managed-instance-name")) + Expect(serviceInstance.Metadata.Labels).To(HaveKeyWithValue("a-label", "a-label-value")) + Expect(serviceInstance.Metadata.Annotations).To(HaveKeyWithValue("an-annotation", "an-annotation-value")) + Expect(serviceInstance.Tags).To(ConsistOf("some", "tags")) + + httpRespService, httpErrorService := adminClient.R().SetResult(&result).Get("/v3/service_instances/" + serviceInstanceGUID) + Expect(httpErrorService).NotTo(HaveOccurred()) + Expect(httpRespService).To(HaveRestyStatusCode(http.StatusOK)) + Expect( + result.resource.Relationships["service_plan"].Data.GUID, + ).To(Equal(plansResp.Resources[1].GUID)) + }) + }) }) Describe("Delete", func() { @@ -226,8 +304,7 @@ var _ = Describe("Service Instances", func() { BeforeEach(func() { brokerGUID = createBroker(serviceBrokerURL) - - serviceInstanceGUID = createManagedServiceInstance(brokerGUID, spaceGUID) + serviceInstanceGUID = createManagedServiceInstance(brokerGUID, spaceGUID, generateGUID("managed-service-instance")) }) AfterEach(func() { @@ -256,7 +333,7 @@ var _ = Describe("Service Instances", func() { BeforeEach(func() { anotherSpaceGUID = createSpace(generateGUID("space1"), commonTestOrgGUID) - anotherInstanceGUID = createServiceInstance(anotherSpaceGUID, generateGUID("service-instance"), nil) + anotherInstanceGUID = createUPServiceInstance(anotherSpaceGUID, generateGUID("service-instance"), nil) }) JustBeforeEach(func() { @@ -278,47 +355,3 @@ var _ = Describe("Service Instances", func() { }) }) }) - -func createManagedServiceInstance(brokerGUID, spaceGUID string) string { - GinkgoHelper() - - 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: uuid.NewString(), - Relationships: relationships{ - "space": { - Data: resource{ - GUID: spaceGUID, - }, - }, - "service_plan": { - Data: resource{ - GUID: plansResp.Resources[0].GUID, - }, - }, - }, - }, - InstanceType: "managed", - } - - var result serviceInstanceResource - httpResp, httpError := adminClient.R(). - SetBody(createPayload). - SetResult(&result). - Post("/v3/service_instances") - Expect(httpError).NotTo(HaveOccurred()) - Expect(httpResp).To(SatisfyAll( - HaveRestyStatusCode(http.StatusAccepted), - HaveRestyHeaderWithValue("Location", ContainSubstring("/v3/jobs/managed_service_instance.create~")), - )) - jobURL := httpResp.Header().Get("Location") - expectJobCompletes(httpResp) - - return strings.Split(jobURL, "~")[1] -} diff --git a/tests/e2e/spaces_test.go b/tests/e2e/spaces_test.go index 72807ad33..e2462e722 100644 --- a/tests/e2e/spaces_test.go +++ b/tests/e2e/spaces_test.go @@ -179,7 +179,7 @@ var _ = Describe("Spaces", func() { spaceGUID = createSpace(generateGUID("space"), commonTestOrgGUID) resultErr = cfErrs{} serviceName = uuid.NewString() - serviceGUID = createServiceInstance(spaceGUID, serviceName, map[string]string{}) + serviceGUID = createUPServiceInstance(spaceGUID, serviceName, map[string]string{}) app1Name = generateGUID("manifested-app-1") app2Name = generateGUID("manifested-app-2") serviceBindingName = uuid.NewString() diff --git a/tests/smoke/services_test.go b/tests/smoke/services_test.go index a7c0b07c5..65deab4fd 100644 --- a/tests/smoke/services_test.go +++ b/tests/smoke/services_test.go @@ -71,4 +71,27 @@ var _ = Describe("Services", func() { )) }) }) + + Describe("cf update-service", func() { + var serviceName string + + BeforeEach(func() { + serviceName = uuid.NewString() + session := helpers.Cf("create-service", "sample-service", "sample", serviceName, "-b", brokerName) + Expect(session).To(Exit(0)) + }) + + It("updates the managed service plan", func() { + session := helpers.Cf("update-service", serviceName, "-p", "sample-2") + Expect(session).To(Exit(0)) + + session = helpers.Cf("services") + Expect(session).To(Exit(0)) + + lines := it.MustCollect(it.LinesString(session.Out)) + Expect(lines).To(ContainElement( + matchSubstrings(serviceName, "sample-service", "sample-2", brokerName), + )) + }) + }) })