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

Add docker-style file variable support (fixes #189) #328

Merged
merged 12 commits into from
Sep 15, 2021

Conversation

SmileyChris
Copy link
Contributor

@SmileyChris SmileyChris commented Sep 13, 2021

Docker (swarm) and Kubernetes are two widely used platforms that store their Secrets in tmpfs inside containers as individual files.

These are a secure way to be able to share configuration data between containers, and having a way to access these in django-environ is useful.

Here's a quote from the official Postgres docker hub image:

As an alternative to passing sensitive information via environment variables, _FILE may be appended to some of the previously listed environment variables, causing the initialization script to load the values for those variables from files present in the container. In particular, this can be used to load passwords from Docker secrets stored in /run/secrets/<secret_name> files. For example:

@sergeyklay sergeyklay self-requested a review September 13, 2021 11:22
@sergeyklay sergeyklay self-assigned this Sep 13, 2021
@sergeyklay sergeyklay added the enhancement New feature or request label Sep 13, 2021
@sergeyklay
Copy link
Collaborator

I'll take a look ASAP

@coveralls
Copy link

coveralls commented Sep 13, 2021

Coverage Status

Coverage increased (+1.0%) to 90.909% when pulling 9f614e2 on SmileyChris:file-aware into caf1ce2 on joke2k:main.

@SmileyChris
Copy link
Contributor Author

Oh, we're supporting 3.5 still. Easy changes to make for that.
Windows CI fails because it can't read temp files 😒

@SmileyChris
Copy link
Contributor Author

One design decision that should be considered: currently if you use a _FILE key on a missing file (or not enough permissions), then it will fail hard with an os exception -- the thinking being that failing loud is better than silently if you explicitly asked to use a file.

@sergeyklay
Copy link
Collaborator

@SmileyChris Could you please update the PR descrption by adding a bit more info about purpose and possible use cases.

@SmileyChris
Copy link
Contributor Author

@sergeyklay updated description, pushed py3.5 fix, 100% coverage, and hopefully fixed windows ci tests too

@SmileyChris
Copy link
Contributor Author

Lint fixed (pfft, should use black's 88 line length now :D)

Copy link
Collaborator

@sergeyklay sergeyklay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, everything looks quite good, except for a few nitpicking from me.

.python-version Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
environ/environ.py Show resolved Hide resolved
@sergeyklay sergeyklay self-requested a review September 15, 2021 07:38
@sergeyklay sergeyklay merged commit a8871d0 into joke2k:main Sep 15, 2021
@sergeyklay
Copy link
Collaborator

Merged! Thank you for the patch, and for helping us make django-environ better.

@SmileyChris SmileyChris deleted the file-aware branch September 17, 2021 03:50
@panteparak
Copy link

is there a timeline for the next release that includes this PR?

@sergeyklay
Copy link
Collaborator

I'll try to find time for release this weekend.

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

Successfully merging this pull request may close these issues.

4 participants