Merge pull request #56 from liorokman/osb214-for-merge

Add missing pieces for supporting OSB 2.14

Author: winnab

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"]

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() {

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() {

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"))

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
 

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"
+    }
+}

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"`

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() {

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")
 )