fix(vector-aggregator): Fix the conditions for enable/disable SA on the HAProxy #122
Conversation
| {{- if .Values.haproxy.serviceAccount.create }} | ||
| serviceAccountName: {{ include "libvector.serviceAccountName" . }}-haproxy | ||
| {{- end }} |
There was a problem hiding this comment.
I think it's common to not wrap this in a conditional, see: https://github.com/haproxytech/helm-charts/blob/main/haproxy/templates/deployment.yaml#L52
Do you have any examples of this pattern in other charts?
There was a problem hiding this comment.
Hello, yes indeed this is not a very good solution, the problem is the name of the "ServiceAccountName" "-haproxy" in the deployment file.
When we don't want to activate the "ServiceAccount" in the haproxy, it will use the default "default" and add "-haproxy" at the end to give this name: (default-haproxy) and the replica will fail because it cannot find this serviceaccount.
The "default" value is supplied by the "libvector.serviceAccountName" function present in the "vector-shared" helm which has no condition on the creation value (true / false) of a "ServiceAccount"
Suddenly, it necessarily returns "default".
However, it would be interesting to use the haproxy.serviceAccountName function as for the "vector" helm to correct this problem. :)
There was a problem hiding this comment.
Doh - oh yeah, much better solution 😄 I missed that completely.
spencergilbert
changed the title
This PR fix the 'if' condition in the serviceaccount.yaml file for use only the value in haproxy block in the value file (haproxy.serviceAccount.create).
And add a condition 'if' in the deployment of haproxy for add or remove the annotation 'serviceAccount' if the value is 'haproxy.serviceAccount.create' is true or false.
If I understood correctly, the SA is only used for the PSP and it is deprecated from version v1.21.