Dialogue fails fast when existing channels are revoked from DNS (#2263)

Dialogue fails fast when existing channels are revoked from DNS

changelog/@unreleased/pr-2263.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Dialogue fails fast when existing channels are revoked from DNS
+  links:
+  - https://github.com/palantir/dialogue/pull/2263

dialogue-apache-hc5-client/src/main/java/com/palantir/dialogue/hc5/ApacheHttpClientBlockingChannel.java

@@ -15,6 +15,9 @@
  */
 package com.palantir.dialogue.hc5;
 
+import com.codahale.metrics.Meter;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.MultimapBuilder;
@@ -29,6 +32,7 @@
 import com.palantir.dialogue.ResponseAttachments;
 import com.palantir.dialogue.blocking.BlockingChannel;
 import com.palantir.dialogue.core.BaseUrl;
+import com.palantir.dialogue.core.DialogueDnsResolver;
 import com.palantir.logsafe.Arg;
 import com.palantir.logsafe.Preconditions;
 import com.palantir.logsafe.SafeArg;
@@ -46,6 +50,7 @@
 import java.io.OutputStream;
 import java.net.InetAddress;
 import java.net.URL;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
@@ -56,6 +61,7 @@
 import java.util.OptionalLong;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.function.BooleanSupplier;
 import javax.annotation.Nullable;
 import org.apache.hc.client5.http.ConnectTimeoutException;
 import org.apache.hc.client5.http.classic.ExecRuntime;
@@ -82,6 +88,8 @@ final class ApacheHttpClientBlockingChannel implements BlockingChannel {
     private final Optional<InetAddress> resolvedHost;
     private final ResponseLeakDetector responseLeakDetector;
     private final OptionalInt uriIndexForInstrumentation;
+    private final BooleanSupplier isValidTarget;
+    private final Meter shortCircuitMeter;
 
     ApacheHttpClientBlockingChannel(
             ApacheHttpClientChannels.CloseableClient client,
@@ -94,10 +102,37 @@ final class ApacheHttpClientBlockingChannel implements BlockingChannel {
         this.resolvedHost = resolvedHost;
         this.responseLeakDetector = responseLeakDetector;
         this.uriIndexForInstrumentation = uriIndexForInstrumentation;
+        this.shortCircuitMeter = DialogueClientMetrics.of(
+                        client.clientConfiguration().taggedMetricRegistry())
+                .shortCircuitUnresolvableTarget(client.name());
+        if (resolvedHost.isPresent() && client.dnsResolver().isPresent()) {
+            InetAddress target = resolvedHost.get();
+            DialogueDnsResolver dnsResolver = client.dnsResolver().get();
+            BooleanSupplier validTargetPredicate = Suppliers.memoizeWithExpiration(
+                    () -> isTargetValid(dnsResolver, baseUrl, target), Duration.ofSeconds(1))::get;
+            // If the bound address is not initially resolvable, this predicate will be ignored.
+            isValidTarget = validTargetPredicate.getAsBoolean() ? validTargetPredicate : () -> true;
+        } else {
+            isValidTarget = () -> true;
+        }
+    }
+
+    private static boolean isTargetValid(DialogueDnsResolver dnsResolver, URL baseUrl, InetAddress resolvedHost) {
+        String host = baseUrl.getHost();
+        if (Strings.isNullOrEmpty(host)) {
+            // we don't have enough confidence to prevent requests from flowing.
+            return true;
+        }
+        return dnsResolver.resolve(host).contains(resolvedHost);
     }
 
     @Override
     public Response execute(Endpoint endpoint, Request request) throws IOException {
+        if (!isValidTarget.getAsBoolean()) {
+            shortCircuitMeter.mark();
+            throw new SafeConnectException("Target address can no longer be resolved");
+        }
+
         // Create base request given the URL
         URL target = baseUrl.render(endpoint, request);
         ClassicRequestBuilder builder =

dialogue-apache-hc5-client/src/main/java/com/palantir/dialogue/hc5/ApacheHttpClientChannels.java

@@ -182,7 +182,8 @@ static CloseableClient wrap(
                 InstrumentedPoolingHttpClientConnectionManager pool,
                 ScheduledFuture<?> connectionEvictorFuture,
                 ClientConfiguration clientConfiguration,
-                @Nullable ExecutorService executor) {
+                @Nullable ExecutorService executor,
+                Optional<DialogueDnsResolver> dnsResolver) {
             ResponseLeakDetector leakDetector =
                     ResponseLeakDetector.of(clientName, clientConfiguration.taggedMetricRegistry());
             CloseableClientImpl newInstance = new CloseableClientImpl(
@@ -192,7 +193,8 @@ static CloseableClient wrap(
                     connectionEvictorFuture,
                     leakDetector,
                     executor,
-                    clientConfiguration);
+                    clientConfiguration,
+                    dnsResolver);
             if (log.isDebugEnabled()) {
                 // If debug is enabled, log the stack trace.
                 log.debug(
@@ -226,6 +228,8 @@ static CloseableClient wrap(
         abstract ExecutorService executor();
 
         abstract ResponseLeakDetector leakDetector();
+
+        abstract Optional<DialogueDnsResolver> dnsResolver();
     }
 
     private static final class CloseableClientWrapper extends CloseableClient {
@@ -262,6 +266,11 @@ ResponseLeakDetector leakDetector() {
             return delegate.leakDetector();
         }
 
+        @Override
+        Optional<DialogueDnsResolver> dnsResolver() {
+            return delegate.dnsResolver();
+        }
+
         @Override
         public void close() throws IOException {
             delegate.close();
@@ -281,6 +290,7 @@ private static final class CloseableClientImpl extends CloseableClient {
         private final InstrumentedPoolingHttpClientConnectionManager pool;
         private final ResponseLeakDetector leakDetector;
         private final ClientConfiguration clientConfiguration;
+        private final Optional<DialogueDnsResolver> dnsResolver;
 
         @Nullable
         private final ExecutorService executor;
@@ -294,13 +304,15 @@ private CloseableClientImpl(
                 ScheduledFuture<?> connectionEvictorFuture,
                 ResponseLeakDetector leakDetector,
                 @Nullable ExecutorService executor,
-                ClientConfiguration clientConfiguration) {
+                ClientConfiguration clientConfiguration,
+                Optional<DialogueDnsResolver> dnsResolver) {
             this.clientName = clientName;
             this.apacheClient = apacheClient;
             this.pool = pool;
             this.leakDetector = leakDetector;
             this.executor = executor;
             this.clientConfiguration = clientConfiguration;
+            this.dnsResolver = dnsResolver;
             closer.register(() -> connectionEvictorFuture.cancel(true));
             closer.register(apacheClient);
             closer.register(pool::closeUnderlyingConnectionManager);
@@ -334,6 +346,11 @@ ResponseLeakDetector leakDetector() {
             return leakDetector;
         }
 
+        @Override
+        Optional<DialogueDnsResolver> dnsResolver() {
+            return dnsResolver;
+        }
+
         @Override
         public void close() throws IOException {
             if (log.isDebugEnabled()) {
@@ -586,7 +603,8 @@ public CloseableClient build() {
             CloseableHttpClient apacheClient = builder.build();
             ScheduledFuture<?> connectionEvictorFuture =
                     ScheduledIdleConnectionEvictor.schedule(connectionManager, Duration.ofSeconds(5));
-            return CloseableClient.wrap(apacheClient, name, connectionManager, connectionEvictorFuture, conf, executor);
+            return CloseableClient.wrap(
+                    apacheClient, name, connectionManager, connectionEvictorFuture, conf, executor, dnsResolver);
         }
     }
 

dialogue-apache-hc5-client/src/main/java/com/palantir/dialogue/hc5/SafeConnectException.java

@@ -0,0 +1,53 @@
+/*
+ * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved.
+ *
+ * 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 com.palantir.dialogue.hc5;
+
+import com.google.errorprone.annotations.CompileTimeConstant;
+import com.palantir.logsafe.Arg;
+import com.palantir.logsafe.SafeLoggable;
+import com.palantir.logsafe.exceptions.SafeExceptions;
+import java.net.ConnectException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * A {@link SafeLoggable} variant of {@link ConnectException}.
+ */
+final class SafeConnectException extends ConnectException implements SafeLoggable {
+
+    @CompileTimeConstant
+    private final String message;
+
+    private final List<Arg<?>> arguments;
+
+    SafeConnectException(@CompileTimeConstant String message, Arg<?>... args) {
+        super(SafeExceptions.renderMessage(message, args));
+        this.message = message;
+        this.arguments = Collections.unmodifiableList(Arrays.asList(args));
+    }
+
+    @Override
+    public String getLogMessage() {
+        return message;
+    }
+
+    @Override
+    public List<Arg<?>> getArgs() {
+        return arguments;
+    }
+}

dialogue-apache-hc5-client/src/main/metrics/dialogue-apache-client-metrics.yml

@@ -121,3 +121,11 @@ namespaces:
           - name: cipher
             docs: The insecure cipher used to connect to this server
         docs: Meter describing the use of insecure ciphers to connect to this server.
+
+      short.circuit.unresolvable.target:
+        type: meter
+        tags:
+          - name: client-name
+          - name: client-type
+            values: [ apache-hc5 ]
+        docs: Rate that requests are immediately rejected due to an unresolvable target address.

dialogue-clients/metrics.md

@@ -48,6 +48,9 @@ Dialogue client response metrics provided by the Apache client channel.
   - `client-name`
   - `client-type` values (`apache-hc5`)
   - `cipher`: The insecure cipher used to connect to this server
+- `dialogue.client.short.circuit.unresolvable.target` (meter): Rate that requests are immediately rejected due to an unresolvable target address.
+  - `client-name`
+  - `client-type` values (`apache-hc5`)
 
 ### dialogue.client.pool
 Connection pool metrics from the dialogue Apache client.