Add service account to k8s-infra-ii-coop by bernokl · Pull Request #2262 · kubernetes/k8s.io

bernokl · 2021-06-23T03:57:15Z

I think this should also use ensure_service_account from lib_iam.sh, but I am unsure where to call it from

k8s-ci-robot · 2021-06-23T03:57:23Z

Hi @bernokl. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

ameukam · 2021-06-28T17:00:48Z

groups/groups.yaml

      - riaan@ii.coop
      - stephen@ii.coop
      - zz@ii.coop
+      - asn-etl@k8s-infra-ii-sandbox.iam.gserviceaccount.com


@bernokl I'm not sure to understand why you need to add this SA to this google group. What is the particularity of this SA ?

@ameukam it will be our reporting user that is intended to run the asn-ip-lookup job on prow. Is there a more appropriate way for a job runner to get permissions to the logs?

I would rather we keep service accounts out of groups. I can't recall the specific example right now, but it doesn't always work. I removed all previous SA memberships in #2010, specifically 9ebc221

Bind the service account directly to what is needed

@bernokl My suggestion would be look at https://cloud.google.com/iam/docs/understanding-roles to identity what permissions you need for this SA and add the binding as a Terraform code in https://github.com/kubernetes/k8s.io/tree/main/infra/gcp/clusters/projects/k8s-infra-ii-sandbox.

Ultimately, every resource used to do PII analysis will be moved to https://github.com/kubernetes/k8s.io/tree/main/infra/gcp/clusters/projects/k8s-infra-public-pii. See : #1968.

ameukam · 2021-06-28T17:02:12Z

I think this should also use ensure_service_account from lib_iam.sh, but I am unsure where to call it from

You can directly create the Service Account with Terraform here : https://github.com/kubernetes/k8s.io/blob/main/infra/gcp/clusters/projects/k8s-infra-ii-sandbox/main.tf

k8s-ci-robot · 2021-07-13T18:58:59Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bernokl
To complete the pull request process, please assign spiffxp after the PR has been reviewed.
You can assign the PR to them by writing /assign @spiffxp in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

infra/gcp/clusters/OWNERS

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

bernokl · 2021-07-13T19:03:14Z

I changed the pr to create the SA and bind-role in permissions.tf, if needed I can move it to main.tf as suggested above.

The project for the role will potentially need to be kubernetes.io but need some guidance.
The logs this SA would currently consume will be in gs://k8s-artifacts-gcslogs/ ideally it pulls from k8s-infra-public-pii, but I think that is only staging data right now.

spiffxp · 2021-07-21T00:06:34Z

/test pull-k8sio-verify

spiffxp · 2021-07-21T00:06:45Z

/ok-to-test

spiffxp

question and some naming nits

spiffxp · 2021-08-06T17:42:01Z

infra/gcp/clusters/projects/k8s-infra-ii-sandbox/permissions.tf

