Repository: samuelcolvin/pydantic
Pull request #1820 (2020-08-12 01:14:58)

Add ability to read secrets from files
<!-- Thank you for your contribution! -->
<!-- Unless your change is trivial, please create an issue to discuss the change before creating a PR -->
<!-- See https://pydantic-docs.helpmanual.io/contributing/ for help on Contributing -->

## Change Summary

Added ability for `BaseSettings` to load `SecretStr` and `SecretBytes` from files. I opted to drop the ability to load complex types from secrets as there is no existing secret type for complex objects.

<!-- Please give a short summary of the changes. -->

## Related issue number

#1279

<!-- Are there any issues opened that will be resolved by merging this change? -->

## Checklist

* [x] Unit tests for the changes exist
* [x] Tests pass on CI and coverage remains at 100%
* [x] Documentation reflects the changes where applicable
* [x] `changes/<pull request or issue id>-<github username>.md` file added describing change
  (see [changes/README.md](https://github.com/samuelcolvin/pydantic/blob/master/changes/README.md) for details)


codecov[bot]: # [Codecov](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820?src=pr&el=h1) Report
> Merging [#1820](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820?src=pr&el=desc) into [master](https://codecov.io/gh/samuelcolvin/pydantic/commit/44616e35cb0ddd97af07b95d7209176a29bfea49&el=desc) will **decrease** coverage by `0.35%`.
> The diff coverage is `92.85%`.


```diff
@@             Coverage Diff             @@
##            master    #1820      +/-   ##
===========================================
- Coverage   100.00%   99.64%   -0.36%     
===========================================
  Files           21       21              
  Lines         3895     3922      +27     
  Branches       783      792       +9     
===========================================
+ Hits          3895     3908      +13     
- Misses           0       10      +10     
- Partials         0        4       +4     
```


| [Impacted Files](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pydantic/env\_settings.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820/diff?src=pr&el=tree#diff-cHlkYW50aWMvZW52X3NldHRpbmdzLnB5) | `96.15% <92.85%> (-3.85%)` | :arrow_down: |
| [pydantic/version.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820/diff?src=pr&el=tree#diff-cHlkYW50aWMvdmVyc2lvbi5weQ==) | `88.23% <0.00%> (-11.77%)` | :arrow_down: |
| [pydantic/typing.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820/diff?src=pr&el=tree#diff-cHlkYW50aWMvdHlwaW5nLnB5) | `98.11% <0.00%> (-1.89%)` | :arrow_down: |
| [pydantic/utils.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820/diff?src=pr&el=tree#diff-cHlkYW50aWMvdXRpbHMucHk=) | `99.21% <0.00%> (-0.79%)` | :arrow_down: |
| [pydantic/networks.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820/diff?src=pr&el=tree#diff-cHlkYW50aWMvbmV0d29ya3MucHk=) | `99.22% <0.00%> (-0.78%)` | :arrow_down: |
| [pydantic/types.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820/diff?src=pr&el=tree#diff-cHlkYW50aWMvdHlwZXMucHk=) | `99.57% <0.00%> (-0.43%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820?src=pr&el=footer). Last update [44616e3...4d5192a](https://codecov.io/gh/samuelcolvin/pydantic/pull/1820?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

samuelcolvin: I've made a few tweaks to the docs, this looks great and I'm ready to merge it except one thing.

I think these values from secrets files should apply to all fields, not just `SecretStr` and `SecretBytes`. This is because:
* it would make it more like other variable substitutions from arguments, environment variables and dot-env files, and thus less confusing
* I often want field types to be something like `HttpUrl` or even `int`, restricting to `SecretStr` and `SecretBytes` would prevent that

Sorry, I should have thought about this before, but it didn't occur to me until now.

@mdgilene and @FichteFoll what do you think? Unless you have strong objections, I can either wait for you to make the change, or make it myself.
FichteFoll: > I think these values from secrets files should apply to all fields, not just `SecretStr` and `SecretBytes`.

I definitely agree with your points there, as they reflect my own concerns regarding gating this behind special classes. While the explicit declaration of loading from secrets makes it easier to see what values are different at a glance, it also limits the functionality compared to loading values from environment variable, where nothing needs to be configured.
mdgilene: > I've made a few tweaks to the docs, this looks great and I'm ready to merge it except one thing.
> 
> I think these values from secrets files should apply to all fields, not just `SecretStr` and `SecretBytes`. This is because:
> 
> * it would make it more like other variable substitutions from arguments, environment variables and dot-env files, and thus less confusing
> * I often want field types to be something like `HttpUrl` or even `int`, restricting to `SecretStr` and `SecretBytes` would prevent that
> 
> Sorry, I should have thought about this before, but it didn't occur to me until now.
> 
> @mdgilene and @FichteFoll what do you think? Unless you have strong objections, I can either wait for you to make the change, or make it myself.

I don't have a strong preference one way or the other. So opening it up to all types works for me. I don't actually have much time at the moment, so if you are able to make the change that would be awesome. Otherwise it might take a few days till I get around to it.

samuelcolvin: okay, thanks. I'll try and do it.

Patch:
diff --git a/changes/1820-mdgilene.md b/changes/1820-mdgilene.md
new file mode 100644
index 000000000..068ef7e51
--- /dev/null
+++ b/changes/1820-mdgilene.md
@@ -0,0 +1,1 @@
+Added ability for `BaseSettings` to load `SecretStr` and `SecretBytes` from files.
diff --git a/docs/usage/settings.md b/docs/usage/settings.md
index 1068d42d0..a06ea52d4 100644
--- a/docs/usage/settings.md
+++ b/docs/usage/settings.md
@@ -128,6 +128,76 @@ Because python-dotenv is used to parse the file, bash-like semantics such as `ex
 (depending on your OS and environment) may allow your dotenv file to also be used with `source`,
 see [python-dotenv's documentation](https://saurabh-kumar.com/python-dotenv/#usages) for more details.
 
+## Secret Support
+
+Placing secret values in files is a common pattern to provide sensitive configuration to an application.
+
+A secret file follows the same principal as a dotenv file except it only contains a single value and the file name 
+is used as the key. A secret file will look like the following:
+
+`/var/run/db_password`:
+```
+super_secret_database_password
+```
+
+Once you have your secret files, *pydantic* supports loading it in two ways:
+
+**1.** setting `secrets_dir` on `Config` in a `BaseSettings` class to the directory where your secret files are stored:
+
+```py
+class Settings(BaseSettings):
+    ...
+    database_password: str
+
+    class Config:
+        secrets_dir = '/var/run'
+```
+
+**2.** instantiating a `BaseSettings` derived class with the `_secrets_dir` keyword argument:
+
+```py
+settings = Settings(_secrets_dir='/var/run')
+```
+
+In either case, the value of the passed argument can be any valid directory, either absolute or relative to the
+current working directory. From there, *pydantic* will handle everything for you by loading in your variables and
+validating them.
+
+Even when using a secrets directory, *pydantic* will still read environment variables from a dotenv file or the environment,
+**a dotenv file and environment variables will always take priority over values loaded from the secrets directory**.
+
+Passing a file path via the `_secrets_dir` keyword argument on instantiation (method 2) will override
+the value (if any) set on the `Config` class.
+
+### Use Case: Docker Secrets
+
+Docker Secrets can be used to provide sensitive configuration to an application running in a Docker container.
+To use these secrets in a *pydantic* application the process is simple. More information regarding creating, managing
+and using secrets in Docker see the official
+[Docker documentation](https://docs.docker.com/engine/reference/commandline/secret/).
+
+First, define your Settings
+```py
+class Settings(BaseSettings):
+    my_secret_data: str
+
+    class Config:
+        secrets_dir = '/run/secrets'
+```
+!!! note
+    By default Docker uses `/run/secrets` as the target mount point. If you want to use a different location, change 
+    `Config.secrets_dir` accordingly.
+
+Then, Create your secret via the Docker CLI
+```bash
+printf "This is a secret" | docker secret create my_secret_data -
+```
+
+Last, run your application inside a Docker container and supply your newly created secret
+```bash
+docker service create --name pydantic-with-secrets --secret my_secret_data pydantic-app:latest
+```
+
 ## Field value priority
 
 In the case where a value is specified for the same `Settings` field in multiple ways,
@@ -136,4 +206,5 @@ the selected value is determined as follows (in descending order of priority):
 1. Arguments passed to the `Settings` class initialiser.
 2. Environment variables, e.g. `my_prefix_special_function` as described above.
 3. Variables loaded from a dotenv (`.env`) file.
-4. The default field values for the `Settings` model.
+4. Variables loaded from the secrets directory.
+5. The default field values for the `Settings` model.
diff --git a/pydantic/env_settings.py b/pydantic/env_settings.py
index 6846e1020..28022d19c 100644
--- a/pydantic/env_settings.py
+++ b/pydantic/env_settings.py
@@ -6,7 +6,7 @@ from typing import AbstractSet, Any, Dict, List, Mapping, Optional, Union
 from .fields import ModelField
 from .main import BaseConfig, BaseModel, Extra
 from .typing import display_as_type
-from .utils import deep_update, sequence_like
+from .utils import deep_update, path_type, sequence_like
 
 env_file_sentinel = str(object())
 
@@ -27,11 +27,14 @@ class BaseSettings(BaseModel):
         __pydantic_self__,
         _env_file: Union[Path, str, None] = env_file_sentinel,
         _env_file_encoding: Optional[str] = None,
+        _secrets_dir: Union[Path, str, None] = None,
         **values: Any,
     ) -> None:
         # Uses something other than `self` the first arg to allow "self" as a settable attribute
         super().__init__(
-            **__pydantic_self__._build_values(values, _env_file=_env_file, _env_file_encoding=_env_file_encoding)
+            **__pydantic_self__._build_values(
+                values, _env_file=_env_file, _env_file_encoding=_env_file_encoding, _secrets_dir=_secrets_dir
+            )
         )
 
     def _build_values(
@@ -39,8 +42,41 @@ class BaseSettings(BaseModel):
         init_kwargs: Dict[str, Any],
         _env_file: Union[Path, str, None] = None,
         _env_file_encoding: Optional[str] = None,
+        _secrets_dir: Union[Path, str, None] = None,
     ) -> Dict[str, Any]:
-        return deep_update(self._build_environ(_env_file, _env_file_encoding), init_kwargs)
+        return deep_update(
+            self._build_secrets_files(_secrets_dir), self._build_environ(_env_file, _env_file_encoding), init_kwargs
+        )
+
+    def _build_secrets_files(self, _secrets_dir: Union[Path, str, None] = None) -> Dict[str, Optional[str]]:
+        """
+        Build fields from "secrets" files.
+        """
+        secrets: Dict[str, Optional[str]] = {}
+
+        secrets_dir = _secrets_dir or self.__config__.secrets_dir
+        if secrets_dir is None:
+            return secrets
+
+        secrets_path = Path(secrets_dir).expanduser()
+
+        if not secrets_path.exists():
+            raise SettingsError(f'directory "{secrets_path}" does not exist')
+        if not secrets_path.is_dir():
+            raise SettingsError(f'secrets_dir must reference a directory, not a {path_type(secrets_path)}')
+
+        for field in self.__fields__.values():
+            for env_name in field.field_info.extra['env_names']:
+                path = secrets_path / env_name
+                if path.is_file():
+                    secrets[field.alias] = path.read_text().strip()
+                elif path.exists():
+                    warnings.warn(
+                        f'attempted to load secret file "{path}" but found a {path_type(path)} instead.',
+                        stacklevel=4,
+                    )
+
+        return secrets
 
     def _build_environ(
         self, _env_file: Union[Path, str, None] = None, _env_file_encoding: Optional[str] = None
@@ -89,6 +125,7 @@ class BaseSettings(BaseModel):
         env_prefix = ''
         env_file = None
         env_file_encoding = None
+        secrets_dir = None
         validate_all = True
         extra = Extra.forbid
         arbitrary_types_allowed = True
diff --git a/pydantic/utils.py b/pydantic/utils.py
index c9b0f3c92..8019a1924 100644
--- a/pydantic/utils.py
+++ b/pydantic/utils.py
@@ -28,6 +28,7 @@ from .version import version_info
 
 if TYPE_CHECKING:
     from inspect import Signature
+    from pathlib import Path
 
     from .dataclasses import DataclassType  # noqa: F401
     from .fields import ModelField  # noqa: F401
@@ -53,6 +54,7 @@ __all__ = (
     'ValueItems',
     'version_info',  # required here to match behaviour in v1.3
     'ClassAttribute',
+    'path_type',
     'ROOT_KEY',
 )
 
@@ -164,13 +166,14 @@ def in_ipython() -> bool:
 KeyType = TypeVar('KeyType')
 
 
-def deep_update(mapping: Dict[KeyType, Any], updating_mapping: Dict[KeyType, Any]) -> Dict[KeyType, Any]:
+def deep_update(mapping: Dict[KeyType, Any], *updating_mappings: Dict[KeyType, Any]) -> Dict[KeyType, Any]:
     updated_mapping = mapping.copy()
-    for k, v in updating_mapping.items():
-        if k in mapping and isinstance(mapping[k], dict) and isinstance(v, dict):
-            updated_mapping[k] = deep_update(mapping[k], v)
-        else:
-            updated_mapping[k] = v
+    for updating_mapping in updating_mappings:
+        for k, v in updating_mapping.items():
+            if k in updated_mapping and isinstance(updated_mapping[k], dict) and isinstance(v, dict):
+                updated_mapping[k] = deep_update(updated_mapping[k], v)
+            else:
+                updated_mapping[k] = v
     return updated_mapping
 
 
@@ -578,6 +581,30 @@ class ClassAttribute:
         raise AttributeError(f'{self.name!r} attribute of {owner.__name__!r} is class-only')
 
 
+path_types = {
+    'is_dir': 'directory',
+    'is_file': 'file',
+    'is_mount': 'mount point',
+    'is_symlink': 'symlink',
+    'is_block_device': 'block device',
+    'is_char_device': 'char device',
+    'is_fifo': 'FIFO',
+    'is_socket': 'socket',
+}
+
+
+def path_type(p: 'Path') -> str:
+    """
+    Find out what sort of thing a path is.
+    """
+    assert p.exists(), 'path does not exist'
+    for method, name in path_types.items():
+        if getattr(p, method)():
+            return name
+
+    return 'unknown'
+
+
 Obj = TypeVar('Obj')
 
 


Test patch:
diff --git a/tests/test_settings.py b/tests/test_settings.py
index 6e734481c..b954b4279 100644
--- a/tests/test_settings.py
+++ b/tests/test_settings.py
@@ -1,11 +1,12 @@
 import os
+import sys
 import uuid
 from pathlib import Path
 from typing import Dict, List, Optional, Set
 
 import pytest
 
-from pydantic import BaseModel, BaseSettings, Field, NoneStr, ValidationError, dataclasses
+from pydantic import BaseModel, BaseSettings, Field, HttpUrl, NoneStr, SecretStr, ValidationError, dataclasses
 from pydantic.env_settings import SettingsError, read_env_file
 
 try:
@@ -423,7 +424,7 @@ def test_env_takes_precedence(env):
         foo: int
         bar: str
 
-        def _build_values(self, init_kwargs, _env_file, _env_file_encoding):
+        def _build_values(self, init_kwargs, _env_file, _env_file_encoding, _secrets_dir):
             return {**init_kwargs, **self._build_environ()}
 
     env.set('BAR', 'env setting')
@@ -443,7 +444,7 @@ def test_config_file_settings_nornir(env):
         param_b: str
         param_c: str
 
-        def _build_values(self, init_kwargs, _env_file, _env_file_encoding):
+        def _build_values(self, init_kwargs, _env_file, _env_file_encoding, _secrets_dir):
             config_settings = init_kwargs.pop('__config_settings__')
             return {**config_settings, **init_kwargs, **self._build_environ()}
 
@@ -754,3 +755,100 @@ def test_frozenset(env):
     assert Settings().dict() == {'foo': 'default foo'}
     env.set('foo_a', 'x')
     assert Settings().dict() == {'foo': 'x'}
+
+
+def test_secrets_path(tmp_path):
+    p = tmp_path / 'foo'
+    p.write_text('foo_secret_value_str')
+
+    class Settings(BaseSettings):
+        foo: str
+
+        class Config:
+            secrets_dir = tmp_path
+
+    assert Settings().dict() == {'foo': 'foo_secret_value_str'}
+
+
+def test_secrets_path_url(tmp_path):
+    (tmp_path / 'foo').write_text('http://www.example.com')
+    (tmp_path / 'bar').write_text('snap')
+
+    class Settings(BaseSettings):
+        foo: HttpUrl
+        bar: SecretStr
+
+        class Config:
+            secrets_dir = tmp_path
+
+    assert Settings().dict() == {'foo': 'http://www.example.com', 'bar': SecretStr('snap')}
+
+
+def test_secrets_missing(tmp_path):
+    class Settings(BaseSettings):
+        foo: str
+
+        class Config:
+            secrets_dir = tmp_path
+
+    with pytest.raises(ValidationError) as exc_info:
+        Settings()
+    assert exc_info.value.errors() == [{'loc': ('foo',), 'msg': 'field required', 'type': 'value_error.missing'}]
+
+
+def test_secrets_invalid_secrets_dir(tmp_path):
+    p1 = tmp_path / 'foo'
+    p1.write_text('foo_secret_value_str')
+
+    class Settings(BaseSettings):
+        foo: str
+
+        class Config:
+            secrets_dir = p1
+
+    with pytest.raises(SettingsError, match='secrets_dir must reference a directory, not a file'):
+        Settings()
+
+
+@pytest.mark.skipif(sys.platform.startswith('win'), reason='windows paths break regex')
+def test_secrets_missing_location(tmp_path):
+    class Settings(BaseSettings):
+        foo: str
+
+        class Config:
+            secrets_dir = tmp_path / 'does_not_exist'
+
+    with pytest.raises(SettingsError, match=f'directory "{tmp_path}/does_not_exist" does not exist'):
+        Settings()
+
+
+@pytest.mark.skipif(sys.platform.startswith('win'), reason='windows paths break regex')
+def test_secrets_file_is_a_directory(tmp_path):
+    p1 = tmp_path / 'foo'
+    p1.mkdir()
+
+    class Settings(BaseSettings):
+        foo: Optional[str]
+
+        class Config:
+            secrets_dir = tmp_path
+
+    with pytest.warns(UserWarning, match=f'attempted to load secret file "{tmp_path}/foo" but found a directory inste'):
+        Settings()
+
+
+@pytest.mark.skipif(not dotenv, reason='python-dotenv not installed')
+def test_secrets_dotenv_precedence(tmp_path):
+    s = tmp_path / 'foo'
+    s.write_text('foo_secret_value_str')
+
+    e = tmp_path / '.env'
+    e.write_text('foo=foo_env_value_str')
+
+    class Settings(BaseSettings):
+        foo: str
+
+        class Config:
+            secrets_dir = tmp_path
+
+    assert Settings(_env_file=e).dict() == {'foo': 'foo_env_value_str'}
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 989176c20..e3db6beb1 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -21,6 +21,7 @@ from pydantic.utils import (
     get_model,
     import_string,
     lenient_issubclass,
+    path_type,
     smart_deepcopy,
     truncate,
     unique_list,
@@ -357,6 +358,32 @@ def test_all_literal_values():
     assert sorted(all_literal_values(L312)) == sorted(('1', '2', '3'))
 
 
+def test_path_type(tmp_path):
+    assert path_type(tmp_path) == 'directory'
+    file = tmp_path / 'foobar.txt'
+    file.write_text('hello')
+    assert path_type(file) == 'file'
+
+
+def test_path_type_unknown(tmp_path):
+    p = type(
+        'FakePath',
+        (),
+        {
+            'exists': lambda: True,
+            'is_dir': lambda: False,
+            'is_file': lambda: False,
+            'is_mount': lambda: False,
+            'is_symlink': lambda: False,
+            'is_block_device': lambda: False,
+            'is_char_device': lambda: False,
+            'is_fifo': lambda: False,
+            'is_socket': lambda: False,
+        },
+    )
+    assert path_type(p) == 'unknown'
+
+
 @pytest.mark.parametrize(
     'obj',
     (1, 1.0, '1', b'1', int, None, test_all_literal_values, len, test_all_literal_values.__code__, lambda: ..., ...),