proxy monitor async version (#2550)

modifying the NetworkConfigurationFilter to do asynchronous DNS lookups for non-IP-address proxies. If address resolution fails, the request will be short circuited by sendLocalReply rather than being sent upstream without being proxied. This PR also changes ConnectivityManager to have a pure API and an Impl for testing. Risk Level: low Testing: unit tests, integration test Docs Changes: n/a Release Notes: n/a part of #1622 fixes #2532 Signed-off-by: Alyssa Wilk <[email protected]>
envoyproxy · Sep 21, 2022 · 5e8d877 · 5e8d877
1 parent 3c4de61
commit 5e8d877
Show file tree

Hide file tree

Showing 17 changed files with 775 additions and 122 deletions.
diff --git a/library/cc/engine.h b/library/cc/engine.h
@@ -8,6 +8,8 @@
 #include "stream_client.h"
 
 namespace Envoy {
+class BaseClientIntegrationTest;
+
 namespace Platform {
 
 class StreamClient;
@@ -27,6 +29,8 @@ class Engine : public std::enable_shared_from_this<Engine> {
   friend class EngineBuilder;
   // required to use envoy_engine_t without exposing it publicly
   friend class StreamPrototype;
+  // for testing only
+  friend class ::Envoy::BaseClientIntegrationTest;
 
   envoy_engine_t engine_;
   StreamClientSharedPtr stream_client_;

diff --git a/library/common/extensions/filters/http/network_configuration/filter.cc b/library/common/extensions/filters/http/network_configuration/filter.cc
@@ -30,34 +30,102 @@ void NetworkConfigurationFilter::setDecoderFilterCallbacks(
   decoder_callbacks_->addUpstreamSocketOptions(options);
 }
 
+void NetworkConfigurationFilter::onLoadDnsCacheComplete(
+    const Common::DynamicForwardProxy::DnsHostInfoSharedPtr& host_info) {
+  if (onAddressResolved(host_info)) {
+    continue_decoding_callback_ = decoder_callbacks_->dispatcher().createSchedulableCallback(
+        [this]() { decoder_callbacks_->continueDecoding(); });
+    continue_decoding_callback_->scheduleCallbackNextIteration();
+    return;
+  }
+}
+
+bool NetworkConfigurationFilter::onAddressResolved(
+    const Common::DynamicForwardProxy::DnsHostInfoSharedPtr& host_info) {
+  if (host_info->address()) {
+    setInfo(decoder_callbacks_->streamInfo().getRequestHeaders()->getHostValue(),
+            host_info->address());
+    return true;
+  }
+  decoder_callbacks_->sendLocalReply(Http::Code::BadRequest,
+                                     "Proxy configured but DNS resolution failed", nullptr,
+                                     absl::nullopt, "no_dns_address_for_proxy");
+  return false;
+}
+
 Http::FilterHeadersStatus
 NetworkConfigurationFilter::decodeHeaders(Http::RequestHeaderMap& request_headers, bool) {
-  const auto proxy_settings = connectivity_manager_->getProxySettings();
-
   ENVOY_LOG(trace, "NetworkConfigurationFilter::decodeHeaders", request_headers);
+
+  const auto authority = request_headers.getHostValue();
+  if (authority.empty()) {
+    return Http::FilterHeadersStatus::Continue;
+  }
+
+  // If there is no proxy configured, continue.
+  const auto proxy_settings = connectivity_manager_->getProxySettings();
   if (proxy_settings == nullptr) {
     return Http::FilterHeadersStatus::Continue;
   }
 
+  ENVOY_LOG(trace, "netconf_filter_processing_proxy_for_request", proxy_settings->asString());
+  // If there is a proxy with a raw address, set the information, and continue.
   const auto proxy_address = proxy_settings->address();
-
   if (proxy_address != nullptr) {
     const auto authorityHeader = request_headers.get(AuthorityHeaderName);
-    if (authorityHeader.empty()) {
-      return Http::FilterHeadersStatus::Continue;
-    }
 
-    const auto authority = authorityHeader[0]->value().getStringView();
+    setInfo(request_headers.getHostValue(), proxy_address);
+    return Http::FilterHeadersStatus::Continue;
+  }
 
-    ENVOY_LOG(trace, "netconf_filter_set_proxy_for_request", proxy_settings->asString());
-    decoder_callbacks_->streamInfo().filterState()->setData(
-        Network::Http11ProxyInfoFilterState::key(),
-        std::make_unique<Network::Http11ProxyInfoFilterState>(authority, proxy_address),
-        StreamInfo::FilterState::StateType::ReadOnly,
-        StreamInfo::FilterState::LifeSpan::FilterChain);
+  // If there's no address or hostname, continue.
+  if (proxy_settings->hostname().empty()) {
+    return Http::FilterHeadersStatus::Continue;
   }
 
-  return Http::FilterHeadersStatus::Continue;
+  // If there's a proxy hostname but no way to do a DNS lookup, fail the request.
+  if (!connectivity_manager_->dnsCache()) {
+    decoder_callbacks_->sendLocalReply(Http::Code::BadRequest,
+                                       "Proxy configured but no DNS cache available", nullptr,
+                                       absl::nullopt, "no_dns_cache_for_proxy");
+    return Http::FilterHeadersStatus::StopIteration;
+  }
+
+  // Attempt to load the proxy's hostname from the DNS cache.
+  auto result = connectivity_manager_->dnsCache()->loadDnsCacheEntry(proxy_settings->hostname(),
+                                                                     proxy_settings->port(), *this);
+
+  // If the hostname is not in the cache, pause filter iteration. The DNS cache will call
+  // onLoadDnsCacheComplete when DNS resolution succeeds, fails, or times out and processing
+  // will resume from there.
+  if (result.status_ == Common::DynamicForwardProxy::DnsCache::LoadDnsCacheEntryStatus::Loading) {
+    dns_cache_handle_ = std::move(result.handle_);
+    return Http::FilterHeadersStatus::StopAllIterationAndWatermark;
+  }
+
+  // If the hostname is in cache, set the info and continue.
+  if (result.host_info_.has_value()) {
+    if (onAddressResolved(*result.host_info_)) {
+      return Http::FilterHeadersStatus::Continue;
+    } else {
+      return Http::FilterHeadersStatus::StopIteration;
+    }
+  }
+
+  // If DNS lookup straight up fails, fail the request.
+  decoder_callbacks_->sendLocalReply(Http::Code::BadRequest,
+                                     "Proxy configured but DNS resolution failed", nullptr,
+                                     absl::nullopt, "no_dns_address_for_proxy");
+  return Http::FilterHeadersStatus::StopIteration;
+}
+
+void NetworkConfigurationFilter::setInfo(absl::string_view authority,
+                                         Network::Address::InstanceConstSharedPtr address) {
+  ENVOY_LOG(trace, "netconf_filter_set_proxy_for_request", authority, address->asString());
+  decoder_callbacks_->streamInfo().filterState()->setData(
+      Network::Http11ProxyInfoFilterState::key(),
+      std::make_unique<Network::Http11ProxyInfoFilterState>(authority, address),
+      StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain);
 }
 
 Http::FilterHeadersStatus NetworkConfigurationFilter::encodeHeaders(Http::ResponseHeaderMap&,
@@ -94,6 +162,8 @@ Http::LocalErrorStatus NetworkConfigurationFilter::onLocalReply(const LocalReply
   return Http::LocalErrorStatus::ContinueAndResetStream;
 }
 
+void NetworkConfigurationFilter::onDestroy() { dns_cache_handle_.reset(); }
+
 } // namespace NetworkConfiguration
 } // namespace HttpFilters
 } // namespace Extensions

diff --git a/library/common/extensions/filters/http/network_configuration/filter.h b/library/common/extensions/filters/http/network_configuration/filter.h
@@ -18,8 +18,10 @@ namespace NetworkConfiguration {
 /**
  * Filter to set upstream socket options based on network conditions.
  */
-class NetworkConfigurationFilter final : public Http::PassThroughFilter,
-                                         public Logger::Loggable<Logger::Id::filter> {
+class NetworkConfigurationFilter final
+    : public Http::PassThroughFilter,
+      public Logger::Loggable<Logger::Id::filter>,
+      public Extensions::Common::DynamicForwardProxy::DnsCache::LoadDnsCacheEntryCallbacks {
 public:
   NetworkConfigurationFilter(Network::ConnectivityManagerSharedPtr connectivity_manager,
                              bool enable_drain_post_dns_refresh, bool enable_interface_binding)
@@ -36,11 +38,25 @@ class NetworkConfigurationFilter final : public Http::PassThroughFilter,
   // Http::StreamFilterBase
   Http::LocalErrorStatus onLocalReply(const LocalReplyData&) override;
 
+  // LoadDnsCacheEntryCallbacks
+  void onLoadDnsCacheComplete(
+      const Extensions::Common::DynamicForwardProxy::DnsHostInfoSharedPtr& host_info) override;
+
+  void onDestroy() override;
+
 private:
+  void setInfo(absl::string_view authority, Network::Address::InstanceConstSharedPtr address);
+  bool
+  onAddressResolved(const Extensions::Common::DynamicForwardProxy::DnsHostInfoSharedPtr& host_info);
+
+  // This is only present if there is an active proxy DNS lookup in progress.
+  std::unique_ptr<Extensions::Common::DynamicForwardProxy::DnsCache::LoadDnsCacheEntryHandle>
+      dns_cache_handle_;
   Network::ConnectivityManagerSharedPtr connectivity_manager_;
   StreamInfo::ExtraStreamInfo* extra_stream_info_;
   bool enable_drain_post_dns_refresh_;
   bool enable_interface_binding_;
+  Event::SchedulableCallbackPtr continue_decoding_callback_;
 };
 
 } // namespace NetworkConfiguration

diff --git a/library/common/main_interface.cc b/library/common/main_interface.cc
@@ -67,7 +67,7 @@ envoy_status_t reset_stream(envoy_engine_t engine, envoy_stream_t stream) {
 
 envoy_status_t set_preferred_network(envoy_engine_t engine, envoy_network_t network) {
   envoy_netconf_t configuration_key =
-      Envoy::Network::ConnectivityManager::setPreferredNetwork(network);
+      Envoy::Network::ConnectivityManagerImpl::setPreferredNetwork(network);
   Envoy::EngineHandle::runOnEngineDispatcher(engine, [configuration_key](auto& engine) -> void {
     engine.networkConnectivityManager().refreshDns(configuration_key, true);
   });