diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 466ff56cb79e..2432b2cd05ee 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -62,6 +62,11 @@ removed_config_or_runtime: Removed runtime flag ``envoy.reloadable_features.dns_details`` and legacy code paths. new_features: +- area: dfp + change: | + The DFP cluster will now use the async lookup path to do DNS resolutions for null hosts. This behavioral change + can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.dfp_cluster_resolves_hosts`` + to false. - area: oauth2 change: | Add the option to specify SameSite cookie attribute values for oauth2 supported cookies. diff --git a/envoy/upstream/load_balancer.h b/envoy/upstream/load_balancer.h index f24b96b8a303..88bde536a386 100644 --- a/envoy/upstream/load_balancer.h +++ b/envoy/upstream/load_balancer.h @@ -59,7 +59,10 @@ struct HostSelectionResponse { HostSelectionResponse(HostConstSharedPtr host, std::unique_ptr cancelable = nullptr) : host(host), cancelable(std::move(cancelable)) {} + HostSelectionResponse(HostConstSharedPtr host, std::string details) + : host(host), details(details) {} HostConstSharedPtr host; + std::string details; std::unique_ptr cancelable; }; @@ -151,8 +154,9 @@ class LoadBalancerContext { /* Called by the load balancer when asynchronous host selection completes * @param host supplies the upstream host selected + * @param details gives optional details about the resolution success/failure. */ - virtual void onAsyncHostSelection(HostConstSharedPtr&& host) PURE; + virtual void onAsyncHostSelection(HostConstSharedPtr&& host, std::string details) PURE; }; /** diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 58ceb4bb291d..71f6959b24a3 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -658,6 +658,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, Network::Socket::appendOptions(upstream_options_, callbacks_->getUpstreamSocketOptions()); } + callbacks_->streamInfo().downstreamTiming().setValue( + "envoy.router.host_selection_start_ms", + callbacks_->dispatcher().timeSource().monotonicTime()); auto host_selection_response = cluster->chooseHost(this); if (!host_selection_response.cancelable || !Runtime::runtimeFeatureEnabled("envoy.reloadable_features.async_host_selection")) { @@ -668,7 +671,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, // well as handling unsupported asynchronous host selection by treating it // as host selection failure and calling sendNoHealthyUpstreamResponse. return continueDecodeHeaders(cluster, headers, end_stream, modify_headers, nullptr, - std::move(host_selection_response.host)); + std::move(host_selection_response.host), + std::string(host_selection_response.details)); } ENVOY_STREAM_LOG(debug, "Doing asynchronous host selection\n", *callbacks_); @@ -677,13 +681,14 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, host_selection_cancelable_ = std::move(host_selection_response.cancelable); // Configure a callback to be called on asynchronous host selection. on_host_selected_ = ([this, cluster, end_stream, - modify_headers](Upstream::HostConstSharedPtr&& host) -> void { + modify_headers](Upstream::HostConstSharedPtr&& host, + std::string host_selection_details) -> void { // It should always be safe to call continueDecodeHeaders. In the case the // stream had a local reply before host selection completed, // the lookup should be canceled. bool should_continue_decoding = false; continueDecodeHeaders(cluster, *downstream_headers_, end_stream, modify_headers, - &should_continue_decoding, std::move(host)); + &should_continue_decoding, std::move(host), host_selection_details); // continueDecodeHeaders can itself send a local reply, in which case should_continue_decoding // should be false. If this is not the case, we can continue the filter chain due to successful // asynchronous host selection. @@ -699,23 +704,26 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } // When asynchronous host selection is complete, call the pre-configured on_host_selected_function. -void Filter::onAsyncHostSelection(Upstream::HostConstSharedPtr&& host) { - ENVOY_STREAM_LOG(debug, "Completing asynchronous host selection\n", *callbacks_); +void Filter::onAsyncHostSelection(Upstream::HostConstSharedPtr&& host, std::string details) { + ENVOY_STREAM_LOG(debug, "Completing asynchronous host selection [{}]\n", *callbacks_, details); std::unique_ptr local_scope = std::move(host_selection_cancelable_); - on_host_selected_(std::move(host)); + on_host_selected_(std::move(host), details); } Http::FilterHeadersStatus Filter::continueDecodeHeaders( Upstream::ThreadLocalCluster* cluster, Http::RequestHeaderMap& headers, bool end_stream, std::function modify_headers, bool* should_continue_decoding, - Upstream::HostConstSharedPtr&& selected_host) { + Upstream::HostConstSharedPtr&& selected_host, + absl::optional host_selection_details) { const StreamInfo::FilterStateSharedPtr& filter_state = callbacks_->streamInfo().filterState(); const DebugConfig* debug_config = filter_state->getDataReadOnly(DebugConfig::key()); + callbacks_->streamInfo().downstreamTiming().setValue( + "envoy.router.host_selection_end_ms", callbacks_->dispatcher().timeSource().monotonicTime()); std::unique_ptr generic_conn_pool = createConnPool(*cluster, selected_host); if (!generic_conn_pool) { - sendNoHealthyUpstreamResponse(); + sendNoHealthyUpstreamResponse(host_selection_details); return Http::FilterHeadersStatus::StopIteration; } Upstream::HostDescriptionConstSharedPtr host = generic_conn_pool->host(); @@ -938,12 +946,14 @@ std::unique_ptr Filter::createConnPool(Upstream::ThreadLocalClu callbacks_->streamInfo().protocol(), this, *message); } -void Filter::sendNoHealthyUpstreamResponse() { +void Filter::sendNoHealthyUpstreamResponse(absl::optional optional_details) { callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::NoHealthyUpstream); chargeUpstreamCode(Http::Code::ServiceUnavailable, {}, false); + absl::string_view details = (optional_details.has_value() && !optional_details->empty()) + ? absl::string_view(*optional_details) + : StreamInfo::ResponseCodeDetails::get().NoHealthyUpstream; callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, "no healthy upstream", modify_headers_, - absl::nullopt, - StreamInfo::ResponseCodeDetails::get().NoHealthyUpstream); + absl::nullopt, details); } Http::FilterDataStatus Filter::decodeData(Buffer::Instance& data, bool end_stream) { @@ -2112,11 +2122,14 @@ void Filter::doRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry const auto cluster = config_->cm_.getThreadLocalCluster(route_entry_->clusterName()); std::unique_ptr generic_conn_pool; if (cluster == nullptr) { - sendNoHealthyUpstreamResponse(); + sendNoHealthyUpstreamResponse({}); cleanup(); return; } + callbacks_->streamInfo().downstreamTiming().setValue( + "envoy.router.host_selection_start_ms", + callbacks_->dispatcher().timeSource().monotonicTime()); auto host_selection_response = cluster->chooseHost(this); if (!host_selection_response.cancelable || !Runtime::runtimeFeatureEnabled("envoy.reloadable_features.async_host_selection")) { @@ -2127,26 +2140,31 @@ void Filter::doRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry // well as handling unsupported asynchronous host selection (by treating it // as host selection failure). continueDoRetry(can_send_early_data, can_use_http3, is_timeout_retry, - std::move(host_selection_response.host), *cluster); + std::move(host_selection_response.host), *cluster, + std::string(host_selection_response.details)); } ENVOY_STREAM_LOG(debug, "Handling asynchronous host selection for retry\n", *callbacks_); // Again latch the cancel handle, and set up the callback to be called when host // selection is complete. host_selection_cancelable_ = std::move(host_selection_response.cancelable); - on_host_selected_ = ([this, can_send_early_data, can_use_http3, is_timeout_retry, - cluster](Upstream::HostConstSharedPtr&& host) -> void { - continueDoRetry(can_send_early_data, can_use_http3, is_timeout_retry, std::move(host), - *cluster); - }); + on_host_selected_ = + ([this, can_send_early_data, can_use_http3, is_timeout_retry, + cluster](Upstream::HostConstSharedPtr&& host, std::string host_selection_details) -> void { + continueDoRetry(can_send_early_data, can_use_http3, is_timeout_retry, std::move(host), + *cluster, host_selection_details); + }); } void Filter::continueDoRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry is_timeout_retry, Upstream::HostConstSharedPtr&& host, - Upstream::ThreadLocalCluster& cluster) { + Upstream::ThreadLocalCluster& cluster, + absl::optional host_selection_details) { + callbacks_->streamInfo().downstreamTiming().setValue( + "envoy.router.host_selection_end_ms", callbacks_->dispatcher().timeSource().monotonicTime()); std::unique_ptr generic_conn_pool = createConnPool(cluster, host); if (!generic_conn_pool) { - sendNoHealthyUpstreamResponse(); + sendNoHealthyUpstreamResponse(host_selection_details); cleanup(); return; } diff --git a/source/common/router/router.h b/source/common/router/router.h index f74ed8a13bb1..a6d1544ec583 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -345,7 +345,8 @@ class Filter : Logger::Loggable, continueDecodeHeaders(Upstream::ThreadLocalCluster* cluster, Http::RequestHeaderMap& headers, bool end_stream, std::function modify_headers, - bool* should_continue_decoding, Upstream::HostConstSharedPtr&& host); + bool* should_continue_decoding, Upstream::HostConstSharedPtr&& host, + absl::optional host_selection_detailsi = {}); Http::FilterDataStatus decodeData(Buffer::Instance& data, bool end_stream) override; Http::FilterTrailersStatus decodeTrailers(Http::RequestTrailerMap& trailers) override; @@ -446,7 +447,7 @@ class Filter : Logger::Loggable, return callbacks_->upstreamOverrideHost(); } - void onAsyncHostSelection(Upstream::HostConstSharedPtr&& host) override; + void onAsyncHostSelection(Upstream::HostConstSharedPtr&& host, std::string details) override; /** * Set a computed cookie to be sent with the downstream headers. @@ -569,7 +570,7 @@ class Filter : Logger::Loggable, // if a "good" response comes back and we return downstream, so there is no point in waiting // for the remaining upstream requests to return. void resetOtherUpstreams(UpstreamRequest& upstream_request); - void sendNoHealthyUpstreamResponse(); + void sendNoHealthyUpstreamResponse(absl::optional details); bool setupRedirect(const Http::ResponseHeaderMap& headers); bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream_headers, const Http::ResponseHeaderMap& upstream_headers, @@ -579,7 +580,8 @@ class Filter : Logger::Loggable, absl::optional code); void doRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry is_timeout_retry); void continueDoRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry is_timeout_retry, - Upstream::HostConstSharedPtr&& host, Upstream::ThreadLocalCluster& cluster); + Upstream::HostConstSharedPtr&& host, Upstream::ThreadLocalCluster& cluster, + absl::optional host_selection_details); void runRetryOptionsPredicates(UpstreamRequest& retriable_request); // Called immediately after a non-5xx header is received from upstream, performs stats accounting @@ -603,7 +605,7 @@ class Filter : Logger::Loggable, std::unique_ptr alt_stat_prefix_; const VirtualCluster* request_vcluster_{}; RouteStatsContextOptRef route_stats_context_; - std::function on_host_selected_; + std::function on_host_selected_; std::unique_ptr host_selection_cancelable_; Event::TimerPtr response_timeout_; TimeoutData timeout_; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index a719a94d2616..a906446e340a 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -34,6 +34,7 @@ RUNTIME_GUARD(envoy_reloadable_features_async_host_selection); RUNTIME_GUARD(envoy_reloadable_features_avoid_dfp_cluster_removal_on_cds_update); RUNTIME_GUARD(envoy_reloadable_features_boolean_to_string_fix); RUNTIME_GUARD(envoy_reloadable_features_check_switch_protocol_websocket_handshake); +RUNTIME_GUARD(envoy_reloadable_features_dfp_cluster_resolves_hosts); RUNTIME_GUARD(envoy_reloadable_features_dfp_fail_on_empty_host_header); RUNTIME_GUARD(envoy_reloadable_features_disallow_quic_client_udp_mmsg); RUNTIME_GUARD(envoy_reloadable_features_dns_nodata_noname_is_success); diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 946f0ae7f934..f37eacf809a6 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -2238,6 +2238,9 @@ HostSelectionResponse ClusterManagerImpl::ThreadLocalClusterManagerImpl::Cluster if (host_selection.host || host_selection.cancelable) { return host_selection; } + cluster_info_->trafficStats()->upstream_cx_none_healthy_.inc(); + ENVOY_LOG(debug, "no healthy host"); + return host_selection; } cluster_info_->trafficStats()->upstream_cx_none_healthy_.inc(); diff --git a/source/common/upstream/load_balancer_context_base.h b/source/common/upstream/load_balancer_context_base.h index 57f0c05f3e58..1c745dd65dd2 100644 --- a/source/common/upstream/load_balancer_context_base.h +++ b/source/common/upstream/load_balancer_context_base.h @@ -35,7 +35,7 @@ class LoadBalancerContextBase : public LoadBalancerContext { absl::optional overrideHostToSelect() const override { return {}; } - void onAsyncHostSelection(HostConstSharedPtr&&) override {} + void onAsyncHostSelection(HostConstSharedPtr&&, std::string) override {} }; } // namespace Upstream diff --git a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc index d6457b585f46..68989a919c53 100644 --- a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc +++ b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc @@ -9,6 +9,7 @@ #include "envoy/stream_info/uint32_accessor.h" #include "source/common/http/utility.h" +#include "source/common/network/filter_state_proxy_info.h" #include "source/common/network/transport_socket_options_impl.h" #include "source/common/router/string_accessor_impl.h" #include "source/common/stream_info/uint32_accessor_impl.h" @@ -46,6 +47,18 @@ class DynamicPortObjectFactory : public StreamInfo::FilterState::ObjectFactory { } }; +bool isProxying(StreamInfo::StreamInfo* stream_info) { + if (!stream_info || !(Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.skip_dns_lookup_for_proxied_requests"))) { + return false; + } + const Envoy::StreamInfo::FilterStateSharedPtr& filter_state = stream_info->filterState(); + return filter_state && filter_state->hasData( + Network::Http11ProxyInfoFilterState::key()); + + return false; +} + } // namespace REGISTER_FACTORY(DynamicHostObjectFactory, StreamInfo::FilterState::ObjectFactory); @@ -394,21 +407,66 @@ Cluster::LoadBalancer::chooseHost(Upstream::LoadBalancerContext* context) { if (raw_host.empty()) { ENVOY_LOG(debug, "host empty"); - return {nullptr}; + return {nullptr, "empty_host_header"}; } - std::string host = Common::DynamicForwardProxy::DnsHostInfo::normalizeHostForDfp(raw_host, port); + std::string hostname = + Common::DynamicForwardProxy::DnsHostInfo::normalizeHostForDfp(raw_host, port); if (cluster_.enableSubCluster()) { - return cluster_.chooseHost(host, context); - } - return findHostByName(host); + return cluster_.chooseHost(hostname, context); + } + Upstream::HostConstSharedPtr host = findHostByName(hostname); + bool force_refresh = + Runtime::runtimeFeatureEnabled("envoy.reloadable_features.reresolve_if_no_connections") && + Runtime::runtimeFeatureEnabled("envoy.reloadable_features.dfp_cluster_resolves_hosts") && + host && !host->used(); + if ((host && !force_refresh) || + !Runtime::runtimeFeatureEnabled("envoy.reloadable_features.dfp_cluster_resolves_hosts")) { + return {host}; + } + + // If the host is not found, the DFP cluster cluster can now do asynchronous lookup. + Upstream::ResourceAutoIncDecPtr handle = cluster_.dns_cache_->canCreateDnsRequest(); + + // Return an immediate failure if there's too many requests already. + if (!handle) { + return {nullptr, "dns_cache_pending_requests_overflow"}; + } + + // Attempt to load the host from cache. Generally this will result in async + // resolution so create a DFPHostSelectionHandle to handle this. + std::unique_ptr cancelable = + std::make_unique(context, cluster_, hostname); + bool is_proxying = isProxying(context->requestStreamInfo()); + auto result = cluster_.dns_cache_->loadDnsCacheEntryWithForceRefresh(raw_host, port, is_proxying, + force_refresh, *cancelable); + switch (result.status_) { + case Common::DynamicForwardProxy::DnsCache::LoadDnsCacheEntryStatus::InCache: + return {nullptr, result.host_info_.has_value() ? result.host_info_.value()->details() : ""}; + case Common::DynamicForwardProxy::DnsCache::LoadDnsCacheEntryStatus::Loading: + // Here the DFP kicks off an async lookup. The DFPHostSelectionHandle will + // call onLoadDnsCacheComplete and onAsyncHostSelection unless the + // resolution is canceled by the stream. + cancelable->handle_ = std::move(result.handle_); + cancelable->auto_dec_ = std::move(handle); + return Upstream::HostSelectionResponse{nullptr, std::move(cancelable)}; + case Common::DynamicForwardProxy::DnsCache::LoadDnsCacheEntryStatus::Overflow: + // In the csae of overflow, return immediate failure. + ENVOY_LOG(debug, "host {} lookup failed due to overflow", hostname); + return {nullptr, "dns_cache_overflow"}; + } + return {nullptr}; } Upstream::HostConstSharedPtr Cluster::LoadBalancer::findHostByName(const std::string& host) const { + return cluster_.findHostByName(host); +} + +Upstream::HostConstSharedPtr Cluster::findHostByName(const std::string& host) const { { - absl::ReaderMutexLock lock{&cluster_.host_map_lock_}; - const auto host_it = cluster_.host_map_.find(host); - if (host_it == cluster_.host_map_.end()) { + absl::ReaderMutexLock lock{&host_map_lock_}; + const auto host_it = host_map_.find(host); + if (host_it == host_map_.end()) { ENVOY_LOG(debug, "host {} not found", host); return nullptr; } else { diff --git a/source/extensions/clusters/dynamic_forward_proxy/cluster.h b/source/extensions/clusters/dynamic_forward_proxy/cluster.h index ad408f5fd0d7..6c7e69fd20f1 100644 --- a/source/extensions/clusters/dynamic_forward_proxy/cluster.h +++ b/source/extensions/clusters/dynamic_forward_proxy/cluster.h @@ -53,6 +53,7 @@ class Cluster : public Upstream::BaseDynamicClusterImpl, const int port) override; bool touch(const std::string& cluster_name) override; void checkIdleSubCluster(); + Upstream::HostConstSharedPtr findHostByName(const std::string& host) const; protected: Cluster(const envoy::config::cluster::v3::Cluster& cluster, @@ -140,6 +141,38 @@ class Cluster : public Upstream::BaseDynamicClusterImpl, const Cluster& cluster_; }; + // This acts as the bridge for asynchronous host lookup. If the host is not + // present in the DFP cluster, the DFPHostSelectionHandle will receive a onLoadDnsCacheComplete + // call unless the LoadDnsCacheEntryHandlePtr is destroyed. Destruction of the + // LoadDnsCacheEntryHandlePtr ensures that no callback will occur, at which + // point it is safe to delete the DFPHostSelectionHandle. + struct DFPHostSelectionHandle + : public Upstream::AsyncHostSelectionHandle, + public Common::DynamicForwardProxy::DnsCache::LoadDnsCacheEntryCallbacks { + + DFPHostSelectionHandle(Upstream::LoadBalancerContext* context, const Cluster& cluster, + std::string hostname) + : context_(context), cluster_(cluster), hostname_(hostname){}; + + virtual void cancel() { + // Cancels the DNS callback. + handle_.reset(); + } + + virtual void + onLoadDnsCacheComplete(const Common::DynamicForwardProxy::DnsHostInfoSharedPtr& info) { + Upstream::HostConstSharedPtr host = cluster_.findHostByName(hostname_); + std::string details = info->details(); + context_->onAsyncHostSelection(std::move(host), details); + } + + Upstream::LoadBalancerContext* context_; + Common::DynamicForwardProxy::DnsCache::LoadDnsCacheEntryHandlePtr handle_; + Upstream::ResourceAutoIncDecPtr auto_dec_; + const Cluster& cluster_; + std::string hostname_; + }; + class LoadBalancerFactory : public Upstream::LoadBalancerFactory { public: LoadBalancerFactory(Cluster& cluster) : cluster_(cluster) {} diff --git a/source/extensions/common/dynamic_forward_proxy/dns_cache_impl.cc b/source/extensions/common/dynamic_forward_proxy/dns_cache_impl.cc index d1b3e5c6c5ba..ae5cb2fa00cc 100644 --- a/source/extensions/common/dynamic_forward_proxy/dns_cache_impl.cc +++ b/source/extensions/common/dynamic_forward_proxy/dns_cache_impl.cc @@ -443,6 +443,10 @@ void DnsCacheImpl::finishResolve(const std::string& host, primary_host_info->active_query_->cancel(Network::ActiveDnsQuery::CancelReason::Timeout); } } + bool failure = status == Network::DnsResolver::ResolutionStatus::Failure || response.empty(); + details_with_maybe_trace = absl::StrCat( + (failure ? "dns_resolution_failure{" : ""), + StringUtil::replaceAllEmptySpace(details_with_maybe_trace), (failure ? "}" : "")); bool first_resolve = false; diff --git a/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc b/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc index 500cc2fee8ef..cf981d37dee4 100644 --- a/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc +++ b/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc @@ -38,7 +38,6 @@ struct ResponseStringValues { struct RcDetailsValues { const std::string DnsCacheOverflow = "dns_cache_overflow"; const std::string PendingRequestOverflow = "dynamic_forward_proxy_pending_request_overflow"; - const std::string DnsResolutionFailure = "dns_resolution_failure"; const std::string SubClusterOverflow = "sub_cluster_overflow"; const std::string SubClusterWarmingTimeout = "sub_cluster_warming_timeout"; const std::string DFPClusterIsGone = "dynamic_forward_proxy_cluster_is_gone"; @@ -422,12 +421,9 @@ void ProxyFilter::onDnsResolutionFail(absl::string_view details) { decoder_callbacks_->streamInfo().setResponseFlag( StreamInfo::CoreResponseFlag::DnsResolutionFailed); - std::string details_str = ""; - details_str = StringUtil::replaceAllEmptySpace(details); - ASSERT(details_str != "not_resolved"); - decoder_callbacks_->sendLocalReply( - Http::Code::ServiceUnavailable, ResponseStrings::get().DnsResolutionFailure, nullptr, - absl::nullopt, absl::StrCat(RcDetails::get().DnsResolutionFailure, "{", details_str, "}")); + decoder_callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, + ResponseStrings::get().DnsResolutionFailure, nullptr, + absl::nullopt, details); } void ProxyFilter::onLoadDnsCacheComplete( diff --git a/source/extensions/load_balancing_policies/subset/subset_lb.h b/source/extensions/load_balancing_policies/subset/subset_lb.h index 2354500303b7..f1410ebcf8b2 100644 --- a/source/extensions/load_balancing_policies/subset/subset_lb.h +++ b/source/extensions/load_balancing_policies/subset/subset_lb.h @@ -199,7 +199,7 @@ class SubsetLoadBalancer : public LoadBalancer, Logger::Loggable overrideHostToSelect() const override { return wrapped_->overrideHostToSelect(); } - void onAsyncHostSelection(Upstream::HostConstSharedPtr&&) override {} + void onAsyncHostSelection(Upstream::HostConstSharedPtr&&, std::string) override {} private: LoadBalancerContext* wrapped_; diff --git a/test/config/utility.cc b/test/config/utility.cc index 0cb69923c878..c752048d46db 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -1008,7 +1008,7 @@ void ConfigHelper::finalize(const std::vector& ports) { #endif } - // Make sure we we don't setAsyncLb() when we intend to use a non-default LB algorithm. + // Make sure we don't setAsyncLb() when we intend to use a non-default LB algorithm. for (int i = 0; i < bootstrap_.mutable_static_resources()->clusters_size(); ++i) { auto* cluster = bootstrap_.mutable_static_resources()->mutable_clusters(i); if (cluster->has_load_balancing_policy() && diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc index 4d3103b2bfff..ba7e24b19720 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc @@ -69,7 +69,11 @@ name: dynamic_forward_proxy Network::Test::ipVersionToDnsFamily(GetParam()), max_hosts, host_ttl_, dns_query_timeout, disable_dns_refresh_on_failure, max_pending_requests, key_value_config_, typed_dns_resolver_config); - config_helper_.prependFilter(use_sub_cluster ? filter_use_sub_cluster : filter_use_dns_cache); + if (use_sub_cluster) { + config_helper_.addRuntimeOverride("envoy.reloadable_features.dfp_cluster_resolves_hosts", + "false"); + config_helper_.prependFilter(use_sub_cluster ? filter_use_sub_cluster : filter_use_dns_cache); + } config_helper_.prependFilter(fmt::format(R"EOF( name: stream-info-to-headers-filter @@ -289,8 +293,8 @@ name: envoy.clusters.dynamic_forward_proxy } } - void requestWithUnknownDomainTest(const std::string& typed_dns_resolver_config = "", - const std::string& hostname = "doesnotexist.example.com") { + void requestWithUnknownDomainTest(const std::string& typed_dns_resolver_config, + const std::string& hostname, const std::string details) { useAccessLog("%RESPONSE_CODE_DETAILS%"); initializeWithArgs(1024, 1024, "", typed_dns_resolver_config); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -300,14 +304,14 @@ name: envoy.clusters.dynamic_forward_proxy ASSERT_TRUE(response->waitForEndStream()); EXPECT_EQ("503", response->headers().getStatusValue()); std::string access_log = waitForAccessLog(access_log_name_); - EXPECT_THAT(access_log, HasSubstr("dns_resolution_failure")); + EXPECT_THAT(access_log, HasSubstr(details)); EXPECT_FALSE(StringUtil::hasEmptySpace(access_log)); response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); ASSERT_TRUE(response->waitForEndStream()); EXPECT_EQ("503", response->headers().getStatusValue()); access_log = waitForAccessLog(access_log_name_, 1); - EXPECT_THAT(access_log, HasSubstr("dns_resolution_failure")); + EXPECT_THAT(access_log, HasSubstr(details)); EXPECT_FALSE(StringUtil::hasEmptySpace(access_log)); } @@ -603,13 +607,25 @@ TEST_P(ProxyFilterIntegrationTest, DisableRefreshOnFailureContainsSuccessfulHost auto response1 = codec_client_->makeHeaderOnlyRequest(default_request_headers_); ASSERT_TRUE(response1->waitForEndStream()); EXPECT_EQ("200", response1->headers().getStatusValue()); + EXPECT_EQ(0, test_server_->counter("dns_cache.foo.cache_load")->value()); + EXPECT_EQ(1, test_server_->counter("dns_cache.foo.dns_query_attempt")->value()); // This should result in a cache hit because the previous DNS query succeeded. - EXPECT_LOG_CONTAINS("debug", "cache hit for host 'localhost", { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.dfp_cluster_resolves_hosts")) { + EXPECT_LOG_CONTAINS("debug", "cache hit for host 'localhost", { + auto response2 = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + ASSERT_TRUE(response2->waitForEndStream()); + EXPECT_EQ("200", response2->headers().getStatusValue()); + // No new query attempt. + EXPECT_EQ(1, test_server_->counter("dns_cache.foo.dns_query_attempt")->value()); + }); + } else { auto response2 = codec_client_->makeHeaderOnlyRequest(default_request_headers_); ASSERT_TRUE(response2->waitForEndStream()); EXPECT_EQ("200", response2->headers().getStatusValue()); - }); + // No new query attempt. + EXPECT_EQ(1, test_server_->counter("dns_cache.foo.dns_query_attempt")->value()); + } } // TODO(yanavlasov) Enable per #26642 @@ -624,7 +640,6 @@ TEST_P(ProxyFilterIntegrationTest, FailOnEmptyHostHeader) { auto response = codec_client_->makeHeaderOnlyRequest(request_headers); ASSERT_TRUE(response->waitForEndStream()); - EXPECT_EQ("400", response->headers().getStatusValue()); EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("empty_host_header")); } #endif @@ -690,7 +705,10 @@ TEST_P(ProxyFilterIntegrationTest, ParallelRequestsWithFakeResolver) { // Currently if the first DNS resolution fails, the filter will continue with // a null address. Make sure this mode fails gracefully. -TEST_P(ProxyFilterIntegrationTest, RequestWithUnknownDomain) { requestWithUnknownDomainTest(); } +TEST_P(ProxyFilterIntegrationTest, RequestWithUnknownDomainCares) { + requestWithUnknownDomainTest("", "doesnotexist.example.com", + "cares_norecords:Domain_name_not_found"); +} // TODO(yanavlasov) Enable per #26642 #ifndef ENVOY_ENABLE_UHV @@ -702,8 +720,13 @@ TEST_P(ProxyFilterIntegrationTest, RequestWithSuspectDomain) { auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); ASSERT_TRUE(response->waitForEndStream()); - // The suspicious host will be set to an empty host resulting in a bad request (400). - EXPECT_EQ("400", response->headers().getStatusValue()); + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.dfp_cluster_resolves_hosts")) { + // The suspicious host will be set to an empty host resulting in host not found (503) + EXPECT_EQ("503", response->headers().getStatusValue()); + } else { + // The suspicious host will be set to an empty host resulting in a bad request (400) + EXPECT_EQ("400", response->headers().getStatusValue()); + } std::string access_log = waitForAccessLog(access_log_name_); EXPECT_THAT(access_log, HasSubstr("empty_host_header")); EXPECT_FALSE(StringUtil::hasEmptySpace(access_log)); @@ -716,7 +739,8 @@ TEST_P(ProxyFilterIntegrationTest, RequestWithUnknownDomainGetAddrInfoResolver) typed_dns_resolver_config: name: envoy.network.dns_resolver.getaddrinfo typed_config: - "@type": type.googleapis.com/envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig)EOF"); + "@type": type.googleapis.com/envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig)EOF", + "doesnotexist.example.com", "Name_or_service_not_known"); } TEST_P(ProxyFilterIntegrationTest, RequestWithUnknownDomainAndNoCaching) { diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc index 03b59570e391..2cb4cdcd4cbb 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc @@ -698,9 +698,8 @@ TEST_F(UpstreamResolvedHostFilterStateHelper, IgnoreFilterStateMetadataNullAddre })); EXPECT_CALL(*host_info, address()); - EXPECT_CALL(callbacks_, - sendLocalReply(Http::Code::ServiceUnavailable, Eq("DNS resolution failure"), _, _, - Eq("dns_resolution_failure{}"))); + EXPECT_CALL(callbacks_, sendLocalReply(Http::Code::ServiceUnavailable, + Eq("DNS resolution failure"), _, _, _)); EXPECT_CALL(callbacks_, encodeHeaders_(_, false)); EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, diff --git a/test/integration/async_round_robin_lb.cc b/test/integration/async_round_robin_lb.cc index 9175ee35add9..9d23416943a8 100644 --- a/test/integration/async_round_robin_lb.cc +++ b/test/integration/async_round_robin_lb.cc @@ -38,7 +38,7 @@ class AsyncRoundRobinLoadBalancer : public RoundRobinLoadBalancer { for (std::shared_ptr& handle : handles_) { if (handle->parent_) { handle->parent_->context_->onAsyncHostSelection( - std::move(handle->parent_->preselected_host_)); + std::move(handle->parent_->preselected_host_), ""); } } handles_.clear(); diff --git a/test/integration/filters/stream_info_to_headers_filter.cc b/test/integration/filters/stream_info_to_headers_filter.cc index 489e4d561440..3b792c495e65 100644 --- a/test/integration/filters/stream_info_to_headers_filter.cc +++ b/test/integration/filters/stream_info_to_headers_filter.cc @@ -2,6 +2,7 @@ #include "envoy/server/filter_config.h" #include "source/common/protobuf/protobuf.h" +#include "source/common/runtime/runtime_features.h" #include "source/extensions/filters/http/common/pass_through_filter.h" #include "test/extensions/filters/http/common/empty_http_filter_config.h" @@ -58,8 +59,12 @@ class StreamInfoToHeadersFilter : public Http::PassThroughFilter { } Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, bool) override { - const std::string dns_start = "envoy.dynamic_forward_proxy.dns_start_ms"; - const std::string dns_end = "envoy.dynamic_forward_proxy.dns_end_ms"; + std::string dns_start = "envoy.dynamic_forward_proxy.dns_start_ms"; + std::string dns_end = "envoy.dynamic_forward_proxy.dns_end_ms"; + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.dfp_cluster_resolves_hosts")) { + dns_start = "envoy.router.host_selection_start_ms"; + dns_end = "envoy.router.host_selection_end_ms"; + } StreamInfo::StreamInfo& stream_info = decoder_callbacks_->streamInfo(); const StreamInfo::StreamInfo& conn_stream_info = decoder_callbacks_->connection()->streamInfo(); diff --git a/test/mocks/upstream/load_balancer_context.h b/test/mocks/upstream/load_balancer_context.h index ed704f160d99..a3716c77e61b 100644 --- a/test/mocks/upstream/load_balancer_context.h +++ b/test/mocks/upstream/load_balancer_context.h @@ -26,7 +26,7 @@ class MockLoadBalancerContext : public LoadBalancerContext { MOCK_METHOD(Network::TransportSocketOptionsConstSharedPtr, upstreamTransportSocketOptions, (), (const)); MOCK_METHOD(absl::optional, overrideHostToSelect, (), (const)); - MOCK_METHOD(void, onAsyncHostSelection, (HostConstSharedPtr && host)); + MOCK_METHOD(void, onAsyncHostSelection, (HostConstSharedPtr && host, std::string details)); private: HealthyAndDegradedLoad priority_load_;