Skip to content

Commit

Permalink
Merge pull request #124 from ClarkSource/config-override-fix
Browse files Browse the repository at this point in the history
fix(get_secrets): do not mutate default params with config_override
  • Loading branch information
an2nb2 authored Feb 9, 2023
2 parents 7bd27a6 + ea1fe2c commit 73edc78
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 8 deletions.
2 changes: 1 addition & 1 deletion k8t/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def hashf(value, method="sha256"):
return hash_method.hexdigest()


def get_secret(key: str, length: Optional[int] = None, config_override: Optional[dict] = {}) -> str:
def get_secret(key: str, length: Optional[int] = None, config_override: Optional[dict] = None) -> str:
provider_name = config.CONFIG.get("secrets", {}).get("provider")

if not provider_name:
Expand Down
7 changes: 4 additions & 3 deletions k8t/secret_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@
DEFAULT_SSM_REGION = "eu-central-1"


def ssm(key: str, length: Optional[int] = None, config_override: Optional[dict] = {}) -> str:
def ssm(key: str, length: Optional[int] = None, config_override: Optional[dict] = None) -> str:
# Merge the config given as an argument with default config.
secrets_config = config.CONFIG.get("secrets", {})
secrets_config.update(config_override)
secrets_config = config.CONFIG.get("secrets", {}).copy()
if config_override is not None:
secrets_config.update(config_override)

prefix = str(secrets_config.get("prefix", DEFAULT_SSM_PREFIX))
region = str(secrets_config.get("region", DEFAULT_SSM_REGION))
Expand Down
8 changes: 4 additions & 4 deletions tests/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,18 @@ def test_get_secret():
config.CONFIG = {"secrets": {"provider": "random"}}
with patch.object(secret_providers, "random") as mock:
get_secret("any")
mock.assert_called_with("any", None, {})
mock.assert_called_with("any", None, None)
get_secret("any", 99)
mock.assert_called_with("any", 99, {})
mock.assert_called_with("any", 99, None)
get_secret("any", 99, {"foo": "bar"})
mock.assert_called_with("any", 99, {"foo": "bar"})

config.CONFIG = {"secrets": {"provider": "ssm"}}
with patch.object(secret_providers, "ssm") as mock:
get_secret("any")
mock.assert_called_with("any", None, {})
mock.assert_called_with("any", None, None)
get_secret("any", 99)
mock.assert_called_with("any", 99, {})
mock.assert_called_with("any", 99, None)
get_secret("any", 99, {"foo": "bar"})
mock.assert_called_with("any", 99, {"foo": "bar"})

Expand Down
2 changes: 2 additions & 0 deletions tests/secret_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ def test_ssm():
"secrets": {"provider": "ssm", "region": region, "prefix": "/app/dev"}
}
assert ssm("/test1", config_override={"prefix": "/dev"}), "string_value"
assert ssm("/password"), "my_secret_value"


config.CONFIG = {"secrets": {"provider": "ssm", "region": "eu-central-1"}}
with pytest.raises(RuntimeError, match=r"Failed to retrieve secret foo: ..."):
Expand Down

0 comments on commit 73edc78

Please sign in to comment.