soheilhy / cmux

Connection multiplexer for GoLang: serve different services on the same port!
Apache License 2.0
2.53k stars 197 forks source link

Notice: grpc-go will stop sending RPCs before HTTP/2 SETTINGS frame is received #64

Open dfawley opened 5 years ago

dfawley commented 5 years ago

This is a notice that grpc-go intends to change in a way that we know will break the way cmux currently works by default. This will bring grpc-go in line with grpc-java's behavior, and C/wrapped languages will be following suit as well. Details and justification for the change can be found in https://github.com/grpc/grpc/pull/17006. grpc-go's migration plan is proposed in https://github.com/grpc/grpc-go/issues/2406. Please feel free to comment in the appropriate PR/issue for questions or concerns about this. Apologies in advance for the breaking change.

Iulian7 commented 5 years ago

Any ideas how to match grpc-go requests then?

dfawley commented 5 years ago

@lulian7 - IIUC you should be able to use the workaround suggested here for Java gRPC clients: https://github.com/soheilhy/cmux#limitations

Iulian7 commented 5 years ago

@dfawley Works. I missed that.

johanbrandhorst commented 5 years ago

Testing with gRPC-Go 1.19.1 I can't get this to work even with

grpcL := mux.MatchWithWriters(cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"))

Has anything changed in 1.19 to make this even harder? I tried debugging by sending non-matched entries to another listener and printing the output and this is what I got:

���c+��]
80�l

or as a byte slice:

[22 3 1 0 217 1 0 0 213 3 3 86 90 234 191 121 47 47 94 124 149 27 35 62]

I'm not sure what format this is. Any ideas? @menghanl @dfawley

menghanl commented 5 years ago

@johanbrandhorst No, this behavior didn't change in 1.19. I also don't recognize the bytes...

Does it work with 1.18? Is the client a simple client?

johanbrandhorst commented 5 years ago

I haven't tried 1.18 yet, but yes it is a simple grpc-go client. I'll try 1.18 and below.

johanbrandhorst commented 5 years ago

Hm, everything is working as expected when serving gRPC without TLS. I guess this is a false alarm. What's the best way to use cmux and still encrypt the connection?

johanbrandhorst commented 5 years ago

For future reference;

I got the following working:

  1. Use tls.Listen to create the listener.
  2. Use (*grpc.Server).Serve without grpc.Creds.
  3. Use grpc.WithTransportCredentials as usual in the client.
glerchundi commented 5 years ago

@johanbrandhorst I finally took some time to sketch a code snippet to make our use case work. It did, but I don't know if it's going to work after reading this commit (as we didn't upgraded to that grpc-go version yet). I want to expose grpc-gateway + pure grpc in the same port without security; It's a microservice never exposed directly to the outside so it's ok. But I don't want to commit to a solution that cannot be extended in the future by adopting an approach like BeyondCorp. This means that our microservice should be able to be exposed publicly by, obviously, using TLS. The mentioned code allows addition TLS over the whole cmux but I don't know if you encountered further problems with this approach?

johanbrandhorst commented 5 years ago

There should be no reason you can't do TLS termination before any packets reach the service.

glerchundi commented 4 years ago

Correct, thanks for confirming. Although we found another issue with the combo grpc-gateway, grpc, port-sharing, cmux and persistent connections kept by a reverse proxy (envoy in this case). It seems that cmux is not very useful if we're behind a proxy which keeps persistent connection without doing any differentiation between the underlying protocol being used.

Take this sequence as an example: As stated by cmux, it matches the connection when it's accepted, so it's possible this to happen:

  1. Someone makes a gRPC call
  2. Envoy creates a new http connection
  3. cmux sees it's a http2 connection and the content-type is gRPC it then creates a new connection against grpc.Serve
  4. grpc.Serve replies to the gRPC request succesfully. While Envoy decides not to close the fresh connection for performance reasons
  5. Someone makes a REST call
  6. Envoy decides to use the same connection created before
  7. As the connection was previously matched grpc.Serve is called again and it fails as it's not a gRPC call

WDYT @johanbrandhorst?

PS.: I don't really know which is the right forum for this question as there are lots of little components taking part in the overall problem...

johanbrandhorst commented 4 years ago

This is probably not the place, no. Maybe raise an issue against Envoy?

glerchundi commented 4 years ago

This is probably not the place, no. Maybe raise an issue against Envoy?

Already asked in Envoy and it seems like nothing could be done there as it keeps one and only connection pool for everything no matter it's plain or gRPC.

UPDATE: It can be done but cannot because we're using Contour to manage Envoy.

@mpuncel wrote:

Envoy uses the same connection pool for gRPC and non-gRPC. You could probably accomplish what you want with 2 different clusters and set up a routing rule that matches on gRPC to go to 1 and everything else goes to the other

johanbrandhorst commented 4 years ago

There's a question to be asked about why you're using cmux behind a proxy like envoy anyway. Envoy can do the traffic splitting for you, so why not just remove the cmux?

glerchundi commented 4 years ago

This is probably not the place, no. Maybe raise an issue against Envoy?

There are several reasons:

Did you have this problem previously @johanbrandhorst? How do you currently operate services that publish both gRPC + REST in your environment?

Thanks!

johanbrandhorst commented 4 years ago

I think we can move this discussion to the #grpc channel on Gophers slack. We've taken up this issue for long enough.

ghost commented 4 years ago
TLS: net/http uses a type assertion to identify TLS connections; since cmux's lookahead-implementing connection wraps the underlying TLS connection, this type assertion fails. Because of that, you can serve HTTPS using cmux but http.Request.TLS would not be set in your handlers.

do I understand this correctly if I assume that I can still serve http2 and http1(grpc server+ grpc gateway server) via cmux on the same port and have TLS for the clients and I only have to enable tls on the listener that I provide to cmux and disable tls for the servers the cmux is routing to? ie. I still can serve secure grpc and grpc gateway via cmux since both servers are linked to the cmux only via memory and not network, hence no security issue with tls "termination" on cmux level?

johanbrandhorst commented 4 years ago

That is correct, TLS is terminated by cmux, but it's still in-memory, so it's about the same level of security.

delwaterman commented 2 years ago

Ran into this issue. Can we at least update the documentation for this?