feat(deployment) replace static secret with projected volume

[mcharriere]

What this PR does / why we need it:

This PR replaces the static service account secret with a projected volume. The projected volume is the standard way to authenticate against the K8s API since v1.21.
With this change we keep the token mounted only on the ingress controller container and gain the security improvements that the TokenRequests API brings.

Which issue this PR fixes

• fixes Use projected token or TokenRequest API kubernetes-ingress-controller#2731

Special notes for your reviewer:

• Rotating tokens.

These tokens rotate. Our existing code has nothing to handle this, though it's possible that either client-go or controller-runtime support it natively. We need to confirm behavior after a rotation and build support for it if none exists in libraries.

client-go introduced support for rotating tokens in kubernetes/kubernetes#67359

• Expiration time

The expiration time has been set to 3607 following the Kubernetes default value.

Checklist

• PR is based off the current tip of the main branch.
• Changes are documented under the "Unreleased" header in CHANGELOG.md
• Commits follow the Kong commit message guidelines

[CLAassistant]

All committers have signed the CLA.

[rainest]

@mcharriere do you know if this will break backwards-compatibility with older k8s versions (and which?) that don't support projected volumes? This looks fine and is something we should do, we just need to know what older versions would break with projected volumes and whether we need to add any additional templates for older-version compatibility.

[mcharriere]

@rainest, yes, you are right. this change breaks on 1.19 and older when Values.deployment.serviceAccount.automountServiceAccountToken is enabled (default).

One option is to increase the K8s required version to 1.20 (https://github.com/Kong/charts/blob/main/charts/kong/README.md#prerequisites) and document somewhere that older versions would work disabling automountServiceAccountToken.
Alternatively, we could check for the KubeVersion and use the static secret instead.

WDYT?

[ubergesundheit]

helm supports restricting kubernetes versions by specifying field kubeVersion: ">=1.22.0-0" in Chart.yaml.
https://helm.sh/docs/topics/charts/#the-kubeversion-field

[pmalek]

Hi @ubergesundheit @mcharriere 👋

Alternatively, we could check for the KubeVersion and use the static secret instead.

If possible I'd much rather prefer this route at this point to not break our users. What do you think about introducing this capabilities check (via helm) in this PR?

BTW: what specific change in k8s is this leveraging that breaks 1.19 and older? I've looked through various docs and couldn't find anything specific that would point to that version.

[mcharriere]

If possible I'd much rather prefer this route at this point to not break our users. What do you think about introducing this capabilities check (via helm) in this PR?

Sounds good.

BTW: what specific change in k8s is this leveraging that breaks 1.19 and older? I've looked through various docs and couldn't find anything specific that would point to that version.

In 1.20, the RootCAConfigMap graduated to beta, which enabled it by default. Meaning that this configMap mount fails in previous versions:

            - configMap:
                items:
                - key: ca.crt
                  path: ca.crt
                name: kube-root-ca.crt

I didn't check if there are alternatives for <1.20, as I think for those cases keeping the current behavior should be fine having in mind that the ingress controller itself doesn't support 1.19 since v2.6.

[pmalek]

In 1.20, the RootCAConfigMap graduated to beta, which enabled it by default. Meaning that this configMap mount fails in previous versions:

            - configMap:
                items:
                - key: ca.crt
                  path: ca.crt
                name: kube-root-ca.crt

This was the missing bit. Thanks!

I didn't check if there are alternatives for <1.20, as I think for those cases keeping the current behavior should be fine having in mind that the ingress controller itself doesn't support 1.19 since v2.6.

We can retain the old behaviour for those versions, no?

[mcharriere]

We can retain the old behaviour for those versions, no?

yes, I just pushed a commit keeping the old behavior for <1.20. I tested it on k8s versions 1.19, 1.20 and 1.25 and it works as expected in all of them.

[rainest]

Just added #725 to test across versions, so if ya'll can fetch rebase onto our current main it should confirm the new and legacy behavior.

@pmalek should we indeed need some additional CI values to test this? Although this isn't configured explicitly in any I know of, that shouldn't be an issue since we create the account by default:

charts/charts/kong/values.yaml

Lines 26 to 27 in 892774e

	serviceAccount:
	create: true

[pmalek]

@pmalek should we indeed need some additional CI values to test this? Although this isn't configured explicitly in any I know of, that shouldn't be an issue since we create the account by default:

charts/charts/kong/values.yaml

Lines 26 to 27 in 892774e

	serviceAccount:
	create: true

Yes, this should be enough for this PR since we want to test the old and new behaviours for pre 1.20 and 1.20 and up.

[mcharriere]

Just added #725 to test across versions, so if ya'll can fetch rebase onto our current main it should confirm the new and legacy behavior.

@rainest thanks! that's great. the branch is up to date now.

[seh]

Will this change make its way over into the manifests that sit in the deploy directory of the Kong/kubernetes-ingress-controller repository?

[rainest]

@seh there's no reason it shouldn't. Kong/kubernetes-ingress-controller#3475 to track that.