feat(translator): allow configuration of hostEnvKeys on WASM extensions #4470

sgargan · 2024-10-17T22:36:52Z

exposes the hostEnvKeys configuration for WASM extensons through envoy extension policies. This enables access to env vars that are set on the host envoy processes and is a convenient way to share secret meterial with WASM extensions.

Release Note: Yes

daniel-white · 2024-10-18T19:00:53Z

this may solve my #4424

arkodg · 2024-10-18T19:25:25Z

internal/ir/xds.go

+
+	// HostEnvKeys is a list of keys for environment variables from the host envoy process
+	// that should be passed into the Wasm VM.
+	HostEnvKeys []string `json:"hostEnvKeys,omitempty"`


hostEnvKeys is pretty self explanatory, also brainstorming some more options

vmConfig.envKeys

env.Keys

envKeys

also linking some past work here from istio https://istio.io/latest/docs/reference/config/proxy_extensions/wasm-plugin/#VmConfig

+1 to adding a vmConfig wrapper for better future-proofing, as additional VM settings may be needed in the future.
Should we also consider supporting explicitly specified key-value pairs?

vmConfig: env: hostEnvKeys: - host_key_1 - host_key_2 customEnvs: - key: custom_key_1 value: custom_value_1 - key: custom_key_2 value: custom_value_2

hey @zhaohuabing what value does customEnvs add that cannot be represented via config ?

Yeah, customEnvs just gives users more flexibility to pass configurations into the Wasm module. It can be handy for the Wasm modules just need some simple key-value pairs as input parameters. It's not a necessity though.

hostEnvKeys is the name that envoy used for this and I figured it would make it easier for someone to join the dots from the envoy config if it matched here.

For a vmConfig wrapper, would this just contain the hostEnvKeys or the config block too? happy to add it, but is there value to the extra nesting?

yeah I'm a +1 for hostEnvKeys, it leaves room for customEnvs . The other way to frame this is env.HostKeys, which leaves room for env.Custom.

arkodg · 2024-10-18T19:28:52Z

hey @sgargan, the PR looks great ! thanks for adding docs and test cases

added one comment around naming, if we get consensus from @envoyproxy/gateway-maintainers @envoyproxy/gateway-reviewers on naming, we could try and get this into the v1.2.0-rc.1 release (late next week)

arkodg · 2024-10-18T21:30:42Z

make gen-check is failing, you can run make generate && make manifests locally and commit those changes

daniel-white · 2024-10-21T14:55:49Z

site/content/en/docs/tasks/extensibility/wasm.md

