stardog-union / helm-charts

Stardog Helm Charts
Apache License 2.0
9 stars 12 forks source link

added option to add a passwordSecretRef for the database setup #95

Closed wenzel-felix closed 1 year ago

wenzel-felix commented 1 year ago

Added a new configuration option admin.passwordSecretRef

The option allows to reference a external secret for the Database password instead of the integrated chart secret. This allows easier integration with Cloud secret services like AWS Secret Manager.

Implementation is very simple - if the admin.passwordSecretRef is set, the admin.password will be ignored. Meaning, the helm chart does not create a secret and it is replaced by the external secret.

fw-netlution commented 1 year ago

@ctffarley

scardena commented 1 year ago

Although the main idea sounds better than just having admin password in values files, I can't really see how is this any different from the current implementation. I also don't see how is this doing any actual reference to an external secrets manager like AWS Secrets Manager.

ctffarley commented 1 year ago

Can you provide more details on how this will be used? For example, are you creating the kubernetes secret outside the helm chart inside the namespace where the stardog helm chart is being deployed?

fw-netlution commented 1 year ago

Hi guys, so the specific use case: We want to deploy stardog via the chart through FluxCD without manual steps. Here setting the pw through the chart's values is not want we would like to do as it means saving it in our git repository. Obviously we could use some kind of hacky work around, but the cleanest solution would be to use an external secrets provider or CSI driver to get secrets from AWS, Azure etc. into the environment. In our specific case it would be this.

To answer @ctffarley - exactly this is what we would wanna use it for. Instead of bloating your chart with different ways of using external secret sources for the password, it would allow a "bring your own secret" setup. To be fair, with my current changes it would not support for example the AWS SM CSI driver, but it would be a first step towards allowing external secrets through the cloud agnostic tool I shared before and would also allow to just use a external secret in general.

One question to you guys, is there a specific reason for the secret being stored in the fs instead of defining it as environment variable?

ctffarley commented 1 year ago

OK that makes sense. Can you document this option by adding it to the table in this readme: https://github.com/stardog-union/helm-charts/blob/develop/charts/stardog/README.md

wenzel-felix commented 1 year ago

Ok, the README is updated.

fw-netlution commented 1 year ago

Hi @ctffarley, any chance to merge and release this change in the coming days?

fw-netlution commented 1 year ago

Thank you!

ctffarley commented 1 year ago

@fw-netlution I've just published release 2.0.9 which includes your changes.