Skip to content

Commit

Permalink
chore: Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
lqiu96 committed Jan 24, 2024
1 parent 371a69a commit 21eb9b3
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -295,11 +295,6 @@ private void logDirectPathMisconfig() {
Level.WARNING,
"DirectPath is misconfigured. DirectPath is only available in a GCE environment.");
}
// Case 4: Universe Domain is non-GDU
if (!canUseDirectPathWithUniverseDomain()) {
LOG.log(
Level.WARNING, "DirectPath will only work in the the googleapis.com Universe Domain");
}
}
}
}
Expand Down Expand Up @@ -338,13 +333,12 @@ static boolean isOnComputeEngine() {
// Universe Domain configuration is currently only supported in the GDU
@VisibleForTesting
boolean canUseDirectPathWithUniverseDomain() {
// ClientContext will set an endpoint if it isn't provided to the TransportChannelProvider
// If the endpoint is null, then the endpoint hasn't been fully resolved yet (client not
// initialized). Before a call goes through with DirectPath, this validation must succeed.
if (endpoint == null) {
if (endpoint.contains(Credentials.GOOGLE_DEFAULT_UNIVERSE)) {
return true;
}
return endpoint.contains(Credentials.GOOGLE_DEFAULT_UNIVERSE);
// This is only logged if DirectPath is enabled
LOG.log(Level.WARNING, "DirectPath will only work in the the googleapis.com Universe Domain");
return false;
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,16 +292,6 @@ public void testDirectPathDisallowNullCredentials() throws IOException {
assertThat(provider.isCredentialDirectPathCompatible()).isFalse();
}

@Test
public void testDirectPathWithNullEndpoint() {
InstantiatingGrpcChannelProvider provider =
InstantiatingGrpcChannelProvider.newBuilder()
.setAttemptDirectPath(true)
.setAttemptDirectPathXds()
.build();
assertThat(provider.canUseDirectPathWithUniverseDomain()).isTrue();
}

@Test
public void testDirectPathWithGDUEndpoint() {
InstantiatingGrpcChannelProvider provider =
Expand Down Expand Up @@ -597,22 +587,6 @@ public void testLogDirectPathMisconfigNotOnGCE() {
InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
}

@Test
public void testLogDirectPathMisconfigNotInGDU() {
FakeLogHandler logHandler = new FakeLogHandler();
InstantiatingGrpcChannelProvider.LOG.addHandler(logHandler);
InstantiatingGrpcChannelProvider provider =
InstantiatingGrpcChannelProvider.newBuilder()
.setAttemptDirectPathXds()
.setAttemptDirectPath(true)
.setAllowNonDefaultServiceAccount(true)
.setEndpoint("test.random.endpoint.com:443")
.build();
assertThat(logHandler.getAllMessages())
.contains("DirectPath will only work in the the googleapis.com Universe Domain");
InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
}

private static class FakeLogHandler extends Handler {
List<LogRecord> records = new ArrayList<>();

Expand Down

0 comments on commit 21eb9b3

Please sign in to comment.