Skip to content

Commit

Permalink
feat!: swap gorilla/mux for go-chi/chi (#246)
Browse files Browse the repository at this point in the history
gorilla/mux has been archived as a project, and it's preferable to
depend on a maintained HTTP handler. go-chi/chi is the closest fit
for gorilla/mux and appears to be well used and maintained.

An additional breaking change is that go-chi/chi will ignore URL-encoded
slashes for routing purposes, so PUT /v2/service_instances/foo%2Fbar
will treat "foo%2Fbar" as an instance ID and will not try to route to
/v2/service_instances/foo/bar as the gorilla/mux version would have done
by default. Previously the WithEncodedPath() option was used to configure
gorilla/mux to treat "foo%2Fbar" as an instance ID, but as this is now
the default, WithEncodedPath() option has been removed. Treating
"foo%2Fbar" as foo/bar is not an option with go-chi/chi.
  • Loading branch information
blgm authored May 26, 2023
1 parent f41cdd2 commit 1d35eed
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 93 deletions.
15 changes: 8 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

A Go package for building [V2 Open Service Broker API](https://github.com/openservicebrokerapi/servicebroker/) compliant Service Brokers.

## [Docs](https://godoc.org/github.com/pivotal-cf/brokerapi)
## [Docs](https://godoc.org/github.com/pivotal-cf/brokerapi/v9)

## Dependencies

Expand All @@ -18,14 +18,15 @@ We appreciate and welcome open source contibution. We will try to review the cha
## Usage

`brokerapi` defines a
[`ServiceBroker`](https://godoc.org/github.com/pivotal-cf/brokerapi#ServiceBroker)
[`ServiceBroker`](https://godoc.org/github.com/pivotal-cf/brokerapi/v9#ServiceBroker)
interface. Pass an implementation of this to
[`brokerapi.New`](https://godoc.org/github.com/pivotal-cf/brokerapi#New), which
returns an `http.Handler` that you can use to serve handle HTTP requests.
[`brokerapi.New`](https://godoc.org/github.com/pivotal-cf/brokerapi/v9#New)
or [`brokerapi.NewWithOptions`](https://pkg.go.dev/github.com/pivotal-cf/brokerapi/v9#NewWithOptions),
which returns an `http.Handler` that you can use to serve handle HTTP requests.

Alternatively, if you already have a `*mux.Router` that you want to attach
Alternatively, if you already have a `*chi.Mux` that you want to attach
service broker routes to, you can use
[`brokerapi.AttachRoutes`](https://godoc.org/github.com/pivotal-cf/brokerapi#AttachRoutes).
[`brokerapi.AttachRoutes`](https://godoc.org/github.com/pivotal-cf/brokerapi/v9#AttachRoutes).
Note in this case, the Basic Authentication and Originating Identity middleware
will not be set up, so you will have to attach them manually if required.

Expand Down Expand Up @@ -55,7 +56,7 @@ values can be retrieved in a `brokerapi.ServiceBroker` implementation using
utility methods `RetrieveServiceFromContext` and `RetrieveServicePlanFromContext`
as shown below.

```golang
```go
func (sb *ServiceBrokerImplementation) Provision(ctx context.Context,
instanceID string, details brokerapi.ProvisionDetails, asyncAllowed bool) {

Expand Down
28 changes: 14 additions & 14 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"net/http"

"code.cloudfoundry.org/lager/v3"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
"github.com/pivotal-cf/brokerapi/v9/handlers"
)

Expand All @@ -32,27 +32,27 @@ func New(serviceBroker ServiceBroker, logger lager.Logger, brokerCredentials Bro
return NewWithOptions(serviceBroker, logger, WithBrokerCredentials(brokerCredentials))
}

func NewWithCustomAuth(serviceBroker ServiceBroker, logger lager.Logger, authMiddleware mux.MiddlewareFunc) http.Handler {
func NewWithCustomAuth(serviceBroker ServiceBroker, logger lager.Logger, authMiddleware middlewareFunc) http.Handler {
return NewWithOptions(serviceBroker, logger, WithCustomAuth(authMiddleware))
}

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

func attachRoutes(router *mux.Router, serviceBroker ServiceBroker, logger lager.Logger) {
func attachRoutes(router *chi.Mux, serviceBroker ServiceBroker, logger lager.Logger) {
apiHandler := handlers.NewApiHandler(serviceBroker, logger)
router.HandleFunc("/v2/catalog", apiHandler.Catalog).Methods("GET")
router.Get("/v2/catalog", apiHandler.Catalog)

router.HandleFunc("/v2/service_instances/{instance_id}", apiHandler.GetInstance).Methods("GET")
router.HandleFunc("/v2/service_instances/{instance_id}", apiHandler.Provision).Methods("PUT")
router.HandleFunc("/v2/service_instances/{instance_id}", apiHandler.Deprovision).Methods("DELETE")
router.HandleFunc("/v2/service_instances/{instance_id}/last_operation", apiHandler.LastOperation).Methods("GET")
router.HandleFunc("/v2/service_instances/{instance_id}", apiHandler.Update).Methods("PATCH")
router.Get("/v2/service_instances/{instance_id}", apiHandler.GetInstance)
router.Put("/v2/service_instances/{instance_id}", apiHandler.Provision)
router.Delete("/v2/service_instances/{instance_id}", apiHandler.Deprovision)
router.Get("/v2/service_instances/{instance_id}/last_operation", apiHandler.LastOperation)
router.Patch("/v2/service_instances/{instance_id}", apiHandler.Update)

router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.GetBinding).Methods("GET")
router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.Bind).Methods("PUT")
router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.Unbind).Methods("DELETE")
router.Get("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.GetBinding)
router.Put("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.Bind)
router.Delete("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.Unbind)

router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}/last_operation", apiHandler.LastBindingOperation).Methods("GET")
router.Get("/v2/service_instances/{instance_id}/service_bindings/{binding_id}/last_operation", apiHandler.LastBindingOperation)
}
23 changes: 15 additions & 8 deletions api_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ import (
"net/http"

"code.cloudfoundry.org/lager/v3"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
"github.com/pivotal-cf/brokerapi/v9/auth"
"github.com/pivotal-cf/brokerapi/v9/domain"
"github.com/pivotal-cf/brokerapi/v9/middlewares"
)

type middlewareFunc func(http.Handler) http.Handler

func NewWithOptions(serviceBroker domain.ServiceBroker, logger lager.Logger, opts ...Option) http.Handler {
cfg := newDefaultConfig(logger)
WithOptions(append(opts, withDefaultMiddleware())...)(cfg)
Expand All @@ -35,7 +37,7 @@ func NewWithOptions(serviceBroker domain.ServiceBroker, logger lager.Logger, opt

type Option func(*config)

func WithRouter(router *mux.Router) Option {
func WithRouter(router *chi.Mux) Option {
return func(c *config) {
c.router = router
c.customRouter = true
Expand All @@ -48,16 +50,21 @@ func WithBrokerCredentials(brokerCredentials BrokerCredentials) Option {
}
}

func WithCustomAuth(authMiddleware mux.MiddlewareFunc) Option {
func WithCustomAuth(authMiddleware middlewareFunc) Option {
return func(c *config) {
c.router.Use(authMiddleware)
}
}

// WithEncodedPath used to opt in to a gorilla/mux behaviour that would treat encoded
// slashes "/" as IDs. For example, it would change `PUT /v2/service_instances/foo%2Fbar`
// to treat `foo%2Fbar` as an instance ID, while the default behavior was to treat it
// as `foo/bar`. However, with moving to go-chi/chi, this is now the default behavior
// so this option no longer does anything.
//
// Deprecated: no longer has any effect
func WithEncodedPath() Option {
return func(c *config) {
c.router.UseEncodedPath()
}
return func(*config) {}
}

func withDefaultMiddleware() Option {
Expand All @@ -82,14 +89,14 @@ func WithOptions(opts ...Option) Option {

func newDefaultConfig(logger lager.Logger) *config {
return &config{
router: mux.NewRouter(),
router: chi.NewRouter(),
customRouter: false,
logger: logger,
}
}

type config struct {
router *mux.Router
router *chi.Mux
customRouter bool
logger lager.Logger
}
25 changes: 8 additions & 17 deletions api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"code.cloudfoundry.org/lager/v3"
"code.cloudfoundry.org/lager/v3/lagertest"
"github.com/drewolson/testflight"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/pivotal-cf/brokerapi/v9"
Expand Down Expand Up @@ -2736,13 +2736,13 @@ var _ = Describe("Service Broker API", func() {

Describe("WithRouter()", func() {
It("can take a supplied router", func() {
router := mux.NewRouter()
router := chi.NewRouter()
brokerAPI = brokerapi.NewWithOptions(fakeServiceBroker, brokerLogger, brokerapi.WithRouter(router))
Expect(router).To(Equal(brokerAPI))
})

It("does not attach middleware to the router", func() {
brokerAPI = brokerapi.NewWithOptions(fakeServiceBroker, brokerLogger, brokerapi.WithRouter(mux.NewRouter()))
brokerAPI = brokerapi.NewWithOptions(fakeServiceBroker, brokerLogger, brokerapi.WithRouter(chi.NewRouter()))
apiVersion = "1.14" // Wrong version

instanceID := uniqueInstanceID()
Expand All @@ -2752,21 +2752,12 @@ var _ = Describe("Service Broker API", func() {
})
})

Describe("WithEncodedPath()", func() {
It("will accept URL-encoded paths", func() {
const encodedInstanceID = "foo%2Fbar"

It("does not accept URL-encoded paths by default", func() {
brokerAPI = brokerapi.NewWithOptions(fakeServiceBroker, brokerLogger, brokerapi.WithBrokerCredentials(credentials))
response := makeInstanceProvisioningRequest(encodedInstanceID, provisionDetails, "")
Expect(response.RawResponse).To(HaveHTTPStatus(http.StatusNotFound))
})

It("will accept URL-encoded paths when configured", func() {
brokerAPI = brokerapi.NewWithOptions(fakeServiceBroker, brokerLogger, brokerapi.WithEncodedPath(), brokerapi.WithBrokerCredentials(credentials))
response := makeInstanceProvisioningRequest(encodedInstanceID, provisionDetails, "")
Expect(response.RawResponse).To(HaveHTTPStatus(http.StatusCreated))
Expect(fakeServiceBroker.ProvisionedInstances).To(HaveKey(encodedInstanceID))
})
brokerAPI = brokerapi.NewWithOptions(fakeServiceBroker, brokerLogger, brokerapi.WithBrokerCredentials(credentials))
response := makeInstanceProvisioningRequest(encodedInstanceID, provisionDetails, "")
Expect(response.RawResponse).To(HaveHTTPStatus(http.StatusCreated))
Expect(fakeServiceBroker.ProvisionedInstances).To(HaveKey(encodedInstanceID))
})
})
})
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.19
require (
code.cloudfoundry.org/lager/v3 v3.0.1
github.com/drewolson/testflight v1.0.0
github.com/gorilla/mux v1.8.0
github.com/go-chi/chi/v5 v5.0.8
github.com/maxbrunsfeld/counterfeiter/v6 v6.6.1
github.com/onsi/ginkgo/v2 v2.9.5
github.com/onsi/gomega v1.27.7
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/drewolson/testflight v1.0.0 h1:jgA0pHcFIPnXoBmyFzrdoR2ka4UvReMDsjYc7Jcvl80=
github.com/drewolson/testflight v1.0.0/go.mod h1:t9oKuuEohRGLb80SWX+uxJHuhX98B7HnojqtW+Ryq30=
github.com/go-chi/chi/v5 v5.0.8 h1:lD+NLqFcAi1ovnVZpsnObHGW4xb4J8lNmoYVfECH1Y0=
github.com/go-chi/chi/v5 v5.0.8/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
github.com/go-logr/logr v1.2.4 h1:g01GSCwiDw2xSZfjJ2/T9M+S6pFdcNtFYsp+Y43HYDQ=
github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI=
Expand All @@ -25,8 +27,6 @@ github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 h1:yAJXTCF9TqKcTiHJAE
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
github.com/google/uuid v1.0.0 h1:b4Gk+7WdP/d3HZH8EJsZpvV7EtDOgaZLtnaNGIu1adA=
github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI=
github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/kr/pretty v0.0.0-20160823170715-cfb55aafdaf3 h1:dhwb1Ev84SKKVBfLuhR4bw/29yYHzwtTyTLUWWnvYxI=
github.com/kr/pretty v0.0.0-20160823170715-cfb55aafdaf3/go.mod h1:Bvhd+E3laJ0AVkG0c9rmtZcnhV0HQ3+c3YxxqTvc/gA=
Expand Down
7 changes: 3 additions & 4 deletions handlers/bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"net/http"

"code.cloudfoundry.org/lager/v3"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
"github.com/pivotal-cf/brokerapi/v9/domain"
"github.com/pivotal-cf/brokerapi/v9/domain/apiresponses"
"github.com/pivotal-cf/brokerapi/v9/middlewares"
Expand All @@ -19,9 +19,8 @@ const (
)

func (h APIHandler) Bind(w http.ResponseWriter, req *http.Request) {
vars := mux.Vars(req)
instanceID := vars["instance_id"]
bindingID := vars["binding_id"]
instanceID := chi.URLParam(req, "instance_id")
bindingID := chi.URLParam(req, "binding_id")

logger := h.logger.Session(bindLogKey, lager.Data{
instanceIDLogKey: instanceID,
Expand Down
5 changes: 2 additions & 3 deletions handlers/deprovision.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"net/http"

"code.cloudfoundry.org/lager/v3"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
"github.com/pivotal-cf/brokerapi/v9/domain"
"github.com/pivotal-cf/brokerapi/v9/domain/apiresponses"
"github.com/pivotal-cf/brokerapi/v9/middlewares"
Expand All @@ -15,8 +15,7 @@ import (
const deprovisionLogKey = "deprovision"

func (h APIHandler) Deprovision(w http.ResponseWriter, req *http.Request) {
vars := mux.Vars(req)
instanceID := vars["instance_id"]
instanceID := chi.URLParam(req, "instance_id")

logger := h.logger.Session(deprovisionLogKey, lager.Data{
instanceIDLogKey: instanceID,
Expand Down
7 changes: 3 additions & 4 deletions handlers/get_binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"net/http"

"code.cloudfoundry.org/lager/v3"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
"github.com/pivotal-cf/brokerapi/v9/domain"
"github.com/pivotal-cf/brokerapi/v9/domain/apiresponses"
"github.com/pivotal-cf/brokerapi/v9/middlewares"
Expand All @@ -16,9 +16,8 @@ import (
const getBindLogKey = "getBinding"

func (h APIHandler) GetBinding(w http.ResponseWriter, req *http.Request) {
vars := mux.Vars(req)
instanceID := vars["instance_id"]
bindingID := vars["binding_id"]
instanceID := chi.URLParam(req, "instance_id")
bindingID := chi.URLParam(req, "binding_id")

logger := h.logger.Session(getBindLogKey, lager.Data{
instanceIDLogKey: instanceID,
Expand Down
8 changes: 3 additions & 5 deletions handlers/get_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ import (
"fmt"
"net/http"

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

"code.cloudfoundry.org/lager/v3"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
"github.com/pivotal-cf/brokerapi/v9/domain"
"github.com/pivotal-cf/brokerapi/v9/domain/apiresponses"
"github.com/pivotal-cf/brokerapi/v9/middlewares"
"github.com/pivotal-cf/brokerapi/v9/utils"
Expand All @@ -17,8 +16,7 @@ import (
const getInstanceLogKey = "getInstance"

func (h APIHandler) GetInstance(w http.ResponseWriter, req *http.Request) {
vars := mux.Vars(req)
instanceID := vars["instance_id"]
instanceID := chi.URLParam(req, "instance_id")

logger := h.logger.Session(getInstanceLogKey, lager.Data{
instanceIDLogKey: instanceID,
Expand Down
7 changes: 3 additions & 4 deletions handlers/last_binding_operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"net/http"

"code.cloudfoundry.org/lager/v3"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
"github.com/pivotal-cf/brokerapi/v9/domain"
"github.com/pivotal-cf/brokerapi/v9/domain/apiresponses"
"github.com/pivotal-cf/brokerapi/v9/middlewares"
Expand All @@ -16,9 +16,8 @@ import (
const lastBindingOperationLogKey = "lastBindingOperation"

func (h APIHandler) LastBindingOperation(w http.ResponseWriter, req *http.Request) {
vars := mux.Vars(req)
instanceID := vars["instance_id"]
bindingID := vars["binding_id"]
instanceID := chi.URLParam(req, "instance_id")
bindingID := chi.URLParam(req, "binding_id")
pollDetails := domain.PollDetails{
PlanID: req.FormValue("plan_id"),
ServiceID: req.FormValue("service_id"),
Expand Down
22 changes: 11 additions & 11 deletions handlers/last_binding_operation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,16 @@ import (
"net/http"
"net/url"

"github.com/pivotal-cf/brokerapi/v9/middlewares"

"code.cloudfoundry.org/lager/v3"
"github.com/gorilla/mux"
"github.com/go-chi/chi/v5"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/pivotal-cf/brokerapi/v9/domain"
"github.com/pivotal-cf/brokerapi/v9/domain/apiresponses"
brokerFakes "github.com/pivotal-cf/brokerapi/v9/fakes"
"github.com/pivotal-cf/brokerapi/v9/handlers"
"github.com/pivotal-cf/brokerapi/v9/handlers/fakes"
"github.com/pivotal-cf/brokerapi/v9/middlewares"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -129,17 +128,18 @@ func newRequest(instanceID, bindingID, planID, serviceID, operation string) *htt
Expect(err).ToNot(HaveOccurred())
request.Header.Add("X-Broker-API-Version", "2.14")

request = mux.SetURLVars(request, map[string]string{
"instance_id": instanceID,
"binding_id": bindingID,
})

request.Form = url.Values{}
request.Form.Add("plan_id", planID)
request.Form.Add("service_id", serviceID)
request.Form.Add("operation", operation)

newCtx := context.WithValue(request.Context(), middlewares.CorrelationIDKey, "fake-correlation-id")
request = request.WithContext(newCtx)
return request
rc := chi.NewRouteContext()
rc.URLParams.Add("instance_id", instanceID)
rc.URLParams.Add("binding_id", bindingID)

ctx := request.Context()
ctx = context.WithValue(ctx, chi.RouteCtxKey, rc)
ctx = context.WithValue(ctx, middlewares.CorrelationIDKey, "fake-correlation-id")

return request.WithContext(ctx)
}
Loading

0 comments on commit 1d35eed

Please sign in to comment.