Skip to content

Fail the Artifact Registry image cleanup when gcloud fails - #4143

Merged
anth-volk merged 1 commit into
masterfrom
fix/artifact-registry-cleanup-errors
Oct 30, 2026
Merged

anth-volk merged 1 commit into
masterfrom
fix/artifact-registry-cleanup-errors

Conversation

@anth-volk

@anth-volk anth-volk commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The cleanup-cloud-run-images job in push.yml runs .github/scripts/cleanup_cloud_run_images.sh after a successful production deploy. The script sent gcloud's output to /dev/null and exited 0 even when every deletion failed. Run 40968846055 (29 Oct 2026) logged Images: 123 (keeping newest 15), Deleting: 108, Deleted 0/108 image(s). and the job passed. The policyengine-api Artifact Registry repository is about 42 GB.

Changes:

  • Each failed deletion now prints Failed to delete <image>@<digest>: followed by gcloud's error output. If any deletion fails, the script ends with one ::error::Failed to delete N/M image(s) annotation and exits 1.
  • A failed gcloud artifacts docker images list now exits 1 with ::error::Could not list images in <repo>. Before this change, the listing ran inside a process substitution, and bash ignores a process substitution's exit status, so a failed listing was read as "0 images" and the job passed.
  • The newest-15 retention rule, DRY_RUN=1 and the target repository are unchanged. The workflow file is unchanged.

Cause of the failed deletions

  • Account: the job authenticates as github-deployment@policyengine-api.iam.gserviceaccount.com. The GCP_DEPLOY_SERVICE_ACCOUNT secret is masked in the log, but the Admin Activity audit log names this account as the caller for every delete request during the run (23:49–23:51 UTC).
  • Missing permissions: at the time of the run, the account's only Artifact Registry binding was roles/artifactregistry.writer, on the repository. There are no Artifact Registry bindings at the project, folder or organization level. That role has neither artifactregistry.tags.delete nor artifactregistry.versions.delete.
  • What gcloud does: gcloud artifacts docker images delete <image>@<digest> --delete-tags first calls DeleteTag for each tag on the image, then calls DeleteVersion (see DeleteDockerImage in googlecloudsdk/command_lib/artifacts/docker_util.py).
  • Exact error: the audit log has 92 DeleteTag calls from this run that were refused with status 7 (PERMISSION_DENIED), one per tagged image. For example: Permission 'artifactregistry.tags.delete' denied on resource '//artifactregistry.googleapis.com/projects/policyengine-api/locations/us-central1/repositories/policyengine-api/packages/policyengine-api/tags/cdc25d58…' (or it may not exist).
  • Untagged images: the other 16 images have no tags, so gcloud calls DeleteVersion directly, which needs artifactregistry.versions.delete. Those 16 calls left no audit entries, because Data Access audit logs are not enabled in this project. Their failure follows from the role's permission list; it was not observed in the logs.

IAM change (applied 30 Oct 2026, outside this PR)

For deletions to succeed, the deploy account needs artifactregistry.tags.delete and artifactregistry.versions.delete on the policyengine-api repository. These were granted at 16:40 UTC on 30 Oct 2026, using the predefined role scoped to this one repository:

gcloud artifacts repositories add-iam-policy-binding policyengine-api \
  --project=policyengine-api --location=us-central1 \
  --member=serviceAccount:github-deployment@policyengine-api.iam.gserviceaccount.com \
  --role=roles/artifactregistry.repoAdmin

The repository's IAM policy now binds the account to both roles/artifactregistry.repoAdmin and the existing roles/artifactregistry.writer. On top of writer, repoAdmin also allows files.delete, packages.delete, repositories.deleteArtifacts, rules.create/update/delete and versions.update. A custom role with only the two delete permissions would have been the narrower alternative.

The script on master already attempts the deletions. So the next production release run deletes every image older than the newest 15 (108 at the time of writing), whether or not this PR has merged. Cloud Run revisions that use those images will no longer be able to start. Automated rollback goes back to the revision that was serving just before the deploy, by revision name, and that image is always among the newest 15. The script header already records this trade-off. The 6 policyengine-api-app-engine images are not used: App Engine is disabled (USER_DISABLED), and its only version is stopped and uses an image in us.gcr.io.

