From 0595f91bcbf11189017c17305391b309a7ae3058 Mon Sep 17 00:00:00 2001 From: Angel Aleksandrov Date: Wed, 11 Sep 2019 13:36:37 +0300 Subject: [PATCH] Return statusOK when resource exists (#77) * Return statusOK when resource exists * Apply comments --- api_test.go | 166 +++++++++++++++++++++++++---------- domain/service_broker.go | 2 + fakes/fake_service_broker.go | 131 +++++++++++++++------------ handlers/bind.go | 10 +++ handlers/provision.go | 6 +- 5 files changed, 210 insertions(+), 105 deletions(-) diff --git a/api_test.go b/api_test.go index 7caa63da..6bed0878 100644 --- a/api_test.go +++ b/api_test.go @@ -81,9 +81,7 @@ var _ = Describe("Service Broker API", func() { lastLogLine := func() lager.LogFormat { noOfLogLines := len(brokerLogger.Logs()) if noOfLogLines == 0 { - // better way to raise error? - err := errors.New("expected some log lines but there were none") - Expect(err).NotTo(HaveOccurred()) + Fail("expected some log lines but there were none") } return brokerLogger.Logs()[noOfLogLines-1] @@ -92,9 +90,11 @@ var _ = Describe("Service Broker API", func() { BeforeEach(func() { apiVersion = "2.14" fakeServiceBroker = &fakes.FakeServiceBroker{ - InstanceLimit: 3, - ServiceID: "0A789746-596F-4CEA-BFAC-A0795DA056E3", - PlanID: "plan-id", + ProvisionedInstances: map[string]brokerapi.ProvisionDetails{}, + BoundBindings: map[string]brokerapi.BindDetails{}, + InstanceLimit: 3, + ServiceID: "0A789746-596F-4CEA-BFAC-A0795DA056E3", + PlanID: "plan-id", } brokerLogger = lagertest.NewTestLogger("broker-api") brokerAPI = brokerapi.New(fakeServiceBroker, brokerLogger, credentials) @@ -409,7 +409,7 @@ var _ = Describe("Service Broker API", func() { It("calls Provision on the service broker with all params", func() { makeInstanceProvisioningRequest(instanceID, provisionDetails, "") - Expect(fakeServiceBroker.ProvisionDetails).To(Equal(brokerapi.ProvisionDetails{ + Expect(fakeServiceBroker.ProvisionedInstances[instanceID]).To(Equal(brokerapi.ProvisionDetails{ ServiceID: fakeServiceBroker.ServiceID, PlanID: "plan-id", OrganizationGUID: "organization-guid", @@ -425,12 +425,14 @@ var _ = Describe("Service Broker API", func() { It("calls Provision on the service broker with the instance id", func() { makeInstanceProvisioningRequest(instanceID, provisionDetails, "") - Expect(fakeServiceBroker.ProvisionedInstanceIDs).To(ContainElement(instanceID)) + _, ok := fakeServiceBroker.ProvisionedInstances[instanceID] + Expect(ok).To(BeTrue()) }) It("calls GetInstance on the service broker with the instance id", func() { makeInstanceProvisioningRequest(instanceID, provisionDetails, "") - Expect(fakeServiceBroker.ProvisionedInstanceIDs).To(ContainElement(instanceID)) + _, ok := fakeServiceBroker.ProvisionedInstances[instanceID] + Expect(ok).To(BeTrue()) fakeServiceBroker.DashboardURL = "https://example.com/dashboard/some-instance" resp := makeGetInstanceRequest(instanceID) Expect(fakeServiceBroker.GetInstanceIDs).To(ContainElement(instanceID)) @@ -440,6 +442,8 @@ var _ = Describe("Service Broker API", func() { Context("when the broker returns some operation data", func() { BeforeEach(func() { fakeServiceBroker = &fakes.FakeServiceBroker{ + ProvisionedInstances: map[string]brokerapi.ProvisionDetails{}, + BoundBindings: map[string]brokerapi.BindDetails{}, InstanceLimit: 3, OperationDataToReturn: "some-operation-data", ServiceID: fakeServiceBroker.ServiceID, @@ -491,19 +495,19 @@ var _ = Describe("Service Broker API", func() { It("calls Provision on the service broker with all params", func() { makeInstanceProvisioningRequest(instanceID, provisionDetails, "") - Expect(string(fakeServiceBroker.ProvisionDetails.RawParameters)).To(MatchJSON(rawParams)) + Expect(string(fakeServiceBroker.ProvisionedInstances[instanceID].RawParameters)).To(MatchJSON(rawParams)) }) It("calls Provision with details with raw parameters", func() { makeInstanceProvisioningRequest(instanceID, provisionDetails, "") - detailsWithRawParameters := brokerapi.DetailsWithRawParameters(fakeServiceBroker.ProvisionDetails) + detailsWithRawParameters := brokerapi.DetailsWithRawParameters(fakeServiceBroker.ProvisionedInstances[instanceID]) rawParameters := detailsWithRawParameters.GetRawParameters() Expect(string(rawParameters)).To(MatchJSON(rawParams)) }) It("calls Provision with details with raw context", func() { makeInstanceProvisioningRequest(instanceID, provisionDetails, "") - detailsWithRawContext := brokerapi.DetailsWithRawContext(fakeServiceBroker.ProvisionDetails) + detailsWithRawContext := brokerapi.DetailsWithRawContext(fakeServiceBroker.ProvisionedInstances[instanceID]) rawContext := detailsWithRawContext.GetRawContext() Expect(string(rawContext)).To(MatchJSON(rawCtx)) }) @@ -665,9 +669,33 @@ var _ = Describe("Service Broker API", func() { makeInstanceProvisioningRequest(instanceID, provisionDetails, "") }) - It("returns a 409", func() { + Context("returns a StatusOK on", func() { + It("sync broker response", func() { + response := makeInstanceProvisioningRequest(instanceID, provisionDetails, "") + Expect(response.StatusCode).To(Equal(http.StatusOK)) + }) + + It("async broker response", func() { + fakeAsyncServiceBroker := &fakes.FakeAsyncServiceBroker{ + FakeServiceBroker: *fakeServiceBroker, + ShouldProvisionAsync: true, + } + brokerAPI = brokerapi.New(fakeAsyncServiceBroker, brokerLogger, credentials) + + response := makeInstanceProvisioningRequest(instanceID, provisionDetails, "") + Expect(response.StatusCode).To(Equal(http.StatusOK)) + }) + }) + + It("returns a StatusConflict", func() { + By("Service Instance with the same id already exists and is being provisioned but with different attributes") + provisionDetails["space_guid"] = "fake-space_guid" + defer func() { + By("Return default value") + provisionDetails["space_guid"] = "space_guid" + }() response := makeInstanceProvisioningRequest(instanceID, provisionDetails, "") - Expect(response.StatusCode).To(Equal(409)) + Expect(response.StatusCode).To(Equal(http.StatusConflict)) }) It("returns an empty JSON object", func() { @@ -675,7 +703,13 @@ var _ = Describe("Service Broker API", func() { Expect(response.Body).To(MatchJSON(`{}`)) }) - It("logs an appropriate error", func() { + It("logs an appropriate error when respond with statusConflict", func() { + By("Service Instance with the same id already exists and is being provisioned but with different attributes") + provisionDetails["space_guid"] = "fake-space_guid" + defer func() { + By("Return default value") + provisionDetails["space_guid"] = "space_guid" + }() makeInstanceProvisioningRequest(instanceID, provisionDetails, "") Expect(lastLogLine().Message).To(ContainSubstring(".provision.instance-already-exists")) Expect(lastLogLine().Data["error"]).To(ContainSubstring("instance already exists")) @@ -687,7 +721,7 @@ var _ = Describe("Service Broker API", func() { It("calls ProvisionAsync on the service broker", func() { acceptsIncomplete := true makeInstanceProvisioningRequestWithAcceptsIncomplete(instanceID, provisionDetails, acceptsIncomplete) - Expect(fakeServiceBroker.ProvisionDetails).To(Equal(brokerapi.ProvisionDetails{ + Expect(fakeServiceBroker.ProvisionedInstances[instanceID]).To(Equal(brokerapi.ProvisionDetails{ ServiceID: fakeServiceBroker.ServiceID, PlanID: "plan-id", OrganizationGUID: "organization-guid", @@ -700,15 +734,18 @@ var _ = Describe("Service Broker API", func() { }, })) - Expect(fakeServiceBroker.ProvisionedInstanceIDs).To(ContainElement(instanceID)) + _, ok := fakeServiceBroker.ProvisionedInstances[instanceID] + Expect(ok).To(BeTrue()) }) Context("when the broker chooses to provision asynchronously", func() { BeforeEach(func() { fakeServiceBroker = &fakes.FakeServiceBroker{ - InstanceLimit: 3, - ServiceID: fakeServiceBroker.ServiceID, - PlanID: fakeServiceBroker.PlanID, + ProvisionedInstances: map[string]brokerapi.ProvisionDetails{}, + BoundBindings: map[string]brokerapi.BindDetails{}, + InstanceLimit: 3, + ServiceID: fakeServiceBroker.ServiceID, + PlanID: fakeServiceBroker.PlanID, } fakeAsyncServiceBroker := &fakes.FakeAsyncServiceBroker{ FakeServiceBroker: *fakeServiceBroker, @@ -726,9 +763,11 @@ var _ = Describe("Service Broker API", func() { Context("when the broker chooses to provision synchronously", func() { BeforeEach(func() { fakeServiceBroker = &fakes.FakeServiceBroker{ - InstanceLimit: 3, - ServiceID: fakeServiceBroker.ServiceID, - PlanID: fakeServiceBroker.PlanID, + ProvisionedInstances: map[string]brokerapi.ProvisionDetails{}, + BoundBindings: map[string]brokerapi.BindDetails{}, + InstanceLimit: 3, + ServiceID: fakeServiceBroker.ServiceID, + PlanID: fakeServiceBroker.PlanID, } fakeAsyncServiceBroker := &fakes.FakeAsyncServiceBroker{ FakeServiceBroker: *fakeServiceBroker, @@ -753,9 +792,11 @@ var _ = Describe("Service Broker API", func() { Context("when broker can only respond asynchronously", func() { BeforeEach(func() { fakeServiceBroker = &fakes.FakeServiceBroker{ - InstanceLimit: 3, - ServiceID: fakeServiceBroker.ServiceID, - PlanID: fakeServiceBroker.PlanID, + ProvisionedInstances: map[string]brokerapi.ProvisionDetails{}, + BoundBindings: map[string]brokerapi.BindDetails{}, + InstanceLimit: 3, + ServiceID: fakeServiceBroker.ServiceID, + PlanID: fakeServiceBroker.PlanID, } fakeAsyncServiceBroker := &fakes.FakeAsyncOnlyServiceBroker{ FakeServiceBroker: *fakeServiceBroker, @@ -781,9 +822,11 @@ var _ = Describe("Service Broker API", func() { Context("when broker can only respond asynchronously", func() { BeforeEach(func() { fakeServiceBroker = &fakes.FakeServiceBroker{ - InstanceLimit: 3, - ServiceID: fakeServiceBroker.ServiceID, - PlanID: fakeServiceBroker.PlanID, + ProvisionedInstances: map[string]brokerapi.ProvisionDetails{}, + BoundBindings: map[string]brokerapi.BindDetails{}, + InstanceLimit: 3, + ServiceID: fakeServiceBroker.ServiceID, + PlanID: fakeServiceBroker.PlanID, } fakeAsyncServiceBroker := &fakes.FakeAsyncOnlyServiceBroker{ FakeServiceBroker: *fakeServiceBroker, @@ -1535,8 +1578,9 @@ var _ = Describe("Service Broker API", func() { It("calls Bind on the service broker with the instance and binding ids", func() { makeBindingRequest(instanceID, bindingID, details) Expect(fakeServiceBroker.BoundInstanceIDs).To(ContainElement(instanceID)) - Expect(fakeServiceBroker.BoundBindingIDs).To(ContainElement(bindingID)) - Expect(fakeServiceBroker.BoundBindingDetails).To(Equal(brokerapi.BindDetails{ + _, ok := fakeServiceBroker.BoundBindings[bindingID] + Expect(ok).To(BeTrue()) + Expect(fakeServiceBroker.BoundBindings[bindingID]).To(Equal(brokerapi.BindDetails{ AppGUID: "app_guid", PlanID: "plan_id", ServiceID: "service_id", @@ -1546,7 +1590,7 @@ var _ = Describe("Service Broker API", func() { It("calls bind with details with raw parameters", func() { makeBindingRequest(instanceID, bindingID, details) - detailsWithRawParameters := brokerapi.DetailsWithRawParameters(fakeServiceBroker.BoundBindingDetails) + detailsWithRawParameters := brokerapi.DetailsWithRawParameters(fakeServiceBroker.BoundBindings[bindingID]) rawParameters := detailsWithRawParameters.GetRawParameters() Expect(rawParameters).To(Equal(json.RawMessage(`{"new-param":"new-param-value"}`))) }) @@ -1663,19 +1707,19 @@ var _ = Describe("Service Broker API", func() { It("calls Bind on the service broker with all params", func() { makeBindingRequest(instanceID, bindingID, details) - Expect(string(fakeServiceBroker.BoundBindingDetails.RawParameters)).To(MatchJSON(rawParams)) + Expect(string(fakeServiceBroker.BoundBindings[bindingID].RawParameters)).To(MatchJSON(rawParams)) }) It("calls Bind with details with raw parameters", func() { makeBindingRequest(instanceID, bindingID, details) - detailsWithRawParameters := brokerapi.DetailsWithRawParameters(fakeServiceBroker.BoundBindingDetails) + detailsWithRawParameters := brokerapi.DetailsWithRawParameters(fakeServiceBroker.BoundBindings[bindingID]) rawParameters := detailsWithRawParameters.GetRawParameters() Expect(string(rawParameters)).To(MatchJSON(rawParams)) }) It("calls Bind with details with raw context", func() { makeBindingRequest(instanceID, bindingID, details) - detailsWithRawContext := brokerapi.DetailsWithRawContext(fakeServiceBroker.BoundBindingDetails) + detailsWithRawContext := brokerapi.DetailsWithRawContext(fakeServiceBroker.BoundBindings[bindingID]) rawContext := detailsWithRawContext.GetRawContext() Expect(string(rawContext)).To(MatchJSON(rawCtx)) }) @@ -1693,11 +1737,11 @@ var _ = Describe("Service Broker API", func() { } makeBindingRequest(instanceID, bindingID, details) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource).NotTo(BeNil()) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource.AppGuid).To(Equal("a-guid")) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource.SpaceGuid).To(Equal("a-space-guid")) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource.Route).To(Equal("route.cf-apps.com")) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource.CredentialClientID).To(Equal("some-credentials")) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource).NotTo(BeNil()) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource.AppGuid).To(Equal("a-guid")) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource.SpaceGuid).To(Equal("a-space-guid")) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource.Route).To(Equal("route.cf-apps.com")) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource.CredentialClientID).To(Equal("some-credentials")) }) }) @@ -1708,11 +1752,11 @@ var _ = Describe("Service Broker API", func() { details["bind_resource"] = map[string]interface{}{} makeBindingRequest(instanceID, bindingID, details) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource).NotTo(BeNil()) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource.AppGuid).To(BeEmpty()) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource.SpaceGuid).To(BeEmpty()) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource.Route).To(BeEmpty()) - Expect(fakeServiceBroker.BoundBindingDetails.BindResource.CredentialClientID).To(BeEmpty()) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource).NotTo(BeNil()) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource.AppGuid).To(BeEmpty()) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource.SpaceGuid).To(BeEmpty()) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource.Route).To(BeEmpty()) + Expect(fakeServiceBroker.BoundBindings[bindingID].BindResource.CredentialClientID).To(BeEmpty()) }) }) @@ -1744,15 +1788,41 @@ var _ = Describe("Service Broker API", func() { }) Context("when the requested binding already exists", func() { - var instanceID string + var instanceID, bindingID string BeforeEach(func() { fakeServiceBroker.BindError = brokerapi.ErrBindingAlreadyExists }) - It("returns a 409", func() { + Context("returns a statusOK", func() { + BeforeEach(func() { + fakeServiceBroker.BindError = nil + + instanceID = uniqueInstanceID() + bindingID = uniqueBindingID() + + makeBindingRequest(instanceID, bindingID, details) + }) + + It("sync broker response", func() { + response := makeBindingRequest(instanceID, bindingID, details) + Expect(response.StatusCode).To(Equal(http.StatusOK)) + }) + + It("async broker response", func() { + fakeAsyncServiceBroker := &fakes.FakeAsyncServiceBroker{ + FakeServiceBroker: *fakeServiceBroker, + } + brokerAPI = brokerapi.New(fakeAsyncServiceBroker, brokerLogger, credentials) + + response := makeAsyncBindingRequest(instanceID, bindingID, details) + Expect(response.StatusCode).To(Equal(http.StatusOK)) + }) + }) + + It("returns a statusConflict", func() { response := makeBindingRequest(uniqueInstanceID(), uniqueBindingID(), details) - Expect(response.StatusCode).To(Equal(409)) + Expect(response.StatusCode).To(Equal(http.StatusConflict)) }) It("returns an error JSON object", func() { diff --git a/domain/service_broker.go b/domain/service_broker.go index ed8df045..d8ea4633 100644 --- a/domain/service_broker.go +++ b/domain/service_broker.go @@ -91,6 +91,7 @@ type ProvisionDetails struct { type ProvisionedServiceSpec struct { IsAsync bool + AlreadyExists bool DashboardURL string OperationData string } @@ -169,6 +170,7 @@ type UnbindSpec struct { type Binding struct { IsAsync bool `json:"is_async"` + AlreadyExists bool `json:"already_exists"` OperationData string `json:"operation_data"` Credentials interface{} `json:"credentials"` SyslogDrainURL string `json:"syslog_drain_url"` diff --git a/fakes/fake_service_broker.go b/fakes/fake_service_broker.go index f66b1fbc..c6175432 100644 --- a/fakes/fake_service_broker.go +++ b/fakes/fake_service_broker.go @@ -3,40 +3,42 @@ package fakes import ( "context" "errors" + "reflect" + + "github.com/pivotal-cf/brokerapi/domain/apiresponses" "github.com/pivotal-cf/brokerapi" ) type FakeServiceBroker struct { - ProvisionDetails brokerapi.ProvisionDetails + ProvisionedInstances map[string]brokerapi.ProvisionDetails + UpdateDetails brokerapi.UpdateDetails DeprovisionDetails brokerapi.DeprovisionDetails - ProvisionedInstanceIDs []string DeprovisionedInstanceIDs []string UpdatedInstanceIDs []string GetInstanceIDs []string - BoundInstanceIDs []string - BoundBindingIDs []string - BoundBindingDetails brokerapi.BindDetails - SyslogDrainURL string - RouteServiceURL string - VolumeMounts []brokerapi.VolumeMount + BoundInstanceIDs []string + BoundBindings map[string]brokerapi.BindDetails + SyslogDrainURL string + RouteServiceURL string + VolumeMounts []brokerapi.VolumeMount UnbindingDetails brokerapi.UnbindDetails InstanceLimit int - ProvisionError error - BindError error - UnbindError error - DeprovisionError error - LastOperationError error + ProvisionError error + BindError error + UnbindError error + DeprovisionError error + LastOperationError error LastBindingOperationError error - UpdateError error - GetInstanceError error - GetBindingError error + UpdateError error + GetInstanceError error + GetBindingError error BrokerCalled bool LastOperationState brokerapi.LastOperationState @@ -167,17 +169,20 @@ func (fakeBroker *FakeServiceBroker) Provision(context context.Context, instance return brokerapi.ProvisionedServiceSpec{}, fakeBroker.ProvisionError } - if len(fakeBroker.ProvisionedInstanceIDs) >= fakeBroker.InstanceLimit { + if len(fakeBroker.ProvisionedInstances) >= fakeBroker.InstanceLimit { return brokerapi.ProvisionedServiceSpec{}, brokerapi.ErrInstanceLimitMet } - if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) { - return brokerapi.ProvisionedServiceSpec{}, brokerapi.ErrInstanceAlreadyExists + if _, ok := fakeBroker.ProvisionedInstances[instanceID]; !ok { + fakeBroker.ProvisionedInstances[instanceID] = details + return brokerapi.ProvisionedServiceSpec{DashboardURL: fakeBroker.DashboardURL}, nil } - fakeBroker.ProvisionDetails = details - fakeBroker.ProvisionedInstanceIDs = append(fakeBroker.ProvisionedInstanceIDs, instanceID) - return brokerapi.ProvisionedServiceSpec{DashboardURL: fakeBroker.DashboardURL}, nil + if reflect.DeepEqual(fakeBroker.ProvisionedInstances[instanceID], details) { + return brokerapi.ProvisionedServiceSpec{AlreadyExists: true, DashboardURL: fakeBroker.DashboardURL}, nil + } + + return brokerapi.ProvisionedServiceSpec{}, apiresponses.ErrInstanceAlreadyExists } func (fakeBroker *FakeAsyncServiceBroker) Provision(context context.Context, instanceID string, details brokerapi.ProvisionDetails, asyncAllowed bool) (brokerapi.ProvisionedServiceSpec, error) { @@ -187,17 +192,20 @@ func (fakeBroker *FakeAsyncServiceBroker) Provision(context context.Context, ins return brokerapi.ProvisionedServiceSpec{}, fakeBroker.ProvisionError } - if len(fakeBroker.ProvisionedInstanceIDs) >= fakeBroker.InstanceLimit { + if len(fakeBroker.ProvisionedInstances) >= fakeBroker.InstanceLimit { return brokerapi.ProvisionedServiceSpec{}, brokerapi.ErrInstanceLimitMet } - if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) { - return brokerapi.ProvisionedServiceSpec{}, brokerapi.ErrInstanceAlreadyExists + if _, ok := fakeBroker.ProvisionedInstances[instanceID]; !ok { + fakeBroker.ProvisionedInstances[instanceID] = details + return brokerapi.ProvisionedServiceSpec{IsAsync: fakeBroker.ShouldProvisionAsync, DashboardURL: fakeBroker.DashboardURL, OperationData: fakeBroker.OperationDataToReturn}, nil } - fakeBroker.ProvisionDetails = details - fakeBroker.ProvisionedInstanceIDs = append(fakeBroker.ProvisionedInstanceIDs, instanceID) - return brokerapi.ProvisionedServiceSpec{IsAsync: fakeBroker.ShouldProvisionAsync, DashboardURL: fakeBroker.DashboardURL, OperationData: fakeBroker.OperationDataToReturn}, nil + if reflect.DeepEqual(fakeBroker.ProvisionedInstances[instanceID], details) { + return brokerapi.ProvisionedServiceSpec{IsAsync: fakeBroker.ShouldProvisionAsync, AlreadyExists: true, DashboardURL: fakeBroker.DashboardURL, OperationData: fakeBroker.OperationDataToReturn}, nil + } + + return brokerapi.ProvisionedServiceSpec{}, apiresponses.ErrInstanceAlreadyExists } func (fakeBroker *FakeAsyncOnlyServiceBroker) Provision(context context.Context, instanceID string, details brokerapi.ProvisionDetails, asyncAllowed bool) (brokerapi.ProvisionedServiceSpec, error) { @@ -207,20 +215,23 @@ func (fakeBroker *FakeAsyncOnlyServiceBroker) Provision(context context.Context, return brokerapi.ProvisionedServiceSpec{}, fakeBroker.ProvisionError } - if len(fakeBroker.ProvisionedInstanceIDs) >= fakeBroker.InstanceLimit { + if len(fakeBroker.ProvisionedInstances) >= fakeBroker.InstanceLimit { return brokerapi.ProvisionedServiceSpec{}, brokerapi.ErrInstanceLimitMet } - if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) { - return brokerapi.ProvisionedServiceSpec{}, brokerapi.ErrInstanceAlreadyExists + if _, ok := fakeBroker.ProvisionedInstances[instanceID]; ok { + if reflect.DeepEqual(fakeBroker.ProvisionedInstances[instanceID], details) { + return brokerapi.ProvisionedServiceSpec{IsAsync: asyncAllowed, AlreadyExists: true, DashboardURL: fakeBroker.DashboardURL}, nil + } + + return brokerapi.ProvisionedServiceSpec{}, apiresponses.ErrInstanceAlreadyExists } if !asyncAllowed { return brokerapi.ProvisionedServiceSpec{}, brokerapi.ErrAsyncRequired } - fakeBroker.ProvisionDetails = details - fakeBroker.ProvisionedInstanceIDs = append(fakeBroker.ProvisionedInstanceIDs, instanceID) + fakeBroker.ProvisionedInstances[instanceID] = details return brokerapi.ProvisionedServiceSpec{IsAsync: true, DashboardURL: fakeBroker.DashboardURL}, nil } @@ -273,7 +284,7 @@ func (fakeBroker *FakeServiceBroker) Deprovision(context context.Context, instan fakeBroker.DeprovisionDetails = details fakeBroker.DeprovisionedInstanceIDs = append(fakeBroker.DeprovisionedInstanceIDs, instanceID) - if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) { + if _, ok := fakeBroker.ProvisionedInstances[instanceID]; ok { return brokerapi.DeprovisionServiceSpec{}, nil } return brokerapi.DeprovisionServiceSpec{IsAsync: false}, brokerapi.ErrInstanceDoesNotExist @@ -293,7 +304,7 @@ func (fakeBroker *FakeAsyncOnlyServiceBroker) Deprovision(context context.Contex fakeBroker.DeprovisionedInstanceIDs = append(fakeBroker.DeprovisionedInstanceIDs, instanceID) fakeBroker.DeprovisionDetails = details - if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) { + if _, ok := fakeBroker.ProvisionedInstances[instanceID]; ok { return brokerapi.DeprovisionServiceSpec{IsAsync: true, OperationData: fakeBroker.OperationDataToReturn}, nil } @@ -310,7 +321,7 @@ func (fakeBroker *FakeAsyncServiceBroker) Deprovision(context context.Context, i fakeBroker.DeprovisionedInstanceIDs = append(fakeBroker.DeprovisionedInstanceIDs, instanceID) fakeBroker.DeprovisionDetails = details - if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) { + if _, ok := fakeBroker.ProvisionedInstances[instanceID]; ok { return brokerapi.DeprovisionServiceSpec{IsAsync: asyncAllowed, OperationData: fakeBroker.OperationDataToReturn}, nil } @@ -340,19 +351,20 @@ func (fakeBroker *FakeServiceBroker) GetBinding(context context.Context, instanc func (fakeBroker *FakeAsyncServiceBroker) Bind(context context.Context, instanceID, bindingID string, details brokerapi.BindDetails, asyncAllowed bool) (brokerapi.Binding, error) { fakeBroker.BrokerCalled = true - fakeBroker.BoundBindingDetails = details - - fakeBroker.BoundInstanceIDs = append(fakeBroker.BoundInstanceIDs, instanceID) - fakeBroker.BoundBindingIDs = append(fakeBroker.BoundBindingIDs, bindingID) - if asyncAllowed { + if _, ok := fakeBroker.BoundBindings[bindingID]; ok { + return fakeBroker.FakeServiceBroker.Bind(context, instanceID, bindingID, details, true) + } + + fakeBroker.BoundInstanceIDs = append(fakeBroker.BoundInstanceIDs, instanceID) + fakeBroker.BoundBindings[bindingID] = details return brokerapi.Binding{ IsAsync: true, OperationData: "0xDEADBEEF", }, nil - } else { - return fakeBroker.FakeServiceBroker.Bind(context, instanceID, bindingID, details, false) } + + return fakeBroker.FakeServiceBroker.Bind(context, instanceID, bindingID, details, false) } func (fakeBroker *FakeServiceBroker) Bind(context context.Context, instanceID, bindingID string, details brokerapi.BindDetails, asyncAllowed bool) (brokerapi.Binding, error) { @@ -362,16 +374,7 @@ func (fakeBroker *FakeServiceBroker) Bind(context context.Context, instanceID, b fakeBroker.ReceivedContext = val } - if fakeBroker.BindError != nil { - return brokerapi.Binding{}, fakeBroker.BindError - } - - fakeBroker.BoundBindingDetails = details - - fakeBroker.BoundInstanceIDs = append(fakeBroker.BoundInstanceIDs, instanceID) - fakeBroker.BoundBindingIDs = append(fakeBroker.BoundBindingIDs, bindingID) - - return brokerapi.Binding{ + binding := brokerapi.Binding{ Credentials: FakeCredentials{ Host: "127.0.0.1", Port: 3000, @@ -381,7 +384,23 @@ func (fakeBroker *FakeServiceBroker) Bind(context context.Context, instanceID, b SyslogDrainURL: fakeBroker.SyslogDrainURL, RouteServiceURL: fakeBroker.RouteServiceURL, VolumeMounts: fakeBroker.VolumeMounts, - }, nil + } + + if _, ok := fakeBroker.BoundBindings[bindingID]; ok { + if reflect.DeepEqual(fakeBroker.BoundBindings[bindingID], details) { + binding.AlreadyExists = true + return binding, nil + } + } + + if fakeBroker.BindError != nil { + return brokerapi.Binding{}, fakeBroker.BindError + } + + fakeBroker.BoundInstanceIDs = append(fakeBroker.BoundInstanceIDs, instanceID) + fakeBroker.BoundBindings[bindingID] = details + + return binding, nil } func (fakeBroker *FakeServiceBroker) Unbind(context context.Context, instanceID, bindingID string, details brokerapi.UnbindDetails, asyncAllowed bool) (brokerapi.UnbindSpec, error) { @@ -397,8 +416,8 @@ func (fakeBroker *FakeServiceBroker) Unbind(context context.Context, instanceID, fakeBroker.UnbindingDetails = details - if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) { - if sliceContains(bindingID, fakeBroker.BoundBindingIDs) { + if _, ok := fakeBroker.ProvisionedInstances[instanceID]; ok { + if _, ok := fakeBroker.BoundBindings[bindingID]; ok { return brokerapi.UnbindSpec{}, nil } return brokerapi.UnbindSpec{}, brokerapi.ErrBindingDoesNotExist diff --git a/handlers/bind.go b/handlers/bind.go index 4a03c367..a24710d2 100644 --- a/handlers/bind.go +++ b/handlers/bind.go @@ -80,6 +80,16 @@ func (h APIHandler) Bind(w http.ResponseWriter, req *http.Request) { return } + if binding.AlreadyExists { + h.respond(w, http.StatusOK, apiresponses.BindingResponse{ + Credentials: binding.Credentials, + SyslogDrainURL: binding.SyslogDrainURL, + RouteServiceURL: binding.RouteServiceURL, + VolumeMounts: binding.VolumeMounts, + }) + return + } + if binding.IsAsync { h.respond(w, http.StatusAccepted, apiresponses.AsyncBindResponse{ OperationData: binding.OperationData, diff --git a/handlers/provision.go b/handlers/provision.go index 8b00004e..9a5e4b23 100644 --- a/handlers/provision.go +++ b/handlers/provision.go @@ -110,7 +110,11 @@ func (h *APIHandler) Provision(w http.ResponseWriter, req *http.Request) { return } - if provisionResponse.IsAsync { + if provisionResponse.AlreadyExists { + h.respond(w, http.StatusOK, apiresponses.ProvisioningResponse{ + DashboardURL: provisionResponse.DashboardURL, + }) + } else if provisionResponse.IsAsync { h.respond(w, http.StatusAccepted, apiresponses.ProvisioningResponse{ DashboardURL: provisionResponse.DashboardURL, OperationData: provisionResponse.OperationData,