Skip to content

Support retrieving the names from wrapped resource containers. #10975

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Mar 5, 2024
Merged

Support retrieving the names from wrapped resource containers. #10975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
528c55d
Support retrieving the names from wrapped resource containers. Curren…
anicr7 Mar 1, 2024
498def1
Remove unintended addition of file
anicr7 Mar 1, 2024
1b90ed0
Add a comment describing the logic of fallback
anicr7 Mar 1, 2024
7bdb936
Fix to keep all lines within recommended limits and remove unused imp…
anicr7 Mar 1, 2024
fdb35e7
Merge branch 'master' into resource_name_wrapped
anicr7 Mar 1, 2024
2c51185
Remove unnecessary imports
anicr7 Mar 1, 2024
c4d1461
Don't check if the xDS resource name is empty, instead check if its null
anicr7 Mar 4, 2024
da82006
Fix the logic to only consider the Resource Name and Ignore the Resou…
anicr7 Mar 4, 2024
d244a96
Remove any helper functions related to Resource.ResourceName
anicr7 Mar 4, 2024
b58e8fb
Git unnecessary auto import addition
anicr7 Mar 4, 2024
d09aea9
Rename the test function and fix the logic to be more logical
anicr7 Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions xds/src/main/java/io/grpc/xds/client/XdsResourceType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,24 @@ ValidatedResourceUpdate<T> parse(Args args, List<Any> resources) {
Any resource = resources.get(i);

Message unpackedMessage;
String name = "";
try {
resource = maybeUnwrapResources(resource);
if (resource.getTypeUrl().equals(TYPE_URL_RESOURCE)) {
Resource wrappedResource = unpackCompatibleType(resource, Resource.class,
TYPE_URL_RESOURCE, null);
resource = wrappedResource.getResource();
name = wrappedResource.getName();
}
unpackedMessage = unpackCompatibleType(resource, unpackedClassName(), typeUrl(), null);
} catch (InvalidProtocolBufferException e) {
errors.add(String.format("%s response Resource index %d - can't decode %s: %s",
typeName(), i, unpackedClassName().getSimpleName(), e.getMessage()));
continue;
}
String name = extractResourceName(unpackedMessage);
// Fallback to inner resource name if the outer resource didn't have a name.
if (name.isEmpty()) {
name = extractResourceName(unpackedMessage);
}
if (name == null || !isResourceNameValid(name, resource.getTypeUrl())) {
errors.add(
"Unsupported resource name: " + name + " for type: " + typeName());
Expand Down Expand Up @@ -209,16 +218,6 @@ protected static <T extends com.google.protobuf.Message> T unpackCompatibleType(
return any.unpack(clazz);
}

private Any maybeUnwrapResources(Any resource)
throws InvalidProtocolBufferException {
if (resource.getTypeUrl().equals(TYPE_URL_RESOURCE)) {
return unpackCompatibleType(resource, Resource.class, TYPE_URL_RESOURCE,
null).getResource();
} else {
return resource;
}
}

static final class ParsedResource<T extends ResourceUpdate> {
private final T resourceUpdate;
private final Any rawResource;
Expand Down
21 changes: 21 additions & 0 deletions xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,25 @@ public void wrappedLdsResource() {
verifySubscribedResourcesMetadataSizes(1, 0, 0, 0);
}

@Test
public void wrappedLdsResource_preferWrappedName() {
DiscoveryRpcCall call = startResourceWatcher(XdsListenerResource.getInstance(), LDS_RESOURCE,
ldsResourceWatcher);

Any innerResource = Any.pack(mf.buildListenerWithApiListener("random_name" /* name */,
mf.buildRouteConfiguration("do not care", mf.buildOpaqueVirtualHosts(VHOST_SIZE))));

// Client sends an ACK LDS request.
call.sendResponse(LDS, mf.buildWrappedResourceWithName(innerResource, LDS_RESOURCE), VERSION_1,
"0000");
call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE);
verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture());
verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue());
assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty();
verifyResourceMetadataAcked(LDS, LDS_RESOURCE, innerResource, VERSION_1, TIME_INCREMENT);
verifySubscribedResourcesMetadataSizes(1, 0, 0, 0);
}

@Test
public void ldsResourceFound_containsRdsName() {
DiscoveryRpcCall call = startResourceWatcher(XdsListenerResource.getInstance(), LDS_RESOURCE,
Expand Down Expand Up @@ -3836,6 +3855,8 @@ protected abstract static class MessageFactory {

protected abstract Any buildWrappedResource(Any originalResource);

protected abstract Any buildWrappedResourceWithName(Any originalResource, String name);

protected Message buildListenerWithApiListener(String name, Message routeConfiguration) {
return buildListenerWithApiListener(
name, routeConfiguration, Collections.<Message>emptyList());
Expand Down
8 changes: 8 additions & 0 deletions xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,14 @@ protected Any buildWrappedResource(Any originalResource) {
.build());
}

@Override
protected Any buildWrappedResourceWithName(Any originalResource, String name) {
return Any.pack(Resource.newBuilder()
.setResource(originalResource)
.setName(name)
.build());
}

@SuppressWarnings("unchecked")
@Override
protected Message buildListenerWithApiListener(
Expand Down