Support for docker/podman secrets loading secrets from secret files

[mredig]

A common convention for using docker and podman is to provide a secondary environment variable option with a suffix of _FILE for secret variables.

For example, the Postgres docker image supports the env var POSTGRES_USER, but it also supports POSTGRES_USER_FILE. You can then set POSTGRES_USER_FILE to the path of a file containing the expected username.

This is how docker and podman secrets work as they expose secrets at container runtime via the path /run/secrets/[secretname]

I simply added a step in the entry point script that scans for the environment variables that are secret worthy, if they don't exist already, it checks their _FILE variant. If that exists, it'll read it in and store it in the expected variable.

Ideally, I think this would be better done in the main program, but I don't have time to learn Go and all that. In the meantime, this provides nearly the same functionality in the entry point script. If possible, it'd be great to get this merged in until someone else can take on that responsibility. (should be an easy "first contributor" task, I would imagine)

[knadh]

Makes sense @mredig. Is it possible to contain this logic inside the Docker compose file entirely? entry-point.sh is currently fully agnostic and it doesn't seem right to bring config logic into it, especially hardcoded listmonk config variables.

[mredig]

What if in the entrypoint we iterate over all env vars and process the ones ending in _FILE? I'm not sure how we'd be able to put this logic in the docker compose file.

[knadh]

Could the ${VAR:+ALT} syntax work?

It's a bit hacky but if it works, it would be a fully self-contained .yml file without the .sh

x-db-credentials: &db-credentials
  POSTGRES_USER: ${POSTGRES_USER:-listmonk}
  ...

...
    environment:
      LISTMONK_db__user: ${POSTGRES_USER_FILE:+$(cat ${POSTGRES_USER_FILE})}${POSTGRES_USER_FILE:-${POSTGRES_USER}}
      ...

Also, could you point to a couple prominent projects that use the _FILE pattern? I'm unfamiliar with it.

[mredig]

I'm unfamiliar with that syntax (It looks like bash variable syntax? Am I right that it's just bash embedded in yaml?) It looks like it might work, but it hurts my eyes.

IMO, the best thing to do would be to build support right into the go code.

Second best would be to have the entrypoint iterate over the _FILE env vars and load the appropriate values, but I barely got the sh script edits I submitted here.

My submission is the third. 😆

I understand the desire not to couple the entrypoint with specific variables, but I would argue that the second option is fully decoupled and still agnostic. If your desire is for the script to be easy to migrate to other projects, I'd additionally argue this is a feature that'd be worth bringing along. 😄

Postgres uses it (under the Docker Secrets heading for documentation) Wordpress (same section header) Nextcloud uses it too (I think this is where I saw it). I did a little research after that and found it to be a common pattern. Honestly, this is the first server side app with secrets I've come across since learning about it that doesn't have it. (Not an insult!)

[mredig]

If you're wondering how the others go about it, the only one I've ever dug into was Nextcloud. But it was long enough ago I don't remember what they did. I THINK it was sometime during the launch, in or around the entrypoint, but I really don't remember.

[knadh]

I've never come across the _FILE pattern indeed :)

IMO, the best thing to do would be to build support right into the go code.

I don't think that's a good idea at all, bringing opinionated Docker conventions into the business logic of the app. The app can remain fully agnostic of its environment.

Putting it in the .sh file is o-kay, just that some config logic being there, which can't be discerned by looking at the compose file, seems a bit off. Either way, it's an okay trade-off I guess.

I tested this in a .sh file in isolation, but it'll have to be tested in a container.

load_secret_files() {
  # Capture all env variables starting with LISTMONK_ and ending with _FILE.
  for var in $(compgen -v "LISTMONK_" | grep "_FILE$"); do
    fpath="${!var}"
    if [ -f "$fpath" ]; then
      # If it's a valid file, read its contents and assign that to the var
      # without the _FILE suffix.
      # Eg: LISTMONK_DB_USER_FILE=/run/secrets/user -> LISTMONK_DB_USER=$(contents of /run/secrets/user)
      export "${var%_FILE}"="$(cat "$fpath")"
    fi
  done
}

load_secret_files

[mredig]

I don't think that's a good idea at all, bringing opinionated Docker conventions into the business logic of the app. The app can remain fully agnostic of its environment.

Fair enough! 😄 Not that it would find a lot of use, I personally see it as an option to be used anywhere, Docker or no.

I think it looks great! I'll throw it in my deployment for a bit of testing myself.

[knadh]

Hi @mredig. Did you get a chance to test this?

[mredig]

Hi @mredig. Did you get a chance to test this?

Sorry I have not had a chance! I'll try to do that this weekend.

[mredig]

Hi @mredig. Did you get a chance to test this?

I've never come across the _FILE pattern indeed :)

IMO, the best thing to do would be to build support right into the go code.

I don't think that's a good idea at all, bringing opinionated Docker conventions into the business logic of the app. The app can remain fully agnostic of its environment.

Putting it in the .sh file is o-kay, just that some config logic being there, which can't be discerned by looking at the compose file, seems a bit off. Either way, it's an okay trade-off I guess.

I tested this in a .sh file in isolation, but it'll have to be tested in a container.

load_secret_files() {
  # Capture all env variables starting with LISTMONK_ and ending with _FILE.
  for var in $(compgen -v "LISTMONK_" | grep "_FILE$"); do
    fpath="${!var}"
    if [ -f "$fpath" ]; then
      # If it's a valid file, read its contents and assign that to the var
      # without the _FILE suffix.
      # Eg: LISTMONK_DB_USER_FILE=/run/secrets/user -> LISTMONK_DB_USER=$(contents of /run/secrets/user)
      export "${var%_FILE}"="$(cat "$fpath")"
    fi
  done
}

load_secret_files

I updated the PR to use this approach instead. It does, however, require a couple other changes. compgen appears to be a bash builtin (never used it before, so I had to look it up). However, the docker image uses Alpine, which by default doesn't include bash. So, we have to update the Dockerfile to include bash during setup and update the entry file to shebang with bash instead of sh.

If you're okay with these changes, the PR is tested on my end and ready to go from my perspective!

[knadh]

You are right. On second thought, it can be simpler, and there's no need to switch from sh to bash.

load_secret_files() {
  # Capture all env variables starting with LISTMONK_ and ending with _FILE.
  # It's value is assumed to be a file path with its actual value.
  env | grep '^LISTMONK_.*_FILE=' | while IFS='=' read -r var fpath; do
    if [ -f "$fpath" ]; then
      # If it's a valid file, read its contents and assign it to the var
      # without the _FILE suffix.
      # Eg: LISTMONK_DB_USER_FILE=/run/secrets/user -> LISTMONK_DB_USER=$(contents of /run/secrets/user)
      new_var="${var%_FILE}"
      export "$new_var"="$(cat "$fpath")"
    fi
  done
}

load_secret_files

[knadh]

@mredig would you be able to test this some time?