Skip to content

Commit 867e469

Browse files
authored
xds: Support retrieving names from wrapped resource containers (#10975)
The xDS library only honored names retrieved from the inner resource containers, but for wrapped resources the outer layer could contain the required name. This commit prefers the name on the wrapped container over the inner resource name.
1 parent e5ed553 commit 867e469

File tree

3 files changed

+40
-12
lines changed

3 files changed

+40
-12
lines changed

xds/src/main/java/io/grpc/xds/client/XdsResourceType.java

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,24 @@ ValidatedResourceUpdate<T> parse(Args args, List<Any> resources) {
148148
Any resource = resources.get(i);
149149

150150
Message unpackedMessage;
151+
String name = "";
151152
try {
152-
resource = maybeUnwrapResources(resource);
153+
if (resource.getTypeUrl().equals(TYPE_URL_RESOURCE)) {
154+
Resource wrappedResource = unpackCompatibleType(resource, Resource.class,
155+
TYPE_URL_RESOURCE, null);
156+
resource = wrappedResource.getResource();
157+
name = wrappedResource.getName();
158+
}
153159
unpackedMessage = unpackCompatibleType(resource, unpackedClassName(), typeUrl(), null);
154160
} catch (InvalidProtocolBufferException e) {
155161
errors.add(String.format("%s response Resource index %d - can't decode %s: %s",
156162
typeName(), i, unpackedClassName().getSimpleName(), e.getMessage()));
157163
continue;
158164
}
159-
String name = extractResourceName(unpackedMessage);
165+
// Fallback to inner resource name if the outer resource didn't have a name.
166+
if (name.isEmpty()) {
167+
name = extractResourceName(unpackedMessage);
168+
}
160169
if (name == null || !isResourceNameValid(name, resource.getTypeUrl())) {
161170
errors.add(
162171
"Unsupported resource name: " + name + " for type: " + typeName());
@@ -209,16 +218,6 @@ protected static <T extends com.google.protobuf.Message> T unpackCompatibleType(
209218
return any.unpack(clazz);
210219
}
211220

212-
private Any maybeUnwrapResources(Any resource)
213-
throws InvalidProtocolBufferException {
214-
if (resource.getTypeUrl().equals(TYPE_URL_RESOURCE)) {
215-
return unpackCompatibleType(resource, Resource.class, TYPE_URL_RESOURCE,
216-
null).getResource();
217-
} else {
218-
return resource;
219-
}
220-
}
221-
222221
static final class ParsedResource<T extends ResourceUpdate> {
223222
private final T resourceUpdate;
224223
private final Any rawResource;

xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,25 @@ public void wrappedLdsResource() {
852852
verifySubscribedResourcesMetadataSizes(1, 0, 0, 0);
853853
}
854854

855+
@Test
856+
public void wrappedLdsResource_preferWrappedName() {
857+
DiscoveryRpcCall call = startResourceWatcher(XdsListenerResource.getInstance(), LDS_RESOURCE,
858+
ldsResourceWatcher);
859+
860+
Any innerResource = Any.pack(mf.buildListenerWithApiListener("random_name" /* name */,
861+
mf.buildRouteConfiguration("do not care", mf.buildOpaqueVirtualHosts(VHOST_SIZE))));
862+
863+
// Client sends an ACK LDS request.
864+
call.sendResponse(LDS, mf.buildWrappedResourceWithName(innerResource, LDS_RESOURCE), VERSION_1,
865+
"0000");
866+
call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE);
867+
verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture());
868+
verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue());
869+
assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty();
870+
verifyResourceMetadataAcked(LDS, LDS_RESOURCE, innerResource, VERSION_1, TIME_INCREMENT);
871+
verifySubscribedResourcesMetadataSizes(1, 0, 0, 0);
872+
}
873+
855874
@Test
856875
public void ldsResourceFound_containsRdsName() {
857876
DiscoveryRpcCall call = startResourceWatcher(XdsListenerResource.getInstance(), LDS_RESOURCE,
@@ -3836,6 +3855,8 @@ protected abstract static class MessageFactory {
38363855

38373856
protected abstract Any buildWrappedResource(Any originalResource);
38383857

3858+
protected abstract Any buildWrappedResourceWithName(Any originalResource, String name);
3859+
38393860
protected Message buildListenerWithApiListener(String name, Message routeConfiguration) {
38403861
return buildListenerWithApiListener(
38413862
name, routeConfiguration, Collections.<Message>emptyList());

xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,14 @@ protected Any buildWrappedResource(Any originalResource) {
290290
.build());
291291
}
292292

293+
@Override
294+
protected Any buildWrappedResourceWithName(Any originalResource, String name) {
295+
return Any.pack(Resource.newBuilder()
296+
.setResource(originalResource)
297+
.setName(name)
298+
.build());
299+
}
300+
293301
@SuppressWarnings("unchecked")
294302
@Override
295303
protected Message buildListenerWithApiListener(

0 commit comments

Comments
 (0)