Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(helm): Sidecar and Service Account Update #509

Merged
merged 13 commits into from
May 11, 2024

Conversation

jluckett-panw
Copy link
Contributor

@jluckett-panw jluckett-panw commented May 10, 2024

Enable extra containers and service account customization to facilitate running a sql auth proxy as a sidecar. The service account customization was necessary to enable alternative authentication methods such as workload identity.

Please squash when merging.

resolves #507

@jluckett-panw
Copy link
Contributor Author

link #507 to this PR

@jluckett-panw
Copy link
Contributor Author

Bumped chart version a little so the lint won't fail so hard.

@burningalchemist
Copy link
Owner

burningalchemist commented May 10, 2024

There are some small linting issues for values.yaml:

Error: 33:25 [trailing-spaces] trailing spaces
Error: 181:28 [new-line-at-end-of-file] no new line character at the end of file

@jluckett-panw
Copy link
Contributor Author

My mistake - every pipeline is a little different. Will push a fix now.

@burningalchemist
Copy link
Owner

Yeah, no rush. The linter is quite strict, just to make sure the helm install/upgrade tests are stable.

I guess you need to make sure the last line has a new line control character, but is not an empty line itself.

@jluckett-panw
Copy link
Contributor Author

As many a famous developer has said "it passes on my machine now, give it a shot"
🙏

@jluckett-panw
Copy link
Contributor Author

I should get a first MR badge from this! I am so sorry for the trouble - my conditional wasn't complete I should've caught that.

@burningalchemist
Copy link
Owner

@jluckett-panw no worries at all, take your time. 😃👍

@jluckett-panw
Copy link
Contributor Author

jluckett-panw commented May 11, 2024

Last time today - I have manually run through your ci process on a GKE cluster and upgrades go off without a hitch. I will not bother you again until Monday if this one doesn't work.

TIA!

@burningalchemist burningalchemist self-requested a review May 11, 2024 07:30
Copy link
Owner

@burningalchemist burningalchemist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jluckett-panw LGTM! 🚀 Thanks a lot for your contribution! 👏

@burningalchemist burningalchemist changed the title Sidecar and Service Account Update feat(helm): Sidecar and Service Account Update May 11, 2024
@burningalchemist burningalchemist merged commit 18936a6 into burningalchemist:master May 11, 2024
4 checks passed
@burningalchemist
Copy link
Owner

burningalchemist commented May 11, 2024

@jluckett-panw just a heads up - I've readjusted the logic for serviceAccount object creation/referencing here. Your changes are already released as of v0.4.6. v0.5.0 has a slightly different mechanism, so it's a breaking change. Please be aware in case of upgrade. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm chart improvement: additionalContainers
2 participants