feat: add optional DynamoDB runner config storage - #5266
Conversation
afa76c2 to
eed4332
Compare
eed4332 to
661eb31
Compare
|
@wadherv interesting idea. Since you added |
There was a problem hiding this comment.
Pull request overview
Adds optional per-label DynamoDB storage for runner bootstrap and registration configuration while retaining SSM as the default backend.
Changes:
- Provisions configurable DynamoDB tables, alarms, autoscaling, encryption, and IAM policies.
- Integrates backend selection into control-plane Lambdas and runner startup scripts.
- Adds optional SSM housekeeper creation and multi-runner support.
Reviewed changes
Copilot reviewed 39 out of 42 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
variables.tf |
Exposes storage and housekeeper settings. |
outputs.tf |
Exposes selected backend and table. |
main.tf |
Passes storage and housekeeper configuration. |
CHANGELOG.md |
Removes the 7.10.2 entry. |
modules/runners/variables.tf |
Defines runner-module storage options. |
modules/runners/main.tf |
Injects storage settings into userdata. |
modules/runners/local.tf |
Derives backend keys and TTL values. |
modules/runners/dynamodb.tf |
Provisions tables, items, scaling, and alarms. |
modules/runners/runner-config.tf |
Makes SSM bootstrap parameters conditional. |
modules/runners/logging.tf |
Stores CloudWatch configuration by backend. |
modules/runners/scale-up.tf |
Configures DynamoDB for scale-up. |
modules/runners/pool.tf |
Passes storage configuration to pool. |
modules/runners/ssm-housekeeper.tf |
Adds conditional resource creation. |
modules/runners/policies-runner.tf |
Selects runner IAM policy by backend. |
modules/runners/outputs.tf |
Adds storage-related outputs. |
modules/runners/templates/start-runner.sh |
Adds Linux DynamoDB retrieval. |
modules/runners/templates/start-runner-osx.sh |
Adds macOS DynamoDB retrieval. |
modules/runners/templates/start-runner.ps1 |
Adds Windows DynamoDB retrieval. |
modules/runners/pool/variables.tf |
Extends pool configuration schema. |
modules/runners/pool/main.tf |
Configures pool Lambda and IAM. |
modules/runners/pool/policies/lambda-pool.json |
Scopes pool SSM access. |
modules/runners/policies/lambda-scale-up.json |
Scopes scale-up SSM access. |
modules/runners/policies/lambda-dynamodb-runner-config.json |
Adds Lambda DynamoDB permissions. |
modules/runners/policies/instance-dynamodb-runner-config-policy.json |
Adds runner DynamoDB permissions. |
modules/runners/policies/instance-cloudwatch-policy.json |
Makes SSM CloudWatch access conditional. |
modules/multi-runner/variables.tf |
Adds per-runner storage configuration. |
modules/multi-runner/runners.tf |
Passes storage and housekeeper settings. |
modules/multi-runner/outputs.tf |
Exposes per-runner storage details. |
lambdas/yarn.lock |
Locks new DynamoDB dependencies. |
lambdas/libs/runner-providers/core/index.ts |
Adds shared storage types. |
lambdas/functions/control-plane/package.json |
Adds DynamoDB SDK dependencies. |
lambdas/functions/control-plane/src/modules.d.ts |
Declares storage environment variables. |
lambdas/functions/control-plane/src/scale-runners/types.ts |
Re-exports storage types. |
lambdas/functions/control-plane/src/scale-runners/scale-up.ts |
Loads storage configuration. |
lambdas/functions/control-plane/src/scale-runners/runner-config-storage.ts |
Implements SSM and DynamoDB stores. |
lambdas/functions/control-plane/src/scale-runners/runner-config-storage.test.ts |
Tests DynamoDB storage behavior. |
lambdas/functions/control-plane/src/scale-runners/github-runner.ts |
Uses the selected storage backend. |
lambdas/functions/control-plane/src/pool/pool.ts |
Loads storage settings for pools. |
lambdas/functions/termination-watcher/src/types.d.ts |
Removes state-change event types. |
lambdas/functions/termination-watcher/src/termination-warning.ts |
Narrows termination event handling. |
lambdas/functions/termination-watcher/src/termination-warning.test.ts |
Removes state-change coverage. |
lambdas/functions/termination-watcher/src/lambda.ts |
Narrows interruption handler typing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const isSpotInterruption = event['detail-type'] === 'EC2 Spot Instance Interruption Warning'; | ||
| const metricName = isSpotInterruption && config.createSpotWarningMetric ? 'SpotInterruptionWarning' : undefined; | ||
| metricEvent(instance, event, metricName, logger); | ||
| metricEvent(instance, event, config.createSpotWarningMetric ? 'SpotInterruptionWarning' : undefined, logger); |
| runner_config_dynamodb_token_leading_keys = ( | ||
| var.runner_config_storage.dynamodb.token_key_prefix == null | ||
| ? ["$${ec2:SourceInstanceARN}"] | ||
| : ["${var.runner_config_storage.dynamodb.token_key_prefix}*"] |
| runner_config_dynamodb_ttl_seconds = coalesce( | ||
| var.runner_config_storage.dynamodb.token_ttl_seconds, | ||
| local.ssm_housekeeper.config.minimumDaysOld * 86400, | ||
| ) |
| validation { | ||
| condition = var.runner_config_storage.dynamodb.token_ttl_seconds == null ? true : var.runner_config_storage.dynamodb.token_ttl_seconds > 0 | ||
| error_message = "`runner_config_storage.dynamodb.token_ttl_seconds` must be null or greater than zero." | ||
| } |
| validation { | ||
| condition = var.runner_config_storage.dynamodb.client_max_attempts > 0 | ||
| error_message = "`runner_config_storage.dynamodb.client_max_attempts` must be greater than zero." | ||
| } | ||
|
|
||
| validation { | ||
| condition = contains(["standard", "adaptive"], var.runner_config_storage.dynamodb.client_retry_mode) | ||
| error_message = "`runner_config_storage.dynamodb.client_retry_mode` must be either `standard` or `adaptive`." | ||
| } | ||
|
|
||
| validation { | ||
| condition = var.runner_config_storage.dynamodb.client_http_max_sockets > 0 | ||
| error_message = "`runner_config_storage.dynamodb.client_http_max_sockets` must be greater than zero." | ||
| } | ||
|
|
||
| validation { | ||
| condition = var.runner_config_storage.dynamodb.client_http_keep_alive_msecs == null ? true : var.runner_config_storage.dynamodb.client_http_keep_alive_msecs >= 0 | ||
| error_message = "`runner_config_storage.dynamodb.client_http_keep_alive_msecs` must be null or greater than or equal to zero." | ||
| } |
| user_errors = { | ||
| alarm_name = "${var.prefix}-runner-config-dynamodb-user-errors" | ||
| alarm_description = "DynamoDB user errors on the runner config table." | ||
| metric_name = "UserErrors" |
|
+1 - had the same pain. I independently built the same approach (per-instance token/config in DynamoDB with a TTL, read-then-delete mirroring the SSM lifecycle) and it held up in a live run with no SSM token writes. |
## Description Introduces the provider-neutral `RunnerConfigStore` contract and moves the existing SSM implementation behind it. Scale-up and runner creation now write registration-token or JIT configuration through the store's `create` operation, while SSM-specific parameter paths, tags, encryption behavior, and environment parsing stay in the AWS adapter. The adapter preserves the existing stored values and metadata, exposes its maximum write throughput so callers can pace batches without hard-coding SSM limits, and accepts provider-neutral runner metadata that is translated into SSM tags. This establishes the storage boundary without adding another backend or changing Terraform resource addresses. ## Test Plan - Added/updated runner-config store and control-plane tests. - `git diff --check` passed. - Runtime Yarn tests could not be run locally because the repository's pinned Yarn launcher is unavailable in this environment; CI should provide the full test result. ## Related Issues - Related storage-backend work: #5266
guicaulada
left a comment
There was a problem hiding this comment.
LGTM!
Minor comment on the DynamoDB TTL, not sure if it's actually relevant here.
On the prefix, I noticed if we set a token_key_prefix it allows runners to read and delete any other token from other runners which is less safe from the instance-specific access we have at SSM.
| condition = var.runner_config_storage.dynamodb.token_key_prefix == null ? true : trimspace(var.runner_config_storage.dynamodb.token_key_prefix) != "" | ||
| error_message = "`runner_config_storage.dynamodb.token_key_prefix` must be null or non-empty." |
There was a problem hiding this comment.
A custom token prefix changes the IAM condition to <prefix>*, allowing any runner using this role to read or delete another runner’s token. Please retain instance-specific access. A safe interim fix is to reject custom prefixes:
| condition = var.runner_config_storage.dynamodb.token_key_prefix == null ? true : trimspace(var.runner_config_storage.dynamodb.token_key_prefix) != "" | |
| error_message = "`runner_config_storage.dynamodb.token_key_prefix` must be null or non-empty." | |
| condition = var.runner_config_storage.dynamodb.token_key_prefix == null | |
| error_message = "Custom token_key_prefix values are not supported because runner token access must remain instance-scoped." |
| --table-name "$runner_config_dynamodb_table_name" \ | ||
| --key "$key_json" \ | ||
| "$${consistent_read_args[@]}" \ | ||
| --projection-expression "#value" \ |
There was a problem hiding this comment.
DynamoDB does not delete a record as soon as its TTL timestamp passes. Until that background deletion happens, GetItem can still return it. This read requests only the token value, so the startup script cannot tell that the stored token record has expired and will try to use it.
Please check the configured TTL attribute before using a token, and treat expired records as unavailable. Apply the same check to the Linux, macOS, and Windows startup scripts.
One option is to consume the token with a conditional DeleteItem: require its expiry timestamp to be later than the current time and use ReturnValues: ALL_OLD to return the deleted record. This checks expiry and removes the token in one operation, so two consumers cannot both retrieve it successfully. Apply this only to short-lived token records; durable bootstrap configuration has no expiry.
## Description Introduces the provider-neutral `RunnerConfigStore` contract and moves the existing SSM implementation behind it. Scale-up and runner creation now write registration-token or JIT configuration through the store's `create` operation, while SSM-specific parameter paths, tags, encryption behavior, and environment parsing stay in the AWS adapter. The adapter preserves the existing stored values and metadata, exposes its maximum write throughput so callers can pace batches without hard-coding SSM limits, and accepts provider-neutral runner metadata that is translated into SSM tags. This establishes the storage boundary without adding another backend or changing Terraform resource addresses. ## Test Plan - Added/updated runner-config store and control-plane tests. - `git diff --check` passed. - Runtime Yarn tests could not be run locally because the repository's pinned Yarn launcher is unavailable in this environment; CI should provide the full test result. ## Related Issues - Related storage-backend work: #5266
## Description Extracts runner-group cache access into the provider-neutral `RunnerGroupCacheStore` capability and adds the AWS SSM adapter. The control plane now asks the store to read or create the cached runner-group ID instead of importing SSM directly; SSM-specific parameter paths, tags, and AWS calls remain inside the adapter. The adapter returns `undefined` only when the cache parameter is missing, propagates other AWS failures, preserves cache creation behavior and metadata, and rejects malformed cached IDs before they are used. Focused tests cover the not-found, AWS-error, invalid-ID, and tag paths. ## Test Plan - Added SSM group-cache tests for not-found, AWS error, invalid ID, and tag behavior. - `git diff --check` passed. - Runtime Yarn tests could not be run locally because the repository's pinned Yarn launcher is unavailable in this environment; CI should provide the full test result. ## Related Issues - Related storage-backend work: #5266
## Description Moves runner-config cleanup behind the provider-neutral `RunnerConfigHousekeeper` capability. The AWS SSM adapter loads the configured token path, paginates through its parameters, identifies entries older than the configured age, supports dry-run reporting, and deletes eligible parameters while isolating individual deletion failures. The control-plane Lambda now constructs and invokes the capability directly. The existing `ssmHousekeeper` handler export and the old `cleanSSMTokens` import path remain as direct deprecated compatibility aliases so current Terraform handler configuration and deployed integrations continue to work during migration. ## Test Plan - Added/updated housekeeping and Lambda compatibility tests. - `git diff --check` passed. - Runtime Yarn tests could not be run locally because the repository's pinned Yarn launcher is unavailable in this environment; CI should provide the full test result. ## Related Issues - Related storage-backend work: #5266
## Description Extract `GitHubAppCredentialsStore` and keep GitHub App credential reads independently backed by SSM. Batched reads, decryption, multi-app parsing, authentication, and rate-limit attribution now use the credentials capability without coupling it to runner-config storage. ## Test Plan - Added/updated SSM credential, authentication, and rate-limit tests. - `git diff --check` passed. - Runtime Yarn tests could not be run locally because the repository's pinned Yarn launcher is unavailable in this environment; CI should provide the full test result. ## Related Issues - Related storage-backend work: #5266
## Description Adds the provider-neutral runner-config consumer and its AWS SSM implementation. The consumer polls the configured per-runner token path until a one-time JIT or registration configuration is available, applies bounded per-call and overall timeouts, and supports cancellation during polling and backoff. After reading a configuration it deletes the consumed parameter with bounded retries, classifies retryable provider failures, validates the storage context and environment-derived limits, and exposes safe failure behavior to compute providers. The old environment-based factory remains as a deprecated compatibility alias while callers migrate to `createRunnerConfigConsumer`. ## Test Plan - Added/updated consumer tests for polling, timeout, cancellation, retry, deletion, and error sanitization. - `git diff --check` passed. - Runtime Yarn tests could not be run locally because the repository's pinned Yarn launcher is unavailable in this environment; CI should provide the full test result. ## Related Issues - Related storage-backend work: #5266
## Description Compose the stable storage capabilities in one factory and add runner-config provider selection. Runner-config storage is selected independently from common GitHub App credentials, environment values are parsed once into immutable adapter configuration, and the old parallel provider factories are removed. The storage-provider package now declares its MIT license metadata. ## Test Plan - Added provider-resolution and storage-composition tests. - `git diff --check` passed. - Runtime Yarn tests could not be run locally because the repository's pinned Yarn launcher is unavailable in this environment; CI should provide the full test result. ## Related Issues - Related storage-backend work: #5266 --------- Co-authored-by: Guilherme Caulada <guilherme.caulada@gmail.com>
## Description Adds structured diagnostics at the asynchronous AWS SSM storage adapter boundaries. A shared logger factory attaches the canonical `aws_ssm` provider identity while each adapter keeps its own module name. Runner-config read and delete failures now preserve their original causes, retryable polling misses remain debug-level events, and terminal failures include bounded error-name chains. Credential values, private keys, Base64 contents, and stored runner configuration values are intentionally excluded from logs. Direct SSM consumer tests now pass only the SSM-specific configuration instead of the generic provider-selection field. This PR is stacked on #5359. ## Test Plan - Ran the storage-provider test suite: 9 files and 74 tests passed. - Ran the storage-provider TypeScript type-check. - Ran ESLint for `lambdas/libs/storage-providers`. - Ran Prettier check for storage-provider TypeScript files. - Ran `git diff --check` and a focused sensitive-log-field scan. ## Related Issues - Related storage-backend work: #5266
Summary
runner_config_storagesupport withssmas the default backend and optionaldynamodbstorage for label-scoped runner config.and CloudWatch alarms.
createflag for SSM housekeeping resources and align DynamoDB token TTL with SSM housekeeping retention.Testing
node_modules/.bin/vitest runnode_modules/.bin/tsc -p functions/control-plane/tsconfig.json --noEmitnode_modules/.bin/eslint ...node_modules/.bin/ncc build functions/control-plane/src/lambda.ts -o /private/tmp/control-plane-ncc-checkgit diff --check