Skip to content

Commit

Permalink
Revert "header_to_metadata: refactor to preferred style (envoyproxy#2…
Browse files Browse the repository at this point in the history 
…9035)"

This reverts commit 97a340c.

Signed-off-by: Ryan Northey <[email protected]>
  • Loading branch information
@phlax
phlax committed Aug 17, 2023
1 parent b271307 commit 91cf20b
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 62 deletions.
13 changes: 10 additions & 3 deletions source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ void HeaderToMetadataFilter::setEncoderFilterCallbacks(
encoder_callbacks_ = &callbacks;
}

bool HeaderToMetadataFilter::addMetadata(StructMap& struct_map, const std::string& meta_namespace,
bool HeaderToMetadataFilter::addMetadata(StructMap& map, const std::string& meta_namespace,
const std::string& key, std::string value, ValueType type,
ValueEncode encode) const {
ProtobufWkt::Value val;
Expand Down Expand Up @@ -196,8 +196,15 @@ bool HeaderToMetadataFilter::addMetadata(StructMap& struct_map, const std::strin
}
}

auto& keyval = struct_map[meta_namespace];
(*keyval.mutable_fields())[key] = std::move(val);
// Have we seen this namespace before?
auto namespace_iter = map.find(meta_namespace);
if (namespace_iter == map.end()) {
map[meta_namespace] = ProtobufWkt::Struct();
namespace_iter = map.find(meta_namespace);
}

auto& keyval = namespace_iter->second;
(*keyval.mutable_fields())[key] = val;

return true;
}
Expand Down
15 changes: 11 additions & 4 deletions ...ions/filters/network/thrift_proxy/filters/header_to_metadata/header_to_metadata_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Rule::Rule(const ProtoRule& rule) : rule_(rule) {
}

if (rule.has_on_missing() && rule.on_missing().value().empty()) {
throw EnvoyException("Cannot specify on_missing rule with empty value");
throw EnvoyException("Cannot specify on_missing rule without non-empty value");
}

if (rule.has_on_present() && rule.on_present().has_regex_value_rewrite()) {
Expand Down Expand Up @@ -72,7 +72,7 @@ const std::string& HeaderToMetadataFilter::decideNamespace(const std::string& ns
return nspace.empty() ? headerToMetadata : nspace;
}

bool HeaderToMetadataFilter::addMetadata(StructMap& struct_map, const std::string& meta_namespace,
bool HeaderToMetadataFilter::addMetadata(StructMap& map, const std::string& meta_namespace,
const std::string& key, std::string value, ValueType type,
ValueEncode encode) const {
ProtobufWkt::Value val;
Expand Down Expand Up @@ -122,8 +122,15 @@ bool HeaderToMetadataFilter::addMetadata(StructMap& struct_map, const std::strin
}
}

auto& keyval = struct_map[meta_namespace];
(*keyval.mutable_fields())[key] = std::move(val);
// Have we seen this namespace before?
auto namespace_iter = map.find(meta_namespace);
if (namespace_iter == map.end()) {
map[meta_namespace] = ProtobufWkt::Struct();
namespace_iter = map.find(meta_namespace);
}

auto& keyval = namespace_iter->second;
(*keyval.mutable_fields())[key] = val;

return true;
}
Expand Down
11 changes: 9 additions & 2 deletions ...ns/filters/network/thrift_proxy/filters/payload_to_metadata/payload_to_metadata_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,15 @@ bool PayloadToMetadataFilter::addMetadata(const std::string& meta_namespace, con
}
}

auto& keyval = structs_by_namespace_[meta_namespace];
(*keyval.mutable_fields())[key] = std::move(val);
// Have we seen this namespace before?
auto namespace_iter = structs_by_namespace_.find(meta_namespace);
if (namespace_iter == structs_by_namespace_.end()) {
structs_by_namespace_[meta_namespace] = ProtobufWkt::Struct();
namespace_iter = structs_by_namespace_.find(meta_namespace);
}

auto& keyval = namespace_iter->second;
(*keyval.mutable_fields())[key] = val;

return true;
}
Expand Down
4 changes: 4 additions & 0 deletions ...ons/filters/network/thrift_proxy/filters/payload_to_metadata/payload_to_metadata_filter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ class PayloadToMetadataFilter : public MetadataHandler,
void handleOnMissing() override;

private:
static TransportPtr createTransport(TransportType transport) {
return NamedTransportConfigFactory::getFactory(transport).createTransport();
}

static ProtocolPtr createProtocol(ProtocolType protocol) {
return NamedProtocolConfigFactory::getFactory(protocol).createProtocol();
}
Expand Down
37 changes: 18 additions & 19 deletions test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class HeaderToMetadataTest : public testing::Test {
TEST_F(HeaderToMetadataTest, BasicRequestTest) {
initializeFilter(request_config_yaml);
Http::TestRequestHeaderMapImpl incoming_headers{{"X-VERSION", "0xdeadbeef"}};
const std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};
std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand All @@ -111,7 +111,7 @@ TEST_F(HeaderToMetadataTest, BasicRequestTest) {
TEST_F(HeaderToMetadataTest, BasicRequestDoubleHeadersTest) {
initializeFilter(request_config_yaml);
Http::TestRequestHeaderMapImpl incoming_headers{{"X-VERSION", "foo"}, {"X-VERSION", "bar"}};
const std::map<std::string, std::string> expected = {{"version", "foo,bar"}};
std::map<std::string, std::string> expected = {{"version", "foo,bar"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand All @@ -129,7 +129,7 @@ TEST_F(HeaderToMetadataTest, PerRouteOverride) {
// Global config is empty.
initializeFilter("{}");
Http::TestRequestHeaderMapImpl incoming_headers{{"X-VERSION", "0xdeadbeef"}};
const std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};
std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};

// Setup per route config.
envoy::extensions::filters::http::header_to_metadata::v3::Config config_proto;
Expand All @@ -154,7 +154,7 @@ TEST_F(HeaderToMetadataTest, ConfigIsCached) {
// Global config is empty.
initializeFilter("{}");
Http::TestRequestHeaderMapImpl incoming_headers{{"X-VERSION", "0xdeadbeef"}};
const std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};
std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};

// Setup per route config.
envoy::extensions::filters::http::header_to_metadata::v3::Config config_proto;
Expand All @@ -173,7 +173,7 @@ TEST_F(HeaderToMetadataTest, ConfigIsCached) {
TEST_F(HeaderToMetadataTest, DefaultEndpointsTest) {
initializeFilter(request_config_yaml);
Http::TestRequestHeaderMapImpl incoming_headers{{"X-FOO", "bar"}};
const std::map<std::string, std::string> expected = {{"default", "true"}};
std::map<std::string, std::string> expected = {{"default", "true"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand All @@ -194,7 +194,7 @@ TEST_F(HeaderToMetadataTest, HeaderRemovedTest) {
)EOF";
initializeFilter(response_config_yaml);
Http::TestResponseHeaderMapImpl incoming_headers{{"x-authenticated", "1"}};
const std::map<std::string, std::string> expected = {{"auth", "1"}};
std::map<std::string, std::string> expected = {{"auth", "1"}};
Http::TestResponseHeaderMapImpl empty_headers;

EXPECT_CALL(encoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
Expand Down Expand Up @@ -250,7 +250,7 @@ TEST_F(HeaderToMetadataTest, StringTypeInBase64UrlTest) {
std::string data = "Non-ascii-characters";
const auto encoded = Base64::encode(data.c_str(), data.size());
Http::TestResponseHeaderMapImpl incoming_headers{{"x-authenticated", encoded}};
const std::map<std::string, std::string> expected = {{"auth", data}};
std::map<std::string, std::string> expected = {{"auth", data}};
Http::TestResponseHeaderMapImpl empty_headers;

EXPECT_CALL(encoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
Expand Down Expand Up @@ -381,8 +381,7 @@ TEST_F(HeaderToMetadataTest, MultipleHeadersMatch) {
{"X-PYTHON-VERSION", "3.7"},
{"X-IGNORE", "nothing"},
};
const std::map<std::string, std::string> expected = {{"version", "v4.0"},
{"python_version", "3.7"}};
std::map<std::string, std::string> expected = {{"version", "v4.0"}, {"python_version", "3.7"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand Down Expand Up @@ -429,7 +428,7 @@ TEST_F(HeaderToMetadataTest, IgnoreHeaderValueUseConstant) {
)EOF";
initializeFilter(response_config_yaml);
Http::TestResponseHeaderMapImpl incoming_headers{{"x-something", "thing"}};
const std::map<std::string, std::string> expected = {{"something", "else"}};
std::map<std::string, std::string> expected = {{"something", "else"}};
Http::TestResponseHeaderMapImpl empty_headers;

EXPECT_CALL(encoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
Expand Down Expand Up @@ -547,7 +546,7 @@ TEST_F(HeaderToMetadataTest, RegexSubstitution) {
// Match with additional path elements.
{
Http::TestRequestHeaderMapImpl headers{{":path", "/cluster-prod-001/x/y"}};
const std::map<std::string, std::string> expected = {{"cluster", "cluster-prod-001"}};
std::map<std::string, std::string> expected = {{"cluster", "cluster-prod-001"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand All @@ -557,7 +556,7 @@ TEST_F(HeaderToMetadataTest, RegexSubstitution) {
// Match with no additional path elements.
{
Http::TestRequestHeaderMapImpl headers{{":path", "/cluster-prod-001"}};
const std::map<std::string, std::string> expected = {{"cluster", "cluster-prod-001"}};
std::map<std::string, std::string> expected = {{"cluster", "cluster-prod-001"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand All @@ -567,7 +566,7 @@ TEST_F(HeaderToMetadataTest, RegexSubstitution) {
// No match.
{
Http::TestRequestHeaderMapImpl headers{{":path", "/foo"}};
const std::map<std::string, std::string> expected = {{"cluster", "/foo"}};
std::map<std::string, std::string> expected = {{"cluster", "/foo"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand All @@ -577,7 +576,7 @@ TEST_F(HeaderToMetadataTest, RegexSubstitution) {
// No match with additional path elements.
{
Http::TestRequestHeaderMapImpl headers{{":path", "/foo/bar?x=2"}};
const std::map<std::string, std::string> expected = {{"cluster", "/foo/bar?x=2"}};
std::map<std::string, std::string> expected = {{"cluster", "/foo/bar?x=2"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand Down Expand Up @@ -671,7 +670,7 @@ TEST_F(HeaderToMetadataTest, CookieValueUsed) {
)EOF";
initializeFilter(response_config_yaml);
Http::TestResponseHeaderMapImpl incoming_headers{{"cookie", "bar=foo"}};
const std::map<std::string, std::string> expected = {{"bar", "foo"}};
std::map<std::string, std::string> expected = {{"bar", "foo"}};

EXPECT_CALL(encoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_,
Expand All @@ -694,7 +693,7 @@ TEST_F(HeaderToMetadataTest, IgnoreCookieValueUseConstant) {
)EOF";
initializeFilter(response_config_yaml);
Http::TestResponseHeaderMapImpl incoming_headers{{"cookie", "meh=foo"}};
const std::map<std::string, std::string> expected = {{"meh", "some_value"}};
std::map<std::string, std::string> expected = {{"meh", "some_value"}};

EXPECT_CALL(encoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_,
Expand All @@ -717,7 +716,7 @@ TEST_F(HeaderToMetadataTest, NoCookieValue) {
)EOF";
initializeFilter(config);
Http::TestRequestHeaderMapImpl headers{{"cookie", ""}};
const std::map<std::string, std::string> expected = {{"foo", "some_value"}};
std::map<std::string, std::string> expected = {{"foo", "some_value"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand Down Expand Up @@ -745,7 +744,7 @@ TEST_F(HeaderToMetadataTest, CookieRegexSubstitution) {
// match.
{
Http::TestRequestHeaderMapImpl headers{{"cookie", "foo=cluster-prod-001"}};
const std::map<std::string, std::string> expected = {{"cluster", "cluster-prod-001 matched"}};
std::map<std::string, std::string> expected = {{"cluster", "cluster-prod-001 matched"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand All @@ -755,7 +754,7 @@ TEST_F(HeaderToMetadataTest, CookieRegexSubstitution) {
// No match.
{
Http::TestRequestHeaderMapImpl headers{{"cookie", "foo=cluster"}};
const std::map<std::string, std::string> expected = {{"cluster", "cluster"}};
std::map<std::string, std::string> expected = {{"cluster", "cluster"}};

EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));
Expand Down
2 changes: 1 addition & 1 deletion test/extensions/filters/network/thrift_proxy/filters/header_to_metadata/config_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ TEST(HeaderToMetadataFilterConfigTest, OnHeaderMissingEmptyValue) {
type: STRING
)EOF";

testForbiddenConfig(yaml, "Cannot specify on_missing rule with empty value");
testForbiddenConfig(yaml, "Cannot specify on_missing rule without non-empty value");
}

} // namespace HeaderToMetadataFilter
Expand Down
18 changes: 9 additions & 9 deletions ...filters/network/thrift_proxy/filters/header_to_metadata/header_to_metadata_filter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ TEST_F(HeaderToMetadataTest, BasicRequestTest) {
key: version
)EOF";
initializeFilter(request_config_yaml);
const std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};
std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));

auto metadata = std::make_shared<Extensions::NetworkFilters::ThriftProxy::MessageMetadata>();
Expand All @@ -92,7 +92,7 @@ TEST_F(HeaderToMetadataTest, DefaultNamespaceTest) {
key: version
)EOF";
initializeFilter(request_config_yaml);
const std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};
std::map<std::string, std::string> expected = {{"version", "0xdeadbeef"}};
EXPECT_CALL(req_info_,
setDynamicMetadata("envoy.filters.thrift.header_to_metadata", MapEq(expected)));

Expand All @@ -113,7 +113,7 @@ TEST_F(HeaderToMetadataTest, ReplaceValueTest) {
value: world
)EOF";
initializeFilter(request_config_yaml);
const std::map<std::string, std::string> expected = {{"replace", "world"}};
std::map<std::string, std::string> expected = {{"replace", "world"}};
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));

auto metadata = std::make_shared<Extensions::NetworkFilters::ThriftProxy::MessageMetadata>();
Expand All @@ -137,7 +137,7 @@ TEST_F(HeaderToMetadataTest, SubstituteValueTest) {
substitution: "\\1"
)EOF";
initializeFilter(request_config_yaml);
const std::map<std::string, std::string> expected = {{"subbed", "world"}};
std::map<std::string, std::string> expected = {{"subbed", "world"}};
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));

auto metadata = std::make_shared<Extensions::NetworkFilters::ThriftProxy::MessageMetadata>();
Expand All @@ -161,7 +161,7 @@ TEST_F(HeaderToMetadataTest, NoMatchSubstituteValueTest) {
substitution: "\\1"
)EOF";
initializeFilter(request_config_yaml);
const std::map<std::string, std::string> expected = {{"subbed", "does not match"}};
std::map<std::string, std::string> expected = {{"subbed", "does not match"}};
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));

auto metadata = std::make_shared<Extensions::NetworkFilters::ThriftProxy::MessageMetadata>();
Expand Down Expand Up @@ -257,7 +257,7 @@ TEST_F(HeaderToMetadataTest, StringTypeInBase64UrlTest) {
)EOF";
initializeFilter(request_config_yaml);
std::string data = "Non-ascii-characters";
const std::map<std::string, std::string> expected = {{"base64_key", data}};
std::map<std::string, std::string> expected = {{"base64_key", data}};
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));

const auto encoded = Base64::encode(data.c_str(), data.size());
Expand Down Expand Up @@ -366,7 +366,7 @@ TEST_F(HeaderToMetadataTest, SetMissingValueTest) {
value: hi
)EOF";
initializeFilter(request_config_yaml);
const std::map<std::string, std::string> expected = {{"set", "hi"}};
std::map<std::string, std::string> expected = {{"set", "hi"}};
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));

auto metadata = std::make_shared<Extensions::NetworkFilters::ThriftProxy::MessageMetadata>();
Expand Down Expand Up @@ -413,7 +413,7 @@ TEST_F(HeaderToMetadataTest, RemoveHeaderTest) {
value: world
)EOF";
initializeFilter(request_config_yaml);
const std::map<std::string, std::string> expected = {{"remove", "hello"}, {"keep", "world"}};
std::map<std::string, std::string> expected = {{"remove", "hello"}, {"keep", "world"}};
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));

auto metadata = std::make_shared<Extensions::NetworkFilters::ThriftProxy::MessageMetadata>();
Expand Down Expand Up @@ -486,7 +486,7 @@ TEST_F(HeaderToMetadataTest, MultipleRulesTest) {
value: world
)EOF";
initializeFilter(request_config_yaml);
const std::map<std::string, std::string> expected = {{"set", "hello"}, {"replace", "world"}};
std::map<std::string, std::string> expected = {{"set", "hello"}, {"replace", "world"}};
EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected)));

auto metadata = std::make_shared<Extensions::NetworkFilters::ThriftProxy::MessageMetadata>();
Expand Down
Loading

0 comments on commit 91cf20b

Please sign in to comment.