Skip to content

Commit 1567026

Browse files
committed
xds: Handle empty target URI authorities the same as null.
Fixes an oversight in grpc#12660. io.grpc.Uri#getAuthority makes a distinction between xds:///service and xds:/service but java.net.URI does not. XdsNameResolver wants to treat those two cases the same.
1 parent 9e92242 commit 1567026

File tree

3 files changed

+49
-2
lines changed

3 files changed

+49
-2
lines changed

xds/src/main/java/io/grpc/xds/XdsNameResolver.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,13 @@ final class XdsNameResolver extends NameResolver {
138138
private CallCounterProvider callCounterProvider;
139139
private ResolveState resolveState;
140140

141+
/**
142+
* Constructs a new instance.
143+
*
144+
* @param target the target URI to resolve
145+
* @param targetAuthority the authority component of `target`, possibly the empty string, or null
146+
* if 'target' has no such component
147+
*/
141148
XdsNameResolver(
142149
String target, @Nullable String targetAuthority, String name,
143150
@Nullable String overrideAuthority, ServiceConfigParser serviceConfigParser,
@@ -208,7 +215,9 @@ public void start(Listener2 listener) {
208215
}
209216
BootstrapInfo bootstrapInfo = xdsClient.getBootstrapInfo();
210217
String listenerNameTemplate;
211-
if (targetAuthority == null) {
218+
if (targetAuthority == null || targetAuthority.isEmpty()) {
219+
// Both https://github.com/grpc/proposal/blob/master/A27-xds-global-load-balancing.md and
220+
// A47-xds-federation.md seem to treat an empty authority the same as an undefined one.
212221
listenerNameTemplate = bootstrapInfo.clientDefaultListenerResourceNameTemplate();
213222
} else {
214223
AuthorityInfo authorityInfo = bootstrapInfo.authorities().get(targetAuthority);

xds/src/main/java/io/grpc/xds/XdsNameResolverProvider.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ public XdsNameResolver newNameResolver(URI targetUri, Args args) {
8787
targetPath,
8888
targetUri);
8989
String name = targetPath.substring(1);
90-
return newNameResolver(targetUri.toString(), targetUri.getAuthority(), name, args);
90+
// TODO(jdcormie): java.net.URI#getAuthority incorrectly returns null for both xds:///service
91+
// and xds:/service. This doesn't matter for now since XdsNameResolver treats them the same
92+
// anyway and all this code will go away once newNameResolver(io.grpc.Uri) launches.
93+
String targetAuthority = targetUri.getAuthority();
94+
return newNameResolver(targetUri.toString(), targetAuthority, name, args);
9195
}
9296
return null;
9397
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,40 @@ public void resolving_noTargetAuthority_templateWithoutXdstp() {
297297
verify(mockListener, never()).onError(any(Status.class));
298298
}
299299

300+
@Test
301+
public void resolving_emptyTargetAuthority_templateWithXdstp() {
302+
bootstrapInfo =
303+
BootstrapInfo.builder()
304+
.servers(
305+
ImmutableList.of(
306+
ServerInfo.create("td.googleapis.com", InsecureChannelCredentials.create())))
307+
.node(Node.newBuilder().build())
308+
.clientDefaultListenerResourceNameTemplate(
309+
"xdstp://xds.authority.com/envoy.config.listener.v3.Listener/%s?id=1")
310+
.build();
311+
String serviceAuthority = "[::FFFF:129.144.52.38]:80";
312+
expectedLdsResourceName =
313+
"xdstp://xds.authority.com/envoy.config.listener.v3.Listener/"
314+
+ "%5B::FFFF:129.144.52.38%5D:80?id=1";
315+
resolver =
316+
new XdsNameResolver(
317+
"xds:///foo.googleapis.com",
318+
"",
319+
serviceAuthority,
320+
null,
321+
serviceConfigParser,
322+
syncContext,
323+
scheduler,
324+
xdsClientPoolFactory,
325+
mockRandom,
326+
FilterRegistry.getDefaultRegistry(),
327+
rawBootstrap,
328+
metricRecorder,
329+
nameResolverArgs);
330+
resolver.start(mockListener);
331+
verify(mockListener, never()).onError(any(Status.class));
332+
}
333+
300334
@Test
301335
public void resolving_noTargetAuthority_templateWithXdstp() {
302336
bootstrapInfo = BootstrapInfo.builder()

0 commit comments

Comments
 (0)