|
| 1 | +<!-- |
| 2 | +**Note:** When your Enhancement Proposal (EP) is complete, all of these comment blocks should be removed. |
| 3 | +
|
| 4 | +This template is inspired by the Kubernetes Enhancement Proposal (KEP) template: https://github.com/kubernetes/enhancements/blob/master/keps/sig-architecture/0000-kep-process/README.md |
| 5 | +
|
| 6 | +To get started with this template: |
| 7 | +
|
| 8 | +- [ ] **Create an issue in kgateway-dev/kgateway** |
| 9 | +- [ ] **Make a copy of this template.** |
| 10 | + `EP-[ID]: [Feature/Enhancement Name]`, where `ID` is the issue number (with no |
| 11 | + leading-zero padding) assigned to your enhancement above. |
| 12 | +- [ ] **Fill out this file as best you can.** |
| 13 | + At minimum, you should fill in the "Summary" and "Motivation" sections. |
| 14 | +- [ ] **Create a PR for this EP.** |
| 15 | + Assign it to maintainers with relevant context. |
| 16 | +- [ ] **Merge early and iterate.** |
| 17 | + Avoid getting hung up on specific details and instead aim to get the goals of |
| 18 | + the EP clarified and merged quickly. The best way to do this is to just |
| 19 | + start with the high-level sections and fill out details incrementally in |
| 20 | + subsequent PRs. |
| 21 | +
|
| 22 | +Just because a EP is merged does not mean it is complete or approved. Any EP |
| 23 | +marked as `provisional` is a working document and subject to change. You can |
| 24 | +denote sections that are under active debate as follows: |
| 25 | +
|
| 26 | +``` |
| 27 | +<<[UNRESOLVED optional short context or usernames ]>> |
| 28 | +Stuff that is being argued. |
| 29 | +<<[/UNRESOLVED]>> |
| 30 | +``` |
| 31 | +
|
| 32 | +When editing EPS, aim for tightly-scoped, single-topic PRs to keep discussions |
| 33 | +focused. If you disagree with what is already in a document, open a new PR |
| 34 | +with suggested changes. |
| 35 | +
|
| 36 | +One EP corresponds to one "feature" or "enhancement" for its whole lifecycle. Once a feature has become |
| 37 | +"implemented", major changes should get new EPs. |
| 38 | +--> |
| 39 | +# EP-10574: Basic Transformations for Request and Response |
| 40 | + |
| 41 | + |
| 42 | +* Issue: [#10574](https://github.com/kgateway-dev/kgateway/issues/10574) |
| 43 | + |
| 44 | +## Background |
| 45 | +The previous project (gloo) had a pretty widely used staged transformation concept which could be slotted into both request and response paths. This Transformation filter most critically was used to perform simple modification with inja templates and to extract data for usage in access logging. Because some applications of transformation can be rather heavy (rewrite whole responses perform regex lookups) this was built as a custom filter and packaged with a seperate repository called envoy-gloo to make sure that it is secure and performant. |
| 46 | + |
| 47 | + |
| 48 | + |
| 49 | +## Motivation |
| 50 | +Being able to peform conditional header mutation and enrich access logs via dynamic metadata in a performant way is a great quality of life feature that the gloo had and we should support. |
| 51 | + |
| 52 | + |
| 53 | +### Goals |
| 54 | +* Allow for extraction of data from a header and place it in dynamic metadata or filter state such that it can be pulled into access logs |
| 55 | +* Allow for mutations that are based on the contents of the request / response |
| 56 | +* Have transformations support be stably defined and not require a network hop |
| 57 | +* Have an opinionated api to keep functionality be locked to transforming envoy constructs such as filter state, headers, body, trailers and metadata |
| 58 | +* Keep all kgateway implementation specifics in kgateway repository. |
| 59 | + |
| 60 | + |
| 61 | +### Non-Goals |
| 62 | +* Have full feature parity with gloo transformations (at first) |
| 63 | +* Keep the same user facing api as gloo |
| 64 | + |
| 65 | +## Implementation Details |
| 66 | +As part of this we should look at leveraging the quickly stabilizing [Dynamic Module support](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/dynamic_modules) to reduce overhead in the repository and grant us the potential to use raw envoy releases as the default standard for kgateway builds. |
| 67 | +An example of how we might start to do this can be found in [poc](https://github.com/kgateway-dev/kgateway/pull/10677). This would allow for side by side of old and new functionality to make sure that as users migrate to kgateway we can expose a single user facing api with the ability to filp the underlying engine on the fly. |
| 68 | + |
| 69 | +In order to drive this side by side comparison we can use a set of environment variables as we shore up any gaps in implementation. |
| 70 | +Lets consider the following situation where dynamic modules are enabled as (dual filters)[https://github.com/alyssawilk/envoy/blob/main/source/docs/upstream_filters.md] and we want to transition the ability to get host metadata. In this case we could add a "UPSTREAM_TRANSFORM_USE_LEGACY" variable, shift the default configuration for upstream filter configuration to use dynamic modules with the ability to revert via setting the variable to true. After a reasonable period and based on any feedback we remove the environment variable and the code path. |
| 71 | + |
| 72 | +Once we reach full parity and remove all temporary environment variables we could be entirely on upstream envoy and contain the entirety of kgateway functionality in the kgateway repository! |
| 73 | + |
| 74 | +At first we should start with mutating headers via rich inja templates and settinginformation in one of dynamic metadata or filter state so that we can enrich access logs. |
| 75 | + |
| 76 | +Given the above we should start with a minimal API where we expose Transformations as something like the following |
| 77 | + |
| 78 | +``` |
| 79 | +type TransformationPolicy struct { |
| 80 | + // +optional |
| 81 | + Request *Transform `json:"request,omitempty"` |
| 82 | + // +optional |
| 83 | + Response *Transform `json:"response,omitempty"` |
| 84 | +} |
| 85 | +
|
| 86 | +type Transform struct { |
| 87 | +
|
| 88 | + // +optional |
| 89 | + // +listType=map |
| 90 | + // +listMapKey=name |
| 91 | + // +kubebuilder:validation:MaxItems=16 |
| 92 | + Set []HeaderTransformation `json:"set,omitempty"` |
| 93 | +
|
| 94 | + // +optional |
| 95 | + // +listType=map |
| 96 | + // +listMapKey=name |
| 97 | + // +kubebuilder:validation:MaxItems=16 |
| 98 | + Add []HeaderTransformation `json:"add,omitempty"` |
| 99 | +
|
| 100 | + // +optional |
| 101 | + // +listType=set |
| 102 | + // +kubebuilder:validation:MaxItems=16 |
| 103 | + Remove []string `json:"remove,omitempty"` |
| 104 | +
|
| 105 | + // +optional |
| 106 | + // +listType=map |
| 107 | + // +listMapKey=name |
| 108 | + // +kubebuilder:validation:MaxItems=16 |
| 109 | + SetMetdata []MetadataTransformation `json:"set,omitempty"` |
| 110 | +
|
| 111 | +} |
| 112 | +
|
| 113 | +type HeaderTransformation struct { |
| 114 | +
|
| 115 | + // +required |
| 116 | + Name gwv1.HeaderName `json:"name,omitempty"` |
| 117 | + Value InjaTemplate `json:"value,omitempty"` |
| 118 | +} |
| 119 | +
|
| 120 | +type MetadataTransformation struct { |
| 121 | +
|
| 122 | + // +required |
| 123 | + Namespace string `json:"name,omitempty"` |
| 124 | + // +required |
| 125 | + Name string `json:"name,omitempty"` |
| 126 | + Value InjaTemplate `json:"value,omitempty"` |
| 127 | +} |
| 128 | +
|
| 129 | +``` |
| 130 | + |
| 131 | +We then can start building common tooling to enhance inja to support repeated use cases. |
| 132 | +For example the gloo supported the following inja enhancements: |
| 133 | + "substring" , "trim" , "base64_encode" , "base64url_encode" , "base64_decode" , "base64url_decode" , "replace_with_random" , "raw_string" , "header" , "request_header" , "extraction" , "body" , "dynamic_metadata" , "data_source" , "host_metadata" , "cluster_metadata" , "context" , "env" |
| 134 | + |
| 135 | +In order to properly implement enriching access logs with information based on headers we would minimally need the "header" extension. |
| 136 | + |
| 137 | +Given this the first addition here should contain the following functions: |
| 138 | +"base64_encode", "header", "replace_with_random" |
| 139 | + |
| 140 | +## Dependency |
| 141 | +A main differentiator of the classic transformation filter is its ability to make complex templated substitutions based on the current state of the request. |
| 142 | + |
| 143 | +A simple version of this is where an internal system returns incompatible or non-compliant status codes and could be achieved with the proposed api something like this. |
| 144 | + |
| 145 | +ResponseTransformation.Set["status": "{% if header(status) == \"418\" %} 405 {% else %} header(status) {% endif %}}] |
| 146 | + |
| 147 | +In order to power this we should rely on a mature inja implementation such as gloo's previous reliance on (pantor's inja)[https://github.com/solo-io/inja]. |
| 148 | +We could call out to the same library to maintain compatibility but it would make adding bindings much harder for contributors. |
| 149 | + |
| 150 | +In order to simplify contributions we should adopt a rust based inja implementation such as (minijinja)[https://docs.rs/minijinja/latest/minijinja/] |
| 151 | + |
| 152 | +As part of this investigation we tested advanced inja functionality that has been leveraged by users in gloo and made sure that it behaves the same. Testing was not exhaustive and identified that minijinja has parity as long as adjacent_loop_items was not disabled. |
| 153 | + |
| 154 | + |
| 155 | +### Plugin |
| 156 | +Given that transformations are tightly coupled with route based decisions this new transformation policy should be part of the existing route policy and be colocated in the code base with the existing routepolicy. |
| 157 | + |
| 158 | + |
| 159 | +### Test Plan |
| 160 | +Testing should include unit tests per Inja extension, end-to-end tests for basic inja functionality and a integration test with enriched access logs to prove out the two user stories. |
| 161 | + |
| 162 | + |
| 163 | +## Alternatives |
| 164 | +Port the existing Transformation from the old project and figure out how to migrate the custom envoy builds off of the legacy cloud builder run by Solo.io onto github actions. |
| 165 | +To be precise this is to mirror some version of the envoy-gloo repo to kgateway-dev and make it be able to build without Solo.io vendor lock. |
| 166 | + |
| 167 | + |
| 168 | + |
| 169 | +## Open Questions |
| 170 | +What is the timeline to removal of the current extended envoy image. |
| 171 | +How do we want to handle route level overrides. |
| 172 | + |
| 173 | + |
0 commit comments