+
+This is especially useful for sharing secure data from environment vars on the envoy process set using [valueFrom](https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-environment-variables) a Kubernetes secret.
+
+Note that setting an env var on the envoy process requires a custom [EnvoyProxy](../../api/extension_types#envoyproxy) configuration.


it would be great to show an example here!

👍 I've added an example to the envoyproxy customization docs and referenced it here.

zhaohuabing · 2024-10-23T01:57:37Z

@sgargan After discussing with @arkodg and @guydc in today's EG meeting, it was suggested to consider using Kubernets Secrets for the CustomEnvs to securely pass sensitive data to the Wasm modules. Additionally, ConfigMap should be supported as a storage option for non-sensitive configurations.

As the next step, I suggest raising a PR for the API design, followed by a follow-up PR for the implementation, once the community reaches a consensus on the API.

A similar example can be found in the CustomResponseBody API:

gateway/api/v1alpha1/shared_types.go

Lines 706 to 729 in 7188dad

    
           type CustomResponseBody struct { 
        
           	// Type is the type of method to use to read the body value. 
        
           	// Valid values are Inline and ValueRef, default is Inline. 
        
           	// 
        
           	// +kubebuilder:default=Inline 
        
           	// +kubebuilder:validation:Enum=Inline;ValueRef 
        
           	// +unionDiscriminator 
        
           	Type *ResponseValueType `json:"type"` 
        
           	// Inline contains the value as an inline string. 
        
           	// 
        
           	// +optional 
        
           	Inline *string `json:"inline,omitempty"` 
        
           	// ValueRef contains the contents of the body 
        
           	// specified as a local object reference. 
        
           	// Only a reference to ConfigMap is supported. 
        
           	// 
        
           	// The value of key `response.body` in the ConfigMap will be used as the response body. 
        
           	// If the key is not found, the first value in the ConfigMap will be used. 
        
           	// 
        
           	// +optional 
        
           	ValueRef *gwapiv1.LocalObjectReference `json:"valueRef,omitempty"` 
        
           }

arkodg · 2024-10-24T03:33:20Z

@sgargan After discussing with @arkodg and @guydc in today's EG meeting, it was suggested to consider using Kubernets Secrets for the CustomEnvs to securely pass sensitive data to the Wasm modules. Additionally, ConfigMap should be supported as a storage option for non-sensitive configurations.

As the next step, I suggest raising a PR for the API design, followed by a follow-up PR for the implementation, once the community reaches a consensus on the API.

A similar example can be found in the CustomResponseBody API:

gateway/api/v1alpha1/shared_types.go

Lines 706 to 729 in 7188dad

type CustomResponseBody struct {

// Type is the type of method to use to read the body value.

// Valid values are Inline and ValueRef, default is Inline.

//

// +kubebuilder:default=Inline

// +kubebuilder:validation:Enum=Inline;ValueRef

// +unionDiscriminator

Type *ResponseValueType `json:"type"`

// Inline contains the value as an inline string.

//

// +optional

Inline *string `json:"inline,omitempty"`

// ValueRef contains the contents of the body

// specified as a local object reference.

// Only a reference to ConfigMap is supported.

//

// The value of key `response.body` in the ConfigMap will be used as the response body.

// If the key is not found, the first value in the ConfigMap will be used.

//

// +optional

ValueRef *gwapiv1.LocalObjectReference `json:"valueRef,omitempty"`

}

rethinking this one @guydc @zhaohuabing we can't allow the app dev who has access to the EEP to inject any env vars into the proxy, so we'll need to split up the task of injecting env vars (using EnvoyProxy resource by the platform admin) and allowing the app dev to use it by specifying this field

sgargan · 2024-10-24T16:08:45Z

rethinking this one @guydc @zhaohuabing we can't allow the app dev who has access to the EEP to inject any env vars into the proxy, so we'll need to split up the task of injecting env vars (using EnvoyProxy resource by the platform admin) and allowing the app dev to use it by specifying this field

there is definitely the danger that a dev with access to the EEP could exfil secrets that they shouldn't have access to if they could reason about what keys to use. This is mostly a problem if the EP is shared

Perhaps the EEP should only be able to use a secret from the same namespace that it explicitly calls out by name along with the key names to expose it to wasm? If the target Proxy is in a different namespace it might require duplicating the secret into that namespace with a reconciliation process to remove it when the referencing EEP is removed.

sgargan · 2024-10-24T16:13:42Z

As the next step, I suggest raising a PR for the API design, followed by a follow-up PR for the implementation, once the community reaches a consensus on the API.

Sure thing. Just so I'm clear on the process, are you suggesting a PR with just the wasm_types.go in it to capture the api conversation?

arkodg · 2024-10-24T20:11:20Z

hey @sgargan , @guydc and I discussed this issue again today in the community meeting and prefer if the API was env.HostKeys instead of hostEnvKeys .

It should be fine to make all the changes in this PR, we ideally prefer if the API and Implementation PR are separate to keep the PR sizes small and easier to review, but also the API discussion ( and churn) may cause a lot of work/change in the PR for the commiter

daniel-white · 2024-10-28T17:43:00Z

@arkodg i'm currently implementing a wasm plugin and i need to pass some sensitive values. is this likely going to land in 1.2?

arkodg · 2024-10-28T18:04:47Z

@arkodg i'm currently implementing a wasm plugin and i need to pass some sensitive values. is this likely going to land in 1.2?

this won't make it into v1.2, we just cut the RC release and can't add anymore API changes to this release, the workaround would be to use https://gateway.envoyproxy.io/docs/tasks/extensibility/envoy-patch-policy/

ChatGPT can be helpful here, path and name field will need to be rewritten/edited though

can you write a EnvoyPatchPolicy to set hostEnvKeys for wasm
ChatGPT said:

ChatGPT
To write an EnvoyPatchPolicy that sets the hostEnvKeys for a WebAssembly (Wasm) extension in Envoy Gateway, you would use the JSONPatch mechanism. The hostEnvKeys parameter is used to pass environment variables from the host to the Wasm runtime, enabling custom behavior based on external values.

Here's an example of how you can define the policy to add hostEnvKeys for a Wasm filter:

yaml
Copy code
apiVersion: gateway.envoyproxy.io/v1alpha1
kind: EnvoyPatchPolicy
metadata:
  name: wasm-env-keys-patch-policy
  namespace: default
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: eg
    namespace: default
  type: JSONPatch
  jsonPatches:
    - type: "type.googleapis.com/envoy.config.listener.v3.Listener"
      name: default/eg/http
      operation:
        op: add
        path: "/default_filter_chain/filters/0/typed_config/http_filters/0"
      value:
        name: "envoy.filters.http.wasm"
        typed_config:
          "@type": "type.googleapis.com/envoy.extensions.filters.http.wasm.v3.Wasm"
          config:
            vm_config:
              vm_id: "my_wasm_vm"
              runtime: "envoy.wasm.runtime.v8"
              code:
                local:
                  filename: "/etc/envoy/wasm/my-wasm-extension.wasm"
              environment_variables:
                key_values:
                  ENV_VAR_ONE: "value1"
                  ENV_VAR_TWO: "value2"
                host_env_keys:
                  - "HOST_ENV_VAR1"
                  - "HOST_ENV_VAR2"
Explanation:
The EnvoyPatchPolicy modifies the Envoy configuration using a JSONPatch to target the HTTP listener (default/eg/http) and insert a Wasm filter configuration.
The host_env_keys field within the vm_config block specifies which environment variables from the host should be passed to the Wasm VM. For example, HOST_ENV_VAR1 and HOST_ENV_VAR2 are included.
You can customize the paths and variable names as per your specific deployment.
Once this configuration is applied, your Wasm extension will have access to the specified host environment variables.

Make sure that the Wasm binary (my-wasm-extension.wasm) is correctly mounted in the expected location, and that the host environment variables (HOST_ENV_VAR1, HOST_ENV_VAR2) are set on the Envoy Gateway host.

This example gives you the flexibility to leverage host environment variables within your Wasm extensions, enabling custom logic based on external inputs

zhaohuabing · 2024-10-29T04:15:21Z

rethinking this one @guydc @zhaohuabing we can't allow the app dev who has access to the EEP to inject any env vars into the proxy, so we'll need to split up the task of injecting env vars (using EnvoyProxy resource by the platform admin) and allowing the app dev to use it by specifying this field

@arkodg Sorry, I didn't quite get the security concern behind this. Does this mean that we can still use a ValueRef to a local Secret, just like what EG does with the SecuritPolicy, but not an env from the host because the host envs are global scope and the app dev may not be allowed to access them?

arkodg · 2024-10-29T18:43:46Z

rethinking this one @guydc @zhaohuabing we can't allow the app dev who has access to the EEP to inject any env vars into the proxy, so we'll need to split up the task of injecting env vars (using EnvoyProxy resource by the platform admin) and allowing the app dev to use it by specifying this field

@arkodg Sorry, I didn't quite get the security concern behind this. Does this mean that we can still use a ValueRef to a local Secret, just like what EG does with the SecuritPolicy, but not an env from the host because the host envs are global scope and the app dev may not be allowed to access them?

yes, retrieving a value here requires inserting it to the host first (for hostEnvKeys, not customKeys), which means you could overwrite any existing val, that maybe critical for operation of the proxy or for another route

daniel-white · 2024-10-29T19:22:16Z

Thanks @arkodg - your sample got me started and i found the minimal patch required for those else interested. can't wait till this is a first class feature!

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: EnvoyPatchPolicy
metadata:
  name: patch-name
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: eg
  type: JSONPatch
  jsonPatches:
    - type: "type.googleapis.com/envoy.config.listener.v3.Listener"
      name: ns/gc/http
      operation:
        op: add
        path: "/default_filter_chain/filters/0/typed_config/http_filters/0/typed_config/config/vm_config/environment_variables"
        value:
          host_env_keys:
            - "ENV_VAR_1"
            - "ENV_VAR_2"

zhaohuabing · 2024-10-30T04:08:59Z

yes, retrieving a value here requires inserting it to the host first (for hostEnvKeys, not customKeys), which means you could overwrite any existing val, that maybe critical for operation of the proxy or for another route

Got it, since envs are used host wide, so it should be manged by the admin who has a global view to all variables. Thanks for the explaination!

sgargan · 2024-11-04T21:19:28Z

this won't make it into v1.2, we just cut the RC release and can't add anymore API changes to this release

Apologies for the delay, I was ooo last week and couldn't get to it. I've made the suggested changes to introduce an env grouping for the keys.

codecov · 2024-11-06T11:10:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.57%. Comparing base (36d0193) to head (ced3e03).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4470      +/-   ##
==========================================
+ Coverage   65.54%   65.57%   +0.02%     
==========================================
  Files         211      211              
  Lines       31954    31964      +10     
==========================================
+ Hits        20944    20960      +16     
+ Misses       9767     9764       -3     
+ Partials     1243     1240       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

api/v1alpha1/wasm_types.go

arkodg

LGTM thanks !

thanks for addressing all the comments

arkodg · 2024-11-08T18:23:38Z

hey @sgargan there's a lint error

tools/bin/yamllint --config-file=tools/linter/yamllint/.yamllint $(git ls-files :*.yml :*.yaml | xargs -L1 dirname | sort -u) 
./internal/xds/translator/testdata/in/xds-ir/wasm.yaml
  Error: 94:22 [new-line-at-end-of-file] no new line character at the end of file

exposes the hostEnvKeys configuration for WASM extensons through envoy extension policies. This enables access to env vars that are set on the host envoy processes and is a convenient way to share secret meterial with WASM extensions. Signed-off-by: Steve Gargan <steve.gargan@gmail.com>

sgargan · 2024-11-13T22:23:26Z

@arkodg I've updated the lint error, can this be merged?

sgargan requested a review from a team as a code owner October 17, 2024 22:36

sgargan force-pushed the main branch from 871a3f4 to 3ada056 Compare October 18, 2024 12:20

arkodg reviewed Oct 18, 2024

View reviewed changes

arkodg requested review from a team October 18, 2024 20:08

daniel-white reviewed Oct 21, 2024

View reviewed changes

sgargan force-pushed the main branch 3 times, most recently from ac9a5c9 to b2da559 Compare October 22, 2024 19:33

sgargan force-pushed the main branch 5 times, most recently from 9d954a4 to c532009 Compare November 4, 2024 16:24

sgargan force-pushed the main branch from c532009 to 50deb34 Compare November 5, 2024 10:24

sgargan force-pushed the main branch 3 times, most recently from fa1c0d1 to 0f26b83 Compare November 6, 2024 22:58

arkodg reviewed Nov 7, 2024

View reviewed changes

api/v1alpha1/wasm_types.go Outdated Show resolved Hide resolved

zhaohuabing reviewed Nov 7, 2024

View reviewed changes

api/v1alpha1/wasm_types.go Outdated Show resolved Hide resolved

sgargan force-pushed the main branch 3 times, most recently from e2c2a8b to dc28246 Compare November 7, 2024 10:08

arkodg previously approved these changes Nov 7, 2024

View reviewed changes

arkodg requested review from a team November 7, 2024 19:13

sgargan force-pushed the main branch from dc28246 to 9de9661 Compare November 8, 2024 08:32

sgargan dismissed arkodg’s stale review via db52ba3 November 11, 2024 18:01

sgargan force-pushed the main branch 2 times, most recently from db52ba3 to 05923ef Compare November 11, 2024 22:48

sgargan force-pushed the main branch from 05923ef to ced3e03 Compare November 13, 2024 22:22

arkodg approved these changes Nov 14, 2024

View reviewed changes

arkodg requested review from zhaohuabing and a team November 14, 2024 16:08

zhaohuabing approved these changes Nov 14, 2024

View reviewed changes

zhaohuabing merged commit c2b0ee3 into envoyproxy:main Nov 14, 2024
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(translator): allow configuration of hostEnvKeys on WASM extensions #4470

feat(translator): allow configuration of hostEnvKeys on WASM extensions #4470

sgargan commented Oct 17, 2024 •

edited by zhaohuabing

Loading

daniel-white commented Oct 18, 2024

arkodg Oct 18, 2024

arkodg Oct 18, 2024

zhaohuabing Oct 21, 2024 •

edited

Loading

arkodg Oct 22, 2024

zhaohuabing Oct 22, 2024 •

edited

Loading

sgargan Oct 22, 2024

arkodg Oct 22, 2024

arkodg commented Oct 18, 2024

arkodg commented Oct 18, 2024 •

edited

Loading

daniel-white Oct 21, 2024

sgargan Oct 22, 2024

zhaohuabing commented Oct 23, 2024 •

edited

Loading

arkodg commented Oct 24, 2024

sgargan commented Oct 24, 2024 •

edited

Loading

sgargan commented Oct 24, 2024

arkodg commented Oct 24, 2024

daniel-white commented Oct 28, 2024

arkodg commented Oct 28, 2024 •

edited

Loading

zhaohuabing commented Oct 29, 2024 •

edited

Loading

arkodg commented Oct 29, 2024

daniel-white commented Oct 29, 2024

zhaohuabing commented Oct 30, 2024 •

edited

Loading

sgargan commented Nov 4, 2024

codecov bot commented Nov 6, 2024 •

edited

Loading

arkodg left a comment

arkodg commented Nov 8, 2024

sgargan commented Nov 13, 2024


		This is especially useful for sharing secure data from environment vars on the envoy process set using [valueFrom](https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-environment-variables) a Kubernetes secret.

		Note that setting an env var on the envoy process requires a custom [EnvoyProxy](../../api/extension_types#envoyproxy) configuration.

feat(translator): allow configuration of hostEnvKeys on WASM extensions #4470

feat(translator): allow configuration of hostEnvKeys on WASM extensions #4470

Conversation

sgargan commented Oct 17, 2024 • edited by zhaohuabing Loading

daniel-white commented Oct 18, 2024

arkodg Oct 18, 2024

Choose a reason for hiding this comment

arkodg Oct 18, 2024

Choose a reason for hiding this comment

zhaohuabing Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

arkodg Oct 22, 2024

Choose a reason for hiding this comment

zhaohuabing Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

sgargan Oct 22, 2024

Choose a reason for hiding this comment

arkodg Oct 22, 2024

Choose a reason for hiding this comment

arkodg commented Oct 18, 2024

arkodg commented Oct 18, 2024 • edited Loading

daniel-white Oct 21, 2024

Choose a reason for hiding this comment

sgargan Oct 22, 2024

Choose a reason for hiding this comment

zhaohuabing commented Oct 23, 2024 • edited Loading

arkodg commented Oct 24, 2024

sgargan commented Oct 24, 2024 • edited Loading

sgargan commented Oct 24, 2024

arkodg commented Oct 24, 2024

daniel-white commented Oct 28, 2024

arkodg commented Oct 28, 2024 • edited Loading

zhaohuabing commented Oct 29, 2024 • edited Loading

arkodg commented Oct 29, 2024

daniel-white commented Oct 29, 2024

zhaohuabing commented Oct 30, 2024 • edited Loading

sgargan commented Nov 4, 2024

codecov bot commented Nov 6, 2024 • edited Loading

Codecov Report

arkodg left a comment

Choose a reason for hiding this comment

arkodg commented Nov 8, 2024

sgargan commented Nov 13, 2024

sgargan commented Oct 17, 2024 •

edited by zhaohuabing

Loading

zhaohuabing Oct 21, 2024 •

edited

Loading

zhaohuabing Oct 22, 2024 •

edited

Loading

arkodg commented Oct 18, 2024 •

edited

Loading

zhaohuabing commented Oct 23, 2024 •

edited

Loading

sgargan commented Oct 24, 2024 •

edited

Loading

arkodg commented Oct 28, 2024 •

edited

Loading

zhaohuabing commented Oct 29, 2024 •

edited

Loading

zhaohuabing commented Oct 30, 2024 •

edited

Loading

codecov bot commented Nov 6, 2024 •

edited

Loading