✨ Add IRSA to EKS workload clusters #2070

MarcusNoble · 2020-10-26T06:57:09Z

What this PR does / why we need it:
Creates an OIDC Identity Provider and associates it with the managed control plane so IAM Roles for Service Accounts can be used in the workload cluster.
An example boilerplate trust policy needed for IAM role to be usable by service accounts is provided as a ConfigMap in the default namespace of the workload cluster (envsubt compatible).

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2054

k8s-ci-robot · 2020-10-26T06:57:17Z

Hi @MarcusNoble. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

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.

voor · 2020-10-27T10:57:20Z

Maybe the iam stuff should be toggled? Those seem like pretty heavy hitting permissions.

pkg/cloud/services/eks/oidc.go

controlplane/eks/api/v1alpha3/awsmanagedcontrolplane_types.go

randomvariable · 2020-10-27T11:36:24Z

All of this functionality should be behind the EnableIAM flag as there is a risk of privilege escalation.

randomvariable · 2020-10-27T15:41:24Z

/ok-to-test
/hold

I think we need to get e2e tests in place before we add more features.

richardcase · 2020-10-27T17:28:48Z

I think we need to get e2e tests in place before we add more features.

Not too long until they are ready....got distracted by other work last week.

richardcase

Generally looking good to me.

controlplane/eks/api/v1alpha3/awsmanagedcontrolplane_types.go

pkg/cloud/services/eks/oidc.go

MarcusNoble · 2020-10-29T06:52:09Z

All of this functionality should be behind the EnableIAM flag as there is a risk of privilege escalation.

@randomvariable most of the code is behind the AssociateOIDCProvider property being set to true. Are you referring to the additional IAM permissions needed for managing OIDC?

MarcusNoble · 2020-11-02T06:40:33Z

/assign @vincepri

pkg/cloud/services/eks/oidc.go

richardcase · 2020-11-03T10:52:51Z

pkg/cloud/services/eks/oidc.go

