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.

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

AWS: Enable EKS Pod Identity #6137

Merged
merged 4 commits into from
Dec 4, 2023

Conversation

ameukam
Copy link
Member

@ameukam ameukam commented Dec 1, 2023

Enable EKS Pod Identity for EKS cluster k8s-infra-kops-prow-build.

See: https://aws.amazon.com/blogs/aws/amazon-eks-pod-identity-simplifies-iam-permissions-for-applications-on-amazon-eks-clusters/

Signed-off-by: Arnaud Meukam ameukam@gmail.com

Enable EKS Pod Identity for EKS cluster `k8s-infra-kops-prow-build`.

See: https://aws.amazon.com/blogs/aws/amazon-eks-pod-identity-simplifies-iam-permissions-for-applications-on-amazon-eks-clusters/

Signed-off-by: Arnaud Meukam <ameukam@gmail.com>
Bump provider get support of EKS Pod Identity

Signed-off-by: Arnaud Meukam <ameukam@gmail.com>
Signed-off-by: Arnaud Meukam <ameukam@gmail.com>
@k8s-ci-robot 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. area/infra Infrastructure management, infrastructure design, code in infra/ area/infra/aws Issues or PRs related to Kubernetes AWS infrastructure sig/k8s-infra Categorizes an issue or PR as relevant to SIG K8s Infra. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 1, 2023
@ameukam
Copy link
Member Author

ameukam commented Dec 1, 2023

cc @bryantbiggs @dims

@ameukam ameukam added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 1, 2023
@bryantbiggs
Copy link
Contributor

What's the current platform version on the clusters?

@@ -53,3 +53,37 @@ resource "aws_iam_role" "google_prow_trust_role" {
max_session_duration = 43200
assume_role_policy = data.aws_iam_policy_document.google_prow_trust_policy.json
}


// Leveraging EKS Pod Identity feature allow kOps prowjobs to run E2E tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@bryantbiggs Prow don't relies on AWS SDK for authentification. The AWS SDK is required for a another binary (aws-stockout) present in the test-infra repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

what does it use? the credential provider logic for Pod Identity is provided in the AWS SDKs

Copy link
Member Author

Choose a reason for hiding this comment

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

Prow don't handle authn itself to each cloud provider we use. it only responsible to schedule each job to the kubernetes cluster. How the job interact with the cloud provider environment is up to the maintainer of the prowjob. Currently we only use access keys and secrets key for authn; they are mounted as env variables to be consumed by the job.

@ameukam
Copy link
Member Author

ameukam commented Dec 1, 2023

What's the current platform version on the clusters?

1.26. My understanding is that we need 1.27 at minimum to make it works.

@bryantbiggs
Copy link
Contributor

1.24 - 1.28 are supported, but they need to be on a platform version that supports pod identity. See here for the min supported platform versions for the given K8s version https://docs.aws.amazon.com/eks/latest/userguide/pod-identities.html#pod-id-cluster-versions

Create a dedicated namespace for the prowjobs scheduled by prow.k8s.io

Signed-off-by: Arnaud Meukam <ameukam@gmail.com>
@ameukam
Copy link
Member Author

ameukam commented Dec 2, 2023

1.24 - 1.28 are supported, but they need to be on a platform version that supports pod identity. See here for the min supported platform versions for the given K8s version docs.aws.amazon.com/eks/latest/userguide/pod-identities.html#pod-id-cluster-versions

I see. Currently on eks.8 for 1.26. I'll bump the cluster version in a follow-up PR.

@dims
Copy link
Member

dims commented Dec 4, 2023

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 4, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 merged commit d829056 into kubernetes:main Dec 4, 2023
3 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.29 milestone Dec 4, 2023
Comment on lines +84 to +89
resource "aws_iam_role_policy_attachment" "eks_pod_identity_policy" {
provider = aws.kops-infra-ci

policy_arn = "arn:aws:iam::aws:policy/AdministratorAccess"
role = aws_iam_role.eks_pod_identity_role.name
}
Copy link
Contributor

Choose a reason for hiding this comment

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

How did we pick this policy? It's privileged.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sftim It's intended to use a privileged account. The pod using this role need to be able to do some operations requiring Admin access. It's not possible to currently identity the entire scope of permissions needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Minimum permissions required: https://kops.sigs.k8s.io/getting_started/aws/.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's useful, we can craft a permissions boundary to limit what these Pods can do, including limits on elevation of those privileges. Kops does support this.

Want an issue for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. And thank you!

@ameukam
Copy link
Member Author

ameukam commented Dec 8, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/infra/aws Issues or PRs related to Kubernetes AWS infrastructure area/infra Infrastructure management, infrastructure design, code in infra/ 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/k8s-infra Categorizes an issue or PR as relevant to SIG K8s Infra. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants