xds: Add support for RFC 3986 URIs

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

@@ -67,7 +67,6 @@
 import io.grpc.xds.client.XdsInitializationException;
 import io.grpc.xds.client.XdsLogger;
 import io.grpc.xds.client.XdsLogger.XdsLogLevel;
-import java.net.URI;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -109,7 +108,6 @@ final class XdsNameResolver extends NameResolver {
   private final XdsLogger logger;
   @Nullable
   private final String targetAuthority;
-  private final String target;
   private final String serviceAuthority;
   // Encoded version of the service authority as per 
   // https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.
@@ -140,12 +138,12 @@ final class XdsNameResolver extends NameResolver {
   private ResolveState resolveState;
 
   XdsNameResolver(
-      URI targetUri, String name, @Nullable String overrideAuthority,
-      ServiceConfigParser serviceConfigParser,
+      String target, @Nullable String targetAuthority, String name,
+      @Nullable String overrideAuthority, ServiceConfigParser serviceConfigParser,
       SynchronizationContext syncContext, ScheduledExecutorService scheduler,
       @Nullable Map<String, ?> bootstrapOverride,
       MetricRecorder metricRecorder, Args nameResolverArgs) {
-    this(targetUri, targetUri.getAuthority(), name, overrideAuthority, serviceConfigParser,
+    this(target, targetAuthority, name, overrideAuthority, serviceConfigParser,
         syncContext, scheduler,
         bootstrapOverride == null
           ? SharedXdsClientPoolProvider.getDefaultProvider()
@@ -156,14 +154,13 @@ final class XdsNameResolver extends NameResolver {
 
   @VisibleForTesting
   XdsNameResolver(
-      URI targetUri, @Nullable String targetAuthority, String name,
+      String target, @Nullable String targetAuthority, String name,
       @Nullable String overrideAuthority, ServiceConfigParser serviceConfigParser,
       SynchronizationContext syncContext, ScheduledExecutorService scheduler,
       XdsClientPoolFactory xdsClientPoolFactory, ThreadSafeRandom random,
       FilterRegistry filterRegistry, @Nullable Map<String, ?> bootstrapOverride,
       MetricRecorder metricRecorder, Args nameResolverArgs) {
     this.targetAuthority = targetAuthority;
-    target = targetUri.toString();
 
     // The name might have multiple slashes so encode it before verifying.
     serviceAuthority = checkNotNull(name, "name");

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

@@ -22,6 +22,7 @@
 import io.grpc.Internal;
 import io.grpc.NameResolver.Args;
 import io.grpc.NameResolverProvider;
+import io.grpc.Uri;
 import io.grpc.xds.client.XdsClient;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
@@ -86,16 +87,39 @@ public XdsNameResolver newNameResolver(URI targetUri, Args args) {
           targetPath,
           targetUri);
       String name = targetPath.substring(1);
-      return new XdsNameResolver(
-          targetUri, name, args.getOverrideAuthority(),
-          args.getServiceConfigParser(), args.getSynchronizationContext(),
-          args.getScheduledExecutorService(),
-          bootstrapOverride,
-          args.getMetricRecorder(), args);
+      return newNameResolver(targetUri.toString(), targetUri.getAuthority(), name, args);
     }
     return null;
   }
 
+  @Override
+  public XdsNameResolver newNameResolver(Uri targetUri, Args args) {
+    if (scheme.equals(targetUri.getScheme())) {
+      Preconditions.checkArgument(
+          targetUri.isPathAbsolute(),
+          "the path component of the target (%s) must start with '/'",
+          targetUri);
+      return newNameResolver(
+          targetUri.toString(), targetUri.getAuthority(), targetUri.getPath().substring(1), args);
+    }
+    return null;
+  }
+
+  private XdsNameResolver newNameResolver(
+      String targetUri, String targetAuthority, String name, Args args) {
+    return new XdsNameResolver(
+        targetUri.toString(),
+        targetAuthority,
+        name,
+        args.getOverrideAuthority(),
+        args.getServiceConfigParser(),
+        args.getSynchronizationContext(),
+        args.getScheduledExecutorService(),
+        bootstrapOverride,
+        args.getMetricRecorder(),
+        args);
+  }
+
   @Override
   public String getDefaultScheme() {
     return scheme;

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

@@ -29,18 +29,22 @@
 import io.grpc.NameResolverProvider;
 import io.grpc.NameResolverRegistry;
 import io.grpc.SynchronizationContext;
+import io.grpc.Uri;
 import io.grpc.internal.FakeClock;
 import io.grpc.internal.GrpcUtil;
 import java.net.URI;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
 /** Unit tests for {@link XdsNameResolverProvider}. */
-@RunWith(JUnit4.class)
+@RunWith(Parameterized.class)
 public class XdsNameResolverProviderTest {
   private final SynchronizationContext syncContext = new SynchronizationContext(
       new Thread.UncaughtExceptionHandler() {
@@ -63,6 +67,13 @@ public void uncaughtException(Thread t, Throwable e) {
 
   private XdsNameResolverProvider provider = new XdsNameResolverProvider();
 
+  @Parameters(name = "enableRfc3986UrisParam={0}")
+  public static Iterable<Object[]> data() {
+    return Arrays.asList(new Object[][] {{true}, {false}});
+  }
+
+  @Parameter public boolean enableRfc3986UrisParam;
+
   @Test
   public void provided() {
     for (NameResolverProvider current
@@ -81,48 +92,46 @@ public void isAvailable() {
   }
 
   @Test
-  public void newNameResolver() {
-    assertThat(
-        provider.newNameResolver(URI.create("xds://1.1.1.1/foo.googleapis.com"), args))
+  public void newNameResolver_returnsExpectedType() {
+    assertThat(newNameResolver(provider, "xds://1.1.1.1/foo.googleapis.com", args))
         .isInstanceOf(XdsNameResolver.class);
-    assertThat(
-        provider.newNameResolver(URI.create("xds:///foo.googleapis.com"), args))
+    assertThat(newNameResolver(provider, "xds:///foo.googleapis.com", args))
         .isInstanceOf(XdsNameResolver.class);
-    assertThat(
-        provider.newNameResolver(URI.create("notxds://1.1.1.1/foo.googleapis.com"),
-            args))
-        .isNull();
+  }
+
+  @Test
+  public void newNameResolver_matchesExpectedScheme() {
+    assertThat(newNameResolver(provider, "notxds://1.1.1.1/foo.googleapis.com", args)).isNull();
   }
 
   @Test
   public void validName_withAuthority() {
-    XdsNameResolver resolver =
-        provider.newNameResolver(
-            URI.create("xds://trafficdirector.google.com/foo.googleapis.com"), args);
+    NameResolver resolver =
+        newNameResolver(provider, "xds://trafficdirector.google.com/foo.googleapis.com", args);
     assertThat(resolver).isNotNull();
     assertThat(resolver.getServiceAuthority()).isEqualTo("foo.googleapis.com");
   }
 
   @Test
   public void validName_noAuthority() {
-    XdsNameResolver resolver =
-        provider.newNameResolver(URI.create("xds:///foo.googleapis.com"), args);
+    NameResolver resolver = newNameResolver(provider, "xds:///foo.googleapis.com", args);
     assertThat(resolver).isNotNull();
     assertThat(resolver.getServiceAuthority()).isEqualTo("foo.googleapis.com");
   }
 
   @Test
   public void validName_urlExtractedAuthorityInvalidWithoutEncoding() {
-    XdsNameResolver resolver = 
-        provider.newNameResolver(URI.create("xds:///1234/path/foo.googleapis.com:8080"), args);
+    NameResolver resolver =
+        newNameResolver(provider, "xds:///1234/path/foo.googleapis.com:8080", args);
     assertThat(resolver).isNotNull();
     assertThat(resolver.getServiceAuthority()).isEqualTo("1234%2Fpath%2Ffoo.googleapis.com:8080");
   }
 
   @Test
   public void validName_urlwithTargetAuthorityAndExtractedAuthorityInvalidWithoutEncoding() {
-    XdsNameResolver resolver = provider.newNameResolver(URI.create(
-        "xds://trafficdirector.google.com/1234/path/foo.googleapis.com:8080"), args);
+    NameResolver resolver =
+        newNameResolver(
+            provider, "xds://trafficdirector.google.com/1234/path/foo.googleapis.com:8080", args);
     assertThat(resolver).isNotNull();
     assertThat(resolver.getServiceAuthority()).isEqualTo("1234%2Fpath%2Ffoo.googleapis.com:8080");
   }
@@ -135,18 +144,14 @@ public void newProvider_multipleScheme() {
     XdsNameResolverProvider provider1 = XdsNameResolverProvider.createForTest("new-xds-scheme",
             new HashMap<String, String>());
     registry.register(provider1);
-    assertThat(registry.asFactory()
-            .newNameResolver(URI.create("xds:///localhost"), args)).isNotNull();
-    assertThat(registry.asFactory()
-            .newNameResolver(URI.create("new-xds-scheme:///localhost"), args)).isNotNull();
-    assertThat(registry.asFactory()
-            .newNameResolver(URI.create("no-scheme:///localhost"), args)).isNotNull();
+    assertThat(newNameResolver(registry.asFactory(), "xds:///localhost", args)).isNotNull();
+    assertThat(newNameResolver(registry.asFactory(), "new-xds-scheme:///localhost", args))
+        .isNotNull();
+    assertThat(newNameResolver(registry.asFactory(), "no-scheme:///localhost", args)).isNotNull();
     registry.deregister(provider1);
-    assertThat(registry.asFactory()
-            .newNameResolver(URI.create("new-xds-scheme:///localhost"), args)).isNull();
+    assertThat(newNameResolver(registry.asFactory(), "new-xds-scheme:///localhost", args)).isNull();
     registry.deregister(provider0);
-    assertThat(registry.asFactory()
-            .newNameResolver(URI.create("xds:///localhost"), args)).isNotNull();
+    assertThat(newNameResolver(registry.asFactory(), "xds:///localhost", args)).isNotNull();
   }
 
   @Test
@@ -176,4 +181,11 @@ public void newProvider_overrideBootstrap() {
     resolver.shutdown();
     registry.deregister(provider);
   }
+
+  private NameResolver newNameResolver(
+      NameResolver.Factory factory, String uriString, NameResolver.Args args) {
+    return enableRfc3986UrisParam
+        ? factory.newNameResolver(Uri.create(uriString), args)
+        : factory.newNameResolver(URI.create(uriString), args);
+  }
 }

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

@@ -104,8 +104,6 @@
 import io.grpc.xds.client.XdsClient;
 import io.grpc.xds.client.XdsResourceType;
 import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -201,7 +199,7 @@ public ConfigOrError parseServiceConfig(Map<String, ?> rawServiceConfig) {
   private XdsNameResolver resolver;
   private TestCall<?, ?> testCall;
   private boolean originalEnableTimeout;
-  private URI targetUri;
+  private String targetUri = AUTHORITY;
   private final NameResolver.Args nameResolverArgs = NameResolver.Args.newBuilder()
       .setDefaultPort(8080)
       .setProxyDetector(GrpcUtil.DEFAULT_PROXY_DETECTOR)
@@ -216,12 +214,6 @@ public ConfigOrError parseServiceConfig(Map<String, ?> rawServiceConfig) {
   public void setUp() {
     lenient().doReturn(Status.OK).when(mockListener).onResult2(any());
 
-    try {
-      targetUri = new URI(AUTHORITY);
-    } catch (URISyntaxException e) {
-      targetUri = null;
-    }
-
     originalEnableTimeout = XdsNameResolver.enableTimeout;
     XdsNameResolver.enableTimeout = true;