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

empower KSA used in the promoter's prow jobs to use Workload Identity #655

Merged
merged 4 commits into from
Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions infra/gcp/ensure-prod-storage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -220,4 +220,9 @@ empower_group_to_admin_artifact_auditor \
empower_artifact_auditor "${PROD_PROJECT}"
empower_artifact_auditor_invoker "${PROD_PROJECT}"

# Special case: empower Prow account to have access to GCR for Workload Identity.
empower_prowacct_for_workload_identity \
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this should be more like:

  • empower_pods_to_identify_as_gcp_service_account
  • bind_k8s_service_account_to_act_as_gcp_service_account

"k8s-prow-builds.svc.id.googtest-pods/k8s-artifacts-prod" \
Copy link
Contributor

Choose a reason for hiding this comment

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

k8s-prow-builds.svc.id.goog[test-pods/k8s-artifacts-prod] (you need the [])

$(svc_acct_email "${PROD_PROJECT}" "${PROMOTER_SVCACCT}")

color 6 "Done"
19 changes: 19 additions & 0 deletions infra/gcp/lib_gcr.sh
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,22 @@ function empower_svcacct_to_admin_gcr () {

empower_svcacct_to_admin_gcs_bucket "${group}" "${bucket}"
}

Copy link
Member

Choose a reason for hiding this comment

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

First note - this does not belong in this file. This file just for very low-level GCR-centric primitives. Put this back in lib.sh until that can be broken up more.

# Grant the Prow service account access to the same perms as the given GCP
# service account (for Workload Identity).
# $1: The Prow service account being empowered
# $2: The GCP service account to draw powers from
function empower_prowacct_for_workload_identity() {
if [ ! $# -eq 2 -o -z "$1" -o -z "$2" ]; then
echo "empower_prowacct_for_workload_identity(prow_acct, gcp_acct) requires 2 arguments" >&2
return 1
fi

prow_acct="$1"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd consider separating out these variables into their parts so its more clear, I'd recommend this:

gke_project=$1
namespace=$2
name=$3
gcp_acct=$4
gcp_project=$5

gcloud iam service-accounts add-iam-policy-binding \
  --role roles/iam.workloadIdentityUser \
  --member "serviceAccount:${gke_project}.svc.id.goog[${namespace}/${name}]" \
  "$gcp_acct" --project "$gcp_project"

Copy link
Member

Choose a reason for hiding this comment

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

I don;t think we need to go quite this far.

I would position this function as empower_ksa_to_svcacct(project, k8s_svcacct, gcp_svcacct)

gcp_acct="$2"

gcloud iam service-accounts add-iam-policy-binding \
Copy link
Member

Choose a reason for hiding this comment

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

you need --project, don't you?

--role roles/iam.workloadIdentityUser \
--member "serviceAccount:${prow_acct}" \
"${gcp_acct}"
}