banzaicloud / banzai-charts

Curated list of Banzai Cloud Helm charts used by the Pipeline Platform
Apache License 2.0
367 stars 278 forks source link

[Cadence] Pull value from secret rather than plaintext in job #1302

Closed johnkost closed 2 years ago

johnkost commented 2 years ago
Q A
Bug fix? yes
New feature? no
API breaks? no
Deprecations? no
Related tickets fixes #1301
License Apache 2.0

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

sagikazarmark commented 2 years ago

@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.

johnkost commented 2 years ago

@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.

johnkost commented 2 years ago

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!