Add new MergeJsonKeys functionality to transformation filter.
#350
Conversation
source/extensions/filters/http/transformation/inja_transformer.cc
Outdated
Show resolved
Hide resolved
source/extensions/filters/http/transformation/inja_transformer.cc
Outdated
Show resolved
Hide resolved
| // prepare variables for non-advanced_templates_ scenario | ||
| absl::string_view name_to_split; | ||
| json* current = nullptr; | ||
| // if (!advanced_templates_) { |
There was a problem hiding this comment.
If we don't end up supporting json pointer templates, I think we should keep this check in and throw in the constructor at config time
There was a problem hiding this comment.
I need to understand better what would be entailed, I honestly don't really know the different 🙈
There was a problem hiding this comment.
It's largely just handling of / and ~
spec: https://datatracker.ietf.org/doc/html/rfc6901
our fork to re-introduce support in inja: solo-io/inja#6
There was a problem hiding this comment.
I'm now throwing an exception at config time
Co-authored-by: Jacob Bohanon <[email protected]>
api/envoy/config/filter/http/transformation/v2/transformation_filter.proto
Show resolved
Hide resolved
source/extensions/filters/http/transformation/inja_transformer.cc
Outdated
Show resolved
Hide resolved
test/extensions/filters/http/transformation/inja_transformer_test.cc
Outdated
Show resolved
Hide resolved
test/extensions/filters/http/transformation/inja_transformer_test.cc
Outdated
Show resolved
Hide resolved
…filter.proto Co-authored-by: Nathan Fudenberg <[email protected]>
source/extensions/filters/http/transformation/inja_transformer.cc
Outdated
Show resolved
Hide resolved
source/extensions/filters/http/transformation/inja_transformer.cc
Outdated
Show resolved
Hide resolved
source/extensions/filters/http/transformation/inja_transformer.cc
Outdated
Show resolved
Hide resolved
source/extensions/filters/http/transformation/inja_transformer.cc
Outdated
Show resolved
Hide resolved
source/extensions/filters/http/transformation/inja_transformer.cc
Outdated
Show resolved
Hide resolved
|
Issues linked to changelog: |
api/envoy/config/filter/http/transformation/v2/transformation_filter.proto
Outdated
Show resolved
Hide resolved
…filter.proto Co-authored-by: Nathan Fudenberg <[email protected]>
|
lgtm pending yuval check |
| 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; |
There was a problem hiding this comment.
please add tests for the dot logic. include edge cases to make sure we don't crash. i.e.
- "foo.bar"
- "foo..bar"
- ".foo."
- "..."
- ".foo
- "bar."
There was a problem hiding this comment.
Discussed offline, I am going to remove this ability for now as it's not necessary. In order to ensure we can potentially add it back later I will throw an exception at config_time if a "." is present in the key
Add a new feature to the
inja_transformercalledMergeJsonKeyswhich allows merging of rendered templates into the JSON body without needing to rewrite the entire body