datasource: add environment variable support #18498

anrossi · 2021-10-07T09:38:33Z

Signed-off-by: Anthony Rossi [email protected]

Commit Message: Add support to DataSource to read data from environment variables.
Additional Description:
Risk Level: Low
Testing: Unit test for well-known environment variable, no environment variable, and empty environment variable.
Docs Changes: N/A
Release Notes: config: added environment_variable to the DataSource.
Platform Specific Features: N/A
Fixes #18277

Signed-off-by: Anthony Rossi <[email protected]>

repokitteh-read-only · 2021-10-07T09:38:39Z

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

🐱

Caused by: #18498 was opened by anrossi.

see: more, trace.

Signed-off-by: Anthony Rossi <[email protected]>

source/common/config/datasource.cc

davinci26

Thanks, overall looks good, small question regarding the error handling

davinci26 · 2021-10-07T17:59:05Z

rendered docs are at https://storage.googleapis.com/envoy-pr/77eb7d6/docs/version_history/current.html and they look good.

You can find them in Upload Docs to GCS step of the docs CI job

test/common/config/datasource_test.cc

htuch

/lgtm api

This is the same validation logic as envoy.extensions.matching.common_inputs.environment_variable.v3 FWIW.

docs/root/version_history/current.rst

htuch · 2021-10-08T04:45:25Z

/assign-from @envoyproxy/first-pass-reviewers

repokitteh-read-only · 2021-10-08T04:45:29Z

@envoyproxy/first-pass-reviewers assignee is @rojkov

🐱

Caused by: a #18498 (comment) was created by @htuch.

see: more, trace.

Signed-off-by: Anthony Rossi <[email protected]>

rojkov

Thank you! Added a couple of comments.

/wait

source/common/config/datasource.cc

test/common/config/datasource_test.cc

source/common/config/datasource.cc

Signed-off-by: Anthony Rossi <[email protected]>

rojkov

Thank you! This looks good, needs fixing the format check.

To test it you can run ./tools/code_format/check_format.py fix

api/envoy/config/core/v3/base.proto

source/common/config/datasource.cc

davinci26

LGTM, assuming the CI passes now and the remaining comments are addressed.

rojkov · 2021-10-12T06:30:52Z

/wait

Signed-off-by: Anthony Rossi <[email protected]>

davinci26

thanks

Signed-off-by: Anthony Rossi <[email protected]>

davinci26

see https://github.com/envoyproxy/envoy/blob/main/STYLE.md for formatting

edit: see also https://google.github.io/styleguide/cppguide.html

test/common/config/datasource_test.cc

Signed-off-by: Anthony Rossi <[email protected]>

rojkov

Thanks! LGTM

htuch

LGTM modulo two final nits.
/wait

test/common/config/datasource_test.cc

Signed-off-by: Anthony Rossi <[email protected]>

rojkov · 2021-10-18T07:47:55Z

Sorry, you need to merge the main branch again to fix the coverage check.

/wait

anrossi · 2021-10-18T19:04:05Z

It looks like there's some kind of outage preventing the CI from running on the merged changes.

davinci26 · 2021-10-18T22:47:40Z

/retest

repokitteh-read-only · 2021-10-18T22:47:44Z

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

🐱

Caused by: a #18498 (comment) was created by @davinci26.

see: more, trace.

htuch · 2021-10-19T04:01:01Z

/retest

repokitteh-read-only · 2021-10-19T04:01:05Z

Retrying Azure Pipelines:
Check envoy-presubmit didn't fail.

🐱

Caused by: a #18498 (comment) was created by @htuch.

see: more, trace.

htuch

LGTM, thanks!

htuch · 2021-10-19T04:05:40Z

Known flake on the CodeQL build, will force merge.

Addresses issue envoyproxy#18498 - likely shows up legacy changes required for c++20 Signed-off-by: William A Rowe Jr <[email protected]>

Add supprt for environment variables as a DataSource.

1d068b6

Signed-off-by: Anthony Rossi <[email protected]>

repokitteh-read-only bot added the api label Oct 7, 2021

repokitteh-read-only bot assigned htuch Oct 7, 2021

Fix formatting of changelog.

c4001d5

Signed-off-by: Anthony Rossi <[email protected]>

davinci26 reviewed Oct 7, 2021

View reviewed changes

source/common/config/datasource.cc Outdated Show resolved Hide resolved

davinci26 reviewed Oct 7, 2021

View reviewed changes

test/common/config/datasource_test.cc Outdated Show resolved Hide resolved

davinci26 self-assigned this Oct 7, 2021

htuch reviewed Oct 8, 2021

View reviewed changes

docs/root/version_history/current.rst Outdated Show resolved Hide resolved

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

repokitteh-read-only bot assigned rojkov Oct 8, 2021

Throw on non-existent environment variable.

d3a898f

Signed-off-by: Anthony Rossi <[email protected]>

rojkov suggested changes Oct 8, 2021

View reviewed changes

source/common/config/datasource.cc Outdated Show resolved Hide resolved

test/common/config/datasource_test.cc Outdated Show resolved Hide resolved

source/common/config/datasource.cc Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Oct 8, 2021

Add test for empty environment variable.

9a7fb11

Signed-off-by: Anthony Rossi <[email protected]>

repokitteh-read-only bot removed the waiting label Oct 8, 2021

rojkov reviewed Oct 11, 2021

View reviewed changes

api/envoy/config/core/v3/base.proto Outdated Show resolved Hide resolved

source/common/config/datasource.cc Show resolved Hide resolved

davinci26 previously approved these changes Oct 11, 2021

View reviewed changes

repokitteh-read-only bot added the waiting label Oct 12, 2021

Fix formatting.

e64c06a

Signed-off-by: Anthony Rossi <[email protected]>

anrossi dismissed davinci26’s stale review via e64c06a October 12, 2021 20:13

repokitteh-read-only bot added api and removed waiting labels Oct 12, 2021

Merge branch 'main' into feature/env-var-datasource

26a5dc7

davinci26 previously approved these changes Oct 12, 2021

View reviewed changes

repokitteh-read-only bot added the waiting label Oct 14, 2021

Auto cleanup environment variable when leaving scope.

5a545aa

Signed-off-by: Anthony Rossi <[email protected]>

repokitteh-read-only bot removed the waiting label Oct 14, 2021

davinci26 suggested changes Oct 14, 2021

View reviewed changes

test/common/config/datasource_test.cc Outdated Show resolved Hide resolved

test/common/config/datasource_test.cc Outdated Show resolved Hide resolved

Fix style.

7d2d815

Signed-off-by: Anthony Rossi <[email protected]>

rojkov previously approved these changes Oct 15, 2021

View reviewed changes

htuch suggested changes Oct 15, 2021

View reviewed changes

test/common/config/datasource_test.cc Outdated Show resolved Hide resolved

test/common/config/datasource_test.cc Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Oct 15, 2021

anrossi added 3 commits October 16, 2021 03:13

Expect specific exceptions. Use existing RAII wrapper.

47c3c05

Signed-off-by: Anthony Rossi <[email protected]>

Merge branch 'main' into feature/env-var-datasource

bbe6283

Sort release notes.

af34acb

Signed-off-by: Anthony Rossi <[email protected]>

anrossi dismissed rojkov’s stale review via af34acb October 16, 2021 10:15

repokitteh-read-only bot removed the waiting label Oct 16, 2021

repokitteh-read-only bot added the waiting label Oct 18, 2021

Merge branch 'main' into feature/env-var-datasource

0d3b7dd

repokitteh-read-only bot removed the waiting label Oct 18, 2021

htuch approved these changes Oct 19, 2021

View reviewed changes

repokitteh-read-only bot removed the api label Oct 19, 2021

htuch merged commit 5d4a457 into envoyproxy:main Oct 19, 2021

anrossi deleted the feature/env-var-datasource branch October 19, 2021 07:03

wrowe mentioned this pull request May 24, 2022

Test PR - bump to c++20 for designated initializers #21422

Closed

wrowe added a commit to wrowe/envoy that referenced this pull request Aug 23, 2022

Test PR - bump to c++20 for designated initializers

ca39161

Addresses issue envoyproxy#18498 - likely shows up legacy changes required for c++20 Signed-off-by: William A Rowe Jr <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

datasource: add environment variable support #18498

datasource: add environment variable support #18498

anrossi commented Oct 7, 2021 •

edited

Loading

repokitteh-read-only bot commented Oct 7, 2021

davinci26 left a comment

davinci26 commented Oct 7, 2021

htuch left a comment

htuch commented Oct 8, 2021

repokitteh-read-only bot commented Oct 8, 2021

rojkov left a comment

rojkov left a comment

davinci26 left a comment

rojkov commented Oct 12, 2021

davinci26 left a comment

davinci26 left a comment •

edited

Loading

rojkov left a comment

htuch left a comment

rojkov commented Oct 18, 2021

anrossi commented Oct 18, 2021

davinci26 commented Oct 18, 2021

repokitteh-read-only bot commented Oct 18, 2021

htuch commented Oct 19, 2021

repokitteh-read-only bot commented Oct 19, 2021

htuch left a comment

htuch commented Oct 19, 2021

datasource: add environment variable support #18498

datasource: add environment variable support #18498

Conversation

anrossi commented Oct 7, 2021 • edited Loading

repokitteh-read-only bot commented Oct 7, 2021

davinci26 left a comment

Choose a reason for hiding this comment

davinci26 commented Oct 7, 2021

htuch left a comment

Choose a reason for hiding this comment

htuch commented Oct 8, 2021

repokitteh-read-only bot commented Oct 8, 2021

rojkov left a comment

Choose a reason for hiding this comment

rojkov left a comment

Choose a reason for hiding this comment

davinci26 left a comment

Choose a reason for hiding this comment

rojkov commented Oct 12, 2021

davinci26 left a comment

Choose a reason for hiding this comment

davinci26 left a comment • edited Loading

Choose a reason for hiding this comment

rojkov left a comment

Choose a reason for hiding this comment

htuch left a comment

Choose a reason for hiding this comment

rojkov commented Oct 18, 2021

anrossi commented Oct 18, 2021

davinci26 commented Oct 18, 2021

repokitteh-read-only bot commented Oct 18, 2021

htuch commented Oct 19, 2021

repokitteh-read-only bot commented Oct 19, 2021

htuch left a comment

Choose a reason for hiding this comment

htuch commented Oct 19, 2021

anrossi commented Oct 7, 2021 •

edited

Loading

davinci26 left a comment •

edited

Loading