[Cadence] Pull value from secret rather than plaintext in job #1302
Conversation
|
@johnkost Thanks a lot for sending a PR! The problem with this change is that it won't work in every case. When the job is executed as a pre-install hook, the secret won't be there (yet). It would be nice if Helm could provide additional tools to make sure secrets get installed before hooks, but I'm not aware of anything like that. |
I just pushed a fix that uses the conditional for the pre/post hook when determining if we should use the value or secret. Let me know if this addresses your concerns. |
Co-authored-by: Márk Sági-Kazár <sagikazarmark@users.noreply.github.com>
There was a problem hiding this comment.
Your patience is appreciated, I was out of office last week.
Thank you for the PR, I like it in general, I'm also almost perfectly satisfied by it, left a question regarding one of the conditions and an optionally fixable typo comment, I leave the latter to decide up to you.
cadence/templates/server-job.yaml
Outdated
| {{- if or .Values.cassandra.enabled .Values.mysql.enabled }} | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} |
There was a problem hiding this comment.
Optional: typo.
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} | |
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} |
Same at lines 171 and 302.
I am going to add references to those lines as well for tracking and web-UI suggestion committing purposes.
cadence/templates/server-job.yaml
Outdated
| {{- if or .Values.cassandra.enabled .Values.mysql.enabled }} | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} |
There was a problem hiding this comment.
Reference: https://github.com/banzaicloud/banzai-charts/pull/1302/files#r742588952.
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} | |
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} |
cadence/templates/server-job.yaml
Outdated
| {{ - if or .Values.cassandra.enabled .Values.mysql.enabled }} | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} |
There was a problem hiding this comment.
Reference: https://github.com/banzaicloud/banzai-charts/pull/1302/files#r742588952.
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} | |
| name: {{ include "cadence.persistence.secretName" (list $ $store) | quote }} |
pregnor
changed the title
Co-authored-by: Patrik Egyed <8093632+pregnor@users.noreply.github.com>
|
After further testing, I realized what I'm trying to accomplish won't work because of the cadence-sql-cli tool. I was hoping that it would be idempotent but it doesn't do a check to see if the tables exists first resulting in an error if they are run more than once. I'll have to find another work around. Appreciate the help on both PRs! |
What's in this PR?
This PR has the setup job pull from the secret rather than requiring a plaintext password to be passed in
Why?
When deploying the chart using an existing secret and setup job, you have to pass in both the secret and the plaintext password for the job.
Checklist