Code Monkey home page Code Monkey logo

brokerapi's People

Contributors

ataleksandrov avatar avade avatar blgm avatar craigfurman avatar cwlbraa avatar dependabot-preview[bot] avatar dependabot[bot] avatar dprotaso avatar drnic avatar felisiam avatar gabrielecipriano avatar goonzoid avatar iamralch avatar jacknewberry avatar jimbo459 avatar joek avatar kieron-dev avatar kisamoto avatar liorokman avatar lwoydziak avatar mariantalla avatar peterellisjones avatar pivotal-marcela-campo avatar simonjjones avatar st3v avatar terminatingcode avatar tinygrasshopper avatar williammartin avatar x6j8x avatar zachgersh avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

brokerapi's Issues

Add support for dynamic authentication

Currently the library only seems to support static credentials for authentication. We're developing a broker where we want to let users pass API credentials through the broker basic auth.

I'm thinking that BrokerCredentials in the New function could be replaced with an interface with a single function for validation. BrokerCredentials could then implement this interface making the change backwards compatible. The passed credentials would probably have to be accessible in all the ServiceBroker functions, perhaps as part of the details parameter.

Allow returning custom error status codes and responses

At the moment, brokerapi users can return predefined error types to control status codes and response bodies: ErrRawParamsInvalid, ErrInstanceAlreadyExists, etc. Other errors always throw a 500. What do you think about accepting an error type that would allow developers more control over error responses, like this:

type APIError struct {
	StatusCode int
	Body map[string]interface{}
}

func (e APIError) String() string {
	body, err := json.Marshal(e.Body)
	if err != nil {
		return "Unknown error"
	}
	return string(body)
}

Then we'd be able to move long switch statements like https://github.com/pivotal-cf/brokerapi/blob/master/api.go#L103-L128 into a series of error instances, and users would be able to customize errors. WDYT?

Ability to implement custom ServicePlanCost?

For the given route /v2/catalog, our broker will be expected to return the services with these fields

type ServicePlanMetadata struct {
	DisplayName        string            `json:"displayName,omitempty"`
	Bullets            []string          `json:"bullets,omitempty"`
	Costs              []ServicePlanCost `json:"costs,omitempty"`
	AdditionalMetadata map[string]interface{}
}

type ServicePlanCost struct {
	UnitID     string `json:"unitId"`
	Unit       string `json:"unit"`
	PartNumber string `json:"partNumber"`
}

I know there is an AdditionalMetaData field, but our added fields need to be nested with costs. Is there a way we could implement this custom struct without having to implement our own ServiceBroker interface?

Missing StatusOK 200

I am currently working on a service broker and I noticed that the brokerapi isn't returning StatusOK 200 and the respond() function isn't public either. In other words there isn't a way to properly get StatusOK 200 from the brokerapi.

I currently need this functionality as described by the servicebrokerapi for #provisioning and the #binding when an instance/binding already exists and the requested parameters are identical to the existing instance/binding.

Any help or clarification on this matter would be appreciated.

Use of authentication methods other than Basic Auth

The service broker API spec defines that «it is RECOMMENDED that all communications between a Platform and a Service Broker are […] authenticated Also that «Platforms and Service Brokers MAY agree on an authentication mechanism other than basic authentication, but the specific agreements are not covered by this specification.»

This stands in contrast to the implementation of this project, which mandates Basic Auth, at least when initialized using api.New(…).

In our project, we would like to implement authentication based on Bearer Tokens. I'm tasked to implement this.

Currently, my only option is to replicate the api.New(…) in our own code. But I would like to avoid that, because it increases the possibility that something breaks unexpectedly when we update to a later version of the brokerapi:
We would have to establish some means to check that our replication of api.New(…) still matches the upstream version, except for the authentication part.

I therefore propose a refactoring of that method. With backwards-compatibility in mind, I suggest to introduce a new function func NewWithCustomAuth(serviceBroker ServiceBroker, logger lager.Logger, authMiddleware http.Handler) in api.go like so:

func New(serviceBroker ServiceBroker, logger lager.Logger, brokerCredentials BrokerCredentials) http.Handler {
	authMiddleware := auth.NewWrapper(brokerCredentials.Username, brokerCredentials.Password).Wrap
	return NewWithCustomAuth(serviceBroker, logger, authMiddleware)
}

func NewWithCustomAuth(serviceBroker ServiceBroker, logger lager.Logger, authMiddleware mux.MiddlewareFunc) http.Handler {
	router := mux.NewRouter()

	AttachRoutes(router, serviceBroker, logger)

	apiVersionMiddleware := middlewares.APIVersionMiddleware{LoggerFactory: logger}

	router.Use(middlewares.AddCorrelationIDToContext)
	router.Use(authMiddleware)
	router.Use(middlewares.AddOriginatingIdentityToContext)
	router.Use(apiVersionMiddleware.ValidateAPIVersionHdr)
	router.Use(middlewares.AddInfoLocationToContext)
	router.Use(middlewares.AddRequestIdentityToContext)

	return router
}

Perhaps you find a better name for NewWithCustomAuth, as it's not pretty. Yet it does convey the message what it differentiates from func New(…).

Now:

  1. Did I miss a way to implement authentication using Bearer Tokens without modifying or replicating api.New(…)?
  2. Would you welcome a PR that changes the implementation to the proposed implementation?
  3. What else did I miss?

Unversioned handlers shouldn't not use versioned domain/apiresponses

Unversioned handlers e.g.:

"github.com/pivotal-cf/brokerapi/v7/domain/apiresponses"

shouldn't not import versioned apiresponses:

"github.com/pivotal-cf/brokerapi/v7/domain/apiresponses" 

but only unversioned apiresponses:

"github.com/pivotal-cf/brokerapi/domain/apiresponses" 

Otherwise handler responses with custom response code e.g. 400 are returned with 500 (http.StatusInternalServerError) response code due to used logic. Error type in this handler's switch is actually *v7/apiresponses.FailureResponse, so it is not matching right branch and default branch is used instead:

if err != nil {
switch err := err.(type) {
case *apiresponses.FailureResponse:
logger.Error(err.LoggerAction(), err)
h.respond(w, err.ValidatedStatusCode(logger), err.ErrorResponse())
default:
logger.Error(unknownErrorKey, err)
h.respond(w, http.StatusInternalServerError, apiresponses.ErrorResponse{
Description: err.Error(),
})
}
return
}

Users with correct usage of versioning/Go modules won't notice this issue. Generally, codes from v7 folder should be used only in v7 folder codes. Similar problems can be also with utils/middlewares.

Setting CF labels not supported?

My broker has a method

func (esb *MyBroker) Provision(ctx context.Context, instanceID string,
	details brokerapi_domain.ProvisionDetails,
	asyncAllowed bool) (brokerapi_domain.ProvisionedServiceSpec, error)

which returns something like this:

var spec brokerapi_domain.ProvisionedServiceSpec
labels := map[string]string{"foo": "bar"}
spec.Metadata = brokerapi_domain.InstanceMetadata{Labels: labels}
return spec, nil

But after creating a service instance foo, cf labels service-instance foo returns nothing.

for v11: NewFailureResponse() should return error not *apiresponses.FailureResponse

Describe the bug

See: #252 (comment)

Reproduction steps

See: #252 (comment)

Expected behavior

A New... function arguably should return the type that it mentions, but in this case returning an error type may make more sense as it would encourage the use of var error rather than var *apierrors.FailureResponse which is not idiomatic Go, and can lead to strange situations.

Additional context

Would be a breaking change, so wait until v11

Types in details structs are different

Hi,
as you might have ssen I opend a pull request, which was not done so far. #35

What I like to know is, which direction should we go. Should all the *Details Structs have a RawParameteres field, should the PrivisionDetails struct be changed back to a map type Parameters field or should everything stay as it is ;-)

Cheers
Johannes

Broker does not accept url-encoded forward slash in instance_id

We expect all our service's instance_ids to have a forward slash among other non-standard characters e.g. a/81eb:59d6 The forward slash gets encoded to %2F but the broker still returns a 404 not found error when making a provision call. A normal string as the instance_id works perfectly fine.

Your .dependabot/config.yml contained invalid details

Dependabot encountered the following error when parsing your .dependabot/config.yml:

The property '#/update_configs/0/package_manager' value "go:mod" did not match one of the following values: javascript, ruby:bundler, php:composer, java:maven, elixir:hex, rust:cargo, java:gradle, dotnet:nuget, go:dep, go:modules, docker, elm, submodules, github_actions, python, terraform

