Update our ECR publish so that we can gc#2759
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdates publish-image workflow to determine deploy target from commit messages and push images with both commit-SHA and deploy-target tags using a build-cache repo; updates Elastic Beanstalk environment name; adds ECR lifecycle policy documentation for retention and cache expiry. ChangesDeployment Infrastructure and Lifecycle Management
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server/config/ecr/README.md (2)
98-113: ⚡ Quick winConsider storing policies in separate JSON files.
The current approach requires manually copying and pasting JSON from the README, which is error-prone. Consider creating
instant-prod-ecr-policy.jsonandinstant-prod-ecr-buildcache-policy.jsonfiles, then reference them directly:♻️ Proposed improvement
Create
server/config/ecr/instant-prod-ecr-policy.json:{ "rules": [ { ... policy from lines 24-61 ... } ] }Create
server/config/ecr/instant-prod-ecr-buildcache-policy.json:{ "rules": [ { ... policy from lines 72-85 ... } ] }Update CLI commands:
-aws --region us-east-1 ecr put-lifecycle-policy \ - --repository-name instant-prod-ecr \ - --lifecycle-policy-text "$(jq -c '.' <<'JSON' -<paste the instant-prod-ecr policy JSON above> -JSON -)" +aws --region us-east-1 ecr put-lifecycle-policy \ + --repository-name instant-prod-ecr \ + --lifecycle-policy-text "$(jq -c '.' server/config/ecr/instant-prod-ecr-policy.json)"🤖 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 `@server/config/ecr/README.md` around lines 98 - 113, Move the inline heredoc JSON into separate files and update the README commands to reference them: create server/config/ecr/instant-prod-ecr-policy.json and server/config/ecr/instant-prod-ecr-buildcache-policy.json containing the respective lifecycle policy objects (the rules shown in the README), then replace the heredoc usage in the README with aws ecr put-lifecycle-policy --repository-name instant-prod-ecr --lifecycle-policy-text file://server/config/ecr/instant-prod-ecr-policy.json and the analogous command for instant-prod-ecr-buildcache so the CLI reads the JSON files directly.
23-36: 💤 Low valueConsider documenting the countNumber workaround.
The
countNumber: 9999combined withimageCountMoreThanis a workaround to keep images indefinitely (they'll only expire if count exceeds 9999, which won't happen). While this works correctly, consider adding a comment explaining that ECR doesn't support a native "never expire" rule.📝 Suggested clarification
{ "rulePriority": 1, - "description": "Keep the moving prod and staging tags forever", + "description": "Keep the moving prod and staging tags forever (countNumber > 9999 = never expire)", "selection": {🤖 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 `@server/config/ecr/README.md` around lines 23 - 36, Add a short explanatory note to the README near the ECR lifecycle rule example clarifying that using "countType": "imageCountMoreThan" with "countNumber": 9999 (as shown in the rule containing tagPatternList ["prod","staging"]) is an intentional workaround to effectively never expire those tags because ECR has no native "never expire" setting; mention that the high countNumber prevents accidental expiration and note this is intentional behavior to preserve moving tags like prod and staging.
🤖 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/clojure.yml:
- Around line 361-371: The publish-eb job/step is not using the DEPLOY_TARGET
tag set during the Docker build, so Elastic Beanstalk always deploys the same
environment; update the publish-eb step (the GitHub Actions step named
"publish-eb") to read the DEPLOY_TARGET environment variable (same name used in
the Build step) and map it to the correct EB environment name (e.g., use a
conditional or lookup to choose "Instant-docker-prod-env" vs your staging env)
before calling the EB deploy command, or explicitly inject DEPLOY_TARGET into
the EB CLI/API call so the deployment target changes with the tag; ensure the
step uses the same env var name and update any hardcoded env names in the
publish-eb step accordingly.
---
Nitpick comments:
In `@server/config/ecr/README.md`:
- Around line 98-113: Move the inline heredoc JSON into separate files and
update the README commands to reference them: create
server/config/ecr/instant-prod-ecr-policy.json and
server/config/ecr/instant-prod-ecr-buildcache-policy.json containing the
respective lifecycle policy objects (the rules shown in the README), then
replace the heredoc usage in the README with aws ecr put-lifecycle-policy
--repository-name instant-prod-ecr --lifecycle-policy-text
file://server/config/ecr/instant-prod-ecr-policy.json and the analogous command
for instant-prod-ecr-buildcache so the CLI reads the JSON files directly.
- Around line 23-36: Add a short explanatory note to the README near the ECR
lifecycle rule example clarifying that using "countType": "imageCountMoreThan"
with "countNumber": 9999 (as shown in the rule containing tagPatternList
["prod","staging"]) is an intentional workaround to effectively never expire
those tags because ECR has no native "never expire" setting; mention that the
high countNumber prevents accidental expiration and note this is intentional
behavior to preserve moving tags like prod and staging.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ad42f737-2722-4c78-9b99-5f401ab069c2
📒 Files selected for processing (3)
.github/workflows/clojure.ymlserver/.elasticbeanstalk/config.ymlserver/config/ecr/README.md
nezaj
reviewed
Jun 11, 2026
nezaj
reviewed
Jun 11, 2026
nezaj
reviewed
Jun 11, 2026
Co-authored-by: Joe Averbukh <joeaverbukh@gmail.com>
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.
Changes our ECR publishing so that we can set up lifecycle rules to delete old containers.
Two changes to how we publish:
prodtag, so that we definitely don't delete the version we rely on in prodThen the other change is to add a lifecycle rule that will expire old images. I put that in the
config/ecr/README.mdfile.After merging this in, I'll add the lifecycle rules for the ecr repo and preview what it deletes to confirm we're not deleting stuff we need.