feat(storage): add shared DynamoDB runner storage - #5279
Draft
edersonbrilhante wants to merge 3 commits into
Draft
feat(storage): add shared DynamoDB runner storage#5279edersonbrilhante wants to merge 3 commits into
edersonbrilhante wants to merge 3 commits into
Conversation
Contributor
Dependency ReviewThe following issues were found:
License Issueslambdas/functions/webhook/package.json
OpenSSF ScorecardScorecard details
Scanned Files
|
guicaulada
reviewed
Sep 8, 2026
guicaulada
left a comment
guicaulada
left a comment
Contributor
There was a problem hiding this comment.
Overall looks good!
One suggestion on scale-down related to orphan runners: it would be nice if we could retry cleanup. Could be a subject for a follow-up PR.
Another point is related to KMS, I don't think the Lambda and runner roles have access to KMS to read encrypted tables.
Feel free to correct me if I'm missing something here!
| } | ||
| } else { | ||
| await unMarkOrphan(runner.id, computeProvider); | ||
| await unMarkOrphan(runner, computeProvider); |
Contributor
There was a problem hiding this comment.
A runner can get stuck if scale-down sets its state to terminating and then crashes before removing it from GitHub. On the next run, the runner is still registered in GitHub, so this branch calls unMarkOrphan. That function does not clear terminating, and normal scale-down skips runners in that state—even after the termination claim expires.
Please let a later scale-down run take over an expired termination claim and retry cleanup. Before removing the runner, check whether it is now busy so we do not interrupt a job.
I reproduced this with an online, idle runner whose termination claim was one hour old: scale-down never called beginTermination to retry cleanup.
Comment on lines
+178
to
+183
| Statement = [ | ||
| local.github_app_read_statement, | ||
| local.entry_runner_group_statements[entry_id], | ||
| local.runner_config_write_statements[entry_id], | ||
| local.runner_state_write_statements[entry_id], | ||
| ] |
Contributor
There was a problem hiding this comment.
The generated policies allow DynamoDB access but do not grant access to the configured table KMS keys. With a key policy that delegates permissions to IAM, the Lambda and runner roles can fail to read the encrypted tables.
Please include scoped KMS permissions for each table’s consumers and add policy tests using non-null custom key ARNs.
edersonbrilhante
force-pushed
the
feat-dynamodb-storage-provider
branch
from
September 8, 2026 21:15
0563c7c to
2c05467
Compare
edersonbrilhante
force-pushed
the
feat-dynamodb-storage-provider
branch
from
September 8, 2026 21:23
08afbd2 to
97d9141
Compare
edersonbrilhante
changed the base branch from
refactor-webhook-matcher-config-storage
to
main
September 8, 2026 21:39
edersonbrilhante
force-pushed
the
feat-dynamodb-storage-provider
branch
from
September 8, 2026 21:41
97d9141 to
1d9fa0e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds an opt-in
aws_dynamodbrunner-config storage provider for experimental multi-runner v2 deployments.<prefix>-configfor durable global and entry-scoped configuration, including GitHub App credentials, webhook secret, matcher config, runner bootstrap config, and runner-group cache.<prefix>-runner-statefor TTL-backed one-time runner configuration plus provider-neutral runner lifecycle/inventory records.scopeandidkeys to isolate global, runner-entry, lifecycle, and compute-resource records without creating a table per runner entry.aws_ssm. The termination watcher intentionally remains on SSM in this PR.DynamoDB selection requires direct GitHub App values; externally managed
*_ssmreferences continue using the SSM provider so Terraform does not copy those external SecureStrings into state.Test Plan
tsc --noEmit: control-plane, storage-providers, and webhook passed.terraform validate: storage provider, multi-runner, and webhook modules passed.terraform fmt -check -recursiveandgit diff --checkpassed.Related Issues
Credits to @wadherv ##5266