Please update the config file to conform with Dependabot's specification using our docs and online validator.

Use chi.Router interface for AttachRoutes method

Is your feature request related to a problem? Please describe.

For some methods chi returns the interface type chi.Router, e.g. when using the Route method on *chi.Mux to create a subrouter.

The AttachRoutes method currently uses the concrete implementation instead of the chi.Router interface, although it only relies on methods exposed by chi.Router.

Describe the solution you'd like

Changing the AttachRoutes method to accept the chi.Router interface type, instead of *chi.Mux would make it easier to use in some scenarios.

func AttachRoutes(router chi.Router, serviceBroker ServiceBroker, logger lager.Logger) {
	attachRoutes(router, serviceBroker, logger)
}

func attachRoutes(router chi.Router, serviceBroker ServiceBroker, logger lager.Logger) {
    // ...
}

Describe alternatives you've considered

Using a subrouter can still be achieved by creating a new router and mounting it manually, but methods such as Route or Group could only be used with type assertions.

subRouter := chi.NewRouter()
router.Mount("/my-route", subRouter)

Additional context

No response

Semver

I noticed that there have been a few minor breaking changes lately--would you all be open to using semantic versioning and tagging releases so that users can get a better sense of what's changing?

Dependabot can't parse your go.mod

Dependabot couldn't parse the go.mod found at /go.mod.

The error Dependabot encountered was:

