From db4206ee891304625ddcb81d0edd9c7a6185a99e Mon Sep 17 00:00:00 2001 From: Shreshta Kethireddy Date: Mon, 3 Nov 2025 23:49:46 +0000 Subject: [PATCH 1/5] ensuring case where user only passes in host uses the correct port --- .../grpc/alts/HandshakerServiceChannel.java | 22 ++++++++++++++----- .../alts/HandshakerServiceChannelTest.java | 17 ++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java index f03cc6f8c94..017eed78e72 100644 --- a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java +++ b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java @@ -38,13 +38,25 @@ * application will have at most one connection to the handshaker service. */ final class HandshakerServiceChannel { + + private static final int ALTS_PORT = 8080; + private static final String DEFAULT_TARGET = "metadata.google.internal.:8080"; + private static final String METADATA_HOST_ENV_VAR = System.getenv("GCE_METADATA_HOST"); static final Resource SHARED_HANDSHAKER_CHANNEL = - new ChannelResource( - MoreObjects.firstNonNull( - System.getenv("GCE_METADATA_HOST"), "metadata.google.internal.:8080")); - - + new ChannelResource(getHandshakerTarget(METADATA_HOST_ENV_VAR)); + + static String getHandshakerTarget(String envValue) { + if (envValue == null || envValue.isEmpty()) { + return DEFAULT_TARGET; + } + int colonIndex = envValue.lastIndexOf(':'); + if (colonIndex != -1) { + return envValue; + } else { + return envValue + ":" + ALTS_PORT; + } + } /** Returns a resource of handshaker service channel for testing only. */ static Resource getHandshakerChannelForTesting(String handshakerAddress) { return new ChannelResource(handshakerAddress); diff --git a/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java b/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java index a3937904cd7..03567b01305 100644 --- a/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java +++ b/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java @@ -67,6 +67,23 @@ public void sharedChannel_authority() { } } + @Test + public void getHandshakerTarget_nullEnvVar() { + assertThat(HandshakerServiceChannel.getHandshakerTarget(null)).isEqualTo("metadata.google.internal.:8080"); + } + + @Test + public void getHandshakerTarget_envVarWithPort() { + assertThat(HandshakerServiceChannel.getHandshakerTarget("169.254.169.254:80")) + .isEqualTo("169.254.169.254:80"); + } + + @Test + public void getHandshakerTarget_envVarWithHostOnly() { + assertThat(HandshakerServiceChannel.getHandshakerTarget("169.254.169.254")) + .isEqualTo("169.254.169.254:8080"); + } + @Test public void resource_works() { Channel channel = resource.create(); From d9a64146dcdc13c122a1d66e53ca52cb4321fb8b Mon Sep 17 00:00:00 2001 From: Shreshta Kethireddy Date: Tue, 4 Nov 2025 00:31:45 +0000 Subject: [PATCH 2/5] styling --- alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java | 2 +- .../test/java/io/grpc/alts/HandshakerServiceChannelTest.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java index 017eed78e72..5b3c3fa71ca 100644 --- a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java +++ b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java @@ -16,7 +16,6 @@ package io.grpc.alts; -import com.google.common.base.MoreObjects; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -57,6 +56,7 @@ static String getHandshakerTarget(String envValue) { return envValue + ":" + ALTS_PORT; } } + /** Returns a resource of handshaker service channel for testing only. */ static Resource getHandshakerChannelForTesting(String handshakerAddress) { return new ChannelResource(handshakerAddress); diff --git a/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java b/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java index 03567b01305..f02df3557fa 100644 --- a/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java +++ b/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java @@ -69,7 +69,8 @@ public void sharedChannel_authority() { @Test public void getHandshakerTarget_nullEnvVar() { - assertThat(HandshakerServiceChannel.getHandshakerTarget(null)).isEqualTo("metadata.google.internal.:8080"); + assertThat(HandshakerServiceChannel.getHandshakerTarget(null)) + .isEqualTo("metadata.google.internal.:8080"); } @Test From 77d99c6339ef62f6b86e500fe2c3856da814b346 Mon Sep 17 00:00:00 2001 From: Shreshta Kethireddy Date: Wed, 5 Nov 2025 00:00:49 +0000 Subject: [PATCH 3/5] removing unecessary variable --- alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java index 5b3c3fa71ca..9de76a57e6d 100644 --- a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java +++ b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java @@ -40,10 +40,9 @@ final class HandshakerServiceChannel { private static final int ALTS_PORT = 8080; private static final String DEFAULT_TARGET = "metadata.google.internal.:8080"; - private static final String METADATA_HOST_ENV_VAR = System.getenv("GCE_METADATA_HOST"); static final Resource SHARED_HANDSHAKER_CHANNEL = - new ChannelResource(getHandshakerTarget(METADATA_HOST_ENV_VAR)); + new ChannelResource(getHandshakerTarget(System.getenv("GCE_METADATA_HOST"))); static String getHandshakerTarget(String envValue) { if (envValue == null || envValue.isEmpty()) { From 853df74d43ec689201d65abae492d2ae37e70f7b Mon Sep 17 00:00:00 2001 From: Shreshta Kethireddy Date: Wed, 5 Nov 2025 20:44:31 +0000 Subject: [PATCH 4/5] when port is specified, change to use port 8080 --- .../java/io/grpc/alts/HandshakerServiceChannel.java | 10 +++++----- .../io/grpc/alts/HandshakerServiceChannelTest.java | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java index 9de76a57e6d..eea6352da8e 100644 --- a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java +++ b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java @@ -48,12 +48,12 @@ static String getHandshakerTarget(String envValue) { if (envValue == null || envValue.isEmpty()) { return DEFAULT_TARGET; } - int colonIndex = envValue.lastIndexOf(':'); - if (colonIndex != -1) { - return envValue; - } else { - return envValue + ":" + ALTS_PORT; + String host = envValue; + int portIndex = host.lastIndexOf(':'); + if (portIndex != -1) { + host = host.substring(0, portIndex); } + return host + ":" + ALTS_PORT; } /** Returns a resource of handshaker service channel for testing only. */ diff --git a/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java b/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java index f02df3557fa..221001157f1 100644 --- a/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java +++ b/alts/src/test/java/io/grpc/alts/HandshakerServiceChannelTest.java @@ -76,7 +76,7 @@ public void getHandshakerTarget_nullEnvVar() { @Test public void getHandshakerTarget_envVarWithPort() { assertThat(HandshakerServiceChannel.getHandshakerTarget("169.254.169.254:80")) - .isEqualTo("169.254.169.254:80"); + .isEqualTo("169.254.169.254:8080"); } @Test From d978c6a796bedaeec6899792c1c9041cb06a83a5 Mon Sep 17 00:00:00 2001 From: Shreshta Kethireddy Date: Tue, 11 Nov 2025 19:12:24 +0000 Subject: [PATCH 5/5] adding comments --- .../java/io/grpc/alts/HandshakerServiceChannel.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java index eea6352da8e..5e32d22d901 100644 --- a/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java +++ b/alts/src/main/java/io/grpc/alts/HandshakerServiceChannel.java @@ -37,13 +37,17 @@ * application will have at most one connection to the handshaker service. */ final class HandshakerServiceChannel { - + // Port 8080 is necessary for ALTS handshake. private static final int ALTS_PORT = 8080; private static final String DEFAULT_TARGET = "metadata.google.internal.:8080"; static final Resource SHARED_HANDSHAKER_CHANNEL = new ChannelResource(getHandshakerTarget(System.getenv("GCE_METADATA_HOST"))); - + + /** + * Returns handshaker target. When GCE_METADATA_HOST is provided, it might contain port which we + * will discard and use ALTS_PORT instead. + */ static String getHandshakerTarget(String envValue) { if (envValue == null || envValue.isEmpty()) { return DEFAULT_TARGET; @@ -51,9 +55,9 @@ static String getHandshakerTarget(String envValue) { String host = envValue; int portIndex = host.lastIndexOf(':'); if (portIndex != -1) { - host = host.substring(0, portIndex); + host = host.substring(0, portIndex); // Discard port if specified } - return host + ":" + ALTS_PORT; + return host + ":" + ALTS_PORT; // Utilize ALTS port in all cases } /** Returns a resource of handshaker service channel for testing only. */