diff --git a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto index 32de2ec2..62cac819 100644 --- a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto +++ b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto @@ -267,6 +267,13 @@ message TransformationTemplate { // If you want to nest elements inside the body, use dot separator in the // extractor name. MergeExtractorsToBody merge_extractors_to_body = 6; + // A set of key-value pairs to merge into the JSON body. + // Each value will be rendered separately, and then placed into the JSON body at + // the specified key. + // There are a number of important caveats to using this feature: + // * This can only be used when the body is parsed as JSON. + // * This option does NOT work with advanced templates currently + MergeJsonKeys merge_json_keys = 13; } // Determines how the body will be parsed. @@ -326,6 +333,33 @@ message Passthrough {} message MergeExtractorsToBody {} +message MergeJsonKeys { + message OverridableTemplate { + // Template to render + InjaTemplate tmpl = 1; + // If set to true, the template will be set even if the rendered value is empty. + bool override_empty = 2; + } + /* + Map of key name -> template to render into the JSON body + For example, given the following JSON body: + { + "key1": "value1" + } + and the following MergeJsonKeys: + { + "key1": "{{ header("header1") }}", + "key2": "{{ header("header2") }}" + } + The resulting JSON body will be: + { + "key1": "header1_value", + "key2": "header2_value" + } + */ + map json_keys = 2; +} + message HeaderBodyTransform { // When transforming a request, setting this to true will additionally add // "queryString", "queryStringParameters", "multiValueQueryStringParameters", diff --git a/changelog/v1.30.4-patch2/merge-json-keys.yaml b/changelog/v1.30.4-patch2/merge-json-keys.yaml new file mode 100644 index 00000000..bcd3a299 --- /dev/null +++ b/changelog/v1.30.4-patch2/merge-json-keys.yaml @@ -0,0 +1,6 @@ +changelog: +- type: NEW_FEATURE + issueLink: https://github.com/solo-io/envoy-gloo/issues/356 + resolvesIssue: false + description: >- + Create body transformation mechanism to modify keys rather than overwrite the whole body diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index 4b23a451..a3788d5d 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/source/extensions/filters/http/transformation/inja_transformer.cc @@ -236,13 +236,13 @@ TransformerInstance::TransformerInstance(ThreadLocal::Slot &tls, Envoy::Random:: env_.add_callback("clusterMetadata", 1, [this](Arguments &args) { return cluster_metadata_callback_deprecated(args); }); - env_.add_callback("cluster_metadata", 1, [this](Arguments &args) { + env_.add_callback("cluster_metadata", [this](Arguments &args) { return cluster_metadata_callback(args); }); - env_.add_callback("dynamic_metadata", 1, [this](Arguments &args) { + env_.add_callback("dynamic_metadata", [this](Arguments &args) { return dynamic_metadata_callback(args); }); - env_.add_callback("host_metadata", 1, [this](Arguments &args) { + env_.add_callback("host_metadata", [this](Arguments &args) { return host_metadata_callback(args); }); env_.add_callback("base64_encode", 1, [this](Arguments &args) { @@ -430,9 +430,18 @@ json TransformerInstance::cluster_metadata_callback(const inja::Arguments &args) json TransformerInstance::parse_metadata(const envoy::config::core::v3::Metadata* metadata, const inja::Arguments &args) { + // If no args are provided, return an empty string + if (args.size() == 0) { + return ""; + } + + // If the first arg is not a string it's technically an error, so return an empty string + if (!args.at(0)->is_string()) { + return ""; + } + const std::string &key = args.at(0)->get_ref(); // If a 2nd args is provided, use it as the filter const std::string &filter = args.size() > 1 ? args.at(1)->get_ref() : SoloHttpFilterNames::get().Transformation; - const std::string &key = args.at(0)->get_ref(); const ProtobufWkt::Value &value = Envoy::Config::Metadata::metadataValue( metadata, filter, key); @@ -772,6 +781,29 @@ InjaTransformer::InjaTransformer(const TransformationTemplate &transformation, merged_extractors_to_body_ = true; break; } + case TransformationTemplate::kMergeJsonKeys: { + if (transformation.parse_body_behavior() != TransformationTemplate::ParseAsJson) { + throw EnvoyException("MergeJsonKeys requires parsing the body"); + } else if (transformation.advanced_templates()) { + throw EnvoyException("MergeJsonKeys is not supported with advanced templates"); + } + for (const auto &[name, tmpl] : transformation.merge_json_keys().json_keys()) { + // Don't support "." in the key name for now. + // This is so that we can potentially bring back nested keys in the future + // if we need/want to. + if (name.find(".") != std::string::npos) { + throw EnvoyException( + fmt::format("Invalid key name for merge_json_keys: ({})", name)); + } + try { + merge_templates_.emplace_back(std::make_tuple(name, tmpl.override_empty(), instance_->parse(tmpl.tmpl().text()))); + } catch (const std::exception) { + throw EnvoyException( + fmt::format("Failed to parse merge_body_key template for key: ({})", name)); + } + } + break; + } case TransformationTemplate::kPassthrough: break; case TransformationTemplate::BODY_TRANSFORMATION_NOT_SET: { @@ -945,6 +977,20 @@ void InjaTransformer::transform(Http::RequestOrResponseHeaderMap &header_map, } else if (merged_extractors_to_body_) { std::string output = json_body.dump(); maybe_body.emplace(output); + } else if (!merge_templates_.empty()) { + + for (const auto &merge_template : merge_templates_) { + const std::string &name = std::get<0>(merge_template); + + const auto rendered = instance_->render(std::get<2>(merge_template)); + // Do not overwrite with empty unless specified + if (rendered.size() > 0 || std::get<1>(merge_template)) { + auto rendered_json = json::parse(rendered); + json_body[std::string(name)] = rendered_json; + } + } + std::string output = json_body.dump(); + maybe_body.emplace(output); } // DynamicMetadata transform: diff --git a/source/extensions/filters/http/transformation/inja_transformer.h b/source/extensions/filters/http/transformation/inja_transformer.h index b121e051..91ce8df9 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.h +++ b/source/extensions/filters/http/transformation/inja_transformer.h @@ -116,7 +116,7 @@ class Extractor : Logger::Loggable { const ExtractionApi::Mode mode_; }; -class InjaTransformer : public Transformer { +class InjaTransformer : public Transformer, Logger::Loggable { public: InjaTransformer(const envoy::api::v2::filter::http::TransformationTemplate &transformation, Envoy::Random::RandomGenerator &rng, @@ -154,6 +154,11 @@ class InjaTransformer : public Transformer { absl::optional body_template_; bool merged_extractors_to_body_{}; + // merged_templates_ is a vector of tuples with the following fields: + // 1. The json path to merge the template into + // 2. Whether to override the value at the json path if empty + // 3. The template to merge + std::vector> merge_templates_; ThreadLocal::SlotPtr tls_; std::unique_ptr instance_; }; diff --git a/test/extensions/filters/http/transformation/inja_transformer_test.cc b/test/extensions/filters/http/transformation/inja_transformer_test.cc index 54139443..9e73a334 100644 --- a/test/extensions/filters/http/transformation/inja_transformer_test.cc +++ b/test/extensions/filters/http/transformation/inja_transformer_test.cc @@ -759,6 +759,86 @@ TEST_F(InjaTransformerTest, UseBodyFunction) { EXPECT_EQ(body.toString(), "1 1"); } +TEST_F(InjaTransformerTest, MergeJsonKeys) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + + + // TransformationTemplate transformation; + // transformation.mutable_body()->set_text("{\"ext2\": \"{{header(\":path\")}\" }"); + + envoy::api::v2::filter::http::InjaTemplate inja_template; + inja_template.set_text("\"{{header(\":path\")}}\""); + envoy::api::v2::filter::http::MergeJsonKeys_OverridableTemplate tmpl; + (*tmpl.mutable_tmpl()) = inja_template; + (*transformation.mutable_merge_json_keys()->mutable_json_keys())["ext2"] = tmpl; + (*transformation.mutable_merge_json_keys()->mutable_json_keys())["ext1"] = tmpl; + + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + Buffer::OwnedImpl body("{\"ext1\":\"123\"}"); + transformer.transform(headers, &headers, body, callbacks); + EXPECT_EQ(body.toString(), "{\"ext1\":\"/foo\",\"ext2\":\"/foo\"}"); +} + +TEST_F(InjaTransformerTest, MergeJsonKeysNoOverrideEmpty ) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + + envoy::api::v2::filter::http::InjaTemplate inja_template; + // Should return "" and therefore not write the key at all + inja_template.set_text("{{header(\":status\")}}"); + envoy::api::v2::filter::http::MergeJsonKeys_OverridableTemplate tmpl; + (*tmpl.mutable_tmpl()) = inja_template; + (*transformation.mutable_merge_json_keys()->mutable_json_keys())["ext2"] = tmpl; + + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + Buffer::OwnedImpl body("{\"ext1\":\"123\"}"); + transformer.transform(headers, &headers, body, callbacks); + EXPECT_EQ(body.toString(), "{\"ext1\":\"123\"}"); +} + +TEST_F(InjaTransformerTest, MergeJsonKeysEmptyBody ) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + + envoy::api::v2::filter::http::InjaTemplate inja_template; + // Should return "" and therefore not write the key at all + inja_template.set_text("\"{{header(\":method\")}}\""); + envoy::api::v2::filter::http::MergeJsonKeys_OverridableTemplate tmpl; + (*tmpl.mutable_tmpl()) = inja_template; + (*transformation.mutable_merge_json_keys()->mutable_json_keys())["ext2"] = tmpl; + + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + Buffer::OwnedImpl body(""); + transformer.transform(headers, &headers, body, callbacks); + EXPECT_EQ(body.toString(), "{\"ext2\":\"GET\"}"); +} + +TEST_F(InjaTransformerTest, MergeJsonKeysEmptyJSONBody ) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + + envoy::api::v2::filter::http::InjaTemplate inja_template; + // Should return "" and therefore not write the key at all + inja_template.set_text("\"{{header(\":method\")}}\""); + envoy::api::v2::filter::http::MergeJsonKeys_OverridableTemplate tmpl; + (*tmpl.mutable_tmpl()) = inja_template; + (*transformation.mutable_merge_json_keys()->mutable_json_keys())["ext2"] = tmpl; + + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + Buffer::OwnedImpl body("{}"); + transformer.transform(headers, &headers, body, callbacks); + EXPECT_EQ(body.toString(), "{\"ext2\":\"GET\"}"); +} + TEST_F(InjaTransformerTest, UseDefaultNS) { Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; TransformationTemplate transformation;