Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting backend passwords with docker compose secrets #3606

Closed
aljora opened this issue May 15, 2024 Discussed in #3291 · 9 comments · Fixed by #3656
Closed

Setting backend passwords with docker compose secrets #3606

aljora opened this issue May 15, 2024 Discussed in #3291 · 9 comments · Fixed by #3656

Comments

@aljora
Copy link
Contributor

aljora commented May 15, 2024

Discussed in #3291

Originally posted by Weizenritter March 11, 2024
Hey there. Since I like checking in all my configs I like them to be free of secrets in clear text. I wonder if it's possible to provide the value of the variable SMTP_PASSWORD with an docker compose secret. Here an example of what I would imagine:

---
services:
  mealie:
    image: ghcr.io/mealie-recipes/mealie:v1.3.2
    container_name: mealie
    ports:
      - "9925:9000"
    volumes:
      - ./mealie_data:/etc/mealie/data
    environment:
      - TZ=Europe/Berlin
      - BASE_URL=https://mealie.example.com
      - SMTP_AUTH_STRATEGY=TLS
      - [email protected]
      - SMTP_HOST=smtp.example.com
      - SMTP_PASSWORD_FILE=/run/secrets/smtp_password  # <-- Look here
      - SMTP_PORT=587
      - [email protected]
    secrets:
      - source: smtp_password
secrets:
  smtp_password:
    name: my_smtp_password
    file: /path/to/smtp_password

Note the _FILE suffix, it's a convention that some official images implement, as stated in the documentation of docker compose (scroll to the bottom for the note).

If it's possible in some other way I'd like to here how, thanks in advance!

@hay-kot
Copy link
Collaborator

hay-kot commented May 15, 2024

We don’t support secrets and we don't have any specific plans to add support. I think you could accomplish what you want with an env file though.

https://docs.docker.com/compose/environment-variables/set-environment-variables/#use-the-env_file-attribute

@hay-kot
Copy link
Collaborator

hay-kot commented May 15, 2024

Also, closing this since it’s not an issues and just links to a discussion.

@hay-kot hay-kot closed this as completed May 15, 2024
@aljora
Copy link
Contributor Author

aljora commented May 15, 2024

I created this issue to ask if there would be interest in a PR to enable the built-in support from pydantic. It seems GitHub created the issue content only from the original post on the discussion.
The page you linked to specifically recommends against using the environment for secrets. So would you be interested in a pull request adding the secrets directory to the pydantic configuration?
aljora@bc1d1b9

@hay-kot
Copy link
Collaborator

hay-kot commented May 15, 2024

Happy to look at a PR thanks!

@andrewvaughan
Copy link

I would request reconsidering supporting secrets - here are two fairly-trivial examples of how this can be implemented in a backwards-compatible way, if interested:

MySQL (does so during Entrypoint, and is my generally-recommended method): https://github.com/docker-library/mysql/blob/1a703318803fa85bf69cb5d2e5b3f1c4087627b5/docker-entrypoint.sh#L24-L43

WordPress (does so dynamically during runtime): https://github.com/docker-library/wordpress/blob/8c2ded17022e67c6accacfae637c5a577ce169af/wp-config-docker.php#L26-L40

I mentioned this in this discussion, but the existing documentation actually is an anti-pattern that can actually decrease security for users due to Mealie's incorrect implementation of the Docker secret pattern.

@andrewvaughan
Copy link

andrewvaughan commented Jun 22, 2024

If you convert my discussion (or another) to an Issue, I will submit a PR today, as it seems this can be added fairly-trivially in your entry.sh file.

Edit: Additionally, I would likely not have to undo some of the work in #3606 and implement the solution with the Docker Compose recommended pattern. I just wanted to make the contributors aware of the fact that I see this change, and explain how this is an incorrect implementation in my other discussion. While this is an anti-pattern solution, I don't want to break backwards-compatibility for any users that followed it.

@aljora
Copy link
Contributor Author

aljora commented Jun 22, 2024

If you convert my discussion (or another) to an Issue, I will submit a PR today, as it seems this can be added fairly-trivially in your entry.sh file.

As the outside contributor who made this issue and the PR, my advice is don't convert your discussion to an issue. If you want to enhance the project, the project policy seems to be go directly to a PR.

@andrewvaughan
Copy link

@aljora I appreciate that advice, thanks! I believe your changes can be kept (we don't want to break backwards compatibility); the _FILE standard can simply be added.

@michael-genson
Copy link
Collaborator

the project policy seems to be go directly to a PR

This is pretty much true, though there are no guarantees that a PR is accepted. In this case, simply adding support for the _FILE standard seems to make sense to me, as long as we don't break existing installs.

Issues are really only for bugs/defects, although they're not required. There are plenty of PRs that get merged without an issue being filed. Discussions are sufficient for new features (they're still not required, but there's a possibility we decide not to merge it for whatever reason, e.g. if we feel it's needlessly complicated, or there's a broader issue that isn't resolved)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants