[rustformation] support parsing body as json or string #12950

andy-fong · 2025-11-21T16:11:53Z

Description

This is the continuing effort of implementing all the documented feature in rustformation so we can remove the dependency of envoy-gloo.

added all the jinja custom functions documented here
have e2e tests covering all the custom functions and make sure the behavior and outputs are the same between the rustformation and classica transformation
switch to go-httpbin for transformation e2e tests because I need to verify the request
modify the test to cache the response body and restore it when returning the http.Response object
create jinja env only once per process lifetime as it's immutable
check globals for undeclared_variables to mimic classic transformation behavior for returning 400 response when body is not parsed as json but attempt to use undeclared json variables.

Change Type

/kind feature

Changelog

[rustformation] support parsing body as json and implemented all documented jinja custom functions

Additional Notes

There are some differences between minijinja in rust vs the inja lib we use in C++:

	C++	Rust	Note
White Spaces	Leave all white spaces alone	Right trim whitespace at the end	This is by observation during testing. There might be some options in minijina that can change this behavior but have not dug too deep yet.
replace_with_random	Each random pattern is generated once per request for a specific string to replace. For example, replace_with_random(“abc”, “a”) and replace_with_random(“cba”, “a”) will replace “a” with the same pattern within the same request context	Each random pattern is generated per function call. So, with the same example, “a” will be replaced with different random pattern	I spent a little time digging (between rust and minijinja). I cannot figure out a way to do this because unlike C++, I cannot just capture the entire filter object and set the callback and use that in the custom function callback. In rust/minijinja, the custom function is setup globally at filter creation time and the “context” we can setup per request is read only, so the custom function cannot update the context for subsequent call to use
Default body parsing	Default to parse as Json as long as any transformation is enabled regardless of if you actually use the body or not	Default to parse as String. Our document is actually wrong. We need to either fix the doc or change the default in the code; however, I think default to parsing json will use extra resources for parsing the body and setting up the context even if the user never uses the body.	This is because of how the enum in the API is defined. In the old transformation API:`// Determines how the body will be parsed. enum RequestBodyParse { // Will attempt to parse the request/response body as JSON ParseAsJson = 0; // The request/response body will be treated as plain text DontParse = 1; } RequestBodyParse parse_body_behavior = 7;`ParseAsJson is 0, so that’s the default. But in the new TrafficPolicyAPI in kgateway:``` // +kubebuilder:default=AsString ParseAs BodyParseBehavior `json:"parseAs"````We set the default with kubebuilder directly to AsString
Body parsedAsJson accessor syntax	level_{%- if headers != "" -%}{{headers.X-Incoming-Stuff.0}}{% else %}unknown{% endif %}	level_{%- if headers != "" -%}{{headers["X-Incoming-Stuff"][0]}}{% else %}unknown{% endif %}"	There are 2 differences:the .0 notation to access item 0 is not supported in the rust minijinja, it needs to be [0] but worse, the C++ implementation ONLY supports .0 and fail the config validation if [0] is usedwhen the variable name contains - , the [] notation has to be used in minijinja. So, it needs to be headers["X-Incoming-Stuff"] but again, the C++ implementation doesn't support that.
Json field name and custom function name collision	C++ doesn’t have this problem.	When we have the body parseAsJson set, all the json fields are put into the template context so they can be accessed directly. However, if a field name is the same as one of our custom function names, the rust minijinja template parsing seems to prefer to resolve it to the json value. For example, we have a customer function context() but if the json body also has a field named context, the template rendering will resolve context() to the json value from the body and complain it’s not callable.	Might look into this further and possibly file an issue in upstream minijina to see if this is intentional behavior.
Body buffering	Has a passthrough setting in the Transformation API to avoid buffering the body and skip all parsing	No passthrough setting	By default, the behavior for both are the same. We have envoy to buffer the entire body regardless of any transformations actually using the body.
Adding Multiple Headers			The kgateway transformation API defines the list of header as a map. So, you cannot add the same header more than 1 times even though the backend supports it. This behavior is the same in kgateway for both the classic and rust transformation as this is an API limit but just want to point this out.

…nja custom functions Signed-off-by: Andy Fong <[email protected]> - switch to go-httpbin for transformation e2e tests - modify the test to cache the response body and restore it when returning the http.Response object - create jinja env once; check globals for undeclared_variables Signed-off-by: Andy Fong <[email protected]>

andy-fong · 2025-11-21T16:16:43Z

Certain area definitely doesn't look DRY enough, especially the transform_request() and transform_response() function. Normally, I would create a common function and pass in the corresponding function pointers (in C++ term) to do the work. However, it's easy to do that in rust if you only have one function pointer in the struct that you need to pass. Passing multiple function pointers in the same struct causes some violation I was not able to solve easily because that would cause multiple mutatable reference of the struct to be created. While I know they won't be called at the same time, the rust compiler does not trust nobody for that. There should be a way but for now, I want to get this out first.

Signed-off-by: Andy Fong <[email protected]>

howardjohn · 2025-11-25T19:33:32Z

internal/envoyinit/rustformations/src/lib.rs

+TODO: look into enabling this to avoid accidental use of unwrap() and
+crash the process. However, there are many tests using unwrap() that
+will make the linter unhappy.
+#![deny(clippy::unwrap_used, clippy::expect_used)]


hmm, it seems like the linter should understand a test module vs non-test. Maybe people are normally explicitly allowing the test modules

Let me look into that but probably will do that in my next PR.

howardjohn · 2025-11-25T19:34:00Z

internal/envoyinit/rustformations/src/http_simple_mutations.rs

+        if buffers.is_empty() {
+            return Ok(JsonValue::Null);
+        }
+        // TODO: implement Reader for EnvoyBuffer and use serde_json::from_reader to avoid making copy first?


I'm pretty sure I saw this in some example repo

howardjohn · 2025-11-25T19:35:19Z

internal/envoyinit/rustformations/src/http_simple_mutations.rs

+                        envoy_filter.send_response(400, Vec::default(), None);
+                        return false;
+                    } else {
+                        envoy_log_warn!("{:#}", err);


Why do we not 400 on other error types?

in my early testing, the c++ transformation doesn't return error, so this is just trying to be the same behavior but I should add more negative test cases in a separate commit to be sure.

internal/envoyinit/transformations/src/jinja.rs

test/e2e/tests/base/base_suite.go

Signed-off-by: Andy Fong <[email protected]>

Implausiblyfun

Looks pretty cool havent tested in practice but it seems like a good step.

Ill be out of pocket soon but will check cncf slack if any questions come up. That being said it I think others are currently closer to this and definitely others have a better handle on rust so I defer to their wisdom in the meantime.

Implausiblyfun · 2025-11-25T21:30:06Z

internal/envoyinit/transformations/src/jinja.rs

+            for v in &undeclared_variables {
+                // Unfortunately, custom function is also reported as undeclared variables
+                // by minijinja, so only return error if the undeclared variables are not
+                // customer functions. GLOBALS_LOCKUP is lazily constructed once and is


Suggested change

// customer functions. GLOBALS_LOCKUP is lazily constructed once and is

// custom functions. GLOBALS_LOCKUP is lazily constructed once and is

I have another PR that will probably merge to this one because upstream introduced an API breakage on a point release. I will fix this in there.

Implausiblyfun · 2025-11-25T21:31:07Z

internal/envoyinit/transformations/src/jinja.rs

+                ops.append_request_body(rendered_body);
+            } else {
+                ops.set_request_header("content-length", b"0");
+                // In classic transformation, we remove content-type only when "passthrough_body"


Are we intending to change the c++ impl too to smooth transitions?

It's probably hard to modify the classic transformation behavior but I think this is very minor as there is no body bytes, should not cause any issue with or without content-type.

test/e2e/tests/base/base_suite.go

test/e2e/testutils/assertions/curl.go

nfuden

Looks pretty cool havent tested in practice but it seems like a good step.

Ill be out of pocket soon but will check cncf slack if any questions come up. That being said it I think others are currently closer to this and definitely others have a better handle on rust so I defer to their wisdom in the meantime.

Sorry about the confusion I reviewed first with my other account will update now

nfuden · 2025-11-25T21:39:39Z

test/e2e/tests/base/base_suite.go

 		return
 	}

+	fmt.Printf("testName: %s applying %v\n", testName, testCase)


https://github.com/kgateway-dev/kgateway/pull/12950/files#r2561182726

nfuden · 2025-11-25T21:39:47Z

test/e2e/testutils/assertions/curl.go

 		WithContext(ctx).
 		Should(Succeed(), "failed to get expected response")

+	if len(cachedBodyBytes) > 0 {


why is this only in this case? is there a version where we have this in a defer or something early?

no, just if the body is empty to begin with, no need to create an NopCloser.

andy-fong · 2025-11-26T21:18:25Z

Sorry about the confusion I reviewed first with my other account will update now

No problem. Appreciate taking a look. Have a Great Thanksgiving!

gateway-bot added kind/feature Categorizes issue or PR as related to a new feature. release-note labels Nov 21, 2025

andy-fong added 3 commits November 21, 2025 11:20

remove deny unwrap() and expect() linting for now

cfe7ca7

Signed-off-by: Andy Fong <[email protected]>

fixed linting error on transformation test suit

807671c

Signed-off-by: Andy Fong <[email protected]>

fixed comment

bae31de

Signed-off-by: Andy Fong <[email protected]>

andy-fong requested a review from nfuden November 25, 2025 17:27

howardjohn reviewed Nov 25, 2025

View reviewed changes

addressed review comment

e74a135

Signed-off-by: Andy Fong <[email protected]>

Implausiblyfun reviewed Nov 25, 2025

View reviewed changes

nfuden reviewed Nov 25, 2025

View reviewed changes

	// customer functions. GLOBALS_LOCKUP is lazily constructed once and is
	// custom functions. GLOBALS_LOCKUP is lazily constructed once and is

[rustformation] support parsing body as json or string #12950

Are you sure you want to change the base?

[rustformation] support parsing body as json or string #12950

Uh oh!

Conversation

andy-fong commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Change Type

Changelog

Additional Notes

Uh oh!

andy-fong commented Nov 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andy-fong Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Implausiblyfun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nfuden left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andy-fong commented Nov 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andy-fong commented Nov 21, 2025 •

edited

Loading

andy-fong Nov 25, 2025 •

edited

Loading