docs: clarify gcloud release upload permissions - #14
ibetitsmike wants to merge 2 commits into
Conversation
Use direct authenticated object PUTs instead of gcloud cp, which probes new destinations with a bucket listing. Preserve generation-zero preconditions, verify transport MD5, and upload the manifest last. > Xum prepared this change on behalf of @ibetitsmike. Signed-off-by: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com>
ibetitsmike
changed the title
| for artifact in artifacts/mutagen-* artifacts/SHA256SUMS; do | ||
| content_md5="$(openssl dgst -md5 -binary "$artifact" | openssl base64 -A)" | ||
| curl --fail-with-body --silent --show-error \ | ||
| --header "Authorization: Bearer $GCLOUD_ACCESS_TOKEN" \ | ||
| --header "Content-Type: application/octet-stream" \ | ||
| --header "x-goog-if-generation-match: 0" \ | ||
| --header "Content-MD5: $content_md5" \ | ||
| --upload-file "$artifact" \ | ||
| "https://storage.googleapis.com/coder-desktop/mutagen/$RELEASE_TAG/${artifact##*/}" | ||
| printf 'Uploaded gs://coder-desktop/mutagen/%s/%s\n' "$RELEASE_TAG" "${artifact##*/}" | ||
| done |
There was a problem hiding this comment.
why? I'd so much rather use gcloud here
There was a problem hiding this comment.
I’d prefer gcloud too if we can preserve the same permissions. The original live run failed on storage.objects.list before uploading anything.
The SDK 575.0.1 localhost reproduction noted under this PR’s Validation section also tried an exact destination name with --if-generation-match=0: object GET → 404 → bucket LIST with a prefix filter. The SDK treats a missing exact name as a possible directory. --no-clobber does not bypass that destination-discovery path according to source inspection; the saved probe used the generation precondition, so I’m not claiming a separately recorded --no-clobber run.
The uploader intentionally has create-only permission scoped to mutagen/. Bucket listing is not restricted by that object-prefix condition, so granting it would broaden access. Direct named-object PUT avoids discovery, with generation zero preventing overwrites, Content-MD5 validating transfer integrity, and SHA256SUMS last. The existing eight local tests cover those safeguards and failure paths; the live run on this head succeeded, and all six public downloads matched GitHub byte-for-byte.
Happy to switch back with a demonstrated list-free invocation, or a separately authorized IAM design change.
Xum replied on behalf of @ibetitsmike.
There was a problem hiding this comment.
we can, but we need to expand the gcp scope to be able to list the bucket (cli needs it to make sure there are no duplicates)
There was a problem hiding this comment.
Agreed. Restored gcloud storage cp in 114c024f and opened coder/gcp#233 for the missing bucket read/list permissions. The workflow is now identical to main, so this PR's net diff is the permissions documentation.
The IAM follow-up adds Mutagen-only bucket-level roles/storage.objectViewer, keeps writes restricted to mutagen/, and leaves public access and delete permissions unchanged. Generation-zero overwrite protection and manifest-last ordering remain. Local gcloud tests passed; no IAM apply or live promotion was run.
Leaving this thread open for your confirmation.
Xum replied on behalf of @ibetitsmike.
Signed-off-by: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com>
ibetitsmike
changed the title
2 participants
Why
Use
gcloud storage cpas requested in the review discussion, rather than maintaining direct curl uploads. The original failure was caused by missing bucket-list permission. coder/gcp#233 adds the reader binding as a follow-up to coder/gcp#229.Changes
main: standard Google auth/setup actions, generation-zero preconditions, full checksum-manifest verification, and a separateSHA256SUMSupload after payloads succeed.roles/storage.objectVieweraccess alongside the existing prefix-scopedroles/storage.objectCreatorbinding.mutagen/, with no delete permission. Public access is unchanged.Because main already uses gcloud, this PR's net diff is now documentation only. Apply coder/gcp#233 before the next promotion. A failed gcloud batch can leave some payloads uploaded, but the manifest is withheld; the documented operator cleanup remains necessary.
Validation
origin/mainverified by Git blob hash.Delivery record
114c024f6aa1ab95bb7bae67d45b7d7b11bc5a32. No history rewrite. DCO-signed and locally verified SSH signature; GitHub reportsunknown_key, as it did for the previous head.87aa6c1fd0reviewed the exact source blobs now committed in this head (workflowaaeb657; release docs744a3b4) and the companion GCP diff. No material findings or deferred code issues. The seven-test pass was completed after that advisory.262ba17and the successful live v0.18.4 promotion covered the superseded curl implementation only. They do not validate the new gcloud/IAM path.