qq, I believe below has a default value for the certificateBasedAccess. What's the reason this should be Nullable?

So the "CertificateBasedAccess" is essentially a couple of helper functions that was moved out of the original MtlsProvider implementation, and since the previous MtlsProvider was Nullable, this was also marked Nullable to retain the same semantics - my hunch is that we should keep it Nullable for maximum compatibility/flexibility.

I see. The old MtlsProvider was public in name, but we didn't allow users to set it into the client. I don't think we have direct use cases of users using it and I don't think we need to maintain compatibility for it.

If possible, I would rather have this be non-null so we don't need the null checks. I think this could also be the same with the new MtlsProvider above since there is a default one created below.

Oh wait, I think the MtlsProvider must remain Nullable given the possibility of CertificateSourceUnavailableException.

So yes, I think MtlsProvider should remain Nullable. Would you like to keep CertificateBasedAccess Nullable as well or change it to non-nullable? I don't have a strong preference either way. LMK Thanks!

I think MtlsProvider must remain Nullable and CertificateBasedAccess can be non-Nullable. Preference for CertificateBasedAccess to be non-nullable so that we don't need so many null checks in the logic below and in other files.

I updated the latest revision to mark MtlsProvider Nullable and CertificateBasedAccess NonNullable in the HTTP/gRPC channel providers for consistency. However, CertificateBasedAccess is left as @nullable in EndpointContext, since removing it broke compilation for a ton of tests... (will revisit if needed.)

qq, I noticed that this rough logic also exists here and in the EndpointContext. What's the reason that it needs to exist in the gRPC Channel Provider as well as EndpointContext?

Yes good question. The issue is that EndpointContext is responsible for "endpoint resolution", which takes a dependency on the availability of mTLS (i.e. use mTLS endpoint only if mTLS support is available) - see the complicated "determineEndpoint" function. In the other 2 locations (gRPC/HTTP channel provider), they are used for configuring the TLS settings of the Channel themselves.

In the other 2 locations (gRPC/HTTP channel provider), they are used for configuring the TLS settings of the Channel themselves.

Sorry, can you elaborate on this point? I wasn't aware there was settings that MtlsProvider itself was configuring anything. Can you link me to how it's configure TLS settings on the channel?

My assumption was that it was only using the MtlsProvider logic (i.e. checking for env var) to see if Mtls was to be enabled and not touching anything on the channel.

Let me clarify, for gRPC/HTTP channel provider, the "mtlsKeyStore" is used for creating the mTLS-enabled transport as seen in the code snippet below:

HttpTransport createHttpTransport() throws IOException, GeneralSecurityException {
if (certificateBasedAccess == null || mtlsProvider == null) {
return null;
}
if (certificateBasedAccess.useMtlsClientCertificate()) {
KeyStore mtlsKeyStore = mtlsProvider.getKeyStore();
if (mtlsKeyStore != null) {
return new NetHttpTransport.Builder().trustCertificates(null, mtlsKeyStore, "").build();
}
}
return null;
}

The channel providers have no reference to the EndpointContext, so need to independently calculate and bootstrap the mTLS provider.

I see, thanks for the explanation.

In this case, I think what we have previously configured is probably a mistake (i.e. configuring multiple separate MtlsProviders). I think realistically the flow should be

1. EndpointContext creates the MtlsProvider and CertificateBasedAccess class that is used to compute the endpoint
2. gRPC and HttpJson channel providers get the MtlsProvider and CertificateBasedAccess classes that were created in the endpointcontext

However, I think in order for that to be done, we'll need to create public methods inside TransportChannelProvider to allow access to them. It is possible with adding InternalApi and we have done previously to access mtlsEndpoint.

Let me think about the options a bit more. I think what you have is based on the existing code and should work, but I think it previously wasn't configured the best/ correctly and would love to try and fix it if possible.

should this be GOOGLE_API_USE_CLIENT_CERTIFICATE ? true : false

I can update this to the below equivalent for clarity:

name.equals("GOOGLE_API_USE_CLIENT_CERTIFICATE") ? "true" : "auto")

same as above GOOGLE_API_USE_CLIENT_CERTIFICATE ? false : true?