From be5334a9d61b385322cc141843fb2663a010d849 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 23 May 2022 08:16:11 -0700 Subject: [PATCH] xds: Protect xdstp processing with federation env var There are still some cases for xdstp processing, but they are to percent encoding replacement strings. Those seem better to leave running since it looks like it they could be triggered even with federation disabled in the bootstrap processing. --- xds/src/main/java/io/grpc/xds/BootstrapperImpl.java | 1 - xds/src/main/java/io/grpc/xds/ClientXdsClient.java | 2 +- .../test/java/io/grpc/xds/ClientXdsClientTestBase.java | 10 ++++++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/BootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/BootstrapperImpl.java index 4796f5e0361..0c6b48606fe 100644 --- a/xds/src/main/java/io/grpc/xds/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/BootstrapperImpl.java @@ -61,7 +61,6 @@ class BootstrapperImpl extends Bootstrapper { @VisibleForTesting static final String CLIENT_FEATURE_DISABLE_OVERPROVISIONING = "envoy.lb.does_not_support_overprovisioning"; - @VisibleForTesting static boolean enableFederation = !Strings.isNullOrEmpty(System.getenv("GRPC_EXPERIMENTAL_XDS_FEDERATION")) && Boolean.parseBoolean(System.getenv("GRPC_EXPERIMENTAL_XDS_FEDERATION")); diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index 95b3bbfcfdf..c0ba6666654 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -2417,7 +2417,7 @@ private final class ResourceSubscriber { } private ServerInfo getServerInfo(String resource) { - if (resource.startsWith(XDSTP_SCHEME)) { + if (BootstrapperImpl.enableFederation && resource.startsWith(XDSTP_SCHEME)) { URI uri = URI.create(resource); String authority = uri.getAuthority(); if (authority == null) { diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java index ba182be76d1..0ac444ea048 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -259,6 +259,7 @@ public long currentTimeNanos() { private boolean originalEnableFaultInjection; private boolean originalEnableRbac; private boolean originalEnableLeastRequest; + private boolean originalEnableFederation; @Before public void setUp() throws IOException { @@ -275,6 +276,7 @@ public void setUp() throws IOException { assertThat(originalEnableRbac).isTrue(); originalEnableLeastRequest = ClientXdsClient.enableLeastRequest; ClientXdsClient.enableLeastRequest = true; + originalEnableFederation = BootstrapperImpl.enableFederation; final String serverName = InProcessServerBuilder.generateName(); cleanupRule.register( InProcessServerBuilder @@ -349,6 +351,7 @@ public void tearDown() { ClientXdsClient.enableFaultInjection = originalEnableFaultInjection; ClientXdsClient.enableRbac = originalEnableRbac; ClientXdsClient.enableLeastRequest = originalEnableLeastRequest; + BootstrapperImpl.enableFederation = originalEnableFederation; xdsClient.shutdown(); channel.shutdown(); // channel not owned by XdsClient assertThat(adsEnded.get()).isTrue(); @@ -760,6 +763,7 @@ public void ldsResourceUpdated() { @Test public void ldsResourceUpdated_withXdstpResourceName() { + BootstrapperImpl.enableFederation = true; String ldsResourceName = useProtocolV3() ? "xdstp://authority.xds.com/envoy.config.listener.v3.Listener/listener1" : "xdstp://authority.xds.com/envoy.api.v2.Listener/listener1"; @@ -780,6 +784,7 @@ public void ldsResourceUpdated_withXdstpResourceName() { @Test public void ldsResourceUpdated_withXdstpResourceName_withEmptyAuthority() { + BootstrapperImpl.enableFederation = true; String ldsResourceName = useProtocolV3() ? "xdstp:///envoy.config.listener.v3.Listener/listener1" : "xdstp:///envoy.api.v2.Listener/listener1"; @@ -800,6 +805,7 @@ public void ldsResourceUpdated_withXdstpResourceName_withEmptyAuthority() { @Test public void ldsResourceUpdated_withXdstpResourceName_witUnorderedContextParams() { + BootstrapperImpl.enableFederation = true; String ldsResourceName = useProtocolV3() ? "xdstp://authority.xds.com/envoy.config.listener.v3.Listener/listener1/a?bar=2&foo=1" : "xdstp://authority.xds.com/envoy.api.v2.Listener/listener1/a?bar=2&foo=1"; @@ -819,6 +825,7 @@ public void ldsResourceUpdated_withXdstpResourceName_witUnorderedContextParams() @Test public void ldsResourceUpdated_withXdstpResourceName_withWrongType() { + BootstrapperImpl.enableFederation = true; String ldsResourceName = useProtocolV3() ? "xdstp://authority.xds.com/envoy.config.listener.v3.Listener/listener1" : "xdstp://authority.xds.com/envoy.api.v2.Listener/listener1"; @@ -839,6 +846,7 @@ public void ldsResourceUpdated_withXdstpResourceName_withWrongType() { @Test public void rdsResourceUpdated_withXdstpResourceName_withWrongType() { + BootstrapperImpl.enableFederation = true; String rdsResourceName = useProtocolV3() ? "xdstp://authority.xds.com/envoy.config.route.v3.RouteConfiguration/route1" : "xdstp://authority.xds.com/envoy.api.v2.RouteConfiguration/route1"; @@ -858,6 +866,7 @@ public void rdsResourceUpdated_withXdstpResourceName_withWrongType() { @Test public void cdsResourceUpdated_withXdstpResourceName_withWrongType() { + BootstrapperImpl.enableFederation = true; String cdsResourceName = useProtocolV3() ? "xdstp://authority.xds.com/envoy.config.cluster.v3.Cluster/cluster1" : "xdstp://authority.xds.com/envoy.api.v2.Cluster/cluster1"; @@ -878,6 +887,7 @@ public void cdsResourceUpdated_withXdstpResourceName_withWrongType() { @Test public void edsResourceUpdated_withXdstpResourceName_withWrongType() { + BootstrapperImpl.enableFederation = true; String edsResourceName = useProtocolV3() ? "xdstp://authority.xds.com/envoy.config.endpoint.v3.ClusterLoadAssignment/cluster1" : "xdstp://authority.xds.com/envoy.api.v2.ClusterLoadAssignment/cluster1";