Skip to content

Commit 4697693

Browse files
committed
fix: resolve DirectPath validation regression and add misconfig tests\n\n- Fixed logic in validateDirectPathState to ensure credential and GCE checks are always performed when DirectPath is enabled.\n- Added explicit warnings for DirectPath misconfigurations (xDS mismatch).\n- Added regression tests in InstantiatingGrpcChannelProviderTest.\n\n[Generated-by: AI]
1 parent d4a4c65 commit 4697693

2 files changed

Lines changed: 66 additions & 9 deletions

File tree

sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ public TransportChannel getTransportChannel() throws IOException {
397397
} else if (needsEndpoint()) {
398398
throw new IllegalStateException("getTransportChannel() called when needsEndpoint() is true");
399399
} else {
400-
logDirectPathMisconfig();
400+
validateDirectPathState();
401401
return createChannel();
402402
}
403403
}
@@ -454,15 +454,11 @@ public boolean isDirectPathXdsEnabled() {
454454
// This method should be called once per client initialization, hence can not be called in the
455455
// builder or createSingleChannel, only in getTransportChannel which creates the first channel
456456
// for a client.
457-
private void logDirectPathMisconfig() {
458-
if (!isDirectPathXdsEnabled()) {
459-
return;
460-
}
461-
457+
@InternalApi
458+
public void validateDirectPathState() {
462459
Level level = isOnComputeEngine() ? Level.WARNING : Level.FINE;
463460

464461
if (!isDirectPathEnabled()) {
465-
// This misconfiguration occurs when Direct Path xDS is enabled, but Direct Path is not
466462
// Direct Path xDS can be enabled two ways: via environment variable or via builder.
467463
// Case 1: Direct Path is only enabled via xDS env var. We will _warn_ the user that this is
468464
// a misconfiguration if they intended to set the env var.
@@ -483,15 +479,21 @@ else if (isDirectPathXdsEnabledViaBuilderOption()) {
483479
"DirectPath is misconfigured. The DirectPath XDS option was set, but the attemptDirectPath option was not. Please set both the attemptDirectPath and attemptDirectPathXds options.");
484480
}
485481
} else {
486-
// Case 3: credential is not correctly set
482+
// Case 3: DirectPath is enabled, but xDS is not.
483+
if (!isDirectPathXdsEnabled()) {
484+
LOG.log(
485+
level,
486+
"DirectPath is enabled, but DirectPath xDS is not. Please note that DirectPath will soon require xDS to be enabled. Please set the attemptDirectPathXds option.");
487+
}
488+
// Case 4: credential is not correctly set
487489
if (!isCredentialDirectPathCompatible()) {
488490
LOG.log(
489491
level,
490492
"DirectPath is misconfigured. Please make sure the credential is an instance of "
491493
+ ComputeEngineCredentials.class.getName()
492494
+ " .");
493495
}
494-
// Case 4: not running on GCE
496+
// Case 5: not running on GCE
495497
if (!isOnComputeEngine() && !isAttemptDirectPathXdsOverInterconnect()) {
496498
LOG.log(
497499
level,

sdk-platform-java/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,6 +1583,61 @@ private static String extractTargetFromChannelBuilder(ManagedChannelBuilder<?> c
15831583
return channelBuilder.toString();
15841584
}
15851585

1586+
@Test
1587+
void testLogDirectPathMisconfigXdsSetDirectPathNotSet() throws Exception {
1588+
FakeLogHandler logHandler = new FakeLogHandler();
1589+
InstantiatingGrpcChannelProvider.LOG.setLevel(Level.FINE);
1590+
InstantiatingGrpcChannelProvider.LOG.addHandler(logHandler);
1591+
InstantiatingGrpcChannelProvider provider =
1592+
InstantiatingGrpcChannelProvider.newBuilder()
1593+
.setAttemptDirectPathXds()
1594+
.setAttemptDirectPath(false)
1595+
.setHeaderProvider(
1596+
mock(HeaderProvider.class, Mockito.withSettings().withoutAnnotations()))
1597+
.setExecutor(mock(Executor.class, Mockito.withSettings().withoutAnnotations()))
1598+
.setEndpoint(DEFAULT_ENDPOINT)
1599+
.setCertificateBasedAccess(certificateBasedAccess)
1600+
.build();
1601+
1602+
try {
1603+
provider.getTransportChannel();
1604+
} catch (Exception e) {
1605+
// ignore
1606+
}
1607+
1608+
assertThat(logHandler.getAllMessages())
1609+
.contains(
1610+
"DirectPath is misconfigured. The DirectPath XDS option was set, but the attemptDirectPath option was not. Please set both the attemptDirectPath and attemptDirectPathXds options.");
1611+
InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
1612+
}
1613+
1614+
@Test
1615+
void testLogDirectPathMisconfigDirectPathSetXdsNotSet() throws Exception {
1616+
FakeLogHandler logHandler = new FakeLogHandler();
1617+
InstantiatingGrpcChannelProvider.LOG.setLevel(Level.FINE);
1618+
InstantiatingGrpcChannelProvider.LOG.addHandler(logHandler);
1619+
InstantiatingGrpcChannelProvider provider =
1620+
InstantiatingGrpcChannelProvider.newBuilder()
1621+
.setAttemptDirectPath(true)
1622+
.setHeaderProvider(
1623+
mock(HeaderProvider.class, Mockito.withSettings().withoutAnnotations()))
1624+
.setExecutor(mock(Executor.class, Mockito.withSettings().withoutAnnotations()))
1625+
.setEndpoint(DEFAULT_ENDPOINT)
1626+
.setCertificateBasedAccess(certificateBasedAccess)
1627+
.build();
1628+
1629+
try {
1630+
provider.getTransportChannel();
1631+
} catch (Exception e) {
1632+
// ignore
1633+
}
1634+
1635+
assertThat(logHandler.getAllMessages())
1636+
.contains(
1637+
"DirectPath is enabled, but DirectPath xDS is not. Please note that DirectPath will soon require xDS to be enabled. Please set the attemptDirectPathXds option.");
1638+
InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
1639+
}
1640+
15861641
private static class FakeLogHandler extends Handler {
15871642

15881643
List<LogRecord> records = new ArrayList<>();

0 commit comments

Comments
 (0)