Skip to content

Commit

Permalink
automate-gateway: cleanup GRPC/HTTP multiplexing cruft (#402)
Browse files Browse the repository at this point in the history
* automate-gateway: cleanup GRPC/HTTP multiplexing cruft

This was carried over from some ancient grpc-gateway example code[1]; and
we're not exposing the GRPC API that way right now.

Using (*grpc.Server).ServeHTTP has some issues:

1. it's known to be slow, very slow[2]
2. it's seems to be not working with go-grpc >= 1.19 [3]

Furthermore, we're exposing the proper GRPC server (the native go-grpc
one) on a different port. If decide to (finally!) expose our GRPC API,
that's the thing we should expose.

So, since we don't need this multiplexing business, we shouldn't keep
the code around. This is the cleanup.

[1]: https://github.com/philips/grpc-gateway-example/
[2]: grpc/grpc-go#586
[3]: soheilhy/cmux#64 (comment)

* [integration] automate-gateway: fix tests to use GRPC port

So apparently these HAD used the multiplexing code I've deleted. Well,
they don't have to. Changed the hardcoded port to automate-gateway's
default grpc port.

Signed-off-by: Stephan Renatus <srenatus@chef.io>
  • Loading branch information
srenatus committed May 22, 2019
1 parent 468a0e1 commit 7a5a2ec
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 19 deletions.
18 changes: 1 addition & 17 deletions components/automate-gateway/gateway/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"net"
"net/http"
"net/url"
"strings"

grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
Expand Down Expand Up @@ -254,19 +253,6 @@ func WithRouteFeatureToggles(c *Config) Opts {
}
}

// handle GRPC returns
func (s *Server) grpcHandlerFunc(grpcServer *grpc.Server, muxHandler http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// partial check of https://github.com/grpc/grpc-go/blob/master/transport/handler_server.go#L50
if r.ProtoMajor >= 2 && strings.Contains(r.Header.Get("Content-Type"), "application/grpc") {
// TODO: proxy to s.grpcURI instead
grpcServer.ServeHTTP(w, r)
} else {
muxHandler.ServeHTTP(w, r)
}
})
}

// NewGRPCServer returns a *grpc.Server instance
func (s *Server) NewGRPCServer() (*grpc.Server, error) {
authClient, err := s.clientsFactory.AuthenticationClient()
Expand Down Expand Up @@ -452,13 +438,11 @@ func (s *Server) Serve() error {
// Register Prometheus metrics handler.
mux.Handle("/metrics", promhttp.Handler())

handler := s.grpcHandlerFunc(grpcServer, mux)

// start server
uri := fmt.Sprintf("%s:%d", s.httpListenHost, s.httpListenPort)
srv := &http.Server{
Addr: uri,
Handler: handler,
Handler: mux,
TLSConfig: &tls.Config{
Certificates: []tls.Certificate{*s.serviceKeyPair},
NextProtos: []string{"h2"},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
const (
complianceEndpoint = "127.0.0.1:10121"
secretsEndpoint = "127.0.0.1:10131"
gatewayEndpoint = "127.0.0.1:2000"
gatewayEndpoint = "127.0.0.1:2001"
nodesEndpoint = "127.0.0.1:10120"
)

Expand Down
2 changes: 1 addition & 1 deletion components/automate-gateway/integration/nodes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var host = os.Getenv("AUTOMATE_ACCEPTANCE_TARGET_HOST")
func TestGatewayNodesClient(t *testing.T) {
complianceEndpoint := "127.0.0.1:10121"
secretsEndpoint := "127.0.0.1:10131"
gatewayEndpoint := "127.0.0.1:2000"
gatewayEndpoint := "127.0.0.1:2001"
nodesEndpoint := "127.0.0.1:10120"
ctx := context.Background()
connFactory := helpers.SecureConnFactoryHabWithDeploymentServiceCerts()
Expand Down

0 comments on commit 7a5a2ec

Please sign in to comment.