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..d7ade2dfe 100644 --- a/api/handlers/service_instance_test.go +++ b/api/handlers/service_instance_test.go @@ -835,103 +835,173 @@ var _ = Describe("ServiceInstance", func() { }) Describe("PATCH /v3/service_instances/:guid", func() { - BeforeEach(func() { - requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServiceInstancePatch{ - Name: tools.PtrTo("new-name"), - Tags: &[]string{"alice", "bob"}, - Credentials: &map[string]any{"foo": "bar"}, - Metadata: payloads.MetadataPatch{ - Annotations: map[string]*string{"ann2": tools.PtrTo("ann_val2")}, - Labels: map[string]*string{"lab2": tools.PtrTo("lab_val2")}, - }, - }) + When("updating a user provided 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"}, + Metadata: payloads.MetadataPatch{ + Annotations: map[string]*string{"ann2": tools.PtrTo("ann_val2")}, + Labels: map[string]*string{"lab2": tools.PtrTo("lab_val2")}, + }, + }) - serviceInstanceRepo.PatchServiceInstanceReturns(repositories.ServiceInstanceRecord{ - Name: "new-name", - GUID: "service-instance-guid", - }, nil) + serviceInstanceRepo.PatchUserProvidedServiceInstanceReturns(repositories.ServiceInstanceRecord{ + Name: "new-name", + GUID: "service-instance-guid", + }, nil) - reqPath += "/service-instance-guid" - reqMethod = http.MethodPatch - }) + reqPath += "/service-instance-guid" + reqMethod = http.MethodPatch + }) + It("patches the user provided service instance", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + Expect(bodyString(actualReq)).To(Equal("the-json-body")) - It("patches the service instance", func() { - Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) - actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) - Expect(bodyString(actualReq)).To(Equal("the-json-body")) + Expect(serviceInstanceRepo.GetServiceInstanceCallCount()).To(Equal(1)) + _, actualAuthInfo, actualGUID := serviceInstanceRepo.GetServiceInstanceArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualGUID).To(Equal("service-instance-guid")) - Expect(serviceInstanceRepo.GetServiceInstanceCallCount()).To(Equal(1)) - _, actualAuthInfo, actualGUID := serviceInstanceRepo.GetServiceInstanceArgsForCall(0) - Expect(actualAuthInfo).To(Equal(authInfo)) - Expect(actualGUID).To(Equal("service-instance-guid")) + Expect(serviceInstanceRepo.PatchUserProvidedServiceInstanceCallCount()).To(Equal(1)) + _, actualAuthInfo, patchMessage := serviceInstanceRepo.PatchUserProvidedServiceInstanceArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(patchMessage).To(Equal(repositories.PatchUPSIMessage{ + GUID: "service-instance-guid", + SpaceGUID: "space-guid", + Name: tools.PtrTo("new-name"), + Credentials: &map[string]any{"foo": "bar"}, + 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")}, + }, + })) - Expect(serviceInstanceRepo.PatchServiceInstanceCallCount()).To(Equal(1)) - _, actualAuthInfo, patchMessage := serviceInstanceRepo.PatchServiceInstanceArgsForCall(0) - Expect(actualAuthInfo).To(Equal(authInfo)) - Expect(patchMessage).To(Equal(repositories.PatchServiceInstanceMessage{ - GUID: "service-instance-guid", - SpaceGUID: "space-guid", - Name: tools.PtrTo("new-name"), - Credentials: &map[string]any{"foo": "bar"}, - 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")}, - }, - })) + Expect(rr).To(HaveHTTPStatus(http.StatusOK)) + Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.guid", "service-instance-guid"), + MatchJSONPath("$.name", "new-name"), + MatchJSONPath("$.links.self.href", "https://api.example.org/v3/service_instances/service-instance-guid"), + ))) + }) - Expect(rr).To(HaveHTTPStatus(http.StatusOK)) - Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) - Expect(rr).To(HaveHTTPBody(SatisfyAll( - MatchJSONPath("$.guid", "service-instance-guid"), - MatchJSONPath("$.name", "new-name"), - MatchJSONPath("$.links.self.href", "https://api.example.org/v3/service_instances/service-instance-guid"), - ))) - }) + When("decoding the payload fails", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadReturns(apierrors.NewUnprocessableEntityError(nil, "nope")) + }) - When("decoding the payload fails", func() { - BeforeEach(func() { - requestValidator.DecodeAndValidateJSONPayloadReturns(apierrors.NewUnprocessableEntityError(nil, "nope")) + It("returns an error", func() { + expectUnprocessableEntityError("nope") + }) }) - It("returns an error", func() { - expectUnprocessableEntityError("nope") + When("getting the service instance fails with not found", func() { + BeforeEach(func() { + serviceInstanceRepo.GetServiceInstanceReturns( + repositories.ServiceInstanceRecord{}, + apierrors.NewNotFoundError(nil, repositories.ServiceInstanceResourceType), + ) + }) + + It("returns 404 Not Found", func() { + expectNotFoundError("Service Instance") + }) }) - }) - When("getting the service instance fails with not found", func() { - BeforeEach(func() { - serviceInstanceRepo.GetServiceInstanceReturns( - repositories.ServiceInstanceRecord{}, - apierrors.NewNotFoundError(nil, repositories.ServiceInstanceResourceType), - ) + When("getting the service instance fails with forbidden", func() { + BeforeEach(func() { + serviceInstanceRepo.GetServiceInstanceReturns( + repositories.ServiceInstanceRecord{}, + apierrors.NewForbiddenError(nil, repositories.ServiceInstanceResourceType), + ) + }) + + It("returns 404 Not Found", func() { + expectNotFoundError("Service Instance") + }) }) - It("returns 404 Not Found", func() { - expectNotFoundError("Service Instance") + When("patching the user provided service instances fails", func() { + BeforeEach(func() { + serviceInstanceRepo.PatchUserProvidedServiceInstanceReturns(repositories.ServiceInstanceRecord{}, errors.New("oops")) + }) + + It("returns the error", func() { + expectUnknownError() + }) }) }) - - When("getting the service instance fails with forbidden", func() { + When("updating a managed service instance", func() { BeforeEach(func() { - serviceInstanceRepo.GetServiceInstanceReturns( - repositories.ServiceInstanceRecord{}, - apierrors.NewForbiddenError(nil, repositories.ServiceInstanceResourceType), - ) - }) + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServiceInstancePatch{ + Name: tools.PtrTo("new-name"), + Type: "managed", + 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")}, + }, + }) - It("returns 404 Not Found", func() { - expectNotFoundError("Service Instance") + 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) + + reqPath += "/service-instance-guid" + reqMethod = http.MethodPatch }) - }) - When("patching the service instances fails", func() { - BeforeEach(func() { - serviceInstanceRepo.PatchServiceInstanceReturns(repositories.ServiceInstanceRecord{}, errors.New("oops")) + 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.patch~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() + It("returns the error", func() { + expectUnknownError() + }) }) }) }) diff --git a/api/main.go b/api/main.go index 71f33bf64..4c28ca559 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..35ece4971 100644 --- a/api/payloads/service_instance.go +++ b/api/payloads/service_instance.go @@ -171,10 +171,12 @@ 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"` + Type string `json:"type"` + 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 +185,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 +199,24 @@ func (p ServiceInstancePatch) ToServiceInstancePatchMessage(spaceGUID, appGUID s } } +func (p ServiceInstancePatch) ToManagedSIPatchMessage(spaceGUID, appGUID string) repositories.PatchManagedSIMessage { + var planGUID *string + if p.Relationships.ServicePlan.Data.GUID != "" { + 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/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..c06e9c27a 100644 --- a/api/repositories/service_instance_repository_test.go +++ b/api/repositories/service_instance_repository_test.go @@ -504,7 +504,7 @@ var _ = Describe("ServiceInstanceRepository", func() { cfServiceInstance *korifiv1alpha1.CFServiceInstance secret *corev1.Secret serviceInstanceRecord repositories.ServiceInstanceRecord - patchMessage repositories.PatchServiceInstanceMessage + patchMessage repositories.PatchUPSIMessage err error ) @@ -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() { diff --git a/controllers/api/v1alpha1/cfserviceinstance_types.go b/controllers/api/v1alpha1/cfserviceinstance_types.go index 020a65eb1..bb469d5c3 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,10 @@ type CFServiceInstanceStatus struct { //+kubebuilder:validation:Optional MaintenanceInfo MaintenanceInfo `json:"maintenanceInfo"` + // The service instance actual plan + //+kubebuilder:validation:Optional + PlanGUID string `json:"planGuid"` + // 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..e8ca20c5d 100644 --- a/controllers/controllers/services/instances/managed/controller.go +++ b/controllers/controllers/services/instances/managed/controller.go @@ -148,7 +148,7 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, serviceInstance *kor } if isFailed(serviceInstance) { - return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisioningFailed").WithNoRequeue() + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ReconciliationFailed").WithNoRequeue() } planVisible, err := r.isServicePlanVisible(ctx, serviceInstance, serviceInstanceAssets.ServicePlan) @@ -166,20 +166,36 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, serviceInstance *kor serviceInstance.Spec.ServiceLabel = tools.PtrTo(serviceInstanceAssets.ServiceOffering.Spec.Name) } - provisionResponse, err := r.provisionServiceInstance(ctx, serviceInstance, serviceInstanceAssets, osbapiClient) - if err != nil { - log.Error(err, "failed to provision service instance") - return ctrl.Result{}, fmt.Errorf("failed to provision service instance: %w", err) - } + if serviceInstance.Status.PlanGUID == "" { + provisionResponse, err := r.provisionServiceInstance(ctx, serviceInstance, serviceInstanceAssets, osbapiClient) + if err != nil { + log.Error(err, "failed to provision service instance") + return ctrl.Result{}, fmt.Errorf("failed to provision service instance: %w", err) + } - if provisionResponse.IsAsync { - lastOpResponse, err := r.pollLastOperation(ctx, serviceInstance, serviceInstanceAssets, osbapiClient, provisionResponse.Operation) + if provisionResponse.IsAsync { + lastOpResponse, err := r.pollLastOperation(ctx, serviceInstance, serviceInstanceAssets, osbapiClient, provisionResponse.Operation) + if err != nil { + return ctrl.Result{}, err + } + return r.processProvisionOperation(serviceInstance, lastOpResponse) + } + } else { + updateResponse, err := r.updateServiceInstance(ctx, serviceInstance, serviceInstanceAssets, osbapiClient) if err != nil { - return ctrl.Result{}, err + 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) } - return r.processProvisionOperation(serviceInstance, lastOpResponse) } + serviceInstance.Status.PlanGUID = serviceInstance.Spec.PlanGUID serviceInstance.Status.MaintenanceInfo = serviceInstanceAssets.ServicePlan.Spec.MaintenanceInfo serviceInstance.Status.LastOperation.State = "succeeded" return ctrl.Result{}, nil @@ -249,6 +265,7 @@ func (r *Reconciler) processProvisionOperation( lastOpResponse osbapi.LastOperationResponse, ) (ctrl.Result, error) { if lastOpResponse.State == "succeeded" { + setObservedGeneration(serviceInstance) return ctrl.Result{}, nil } @@ -261,16 +278,88 @@ func (r *Reconciler) processProvisionOperation( Reason: "ProvisionFailed", Message: lastOpResponse.Description, }) + setObservedGeneration(serviceInstance) return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisionFailed") } 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") + } + + return osbapi.UpdateResponse{}, err + } + + return updateResponse, nil +} + +func (r *Reconciler) processUpdateOperation( + serviceInstance *korifiv1alpha1.CFServiceInstance, + lastOpResponse osbapi.LastOperationResponse, +) (ctrl.Result, error) { + if lastOpResponse.State == "succeeded" { + setObservedGeneration(serviceInstance) + 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, + }) + setObservedGeneration(serviceInstance) + 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, ) (ctrl.Result, error) { + setObservedGeneration(serviceInstance) if !controllerutil.ContainsFinalizer(serviceInstance, korifiv1alpha1.CFServiceInstanceFinalizerName) { return ctrl.Result{}, nil } @@ -472,9 +561,14 @@ func (r *Reconciler) getNamespace(ctx context.Context, namespaceName string) (*c } func isFailed(instance *korifiv1alpha1.CFServiceInstance) bool { - return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.ProvisioningFailedCondition) + return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.ProvisioningFailedCondition) || + meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.UpdateFailedCondition) } func isReady(instance *korifiv1alpha1.CFServiceInstance) bool { return meta.IsStatusConditionTrue(instance.Status.Conditions, korifiv1alpha1.StatusConditionReady) } + +func setObservedGeneration(instance *korifiv1alpha1.CFServiceInstance) { + instance.Status.ObservedGeneration = instance.Generation +} diff --git a/controllers/controllers/services/instances/managed/controller_test.go b/controllers/controllers/services/instances/managed/controller_test.go index a04f2e713..ecffc22de 100644 --- a/controllers/controllers/services/instances/managed/controller_test.go +++ b/controllers/controllers/services/instances/managed/controller_test.go @@ -29,6 +29,7 @@ var _ = Describe("CFServiceInstance", func() { instance *korifiv1alpha1.CFServiceInstance serviceBroker *korifiv1alpha1.CFServiceBroker servicePlan *korifiv1alpha1.CFServicePlan + servicePlan2 *korifiv1alpha1.CFServicePlan ) BeforeEach(func() { @@ -102,6 +103,29 @@ var _ = Describe("CFServiceInstance", func() { } Expect(adminClient.Create(ctx, servicePlan)).To(Succeed()) + 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()) + instance = &korifiv1alpha1.CFServiceInstance{ ObjectMeta: metav1.ObjectMeta{ Name: uuid.NewString(), @@ -116,7 +140,6 @@ var _ = Describe("CFServiceInstance", func() { PlanGUID: servicePlan.Name, }, } - Expect(adminClient.Create(ctx, instance)).To(Succeed()) }) @@ -127,6 +150,13 @@ var _ = Describe("CFServiceInstance", func() { }).Should(Succeed()) }) + It("sets the PlanGUID status field", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Status.PlanGUID).To(Equal(instance.Spec.PlanGUID)) + }).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()) @@ -753,6 +783,264 @@ var _ = Describe("CFServiceInstance", func() { }) }) + When("updates instance", func() { + BeforeEach(func() { + brokerClient.UpdateReturns(osbapi.UpdateResponse{}, nil) + }) + JustBeforeEach(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.PlanGUID).To(Equal(servicePlan.Name)) + }).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(servicePlan2), servicePlan2)).To(Succeed()) + }).Should(Succeed()) + + Expect(k8s.Patch(ctx, adminClient, instance, func() { + instance.Spec.PlanGUID = servicePlan2.Name + meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.StatusConditionReady, + Status: metav1.ConditionFalse, + Reason: "UpdateRequested", + Message: "managed service instance update is requested", + }) + })).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 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.PlanGUID).To(Equal(instance.Spec.PlanGUID)) + g.Expect(instance.Status.PlanGUID).To(Equal(servicePlan2.Name)) + }).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)), + ))) + }).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(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 initial 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()) + }) + }) + + 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 5bbc9d89b..d58912694 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/controllers/webhooks/networking/security_groups/validator_test.go b/controllers/webhooks/networking/security_groups/validator_test.go index 5472edaf5..fb777367d 100644 --- a/controllers/webhooks/networking/security_groups/validator_test.go +++ b/controllers/webhooks/networking/security_groups/validator_test.go @@ -438,7 +438,6 @@ var _ = Describe("CFSecurityGroupValidatingWebhook", func() { )) }) }) - }) Describe("ValidateDelete", func() { @@ -468,5 +467,4 @@ var _ = Describe("CFSecurityGroupValidatingWebhook", func() { }) }) }) - }) diff --git a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml index 10ae7b013..064130312 100644 --- a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml +++ b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml @@ -217,6 +217,9 @@ spec: the CFServiceInstance that has been reconciled format: int64 type: integer + planGuid: + description: The service instance actual plan + type: string 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..7050723d1 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,9 +163,24 @@ var _ = Describe("Service Instances", func() { }) Describe("Update", func() { + var ( + updateRequestBody serviceInstanceResource + serviceInstanceGUID string + brokerGUID string + plansResp resourceList[resource] + result serviceInstanceResource + ) + JustBeforeEach(func() { httpResp, httpError = adminClient.R(). - SetBody(serviceInstanceResource{ + SetBody(updateRequestBody). + Patch("/v3/service_instances/" + serviceInstanceGUID) + }) + + When("updating a user-provided service instance", func() { + BeforeEach(func() { + serviceInstanceGUID = upsiGUID + updateRequestBody = serviceInstanceResource{ resource: resource{ Name: "new-instance-name", Metadata: &metadata{ @@ -179,21 +192,91 @@ var _ = Describe("Service Instances", func() { "object-new": map[string]any{"new-a": "new-b"}, }, Tags: []string{"some", "tags"}, - }).Patch("/v3/service_instances/" + upsiGUID) + } + }) + + It("succeeds", func() { + Expect(httpError).NotTo(HaveOccurred()) + Expect(httpResp).To(HaveRestyStatusCode(http.StatusOK)) + + serviceInstances := listServiceInstances("new-instance-name") + Expect(serviceInstances.Resources).To(HaveLen(1)) + + serviceInstance := serviceInstances.Resources[0] + Expect(serviceInstance.Name).To(Equal("new-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")) + }) }) - It("succeeds", func() { - Expect(httpError).NotTo(HaveOccurred()) - Expect(httpResp).To(HaveRestyStatusCode(http.StatusOK)) + 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).NotTo(BeEmpty()) - serviceInstances := listServiceInstances("new-instance-name") - Expect(serviceInstances.Resources).To(HaveLen(1)) + serviceInstanceGUID = createManagedServiceInstance(brokerGUID, spaceGUID, generateGUID("managed-service-instance")) - serviceInstance := serviceInstances.Resources[0] - Expect(serviceInstance.Name).To(Equal("new-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")) + 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")) + }) + It("changes a plan", func() { + expectJobCompletes(httpResp) + + 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)) + }) }) }) @@ -226,8 +309,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 +338,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 +360,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()