From 1c12662860c541453187f3630e1631717ac3fbad Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 26 Oct 2022 21:43:27 +0000 Subject: [PATCH 1/5] c2p resolver: if federation is enabled then use xdstp LDS names and dont fallback due on a bootstrap file --- .../GoogleCloudToProdNameResolver.java | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java b/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java index cfada001d40..edba612f4cb 100644 --- a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java +++ b/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java @@ -54,6 +54,7 @@ final class GoogleCloudToProdNameResolver extends NameResolver { @VisibleForTesting static final String METADATA_URL_SUPPORT_IPV6 = "http://metadata.google.internal/computeMetadata/v1/instance/network-interfaces/0/ipv6s"; + static final String C2P_AUTHORITY = "traffic-director-c2p.xds.googleapis.com"; @VisibleForTesting static boolean isOnGcp = InternalCheckGcpEnvironment.isOnGcp(); @VisibleForTesting @@ -62,6 +63,10 @@ final class GoogleCloudToProdNameResolver extends NameResolver { || System.getProperty("io.grpc.xds.bootstrap") != null || System.getenv("GRPC_XDS_BOOTSTRAP_CONFIG") != null || System.getProperty("io.grpc.xds.bootstrapConfig") != null; + @VisibleForTesting + static boolean enableFederation = + !Strings.isNullOrEmpty(System.getenv("GRPC_EXPERIMENTAL_XDS_FEDERATION")) + && Boolean.parseBoolean(System.getenv("GRPC_EXPERIMENTAL_XDS_FEDERATION")); private static final String serverUriOverride = System.getenv("GRPC_TEST_ONLY_GOOGLE_C2P_RESOLVER_TRAFFIC_DIRECTOR_URI"); @@ -76,7 +81,7 @@ final class GoogleCloudToProdNameResolver extends NameResolver { private final boolean usingExecutorResource; // It's not possible to use both PSM and DirectPath C2P in the same application. // Delegate to DNS if user-provided bootstrap is found. - private final String schemeOverride = !isOnGcp || xdsBootstrapProvided ? "dns" : "xds"; + private final boolean useXds = isOnGcp && (enableFederation || !xdsBootstrapProvided); private Executor executor; private Listener2 listener; private boolean succeeded; @@ -103,8 +108,16 @@ final class GoogleCloudToProdNameResolver extends NameResolver { targetUri); authority = GrpcUtil.checkAuthority(targetPath.substring(1)); syncContext = checkNotNull(args, "args").getSynchronizationContext(); + if (useXds) { + targetUri = overrideUriScheme(targetUri, "xds"); + if (enableFederation) { + targetUri = overrideUriAuthority(targetUri, C2P_AUTHORITY); + } + } else { + targetUri = overrideUriScheme(targetUri, "dns"); + } delegate = checkNotNull(nameResolverFactory, "nameResolverFactory").newNameResolver( - overrideUriScheme(targetUri, schemeOverride), args); + targetUri, args); executor = args.getOffloadExecutor(); usingExecutorResource = executor == null; } @@ -193,7 +206,7 @@ public void run() { serverBuilder.put("server_features", ImmutableList.of("xds_v3")); ImmutableMap.Builder authoritiesBuilder = ImmutableMap.builder(); authoritiesBuilder.put( - "traffic-director-c2p.xds.googleapis.com", + C2P_AUTHORITY, ImmutableMap.of("xds_servers", ImmutableList.of(serverBuilder.buildOrThrow()))); return ImmutableMap.of( "node", nodeBuilder.buildOrThrow(), @@ -271,6 +284,16 @@ private static URI overrideUriScheme(URI uri, String scheme) { return res; } + private static URI overrideUriAuthority(URI uri, String authority) { + URI res; + try { + res = new URI(uri.getScheme(), authority, uri.getPath(), uri.getQuery(), uri.getFragment()); + } catch (URISyntaxException ex) { + throw new IllegalArgumentException("Invalid authority: " + authority, ex); + } + return res; + } + private enum HttpConnectionFactory implements HttpConnectionProvider { INSTANCE; From 35f163deb6d28d54e01c900d11624dcf448b5b2e Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 26 Oct 2022 21:59:53 +0000 Subject: [PATCH 2/5] update --- .../googleapis/GoogleCloudToProdNameResolver.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java b/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java index edba612f4cb..4b6101f1951 100644 --- a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java +++ b/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java @@ -21,6 +21,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Charsets; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.io.CharStreams; @@ -81,7 +82,7 @@ final class GoogleCloudToProdNameResolver extends NameResolver { private final boolean usingExecutorResource; // It's not possible to use both PSM and DirectPath C2P in the same application. // Delegate to DNS if user-provided bootstrap is found. - private final boolean useXds = isOnGcp && (enableFederation || !xdsBootstrapProvided); + private final String schemeOverride = !isOnGcp || (xdsBootstrapProvided && !enableFederation) ? "dns" : "xds"; private Executor executor; private Listener2 listener; private boolean succeeded; @@ -108,13 +109,9 @@ final class GoogleCloudToProdNameResolver extends NameResolver { targetUri); authority = GrpcUtil.checkAuthority(targetPath.substring(1)); syncContext = checkNotNull(args, "args").getSynchronizationContext(); - if (useXds) { - targetUri = overrideUriScheme(targetUri, "xds"); - if (enableFederation) { - targetUri = overrideUriAuthority(targetUri, C2P_AUTHORITY); - } - } else { - targetUri = overrideUriScheme(targetUri, "dns"); + targetUri = overrideUriScheme(targetUri, schemeOverride); + if (schemeOverride.equals("xds") && enableFederation) { + targetUri = overrideUriAuthority(targetUri, C2P_AUTHORITY); } delegate = checkNotNull(nameResolverFactory, "nameResolverFactory").newNameResolver( targetUri, args); From d74ae85559a3e4b48be121e611daf9e9a51a6051 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 26 Oct 2022 22:08:15 +0000 Subject: [PATCH 3/5] fix style --- .../io/grpc/googleapis/GoogleCloudToProdNameResolver.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java b/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java index 4b6101f1951..df592802fa8 100644 --- a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java +++ b/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java @@ -82,7 +82,10 @@ final class GoogleCloudToProdNameResolver extends NameResolver { private final boolean usingExecutorResource; // It's not possible to use both PSM and DirectPath C2P in the same application. // Delegate to DNS if user-provided bootstrap is found. - private final String schemeOverride = !isOnGcp || (xdsBootstrapProvided && !enableFederation) ? "dns" : "xds"; + private final String schemeOverride = + !isOnGcp + || (xdsBootstrapProvided && !enableFederation) + ? "dns" : "xds"; private Executor executor; private Listener2 listener; private boolean succeeded; From f2979a5fdbd57671e6c914d82e539fce46218db8 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Thu, 27 Oct 2022 20:10:27 +0000 Subject: [PATCH 4/5] update unit test --- .../GoogleCloudToProdNameResolverTest.java | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java b/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java index 562798c0183..3dfeee2a735 100644 --- a/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java +++ b/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java @@ -66,7 +66,7 @@ public class GoogleCloudToProdNameResolverTest { @Rule public final MockitoRule mocks = MockitoJUnit.rule(); - private static final URI TARGET_URI = URI.create("google-c2p-experimental:///googleapis.com"); + private static final URI TARGET_URI = URI.create("google-c2p:///googleapis.com"); private static final String ZONE = "us-central1-a"; private static final int DEFAULT_PORT = 887; @@ -187,6 +187,39 @@ public void onGcpAndNoProvidedBootstrapDelegateToXds() { "server_uri", "directpath-pa.googleapis.com", "channel_creds", ImmutableList.of(ImmutableMap.of("type", "google_default")), "server_features", ImmutableList.of("xds_v3")); + Map authorities = (Map) bootstrap.get("authorities"); + assertThat(authorities).containsExactly( + "traffic-director-c2p.xds.googleapis.com", + ImmutableMap.of("xds_servers", ImmutableList.of(server))); + } + + @SuppressWarnings("unchecked") + @Test + public void onGcpAndProvidedBootstrapAndFederationEnabledDelegateToXds() { + GoogleCloudToProdNameResolver.isOnGcp = true; + GoogleCloudToProdNameResolver.xdsBootstrapProvided = true; + GoogleCloudToProdNameResolver.enableFederation = true; + createResolver(); + resolver.start(mockListener); + assertThat(delegatedResolver.keySet()).containsExactly("xds"); + verify(Iterables.getOnlyElement(delegatedResolver.values())).start(mockListener); + // check bootstrap + Map bootstrap = fakeBootstrapSetter.bootstrapRef.get(); + Map node = (Map) bootstrap.get("node"); + assertThat(node).containsExactly( + "id", "C2P-991614323", + "locality", ImmutableMap.of("zone", ZONE), + "metadata", ImmutableMap.of("TRAFFICDIRECTOR_DIRECTPATH_C2P_IPV6_CAPABLE", true)); + Map server = Iterables.getOnlyElement( + (List>) bootstrap.get("xds_servers")); + assertThat(server).containsExactly( + "server_uri", "directpath-pa.googleapis.com", + "channel_creds", ImmutableList.of(ImmutableMap.of("type", "google_default")), + "server_features", ImmutableList.of("xds_v3")); + Map authorities = (Map) bootstrap.get("authorities"); + assertThat(authorities).containsExactly( + "traffic-director-c2p.xds.googleapis.com", + ImmutableMap.of("xds_servers", ImmutableList.of(server))); } @Test From b8dd88264951ef77cbd38f692b935427d1ee9b12 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Thu, 27 Oct 2022 20:24:50 +0000 Subject: [PATCH 5/5] Fix test --- .../io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java b/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java index 3dfeee2a735..a7c4ab059b6 100644 --- a/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java +++ b/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java @@ -201,6 +201,7 @@ public void onGcpAndProvidedBootstrapAndFederationEnabledDelegateToXds() { GoogleCloudToProdNameResolver.enableFederation = true; createResolver(); resolver.start(mockListener); + fakeExecutor.runDueTasks(); assertThat(delegatedResolver.keySet()).containsExactly("xds"); verify(Iterables.getOnlyElement(delegatedResolver.values())).start(mockListener); // check bootstrap