chore: Allow to set automountServiceAccountToken in ServiceAccount

[kahirokunn]

Is this a bug fix or adding new feature?

new feature

What is this PR about? / Why do we need it?

https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.24.md#urgent-upgrade-notes

The LegacyServiceAccountTokenNoAutoGeneration feature gate is beta, and enabled by default. When enabled, Secret API objects containing service account tokens are no longer auto-generated for every ServiceAccount. Use the TokenRequest API to acquire service account tokens, or if a non-expiring token is required, create a Secret API object for the token controller to populate with a service account token by following this guide. (kubernetes/kubernetes#108309, @zshihang)

Since k8s 1.24, TOKEN is not mounted automatically.
If you want to access with IRSA, you need to use a token.

Allow to set automountServiceAccountToken in ServiceAccount
https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/

What testing is done?

done.

[k8s-ci-robot]

Welcome @kahirokunn!

It looks like this is your first PR to kubernetes-sigs/aws-ebs-csi-driver 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/aws-ebs-csi-driver has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @kahirokunn. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ConnorJC3]

Needs testing with IRSA:

If this breaks IRSA, it should default on

If this doesn't break IRSA, I think we should consider just setting it to false directly, rather than exposing an option (as the option should never matter)

Otherwise lgtm, I'll test and report back on IRSA interactions

[kahirokunn]

@ConnorJC3
In ServiceAccounts that use IRSA in the world, everyone has the accountServiceAccountToken set to True by default.
I actually think it is better that way.
Would you change it that way?

[ConnorJC3]

Actually, upon further reflection, I think this is fine as is. There are usecases where we need the creds, such as the node untainting feature, so this config should be left up to the user.

lgtm, cc @torredil for review

[torredil]

/retest
/lgtm

Hey there, thanks for your contribution. Just one quick thing from me:

• Could we update the docs? the parameter name is pretty intuitive, but ideally new configurable parameters should be documented as it helps other users understand what the parameter does (and why they might want to set it to true or false).

Some general feedback: Suggest providing more context in the PR description. We mention that we're allowing users to configure the parameter and link to the relevant docs -- which is very helpful -- but it would be excellent to explain why a user might want to configure this setting and the implications for doing so. I've had some success in situations like this in the past by spending a few extra minutes providing clarity which might help reviewers get your changes merged in a timely manner 👍

[ConnorJC3]

/ok-to-test
/retest

[kahirokunn]

@torredil Okay. How about now?

[torredil]

Thanks for this @kahirokunn /lgtm

[kahirokunn]

@torredil https://github.com/aws/eks-charts/blob/620e0169ec96d79086f004235d7b1764a60dbc40/stable/aws-load-balancer-controller/values.yaml#L25
By the way, aws-load-balancer-controller was true by default.
I think the same default of true would be fine, what do you think?

[kahirokunn]

Since there are other environments than the EKS environment, default false may be acceptable.

[kahirokunn]

Either is fine.
Can't wait to be merged. 🙌

[ConnorJC3]

/hold

@kahirokunn yeah, I think we should set this to default true. The large majority of our users use IRSA, so changing this out from under them would be a poor experience.

Once the default is changed (don't forget to regenerate the kustomize files too), I can merge this in 👍

[kahirokunn]

@ConnorJC3 thx. done. 👍

[k8s-ci-robot]

@kahirokunn: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-aws-ebs-csi-driver-test-e2e-external-eks-windows	9a30a54	link	false	/test pull-aws-ebs-csi-driver-test-e2e-external-eks-windows

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[ConnorJC3]

I went ahead and manually cleaned the commit message to make the bot happy.

/remove-hold
/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ConnorJC3

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ConnorJC3]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[liggitt]

Since k8s 1.24, TOKEN is not mounted automatically.

This is not accurate, kubernetes 1.24 still mounts tokens as it did before. Those tokens are no longer stored in Secret API objects, but are still mounted by default.

[namgk]

what if you don't install using HELM, what's the way to set this automountServiceAccountToken? For example, people might install through aws eks create-addon --addon-name aws-ebs-csi-driver.

[kahirokunn]

I'd like to know too.