Skip to content

aws: fix usage of reloadable_features.use_http_client_to_fetch_aws_credentials#31135

Merged
mattklein123 merged 4 commits intoenvoyproxy:mainfrom
suniltheta:fix_aws_credentials_ff
Dec 1, 2023
Merged

aws: fix usage of reloadable_features.use_http_client_to_fetch_aws_credentials#31135
mattklein123 merged 4 commits intoenvoyproxy:mainfrom
suniltheta:fix_aws_credentials_ff

Conversation

@suniltheta
Copy link
Copy Markdown
Contributor

Commit Message: aws: fix usage of reloadable_features.use_http_client_to_fetch_aws_credentials
Additional Description: This feature flag will be release in v1.29. In v1.30 we will flip this feature flag to be default enabled. Finally in v1.31 release this feature flag will be removed.
Risk Level:
Testing:
Docs Changes: Yes
Release Notes: Yes
Fixes: #31119

…aws_credentials

Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #31135 was opened by suniltheta.

see: more, trace.

@suniltheta
Copy link
Copy Markdown
Contributor Author

suniltheta commented Nov 30, 2023

/assign @JuniorHsu

@repokitteh-read-only
Copy link
Copy Markdown

🙀 Error while processing event:

evaluation error
error: function _rk_error error: path contains forbidden characters:
 Traceback (most recent call last):
  bootstrap:143: in <toplevel>
  bootstrap:135: in _main
  bootstrap:62: in _call
  bootstrap:15: in _call1
  github.com/repokitteh/modules/assign.star:18: in _assign
  github:395: in issue_check_assignee
  github:131: in call
Error: function _rk_error error: path contains forbidden characters
🐱

Caused by: a #31135 (comment) was created by @suniltheta.

see: more, trace.

Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Copy link
Copy Markdown
Contributor

@JuniorHsu JuniorHsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for fixing this. just one nit from me!

Comment thread docs/root/configuration/http/http_filters/_include/aws_credentials.rst Outdated
Sunil Narasimhamurthy added 2 commits November 30, 2023 22:47
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
@suniltheta
Copy link
Copy Markdown
Contributor Author

/retest

@mattklein123 mattklein123 merged commit 6d9a6e9 into envoyproxy:main Dec 1, 2023
jbohanon added a commit to solo-io/envoy-gloo that referenced this pull request Mar 6, 2024
soloio-bulldozer Bot pushed a commit to solo-io/envoy-gloo that referenced this pull request Mar 12, 2024
* bump envoy-fork, fix extensions

* fix FactoryContext from envoyproxy/envoy#31189

* more build fixes

* fix MockFactoryContext

* use xds pkg for unified matcher instead of udpa

* export the build options

* another ServerFactoryContext

* use typed_config for lambda filter test

* runtime flag...?

* runtime flag...as a string...?

* use nullopt instead of server factory context. why? not sure yet

* put the server factory context back and try a different runtime flag per envoyproxy/envoy#31135

* nuke le asan

* uhh

* use libcurl, patch out virtual call to ServerFactoryContext::clusterManager()

* forgot the patch d'oh

* ls on the out dirs

* undo commenting creds

* add a build dir

* derp

* what the heck, respect my vars bro

* WHY AREN'T WE USING THE RIGHT ENV VARS

* look at me. i am the captain now.

* shhh, it can't see you if you don't move

* well this is nasty but we're getting somewhere

* alt+f4

* asan

* lower jobs oom-killed

* try some more goofiness with the optref

* ci shenanigans

* server factory context as nullopt

* ci nonsense

* getting closer

* cleanup

* remove hard-coding override in initialize()

* bump to 1.29.2

* bump to merged

---------

Co-authored-by: Nathan Fudenberg <nathan.fudenberg@solo.io>
@alyssawilk
Copy link
Copy Markdown
Contributor

@suniltheta is there a plan to flip this true or would it make more sense to remove the flag?

@suniltheta
Copy link
Copy Markdown
Contributor Author

I will check with @nbaws if we can flip this to true for upcoming release. And remove it in a release following that.

@nbaws
Copy link
Copy Markdown
Contributor

nbaws commented Jun 6, 2024

I would prefer to keep this default false for one more release. #34138 was a fairly substantial change to async providers, whilst I'm comfortable it has fixed the original reported issues with this PR the fix has not had sufficient burn time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

misuse envoy.reloadable_features in aws credential fetching

5 participants