wasm: add api for configuring environment variable in VmConfig #15136

mathetake · 2021-02-22T14:17:49Z

Commit Message: wasm: add api for configuring environment variables in VmConfig. This commit adds an API on VmConfig so that users can configure env vars to be injected to Wasm VMs. Resolves #14958

Signed-off-by: Takeshi Yoneda [email protected]

Signed-off-by: Takeshi Yoneda <[email protected]>

repokitteh-read-only · 2021-02-22T14:17:56Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
API shepherd assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #15136 was opened by mathetake.

see: more, trace.

htuch

Thanks. @PiotrSikora could you take a first pass from Wasm perspective?

api/envoy/extensions/wasm/v3/wasm.proto

htuch · 2021-02-23T03:02:48Z

api/envoy/extensions/wasm/v3/wasm.proto

+message EnvironmentVariables {
+  // The keys of *Envoy's* environment variables exposed to this VM. In other words, if a key exists in Envoy's environment
+  // variables, then that key-value pair will be injected. Note that if a key does not exist, it will be ignored.
+  repeated string host_env_keys = 1;


I wonder if we want some notion of env vars elsewhere in xDS as well, so we might want to factor this message to base types. @markdroth thoughts?

PiotrSikora

LGTM, with a note that it's not defined what happens if both host_env_keys and key_values define the same key. Should we reject such configurations?

mathetake · 2021-02-24T04:03:37Z

what happens if both host_env_keys and key_values define the same key. Should we reject such configurations?

Thanks, that's a good point. I think we should reject since that case may cause some unintended behavior in Wasm programs. WDYT?

PiotrSikora · 2021-02-24T05:08:32Z

Thanks, that's a good point. I think we should reject since that case may cause some unintended behavior in Wasm programs. WDYT?

SGTM.

Signed-off-by: Takeshi Yoneda <[email protected]>

mathetake · 2021-02-24T08:03:35Z

/retest

repokitteh-read-only · 2021-02-24T08:03:38Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15136 (comment) was created by @mathetake.

see: more, trace.

htuch

API LGTM modulo nits, do you want to add the implementation to this PR?
/wait

api/envoy/extensions/wasm/v3/wasm.proto

Signed-off-by: Takeshi Yoneda <[email protected]>

mathetake · 2021-02-25T07:54:38Z

do you want to add the implementation to this PR?

Will do.

Signed-off-by: Takeshi Yoneda <[email protected]>

repokitteh-read-only · 2021-03-03T09:07:41Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #15136 was synchronize by mathetake.

see: more, trace.

Signed-off-by: Takeshi Yoneda <[email protected]>

moderation · 2021-03-05T05:04:59Z

/lgtm deps

Signed-off-by: Takeshi Yoneda <[email protected]>

PiotrSikora

Thanks! @htuch does this work for you?

htuch

LGTM modulo a few small nits, ready to merge when fixed. Thanks!

source/extensions/common/wasm/plugin.cc

Signed-off-by: Takeshi Yoneda <[email protected]>

mathetake · 2021-03-06T01:37:54Z

@htuch Thanks for the review! PTAL 👍

source/extensions/common/wasm/plugin.cc

PiotrSikora · 2021-03-06T20:59:26Z

/retest

repokitteh-read-only · 2021-03-06T20:59:30Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15136 (comment) was created by @PiotrSikora.

see: more, trace.

htuch

LGTM, thanks!

htuch · 2021-03-08T03:40:54Z

/lgtm deps

Add environment_variables field in VmConfig

c389247

Signed-off-by: Takeshi Yoneda <[email protected]>

repokitteh-read-only bot added the api label Feb 22, 2021

repokitteh-read-only bot assigned markdroth Feb 22, 2021

htuch reviewed Feb 23, 2021

View reviewed changes

htuch assigned PiotrSikora Feb 23, 2021

PiotrSikora previously approved these changes Feb 23, 2021

View reviewed changes

review: fix typo

2077a4e

Signed-off-by: Takeshi Yoneda <[email protected]>

mathetake dismissed PiotrSikora’s stale review via 2077a4e February 24, 2021 05:23

PiotrSikora previously approved these changes Feb 24, 2021

View reviewed changes

mathetake requested a review from htuch February 24, 2021 12:48

htuch suggested changes Feb 25, 2021

View reviewed changes

api/envoy/extensions/wasm/v3/wasm.proto Outdated Show resolved Hide resolved

api/envoy/extensions/wasm/v3/wasm.proto Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Feb 25, 2021

Review: add warning and fix style

fda8ed6

Signed-off-by: Takeshi Yoneda <[email protected]>

mathetake dismissed PiotrSikora’s stale review via fda8ed6 February 25, 2021 05:29

repokitteh-read-only bot removed the waiting label Feb 25, 2021

mathetake added 2 commits February 25, 2021 05:41

Merge remote-tracking branch 'origin/main' into wasm-env-api

8bab14a

Write test

194471b

Signed-off-by: Takeshi Yoneda <[email protected]>

htuch added the waiting label Feb 26, 2021

mathetake added 2 commits March 3, 2021 00:06

Merge remote-tracking branch 'origin/main' into wasm-env-api

1fec667

Fix test and setenv for NullVm consistency

dd00186

Signed-off-by: Takeshi Yoneda <[email protected]>

mathetake requested a review from lizan as a code owner March 3, 2021 09:07

repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Mar 3, 2021

mathetake added 2 commits March 5, 2021 03:49

Merge remote-tracking branch 'origin/main' into wasm-env-api

b4e5d56

review: fix styles

3ed2309

Signed-off-by: Takeshi Yoneda <[email protected]>

mathetake dismissed PiotrSikora’s stale review via 3ed2309 March 5, 2021 04:02

repokitteh-read-only bot removed the waiting label Mar 5, 2021

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Mar 5, 2021

mathetake added 3 commits March 5, 2021 15:00

Relax the restriction on NullVm

435f69a

Signed-off-by: Takeshi Yoneda <[email protected]>

fix format

3d6dc03

Signed-off-by: Takeshi Yoneda <[email protected]>

fix format: part 2

78ac3f8

Signed-off-by: Takeshi Yoneda <[email protected]>

PiotrSikora previously approved these changes Mar 5, 2021

View reviewed changes

htuch suggested changes Mar 5, 2021

View reviewed changes

source/extensions/common/wasm/plugin.cc Outdated Show resolved Hide resolved

source/extensions/common/wasm/plugin.cc Outdated Show resolved Hide resolved

source/extensions/common/wasm/plugin.cc Outdated Show resolved Hide resolved

htuch added the waiting label Mar 5, 2021

mathetake added 2 commits March 6, 2021 10:30

review: fix styles

4e4ccf7

Signed-off-by: Takeshi Yoneda <[email protected]>

Merge remote-tracking branch 'origin/main' into wasm-env-api

84a4f5c

mathetake dismissed PiotrSikora’s stale review via 84a4f5c March 6, 2021 01:31

repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Mar 6, 2021

PiotrSikora suggested changes Mar 6, 2021

View reviewed changes

source/extensions/common/wasm/plugin.cc Show resolved Hide resolved

PiotrSikora approved these changes Mar 6, 2021

View reviewed changes

htuch approved these changes Mar 8, 2021

View reviewed changes

repokitteh-read-only bot removed the api label Mar 8, 2021

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Mar 8, 2021

htuch merged commit 789f8a0 into envoyproxy:main Mar 8, 2021

mathetake deleted the wasm-env-api branch March 16, 2021 08:07

htuch mentioned this pull request Sep 27, 2021

Securely provide password for private key/PFX via Environment Variable #18277

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wasm: add api for configuring environment variable in VmConfig #15136

wasm: add api for configuring environment variable in VmConfig #15136

mathetake commented Feb 22, 2021 •

edited

Loading

repokitteh-read-only bot commented Feb 22, 2021

htuch left a comment

htuch Feb 23, 2021

PiotrSikora left a comment

mathetake commented Feb 24, 2021

PiotrSikora commented Feb 24, 2021

mathetake commented Feb 24, 2021

repokitteh-read-only bot commented Feb 24, 2021

htuch left a comment

mathetake commented Feb 25, 2021

repokitteh-read-only bot commented Mar 3, 2021

moderation commented Mar 5, 2021

PiotrSikora left a comment

htuch left a comment

mathetake commented Mar 6, 2021

PiotrSikora commented Mar 6, 2021

repokitteh-read-only bot commented Mar 6, 2021

htuch left a comment

htuch commented Mar 8, 2021

wasm: add api for configuring environment variable in VmConfig #15136

wasm: add api for configuring environment variable in VmConfig #15136

Conversation

mathetake commented Feb 22, 2021 • edited Loading

repokitteh-read-only bot commented Feb 22, 2021

htuch left a comment

Choose a reason for hiding this comment

htuch Feb 23, 2021

Choose a reason for hiding this comment

PiotrSikora left a comment

Choose a reason for hiding this comment

mathetake commented Feb 24, 2021

PiotrSikora commented Feb 24, 2021

mathetake commented Feb 24, 2021

repokitteh-read-only bot commented Feb 24, 2021

htuch left a comment

Choose a reason for hiding this comment

mathetake commented Feb 25, 2021

repokitteh-read-only bot commented Mar 3, 2021

moderation commented Mar 5, 2021

PiotrSikora left a comment

Choose a reason for hiding this comment

htuch left a comment

Choose a reason for hiding this comment

mathetake commented Mar 6, 2021

PiotrSikora commented Mar 6, 2021

repokitteh-read-only bot commented Mar 6, 2021

htuch left a comment

Choose a reason for hiding this comment

htuch commented Mar 8, 2021

mathetake commented Feb 22, 2021 •

edited

Loading