Skip to content

[release-1.29] Add e2e test for proxy baggage-based metadata discovery + fixes #6802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
krinkinmu wants to merge 1 commit into
base: release-1.29
Choose a base branch
from
Open

[release-1.29] Add e2e test for proxy baggage-based metadata discovery + fixes #6802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 55 additions & 15 deletions extensions/common/metadata_object.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,25 +90,42 @@ std::string WorkloadMetadataObject::baggage() const {
if (!workload_name_.empty()) {
parts.push_back("k8s." + std::string(workload_type) + ".name=" + std::string(workload_name_));
}

const auto appName = field(Istio::Common::AppNameToken).value_or("");
const auto serviceName = field(Istio::Common::ServiceNameToken).value_or(appName);

if (!serviceName.empty()) {
parts.push_back(absl::StrCat(Istio::Common::ServiceNameBaggageToken, "=", serviceName));
}

if (!appName.empty() && appName != serviceName) {
parts.push_back(absl::StrCat(Istio::Common::AppNameBaggageToken, "=", appName));
}

const auto appVersion = field(Istio::Common::AppVersionToken).value_or("");
const auto serviceVersion = field(Istio::Common::ServiceVersionToken).value_or(appVersion);

if (!serviceVersion.empty()) {
parts.push_back(absl::StrCat(Istio::Common::ServiceVersionBaggageToken, "=", serviceVersion));
}

if (!appVersion.empty() && appVersion != serviceVersion) {
parts.push_back(absl::StrCat(Istio::Common::AppVersionBaggageToken, "=", appVersion));
}

// Map the workload metadata fields to baggage tokens
const std::vector<std::pair<absl::string_view, absl::string_view>> field_to_baggage = {
{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},
{Istio::Common::RegionToken, Istio::Common::LocalityRegionBaggageToken},
{Istio::Common::ZoneToken, Istio::Common::LocalityZoneBaggageToken},
};

