meilisearch / meilisearch-kubernetes

Meilisearch on Kubernetes Helm charts and manifests
https://www.meilisearch.com
MIT License
212 stars 59 forks source link

Generate secret for master key if the env is set to production #62

Closed carlreid closed 3 years ago

carlreid commented 3 years ago

With these changes, if someone were to set MEILI_ENV to production and not set MEILI_MASTER_KEY, then a cryptographically secure random secret (using randAlphaNum) will be created automatically and then loaded as an environment variable.

If you set MEILI_MASTER_KEY in the values.yaml, then that is used instead, so no secret is generated/used. If the env is development then no secret is created at all. You could still set MEILI_MASTER_KEY in the values.yaml, as those just get loaded in no matter what.

Resolves #19 and maybe #38

carlreid commented 3 years ago

I've bumped the versions of the actions, mainly to fix the linting issue (though they were also a bit outdated) as it's a newer helm feature.

carlreid commented 3 years ago
* Mentioning this behavior also in [this section of the chart README](MEILI_ENV: development)

The link is broken, so I made a guess as to where it may be best placed 😄

Finally, regarding #38, I think we can definitely add explicitly MEILI_MASTER_KEY to the values file, but ignore it if it is empty, what do you think? This can be part of a different PR and doesn't;t have to be handled here.

The problem is the value is then nil and since the values are mapped with configMapRef which essentially grabs the whole bunch, then it causes an error. Could would around it by mapping the values individually but then you'd need to do that for all possibilities in the environment. I've put it in the values as a commented out value and then placed the text you wrote above it instead of above the MEILI_ENV.

If the value is there and defined as nil then it may make people think they can have a production environment with a key of nothing (like in development).

You can always feel free to write directly to my fork/branch to make any changes around wording and such. You should have permissions since it's a fork of this repo and it inherits those permissions with fork creation.

eskombro commented 3 years ago

The link is broken, so I made a guess as to where it may be best placed 😄

Oh, I lost the message from my review after writing it, and had to re-write it again, sorry for this error! But you guessed more than right! 😂

Regarding #38: thanks for your explanation! Indeed, adding

# MEILI_MASTER_KEY:

seem to be a good option, as it is at least explicit and self-explanatory, and doesn't add unnecessary complexity!

Thanks a lot for taking all that feedback into account, this LGTM! 🔝

eskombro commented 3 years ago

Bors try

bors[bot] commented 3 years ago

try

Build succeeded:

eskombro commented 3 years ago

Bors merge

bors[bot] commented 3 years ago

Build succeeded: