Skip to content
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

gcp/lib: set builder permission instead of editor #1773

Closed
wants to merge 1 commit into from

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented Mar 10, 2021

The role roles/cloudbuild.builds.editor looks like does not have all the necessary permissions to push images from gcb (https://cloud.google.com/build/docs/iam-roles-permissions)

the builder role (roles/cloudbuild.builds.builder) have the permission (storage.buckets.get) to make it able to push the images.

I'm not sure if this is the correct way to deal or we add another role just to add the storage.buckets.get permission.

/assign @spiffxp

Fixes: #1772

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 10, 2021
@k8s-ci-robot k8s-ci-robot requested review from nikhita and thockin March 10, 2021 10:44
@k8s-ci-robot k8s-ci-robot added wg/k8s-infra size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 10, 2021
Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

I'm honestly not sure myself, seems ok but I will give this a shot tomorrow. Want to make sure we don't break other staging builds.

If you move this to just the special case for your staging repo I will approve and deploy today.

Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

Changed my mind, responded in linked issue

@@ -215,7 +215,7 @@ function empower_group_for_gcb() {
gcloud \
projects add-iam-policy-binding "${project}" \
--member "group:${group}" \
--role roles/cloudbuild.builds.editor
--role roles/cloudbuild.builds.builder
Copy link
Member

Choose a reason for hiding this comment

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

I think... that this isn't what we want. This role should be what the cloud build SA uses when it runs a build you trigger. As someone who wants to run builds I'm not sure you need permissions on more than build resources.

Copy link
Member

Choose a reason for hiding this comment

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

Right, the GCB SA needs access to write to GCR, not the operator.
The operator only needs access to execute the GCB job.

Copy link
Member Author

Choose a reason for hiding this comment

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

got it will close this and see other alternatives

/close

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cpanato
To complete the pull request process, please assign spiffxp after the PR has been reviewed.
You can assign the PR to them by writing /assign @spiffxp in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot
Copy link
Contributor

@cpanato: Closed this PR.

In response to this:

got it will close this and see other alternatives

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@cpanato cpanato deleted the GH-1772 branch March 11, 2021 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

failed to push image after GCB build
4 participants