for (const auto& [field_name, baggage_key] : field_to_baggage) {
const auto field_result = getField(field_name);
if (auto field_value = std::get_if<absl::string_view>(&field_result)) {
if (!field_value->empty()) {
parts.push_back(absl::StrCat(baggage_key, "=", *field_value));
}
const auto value = field(field_name);
if (value && !value->empty()) {
parts.push_back(absl::StrCat(baggage_key, "=", *value));
}
}
return absl::StrJoin(parts, ",");
Expand Down Expand Up @@ -333,9 +350,13 @@ convertStructToWorkloadMetadata(const google::protobuf::Struct& metadata,
canonical_name = labels_it.second.string_value();
} else if (labels_it.first == CanonicalRevisionLabel) {
canonical_revision = labels_it.second.string_value();
} else if (labels_it.first == AppNameLabel) {
} else if (labels_it.first == AppNameQualifiedLabel) {
app_name = labels_it.second.string_value();
} else if (labels_it.first == AppNameLabel && app_name.empty()) {
app_name = labels_it.second.string_value();
} else if (labels_it.first == AppVersionLabel) {
} else if (labels_it.first == AppVersionQualifiedLabel) {
app_version = labels_it.second.string_value();
} else if (labels_it.first == AppVersionLabel && app_version.empty()) {
app_version = labels_it.second.string_value();
} else if (!additional_labels.empty() &&
additional_labels.contains(std::string(labels_it.first))) {
Expand Down Expand Up @@ -386,8 +407,8 @@ std::string serializeToStringDeterministic(const google::protobuf::Struct& metad
return out;
}

WorkloadMetadataObject::FieldType
WorkloadMetadataObject::getField(absl::string_view field_name) const {
absl::optional<absl::string_view>
WorkloadMetadataObject::field(absl::string_view field_name) const {
const auto it = ALL_METADATA_FIELDS.find(field_name);
if (it != ALL_METADATA_FIELDS.end()) {
switch (it->second) {
Expand Down Expand Up @@ -418,6 +439,15 @@ WorkloadMetadataObject::getField(absl::string_view field_name) const {
return locality_zone_;
}
}
return absl::nullopt;
}

WorkloadMetadataObject::FieldType
WorkloadMetadataObject::getField(absl::string_view field_name) const {
const auto value = field(field_name);
if (value) {
return *value;
}
return {};
}

Expand Down Expand Up @@ -454,18 +484,28 @@ convertBaggageToWorkloadMetadata(absl::string_view data, absl::string_view ident
case BaggageToken::ServiceName:
// canonical name and app name are always the same
canonical_name = parts.second;
app_name = parts.second;
if (app_name.empty()) {
app_name = parts.second;
}
break;
case BaggageToken::ServiceVersion:
// canonical revision and app version are always the same
canonical_revision = parts.second;
app_version = parts.second;
if (app_version.empty()) {
app_version = parts.second;
}
break;
case BaggageToken::AppName:
app_name = parts.second;
if (canonical_name.empty()) {
canonical_name = parts.second;
}
break;
case BaggageToken::AppVersion:
app_version = parts.second;
if (canonical_revision.empty()) {
canonical_revision = parts.second;
}
break;
case BaggageToken::WorkloadName: {
workload = parts.second;
Expand Down
3 changes: 3 additions & 0 deletions extensions/common/metadata_object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ constexpr absl::string_view NoPeer = "peer_not_found";
// Special labels used in the peer metadata.
constexpr absl::string_view CanonicalNameLabel = "service.istio.io/canonical-name";
constexpr absl::string_view CanonicalRevisionLabel = "service.istio.io/canonical-revision";
constexpr absl::string_view AppNameQualifiedLabel = "app.kubernetes.io/name";
constexpr absl::string_view AppNameLabel = "app";
constexpr absl::string_view AppVersionQualifiedLabel = "app.kubernetes.io/version";
constexpr absl::string_view AppVersionLabel = "version";

enum class WorkloadType {
Expand Down Expand Up @@ -141,6 +143,7 @@ class WorkloadMetadataObject : public Envoy::StreamInfo::FilterState::Object,
bool hasFieldSupport() const override { return true; }
using Envoy::StreamInfo::FilterState::Object::FieldType;
FieldType getField(absl::string_view) const override;
absl::optional<absl::string_view> field(absl::string_view field_name) const;
void setLabels(std::vector<std::pair<std::string, std::string>> labels) { labels_ = labels; }
std::vector<std::pair<std::string, std::string>> getLabels() const { return labels_; }
std::string baggage() const;
Expand Down
31 changes: 30 additions & 1 deletion extensions/common/metadata_object_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace Common {
using Envoy::Protobuf::util::MessageDifferencer;
using ::testing::NiceMock;

TEST(WorkloadMetadataObjectTest, Baggage) {
TEST(WorkloadMetadataObjectTest, SerializeAsString) {
constexpr absl::string_view identity = "spiffe://cluster.local/ns/default/sa/default";
WorkloadMetadataObject deploy("pod-foo-1234", "my-cluster", "default", "foo", "foo-service",
"v1alpha3", "", "", WorkloadType::Deployment, identity, "", "");
Expand Down Expand Up @@ -166,6 +166,35 @@ TEST(WorkloadMetadataObjectTest, Conversion) {
checkStructConversion(*r);
}

{
const auto r =
convertBaggageToWorkloadMetadata("service.name=foo-service,service.version=v1alpha3");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("service")), "foo-service");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("revision")), "v1alpha3");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("app")), "foo-service");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("version")), "v1alpha3");
checkStructConversion(*r);
}

{
const auto r = convertBaggageToWorkloadMetadata("app.name=foo-app,app.version=latest");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("service")), "foo-app");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("revision")), "latest");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("app")), "foo-app");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("version")), "latest");
checkStructConversion(*r);
}

{
const auto r = convertBaggageToWorkloadMetadata(
"service.name=foo-service,service.version=v1alpha3,app.name=foo-app,app.version=latest");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("service")), "foo-service");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("revision")), "v1alpha3");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("app")), "foo-app");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("version")), "latest");
checkStructConversion(*r);
}

{
const auto r = convertBaggageToWorkloadMetadata("k8s.namespace.name=default");
EXPECT_EQ(absl::get<absl::string_view>(r->getField("namespace")), "default");
Expand Down
2 changes: 0 additions & 2 deletions source/extensions/filters/http/peer_metadata/filter_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,8 +702,6 @@ TEST_F(PeerMetadataTest, BaggagePropagationWithNodeMetadata) {
// 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.pod.name=test-workload"));
Expand Down
1 change: 1 addition & 0 deletions test/envoye2e/inventory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,6 @@ func init() {
"TestAdditionalLabels",
"TestTCPMXAdditionalLabels",
"TestStatsClientSidecarCONNECT",
"TestStatsWithBaggageWaypointProxy",
}...)
}
104 changes: 104 additions & 0 deletions test/envoye2e/stats_plugin/stats_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,110 @@ func TestStatsServerWaypointProxy(t *testing.T) {
}
}

