Skip to content

Load vault secrets from environment less stores or which are not written by dynaconf#725

Merged
rochacbruno merged 1 commit intodynaconf:masterfrom
jyejare:nonenvironments_vault
Mar 30, 2022
Merged

Load vault secrets from environment less stores or which are not written by dynaconf#725
rochacbruno merged 1 commit intodynaconf:masterfrom
jyejare:nonenvironments_vault

Conversation

@jyejare
Copy link
Copy Markdown
Contributor

@jyejare jyejare commented Mar 9, 2022

Problem Statement:

This PR fixes #723 !

The issue is observed when the secrets are created in Vault store without using DynaConf way of creating it that is CLI way of dynaconf write or using dynaconf APIs.

The secrets were directly created to MOUNT_POINT/PATH using Hashicorp vault CLI / API and hence the dynaconf environments were not created there.

So while reading these non-dynaconf/non-environemnts based secrets from Vault using DynaConf, we were hitting to the wrong path, that is:

  • if we set environments of dynaconf to True (Dynaconf(environments=True)) then we hit the developement enviroments path that is : mount_point/path/developement.
  • if we set environments of dynaconf to False (Dynaconf()) then we hit the main environments path that is : mount_point/path/main.

Whereas in our case, we should hit to the mount_point/path path since we havent used DynaConfs environments based way of creating the secrets.

Solution:

The proposed solution allows users to access both "DynaConfs environments=False secrets" as well as "Vault CLI/API pushed secrets (non-environment based)" by looking into both mount_point/path/main and direct mount_point/path directories.

This PR also fixes some vault v2 APIs accessing and removes unnecessary looping over data key for v2 secrets.


# If provided environments will be loaded separatelly
ENVIRONMENTS_FOR_DYNACONF = get("ENVIRONMENTS_FOR_DYNACONF", False)
MAIN_ENV_FOR_DYNACONF = get("MAIN_ENVIRONMENTS_FOR_DYNACONF", "MAIN")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
MAIN_ENV_FOR_DYNACONF = get("MAIN_ENVIRONMENTS_FOR_DYNACONF", "MAIN")
MAIN_ENVIRONMENT_FOR_DYNACONF = get("MAIN_ENVIRONMENT_FOR_DYNACONF", "MAIN")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would like to stick to the standard of naming the specific environment for dynaconf. E.g DEFAULT_ENV_FOR_DYNACONF.

Similarly, I will rename MAIN_ENVIRONMENT_FOR_DYNACONF as MAIN_ENV_FOR_DYNACONF

@jyejare jyejare force-pushed the nonenvironments_vault branch from a769aef to 4ee5343 Compare March 9, 2022 18:05
Copy link
Copy Markdown
Contributor Author

@jyejare jyejare left a comment

Choose a reason for hiding this comment

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

Fixed Comments !


# If provided environments will be loaded separatelly
ENVIRONMENTS_FOR_DYNACONF = get("ENVIRONMENTS_FOR_DYNACONF", False)
MAIN_ENV_FOR_DYNACONF = get("MAIN_ENVIRONMENTS_FOR_DYNACONF", "MAIN")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would like to stick to the standard of naming the specific environment for dynaconf. E.g DEFAULT_ENV_FOR_DYNACONF.

Similarly, I will rename MAIN_ENVIRONMENT_FOR_DYNACONF as MAIN_ENV_FOR_DYNACONF

@jyejare
Copy link
Copy Markdown
Contributor Author

jyejare commented Mar 17, 2022

Ping @rochacbruno , Need to get this in as we are blocked! Are you good with the changes or would you want to add changes in Doc and Version Doc ?

@rochacbruno
Copy link
Copy Markdown
Member

@jyejare some of the ci tests are not passing I was not able to verify yet the reason

@jyejare
Copy link
Copy Markdown
Contributor Author

jyejare commented Mar 17, 2022

@rochacbruno I hope that's not related to this change, but I can take a look! I didnt observe it first as all the ticks looked green.

@jyejare jyejare force-pushed the nonenvironments_vault branch from 4ee5343 to 9a4be35 Compare March 30, 2022 18:34
@jyejare
Copy link
Copy Markdown
Contributor Author

jyejare commented Mar 30, 2022

@rochacbruno Fixed tests failing ! Please revisit .

@rochacbruno
Copy link
Copy Markdown
Member

Ci is broken, not related to your PR, I will test locally and if pass I will merge

@rochacbruno rochacbruno merged commit 2be236e into dynaconf:master Mar 30, 2022
@jyejare
Copy link
Copy Markdown
Contributor Author

jyejare commented Mar 31, 2022

@rochacbruno Sorry, in my last comment, I wanted to say "failing tests passed, please revisit", but said "ulta" :)

@jyejare
Copy link
Copy Markdown
Contributor Author

jyejare commented Apr 12, 2022

@rochacbruno Whats the plan on releasing a new version of dynaConf with this change? Is it sooner or can you release it soon for us ?

@rochacbruno
Copy link
Copy Markdown
Member

@jyejare I can cut a 3.1.8 from master branch this week, I will have some free time by friday to do the triage.

@jyejare
Copy link
Copy Markdown
Contributor Author

jyejare commented Apr 12, 2022

@rochacbruno That's good news for us !! Thanks :)

rochacbruno added a commit that referenced this pull request Apr 15, 2022
Shortlog of commits since last release:

    Anderson Sousa (1):
          Document the usage with python -m (#710)

    Andressa Cabistani (2):
          Add unique label when merging lists to fix issue #653 (#661)
          Add new validation to fix issue #585 (#667)

    Armin Berres (1):
          Fix typo in error message

    Bruno Rocha (7):
          Release version 3.1.7
          Found this bug that was duplicating the generated envlist (#663)
          Add support for Python 3.10 (#665)
          Attempt to fix #555 (#669)
          Create update_contributors.yml
          Fixing pre-coomit and docs CI
          Added `dynaconf get` command to cli (#730)

    Caneco (2):
          improvement: add brand new logo to the project (#686)
          improvement: update socialcard to match the python way (#687)

    EdwardCuiPeacock (2):
          Feature: add @Jinja and @Format casting (#704)
          Combo converter doc (#735)

    Eitan Mosenkis (1):
          Fix FlaskConfig.setdefault (#706)

    Enderson Menezes (Mr. Enderson) (2):
          Force PYTHONIOENCODING to utf-8 to fix #664 (#672)
          edit: move discussions to github tab (#682)

    Eugene Triguba (1):
          Fix custom prefix link in envvar documentation (#680)

    Gibran Herrera (1):
          Fix Issue 662 Lazy validation (#675)

    Jitendra Yejare (2):
          Load vault secrets from environment less stores or which are not written by dynaconf (#725)
          Use default value when settings is blank (#729)

    Pavel Alimpiev (1):
          Update docs link (#678)

    Ugo Benassayag (1):
          Added validate_only_current_env to validator (issue #734) (#736)

    Waylon Walker (1):
          Docs Fix Spelling (#696)

    dependabot[bot] (3):
          Bump django from 2.1.5 to 2.2.26 in /example/django_pytest_pure (#711)
          Bump mkdocs from 1.1.2 to 1.2.3 (#715)
          Bump django from 2.2.26 to 2.2.27 in /example/django_pytest_pure (#717)

    github-actions[bot] (2):
          [automated] Update Contributors File (#691)
          [automated] Update Contributors File (#732)

    lowercase00 (1):
          Makes Django/Flask kwargs case insensitive (#721)
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 this pull request may close these issues.

[bug] Get Vault Secret via dynaconf throws permission denied error

2 participants