feat(helm): Set automountServiceAccountToken on service accounts#6873
feat(helm): Set automountServiceAccountToken on service accounts#6873jbw976 merged 1 commit intocrossplane:mainfrom
Conversation
…counts Signed-off-by: Brady Zhang <brady.zhang@appian.com>
📝 WalkthroughWalkthroughTwo Crossplane Helm ServiceAccount templates are modified to add Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Pre-merge checks✅ Passed checks (4 passed)
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. Comment |
|
Do we really need these changes, is it a standard now to set this value explicitly in ServiceAccounts? As its |
|
It is normal for organizations with policy engines like Gatekeeper to require it. Since Gatekeeper cannot distinguish between the defaulting being an intentional setting versus Kubernetes defaulting, it is helpful to show explicit intent to ensure compliance. |
Ok I see the use case. I guess this change does not hurt and could help someone. Could we instead of just setting it to false in the template expose this as a option in the helm chart (default true)? |
|
I attempted to run Crossplane with |
Yeah you would need to mount the token yourself to make it work. Maybe adding the option to helm is to nitpicky and we dont really need it until somebody has a use case. IMO this PR is ok and we can move forward with merging it if a maintainer agrees. |
|
Closing and reopening to see if CI gets unblocked |
jbw976
left a comment
There was a problem hiding this comment.
CI is stuck once again, I wonder if it's because we need to approve the workflow runs too and if that doesn't happen then it gets stuck?
kicking it once again 🙏
Description of your changes
It is considered best practice (see: CIS EKS Benchmark 4.1.6) to only mount the service account token when necessary. In the case of the crossplane charts, the API token is needed as there is are RBAC role bindings to the service accounts. Since
automountServiceAccountTokendefaults totrue, there is no functional change - this only adds clarity to the chart.Fixes #6874
I have:
earthly +reviewableto ensure this PR is ready for review.Added or updated unit tests.Added or updated e2e tests.Linked a PR or a docs tracking issue to document this change.backport release-x.ylabels to auto-backport this PR.Followed the API promotion workflow if this PR introduces, removes, or promotes an API.Need help with this checklist? See the cheat sheet.