search

fullstorydev / grpcurl

Like cURL, but for gRPC: Command-line tool for interacting with gRPC servers
MIT License
10.35k stars 497 forks source link

Enable xDS credentials #424

Closed PapaCharlie closed 4 months ago

PapaCharlie commented 8 months ago

This change should be relatively straightforward. It is a noop outside of the context of xDS (as demonstrated by the fact that the tests all pass), but it enables xDS-provided certificates (i.e. the ones that would be provided/specified in GRPC_XDS_BOOTSTRAP). See proposal A29 for additional detail.

dragonsinth commented 5 months ago

This probably needs a rebase. Are you sure that the XDS creds should wrap the errSignalingCreds and not the other way around? The code flow is a little tricky to absorb.

PapaCharlie commented 5 months ago

Let me check

PapaCharlie commented 5 months ago

I think you're right, it's likely best to wrap the final creds with the errSignalingCreds.

PapaCharlie commented 5 months ago

Yeah I confirmed that this works! Since errSignalingCreds embeds the credentials.TransportCredentials, it still implements the UsesXDS interface that's required by the code.

dragonsinth commented 5 months ago

Yeah I confirmed that this works! Since errSignalingCreds embeds the credentials.TransportCredentials, it still implements the UsesXDS interface that's required by the code.

Waaaait a second. That's definitely not true.

dragonsinth commented 5 months ago

I had to open another PR to get CI to work, for some reason it's getting stuck on this one. https://github.com/fullstorydev/grpcurl/pull/441

WDYT?

PapaCharlie commented 5 months ago

Yeah I confirmed that this works! Since errSignalingCreds embeds the credentials.TransportCredentials, it still implements the UsesXDS interface that's required by the code.

Waaaait a second. That's definitely not true.

Why not? This prints true: https://go.dev/play/p/TxQSrKPngtc?v=

dragonsinth commented 5 months ago

Why not? This prints true: https://go.dev/play/p/TxQSrKPngtc?v=

That one works because you embedded the concrete fooBar, which implements both interfaces. If you change it to this, it doesn't work:

https://go.dev/play/p/OyIwdHFyJ3x

(I had to rename the interfaces so that the interface names and method names dont clash.)

What we're trying to do here is more similar to this example.