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.
grpc · May 23, 2022 · f9e1c0f · f9e1c0f
1 parent 4a5f6ad
commit f9e1c0f
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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";