-
Notifications
You must be signed in to change notification settings - Fork 623
✨ Add EKS Pod Identities to AWSManagedControlPlane #4906
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
Open
stefanmcshane
wants to merge
7
commits into
kubernetes-sigs:main
Choose a base branch
from
stefanmcshane:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+727
−11
Open
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
bae312d
Add eks pod identities
stefanmcshane 57d769f
semantic changes
stefanmcshane 8b271f6
memory aliasing patch
stefanmcshane bec51fd
memory aliasing patch
stefanmcshane 82c3c48
Update types.go
stefanmcshane 342ca84
update json tag
stefanmcshane 1cfd3e7
update deps
stefanmcshane File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
# EKS Pod Identity Associations | ||
|
||
[EKS Pod Identity Associations](https://aws.amazon.com/blogs/containers/introducing-amazon-eks-add-ons/) can be used with EKS clusters created using Cluster API Provider AWS. | ||
|
||
## Prerequisites | ||
|
||
### Setting up the IAM role in AWS | ||
|
||
Outside of CAPI/CAPA, you must first create an IAM Role which allows the `pods.eks.amazonaws.com` service principal in the trust policy. EKS Identities trust relationships must also include the `sts:TagSession` permission (on top of the `sts:AssumeRole` permission). | ||
|
||
This is a sample trust policy which allows a kubernetes service account to assume this role. We'll call the role `capi-test-role` in the next steps. | ||
|
||
```yaml | ||
{ | ||
"Version": "2012-10-17", | ||
"Statement": | ||
[ | ||
{ | ||
"Effect": "Allow", | ||
"Principal": { "Service": "pods.eks.amazonaws.com" }, | ||
"Action": ["sts:AssumeRole", "sts:TagSession"], | ||
}, | ||
], | ||
} | ||
``` | ||
|
||
### Installing the EKS Pod Identity Agent | ||
|
||
The EKS Pod Identity Agent can be installed as a Managed Add-on through the AWS Console, or through CAPA. | ||
To install the addon through CAPA, add it to `AWSManagedControlPlane`. Please ensure that the version is up to date, according to the [addons section](addons.md). | ||
|
||
```yaml | ||
# [...] | ||
kind: AWSManagedControlPlane | ||
spec: | ||
# [...] | ||
addons: | ||
# [...] | ||
- conflictResolution: overwrite | ||
name: eks-pod-identity-agent | ||
version: v1.1.0-eksbuild.1 | ||
``` | ||
|
||
You can verify that this is running on your kubernetes cluster with `kubectl get deploy -A | grep eks` | ||
|
||
## Mapping a service account to an IAM role | ||
|
||
Now that you have created a role `capi-test-role` in AWS, and have added the EKS agent to your cluster, we must add the following to our `AWSManagedControlPlane` under `.spec.podIdentityAssociations` | ||
|
||
```yaml | ||
# [...] | ||
kind: AWSManagedControlPlane | ||
spec: | ||
# [...] | ||
podIdentityAssociations: | ||
- serviceAccount: | ||
namespace: default | ||
name: myserviceaccount | ||
roleARN: arn:aws:iam::012345678901:role/capi-test-role | ||
``` | ||
|
||
- `serviceAccount.namespace` and `serviceAccount.name` refer to the [`ServiceAccount`](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/) object in the Kubernetes cluster | ||
- `serviceAccount.roleARN` is the AWS ARN for the IAM role you created in step 1 (named `capi-test-role` in this tutorial). Make sure to copy this exactly from your AWS console (`IAM > Roles`). | ||
|
||
To use the same IAM role across multiple service accounts/namespaces, you must create multiple associations. | ||
|
||
A full CAPA example of everything mentioned above, including 2 role mappings, is shown below: | ||
|
||
```yaml | ||
kind: AWSManagedControlPlane | ||
apiVersion: controlplane.cluster.x-k8s.io/v1beta1 | ||
metadata: | ||
name: "capi-managed-test" | ||
spec: | ||
region: "eu-west-2" | ||
sshKeyName: "capi-management" | ||
version: "v1.27.0" | ||
addons: | ||
- conflictResolution: overwrite | ||
name: eks-pod-identity-agent | ||
version: v1.1.0-eksbuild.1 | ||
podIdentityAssociations: | ||
- serviceAccountNamespace: default | ||
serviceAccountName: myserviceaccount | ||
serviceAccountRoleARN: arn:aws:iam::012345678901:role/capi-test-role | ||
- serviceAccountNamespace: another-namespace | ||
serviceAccountName: another-service-account | ||
serviceAccountRoleARN: arn:aws:iam::012345678901:role/capi-test-role | ||
``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're all called
serviceAccountXXX
which is an antipattern in Kubernetes manifests. I'd recommend moving these to.serviceAccount.{name,namespace,roleARN}
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal here was to be overtly explicit as I have found that our customers get confused on that sometimes. Nothing that docs cannot fix however
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In thinking more about this, I remembered that the kubernetes Pod spec switched from
serviceAccount
toserviceAccountName
a few years back.I do err on the side of clarity for maintainability's sake when it comes to naming, but would definitely not go against whatever is considered standard. Do you have a reference for the naming convention to help me decide?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In thinking about this more, whilst the medium is a CRD, the keys are in reference to an AWS object. I would expect a PodIdentityAssociation.Name to refer to the name of an AWS object when using the AWS API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pod.spec.serviceAccountName
is a singular field that won't change. It doesn't allow referring to another namespace on purpose.The 3 fields you're adding will always be filled and none can be optional or defaulted (even the namespace), so a common prefix is unusual for newly-designed CRDs. See for example
Ingress.spec.defaultBackend.service.{name,port}
or Gateway API'sGateway.[...].listeners[*].{name,protocol,port}
. Core APIs such asPod
aren't great examples since they can't be evolved without lots of breaking changes, and also a pod spec mostly only refers to names and doesn't provide many comparable examples here.Therefore, here's my suggestion (dedenting
roleARN
as compared to my initial comment, since it's not referring to a service account):There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is definitely a back and forth on the semantics on this. I personally dont see this as being a blocking change, and moreso a semantic preference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least the JSON field name and struct name should be aligned, since currently they're not lowercase-equal which could be confusing:
Or the other way around (
json:"roleARN"
) – doesn't matter.Then the documented examples in
docs/book/src/topics/eks/pod-identity-associations.md
must be converted to the final naming choice.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, the roleARN is an AWS resource, not a service account, so the naming shouldnt be consistent. The suggested shouldn't be committed imo