🐛 Add 7-day screenshot retention policy with automated cleanup#4352
🐛 Add 7-day screenshot retention policy with automated cleanup#4352clubanderson merged 2 commits intomainfrom
Conversation
- Create cleanup-screenshots.yml: scheduled daily workflow that deletes screenshot files older than 7 days from .github/screenshots/ - Before deletion, rewrites any branch-based raw URLs in issue comments to commit-SHA-based URLs so images remain visible in issues - Update process-screenshots.yml to emit permanent commit-SHA URLs from the start, eliminating the need for URL rewriting on new screenshots Closes #4349 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole canceled.
|
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
- Use --diff-filter=A in git log to find the original add-commit (not the most recent commit that touched the file) for correct age and SHA - Switch from execSync to execFileSync to prevent shell injection from untrusted request ID path components - Use replaceAll instead of replace for URL rewriting in comments - Paginate listComments via github.paginate() to handle issues with more than 100 comments - Add git pull --rebase before push to handle concurrent main commits Signed-off-by: Andrew Anderson <andy@clubanderson.com>
There was a problem hiding this comment.
Pull request overview
Implements a 7-day retention policy for issue screenshot artifacts stored in-repo, while keeping issue-embedded images working by switching to commit-SHA-based raw URLs and adding an automated cleanup workflow.
Changes:
- Update screenshot processing workflow to emit commit-SHA-based
raw.githubusercontent.com/.../{sha}/...URLs in issue comments. - Add a scheduled cleanup workflow that deletes screenshots older than 7 days and rewrites legacy branch-based URLs in issue comments before deletion.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/process-screenshots.yml | Switches generated screenshot links in comments from branch-based URLs to commit-SHA-based permanent raw URLs. |
| .github/workflows/cleanup-screenshots.yml | Adds a daily scheduled job to rewrite legacy URLs and remove expired screenshots from .github/screenshots/. |
| const issueNumber = parseInt(issueMatch[1]); | ||
| const branchUrl = `${RAW_PREFIX}/main/${file}`; | ||
| const permanentUrl = `${RAW_PREFIX}/${commitSha}/${file}`; |
There was a problem hiding this comment.
branchUrl is hardcoded to /main/, which will fail if the repo default branch is not main (or if it changes). Use context.payload.repository.default_branch (available on schedule/dispatch) to build the branch-based URL you’re trying to rewrite.
| const { data: comments } = await github.rest.issues.listComments({ | ||
| owner, | ||
| repo, | ||
| issue_number: issueNumber, | ||
| per_page: 100, | ||
| }); |
There was a problem hiding this comment.
issues.listComments only fetches the first 100 comments. For long-running issues with many comments, URLs beyond the first page won’t be rewritten and links may break after deletion. Consider using Octokit pagination (github.paginate(...)) or iterating page until empty.
| const { data: comments } = await github.rest.issues.listComments({ | |
| owner, | |
| repo, | |
| issue_number: issueNumber, | |
| per_page: 100, | |
| }); | |
| const comments = await github.paginate( | |
| github.rest.issues.listComments, | |
| { | |
| owner, | |
| repo, | |
| issue_number: issueNumber, | |
| per_page: 100, | |
| } | |
| ); |
| owner, | ||
| repo, | ||
| comment_id: comment.id, | ||
| body: comment.body.replace(branchUrl, permanentUrl), |
There was a problem hiding this comment.
comment.body.replace(branchUrl, permanentUrl) only replaces the first occurrence. If a comment contains multiple screenshots (or repeated URLs), some links will remain branch-based. Use replaceAll (Node 16+) or a global regex to ensure every occurrence is rewritten.
| body: comment.body.replace(branchUrl, permanentUrl), | |
| body: comment.body.replaceAll(branchUrl, permanentUrl), |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 2 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
Summary
cleanup-screenshots.yml) that runs daily and deletes screenshot files older than 7 days from.github/screenshots/process-screenshots.ymlto emit permanent commit-SHA URLs from the start, so new screenshots survive cleanup without needing URL rewritingHow it works
Problem: Screenshots committed to
.github/screenshots/accumulate indefinitely, growing repository size.Solution (two-part):
process-screenshots.ymlchange: New screenshots now usehttps://raw.githubusercontent.com/{owner}/{repo}/{commitSha}/{path}instead ofdownload_url(which uses branch refs like/main/). SHA-based URLs reference the exact commit that added the file, so they remain valid even after the file is deleted from the default branch.cleanup-screenshots.yml(new): Runs daily at 2 AM UTC. For each screenshot older than 7 days:git rmTest plan
process-screenshots.ymlcorrectly usesdata.commit.shafrom the Contents API responseworkflow_dispatchraw.githubusercontent.com/{owner}/{repo}/{sha}/...) resolve correctly even after the file is deleted frommainCloses #4349