search

linuxserver / docker-healthchecks

GNU General Public License v3.0
178 stars 38 forks source link

Add SECRET_KEY to env documentation #61

Closed dudeofawesome closed 2 years ago

dudeofawesome commented 3 years ago

Description & Benefits of this PR and context:

The official Healthchecks image says to include a SECRET_KEY env var, but this image doesn't, which leads to a banner at the top of the page saying "Running with an insecure SECRET_KEY value, do not use in production." Simply adding the env var gets rid of this message in the current linuxserver image, so I figured it would be good to document it.

How Has This Been Tested?

I'm using SECRET_KEY on my container.

Source / References:

https://github.com/healthchecks/healthchecks/tree/master/docker

aptalca commented 3 years ago

Wouldn't it be better to generate one during init and store in the config folder?

dudeofawesome commented 3 years ago

Yeah, that would make a lot of sense. I was just copying what they've done in the official image.

github-actions[bot] commented 3 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

chasebolt commented 3 years ago

this looks good to merge in

chasebolt commented 3 years ago

Wouldn't it be better to generate one during init and store in the config folder?

if this was stored in the config then this app becomes stateful, versus currently we are able to run it stateless in k8s. a deploy would destroy all sessions, password reset tokens, etc (according to django docs). it may not affect this app, in particular, a lot. but i don't think anyone has time to go through that amount of testing - env is easier for us to know it will work in the future as intended.

aptalca commented 3 years ago

Then have the env var override any stored value. But if there is no env var, use the stored one, and generate if needed. Best of both worlds.

chasebolt commented 3 years ago

you should definitely add that! or else have it in a different PR so this can move forward.

On Tue, Apr 20, 2021 at 16:55 aptalca @.***> wrote:

Then have the env var override any stored value. But if there is no env var, use the stored one, and generate if needed. Best of both worlds.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/linuxserver/docker-healthchecks/pull/61#issuecomment-823675879, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJKSSBSXFC7V2AKKMLVJH3TJYH77ANCNFSM4XN3LDVQ .

github-actions[bot] commented 3 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dudeofawesome commented 3 years ago

Can a maintainer take a look at this? I believe this change is still relevant, and it's just extra documentation.

Roxedus commented 3 years ago

We have, and came with some feedback on wanted changes

dudeofawesome commented 3 years ago

Ah! I missed that those behavioral changes were wanted in this PR.

I'm not really familiar with the structure of LSIO images. Would the appropriate way to handle this be to create a /config/secret_key file at runtime?

Roxedus commented 3 years ago

That would be a good place to have it, yea.

github-actions[bot] commented 3 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.