Add new MergeJsonKeys functionality to transformation filter.

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<string, OverridableTemplate> json_keys = 2;
+}
+
 message HeaderBodyTransform {
   // When transforming a request, setting this to true will additionally add
   // "queryString", "queryStringParameters", "multiValueQueryStringParameters",

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

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<const std::string &>();
   // If a 2nd args is provided, use it as the filter
   const std::string &filter = args.size() > 1 ? args.at(1)->get_ref<const std::string &>() : SoloHttpFilterNames::get().Transformation;
-  const std::string &key = args.at(0)->get_ref<const std::string &>();
 
   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:

source/extensions/filters/http/transformation/inja_transformer.h

@@ -116,7 +116,7 @@ class Extractor : Logger::Loggable<Logger::Id::filter> {
   const ExtractionApi::Mode mode_;
 };
 
-class InjaTransformer : public Transformer {
+class InjaTransformer : public Transformer, Logger::Loggable<Logger::Id::filter> {
 public:
   InjaTransformer(const envoy::api::v2::filter::http::TransformationTemplate &transformation,
                   Envoy::Random::RandomGenerator &rng,
@@ -154,6 +154,11 @@ class InjaTransformer : public Transformer {
 
   absl::optional<inja::Template> 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<std::tuple<std::string, bool, inja::Template>> merge_templates_;
   ThreadLocal::SlotPtr tls_;
   std::unique_ptr<TransformerInstance> instance_;
 };

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<Http::MockStreamDecoderFilterCallbacks> 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<Http::MockStreamDecoderFilterCallbacks> 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<Http::MockStreamDecoderFilterCallbacks> 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<Http::MockStreamDecoderFilterCallbacks> 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;