-
Notifications
You must be signed in to change notification settings - Fork 827
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
audit: update as of 2021-07-16 #2322
Conversation
Hi @cncf-ci. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
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.
/ok-to-test
Not merging because I'm not sure what I want to do about the magic IAM changes just yet
@@ -10,7 +10,8 @@ | |||
}, | |||
{ | |||
"members": [ | |||
"projectViewer:k8s-release" | |||
"projectViewer:k8s-release", | |||
"serviceAccount:project-304687256732@storage-transfer-service.iam.gserviceaccount.com" |
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.
Thanks for the free unannounced IAM change https://cloud.google.com/storage-transfer
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.
https://cloud.google.com/storage-transfer/docs/configure-access
Storage Transfer Service uses a Google-managed service account to move your data. If you create a transfer from Google Cloud Console and have permissions to update IAM policies for Cloud Storage resources, then transfers created from Google Cloud Console automatically grant the Google-managed service account used by Storage Transfer Service the required permissions for the transfer.
So that's what happened when I did #1569 (comment)
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.
You know what's neat? I can't actually find the code that provisions this bucket. Not in git history either. I think this was manually created (who knows, maybe by me to prevent name-squatting)
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.
gsutil iam ch -d serviceAccount:project-304687256732@storage-transfer-service.iam.gserviceaccount.com:roles/storage.legacyBucketReader gs://k8s-release
@@ -25,7 +26,8 @@ | |||
"members": [ | |||
"group:[email protected]", | |||
"group:[email protected]", | |||
"group:[email protected]" | |||
"group:[email protected]", | |||
"serviceAccount:project-304687256732@storage-transfer-service.iam.gserviceaccount.com" |
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.
Same thing, but now I'm a little more annoyed, since this is a higher-blast-radius role and I didn't setup a job that required all of it.
https://cloud.google.com/storage-transfer/docs/iam-transfer#sink-permissions says we could use roles/storage.legacyBucketWriter
instead, but I'm trying to avoid that role (ref: #2007)
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.
gsutil iam ch -d serviceAccount:project-304687256732@storage-transfer-service.iam.gserviceaccount.com:roles/storage.objectAdmin gs://k8s-release
audit/projects/k8s-release/iam.json
Outdated
@@ -50,6 +50,12 @@ | |||
], | |||
"role": "roles/editor" | |||
}, | |||
{ | |||
"members": [ | |||
"serviceAccount:[email protected]" |
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.
Near as I can tell this is a static service account for "Transfer Appliance" which... this isn't transferring to or from on-prem storage, so what gives?
I'm inclined to revoke this
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.
gcloud projects remove-iam-policy-binding k8s-release --member=serviceAccount:[email protected] --role=roles/pubsub.editor
a7b3712
to
7e4083e
Compare
a7c81b3
to
d3fc515
Compare
85f3691
to
9cce6f5
Compare
cb22812
to
1a0efd6
Compare
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.
Some manual things to resolve before accepting these changes
@@ -3,6 +3,7 @@ | |||
{ | |||
"members": [ | |||
"group:[email protected]", | |||
"group:[email protected]", |
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.
Expected, all the IAM changes to the k8s-release-dev
buckets are part of #2333
@@ -0,0 +1 @@ | |||
{"logBucket": "k8s-infra-artifacts-gcslogs", "logObjectPrefix": "k8s-release-pull"} |
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.
Expected, all the logging changes to k8s-release-dev
et al are part of #2333
NAME TITLE | ||
cloudkms.googleapis.com Cloud Key Management Service (KMS) API | ||
NAME TITLE | ||
cloudbuild.googleapis.com Cloud Build API | ||
cloudkms.googleapis.com Cloud Key Management Service (KMS) API | ||
containerregistry.googleapis.com Container Registry API | ||
logging.googleapis.com Cloud Logging API | ||
pubsub.googleapis.com Cloud Pub/Sub API | ||
secretmanager.googleapis.com Secret Manager API | ||
storage-api.googleapis.com Google Cloud Storage JSON API | ||
storage-component.googleapis.com Cloud Storage |
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.
So this is fun. FYI @kubernetes/release-engineering
When running ./infra/gcp/ensure-release-projects.sh
to deploy #2333, a whole bunch of other stuff was created.
The problem is that I put k8s-releng-prod
under release
in infra/gcp/infra.yaml, thinking that it's effectively the same scope/group of people, and so ensure-release-projects.sh
provisioned it the same as k8s-release
I will manually undo all of this, and can setup a separate releng
group, but maybe now is a good time to ask if it's still necessary to have k8s-releng-prod
as its own special-purpose project?
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.
Discussion in https://kubernetes.slack.com/archives/CJH2GBF7Y/p1626374201181100 led to: going to keep k8s-releng-prod as its own special purpose project, since there are google.com projects that are tangled up with it that are not nearly as easy to change as stuff here
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.
I ran the following to undo the oops:
#!/usr/bin/env bash
# buckets
gsutil rb gs://k8s-releng-prod
gsutil rb gs://k8s-releng-prod-gcb
gsutil rb gs://artifacts.k8s-releng-prod.appspot.com/
# services
gcloud services disable --force cloudbuild.googleapis.com --project=k8s-releng-prod
gcloud services disable --force containerregistry.googleapis.com --project=k8s-releng-prod
gcloud services disable --force logging.googleapis.com --project=k8s-releng-prod
gcloud services disable --force pubsub.googleapis.com --project=k8s-releng-prod
gcloud services disable --force secretmanager.googleapis.com --project=k8s-releng-prod
gcloud services disable --force storage-api.googleapis.com --project=k8s-releng-prod
gcloud services disable --force storage-component.googleapis.com --project=k8s-releng-prod
# project iam bindings
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=serviceAccount:[email protected] --role=roles/cloudbuild.builds.builder
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=serviceAccount:[email protected] --role=roles/cloudbuild.builds.builder
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=group:[email protected] --role=roles/cloudbuild.builds.editor
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=group:[email protected] --role=roles/cloudbuild.builds.editor
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=serviceAccount:[email protected] --role=roles/cloudbuild.serviceAgent
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=serviceAccount:[email protected] --role=roles/containerregistry.ServiceAgent
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=serviceAccount:[email protected] --role=roles/pubsub.serviceAgent
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=group:[email protected] --role=roles/serviceusage.serviceUsageConsumer
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=group:[email protected] --role=roles/serviceusage.serviceUsageConsumer
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=group:[email protected] --role=roles/viewer
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=group:[email protected] --role=roles/viewer
gcloud projects remove-iam-policy-binding k8s-releng-prod --member=group:[email protected] --role=roles/viewer
I verified locally by running audit for k8s-releng-prod and verifying there were no differences compared to the current files in HEAD
$ make -C images/k8s-infra TAG=latest run WHAT="./audit/audit-gcp.sh k8s-releng-prod"
$ git diff
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.
#2349 will prevent this from happening again
"currentNodeVersion": "1.18.17-gke.1901 *", | ||
"currentNodeVersion": "1.19.9-gke.1900", |
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.
aaa is at 1.19 now 🎉
@spiffxp: GitHub didn't allow me to request PR reviews from the following users: tylerferrara. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
OK I've manually reverted the things I wasn't comfortable accepting, I'm going to wait for an update to this PR to confirm. Should see many fewer changes |
I triggered a re-run of the most recent job: https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-k8sio-audit/1415815857518415872 Should see an update in the next 30-50 minutes or so |
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.
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cncf-ci, spiffxp 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 |
Audit Updates wg-k8s-infra