From a404f0d8dad39b3adc1d51353ba06bbd9ff92176 Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Wed, 15 Oct 2025 23:46:03 +0200 Subject: [PATCH] dfp: deprecate flag dfp_cluster_resolves_hosts and remove legacy code paths Signed-off-by: Rohit Agrawal --- changelogs/current.yaml | 3 ++ source/common/runtime/runtime_features.cc | 1 - .../clusters/dynamic_forward_proxy/cluster.cc | 4 +- .../filters/http/dynamic_forward_proxy/BUILD | 36 --------------- .../proxy_filter_integration_test.cc | 44 ++++--------------- .../filters/stream_info_to_headers_filter.cc | 6 +-- 6 files changed, 15 insertions(+), 79 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 6d938998b49fd..3dee70ab05d25 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -24,6 +24,9 @@ removed_config_or_runtime: - area: http change: | Removed runtime guard ``envoy.reloadable_features.http1_balsa_allow_cr_or_lf_at_request_start`` and legacy code paths. +- area: dfp + change: | + Removed runtime guard ``envoy.reloadable_features.dfp_cluster_resolves_hosts`` and legacy code paths. new_features: diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 70249299a7670..27e9344ed86f8 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -36,7 +36,6 @@ // problem of the bugs being found after the old code path has been removed. RUNTIME_GUARD(envoy_reloadable_features_async_host_selection); RUNTIME_GUARD(envoy_reloadable_features_decouple_explicit_drain_pools_and_dns_refresh); -RUNTIME_GUARD(envoy_reloadable_features_dfp_cluster_resolves_hosts); RUNTIME_GUARD(envoy_reloadable_features_disallow_quic_client_udp_mmsg); RUNTIME_GUARD(envoy_reloadable_features_enable_cel_regex_precompilation); RUNTIME_GUARD(envoy_reloadable_features_enable_compression_bomb_protection); diff --git a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc index f9a63fa12743f..4f479acf196db 100644 --- a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc +++ b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc @@ -432,10 +432,8 @@ Cluster::LoadBalancer::chooseHost(Upstream::LoadBalancerContext* 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")) { + if (host && !force_refresh) { return {host}; } diff --git a/test/extensions/filters/http/dynamic_forward_proxy/BUILD b/test/extensions/filters/http/dynamic_forward_proxy/BUILD index 2a0946b675689..ae4bcf82cc0d7 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/BUILD +++ b/test/extensions/filters/http/dynamic_forward_proxy/BUILD @@ -112,39 +112,3 @@ envoy_extension_cc_test( "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", ], ) - -envoy_extension_cc_test( - name = "legacy_proxy_filter_integration_test", - size = "large", - srcs = ["proxy_filter_integration_test.cc"], - args = [ - "--runtime-feature-disable-for-tests=envoy.reloadable_features.dfp_cluster_resolves_hosts", - ], - data = [ - "//test/config/integration/certs", - ], - extension_names = ["envoy.filters.http.dynamic_forward_proxy"], - rbe_pool = "2core", - # TODO(envoyproxy/windows-dev): Diagnose failure shown on clang-cl build, see: - # https://gist.github.com/wrowe/a152cb1d12c2f751916122aed39d8517 - tags = ["fails_on_clang_cl"], - deps = [ - ":modify_host_filter_lib", - ":test_resolver_lib", - "//source/extensions/clusters/dns:dns_cluster_lib", - "//source/extensions/clusters/dynamic_forward_proxy:cluster", - "//source/extensions/filters/http/dynamic_forward_proxy:config", - "//source/extensions/filters/http/set_filter_state:config", - "//source/extensions/key_value/file_based:config_lib", - "//source/extensions/network/dns_resolver/getaddrinfo:config", - "//test/integration:http_integration_lib", - "//test/integration/filters:stream_info_to_headers_filter_lib", - "//test/test_common:test_runtime_lib", - "//test/test_common:threadsafe_singleton_injector_lib", - "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", - "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", - "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", - "@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto", - "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", - ], -) 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 ad7ae5dcf4d9a..3a6af438fc6c2 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 @@ -148,14 +148,9 @@ name: stream-info-to-headers-filter if (prepend_custom_filter_config_yaml.has_value()) { // Prepend DFP filter. - if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.dfp_cluster_resolves_hosts") || - use_dfp_even_when_cluster_resolves_hosts) { + if (use_dfp_even_when_cluster_resolves_hosts) { config_helper_.prependFilter(use_sub_cluster ? filter_use_sub_cluster : filter_use_dns_cache); - } else if (use_sub_cluster) { - config_helper_.addRuntimeOverride("envoy.reloadable_features.dfp_cluster_resolves_hosts", - "false"); - config_helper_.prependFilter(filter_use_sub_cluster); } // Prepend the custom_filter from the parameter. @@ -164,14 +159,9 @@ name: stream-info-to-headers-filter // Prepend stream_info_filter. config_helper_.prependFilter(stream_info_filter_config_str); } else { - if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.dfp_cluster_resolves_hosts") || - use_dfp_even_when_cluster_resolves_hosts) { + if (use_dfp_even_when_cluster_resolves_hosts) { config_helper_.prependFilter(use_sub_cluster ? filter_use_sub_cluster : filter_use_dns_cache); - } else if (use_sub_cluster) { - config_helper_.addRuntimeOverride("envoy.reloadable_features.dfp_cluster_resolves_hosts", - "false"); - config_helper_.prependFilter(filter_use_sub_cluster); } config_helper_.prependFilter(stream_info_filter_config_str); @@ -687,21 +677,11 @@ TEST_P(ProxyFilterIntegrationTest, DisableRefreshOnFailureContainsSuccessfulHost 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. - 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()); - } + 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 @@ -788,13 +768,8 @@ TEST_P(ProxyFilterIntegrationTest, RequestWithSuspectDomain) { auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); ASSERT_TRUE(response->waitForEndStream()); - 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()); - } + // The suspicious host will be set to an empty host resulting in host not found (503). + EXPECT_EQ("503", 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)); @@ -1687,7 +1662,6 @@ TEST_P(ProxyFilterIntegrationTest, ResetStreamDuringDnsLookup) { // also requires the Host header to be modified between DFP and Router filters to trigger abnormal // behavior in the DNS resolution processing loop. TEST_P(ProxyFilterIntegrationTest, DoubleResolution) { - config_helper_.addRuntimeOverride("envoy.reloadable_features.dfp_cluster_resolves_hosts", "true"); config_helper_.addRuntimeOverride( "envoy.reloadable_features.skip_dns_lookup_for_proxied_requests", "true"); upstream_tls_ = false; diff --git a/test/integration/filters/stream_info_to_headers_filter.cc b/test/integration/filters/stream_info_to_headers_filter.cc index 2036909af97af..e2da6177bbb06 100644 --- a/test/integration/filters/stream_info_to_headers_filter.cc +++ b/test/integration/filters/stream_info_to_headers_filter.cc @@ -61,10 +61,8 @@ class StreamInfoToHeadersFilter : public Http::PassThroughFilter { Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, bool) override { 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"; - } + 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();