+func (s *Service) buildOIDCTrustPolicy() string {
+	providerARN := s.scope.ControlPlane.Status.OIDCProvider.ARN
+
+	return fmt.Sprintf(`{


I think we should probably change this from a embedded string to use the iam types we have:
https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/master/cmd/clusterawsadm/api/iam/v1alpha1/types.go#L66

pkg/cloud/services/eks/oidc.go

richardcase · 2020-11-03T11:55:48Z

@MarcusNoble - this is looking good to me. Would you be able to squash the commits so that it tells a story?

randomvariable · 2021-01-12T13:02:33Z

/unhold

We can bring this back to consideration

MarcusNoble · 2021-01-12T13:24:36Z

/test ?

k8s-ci-robot · 2021-01-12T13:24:37Z

@MarcusNoble: The following commands are available to trigger jobs:

/test pull-cluster-api-provider-aws-test
/test pull-cluster-api-provider-aws-build
/test pull-cluster-api-provider-aws-verify
/test pull-cluster-api-provider-aws-e2e-conformance
/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-eks

Use /test all to run the following jobs:

pull-cluster-api-provider-aws-test
pull-cluster-api-provider-aws-build
pull-cluster-api-provider-aws-verify

In response to this:

/test ?

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.

MarcusNoble · 2021-01-12T13:24:56Z

/test pull-cluster-api-provider-aws-e2e-eks

richardcase · 2021-01-12T13:59:08Z

@MarcusNoble - that failed quick :( Looks like there are some files have been removed from the hack/ folder. Let me fix the tests

MarcusNoble · 2021-01-12T14:03:51Z

Ah! I thought it was me not rebasing. Looks like it was removed here: 30d870e#diff-4f4d6d4dd9d639f8061109d558a575d7b2d69335cc9427579ad1bd8b01842406

richardcase · 2021-01-12T14:44:42Z

Just created #2199 with an update to the script. I've just set the tests off on that PR

MarcusNoble · 2021-01-14T06:43:54Z

/test pull-cluster-api-provider-aws-e2e-eks

richardcase · 2021-01-14T09:36:48Z

/test pull-cluster-api-provider-aws-e2e-eks

randomvariable · 2021-01-14T15:43:46Z

lgtm in principle

squash that "clean up" commit perhaps.

/assign @richardcase

richardcase · 2021-01-15T06:18:26Z

/test pull-cluster-api-provider-aws-e2e-eks

MarcusNoble · 2021-01-15T09:00:44Z

/test pull-cluster-api-provider-aws-e2e-eks

randomvariable · 2021-01-15T12:16:01Z

/lgtm

richardcase · 2021-01-15T12:19:06Z

The e2e tests passed so:

/approve

k8s-ci-robot · 2021-01-15T12:19:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

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~~ [richardcase]

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 added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 26, 2020

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 26, 2020

k8s-ci-robot requested review from chuckha and detiber October 26, 2020 06:57

MarcusNoble marked this pull request as ready for review October 26, 2020 10:11

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 26, 2020

randomvariable reviewed Oct 27, 2020

View reviewed changes

pkg/cloud/services/eks/oidc.go Outdated Show resolved Hide resolved

randomvariable reviewed Oct 27, 2020

View reviewed changes

controlplane/eks/api/v1alpha3/awsmanagedcontrolplane_types.go Outdated Show resolved Hide resolved

richardcase reviewed Oct 27, 2020

View reviewed changes

controlplane/eks/api/v1alpha3/awsmanagedcontrolplane_types.go Outdated Show resolved Hide resolved

pkg/cloud/services/eks/oidc.go Show resolved Hide resolved

MarcusNoble force-pushed the oidc branch 2 times, most recently from fbc419d to 3dfbf0f Compare October 29, 2020 07:44

k8s-ci-robot assigned vincepri Nov 2, 2020

richardcase requested changes Nov 3, 2020

View reviewed changes

MarcusNoble force-pushed the oidc branch from 378e648 to 73c6aa4 Compare November 3, 2020 11:29

MarcusNoble mentioned this pull request Nov 3, 2020

Store workload OIDC policy template as ConfigMap in management cluster #2089

Closed

MarcusNoble force-pushed the oidc branch from 5bf5e64 to 2fcfb5e Compare November 3, 2020 13:17

cdenneen mentioned this pull request Nov 11, 2020

Add support for running controllers using kiam via clusterctl #1975

Closed

cdenneen mentioned this pull request Nov 11, 2020

[docs] Update cross-account role assumption documentation with IRSA #2105

Open

MarcusNoble force-pushed the oidc branch from 2fcfb5e to 10675ca Compare November 19, 2020 07:53

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

MarcusNoble force-pushed the oidc branch from 10675ca to e5862c2 Compare January 12, 2021 13:29

MarcusNoble force-pushed the oidc branch from e5862c2 to 5ca8d59 Compare January 14, 2021 06:35

k8s-ci-robot assigned richardcase Jan 14, 2021

MarcusNoble force-pushed the oidc branch from 5ca8d59 to 1773cab Compare January 15, 2021 05:59

Add OIDC provider creation to EKS control plane

f58da95

MarcusNoble force-pushed the oidc branch from 1773cab to f58da95 Compare January 15, 2021 09:00

k8s-ci-robot assigned randomvariable Jan 15, 2021

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

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 15, 2021

richardcase approved these changes Jan 15, 2021

View reviewed changes

k8s-ci-robot merged commit 29f7b24 into kubernetes-sigs:master Jan 15, 2021

MarcusNoble deleted the oidc branch January 15, 2021 12:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Add IRSA to EKS workload clusters #2070

✨ Add IRSA to EKS workload clusters #2070

MarcusNoble commented Oct 26, 2020

k8s-ci-robot commented Oct 26, 2020

voor commented Oct 27, 2020

randomvariable commented Oct 27, 2020

randomvariable commented Oct 27, 2020

richardcase commented Oct 27, 2020

richardcase left a comment

MarcusNoble commented Oct 29, 2020

MarcusNoble commented Nov 2, 2020

richardcase Nov 3, 2020

MarcusNoble Nov 3, 2020

richardcase commented Nov 3, 2020

randomvariable commented Jan 12, 2021

MarcusNoble commented Jan 12, 2021

k8s-ci-robot commented Jan 12, 2021

MarcusNoble commented Jan 12, 2021

richardcase commented Jan 12, 2021

MarcusNoble commented Jan 12, 2021

richardcase commented Jan 12, 2021

MarcusNoble commented Jan 14, 2021

richardcase commented Jan 14, 2021

randomvariable commented Jan 14, 2021

richardcase commented Jan 15, 2021

MarcusNoble commented Jan 15, 2021

randomvariable commented Jan 15, 2021

richardcase commented Jan 15, 2021

k8s-ci-robot commented Jan 15, 2021

✨ Add IRSA to EKS workload clusters #2070

✨ Add IRSA to EKS workload clusters #2070

Conversation

MarcusNoble commented Oct 26, 2020

k8s-ci-robot commented Oct 26, 2020

voor commented Oct 27, 2020

randomvariable commented Oct 27, 2020

randomvariable commented Oct 27, 2020

richardcase commented Oct 27, 2020

richardcase left a comment

Choose a reason for hiding this comment

MarcusNoble commented Oct 29, 2020

MarcusNoble commented Nov 2, 2020

richardcase Nov 3, 2020

Choose a reason for hiding this comment

MarcusNoble Nov 3, 2020

Choose a reason for hiding this comment

richardcase commented Nov 3, 2020

randomvariable commented Jan 12, 2021

MarcusNoble commented Jan 12, 2021

k8s-ci-robot commented Jan 12, 2021

MarcusNoble commented Jan 12, 2021

richardcase commented Jan 12, 2021

MarcusNoble commented Jan 12, 2021

richardcase commented Jan 12, 2021

MarcusNoble commented Jan 14, 2021

richardcase commented Jan 14, 2021

randomvariable commented Jan 14, 2021

richardcase commented Jan 15, 2021

MarcusNoble commented Jan 15, 2021

randomvariable commented Jan 15, 2021

richardcase commented Jan 15, 2021

k8s-ci-robot commented Jan 15, 2021