+resource "google_service_account" "ii-logs-sa@k8s-infra-ii-sandbox.iam.gserviceaccount.com" {
+  account_id   = "ii-logs-sa-id"


Suggested change

resource "google_service_account" "ii-logs-sa@k8s-infra-ii-sandbox.iam.gserviceaccount.com" {

account_id = "ii-logs-sa-id"

resource "google_service_account" "ii_logs_sa" {

account_id = "ii-logs"

spiffxp · 2021-08-06T17:42:40Z

infra/gcp/clusters/projects/k8s-infra-ii-sandbox/permissions.tf

+resource "google_service_account" "ii-logs-sa@k8s-infra-ii-sandbox.iam.gserviceaccount.com" {
+  account_id   = "ii-logs-sa-id"
+  display_name = "ii logs service account"
+  description  = "service-account-to-facilitate-ii-log-analysis"


Suggested change

description = "service-account-to-facilitate-ii-log-analysis"

description = "service account to facilitate log analysis by ii"

spiffxp · 2021-08-06T17:43:59Z

infra/gcp/clusters/projects/k8s-infra-ii-sandbox/permissions.tf

+  project = google_project.project.id
+  role    = "roles/storage.objectViewer"
+  members = [
+    "user:ii-logs-sa@k8s-infra-ii-sandbox.iam.gserviceaccount.com",


Suggested change

"user:ii-logs-sa@k8s-infra-ii-sandbox.iam.gserviceaccount.com",

"user:ii-logs@k8s-infra-ii-sandbox.iam.gserviceaccount.com",

spiffxp · 2021-08-06T17:49:00Z

infra/gcp/clusters/projects/k8s-infra-ii-sandbox/permissions.tf

+  project      = google_project.project.id
+}
+
+resource "google_project_iam_binding" "k8s-infra-ii-sandbox" {


From https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam

google_project_iam_binding: Authoritative for a given role. Updates the IAM policy to grant a role to a list of members. Other roles within the IAM policy for the project are preserved.

Are you sure you want this behavior? It means nobody else gets roles/storage.objectViewer in this project except the service account.

What you might want instead want is to say "this service account should get this role, regardless of who else has the role"

Which would be

Suggested change

resource "google_project_iam_binding" "k8s-infra-ii-sandbox" {

resource "google_project_iam_member" "k8s-infra-ii-sandbox" {

spiffxp

(earlier review was meant to be "request changes")

k8s-ci-robot · 2021-08-06T19:57:27Z

@bernokl: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Rerun command
pull-k8sio-terraform-prow-build-trusted	`8fb21ed`	link	`/test pull-k8sio-terraform-prow-build-trusted`
pull-k8sio-terraform-prow-build	`8fb21ed`	link	`/test pull-k8sio-terraform-prow-build`
pull-k8sio-terraform-kubernetes-public	`8fb21ed`	link	`/test pull-k8sio-terraform-kubernetes-public`

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.

Details

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.

riaankleinhans · 2021-08-25T20:08:52Z

/close

k8s-ci-robot · 2021-08-25T20:09:08Z

@Riaankl: Closed this PR.

Details

In response to this:

/close

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 added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. area/access Define who has access to what via IAM bindings, role bindings, policy, etc. wg/k8s-infra labels Jun 23, 2021

k8s-ci-robot requested review from dims and thockin June 23, 2021 03:57

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 23, 2021

ameukam reviewed Jun 28, 2021

View reviewed changes

bernokl mentioned this pull request Jul 13, 2021

Add sa/role-binding for log viewer #2332

Closed

Add sa/role-binding for log viewer

8fb21ed

bernokl force-pushed the log_viewer_service_account branch from e789d1b to 8fb21ed Compare July 13, 2021 18:58

k8s-ci-robot added area/terraform Terraform modules, testing them, writing more of them, code in infra/gcp/clusters/ size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 13, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 21, 2021

spiffxp reviewed Aug 6, 2021

View reviewed changes

spiffxp suggested changes Aug 6, 2021

View reviewed changes

k8s-ci-robot assigned spiffxp Aug 6, 2021

k8s-ci-robot closed this Aug 25, 2021

		resource "google_service_account" "ii-logs-sa@k8s-infra-ii-sandbox.iam.gserviceaccount.com" {
		account_id = "ii-logs-sa-id"

	description = "service-account-to-facilitate-ii-log-analysis"
	description = "service account to facilitate log analysis by ii"

	"user:ii-logs-sa@k8s-infra-ii-sandbox.iam.gserviceaccount.com",
	"user:ii-logs@k8s-infra-ii-sandbox.iam.gserviceaccount.com",

	resource "google_project_iam_binding" "k8s-infra-ii-sandbox" {
	resource "google_project_iam_member" "k8s-infra-ii-sandbox" {

Conversation

bernokl commented Jun 23, 2021

Uh oh!

k8s-ci-robot commented Jun 23, 2021

Uh oh!

ameukam Jun 28, 2021

Choose a reason for hiding this comment

Uh oh!

bernokl Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spiffxp Jun 29, 2021

Choose a reason for hiding this comment

Uh oh!

ameukam Jun 29, 2021

Choose a reason for hiding this comment

Uh oh!

ameukam commented Jun 28, 2021

Uh oh!

k8s-ci-robot commented Jul 13, 2021

Uh oh!

bernokl commented Jul 13, 2021

Uh oh!

spiffxp commented Jul 21, 2021

Uh oh!

spiffxp commented Jul 21, 2021

Uh oh!

spiffxp left a comment

Choose a reason for hiding this comment

Uh oh!

spiffxp Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

spiffxp Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

spiffxp Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

spiffxp Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

spiffxp left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Aug 6, 2021

Uh oh!

riaankleinhans commented Aug 25, 2021

Uh oh!

k8s-ci-robot commented Aug 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bernokl Jun 28, 2021 •

edited

Loading