chore: add previous build cleanup scripts#1682
Conversation
WalkthroughAdds automated cleanup for preview and PR builds stored in Cloudflare R2, via a new step in build-plugin workflow and two new Bash scripts. Updates main workflow concurrency to cancel only on pull_request events. No changes to exported/public APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions (build-plugin)
participant R2 as Cloudflare R2 (S3 API)
Note over GH: On push and not a release
GH->>R2: List s3://CF_BUCKET_PREVIEW/unraid-api/* (endpoint=CF_ENDPOINT)
GH->>GH: Filter filenames by regex and 7-day cutoff
alt Old files found
loop For each old file
GH->>R2: aws s3 rm s3://.../dynamix.unraid.net-YYYY.MM.DD.HHMM.txz
Note right of GH: continue-on-error
end
GH-->>Dev: Log deletions count
else None found
GH-->>Dev: Log "no old files" message
end
sequenceDiagram
autonumber
actor Op as Operator
participant Script1 as scripts/cleanup-old-builds.sh
participant R2 as Cloudflare R2 (S3 API)
Op->>Script1: Run [keep_days?]
Script1->>Script1: Validate CF_* env vars
Script1->>R2: List preview builds under unraid-api/
Script1->>Script1: Partition OLD vs KEEP by cutoff
alt Confirm deletion
loop Each OLD file
Script1->>R2: aws s3 rm s3://.../file
end
Script1-->>Op: Summary (deleted/failed)
else Cancel
Script1-->>Op: Aborted
end
sequenceDiagram
autonumber
actor Op as Operator
participant Script2 as scripts/cleanup-pr-builds.sh
participant R2 as Cloudflare R2 (S3 API)
Op->>Script2: Run
Script2->>Script2: Validate CF_* env vars
Script2->>R2: List PR build prefixes under unraid-api/tag/
alt PR builds found
Script2-->>Op: Prompt to delete all
alt Confirm
loop Each PR prefix
Script2->>R2: aws s3 rm --recursive s3://.../unraid-api/tag/<PR>
end
Script2-->>Op: Cleanup summary and verification
else Cancel
Script2-->>Op: Aborted
end
else None found
Script2-->>Op: "No PR builds"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
Comment |
|
Claude finished @elibosley's task —— View job PR Review: Build Cleanup ScriptsAnalyzing the changes for critical issues in the diff/changeset: Todo List:
Analysis Results:🔍 CRITICAL SECURITY ISSUE FOUND: Line 204 in grep -E "dynamix\.unraid\.net-[0-9]{4}\.[0-9]{2}\.[0-9]{2}\.[0-9]{4}\.txz"Risk: If S3 bucket contents are maliciously crafted, the Recommendation: Use the standalone cleanup scripts instead of embedded bash in workflows, as they properly validate and sanitize inputs with proper quoting and safer string operations. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1682 +/- ##
=======================================
Coverage 58.58% 58.58%
=======================================
Files 624 624
Lines 30769 30769
Branches 3618 3618
=======================================
Hits 18026 18026
Misses 12706 12706
Partials 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
.github/workflows/build-plugin.yml (3)
187-222: Count only successful deletions; hardcoded prefix; add pipefail
- Count increments even on failed rm (due to
|| true). Track successes/failures separately.- Use
${{ inputs.BUCKET_PATH }}instead of hardcodedunraid-api/to future-proof.- Add
set -euo pipefailto avoid masked errors in pipelines.- name: Clean up old preview builds if: inputs.RELEASE_CREATED == 'false' && github.event_name == 'push' continue-on-error: true env: AWS_ACCESS_KEY_ID: ${{ secrets.CF_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.CF_SECRET_ACCESS_KEY }} AWS_DEFAULT_REGION: auto run: | - echo "🧹 Cleaning up old preview builds (keeping last 7 days)..." + set -euo pipefail + echo "🧹 Cleaning up old preview builds (keeping last 7 days)..." # Calculate cutoff date (7 days ago) CUTOFF_DATE=$(date -d "7 days ago" +"%Y.%m.%d") echo "Deleting builds older than: ${CUTOFF_DATE}" # List and delete old timestamped .txz files - OLD_FILES=$(aws s3 ls "s3://${{ secrets.CF_BUCKET_PREVIEW }}/unraid-api/" \ + OLD_FILES=$(aws s3 ls "s3://${{ secrets.CF_BUCKET_PREVIEW }}/${{ inputs.BUCKET_PATH }}/" \ --endpoint-url ${{ secrets.CF_ENDPOINT }} --recursive | \ grep -E "dynamix\.unraid\.net-[0-9]{4}\.[0-9]{2}\.[0-9]{2}\.[0-9]{4}\.txz" | \ awk '{print $4}' || true) - DELETED_COUNT=0 + DELETED_COUNT=0 + FAILED_COUNT=0 if [ -n "$OLD_FILES" ]; then while IFS= read -r file; do if [[ $file =~ ([0-9]{4}\.[0-9]{2}\.[0-9]{2})\.[0-9]{4}\.txz ]]; then FILE_DATE="${BASH_REMATCH[1]}" if [[ "$FILE_DATE" < "$CUTOFF_DATE" ]]; then echo "Deleting old build: $(basename "$file")" - aws s3 rm "s3://${{ secrets.CF_BUCKET_PREVIEW }}/${file}" \ - --endpoint-url ${{ secrets.CF_ENDPOINT }} || true - ((DELETED_COUNT++)) + if aws s3 rm "s3://${{ secrets.CF_BUCKET_PREVIEW }}/${file}" \ + --endpoint-url ${{ secrets.CF_ENDPOINT }} >/dev/null 2>&1; then + ((DELETED_COUNT++)) + else + ((FAILED_COUNT++)) + fi fi fi done <<< "$OLD_FILES" fi - echo "✅ Deleted ${DELETED_COUNT} old builds" + echo "✅ Deleted ${DELETED_COUNT} old builds" + if [ "$FAILED_COUNT" -gt 0 ]; then + echo "⚠️ Failed deletions: ${FAILED_COUNT}" + fi
195-221: Trim trailing spaces to satisfy YAML lintingTrailing spaces flagged at Lines 196, 200, 206, 221. Remove to keep CI/lint green.
201-209: Optional: prefer s3api + JMESPath over text parsing
aws s3 ls | grep | awkis brittle.aws s3api list-objects-v2 --prefix "${{ inputs.BUCKET_PATH }}/" --querywould avoid format parsing. Keep as-is if you prefer simplicity.scripts/cleanup-pr-builds.sh (3)
6-6: Harden shell: use -euo pipefailPrevents silent failures and use of unset vars during cleanup.
-set -e +set -euo pipefail
60-66: Add non-interactive mode for automationSupport a “yes” flag or env to skip prompt when running headless.
-# Confirmation prompt -read -p "Are you sure you want to delete ALL these PR builds? (yes/no): " -r -echo "" - -if [[ ! $REPLY =~ ^[Yy]es$ ]]; then +# Confirmation prompt (skip with --yes or CONFIRM=yes) +if [[ "${1:-}" == "--yes" || "${CONFIRM:-}" =~ ^[Yy]([eE][sS])?$ ]]; then + REPLY="yes" +else + read -p "Are you sure you want to delete ALL these PR builds? (yes/no): " -r + echo "" +fi +if [[ ! ${REPLY:-} =~ ^[Yy]es$ ]]; then echo -e "${YELLOW}⚠️ Cleanup cancelled${NC}" exit 0 fi
46-47: Directory detection is output-format dependentParsing
aws s3 lswithgrep "PRE PR"assumes AWS CLI’s human format. Consideraws s3api list-objects-v2 --delimiter '/' --prefix 'unraid-api/tag/PR'to list common prefixes robustly. OK to keep if you prefer simplicity.scripts/cleanup-old-builds.sh (3)
6-6: Harden shell: use -euo pipefailAvoids masked failures and unset var usage during deletions.
-set -e +set -euo pipefail
109-117: Optional: allow non-interactive confirmationHelpful for periodic cleanups; can be gated by env var to avoid breaking interactive usage.
-# Confirmation prompt -read -p "Are you sure you want to delete these ${OLD_COUNT} old builds? (yes/no): " -r -echo "" - -if [[ ! $REPLY =~ ^[Yy]es$ ]]; then +# Confirmation prompt (skip with CONFIRM=yes) +if [[ -z "${CONFIRM:-}" ]]; then + read -p "Are you sure you want to delete these ${OLD_COUNT} old builds? (yes/no): " -r + echo "" +fi +if [[ ! ${CONFIRM:-$REPLY} =~ ^[Yy]es$ ]]; then echo -e "${YELLOW}⚠️ Cleanup cancelled${NC}" exit 0 fi
60-63: Prefer s3api over human parsing
aws s3 ls | awk '{print $4}'is fragile.aws s3api list-objects-v2 --prefix 'unraid-api/' --query 'Contents[?contains(Key,dynamix.unraid.net-)].Key' --output textis more robust. Optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build-plugin.yml(1 hunks).github/workflows/main.yml(1 hunks)scripts/cleanup-old-builds.sh(1 hunks)scripts/cleanup-pr-builds.sh(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/build-plugin.yml
[error] 196-196: trailing spaces
(trailing-spaces)
[error] 200-200: trailing spaces
(trailing-spaces)
[error] 206-206: trailing spaces
(trailing-spaces)
[error] 221-221: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/main.yml (1)
11-11: LGTM: conditional cancel-in-progressOnly cancels PR runs; keeps push workflows unaffected. Looks good.
1 participant
Summary by CodeRabbit