feat(xds): per-stream call-credentials hook for the xDS transport#2705
feat(xds): per-stream call-credentials hook for the xDS transport#2705choonkijang wants to merge 3 commits into
Conversation
Per grpc#2444, credential-gated xDS control planes (e.g. GCP Traffic Director / `google_default`) require two things the transport did not provide: per-stream call credentials on the ADS stream, and a secure channel to carry them. Their bootstrap `server_uri` is often scheme-less (e.g. `trafficdirector.googleapis.com:443`), which parses with no scheme — so `Endpoint` has no signal to negotiate TLS, the ADS connection never establishes a secure session, and routing times out with "route config not yet available". xds-client: - Add a minimal `CallCredentials` trait mirroring grpc-go's `PerRPCCredentials`. The tonic transport attaches it on each (re)connect, only over a secure channel. - Construction is builder-based: `TonicTransportBuilder::with_call_credentials` and `with_channel(Channel, secure)` for a pre-built channel. - On the secure path, prepend `https://` to `server_uri` in `TonicTransportBuilder::build` when it's missing the scheme. tonic-xds: - Promote `google_default` to a first-class `ChannelCredentialType::GoogleDefault` (gRFC A27). - Add `XdsChannelConfig::with_call_credentials(...)`, so that applications can give a token source without relying on `grpc` or `grpc-google` crates at this time. examples: - Add the `channel_with_google_default` tonic-xds example (reusing the testutil greeter), demonstrating two ADC token sources. BREAKING CHANGE: * Relocate `TonicTransport::from_channel` to `TonicTransportBuilder::with_channel`, and * Adds a variant to the `Error` enum
|
|
|
@YutaoMa PTAL! |
| #[error("stream error: {0}")] | ||
| Stream(#[from] tonic::Status), | ||
|
|
||
| /// Call credentials failed, or require a secure transport. |
There was a problem hiding this comment.
Let's remove the feature gating on this error variant, non-additive features in Rust can be a footgun: implicit feature enablement with other dependencies can break compilation.
While we are at it, let's also add #[non_exhaustive] to the Error, it's a miss on my end to not mark it so previously. That'll help with future extensions like this.
There was a problem hiding this comment.
Done. Dropped the #[cfg(feature = "transport-tonic")] on CallCredentials and added #[non_exhaustive] to Error. See 447e849.
| /// Generates the authentication metadata for a specific call. | ||
| async fn get_request_metadata( | ||
| &self, | ||
| metadata: &mut tonic::metadata::MetadataMap, |
There was a problem hiding this comment.
Is there any specific reason to prefer out-param style API? I'd recommend keeping this simple and idiomatic by returning an owned map.
There was a problem hiding this comment.
Ah saw the grpc link comment above, seems like it's to keep the style consistent with grpc. In that case yeah it's good to keep it as is.
There was a problem hiding this comment.
Yeah, thanks for confirmation!
| /// | ||
| /// Attached on each (re)connect, only when the channel is secure. | ||
| #[tonic::async_trait] | ||
| pub trait CallCredentials: Send + Sync + std::fmt::Debug + 'static { |
There was a problem hiding this comment.
Could you re-export this type in tonic-xds so user of tonic-xds doesn't need to explicitly depend on xds-client to use this feature? (xds-client is intended to stay as a low-level ADS transport library so gRPC service developers don't have to depend on it explicitly). Also I suggest naming it TonicCallCredentials here: once we support grpc transport in xds-client, similar API will need to be added for GrpcCallCredentials, keeping the name distinguishable helps with future extensions.
choonkijang
force-pushed
the
feat-xds-call-credentials
branch
from
bed4264 to
47030ae
Compare
|
@YutaoMa PTAL! |
- Added #[non_exhaustive] to pub enum Error, and remove #[cfg(feature = "transport-tonic")] from `CallCredentials` error entry. - Rename `CallCredentials` as `TonicCallCredentials`, and let `tonic-xds` re-export it to public.
choonkijang
force-pushed
the
feat-xds-call-credentials
branch
from
47030ae to
447e849
Compare
2 participants
Motivation
Per #2702, credential-gated xDS control planes (e.g. GCP Traffic Director /
google_default) require two things the transport did not provide: per-stream call credentials on the ADS stream, and a secure channel to carry them. Their bootstrapserver_uriis often scheme-less (e.g.trafficdirector.googleapis.com:443), which parses with no scheme — soEndpointhas no signal to negotiate TLS, the ADS connection never establishes a secure session, and routing times out with "route config not yet available".Solution
xds-client:
TonicCallCredentialstrait mirroring grpc-go'sPerRPCCredentials. The tonic transport attaches it on each (re)connect, only over a secure channel.TonicTransportBuilder::with_call_credentialsandwith_channel(Channel, secure)for a pre-built channel.https://toserver_uriinTonicTransportBuilder::buildwhen it's missing the scheme.tonic-xds:
google_defaultto a first-classChannelCredentialType::GoogleDefault(gRFC A27).XdsChannelConfig::with_call_credentials(...), so that applications can give a token source without relying ongrpcorgrpc-googlecrates at this time.examples:
channel_with_google_defaulttonic-xds example (reusing the testutil greeter), demonstrating two ADC token sources.BREAKING CHANGE:
TonicTransport::from_channel(useTonicTransportBuilder::with_channel), andErrorenum