Skip to content

Commit a43f387

Browse files
jdcormiejdcormie
authored
xds: Add support for RFC 3986 URIs (#12660)
1 parent e39c38b commit a43f387

File tree

4 files changed

+77
-52
lines changed

4 files changed

+77
-52
lines changed

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@
6767
import io.grpc.xds.client.XdsInitializationException;
6868
import io.grpc.xds.client.XdsLogger;
6969
import io.grpc.xds.client.XdsLogger.XdsLogLevel;
70-
import java.net.URI;
7170
import java.util.ArrayList;
7271
import java.util.Collections;
7372
import java.util.HashMap;
@@ -110,7 +109,6 @@ final class XdsNameResolver extends NameResolver {
110109
private final XdsLogger logger;
111110
@Nullable
112111
private final String targetAuthority;
113-
private final String target;
114112
private final String serviceAuthority;
115113
// Encoded version of the service authority as per
116114
// https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.
@@ -141,12 +139,12 @@ final class XdsNameResolver extends NameResolver {
141139
private ResolveState resolveState;
142140

143141
XdsNameResolver(
144-
URI targetUri, String name, @Nullable String overrideAuthority,
145-
ServiceConfigParser serviceConfigParser,
142+
String target, @Nullable String targetAuthority, String name,
143+
@Nullable String overrideAuthority, ServiceConfigParser serviceConfigParser,
146144
SynchronizationContext syncContext, ScheduledExecutorService scheduler,
147145
@Nullable Map<String, ?> bootstrapOverride,
148146
MetricRecorder metricRecorder, Args nameResolverArgs) {
149-
this(targetUri, targetUri.getAuthority(), name, overrideAuthority, serviceConfigParser,
147+
this(target, targetAuthority, name, overrideAuthority, serviceConfigParser,
150148
syncContext, scheduler,
151149
bootstrapOverride == null
152150
? SharedXdsClientPoolProvider.getDefaultProvider()
@@ -157,14 +155,13 @@ final class XdsNameResolver extends NameResolver {
157155

158156
@VisibleForTesting
159157
XdsNameResolver(
160-
URI targetUri, @Nullable String targetAuthority, String name,
158+
String target, @Nullable String targetAuthority, String name,
161159
@Nullable String overrideAuthority, ServiceConfigParser serviceConfigParser,
162160
SynchronizationContext syncContext, ScheduledExecutorService scheduler,
163161
XdsClientPoolFactory xdsClientPoolFactory, ThreadSafeRandom random,
164162
FilterRegistry filterRegistry, @Nullable Map<String, ?> bootstrapOverride,
165163
MetricRecorder metricRecorder, Args nameResolverArgs) {
166164
this.targetAuthority = targetAuthority;
167-
target = targetUri.toString();
168165

169166
// The name might have multiple slashes so encode it before verifying.
170167
serviceAuthority = checkNotNull(name, "name");

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

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.grpc.Internal;
2323
import io.grpc.NameResolver.Args;
2424
import io.grpc.NameResolverProvider;
25+
import io.grpc.Uri;
2526
import io.grpc.xds.client.XdsClient;
2627
import java.net.InetSocketAddress;
2728
import java.net.SocketAddress;
@@ -86,16 +87,39 @@ public XdsNameResolver newNameResolver(URI targetUri, Args args) {
8687
targetPath,
8788
targetUri);
8889
String name = targetPath.substring(1);
89-
return new XdsNameResolver(
90-
targetUri, name, args.getOverrideAuthority(),
91-
args.getServiceConfigParser(), args.getSynchronizationContext(),
92-
args.getScheduledExecutorService(),
93-
bootstrapOverride,
94-
args.getMetricRecorder(), args);
90+
return newNameResolver(targetUri.toString(), targetUri.getAuthority(), name, args);
9591
}
9692
return null;
9793
}
9894

95+
@Override
96+
public XdsNameResolver newNameResolver(Uri targetUri, Args args) {
97+
if (scheme.equals(targetUri.getScheme())) {
98+
Preconditions.checkArgument(
99+
targetUri.isPathAbsolute(),
100+
"the path component of the target (%s) must start with '/'",
101+
targetUri);
102+
return newNameResolver(
103+
targetUri.toString(), targetUri.getAuthority(), targetUri.getPath().substring(1), args);
104+
}
105+
return null;
106+
}
107+
108+
private XdsNameResolver newNameResolver(
109+
String targetUri, String targetAuthority, String name, Args args) {
110+
return new XdsNameResolver(
111+
targetUri.toString(),
112+
targetAuthority,
113+
name,
114+
args.getOverrideAuthority(),
115+
args.getServiceConfigParser(),
116+
args.getSynchronizationContext(),
117+
args.getScheduledExecutorService(),
118+
bootstrapOverride,
119+
args.getMetricRecorder(),
120+
args);
121+
}
122+
99123
@Override
100124
public String getDefaultScheme() {
101125
return scheme;

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

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,22 @@
2929
import io.grpc.NameResolverProvider;
3030
import io.grpc.NameResolverRegistry;
3131
import io.grpc.SynchronizationContext;
32+
import io.grpc.Uri;
3233
import io.grpc.internal.FakeClock;
3334
import io.grpc.internal.GrpcUtil;
3435
import java.net.URI;
36+
import java.util.Arrays;
3537
import java.util.Collections;
3638
import java.util.HashMap;
3739
import java.util.Map;
3840
import org.junit.Test;
3941
import org.junit.runner.RunWith;
40-
import org.junit.runners.JUnit4;
42+
import org.junit.runners.Parameterized;
43+
import org.junit.runners.Parameterized.Parameter;
44+
import org.junit.runners.Parameterized.Parameters;
4145

4246
/** Unit tests for {@link XdsNameResolverProvider}. */
43-
@RunWith(JUnit4.class)
47+
@RunWith(Parameterized.class)
4448
public class XdsNameResolverProviderTest {
4549
private final SynchronizationContext syncContext = new SynchronizationContext(
4650
new Thread.UncaughtExceptionHandler() {
@@ -63,6 +67,13 @@ public void uncaughtException(Thread t, Throwable e) {
6367

6468
private XdsNameResolverProvider provider = new XdsNameResolverProvider();
6569

70+
@Parameters(name = "enableRfc3986UrisParam={0}")
71+
public static Iterable<Object[]> data() {
72+
return Arrays.asList(new Object[][] {{true}, {false}});
73+
}
74+
75+
@Parameter public boolean enableRfc3986UrisParam;
76+
6677
@Test
6778
public void provided() {
6879
for (NameResolverProvider current
@@ -81,48 +92,46 @@ public void isAvailable() {
8192
}
8293

8394
@Test
84-
public void newNameResolver() {
85-
assertThat(
86-
provider.newNameResolver(URI.create("xds://1.1.1.1/foo.googleapis.com"), args))
95+
public void newNameResolver_returnsExpectedType() {
96+
assertThat(newNameResolver(provider, "xds://1.1.1.1/foo.googleapis.com", args))
8797
.isInstanceOf(XdsNameResolver.class);
88-
assertThat(
89-
provider.newNameResolver(URI.create("xds:///foo.googleapis.com"), args))
98+
assertThat(newNameResolver(provider, "xds:///foo.googleapis.com", args))
9099
.isInstanceOf(XdsNameResolver.class);
91-
assertThat(
92-
provider.newNameResolver(URI.create("notxds://1.1.1.1/foo.googleapis.com"),
93-
args))
94-
.isNull();
100+
}
101+
102+
@Test
103+
public void newNameResolver_matchesExpectedScheme() {
104+
assertThat(newNameResolver(provider, "notxds://1.1.1.1/foo.googleapis.com", args)).isNull();
95105
}
96106

97107
@Test
98108
public void validName_withAuthority() {
99-
XdsNameResolver resolver =
100-
provider.newNameResolver(
101-
URI.create("xds://trafficdirector.google.com/foo.googleapis.com"), args);
109+
NameResolver resolver =
110+
newNameResolver(provider, "xds://trafficdirector.google.com/foo.googleapis.com", args);
102111
assertThat(resolver).isNotNull();
103112
assertThat(resolver.getServiceAuthority()).isEqualTo("foo.googleapis.com");
104113
}
105114

106115
@Test
107116
public void validName_noAuthority() {
108-
XdsNameResolver resolver =
109-
provider.newNameResolver(URI.create("xds:///foo.googleapis.com"), args);
117+
NameResolver resolver = newNameResolver(provider, "xds:///foo.googleapis.com", args);
110118
assertThat(resolver).isNotNull();
111119
assertThat(resolver.getServiceAuthority()).isEqualTo("foo.googleapis.com");
112120
}
113121

114122
@Test
115123
public void validName_urlExtractedAuthorityInvalidWithoutEncoding() {
116-
XdsNameResolver resolver =
117-
provider.newNameResolver(URI.create("xds:///1234/path/foo.googleapis.com:8080"), args);
124+
NameResolver resolver =
125+
newNameResolver(provider, "xds:///1234/path/foo.googleapis.com:8080", args);
118126
assertThat(resolver).isNotNull();
119127
assertThat(resolver.getServiceAuthority()).isEqualTo("1234%2Fpath%2Ffoo.googleapis.com:8080");
120128
}
121129

122130
@Test
123131
public void validName_urlwithTargetAuthorityAndExtractedAuthorityInvalidWithoutEncoding() {
124-
XdsNameResolver resolver = provider.newNameResolver(URI.create(
125-
"xds://trafficdirector.google.com/1234/path/foo.googleapis.com:8080"), args);
132+
NameResolver resolver =
133+
newNameResolver(
134+
provider, "xds://trafficdirector.google.com/1234/path/foo.googleapis.com:8080", args);
126135
assertThat(resolver).isNotNull();
127136
assertThat(resolver.getServiceAuthority()).isEqualTo("1234%2Fpath%2Ffoo.googleapis.com:8080");
128137
}
@@ -135,18 +144,14 @@ public void newProvider_multipleScheme() {
135144
XdsNameResolverProvider provider1 = XdsNameResolverProvider.createForTest("new-xds-scheme",
136145
new HashMap<String, String>());
137146
registry.register(provider1);
138-
assertThat(registry.asFactory()
139-
.newNameResolver(URI.create("xds:///localhost"), args)).isNotNull();
140-
assertThat(registry.asFactory()
141-
.newNameResolver(URI.create("new-xds-scheme:///localhost"), args)).isNotNull();
142-
assertThat(registry.asFactory()
143-
.newNameResolver(URI.create("no-scheme:///localhost"), args)).isNotNull();
147+
assertThat(newNameResolver(registry.asFactory(), "xds:///localhost", args)).isNotNull();
148+
assertThat(newNameResolver(registry.asFactory(), "new-xds-scheme:///localhost", args))
149+
.isNotNull();
150+
assertThat(newNameResolver(registry.asFactory(), "no-scheme:///localhost", args)).isNotNull();
144151
registry.deregister(provider1);
145-
assertThat(registry.asFactory()
146-
.newNameResolver(URI.create("new-xds-scheme:///localhost"), args)).isNull();
152+
assertThat(newNameResolver(registry.asFactory(), "new-xds-scheme:///localhost", args)).isNull();
147153
registry.deregister(provider0);
148-
assertThat(registry.asFactory()
149-
.newNameResolver(URI.create("xds:///localhost"), args)).isNotNull();
154+
assertThat(newNameResolver(registry.asFactory(), "xds:///localhost", args)).isNotNull();
150155
}
151156

152157
@Test
@@ -176,4 +181,11 @@ public void newProvider_overrideBootstrap() {
176181
resolver.shutdown();
177182
registry.deregister(provider);
178183
}
184+
185+
private NameResolver newNameResolver(
186+
NameResolver.Factory factory, String uriString, NameResolver.Args args) {
187+
return enableRfc3986UrisParam
188+
? factory.newNameResolver(Uri.create(uriString), args)
189+
: factory.newNameResolver(URI.create(uriString), args);
190+
}
179191
}

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

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,6 @@
104104
import io.grpc.xds.client.XdsClient;
105105
import io.grpc.xds.client.XdsResourceType;
106106
import java.io.IOException;
107-
import java.net.URI;
108-
import java.net.URISyntaxException;
109107
import java.util.ArrayList;
110108
import java.util.Arrays;
111109
import java.util.Collections;
@@ -201,7 +199,7 @@ public ConfigOrError parseServiceConfig(Map<String, ?> rawServiceConfig) {
201199
private XdsNameResolver resolver;
202200
private TestCall<?, ?> testCall;
203201
private boolean originalEnableTimeout;
204-
private URI targetUri;
202+
private String targetUri = AUTHORITY;
205203
private final NameResolver.Args nameResolverArgs = NameResolver.Args.newBuilder()
206204
.setDefaultPort(8080)
207205
.setProxyDetector(GrpcUtil.DEFAULT_PROXY_DETECTOR)
@@ -216,12 +214,6 @@ public ConfigOrError parseServiceConfig(Map<String, ?> rawServiceConfig) {
216214
public void setUp() {
217215
lenient().doReturn(Status.OK).when(mockListener).onResult2(any());
218216

219-
try {
220-
targetUri = new URI(AUTHORITY);
221-
} catch (URISyntaxException e) {
222-
targetUri = null;
223-
}
224-
225217
originalEnableTimeout = XdsNameResolver.enableTimeout;
226218
XdsNameResolver.enableTimeout = true;
227219

0 commit comments

Comments
 (0)