shadow: fixing hostnames for ipv6 addresses

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

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 <envoy_v3_api_field_config.core.v3.GrpcService.envoy_grpc>`. 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).

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",
     ],
 )

source/common/router/shadow_writer_impl.cc

@@ -4,6 +4,7 @@
 #include <string>
 
 #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

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(