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

password skipping config substitution #2005

Closed
EliMorato opened this issue Oct 21, 2020 · 7 comments · Fixed by #2015
Closed

password skipping config substitution #2005

EliMorato opened this issue Oct 21, 2020 · 7 comments · Fixed by #2015
Assignees

Comments

@EliMorato
Copy link

Describe the bug
When using environment variables to provide datasource credentials in great_expectations.yml, example:

    credentials:
      drivername: postgresql+psycopg2
      host: ${GE_REDSHIFT_HOST}
      port: ${GE_REDSHIFT_PORT}
      username: ${GE_REDSHIFT_USER}
      password: ${GE_REDSHIFT_PASSWORD}
      database: ${GE_REDSHIFT_DB}

the password is reading "${GE_REDSHIFT_PASSWORD}" as the password value due to this bugfix.

To Reproduce
Steps to reproduce the behavior:

  1. Use an environment variable in your datasource credentials' password
  2. Run anything that needs to connect to the datasource. For example, you can create a new suite or edit an old one and run the first Jupyter Notebook cell. You'll get an authentication error like this:

Captura de pantalla 2020-10-21 a las 18 11 02

Captura de pantalla 2020-10-21 a las 18 11 55

Expected behavior
Not to crash and also read the value of the environment variable (${GE_REDSHIFT_PASSWORD} in this case) as the password value for the datasource credentials.

Environment (please complete the following information):

  • OS: All
  • GE Version: 0.12.6

Additional context
@anthonyburdi

@eugmandel eugmandel added the triage Used by the GE core team to flag issues that were not yet triaged label Oct 22, 2020
@whatsrupp
Copy link

+1 on this, am currently working around this by generating the yaml file and interpolating in the password from the environment manually.

@martinheld
Copy link

+1

@eugmandel eugmandel added core-team-priority and removed triage Used by the GE core team to flag issues that were not yet triaged labels Oct 26, 2020
@eugmandel
Copy link
Contributor

@EliPino @whatsrupp @martinheld This is indeed a case of a fix breaking something else. We will fix this in a more robust way and cut a release.

@whatsrupp
Copy link

@eugmandel Perfect!! While I'm here, just a bit of positive feedback: I have recently set up GE on a small data pipeline at work, it was invigorating how quick it was to scaffold out some meaningful tests that I needed to put together! So, thank you!

@anthonyburdi
Copy link
Member

Thank you all for your patience! In this branch anthony/bugfix/issue-2005-password-substitution I have a preliminary fix in place (revert the bugfix #1949 & add escaping for $ characters in actual passwords), and am currently writing / cleaning up tests and documentation.

@martinheld
Copy link

🥇 @anthonyburdi thanks a lot!

@anthonyburdi
Copy link
Member

Thanks again for your patience! GE v0.12.7 now:

  • does not handle the password field differently
  • allows for the escaping of $ characters in config values
  • allows for multiple substitutions in a single config value

I hope this fixes your issues, please let us know if it does not!

@anthonyburdi anthonyburdi self-assigned this Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants