From 4c87a04e6aa66ac9e77e87cb0f99710597733c0b Mon Sep 17 00:00:00 2001 From: Keith Mattix II Date: Wed, 21 Jan 2026 01:11:12 +0000 Subject: [PATCH 01/13] Add Baggage metadata propagation Signed-off-by: Keith Mattix II --- .../filters/http/peer_metadata/config.proto | 4 +- .../filters/http/peer_metadata/filter.cc | 46 ++++ .../filters/http/peer_metadata/filter.h | 13 + .../filters/http/peer_metadata/filter_test.cc | 232 ++++++++++++++++++ 4 files changed, 293 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/http/peer_metadata/config.proto b/source/extensions/filters/http/peer_metadata/config.proto index 04e81f4f6b5..e44c2fcffb5 100644 --- a/source/extensions/filters/http/peer_metadata/config.proto +++ b/source/extensions/filters/http/peer_metadata/config.proto @@ -20,8 +20,7 @@ package io.istio.http.peer_metadata; // Peer metadata provider filter. This filter encapsulates the discovery of the // peer telemetry attributes for consumption by the telemetry filters. message Config { - // DEPRECATED. - // This method uses `baggage` header encoding. + // This method uses `baggage` header encoding. Only used for HTTP CONNECT tunnels. message Baggage { } @@ -64,6 +63,7 @@ message Config { message PropagationMethod { oneof method_specifier { IstioHeaders istio_headers = 1; + Baggage baggage = 2; } } diff --git a/source/extensions/filters/http/peer_metadata/filter.cc b/source/extensions/filters/http/peer_metadata/filter.cc index f4b93cad446..48eb0b4b91e 100644 --- a/source/extensions/filters/http/peer_metadata/filter.cc +++ b/source/extensions/filters/http/peer_metadata/filter.cc @@ -176,6 +176,47 @@ void MXPropagationMethod::inject(const StreamInfo::StreamInfo& info, Http::Heade } } +BaggagePropagationMethod::BaggagePropagationMethod( + Server::Configuration::ServerFactoryContext& factory_context, + const io::istio::http::peer_metadata::Config_Baggage&) + : value_(computeBaggageValue(factory_context)) {} + +std::string BaggagePropagationMethod::computeBaggageValue( + Server::Configuration::ServerFactoryContext& factory_context) const { + const auto obj = Istio::Common::convertStructToWorkloadMetadata( + factory_context.localInfo().node().metadata()); + std::vector parts; + + // Map the workload metadata fields to baggage tokens + const std::vector> field_to_baggage = { + {Istio::Common::NamespaceNameToken, "k8s.namespace.name"}, + {Istio::Common::ClusterNameToken, "k8s.cluster.name"}, + {Istio::Common::ServiceNameToken, "service.name"}, + {Istio::Common::ServiceVersionToken, "service.version"}, + {Istio::Common::AppNameToken, "app.name"}, + {Istio::Common::AppVersionToken, "app.version"}, + {Istio::Common::WorkloadNameToken, "k8s.workload.name"}, + {Istio::Common::WorkloadTypeToken, "k8s.workload.type"}, + {Istio::Common::InstanceNameToken, "k8s.instance.name"}, + }; + + for (const auto& [field_name, baggage_key] : field_to_baggage) { + const auto field_result = obj->getField(field_name); + if (auto field_value = std::get_if(&field_result)) { + if (!field_value->empty()) { + parts.push_back(absl::StrCat(baggage_key, "=", *field_value)); + } + } + } + return absl::StrJoin(parts, ","); +} + +void BaggagePropagationMethod::inject(const StreamInfo::StreamInfo&, + Http::HeaderMap& headers, Context&) const { + headers.setReference(Headers::get().Baggage, value_); +} + + FilterConfig::FilterConfig(const io::istio::http::peer_metadata::Config& config, Server::Configuration::FactoryContext& factory_context) : shared_with_upstream_(config.shared_with_upstream()), @@ -233,6 +274,11 @@ std::vector FilterConfig::buildPropagationMethods( std::make_unique(downstream, factory_context.serverFactoryContext(), additional_labels, method.istio_headers())); break; + case io::istio::http::peer_metadata::Config::PropagationMethod::MethodSpecifierCase:: + kBaggage: + methods.push_back(std::make_unique( + factory_context.serverFactoryContext(), method.baggage())); + break; default: break; } diff --git a/source/extensions/filters/http/peer_metadata/filter.h b/source/extensions/filters/http/peer_metadata/filter.h index 94da2a86c83..4434d167bf9 100644 --- a/source/extensions/filters/http/peer_metadata/filter.h +++ b/source/extensions/filters/http/peer_metadata/filter.h @@ -20,6 +20,7 @@ #include "source/extensions/filters/http/peer_metadata/config.pb.h" #include "source/extensions/common/workload_discovery/api.h" #include "source/common/singleton/const_singleton.h" +#include namespace Envoy { namespace Extensions { @@ -30,6 +31,7 @@ using ::Envoy::Extensions::Filters::Common::Expr::CelStatePrototype; using ::Envoy::Extensions::Filters::Common::Expr::CelStateType; struct HeaderValues { + const Http::LowerCaseString Baggage{"baggage"}; const Http::LowerCaseString ExchangeMetadataHeader{"x-envoy-peer-metadata"}; const Http::LowerCaseString ExchangeMetadataHeaderId{"x-envoy-peer-metadata-id"}; }; @@ -99,6 +101,17 @@ class MXPropagationMethod : public PropagationMethod { bool skipMXHeaders(const bool, const StreamInfo::StreamInfo&) const; }; +class BaggagePropagationMethod : public PropagationMethod { +public: + BaggagePropagationMethod(Server::Configuration::ServerFactoryContext& factory_context, + const io::istio::http::peer_metadata::Config_Baggage&); + void inject(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const override; + +private: + std::string computeBaggageValue(Server::Configuration::ServerFactoryContext&) const; + const std::string value_; +}; + class FilterConfig : public Logger::Loggable { public: FilterConfig(const io::istio::http::peer_metadata::Config&, diff --git a/source/extensions/filters/http/peer_metadata/filter_test.cc b/source/extensions/filters/http/peer_metadata/filter_test.cc index 995da18b224..3c9c7368e89 100644 --- a/source/extensions/filters/http/peer_metadata/filter_test.cc +++ b/source/extensions/filters/http/peer_metadata/filter_test.cc @@ -488,6 +488,238 @@ TEST_F(PeerMetadataTest, UpstreamMXPropagationSkipPassthrough) { checkNoPeer(false); } +TEST_F(PeerMetadataTest, DownstreamBaggagePropagation) { + initialize(R"EOF( + downstream_propagation: + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(1, response_headers_.size()); + EXPECT_TRUE(response_headers_.has(Headers::get().Baggage)); + checkNoPeer(true); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, UpstreamBaggagePropagation) { + initialize(R"EOF( + upstream_propagation: + - baggage: {} + )EOF"); + EXPECT_EQ(1, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + EXPECT_TRUE(request_headers_.has(Headers::get().Baggage)); + checkNoPeer(true); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, BothDirectionsBaggagePropagation) { + initialize(R"EOF( + downstream_propagation: + - baggage: {} + upstream_propagation: + - baggage: {} + )EOF"); + EXPECT_EQ(1, request_headers_.size()); + EXPECT_EQ(1, response_headers_.size()); + EXPECT_TRUE(request_headers_.has(Headers::get().Baggage)); + EXPECT_TRUE(response_headers_.has(Headers::get().Baggage)); + checkNoPeer(true); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, BaggagePropagationWithNodeMetadata) { + // Setup node metadata that would be converted to baggage + auto& node = context_.server_factory_context_.local_info_.node_; + TestUtility::loadFromYaml(R"EOF( + metadata: + NAMESPACE: production + CLUSTER_ID: test-cluster + WORKLOAD_NAME: test-workload + NAME: test-instance + LABELS: + app: test-app + version: v1.0 + service.istio.io/canonical-name: test-service + service.istio.io/canonical-revision: main + )EOF", node); + + initialize(R"EOF( + downstream_propagation: + - baggage: {} + )EOF"); + + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(1, response_headers_.size()); + + const auto baggage_header = response_headers_.get(Headers::get().Baggage); + ASSERT_FALSE(baggage_header.empty()); + + std::string baggage_value = std::string(baggage_header[0]->value().getStringView()); + // Verify baggage contains expected key-value pairs + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.namespace.name=production")); + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.cluster.name=test-cluster")); + EXPECT_TRUE(absl::StrContains(baggage_value, "app.name=test-app")); + EXPECT_TRUE(absl::StrContains(baggage_value, "app.version=v1.0")); + EXPECT_TRUE(absl::StrContains(baggage_value, "service.name=test-service")); + EXPECT_TRUE(absl::StrContains(baggage_value, "service.version=main")); + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.workload.name=test-workload")); + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.instance.name=test-instance")); +} + +// Test class specifically for BaggagePropagationMethod unit tests +class BaggagePropagationMethodTest : public testing::Test { +protected: + BaggagePropagationMethodTest() = default; + + void SetUp() override { + TestUtility::loadFromYaml(R"EOF( + metadata: + NAMESPACE: test-namespace + CLUSTER_ID: sample-cluster + WORKLOAD_NAME: sample-workload + NAME: sample-instance + LABELS: + app: sample-app + version: v2.1 + service.istio.io/canonical-name: sample-service + service.istio.io/canonical-revision: stable + )EOF", context_.server_factory_context_.local_info_.node_); + } + + NiceMock context_; + NiceMock stream_info_; +}; + +TEST_F(BaggagePropagationMethodTest, DownstreamBaggageInjection) { + io::istio::http::peer_metadata::Config_Baggage baggage_config; + BaggagePropagationMethod method(context_.server_factory_context_, baggage_config); + + Http::TestResponseHeaderMapImpl headers; + Context ctx; + + method.inject(stream_info_, headers, ctx); + + EXPECT_EQ(1, headers.size()); + const auto baggage_header = headers.get(Headers::get().Baggage); + ASSERT_FALSE(baggage_header.empty()); + + std::string baggage_value = std::string(baggage_header[0]->value().getStringView()); + + // Verify all expected tokens are present + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.namespace.name=test-namespace")); + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.cluster.name=sample-cluster")); + EXPECT_TRUE(absl::StrContains(baggage_value, "service.name=sample-service")); + EXPECT_TRUE(absl::StrContains(baggage_value, "service.version=stable")); + EXPECT_TRUE(absl::StrContains(baggage_value, "app.name=sample-app")); + EXPECT_TRUE(absl::StrContains(baggage_value, "app.version=v2.1")); + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.workload.name=sample-workload")); + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.instance.name=sample-instance")); +} + +TEST_F(BaggagePropagationMethodTest, UpstreamBaggageInjection) { + io::istio::http::peer_metadata::Config_Baggage baggage_config; + BaggagePropagationMethod method(context_.server_factory_context_, baggage_config); + + Http::TestRequestHeaderMapImpl headers; + Context ctx; + + method.inject(stream_info_, headers, ctx); + + EXPECT_EQ(1, headers.size()); + const auto baggage_header = headers.get(Headers::get().Baggage); + ASSERT_FALSE(baggage_header.empty()); + + std::string baggage_value = std::string(baggage_header[0]->value().getStringView()); + + // Verify tokens are properly formatted + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.namespace.name=test-namespace")); + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.cluster.name=sample-cluster")); + + // Check that values are comma-separated + std::vector parts = absl::StrSplit(baggage_value, ','); + EXPECT_GT(parts.size(), 1); + + // Each part should be in key=value format + for (const auto& part : parts) { + EXPECT_TRUE(absl::StrContains(part, "=")); + } +} + +TEST_F(BaggagePropagationMethodTest, EmptyMetadataBaggage) { + // Reset node metadata to empty + context_.server_factory_context_.local_info_.node_.Clear(); + + io::istio::http::peer_metadata::Config_Baggage baggage_config; + BaggagePropagationMethod method(context_.server_factory_context_, baggage_config); + + Http::TestResponseHeaderMapImpl headers; + Context ctx; + + method.inject(stream_info_, headers, ctx); + + EXPECT_EQ(1, headers.size()); + const auto baggage_header = headers.get(Headers::get().Baggage); + ASSERT_FALSE(baggage_header.empty()); + + // With empty metadata, baggage should contain only default workload type + std::string baggage_value = std::string(baggage_header[0]->value().getStringView()); + EXPECT_EQ("k8s.workload.type=unknown", baggage_value); +} + +TEST_F(BaggagePropagationMethodTest, PartialMetadataBaggage) { + // Setup node metadata with only some fields + TestUtility::loadFromYaml(R"EOF( + metadata: + NAMESPACE: partial-namespace + LABELS: + app: partial-app + # Missing other fields like version, cluster, etc. + )EOF", context_.server_factory_context_.local_info_.node_); + + io::istio::http::peer_metadata::Config_Baggage baggage_config; + BaggagePropagationMethod method(context_.server_factory_context_, baggage_config); + + Http::TestRequestHeaderMapImpl headers; + Context ctx; + + method.inject(stream_info_, headers, ctx); + + EXPECT_EQ(1, headers.size()); + const auto baggage_header = headers.get(Headers::get().Baggage); + ASSERT_FALSE(baggage_header.empty()); + + std::string baggage_value = std::string(baggage_header[0]->value().getStringView()); + + // Should contain only the fields that were present + EXPECT_TRUE(absl::StrContains(baggage_value, "k8s.namespace.name=partial-namespace")); + EXPECT_TRUE(absl::StrContains(baggage_value, "app.name=partial-app")); + + // Should not contain fields that were not present + EXPECT_FALSE(absl::StrContains(baggage_value, "app.version=")); + EXPECT_FALSE(absl::StrContains(baggage_value, "k8s.cluster.name=")); +} + +TEST_F(PeerMetadataTest, BaggagePropagationWithMixedConfig) { + initialize(R"EOF( + downstream_propagation: + - baggage: {} + - istio_headers: {} + upstream_propagation: + - baggage: {} + - istio_headers: {} + )EOF"); + + // Baggage should always be propagated, Istio headers are also propagated for upstream only + EXPECT_EQ(3, request_headers_.size()); // baggage + istio headers (id + metadata) + EXPECT_EQ(1, response_headers_.size()); // baggage only (no discovery, so no MX downstream) + + EXPECT_TRUE(request_headers_.has(Headers::get().Baggage)); + EXPECT_TRUE(request_headers_.has(Headers::get().ExchangeMetadataHeaderId)); + EXPECT_TRUE(request_headers_.has(Headers::get().ExchangeMetadataHeader)); + + EXPECT_TRUE(response_headers_.has(Headers::get().Baggage)); +} + } // namespace } // namespace PeerMetadata } // namespace HttpFilters From c429b9d0380a78d0ee67977342d262dc03c5af2b Mon Sep 17 00:00:00 2001 From: Keith Mattix II Date: Wed, 21 Jan 2026 01:23:07 +0000 Subject: [PATCH 02/13] clang-tidy Signed-off-by: Keith Mattix II --- .../extensions/filters/http/peer_metadata/filter.cc | 12 +++++------- .../filters/http/peer_metadata/filter_test.cc | 11 +++++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/source/extensions/filters/http/peer_metadata/filter.cc b/source/extensions/filters/http/peer_metadata/filter.cc index 48eb0b4b91e..1c2c28195d5 100644 --- a/source/extensions/filters/http/peer_metadata/filter.cc +++ b/source/extensions/filters/http/peer_metadata/filter.cc @@ -183,8 +183,8 @@ BaggagePropagationMethod::BaggagePropagationMethod( std::string BaggagePropagationMethod::computeBaggageValue( Server::Configuration::ServerFactoryContext& factory_context) const { - const auto obj = Istio::Common::convertStructToWorkloadMetadata( - factory_context.localInfo().node().metadata()); + const auto obj = + Istio::Common::convertStructToWorkloadMetadata(factory_context.localInfo().node().metadata()); std::vector parts; // Map the workload metadata fields to baggage tokens @@ -211,12 +211,11 @@ std::string BaggagePropagationMethod::computeBaggageValue( return absl::StrJoin(parts, ","); } -void BaggagePropagationMethod::inject(const StreamInfo::StreamInfo&, - Http::HeaderMap& headers, Context&) const { +void BaggagePropagationMethod::inject(const StreamInfo::StreamInfo&, Http::HeaderMap& headers, + Context&) const { headers.setReference(Headers::get().Baggage, value_); } - FilterConfig::FilterConfig(const io::istio::http::peer_metadata::Config& config, Server::Configuration::FactoryContext& factory_context) : shared_with_upstream_(config.shared_with_upstream()), @@ -274,8 +273,7 @@ std::vector FilterConfig::buildPropagationMethods( std::make_unique(downstream, factory_context.serverFactoryContext(), additional_labels, method.istio_headers())); break; - case io::istio::http::peer_metadata::Config::PropagationMethod::MethodSpecifierCase:: - kBaggage: + case io::istio::http::peer_metadata::Config::PropagationMethod::MethodSpecifierCase::kBaggage: methods.push_back(std::make_unique( factory_context.serverFactoryContext(), method.baggage())); break; diff --git a/source/extensions/filters/http/peer_metadata/filter_test.cc b/source/extensions/filters/http/peer_metadata/filter_test.cc index 3c9c7368e89..af463091818 100644 --- a/source/extensions/filters/http/peer_metadata/filter_test.cc +++ b/source/extensions/filters/http/peer_metadata/filter_test.cc @@ -541,7 +541,8 @@ TEST_F(PeerMetadataTest, BaggagePropagationWithNodeMetadata) { version: v1.0 service.istio.io/canonical-name: test-service service.istio.io/canonical-revision: main - )EOF", node); + )EOF", + node); initialize(R"EOF( downstream_propagation: @@ -583,7 +584,8 @@ class BaggagePropagationMethodTest : public testing::Test { version: v2.1 service.istio.io/canonical-name: sample-service service.istio.io/canonical-revision: stable - )EOF", context_.server_factory_context_.local_info_.node_); + )EOF", + context_.server_factory_context_.local_info_.node_); } NiceMock context_; @@ -674,7 +676,8 @@ TEST_F(BaggagePropagationMethodTest, PartialMetadataBaggage) { LABELS: app: partial-app # Missing other fields like version, cluster, etc. - )EOF", context_.server_factory_context_.local_info_.node_); + )EOF", + context_.server_factory_context_.local_info_.node_); io::istio::http::peer_metadata::Config_Baggage baggage_config; BaggagePropagationMethod method(context_.server_factory_context_, baggage_config); @@ -710,7 +713,7 @@ TEST_F(PeerMetadataTest, BaggagePropagationWithMixedConfig) { )EOF"); // Baggage should always be propagated, Istio headers are also propagated for upstream only - EXPECT_EQ(3, request_headers_.size()); // baggage + istio headers (id + metadata) + EXPECT_EQ(3, request_headers_.size()); // baggage + istio headers (id + metadata) EXPECT_EQ(1, response_headers_.size()); // baggage only (no discovery, so no MX downstream) EXPECT_TRUE(request_headers_.has(Headers::get().Baggage)); From 7d228754f03d6c1617a72a369d1e6ca923062d8e Mon Sep 17 00:00:00 2001 From: Gustavo Date: Thu, 22 Jan 2026 09:23:54 +0000 Subject: [PATCH 03/13] basics for baggage discovery downstream --- scripts/release-binary.sh | 293 +++++++------- .../filters/http/peer_metadata/filter.cc | 25 ++ .../filters/http/peer_metadata/filter.h | 11 +- .../filters/http/peer_metadata/filter_test.cc | 357 ++++++++++++++++++ 4 files changed, 539 insertions(+), 147 deletions(-) diff --git a/scripts/release-binary.sh b/scripts/release-binary.sh index b2419a6407e..75b4c7d095b 100755 --- a/scripts/release-binary.sh +++ b/scripts/release-binary.sh @@ -1,168 +1,169 @@ -#!/bin/bash -# -# Copyright 2016 Istio Authors. All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// Copyright Istio Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License aton 2.0 (the "License"); +//you may not use this file except in compliance with the License. +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License.S OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and -# limitations under the License. -# -################################################################################ +#pragma onces under the License. # -set -ex - -# Use clang for the release builds. -export PATH=/usr/lib/llvm/bin:$PATH -export CC=${CC:-/usr/lib/llvm/bin/clang} +#include "source/extensions/filters/common/expr/cel_state.h"#################### +#include "source/extensions/filters/http/common/factory_base.h" +#include "source/extensions/filters/http/common/pass_through_filter.h" +#include "source/extensions/filters/http/peer_metadata/config.pb.h" +#include "source/extensions/common/workload_discovery/api.h" +#include "source/common/singleton/const_singleton.h" +#include usr/lib/llvm/bin/clang} export CXX=${CXX:-/usr/lib/llvm/bin/clang++} - -# ARCH_SUFFIX allows optionally appending a -{ARCH} suffix to published binaries. -# For backwards compatibility, Istio skips this for amd64. -# Note: user provides "arm64"; we expand to "-arm64" for simple usage in script. +namespace Envoy { +namespace Extensions {ptionally appending a -{ARCH} suffix to published binaries. +namespace HttpFilters {bility, Istio skips this for amd64. +namespace PeerMetadata {rm64"; we expand to "-arm64" for simple usage in script. export ARCH_SUFFIX="${ARCH_SUFFIX+-${ARCH_SUFFIX}}" - -# Expliticly stamp. +using ::Envoy::Extensions::Filters::Common::Expr::CelStatePrototype; +using ::Envoy::Extensions::Filters::Common::Expr::CelStateType; BAZEL_BUILD_ARGS="${BAZEL_BUILD_ARGS} --stamp" - -if [[ "$(uname)" == "Darwin" ]]; then - BAZEL_CONFIG_ASAN="--config=macos-asan" -else - BAZEL_CONFIG_ASAN="--config=clang-asan-ci" +struct HeaderValues { + const Http::LowerCaseString Baggage{"baggage"}; + const Http::LowerCaseString ExchangeMetadataHeader{"x-envoy-peer-metadata"}; + const Http::LowerCaseString ExchangeMetadataHeaderId{"x-envoy-peer-metadata-id"}; +};BAZEL_CONFIG_ASAN="--config=clang-asan-ci" fi - +using Headers = ConstSingleton; # The bucket name to store proxy binaries. -DST="" - -# Verify that we're building binaries on Ubuntu 18.04 (Bionic). -CHECK=1 +using PeerInfo = Istio::Common::WorkloadMetadataObject; -# Defines the base binary name for artifacts. For example, this will be "envoy-debug". +struct Context {'re building binaries on Ubuntu 18.04 (Bionic). + bool request_peer_id_received_{false}; + bool request_peer_received_{false}; +};Defines the base binary name for artifacts. For example, this will be "envoy-debug". BASE_BINARY_NAME="${BASE_BINARY_NAME:-"envoy"}" - -# If enabled, we will just build the Envoy binary rather than wasm, etc -BUILD_ENVOY_BINARY_ONLY="${BUILD_ENVOY_BINARY_ONLY:-0}" - -function usage() { - echo "$0 - -d The bucket name to store proxy binary (optional). - If not provided, both envoy binary push and docker image push are skipped. +// Base class for the discovery methods. First derivation wins but all methods perform removal. +class DiscoveryMethod {ust build the Envoy binary rather than wasm, etc +public:NVOY_BINARY_ONLY="${BUILD_ENVOY_BINARY_ONLY:-0}" + virtual ~DiscoveryMethod() = default; + virtual absl::optional derivePeerInfo(const StreamInfo::StreamInfo&, Http::HeaderMap&, + Context&) const PURE; + virtual void remove(Http::HeaderMap&) const {}ptional). +}; If not provided, both envoy binary push and docker image push are skipped. -i Skip Ubuntu Bionic check. DO NOT USE THIS FOR RELEASED BINARIES." - exit 1 +using DiscoveryMethodPtr = std::unique_ptr; } - -while getopts d:i arg ; do - case "${arg}" in - d) DST="${OPTARG}";; - i) CHECK=0;; - *) usage;; - esac +class MXMethod : public DiscoveryMethod { +public:etopts d:i arg ; do + MXMethod(bool downstream, const absl::flat_hash_set additional_labels, + Server::Configuration::ServerFactoryContext& factory_context); + absl::optional derivePeerInfo(const StreamInfo::StreamInfo&, Http::HeaderMap&, + Context&) const override; + void remove(Http::HeaderMap&) const override; done - -if [[ "${BUILD_ENVOY_BINARY_ONLY}" != 1 && "${ARCH_SUFFIX}" != "" ]]; then - # This is not a fundamental limitation; however, the support for the other release types - # has not been updated to support this. - echo "ARCH_SUFFIX currently requires BUILD_ENVOY_BINARY_ONLY" - exit 1 -fi - -echo "Destination bucket: $DST" - +private: + absl::optional lookup(absl::string_view id, absl::string_view value) const; + const bool downstream_;ntal limitation; however, the support for the other release types + struct MXCache : public ThreadLocal::ThreadLocalObject { + absl::flat_hash_map cache_;ARY_ONLY" + };it 1 + mutable ThreadLocal::TypedSlot tls_; + const absl::flat_hash_set additional_labels_; + const int64_t max_peer_cache_size_{500}; +}; if [ "${DST}" == "none" ]; then - DST="" -fi - -# Make sure the release binaries are built on x86_64 Ubuntu 18.04 (Bionic) -if [ "${CHECK}" -eq 1 ] ; then - if [[ "${BAZEL_BUILD_ARGS}" != *"--config=remote-"* ]]; then +// Base class for the propagation methods. +class PropagationMethod { +public: + virtual ~PropagationMethod() = default;t on x86_64 Ubuntu 18.04 (Bionic) + virtual void inject(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const PURE; +};if [[ "${BAZEL_BUILD_ARGS}" != *"--config=remote-"* ]]; then UBUNTU_RELEASE=${UBUNTU_RELEASE:-$(lsb_release -c -s)} - [[ "${UBUNTU_RELEASE}" == 'bionic' ]] || { echo 'Must run on Ubuntu Bionic.'; exit 1; } +using PropagationMethodPtr = std::unique_ptr; Ubuntu Bionic.'; exit 1; } fi - [[ "$(uname -m)" == 'x86_64' ]] || { echo 'Must run on x86_64.'; exit 1; } -fi - -# The proxy binary name. -SHA="$(git rev-parse --verify HEAD)" - +class MXPropagationMethod : public PropagationMethod {on x86_64.'; exit 1; } +public: + MXPropagationMethod(bool downstream, Server::Configuration::ServerFactoryContext& factory_context, + const absl::flat_hash_set& additional_labels, + const io::istio::http::peer_metadata::Config_IstioHeaders&); + void inject(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const override; if [ -n "${DST}" ]; then - # If binary already exists skip. - # Use the name of the last artifact to make sure that everything was uploaded. - BINARY_NAME="${HOME}/istio-proxy-debug-${SHA}.deb" - gsutil stat "${DST}/${BINARY_NAME}" \ - && { echo 'Binary already exists'; exit 0; } \ - || echo 'Building a new binary.' -fi - -ARCH_NAME="k8" +private:inary already exists skip. + const bool downstream_;ast artifact to make sure that everything was uploaded. + std::string computeValue(const absl::flat_hash_set&, + Server::Configuration::ServerFactoryContext&) const; + const std::string id_;ready exists'; exit 0; } \ + const std::string value_; binary.' + const bool skip_external_clusters_; + bool skipMXHeaders(const bool, const StreamInfo::StreamInfo&) const; +};CH_NAME="k8" case "$(uname -m)" in - aarch64) ARCH_NAME="aarch64";; -esac - -# BAZEL_OUT: Symlinks don't work, use full path as a temporary workaround. -# See: https://github.com/istio/istio/issues/15714 for details. +class BaggagePropagationMethod : public PropagationMethod { +public: + BaggagePropagationMethod(Server::Configuration::ServerFactoryContext& factory_context, + const io::istio::http::peer_metadata::Config_Baggage&); + void inject(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const override; # k8-opt is the output directory for x86_64 optimized builds (-c opt, so --config=release-symbol and --config=release). -# k8-dbg is the output directory for -c dbg builds. -for config in release release-symbol asan debug -do - case $config in +private: is the output directory for -c dbg builds. + std::string computeBaggageValue(Server::Configuration::ServerFactoryContext&) const; + const std::string value_; +};case $config in "release" ) - CONFIG_PARAMS="--config=release" - BINARY_BASE_NAME="${BASE_BINARY_NAME}-alpha" - # shellcheck disable=SC2086 - BAZEL_OUT="$(bazel info ${BAZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-opt/bin" - ;; - "release-symbol") - CONFIG_PARAMS="--config=release-symbol" - BINARY_BASE_NAME="${BASE_BINARY_NAME}-symbol" +class FilterConfig : public Logger::Loggable { +public:INARY_BASE_NAME="${BASE_BINARY_NAME}-alpha" + FilterConfig(const io::istio::http::peer_metadata::Config&, + Server::Configuration::FactoryContext&);t_path)/${ARCH_NAME}-opt/bin" + void discoverDownstream(StreamInfo::StreamInfo&, Http::RequestHeaderMap&, Context&) const; + void discoverUpstream(StreamInfo::StreamInfo&, Http::ResponseHeaderMap&, Context&) const; + void injectDownstream(const StreamInfo::StreamInfo&, Http::ResponseHeaderMap&, Context&) const; + void injectUpstream(const StreamInfo::StreamInfo&, Http::RequestHeaderMap&, Context&) const; # shellcheck disable=SC2086 - BAZEL_OUT="$(bazel info ${BAZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-opt/bin" - ;; - "asan") - # Asan is skipped on ARM64 - if [[ "$(uname -m)" != "aarch64" ]]; then - # NOTE: libc++ is dynamically linked in this build. + static const CelStatePrototype& peerInfoPrototype() {t_path)/${ARCH_NAME}-opt/bin" + static const CelStatePrototype* const prototype = new CelStatePrototype( + true, CelStateType::Protobuf, "type.googleapis.com/google.protobuf.Struct", + StreamInfo::FilterState::LifeSpan::FilterChain); + return *prototype;m)" != "aarch64" ]]; then + } # NOTE: libc++ is dynamically linked in this build. CONFIG_PARAMS="${BAZEL_CONFIG_ASAN} --config=release-symbol" - BINARY_BASE_NAME="${BASE_BINARY_NAME}-asan" - # shellcheck disable=SC2086 - BAZEL_OUT="$(bazel info ${BAZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-opt/bin" - fi - ;; - "debug") - CONFIG_PARAMS="-c dbg" - BINARY_BASE_NAME="${BASE_BINARY_NAME}-debug" - # shellcheck disable=SC2086 - BAZEL_OUT="$(bazel info ${BAZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-dbg/bin" - ;; - esac - - export BUILD_CONFIG=${config} - - echo "Building ${config} proxy" - BINARY_NAME="${HOME}/${BINARY_BASE_NAME}-${SHA}${ARCH_SUFFIX}.tar.gz" - DWP_NAME="${HOME}/${BINARY_BASE_NAME}-${SHA}${ARCH_SUFFIX}.dwp" - SHA256_NAME="${HOME}/${BINARY_BASE_NAME}-${SHA}${ARCH_SUFFIX}.sha256" - # shellcheck disable=SC2086 - bazel build ${BAZEL_BUILD_ARGS} ${CONFIG_PARAMS} //:envoy_tar //:envoy.dwp - BAZEL_TARGET="${BAZEL_OUT}/envoy_tar.tar.gz" - DWP_TARGET="${BAZEL_OUT}/envoy.dwp" - cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" +private:BINARY_BASE_NAME="${BASE_BINARY_NAME}-asan" + std::vector buildDiscoveryMethods( + const Protobuf::RepeatedPtrField&, + const absl::flat_hash_set& additional_labels, bool downstream, + Server::Configuration::FactoryContext&) const; + std::vector buildPropagationMethods( + const Protobuf::RepeatedPtrField&, + const absl::flat_hash_set& additional_labels, bool downstream, + Server::Configuration::FactoryContext&) const; + absl::flat_hash_setZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-dbg/bin" + buildAdditionalLabels(const Protobuf::RepeatedPtrField&) const; + StreamInfo::StreamSharingMayImpactPooling sharedWithUpstream() const { + return shared_with_upstream_ + ? StreamInfo::StreamSharingMayImpactPooling::SharedWithUpstreamConnectionOnce + : StreamInfo::StreamSharingMayImpactPooling::None; + }cho "Building ${config} proxy" + void discover(StreamInfo::StreamInfo&, bool downstream, Http::HeaderMap&, Context&) const; + void setFilterState(StreamInfo::StreamInfo&, bool downstream, const PeerInfo& value) const; + const bool shared_with_upstream_;E_NAME}-${SHA}${ARCH_SUFFIX}.sha256" + const std::vector downstream_discovery_; + const std::vector upstream_discovery_;tar //:envoy.dwp + const std::vector downstream_propagation_; + const std::vector upstream_propagation_; +};cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" cp -f "${DWP_TARGET}" "${DWP_NAME}" - sha256sum "${BINARY_NAME}" > "${SHA256_NAME}" - - if [ -n "${DST}" ]; then - # Copy it to the bucket. - echo "Copying ${BINARY_NAME} ${SHA256_NAME} to ${DST}/" - gsutil cp "${BINARY_NAME}" "${SHA256_NAME}" "${DWP_NAME}" "${DST}/" - fi -done - -# Exit early to skip wasm build -if [ "${BUILD_ENVOY_BINARY_ONLY}" -eq 1 ]; then - exit 0 -fi +using FilterConfigSharedPtr = std::shared_ptr; + +class Filter : public Http::PassThroughFilter, + public Logger::Loggable {opy it to the bucket. +public: + Filter(const FilterConfigSharedPtr& config) : config_(config) {} + Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap&, bool) override; + Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap&, bool) override;done + +private: + FilterConfigSharedPtr config_;NVOY_BINARY_ONLY}" -eq 1 ]; then + Context ctx_;exit 0 +};fi +class FilterConfigFactory : public Server::Configuration::NamedHttpFilterConfigFactory {public: std::string name() const override { return "envoy.filters.http.peer_metadata"; } ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } absl::StatusOr createFilterFactoryFromProto(const Protobuf::Message& proto_config, const std::string&, Server::Configuration::FactoryContext&) override;};} // namespace PeerMetadata} // namespace HttpFilters} // namespace Extensions} // namespace Envoy diff --git a/source/extensions/filters/http/peer_metadata/filter.cc b/source/extensions/filters/http/peer_metadata/filter.cc index 1c2c28195d5..11d8ed1171d 100644 --- a/source/extensions/filters/http/peer_metadata/filter.cc +++ b/source/extensions/filters/http/peer_metadata/filter.cc @@ -216,6 +216,27 @@ void BaggagePropagationMethod::inject(const StreamInfo::StreamInfo&, Http::Heade headers.setReference(Headers::get().Baggage, value_); } +BaggageDiscoveryMethod::BaggageDiscoveryMethod(bool /*downstream*/, + Server::Configuration::ServerFactoryContext& /*factory_context*/) { +} + +absl::optional BaggageDiscoveryMethod::derivePeerInfo(const StreamInfo::StreamInfo&, + Http::HeaderMap& headers, + Context&) const { + const auto baggage_header = headers.get(Headers::get().Baggage); + if (baggage_header.empty()) { + ENVOY_LOG(info, "there's no baggage header"); + return {}; + } + ENVOY_LOG(info, "baggage header found"); + absl::string_view baggage_value = baggage_header[0]->value().getStringView(); + const auto workload = Istio::Common::convertBaggageToWorkloadMetadata(baggage_value); + if (workload) { + return *workload; + } + return {}; +} + FilterConfig::FilterConfig(const io::istio::http::peer_metadata::Config& config, Server::Configuration::FactoryContext& factory_context) : shared_with_upstream_(config.shared_with_upstream()), @@ -251,6 +272,9 @@ std::vector FilterConfig::buildDiscoveryMethods( methods.push_back(std::make_unique(downstream, additional_labels, factory_context.serverFactoryContext())); break; + case io::istio::http::peer_metadata::Config::DiscoveryMethod::MethodSpecifierCase::kBaggage: + methods.push_back(std::make_unique(downstream, factory_context.serverFactoryContext())); + break; default: break; } @@ -340,6 +364,7 @@ void FilterConfig::setFilterState(StreamInfo::StreamInfo& info, bool downstream, auto pb = value.serializeAsProto(); auto peer_info = std::make_unique(FilterConfig::peerInfoPrototype()); peer_info->setValue(absl::string_view(pb->SerializeAsString())); + ENVOY_LOG(info, "setting peer_info: {} = {}", key, pb->SerializeAsString()); info.filterState()->setData( key, std::move(peer_info), StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::FilterChain, sharedWithUpstream()); diff --git a/source/extensions/filters/http/peer_metadata/filter.h b/source/extensions/filters/http/peer_metadata/filter.h index 4434d167bf9..af62ebd605c 100644 --- a/source/extensions/filters/http/peer_metadata/filter.h +++ b/source/extensions/filters/http/peer_metadata/filter.h @@ -112,6 +112,14 @@ class BaggagePropagationMethod : public PropagationMethod { const std::string value_; }; +class BaggageDiscoveryMethod : public DiscoveryMethod, public Logger::Loggable { +public: + BaggageDiscoveryMethod(bool downstream, + Server::Configuration::ServerFactoryContext& factory_context); + absl::optional derivePeerInfo(const StreamInfo::StreamInfo&, Http::HeaderMap&, + Context&) const override; +}; + class FilterConfig : public Logger::Loggable { public: FilterConfig(const io::istio::http::peer_metadata::Config&, @@ -155,7 +163,8 @@ class FilterConfig : public Logger::Loggable { using FilterConfigSharedPtr = std::shared_ptr; -class Filter : public Http::PassThroughFilter { +class Filter : public Http::PassThroughFilter, + public Logger::Loggable { public: Filter(const FilterConfigSharedPtr& config) : config_(config) {} Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap&, bool) override; diff --git a/source/extensions/filters/http/peer_metadata/filter_test.cc b/source/extensions/filters/http/peer_metadata/filter_test.cc index af463091818..31963c7fb6c 100644 --- a/source/extensions/filters/http/peer_metadata/filter_test.cc +++ b/source/extensions/filters/http/peer_metadata/filter_test.cc @@ -723,6 +723,363 @@ TEST_F(PeerMetadataTest, BaggagePropagationWithMixedConfig) { EXPECT_TRUE(response_headers_.has(Headers::get().Baggage)); } +// Baggage Discovery Tests + +TEST_F(PeerMetadataTest, DownstreamBaggageDiscoveryEmpty) { + initialize(R"EOF( + downstream_discovery: + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkNoPeer(true); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, UpstreamBaggageDiscoveryEmpty) { + initialize(R"EOF( + upstream_discovery: + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkNoPeer(true); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, DownstreamBaggageDiscovery) { + request_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=test-namespace,k8s.cluster.name=test-cluster," + "service.name=test-service,service.version=v1,k8s.workload.name=test-workload," + "k8s.workload.type=deployment,k8s.instance.name=test-instance-123," + "app.name=test-app,app.version=v2.0"); + initialize(R"EOF( + downstream_discovery: + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkPeerNamespace(true, "test-namespace"); + checkNoPeer(false); + checkShared(false); +} + +TEST_F(PeerMetadataTest, UpstreamBaggageDiscovery) { + response_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=upstream-namespace,k8s.cluster.name=upstream-cluster," + "service.name=upstream-service,service.version=v2,k8s.workload.name=upstream-workload," + "k8s.workload.type=pod,k8s.instance.name=upstream-instance-456," + "app.name=upstream-app,app.version=v3.0"); + initialize(R"EOF( + upstream_discovery: + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkNoPeer(true); + checkPeerNamespace(false, "upstream-namespace"); +} + +TEST_F(PeerMetadataTest, DownstreamBaggageDiscoveryPartialMetadata) { + // Test with only namespace and service name + request_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=partial-namespace,service.name=partial-service"); + initialize(R"EOF( + downstream_discovery: + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkPeerNamespace(true, "partial-namespace"); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, UpstreamBaggageDiscoveryPartialMetadata) { + // Test with only namespace and cluster + response_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=partial-upstream,k8s.cluster.name=partial-cluster"); + initialize(R"EOF( + upstream_discovery: + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkNoPeer(true); + checkPeerNamespace(false, "partial-upstream"); +} + +TEST_F(PeerMetadataTest, BothDirectionsBaggageDiscovery) { + request_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=downstream-ns,service.name=downstream-svc"); + response_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=upstream-ns,service.name=upstream-svc"); + initialize(R"EOF( + downstream_discovery: + - baggage: {} + upstream_discovery: + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkPeerNamespace(true, "downstream-ns"); + checkPeerNamespace(false, "upstream-ns"); +} + +TEST_F(PeerMetadataTest, DownstreamBaggageFallbackFirst) { + // Baggage is present, so XDS should not be called + request_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=baggage-namespace,service.name=baggage-service"); + EXPECT_CALL(*metadata_provider_, GetMetadata(_)).Times(0); + initialize(R"EOF( + downstream_discovery: + - baggage: {} + - workload_discovery: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkPeerNamespace(true, "baggage-namespace"); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, DownstreamBaggageFallbackSecond) { + // No baggage header, so XDS should be called as fallback + const WorkloadMetadataObject pod("pod-foo-1234", "my-cluster", "xds-namespace", "foo", + "foo-service", "v1alpha3", "", "", + Istio::Common::WorkloadType::Pod, ""); + EXPECT_CALL(*metadata_provider_, GetMetadata(_)) + .WillRepeatedly(Invoke([&](const Network::Address::InstanceConstSharedPtr& address) + -> std::optional { + if (absl::StartsWith(address->asStringView(), "127.0.0.1")) { + return {pod}; + } + return {}; + })); + initialize(R"EOF( + downstream_discovery: + - baggage: {} + - workload_discovery: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkPeerNamespace(true, "xds-namespace"); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, UpstreamBaggageFallbackFirst) { + // Baggage is present, so XDS should not be called + response_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=baggage-upstream,service.name=baggage-upstream-service"); + EXPECT_CALL(*metadata_provider_, GetMetadata(_)).Times(0); + initialize(R"EOF( + upstream_discovery: + - baggage: {} + - workload_discovery: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkNoPeer(true); + checkPeerNamespace(false, "baggage-upstream"); +} + +TEST_F(PeerMetadataTest, UpstreamBaggageFallbackSecond) { + // No baggage header, so XDS should be called as fallback + const WorkloadMetadataObject pod("pod-foo-1234", "my-cluster", "xds-upstream", "foo", + "foo-service", "v1alpha3", "", "", + Istio::Common::WorkloadType::Pod, ""); + EXPECT_CALL(*metadata_provider_, GetMetadata(_)) + .WillRepeatedly(Invoke([&](const Network::Address::InstanceConstSharedPtr& address) + -> std::optional { + if (absl::StartsWith(address->asStringView(), "10.0.0.1")) { + return {pod}; + } + return {}; + })); + initialize(R"EOF( + upstream_discovery: + - baggage: {} + - workload_discovery: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkNoPeer(true); + checkPeerNamespace(false, "xds-upstream"); +} + +TEST_F(PeerMetadataTest, DownstreamBaggageWithMXFallback) { + // Baggage is present, so MX should not be used + request_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=baggage-ns,service.name=baggage-svc"); + request_headers_.setReference(Headers::get().ExchangeMetadataHeaderId, "test-pod"); + request_headers_.setReference(Headers::get().ExchangeMetadataHeader, SampleIstioHeader); + initialize(R"EOF( + downstream_discovery: + - baggage: {} + - istio_headers: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + checkPeerNamespace(true, "baggage-ns"); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, DownstreamMXWithBaggageFallback) { + // MX is first, so it should be used even if baggage is present + request_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=baggage-ns,service.name=baggage-svc"); + request_headers_.setReference(Headers::get().ExchangeMetadataHeaderId, "test-pod"); + request_headers_.setReference(Headers::get().ExchangeMetadataHeader, SampleIstioHeader); + initialize(R"EOF( + downstream_discovery: + - istio_headers: {} + - baggage: {} + )EOF"); + EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(0, response_headers_.size()); + // MX header has namespace "default" from SampleIstioHeader + checkPeerNamespace(true, "default"); + checkNoPeer(false); +} + +TEST_F(PeerMetadataTest, BaggageDiscoveryWithPropagation) { + request_headers_.setReference( + Headers::get().Baggage, + "k8s.namespace.name=discovered-ns,service.name=discovered-svc"); + initialize(R"EOF( + downstream_discovery: + - baggage: {} + downstream_propagation: + - baggage: {} + upstream_propagation: + - baggage: {} + )EOF"); + EXPECT_EQ(1, request_headers_.size()); // upstream baggage propagation + EXPECT_EQ(1, response_headers_.size()); // downstream baggage propagation + EXPECT_TRUE(request_headers_.has(Headers::get().Baggage)); + EXPECT_TRUE(response_headers_.has(Headers::get().Baggage)); + checkPeerNamespace(true, "discovered-ns"); + checkNoPeer(false); +} + +// Test class specifically for BaggageDiscoveryMethod unit tests +class BaggageDiscoveryMethodTest : public testing::Test { +protected: + BaggageDiscoveryMethodTest() = default; + + NiceMock context_; + NiceMock stream_info_; +}; + +TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoFromBaggage) { + BaggageDiscoveryMethod method(true, context_.server_factory_context_); + + Http::TestRequestHeaderMapImpl headers; + headers.setReference( + Headers::get().Baggage, + "k8s.namespace.name=unit-test-namespace,k8s.cluster.name=unit-test-cluster," + "service.name=unit-test-service,service.version=v1.0," + "k8s.workload.name=unit-test-workload,k8s.workload.type=deployment," + "k8s.instance.name=unit-test-instance,app.name=unit-test-app,app.version=v2.0"); + Context ctx; + + const auto result = method.derivePeerInfo(stream_info_, headers, ctx); + + ASSERT_TRUE(result.has_value()); + EXPECT_EQ("unit-test-namespace", result->namespace_name_); + EXPECT_EQ("unit-test-cluster", result->cluster_name_); + EXPECT_EQ("unit-test-service", result->canonical_name_); + EXPECT_EQ("v1.0", result->canonical_revision_); + EXPECT_EQ("unit-test-workload", result->workload_name_); + EXPECT_EQ("unit-test-instance", result->instance_name_); + EXPECT_EQ("unit-test-app", result->app_name_); + EXPECT_EQ("v2.0", result->app_version_); + EXPECT_EQ(Istio::Common::WorkloadType::Deployment, result->workload_type_); +} + +TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoEmptyBaggage) { + BaggageDiscoveryMethod method(true, context_.server_factory_context_); + + Http::TestRequestHeaderMapImpl headers; + Context ctx; + + const auto result = method.derivePeerInfo(stream_info_, headers, ctx); + + EXPECT_FALSE(result.has_value()); +} + +TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoPartialBaggage) { + BaggageDiscoveryMethod method(false, context_.server_factory_context_); + + Http::TestResponseHeaderMapImpl headers; + headers.setReference( + Headers::get().Baggage, + "k8s.namespace.name=partial-ns,service.name=partial-svc"); + Context ctx; + + const auto result = method.derivePeerInfo(stream_info_, headers, ctx); + + ASSERT_TRUE(result.has_value()); + EXPECT_EQ("partial-ns", result->namespace_name_); + EXPECT_EQ("partial-svc", result->canonical_name_); + // Other fields should be empty or default + EXPECT_TRUE(result->cluster_name_.empty()); + EXPECT_TRUE(result->workload_name_.empty()); +} + +TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoAllWorkloadTypes) { + BaggageDiscoveryMethod method(true, context_.server_factory_context_); + Context ctx; + + // Test Pod workload type + { + Http::TestRequestHeaderMapImpl headers; + headers.setReference(Headers::get().Baggage, + "k8s.namespace.name=test-ns,k8s.workload.type=pod"); + const auto result = method.derivePeerInfo(stream_info_, headers, ctx); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(Istio::Common::WorkloadType::Pod, result->workload_type_); + } + + // Test Deployment workload type + { + Http::TestRequestHeaderMapImpl headers; + headers.setReference(Headers::get().Baggage, + "k8s.namespace.name=test-ns,k8s.workload.type=deployment"); + const auto result = method.derivePeerInfo(stream_info_, headers, ctx); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(Istio::Common::WorkloadType::Deployment, result->workload_type_); + } + + // Test Job workload type + { + Http::TestRequestHeaderMapImpl headers; + headers.setReference(Headers::get().Baggage, + "k8s.namespace.name=test-ns,k8s.workload.type=job"); + const auto result = method.derivePeerInfo(stream_info_, headers, ctx); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(Istio::Common::WorkloadType::Job, result->workload_type_); + } + + // Test CronJob workload type + { + Http::TestRequestHeaderMapImpl headers; + headers.setReference(Headers::get().Baggage, + "k8s.namespace.name=test-ns,k8s.workload.type=cronjob"); + const auto result = method.derivePeerInfo(stream_info_, headers, ctx); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(Istio::Common::WorkloadType::CronJob, result->workload_type_); + } +} + } // namespace } // namespace PeerMetadata } // namespace HttpFilters From 72394b418ca2015631abc13326ea3d6461904c1b Mon Sep 17 00:00:00 2001 From: Gustavo Date: Thu, 22 Jan 2026 10:32:02 +0000 Subject: [PATCH 04/13] removing unnecessary tests --- .../filters/http/peer_metadata/filter_test.cc | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/source/extensions/filters/http/peer_metadata/filter_test.cc b/source/extensions/filters/http/peer_metadata/filter_test.cc index 31963c7fb6c..5a208429111 100644 --- a/source/extensions/filters/http/peer_metadata/filter_test.cc +++ b/source/extensions/filters/http/peer_metadata/filter_test.cc @@ -782,36 +782,6 @@ TEST_F(PeerMetadataTest, UpstreamBaggageDiscovery) { checkPeerNamespace(false, "upstream-namespace"); } -TEST_F(PeerMetadataTest, DownstreamBaggageDiscoveryPartialMetadata) { - // Test with only namespace and service name - request_headers_.setReference( - Headers::get().Baggage, - "k8s.namespace.name=partial-namespace,service.name=partial-service"); - initialize(R"EOF( - downstream_discovery: - - baggage: {} - )EOF"); - EXPECT_EQ(0, request_headers_.size()); - EXPECT_EQ(0, response_headers_.size()); - checkPeerNamespace(true, "partial-namespace"); - checkNoPeer(false); -} - -TEST_F(PeerMetadataTest, UpstreamBaggageDiscoveryPartialMetadata) { - // Test with only namespace and cluster - response_headers_.setReference( - Headers::get().Baggage, - "k8s.namespace.name=partial-upstream,k8s.cluster.name=partial-cluster"); - initialize(R"EOF( - upstream_discovery: - - baggage: {} - )EOF"); - EXPECT_EQ(0, request_headers_.size()); - EXPECT_EQ(0, response_headers_.size()); - checkNoPeer(true); - checkPeerNamespace(false, "partial-upstream"); -} - TEST_F(PeerMetadataTest, BothDirectionsBaggageDiscovery) { request_headers_.setReference( Headers::get().Baggage, From fba6e72d2ffadfb56da24636ba450a015c7162dd Mon Sep 17 00:00:00 2001 From: Gustavo Date: Thu, 22 Jan 2026 10:36:51 +0000 Subject: [PATCH 05/13] reverting crazy claude changes in release-binary.sh --- scripts/release-binary.sh | 293 +++++++++++++++++++------------------- 1 file changed, 146 insertions(+), 147 deletions(-) diff --git a/scripts/release-binary.sh b/scripts/release-binary.sh index 75b4c7d095b..b2419a6407e 100755 --- a/scripts/release-binary.sh +++ b/scripts/release-binary.sh @@ -1,169 +1,168 @@ -// Copyright Istio Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License aton 2.0 (the "License"); -//you may not use this file except in compliance with the License. -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License.S OF ANY KIND, either express or implied. +#!/bin/bash +# +# Copyright 2016 Istio Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and -#pragma onces under the License. +# limitations under the License. +# +################################################################################ # -#include "source/extensions/filters/common/expr/cel_state.h"#################### -#include "source/extensions/filters/http/common/factory_base.h" -#include "source/extensions/filters/http/common/pass_through_filter.h" -#include "source/extensions/filters/http/peer_metadata/config.pb.h" -#include "source/extensions/common/workload_discovery/api.h" -#include "source/common/singleton/const_singleton.h" -#include usr/lib/llvm/bin/clang} +set -ex + +# Use clang for the release builds. +export PATH=/usr/lib/llvm/bin:$PATH +export CC=${CC:-/usr/lib/llvm/bin/clang} export CXX=${CXX:-/usr/lib/llvm/bin/clang++} -namespace Envoy { -namespace Extensions {ptionally appending a -{ARCH} suffix to published binaries. -namespace HttpFilters {bility, Istio skips this for amd64. -namespace PeerMetadata {rm64"; we expand to "-arm64" for simple usage in script. + +# ARCH_SUFFIX allows optionally appending a -{ARCH} suffix to published binaries. +# For backwards compatibility, Istio skips this for amd64. +# Note: user provides "arm64"; we expand to "-arm64" for simple usage in script. export ARCH_SUFFIX="${ARCH_SUFFIX+-${ARCH_SUFFIX}}" -using ::Envoy::Extensions::Filters::Common::Expr::CelStatePrototype; -using ::Envoy::Extensions::Filters::Common::Expr::CelStateType; + +# Expliticly stamp. BAZEL_BUILD_ARGS="${BAZEL_BUILD_ARGS} --stamp" -struct HeaderValues { - const Http::LowerCaseString Baggage{"baggage"}; - const Http::LowerCaseString ExchangeMetadataHeader{"x-envoy-peer-metadata"}; - const Http::LowerCaseString ExchangeMetadataHeaderId{"x-envoy-peer-metadata-id"}; -};BAZEL_CONFIG_ASAN="--config=clang-asan-ci" + +if [[ "$(uname)" == "Darwin" ]]; then + BAZEL_CONFIG_ASAN="--config=macos-asan" +else + BAZEL_CONFIG_ASAN="--config=clang-asan-ci" fi -using Headers = ConstSingleton; + # The bucket name to store proxy binaries. -using PeerInfo = Istio::Common::WorkloadMetadataObject; +DST="" + +# Verify that we're building binaries on Ubuntu 18.04 (Bionic). +CHECK=1 -struct Context {'re building binaries on Ubuntu 18.04 (Bionic). - bool request_peer_id_received_{false}; - bool request_peer_received_{false}; -};Defines the base binary name for artifacts. For example, this will be "envoy-debug". +# Defines the base binary name for artifacts. For example, this will be "envoy-debug". BASE_BINARY_NAME="${BASE_BINARY_NAME:-"envoy"}" -// Base class for the discovery methods. First derivation wins but all methods perform removal. -class DiscoveryMethod {ust build the Envoy binary rather than wasm, etc -public:NVOY_BINARY_ONLY="${BUILD_ENVOY_BINARY_ONLY:-0}" - virtual ~DiscoveryMethod() = default; - virtual absl::optional derivePeerInfo(const StreamInfo::StreamInfo&, Http::HeaderMap&, - Context&) const PURE; - virtual void remove(Http::HeaderMap&) const {}ptional). -}; If not provided, both envoy binary push and docker image push are skipped. + +# If enabled, we will just build the Envoy binary rather than wasm, etc +BUILD_ENVOY_BINARY_ONLY="${BUILD_ENVOY_BINARY_ONLY:-0}" + +function usage() { + echo "$0 + -d The bucket name to store proxy binary (optional). + If not provided, both envoy binary push and docker image push are skipped. -i Skip Ubuntu Bionic check. DO NOT USE THIS FOR RELEASED BINARIES." -using DiscoveryMethodPtr = std::unique_ptr; + exit 1 } -class MXMethod : public DiscoveryMethod { -public:etopts d:i arg ; do - MXMethod(bool downstream, const absl::flat_hash_set additional_labels, - Server::Configuration::ServerFactoryContext& factory_context); - absl::optional derivePeerInfo(const StreamInfo::StreamInfo&, Http::HeaderMap&, - Context&) const override; - void remove(Http::HeaderMap&) const override; + +while getopts d:i arg ; do + case "${arg}" in + d) DST="${OPTARG}";; + i) CHECK=0;; + *) usage;; + esac done -private: - absl::optional lookup(absl::string_view id, absl::string_view value) const; - const bool downstream_;ntal limitation; however, the support for the other release types - struct MXCache : public ThreadLocal::ThreadLocalObject { - absl::flat_hash_map cache_;ARY_ONLY" - };it 1 - mutable ThreadLocal::TypedSlot tls_; - const absl::flat_hash_set additional_labels_; - const int64_t max_peer_cache_size_{500}; -}; + +if [[ "${BUILD_ENVOY_BINARY_ONLY}" != 1 && "${ARCH_SUFFIX}" != "" ]]; then + # This is not a fundamental limitation; however, the support for the other release types + # has not been updated to support this. + echo "ARCH_SUFFIX currently requires BUILD_ENVOY_BINARY_ONLY" + exit 1 +fi + +echo "Destination bucket: $DST" + if [ "${DST}" == "none" ]; then -// Base class for the propagation methods. -class PropagationMethod { -public: - virtual ~PropagationMethod() = default;t on x86_64 Ubuntu 18.04 (Bionic) - virtual void inject(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const PURE; -};if [[ "${BAZEL_BUILD_ARGS}" != *"--config=remote-"* ]]; then + DST="" +fi + +# Make sure the release binaries are built on x86_64 Ubuntu 18.04 (Bionic) +if [ "${CHECK}" -eq 1 ] ; then + if [[ "${BAZEL_BUILD_ARGS}" != *"--config=remote-"* ]]; then UBUNTU_RELEASE=${UBUNTU_RELEASE:-$(lsb_release -c -s)} -using PropagationMethodPtr = std::unique_ptr; Ubuntu Bionic.'; exit 1; } + [[ "${UBUNTU_RELEASE}" == 'bionic' ]] || { echo 'Must run on Ubuntu Bionic.'; exit 1; } fi -class MXPropagationMethod : public PropagationMethod {on x86_64.'; exit 1; } -public: - MXPropagationMethod(bool downstream, Server::Configuration::ServerFactoryContext& factory_context, - const absl::flat_hash_set& additional_labels, - const io::istio::http::peer_metadata::Config_IstioHeaders&); - void inject(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const override; + [[ "$(uname -m)" == 'x86_64' ]] || { echo 'Must run on x86_64.'; exit 1; } +fi + +# The proxy binary name. +SHA="$(git rev-parse --verify HEAD)" + if [ -n "${DST}" ]; then -private:inary already exists skip. - const bool downstream_;ast artifact to make sure that everything was uploaded. - std::string computeValue(const absl::flat_hash_set&, - Server::Configuration::ServerFactoryContext&) const; - const std::string id_;ready exists'; exit 0; } \ - const std::string value_; binary.' - const bool skip_external_clusters_; - bool skipMXHeaders(const bool, const StreamInfo::StreamInfo&) const; -};CH_NAME="k8" + # If binary already exists skip. + # Use the name of the last artifact to make sure that everything was uploaded. + BINARY_NAME="${HOME}/istio-proxy-debug-${SHA}.deb" + gsutil stat "${DST}/${BINARY_NAME}" \ + && { echo 'Binary already exists'; exit 0; } \ + || echo 'Building a new binary.' +fi + +ARCH_NAME="k8" case "$(uname -m)" in -class BaggagePropagationMethod : public PropagationMethod { -public: - BaggagePropagationMethod(Server::Configuration::ServerFactoryContext& factory_context, - const io::istio::http::peer_metadata::Config_Baggage&); - void inject(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const override; + aarch64) ARCH_NAME="aarch64";; +esac + +# BAZEL_OUT: Symlinks don't work, use full path as a temporary workaround. +# See: https://github.com/istio/istio/issues/15714 for details. # k8-opt is the output directory for x86_64 optimized builds (-c opt, so --config=release-symbol and --config=release). -private: is the output directory for -c dbg builds. - std::string computeBaggageValue(Server::Configuration::ServerFactoryContext&) const; - const std::string value_; -};case $config in +# k8-dbg is the output directory for -c dbg builds. +for config in release release-symbol asan debug +do + case $config in "release" ) -class FilterConfig : public Logger::Loggable { -public:INARY_BASE_NAME="${BASE_BINARY_NAME}-alpha" - FilterConfig(const io::istio::http::peer_metadata::Config&, - Server::Configuration::FactoryContext&);t_path)/${ARCH_NAME}-opt/bin" - void discoverDownstream(StreamInfo::StreamInfo&, Http::RequestHeaderMap&, Context&) const; - void discoverUpstream(StreamInfo::StreamInfo&, Http::ResponseHeaderMap&, Context&) const; - void injectDownstream(const StreamInfo::StreamInfo&, Http::ResponseHeaderMap&, Context&) const; - void injectUpstream(const StreamInfo::StreamInfo&, Http::RequestHeaderMap&, Context&) const; + CONFIG_PARAMS="--config=release" + BINARY_BASE_NAME="${BASE_BINARY_NAME}-alpha" + # shellcheck disable=SC2086 + BAZEL_OUT="$(bazel info ${BAZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-opt/bin" + ;; + "release-symbol") + CONFIG_PARAMS="--config=release-symbol" + BINARY_BASE_NAME="${BASE_BINARY_NAME}-symbol" # shellcheck disable=SC2086 - static const CelStatePrototype& peerInfoPrototype() {t_path)/${ARCH_NAME}-opt/bin" - static const CelStatePrototype* const prototype = new CelStatePrototype( - true, CelStateType::Protobuf, "type.googleapis.com/google.protobuf.Struct", - StreamInfo::FilterState::LifeSpan::FilterChain); - return *prototype;m)" != "aarch64" ]]; then - } # NOTE: libc++ is dynamically linked in this build. + BAZEL_OUT="$(bazel info ${BAZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-opt/bin" + ;; + "asan") + # Asan is skipped on ARM64 + if [[ "$(uname -m)" != "aarch64" ]]; then + # NOTE: libc++ is dynamically linked in this build. CONFIG_PARAMS="${BAZEL_CONFIG_ASAN} --config=release-symbol" -private:BINARY_BASE_NAME="${BASE_BINARY_NAME}-asan" - std::vector buildDiscoveryMethods( - const Protobuf::RepeatedPtrField&, - const absl::flat_hash_set& additional_labels, bool downstream, - Server::Configuration::FactoryContext&) const; - std::vector buildPropagationMethods( - const Protobuf::RepeatedPtrField&, - const absl::flat_hash_set& additional_labels, bool downstream, - Server::Configuration::FactoryContext&) const; - absl::flat_hash_setZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-dbg/bin" - buildAdditionalLabels(const Protobuf::RepeatedPtrField&) const; - StreamInfo::StreamSharingMayImpactPooling sharedWithUpstream() const { - return shared_with_upstream_ - ? StreamInfo::StreamSharingMayImpactPooling::SharedWithUpstreamConnectionOnce - : StreamInfo::StreamSharingMayImpactPooling::None; - }cho "Building ${config} proxy" - void discover(StreamInfo::StreamInfo&, bool downstream, Http::HeaderMap&, Context&) const; - void setFilterState(StreamInfo::StreamInfo&, bool downstream, const PeerInfo& value) const; - const bool shared_with_upstream_;E_NAME}-${SHA}${ARCH_SUFFIX}.sha256" - const std::vector downstream_discovery_; - const std::vector upstream_discovery_;tar //:envoy.dwp - const std::vector downstream_propagation_; - const std::vector upstream_propagation_; -};cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" + BINARY_BASE_NAME="${BASE_BINARY_NAME}-asan" + # shellcheck disable=SC2086 + BAZEL_OUT="$(bazel info ${BAZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-opt/bin" + fi + ;; + "debug") + CONFIG_PARAMS="-c dbg" + BINARY_BASE_NAME="${BASE_BINARY_NAME}-debug" + # shellcheck disable=SC2086 + BAZEL_OUT="$(bazel info ${BAZEL_BUILD_ARGS} output_path)/${ARCH_NAME}-dbg/bin" + ;; + esac + + export BUILD_CONFIG=${config} + + echo "Building ${config} proxy" + BINARY_NAME="${HOME}/${BINARY_BASE_NAME}-${SHA}${ARCH_SUFFIX}.tar.gz" + DWP_NAME="${HOME}/${BINARY_BASE_NAME}-${SHA}${ARCH_SUFFIX}.dwp" + SHA256_NAME="${HOME}/${BINARY_BASE_NAME}-${SHA}${ARCH_SUFFIX}.sha256" + # shellcheck disable=SC2086 + bazel build ${BAZEL_BUILD_ARGS} ${CONFIG_PARAMS} //:envoy_tar //:envoy.dwp + BAZEL_TARGET="${BAZEL_OUT}/envoy_tar.tar.gz" + DWP_TARGET="${BAZEL_OUT}/envoy.dwp" + cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" cp -f "${DWP_TARGET}" "${DWP_NAME}" -using FilterConfigSharedPtr = std::shared_ptr; - -class Filter : public Http::PassThroughFilter, - public Logger::Loggable {opy it to the bucket. -public: - Filter(const FilterConfigSharedPtr& config) : config_(config) {} - Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap&, bool) override; - Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap&, bool) override;done - -private: - FilterConfigSharedPtr config_;NVOY_BINARY_ONLY}" -eq 1 ]; then - Context ctx_;exit 0 -};fi -class FilterConfigFactory : public Server::Configuration::NamedHttpFilterConfigFactory {public: std::string name() const override { return "envoy.filters.http.peer_metadata"; } ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } absl::StatusOr createFilterFactoryFromProto(const Protobuf::Message& proto_config, const std::string&, Server::Configuration::FactoryContext&) override;};} // namespace PeerMetadata} // namespace HttpFilters} // namespace Extensions} // namespace Envoy + sha256sum "${BINARY_NAME}" > "${SHA256_NAME}" + + if [ -n "${DST}" ]; then + # Copy it to the bucket. + echo "Copying ${BINARY_NAME} ${SHA256_NAME} to ${DST}/" + gsutil cp "${BINARY_NAME}" "${SHA256_NAME}" "${DWP_NAME}" "${DST}/" + fi +done + +# Exit early to skip wasm build +if [ "${BUILD_ENVOY_BINARY_ONLY}" -eq 1 ]; then + exit 0 +fi From 5d84f5ab287b25e3e366ee2798c5c2ef8e5b0ebd Mon Sep 17 00:00:00 2001 From: Gustavo Date: Thu, 22 Jan 2026 13:11:45 +0000 Subject: [PATCH 06/13] fixing tests, fixing baggage key tokens --- extensions/common/metadata_object.cc | 37 ++++++++++++++----- extensions/common/metadata_object.h | 12 ++++++ .../filters/http/peer_metadata/filter.cc | 4 +- .../filters/http/peer_metadata/filter_test.cc | 28 +++++++------- 4 files changed, 55 insertions(+), 26 deletions(-) diff --git a/extensions/common/metadata_object.cc b/extensions/common/metadata_object.cc index e5f763b2332..24adf372633 100644 --- a/extensions/common/metadata_object.cc +++ b/extensions/common/metadata_object.cc @@ -24,7 +24,8 @@ namespace Istio { namespace Common { namespace { -static absl::flat_hash_map ALL_BAGGAGE_TOKENS = { +// TODO: find a propper name for this mapping +static absl::flat_hash_map ALL_METADATA_FIELDS = { {NamespaceNameToken, BaggageToken::NamespaceName}, {ClusterNameToken, BaggageToken::ClusterName}, {ServiceNameToken, BaggageToken::ServiceName}, @@ -36,6 +37,20 @@ static absl::flat_hash_map ALL_BAGGAGE_TOKENS = {InstanceNameToken, BaggageToken::InstanceName}, }; +static absl::flat_hash_map ALL_BAGGAGE_TOKENS = { + {NamespaceNameBaggageToken, BaggageToken::NamespaceName}, + {ClusterNameBaggageToken, BaggageToken::ClusterName}, + {ServiceNameBaggageToken, BaggageToken::ServiceName}, + {ServiceVersionBaggageToken, BaggageToken::ServiceVersion}, + {AppNameBaggageToken, BaggageToken::AppName}, + {AppVersionBaggageToken, BaggageToken::AppVersion}, + {DeploymentNameBaggageToken, BaggageToken::WorkloadName}, + {PodNameBaggageToken, BaggageToken::WorkloadName}, + {CronjobNameBaggageToken, BaggageToken::WorkloadName}, + {JobNameBaggageToken, BaggageToken::WorkloadName}, + {InstanceNameBaggageToken, BaggageToken::InstanceName}, +}; + static absl::flat_hash_map ALL_WORKLOAD_TOKENS = { {PodSuffix, WorkloadType::Pod}, {DeploymentSuffix, WorkloadType::Deployment}, @@ -292,8 +307,8 @@ std::string serializeToStringDeterministic(const google::protobuf::Struct& metad WorkloadMetadataObject::FieldType WorkloadMetadataObject::getField(absl::string_view field_name) const { - const auto it = ALL_BAGGAGE_TOKENS.find(field_name); - if (it != ALL_BAGGAGE_TOKENS.end()) { + const auto it = ALL_METADATA_FIELDS.find(field_name); + if (it != ALL_METADATA_FIELDS.end()) { switch (it->second) { case BaggageToken::NamespaceName: return namespace_name_; @@ -355,15 +370,19 @@ std::unique_ptr convertBaggageToWorkloadMetadata(absl::s case BaggageToken::AppVersion: app_version = parts.second; break; - case BaggageToken::WorkloadName: - workload = parts.second; - break; - case BaggageToken::WorkloadType: - workload_type = fromSuffix(parts.second); - break; + case BaggageToken::WorkloadName: { + workload = parts.second; + std::vector splitWorkloadKey = absl::StrSplit(parts.first, "."); + if (splitWorkloadKey.size() >= 2 && splitWorkloadKey[0] == "k8s") { + workload_type = fromSuffix(splitWorkloadKey[1]); + } + break; + } case BaggageToken::InstanceName: instance = parts.second; break; + default: + break; } } } diff --git a/extensions/common/metadata_object.h b/extensions/common/metadata_object.h index 2a0e65a8e34..fdcebfa113b 100644 --- a/extensions/common/metadata_object.h +++ b/extensions/common/metadata_object.h @@ -79,6 +79,18 @@ constexpr absl::string_view InstanceNameToken = "name"; constexpr absl::string_view LabelsToken = "labels"; constexpr absl::string_view IdentityToken = "identity"; +constexpr absl::string_view NamespaceNameBaggageToken = "k8s.namespace.name"; +constexpr absl::string_view ClusterNameBaggageToken = "k8s.cluster.name"; +constexpr absl::string_view ServiceNameBaggageToken = "service.name"; +constexpr absl::string_view ServiceVersionBaggageToken = "service.version"; +constexpr absl::string_view AppNameBaggageToken = "app.name"; +constexpr absl::string_view AppVersionBaggageToken = "app.version"; +constexpr absl::string_view DeploymentNameBaggageToken = "k8s.deployment.name"; +constexpr absl::string_view PodNameBaggageToken = "k8s.pod.name"; +constexpr absl::string_view CronjobNameBaggageToken = "k8s.cronjob.name"; +constexpr absl::string_view JobNameBaggageToken = "k8s.job.name"; +constexpr absl::string_view InstanceNameBaggageToken = "k8s.instance.name"; + constexpr absl::string_view InstanceMetadataField = "NAME"; constexpr absl::string_view NamespaceMetadataField = "NAMESPACE"; constexpr absl::string_view ClusterMetadataField = "CLUSTER_ID"; diff --git a/source/extensions/filters/http/peer_metadata/filter.cc b/source/extensions/filters/http/peer_metadata/filter.cc index 11d8ed1171d..a023d74e342 100644 --- a/source/extensions/filters/http/peer_metadata/filter.cc +++ b/source/extensions/filters/http/peer_metadata/filter.cc @@ -225,11 +225,9 @@ absl::optional BaggageDiscoveryMethod::derivePeerInfo(const StreamInfo Context&) const { const auto baggage_header = headers.get(Headers::get().Baggage); if (baggage_header.empty()) { - ENVOY_LOG(info, "there's no baggage header"); return {}; } - ENVOY_LOG(info, "baggage header found"); - absl::string_view baggage_value = baggage_header[0]->value().getStringView(); + const auto baggage_value = baggage_header[0]->value().getStringView(); const auto workload = Istio::Common::convertBaggageToWorkloadMetadata(baggage_value); if (workload) { return *workload; diff --git a/source/extensions/filters/http/peer_metadata/filter_test.cc b/source/extensions/filters/http/peer_metadata/filter_test.cc index 5a208429111..c1c1e2cdd82 100644 --- a/source/extensions/filters/http/peer_metadata/filter_test.cc +++ b/source/extensions/filters/http/peer_metadata/filter_test.cc @@ -751,14 +751,14 @@ TEST_F(PeerMetadataTest, DownstreamBaggageDiscovery) { request_headers_.setReference( Headers::get().Baggage, "k8s.namespace.name=test-namespace,k8s.cluster.name=test-cluster," - "service.name=test-service,service.version=v1,k8s.workload.name=test-workload," + "service.name=test-service,service.version=v1,k8s.deployment.name=test-workload," "k8s.workload.type=deployment,k8s.instance.name=test-instance-123," "app.name=test-app,app.version=v2.0"); initialize(R"EOF( downstream_discovery: - baggage: {} )EOF"); - EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(1, request_headers_.size()); EXPECT_EQ(0, response_headers_.size()); checkPeerNamespace(true, "test-namespace"); checkNoPeer(false); @@ -777,7 +777,7 @@ TEST_F(PeerMetadataTest, UpstreamBaggageDiscovery) { - baggage: {} )EOF"); EXPECT_EQ(0, request_headers_.size()); - EXPECT_EQ(0, response_headers_.size()); + EXPECT_EQ(1, response_headers_.size()); checkNoPeer(true); checkPeerNamespace(false, "upstream-namespace"); } @@ -795,8 +795,8 @@ TEST_F(PeerMetadataTest, BothDirectionsBaggageDiscovery) { upstream_discovery: - baggage: {} )EOF"); - EXPECT_EQ(0, request_headers_.size()); - EXPECT_EQ(0, response_headers_.size()); + EXPECT_EQ(1, request_headers_.size()); + EXPECT_EQ(1, response_headers_.size()); checkPeerNamespace(true, "downstream-ns"); checkPeerNamespace(false, "upstream-ns"); } @@ -812,7 +812,7 @@ TEST_F(PeerMetadataTest, DownstreamBaggageFallbackFirst) { - baggage: {} - workload_discovery: {} )EOF"); - EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(1, request_headers_.size()); EXPECT_EQ(0, response_headers_.size()); checkPeerNamespace(true, "baggage-namespace"); checkNoPeer(false); @@ -854,7 +854,7 @@ TEST_F(PeerMetadataTest, UpstreamBaggageFallbackFirst) { - workload_discovery: {} )EOF"); EXPECT_EQ(0, request_headers_.size()); - EXPECT_EQ(0, response_headers_.size()); + EXPECT_EQ(1, response_headers_.size()); checkNoPeer(true); checkPeerNamespace(false, "baggage-upstream"); } @@ -895,7 +895,7 @@ TEST_F(PeerMetadataTest, DownstreamBaggageWithMXFallback) { - baggage: {} - istio_headers: {} )EOF"); - EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(1, request_headers_.size()); EXPECT_EQ(0, response_headers_.size()); checkPeerNamespace(true, "baggage-ns"); checkNoPeer(false); @@ -913,7 +913,7 @@ TEST_F(PeerMetadataTest, DownstreamMXWithBaggageFallback) { - istio_headers: {} - baggage: {} )EOF"); - EXPECT_EQ(0, request_headers_.size()); + EXPECT_EQ(1, request_headers_.size()); EXPECT_EQ(0, response_headers_.size()); // MX header has namespace "default" from SampleIstioHeader checkPeerNamespace(true, "default"); @@ -957,7 +957,7 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoFromBaggage) { Headers::get().Baggage, "k8s.namespace.name=unit-test-namespace,k8s.cluster.name=unit-test-cluster," "service.name=unit-test-service,service.version=v1.0," - "k8s.workload.name=unit-test-workload,k8s.workload.type=deployment," + "k8s.deployment.name=unit-test-workload,k8s.workload.type=deployment," "k8s.instance.name=unit-test-instance,app.name=unit-test-app,app.version=v2.0"); Context ctx; @@ -1013,7 +1013,7 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoAllWorkloadTypes) { { Http::TestRequestHeaderMapImpl headers; headers.setReference(Headers::get().Baggage, - "k8s.namespace.name=test-ns,k8s.workload.type=pod"); + "k8s.namespace.name=test-ns,k8s.pod.name=pod-name"); const auto result = method.derivePeerInfo(stream_info_, headers, ctx); ASSERT_TRUE(result.has_value()); EXPECT_EQ(Istio::Common::WorkloadType::Pod, result->workload_type_); @@ -1023,7 +1023,7 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoAllWorkloadTypes) { { Http::TestRequestHeaderMapImpl headers; headers.setReference(Headers::get().Baggage, - "k8s.namespace.name=test-ns,k8s.workload.type=deployment"); + "k8s.namespace.name=test-ns,k8s.deployment.name=deployment-name"); const auto result = method.derivePeerInfo(stream_info_, headers, ctx); ASSERT_TRUE(result.has_value()); EXPECT_EQ(Istio::Common::WorkloadType::Deployment, result->workload_type_); @@ -1033,7 +1033,7 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoAllWorkloadTypes) { { Http::TestRequestHeaderMapImpl headers; headers.setReference(Headers::get().Baggage, - "k8s.namespace.name=test-ns,k8s.workload.type=job"); + "k8s.namespace.name=test-ns,k8s.job.name=job-name"); const auto result = method.derivePeerInfo(stream_info_, headers, ctx); ASSERT_TRUE(result.has_value()); EXPECT_EQ(Istio::Common::WorkloadType::Job, result->workload_type_); @@ -1043,7 +1043,7 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoAllWorkloadTypes) { { Http::TestRequestHeaderMapImpl headers; headers.setReference(Headers::get().Baggage, - "k8s.namespace.name=test-ns,k8s.workload.type=cronjob"); + "k8s.namespace.name=test-ns,k8s.cronjob.name=cronjob-name"); const auto result = method.derivePeerInfo(stream_info_, headers, ctx); ASSERT_TRUE(result.has_value()); EXPECT_EQ(Istio::Common::WorkloadType::CronJob, result->workload_type_); From 08d575120d72670be80cb7ff2b2654180cefed7a Mon Sep 17 00:00:00 2001 From: Gustavo Date: Thu, 22 Jan 2026 13:15:15 +0000 Subject: [PATCH 07/13] removing comment --- extensions/common/metadata_object.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/common/metadata_object.cc b/extensions/common/metadata_object.cc index 24adf372633..b9df3fb209f 100644 --- a/extensions/common/metadata_object.cc +++ b/extensions/common/metadata_object.cc @@ -24,7 +24,7 @@ namespace Istio { namespace Common { namespace { -// TODO: find a propper name for this mapping + static absl::flat_hash_map ALL_METADATA_FIELDS = { {NamespaceNameToken, BaggageToken::NamespaceName}, {ClusterNameToken, BaggageToken::ClusterName}, From 0e0806fd7f3b13309f6e021fdb0e56933a1d07b6 Mon Sep 17 00:00:00 2001 From: Gustavo Date: Thu, 22 Jan 2026 16:38:35 +0000 Subject: [PATCH 08/13] make lint --- extensions/common/metadata_object.cc | 12 +++---- .../filters/http/peer_metadata/filter.cc | 12 +++---- .../filters/http/peer_metadata/filter.h | 5 ++- .../filters/http/peer_metadata/filter_test.cc | 33 ++++++++----------- 4 files changed, 27 insertions(+), 35 deletions(-) diff --git a/extensions/common/metadata_object.cc b/extensions/common/metadata_object.cc index c9ae206d454..2795fc95ec2 100644 --- a/extensions/common/metadata_object.cc +++ b/extensions/common/metadata_object.cc @@ -399,13 +399,13 @@ std::unique_ptr convertBaggageToWorkloadMetadata(absl::s app_version = parts.second; break; case BaggageToken::WorkloadName: { - workload = parts.second; - std::vector splitWorkloadKey = absl::StrSplit(parts.first, "."); - if (splitWorkloadKey.size() >= 2 && splitWorkloadKey[0] == "k8s") { - workload_type = fromSuffix(splitWorkloadKey[1]); - } - break; + workload = parts.second; + std::vector splitWorkloadKey = absl::StrSplit(parts.first, "."); + if (splitWorkloadKey.size() >= 2 && splitWorkloadKey[0] == "k8s") { + workload_type = fromSuffix(splitWorkloadKey[1]); } + break; + } case BaggageToken::InstanceName: instance = parts.second; break; diff --git a/source/extensions/filters/http/peer_metadata/filter.cc b/source/extensions/filters/http/peer_metadata/filter.cc index ec0f833bfff..43a663a2c47 100644 --- a/source/extensions/filters/http/peer_metadata/filter.cc +++ b/source/extensions/filters/http/peer_metadata/filter.cc @@ -193,13 +193,12 @@ void BaggagePropagationMethod::inject(const StreamInfo::StreamInfo&, Http::Heade headers.setReference(Headers::get().Baggage, value_); } -BaggageDiscoveryMethod::BaggageDiscoveryMethod(bool /*downstream*/, - Server::Configuration::ServerFactoryContext& /*factory_context*/) { -} +BaggageDiscoveryMethod::BaggageDiscoveryMethod( + bool /*downstream*/, Server::Configuration::ServerFactoryContext& /*factory_context*/) {} absl::optional BaggageDiscoveryMethod::derivePeerInfo(const StreamInfo::StreamInfo&, - Http::HeaderMap& headers, - Context&) const { + Http::HeaderMap& headers, + Context&) const { const auto baggage_header = headers.get(Headers::get().Baggage); if (baggage_header.empty()) { return {}; @@ -248,7 +247,8 @@ std::vector FilterConfig::buildDiscoveryMethods( factory_context.serverFactoryContext())); break; case io::istio::http::peer_metadata::Config::DiscoveryMethod::MethodSpecifierCase::kBaggage: - methods.push_back(std::make_unique(downstream, factory_context.serverFactoryContext())); + methods.push_back(std::make_unique( + downstream, factory_context.serverFactoryContext())); break; default: break; diff --git a/source/extensions/filters/http/peer_metadata/filter.h b/source/extensions/filters/http/peer_metadata/filter.h index af62ebd605c..384c0867374 100644 --- a/source/extensions/filters/http/peer_metadata/filter.h +++ b/source/extensions/filters/http/peer_metadata/filter.h @@ -115,7 +115,7 @@ class BaggagePropagationMethod : public PropagationMethod { class BaggageDiscoveryMethod : public DiscoveryMethod, public Logger::Loggable { public: BaggageDiscoveryMethod(bool downstream, - Server::Configuration::ServerFactoryContext& factory_context); + Server::Configuration::ServerFactoryContext& factory_context); absl::optional derivePeerInfo(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const override; }; @@ -163,8 +163,7 @@ class FilterConfig : public Logger::Loggable { using FilterConfigSharedPtr = std::shared_ptr; -class Filter : public Http::PassThroughFilter, - public Logger::Loggable { +class Filter : public Http::PassThroughFilter, public Logger::Loggable { public: Filter(const FilterConfigSharedPtr& config) : config_(config) {} Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap&, bool) override; diff --git a/source/extensions/filters/http/peer_metadata/filter_test.cc b/source/extensions/filters/http/peer_metadata/filter_test.cc index 8815dad7bec..91856686a42 100644 --- a/source/extensions/filters/http/peer_metadata/filter_test.cc +++ b/source/extensions/filters/http/peer_metadata/filter_test.cc @@ -778,12 +778,10 @@ TEST_F(PeerMetadataTest, UpstreamBaggageDiscovery) { } TEST_F(PeerMetadataTest, BothDirectionsBaggageDiscovery) { - request_headers_.setReference( - Headers::get().Baggage, - "k8s.namespace.name=downstream-ns,service.name=downstream-svc"); - response_headers_.setReference( - Headers::get().Baggage, - "k8s.namespace.name=upstream-ns,service.name=upstream-svc"); + request_headers_.setReference(Headers::get().Baggage, + "k8s.namespace.name=downstream-ns,service.name=downstream-svc"); + response_headers_.setReference(Headers::get().Baggage, + "k8s.namespace.name=upstream-ns,service.name=upstream-svc"); initialize(R"EOF( downstream_discovery: - baggage: {} @@ -799,8 +797,7 @@ TEST_F(PeerMetadataTest, BothDirectionsBaggageDiscovery) { TEST_F(PeerMetadataTest, DownstreamBaggageFallbackFirst) { // Baggage is present, so XDS should not be called request_headers_.setReference( - Headers::get().Baggage, - "k8s.namespace.name=baggage-namespace,service.name=baggage-service"); + Headers::get().Baggage, "k8s.namespace.name=baggage-namespace,service.name=baggage-service"); EXPECT_CALL(*metadata_provider_, GetMetadata(_)).Times(0); initialize(R"EOF( downstream_discovery: @@ -880,9 +877,8 @@ TEST_F(PeerMetadataTest, UpstreamBaggageFallbackSecond) { TEST_F(PeerMetadataTest, DownstreamBaggageWithMXFallback) { // Baggage is present, so MX should not be used - request_headers_.setReference( - Headers::get().Baggage, - "k8s.namespace.name=baggage-ns,service.name=baggage-svc"); + request_headers_.setReference(Headers::get().Baggage, + "k8s.namespace.name=baggage-ns,service.name=baggage-svc"); request_headers_.setReference(Headers::get().ExchangeMetadataHeaderId, "test-pod"); request_headers_.setReference(Headers::get().ExchangeMetadataHeader, SampleIstioHeader); initialize(R"EOF( @@ -898,9 +894,8 @@ TEST_F(PeerMetadataTest, DownstreamBaggageWithMXFallback) { TEST_F(PeerMetadataTest, DownstreamMXWithBaggageFallback) { // MX is first, so it should be used even if baggage is present - request_headers_.setReference( - Headers::get().Baggage, - "k8s.namespace.name=baggage-ns,service.name=baggage-svc"); + request_headers_.setReference(Headers::get().Baggage, + "k8s.namespace.name=baggage-ns,service.name=baggage-svc"); request_headers_.setReference(Headers::get().ExchangeMetadataHeaderId, "test-pod"); request_headers_.setReference(Headers::get().ExchangeMetadataHeader, SampleIstioHeader); initialize(R"EOF( @@ -916,9 +911,8 @@ TEST_F(PeerMetadataTest, DownstreamMXWithBaggageFallback) { } TEST_F(PeerMetadataTest, BaggageDiscoveryWithPropagation) { - request_headers_.setReference( - Headers::get().Baggage, - "k8s.namespace.name=discovered-ns,service.name=discovered-svc"); + request_headers_.setReference(Headers::get().Baggage, + "k8s.namespace.name=discovered-ns,service.name=discovered-svc"); initialize(R"EOF( downstream_discovery: - baggage: {} @@ -985,9 +979,8 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoPartialBaggage) { BaggageDiscoveryMethod method(false, context_.server_factory_context_); Http::TestResponseHeaderMapImpl headers; - headers.setReference( - Headers::get().Baggage, - "k8s.namespace.name=partial-ns,service.name=partial-svc"); + headers.setReference(Headers::get().Baggage, + "k8s.namespace.name=partial-ns,service.name=partial-svc"); Context ctx; const auto result = method.derivePeerInfo(stream_info_, headers, ctx); From efd48f5153c30b908fc76c46c5d45460e2113c58 Mon Sep 17 00:00:00 2001 From: Gustavo Date: Thu, 22 Jan 2026 17:10:36 +0000 Subject: [PATCH 09/13] fixing unit tests for metadata_object --- extensions/common/metadata_object_test.cc | 28 +++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/extensions/common/metadata_object_test.cc b/extensions/common/metadata_object_test.cc index 90003b95570..342bbb2d2ce 100644 --- a/extensions/common/metadata_object_test.cc +++ b/extensions/common/metadata_object_test.cc @@ -82,8 +82,8 @@ TEST(WorkloadMetadataObjectTest, ConversionWithLabels) { TEST(WorkloadMetadataObjectTest, Conversion) { { const auto r = convertBaggageToWorkloadMetadata( - "type=deployment,workload=foo,cluster=my-cluster," - "namespace=default,service=foo-service,revision=v1alpha3,app=foo-app,version=latest"); + "k8s.deployment.name=foo,k8s.cluster.name=my-cluster," + "k8s.namespace.name=default,service.name=foo-service,service.version=v1alpha3,app.name=foo-app,app.version=latest"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1alpha3"); EXPECT_EQ(absl::get(r->getField("type")), DeploymentSuffix); @@ -98,13 +98,13 @@ TEST(WorkloadMetadataObjectTest, Conversion) { { const auto r = - convertBaggageToWorkloadMetadata("type=pod,name=foo-pod-435,cluster=my-cluster,namespace=" - "test,service=foo-service,revision=v1beta2"); + convertBaggageToWorkloadMetadata("k8s.pod.name=foo-pod-435,k8s.cluster.name=my-cluster,k8s.namespace.name=" + "test,k8s.instance.name=foo-instance-435,service.name=foo-service,service.version=v1beta2"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1beta2"); EXPECT_EQ(absl::get(r->getField("type")), PodSuffix); - EXPECT_EQ(absl::get(r->getField("workload")), ""); - EXPECT_EQ(absl::get(r->getField("name")), "foo-pod-435"); + EXPECT_EQ(absl::get(r->getField("workload")), "foo-pod-435"); + EXPECT_EQ(absl::get(r->getField("name")), "foo-instance-435"); EXPECT_EQ(absl::get(r->getField("namespace")), "test"); EXPECT_EQ(absl::get(r->getField("cluster")), "my-cluster"); EXPECT_EQ(absl::get(r->getField("app")), ""); @@ -114,13 +114,13 @@ TEST(WorkloadMetadataObjectTest, Conversion) { { const auto r = - convertBaggageToWorkloadMetadata("type=job,name=foo-job-435,cluster=my-cluster,namespace=" - "test,service=foo-service,revision=v1beta4"); + convertBaggageToWorkloadMetadata("k8s.job.name=foo-job-435,k8s.cluster.name=my-cluster,k8s.namespace.name=" + "test,k8s.instance.name=foo-instance-435,service.name=foo-service,service.version=v1beta4"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1beta4"); EXPECT_EQ(absl::get(r->getField("type")), JobSuffix); - EXPECT_EQ(absl::get(r->getField("workload")), ""); - EXPECT_EQ(absl::get(r->getField("name")), "foo-job-435"); + EXPECT_EQ(absl::get(r->getField("workload")), "foo-job-435"); + EXPECT_EQ(absl::get(r->getField("name")), "foo-instance-435"); EXPECT_EQ(absl::get(r->getField("namespace")), "test"); EXPECT_EQ(absl::get(r->getField("cluster")), "my-cluster"); checkStructConversion(*r); @@ -128,8 +128,8 @@ TEST(WorkloadMetadataObjectTest, Conversion) { { const auto r = - convertBaggageToWorkloadMetadata("type=cronjob,workload=foo-cronjob,cluster=my-cluster," - "namespace=test,service=foo-service,revision=v1beta4"); + convertBaggageToWorkloadMetadata("k8s.cronjob.name=foo-cronjob,k8s.cluster.name=my-cluster," + "k8s.namespace.name=test,service.name=foo-service,service.version=v1beta4"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1beta4"); EXPECT_EQ(absl::get(r->getField("type")), CronJobSuffix); @@ -142,7 +142,7 @@ TEST(WorkloadMetadataObjectTest, Conversion) { { const auto r = convertBaggageToWorkloadMetadata( - "type=deployment,workload=foo,namespace=default,service=foo-service,revision=v1alpha3"); + "k8s.deployment.name=foo,k8s.namespace.name=default,service.name=foo-service,service.version=v1alpha3"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1alpha3"); EXPECT_EQ(absl::get(r->getField("type")), DeploymentSuffix); @@ -153,7 +153,7 @@ TEST(WorkloadMetadataObjectTest, Conversion) { } { - const auto r = convertBaggageToWorkloadMetadata("namespace=default"); + const auto r = convertBaggageToWorkloadMetadata("k8s.namespace.name=default"); EXPECT_EQ(absl::get(r->getField("namespace")), "default"); checkStructConversion(*r); } From 7407537770825be02391ad9319016afb08058e43 Mon Sep 17 00:00:00 2001 From: Gustavo Date: Thu, 22 Jan 2026 17:14:05 +0000 Subject: [PATCH 10/13] make lint --- extensions/common/metadata_object_test.cc | 26 ++++++++++++----------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/extensions/common/metadata_object_test.cc b/extensions/common/metadata_object_test.cc index 342bbb2d2ce..188defa777f 100644 --- a/extensions/common/metadata_object_test.cc +++ b/extensions/common/metadata_object_test.cc @@ -83,7 +83,8 @@ TEST(WorkloadMetadataObjectTest, Conversion) { { const auto r = convertBaggageToWorkloadMetadata( "k8s.deployment.name=foo,k8s.cluster.name=my-cluster," - "k8s.namespace.name=default,service.name=foo-service,service.version=v1alpha3,app.name=foo-app,app.version=latest"); + "k8s.namespace.name=default,service.name=foo-service,service.version=v1alpha3,app.name=foo-" + "app,app.version=latest"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1alpha3"); EXPECT_EQ(absl::get(r->getField("type")), DeploymentSuffix); @@ -97,9 +98,9 @@ TEST(WorkloadMetadataObjectTest, Conversion) { } { - const auto r = - convertBaggageToWorkloadMetadata("k8s.pod.name=foo-pod-435,k8s.cluster.name=my-cluster,k8s.namespace.name=" - "test,k8s.instance.name=foo-instance-435,service.name=foo-service,service.version=v1beta2"); + const auto r = convertBaggageToWorkloadMetadata( + "k8s.pod.name=foo-pod-435,k8s.cluster.name=my-cluster,k8s.namespace.name=" + "test,k8s.instance.name=foo-instance-435,service.name=foo-service,service.version=v1beta2"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1beta2"); EXPECT_EQ(absl::get(r->getField("type")), PodSuffix); @@ -113,9 +114,9 @@ TEST(WorkloadMetadataObjectTest, Conversion) { } { - const auto r = - convertBaggageToWorkloadMetadata("k8s.job.name=foo-job-435,k8s.cluster.name=my-cluster,k8s.namespace.name=" - "test,k8s.instance.name=foo-instance-435,service.name=foo-service,service.version=v1beta4"); + const auto r = convertBaggageToWorkloadMetadata( + "k8s.job.name=foo-job-435,k8s.cluster.name=my-cluster,k8s.namespace.name=" + "test,k8s.instance.name=foo-instance-435,service.name=foo-service,service.version=v1beta4"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1beta4"); EXPECT_EQ(absl::get(r->getField("type")), JobSuffix); @@ -127,9 +128,9 @@ TEST(WorkloadMetadataObjectTest, Conversion) { } { - const auto r = - convertBaggageToWorkloadMetadata("k8s.cronjob.name=foo-cronjob,k8s.cluster.name=my-cluster," - "k8s.namespace.name=test,service.name=foo-service,service.version=v1beta4"); + const auto r = convertBaggageToWorkloadMetadata( + "k8s.cronjob.name=foo-cronjob,k8s.cluster.name=my-cluster," + "k8s.namespace.name=test,service.name=foo-service,service.version=v1beta4"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1beta4"); EXPECT_EQ(absl::get(r->getField("type")), CronJobSuffix); @@ -141,8 +142,9 @@ TEST(WorkloadMetadataObjectTest, Conversion) { } { - const auto r = convertBaggageToWorkloadMetadata( - "k8s.deployment.name=foo,k8s.namespace.name=default,service.name=foo-service,service.version=v1alpha3"); + const auto r = + convertBaggageToWorkloadMetadata("k8s.deployment.name=foo,k8s.namespace.name=default," + "service.name=foo-service,service.version=v1alpha3"); EXPECT_EQ(absl::get(r->getField("service")), "foo-service"); EXPECT_EQ(absl::get(r->getField("revision")), "v1alpha3"); EXPECT_EQ(absl::get(r->getField("type")), DeploymentSuffix); From dbf83ef33fcd9131d585e522a846090986a8a84f Mon Sep 17 00:00:00 2001 From: Gustavo Date: Fri, 23 Jan 2026 11:00:37 +0000 Subject: [PATCH 11/13] suggestions from PR --- .../filters/http/peer_metadata/filter.cc | 11 +++--- .../filters/http/peer_metadata/filter.h | 3 +- .../filters/http/peer_metadata/filter_test.cc | 36 +++++++++++++------ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/source/extensions/filters/http/peer_metadata/filter.cc b/source/extensions/filters/http/peer_metadata/filter.cc index 43a663a2c47..be097cdf08b 100644 --- a/source/extensions/filters/http/peer_metadata/filter.cc +++ b/source/extensions/filters/http/peer_metadata/filter.cc @@ -193,8 +193,7 @@ void BaggagePropagationMethod::inject(const StreamInfo::StreamInfo&, Http::Heade headers.setReference(Headers::get().Baggage, value_); } -BaggageDiscoveryMethod::BaggageDiscoveryMethod( - bool /*downstream*/, Server::Configuration::ServerFactoryContext& /*factory_context*/) {} +BaggageDiscoveryMethod::BaggageDiscoveryMethod() {} absl::optional BaggageDiscoveryMethod::derivePeerInfo(const StreamInfo::StreamInfo&, Http::HeaderMap& headers, @@ -247,8 +246,11 @@ std::vector FilterConfig::buildDiscoveryMethods( factory_context.serverFactoryContext())); break; case io::istio::http::peer_metadata::Config::DiscoveryMethod::MethodSpecifierCase::kBaggage: - methods.push_back(std::make_unique( - downstream, factory_context.serverFactoryContext())); + if (downstream) { + methods.push_back(std::make_unique()); + } else { + ENVOY_LOG(warn, "Baggage discovery configuration found for upstream, this is not supported!"); + } break; default: break; @@ -339,7 +341,6 @@ void FilterConfig::setFilterState(StreamInfo::StreamInfo& info, bool downstream, auto pb = value.serializeAsProto(); auto peer_info = std::make_unique(FilterConfig::peerInfoPrototype()); peer_info->setValue(absl::string_view(pb->SerializeAsString())); - ENVOY_LOG(info, "setting peer_info: {} = {}", key, pb->SerializeAsString()); info.filterState()->setData( key, std::move(peer_info), StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::FilterChain, sharedWithUpstream()); diff --git a/source/extensions/filters/http/peer_metadata/filter.h b/source/extensions/filters/http/peer_metadata/filter.h index 384c0867374..33c470826f4 100644 --- a/source/extensions/filters/http/peer_metadata/filter.h +++ b/source/extensions/filters/http/peer_metadata/filter.h @@ -114,8 +114,7 @@ class BaggagePropagationMethod : public PropagationMethod { class BaggageDiscoveryMethod : public DiscoveryMethod, public Logger::Loggable { public: - BaggageDiscoveryMethod(bool downstream, - Server::Configuration::ServerFactoryContext& factory_context); + BaggageDiscoveryMethod(); absl::optional derivePeerInfo(const StreamInfo::StreamInfo&, Http::HeaderMap&, Context&) const override; }; diff --git a/source/extensions/filters/http/peer_metadata/filter_test.cc b/source/extensions/filters/http/peer_metadata/filter_test.cc index 91856686a42..f5399a2fd9b 100644 --- a/source/extensions/filters/http/peer_metadata/filter_test.cc +++ b/source/extensions/filters/http/peer_metadata/filter_test.cc @@ -732,6 +732,8 @@ TEST_F(PeerMetadataTest, DownstreamBaggageDiscoveryEmpty) { } TEST_F(PeerMetadataTest, UpstreamBaggageDiscoveryEmpty) { + // The baggage discovery filter should only be used for downstream + // peer metadata detection. initialize(R"EOF( upstream_discovery: - baggage: {} @@ -774,7 +776,8 @@ TEST_F(PeerMetadataTest, UpstreamBaggageDiscovery) { EXPECT_EQ(0, request_headers_.size()); EXPECT_EQ(1, response_headers_.size()); checkNoPeer(true); - checkPeerNamespace(false, "upstream-namespace"); + // Baggage discovery should ignore upstream. + checkNoPeer(false); } TEST_F(PeerMetadataTest, BothDirectionsBaggageDiscovery) { @@ -791,7 +794,8 @@ TEST_F(PeerMetadataTest, BothDirectionsBaggageDiscovery) { EXPECT_EQ(1, request_headers_.size()); EXPECT_EQ(1, response_headers_.size()); checkPeerNamespace(true, "downstream-ns"); - checkPeerNamespace(false, "upstream-ns"); + // Baggage discovery should ignore upstream + checkNoPeer(false); } TEST_F(PeerMetadataTest, DownstreamBaggageFallbackFirst) { @@ -835,11 +839,22 @@ TEST_F(PeerMetadataTest, DownstreamBaggageFallbackSecond) { } TEST_F(PeerMetadataTest, UpstreamBaggageFallbackFirst) { - // Baggage is present, so XDS should not be called + // Baggage is present, but ignored as it's coming from upstream. response_headers_.setReference( Headers::get().Baggage, "k8s.namespace.name=baggage-upstream,service.name=baggage-upstream-service"); - EXPECT_CALL(*metadata_provider_, GetMetadata(_)).Times(0); + // WDS information is also present, and this is the one tha tshould be used. + const WorkloadMetadataObject pod("pod-foo-1234", "my-cluster", "xds-upstream", "foo", + "foo-service", "v1alpha3", "", "", + Istio::Common::WorkloadType::Pod, ""); + EXPECT_CALL(*metadata_provider_, GetMetadata(_)) + .WillRepeatedly(Invoke([&](const Network::Address::InstanceConstSharedPtr& address) + -> std::optional { + if (absl::StartsWith(address->asStringView(), "10.0.0.1")) { + return {pod}; + } + return {}; + })); initialize(R"EOF( upstream_discovery: - baggage: {} @@ -848,11 +863,12 @@ TEST_F(PeerMetadataTest, UpstreamBaggageFallbackFirst) { EXPECT_EQ(0, request_headers_.size()); EXPECT_EQ(1, response_headers_.size()); checkNoPeer(true); - checkPeerNamespace(false, "baggage-upstream"); + checkPeerNamespace(false, "xds-upstream"); } TEST_F(PeerMetadataTest, UpstreamBaggageFallbackSecond) { - // No baggage header, so XDS should be called as fallback + // No baggage header, baggage is ignored as it's coming from upstream, + // but workload discovery should pick up the details. const WorkloadMetadataObject pod("pod-foo-1234", "my-cluster", "xds-upstream", "foo", "foo-service", "v1alpha3", "", "", Istio::Common::WorkloadType::Pod, ""); @@ -939,7 +955,7 @@ class BaggageDiscoveryMethodTest : public testing::Test { }; TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoFromBaggage) { - BaggageDiscoveryMethod method(true, context_.server_factory_context_); + BaggageDiscoveryMethod method; Http::TestRequestHeaderMapImpl headers; headers.setReference( @@ -965,7 +981,7 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoFromBaggage) { } TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoEmptyBaggage) { - BaggageDiscoveryMethod method(true, context_.server_factory_context_); + BaggageDiscoveryMethod method; Http::TestRequestHeaderMapImpl headers; Context ctx; @@ -976,7 +992,7 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoEmptyBaggage) { } TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoPartialBaggage) { - BaggageDiscoveryMethod method(false, context_.server_factory_context_); + BaggageDiscoveryMethod method; Http::TestResponseHeaderMapImpl headers; headers.setReference(Headers::get().Baggage, @@ -994,7 +1010,7 @@ TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoPartialBaggage) { } TEST_F(BaggageDiscoveryMethodTest, DerivePeerInfoAllWorkloadTypes) { - BaggageDiscoveryMethod method(true, context_.server_factory_context_); + BaggageDiscoveryMethod method; Context ctx; // Test Pod workload type From ac7c5ef0c36296d8a2ecb03d0809335fef2c5800 Mon Sep 17 00:00:00 2001 From: Gustavo Date: Fri, 23 Jan 2026 11:51:52 +0000 Subject: [PATCH 12/13] clarifying use of mappings for baggage and field access --- extensions/common/metadata_object.cc | 18 +++++++++++------- extensions/common/metadata_object.h | 3 +++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/extensions/common/metadata_object.cc b/extensions/common/metadata_object.cc index 2795fc95ec2..8a2a714468c 100644 --- a/extensions/common/metadata_object.cc +++ b/extensions/common/metadata_object.cc @@ -25,6 +25,8 @@ namespace Common { namespace { +// This maps field names into baggage tokens. We use it to decode field names +// when WorkloadMetadataObject content is accessed through the Envoy API. static absl::flat_hash_map ALL_METADATA_FIELDS = { {NamespaceNameToken, BaggageToken::NamespaceName}, {ClusterNameToken, BaggageToken::ClusterName}, @@ -37,6 +39,8 @@ static absl::flat_hash_map ALL_METADATA_FIELDS {InstanceNameToken, BaggageToken::InstanceName}, }; +// This maps baggage keys into baggage tokens. We use it to decode baggage keys +// coming over the wire when building WorkloadMetadataObject. static absl::flat_hash_map ALL_BAGGAGE_TOKENS = { {NamespaceNameBaggageToken, BaggageToken::NamespaceName}, {ClusterNameBaggageToken, BaggageToken::ClusterName}, @@ -84,13 +88,13 @@ std::string WorkloadMetadataObject::baggage() const { } // Map the workload metadata fields to baggage tokens const std::vector> field_to_baggage = { - {Istio::Common::NamespaceNameToken, "k8s.namespace.name"}, - {Istio::Common::ClusterNameToken, "k8s.cluster.name"}, - {Istio::Common::ServiceNameToken, "service.name"}, - {Istio::Common::ServiceVersionToken, "service.version"}, - {Istio::Common::AppNameToken, "app.name"}, - {Istio::Common::AppVersionToken, "app.version"}, - {Istio::Common::InstanceNameToken, "k8s.instance.name"}, + {Istio::Common::NamespaceNameToken, Istio::Common::NamespaceNameBaggageToken}, + {Istio::Common::ClusterNameToken, Istio::Common::ClusterNameBaggageToken}, + {Istio::Common::ServiceNameToken, Istio::Common::ServiceNameBaggageToken}, + {Istio::Common::ServiceVersionToken, Istio::Common::ServiceVersionBaggageToken}, + {Istio::Common::AppNameToken, Istio::Common::AppNameBaggageToken}, + {Istio::Common::AppVersionToken, Istio::Common::AppVersionBaggageToken}, + {Istio::Common::InstanceNameToken, Istio::Common::InstanceNameBaggageToken}, }; for (const auto& [field_name, baggage_key] : field_to_baggage) { diff --git a/extensions/common/metadata_object.h b/extensions/common/metadata_object.h index 53198e78d09..8e29767aee6 100644 --- a/extensions/common/metadata_object.h +++ b/extensions/common/metadata_object.h @@ -67,6 +67,7 @@ enum class BaggageToken { InstanceName, }; +// Field names accessible from WorkloadMetadataObject. constexpr absl::string_view NamespaceNameToken = "namespace"; constexpr absl::string_view ClusterNameToken = "cluster"; constexpr absl::string_view ServiceNameToken = "service"; @@ -79,6 +80,8 @@ constexpr absl::string_view InstanceNameToken = "name"; constexpr absl::string_view LabelsToken = "labels"; constexpr absl::string_view IdentityToken = "identity"; +// Field names used to translate baggage content into +// WorkloadMetadataObject information. constexpr absl::string_view NamespaceNameBaggageToken = "k8s.namespace.name"; constexpr absl::string_view ClusterNameBaggageToken = "k8s.cluster.name"; constexpr absl::string_view ServiceNameBaggageToken = "service.name"; From 1a87c89245566bec4d24e4e4bb1df6c6b168e3e6 Mon Sep 17 00:00:00 2001 From: Gustavo Date: Fri, 23 Jan 2026 12:27:30 +0000 Subject: [PATCH 13/13] make lint --- source/extensions/filters/http/peer_metadata/filter.cc | 3 ++- source/extensions/filters/http/peer_metadata/filter_test.cc | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/http/peer_metadata/filter.cc b/source/extensions/filters/http/peer_metadata/filter.cc index be097cdf08b..93b36b667ac 100644 --- a/source/extensions/filters/http/peer_metadata/filter.cc +++ b/source/extensions/filters/http/peer_metadata/filter.cc @@ -249,7 +249,8 @@ std::vector FilterConfig::buildDiscoveryMethods( if (downstream) { methods.push_back(std::make_unique()); } else { - ENVOY_LOG(warn, "Baggage discovery configuration found for upstream, this is not supported!"); + ENVOY_LOG(warn, + "Baggage discovery configuration found for upstream, this is not supported!"); } break; default: diff --git a/source/extensions/filters/http/peer_metadata/filter_test.cc b/source/extensions/filters/http/peer_metadata/filter_test.cc index f5399a2fd9b..7824c9a3d9b 100644 --- a/source/extensions/filters/http/peer_metadata/filter_test.cc +++ b/source/extensions/filters/http/peer_metadata/filter_test.cc @@ -732,8 +732,8 @@ TEST_F(PeerMetadataTest, DownstreamBaggageDiscoveryEmpty) { } TEST_F(PeerMetadataTest, UpstreamBaggageDiscoveryEmpty) { - // The baggage discovery filter should only be used for downstream - // peer metadata detection. + // The baggage discovery filter should only be used for downstream + // peer metadata detection. initialize(R"EOF( upstream_discovery: - baggage: {}