// TestStatsWithBaggageWaypointProxy verifies that baggage-based metadata discovery works correctly.
//
// The test setup is somewhat simplified version of the configuration that pilot generates. The major differences
// relevant to understanding the test below are as follows:
//
// 1. We use just 2 Envoys - client and server, that's what most of the tests here use and we just build
// on top of that
// - server plays a role of a destination workload ztunnel - it terminates HBONE connection
// - client plays a role of a waypoint
// 2. For connection from the test to the client we don't use HBONE:
// - downstream peer metadata discovery works with or without HBONE as long as baggage is provided
// - for connection from client to server we have to use HBONE, because upstream metadata discovery
// relies on network filters in the upstream internal listener.
func TestStatsWithBaggageWaypointProxy(t *testing.T) {
params := driver.NewTestParams(t, map[string]string{
// We are testing baggage-based metadata discovery, so no need for xDS-based metadata discovery
"EnableMetadataDiscovery": "false",
"RequestCount": "10",
// This makes internal_outbound cluster populate endpoint metadata with original destination details
"EnableTunnelEndpointMetadata": "true",
// This overrides the server port from ServerPort to ServerTunnelPort, and the listener that can
// terminate HBONE listens on the ServerTunnelPort, so that's what we need.
"EnableOriginalDstPortOverride": "true",
}, envoye2e.ProxyE2ETests)

params.Vars["ClientMetadata"] = driver.LoadTestData("testdata/client_node_metadata.json.tmpl")
params.Vars["ServerMetadata"] = driver.LoadTestData("testdata/server_node_metadata.json.tmpl")
params.Vars["ServerClusterName"] = "internal_outbound"
params.Vars["ServerInternalAddress"] = "internal_inbound"
params.Vars["StatsFilterClientConfig"] = "disable_host_header_fallback: true\nreporter: SERVER_GATEWAY"
params.Vars["ClientHTTPFilters"] = driver.LoadTestData("testdata/filters/baggage_peer_metadata.yaml.tmpl") +
"\n" + driver.LoadTestData("testdata/filters/stats_outbound.yaml.tmpl")
// This filter is what modifies the default framework HTTP response to include the baggage field from
// the request as one of the headers in the response
params.Vars["ServerHTTPFilters"] = driver.LoadTestData("testdata/filters/respond_with_baggage.yaml.tmpl")
params.Vars["UpstreamNetworkFilters"] = driver.LoadTestData("testdata/filters/upstream_peer_metadata.yaml.tmpl")

clientBaggage := "k8s.deployment.name=productpage-v1,service.name=productpage-v1,app.name=productpage," +
"service.version=version-1,app.version=v1,k8s.namespace.name=default,k8s.cluster.name=client-cluster," +
"k8s.instance.name=productpage-v1-84975bc778-pxz2w"
testBaggage := "k8s.deployment.name=curl,service.name=curl,service.version=v1,k8s.namespace.name=default," +
"k8s.cluster.name=curl-cluster,k8s.instance.name=curl-xxxxxxxxxx-xxxxx"

if err := (&driver.Scenario{
Steps: []driver.Step{
&driver.XDS{},
&driver.Update{
Node: "client", Version: "0",
Clusters: []string{
params.LoadTestData("testdata/cluster/internal_outbound.yaml.tmpl"),
params.LoadTestData("testdata/cluster/original_dst.yaml.tmpl"),
},
Listeners: []string{
params.LoadTestData("testdata/listener/client.yaml.tmpl"),
params.LoadTestData("testdata/listener/baggage_peer_metadata.yaml.tmpl"),
},
Secrets: []string{
params.LoadTestData("testdata/secret/client.yaml.tmpl"),
},
},
&driver.Update{
Node: "server", Version: "0",
Clusters: []string{
params.LoadTestData("testdata/cluster/internal_inbound.yaml.tmpl"),
},
Listeners: []string{
params.LoadTestData("testdata/listener/terminate_connect.yaml.tmpl"),
params.LoadTestData("testdata/listener/server.yaml.tmpl"),
},
Secrets: []string{
params.LoadTestData("testdata/secret/server.yaml.tmpl"),
},
},
&driver.Envoy{Bootstrap: params.LoadTestData("testdata/bootstrap/client.yaml.tmpl")},
&driver.Envoy{Bootstrap: params.LoadTestData("testdata/bootstrap/server.yaml.tmpl")},
&driver.Sleep{Duration: 1 * time.Second},
&driver.Repeat{
N: 10,
Step: &driver.HTTPCall{
Port: params.Ports.ClientPort,
RequestHeaders: map[string]string{
"baggage": testBaggage,
},
ResponseCode: 200,
ResponseHeaders: map[string]string{
// This is what we got from the client
"baggage": clientBaggage,
// This is what the server got from the client
"request-baggage": clientBaggage,
},
},
},
&driver.Stats{
AdminPort: params.Ports.ClientAdmin,
Matchers: map[string]driver.StatMatcher{
"istio_requests_total": &driver.ExactStat{Metric: "testdata/metric/client_request_total_baggage.yaml.tmpl"},
},
},
},
}).Run(params); err != nil {
t.Fatal(err)
}
}

