Papa charlie/master #441
Papa charlie/master #441
Conversation
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](https://github.com/grpc/proposal/blob/master/A29-xds-tls-security.md#go) for additional detail.
grpcurl.go
Outdated
|
|
||
| func (c *errSignalingCreds) UsesXDS() bool { | ||
| xc, ok := c.TransportCredentials.(interface{ UsesXDS() bool }) | ||
| return ok && xc.UsesXDS() | ||
| } |
There was a problem hiding this comment.
I don't think this is necessary. I checked through it with a debugger, errSignalingCreds will implement UsesXDS as long as the TransportCredentials does
There was a problem hiding this comment.
If you remove ths thunk, TestXdsWrapping will fail.
There was a problem hiding this comment.
Nevermind, you are totally right, you do need to explicitly implement the interface. The part I don't like about this in general though is that that is completely hidden in the code. It feels strange that that's how it's wired in. In the later versions of grpc-go, are there new interfaces like this?
There was a problem hiding this comment.
No clue. See my other comment tho-- it looks to me like grpclib only cares about UsesXDS on the server side transport. We're only using client, so I think we could just rip this out. But you'd need to test it for me.
|
Oh also, it seems the |
So we're on 1.59.0. I scanned through the gRPC lib and it looks like all of the existing uses even in that version are server-side only. So I think maybe we don't need this on the client at all. Can you try it out without:? |
dragonsinth
force-pushed
the
PapaCharlie/master
branch
from
3ca27b7 to
3fc2238
Compare
No description provided.