Skip to content

feat(installations): support secrets file for all environment variables#5125

Open
cethien wants to merge 1 commit intohomarr-labs:devfrom
cethien:feat/docker-env-file-dir-support
Open

feat(installations): support secrets file for all environment variables#5125
cethien wants to merge 1 commit intohomarr-labs:devfrom
cethien:feat/docker-env-file-dir-support

Conversation

@cethien
Copy link

@cethien cethien commented Feb 25, 2026

was trying to deploy it in docker swarm with secret managment which doesnt work without some fragile entrypoint workaround.
added optional __FILE suffix for environment variables.
also seems that its more or less a convention to use optional _FILE

sidenote: this was hacked together in a minute, tho in theory shouldnt break anything from a quick grep of the environment variables

@cethien cethien requested a review from a team as a code owner February 25, 2026 11:31
@deepsource-io
Copy link
Contributor

deepsource-io bot commented Feb 25, 2026

DeepSource Code Review

We reviewed changes in 7ca38c3...3d4e5ae on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Feb 25, 2026 11:33a.m. Review ↗

@cethien cethien force-pushed the feat/docker-env-file-dir-support branch from fd54810 to 3d4e5ae Compare February 25, 2026 11:33
@manuel-rw
Copy link
Member

Hi, thanks for the contribution.
Could you elaborate why the suffix doesn't work for Swarms?
Perhaps instead of fighting the consequence, we could rename it and fix the root problem?

@manuel-rw manuel-rw added the decision A decision needs to be taken label Feb 25, 2026
@cethien
Copy link
Author

cethien commented Feb 25, 2026

Hi,

could be that my wording was wrong, english is not my native, sorry about that 😄

deploying homarr on swarm does work without issues, the "problem" is when working with secrets property in the compose.
usually deployed MY_SECRET=/run/secrets/my-secret.

in case of homarr when doing eg SECRET_ENCRYPTION_KEY: /run/secrets/encryption-key it results in homarr taking the directive as the value, causing an error.

many containers support _FILE or __FILE alternatives for environment variables so figured would be cleaner that way and propably help others. since it mainly an entrypoint wrapper i dont think it should break anything either

@Meierschlumpf
Copy link
Member

We already have this for the secret encryption key:

homarr/scripts/run.sh

Lines 23 to 25 in 12ba14b

if [ -n "$SECRET_ENCRYPTION_KEY_FILE" ] && [ -f "$SECRET_ENCRYPTION_KEY_FILE" ]; then
export SECRET_ENCRYPTION_KEY=$(cat $SECRET_ENCRYPTION_KEY_FILE)
fi

However I think it makes sense to support it for all environment variables. The only thing we have to decide in my opinion is if we

  1. Use the _FILE suffix like it already exists for the encryption key and therefore prevent a breaking change (that is required at some point, but not has to be now). However then we have problems with environment variables that actually end with _FILE which would then be removed
  2. Use the new __FILE suffix like suggested while still supporting the _FILE suffix for secret encryption key until the next major version where we then remove it as breaking change.

@Meierschlumpf Meierschlumpf changed the title add(docker): added support for __FILE suffix in docker feat(installations): support secrets file for all environment variables Feb 25, 2026
@Meierschlumpf
Copy link
Member

Meierschlumpf commented Feb 25, 2026

In the secrets in compose example from docker they use the _FILE suffix
image

@Meierschlumpf
Copy link
Member

We probably should use something like this for the iteration:

matching_var_names=$(env | cut -d '=' -f 1 | grep "_PATH$")
cat "$(echo ${!var})"

@cethien
Copy link
Author

cethien commented Feb 26, 2026

We already have this for the secret encryption key:

homarr/scripts/run.sh

Lines 23 to 25 in 12ba14b

if [ -n "$SECRET_ENCRYPTION_KEY_FILE" ] && [ -f "$SECRET_ENCRYPTION_KEY_FILE" ]; then
export SECRET_ENCRYPTION_KEY=$(cat $SECRET_ENCRYPTION_KEY_FILE)
fi

okay, i checked and was in this case an error on my end, the version i had running was old and the run script didnt contain the _FILE suffix

However I think it makes sense to support it for all environment variables. The only thing we have to decide in my opinion is if we

1. Use the `_FILE` suffix like it already exists for the encryption key and therefore prevent a breaking change (that is required at some point, but not has to be now). However then we have problems with environment variables that actually end with `_FILE` which would then be removed

2. Use the new `__FILE` suffix like suggested while still supporting the `_FILE` suffix for secret encryption key until the next major version where we then remove it as breaking change.

since this is not my project i let you guys decide. my 2 cents:
the references use _FILE, afaik linuxcontainers.io images also use the _FILE directive, so _FILE is propably the way to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

decision A decision needs to be taken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants