Skip to content

Add pipeline artifact cleanup workflow - #65

Closed
isabelle-galleberg wants to merge 2 commits into
mainfrom
artifact-cleanup-schedule
Closed

isabelle-galleberg wants to merge 2 commits into
mainfrom
artifact-cleanup-schedule

Conversation

@isabelle-galleberg

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI lite review requested due to automatic review settings August 27, 2026 11:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new reusable GitHub Actions workflow to periodically clean up stale build artifacts in the shared “deployment delivery pipeline artifacts” S3 bucket, so artifact storage doesn’t grow without bounds across consuming repos.

Changes:

  • Introduces .github/workflows/deployment.artifact-cleanup-schedule.yml as a workflow_call-based cleanup workflow with configurable retention rules for trunk vs. non-trunk branches.
  • Implements artifact discovery via s3api list-objects-v2, branch attribution via uploaded object metadata (Metadata.tags), and deletion of stale artifacts.

Changed files: .github/workflows/deployment.artifact-cleanup-schedule.yml

.github/workflows/deployment.artifact-cleanup-schedule.yml

  • What changed: New reusable workflow that deletes stale .zip/.jar artifacts from the pipeline artifact bucket based on branch-aware retention rules.
  • Validation result: ⚠️ Warning
  • Issues found:
    • trunk-branches parsing doesn’t trim whitespace, so inputs like main, master will misclassify master as non-trunk (affects retention logic).
    • Deleting objects one-by-one with aws s3 rm can be very slow/throttle-prone at scale; batching via s3api delete-objects is more operationally robust.
Suppressed comments (1)

.github/workflows/deployment.artifact-cleanup-schedule.yml:147

  • Same whitespace issue as trunk parsing: trunk-branches is split on commas but not trimmed, so passing main, master causes master (with leading space) to be considered non-trunk and included in the per-branch retention loop.
            BRANCHES_IN_DIR=$(echo "$ENRICHED" | jq -r --arg dir "$DIR" --arg branches "$TRUNK_BRANCHES" \
              '($branches | split(",")) as $tb | [.[] | select(.dir == $dir) | select(.branch as $b | $tb | index($b) == null) | .branch] | unique | .[]')

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/deployment.artifact-cleanup-schedule.yml Outdated
Comment thread .github/workflows/deployment.artifact-cleanup-schedule.yml Outdated
Copilot AI review requested due to automatic review settings August 27, 2026 13:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

.github/workflows/deployment.artifact-cleanup-schedule.yml:87

  • TRUNK_KEEP / BRANCH_KEEP / BRANCH_DAYS are declared as number inputs but later used in bash arithmetic and jq slice indexes (e.g., CUTOFF_SECONDS=$(( BRANCH_DAYS * ... )) and .[${TRUNK_KEEP}:]), which will fail if a caller passes a non-integer (e.g. 30.5 or 5.0). Add an early integer validation (and fail fast with a clear error) before using these values.
          set -euo pipefail

          REPO_NAME_WITHOUT_OWNER="${REPO_NAME##*/}"
          echo "Cleaning up s3://${S3_BUCKET}/${REPO_NAME_WITHOUT_OWNER}/"

.github/workflows/deployment.artifact-cleanup-schedule.yml:126

  • ENRICHED is rebuilt by piping the entire JSON array through jq on every object (ENRICHED=$(echo "$ENRICHED" | jq ...)). This is O(n²) and will get slow (and potentially time out) when there are many artifacts. Prefer accumulating enriched items (e.g., write one JSON object per line to a temp file) and building the final array once with jq -s.
            ENRICHED=$(echo "$ENRICHED" | jq --arg key "$KEY" --arg dir "$DIR" --arg branch "$BRANCH" --arg lm "$LAST_MODIFIED" \
              '. += [{"key": $key, "dir": $dir, "branch": $branch, "lastModified": $lm}]')

.github/workflows/deployment.artifact-cleanup-schedule.yml:118

  • This loop performs an aws s3api head-object call for every .zip/.jar key to read Metadata.tags. For repositories with many artifacts, that can significantly increase runtime and AWS API throttling risk. Consider reducing per-object calls (e.g., store branch in the key prefix going forward) or at least parallelizing the head-object lookups with a bounded concurrency to keep the scheduled workflow reliable.
            TAGS_RAW=$(aws s3api head-object --bucket "${S3_BUCKET}" --key "$KEY" \
              --query 'Metadata.tags' --output text 2>/dev/null || echo "")
            BRANCH=$(echo "$TAGS_RAW" | grep -oE '"[^"]+-branch"' | sed -E 's/^"//; s/-branch"$//' || true)

- Trim whitespace and drop empty entries when parsing trunk-branches,
  so "main, master" doesn't misclassify master as a branch
- Batch object deletion via s3api delete-objects (up to 1000 keys per
  call) instead of one aws s3 rm per object, to avoid slow runs and
  API throttling when many artifacts are stale
@isabelle-galleberg
isabelle-galleberg force-pushed the artifact-cleanup-schedule branch from 49ec9e6 to 4b97ab9 Compare August 27, 2026 13:50
Copilot AI review requested due to automatic review settings August 27, 2026 13:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants