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

infra/gcp/prod-storage: fix ensure-prod-storage.sh #1998

Merged
merged 3 commits into from
May 4, 2021

Conversation

spiffxp
Copy link
Member

@spiffxp spiffxp commented May 4, 2021

This is part of #1730 and an attempt to unblock deploying #1966 (comment)

The first two commits are mechnical refactoring that was easy enough to do while here:

  • pull blocks of related code into functions, add a main function, call the functions in main
  • fix shellcheck nits my editor was displaying to me

The actual surgical fix is the third commit:

  • ensure the auditor service accounts are created before trying to create an IAM policy binding on them

I would like to redo the logic in this script to be less hidden in lib.sh but will save that for a future PR

spiffxp added 3 commits May 4, 2021 16:49
Ensure the auditor service accounts are created _before_ attempting to
set an iam policy binding on the service accounts.

I will save redoing this to pull out the logic hidden in lib.sh for a
future PR
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 4, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested review from nikhita and thockin May 4, 2021 21:01
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 4, 2021
@spiffxp
Copy link
Member Author

spiffxp commented May 4, 2021

/cc @hh @ameukam @dims
Since you were involved in the PR this is intended to unblock

@k8s-ci-robot k8s-ci-robot requested review from ameukam, dims and hh May 4, 2021 21:04
@ameukam
Copy link
Member

ameukam commented May 4, 2021

/lgtm
/hold
hold cancel when you're ready to deploy.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 4, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2021
@spiffxp
Copy link
Member Author

spiffxp commented May 4, 2021

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 4, 2021
@spiffxp
Copy link
Member Author

spiffxp commented May 4, 2021

Ran ./infra/gcp/ensure-prod-storage.sh

The relevant changes as a result of this PR are:

Empowering artifact auditor
  Created service account [k8s-infra-gcr-auditor].
  Updated IAM policy for project [k8s-artifacts-prod].
  @@ -16,6 +16,8 @@
     role: roles/editor
   - member: serviceAccount:k8s-infra-gcr-auditor-invoker@k8s-artifacts-prod.iam.gserviceaccount.com
     role: roles/run.invoker
  +- member: serviceAccount:[email protected]
  +  role: roles/logging.logWriter
   - member: serviceAccount:k8s-infra-gcr-vuln-dashboard@k8s-artifacts-prod.iam.gserviceaccount.com
     role: roles/containeranalysis.occurrences.viewer
   - member: serviceAccount:[email protected]
  Updated IAM policy for project [k8s-artifacts-prod].
  @@ -16,6 +16,8 @@
     role: roles/editor
   - member: serviceAccount:k8s-infra-gcr-auditor-invoker@k8s-artifacts-prod.iam.gserviceaccount.com
     role: roles/run.invoker
  +- member: serviceAccount:[email protected]
  +  role: roles/errorreporting.writer
   - member: serviceAccount:[email protected]
     role: roles/logging.logWriter
   - member: serviceAccount:k8s-infra-gcr-vuln-dashboard@k8s-artifacts-prod.iam.gserviceaccount.com
  Updated IAM policy for service [cip-auditor].
  bindings:
  - members:
    - serviceAccount:k8s-infra-gcr-auditor-invoker@k8s-artifacts-prod.iam.gserviceaccount.com
    role: roles/run.invoker
  etag: BwXBh4-2Bag=
  version: 1
  Empowering artifact-admins to release prod auditor
  Updated IAM policy for serviceAccount [[email protected]].
  bindings:
  - members:
    - group:[email protected]
    role: roles/iam.serviceAccountUser
  etag: BwXBh5ADynw=
  version: 1

This run also picked up changes from #1966

  @@ -6,6 +6,8 @@
     role: roles/storage.legacyBucketOwner
   - member: group:[email protected]
     role: roles/storage.objectAdmin
  +- member: group:[email protected]
  +  role: roles/storage.objectViewer
   - member: projectEditor:k8s-artifacts-prod
     role: roles/storage.legacyBucketOwner
   - member: projectEditor:k8s-artifacts-prod

@k8s-ci-robot k8s-ci-robot merged commit 604edd8 into kubernetes:main May 4, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone May 4, 2021
@spiffxp spiffxp deleted the fix-ensure-prod-storage branch May 4, 2021 21:37
@spiffxp
Copy link
Member Author

spiffxp commented Jun 11, 2021

Related to refactoring infra/gcp, ref: #516

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants