grpc / proposal

A repository for gRFCs
Apache License 2.0
712 stars 231 forks source link

A81: xDS Authority Rewriting #435

Closed markdroth closed 1 month ago

atollena commented 4 months ago

A few comments:

markdroth commented 4 months ago

Thanks for those comments!

  • I think it would help to provide a very brief explanation of when this feature is expected to be useful, if only for gRPC maintainers' own culture. As far as I know it is necessary when the server is using the authority header to make decisions. This is common in reverse proxies where each deployment serve multiple domains. Servers can then take routing decisions based on it (forward the request to different backends). I think it is fairly uncommon to make use of the authority header for direct gRPC client -> gRPC servers, but I could be wrong on this.

I've added a sentence about this.

  • It would help to specify whether you expect implementations to use the rewritten authority for the authentication handshake so that servers can provide different TLS certificates based on the requested domain (SNI), rather than just for the authority header. I think Envoy supports this, although I'm not sure how this is configured.

This feature is purely about rewriting the authority header on data plane RPCs, which happens independently of the TLS handshake. The same is true in Envoy. I've added a sentence about this.

(Both Envoy and gRPC have other features that allow controlling the TLS handshake, but that's unrelated to this gRFC.)

Good question! We should figure out the right precedence rules here.

All languages have an option similar to the one you mentioned for setting the default authority for the channel as a whole. C++ also has options to set the authority on a per-call basis, although I don't think other languages have that.

For the channel-level option, my inclination is that the xDS config should take precedence over the channel-level default, especially since there will be a knob in the bootstrap file to disable that if it's not desirable for some reason.

For the C++ option to set the authority on a per-RPC basis, I think that the principled answer is that that API should take precedence over the xDS config. However, in practice, I suspect no one will ever try to use these two options together, and it's less work to allow the xDS config to take precedence, so I'm leaning in that direction.

@ejona86 @dfawley @ctiller Thoughts on that?

dfawley commented 4 months ago

For the channel-level option, my inclination is that the xDS config should take precedence over the channel-level default, especially since there will be a knob in the bootstrap file to disable that if it's not desirable for some reason.

For the C++ option to set the authority on a per-RPC basis, I think that the principled answer is that that API should take precedence over the xDS config. However, in practice, I suspect no one will ever try to use these two options together, and it's less work to allow the xDS config to take precedence, so I'm leaning in that direction.

That all SGTM.

markdroth commented 3 months ago

For the C++ option to set the authority on a per-RPC basis, I think that the principled answer is that that API should take precedence over the xDS config. However, in practice, I suspect no one will ever try to use these two options together, and it's less work to allow the xDS config to take precedence, so I'm leaning in that direction.

It turns out that the easy thing here is actually the principled answer: if the application explicitly sets the authority on a per-RPC basis, that will take precedence over the xDS config.