ensure k8s-conform service accounts have objectAdmin rights #1890
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
approved
specifically - add secret_full_name: generates "projects/p/secrets/s" - add ensure_secret: creates a secret if it doesn't already exist - add ensure_seviceaccount_key_secret: if a secret doesn't exist, create it and pre-populate with a newly created serviceaccount key
specifically - reorganize into functions - use lib_gsm functions - redo to ensure all resources (except one-time private key provisioning) - switch from "repo" to "offering" as the term for what we're provisioning for, borrowed from CNCF's conformance language - prevent provisioning of unrecognized offerings
spiffxp
force-pushed
the
redo-ensure-conformance
branch
from
0360438 to
5890959
Compare
|
Ran |
spiffxp
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
It looks like this decided to create new keys for two service accounts, because neither of these had corresponding secrets provisioned beforehand for email in $(gcloud iam service-accounts list --project=k8s-conform --filter='email ~ "iam.gserviceaccount.com"' --format='value(email)'); do
echo "${email}"...;
gcloud iam service-accounts keys list --projct=k8s-conform --iam-account=${email} --managed-by=user;
doneThe two accounts that have new keys were created before 2020-05-28, when #873 added the provision-to-secret pattern. The previous code gated creating new secrets on service account existence, where the code now gates on secret existence. I opened #1895 as a followup issue to get the key users to switch to the new keys, but I don't consider it a big security risk to leave both keys enabled. |
|
#1892 reflects basically all of the changes these scripts caused. It was too early to capture the latest run I did in #1890 (comment). I'll point those changes out in the next audit PR |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: saschagrunert, 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 |
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
/hold cancel /assign @thockin |
k8s-ci-robot
removed
the
do-not-merge/hold
|
Aaron, your bash is so pleasant to read that I'm going to invite you to collab on my bash-only rewrite of Kubernetes. /lgtm |
|
|
||
| local project="${1}" | ||
| local secret="${2}" | ||
| local serviceaccount="${3}" |
| # $3: The service-account (e.g. "[email protected]") | ||
| function ensure_serviceaccount_key_secret() { | ||
| if [ ! $# -eq 3 -o -z "$1" -o -z "$2" -o -z "$3" ]; then | ||
| echo "ensure_serviceaccount_key_secret(project, secret, serviceaccountt) requires 3 arguments" >&2 |
There was a problem hiding this comment.
email instead of serviceaccountt
This ensures that for each conformance bucket
k8s-conform-foo, the same write privileges are given to:group:[email protected]serviceAccount:[email protected]The code before this PR looked like it would probably do this for any newly provisioned GCS bucket. However
ensure-conformance-storage.shwas written in a way that it wasn't always refreshing resources and IAM bindings. So I rewrote it to refresh as much as possible.While I was here I fixed a few other bugs in the course of troubleshooting / verifying this refactor:
ensure-conformance-storage.shcan no longer provision arbitrary buckets if invoked with a typo'ed argumentlib_iam.shdoesn't crash on empty IAM policiesK8S_INFRA_DEBUG=true ./ensure-whatever.shwill list and leave behind the contents of$TMPDIRinstead of removing itlib*.shis no longer executable (these files should be sourced, not executed)Fixes #1850