Skip to content

Commit

Permalink
Avoid saving secrets in SecretContext (#4665) (#4672)
Browse files Browse the repository at this point in the history
  • Loading branch information
Nathaniel May authored Feb 3, 2022
1 parent e2a6c25 commit 3be057b
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- Fix bug in retry logic for bad response from hub and when there is a bad git tarball download. ([#4577](https:/dbt-labs/dbt-core/issues/4577), [#4579](https:/dbt-labs/dbt-core/issues/4579), [#4609](https:/dbt-labs/dbt-core/pull/4609))
- Restore previous log level (DEBUG) when a test depends on a disabled resource. Still WARN if the resource is missing ([#4594](https:/dbt-labs/dbt-core/issues/4594), [#4647](https:/dbt-labs/dbt-core/pull/4647))
- User wasn't asked for permission to overwite a profile entry when running init inside an existing project ([#4375](https:/dbt-labs/dbt-core/issues/4375), [#4447](https:/dbt-labs/dbt-core/pull/4447))
- A change in secret environment variables won't trigger a full reparse [#4650](https:/dbt-labs/dbt-core/issues/4650) [4665](https:/dbt-labs/dbt-core/pull/4665)

Contributors:
- [@NiallRees](https:/NiallRees) ([#4447](https:/dbt-labs/dbt-core/pull/4447))
Expand Down
7 changes: 6 additions & 1 deletion core/dbt/context/secret.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from .base import BaseContext, contextmember

from dbt.exceptions import raise_parsing_error
from dbt.logger import SECRET_ENV_PREFIX


class SecretContext(BaseContext):
Expand All @@ -27,7 +28,11 @@ def env_var(self, var: str, default: Optional[str] = None) -> str:
return_value = default

if return_value is not None:
self.env_vars[var] = return_value
# do not save secret environment variables
if not var.startswith(SECRET_ENV_PREFIX):
self.env_vars[var] = return_value

# return the value even if its a secret
return return_value
else:
msg = f"Env var required but not provided: '{var}'"
Expand Down
59 changes: 59 additions & 0 deletions test/integration/068_partial_parsing_tests/test_pp_vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from dbt.contracts.graph.manifest import Manifest
from dbt.contracts.files import ParseFileType
from dbt.contracts.results import TestStatus
from dbt.logger import SECRET_ENV_PREFIX
from dbt.parser.partial import special_override_macros
from test.integration.base import DBTIntegrationTest, use_profile, normalize, get_manifest
import shutil
Expand Down Expand Up @@ -300,6 +301,7 @@ def test_postgres_project_env_vars(self):
# cleanup
del os.environ['ENV_VAR_NAME']


class ProfileEnvVarTest(BasePPTest):

@property
Expand Down Expand Up @@ -352,3 +354,60 @@ def test_postgres_profile_env_vars(self):
manifest = get_manifest()
self.assertNotEqual(env_vars_checksum, manifest.state_check.profile_env_vars_hash.checksum)


class ProfileSecretEnvVarTest(BasePPTest):

@property
def profile_config(self):
# Need to set these here because the base integration test class
# calls 'load_config' before the tests are run.
# Note: only the specified profile is rendered, so there's no
# point it setting env_vars in non-used profiles.
os.environ['ENV_VAR_USER'] = 'root'
os.environ[SECRET_ENV_PREFIX + 'PASS'] = 'password'
return {
'config': {
'send_anonymous_usage_stats': False
},
'test': {
'outputs': {
'dev': {
'type': 'postgres',
'threads': 1,
'host': self.database_host,
'port': 5432,
'user': "root",
'pass': "password",
'user': "{{ env_var('ENV_VAR_USER') }}",
'pass': "{{ env_var('DBT_ENV_SECRET_PASS') }}",
'dbname': 'dbt',
'schema': self.unique_schema()
},
},
'target': 'dev'
}
}

@use_profile('postgres')
def test_postgres_profile_secret_env_vars(self):

# Initial run
os.environ['ENV_VAR_USER'] = 'root'
os.environ[SECRET_ENV_PREFIX + 'PASS'] = 'password'
self.setup_directories()
self.copy_file('test-files/model_one.sql', 'models/model_one.sql')
results = self.run_dbt(["run"])
manifest = get_manifest()
env_vars_checksum = manifest.state_check.profile_env_vars_hash.checksum

# Change a secret var, it shouldn't register because we shouldn't save secrets.
os.environ[SECRET_ENV_PREFIX + 'PASS'] = 'password2'
# this dbt run is going to fail because the password isn't actually the right one,
# but that doesn't matter because we just want to see if the manifest has included
# the secret in the hash of environment variables.
(results, log_output) = self.run_dbt_and_capture(["run"], expect_pass=False)
# I020 is the event code for "env vars used in profiles.yml have changed"
self.assertFalse('I020' in log_output)
manifest = get_manifest()
self.assertEqual(env_vars_checksum, manifest.state_check.profile_env_vars_hash.checksum)

0 comments on commit 3be057b

Please sign in to comment.