Kapp controller doesnt generate new token for a SA with the same name as a deleted SA

[gmrodgers]

What steps did you take:

1. Deploy kapp controller
2. Apply a Package, PackageInstall, ServiceAccount (used in PackageInstall as .spec.serviceAccountName)
3. Delete the ServiceAccount
4. Recreate the ServiceAccount with the same name

What happened: PackageInstall fails as it get the token for the old ServiceAccount which is now invalid.

What did you expect: A new token to be generated for this new ServiceAccount.

Anything else you would like to add: Looks like it was due to the key of the TokenManager cache being non-unique across all ServiceAccounts across the lifetime of the cluster. Two ways around this are using something that is unique (UID of ServiceAccount) or use the TokenReview API which does track whether the ServiceAccount is new or not.

Environment:

• Any branch including https://github.com/vmware-tanzu/carvel-kapp-controller/pull/695.
• Will hit on Kubernetes 1.24.0 and above where ServiceAccounts no longer autogenerate tokens and the TokenRequest API is used

---

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

[neil-hickey]

Hmm, thanks for the issue @gmrodgers !

Funny, I did actually think about this case.

I suspect the only time this will be needed is when a service account is deleted / renamed by the user and we still maybe have it in our cache.

https://github.com/vmware-tanzu/carvel-kapp-controller/pull/695#discussion_r879731513

I didn't really expect the use case to happen - create SA, delete SA, recreate SA. But I guess it can, and it has ;)

that is unique (UID of ServiceAccount) or use the TokenReview API which does track whether the ServiceAccount is new or not.

I'm always in favour of less k8s API calls, we are already known for taking down GKE. So the less the better, a more unique key makes sense.

[cppforlife]

nice catch!

[gmrodgers]

Hey @neil-hickey !

Yep, we catch it when we delete/recreate both the secrettemplate and serviceaccount (in the same file) during testing! So may be quite common.

Less API calls makes sense to me! One thing though is we'd still need to do a Get request for the SA (to obtain the UID) so there would be the same number of calls unless I'm missing something i.e. in UID case it's one SA Get per reconcile, in TokenReview case it's one TokenReview Create per reconcile.
Maybe there is a smarter way to reduce the need of each!

I've already added a TokenReview implementation in this SecretGen PR and would be keen not to block too much on this so if you decide to go down another direction, it would be great if it could be changed in a subsequent PR there and here!