-
-
Notifications
You must be signed in to change notification settings - Fork 362
feat: add pod identity association support for EKS addons #256
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
feat: add pod identity association support for EKS addons #256
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
variables.tf (1)
175-180
: Add an explanatory comment for the new attribute
pod_identity_association
is 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-system
The nested block allows only
service_account
androle_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 |
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.
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