func TestStatsClientSidecarCONNECT(t *testing.T) {
params := driver.NewTestParams(t, map[string]string{
"RequestCount": "10",
Expand Down
4 changes: 4 additions & 0 deletions testdata/cluster/internal_outbound.yaml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ load_assignment:
istio:
workload: ratings-v1;default;ratings;version-1;server-cluster
{{- end }}
{{- if .Vars.UpstreamNetworkFilters }}
filters:
{{ .Vars.UpstreamNetworkFilters | fill }}
{{- end }}
transport_socket:
name: envoy.transport_sockets.internal_upstream
typed_config:
Expand Down
12 changes: 12 additions & 0 deletions testdata/filters/baggage_peer_metadata.yaml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
- name: waypoint_peer_metadata
typed_config:
"@type": type.googleapis.com/udpa.type.v1.TypedStruct
type_url: type.googleapis.com/io.istio.http.peer_metadata.Config
value:
downstream_discovery:
- baggage: {}
downstream_propagation:
- baggage: {}
upstream_discovery:
- upstream_filter_state:
peer_metadata_key: upstream_peer
12 changes: 12 additions & 0 deletions testdata/filters/respond_with_baggage.yaml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
- name: envoy.filters.http.header_mutation
typed_config:
"@type": type.googleapis.com/udpa.type.v1.TypedStruct
type_url: type.googleapis.com/envoy.extensions.filters.http.header_mutation.v3.HeaderMutation
value:
mutations:
response_mutations:
- append:
header:
key: "request-baggage"
value: "%FILTER_STATE(io.istio.baggage:PLAIN)%"
append_action: OVERWRITE_IF_EXISTS_OR_ADD
5 changes: 5 additions & 0 deletions testdata/filters/upstream_peer_metadata.yaml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- name: upstream_peer_metadata
typed_config:
"@type": type.googleapis.com/udpa.type.v1.TypedStruct
type_url: type.googleapis.com/envoy.extensions.network_filters.peer_metadata.UpstreamConfig
value: {}
29 changes: 29 additions & 0 deletions testdata/listener/baggage_peer_metadata.yaml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: internal_outbound
use_original_dst: false
internal_listener: {}
listener_filters:
- name: set_dst_address
typed_config:
"@type": type.googleapis.com/udpa.type.v1.TypedStruct
type_url: type.googleapis.com/envoy.extensions.filters.listener.original_dst.v3.OriginalDst
filter_chains:
- filters:
- name: envoy.filters.network.peer_metadata
typed_config:
"@type": type.googleapis.com/udpa.type.v1.TypedStruct
type_url: type.googleapis.com/envoy.extensions.network_filters.peer_metadata.Config
value:
baggage_key: "io.istio.baggage"
- name: connect_originate
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
stat_prefix: connect_originate
cluster: original_dst
tunneling_config:
hostname: "%DOWNSTREAM_LOCAL_ADDRESS%"
propagate_response_headers: true
headers_to_add:
- header:
key: "baggage"
value: "%FILTER_STATE(io.istio.baggage:PLAIN)%"
append_action: OVERWRITE_IF_EXISTS_OR_ADD
Loading