[helm] Add existingSecret support for SASL credentials#3171
[helm] Add existingSecret support for SASL credentials#3171polyzos merged 2 commits intoapache:mainfrom
Conversation
affo
left a comment
There was a problem hiding this comment.
Hello @fresh-borzoni and thanks for the contribution!
This is definitely a must-have 🤝
I do have doubt for the solution implementation, not for the values design that looks perfect 🤝
This PR is actually duplicating the contents of the JAAS configuration into a "hidden" script in templates, we would end up having the same configuration template in both the configmap (https://github.com/apache/fluss/blob/main/helm/templates/secret-jaas-config.yaml) and in the scripts.
We may tackle this by making both ways of rendering pass through the script.
However, I don't like that much to have that as it is quite hard to maintain and the resulting rendering can only be tested by actually running a Fluss cluster.
The other solution that I have in mind would be: why don't we treat the contents of jaas conf as text file with and load it, and make the init script template it?
Best would be if the text file is actually a gomplate template, so that we can also easily template it locally for verifying the context.
This would need the init container to be one that ships gomplate, see https://docs.gomplate.ca/installing/#use-with-docker.
I see drawbacks in this, as we would require to download that image as init container.
Happy to debate 🤝
587f930 to
9ba3a5f
Compare
|
@affo Thank for the nice suggestions, I really liked the idea with envsubst. |
affo
left a comment
There was a problem hiding this comment.
@fresh-borzoni thanks for the prompt addressing, now this looks perfect:
- favor testable and maintainable approach for ConfigMap for JAAS conf by
envsubstwith env vars - consistency between internal and client security
* [helm] Add existingSecret support for SASL credentials * fix leftover from initial sketch
closes #3172
Allow SASL credentials to come from pre-existing Kubernetes Secrets via existingSecret (internal, zookeeper) and per-user passwordSecretRef (client), so passwords don't need to live in values.yaml or the Helm release.
On rendering path, an init container reuses the main Fluss image to write jaas.conf from secretKeyRef env at pod startup, the literal-values path is unchanged.