Skip to content

aws: remove hang when signing a dynamic modules bootstrap extension callout during server init - #46930

Open
isker wants to merge 1 commit into
envoyproxy:mainfrom
isker:aws-sign-bootstrap-callouts
Open

aws: remove hang when signing a dynamic modules bootstrap extension callout during server init#46930
isker wants to merge 1 commit into
envoyproxy:mainfrom
isker:aws-sign-bootstrap-callouts

Conversation

@isker

@isker isker commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Commit Message:
aws: remove hang when signing a dynamic modules bootstrap extension callout during server init

It is not currently possible to make HTTP callouts that block server initialization from a dynamic modules bootstrap extension to a cluster with an upstream AWS signing filter on it that asynchronously resolves credentials. This is because
MetadataCredentialsProviderBase::setCredentialsToAllThreads does not notify signers that credentials are available until all worker threads have had their ThreadLocalCredentialsCache updated, but this operation is blocked on worker threads being started, which is itself blocked on the pending server initialization. A cycle!

Change setCredentialsToAllThreads to notify signers immediately after starting runOnAllThreads, which synchronously updates only the main thread, in addition to doing it after the completion of that operation across all threads. I believe this to be safe because that notification will itself only be handled in worker threads after the runOnAllThreads dispatch is handled.

However, one wrinkle with this strategy is that it's now possible for workers to observe tears between the per-thread credentials and the global "pending" flag. So, I've made the pending flag also be per-thread, next to the credentials. Updates that set that value to true are broadcast to all threads from the main thread - all such updates were already happening there, and now runOnAllThreads asserts as much.

Additional Description:
AI disclosure: this fix comes from a real situation I found trying to use Envoy, but it was investigated by, and the solution and the tests were written by, Claude. I do not have expertise in any of the systems involved (Envoy's threading model, Envoy's AWS credentials implementation, or... C++ in general), so I appreciate careful reviews.

Risk Level: Low
Testing: I've added an integration test covering the stated use case, and some more targeted tests in credentials_provider_test. All the added tests fail without the changes in source/.
Docs Changes: N/A
Release Notes: Yes.
Platform Specific Features: No.

@repokitteh-read-only

Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #46930 was opened by isker.

see: more, trace.

It is not currently possible to make HTTP callouts from a dynamic
modules bootstrap extension that block server initialization to a
cluster with an upstream AWS signing filter on it that asynchronously
resolves credentials. This is because
`MetadataCredentialsProviderBase::setCredentialsToAllThreads` does not
notify signers that credentials are available until all worker threads
have had their ThreadLocalCredentialsCache updated, but this operation
is blocked on worker threads being started, which is itself blocked on
the pending server initialization. A cycle!

Change setCredentialsToAllThreads to notify signers immediately after
starting `runOnAllThreads`, which synchronously updates only the main
thread, in addition to doing it after the completion of that operation
across all threads. I believe this to be safe because that notification
will itself only be handled in worker threads after the
`runOnAllThreads` dispatch is handled.

However, one wrinkle with this strategy is that it's now possible for
workers to observe tears between the per-thread credentials and the
global "pending" flag. So, I've made the pending flag also be
per-thread, next to the credentials. Updates that set that value to
`true` are broadcast to all threads from the main thread - all such
updates were already happening there, and now `runOnAllThreads` asserts
as much.

I've added an integration test covering the stated use case, some more
targeted tests in credentials_provider_test, and had to replace some
no-longer-valid tests on specific credentials providers with ones that I
think are equivalent in spirit. All the added tests fail without the
changes in `source/`.

AI disclosure: this fix comes from a real situation I found trying to
use Envoy, but it was investigated by, and the solution and the tests
were written by, Claude. I do not have expertise in any of the systems
involved (Envoy approach to propagating state across worker threads,
Envoy's AWS credentials implementation, or... C++ in general), so I
appreciate careful reviews.

Signed-off-by: Ian Kerins <git@isk.haus>
@isker
isker force-pushed the aws-sign-bootstrap-callouts branch from 3d10483 to c7068aa Compare August 25, 2026 03:46
@isker
isker marked this pull request as ready for review August 25, 2026 04:25
@isker

isker commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

cc @nbaws

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.

1 participant