Repository navigation
feat(scale-set): add scale set orchestration in Terraform - #5299
Conversation
Dependency ReviewThe following issues were found:
License Issueslambdas/services/scale-set/package.json
OpenSSF ScorecardScorecard details
Scanned Files
|
5eafe5c to
2b6bb21
Compare
c979cbb to
4d9e31c
Compare
b95c8c0 to
28c31de
Compare
5ab14e1 to
4375247
Compare
00c76ce to
01c4a78
Compare
4375247 to
de5869c
Compare
01c4a78 to
5b2fbf4
Compare
de5869c to
8ac821d
Compare
5b2fbf4 to
afc760b
Compare
8ac821d to
a3773f7
Compare
afc760b to
e17ae16
Compare
|
@npalm Done |
npalm
left a comment
There was a problem hiding this comment.
Solid foundation. The security posture stands out: container hardening in task.tf, tag-conditioned EC2 IAM in the compute provider, credential values kept out of Terraform entirely (only SSM parameter names threaded through), and workflow-level supply chain hygiene at release (sbom: true, provenance: mode=max, actions/attest, SHA-pinned actions, harden-runner). Nice work.
One thing to fix before merge
The task role in modules/orchestration-providers/scale-set/iam.tf grants only SSM read actions, but the controller writes discovered IDs back to SSM at credentials.ts:189 (installation ID) and reconciler.ts:261 (runner group ID) through parameterStore.put. First cache write will fail with AccessDenied at runtime. Either add a scoped ssm:PutParameter on those two ARNs or drop the write-back path. Details in the inline comment.
Docs missing from this PR
This change introduces a new orchestration mode, a new deployment shape (ECS Fargate controller alongside the existing Lambdas), a new grouping model, and a new set of experimental variables. The only doc touched is docs/security.md. Please add:
docs/index.mdanddocs/configuration.md: extend the architecture description and the configuration reference to cover the scale-set orchestration provider, the controller group model, and the new experimental variables.docs/multi-runner-v1-to-v2-configuration.mdanddocs/multi-runner-v1-v2-migration.md: add the scale-set option so v2 adopters can find it and understand when to pick it.- A new ADR under
docs/adr/capturing the "adopt existing scale sets by name, do not manage them" contract, alongsidedocs/adr/002-runner-orchestration-provider-boundary.md. modules/orchestration-providers/scale-set/README.mdandvariables.tf: make the security trade-off on the default0.0.0.0/0egress explicit (GitHub's outbound ranges athttps://api.github.com/metaare pinnable), so adopters make the call consciously.
The remaining inline comments are suggestions and small nudges. Happy to iterate on any of them.
|
Implemented the documentation requested in this review: |
Description
Consolidates the complete experimental GitHub Actions runner scale-set stack into one PR. It provides the Terraform orchestration, the ECS controller that consumes it, the EC2 compute-provider implementation, the example deployment, and the validation and delivery workflows needed to operate the stack.
Status and implementation basis
actions/scaleset, which provides the GitHub Actions Runner Scale Set API client and message-session primitives.Terraform and AWS orchestration
modules/orchestration-providers/scale-setmodule, which deploys one hardened ECS Fargate controller service per resolved controller group, with private networking, security groups, CloudWatch logging, health checks, deployment rollback, and task-definition safeguards.Scale-set controller and runtime
Example, CI, and integration coverage
examples/multi-runner-scale-setdeployment, provider locks, outputs, documentation, and the required multi-runner wiring.no-new-privileges, and no network access.Merged stack contributions
This PR now contains the following merged scale-set PRs:
Test Plan
tofu test, TypeScript type-check/build/format/lint/test targets, container smoke tests, and MiniStack lifecycle workflows cover the affected paths.git diff --checkand the repository CI workflows were run for the combined stack.Related Issues
actions/scaleset.