From f9e1c0f37faa8a4f75bc9d58def13c9225bb3a4b 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 33bda395eab..e858d9ee466 100644 --- a/xds/src/main/java/io/grpc/xds/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/BootstrapperImpl.java @@ -60,7 +60,6 @@ class BootstrapperImpl extends Bootstrapper { "envoy.lb.does_not_support_overprovisioning"; @VisibleForTesting static final String CLIENT_FEATURE_RESOURCE_IN_SOTW = "xds.config.resource-in-sotw"; - @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 47aab05aad8..18888eb56f6 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -2444,7 +2444,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 86382de9a21..436617f5b23 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -263,6 +263,7 @@ public long currentTimeNanos() { private boolean originalEnableFaultInjection; private boolean originalEnableRbac; private boolean originalEnableLeastRequest; + private boolean originalEnableFederation; @Before public void setUp() throws IOException { @@ -279,6 +280,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 @@ -353,6 +355,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(); @@ -788,6 +791,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"; @@ -808,6 +812,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"; @@ -828,6 +833,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"; @@ -847,6 +853,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"; @@ -867,6 +874,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"; @@ -886,6 +894,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"; @@ -906,6 +915,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";