-
Notifications
You must be signed in to change notification settings - Fork 8
ci: build expirable images for pull-requests #167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Event
participant Actions as GitHub Actions
participant Meta as Tag/Meta Step
participant Builder as docker/build-push
participant Registry as Container Registry
GitHub->>Actions: push or pull_request event
Actions->>Actions: evaluate PUSH/gate conditions (include PR head repo & label)
Actions->>Meta: extract metadata (use PR data when event=pr)
Meta-->>Actions: outputs.tags, outputs.meta (including quay.expires-after)
Actions->>Builder: build with selected tags and metadata
Builder->>Registry: push image(s) and labels
Registry-->>Actions: push status
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
can we trigger deletion when the PR is closed (merged or otherwise)? |
Probably, but it would require additional work :) that would be better since as long as the PR is up, the image would be up.. |
|
let me rebase |
.beads/issues.jsonl
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mangelajo what does this do? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, that should not be added, removing
It's from a tool I use with cursor but it's supposed to be a local "issue" tracker to organize work, should not be posted here (at least for our workflow, may be other people will commit it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not done.. why is it still here.. hmmm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bd had a git pre-commit hook installed 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build-images.yaml:
- Around line 17-18: The PUSH environment boolean allows PR runs from forks
because it only checks github.repository_owner and event type; update the
env.PUSH expression so PR-triggered runs are allowed only when the PR head repo
is the same repo (i.e., not a fork). Edit the env.PUSH value (symbol: env.PUSH)
to include an extra clause that, for github.event_name == 'pull_request',
requires github.event.pull_request.head.repo.full_name == github.repository (so
secrets like secrets.QUAY_TOKEN used later during the login step are only used
for same-repo PRs); keep the existing main/tag/release checks unchanged.
🧹 Nitpick comments (1)
.github/workflows/build-images.yaml (1)
126-129: Avoid emitting an emptyquay.expires-afterlabel on non‑PR builds.The current code produces
quay.expires-after=(empty value) on non-PR events, anddocker/metadata-actiondoes not auto-drop labels with empty values—they are included in the output as-is. To omit the label entirely when not a PR, conditionally construct the entirelabels:input.♻️ Suggested refactor
- labels: | - quay.expires-after=${{ github.event_name == 'pull_request' && '7d' || '' }} + labels: ${{ github.event_name == 'pull_request' && 'quay.expires-after=7d' || '' }}
This PR enables the creation of a pr- tagged set of images in quay for each PR, those expire after a week.
The images are only built if the "build-pr-images" is applied. This avoid unnecessary builds, CO2 and wasted storage space in quay.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.