Remove old policy bindings so that setup-project is reentrant #73
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: davidz627 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
deploy/setup-project.sh
Outdated
| @@ -10,10 +10,11 @@ if [ -f $SA_FILE ]; then | |||
| rm "$SA_FILE" | |||
| fi | |||
| gcloud iam service-accounts delete "$IAM_NAME" --quiet || true | |||
| # TODO: Delete ALL policy bindings | |||
| gcloud projects remove-iam-policy-binding "${PROJECT}" --member serviceAccount:"${IAM_NAME}" --role roles/compute.admin --quiet || true | |||
There was a problem hiding this comment.
With this change, can we reduce the compute.admin role?
There was a problem hiding this comment.
good point, I will look into it
davidz627
force-pushed
the
fix/projectBindingDelete
branch
from
42d93f1 to
b5a0f14
Compare
k8s-ci-robot
added
size/M
role scopes and added custom role for necessary extras
davidz627
force-pushed
the
fix/projectBindingDelete
branch
from
b5a0f14 to
3499c3a
Compare
|
@msau42 updated so that roles are fairly minimal |
deploy/setup-project.sh
Outdated
| # Create or Update Custom Role | ||
| if gcloud iam roles describe gcp_compute_persistent_disk_csi_driver_custom_role --project "${PROJECT}"; | ||
| then | ||
| yes | gcloud iam roles update gcp_compute_persistent_disk_csi_driver_custom_role \ |
There was a problem hiding this comment.
does the -q option not work for suppressing interactive mode?
|
|
||
| # Create new Service Account and Keys | ||
| gcloud iam service-accounts create "${GCEPD_SA_NAME}" | ||
| gcloud iam service-accounts keys create "${SA_FILE}" --iam-account "${IAM_NAME}" | ||
| gcloud projects add-iam-policy-binding "${PROJECT}" --member serviceAccount:"${IAM_NAME}" --role roles/compute.admin |
There was a problem hiding this comment.
your role-binding teardown logic will not be backwards compatible anymore. maybe it needs to somehow iterate through all the rolebindings the SA has, and delete them
deploy/setup-project.sh
Outdated
| done | ||
| # Delete ALL EXISTING Bindings | ||
| gcloud projects get-iam-policy "${PROJECT}" --format json > "${PKGDIR}/deploy/iam.json" | ||
| sed -i "/${IAM_NAME}/d" "${PKGDIR}/deploy/iam.json" |
There was a problem hiding this comment.
Could this accidentally match a substring of the IAM_NAME?
Say you had something called "foobar", and your IAM_NAME was "bar"
There was a problem hiding this comment.
good catch, amended with prefix serviceAccount:
davidz627
force-pushed
the
fix/projectBindingDelete
branch
from
d3f87c8 to
6ad9a4b
Compare
deploy/setup-project.sh
Outdated
| # Delete ALL EXISTING Bindings | ||
| gcloud projects get-iam-policy "${PROJECT}" --format json > "${PKGDIR}/deploy/iam.json" | ||
| sed -i "/serviceAccount:${IAM_NAME}/d" "${PKGDIR}/deploy/iam.json" | ||
| gcloud projects set-iam-policy "${PROJECT}" "${PKGDIR}/deploy/iam.json" |
There was a problem hiding this comment.
hmm although in general I find this kind of scary because it sets all the rolebindings in your entire project to the .json, not just this particular service account. If someone is also modifying the iam policies at the same time, this could override their changes.
There was a problem hiding this comment.
theres an etag that is used for consistency. gcloud projects set-iam-policy will fail if the etag is out of date. I haven't written in any failure handling logic. It shouldn't be too difficult to just run the script again.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
…ncy-openshift-4.19-ose-gcp-pd-csi-driver OCPBUGS-45567: Updating ose-gcp-pd-csi-driver-container image to be consistent with ART for 4.19
Have the
setup-project.shscript remove old policy bindings so that they can be recreated successfully.Part of: #73
/assign @msau42