go: github.com/maxbrunsfeld/counterfeiter/[email protected] requires
	gopkg.in/[email protected]: invalid version: git fetch -f origin refs/heads/*:refs/heads/* refs/tags/*:refs/tags/* in /opt/go/gopath/pkg/mod/cache/vcs/9241c28341fcedca6a799ab7a465dd6924dc5d94044cbfabb75778817250adfc: exit status 128:
	fatal: The remote end hung up unexpectedly

View the update logs.

Replace lager.Logger with log/slog.Logger

Is your feature request related to a problem? Please describe.

At the moment the brokerapi requires an import of lager.Logger as another third party dependency.
As the lager.Logger interface contains LogFormat in Sink, it can't be implemented without the dependency to lager.Logger.
I would like to avoid the dependency to another third party dependency.

Describe the solution you'd like

Replace lager.Logger with an own interface or use log/slog.Logger as a solution from the standard library.

Describe alternatives you've considered

No response

Additional context

No response

Missing field in ServicePlan

cf curl /v2/service_plans returns something like

{
   "total_results": 1,
   "total_pages": 1,
   "prev_url": null,
   "next_url": null,
   "resources": [
      {
         "metadata": {
            "guid": "72e63697-fbed-4db1-8d40-f05826c43baf",
            "url": "/v2/service_plans/72e63697-fbed-4db1-8d40-f05826c43baf",
            "created_at": "2017-04-13T10:42:07Z",
            "updated_at": "2017-04-13T10:42:07Z"
         },
         "entity": {
            "name": ...,
            "free": true,
            "description": ...,
            "service_guid": "bb3d80c6-7d5b-4930-96c9-5bdb6c26a7b5",
            "extra": ...,
            "unique_id": "3e466fc6-833e-430a-a5a1-b9aa120850c8",
            "public": true,
            "bindable": true,
            "active": true,
            "service_url": "/v2/services/bb3d80c6-7d5b-4930-96c9-5bdb6c26a7b5",
            "service_instances_url": "/v2/service_plans/72e63697-fbed-4db1-8d40-f05826c43baf/service_instances"
         }
      }
   ]
}

but the ServicePlan struct doesn't contain the public field. Thus I'd propose to change the ServicePlan definition to

type ServicePlan struct {
	ID          string               `json:"id"`
	Name        string               `json:"name"`
	Description string               `json:"description"`
	Free        *bool                `json:"free,omitempty"`
	Public      *bool                `json:"public,omitempty"`
	Bindable    *bool                `json:"bindable,omitempty"`
	Metadata    *ServicePlanMetadata `json:"metadata,omitempty"`
}

why ErrInstanceAlreadyExists respond "EmptyResponse"

if err != nil {
switch err {
case ErrInstanceAlreadyExists:
logger.Error(instanceAlreadyExistsErrorKey, err)
respond(w, http.StatusConflict, EmptyResponse{})
case ErrInstanceLimitMet:
logger.Error(instanceLimitReachedErrorKey, err)
respond(w, http.StatusInternalServerError, ErrorResponse{
Description: err.Error(),
})

why ErrInstanceAlreadyExists does not respond detail error message like ErrInstanceLimitMet?

Proposal - Add Service/Plan instances to Context Object

What?

In relevant HTTP handlers add checked service/plan instances to HTTP context for further reuse throughout the platform.

Why?

The API implemented in the brokerapi library is already retrieving the list of services and performing various checks (serviceID & planID valid etc.) however this data is just discarded and methods looking to reuse the service instance have to retrieve the service instance again.

This inefficiency can be removed by adding the valid service instance to the context.Context of the request being processed.

How?

Code untested

Example using brokerapi/api.go:serviceBrokerHandler.provision

Add new methods for adding & extracting Service and Plan objects from Context:

type contextKey string

const (
	contextKeyService contextKey = "brokerapi_service"
	contextKeyPlan    contextKey = "brokerapi_plan"
)

func AddServiceToContext(service *Service, ctx context.Context) context.Context {
	return context.WithValue(ctx, contextKeyService, service)
}

func AddPlanToContext(plan *Plan, ctx context.Context) context.Context {
	return context.WithValue(ctx, contextKeyPlan, plan)
}

func ServiceFromContext(ctx context.Context) *Service {
	if v := ctx.Value(contextKeyService); v != nil {
		return v.(*Service)
	}
	return nil
}

func PlanFromContext(ctx, context.Context) *Plan {
	if v := ctx.Value(contextKeyPlan); v != nil {
		return v.(*Plan)
	}
	return nil
}

Update api to add details to Context

[..]
	services, _ := h.serviceBroker.Services(req.Context())
	for _, service := range services {
		if service.ID == details.ServiceID {
			req = req.WithContext(AddServiceToContext(*service, req.Context())) // Add to Context
			valid = true
			break
		}
	}
[..]
	for _, service := range services {
		for _, plan := range service.Plans {
			if plan.ID == details.PlanID {
				req = req.WithContext(AddPlanToContext(*plan, req.Context())) // Add to Context
				valid = true
				break
			}
		}
	}
[..]

Easily re-use these instances from Context in ServiceBroker implementation

service := brokerapi.ServiceFromContext(ctx)
if service == nil {
        // fallback to looking up service from database
}

Next steps

Open to discussion about pros/cons of this implementation. If it's something that may be of interest I can write a complete pull request.

ErrConcurrentInstanceAccess returns Builder instead of 422 Response

In apiresponses/errors, all errors return FailureResponses. However, the 'ErrConcurrentInstanceAccess' returns a FailureResponseBuilder. Will cause 500 error instead of 422. Is this intended?

https://github.com/pivotal-cf/brokerapi/blob/master/domain/apiresponses/errors.go#L85

Recommended Fix:

ErrConcurrentInstanceAccess = NewFailureResponseBuilder(
		errors.New(concurrentInstanceAccessMsg), http.StatusUnprocessableEntity, concurrentAccessKey,
	).WithErrorKey("ConcurrencyError").Build()

v8 backwards compatiblity

Can we get a backwards compatible commit in for the brokerapi v8 folder similar to what was done here for v7 please?

3d253be

This is preventing us from consuming the latest broker-api. thanks for the help.

brokerapi.New Dictates Logging library

I would prefer to use a different logger implementation, but using this sdk requires using code.cloudfoundry.org/lager. I could use a different logging library elsewhere in the code, but that also seems more problematic as messages would have different formats. stdlib's logger package is little bit anemic, but as a library, it would be friendlier to let implementers choose their logging solution and have brokerapi.New() accept a log.Logger instead of a particular implementation - especially one so niche.

Support for updating service instances

Hi,

I am going to work on adding support for updating service instances. From the start, I would like to discuss things to be sure I keep following the right direction.

These are the public-facing changes I am going to make:

  • Update brokerapi.Service with a PlanUpdatable field:
    ...
    PlanUpdatable bool `json:"plan_updateable"`
  • Extend ServiceBroker with an Update method:
    ...
    Update(instanceID string, details UpdateDetails, asyncAllowed bool) (IsAsync, error)
  • With UpdateDetails declared as:
type UpdateDetails struct {
    ServiceID string `json:"service_id"`
    PlanID string `json:"plan_id"`
    Parameters map[string]interface{} `json:"parameters"`
}

Both the public-facing part and the work to be done under the hood seem very similar to what is done for instance provisioning, and I do not expect to encounter any pitfalls for now.

Looking for your thoughts.

Feature request: Simple way to add prometheus exporter

Hello,

From what I have seen, there is no easy way to add a metrics exporter (typically for Prometheus). We have to add it from scratch.

Adding a route /metrics by default or if we add a parameter would be interesting.

Example: operator-sdk is a framework to build kubernetes operator, it has a simple way to export metrics(https://docs.openshift.com/container-platform/4.9/operators/operator_sdk/osdk-monitoring-prometheus.html)

Have a good day,

Dependabot couldn't find a Gopkg.toml for this project

Dependabot couldn't find a Gopkg.toml for this project.

Dependabot requires a Gopkg.toml to evaluate your project's current Go dependencies. It had expected to find one at the path: /Gopkg.toml.

If this isn't a Go project, or if it is a library, you may wish to disable updates for it in the .dependabot/config.yml file in this repo.

Don't vendor exposed API packaged - mux in particular

Please do not vendor the mux/logger package. This makes it very difficult to use your package. I can't pass in my own mux Router. I have to create a router using the package:

github.com/pivotal-cf/brokerapi/vendor/github.com/gorilla/mux

Or else it is not compatible. The mux Router is not an interface so it can't be just passed in.
Same goes for lager logger.

Actually it is worse. I just tried it and go won't let me use it. It gives an error that I can't use an internal vendor package. So I'm stuck.

Reusing brokerapi middlewares

I am working on a service broker that is supposed to do audit logging. In order for us to achieve that we need to use custom authentication middleware and later AttachRoutes to our router.

Since adding reusable code in pkg folder is commonly used but not considered a standard in Go, my question is, can we attach the brokerapi middlewares instead of implementing our own? Is this functionality supposed to be reused?

Support for Binding rotation

I would like to implement service binding rotation in my service broker while I cannot find the corresponding function in interface ServiceBroker. And for func Bind(), there's no predecessor_binding_id field in BindDetails, which is required for binding rotation according to the OSB specification. I'm wondering if there's any on-going task about this topic?

Inconsistent type for LastOperation

I'm very new to cloud foundry brokers and go so apologies if I'm missing something obvious but I seem to be running into a type inconsistency. I've been trying to implement the brokerapi interface and have the code

lastOperationResponse := brokerapi.LastOperationResponse{State: brokerapi.Failed)

This fails with the compiler error

cannot use state (type brokerapi.LastOperationState) as type string in assignment

This seems to be because brokerapi.Failed is a LastOperationState but the State field of the LastOperationResponse is a vanilla string. Am I missing something or should the State field of the LastOperationResponse struct be a LastOperationState?

Enrich request context with correlation id

Hello,

I have a use case where I need the correlation id from inbound requests to my broker so that a request can be uniquely traced through logs in case of an error. This would require enriching the request context to include the correlation id.

This could be implemented as a middleware attached to the router in the New function. So the correlation id, if present, will be added to the request context - it can be fetched from the request header. If it is not present it can be generated and then attached to the request context. And then in each handler it should be added to the logger e.g.:

logger := h.logger.Session(updateLogKey, lager.Data{
	instanceIDLogKey: instanceID,
	correlationIDKey: correlationID,
})

And then the broker implementation will have access to it from the request context so that all further outbound requests can be made with the same correlation id.

I'm willing to contribute this functionality if it is all right with you.

Dependabot couldn't find a Gopkg.toml for this project

Dependabot couldn't find a Gopkg.toml for this project.

Dependabot requires a Gopkg.toml to evaluate your project's current Go dependencies. It had expected to find one at the path: /Gopkg.toml.

If this isn't a Go project, or if it is a library, you may wish to disable updates for it in the .dependabot/config.yml file in this repo.

Semver and Releases

As of filing this, the latest release is v3.0.2, but the semver tags go up through v3.0.7. Should consumers of this package use the explicitly released version or the latest semver tag?

Missing metadata field in ProvisionedServiceSpec

Hi,

According to OSB API, we should be allowed to return the metadata field in a provisioning response.
Do you plan to update ProvisionedServiceSpec struct in the near future?

If no, then we can make a contribution, but to be honest, we really need this fast so my second question will be: there is a chance all processes from your side about the review and merge will be quick?

Proposal - Capture the X-Api-Info-Location header on all broker requests

What

Add infoLocation to HTTP context. This will allow to obtain the foundation details if required.

How

Code untested
Example of brokerapi/middlewares/api-info-location-header.go

const (
	infoLocationKey = "infoLocation"
)

func AddInfoLocationToContext(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
		infoLocation := req.Header.Get("X-Api-Info-Location")
		newCtx := context.WithValue(req.Context(), infoLocationKey, infoLocation)
		next.ServeHTTP(w, req.WithContext(newCtx))
	})
}

Update api to add MiddlewareFunc to the chain:

[...]
router.Use(middlewares.AddInfoLocationToContext)
[...]

http request context handling

The http request in golang has an attached context that is used to determine if client abandoned request. Some brokers make long running and/or retryable operations and could cancel them in the case that the client abandons the request.

Please pass along this information to the callbacks because it will make for a more complete/production ready SDK.

Audit log by the APIHandler type

Hello everyone,

We are looking into adding audit logging to our brokerapi-powered application. That is, we would like to have one log message each time someone uses our broker.
We are currently doing the following:

  1. adding a middleware that logs some properties of all incoming requests;
  2. adding a proxy broker that logs before calling its backend.

Both options are suboptimal:

  1. requires us to look for a combination of URL and method when querying our logs;
  2. shows calls to Services when Provision is called, for example.

An ideal option would be to have the handlers. APIHandler methods write a log message just before it calls any broker method. Those messages could have the INFO level.
How does that sound? We would be happy to provide a patch for that.

How to use brokerapi for multiple services

If I want to write a service broker for multiple services (e.g. mysql and redis), is it doable?

From my understanding, since the route /v2/service_instances/:instance_id doesn't contain service_id, so I can't define two brokers like this:

mysqlBrokerAPI := brokerapi.New(mysqlServiceBroker, brokerLogger, brokerCredentials)
http.Handle("/route-for-mysql", mysqlBrokerAPI)
redisBrokerAPI := brokerapi.New(redisServiceBroker, brokerLogger, brokerCredentials)
http.Handle("/route-for-redis", redisBrokerAPI)

It seems that I need to write different logics for different service in the Provision. Is there any guidance for multiple services?
Thanks.

Missing allow_context_updates field

According to OSB API Spec, the Service Offering Object contains an allow_context_updates which is missing in the current implementation. The flag was introduces in OSB API 2.15.

The PR which adds the field: #129

Requirement to expose additional API endpoints

Our Platform expects certain additional API endpoints (other than those in the Open Service Broker specs). For instance one of those endpoints would be v2/service_instances/{instance_id}/name which would return a service name that the Platform expects the Broker is provide.

With brokerapi.New returning an http.Handler we cannot configure additional routes. I was able to modify the brokerapi.New definition as below
func New(serviceBroker ServiceBroker, logger lager.Logger, brokerCredentials BrokerCredentials) *mux.Router
and then configure additional routes in my code like this
brokerAPI := brokerapi.New(ServiceBroker, logger, brokerCredential) brokerAPI.HandleFunc("/v2/service_instances/{instance_id}/name", ServiceBroker.GetInstanceName).Methods("GET") http.Handle("/", brokerAPI) logger.Fatal("http-listen", http.ListenAndServe(":8080", nil))

Would this update make sense? If there is another way of implementing this use case (without having to update the brokerapi codebase) could you please let me know?

Check for API version in Unbind operation

Hello all,

There is this check for API version > 2.14 for async unbind operations which fails the request when accepts_incomplete is true.

accepts_incomplete is not supported in versions <14, so I understand the point of the check.
However, in my use case, the broker is now deployed on K8S and the Service Catalog sends an async request with API version header 2.13. The service catalog uses the kubernetes-sigs/go-open-service-broker-client package and here we can see it uses the default configuration of the client lib, where the latest API version is 2.13.

I've also noticed that the check is not executed for deprovision, which is inconsistent.

What do you think about removing the check for now and rely more on the value passed as accepts_incomplete? If you agree, I can make the PR.

Incorrect type for metadata fields in Get/Provision/Deprovision responses

I noticed in this PR a new metadata field (with labels/attributes) was added as per latest OSB specification.

Is there any reason for creating labels and attributes as a map[string]string instead of interface{} as per OSB specification where metadata labels and attributes fields are object type like parameters field (or at the least making it map[string]interface{} so integers and structs can be used as values) ?

Current implementation limits proper usage of labels and attributes of metadata in ServiceBroker implementation. I can add PR changing labels and attributes to interface{} (or map[string]interface{}) if you agree.

Is the FailureResponse status code used?

I ask because I have an implementation like this:

func (f *serviceBroker) Services(ctx context.Context) ([]domain.Service, error) {
	return nil, apiresponses.NewFailureResponse(errors.New("TODO"), http.StatusNotImplemented, http.StatusText(http.StatusNotImplemented)) // TODO: Implement
}

but going to /v2/catalog gives me this http response:

$> curl -i -u ":" -H "X-Broker-API-Version: 2.13" http://localhost:8080/v2/catalog
HTTP/1.1 500 Internal Server Error
Content-Type: application/json
Correlation-Id: c2g7orea0bri9krepjs0
Date: Sun, 16 May 2021 01:59:09 GMT
Content-Length: 23

{"description":"TODO"}

Using v8 of this module.

Based on the docs here, I expected the response status code to be 501 instead.

Possibility to register a custom middle ware

Dear Brokerapi golang community,

I am in need to register my custom middleware with brokerapi in order to interpret the http request. My simple requirement is to be able to interpret http request and to log the client ip.

// middle ware to log client ip address
func LogClientInfoMiddleware(next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        // log origin ip, host url
        IPAddress := r.Header.Get("X-Real-Ip")
        log.Debug("Req: %s %s\n", IPAddress, r.RequestURI) 
        next.ServeHTTP(w, r)
    })
}

Right now the only way i see according to the documentation is to use brokerapi.AttachRoutes,

Another way with which it is possible is by using brokerapi.NewWithOptions and by using a custom Option variable. But i realized that there is no way i can construct Option type due the protected *config variable Option. However this method also allows us to define custom Router and to be passed as an Option, which is how i am planning to realize this requirement at the moment.

        brokerCredentials := brokerapi.BrokerCredentials{
		Username: configmgr.GetBrokerUsername(),
		Password: configmgr.GetBrokerPassword(),
	}
	router := mux.NewRouter()
	router.Use(middlewares.APIVersionMiddleware{LoggerFactory: laggerForBrokerApi}.ValidateAPIVersionHdr)
	router.Use(middlewares.AddCorrelationIDToContext)
	router.Use(middlewares.AddOriginatingIdentityToContext)
	router.Use(middlewares.AddInfoLocationToContext)
	router.Use(middlewares.AddRequestIdentityToContext)
	// my custom middle ware
	router.Use(logClientInfoMiddleware)
	brokerAPI := brokerapi.NewWithOptions(serviceBroker, laggerForBrokerApi, brokerapi.WithBrokerCredentials(brokerCredentials), brokerapi.WithRouter(router))
	http.Handle("/", brokerAPI)

But with both of the approaches one need to manually attach the default middlewares. And this is error prone especialliy when ever there is a new broker version released we need verify if there are any new middlewares that are added and add missing middlewares.

So i was wondering if the api gives any other ways to register my custom middleware. I would really appreciate any hints on this.

Otherwise, may be the api could also give a new function as described below to register a custom middle ware. Please see the below example :

// api_options.go

// returns custom handler wrapped inside Option, which can be used with brokerapi.NewWithOptions() method
func WithCustomOption(handler http.Handler) Option {
	return func(c *config) {
		c.router.Use(handler)
	}
}

I think such a method would also help other teams which have similar need.
Later in my code, i could use the above method to register my middle ware

brokerAPI := brokerapi.NewWithOptions(serviceBroker, laggerForBrokerApi, brokerCredentials, WithCustomOption(LogClientInfoMiddleware()))

It would be really great if the community could look in to this topic and share me your thoughts on this. Thank you!

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.