Skip to content

s2a: Single S2AChannelCredentials. #3989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rmehta19 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

s2a: Single S2AChannelCredentials. #3989

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8ddbe44
Single S2AChannelCredentials + use AdvancedTls.
rmehta19 Nov 14, 2025
e35914f
add comment.
rmehta19 Nov 14, 2025
217737b
address comments.
rmehta19 Dec 5, 2025
5f52e87
correct variable name.
rmehta19 Dec 5, 2025
b3d457b
Merge branch 'main' into singe-channel-creds
rmehta19 Dec 5, 2025
953e1a0
move adv tls changes to another PR.
rmehta19 Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 53 additions & 35 deletions ...java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP
@Nullable
private final ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> channelConfigurator;

// This is initialized once for the lifetime of the application. This enables re-using
// channels to S2A.
private static volatile ChannelCredentials s2aChannelCredentialsObject;
Copy link

@xmenxk xmenxk Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe add a comment here that we are using this single credentials object?

Copy link
Contributor Author

@rmehta19 rmehta19 Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, added a comment about this.


/*
* Experimental feature
*
Expand Down Expand Up @@ -595,43 +599,57 @@ ChannelCredentials createPlaintextToS2AChannelCredentials(String plaintextAddres
* @return {@link ChannelCredentials} configured to use S2A to create mTLS connection.
*/
ChannelCredentials createS2ASecuredChannelCredentials() {
SecureSessionAgentConfig config = s2aConfigProvider.getConfig();
String plaintextAddress = config.getPlaintextAddress();
String mtlsAddress = config.getMtlsAddress();
if (Strings.isNullOrEmpty(mtlsAddress)) {
// Fallback to plaintext connection to S2A.
LOG.log(
Level.INFO,
"Cannot establish an mTLS connection to S2A because autoconfig endpoint did not return a mtls address to reach S2A.");
return createPlaintextToS2AChannelCredentials(plaintextAddress);
}
// Currently, MTLS to MDS is only available on GCE. See:
// https://cloud.google.com/compute/docs/metadata/overview#https-mds
// Try to load MTLS-MDS creds.
File rootFile = new File(MTLS_MDS_ROOT_PATH);
File certKeyFile = new File(MTLS_MDS_CERT_CHAIN_AND_KEY_PATH);
if (rootFile.isFile() && certKeyFile.isFile()) {
// Try to connect to S2A using mTLS.
ChannelCredentials mtlsToS2AChannelCredentials = null;
try {
mtlsToS2AChannelCredentials =
createMtlsToS2AChannelCredentials(rootFile, certKeyFile, certKeyFile);
} catch (IOException ignore) {
// Fallback to plaintext-to-S2A connection on error.
LOG.log(
Level.WARNING,
"Cannot establish an mTLS connection to S2A due to error creating MTLS to MDS TlsChannelCredentials credentials, falling back to plaintext connection to S2A: "
+ ignore.getMessage());
return createPlaintextToS2AChannelCredentials(plaintextAddress);
if (s2aChannelCredentialsObject == null) {
synchronized (InstantiatingGrpcChannelProvider.class) {
if (s2aChannelCredentialsObject == null) {
Comment on lines +602 to +604
Copy link

@xmenxk xmenxk Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why doing the null check on both line 609 and 611?

Copy link
Contributor Author

@rmehta19 rmehta19 Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing! This is part of the double checked locking pattern, WDYT?

SecureSessionAgentConfig config = s2aConfigProvider.getConfig();
String plaintextAddress = config.getPlaintextAddress();
String mtlsAddress = config.getMtlsAddress();
if (Strings.isNullOrEmpty(mtlsAddress)) {
// Fallback to plaintext connection to S2A.
LOG.log(
Level.INFO,
"Cannot establish an mTLS connection to S2A because autoconfig endpoint did not return a mtls address to reach S2A.");
s2aChannelCredentialsObject = createPlaintextToS2AChannelCredentials(plaintextAddress);
return s2aChannelCredentialsObject;
}
// Currently, MTLS to MDS is only available on GCE. See:
// https://cloud.google.com/compute/docs/metadata/overview#https-mds
// Try to load MTLS-MDS creds.
File rootFile = new File(MTLS_MDS_ROOT_PATH);
File certKeyFile = new File(MTLS_MDS_CERT_CHAIN_AND_KEY_PATH);
if (rootFile.isFile() && certKeyFile.isFile()) {
// Try to connect to S2A using mTLS.
ChannelCredentials mtlsToS2AChannelCredentials = null;
try {
mtlsToS2AChannelCredentials =
createMtlsToS2AChannelCredentials(rootFile, certKeyFile, certKeyFile);
} catch (IOException | GeneralSecurityException ignore) {
// Fallback to plaintext-to-S2A connection on error.
LOG.log(
Level.WARNING,
"Cannot establish an mTLS connection to S2A due to error creating MTLS to MDS TlsChannelCredentials credentials, falling back to plaintext connection to S2A: "
+ ignore.getMessage());
s2aChannelCredentialsObject =
createPlaintextToS2AChannelCredentials(plaintextAddress);
return s2aChannelCredentialsObject;
}
s2aChannelCredentialsObject =
buildS2AChannelCredentials(mtlsAddress, mtlsToS2AChannelCredentials);
return s2aChannelCredentialsObject;
} else {
// Fallback to plaintext-to-S2A connection if MTLS-MDS creds do not exist.
LOG.log(
Level.INFO,
"Cannot establish an mTLS connection to S2A because MTLS to MDS credentials do not"
+ " exist on filesystem, falling back to plaintext connection to S2A");
s2aChannelCredentialsObject = createPlaintextToS2AChannelCredentials(plaintextAddress);
return s2aChannelCredentialsObject;
}
}
}
return buildS2AChannelCredentials(mtlsAddress, mtlsToS2AChannelCredentials);
} else {
// Fallback to plaintext-to-S2A connection if MTLS-MDS creds do not exist.
LOG.log(
Level.INFO,
"Cannot establish an mTLS connection to S2A because MTLS to MDS credentials do not exist on filesystem, falling back to plaintext connection to S2A");
return createPlaintextToS2AChannelCredentials(plaintextAddress);
}
return s2aChannelCredentialsObject;
}
Comment on lines 601 to 653
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic inside the synchronized block has become quite complex and contains duplicated code, particularly the fallback to createPlaintextToS2AChannelCredentials(plaintextAddress). This makes the method harder to read and maintain.

Consider refactoring this block to simplify the control flow. A clearer structure would be to attempt creating mTLS credentials first, and if that fails for any reason, then fall back to creating plaintext credentials. This would centralize the fallback logic.

For example, you could extract the creation logic into a helper method that returns the created credentials, which simplifies the double-checked locking block:

// In createS2ASecuredChannelCredentials():
if (s2aChannelCredentialsObject == null) {
  synchronized (InstantiatingGrpcChannelProvider.class) {
    if (s2aChannelCredentialsObject == null) {
      s2aChannelCredentialsObject = createS2ACredentialsOnce();
    }
  }
}
return s2aChannelCredentialsObject;

// New helper method:
private ChannelCredentials createS2ACredentialsOnce() {
  // ... logic to try mTLS and fallback to plaintext ...
}


private ManagedChannel createSingleChannel() throws IOException {
Expand Down
Loading