Possibility to register a custom middle ware

[vish-c]

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!

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/186199838

The labels on this github issue will be updated when the story is started.

[blgm]

Hi @vish-c. It's deliberate that the config type used by options functions is private as it allows it to be refactored in the future without breaking changes. The interface into it is meant to be the options functions. I can see a couple of way to solve this:

1. We could rename withDefaultMiddleware() to WithDefaultMiddleware() so that it becomes public and you can use it with the NewRouter() simplifying the above code:

        brokerCredentials := brokerapi.BrokerCredentials{
		Username: configmgr.GetBrokerUsername(),
		Password: configmgr.GetBrokerPassword(),
	}
	router := mux.NewRouter()
	// my custom middle ware
	router.Use(logClientInfoMiddleware)
	brokerAPI := brokerapi.NewWithOptions(serviceBroker, laggerForBrokerApi, brokerapi.WithBrokerCredentials(brokerCredentials), brokerapi.WithRouter(router), brokerapi.WithDefaultMiddleware())
	http.Handle("/", brokerAPI)

2. Or alternatively we could implement a WithAdditionalMiddleware() option:

        brokerCredentials := brokerapi.BrokerCredentials{
		Username: configmgr.GetBrokerUsername(),
		Password: configmgr.GetBrokerPassword(),
	}
	brokerAPI := brokerapi.NewWithOptions(serviceBroker, laggerForBrokerApi, brokerapi.WithBrokerCredentials(brokerCredentials), brokerapi.WithAdditionalMiddleware(logClientInfoMiddleware))
	http.Handle("/", brokerAPI)

They are not mutually exclusive. (1) is the simplest to implement. But (2) is not hard. Do you have a preference?

[pivotal-marcela-campo]

@blgm @vish-c I believe option 2 makes more sense. Specially because option 1 doesn't just imply making that method public as its implementation basically does nothing on a custom router, so it would still require more changes than just capitalising the method name.

[vish-c]

Hello @blgm and @pivotal-marcela-campo

I am extremely sorry for a very delayed response. This notification is simply lost in the flooded spams i got in my mail box and this combined with my long vacation caused this delay.

As @pivotal-marcela-campo suggested, my preference also would be Option 2) WithAdditionalMiddleware() option.

I would greatly appreciated if the community could implement and rollout this change.

Thanks.
Vish

[blgm]

I've created PR #291 which I think solves this. Feedback welcome.

I noticed that the WithCustomAuth() option almost does what is wanted except:

• the name is rather specific (using it for non-auth middleware could be confusing)
• it adds the middleware before the default middleware

So the PR adds a new WithAdditionalMiddleware() option that adds middleware after the default middleware.