Skip to content

Ensure gcs bucket for k8s-infra prow instance #1909

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.

Sign up for GitHub

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

Closed
ameukam wants to merge 1 commit into from
Closed

Ensure gcs bucket for k8s-infra prow instance #1909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 51 additions & 0 deletions infra/gcp/ensure-main-project.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ readonly TERRAFORM_STATE_BUCKET_ENTRIES=(
k8s-infra-tf-sandbox-ii:k8s-infra-ii-coop@kubernetes.io
)


#GCS buckets for k8s-infra-prow
readonly PROW_BUCKETS=(
k8s-infra-prow-results
)

# The services we explicitly want enabled for the main project
#
# NOTE: Expected services include dependencies of these services, which may be
Expand Down Expand Up @@ -169,6 +175,48 @@ function ensure_terraform_state_buckets() {
done
}

function ensure_prow_buckets() {
if [ $# -ne 1 ] || [ -z "$1" ]; then
echo "${FUNCNAME[0]}(gcp_project) requires 1 argument" >&2
return 1
fi

local project="${1}"

for bucket in "${PROW_BUCKETS[@]}"; do
local svc_acct_name="${bucket}-sa"
Copy link
Copy Markdown
Contributor

@spiffxp spiffxp May 19, 2021

Choose a reason for hiding this comment

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

I think you are going to want to reuse the existing service account(s) that prow jobs run as instead of creating a new service account

Copy link
Copy Markdown
Contributor

@spiffxp spiffxp May 19, 2021

Choose a reason for hiding this comment

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

Ideally this would be happening via workload identity but I think we're still living in a world where pod-utils are ultimately what write to GCS, and they still get their credentials from a kubernetes secret named "service-account", according prow's config.yaml: https://github.com/kubernetes/test-infra/blob/c1d43437727eab8c84c5ac989b278feefd556a7d/config/prow/config.yaml#L23

The way I did this for k8s-infra-prow-build-trusted/prow-build-trusted was to setup a service account named k8s-infra-prow-build-trusted bindable via workload identity using terraform. Then manually create a key and store in a kubernetes secret named service-account in the test-pods namespace, to match the name in prow's config.yaml.

Looking at it now, I think I should have set it up so the WI binding was [test-pods/default] instead of [test-pods/k8s-infra-prow-build-trusted], because I doubt anything is actually using it. And we can also use external secrets instead of manually creating the secret

So that's build clusters.

Then there's the prow control plane. For example, I know that at least crier will write to the same GCS bucket.

Currently, prow.k8s.io uses a control-plane GCP service account, and then kubernetes service accounts that can bind to it for each component, e.g. https://github.com/kubernetes/test-infra/blob/c1d43437727eab8c84c5ac989b278feefd556a7d/config/prow/cluster/crier_rbac.yaml#L16-L22

Copy link
Copy Markdown
Contributor

@spiffxp spiffxp May 19, 2021

Choose a reason for hiding this comment

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

So tl;dr I would create a k8s-infra-prow@kubernetes-public service account for the prow control plane, since it may require more than just write access to this one bucket.

Since you're doing this for the cluster in kubernetes-public, you could do all this in https://github.com/kubernetes/k8s.io/blob/main/infra/gcp/clusters/projects/kubernetes-public/aaa

But no objections to iterating in bash if you need to.

Copy link
Copy Markdown
Member Author

@ameukam ameukam May 20, 2021

Choose a reason for hiding this comment

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

@spiffxp I'll add a second commit with a HCL version.

Copy link
Copy Markdown
Member Author

@ameukam ameukam May 22, 2021

Choose a reason for hiding this comment

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

@spiffxp The Terraform version is done in 03d55ef.

local svc_acct_email="$(svc_acct_email "${project}" \
"${svc_acct_name}")"
local SECRET_ID="${svc_acct_name}-key"

color 6 "Ensuring bucket ${bucket} exists and is only word-readable"
ensure_public_gcs_bucket "${project}" "gs://${bucket}"


color 6 "Creating service account: ${svc_acct_name}"
ensure_service_account \
"${project}" \
"${svc_acct_name}" \
"${svc_acct_name}"

color 6 "Empowering service account: ${svc_acct_name}"
empower_svcacct_to_write_gcs_bucket "${svc_acct_email}" "gs://${bucket}"

color 6 "Ensure secret ${SECRET_ID} exists in project ${PROJECT}"
ensure_secret "${project}" "${SECRET_ID}"

color "Ensure ${SECRET_ID} contains secret key for ${svc_acct_name}"
ensure_serviceaccount_key_secret "${project}" "${SECRET_ID}" "${svc_acct_email}"

color 6 "Empowering k8s-infra-prow-oncall@kubernetes.io to read secret ${SECRET_ID}"
ensure_secrets_role_binding \
"projects/${project}/secrets/${SECRET_ID}" \
"group:k8s-infra-prow-oncall@kubernetes.io" \
"roles/secretmanager.secretAccessor"
Comment on lines +196 to +215
Copy link
Copy Markdown
Contributor

@spiffxp spiffxp May 19, 2021

Choose a reason for hiding this comment

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

This is basically

k8s.io/infra/gcp/ensure-conformance-storage.sh

Lines 104 to 131 in 4d98e1c

# ensure_conformance_serviceaccount ensures that:
# - a serviceaccount of the given name exists in PROJECT
# - it can write to the given bucket
# - it has a private key stored in a secret in PROJECT accessible to the given group
function ensure_conformance_serviceaccount() {
local name="${1}"
local bucket="${2}"
local secret_accessors="${3}"
local email="$(svc_acct_email "${PROJECT}" "${name}")"
local secret="${name}-key"
local private_key_file="${TMPDIR}/key.json"
color 6 "Ensuring service account exists: ${email}"
ensure_service_account "${PROJECT}" "${name}" "Grants write access to ${bucket}"
color 6 "Ensuring ${PROJECT} contains secret ${secret} with private key for ${email}"
ensure_serviceaccount_key_secret "${PROJECT}" "${secret}" "${email}"
color 6 "Empowering ${secret_accessors} to access secret: ${secret}"
ensure_secret_role_binding \
"projects/${PROJECT}/secrets/${secret}" \
"group:${secret_accessors}" \
"roles/secretmanager.secretAccessor"
color 6 "Empowering ${email} to write to ${bucket}"
empower_svcacct_to_write_gcs_bucket "${email}" "${bucket}"
}

You could parameterize it on project and pull into lib_iam.sh for re-use. Can be done as followup though


done
}

function empower_cluster_admins_and_users() {
if [ $# -ne 1 ] || [ -z "$1" ]; then
echo "${FUNCNAME[0]}(gcp_project) requires 1 argument" >&2
Expand Down Expand Up @@ -379,6 +427,9 @@ function ensure_main_project() {
color 6 "Ensuring terraform state buckets exist with correct permissions in: ${project}"
ensure_terraform_state_buckets "${project}" 2>&1 | indent

color 6 "Ensuring prow buckets exist in: ${project}"
ensure_prow_buckets "${project}" 2>&1 | indent

color 6 "Empowering cluster users and admins for clusters in: ${project}"
empower_cluster_admins_and_users "${project}" 2>&1 | indent

Expand Down
5 changes: 5 additions & 0 deletions infra/gcp/lib_gsm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,13 @@ function ensure_secret_with_admins() {
# $2: The secret name (e.g. "my-secret")
# $3: The service-account (e.g. "foo@k8s-infra.iam.gserviceaccount.com")
function ensure_serviceaccount_key_secret() {
<<<<<<< HEAD
if [ ! $# -eq 3 ] || [ -z "$1" ] || [ -z "$2" ] || [ -z "$3" ]; then
echo "${FUNCNAME[0]}(project, secret, serviceaccountt) requires 3 arguments" >&2
=======
if [ ! $# -eq 3 -o -z "$1" -o -z "$2" -o -z "$3" ]; then
echo "ensure_serviceaccount_key_secret(project, secret, serviceaccount) requires 3 arguments" >&2
>>>>>>> 0b821977 (Add gcs public bucket for k8s-infra-prow logs.)
Comment on lines +105 to +111
Copy link
Copy Markdown
Contributor

@spiffxp spiffxp Jun 11, 2021

Choose a reason for hiding this comment

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

merge conflict, you want to keep HEAD

return 1
fi

Expand Down