Environment variables

The changed script reads these variables. None are new.

Variable Provider Wiring
KEEP, DRY_RUN Script defaults (15, 0) Not set by the workflow, so the defaults apply.
AR_LOCATION, AR_PROJECT, AR_REPO Script defaults from CLOUD_RUN_REGION, CLOUD_RUN_PROJECT, CLOUD_RUN_ARTIFACT_REPOSITORY Not set by the workflow.
CLOUD_RUN_REGION, CLOUD_RUN_PROJECT, CLOUD_RUN_ARTIFACT_REPOSITORY (and the other CLOUD_RUN_* defaults) cloud_run_set_defaults in cloud_run_env.sh No workflow overrides them for this job. They resolve to us-central1-docker.pkg.dev/policyengine-api/policyengine-api, which matches the run log and the repository the deploy jobs push to.
GITHUB_SHA, GITHUB_RUN_NUMBER GitHub Actions runner Read by cloud_run_set_defaults only to build tag names. The cleanup does not use them.
GCP_WORKLOAD_IDENTITY_PROVIDER, GCP_DEPLOY_SERVICE_ACCOUNT Repository-level GitHub Actions secrets (the production environment has no secrets with these names) Passed as with: inputs to google-github-actions/auth@v2 in the same job. The account's IAM policy grants roles/iam.workloadIdentityUser to principalSet://…/workloadIdentityPools/github/attribute.repository/PolicyEngine/policyengine-api. The audit log confirms the job authenticated as github-deployment@.
CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE, GOOGLE_APPLICATION_CREDENTIALS Exported by google-github-actions/auth@v2 Exported to $GITHUB_ENV, so later steps in the job see them. The run log shows both set.
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION, OLD_SIMULATION_GATEWAY_URL Workflow-level env: (a literal value, and a repository variable) Visible in the job; the script does not use them.

Testing

  • tests/unit/test_workflow_guard_scripts.py: replaced test_cleanup_counts_only_successful_deletions, which asserted exit 0 when every deletion failed, with test_cleanup_reports_each_failed_deletion_and_fails. Added test_cleanup_fails_when_the_listing_fails. The fake gcloud can now fail specific images and the listing.
  • Ran these by node ID: the two tests above plus the three existing cleanup tests that use the changed helper functions (test_cleanup_deletes_all_but_the_newest_images, test_cleanup_dry_run_deletes_nothing, test_cleanup_keeps_everything_within_the_limit). All 5 passed.
  • Ran the old script against the same fake gcloud: it exits 0 with Deleted 0/2 image(s)., and exits 0 with Images: 0 when the listing fails. The new script exits 1 in both cases.
  • python scripts/run_quality_guards.py (shellcheck over every script, and the other static guards), ruff format --check and ruff check on the test file: all pass.
  • Not run against GCP. After the IAM change, I re-read the repository's IAM policy to confirm the new binding. No deletion has been attempted since, so the next production release run is the first real test.

🤖 Generated with Claude Code

The cleanup job sent gcloud's output to /dev/null and exited 0 whether or
not any deletion worked. Run 40968846055 logged "Deleted 0/108 image(s)"
and passed: the deploy service account has roles/artifactregistry.writer,
which lacks artifactregistry.tags.delete and
artifactregistry.versions.delete.

Each failed deletion now prints gcloud's error, and the script exits 1
when any deletion fails. A failed listing also exits 1 instead of being
read as an empty repository, since bash ignores a process substitution's
exit status. The newest-15 retention is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.15%. Comparing base (6c4d53a) to head (c2ba478).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4143       +/-   ##
===========================================
+ Coverage   50.63%   88.15%   +37.52%     
===========================================
  Files         258      274       +16     
  Lines       14955    16393     +1438     
  Branches     2399     2689      +290     
===========================================
+ Hits         7573    14452     +6879     
+ Misses       6751     1271     -5480     
- Partials      631      670       +39     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@anth-volk
anth-volk merged commit 5b827c4 into master Oct 30, 2026
15 checks passed
@anth-volk
anth-volk deleted the fix/artifact-registry-cleanup-errors branch October 30, 2026 16:45

This branch was previously deployed

1 inactive deployment
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.

1 participant