From 9f046d0edf2ba6a8ec12d07374ece2ae9b05e88b Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Wed, 15 Aug 2018 13:21:27 +0300 Subject: [PATCH 1/3] Add support for async. binding and unbinding operations Based on the new functionality detailed in version 2.14 of the Open Service Broker API, the ServiceBroker interface is updated to include the two new operations required for async bind/unbind operations: GetLastBindingOperation and GetServiceBinding These operations are only called if the caller provides a required version of at least 2.14 in the OSB version header. Similarly, requesting an Async operation is only permitted if the caller specifies a required version of at least 2.14 in the OSB version header. --- Gopkg.lock | 12 +- api.go | 214 ++++++++++++++++++++++++++++++----- api_test.go | 10 +- catalog_test.go | 2 +- fakes/fake_service_broker.go | 29 +++-- response.go | 20 ++++ response_test.go | 2 +- service_broker.go | 42 +++++-- 8 files changed, 278 insertions(+), 53 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 6fe195ca..8f95bd88 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -80,6 +80,16 @@ packages = ["."] revision = "c65b2f87fee37d1c7854c9164a450713c28d50cd" +[[projects]] + name = "github.com/pivotal-cf/brokerapi" + packages = [ + ".", + "auth", + "fakes" + ] + revision = "d1fcddd951a6776bc01659143dda6aca057e98b7" + version = "v2.0.4" + [[projects]] branch = "master" name = "golang.org/x/net" @@ -127,6 +137,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "f5bfe5f275adcd02ff0727a3f378547c393e64ad2de6a2ce7e21f4b617d83456" + inputs-digest = "45092a33e183a6dc6304f8e76a8f2e22031ef45ca85cb1f7d01653c149e3c774" solver-name = "gps-cdcl" solver-version = 1 diff --git a/api.go b/api.go index 56bb4883..ea7f3bf8 100644 --- a/api.go +++ b/api.go @@ -18,23 +18,25 @@ package brokerapi import ( "encoding/json" "errors" + "fmt" "net/http" "strconv" - "strings" "code.cloudfoundry.org/lager" "github.com/gorilla/mux" - "github.com/pivotal-cf/brokerapi/auth" + "github.com/liorokman/brokerapi/auth" ) const ( - provisionLogKey = "provision" - deprovisionLogKey = "deprovision" - bindLogKey = "bind" - unbindLogKey = "unbind" - updateLogKey = "update" - lastOperationLogKey = "lastOperation" - catalogLogKey = "catalog" + provisionLogKey = "provision" + deprovisionLogKey = "deprovision" + bindLogKey = "bind" + getBindLogKey = "getBinding" + unbindLogKey = "unbind" + updateLogKey = "update" + lastOperationLogKey = "lastOperation" + lastBindingOperationLogKey = "lastBindingOperation" + catalogLogKey = "catalog" instanceIDLogKey = "instance-id" instanceDetailsLogKey = "instance-details" @@ -47,6 +49,7 @@ const ( bindingAlreadyExistsErrorKey = "binding-already-exists" instanceMissingErrorKey = "instance-missing" bindingMissingErrorKey = "binding-missing" + bindingNotFoundErrorKey = "binding-not-found" asyncRequiredKey = "async-required" planChangeNotSupportedKey = "plan-change-not-supported" unknownErrorKey = "unknown-error" @@ -88,6 +91,8 @@ func AttachRoutes(router *mux.Router, serviceBroker ServiceBroker, logger lager. router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", handler.bind).Methods("PUT") router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", handler.unbind).Methods("DELETE") + + router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}/last_operation", handler.lastBindingOperation).Methods("GET") } type serviceBrokerHandler struct { @@ -98,7 +103,7 @@ type serviceBrokerHandler struct { func (h serviceBrokerHandler) catalog(w http.ResponseWriter, req *http.Request) { logger := h.logger.Session(catalogLogKey, lager.Data{}) - if err := checkBrokerAPIVersionHdr(req); err != nil { + if _, err := checkBrokerAPIVersionHdr(req); err != nil { h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ Description: err.Error(), }) @@ -129,7 +134,7 @@ func (h serviceBrokerHandler) provision(w http.ResponseWriter, req *http.Request instanceIDLogKey: instanceID, }) - if err := checkBrokerAPIVersionHdr(req); err != nil { + if _, err := checkBrokerAPIVersionHdr(req); err != nil { h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ Description: err.Error(), }) @@ -237,7 +242,7 @@ func (h serviceBrokerHandler) update(w http.ResponseWriter, req *http.Request) { instanceIDLogKey: instanceID, }) - if err := checkBrokerAPIVersionHdr(req); err != nil { + if _, err := checkBrokerAPIVersionHdr(req); err != nil { h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ Description: err.Error(), }) @@ -293,7 +298,7 @@ func (h serviceBrokerHandler) deprovision(w http.ResponseWriter, req *http.Reque instanceIDLogKey: instanceID, }) - if err := checkBrokerAPIVersionHdr(req); err != nil { + if _, err := checkBrokerAPIVersionHdr(req); err != nil { h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ Description: err.Error(), }) @@ -346,6 +351,58 @@ func (h serviceBrokerHandler) deprovision(w http.ResponseWriter, req *http.Reque } } +func (h serviceBrokerHandler) getBinding(w http.ResponseWriter, req *http.Request) { + vars := mux.Vars(req) + instanceID := vars["instance_id"] + bindingID := vars["binding_id"] + + logger := h.logger.Session(getBindLogKey, lager.Data{ + instanceIDLogKey: instanceID, + bindingIDLogKey: bindingID, + }) + + 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 { + h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + Description: "get binding endpoint only supported starting with OSB version 2.14", + }) + logger.Error(apiVersionInvalidKey, err) + return + } + + binding, err := h.serviceBroker.GetBinding(req.Context(), instanceID, bindingID) + 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, GetBindingResponse{ + BindingResponse: BindingResponse{ + Credentials: binding.Credentials, + SyslogDrainURL: binding.SyslogDrainURL, + RouteServiceURL: binding.RouteServiceURL, + VolumeMounts: binding.VolumeMounts, + }, + Parameters: binding.Parameters, + }) +} + func (h serviceBrokerHandler) bind(w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) instanceID := vars["instance_id"] @@ -356,7 +413,8 @@ func (h serviceBrokerHandler) bind(w http.ResponseWriter, req *http.Request) { bindingIDLogKey: bindingID, }) - if err := checkBrokerAPIVersionHdr(req); err != nil { + versionCompatibility, err := checkBrokerAPIVersionHdr(req) + if err != nil { h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ Description: err.Error(), }) @@ -389,7 +447,16 @@ func (h serviceBrokerHandler) bind(w http.ResponseWriter, req *http.Request) { return } - binding, err := h.serviceBroker.Bind(req.Context(), instanceID, bindingID, details) + asyncAllowed := req.FormValue("accepts_incomplete") == "true" + if asyncAllowed && versionCompatibility.Minor < 14 { + h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + Description: "async binding only supported from OSB version 2.14 and up", + }) + logger.Error(apiVersionInvalidKey, err) + return + } + + binding, err := h.serviceBroker.Bind(req.Context(), instanceID, bindingID, details, asyncAllowed) if err != nil { switch err := err.(type) { case *FailureResponse: @@ -413,8 +480,13 @@ func (h serviceBrokerHandler) bind(w http.ResponseWriter, req *http.Request) { return } - brokerAPIVersion := req.Header.Get("X-Broker-Api-Version") - if brokerAPIVersion == "2.8" || brokerAPIVersion == "2.9" { + if binding.IsAsync { + h.respond(w, http.StatusAccepted, AsyncBindResponse{ + OperationData: binding.OperationData, + }) + } + + if versionCompatibility.Minor == 8 || versionCompatibility.Minor == 9 { experimentalVols := []ExperimentalVolumeMount{} for _, vol := range binding.VolumeMounts { @@ -446,7 +518,12 @@ func (h serviceBrokerHandler) bind(w http.ResponseWriter, req *http.Request) { return } - h.respond(w, http.StatusCreated, binding) + h.respond(w, http.StatusCreated, BindingResponse{ + Credentials: binding.Credentials, + SyslogDrainURL: binding.SyslogDrainURL, + RouteServiceURL: binding.RouteServiceURL, + VolumeMounts: binding.VolumeMounts, + }) } func (h serviceBrokerHandler) unbind(w http.ResponseWriter, req *http.Request) { @@ -459,7 +536,8 @@ func (h serviceBrokerHandler) unbind(w http.ResponseWriter, req *http.Request) { bindingIDLogKey: bindingID, }) - if err := checkBrokerAPIVersionHdr(req); err != nil { + versionCompatibility, err := checkBrokerAPIVersionHdr(req) + if err != nil { h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ Description: err.Error(), }) @@ -488,7 +566,17 @@ func (h serviceBrokerHandler) unbind(w http.ResponseWriter, req *http.Request) { return } - if err := h.serviceBroker.Unbind(req.Context(), instanceID, bindingID, details); err != nil { + asyncAllowed := req.FormValue("accepts_incomplete") == "true" + if asyncAllowed && versionCompatibility.Minor < 14 { + h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + Description: "async unbinding only supported from OSB version 2.14 and up", + }) + logger.Error(apiVersionInvalidKey, err) + return + } + + unbindResponse, err := h.serviceBroker.Unbind(req.Context(), instanceID, bindingID, details, asyncAllowed) + if err != nil { switch err := err.(type) { case *FailureResponse: logger.Error(err.LoggerAction(), err) @@ -502,19 +590,80 @@ func (h serviceBrokerHandler) unbind(w http.ResponseWriter, req *http.Request) { return } - h.respond(w, http.StatusOK, EmptyResponse{}) + if unbindResponse.IsAsync { + h.respond(w, http.StatusAccepted, UnbindResponse{ + OperationData: unbindResponse.OperationData, + }) + } else { + h.respond(w, http.StatusOK, EmptyResponse{}) + } + +} + +func (h serviceBrokerHandler) lastBindingOperation(w http.ResponseWriter, req *http.Request) { + vars := mux.Vars(req) + instanceID := vars["instance_id"] + bindingID := vars["binding_id"] + pollDetails := PollDetails{ + PlanID: req.FormValue("plan_id"), + ServiceID: req.FormValue("service_id"), + OperationData: req.FormValue("operation"), + } + + logger := h.logger.Session(lastBindingOperationLogKey, lager.Data{ + instanceIDLogKey: instanceID, + }) + + if _, err := checkBrokerAPIVersionHdr(req); err != nil { + h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + Description: err.Error(), + }) + logger.Error(apiVersionInvalidKey, err) + return + } + + logger.Info("starting-check-for-binding-operation") + + lastOperation, err := h.serviceBroker.LastBindingOperation(req.Context(), instanceID, bindingID, pollDetails) + + 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 + } + + logger.WithData(lager.Data{"state": lastOperation.State}).Info("done-check-for-binding-operation") + + lastOperationResponse := LastOperationResponse{ + State: lastOperation.State, + Description: lastOperation.Description, + } + h.respond(w, http.StatusOK, lastOperationResponse) + } func (h serviceBrokerHandler) lastOperation(w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) instanceID := vars["instance_id"] - operationData := req.FormValue("operation") + pollDetails := PollDetails{ + PlanID: req.FormValue("plan_id"), + ServiceID: req.FormValue("service_id"), + OperationData: req.FormValue("operation"), + } logger := h.logger.Session(lastOperationLogKey, lager.Data{ instanceIDLogKey: instanceID, }) - if err := checkBrokerAPIVersionHdr(req); err != nil { + if _, err := checkBrokerAPIVersionHdr(req); err != nil { h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ Description: err.Error(), }) @@ -524,7 +673,7 @@ func (h serviceBrokerHandler) lastOperation(w http.ResponseWriter, req *http.Req logger.Info("starting-check-for-operation") - lastOperation, err := h.serviceBroker.LastOperation(req.Context(), instanceID, operationData) + lastOperation, err := h.serviceBroker.LastOperation(req.Context(), instanceID, pollDetails) if err != nil { switch err := err.(type) { @@ -561,14 +710,21 @@ func (h serviceBrokerHandler) respond(w http.ResponseWriter, status int, respons } } -func checkBrokerAPIVersionHdr(req *http.Request) error { +type brokerVersion struct { + Major int + Minor int +} + +func checkBrokerAPIVersionHdr(req *http.Request) (brokerVersion, error) { + var version brokerVersion apiVersion := req.Header.Get("X-Broker-API-Version") if apiVersion == "" { - return errors.New("X-Broker-API-Version Header not set") + return version, errors.New("X-Broker-API-Version Header not set") } + fmt.Sscanf("%d.%d", apiVersion, &version.Major, &version.Minor) - if !strings.HasPrefix(apiVersion, "2.") { - return errors.New("X-Broker-API-Version Header must be 2.x") + if version.Major != 2 { + return version, errors.New("X-Broker-API-Version Header must be 2.x") } - return nil + return version, nil } diff --git a/api_test.go b/api_test.go index dd95ad22..2ebf3bd6 100644 --- a/api_test.go +++ b/api_test.go @@ -29,10 +29,10 @@ import ( "code.cloudfoundry.org/lager" "code.cloudfoundry.org/lager/lagertest" "github.com/drewolson/testflight" + "github.com/liorokman/brokerapi" + "github.com/liorokman/brokerapi/fakes" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/pivotal-cf/brokerapi" - "github.com/pivotal-cf/brokerapi/fakes" ) var _ = Describe("Service Broker API", func() { @@ -1439,9 +1439,9 @@ var _ = Describe("Service Broker API", func() { It("calls Bind on the service broker with the bind_resource", func() { details["bind_resource"] = map[string]interface{}{ - "app_guid": "a-guid", - "space_guid": "a-space-guid", - "route": "route.cf-apps.com", + "app_guid": "a-guid", + "space_guid": "a-space-guid", + "route": "route.cf-apps.com", "credential_client_id": "some-credentials", } diff --git a/catalog_test.go b/catalog_test.go index d87412dc..47624dbb 100644 --- a/catalog_test.go +++ b/catalog_test.go @@ -19,9 +19,9 @@ import ( "encoding/json" "reflect" + "github.com/liorokman/brokerapi" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/pivotal-cf/brokerapi" ) var _ = Describe("Catalog", func() { diff --git a/fakes/fake_service_broker.go b/fakes/fake_service_broker.go index 1517243f..3142cb66 100644 --- a/fakes/fake_service_broker.go +++ b/fakes/fake_service_broker.go @@ -4,7 +4,7 @@ import ( "context" "errors" - "github.com/pivotal-cf/brokerapi" + "github.com/liorokman/brokerapi" ) type FakeServiceBroker struct { @@ -290,7 +290,13 @@ func (fakeBroker *FakeAsyncServiceBroker) Deprovision(context context.Context, i return brokerapi.DeprovisionServiceSpec{OperationData: fakeBroker.OperationDataToReturn, IsAsync: asyncAllowed}, brokerapi.ErrInstanceDoesNotExist } -func (fakeBroker *FakeServiceBroker) Bind(context context.Context, instanceID, bindingID string, details brokerapi.BindDetails) (brokerapi.Binding, error) { +func (fakeBroker *FakeServiceBroker) GetBinding(ctx context.Context, instanceID, bindingID string) (brokerapi.GetBindingSpec, error) { + fakeBroker.BrokerCalled = true + + return brokerapi.GetBindingSpec{}, nil +} + +func (fakeBroker *FakeServiceBroker) Bind(context context.Context, instanceID, bindingID string, details brokerapi.BindDetails, asyncAllowed bool) (brokerapi.Binding, error) { fakeBroker.BrokerCalled = true if val, ok := context.Value("test_context").(bool); ok { @@ -319,7 +325,7 @@ func (fakeBroker *FakeServiceBroker) Bind(context context.Context, instanceID, b }, nil } -func (fakeBroker *FakeServiceBroker) Unbind(context context.Context, instanceID, bindingID string, details brokerapi.UnbindDetails) error { +func (fakeBroker *FakeServiceBroker) Unbind(context context.Context, instanceID, bindingID string, details brokerapi.UnbindDetails, asyncAllowed bool) (brokerapi.UnbindSpec, error) { fakeBroker.BrokerCalled = true if val, ok := context.Value("test_context").(bool); ok { @@ -327,24 +333,29 @@ func (fakeBroker *FakeServiceBroker) Unbind(context context.Context, instanceID, } if fakeBroker.UnbindError != nil { - return fakeBroker.UnbindError + return brokerapi.UnbindSpec{}, fakeBroker.UnbindError } fakeBroker.UnbindingDetails = details if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) { if sliceContains(bindingID, fakeBroker.BoundBindingIDs) { - return nil + return brokerapi.UnbindSpec{}, nil } - return brokerapi.ErrBindingDoesNotExist + return brokerapi.UnbindSpec{}, brokerapi.ErrBindingDoesNotExist } - return brokerapi.ErrInstanceDoesNotExist + return brokerapi.UnbindSpec{}, brokerapi.ErrInstanceDoesNotExist +} + +func (fakeBroker *FakeServiceBroker) LastBindingOperation(context context.Context, instanceID, bindingID string, details brokerapi.PollDetails) (brokerapi.LastOperation, error) { + + return brokerapi.LastOperation{}, nil } -func (fakeBroker *FakeServiceBroker) LastOperation(context context.Context, instanceID, operationData string) (brokerapi.LastOperation, error) { +func (fakeBroker *FakeServiceBroker) LastOperation(context context.Context, instanceID string, details brokerapi.PollDetails) (brokerapi.LastOperation, error) { fakeBroker.LastOperationInstanceID = instanceID - fakeBroker.LastOperationData = operationData + fakeBroker.LastOperationData = details.OperationData if val, ok := context.Value("test_context").(bool); ok { fakeBroker.ReceivedContext = val diff --git a/response.go b/response.go index acc22d3d..c25ea0ec 100644 --- a/response.go +++ b/response.go @@ -44,6 +44,26 @@ type LastOperationResponse struct { Description string `json:"description,omitempty"` } +type AsyncBindResponse struct { + OperationData string `json:"operation,omitempty"` +} + +type BindingResponse struct { + Credentials interface{} `json:"credentials"` + SyslogDrainURL string `json:"syslog_drain_url,omitempty"` + RouteServiceURL string `json:"route_service_url,omitempty"` + VolumeMounts []VolumeMount `json:"volume_mounts,omitempty"` +} + +type GetBindingResponse struct { + BindingResponse + Parameters interface{} `json:"parameters"` +} + +type UnbindResponse struct { + OperationData string `json:"operation,omitempty"` +} + type ExperimentalVolumeMountBindingResponse struct { Credentials interface{} `json:"credentials"` SyslogDrainURL string `json:"syslog_drain_url,omitempty"` diff --git a/response_test.go b/response_test.go index 9b2ca17a..ff12f84f 100644 --- a/response_test.go +++ b/response_test.go @@ -18,9 +18,9 @@ package brokerapi_test import ( "encoding/json" + "github.com/liorokman/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 b11d5788..acea861e 100644 --- a/service_broker.go +++ b/service_broker.go @@ -28,12 +28,14 @@ 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) - Bind(ctx context.Context, instanceID, bindingID string, details BindDetails) (Binding, error) - Unbind(ctx context.Context, instanceID, bindingID string, details UnbindDetails) 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) + GetBinding(ctx context.Context, instanceID, bindingID string) (GetBindingSpec, error) Update(ctx context.Context, instanceID string, details UpdateDetails, asyncAllowed bool) (UpdateServiceSpec, error) - LastOperation(ctx context.Context, instanceID, operationData string) (LastOperation, error) + LastOperation(ctx context.Context, instanceID string, details PollDetails) (LastOperation, error) + LastBindingOperation(ctx context.Context, instanceID, bindingID string, details PollDetails) (LastOperation, error) } type DetailsWithRawParameters interface { @@ -79,6 +81,11 @@ type ProvisionedServiceSpec struct { OperationData string } +type UnbindSpec struct { + IsAsync bool + OperationData string +} + type BindDetails struct { AppGUID string `json:"app_guid"` PlanID string `json:"plan_id"` @@ -130,6 +137,12 @@ type PreviousValues struct { SpaceID string `json:"space_id"` } +type PollDetails struct { + ServiceID string `json:"service_id"` + PlanID string `json:"plan_id"` + OperationData string `json:"operation"` +} + type LastOperation struct { State LastOperationState Description string @@ -144,10 +157,20 @@ const ( ) type Binding struct { - Credentials interface{} `json:"credentials"` - SyslogDrainURL string `json:"syslog_drain_url,omitempty"` - RouteServiceURL string `json:"route_service_url,omitempty"` - VolumeMounts []VolumeMount `json:"volume_mounts,omitempty"` + IsAsync bool + OperationData string + Credentials interface{} + SyslogDrainURL string + RouteServiceURL string + VolumeMounts []VolumeMount +} + +type GetBindingSpec struct { + Credentials interface{} + SyslogDrainURL string + RouteServiceURL string + VolumeMounts []VolumeMount + Parameters interface{} } type VolumeMount struct { @@ -171,6 +194,7 @@ const ( serviceQuotaExceededMsg = "The quota for this service has been exceeded. Please contact your Operator for help." bindingExistsMsg = "binding already exists" bindingDoesntExistMsg = "binding does not exist" + bindingNotFoundMsg = "binding cannot be fetched" asyncRequiredMsg = "This service plan requires client support for asynchronous service operations." planChangeUnsupportedMsg = "The requested plan migration cannot be performed" rawInvalidParamsMsg = "The format of the parameters is not valid JSON" @@ -198,6 +222,10 @@ var ( errors.New(bindingDoesntExistMsg), http.StatusGone, bindingMissingErrorKey, ).WithEmptyResponse().Build() + ErrBindingNotFound = NewFailureResponseBuilder( + errors.New(bindingNotFoundMsg), http.StatusNotFound, bindingNotFoundErrorKey, + ).WithEmptyResponse().Build() + ErrAsyncRequired = NewFailureResponseBuilder( errors.New(asyncRequiredMsg), http.StatusUnprocessableEntity, asyncRequiredKey, ).WithErrorKey("AsyncRequired").Build() From d8f27d681e8c8db1d0fc2365790a825f2208502b Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Thu, 16 Aug 2018 10:00:30 +0300 Subject: [PATCH 2/3] Added unit tests for the new code --- api.go | 21 ++++- api_test.go | 133 +++++++++++++++++++++++++++--- fakes/fake_service_broker.go | 47 ++++++++++- fixtures/async_bind_response.json | 3 + response.go | 2 +- response_test.go | 2 +- 6 files changed, 188 insertions(+), 20 deletions(-) create mode 100644 fixtures/async_bind_response.json diff --git a/api.go b/api.go index ea7f3bf8..f8a7240d 100644 --- a/api.go +++ b/api.go @@ -89,6 +89,7 @@ func AttachRoutes(router *mux.Router, serviceBroker ServiceBroker, logger lager. router.HandleFunc("/v2/service_instances/{instance_id}/last_operation", handler.lastOperation).Methods("GET") router.HandleFunc("/v2/service_instances/{instance_id}", handler.update).Methods("PATCH") + router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", handler.getBinding).Methods("GET") router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", handler.bind).Methods("PUT") router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", handler.unbind).Methods("DELETE") @@ -104,6 +105,7 @@ func (h serviceBrokerHandler) catalog(w http.ResponseWriter, req *http.Request) logger := h.logger.Session(catalogLogKey, lager.Data{}) if _, err := checkBrokerAPIVersionHdr(req); err != nil { + logger.Error("Check failed", err) h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ Description: err.Error(), }) @@ -449,7 +451,7 @@ func (h serviceBrokerHandler) bind(w http.ResponseWriter, req *http.Request) { asyncAllowed := req.FormValue("accepts_incomplete") == "true" if asyncAllowed && versionCompatibility.Minor < 14 { - h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + h.respond(w, http.StatusUnprocessableEntity, ErrorResponse{ Description: "async binding only supported from OSB version 2.14 and up", }) logger.Error(apiVersionInvalidKey, err) @@ -484,6 +486,7 @@ func (h serviceBrokerHandler) bind(w http.ResponseWriter, req *http.Request) { h.respond(w, http.StatusAccepted, AsyncBindResponse{ OperationData: binding.OperationData, }) + return } if versionCompatibility.Minor == 8 || versionCompatibility.Minor == 9 { @@ -568,7 +571,7 @@ func (h serviceBrokerHandler) unbind(w http.ResponseWriter, req *http.Request) { asyncAllowed := req.FormValue("accepts_incomplete") == "true" if asyncAllowed && versionCompatibility.Minor < 14 { - h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + h.respond(w, http.StatusUnprocessableEntity, ErrorResponse{ Description: "async unbinding only supported from OSB version 2.14 and up", }) logger.Error(apiVersionInvalidKey, err) @@ -614,13 +617,21 @@ func (h serviceBrokerHandler) lastBindingOperation(w http.ResponseWriter, req *h instanceIDLogKey: instanceID, }) - if _, err := checkBrokerAPIVersionHdr(req); err != nil { + 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 { + h.respond(w, http.StatusPreconditionFailed, ErrorResponse{ + Description: "get binding endpoint only supported starting with OSB version 2.14", + }) + logger.Error(apiVersionInvalidKey, err) + return + } logger.Info("starting-check-for-binding-operation") @@ -721,7 +732,9 @@ func checkBrokerAPIVersionHdr(req *http.Request) (brokerVersion, error) { if apiVersion == "" { return version, errors.New("X-Broker-API-Version Header not set") } - fmt.Sscanf("%d.%d", apiVersion, &version.Major, &version.Minor) + if n, err := fmt.Sscanf(apiVersion, "%d.%d", &version.Major, &version.Minor); err != nil || n < 2 { + return version, errors.New("X-Broker-API-Version Header must contain a version") + } if version.Major != 2 { return version, errors.New("X-Broker-API-Version Header must be 2.x") diff --git a/api_test.go b/api_test.go index 2ebf3bd6..f35abba5 100644 --- a/api_test.go +++ b/api_test.go @@ -126,7 +126,7 @@ var _ = Describe("Service Broker API", func() { makeRequest := func(method, path, body string) *httptest.ResponseRecorder { recorder := httptest.NewRecorder() request, _ := http.NewRequest(method, path, strings.NewReader(body)) - request.Header.Add("X-Broker-API-Version", "2.13") + request.Header.Add("X-Broker-API-Version", "2.14") request.SetBasicAuth(credentials.Username, credentials.Password) request = request.WithContext(ctx) brokerAPI.ServeHTTP(recorder, request) @@ -168,6 +168,16 @@ var _ = Describe("Service Broker API", func() { Expect(fakeServiceBroker.ReceivedContext).To(BeTrue()) }) + Specify("a get binding operation endpoint which passes the request context to the broker", func() { + makeRequest("GET", "/v2/service_instances/instance-id/service_bindings/binding-id", "{}") + Expect(fakeServiceBroker.ReceivedContext).To(BeTrue()) + }) + + Specify("a last binding operation endpoint which passes the request context to the broker", func() { + makeRequest("GET", "/v2/service_instances/instance-id/service_bindings/binding-id/last_operation", "{}") + Expect(fakeServiceBroker.ReceivedContext).To(BeTrue()) + }) + Specify("a last operation endpoint which passes the request context to the broker", func() { makeRequest("GET", "/v2/service_instances/instance-id/last_operation", "{}") Expect(fakeServiceBroker.ReceivedContext).To(BeTrue()) @@ -1199,11 +1209,56 @@ var _ = Describe("Service Broker API", func() { }) Describe("binding lifecycle endpoint", func() { - makeBindingRequestWithSpecificAPIVersion := func(instanceID, bindingID string, details map[string]interface{}, apiVersion string) *testflight.Response { + + makeLastBindingOperationRequestWithSpecificAPIVersion := func(instanceID, bindingID string, apiVersion string) *testflight.Response { + response := &testflight.Response{} + testflight.WithServer(brokerAPI, func(r *testflight.Requester) { + path := fmt.Sprintf("/v2/service_instances/%s/service_bindings/%s/last_operation", instanceID, bindingID) + + buffer := &bytes.Buffer{} + + request, err := http.NewRequest("GET", path, buffer) + + Expect(err).NotTo(HaveOccurred()) + + if apiVersion != "" { + request.Header.Add("X-Broker-Api-Version", apiVersion) + } + request.Header.Add("Content-Type", "application/json") + request.SetBasicAuth("username", "password") + + response = r.Do(request) + }) + return response + } + + makeGetBindingRequestWithSpecificAPIVersion := func(instanceID, bindingID string, apiVersion string) *testflight.Response { response := &testflight.Response{} testflight.WithServer(brokerAPI, func(r *testflight.Requester) { - path := fmt.Sprintf("/v2/service_instances/%s/service_bindings/%s", - instanceID, bindingID) + path := fmt.Sprintf("/v2/service_instances/%s/service_bindings/%s", instanceID, bindingID) + + buffer := &bytes.Buffer{} + + request, err := http.NewRequest("GET", path, buffer) + + Expect(err).NotTo(HaveOccurred()) + + if apiVersion != "" { + request.Header.Add("X-Broker-Api-Version", apiVersion) + } + request.Header.Add("Content-Type", "application/json") + request.SetBasicAuth("username", "password") + + response = r.Do(request) + }) + return response + } + + makeBindingRequestWithSpecificAPIVersion := func(instanceID, bindingID string, details map[string]interface{}, apiVersion string, async bool) *testflight.Response { + response := &testflight.Response{} + testflight.WithServer(brokerAPI, func(r *testflight.Requester) { + path := fmt.Sprintf("/v2/service_instances/%s/service_bindings/%s?accepts_incomplete=%v", + instanceID, bindingID, async) buffer := &bytes.Buffer{} @@ -1227,7 +1282,11 @@ var _ = Describe("Service Broker API", func() { } makeBindingRequest := func(instanceID, bindingID string, details map[string]interface{}) *testflight.Response { - return makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, details, "2.10") + return makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, details, "2.10", false) + } + + makeAsyncBindingRequest := func(instanceID, bindingID string, details map[string]interface{}) *testflight.Response { + return makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, details, "2.14", true) } Describe("binding", func() { @@ -1256,28 +1315,28 @@ var _ = Describe("Service Broker API", func() { }) It("missing header X-Broker-API-Version", func() { - response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, map[string]interface{}{}, "") + response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, map[string]interface{}{}, "", false) Expect(response.StatusCode).To(Equal(412)) Expect(lastLogLine().Message).To(ContainSubstring(".bind.broker-api-version-invalid")) Expect(lastLogLine().Data["error"]).To(ContainSubstring("X-Broker-API-Version Header not set")) }) It("has wrong version of API", func() { - response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, map[string]interface{}{}, "1.14") + response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, map[string]interface{}{}, "1.14", false) Expect(response.StatusCode).To(Equal(412)) Expect(lastLogLine().Message).To(ContainSubstring(".bind.broker-api-version-invalid")) Expect(lastLogLine().Data["error"]).To(ContainSubstring("X-Broker-API-Version Header must be 2.x")) }) It("missing service-id", func() { - response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, map[string]interface{}{"plan_id": "123"}, "2.14") + response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, map[string]interface{}{"plan_id": "123"}, "2.14", false) Expect(response.StatusCode).To(Equal(400)) Expect(lastLogLine().Message).To(ContainSubstring(".bind.service-id-missing")) Expect(lastLogLine().Data["error"]).To(ContainSubstring("service_id missing")) }) It("missing plan-id", func() { - response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, map[string]interface{}{"service_id": "123"}, "2.14") + response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, map[string]interface{}{"service_id": "123"}, "2.14", false) Expect(response.StatusCode).To(Equal(400)) Expect(lastLogLine().Message).To(ContainSubstring(".bind.plan-id-missing")) Expect(lastLogLine().Data["error"]).To(ContainSubstring("plan_id missing")) @@ -1359,14 +1418,14 @@ var _ = Describe("Service Broker API", func() { Context("when the broker API version is 2.9", func() { It("responds with an experimental volume mount", func() { - response := makeBindingRequestWithSpecificAPIVersion(uniqueInstanceID(), uniqueBindingID(), details, "2.9") + response := makeBindingRequestWithSpecificAPIVersion(uniqueInstanceID(), uniqueBindingID(), details, "2.9", false) Expect(response.Body).To(MatchJSON(fixture("binding_with_experimental_volume_mounts.json"))) }) }) Context("when the broker API version is 2.8", func() { It("responds with an experimental volume mount", func() { - response := makeBindingRequestWithSpecificAPIVersion(uniqueInstanceID(), uniqueBindingID(), details, "2.8") + response := makeBindingRequestWithSpecificAPIVersion(uniqueInstanceID(), uniqueBindingID(), details, "2.8", false) Expect(response.Body).To(MatchJSON(fixture("binding_with_experimental_volume_mounts.json"))) }) }) @@ -1567,6 +1626,58 @@ var _ = Describe("Service Broker API", func() { Expect(lastLogLine().Data["error"]).To(ContainSubstring("I failed in unique and interesting ways")) }) }) + + Context("when an async binding is requested", func() { + var ( + fakeAsyncServiceBroker *fakes.FakeAsyncServiceBroker + ) + BeforeEach(func() { + fakeAsyncServiceBroker = &fakes.FakeAsyncServiceBroker{ + FakeServiceBroker: *fakeServiceBroker, + ShouldBindAsync: true, + } + brokerAPI = brokerapi.New(fakeAsyncServiceBroker, brokerLogger, credentials) + }) + + It("when the api version is before 2.14 for Bind request", func() { + response := makeBindingRequestWithSpecificAPIVersion(instanceID, bindingID, details, "2.13", true) + Expect(response.StatusCode).To(Equal(http.StatusUnprocessableEntity)) + }) + + It("when the api version is before 2.14 for LastBindingOperation request", func() { + response := makeLastBindingOperationRequestWithSpecificAPIVersion(instanceID, bindingID, "1.13") + Expect(response.StatusCode).To(Equal(http.StatusPreconditionFailed)) + response = makeLastBindingOperationRequestWithSpecificAPIVersion(instanceID, bindingID, "2.13") + Expect(response.StatusCode).To(Equal(http.StatusPreconditionFailed)) + }) + + It("when the api version is before 2.14 for GetBinding request", func() { + response := makeGetBindingRequestWithSpecificAPIVersion(instanceID, bindingID, "1.13") + Expect(response.StatusCode).To(Equal(http.StatusPreconditionFailed)) + response = makeGetBindingRequestWithSpecificAPIVersion(instanceID, bindingID, "2.13") + Expect(response.StatusCode).To(Equal(http.StatusPreconditionFailed)) + }) + + It("it returns an appropriate status code and operation data", func() { + response := makeAsyncBindingRequest(instanceID, bindingID, details) + Expect(response.StatusCode).To(Equal(http.StatusAccepted)) + Expect(response.Body).To(MatchJSON(fixture("async_bind_response.json"))) + }) + + It("can be polled with lastBindingOperation", func() { + fakeAsyncServiceBroker.LastOperationState = "succeeded" + fakeAsyncServiceBroker.LastOperationDescription = "some description" + response := makeLastBindingOperationRequestWithSpecificAPIVersion(instanceID, bindingID, "2.14") + Expect(response.StatusCode).To(Equal(http.StatusOK)) + Expect(response.Body).To(MatchJSON(fixture("last_operation_succeeded.json"))) + }) + + It("getBinding returns the binding for the async request", func() { + response := makeGetBindingRequestWithSpecificAPIVersion(instanceID, bindingID, "2.14") + Expect(response.StatusCode).To(Equal(http.StatusOK)) + Expect(response.Body).To(MatchJSON(fixture("binding.json"))) + }) + }) }) Describe("unbinding", func() { diff --git a/fakes/fake_service_broker.go b/fakes/fake_service_broker.go index 3142cb66..884a7006 100644 --- a/fakes/fake_service_broker.go +++ b/fakes/fake_service_broker.go @@ -56,6 +56,7 @@ type FakeServiceBroker struct { type FakeAsyncServiceBroker struct { FakeServiceBroker ShouldProvisionAsync bool + ShouldBindAsync bool } type FakeAsyncOnlyServiceBroker struct { @@ -290,10 +291,42 @@ func (fakeBroker *FakeAsyncServiceBroker) Deprovision(context context.Context, i return brokerapi.DeprovisionServiceSpec{OperationData: fakeBroker.OperationDataToReturn, IsAsync: asyncAllowed}, brokerapi.ErrInstanceDoesNotExist } -func (fakeBroker *FakeServiceBroker) GetBinding(ctx context.Context, instanceID, bindingID string) (brokerapi.GetBindingSpec, error) { +func (fakeBroker *FakeServiceBroker) GetBinding(context context.Context, instanceID, bindingID string) (brokerapi.GetBindingSpec, error) { fakeBroker.BrokerCalled = true - return brokerapi.GetBindingSpec{}, nil + if val, ok := context.Value("test_context").(bool); ok { + fakeBroker.ReceivedContext = val + } + + return brokerapi.GetBindingSpec{ + Credentials: FakeCredentials{ + Host: "127.0.0.1", + Port: 3000, + Username: "batman", + Password: "robin", + }, + SyslogDrainURL: fakeBroker.SyslogDrainURL, + RouteServiceURL: fakeBroker.RouteServiceURL, + VolumeMounts: fakeBroker.VolumeMounts, + }, nil +} + +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 fakeBroker.ShouldBindAsync { + return brokerapi.Binding{ + IsAsync: true, + OperationData: "0xDEADBEEF", + }, nil + } else { + 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) { @@ -350,7 +383,15 @@ func (fakeBroker *FakeServiceBroker) Unbind(context context.Context, instanceID, func (fakeBroker *FakeServiceBroker) LastBindingOperation(context context.Context, instanceID, bindingID string, details brokerapi.PollDetails) (brokerapi.LastOperation, error) { - return brokerapi.LastOperation{}, nil + if val, ok := context.Value("test_context").(bool); ok { + fakeBroker.ReceivedContext = val + } + + if fakeBroker.LastOperationError != nil { + return brokerapi.LastOperation{}, fakeBroker.LastOperationError + } + + return brokerapi.LastOperation{State: fakeBroker.LastOperationState, Description: fakeBroker.LastOperationDescription}, nil } func (fakeBroker *FakeServiceBroker) LastOperation(context context.Context, instanceID string, details brokerapi.PollDetails) (brokerapi.LastOperation, error) { diff --git a/fixtures/async_bind_response.json b/fixtures/async_bind_response.json new file mode 100644 index 00000000..62444616 --- /dev/null +++ b/fixtures/async_bind_response.json @@ -0,0 +1,3 @@ +{ + "operation":"0xDEADBEEF" +} diff --git a/response.go b/response.go index c25ea0ec..1481298e 100644 --- a/response.go +++ b/response.go @@ -57,7 +57,7 @@ type BindingResponse struct { type GetBindingResponse struct { BindingResponse - Parameters interface{} `json:"parameters"` + Parameters interface{} `json:"parameters,omitempty"` } type UnbindResponse struct { diff --git a/response_test.go b/response_test.go index ff12f84f..d1c7e4a6 100644 --- a/response_test.go +++ b/response_test.go @@ -63,7 +63,7 @@ var _ = Describe("Provisioning Response", func() { var _ = Describe("Binding Response", func() { Describe("JSON encoding", func() { It("has a credentials object", func() { - binding := brokerapi.Binding{} + binding := brokerapi.BindingResponse{} jsonString := `{"credentials":null}` Expect(json.Marshal(binding)).To(MatchJSON(jsonString)) From 49ecaf584445d5935596e307b73085d8d3d6ca6a Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Thu, 16 Aug 2018 10:02:42 +0300 Subject: [PATCH 3/3] Fixed the import paths to work with upstream --- api.go | 2 +- api_test.go | 4 ++-- catalog_test.go | 2 +- fakes/fake_service_broker.go | 2 +- response_test.go | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api.go b/api.go index f8a7240d..a9d3fcd5 100644 --- a/api.go +++ b/api.go @@ -24,7 +24,7 @@ import ( "code.cloudfoundry.org/lager" "github.com/gorilla/mux" - "github.com/liorokman/brokerapi/auth" + "github.com/pivotal-cf/brokerapi/auth" ) const ( diff --git a/api_test.go b/api_test.go index f35abba5..1cb3589a 100644 --- a/api_test.go +++ b/api_test.go @@ -29,8 +29,8 @@ import ( "code.cloudfoundry.org/lager" "code.cloudfoundry.org/lager/lagertest" "github.com/drewolson/testflight" - "github.com/liorokman/brokerapi" - "github.com/liorokman/brokerapi/fakes" + "github.com/pivotal-cf/brokerapi" + "github.com/pivotal-cf/brokerapi/fakes" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) diff --git a/catalog_test.go b/catalog_test.go index 47624dbb..9417bc25 100644 --- a/catalog_test.go +++ b/catalog_test.go @@ -19,7 +19,7 @@ import ( "encoding/json" "reflect" - "github.com/liorokman/brokerapi" + "github.com/pivotal-cf/brokerapi" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) diff --git a/fakes/fake_service_broker.go b/fakes/fake_service_broker.go index 884a7006..50d3cd39 100644 --- a/fakes/fake_service_broker.go +++ b/fakes/fake_service_broker.go @@ -4,7 +4,7 @@ import ( "context" "errors" - "github.com/liorokman/brokerapi" + "github.com/pivotal-cf/brokerapi" ) type FakeServiceBroker struct { diff --git a/response_test.go b/response_test.go index d1c7e4a6..c7b3efef 100644 --- a/response_test.go +++ b/response_test.go @@ -18,7 +18,7 @@ package brokerapi_test import ( "encoding/json" - "github.com/liorokman/brokerapi" + "github.com/pivotal-cf/brokerapi" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" )