-
Notifications
You must be signed in to change notification settings - Fork 505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure GCS release bucket has public-read defacl #206
Conversation
@@ -378,6 +379,19 @@ release::gcs::ensure_release_bucket() { | |||
if ! $GSUTIL ls "gs://$bucket" >/dev/null 2>&1 ; then | |||
logecho -n "Creating Google Cloud Storage bucket $bucket: " | |||
logrun -s $GSUTIL mb -p "$GCLOUD_PROJECT" "gs://$bucket" || return 1 | |||
logecho -n "Adding public-read default ACL on bucket $bucket: " | |||
local current_defacl=$(gsutil defacl get "gs://$bucket") || return 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer local definitions up top. Less clutter in the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/definitions/declarations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
echo "$current_defacl" | jq '. + [{"entity": "allUsers", "role": "READER"}]' \ | ||
> "$new_acl_file" || return 1 | ||
logrun -s $GSUTIL defacl set "$new_acl_file" "gs://$bucket" || return 1 | ||
rm -f "$new_acl_file" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be logrun'd (no -s) to capture in the log.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
We do this rather than copying artifacts with the public-read ACL, since doing so will remove any owner ACLs.
$ gsutil defacl get gs://kubernetes-release | jq -r '.[] | select(.entity == "allUsers") | .role'
READER
$ gsutil defacl get gs://kubernetes-release-dev | jq -r '.[] | select(.entity == "allUsers") | .role'
READER
$ gsutil defacl get gs://kubernetes-release-pull | jq -r '.[] | select(.entity == "allUsers") | .role'
READER
$ gsutil defacl get gs://kubernetes-federation-release | jq -r '.[] | select(.entity == "allUsers") | .role'
AccessDeniedException: 403 Forbidden
$ gsutil defacl get gs://kubernetes-federation-release-1-4 | jq -r '.[] | select(.entity == "allUsers") | .role'
AccessDeniedException: 403 Forbidden
$ gsutil defacl get gs://kubernetes-federation-release-1-5 | jq -r '.[] | select(.entity == "allUsers") | .role'
AccessDeniedException: 403 Forbidden @madhusudancs can you check the federation buckets as above to see if the defacls are OK? @david-mcmahon @saad-ali @jessfraz anago mock mode may fail once this is merged, since the gsutil defacl ch -u AllUsers:R "gs://kubernetes-release-$USER" EDIT 2016-11-15 18:17: shorter command |
@madhusudancs never mind, I was auth'd to the wrong account. $ gsutil defacl get gs://kubernetes-federation-release | jq -r '.[] | select(.entity == "allUsers") | .role'
$ gsutil defacl get gs://kubernetes-federation-release-1-4 | jq -r '.[] | select(.entity == "allUsers") | .role'
$ gsutil defacl get gs://kubernetes-federation-release-1-5 | jq -r '.[] | select(.entity == "allUsers") | .role' I will fix. Interestingly, this probably explains why kubernetes/test-infra#990 only affected the federation release: the other release buckets have a public-read defacl set, so the |
All fixed. Merging. |
I hate Jenkins.
FYI @apelisse |
@ixdy interesting. This PR makes the ACLs for federation buckets consistent with everything else right? |
@madhusudancs I think the owner settings on the federation buckets is still different. |
@ixdy Oh! I looked at the owner settings of |
Daily checkpoint for almost-final 1.11 release notes
We do this rather than copying artifacts with the public-read ACL,
since doing so will remove any owner ACLs.
x-ref #195.
I haven't checked all of the Jenkins GCS buckets yet, so this may likely break some builds until those buckets are fixed.
cc @zmerlynn @rmmh @david-mcmahon @saad-ali