fmjstudios / helm

🪖 A collection of MIT-licensed Helm Charts
MIT License
13 stars 13 forks source link

(paperless-ngx) fix(charts/paperless-ngx): Fix Redis External Secret #9

Closed MyChaOS87 closed 11 months ago

MyChaOS87 commented 11 months ago

env PAPERLESS_REDIS was always referencing a specific secret which was never built when usind an externalSecret for Redis, rendering paperless.redis.existingSecret useless, although there was some code to create an URI out of the existing secret. Now the URI is constructed by using dependent ENV variables.

Which issue this PR fixes

Special notes for your reviewer

Checklist

mvprowess commented 11 months ago

Hey @MyChaOS87,

I have just deployed a fix in version 0.2.1 mitigating you issue. Check out the commit for more in-depth information.

TL;DR

The existingSecret now requires an existing Secret with a valid Redis connection URI since only using keys from a specific Secret would always require me to create a new one with a full URI later, since setting the environment variables directly isn't good practice. This inadvertently creates duplicated or overlapping secrets - a pain to deal with at scale.

I have chosen not to merge your PR since these new changes make them obsolete and I wan't keep the number of environment variables at a minimum since there are enough in Kubernetes. Thank you for your work nonetheless! 👍🏼

MyChaOS87 commented 10 months ago

Although I see your reasoning, this now renders the chart basically unusable for me. As I now have to render the URI into a sealed secret, where at secret creation time I should not need to know the hostname of my redis instance... I know that this comes from the use of that URI directly in paperless-ngx (which in my opinion is a big flaw on that side), but encoding a full URI in a (sealed-) secret also seems way off to me.

mvprowess commented 10 months ago

Responded in #10. Please restrict further conversation to the (reopened) issue.