diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 8b2cfad31f1a..d5152e64d2be 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -34,6 +34,9 @@ minor_behavior_changes: Changes in ``AsyncStreamImpl`` now propagate tracing context headers in bidirectional streams when using :ref:`Envoy gRPC client `. Previously, tracing context headers were not being set when calling external services such as ``ext_proc``. +- area: http + change: | + Fixed host header changes for shadow requests to properly handle ipv6 addresses. - area: tracers change: | Set status code for OpenTelemetry tracers (previously unset). diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 0a9e586305b3..fa4c709b9f3c 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -441,6 +441,7 @@ envoy_cc_library( "//envoy/router:shadow_writer_interface", "//envoy/upstream:cluster_manager_interface", "//source/common/common:assert_lib", + "//source/common/http:header_utility_lib", "//source/common/http:headers_lib", ], ) diff --git a/source/common/router/shadow_writer_impl.cc b/source/common/router/shadow_writer_impl.cc index 299ea3d10573..83449bb4d8b1 100644 --- a/source/common/router/shadow_writer_impl.cc +++ b/source/common/router/shadow_writer_impl.cc @@ -4,6 +4,7 @@ #include #include "source/common/common/assert.h" +#include "source/common/http/header_utility.h" #include "source/common/http/headers.h" #include "absl/strings/str_join.h" @@ -17,9 +18,12 @@ std::string shadowAppendedHost(absl::string_view host) { ASSERT(!host.empty()); // Switch authority to add a shadow postfix. This allows upstream logging to // make more sense. - auto parts = StringUtil::splitToken(host, ":"); - ASSERT(!parts.empty() && parts.size() <= 2); - return parts.size() == 2 ? absl::StrJoin(parts, "-shadow:") : absl::StrCat(host, "-shadow"); + absl::string_view::size_type port_start = Http::HeaderUtility::getPortStart(host); + if (port_start == absl::string_view::npos) { + return absl::StrCat(host, "-shadow"); + } + return absl::StrCat(host.substr(0, port_start), "-shadow", + host.substr(port_start, host.length())); } } // namespace diff --git a/test/integration/shadow_policy_integration_test.cc b/test/integration/shadow_policy_integration_test.cc index c84bb83fed37..1bfd9a4d7b8e 100644 --- a/test/integration/shadow_policy_integration_test.cc +++ b/test/integration/shadow_policy_integration_test.cc @@ -948,6 +948,31 @@ TEST_P(ShadowPolicyIntegrationTest, ShadowedClusterHostHeaderAppendsSuffix) { EXPECT_EQ(mirror_headers_->Host()->value().getStringView(), "sni.lyft.com-shadow"); } +TEST_P(ShadowPolicyIntegrationTest, ShadowedClusterHostHeaderAppendsSuffixAddresses) { + initialConfigSetup("cluster_1", ""); + // By default `disable_shadow_host_suffix_append` is "false". + config_helper_.addConfigModifier( + [=](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { + auto* mirror_policy = hcm.mutable_route_config() + ->mutable_virtual_hosts(0) + ->mutable_routes(0) + ->mutable_route() + ->add_request_mirror_policies(); + mirror_policy->set_cluster("cluster_1"); + }); + + initialize(); + default_request_headers_.setHost(fake_upstreams_[0]->localAddress()->asStringView()); + sendRequestAndValidateResponse(); + // Ensure shadowed host header has suffix "-shadow". + EXPECT_EQ(upstream_headers_->getHostValue(), fake_upstreams_[0]->localAddress()->asStringView()); + EXPECT_EQ(mirror_headers_->getHostValue(), + absl::StrCat(version_ == Network::Address::IpVersion::v4 ? "127.0.0.1" : "[::1]", + "-shadow:", fake_upstreams_[0]->localAddress()->ip()->port())) + << mirror_headers_->getHostValue(); +} + TEST_P(ShadowPolicyIntegrationTest, ShadowedClusterHostHeaderDisabledAppendSuffix) { initialConfigSetup("cluster_1", ""); config_helper_.addConfigModifier(