diff --git a/api/src/test/java/io/grpc/NameResolverRegistryTest.java b/api/src/test/java/io/grpc/NameResolverRegistryTest.java index f35f2f00b9..6002dcdaf5 100644 --- a/api/src/test/java/io/grpc/NameResolverRegistryTest.java +++ b/api/src/test/java/io/grpc/NameResolverRegistryTest.java @@ -21,7 +21,6 @@ import static org.junit.Assert.fail; 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; @@ -155,10 +154,11 @@ public class NameResolverRegistryTest { public void baseProviders() { List providers = NameResolverRegistry.getDefaultRegistry().providers(); assertThat(providers).hasSize(2); - // 2 name resolvers from grpclb and core - for (NameResolverProvider provider : providers) { - assertThat(provider).isInstanceOf(BaseDnsNameResolverProvider.class); - } + // 2 name resolvers from grpclb and core, ordered with decreasing priorities. + assertThat(providers.get(0).getClass().getName()) + .isEqualTo("io.grpc.grpclb.SecretGrpclbNameResolverProvider$Provider"); + assertThat(providers.get(1).getClass().getName()) + .isEqualTo("io.grpc.internal.DnsNameResolverProvider"); 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 deleted file mode 100644 index b623ced6b7..0000000000 --- a/core/src/main/java/io/grpc/internal/BaseDnsNameResolverProvider.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * 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 2b5abf0a19..7241b31e7c 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; +import com.google.common.base.Objects; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; import com.google.common.base.Throwables; @@ -63,7 +64,7 @@ import javax.annotation.Nullable; * * @see DnsNameResolverProvider */ -final class DnsNameResolver extends NameResolver { +public class DnsNameResolver extends NameResolver { private static final Logger logger = Logger.getLogger(DnsNameResolver.class.getName()); @@ -85,8 +86,6 @@ final class DnsNameResolver extends NameResolver { // From https://github.com/grpc/proposal/blob/master/A2-service-configs-in-dns.md private static final String SERVICE_CONFIG_NAME_PREFIX = "_grpc_config."; - // From https://github.com/grpc/proposal/blob/master/A5-grpclb-in-dns.md - private static final String GRPCLB_NAME_PREFIX = "_grpclb._tcp."; private static final String JNDI_PROPERTY = System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_jndi", "true"); @@ -115,7 +114,7 @@ final class DnsNameResolver extends NameResolver { @VisibleForTesting static boolean enableJndiLocalhost = Boolean.parseBoolean(JNDI_LOCALHOST_PROPERTY); @VisibleForTesting - static boolean enableTxt = Boolean.parseBoolean(JNDI_TXT_PROPERTY); + protected static boolean enableTxt = Boolean.parseBoolean(JNDI_TXT_PROPERTY); private static final ResourceResolverFactory resourceResolverFactory = getResourceResolverFactory(DnsNameResolver.class.getClassLoader()); @@ -128,7 +127,7 @@ final class DnsNameResolver extends NameResolver { private final Random random = new Random(); - private volatile AddressResolver addressResolver = JdkAddressResolver.INSTANCE; + protected volatile AddressResolver addressResolver = JdkAddressResolver.INSTANCE; private final AtomicReference resourceResolver = new AtomicReference<>(); private final String authority; @@ -142,13 +141,12 @@ final class DnsNameResolver extends NameResolver { // Following fields must be accessed from syncContext private final Stopwatch stopwatch; - private ResolutionResults cachedResolutionResults; + protected boolean resolved; private boolean shutdown; private Executor executor; /** True if using an executor resource that should be released after use. */ private final boolean usingExecutorResource; - private final boolean enableSrv; private final ServiceConfigParser serviceConfigParser; private boolean resolving; @@ -157,14 +155,13 @@ final class DnsNameResolver extends NameResolver { // from any thread. private NameResolver.Listener2 listener; - DnsNameResolver( + protected DnsNameResolver( @Nullable String nsAuthority, String name, Args args, Resource executorResource, Stopwatch stopwatch, - boolean isAndroid, - boolean enableSrv) { + boolean isAndroid) { 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 @@ -187,7 +184,6 @@ final class DnsNameResolver extends NameResolver { this.syncContext = checkNotNull(args.getSynchronizationContext(), "syncContext"); this.executor = args.getOffloadExecutor(); this.usingExecutorResource = executor == null; - this.enableSrv = enableSrv; this.serviceConfigParser = checkNotNull(args.getServiceConfigParser(), "serviceConfigParser"); } @@ -196,6 +192,11 @@ final class DnsNameResolver extends NameResolver { return authority; } + @VisibleForTesting + protected String getHost() { + return host; + } + @Override public void start(Listener2 listener) { Preconditions.checkState(this.listener == null, "already started"); @@ -212,6 +213,87 @@ final class DnsNameResolver extends NameResolver { resolve(); } + private List resolveAddresses() { + List addresses; + Exception addressesException = null; + try { + addresses = addressResolver.resolveAddress(host); + } catch (Exception e) { + addressesException = e; + Throwables.throwIfUnchecked(e); + throw new RuntimeException(e); + } finally { + if (addressesException != null) { + logger.log(Level.FINE, "Address resolution failure", addressesException); + } + } + // Each address forms an EAG + List servers = new ArrayList<>(addresses.size()); + for (InetAddress inetAddr : addresses) { + servers.add(new EquivalentAddressGroup(new InetSocketAddress(inetAddr, port))); + } + return Collections.unmodifiableList(servers); + } + + @Nullable + private ConfigOrError resolveServiceConfig() { + List txtRecords = Collections.emptyList(); + ResourceResolver resourceResolver = getResourceResolver(); + if (resourceResolver != null) { + try { + txtRecords = resourceResolver.resolveTxt(SERVICE_CONFIG_NAME_PREFIX + host); + } catch (Exception e) { + logger.log(Level.FINE, "ServiceConfig resolution failure", e); + } + } + if (!txtRecords.isEmpty()) { + ConfigOrError rawServiceConfig = parseServiceConfig(txtRecords, random, getLocalHostname()); + if (rawServiceConfig != null) { + if (rawServiceConfig.getError() != null) { + return ConfigOrError.fromError(rawServiceConfig.getError()); + } + + @SuppressWarnings("unchecked") + Map verifiedRawServiceConfig = (Map) rawServiceConfig.getConfig(); + return serviceConfigParser.parseServiceConfig(verifiedRawServiceConfig); + } + } else { + logger.log(Level.FINE, "No TXT records found for {0}", new Object[]{host}); + } + return null; + } + + @Nullable + private EquivalentAddressGroup detectProxy() throws IOException { + InetSocketAddress destination = + InetSocketAddress.createUnresolved(host, port); + ProxiedSocketAddress proxiedAddr = proxyDetector.proxyFor(destination); + if (proxiedAddr != null) { + return new EquivalentAddressGroup(proxiedAddr); + } + return null; + } + + /** + * Main logic of name resolution. + */ + protected InternalResolutionResult doResolve(boolean forceTxt) { + InternalResolutionResult result = new InternalResolutionResult(); + try { + result.addresses = resolveAddresses(); + } catch (Exception e) { + if (!forceTxt) { + result.error = + Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e); + return result; + } + } + if (enableTxt) { + result.config = resolveServiceConfig(); + } + return result; + } + private final class Resolve implements Runnable { private final Listener2 savedListener; @@ -224,105 +306,50 @@ final class DnsNameResolver extends NameResolver { if (logger.isLoggable(Level.FINER)) { logger.finer("Attempting DNS resolution of " + host); } + InternalResolutionResult result = null; try { - resolveInternal(); - } finally { - syncContext.execute(new Runnable() { - @Override - public void run() { - resolving = false; - } - }); - } - } - - @VisibleForTesting - @SuppressWarnings("deprecation") // can migrate after service config error handling is finished - void resolveInternal() { - InetSocketAddress destination = - InetSocketAddress.createUnresolved(host, port); - ProxiedSocketAddress proxiedAddr; - try { - proxiedAddr = proxyDetector.proxyFor(destination); + EquivalentAddressGroup proxiedAddr = detectProxy(); + ResolutionResult.Builder resolutionResultBuilder = ResolutionResult.newBuilder(); + if (proxiedAddr != null) { + if (logger.isLoggable(Level.FINER)) { + logger.finer("Using proxy address " + proxiedAddr); + } + resolutionResultBuilder.setAddresses(Collections.singletonList(proxiedAddr)); + } else { + result = doResolve(false); + if (result.error != null) { + savedListener.onError(result.error); + return; + } + if (result.addresses != null) { + resolutionResultBuilder.setAddresses(result.addresses); + } + if (result.config != null) { + resolutionResultBuilder.setServiceConfig(result.config); + } + if (result.attributes != null) { + resolutionResultBuilder.setAttributes(result.attributes); + } + } + savedListener.onResult(resolutionResultBuilder.build()); } catch (IOException e) { savedListener.onError( Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e)); - return; - } - if (proxiedAddr != null) { - if (logger.isLoggable(Level.FINER)) { - logger.finer("Using proxy address " + proxiedAddr); - } - EquivalentAddressGroup server = new EquivalentAddressGroup(proxiedAddr); - ResolutionResult resolutionResult = - ResolutionResult.newBuilder() - .setAddresses(Collections.singletonList(server)) - .setAttributes(Attributes.EMPTY) - .build(); - savedListener.onResult(resolutionResult); - return; - } - - ResolutionResults resolutionResults; - try { - ResourceResolver resourceResolver = null; - if (shouldUseJndi(enableJndi, enableJndiLocalhost, host)) { - resourceResolver = getResourceResolver(); - } - final ResolutionResults results = resolveAll( - addressResolver, - resourceResolver, - enableSrv, - enableTxt, - host); - resolutionResults = results; + } finally { + final boolean succeed = result != null && result.error == null; syncContext.execute(new Runnable() { - @Override - public void run() { - cachedResolutionResults = results; + @Override + public void run() { + if (succeed) { + resolved = true; if (cacheTtlNanos > 0) { stopwatch.reset().start(); } } - }); - if (logger.isLoggable(Level.FINER)) { - logger.finer("Found DNS results " + resolutionResults + " for " + host); - } - } catch (Exception e) { - savedListener.onError( - Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e)); - return; - } - // Each address forms an EAG - List servers = new ArrayList<>(); - for (InetAddress inetAddr : resolutionResults.addresses) { - servers.add(new EquivalentAddressGroup(new InetSocketAddress(inetAddr, port))); - } - - ResolutionResult.Builder resultBuilder = ResolutionResult.newBuilder().setAddresses(servers); - Attributes.Builder attributesBuilder = Attributes.newBuilder(); - if (!resolutionResults.balancerAddresses.isEmpty()) { - attributesBuilder.set(GrpcAttributes.ATTR_LB_ADDRS, resolutionResults.balancerAddresses); - } - if (!resolutionResults.txtRecords.isEmpty()) { - ConfigOrError rawServiceConfig = - parseServiceConfig(resolutionResults.txtRecords, random, getLocalHostname()); - if (rawServiceConfig != null) { - if (rawServiceConfig.getError() != null) { - savedListener.onError(rawServiceConfig.getError()); - return; + resolving = false; } - - @SuppressWarnings("unchecked") - Map verifiedRawServiceConfig = (Map) rawServiceConfig.getConfig(); - ConfigOrError parsedServiceConfig = - serviceConfigParser.parseServiceConfig(verifiedRawServiceConfig); - resultBuilder.setServiceConfig(parsedServiceConfig); - } - } else { - logger.log(Level.FINE, "No TXT records found for {0}", new Object[]{host}); + }); } - savedListener.onResult(resultBuilder.setAttributes(attributesBuilder.build()).build()); } } @@ -364,7 +391,7 @@ final class DnsNameResolver extends NameResolver { } private boolean cacheRefreshRequired() { - return cachedResolutionResults == null + return !resolved || cacheTtlNanos == 0 || (cacheTtlNanos > 0 && stopwatch.elapsed(TimeUnit.NANOSECONDS) > cacheTtlNanos); } @@ -384,69 +411,6 @@ final class DnsNameResolver extends NameResolver { return port; } - @VisibleForTesting - static ResolutionResults resolveAll( - AddressResolver addressResolver, - @Nullable ResourceResolver resourceResolver, - boolean requestSrvRecords, - boolean requestTxtRecords, - String name) { - List addresses = Collections.emptyList(); - Exception addressesException = null; - List balancerAddresses = Collections.emptyList(); - Exception balancerAddressesException = null; - List txtRecords = Collections.emptyList(); - Exception txtRecordsException = null; - - try { - addresses = addressResolver.resolveAddress(name); - } catch (Exception e) { - addressesException = e; - } - if (resourceResolver != null) { - if (requestSrvRecords) { - try { - balancerAddresses = - resourceResolver.resolveSrv(addressResolver, GRPCLB_NAME_PREFIX + name); - } catch (Exception e) { - balancerAddressesException = e; - } - } - if (requestTxtRecords) { - boolean balancerLookupFailedOrNotAttempted = - !requestSrvRecords || balancerAddressesException != null; - boolean dontResolveTxt = - (addressesException != null) && balancerLookupFailedOrNotAttempted; - // Only do the TXT record lookup if one of the above address resolutions succeeded. - if (!dontResolveTxt) { - try { - txtRecords = resourceResolver.resolveTxt(SERVICE_CONFIG_NAME_PREFIX + name); - } catch (Exception e) { - txtRecordsException = e; - } - } - } - } - try { - if (addressesException != null - && (balancerAddressesException != null || balancerAddresses.isEmpty())) { - Throwables.throwIfUnchecked(addressesException); - throw new RuntimeException(addressesException); - } - } finally { - if (addressesException != null) { - logger.log(Level.FINE, "Address resolution failure", addressesException); - } - if (balancerAddressesException != null) { - logger.log(Level.FINE, "Balancer resolution failure", balancerAddressesException); - } - if (txtRecordsException != null) { - logger.log(Level.FINE, "ServiceConfig resolution failure", txtRecordsException); - } - } - return new ResolutionResults(addresses, txtRecords, balancerAddresses); - } - /** * * @throws IOException if one of the txt records contains improperly formatted JSON. @@ -572,41 +536,64 @@ final class DnsNameResolver extends NameResolver { } /** - * Describes the results from a DNS query. + * Used as a DNS-based name resolver's internal representation of resolution result. + */ + protected static final class InternalResolutionResult { + private Status error; + private List addresses; + private ConfigOrError config; + public Attributes attributes; + + private InternalResolutionResult() {} + } + + /** + * Describes a parsed SRV record. */ @VisibleForTesting - static final class ResolutionResults { - final List addresses; - final List txtRecords; - final List balancerAddresses; + public static final class SrvRecord { + public final String host; + public final int port; - ResolutionResults( - List addresses, - List txtRecords, - List balancerAddresses) { - this.addresses = Collections.unmodifiableList(checkNotNull(addresses, "addresses")); - this.txtRecords = Collections.unmodifiableList(checkNotNull(txtRecords, "txtRecords")); - this.balancerAddresses = - Collections.unmodifiableList(checkNotNull(balancerAddresses, "balancerAddresses")); + public SrvRecord(String host, int port) { + this.host = host; + this.port = port; + } + + @Override + public int hashCode() { + return Objects.hashCode(host, port); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + SrvRecord that = (SrvRecord) obj; + return port == that.port && host.equals(that.host); } @Override public String toString() { - return MoreObjects.toStringHelper(this) - .add("addresses", addresses) - .add("txtRecords", txtRecords) - .add("balancerAddresses", balancerAddresses) - .toString(); + return + MoreObjects.toStringHelper(this) + .add("host", host) + .add("port", port) + .toString(); } } @VisibleForTesting - void setAddressResolver(AddressResolver addressResolver) { + protected void setAddressResolver(AddressResolver addressResolver) { this.addressResolver = addressResolver; } @VisibleForTesting - void setResourceResolver(ResourceResolver resourceResolver) { + protected void setResourceResolver(ResourceResolver resourceResolver) { this.resourceResolver.set(resourceResolver); } @@ -632,7 +619,8 @@ final class DnsNameResolver extends NameResolver { /** * AddressResolver resolves a hostname into a list of addresses. */ - interface AddressResolver { + @VisibleForTesting + public interface AddressResolver { List resolveAddress(String host) throws Exception; } @@ -648,15 +636,18 @@ final class DnsNameResolver extends NameResolver { /** * {@link ResourceResolver} is a Dns ResourceRecord resolver. */ - interface ResourceResolver { + @VisibleForTesting + public interface ResourceResolver { List resolveTxt(String host) throws Exception; - List resolveSrv( - AddressResolver addressResolver, String host) throws Exception; + List resolveSrv(String host) throws Exception; } @Nullable - private ResourceResolver getResourceResolver() { + protected ResourceResolver getResourceResolver() { + if (!shouldUseJndi(enableJndi, enableJndiLocalhost, host)) { + return null; + } ResourceResolver rr; if ((rr = resourceResolver.get()) == null) { if (resourceResolverFactory != null) { @@ -724,7 +715,8 @@ final class DnsNameResolver extends NameResolver { } @VisibleForTesting - static boolean shouldUseJndi(boolean jndiEnabled, boolean jndiLocalhostEnabled, String target) { + protected static boolean shouldUseJndi( + boolean jndiEnabled, boolean jndiLocalhostEnabled, String target) { if (!jndiEnabled) { return false; } diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java index 06ff1c8595..1c9290d2fc 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java @@ -16,6 +16,13 @@ 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}. * @@ -31,14 +38,37 @@ package io.grpc.internal; *
  • {@code "dns:///foo.googleapis.com"} (without port)
    • * */ -public final class DnsNameResolverProvider extends BaseDnsNameResolverProvider { +public final class DnsNameResolverProvider extends NameResolverProvider { - private static final boolean SRV_ENABLED = - Boolean.parseBoolean(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME, "false")); + private static final String SCHEME = "dns"; @Override - protected boolean isSrvEnabled() { - return SRV_ENABLED; + 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; + } + + @Override + protected boolean isAvailable() { + return true; } @Override diff --git a/core/src/main/java/io/grpc/internal/GrpcAttributes.java b/core/src/main/java/io/grpc/internal/GrpcAttributes.java index 5d11238835..0887466b10 100644 --- a/core/src/main/java/io/grpc/internal/GrpcAttributes.java +++ b/core/src/main/java/io/grpc/internal/GrpcAttributes.java @@ -19,24 +19,12 @@ package io.grpc.internal; import io.grpc.Attributes; import io.grpc.EquivalentAddressGroup; import io.grpc.Grpc; -import io.grpc.NameResolver; import io.grpc.SecurityLevel; -import java.util.List; /** * Special attributes that are only useful to gRPC. */ public final class GrpcAttributes { - /** - * Attribute key for gRPC LB server addresses. - * - *

    Deprecated: this will be used for grpclb specific logic, which will be moved out of core. - */ - @Deprecated - @NameResolver.ResolutionResultAttr - public static final Attributes.Key> ATTR_LB_ADDRS = - Attributes.Key.create("io.grpc.grpclb.lbAddrs"); - /** * The naming authority of a gRPC LB server address. It is an address-group-level attribute, * present when the address group is a LoadBalancer. diff --git a/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java b/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java index 346ac1534c..c9cb567b9c 100644 --- a/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java +++ b/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java @@ -19,14 +19,8 @@ package io.grpc.internal; import android.annotation.SuppressLint; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Verify; -import io.grpc.Attributes; -import io.grpc.EquivalentAddressGroup; -import io.grpc.internal.DnsNameResolver.AddressResolver; import io.grpc.internal.DnsNameResolver.ResourceResolver; -import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.net.SocketAddress; -import java.net.UnknownHostException; +import io.grpc.internal.DnsNameResolver.SrvRecord; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -129,82 +123,42 @@ final class JndiResourceResolverFactory implements DnsNameResolver.ResourceResol return Collections.unmodifiableList(serviceConfigTxtRecords); } - @SuppressWarnings("deprecation") @Override - public List resolveSrv( - AddressResolver addressResolver, String grpclbHostname) throws Exception { + public List resolveSrv(String host) throws Exception { if (logger.isLoggable(Level.FINER)) { logger.log( - Level.FINER, "About to query SRV records for {0}", new Object[]{grpclbHostname}); + Level.FINER, "About to query SRV records for {0}", new Object[]{host}); } - List grpclbSrvRecords = - recordFetcher.getAllRecords("SRV", "dns:///" + grpclbHostname); + List rawSrvRecords = + recordFetcher.getAllRecords("SRV", "dns:///" + host); if (logger.isLoggable(Level.FINER)) { logger.log( - Level.FINER, "Found {0} SRV records", new Object[]{grpclbSrvRecords.size()}); + Level.FINER, "Found {0} SRV records", new Object[]{rawSrvRecords.size()}); } - List balancerAddresses = - new ArrayList<>(grpclbSrvRecords.size()); + List srvRecords = new ArrayList<>(rawSrvRecords.size()); Exception first = null; Level level = Level.WARNING; - for (String srvRecord : grpclbSrvRecords) { + for (String rawSrv : rawSrvRecords) { try { - SrvRecord record = parseSrvRecord(srvRecord); + String[] parts = whitespace.split(rawSrv); + Verify.verify(parts.length == 4, "Bad SRV Record: %s", rawSrv); // SRV requires the host name to be absolute - if (!record.host.endsWith(".")) { - throw new RuntimeException("Returned SRV host does not end in period: " + record.host); - } - - // Strip trailing dot for appearance's sake. It _should_ be fine either way, but most - // people expect to see it without the dot. - String authority = record.host.substring(0, record.host.length() - 1); - // But we want to use the trailing dot for the IP lookup. The dot makes the name absolute - // instead of relative and so will avoid the search list like that in resolv.conf. - List addrs = addressResolver.resolveAddress(record.host); - List sockaddrs = new ArrayList<>(addrs.size()); - for (InetAddress addr : addrs) { - sockaddrs.add(new InetSocketAddress(addr, record.port)); - } - Attributes attrs = Attributes.newBuilder() - .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, authority) - .build(); - balancerAddresses.add( - new EquivalentAddressGroup(Collections.unmodifiableList(sockaddrs), attrs)); - } catch (UnknownHostException e) { - logger.log(level, "Can't find address for SRV record " + srvRecord, e); - // TODO(carl-mastrangelo): these should be added by addSuppressed when we have Java 7. - if (first == null) { - first = e; - level = Level.FINE; + if (!parts[3].endsWith(".")) { + throw new RuntimeException("Returned SRV host does not end in period: " + parts[3]); } + srvRecords.add(new SrvRecord(parts[3], Integer.parseInt(parts[2]))); } catch (RuntimeException e) { - logger.log(level, "Failed to construct SRV record " + srvRecord, e); + logger.log(level, "Failed to construct SRV record " + rawSrv, e); if (first == null) { first = e; level = Level.FINE; } } } - if (balancerAddresses.isEmpty() && first != null) { + if (srvRecords.isEmpty() && first != null) { throw first; } - return Collections.unmodifiableList(balancerAddresses); - } - - private static final class SrvRecord { - SrvRecord(String host, int port) { - this.host = host; - this.port = port; - } - - final String host; - final int port; - } - - private static SrvRecord parseSrvRecord(String rawRecord) { - String[] parts = whitespace.split(rawRecord); - Verify.verify(parts.length == 4, "Bad SRV Record: %s", rawRecord); - return new SrvRecord(parts[3], Integer.parseInt(parts[2])); + return Collections.unmodifiableList(srvRecords); } /** diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java index fa52a7d851..5d127b72d1 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java @@ -16,12 +16,9 @@ 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.ChannelLogger; @@ -65,11 +62,4 @@ public class DnsNameResolverProviderTest { 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 26cbc9dd7f..09d1df6921 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -47,16 +48,15 @@ import io.grpc.NameResolver.ServiceConfigParser; import io.grpc.ProxyDetector; import io.grpc.StaticTestingClassLoader; import io.grpc.Status; +import io.grpc.Status.Code; import io.grpc.SynchronizationContext; import io.grpc.internal.DnsNameResolver.AddressResolver; -import io.grpc.internal.DnsNameResolver.ResolutionResults; import io.grpc.internal.DnsNameResolver.ResourceResolver; import io.grpc.internal.DnsNameResolver.ResourceResolverFactory; import io.grpc.internal.JndiResourceResolverFactory.JndiResourceResolver; import io.grpc.internal.JndiResourceResolverFactory.RecordFetcher; import io.grpc.internal.SharedResourceHolder.Resource; import java.io.IOException; -import java.net.Inet4Address; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketAddress; @@ -87,7 +87,6 @@ import org.junit.rules.Timeout; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatchers; import org.mockito.Captor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; @@ -142,6 +141,8 @@ public class DnsNameResolverTest { private NameResolver.Listener2 mockListener; @Captor private ArgumentCaptor resultCaptor; + @Captor + private ArgumentCaptor errorCaptor; @Nullable private String networkaddressCacheTtlPropertyValue; @Mock @@ -155,7 +156,7 @@ public class DnsNameResolverTest { private DnsNameResolver newResolver(String name, int defaultPort, boolean isAndroid) { return newResolver( name, defaultPort, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(), - isAndroid, false); + isAndroid); } private DnsNameResolver newResolver( @@ -163,7 +164,7 @@ public class DnsNameResolverTest { int defaultPort, ProxyDetector proxyDetector, Stopwatch stopwatch) { - return newResolver(name, defaultPort, proxyDetector, stopwatch, false, false); + return newResolver(name, defaultPort, proxyDetector, stopwatch, false); } private DnsNameResolver newResolver( @@ -171,8 +172,7 @@ public class DnsNameResolverTest { final int defaultPort, final ProxyDetector proxyDetector, Stopwatch stopwatch, - boolean isAndroid, - boolean enableSrv) { + boolean isAndroid) { NameResolver.Args args = NameResolver.Args.newBuilder() .setDefaultPort(defaultPort) @@ -181,34 +181,22 @@ public class DnsNameResolverTest { .setServiceConfigParser(mock(ServiceConfigParser.class)) .setChannelLogger(mock(ChannelLogger.class)) .build(); - return newResolver(name, stopwatch, isAndroid, args, enableSrv); - } - - private DnsNameResolver newResolver( - String name, Stopwatch stopwatch, boolean isAndroid, NameResolver.Args args) { - return newResolver(name, stopwatch, isAndroid, args, /* enableSrv= */ false); + return newResolver(name, stopwatch, isAndroid, args); } private DnsNameResolver newResolver( String name, Stopwatch stopwatch, boolean isAndroid, - NameResolver.Args args, - boolean enableSrv) { + NameResolver.Args args) { DnsNameResolver dnsResolver = new DnsNameResolver( - null, name, args, fakeExecutorResource, stopwatch, isAndroid, enableSrv); + null, name, args, fakeExecutorResource, stopwatch, isAndroid); // By default, using the mocked ResourceResolver to avoid I/O dnsResolver.setResourceResolver(new JndiResourceResolver(recordFetcher)); return dnsResolver; } - private DnsNameResolver newSrvEnabledResolver(String name, int defaultPort) { - return newResolver( - name, defaultPort, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(), - false, true); - } - @Before public void setUp() { DnsNameResolver.enableJndi = true; @@ -529,7 +517,8 @@ public class DnsNameResolverTest { } @Test - public void resolve_emptyResult() { + public void resolve_emptyResult() throws Exception { + DnsNameResolver.enableTxt = true; DnsNameResolver nr = newResolver("dns:///addr.fake:1234", 443); nr.setAddressResolver(new AddressResolver() { @Override @@ -537,18 +526,11 @@ public class DnsNameResolverTest { return Collections.emptyList(); } }); - nr.setResourceResolver(new ResourceResolver() { - @Override - public List resolveTxt(String host) throws Exception { - return Collections.emptyList(); - } + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + when(mockResourceResolver.resolveTxt(anyString())) + .thenReturn(Collections.emptyList()); - @Override - public List resolveSrv(AddressResolver addressResolver, String host) - throws Exception { - return Collections.emptyList(); - } - }); + nr.setResourceResolver(mockResourceResolver); nr.start(mockListener); assertThat(fakeExecutor.runDueTasks()).isEqualTo(1); @@ -559,29 +541,81 @@ public class DnsNameResolverTest { assertThat(ac.getValue().getAddresses()).isEmpty(); assertThat(ac.getValue().getAttributes()).isEqualTo(Attributes.EMPTY); assertThat(ac.getValue().getServiceConfig()).isNull(); + verify(mockResourceResolver, never()).resolveSrv(anyString()); } - @SuppressWarnings("deprecation") @Test - public void resolve_balancerAddrsAsAttributes() throws Exception { - InetAddress backendAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 0}); - final EquivalentAddressGroup balancerAddr = - new EquivalentAddressGroup( - new SocketAddress() {}, - Attributes.newBuilder() - .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "foo.example.com") - .build()); + public void resolve_nullResourceResolver() throws Exception { + DnsNameResolver.enableTxt = true; + InetAddress backendAddr = InetAddresses.fromInteger(0x7f000001); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(anyString())) + .thenReturn(Collections.singletonList(backendAddr)); String name = "foo.googleapis.com"; + DnsNameResolver resolver = newResolver(name, 81); + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(null); + resolver.start(mockListener); + assertEquals(1, fakeExecutor.runDueTasks()); + verify(mockListener).onResult(resultCaptor.capture()); + ResolutionResult result = resultCaptor.getValue(); + InetSocketAddress resolvedBackendAddr = + (InetSocketAddress) Iterables.getOnlyElement( + Iterables.getOnlyElement(result.getAddresses()).getAddresses()); + assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr); + verify(mockAddressResolver).resolveAddress(name); + assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY); + assertThat(result.getServiceConfig()).isNull(); + } + + @Test + public void resolve_nullResourceResolver_addressFailure() throws Exception { + DnsNameResolver.enableTxt = true; + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(anyString())) + .thenThrow(new IOException("no addr")); + String name = "foo.googleapis.com"; + + DnsNameResolver resolver = newResolver(name, 81); + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(null); + resolver.start(mockListener); + assertEquals(1, fakeExecutor.runDueTasks()); + verify(mockListener).onError(errorCaptor.capture()); + Status errorStatus = errorCaptor.getValue(); + assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE); + assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr"); + } + + @Test + public void resolve_presentResourceResolver() throws Exception { + DnsNameResolver.enableTxt = true; + InetAddress backendAddr = InetAddresses.fromInteger(0x7f000001); AddressResolver mockAddressResolver = mock(AddressResolver.class); when(mockAddressResolver.resolveAddress(anyString())) .thenReturn(Collections.singletonList(backendAddr)); ResourceResolver mockResourceResolver = mock(ResourceResolver.class); - when(mockResourceResolver.resolveTxt(anyString())).thenReturn(Collections.emptyList()); - when(mockResourceResolver.resolveSrv(ArgumentMatchers.any(AddressResolver.class), anyString())) - .thenReturn(Collections.singletonList(balancerAddr)); + when(mockResourceResolver.resolveTxt(anyString())) + .thenReturn( + Collections.singletonList( + "grpc_config=[{\"clientLanguage\": [\"java\"], \"serviceConfig\": {}}]")); + ServiceConfigParser serviceConfigParser = new ServiceConfigParser() { + @Override + public ConfigOrError parseServiceConfig(Map rawServiceConfig) { + return ConfigOrError.fromConfig(rawServiceConfig); + } + }; + NameResolver.Args args = + NameResolver.Args.newBuilder() + .setDefaultPort(DEFAULT_PORT) + .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR) + .setSynchronizationContext(syncContext) + .setServiceConfigParser(serviceConfigParser) + .build(); - DnsNameResolver resolver = newSrvEnabledResolver(name, 81); + String name = "foo.googleapis.com"; + DnsNameResolver resolver = newResolver(name, Stopwatch.createUnstarted(), false, args); resolver.setAddressResolver(mockAddressResolver); resolver.setResourceResolver(mockResourceResolver); @@ -593,123 +627,89 @@ public class DnsNameResolverTest { (InetSocketAddress) Iterables.getOnlyElement( Iterables.getOnlyElement(result.getAddresses()).getAddresses()); assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr); - assertThat(result.getAttributes().get(GrpcAttributes.ATTR_LB_ADDRS)) - .containsExactly(balancerAddr); + assertThat(result.getServiceConfig().getConfig()).isNotNull(); + verify(mockAddressResolver).resolveAddress(name); + verify(mockResourceResolver).resolveTxt("_grpc_config." + name); } @Test - public void resolveAll_nullResourceResolver() throws Exception { - final String hostname = "addr.fake"; - final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001); - - AddressResolver mockResolver = mock(AddressResolver.class); - when(mockResolver.resolveAddress(anyString())) - .thenReturn(Collections.singletonList(backendAddr)); - ResourceResolver resourceResolver = null; - boolean resovleSrv = true; - boolean resolveTxt = true; - - ResolutionResults res = DnsNameResolver.resolveAll( - mockResolver, resourceResolver, resovleSrv, resolveTxt, hostname); - assertThat(res.addresses).containsExactly(backendAddr); - assertThat(res.balancerAddresses).isEmpty(); - assertThat(res.txtRecords).isEmpty(); - verify(mockResolver).resolveAddress(hostname); - } - - @Test - public void resolveAll_nullResourceResolver_addressFailure() throws Exception { - final String hostname = "addr.fake"; - - AddressResolver mockResolver = mock(AddressResolver.class); - when(mockResolver.resolveAddress(anyString())) + public void resolve_addressFailure_neverLookUpServiceConfig() throws Exception { + DnsNameResolver.enableTxt = true; + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(anyString())) .thenThrow(new IOException("no addr")); - ResourceResolver resourceResolver = null; - boolean resovleSrv = true; - boolean resolveTxt = true; + String name = "foo.googleapis.com"; - thrown.expect(RuntimeException.class); - thrown.expectMessage("no addr"); - - DnsNameResolver.resolveAll(mockResolver, resourceResolver, resovleSrv, resolveTxt, hostname); + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + DnsNameResolver resolver = newResolver(name, 81); + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(mockResourceResolver); + resolver.start(mockListener); + assertEquals(1, fakeExecutor.runDueTasks()); + verify(mockListener).onError(errorCaptor.capture()); + Status errorStatus = errorCaptor.getValue(); + assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE); + assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr"); + verify(mockResourceResolver, never()).resolveTxt(anyString()); } @Test - public void resolveAll_presentResourceResolver() throws Exception { - final String hostname = "addr.fake"; - final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001); - final EquivalentAddressGroup balancerAddr = new EquivalentAddressGroup(new SocketAddress() {}); - + public void resolve_serviceConfigLookupFails_nullServiceConfig() throws Exception { + DnsNameResolver.enableTxt = true; + InetAddress backendAddr = InetAddresses.fromInteger(0x7f000001); AddressResolver mockAddressResolver = mock(AddressResolver.class); when(mockAddressResolver.resolveAddress(anyString())) - .thenReturn(Collections.singletonList(backendAddr)); + .thenReturn(Collections.singletonList(backendAddr)); + String name = "foo.googleapis.com"; ResourceResolver mockResourceResolver = mock(ResourceResolver.class); when(mockResourceResolver.resolveTxt(anyString())) - .thenReturn(Collections.singletonList("service config")); - when(mockResourceResolver.resolveSrv(ArgumentMatchers.any(AddressResolver.class), anyString())) - .thenReturn(Collections.singletonList(balancerAddr)); - boolean resovleSrv = true; - boolean resolveTxt = true; - - ResolutionResults res = DnsNameResolver.resolveAll( - mockAddressResolver, mockResourceResolver, resovleSrv, resolveTxt, hostname); - assertThat(res.addresses).containsExactly(backendAddr); - assertThat(res.balancerAddresses).containsExactly(balancerAddr); - assertThat(res.txtRecords).containsExactly("service config"); - verify(mockAddressResolver).resolveAddress(hostname); - verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname); - verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname); - } - - @Test - public void resolveAll_onlyBalancers() throws Exception { - String hostname = "addr.fake"; - EquivalentAddressGroup balancerAddr = new EquivalentAddressGroup(new SocketAddress() {}); - - AddressResolver mockAddressResolver = mock(AddressResolver.class); - when(mockAddressResolver.resolveAddress(anyString())) - .thenThrow(new UnknownHostException("I really tried")); - ResourceResolver mockResourceResolver = mock(ResourceResolver.class); - when(mockResourceResolver.resolveTxt(anyString())) - .thenReturn(Collections.emptyList()); - when(mockResourceResolver.resolveSrv(ArgumentMatchers.any(AddressResolver.class), anyString())) - .thenReturn(Collections.singletonList(balancerAddr)); - boolean resovleSrv = true; - boolean resolveTxt = true; - - ResolutionResults res = DnsNameResolver.resolveAll( - mockAddressResolver, mockResourceResolver, resovleSrv, resolveTxt, hostname); - assertThat(res.addresses).isEmpty(); - assertThat(res.balancerAddresses).containsExactly(balancerAddr); - assertThat(res.txtRecords).isEmpty(); - verify(mockAddressResolver).resolveAddress(hostname); - verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname); - verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname); - } - - @Test - public void resolveAll_balancerLookupFails() throws Exception { - final String hostname = "addr.fake"; - final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001); - AddressResolver mockAddressResolver = mock(AddressResolver.class); - when(mockAddressResolver.resolveAddress(anyString())) - .thenReturn(Collections.singletonList(backendAddr)); - ResourceResolver mockResourceResolver = mock(ResourceResolver.class); - when(mockResourceResolver.resolveTxt(anyString())) - .thenReturn(Collections.singletonList("service config")); - when(mockResourceResolver.resolveSrv(ArgumentMatchers.any(AddressResolver.class), anyString())) .thenThrow(new Exception("something like javax.naming.NamingException")); - boolean resovleSrv = true; - boolean resolveTxt = true; - ResolutionResults res = DnsNameResolver.resolveAll( - mockAddressResolver, mockResourceResolver, resovleSrv, resolveTxt, hostname); - assertThat(res.addresses).containsExactly(backendAddr); - assertThat(res.balancerAddresses).isEmpty(); - assertThat(res.txtRecords).containsExactly("service config"); - verify(mockAddressResolver).resolveAddress(hostname); - verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname); - verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname); + DnsNameResolver resolver = newResolver(name, 81); + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(mockResourceResolver); + resolver.start(mockListener); + assertEquals(1, fakeExecutor.runDueTasks()); + verify(mockListener).onResult(resultCaptor.capture()); + ResolutionResult result = resultCaptor.getValue(); + InetSocketAddress resolvedBackendAddr = + (InetSocketAddress) Iterables.getOnlyElement( + Iterables.getOnlyElement(result.getAddresses()).getAddresses()); + assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr); + verify(mockAddressResolver).resolveAddress(name); + assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY); + assertThat(result.getServiceConfig()).isNull(); + verify(mockResourceResolver).resolveTxt(anyString()); + } + + @Test + public void resolve_serviceConfigMalformed_serviceConfigError() throws Exception { + DnsNameResolver.enableTxt = true; + InetAddress backendAddr = InetAddresses.fromInteger(0x7f000001); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(anyString())) + .thenReturn(Collections.singletonList(backendAddr)); + String name = "foo.googleapis.com"; + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + when(mockResourceResolver.resolveTxt(anyString())) + .thenReturn(Collections.singletonList("grpc_config=something invalid")); + + DnsNameResolver resolver = newResolver(name, 81); + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(mockResourceResolver); + resolver.start(mockListener); + assertEquals(1, fakeExecutor.runDueTasks()); + verify(mockListener).onResult(resultCaptor.capture()); + ResolutionResult result = resultCaptor.getValue(); + InetSocketAddress resolvedBackendAddr = + (InetSocketAddress) Iterables.getOnlyElement( + Iterables.getOnlyElement(result.getAddresses()).getAddresses()); + assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr); + verify(mockAddressResolver).resolveAddress(name); + assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY); + assertThat(result.getServiceConfig()).isNotNull(); + assertThat(result.getServiceConfig().getError()).isNotNull(); + verify(mockResourceResolver).resolveTxt(anyString()); } @Test diff --git a/core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java b/core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java index 965ef8f51c..dae80649ad 100644 --- a/core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java +++ b/core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java @@ -21,16 +21,10 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import io.grpc.Attributes; -import io.grpc.EquivalentAddressGroup; -import io.grpc.internal.DnsNameResolver.AddressResolver; +import io.grpc.internal.DnsNameResolver.SrvRecord; import io.grpc.internal.JndiResourceResolverFactory.JndiRecordFetcher; import io.grpc.internal.JndiResourceResolverFactory.JndiResourceResolver; import io.grpc.internal.JndiResourceResolverFactory.RecordFetcher; -import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.net.SocketAddress; -import java.net.UnknownHostException; import java.util.Arrays; import java.util.List; import org.junit.Assume; @@ -83,33 +77,15 @@ public class JndiResourceResolverTest { @SuppressWarnings("deprecation") @Test public void srvRecordLookup() throws Exception { - AddressResolver addressResolver = mock(AddressResolver.class); - when(addressResolver.resolveAddress("foo.example.com.")) - .thenReturn(Arrays.asList(InetAddress.getByName("127.1.2.3"))); - when(addressResolver.resolveAddress("bar.example.com.")) - .thenReturn(Arrays.asList( - InetAddress.getByName("127.3.2.1"), InetAddress.getByName("::1"))); - when(addressResolver.resolveAddress("unknown.example.com.")) - .thenThrow(new UnknownHostException("unknown.example.com.")); RecordFetcher recordFetcher = mock(RecordFetcher.class); when(recordFetcher.getAllRecords("SRV", "dns:///service.example.com")) .thenReturn(Arrays.asList( - "0 0 314 foo.example.com.", "0 0 42 bar.example.com.", "0 0 1 unknown.example.com.")); + "0 0 314 foo.example.com.", "0 0 42 bar.example.com.", "0 0 1 discard.example.com")); - List golden = Arrays.asList( - new EquivalentAddressGroup( - Arrays.asList(new InetSocketAddress("127.1.2.3", 314)), - Attributes.newBuilder() - .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "foo.example.com") - .build()), - new EquivalentAddressGroup( - Arrays.asList( - new InetSocketAddress("127.3.2.1", 42), - new InetSocketAddress("::1", 42)), - Attributes.newBuilder() - .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "bar.example.com") - .build())); + List golden = Arrays.asList( + new SrvRecord("foo.example.com.", 314), + new SrvRecord("bar.example.com.", 42)); JndiResourceResolver resolver = new JndiResourceResolver(recordFetcher); - assertThat(resolver.resolveSrv(addressResolver, "service.example.com")).isEqualTo(golden); + assertThat(resolver.resolveSrv("service.example.com")).isEqualTo(golden); } } diff --git a/grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java b/grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java index 7526775727..8d2ca418e5 100644 --- a/grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java +++ b/grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java @@ -42,9 +42,11 @@ public final class GrpclbConstants { static final Attributes.Key TOKEN_ATTRIBUTE_KEY = Attributes.Key.create("lb-token"); - @SuppressWarnings("deprecation") + /** + * Attribute key for gRPC LB server addresses. + */ static final Attributes.Key> ATTR_LB_ADDRS = - io.grpc.internal.GrpcAttributes.ATTR_LB_ADDRS; + Attributes.Key.create("io.grpc.grpclb.lbAddrs"); @SuppressWarnings("deprecation") @EquivalentAddressGroup.Attr diff --git a/grpclb/src/main/java/io/grpc/grpclb/GrpclbNameResolver.java b/grpclb/src/main/java/io/grpc/grpclb/GrpclbNameResolver.java new file mode 100644 index 0000000000..d917d8ed67 --- /dev/null +++ b/grpclb/src/main/java/io/grpc/grpclb/GrpclbNameResolver.java @@ -0,0 +1,142 @@ +/* + * Copyright 2020 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 com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Stopwatch; +import io.grpc.Attributes; +import io.grpc.EquivalentAddressGroup; +import io.grpc.NameResolver; +import io.grpc.internal.DnsNameResolver; +import io.grpc.internal.SharedResourceHolder.Resource; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.Executor; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nullable; + +/** + * A DNS-based {@link NameResolver} with gRPC LB specific add-ons for resolving balancer + * addresses via service records. + * + * @see SecretGrpclbNameResolverProvider + */ +final class GrpclbNameResolver extends DnsNameResolver { + + private static final Logger logger = Logger.getLogger(GrpclbNameResolver.class.getName()); + + // From https://github.com/grpc/proposal/blob/master/A5-grpclb-in-dns.md + private static final String GRPCLB_NAME_PREFIX = "_grpclb._tcp."; + + GrpclbNameResolver( + @Nullable String nsAuthority, + String name, + Args args, + Resource executorResource, + Stopwatch stopwatch, + boolean isAndroid) { + super(nsAuthority, name, args, executorResource, stopwatch, isAndroid); + } + + @Override + protected InternalResolutionResult doResolve(boolean forceTxt) { + List balancerAddrs = resolveBalancerAddresses(); + InternalResolutionResult result = super.doResolve(!balancerAddrs.isEmpty()); + if (!balancerAddrs.isEmpty()) { + result.attributes = + Attributes.newBuilder() + .set(GrpclbConstants.ATTR_LB_ADDRS, balancerAddrs) + .build(); + } + return result; + } + + private List resolveBalancerAddresses() { + List srvRecords = Collections.emptyList(); + Exception srvRecordsException = null; + ResourceResolver resourceResolver = getResourceResolver(); + if (resourceResolver != null) { + try { + srvRecords = resourceResolver.resolveSrv(GRPCLB_NAME_PREFIX + getHost()); + } catch (Exception e) { + srvRecordsException = e; + } + } + List balancerAddresses = new ArrayList<>(srvRecords.size()); + Exception balancerAddressesException = null; + Level level = Level.WARNING; + for (SrvRecord record : srvRecords) { + try { + // Strip trailing dot for appearance's sake. It _should_ be fine either way, but most + // people expect to see it without the dot. + String authority = record.host.substring(0, record.host.length() - 1); + // But we want to use the trailing dot for the IP lookup. The dot makes the name absolute + // instead of relative and so will avoid the search list like that in resolv.conf. + List addrs = addressResolver.resolveAddress(record.host); + List sockAddrs = new ArrayList<>(addrs.size()); + for (InetAddress addr : addrs) { + sockAddrs.add(new InetSocketAddress(addr, record.port)); + } + Attributes attrs = + Attributes.newBuilder() + .set(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY, authority) + .build(); + balancerAddresses.add( + new EquivalentAddressGroup(Collections.unmodifiableList(sockAddrs), attrs)); + } catch (Exception e) { + logger.log(level, "Can't find address for SRV record " + record, e); + if (balancerAddressesException == null) { + balancerAddressesException = e; + level = Level.FINE; + } + } + } + if (srvRecordsException != null + || (balancerAddressesException != null && balancerAddresses.isEmpty())) { + logger.log(Level.FINE, "Balancer resolution failure", srvRecordsException); + } + return Collections.unmodifiableList(balancerAddresses); + } + + @VisibleForTesting + @Override + protected void setAddressResolver(AddressResolver addressResolver) { + super.setAddressResolver(addressResolver); + } + + @VisibleForTesting + @Override + protected void setResourceResolver(ResourceResolver resourceResolver) { + super.setResourceResolver(resourceResolver); + } + + @VisibleForTesting + @Override + protected String getHost() { + return super.getHost(); + } + + @VisibleForTesting + static void setEnableTxt(boolean enableTxt) { + DnsNameResolver.enableTxt = enableTxt; + } +} diff --git a/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java b/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java index 1856c78c14..bc25f28f94 100644 --- a/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java +++ b/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java @@ -16,10 +16,16 @@ package io.grpc.grpclb; -import io.grpc.internal.BaseDnsNameResolverProvider; +import com.google.common.base.Preconditions; +import com.google.common.base.Stopwatch; +import io.grpc.InternalServiceProviders; +import io.grpc.NameResolver.Args; +import io.grpc.NameResolverProvider; +import io.grpc.internal.GrpcUtil; +import java.net.URI; /** - * A provider for {@code io.grpc.internal.DnsNameResolver} for gRPC lb. + * A provider for {@code io.grpc.grpclb.GrpclbNameResolver}. * *

    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 @@ -32,9 +38,6 @@ import io.grpc.internal.BaseDnsNameResolverProvider; * 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. @@ -42,14 +45,39 @@ final class SecretGrpclbNameResolverProvider { private SecretGrpclbNameResolverProvider() {} - public static final class Provider extends BaseDnsNameResolverProvider { + public static final class Provider extends NameResolverProvider { - private static final boolean SRV_ENABLED = - Boolean.parseBoolean(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME, "true")); + private static final String SCHEME = "dns"; @Override - protected boolean isSrvEnabled() { - return SRV_ENABLED; + public GrpclbNameResolver newNameResolver(URI targetUri, 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 GrpclbNameResolver( + targetUri.getAuthority(), + name, + args, + GrpcUtil.SHARED_CHANNEL_EXECUTOR, + Stopwatch.createUnstarted(), + InternalServiceProviders.isAndroid(getClass().getClassLoader())); + } else { + return null; + } + } + + @Override + public String getDefaultScheme() { + return SCHEME; + } + + @Override + protected boolean isAvailable() { + return true; } @Override diff --git a/grpclb/src/test/java/io/grpc/grpclb/GrpclbNameResolverTest.java b/grpclb/src/test/java/io/grpc/grpclb/GrpclbNameResolverTest.java new file mode 100644 index 0000000000..bf44a81a82 --- /dev/null +++ b/grpclb/src/test/java/io/grpc/grpclb/GrpclbNameResolverTest.java @@ -0,0 +1,337 @@ +/* + * Copyright 2020 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 org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.collect.Iterables; +import io.grpc.Attributes; +import io.grpc.ChannelLogger; +import io.grpc.EquivalentAddressGroup; +import io.grpc.NameResolver; +import io.grpc.NameResolver.ConfigOrError; +import io.grpc.NameResolver.ResolutionResult; +import io.grpc.NameResolver.ServiceConfigParser; +import io.grpc.Status; +import io.grpc.Status.Code; +import io.grpc.SynchronizationContext; +import io.grpc.internal.DnsNameResolver.AddressResolver; +import io.grpc.internal.DnsNameResolver.ResourceResolver; +import io.grpc.internal.DnsNameResolver.SrvRecord; +import io.grpc.internal.FakeClock; +import io.grpc.internal.GrpcUtil; +import io.grpc.internal.SharedResourceHolder.Resource; +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.Executor; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.mockito.stubbing.Answer; + +/** Unit tests for {@link GrpclbNameResolver}. */ +@RunWith(JUnit4.class) +public class GrpclbNameResolverTest { + + @Rule + public final MockitoRule mocks = MockitoJUnit.rule(); + + private static final String NAME = "foo.googleapis.com"; + private static final int DEFAULT_PORT = 887; + + private final SynchronizationContext syncContext = new SynchronizationContext( + new Thread.UncaughtExceptionHandler() { + @Override + public void uncaughtException(Thread t, Throwable e) { + throw new AssertionError(e); + } + }); + + private final FakeClock fakeClock = new FakeClock(); + private final FakeExecutorResource fakeExecutorResource = new FakeExecutorResource(); + + private final class FakeExecutorResource implements Resource { + + @Override + public Executor create() { + return fakeClock.getScheduledExecutorService(); + } + + @Override + public void close(Executor instance) {} + } + + @Captor private ArgumentCaptor resultCaptor; + @Captor private ArgumentCaptor errorCaptor; + @Mock private ServiceConfigParser serviceConfigParser; + @Mock private NameResolver.Listener2 mockListener; + + private GrpclbNameResolver resolver; + private String hostName; + + @Before + public void setUp() { + GrpclbNameResolver.setEnableTxt(true); + NameResolver.Args args = + NameResolver.Args.newBuilder() + .setDefaultPort(DEFAULT_PORT) + .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR) + .setSynchronizationContext(syncContext) + .setServiceConfigParser(serviceConfigParser) + .setChannelLogger(mock(ChannelLogger.class)) + .build(); + resolver = + new GrpclbNameResolver( + null, NAME, args, fakeExecutorResource, fakeClock.getStopwatchSupplier().get(), + /* isAndroid */false); + hostName = resolver.getHost(); + assertThat(hostName).isEqualTo(NAME); + } + + @Test + public void resolve_emptyResult() { + resolver.setAddressResolver(new AddressResolver() { + @Override + public List resolveAddress(String host) throws Exception { + return Collections.emptyList(); + } + }); + resolver.setResourceResolver(new ResourceResolver() { + @Override + public List resolveTxt(String host) throws Exception { + return Collections.emptyList(); + } + + @Override + public List resolveSrv(String host) throws Exception { + return Collections.emptyList(); + } + }); + + resolver.start(mockListener); + assertThat(fakeClock.runDueTasks()).isEqualTo(1); + + verify(mockListener).onResult(resultCaptor.capture()); + ResolutionResult result = resultCaptor.getValue(); + assertThat(result.getAddresses()).isEmpty(); + assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY); + assertThat(result.getServiceConfig()).isNull(); + } + + @Test + public void resolve_presentResourceResolver() throws Exception { + InetAddress backendAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 0}); + InetAddress lbAddr = InetAddress.getByAddress(new byte[] {10, 1, 0, 0}); + int lbPort = 8080; + String lbName = "foo.example.com."; // original name in SRV record + SrvRecord srvRecord = new SrvRecord(lbName, 8080); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(hostName)) + .thenReturn(Collections.singletonList(backendAddr)); + when(mockAddressResolver.resolveAddress(lbName)) + .thenReturn(Collections.singletonList(lbAddr)); + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + when(mockResourceResolver.resolveTxt(anyString())) + .thenReturn( + Collections.singletonList( + "grpc_config=[{\"clientLanguage\": [\"java\"], \"serviceConfig\": {}}]")); + when(mockResourceResolver.resolveSrv(anyString())) + .thenReturn(Collections.singletonList(srvRecord)); + when(serviceConfigParser.parseServiceConfig(ArgumentMatchers.anyMap())) + .thenAnswer(new Answer() { + @Override + public ConfigOrError answer(InvocationOnMock invocation) { + Object[] args = invocation.getArguments(); + return ConfigOrError.fromConfig(args[0]); + } + }); + + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(mockResourceResolver); + + resolver.start(mockListener); + assertThat(fakeClock.runDueTasks()).isEqualTo(1); + verify(mockListener).onResult(resultCaptor.capture()); + ResolutionResult result = resultCaptor.getValue(); + InetSocketAddress resolvedBackendAddr = + (InetSocketAddress) Iterables.getOnlyElement( + Iterables.getOnlyElement(result.getAddresses()).getAddresses()); + assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr); + EquivalentAddressGroup resolvedBalancerAddr = + Iterables.getOnlyElement(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS)); + assertThat(resolvedBalancerAddr.getAttributes().get(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY)) + .isEqualTo("foo.example.com"); + InetSocketAddress resolvedBalancerSockAddr = + (InetSocketAddress) Iterables.getOnlyElement(resolvedBalancerAddr.getAddresses()); + assertThat(resolvedBalancerSockAddr.getAddress()).isEqualTo(lbAddr); + assertThat(resolvedBalancerSockAddr.getPort()).isEqualTo(lbPort); + assertThat(result.getServiceConfig().getConfig()).isNotNull(); + verify(mockAddressResolver).resolveAddress(hostName); + verify(mockResourceResolver).resolveTxt("_grpc_config." + hostName); + verify(mockResourceResolver).resolveSrv("_grpclb._tcp." + hostName); + } + + @Test + public void resolve_nullResourceResolver() throws Exception { + InetAddress backendAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 0}); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(anyString())) + .thenReturn(Collections.singletonList(backendAddr)); + ResourceResolver resourceResolver = null; + + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(resourceResolver); + + resolver.start(mockListener); + assertThat(fakeClock.runDueTasks()).isEqualTo(1); + verify(mockListener).onResult(resultCaptor.capture()); + ResolutionResult result = resultCaptor.getValue(); + assertThat(result.getAddresses()) + .containsExactly( + new EquivalentAddressGroup(new InetSocketAddress(backendAddr, DEFAULT_PORT))); + assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY); + assertThat(result.getServiceConfig()).isNull(); + } + + @Test + public void resolve_nullResourceResolver_addressFailure() throws Exception { + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(anyString())).thenThrow(new IOException("no addr")); + ResourceResolver resourceResolver = null; + + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(resourceResolver); + + resolver.start(mockListener); + assertThat(fakeClock.runDueTasks()).isEqualTo(1); + verify(mockListener).onError(errorCaptor.capture()); + Status errorStatus = errorCaptor.getValue(); + assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE); + assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr"); + } + + @Test + public void resolve_addressFailure_stillLookUpBalancersAndServiceConfig() throws Exception { + InetAddress lbAddr = InetAddress.getByAddress(new byte[] {10, 1, 0, 0}); + int lbPort = 8080; + String lbName = "foo.example.com."; // original name in SRV record + SrvRecord srvRecord = new SrvRecord(lbName, 8080); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(hostName)) + .thenThrow(new UnknownHostException("I really tried")); + when(mockAddressResolver.resolveAddress(lbName)) + .thenReturn(Collections.singletonList(lbAddr)); + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + when(mockResourceResolver.resolveTxt(anyString())).thenReturn(Collections.emptyList()); + when(mockResourceResolver.resolveSrv(anyString())) + .thenReturn(Collections.singletonList(srvRecord)); + + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(mockResourceResolver); + + resolver.start(mockListener); + assertThat(fakeClock.runDueTasks()).isEqualTo(1); + verify(mockListener).onResult(resultCaptor.capture()); + ResolutionResult result = resultCaptor.getValue(); + assertThat(result.getAddresses()).isEmpty(); + EquivalentAddressGroup resolvedBalancerAddr = + Iterables.getOnlyElement(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS)); + assertThat(resolvedBalancerAddr.getAttributes().get(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY)) + .isEqualTo("foo.example.com"); + InetSocketAddress resolvedBalancerSockAddr = + (InetSocketAddress) Iterables.getOnlyElement(resolvedBalancerAddr.getAddresses()); + assertThat(resolvedBalancerSockAddr.getAddress()).isEqualTo(lbAddr); + assertThat(resolvedBalancerSockAddr.getPort()).isEqualTo(lbPort); + assertThat(result.getServiceConfig()).isNull(); + verify(mockAddressResolver).resolveAddress(hostName); + verify(mockResourceResolver).resolveTxt("_grpc_config." + hostName); + verify(mockResourceResolver).resolveSrv("_grpclb._tcp." + hostName); + } + + @Test + public void resolveAll_balancerLookupFails_stillLookUpServiceConfig() throws Exception { + InetAddress backendAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 0}); + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(hostName)) + .thenReturn(Collections.singletonList(backendAddr)); + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + when(mockResourceResolver.resolveTxt(anyString())) + .thenReturn(Collections.emptyList()); + when(mockResourceResolver.resolveSrv(anyString())) + .thenThrow(new Exception("something like javax.naming.NamingException")); + + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(mockResourceResolver); + + resolver.start(mockListener); + assertThat(fakeClock.runDueTasks()).isEqualTo(1); + verify(mockListener).onResult(resultCaptor.capture()); + ResolutionResult result = resultCaptor.getValue(); + + InetSocketAddress resolvedBackendAddr = + (InetSocketAddress) Iterables.getOnlyElement( + Iterables.getOnlyElement(result.getAddresses()).getAddresses()); + assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr); + assertThat(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS)).isNull(); + verify(mockAddressResolver).resolveAddress(hostName); + verify(mockResourceResolver).resolveTxt("_grpc_config." + hostName); + verify(mockResourceResolver).resolveSrv("_grpclb._tcp." + hostName); + } + + @Test + public void resolve_addressAndBalancersLookupFail_neverLookupServiceConfig() throws Exception { + AddressResolver mockAddressResolver = mock(AddressResolver.class); + when(mockAddressResolver.resolveAddress(anyString())) + .thenThrow(new UnknownHostException("I really tried")); + ResourceResolver mockResourceResolver = mock(ResourceResolver.class); + lenient().when(mockResourceResolver.resolveTxt(anyString())) + .thenThrow(new Exception("something like javax.naming.NamingException")); + when(mockResourceResolver.resolveSrv(anyString())) + .thenThrow(new Exception("something like javax.naming.NamingException")); + + resolver.setAddressResolver(mockAddressResolver); + resolver.setResourceResolver(mockResourceResolver); + + resolver.start(mockListener); + assertThat(fakeClock.runDueTasks()).isEqualTo(1); + verify(mockListener).onError(errorCaptor.capture()); + Status errorStatus = errorCaptor.getValue(); + assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE); + verify(mockAddressResolver).resolveAddress(hostName); + verify(mockResourceResolver, never()).resolveTxt("_grpc_config." + hostName); + verify(mockResourceResolver).resolveSrv("_grpclb._tcp." + hostName); + } +} \ No newline at end of file diff --git a/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java b/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java index e5d4b3501f..24b1c781f5 100644 --- a/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java +++ b/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java @@ -17,34 +17,92 @@ 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 static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import io.grpc.ChannelLogger; +import io.grpc.NameResolver; +import io.grpc.NameResolver.ServiceConfigParser; +import io.grpc.SynchronizationContext; import io.grpc.internal.DnsNameResolverProvider; +import io.grpc.internal.GrpcUtil; +import java.net.URI; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +/** Unit tests for {@link SecretGrpclbNameResolverProvider}. */ @RunWith(JUnit4.class) public class SecretGrpclbNameResolverProviderTest { + private final SynchronizationContext syncContext = new SynchronizationContext( + new Thread.UncaughtExceptionHandler() { + @Override + public void uncaughtException(Thread t, Throwable e) { + throw new AssertionError(e); + } + }); + private final NameResolver.Args args = NameResolver.Args.newBuilder() + .setDefaultPort(8080) + .setProxyDetector(GrpcUtil.DEFAULT_PROXY_DETECTOR) + .setSynchronizationContext(syncContext) + .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) + .build(); + + private SecretGrpclbNameResolverProvider.Provider provider = + new SecretGrpclbNameResolverProvider.Provider(); + + @Test + public void isAvailable() { + assertThat(provider.isAvailable()).isTrue(); + } + @Test public void priority_shouldBeHigherThanDefaultDnsNameResolver() { DnsNameResolverProvider defaultDnsNameResolver = new DnsNameResolverProvider(); - SecretGrpclbNameResolverProvider.Provider grpclbDnsNameResolver = - new SecretGrpclbNameResolverProvider.Provider(); - assertThat(defaultDnsNameResolver.priority()) - .isLessThan(grpclbDnsNameResolver.priority()); + assertThat(provider.priority()).isGreaterThan(defaultDnsNameResolver.priority()); } @Test - public void isSrvEnabled_trueByDefault() { - assumeTrue(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME) == null); - - SecretGrpclbNameResolverProvider.Provider grpclbDnsNameResolver = - new SecretGrpclbNameResolverProvider.Provider(); - - assertThat(grpclbDnsNameResolver.isSrvEnabled()).isTrue(); + public void newNameResolver() { + assertThat(provider.newNameResolver(URI.create("dns:///localhost:443"), args)) + .isInstanceOf(GrpclbNameResolver.class); + assertThat(provider.newNameResolver(URI.create("notdns:///localhost:443"), args)).isNull(); } -} \ No newline at end of file + + @Test + public void invalidDnsName() throws Exception { + testInvalidUri(new URI("dns", null, "/[invalid]", null)); + } + + @Test + public void validIpv6() throws Exception { + testValidUri(new URI("dns", null, "/[::1]", null)); + } + + @Test + public void validDnsNameWithoutPort() throws Exception { + testValidUri(new URI("dns", null, "/foo.googleapis.com", null)); + } + + @Test + public void validDnsNameWithPort() throws Exception { + testValidUri(new URI("dns", null, "/foo.googleapis.com:456", null)); + } + + private void testInvalidUri(URI uri) { + try { + provider.newNameResolver(uri, args); + fail("Should have failed"); + } catch (IllegalArgumentException e) { + // expected + } + } + + private void testValidUri(URI uri) { + GrpclbNameResolver resolver = provider.newNameResolver(uri, args); + assertThat(resolver).isNotNull(); + } +}