feat: add pod identity association support for EKS addons#256
feat: add pod identity association support for EKS addons#256oycyc merged 6 commits intocloudposse:mainfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes introduce support for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
variables.tf (1)
175-180: Add an explanatory comment for the new attribute
pod_identity_associationis a brand-new field and its purpose is not obvious from the surrounding context. A one-line comment (in the same style as the neighbouring attributes) will make the intent clear for future maintainers and keep the object definition self-documenting.- pod_identity_association = optional(map(string), {}) + # Map of <service_account> => <role_arn> pairs used to create + # `pod_identity_association` blocks inside `aws_eks_addon` + pod_identity_association = optional(map(string), {})main.tf (1)
181-184: Optional: expose namespace when addons are not inkube-systemThe nested block allows only
service_accountandrole_arn, which works for addons living inkube-system.
Some AWS-managed addons (or future custom ones) run in other namespaces. Consider extending the variable to accept an object{ namespace = "foo", role_arn = "…" }for maximum flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
main.tf(1 hunks)variables.tf(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
|
/terratest |
|
Good addition, thanks @litanyofmadness ! |
Head branch was pushed to by a user without write access
|
@oycyc, thanks for reviewing my PR. I've bumped the Kubernetes version and Kubernetes Go packages in tests to fix this error:
I'm unsure if it works, so I would appreciate any help. |
|
/terratest |
|
/terratest |
oycyc
left a comment
There was a problem hiding this comment.
Agh that's annoying. If this doesn't work, let me try to loop someone in from CloudPosse
|
Thanks for fixing the tests @litanyofmadness !! |
|
These changes were released in v4.7.0. |
what
why
references
Terraform EKS Addon Pod Identity: Docs
Closes: #252