Skip to content

Commit 13dbc55

Browse files
fix(parameters): use shared cache key in get_parameters_by_name (#8614)
* fix(parameters): use shared cache key in get_parameters_by_name * fix(parameters): include decrypt in get_parameters_by_name cache key get() stores entries under (name, transform, is_nested, sdk_options) since #8619, so the batch path needs the same shape to share cache. Including decrypt also stops a SecureString fetched with decrypt=True from being served from cache to a later call without it. --------- Co-authored-by: Leandro Damascena <lcdama@amazon.pt>
1 parent 523504d commit 13dbc55

2 files changed

Lines changed: 118 additions & 7 deletions

File tree

‎aws_lambda_powertools/utilities/parameters/ssm.py‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,13 @@ def _get_parameters_by_name_from_cache(self, batch: dict[str, dict]) -> dict[str
572572
"""Fetch each parameter from batch that hasn't been expired"""
573573
cache = {}
574574
for name, options in batch.items():
575-
cache_key = (name, options["transform"])
575+
# Same key shape as get() so both paths share cache entries; decrypt changes the value,
576+
# so a SecureString fetched with decrypt must never be served to a call without it
577+
cache_key = self._build_cache_key(
578+
name=name,
579+
transform=options["transform"],
580+
sdk_options={"decrypt": options["decrypt"]},
581+
)
576582
if self.has_not_expired_in_cache(cache_key):
577583
cache[name] = self.store[cache_key].value
578584

@@ -688,7 +694,11 @@ def _transform_and_cache_get_parameters_response(
688694
transform_errors.append(name)
689695
continue
690696

691-
_cache_key = (name, transform)
697+
_cache_key = self._build_cache_key(
698+
name=name,
699+
transform=transform,
700+
sdk_options={"decrypt": options["decrypt"]},
701+
)
692702
self.add_to_cache(key=_cache_key, value=value, max_age=options["max_age"])
693703

694704
response[name] = value

‎tests/functional/parameters/_boto3/test_utilities_parameters.py‎

Lines changed: 106 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1620,7 +1620,7 @@ def get_parameters(self, **kwargs):
16201620
# THEN the transform failure is reported and not cached
16211621
assert ret["_errors"] == [param]
16221622
assert param not in ret
1623-
assert (param, "json") not in provider.store
1623+
assert provider._build_cache_key(name=param, transform="json", sdk_options={"decrypt": False}) not in provider.store
16241624

16251625
# WHEN fetching again after the parameter is corrected
16261626
ret = provider.get_parameters_by_name(parameters=params, transform="json", raise_on_error=False)
@@ -1629,7 +1629,7 @@ def get_parameters(self, **kwargs):
16291629
assert client.get_parameters_calls == 2
16301630
assert ret["_errors"] == []
16311631
assert ret[param] == corrected_value
1632-
assert (param, "json") in provider.store
1632+
assert provider._build_cache_key(name=param, transform="json", sdk_options={"decrypt": False}) in provider.store
16331633

16341634

16351635
def test_ssm_provider_get_parameters_by_name_raise_on_transform_failure_with_decrypt(mock_name, config):
@@ -1699,7 +1699,9 @@ def get_parameters(self, **kwargs):
16991699
assert ret["_errors"] == []
17001700
assert ret[decrypt_param] is None
17011701
assert ret[batch_param] is None
1702-
assert (batch_param, "json") in provider.store
1702+
assert (
1703+
provider._build_cache_key(name=batch_param, transform="json", sdk_options={"decrypt": False}) in provider.store
1704+
)
17031705

17041706

17051707
def test_ssm_provider_get_parameters_by_name_auto_transform_infers_from_parameter_name(mock_name, mock_value, config):
@@ -3296,7 +3298,7 @@ def _get_parameters_by_name(self, parameters, raise_on_error=True, decrypt=False
32963298
def test_get_parameters_by_name_cache(monkeypatch, mock_name, mock_value, config):
32973299
# GIVEN we have a parameter to fetch but is already in cache
32983300
params = {mock_name: {}}
3299-
cache_key = (mock_name, None)
3301+
cache_key = SSMProvider(boto_config=config)._build_cache_key(name=mock_name, sdk_options={"decrypt": False})
33003302

33013303
class TestProvider(SSMProvider):
33023304
def __init__(self, boto_config: Config = config, **kwargs):
@@ -3306,7 +3308,7 @@ def _get_parameters_by_name(self, *args, **kwargs) -> tuple[dict[str, Any], list
33063308
raise RuntimeError("Should not be called if it's in cache")
33073309

33083310
provider = TestProvider()
3309-
provider.add_to_cache(key=(mock_name, None), value=mock_value, max_age=10)
3311+
provider.add_to_cache(key=cache_key, value=mock_value, max_age=10)
33103312

33113313
monkeypatch.setitem(parameters.base.DEFAULT_PROVIDERS, "ssm", provider)
33123314

@@ -4615,6 +4617,105 @@ def test_secrets_provider_get_multiple_with_json_transform_error(config):
46154617
provider.get_multiple(secret_names, transform="binary", raise_on_transform_error=True)
46164618

46174619

4620+
def test_ssm_provider_get_parameters_by_name_uses_cache_populated_by_get(mock_name, mock_value, config):
4621+
# GIVEN a parameter fetched and cached through get()
4622+
param = f"/a/{mock_name}"
4623+
get_response = {"Parameter": {"Name": param, "Value": mock_value, "Type": "String", "Version": 1}}
4624+
4625+
provider = parameters.SSMProvider(boto_config=config)
4626+
stubber = stub.Stubber(provider.client)
4627+
# only ONE API call is stubbed: a second call would raise UnStubbedResponseError
4628+
stubber.add_response("get_parameter", get_response, {"Name": param, "WithDecryption": False})
4629+
stubber.activate()
4630+
4631+
# WHEN the same parameter is requested through get_parameters_by_name
4632+
try:
4633+
provider.get(param)
4634+
ret = provider.get_parameters_by_name(parameters={param: {}})
4635+
4636+
# THEN it is served from cache, without another API call
4637+
assert ret == {param: mock_value}
4638+
stubber.assert_no_pending_responses()
4639+
finally:
4640+
stubber.deactivate()
4641+
4642+
4643+
def test_ssm_provider_get_uses_cache_populated_by_get_parameters_by_name(mock_name, mock_value, config):
4644+
# GIVEN a parameter fetched and cached through get_parameters_by_name
4645+
param = f"/a/{mock_name}"
4646+
fail = "/does_not_exist" # stub model doesn't support all-success yet
4647+
stub_response = build_get_parameters_stub(params={param: mock_value}, invalid_parameters=[fail])
4648+
4649+
provider = parameters.SSMProvider(boto_config=config)
4650+
stubber = stub.Stubber(provider.client)
4651+
# only ONE API call is stubbed: a second call would raise UnStubbedResponseError
4652+
stubber.add_response("get_parameters", stub_response, {"Names": [param, fail]})
4653+
stubber.activate()
4654+
4655+
# WHEN the same parameter is requested through get()
4656+
try:
4657+
provider.get_parameters_by_name(parameters={param: {}, fail: {}}, raise_on_error=False)
4658+
ret = provider.get(param)
4659+
4660+
# THEN it is served from cache, without another API call
4661+
assert ret == mock_value
4662+
stubber.assert_no_pending_responses()
4663+
finally:
4664+
stubber.deactivate()
4665+
4666+
4667+
def test_ssm_provider_get_parameters_by_name_decrypt_does_not_share_cache(mock_name, mock_value, config):
4668+
# GIVEN a SecureString fetched through get_parameters_by_name with decrypt=True
4669+
param = f"/a/{mock_name}"
4670+
fail = "/does_not_exist" # stub model doesn't support all-success yet
4671+
decrypted = build_get_parameters_stub(params={param: mock_value}, invalid_parameters=[fail])
4672+
encrypted = build_get_parameters_stub(params={param: "AQICAHg..."}, invalid_parameters=[fail])
4673+
4674+
provider = parameters.SSMProvider(boto_config=config)
4675+
stubber = stub.Stubber(provider.client)
4676+
stubber.add_response("get_parameters", decrypted, {"Names": [param, fail], "WithDecryption": True})
4677+
stubber.add_response("get_parameters", encrypted, {"Names": [param, fail]})
4678+
stubber.activate()
4679+
4680+
# WHEN the same parameter is requested again without decrypt
4681+
try:
4682+
first = provider.get_parameters_by_name(parameters={param: {}, fail: {}}, decrypt=True, raise_on_error=False)
4683+
second = provider.get_parameters_by_name(parameters={param: {}, fail: {}}, raise_on_error=False)
4684+
4685+
# THEN the decrypted value is not served from cache and the API is called again
4686+
assert first[param] == mock_value
4687+
assert second[param] == "AQICAHg..."
4688+
stubber.assert_no_pending_responses()
4689+
finally:
4690+
stubber.deactivate()
4691+
4692+
4693+
def test_ssm_provider_get_with_decrypt_does_not_share_cache_with_batch(mock_name, mock_value, config):
4694+
# GIVEN a SecureString fetched through get() with decrypt=True
4695+
param = f"/a/{mock_name}"
4696+
fail = "/does_not_exist" # stub model doesn't support all-success yet
4697+
get_response = {"Parameter": {"Name": param, "Value": mock_value, "Type": "SecureString", "Version": 1}}
4698+
encrypted = build_get_parameters_stub(params={param: "AQICAHg..."}, invalid_parameters=[fail])
4699+
4700+
provider = parameters.SSMProvider(boto_config=config)
4701+
stubber = stub.Stubber(provider.client)
4702+
stubber.add_response("get_parameter", get_response, {"Name": param, "WithDecryption": True})
4703+
stubber.add_response("get_parameters", encrypted, {"Names": [param, fail]})
4704+
stubber.activate()
4705+
4706+
# WHEN the same parameter is requested through get_parameters_by_name without decrypt
4707+
try:
4708+
single = provider.get(param, decrypt=True)
4709+
batch = provider.get_parameters_by_name(parameters={param: {}, fail: {}}, raise_on_error=False)
4710+
4711+
# THEN the decrypted value is not served from cache and the API is called again
4712+
assert single == mock_value
4713+
assert batch[param] == "AQICAHg..."
4714+
stubber.assert_no_pending_responses()
4715+
finally:
4716+
stubber.deactivate()
4717+
4718+
46184719
@pytest.mark.parametrize("force_fetch", [False, True])
46194720
@pytest.mark.parametrize("secret_value", [{"SecretString": "value"}, {"SecretBinary": b"value"}])
46204721
def test_secrets_provider_get_multiple_excludes_prefix_matches(config, force_fetch, secret_value):

0 commit comments

Comments
 (0)