xds: Move JwtTokenFileCallCredentials to xds.client package - #13006
xds: Move JwtTokenFileCallCredentials to xds.client package#13006kannanjgithub wants to merge 1 commit into
Conversation
Sorry, something went wrong.
…package-private Move `JwtTokenFileCallCredentials` from `io.grpc.auth` to `io.grpc.xds.client` and make it package-private as it is only used in xds. To support this move without breaking tests: - Moved `JwtTokenFileCallCredentialsTest` to the same package (`io.grpc.xds.client`). - Created `BootstrapperImplTest` in `io.grpc.xds.client` to test the base `BootstrapperImpl` parsing logic. - Moved `call_creds` parsing tests (which reference `JwtTokenFileCallCredentials`) from `GrpcBootstrapperImplTest` to the new `BootstrapperImplTest`.
| import static org.junit.Assert.fail; | ||
|
|
||
| import com.google.common.collect.Iterables; | ||
| import io.grpc.CallCredentials; |
There was a problem hiding this comment.
I'm surprised there's a reference to io.grpc.CallCredentials in io.grpc.xds.client. Did we let more grpc dependencies leak into io.grpc.xds.client? Seeing cl/964373254 adds a dependency, yep, we did.
Sorry, something went wrong.
There was a problem hiding this comment.
cl/964373254 (import of #12951) introduced dependency on io.grpc.auth from io.grpc.xds.client. io.grpc.xds.client already used to depend on io.grpc classes such as Status even prior to this change.
Sorry, something went wrong.
There was a problem hiding this comment.
We put in effort to make it low dependency, as it is used by non-gRPC transports. We accepted it would use SynchronizationContext, and I guess Status. But it should not contain JwtTokenFileCallCredentials or a reference to it, and we should have definitely not added a dependency to grpc-auth which itself has dependencies on the auth libraries. The JwtTokenFileCallCredentials reference should be in the grpc-specific client code in io.grpc.xds like GrpcBootstrapperImpl. io.grpc.CallCredentials is a small interface, so that by itself isn't that big of a deal, but it is only useful for gRPC, so is clearly a code smell that we're doing something wrong.
Instead of storing CallCredentials as its own field in ServerInfo, we should store it in implSpecificConfig(). I don't think there was any need to change io.grpc.xds.client, as getImplSpecificConfig() was already powerful enough to do this.
(I had argued with Larry that we shouldn't have JSON parsing in io.grpc.xds.client.BootstrapperImpl. I argued the JSON bootstrap format is grpc-specific, so it should be in GrpcBootstrapperImpl (or somewhere else in io.grpc.xds). Larry disagreed and made it "reusable," even though only gRPC uses it.)
Sorry, something went wrong.
There was a problem hiding this comment.
FYI, #12242 was pretty close to being merged, IIRC, but I got too busy and then last memory of its state.
Sorry, something went wrong.
Move JwtTokenFileCallCredentials from io.grpc.auth to io.grpc.xds.client and make it package-private as it is only used in xds.
To support this move without breaking tests: