Skip to content

Commit aee3ca9

Browse files
committed
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
1 parent 15d64a4 commit aee3ca9

2 files changed

Lines changed: 41 additions & 4 deletions

File tree

‎xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -606,8 +606,13 @@ private <T extends ResourceUpdate> void handleResourceUpdate(
606606
Map<String, ResourceSubscriber<? extends ResourceUpdate>> subscribedResources =
607607
resourceSubscribers.getOrDefault(xdsResourceType, Collections.emptyMap());
608608
for (Map.Entry<String, ResourceSubscriber<?>> entry : subscribedResources.entrySet()) {
609-
String resourceName = entry.getKey();
610609
ResourceSubscriber<T> subscriber = (ResourceSubscriber<T>) entry.getValue();
610+
// Only process resources subscribed to the control plane that sent this update.
611+
if (getActiveCpc(subscriber.authority) != controlPlaneClient) {
612+
continue;
613+
}
614+
615+
String resourceName = entry.getKey();
611616
if (parsedResources.containsKey(resourceName)) {
612617
// Happy path: the resource updated successfully. Notify the watchers of the update.
613618
subscriber.onData(parsedResources.get(resourceName), args.versionInfo, updateTime,
@@ -641,9 +646,7 @@ private <T extends ResourceUpdate> void handleResourceUpdate(
641646
// For State of the World services, notify watchers when their watched resource is missing
642647
// from the ADS update. Note that we can only do this if the resource update is coming from
643648
// the same xDS server that the ResourceSubscriber is subscribed to.
644-
if (getActiveCpc(subscriber.authority) == controlPlaneClient) {
645-
subscriber.onAbsent(processingTracker, args.serverInfo);
646-
}
649+
subscriber.onAbsent(processingTracker, args.serverInfo);
647650
}
648651
}
649652

‎xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import com.google.common.collect.ImmutableList;
2828
import com.google.common.collect.ImmutableMap;
29+
import io.envoyproxy.envoy.config.listener.v3.Listener;
2930
import io.grpc.MetricRecorder;
3031
import io.grpc.Status;
3132
import io.grpc.StatusOr;
@@ -142,6 +143,39 @@ public void isolatedResourceDeletions() {
142143
verify(mockDirectPathWatcher, never()).onAmbientError(any());
143144
}
144145

146+
/**
147+
* Assures that resource updates happening in one control plane do not trigger update events
148+
* in watchers of resources owned by other control planes.
149+
*/
150+
@Test
151+
public void isolatedResourceUpdates() {
152+
Listener serverListener = ControlPlaneRule.buildServerListener();
153+
Listener clientListener1 = ControlPlaneRule.buildClientListener("test-server");
154+
Listener clientListener2 = ControlPlaneRule.buildClientListener(
155+
"xdstp://server-one/envoy.config.listener.v3.Listener/test-server");
156+
// trafficdirector (top-level server) erroneously sends a listener owned by server-one.
157+
trafficdirector.getService().setXdsConfig(XdsTestControlPlaneService.ADS_TYPE_URL_LDS,
158+
ImmutableMap.of(
159+
serverListener.getName(), serverListener,
160+
clientListener1.getName(), clientListener1,
161+
clientListener2.getName(), clientListener2));
162+
163+
xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "test-server", mockWatcher);
164+
xdsClient.watchXdsResource(XdsListenerResource.getInstance(),
165+
"xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher);
166+
167+
verify(mockWatcher, timeout(10000)).onResourceChanged(any());
168+
// mockDirectPathWatcher should not receive the update sent by trafficdirector.
169+
verify(mockDirectPathWatcher, never()).onResourceChanged(any());
170+
171+
// When server-one sends its LDS config, mockDirectPathWatcher should then receive the update.
172+
directpathPa.setLdsConfig(ControlPlaneRule.buildServerListener(),
173+
ControlPlaneRule.buildClientListener(
174+
"xdstp://server-one/envoy.config.listener.v3.Listener/test-server"));
175+
176+
verify(mockDirectPathWatcher, timeout(10000)).onResourceChanged(any());
177+
}
178+
145179
/**
146180
* Assures that when an {@link XdsClient} is asked to add cluster locality stats it appropriately
147181
* starts {@link LoadReportClient}s to do that.

0 commit comments

Comments
 (0)