From aee3ca9e9cfb5cde0938c19ecb458e1b9f8f9d29 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Thu, 29 Oct 2026 07:13:05 +0000 Subject: [PATCH 1/2] xds: enforce control plane isolation in handleResourceUpdate Ensure that handleResourceUpdate only applies resource updates, errors, or SotW absent notices to subscribers whose active control plane client matches the controlPlaneClient that received the ADS response. TAG=agy CONV=e27e4459-da3e-4c14-b2a1-2055f0b9b5d1 --- .../io/grpc/xds/client/XdsClientImpl.java | 11 +++--- .../io/grpc/xds/XdsClientFederationTest.java | 34 +++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java index 7e9f712db17..0bc16f6864c 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java @@ -606,8 +606,13 @@ private void handleResourceUpdate( Map> subscribedResources = resourceSubscribers.getOrDefault(xdsResourceType, Collections.emptyMap()); for (Map.Entry> entry : subscribedResources.entrySet()) { - String resourceName = entry.getKey(); ResourceSubscriber subscriber = (ResourceSubscriber) entry.getValue(); + // Only process resources subscribed to the control plane that sent this update. + if (getActiveCpc(subscriber.authority) != controlPlaneClient) { + continue; + } + + String resourceName = entry.getKey(); if (parsedResources.containsKey(resourceName)) { // Happy path: the resource updated successfully. Notify the watchers of the update. subscriber.onData(parsedResources.get(resourceName), args.versionInfo, updateTime, @@ -641,9 +646,7 @@ private void handleResourceUpdate( // For State of the World services, notify watchers when their watched resource is missing // from the ADS update. Note that we can only do this if the resource update is coming from // the same xDS server that the ResourceSubscriber is subscribed to. - if (getActiveCpc(subscriber.authority) == controlPlaneClient) { - subscriber.onAbsent(processingTracker, args.serverInfo); - } + subscriber.onAbsent(processingTracker, args.serverInfo); } } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index da310871c25..45a925ca340 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import io.envoyproxy.envoy.config.listener.v3.Listener; import io.grpc.MetricRecorder; import io.grpc.Status; import io.grpc.StatusOr; @@ -142,6 +143,39 @@ public void isolatedResourceDeletions() { verify(mockDirectPathWatcher, never()).onAmbientError(any()); } + /** + * Assures that resource updates happening in one control plane do not trigger update events + * in watchers of resources owned by other control planes. + */ + @Test + public void isolatedResourceUpdates() { + Listener serverListener = ControlPlaneRule.buildServerListener(); + Listener clientListener1 = ControlPlaneRule.buildClientListener("test-server"); + Listener clientListener2 = ControlPlaneRule.buildClientListener( + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); + // trafficdirector (top-level server) erroneously sends a listener owned by server-one. + trafficdirector.getService().setXdsConfig(XdsTestControlPlaneService.ADS_TYPE_URL_LDS, + ImmutableMap.of( + serverListener.getName(), serverListener, + clientListener1.getName(), clientListener1, + clientListener2.getName(), clientListener2)); + + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "test-server", mockWatcher); + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher); + + verify(mockWatcher, timeout(10000)).onResourceChanged(any()); + // mockDirectPathWatcher should not receive the update sent by trafficdirector. + verify(mockDirectPathWatcher, never()).onResourceChanged(any()); + + // When server-one sends its LDS config, mockDirectPathWatcher should then receive the update. + directpathPa.setLdsConfig(ControlPlaneRule.buildServerListener(), + ControlPlaneRule.buildClientListener( + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server")); + + verify(mockDirectPathWatcher, timeout(10000)).onResourceChanged(any()); + } + /** * Assures that when an {@link XdsClient} is asked to add cluster locality stats it appropriately * starts {@link LoadReportClient}s to do that. From 7a50e0f5c15c7563a4c300e05fb3b93bb4e9df72 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Thu, 29 Oct 2026 07:39:00 +0000 Subject: [PATCH 2/2] xds: update isolatedResourceUpdates test to directly send unsolicited ADS response Add sendDiscoveryResponse to XdsTestControlPlaneService and update isolatedResourceUpdates test in XdsClientFederationTest to directly send an unsolicited cross-authority ADS response on the wire, properly exercising the control plane isolation check in XdsClientImpl. TAG=agy CONV=e27e4459-da3e-4c14-b2a1-2055f0b9b5d1 --- .../io/grpc/xds/XdsClientFederationTest.java | 34 +++++++++++++------ .../grpc/xds/XdsTestControlPlaneService.java | 15 ++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index 45a925ca340..70fb09aed29 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -26,7 +26,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.protobuf.Any; import io.envoyproxy.envoy.config.listener.v3.Listener; +import io.envoyproxy.envoy.service.discovery.v3.DiscoveryResponse; import io.grpc.MetricRecorder; import io.grpc.Status; import io.grpc.StatusOr; @@ -149,22 +151,34 @@ public void isolatedResourceDeletions() { */ @Test public void isolatedResourceUpdates() { - Listener serverListener = ControlPlaneRule.buildServerListener(); - Listener clientListener1 = ControlPlaneRule.buildClientListener("test-server"); - Listener clientListener2 = ControlPlaneRule.buildClientListener( - "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); - // trafficdirector (top-level server) erroneously sends a listener owned by server-one. - trafficdirector.getService().setXdsConfig(XdsTestControlPlaneService.ADS_TYPE_URL_LDS, - ImmutableMap.of( - serverListener.getName(), serverListener, - clientListener1.getName(), clientListener1, - clientListener2.getName(), clientListener2)); + trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), + ControlPlaneRule.buildClientListener("test-server")); xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "test-server", mockWatcher); xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher); verify(mockWatcher, timeout(10000)).onResourceChanged(any()); + + // trafficdirector (top-level server) erroneously sends a response containing both an update + // for test-server and a listener owned by server-one. + Listener alienListener = ControlPlaneRule.buildClientListener( + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); + Listener updatedClientListener = ControlPlaneRule.buildClientListener("test-server", + "new-route-config.googleapis.com"); + DiscoveryResponse rogueResponse = DiscoveryResponse.newBuilder() + .setTypeUrl(XdsTestControlPlaneService.ADS_TYPE_URL_LDS) + .setVersionInfo("rogue-version-1") + .setNonce("rogue-nonce-1") + .addResources(Any.pack(updatedClientListener)) + .addResources(Any.pack(alienListener)) + .build(); + trafficdirector.getService().sendDiscoveryResponse(rogueResponse); + + // Confirm that trafficdirector's second response was processed by waiting for mockWatcher + // to receive the updated test-server config. + verify(mockWatcher, timeout(10000).times(2)).onResourceChanged(any()); + // mockDirectPathWatcher should not receive the update sent by trafficdirector. verify(mockDirectPathWatcher, never()).onResourceChanged(any()); diff --git a/xds/src/test/java/io/grpc/xds/XdsTestControlPlaneService.java b/xds/src/test/java/io/grpc/xds/XdsTestControlPlaneService.java index a54893c9075..7ec446cdeef 100644 --- a/xds/src/test/java/io/grpc/xds/XdsTestControlPlaneService.java +++ b/xds/src/test/java/io/grpc/xds/XdsTestControlPlaneService.java @@ -119,6 +119,21 @@ public void run() { }); } + /** + * Directly sends the given DiscoveryResponse to all active subscribers of the response's type. + */ + public void sendDiscoveryResponse(DiscoveryResponse response) { + syncContext.execute(new Runnable() { + @Override + public void run() { + for (StreamObserver observer : + subscribers.get(response.getTypeUrl()).keySet()) { + observer.onNext(response); + } + } + }); + } + ImmutableMap getCurrentConfig(String type) { HashMap hashMap = xdsResources.get(type); return (hashMap != null) ? ImmutableMap.copyOf(hashMap) : ImmutableMap.of();