Ensure gcs bucket for k8s-infra prow instance #1909

ameukam · 2021-04-09T23:34:21Z

Following prow documentation guidance :
Create a GCS bucket for tide history and build logs.
Create a service account and grant admin access to the bucket.
Create a service account key and add the generated key to Secret
Manager.

Signed-off-by: Arnaud Meukam [email protected]

ameukam · 2021-04-12T21:57:42Z

/retitle Ensure gcs bucket for k8s-infra prow instance

ameukam · 2021-04-12T21:58:16Z

/hold

rikatz · 2021-04-13T01:18:35Z

/lgtm

cpanato

/lgtm

k8s-ci-robot · 2021-04-13T07:53:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ameukam, cpanato

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:

~~OWNERS~~ [ameukam]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

spiffxp · 2021-05-04T20:17:09Z

infra/gcp/ensure-main-project.sh

@@ -57,6 +57,11 @@ CLUSTER_USERS_GROUP="[email protected]"
 # The DNS admins group.
 DNS_GROUP="[email protected]"

+# Buckets for the logs of prow
+PROW_BUCKETS=(
+    k8s-prow-infra-logs


Assuming this is supposed to be the community equivalent of kubernetes-jenkins?

Suggested change

k8s-prow-infra-logs

k8s-infra-prow-results

Naming is hard when I overthink things.

First thought: k8s-infra-prow-logs to get the consistent k8s-infra prefix

But...

"prow logs" sounds like logs of the prow components (plank, sinker, crier, etc)

there are more than "logs" that end up kubernetes-jenkins, there are also "artifacts" like junit.xml files, coverage files, metadata, etc.

OK, so k8s-infra-prow-artifacts for "artifacts" produced by prow, including job results

But...

"artifacts" clashes with the sig-release meaning of "binaries and container images that represent a given release of a given subproject"

makes me thing it's going to be the binaries/images for prow itself, a.k.a. the equivalent to gcr.io/k8s-prow

OK, so how about just k8s-infra-prow and be done with it

But...

If we're doing things over, one of the problems with kubernetes-jenkins is that it allows per-object ACLs. They allowed that single bucket to be used for multiple purposes, including occasionally storing secrets or credentials.

We don't want per-object ACLs in kubernetes.io, as they are painful and laborious to maintain, audit and enforce. We are planning on setting an org policy to explicitly require per-bucket IAM only (aka uniform bucket level access).

Thus we should name this bucket for the main purpose everyone expects of kubernetes-jenkins, public read access to things about and created by prow jobs for the kubernetes project. If other purposes arise, they should result in other buckets named/IAM'ed as such.

OK, so how about k8s-infra-prow-results, as in the "results" of prow job runs

I looked around in prow's codebase and docs and couldn't find a consistent term for "things that are uploaded to GCS or S3 by the pod-utilities sidecar component and crier". Results isn't just "test results" but from the concept of running a prow job and then looking at the "resulting" files in GCS/S3.

I considered "data" but that sounds like "training data" or other metadata that is loaded in to help run a job, where I believe this bucket's intended purpose is storage of things about and created by prow jobs

WDYT about k8s-infra-prow-results?

"artifacts" clashes with the sig-release meaning of "binaries and container images that represent a given release of a given subproject"

The buckets are prefixed (k8s-infra-prow) with a reference. I don't think we should have confusion with other projects.

Let's go with k8s-infra-prow-results, it's more aligned with the needs of this instance.

spiffxp · 2021-05-04T20:22:52Z

infra/gcp/ensure-main-project.sh

@@ -79,6 +84,40 @@ ensure_private_gcs_bucket \
    "${PROJECT}" \
    "gs://${CLUSTER_TERRAFORM_BUCKET}"

+
+color 6 "Ensuring all the prow buckets exist"


/hold
The rest of this I would like to wait to merge until I land a refactor of this file (#1974)

I am happy to take up redoing this part to fit in with the refactor as followup, or leaving it to you to ensure someone other than me understands the structure I tried to put in place

@spiffxp made a refactor now #1974 is merged.

k8s-ci-robot · 2021-05-19T00:24:28Z

@ameukam: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-k8sio-terraform-prow-build-trusted	`8723612`	link	`/test pull-k8sio-terraform-prow-build-trusted`
pull-k8sio-terraform-prow-build	`8723612`	link	`/test pull-k8sio-terraform-prow-build`
pull-k8sio-terraform-aaa	`8723612`	link	`/test pull-k8sio-terraform-aaa`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

ameukam · 2021-05-19T19:20:36Z

/skip

spiffxp

I'm not sure provisioning new service accounts is required here. For the existing prow build clusters their service accounts are setup via terraform

spiffxp · 2021-05-19T19:37:39Z

infra/gcp/ensure-main-project.sh

+    local project="${1}"
+
+    for bucket in "${PROW_BUCKETS[@]}"; do
+        local svc_acct_name="${bucket}-sa"


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

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

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.

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

@spiffxp The Terraform version is done in 03d55ef.

spiffxp · 2021-05-19T20:55:10Z

infra/gcp/ensure-main-project.sh

+    local project="${1}"
+
+    for bucket in "${PROW_BUCKETS[@]}"; do
+        local svc_acct_name="${bucket}-sa"


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

spiffxp · 2021-05-19T21:18:10Z

infra/gcp/ensure-main-project.sh

+    local project="${1}"
+
+    for bucket in "${PROW_BUCKETS[@]}"; do
+        local svc_acct_name="${bucket}-sa"


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.

spiffxp · 2021-05-19T21:21:37Z

infra/gcp/ensure-main-project.sh

+        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 [email protected] to read secret ${SECRET_ID}"
+        ensure_secrets_role_binding \
+            "projects/${project}/secrets/${SECRET_ID}" \
+            "group:[email protected]" \
+            "roles/secretmanager.secretAccessor"


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

Following prow [documentation](https://github.com/kubernetes/test-infra/blob/master/prow/getting_started_deploy.md#configure-a-gcs-buckethttps://github.com/kubernetes/test-infra/blob/master/prow/getting_started_deploy.md#configure-a-gcs-bucket) guidance : Create a GCS bucket for tide history and build logs. Create a service account and grant admin access to the bucket. Create a service account key and add the generated key to Secret Manager. Signed-off-by: Arnaud Meukam <[email protected]>

spiffxp · 2021-06-11T18:14:28Z

infra/gcp/lib_gsm.sh

+<<<<<<< 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.)


merge conflict, you want to keep HEAD

spiffxp · 2021-06-11T21:51:46Z

/close
closing this in favor of #2181

k8s-ci-robot · 2021-06-11T21:51:52Z

@spiffxp: Closed this PR.

In response to this:

/close
closing this in favor of #2181

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.

k8s-ci-robot requested review from dims and thockin April 9, 2021 23:34

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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 Apr 9, 2021

ameukam mentioned this pull request Apr 9, 2021

Add gcs public bucket for prow staging logs. #1474

Closed

k8s-ci-robot changed the title ~~Add gcs public bucket for prow staging logs.~~ Ensure gcs bucket for k8s-infra prow instance Apr 12, 2021

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 12, 2021

k8s-ci-robot assigned rikatz Apr 13, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 13, 2021

cpanato approved these changes Apr 13, 2021

View reviewed changes

k8s-ci-robot assigned cpanato Apr 13, 2021

spiffxp suggested changes May 4, 2021

View reviewed changes

k8s-ci-robot assigned spiffxp May 4, 2021

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 4, 2021

rikatz removed their assignment May 16, 2021

ameukam force-pushed the prow-staging-buckets branch from 8723612 to 0b82197 Compare May 19, 2021 18:59

spiffxp suggested changes May 19, 2021

View reviewed changes

spiffxp reviewed May 19, 2021

View reviewed changes

k8s-ci-robot added sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 22, 2021

ameukam force-pushed the prow-staging-buckets branch from a35359e to 5eb49b0 Compare May 22, 2021 15:46

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 25, 2021

ameukam mentioned this pull request May 25, 2021

Setup a k8s-infra prow instance #1394

Closed

9 tasks

ameukam force-pushed the prow-staging-buckets branch from 5eb49b0 to 951a4e3 Compare June 10, 2021 05:10

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 10, 2021

ameukam mentioned this pull request Jun 10, 2021

infra/gcp/kubernetes-public/aaa : Add GCS bucket for k8s-infra-prow #2181

Merged

spiffxp suggested changes Jun 11, 2021

View reviewed changes

k8s-ci-robot closed this Jun 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure gcs bucket for k8s-infra prow instance #1909

Ensure gcs bucket for k8s-infra prow instance #1909

ameukam commented Apr 9, 2021

ameukam commented Apr 12, 2021

ameukam commented Apr 12, 2021

rikatz commented Apr 13, 2021

cpanato left a comment

k8s-ci-robot commented Apr 13, 2021

spiffxp May 4, 2021

ameukam May 4, 2021

spiffxp May 4, 2021

ameukam May 19, 2021

k8s-ci-robot commented May 19, 2021

ameukam commented May 19, 2021

spiffxp left a comment

spiffxp May 19, 2021

spiffxp May 19, 2021

spiffxp May 19, 2021

ameukam May 20, 2021

ameukam May 22, 2021

spiffxp May 19, 2021

spiffxp May 19, 2021

spiffxp May 19, 2021

spiffxp Jun 11, 2021

spiffxp commented Jun 11, 2021

k8s-ci-robot commented Jun 11, 2021

	# 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}"
	}

Ensure gcs bucket for k8s-infra prow instance #1909

Ensure gcs bucket for k8s-infra prow instance #1909

Conversation

ameukam commented Apr 9, 2021

ameukam commented Apr 12, 2021

ameukam commented Apr 12, 2021

rikatz commented Apr 13, 2021

cpanato left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 19, 2021

ameukam commented May 19, 2021

spiffxp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spiffxp commented Jun 11, 2021

k8s-ci-robot commented Jun 11, 2021