Kong / charts

Helm chart for Kong
Apache License 2.0
249 stars 480 forks source link

automountServiceAccountToken: false causes crashloop with Kuma DP sidecar #467

Closed robertblowersaviva closed 3 years ago

robertblowersaviva commented 3 years ago

A recent issue #388 and related PR #389 addressed that a K8s Service Account shouldnt be auto mounted unless it was necessary for the pod to talk with the APIs i.e. when Kong Ingress Controller is deployed.

An unintended side effect appears to have been generated when deploying Kong Gateway on K8s (full rather than just ingress controller) hence ingressController.enabled=false AND you want to use Kuma/Kong Mesh sidecar which needs the K8s service account.

Without access this causes a crashloopbackoff in the Kuma DP sidecar Error: could not read file /var/run/secrets/kubernetes.io/serviceaccount/token: stat /var/run/secrets/kubernetes.io/serviceaccount/token: no such file or directory

I think this will need to be driven off a seperate property rather than hooking with the ingress controller enablement; or a seperate property for identifying when the Kong Helm Chart is being deployed alongside Kuma/Kong Mesh

I have published further detail in Kong Discuss originally asking for some assistance getting to the root cause of the issue https://discuss.konghq.com/t/kuma-sidecar-missing-serviceaccount-secret-mount-crashloopbackoff-occurs/9131

Any advice or steer on the best way to resolve this will be appreciated. If any further detail is needed I'm happy to provide.

Rob

robertblowersaviva commented 3 years ago

Issue #418 and PR #455 look to address this aspect looking through the Issue history the impact on Kuma sidecars is known but not published. Looks to be resolved if after this PR I set .Values.deployment.serviceAccount.name to be "default" to mount the default security account.

This is probably me being a helm/k8s newbie - but I'm not sure in the case of spinning up mesh with gateway this will be clear from what's been documented that this will need to be set in order not to fall into the sidecar trap?

Happy to be corrected!

Rob

robertblowersaviva commented 3 years ago

{{- define "kong.serviceAccountName" -}} {{- if .Values.ingressController.serviceAccount.create -}} {{ default (include "kong.fullname" .) .Values.ingressController.serviceAccount.name }} {{- if .Values.deployment.serviceAccount.create -}} {{ default (include "kong.fullname" .) .Values.deployment.serviceAccount.name }} {{- else -}} {{ default "default" .Values.ingressController.serviceAccount.name }} {{- end -}}

Does it also need {{ default "default" .Values.deployment.serviceAccount.name }} in _helpers.tpl so it's more out of the box?

rainest commented 3 years ago

Yeah, it should. Unintended holdover from an earlier variation. #468 updates the pre-release version for consistency. Thanks for pointing out the issue.