Bump gotenberg from `8.5.1` to `8.7.0` & add HTTPS (TLS) support

MaikuMori / helm-charts

Maiku's Helm charts

https://artifacthub.io/packages/search?user=MaikuMori&sort=relevance

MIT License

18 stars 11 forks source link

Bump gotenberg from `8.5.1` to `8.7.0` & add HTTPS (TLS) support #35

Closed jonasgeiler closed 3 months ago

jonasgeiler commented 3 months ago

This PR bumps the gotenberg version from 8.5.1 to 8.6.0, which adds HTTPS (TLS) support so I added the new flags to the helm chart.

https://github.com/gotenberg/gotenberg/releases/tag/v8.6.0

Tasks:

[X] I've made changes
[X] I've bumped chart's version in Chart.yaml
[X] I've added changes to charts CHANGELOG.md
[X] I've run helm-docs

MaikuMori commented 3 months ago

Hey, I was going to add this. I want to mimic what other charts do. Usually in this situation I allow referencing a secret with the keys that is set outside the normal helm deployment, but I'll check how this is usually handled in similar charts.

I rebased and added attribution for now.

jonasgeiler commented 3 months ago

@MaikuMori wrote: Hey, I was going to add this. I want to mimic what other charts do. Usually in this situation I allow referencing a secret with the keys that is set outside the normal helm deployment, but I'll check how this is usually handled in similar charts.

I rebased and added attribution for now.

I think most would just use a volume with the TLS files in it and attach use the volumes/volumeMounts values to attach them. Then you can just set the path to the correct files and you're done. But you could also go full-blown and do it like bitnami with auto-generated certificates and a bunch of other stuff 😅 I'll leave it up to you!

jonasgeiler commented 3 months ago

@MaikuMori If you tell me how exactly you want to implement TLS support I could improve or rework this PR myself if you want.

MaikuMori commented 3 months ago

Hi, sorry, I've been slacking.

Basically, the 2 features I'd like are:

Support setting secret name and 2 related keys inside the secret. (The secret itself would be managed outside, this is my use case)
Bonus: Support setting annotations so this can be done via cert-manager

Regarding volumes:

If we use volumes then maybe make it more automatic
Do we use volumes if secrets are available? If we provide the keys via Helm values (I don't like it, but I understand people would use this) then why not just expose 2 values for that and behind the scenes generate the secret.

This way, all cases are Secret based:

Secret managed by Helm
External secret
cert-manager also makes external secret

What do you think? I could try to add these changes tomorrow.

MaikuMori commented 3 months ago

It was easier than I expected. I think there is no disadvantage of not doing it via manual volume mounts. @jonasgeiler Let me know if there are issues doing it this way in your specific workflow. Personally, I think this is more idiomatic and simpler.

Also, this should support cert-manager certificates. It already supported TLS via ingress annotations previously.

jonasgeiler commented 3 months ago

@MaikuMori sorry for not replying earlier but yeah sounds good! Thanks! I'll try it out.