Repository navigation
feat(ci): two-region public AMI distribution + latest-2 retention (#83, #49) - #84
Merged
Merged
Conversation
Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
#83) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
…i.prune (#83) Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
#49) Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
…es, drop dead packer var (#83) - Remove ec2:DescribeSnapshots from IAM policy doc (script never calls it; snapshot IDs are extracted from describe-images projection only) - Remove stale shellcheck disable=SC2034 on QUOTA_CODE (var is actively used in effective_quota; SC2034 does not fire without the suppress) - Expand shellcheck CI step to also lint infra/scripts/test/cases/*.sh - Drop dead variable ami_distribution_regions from variables.pkr.hcl (distribution is driven by the workflow copy step and ami_regions; packer validate still passes after removal) - Add one-line assumption comment above the ensure-free count (dedicated build account, no unrelated public AMIs share the quota) Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
github-actions
Bot
added
documentation
OpenSSF Scorecard — 8.3/10 ✅
|
|
Warning Review limit reached
More reviews will be available in 8 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR implements automatic cleanup of stale public AMIs and restricts distribution to us-east-1 and us-east-2 to reduce costs and prevent quota exhaustion. The core addition is a new ChangesAMI distribution and quota management
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…stricter than local) (#83) Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
CLAUDE.md (1)
159-160:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale AMI distribution wording in Build & Test.
These lines still say AMIs are copied to “all distribution regions,” which now conflicts with the updated two-region model (
us-east-2source +us-east-1copy). Please align this section with the current behavior to avoid operator confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` around lines 159 - 160, Update the wording in the Build & Test examples for the make targets to reflect the current two-region AMI distribution model: change the phrase "all distribution regions" to explicitly state the two-region behavior (source: us-east-2, copy: us-east-1) for the make ami.build-distribute line, and clarify that make ami.copy AMI_ID=ami-xxx will copy an existing AMI from us-east-2 to us-east-1; keep the same make target names (make ami.build-distribute and make ami.copy) but update their descriptive text so operators understand the current two-region deployment.docs/ami-prebaked.md (2)
103-111:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDistribution section still documents deprecated 9-region behavior.
This block conflicts with Line 119 and the new retention policy: distribution is now only
us-east-2(source) +us-east-1copy. Please update the heading and region list accordingly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ami-prebaked.md` around lines 103 - 111, The "Build and distribute to all regions" section still lists the old 9-region distribution for the make target ami.build-distribute; update the heading and the region list under that block (the "Build and distribute to all regions" heading and the code block invoking make ami.build-distribute) to state that the target builds in us-east-2 and copies only to us-east-1 (source us-east-2 + us-east-1 copy) to match the new retention policy, and make any adjacent heading ("Copy an existing AMI to all regions") consistent with this reduced distribution.
258-266:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMulti-region cost estimate is outdated after two-region restriction.
The cost example still assumes 9 distribution regions, which no longer matches the current pipeline. Please recalculate or rewrite this section for the 2-region model.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ami-prebaked.md` around lines 258 - 266, Update the "## Multi-region cost" section to reflect the current 2-region distribution model: change the example that assumed 9 regions to a 2-region example, recalculate totals using the existing per-region figures (one-time cross-region transfer ~$0.06-0.08 per additional region and monthly EBS snapshot storage ~$0.15-0.20 per additional region), and replace the summary line with "With 2 distribution regions: ~$0.06-0.08 one-time + ~$0.15-0.20/month." Ensure the header and bullet lines remain but adjust wording where needed to state the pipeline now uses 2 regions.
🧹 Nitpick comments (2)
.github/workflows/ami-build.yml (1)
236-236: ⚡ Quick winHarden checkout token handling in
prunejob.Set
persist-credentials: falseforactions/checkoutto avoid leaving the token in local git config longer than necessary.Suggested fix
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ami-build.yml at line 236, In the `prune` job where `actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd` is used, add the checkout input `persist-credentials: false` to prevent the workflow token from being written to the repository's local git config; update the `uses: actions/checkout...` step for the `prune` job to include `with: persist-credentials: false` so credentials are not persisted after checkout.Source: Linters/SAST tools
.github/workflows/shellcheck.yml (1)
19-19: ⚡ Quick winDisable credential persistence for checkout.
Add
persist-credentials: falseon Line 19 to reduce token exposure in the workspace.Suggested fix
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/shellcheck.yml at line 19, The checkout action currently uses "uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd"; to disable credential persistence add the key "persist-credentials: false" as a sibling mapping under that checkout step so the action reads the same but includes persist-credentials: false to prevent token exposure in the workspace.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ami-build.yml:
- Around line 175-201: The step currently runs with set -euo pipefail so a
failing aws ec2 copy-image will hard-fail the job; change the call that sets
NEW_AMI (the aws ec2 copy-image invocation that assigns NEW_AMI) to run in a
guarded/conditional form (e.g., test the command return and on failure emit a
::warning:: like the later checks and exit 0) so copy-image failures soft-fail
like the later wait/modify-image-attribute checks; keep the rest of the flow
(TARGET, AMI_ID, AMI_NAME) unchanged and reuse the existing warning text and
exit behavior for consistency.
- Around line 96-104: The workflow step "Pre-build free-slot guard (us-east-2)"
invokes ./infra/scripts/ami-prune.sh but the workflow's pull_request path
filters only include infra/packer/**, so PRs that change the prune script can
skip this workflow; update the ami-build workflow trigger to include the IaC
path(s) for the prune script (for example add infra/scripts/** or infra/** or
specifically infra/scripts/ami-prune.sh) to the pull_request paths so changes to
the script will trigger the ami-build workflow.
In `@infra/scripts/ami-prune.sh`:
- Line 108: The python extraction for variable "snaps" currently yields nulls
and then masks failures with "|| true", causing empty snapshot lists and
orphaned snapshots; update the command that builds "snaps" (the python3 -c
invocation reading "images" and using "${ami}") to filter out null/None entries
(only print truthy snapshot IDs from the Snaps list) and remove the "|| true"
fallback so errors propagate instead of being silenced; ensure the expression
still takes "o['Id']==a" and prints each non-null snap ID on its own line so
downstream deletion/deregister logic receives a correct list.
In `@infra/scripts/test/cases/dryrun.sh`:
- Line 7: The test uses the broken `echo "$out" | grep -q "ami-new1" && fail
"..." || true` pattern which masks failures; replace this with a proper
conditional check (e.g., run `grep -q` and if it succeeds call `fail "dryrun:
ami-new1 (kept) wrongly listed"`) so that `fail` is not bypassed by `|| true`;
update the occurrence of this pattern in the dryrun assertion to use an
if/then/fail/fi flow (or `grep -q ... || fail ...`) referencing the existing
`fail` helper and the `echo "$out" | grep -q "ami-new1"` check.
In `@infra/scripts/test/cases/stack.sh`:
- Around line 13-15: The test currently captures the exit code into rc after
running "${SCRIPT}" and then checks rc; replace this with a direct conditional
that runs the command inside the if statement (e.g. if "${SCRIPT}" --regions
us-east-2 --keep-latest 2 --stack-name jit-runners --apply >/dev/null 2>&1; then
... else ... fi), remove the intermediate rc variable, and keep the existing
pass/fail messages unchanged; locate the invocation using the SCRIPT variable to
modify the conditional.
In `@infra/scripts/test/run.sh`:
- Line 22: The teardown function currently does unsafe rm -rf "${WORK}"; update
teardown() to first verify WORK is set and looks like a temp path before
removing: check that WORK is non-empty (e.g. [[ -n "$WORK" ]]) and matches a
safe pattern (for example begins with /tmp/ or the value returned by mktemp),
then use rm -rf -- "$WORK"; if the checks fail, skip deletion and log or return
a non-zero status. Reference the teardown function and the WORK variable/mktemp
usage when implementing this guard.
- Line 3: The script currently enables -u and -o pipefail via the single line
"set -uo pipefail" but omits -e, allowing commands to fail silently; update that
line (the shell options declaration in run.sh) to include -e so it reads
equivalently to "set -euo pipefail" to ensure the script exits on any command
failure.
- Around line 24-34: The ShellCheck disables on the helper functions use the
wrong code (SC2329); update the disable annotations to SC2317 for the pass(),
fail(), assert_no_call(), and assert_call() function declarations so they
correctly suppress the "command appears unreachable" warning when these helpers
are invoked indirectly by sourced test cases; ensure each line with "#
shellcheck disable=..." referencing pass, fail, assert_no_call, and assert_call
uses SC2317 instead of SC2329.
---
Outside diff comments:
In `@CLAUDE.md`:
- Around line 159-160: Update the wording in the Build & Test examples for the
make targets to reflect the current two-region AMI distribution model: change
the phrase "all distribution regions" to explicitly state the two-region
behavior (source: us-east-2, copy: us-east-1) for the make ami.build-distribute
line, and clarify that make ami.copy AMI_ID=ami-xxx will copy an existing AMI
from us-east-2 to us-east-1; keep the same make target names (make
ami.build-distribute and make ami.copy) but update their descriptive text so
operators understand the current two-region deployment.
In `@docs/ami-prebaked.md`:
- Around line 103-111: The "Build and distribute to all regions" section still
lists the old 9-region distribution for the make target ami.build-distribute;
update the heading and the region list under that block (the "Build and
distribute to all regions" heading and the code block invoking make
ami.build-distribute) to state that the target builds in us-east-2 and copies
only to us-east-1 (source us-east-2 + us-east-1 copy) to match the new retention
policy, and make any adjacent heading ("Copy an existing AMI to all regions")
consistent with this reduced distribution.
- Around line 258-266: Update the "## Multi-region cost" section to reflect the
current 2-region distribution model: change the example that assumed 9 regions
to a 2-region example, recalculate totals using the existing per-region figures
(one-time cross-region transfer ~$0.06-0.08 per additional region and monthly
EBS snapshot storage ~$0.15-0.20 per additional region), and replace the summary
line with "With 2 distribution regions: ~$0.06-0.08 one-time +
~$0.15-0.20/month." Ensure the header and bullet lines remain but adjust wording
where needed to state the pipeline now uses 2 regions.
---
Nitpick comments:
In @.github/workflows/ami-build.yml:
- Line 236: In the `prune` job where
`actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd` is used, add the
checkout input `persist-credentials: false` to prevent the workflow token from
being written to the repository's local git config; update the `uses:
actions/checkout...` step for the `prune` job to include `with:
persist-credentials: false` so credentials are not persisted after checkout.
In @.github/workflows/shellcheck.yml:
- Line 19: The checkout action currently uses "uses:
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd"; to disable
credential persistence add the key "persist-credentials: false" as a sibling
mapping under that checkout step so the action reads the same but includes
persist-credentials: false to prevent token exposure in the workspace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fea8137d-d26e-4a51-8471-145de8a261e5
📒 Files selected for processing (18)
.github/workflows/ami-build.yml.github/workflows/shellcheck.ymlCLAUDE.mdMakefiledocs/ami-prebaked.mddocs/ami-prune-iam.mdinfra/packer/variables.pkr.hclinfra/scripts/ami-prune.shinfra/scripts/test/awsinfra/scripts/test/cases/.gitkeepinfra/scripts/test/cases/apply.shinfra/scripts/test/cases/dryrun.shinfra/scripts/test/cases/ensure_free.shinfra/scripts/test/cases/keeplatest0.shinfra/scripts/test/cases/stack.shinfra/scripts/test/fake-awsinfra/scripts/test/fixtures/describe-images-5.jsoninfra/scripts/test/run.sh
💤 Files with no reviewable changes (1)
- infra/packer/variables.pkr.hcl
Comment on lines
+96
to
+104
| - name: Pre-build free-slot guard (us-east-2) | ||
| if: github.event_name != 'pull_request' | ||
| run: | | ||
| ./infra/scripts/ami-prune.sh \ | ||
| --regions us-east-2 \ | ||
| --stack-name jit-runners \ | ||
| --keep-latest 2 \ | ||
| --ensure-free 1 \ | ||
| --apply |
There was a problem hiding this comment.
Trigger ami-build when prune IaC changes.
Line 99 adds a runtime dependency on infra/scripts/ami-prune.sh, but pull request path filters currently only include infra/packer/**. That can skip this workflow for prune-script changes.
Suggested fix
pull_request:
branches: [main]
paths:
- "infra/packer/**"
+ - "infra/scripts/**"
+ - ".github/workflows/ami-build.yml"As per coding guidelines, .github/workflows/{ami-build,gce-image-build}.yml files must define path filters for IaC changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ami-build.yml around lines 96 - 104, The workflow step
"Pre-build free-slot guard (us-east-2)" invokes ./infra/scripts/ami-prune.sh but
the workflow's pull_request path filters only include infra/packer/**, so PRs
that change the prune script can skip this workflow; update the ami-build
workflow trigger to include the IaC path(s) for the prune script (for example
add infra/scripts/** or infra/** or specifically infra/scripts/ami-prune.sh) to
the pull_request paths so changes to the script will trigger the ami-build
workflow.
Source: Coding guidelines
| local ami | ||
| while IFS= read -r ami; do | ||
| [ -z "${ami}" ] && continue | ||
| local snaps; snaps="$(echo "${images}" | python3 -c "import sys,json;d=json.load(sys.stdin);a=sys.argv[1];print('\n'.join(s for o in d if o['Id']==a for s in (o.get('Snaps') or [])))" "${ami}" 2>/dev/null || true)" |
There was a problem hiding this comment.
Filter null snapshot IDs and stop masking extraction failures.
At Line 108, Snaps can include nulls; the Python join(...) then fails, and || true silently turns snaps empty. That causes deregister to proceed without deleting snapshots, leaving orphaned snapshots behind.
Suggested fix
- local snaps; snaps="$(echo "${images}" | python3 -c "import sys,json;d=json.load(sys.stdin);a=sys.argv[1];print('\n'.join(s for o in d if o['Id']==a for s in (o.get('Snaps') or [])))" "${ami}" 2>/dev/null || true)"
+ local snaps; snaps="$(echo "${images}" | python3 -c "import sys,json;d=json.load(sys.stdin);a=sys.argv[1];print('\n'.join(s for o in d if o['Id']==a for s in (o.get('Snaps') or []) if s))" "${ami}")"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| local snaps; snaps="$(echo "${images}" | python3 -c "import sys,json;d=json.load(sys.stdin);a=sys.argv[1];print('\n'.join(s for o in d if o['Id']==a for s in (o.get('Snaps') or [])))" "${ami}" 2>/dev/null || true)" | |
| local snaps; snaps="$(echo "${images}" | python3 -c "import sys,json;d=json.load(sys.stdin);a=sys.argv[1];print('\n'.join(s for o in d if o['Id']==a for s in (o.get('Snaps') or []) if s))" "${ami}")" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/scripts/ami-prune.sh` at line 108, The python extraction for variable
"snaps" currently yields nulls and then masks failures with "|| true", causing
empty snapshot lists and orphaned snapshots; update the command that builds
"snaps" (the python3 -c invocation reading "images" and using "${ami}") to
filter out null/None entries (only print truthy snapshot IDs from the Snaps
list) and remove the "|| true" fallback so errors propagate instead of being
silenced; ensure the expression still takes "o['Id']==a" and prints each
non-null snap ID on its own line so downstream deletion/deregister logic
receives a correct list.
Comment on lines
+13
to
+15
| "${SCRIPT}" --regions us-east-2 --keep-latest 2 --stack-name jit-runners --apply >/dev/null 2>&1 | ||
| rc=$? | ||
| if [ "${rc}" -eq 0 ]; then pass "stack: None DefaultAMI → falls back to top-N, no error"; else fail "stack: None should not fail (rc=${rc})"; fi |
There was a problem hiding this comment.
Replace $? check with direct if command evaluation.
Lines 13–15 use rc=$? flow, which is ShellCheck-unfriendly and likely contributing to the failing shellcheck job. Use direct conditional execution instead.
Suggested fix
- "${SCRIPT}" --regions us-east-2 --keep-latest 2 --stack-name jit-runners --apply >/dev/null 2>&1
- rc=$?
- if [ "${rc}" -eq 0 ]; then pass "stack: None DefaultAMI → falls back to top-N, no error"; else fail "stack: None should not fail (rc=${rc})"; fi
+ if "${SCRIPT}" --regions us-east-2 --keep-latest 2 --stack-name jit-runners --apply >/dev/null 2>&1; then
+ pass "stack: None DefaultAMI → falls back to top-N, no error"
+ else
+ fail "stack: None should not fail"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "${SCRIPT}" --regions us-east-2 --keep-latest 2 --stack-name jit-runners --apply >/dev/null 2>&1 | |
| rc=$? | |
| if [ "${rc}" -eq 0 ]; then pass "stack: None DefaultAMI → falls back to top-N, no error"; else fail "stack: None should not fail (rc=${rc})"; fi | |
| if "${SCRIPT}" --regions us-east-2 --keep-latest 2 --stack-name jit-runners --apply >/dev/null 2>&1; then | |
| pass "stack: None DefaultAMI → falls back to top-N, no error" | |
| else | |
| fail "stack: None should not fail" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/scripts/test/cases/stack.sh` around lines 13 - 15, The test currently
captures the exit code into rc after running "${SCRIPT}" and then checks rc;
replace this with a direct conditional that runs the command inside the if
statement (e.g. if "${SCRIPT}" --regions us-east-2 --keep-latest 2 --stack-name
jit-runners --apply >/dev/null 2>&1; then ... else ... fi), remove the
intermediate rc variable, and keep the existing pass/fail messages unchanged;
locate the invocation using the SCRIPT variable to modify the conditional.
Source: Pipeline failures
| @@ -0,0 +1,46 @@ | |||
| #!/usr/bin/env bash | |||
| # Test runner for ami-prune.sh. Puts fake-aws on PATH and asserts on recorded calls / output. | |||
| set -uo pipefail | |||
There was a problem hiding this comment.
Add -e flag to prevent silent test failures.
Without set -e, command failures in test cases or helper functions won't exit the script, potentially causing test failures to go undetected. This undermines the test runner's reliability.
🛡️ Proposed fix
-set -uo pipefail
+set -euo pipefail📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| set -uo pipefail | |
| set -euo pipefail |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/scripts/test/run.sh` at line 3, The script currently enables -u and -o
pipefail via the single line "set -uo pipefail" but omits -e, allowing commands
to fail silently; update that line (the shell options declaration in run.sh) to
include -e so it reads equivalently to "set -euo pipefail" to ensure the script
exits on any command failure.
Source: Linters/SAST tools
…dlib vulns) (#83) Signed-off-by: Kaio Fellipe <kaio6fellipe@gmail.com>
This was referenced Jun 8, 2026
Closed
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
1 participant
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.
Implements the approved spec/plan (zettelkasten
Projects/jit-runners/specs/2026-06-07-image-distribution-retention-design.md+ matching plan).What changed
us-east-2(source) +us-east-1. Packer builds & publishes the us-east-2 source only; an explicit post-build step copies it to us-east-1 withwait image-availablethen publishes — publish-before-copy (fixes fix(ci): auto-deregister stale public AMIs to prevent quota exhaustion #49's orphan-in-distribution-region failure mode). Copy/publish failure soft-fails (::warning::, no rollback; source stays usable).infra/scripts/ami-prune.sh(dry-run by default): keep newest-2 ∪--keep-ami∪ live CFNDefaultAMI; flags--regions --keep-latest --keep-ami --stack-name --name-prefix --ensure-free --quota --apply. Numeric arg validation,InvalidSnapshot.InUsenon-fatal, mktemp error capture. 25 bash tests via a fake-aws shim.--ensure-free 1quota guard (us-east-2) + post-buildprunejob (needs: build,if: success() && non-PR) pruning us-east-2 (--stack-name jit-runners) and us-east-1 to latest-2. Newshellcheck.ymllints scripts + runs the tests.concurrencygroup added.distribute; nownon-PR(any non-PR build publishes a public source viaami_groups=["all"]).make ami.prune(APPLY=1to apply).docs/ami-prebaked.md); IAM prerequisite (docs/ami-prune-iam.md); CLAUDE.md bullet.The build OIDC role (
AMI_BUILD_ROLE_ARN) is provisioned out-of-band. Attach the policy indocs/ami-prune-iam.md(ec2:DeregisterImage,ec2:DeleteSnapshot,cloudformation:DescribeStacks,servicequotas:GetServiceQuota) before merging/relying on these jobs.Closes / relates
Follow-up (post-merge, human-gated)
Run the one-time 8-region purge per
docs/ami-prebaked.md(announce → dry-run →--apply→ re-enable block-public-access).Verification
25/25 bash tests pass; shellcheck clean; actionlint clean (no new findings);
packer validatepasses. Final multi-agent review: ready to PR, no critical/important issues.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests