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.

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

Jump to bottom

prow-build-trusted: add kubernetes-external-secrets #2148

Merged
merged 2 commits into from
Jun 8, 2021
Merged

prow-build-trusted: add kubernetes-external-secrets #2148

merged 2 commits into from
Jun 8, 2021

Conversation

spiffxp
Copy link
Member

@spiffxp spiffxp commented Jun 7, 2021

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/terraform Terraform modules, testing them, writing more of them, code in infra/gcp/clusters/ area/prow Setting up or working with prow in general, prow.k8s.io, prow build clusters sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jun 7, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 7, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Jun 7, 2021

/cc @ameukam @dims

@k8s-ci-robot k8s-ci-robot requested review from ameukam and dims June 7, 2021 21:18
spiffxp added 2 commits June 7, 2021 17:59
@spiffxp
prow-build-trusted: merge namespace resources
69864a2
@spiffxp
prow-build-trusted: add kubernetes-external-secrets
9df04e0 
Specifically:

- add manifests
  - copy from /kubernetes-external-secrets
  - move SA from rbac.yaml to .yaml
  - add label "app: kubernetes-external-secrets" to some resources that
    didn't have it in rbac
  - rename the crd to kubernetes-external-secrets_crd.yaml
  - remove TODO on LOG_LEVEL being debug
- add terraform
  - add kubernetes-external-secrets serviceaccount with correct role and
    setup for workload identity
  - add regional ip address for metrics scraping

The "create a service account with X role for use by this cluster via
WI" boilerplate is getting pretty unwieldy, but I didn't want to
refactor (or look for) a module until I got this deployed
@spiffxp spiffxp force-pushed the k8s-infra-prow-build-trusted-external-secrets branch from 29244c3 to 9df04e0 Compare June 7, 2021 22:16
@spiffxp
Copy link
Member Author

spiffxp commented Jun 7, 2021

I had to run terraform apply to get an external IP created, but the resources in here haven't yet been deployed. Your call whether I do this pre-merge or post-merge, but I'll wait for approval before doing so.

@ameukam
Copy link
Member

ameukam commented Jun 7, 2021

Let's do this post-merge.

Also pull-k8sio-terraform-prow-build-trusted was supposed to run but nothing happened. It's not blocking for this PR.

/lgtm
/hold
Remove hold when ready to deploy.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 7, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021

The run_if_changed regex has a typo:prow-build-trusted/*.tf$ would match prow-build-trusted/.tf or prow-build-trusted////.tf but none of the files this PR changes. If I need to make any changes in the jobs due to kubernetes/test-infra#22463 I'll include as a fix

@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 8, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021

/hold
actually I want to wait for the postsubmits to change...

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 8, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021

/hold cancel
ready to watch this deploy

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 8, 2021
@k8s-ci-robot k8s-ci-robot merged commit 5dbcd87 into kubernetes:main Jun 8, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone Jun 8, 2021
@spiffxp spiffxp deleted the k8s-infra-prow-build-trusted-external-secrets branch June 8, 2021 02:04
@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021

https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/post-k8sio-deploy-prow-build-trusted-resources/1402077432638345216
First run failed with:

customresourcedefinition.apiextensions.k8s.io/externalsecrets.kubernetes-client.io created
namespace/kubernetes-external-secrets created
namespace/staging-test-pods unchanged
namespace/test-pods unchanged
Error from server (NotFound): error when creating "prow-build-trusted/resources/kubernetes-external-secrets.yaml": namespaces "kubernetes-external-secrets" not found
Error from server (NotFound): error when creating "prow-build-trusted/resources/kubernetes-external-secrets.yaml": namespaces "kubernetes-external-secrets" not found

Re-ran job, namespaces were found (need to redo job to create cluster-scoped resources before creating namespace-scoped resources)

But it also failed, with

Error from server (Forbidden): error when creating "prow-build-trusted/resources/kubernetes-external-secrets_rbac.yaml": clusterroles.rbac.authorization.k8s.io is forbidden: User "prow-deployer@k8s-infra-prow-build-trusted.iam.gserviceaccount.com" cannot create resource "clusterroles" in API group "rbac.authorization.k8s.io" at the cluster scope: requires one of ["container.clusterRoles.create"] permission(s).
Error from server (Forbidden): error when creating "prow-build-trusted/resources/kubernetes-external-secrets_rbac.yaml": clusterrolebindings.rbac.authorization.k8s.io is forbidden: User "prow-deployer@k8s-infra-prow-build-trusted.iam.gserviceaccount.com" cannot create resource "clusterrolebindings" in API group "rbac.authorization.k8s.io" at the cluster scope: requires one of ["container.clusterRoleBindings.create"] permission(s).
Error from server (Forbidden): error when creating "prow-build-trusted/resources/kubernetes-external-secrets_rbac.yaml": clusterrolebindings.rbac.authorization.k8s.io is forbidden: User "prow-deployer@k8s-infra-prow-build-trusted.iam.gserviceaccount.com" cannot create resource "clusterrolebindings" in API group "rbac.authorization.k8s.io" at the cluster scope: requires one of ["container.clusterRoleBindings.create"] permission(s).

@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021

It looks like there's no (non-service-agent) pre-defined role that contains container.clusterRoles.create except for roles/container.admin, so my options are to either:

  1. manually deploy myself as a cluster admin
  2. give prow-deployer container.admin
  3. create a custom role with container.clusterRoles.* and container.clusterRoleBindings.* and give that to prow-deployer

I'm going to do 2 to confirm the job can deploy, then work on 3

@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021 

gcloud projects add-iam-policy-binding \
  k8s-infra-prow-build-trusted \
  --member=serviceAccount:prow-deployer@k8s-infra-prow-build-trusted.iam.gserviceaccount.com \
  --role=roles/container.admin

@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021

Re-run succeeded: https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/post-k8sio-deploy-prow-build-trusted-resources/1402087372769726464

Although, of note:

Warning: apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition

@spiffxp spiffxp mentioned this pull request Jun 8, 2021
Merged
@spiffxp
Copy link
Member Author

spiffxp commented Jun 8, 2021

Opened #2156 for option 3

@ameukam
Copy link
Member

ameukam commented Jun 8, 2021

Re-run succeeded: prow.k8s.io/view/gs/kubernetes-jenkins/logs/post-k8sio-deploy-prow-build-trusted-resources/1402087372769726464

Although, of note:

Warning: apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition

Fixed in the next major release : https://github.com/external-secrets/kubernetes-external-secrets/releases/tag/8.0.0.

If you're not faster than me, I'll plan do the upgrade in the next weeks.

@spiffxp spiffxp mentioned this pull request Jun 8, 2021
Merged
@spiffxp
Copy link
Member Author

spiffxp commented Jun 10, 2021

Now that #2156 has merged, I'm dropping the roles/container.admin binding

gcloud projects remove-iam-policy-binding \
  k8s-infra-prow-build-trusted \
  --member="serviceAccount:prow-deployer@k8s-infra-prow-build-trusted.iam.gserviceaccount.com" \
  --role="roles/container.admin"

This was referenced Jun 10, 2021
Merged
Merged
Merged
@spiffxp spiffxp mentioned this pull request Jul 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ameukam ameukam ameukam left review comments

@chases2 chases2 Awaiting requested review from chases2

@jeremyrickard jeremyrickard Awaiting requested review from jeremyrickard

@dims dims Awaiting requested review from dims

Assignees

@ameukam ameukam

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/prow Setting up or working with prow in general, prow.k8s.io, prow build clusters area/terraform Terraform modules, testing them, writing more of them, code in infra/gcp/clusters/ cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. 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.
Projects
None yet
Milestone
v1.22
Development

Successfully merging this pull request may close these issues.
3 participants
@spiffxp @k8s-ci-robot @ameukam