diff --git a/api/src/test/java/io/grpc/NameResolverRegistryTest.java b/api/src/test/java/io/grpc/NameResolverRegistryTest.java index 1870f312868..abbe10af2ac 100644 --- a/api/src/test/java/io/grpc/NameResolverRegistryTest.java +++ b/api/src/test/java/io/grpc/NameResolverRegistryTest.java @@ -21,6 +21,7 @@ import static org.mockito.Mockito.mock; import io.grpc.NameResolver.ServiceConfigParser; +import io.grpc.internal.BaseDnsNameResolverProvider; import io.grpc.internal.DnsNameResolverProvider; import java.lang.Thread.UncaughtExceptionHandler; import java.net.URI; @@ -152,8 +153,11 @@ public void newNameResolver_noProvider() { @Test public void baseProviders() { List providers = NameResolverRegistry.getDefaultRegistry().providers(); - assertThat(providers).hasSize(1); - assertThat(providers.get(0)).isInstanceOf(DnsNameResolverProvider.class); + assertThat(providers).hasSize(2); + // 2 name resolvers from grpclb and core + for (NameResolverProvider provider : providers) { + assertThat(provider).isInstanceOf(BaseDnsNameResolverProvider.class); + } assertThat(NameResolverRegistry.getDefaultRegistry().asFactory().getDefaultScheme()) .isEqualTo("dns"); } diff --git a/core/src/main/java/io/grpc/internal/BaseDnsNameResolverProvider.java b/core/src/main/java/io/grpc/internal/BaseDnsNameResolverProvider.java new file mode 100644 index 00000000000..b623ced6b78 --- /dev/null +++ b/core/src/main/java/io/grpc/internal/BaseDnsNameResolverProvider.java @@ -0,0 +1,70 @@ +/* + * Copyright 2019 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.internal; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.base.Stopwatch; +import io.grpc.InternalServiceProviders; +import io.grpc.NameResolver; +import io.grpc.NameResolverProvider; +import java.net.URI; + +/** + * Base provider of name resolvers for name agnostic consumption. + */ +public abstract class BaseDnsNameResolverProvider extends NameResolverProvider { + + private static final String SCHEME = "dns"; + + @VisibleForTesting + public static final String ENABLE_GRPCLB_PROPERTY_NAME = + "io.grpc.internal.DnsNameResolverProvider.enable_grpclb"; + + /** Returns boolean value of system property {@link #ENABLE_GRPCLB_PROPERTY_NAME}. */ + protected abstract boolean isSrvEnabled(); + + @Override + public DnsNameResolver newNameResolver(URI targetUri, NameResolver.Args args) { + if (SCHEME.equals(targetUri.getScheme())) { + String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath"); + Preconditions.checkArgument(targetPath.startsWith("/"), + "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri); + String name = targetPath.substring(1); + return new DnsNameResolver( + targetUri.getAuthority(), + name, + args, + GrpcUtil.SHARED_CHANNEL_EXECUTOR, + Stopwatch.createUnstarted(), + InternalServiceProviders.isAndroid(getClass().getClassLoader()), + isSrvEnabled()); + } else { + return null; + } + } + + @Override + public String getDefaultScheme() { + return SCHEME; + } + + @Override + protected boolean isAvailable() { + return true; + } +} diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index 8e07dbdb8ec..b85dd79ccb9 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -92,8 +92,6 @@ final class DnsNameResolver extends NameResolver { System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_jndi", "true"); private static final String JNDI_LOCALHOST_PROPERTY = System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_jndi_localhost", "false"); - private static final String JNDI_SRV_PROPERTY = - System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_grpclb", "false"); private static final String JNDI_TXT_PROPERTY = System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_service_config", "false"); @@ -117,8 +115,6 @@ final class DnsNameResolver extends NameResolver { @VisibleForTesting static boolean enableJndiLocalhost = Boolean.parseBoolean(JNDI_LOCALHOST_PROPERTY); @VisibleForTesting - static boolean enableSrv = Boolean.parseBoolean(JNDI_SRV_PROPERTY); - @VisibleForTesting static boolean enableTxt = Boolean.parseBoolean(JNDI_TXT_PROPERTY); private static final ResourceResolverFactory resourceResolverFactory = @@ -152,6 +148,7 @@ final class DnsNameResolver extends NameResolver { /** True if using an executor resource that should be released after use. */ private final boolean usingExecutorResource; + private final boolean enableSrv; private boolean resolving; @@ -159,8 +156,14 @@ final class DnsNameResolver extends NameResolver { // from any thread. private NameResolver.Listener2 listener; - DnsNameResolver(@Nullable String nsAuthority, String name, Args args, - Resource executorResource, Stopwatch stopwatch, boolean isAndroid) { + DnsNameResolver( + @Nullable String nsAuthority, + String name, + Args args, + Resource executorResource, + Stopwatch stopwatch, + boolean isAndroid, + boolean enableSrv) { Preconditions.checkNotNull(args, "args"); // TODO: if a DNS server is provided as nsAuthority, use it. // https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java @@ -184,6 +187,7 @@ final class DnsNameResolver extends NameResolver { Preconditions.checkNotNull(args.getSynchronizationContext(), "syncContext"); this.executor = args.getBlockingExecutor(); this.usingExecutorResource = executor == null; + this.enableSrv = enableSrv; } @Override diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java index 67e60351631..06ff1c85953 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java @@ -16,13 +16,6 @@ package io.grpc.internal; -import com.google.common.base.Preconditions; -import com.google.common.base.Stopwatch; -import io.grpc.InternalServiceProviders; -import io.grpc.NameResolver; -import io.grpc.NameResolverProvider; -import java.net.URI; - /** * A provider for {@link DnsNameResolver}. * @@ -38,41 +31,18 @@ *
  • {@code "dns:///foo.googleapis.com"} (without port)
    • * */ -public final class DnsNameResolverProvider extends NameResolverProvider { - - private static final String SCHEME = "dns"; +public final class DnsNameResolverProvider extends BaseDnsNameResolverProvider { - @Override - public DnsNameResolver newNameResolver(URI targetUri, NameResolver.Args args) { - if (SCHEME.equals(targetUri.getScheme())) { - String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath"); - Preconditions.checkArgument(targetPath.startsWith("/"), - "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri); - String name = targetPath.substring(1); - return new DnsNameResolver( - targetUri.getAuthority(), - name, - args, - GrpcUtil.SHARED_CHANNEL_EXECUTOR, - Stopwatch.createUnstarted(), - InternalServiceProviders.isAndroid(getClass().getClassLoader())); - } else { - return null; - } - } - - @Override - public String getDefaultScheme() { - return SCHEME; - } + private static final boolean SRV_ENABLED = + Boolean.parseBoolean(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME, "false")); @Override - protected boolean isAvailable() { - return true; + protected boolean isSrvEnabled() { + return SRV_ENABLED; } @Override - protected int priority() { + public int priority() { return 5; } } diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java index 9dc402a93d6..3f5df9bdbbb 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java @@ -16,9 +16,12 @@ package io.grpc.internal; +import static com.google.common.truth.Truth.assertThat; +import static io.grpc.internal.BaseDnsNameResolverProvider.ENABLE_GRPCLB_PROPERTY_NAME; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; import static org.mockito.Mockito.mock; import io.grpc.NameResolver; @@ -60,4 +63,11 @@ public void newNameResolver() { assertNull( provider.newNameResolver(URI.create("notdns:///localhost:443"), args)); } + + @Test + public void isSrvEnabled_falseByDefault() { + assumeTrue(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME) == null); + + assertThat(provider.isSrvEnabled()).isFalse(); + } } diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index 7214db6edac..6baa87ad0ad 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -182,7 +182,8 @@ private DnsNameResolver newResolver( private DnsNameResolver newResolver( String name, Stopwatch stopwatch, boolean isAndroid, NameResolver.Args args) { DnsNameResolver dnsResolver = - new DnsNameResolver(null, name, args, fakeExecutorResource, stopwatch, isAndroid); + new DnsNameResolver( + null, name, args, fakeExecutorResource, stopwatch, isAndroid, /* enableSrv= */ false); // By default, using the mocked ResourceResolver to avoid I/O dnsResolver.setResourceResolver(new JndiResourceResolver(recordFetcher)); return dnsResolver; diff --git a/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java b/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java new file mode 100644 index 00000000000..1856c78c14e --- /dev/null +++ b/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java @@ -0,0 +1,61 @@ +/* + * Copyright 2019 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.grpclb; + +import io.grpc.internal.BaseDnsNameResolverProvider; + +/** + * A provider for {@code io.grpc.internal.DnsNameResolver} for gRPC lb. + * + *

    It resolves a target URI whose scheme is {@code "dns"}. The (optional) authority of the target + * URI is reserved for the address of alternative DNS server (not implemented yet). The path of the + * target URI, excluding the leading slash {@code '/'}, is treated as the host name and the optional + * port to be resolved by DNS. Example target URIs: + * + *

      + *
    • {@code "dns:///foo.googleapis.com:8080"} (using default DNS)
      • + *
    • {@code "dns://8.8.8.8/foo.googleapis.com:8080"} (using alternative DNS (not implemented + * yet))
      • + *
    • {@code "dns:///foo.googleapis.com"} (without port)
      • + *
    + * + *

    Note: the main difference between {@code io.grpc.DnsNameResolver} is service record is enabled + * by default. + */ +// Make it package-private so that it cannot be directly referenced by users. Java service loader +// requires the provider to be public, but we can hide it under a package-private class. +final class SecretGrpclbNameResolverProvider { + + private SecretGrpclbNameResolverProvider() {} + + public static final class Provider extends BaseDnsNameResolverProvider { + + private static final boolean SRV_ENABLED = + Boolean.parseBoolean(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME, "true")); + + @Override + protected boolean isSrvEnabled() { + return SRV_ENABLED; + } + + @Override + public int priority() { + // Must be higher than DnsNameResolverProvider#priority. + return 6; + } + } +} diff --git a/grpclb/src/main/resources/META-INF/services/io.grpc.NameResolverProvider b/grpclb/src/main/resources/META-INF/services/io.grpc.NameResolverProvider new file mode 100644 index 00000000000..bf02b5e8470 --- /dev/null +++ b/grpclb/src/main/resources/META-INF/services/io.grpc.NameResolverProvider @@ -0,0 +1 @@ +io.grpc.grpclb.SecretGrpclbNameResolverProvider$Provider diff --git a/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java b/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java new file mode 100644 index 00000000000..e5d4b3501f2 --- /dev/null +++ b/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java @@ -0,0 +1,50 @@ +/* + * Copyright 2019 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.grpclb; + +import static com.google.common.truth.Truth.assertThat; +import static io.grpc.internal.BaseDnsNameResolverProvider.ENABLE_GRPCLB_PROPERTY_NAME; +import static org.junit.Assume.assumeTrue; + +import io.grpc.internal.DnsNameResolverProvider; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class SecretGrpclbNameResolverProviderTest { + + @Test + public void priority_shouldBeHigherThanDefaultDnsNameResolver() { + DnsNameResolverProvider defaultDnsNameResolver = new DnsNameResolverProvider(); + SecretGrpclbNameResolverProvider.Provider grpclbDnsNameResolver = + new SecretGrpclbNameResolverProvider.Provider(); + + assertThat(defaultDnsNameResolver.priority()) + .isLessThan(grpclbDnsNameResolver.priority()); + } + + @Test + public void isSrvEnabled_trueByDefault() { + assumeTrue(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME) == null); + + SecretGrpclbNameResolverProvider.Provider grpclbDnsNameResolver = + new SecretGrpclbNameResolverProvider.Provider(); + + assertThat(grpclbDnsNameResolver.isSrvEnabled()).isTrue(); + } +} \ No newline at end of file