diff --git a/api.go b/api.go index e77f5e43..170b7e01 100644 --- a/api.go +++ b/api.go @@ -32,6 +32,7 @@ const ( deprovisionLogKey = "deprovision" bindLogKey = "bind" getBindLogKey = "getBinding" + getInstanceLogKey = "getInstance" unbindLogKey = "unbind" updateLogKey = "update" lastOperationLogKey = "lastOperation" @@ -60,6 +61,7 @@ const ( planIdMissingKey = "plan-id-missing" invalidServiceID = "invalid-service-id" invalidPlanID = "invalid-plan-id" + concurrentAccessKey = "get-instance-during-update" ) var ( @@ -84,6 +86,7 @@ func AttachRoutes(router *mux.Router, serviceBroker ServiceBroker, logger lager. handler := serviceBrokerHandler{serviceBroker: serviceBroker, logger: logger} router.HandleFunc("/v2/catalog", handler.catalog).Methods("GET") + router.HandleFunc("/v2/service_instances/{instance_id}", handler.getInstance).Methods("GET") router.HandleFunc("/v2/service_instances/{instance_id}", handler.provision).Methods("PUT") router.HandleFunc("/v2/service_instances/{instance_id}", handler.deprovision).Methods("DELETE") router.HandleFunc("/v2/service_instances/{instance_id}/last_operation", handler.lastOperation).Methods("GET") @@ -292,7 +295,7 @@ func (h serviceBrokerHandler) update(w http.ResponseWriter, req *http.Request) { } h.respond(w, statusCode, UpdateResponse{ OperationData: updateServiceSpec.OperationData, - DashboardURL: updateServiceSpec.DashboardURL, + DashboardURL: updateServiceSpec.DashboardURL, }) } @@ -356,6 +359,54 @@ func (h serviceBrokerHandler) deprovision(w http.ResponseWriter, req *http.Reque } } +func (h serviceBrokerHandler) getInstance(w http.ResponseWriter, req *http.Request) { + vars := mux.Vars(req) + instanceID := vars["instance_id"] + + logger := h.logger.Session(getInstanceLogKey, lager.Data{ + instanceIDLogKey: instanceID, + }) + + versionCompatibility, err := checkBrokerAPIVersionHdr(req) + if err != nil { + h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + Description: err.Error(), + }) + logger.Error(apiVersionInvalidKey, err) + return + } + if versionCompatibility.Minor < 14 { + err = errors.New("get instance endpoint only supported starting with OSB version 2.14") + h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + Description: err.Error(), + }) + logger.Error(apiVersionInvalidKey, err) + return + } + + instanceDetails, err := h.serviceBroker.GetInstance(req.Context(), instanceID) + if err != nil { + switch err := err.(type) { + case *FailureResponse: + logger.Error(err.LoggerAction(), err) + h.respond(w, err.ValidatedStatusCode(logger), err.ErrorResponse()) + default: + logger.Error(unknownErrorKey, err) + h.respond(w, http.StatusInternalServerError, ErrorResponse{ + Description: err.Error(), + }) + } + return + } + + h.respond(w, http.StatusOK, GetInstanceResponse{ + ServiceID: instanceDetails.ServiceID, + PlanID: instanceDetails.PlanID, + DashboardURL: instanceDetails.DashboardURL, + Parameters: instanceDetails.Parameters, + }) +} + func (h serviceBrokerHandler) getBinding(w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) instanceID := vars["instance_id"] diff --git a/api_test.go b/api_test.go index 5fec0917..85c17731 100644 --- a/api_test.go +++ b/api_test.go @@ -182,6 +182,11 @@ var _ = Describe("Service Broker API", func() { makeRequest("GET", "/v2/service_instances/instance-id/last_operation", "{}") Expect(fakeServiceBroker.ReceivedContext).To(BeTrue()) }) + + Specify("a get instance operation endpoint which passes the request context to the broker", func() { + makeRequest("GET", "/v2/service_instances/instance-id", "{}") + Expect(fakeServiceBroker.ReceivedContext).To(BeTrue()) + }) }) Describe("authentication", func() { @@ -297,6 +302,19 @@ var _ = Describe("Service Broker API", func() { }) Describe("instance lifecycle endpoint", func() { + makeGetInstanceRequest := func(instanceID string) *testflight.Response { + response := &testflight.Response{} + testflight.WithServer(brokerAPI, func(r *testflight.Requester) { + path := fmt.Sprintf("/v2/service_instances/%s", instanceID) + request, err := http.NewRequest("GET", path, strings.NewReader("")) + Expect(err).NotTo(HaveOccurred()) + request.Header.Add("X-Broker-API-Version", apiVersion) + request.SetBasicAuth("username", "password") + response = r.Do(request) + }) + return response + } + makeInstanceDeprovisioningRequestFull := func(instanceID, serviceID, planID, queryString string) *testflight.Response { response := &testflight.Response{} testflight.WithServer(brokerAPI, func(r *testflight.Requester) { @@ -348,6 +366,15 @@ var _ = Describe("Service Broker API", func() { Expect(fakeServiceBroker.ProvisionedInstanceIDs).To(ContainElement(instanceID)) }) + It("calls GetInstance on the service broker with the instance id", func() { + makeInstanceProvisioningRequest(instanceID, provisionDetails, "") + Expect(fakeServiceBroker.ProvisionedInstanceIDs).To(ContainElement(instanceID)) + fakeServiceBroker.DashboardURL = "https://example.com/dashboard/some-instance" + resp := makeGetInstanceRequest(instanceID) + Expect(fakeServiceBroker.GetInstanceIDs).To(ContainElement(instanceID)) + Expect(resp.Body).To(MatchJSON(fixture("get_instance.json"))) + }) + Context("when the broker returns some operation data", func() { BeforeEach(func() { fakeServiceBroker = &fakes.FakeServiceBroker{ @@ -1218,6 +1245,60 @@ var _ = Describe("Service Broker API", func() { }) }) }) + + Describe("getting instance", func() { + It("returns the appropriate status code when it fails with a known error", func() { + fakeServiceBroker.GetInstanceError = brokerapi.NewFailureResponse(errors.New("some error"), http.StatusUnprocessableEntity, "fire") + + response := makeGetInstanceRequest("instance-id") + + Expect(response.StatusCode).To(Equal(http.StatusUnprocessableEntity)) + Expect(lastLogLine().Message).To(ContainSubstring("broker-api.getInstance.fire")) + Expect(lastLogLine().Data["error"]).To(ContainSubstring("some error")) + }) + + It("returns 500 when it fails with an unknown error", func() { + fakeServiceBroker.GetInstanceError = errors.New("failed to get instance") + + response := makeGetInstanceRequest("instance-id") + + Expect(response.StatusCode).To(Equal(500)) + Expect(lastLogLine().Message).To(ContainSubstring("broker-api.getInstance.unknown-error")) + Expect(lastLogLine().Data["error"]).To(ContainSubstring("failed to get instance")) + }) + + Context("the request is malformed", func() { + It("missing header X-Broker-API-Version", func() { + apiVersion = "" + response := makeGetInstanceRequest("instance-id") + Expect(response.StatusCode).To(Equal(412)) + Expect(lastLogLine().Message).To(ContainSubstring(".getInstance.broker-api-version-invalid")) + Expect(lastLogLine().Data["error"]).To(ContainSubstring("X-Broker-API-Version Header not set")) + }) + + It("has wrong version of API", func() { + apiVersion = "1.1" + response := makeGetInstanceRequest("instance-id") + Expect(response.StatusCode).To(Equal(412)) + Expect(lastLogLine().Message).To(ContainSubstring(".getInstance.broker-api-version-invalid")) + Expect(lastLogLine().Data["error"]).To(ContainSubstring("X-Broker-API-Version Header must be 2.x")) + }) + + It("is using api version < 2.14", func() { + apiVersion = "2.13" + response := makeGetInstanceRequest("instance-id") + Expect(response.StatusCode).To(Equal(412)) + + Expect(lastLogLine().Message).To(ContainSubstring(".getInstance.broker-api-version-invalid")) + Expect(lastLogLine().Data["error"]).To(ContainSubstring("get instance endpoint only supported starting with OSB version 2.14")) + }) + + It("missing instance-id", func() { + response := makeGetInstanceRequest("") + Expect(response.StatusCode).To(Equal(404)) + }) + }) + }) }) Describe("binding lifecycle endpoint", func() { diff --git a/catalog_test.go b/catalog_test.go index 175d4717..22a6838d 100644 --- a/catalog_test.go +++ b/catalog_test.go @@ -20,9 +20,9 @@ import ( "reflect" "sync" - "github.com/pivotal-cf/brokerapi" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/pivotal-cf/brokerapi" ) var _ = Describe("Catalog", func() { diff --git a/failure_response_test.go b/failure_response_test.go index 059c7d9e..8a203335 100644 --- a/failure_response_test.go +++ b/failure_response_test.go @@ -65,7 +65,7 @@ var _ = Describe("FailureResponse", func() { Expect(newError.Error()).To(Equal("my error message and some more details")) Expect(newError.ValidatedStatusCode(nil)).To(Equal(http.StatusForbidden)) Expect(newError.LoggerAction()).To(Equal(failureResponse.LoggerAction())) - + errorResponse, typeCast := newError.ErrorResponse().(brokerapi.ErrorResponse) Expect(typeCast).To(BeTrue()) Expect(errorResponse.Error).To(Equal("some-key")) diff --git a/fakes/fake_service_broker.go b/fakes/fake_service_broker.go index aac7d15e..6859c08f 100644 --- a/fakes/fake_service_broker.go +++ b/fakes/fake_service_broker.go @@ -15,6 +15,7 @@ type FakeServiceBroker struct { ProvisionedInstanceIDs []string DeprovisionedInstanceIDs []string UpdatedInstanceIDs []string + GetInstanceIDs []string BoundInstanceIDs []string BoundBindingIDs []string @@ -33,6 +34,7 @@ type FakeServiceBroker struct { DeprovisionError error LastOperationError error UpdateError error + GetInstanceError error BrokerCalled bool LastOperationState brokerapi.LastOperationState @@ -232,6 +234,24 @@ func (fakeBroker *FakeServiceBroker) Update(context context.Context, instanceID return brokerapi.UpdateServiceSpec{IsAsync: fakeBroker.ShouldReturnAsync, OperationData: fakeBroker.OperationDataToReturn, DashboardURL: fakeBroker.DashboardURL}, nil } +func (fakeBroker *FakeServiceBroker) GetInstance(context context.Context, instanceID string) (brokerapi.GetInstanceDetailsSpec, error) { + fakeBroker.BrokerCalled = true + + if val, ok := context.Value("test_context").(bool); ok { + fakeBroker.ReceivedContext = val + } + + fakeBroker.GetInstanceIDs = append(fakeBroker.GetInstanceIDs, instanceID) + return brokerapi.GetInstanceDetailsSpec{ + ServiceID: fakeBroker.ServiceID, + PlanID: fakeBroker.PlanID, + DashboardURL: fakeBroker.DashboardURL, + Parameters: map[string]interface{}{ + "param1": "value1", + }, + }, fakeBroker.GetInstanceError +} + func (fakeBroker *FakeServiceBroker) Deprovision(context context.Context, instanceID string, details brokerapi.DeprovisionDetails, asyncAllowed bool) (brokerapi.DeprovisionServiceSpec, error) { fakeBroker.BrokerCalled = true diff --git a/fixtures/get_instance.json b/fixtures/get_instance.json new file mode 100644 index 00000000..06e1828a --- /dev/null +++ b/fixtures/get_instance.json @@ -0,0 +1,8 @@ +{ + "service_id" : "0A789746-596F-4CEA-BFAC-A0795DA056E3", + "plan_id" : "plan-id", + "dashboard_url" : "https://example.com/dashboard/some-instance", + "parameters": { + "param1" : "value1" + } +} diff --git a/response.go b/response.go index 46b9e428..d49a257e 100644 --- a/response.go +++ b/response.go @@ -31,6 +31,13 @@ type ProvisioningResponse struct { OperationData string `json:"operation,omitempty"` } +type GetInstanceResponse struct { + ServiceID string `json:"service_id"` + PlanID string `json:"plan_id"` + DashboardURL string `json:"dashboard_url,omitempty"` + Parameters interface{} `json:"parameters,omitempty"` +} + type UpdateResponse struct { DashboardURL string `json:"dashboard_url,omitempty"` OperationData string `json:"operation,omitempty"` diff --git a/response_test.go b/response_test.go index df9e8b05..adc2338d 100644 --- a/response_test.go +++ b/response_test.go @@ -18,9 +18,9 @@ package brokerapi_test import ( "encoding/json" - "github.com/pivotal-cf/brokerapi" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/pivotal-cf/brokerapi" ) var _ = Describe("Catalog Response", func() { diff --git a/service_broker.go b/service_broker.go index 8efa0c0d..b781740e 100644 --- a/service_broker.go +++ b/service_broker.go @@ -27,6 +27,7 @@ type ServiceBroker interface { Provision(ctx context.Context, instanceID string, details ProvisionDetails, asyncAllowed bool) (ProvisionedServiceSpec, error) Deprovision(ctx context.Context, instanceID string, details DeprovisionDetails, asyncAllowed bool) (DeprovisionServiceSpec, error) + GetInstance(ctx context.Context, instanceID string) (GetInstanceDetailsSpec, error) Bind(ctx context.Context, instanceID, bindingID string, details BindDetails, asyncAllowed bool) (Binding, error) Unbind(ctx context.Context, instanceID, bindingID string, details UnbindDetails, asyncAllowed bool) (UnbindSpec, error) @@ -81,6 +82,13 @@ type ProvisionedServiceSpec struct { OperationData string } +type GetInstanceDetailsSpec struct { + ServiceID string `json:"service_id"` + PlanID string `json:"plan_id"` + DashboardURL string `json:"dashboard_url"` + Parameters interface{} `json:"parameters"` +} + type UnbindSpec struct { IsAsync bool OperationData string @@ -200,6 +208,7 @@ const ( planChangeUnsupportedMsg = "The requested plan migration cannot be performed" rawInvalidParamsMsg = "The format of the parameters is not valid JSON" appGuidMissingMsg = "app_guid is a required field but was not provided" + concurrentInstanceAccessMsg = "instance is being updated and cannot be retrieved" ) var ( @@ -245,4 +254,8 @@ var ( ErrPlanQuotaExceeded = errors.New(servicePlanQuotaExceededMsg) ErrServiceQuotaExceeded = errors.New(serviceQuotaExceededMsg) + + ErrConcurrentInstanceAccess = NewFailureResponseBuilder( + errors.New(concurrentInstanceAccessMsg), http.StatusUnprocessableEntity, concurrentAccessKey, + ).WithErrorKey("ConcurrencyError") )