pydantic-settings icon indicating copy to clipboard operation
pydantic-settings copied to clipboard

BaseSettings env_prefix also apply for secrets

Open JerPk opened this issue 4 years ago • 10 comments

Checks

  • [x] I added a descriptive title to this issue
  • [x] I have searched (google, github) for similar issues and couldn't find anything
  • [x] I have read and followed the docs and still think this is a bug

Bug

The env_prefix is used for both env vars and secrets, which makes it impossible to combine env vars (with prefix) and secrets (without prefix) into a single settings model.

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.7.2
            pydantic compiled: False
                 install path: /usr/local/src/pydantic/pydantic
               python version: 3.7.7 (default, Mar 11 2020, 00:27:03)  [GCC 8.3.0]
                     platform: Linux-5.4.0-54-generic-x86_64-with-debian-10.3
     optional deps. installed: []

Environment vars:

printenv
RESTAPI_USE_PRODUCTION_DB=1
RESTAPI_DB_READ_ONLY=0

Secrets in container:

ls /run/secrets/
db_read_only_password  db_read_only_user  db_read_write_password  db_read_write_user
from pydantic import BaseSettings

class DBSettings(BaseSettings):
    use_production_db: bool = False     # env var
    db_read_only: bool = False          # env var
    db_read_only_user: str = None       # secret
    db_read_only_password: str = None   # secret
    db_read_write_user: str = None      # secret
    db_read_write_password: str = None  # secret

    class Config:
        env_prefix = 'RESTAPI_'
        secrets_dir = '/run/secrets'

db_sett = DBSettings()

Tested without the env_prefix = 'RESTAPI_' line, which filled the settings from the secrets correctly, env vars were not read as expected.

Possible solutions

  • Make it clear in the documentation that the env_prefix is also used for secrets
  • Just do not use the env_prefix for the secrets
  • Make an secrets_prefix to be used as secrets prefix

JerPk avatar Dec 09 '20 12:12 JerPk

Hi @JerPk and thanks a lot for reporting! IMO you're right it's a bug. The behaviour with both secrets and env_prefix was not tested so I guess that's why we never saw it. I chose the 2nd solution on the 3 you suggested because for me the 1st doesn't make sense (it shouldn't use env_prefix when we talk about secrets) and the 3rd doesn't really make sense when you already have folders! Feedback more than welcome :) Cheers

PrettyWood avatar Dec 09 '20 21:12 PrettyWood

Hi @PrettyWood, I didn't expect such a fast response and implementation. I completely agree with the solution. (although I will need to change some things in my code when I am updating to this future version.) Thanks!

JerPk avatar Dec 11 '20 15:12 JerPk

Similar issue exists, if "env" is set for a field.

class Settings(BaseSettings):
    auth_key: str
    api_key: str = Field(..., env="my_api_key")

    redis_dsn: RedisDsn = "redis://user:pass@localhost:6379/1"
    pg_dsn: PostgresDsn = "postgres://postgres:password@localhost:5432/postgres"

    # to override domains:
    # export my_prefix_domains='["foo.com", "bar.com"]'
    domains: Set[str] = set()

    # to override more_settings:
    # export my_prefix_more_settings='{"foo": "x", "apple": 1}'
    more_settings: SubModel = SubModel()

    class Config:
        # env_prefix = "my_prefix_"  # defaults to no prefix, i.e. ""
        fields = {
            "auth_key": {"env": "my_auth_key",},
            "redis_dsn": {"env": ["service_redis_dsn", "redis_url"]},
        }
        secrets_dir = "/var/run"

My secret had to be renamed to my_auth_key to succeed.

vlcinsky avatar Jan 15 '21 17:01 vlcinsky

Proposal

Keep the solution as is now - thus use the environment variable name in all situations - not only for trying to read values from env var (and from dotenv file), but also from secrets.

Action needed: update the docs to clarify this rule.

Reasoning

Keep things simple. With unique naming rule for named configuration sources it is much simpler to document the applicaiton - simply give single set of names and add, that they can be provided via env vars, dotenv file or via secrets in a directory.

Another advantage: all existing deployment will work.

vlcinsky avatar Jan 15 '21 17:01 vlcinsky

To quote @JerPk we have two solutions

  1. Make it clear in the documentation that env_prefix is also used for secrets
  2. Do not use the env_prefix for the secrets

I thought option 2 made more sense but reading @vlcinsky opinion, I understand option 1 can be desired. I will also add option 3 just to be sure

  1. Try without env_prefix and then with it (or the other way)

This would hence break nothing and allow both behaviours but could have side effects and is definitely less clear...

I add "feedback wanted" label and hope others will share their point of view :)

PrettyWood avatar Jan 17 '21 22:01 PrettyWood

Thanks @PrettyWood for reaction.

Here is my feedback:

Following the KISS I would invite not to add any extra option (such as "for secrets use env_prefix yes/no") unless it is really needed.

$ python -c "import this"|grep -E "Simple|Special cases|There should be one|hard to explain"
Simple is better than complex.
Special cases aren't special enough to break the rules.
There should be one-- and preferably only one --obvious way to do it.
If the implementation is hard to explain, it's a bad idea.

vlcinsky avatar Jan 17 '21 22:01 vlcinsky

But, if pydantic/pydantic#2190 get merged, we won't be able to transparently prefix secrets? My stack has many services, and I would like to store secrets as /run/secrets/my_project_api_key, but I really don't want to add my_project_ to all field names. Maybe we should have a secrets_prefix in Config??

cauebs avatar Jun 10 '21 17:06 cauebs

Proposal

Keep the solution as is now - thus use the environment variable name in all situations - not only for trying to read values from env var (and from dotenv file), but also from secrets.

Action needed: update the docs to clarify this rule.

Agree. But if we go this way, then aliases passed via Field(env=...) must honor env_prefix too. Because now they don't. After that we will get a simple consistent rule: "env_prefix affects every setting source by default. Don't use it or create your own settings source if you want different behavior."

matpuk avatar Dec 15 '22 12:12 matpuk

The workaround to ignore the env_prefix for secrets is adding this method to your settings class:

@classmethod
def settings_customise_sources(
    cls,
    settings_cls: Type[BaseSettings],
    init_settings: PydanticBaseSettingsSource,
    env_settings: PydanticBaseSettingsSource,
    dotenv_settings: PydanticBaseSettingsSource,
    file_secret_settings: PydanticBaseSettingsSource,
) -> Tuple[PydanticBaseSettingsSource, ...]:
    return init_settings, env_settings, dotenv_settings, SecretsSettingsSource(settings_cls, env_prefix=None)

smorokin avatar Apr 03 '24 07:04 smorokin

Released a small package that solves this problem (available on PyPi): https://github.com/makukha/pydantic-file-secrets

Some features:

  • Use secret file source in nested settings models
  • Drop-in replacement of standard SecretsSettingsSource
  • Plain or nested directory layout: /run/secrets/dir__key or /run/secrets/dir/key
  • Respects env_prefix, env_nested_delimiter and other config options
  • Has secrets_prefix, secrets_nested_delimiter, etc. to configure secrets and env vars separately
  • Pure Python thin wrapper over standard EnvSettingsSource
  • No third party dependencies except pydantic-settings
  • 100% test coverage

makukha avatar Aug 16 '24 11:08 makukha