Use the TransformationPolicy API directly as rustformation config #12803

andy-fong · 2025-10-31T22:29:32Z

Description

The shape of the rustformation config was loosely following the classic transformation filter. From envoy's dynamic module config, it's actually a free form json string. There is no reason to have yet another message format. So, just serialize the yaml from the TransformationPolicy API and pass it down. This is kind of what we do for agentgateway (except in the body transformation, there is no "parseAs" field for agentgateway).

Change Type

/kind cleanup

Changelog

Use the TransformationPolicy API directly as rustformation config

Additional Notes

Signed-off-by: Andy Fong <[email protected]> fixed unit tests Signed-off-by: Andy Fong <[email protected]> mimic existing behavior for set header with empty value Signed-off-by: Andy Fong <[email protected]>

Implausiblyfun · 2025-10-31T23:51:32Z

I think it was 2 meetings ago or so but there was a statement that we kept our api too close to envoy because now we have multiple sub data planes. Given this I think @ashleywang1 may have opinions on our api structure and how instead of this iteration we should remove this sort of transformation rather than enabling it

andy-fong · 2025-11-03T14:58:12Z

cmd/envoyinit/Dockerfile


 # Update our deps to make cve toil lower
 # install wget for our default probes
-USER root


This was added in my last PR but turns out it's not needed, so removing this.

jenshu · 2025-11-03T18:35:58Z

test/e2e/features/transformation/suite.go

+					// in testdata/gateway-attached-transform.yaml,
+					//    for x-empty, the value is set to ""
+					//    for x-not-set, the value is not set
+					// The behavior for both is removing the existing header


does this mean that a header with empty string value is treated the same as the header not being present?

Technically, http spec allow empty value in a header. So, ideally, the "not set" case so act as remove (but why not use "remove") and the "empty" case should still set the header with empty value. However, the behavior of the C++ transformation filter is to remove the headers in both cases. So, just doing the same here.

npolshakova

Looks good! Just some minor questions for my own understanding.

npolshakova · 2025-11-06T21:19:19Z

internal/envoyinit/rustformations/src/http_simple_mutations.rs

How does this work for requestion transformations that get run on both on_request_headers and on_request_body? Does it only apply the transformation once?

The on_request_body is not implemented yet and will be next. Basically, if there is a body, we will ask envoy to buffer the entire body first because calling us with on_request_body and only do the transformation there and not in on_request_headers. That's kind of the behavior of the C++ filter (except it buffer the body bytes inside the filter instead of having envoy to do that) but it buffer regardless if we use the body or not. We cannot buffer the body ourselves here because the buffer_bytes limit is not exposed by the rust sdk and I will see if we can skip body buffering if not needed. Stay tuned.

npolshakova · 2025-11-06T21:21:25Z

internal/envoyinit/rustformations/src/http_simple_mutations.rs

-    request_headers_setter: Vec<(String, String)>,
-    #[serde(default)]
-    response_headers_setter: Vec<(String, String)>,
+impl Deref for FilterConfig {


Why do we need this? Can we just expose calling .transformations() on the FilterConfig?

correct, technically we don't need this. I don't remember what I was trying to do but end up not doing that. This can be removed later with some minor change as there are only a few places using this.

npolshakova · 2025-11-06T21:24:16Z

internal/envoyinit/transformations/src/jinja.rs

+        if value.is_empty() {
+            ops.remove_response_header(key);
+            continue;
+        }


Not related to your code- but I think template_from_str can return an error? Can we check that with match and then only do the render if it's not an error?

good catch. yeah, I should check all the unwrap() call. I think originally as a POC, we don't care much if it just crash. I will change this.

npolshakova · 2025-11-06T21:27:10Z

internal/envoyinit/rustformations/src/http_simple_mutations.rs

 }

+// Since PerRouteConfig is the same as the FilterConfig, for now just just a type alias
+pub type PerRouteConfig = FilterConfig;


Does FilterConfig have the same envoy error log when it fails to parse the input config?

Yes. Currently, the control-plane in kgateway actually never use FilterConfig and always use PerRouteConfig.

npolshakova · 2025-11-06T21:28:09Z

internal/envoyinit/transformations/src/lib.rs

 use serde::Deserialize;
-use serde_with::serde_as;
+use serde_json::Value;
 type Strng = String;


What was this alias for?

It's another thing originally, I was trying to mimic the config structure of agentgateway's transformation, in there, it uses ArcStr but I ended up not using that because it also require a fork from John's repo for some other lib to work with ArcStr. So, I just alias that to normal String here. It's really not necessary now and I probably will remove it.

npolshakova · 2025-11-06T21:30:14Z

internal/envoyinit/transformations/src/lib.rs

    pub add: Vec<(Strng, Strng)>,
    #[serde(default)]
-    #[serde_as(as = "serde_with::Map<_, _>")]
+    #[serde(deserialize_with = "deserialize_name_value")]


Is the Vec<(String, String)> just always a name and value pair? Can we just define that as a struct and then avoid using the custom deserializer and do something like:

#[derive(Deserialize, Clone)] struct NameValue {}

That's a very good point. I was mimicing agentgateway and was thinking if we have the same interface, we can pull code from there to here and vice versa but that's really not easily feasible, so no reason to do this.

npolshakova · 2025-11-06T21:30:39Z

internal/envoyinit/transformations/src/lib.rs

+#[derive(Default, Clone, Deserialize)]
+pub struct BodyTransform {
+    #[serde(default)]
+    pub parse_as: Strng,


Should parse_as be a string or an enum to match the kgateway api fields?

Oh... serde can map json string to enum, nice! I will do that.

npolshakova · 2025-11-06T21:33:33Z

internal/envoyinit/rustformations/src/http_simple_mutations.rs

+}
+
+impl TransformationOps for EnvoyTransformationOps<'_> {
+    fn set_request_header(&mut self, key: &str, value: &[u8]) -> bool {


Do these need to match some envoy style for the transformations? Or can we return an error here with the failure message if it doesn't apply?

This is the envoy rust sdk functions, so have to follow the same function signature. However, we can return error in the main transform_request_header() if this return false. I actually plan to do that because in the trasnformation crate, I intentionally not to have any envoy specific logic and dependency in there, so I don't have access to the envoy_log_*! macro neither. I plan to bubble up the error to the rustformation crate and use envoy_log_*! there.

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

Use the Transformation Policy API directly for rustformation config

8a41518

Signed-off-by: Andy Fong <[email protected]> fixed unit tests Signed-off-by: Andy Fong <[email protected]> mimic existing behavior for set header with empty value Signed-off-by: Andy Fong <[email protected]>

github-actions bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. release-note labels Oct 31, 2025

andy-fong commented Nov 3, 2025

View reviewed changes

jenshu reviewed Nov 3, 2025

View reviewed changes

npolshakova approved these changes Nov 6, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into rustformation-config-…

12658c7

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

Use the TransformationPolicy API directly as rustformation config #12803

Are you sure you want to change the base?

Use the TransformationPolicy API directly as rustformation config #12803

Uh oh!

Conversation

andy-fong commented Oct 31, 2025

Description

Change Type

Changelog

Additional Notes

Uh oh!

Implausiblyfun commented Oct 31, 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!

npolshakova 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants