Skip to content

fix(ci): request a role that grants storage.buckets.get, and allow a manual retry - #124

Merged
sgardoll merged 2 commits into
mainfrom
fix/runner-sa-storage-role
Oct 5, 2026
Merged

sgardoll merged 2 commits into
mainfrom
fix/runner-sa-storage-role

Conversation

@sgardoll

@sgardoll sgardoll commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

The doc was wrong a third time

#123 documented roles/storage.objectAdmin for the source bucket. It doesn't work, and I verified why rather than guessing again:

$ gcloud iam roles describe roles/storage.objectAdmin --format="value(includedPermissions)" | tr ';' '\n'
... 32 permissions, every one of them storage.objects.*, storage.folders.*,
    storage.managedFolders.* — no storage.buckets.* at all

$ # and the workflow, after the grant was applied:
ERROR: ... does not have storage.buckets.get access to the Google Cloud Storage bucket

The deploy reads the bucket's own metadata before it writes a single object, so objectAdmin alone can never satisfy it.

The fix

  • roles/storage.legacyBucketReader supplies storage.buckets.get — the narrow pairing with the existing objectAdmin.
  • roles/storage.admin has both buckets.get and buckets.list if you'd rather grant once.
  • Documents that objectAdmin explicitly is not sufficient, so the next person doesn't repeat this.

Also adds workflow_dispatch to deploy-runner.yml. It has now failed twice, and the only way to retry was pushing a no-op commit to main — a bad habit to bake into the branch that deploys production.

Docs + workflow only.

To unblock the automated deploy

gcloud storage buckets add-iam-policy-binding gs://run-sources-low-code-connect-us-west1 \
  --member="serviceAccount:github-action@low-code-connect.iam.gserviceaccount.com" \
  --role="roles/storage.legacyBucketReader"

Nothing is blocked by this — the runner ships fine via ./scripts/deploy_cloud_run_ffai.sh, which is what is live now.


Devin Review

…manual retry

storage.objectAdmin was documented as sufficient for the source bucket. It is
not: it grants object permissions only and contains no storage.buckets.* at all,
while the deploy reads the bucket's metadata before it writes anything. Verified
against the live IAM role definitions - objectAdmin has 32 permissions, none of
them buckets.get, and the workflow still fails on exactly that after the grant.

legacyBucketReader supplies storage.buckets.get, so the pair is the narrow fix;
storage.admin (which has buckets.get and buckets.list) covers both in one.

Also adds workflow_dispatch. A deploy can fail for reasons outside the code - a
missing grant, a Cloud Build outage - and until now the only way to retry was to
push a no-op commit to main, which is a bad thing to teach on the branch that
deploys production.

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 1 potential issue.

Devin Review

# Deploys can fail for reasons outside the code (a missing IAM grant, a Cloud
# Build outage). Without this the only way to retry was to push a no-op
# commit, which is worse than useless on the branch that deploys production.
workflow_dispatch:

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.

🔴 Manual retry can deploy unmerged runner code

When an operator selects another branch, workflow_dispatch deploys that branch's runner to production. Checkout uses the selected ref, so the retry can replace the main revision with unmerged code.

Learn more

A manual workflow run lets the operator choose a branch. Actions checkout uses that selected ref by default, and deploy script sends the checked-out cloud-run/ffai-runner source to the production service. The new trigger has no branch restriction, so choosing a feature branch deploys its unmerged runner instead of retrying the production revision.

Example: An operator selects fix/runner-experiment in Run workflow after a failed main deploy. The workflow checks out that branch and publishes its runner, even though main still contains the production-approved source.

Recommended fix: Restrict this job to refs/heads/main, or explicitly check out main for manual retries. If manual runs from other branches are needed, route them to a separate non-production target.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

…oles

Recommendation three kept being wrong because I reasoned from role names instead
of the permission lists. Measured: objectAdmin has 32 permissions, none of them
storage.buckets.*; legacyBucketReader supplies buckets.get but not buckets.list;
a source deploy needs buckets.get, buckets.list and objects.create. Only
storage.admin supplies all three, so the failures arrive one at a time and each
one looks like the previous guess was simply the wrong role.
@sgardoll
sgardoll merged commit d9dfff9 into main Oct 5, 2026
7 checks passed
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