feat: Support Pod Identity for EKS add-ons #465
Conversation
| service_account_role_arn = try(each.value.service_account_role_arn, null) | ||
|
|
||
| dynamic "pod_identity_association" { | ||
| for_each = try(each.value.pod_identity_association, []) |
There was a problem hiding this comment.
Is a list appropriate? An addon can only be associated to a single pod identity, right? Maybe an object would be better?
| for_each = try(each.value.pod_identity_association, []) | |
| for_each = try(each.value.pod_identity_association, null) != null ? [each.value.pod_identity_association] : [] |
There was a problem hiding this comment.
Hello @lorengordon ,
Thank you for your time in reviewing this PR, as well as your feedback. Let me explain why I think keeping this a list would be best for forward maintainability.
An addon can only be associated to a single pod identity, right?
It is accurate to say today there is a 1-1 relationship between AWS add-on and IAM role (and subsequent Pod Identity association). This is due to the current add-on state where each add-on has a single service account which needs IAM access. I verified this by reviewing both AWS documentation on add-ons and review of the pre-baked pod identities under terraform-aws-modules.
However, it is technically possible for future add-ons to have multiple IAM roles/Pod Identities associated with them. This is because the Pod Identity relationship is 1-1 between IAM role and service-account. A future hypothetical example of a 1-M add-on to Pod Identity relationship follows. Imagine an add-on which combined the various observability stacks into a single, cooperative stack, i.e. a combination of amazon-managed-service-prometheus, aws-cloudwatch-observability, otel, etc. In this situation to maintain least privilege RBAC, the add-on would need at least 2 different IAM role -> service-account Pod Identity mappings, 1 for prometheus service-account, a separate one for observablity service-account, etc.
Now granted this is entirely hypothetical, and may end up being YAGNI. In my opinion, since the option technically exists, I recommend to make this a list, so if the module needs to support it later we don't have to make a breaking change to the input and potential associated major release of the module. All that being said I am not a maintainer of the project, and defer this kind of maintainability decision to y'all; I am cool either way.
Given this context above, please let me know if you wish to proceed with the interface change; I would not mind making the change and re-running tests.
There was a problem hiding this comment.
TIL, the api doc would appear to agree with you. It accepts an array instead of an object...
Fyi, I'm not a maintainer here, I was just interested in this same feature.
|
This PR has been automatically marked as stale because it has been open 30 days |
|
Not stale |
|
This PR has been automatically marked as stale because it has been open 30 days |
|
Still not stale |
2 participants
What does this PR do?
Support Pod Identity for EKS add-ons. This is an alternative to IRSA available for many EKS add-ons such as VPC CNI, CloudWatch Observability, EBS-CSI, etc.
Motivation
pod_identity_associationforaws_eks_addon#463More
pre-commit run -awith this PRFor Moderators
Additional Notes
Deploying the minimal test case into my account was successful for most services and add-ons, including VPC-CNI.
During the test there were 2 issues seen which do not appear related to the changes in this PR:
Click to see screenshot of end of terraform apply log
For prod operations, I usually split out each add-on into their own terraform state, or their own blueprint module instantiation, so that we can order the deploy appropriately using
depends_onor other orchestration such as terragrunt dependencies. Doing such a refactor did not seem appropriate for the scope of this PR.Despite the apply issue, the cluster was generally in good health, based on inspection of changed service logs and looking at the pod overview. We see also evidence in logs the eks pod identity is used.
Click to see pods overview
Click to see logs of changed services
There was no issues seen during the terraform destroy of the resources.