Skip to content

Commit 04c55fe

Browse files
fix(parameters): include sdk_options in the cache key (#8619)
decrypt, recursive and the other sdk_options change what the provider returns, but the cache key only used (name, transform, is_nested). A call with different options was served the first call's cached value, for example ciphertext for decrypt=True or a non-recursive subset for recursive=True. Closes #8591 Signed-off-by: mayuriphad <mayuriphad656@gmail.com> Co-authored-by: Leandro Damascena <lcdama@amazon.pt>
1 parent 51b1e49 commit 04c55fe

3 files changed

Lines changed: 168 additions & 6 deletions

File tree

‎aws_lambda_powertools/utilities/parameters/base.py‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from __future__ import annotations
88

9+
import json
910
import os
1011
from abc import ABC, abstractmethod
1112
from collections.abc import Callable
@@ -98,7 +99,7 @@ def get(
9899
# parameter will always be used in a specific transform, this should be
99100
# an acceptable tradeoff.
100101
value: str | bytes | dict | None = None
101-
key = self._build_cache_key(name=name, transform=transform)
102+
key = self._build_cache_key(name=name, transform=transform, sdk_options=sdk_options)
102103

103104
# If max_age is not set, resolve it from the environment variable, defaulting to DEFAULT_MAX_AGE_SECS
104105
max_age = resolve_max_age(env=os.getenv(constants.PARAMETERS_MAX_AGE_ENV, DEFAULT_MAX_AGE_SECS), choice=max_age)
@@ -172,7 +173,7 @@ def get_multiple(
172173
TransformParameterError
173174
When the parameter provider fails to transform a parameter value.
174175
"""
175-
key = self._build_cache_key(name=path, transform=transform, is_nested=True)
176+
key = self._build_cache_key(name=path, transform=transform, is_nested=True, sdk_options=sdk_options)
176177

177178
# If max_age is not set, resolve it from the environment variable, defaulting to DEFAULT_MAX_AGE_SECS
178179
max_age = resolve_max_age(env=os.getenv(constants.PARAMETERS_MAX_AGE_ENV, DEFAULT_MAX_AGE_SECS), choice=max_age)
@@ -219,6 +220,7 @@ def _build_cache_key(
219220
name: str,
220221
transform: TransformOptions = None,
221222
is_nested: bool = False,
223+
sdk_options: dict[str, Any] | None = None,
222224
):
223225
"""Creates cache key for parameters
224226
@@ -230,13 +232,20 @@ def _build_cache_key(
230232
Transform method used, by default None
231233
is_nested : bool, optional
232234
Whether it's a single parameter or multiple nested parameters, by default False
235+
sdk_options : dict, optional
236+
Options passed to the underlying API call (e.g. decrypt, recursive, VersionStage).
237+
They change the returned value, so calls with different options must not share a cache entry.
233238
234239
Returns
235240
-------
236-
tuple[str, TransformOptions, bool]
241+
tuple
237242
Cache key
238243
"""
239-
return (name, transform, is_nested)
244+
if not sdk_options:
245+
return (name, transform, is_nested)
246+
247+
# sdk_options may hold unhashable values (dicts, lists), so serialize them deterministically
248+
return (name, transform, is_nested, json.dumps(sdk_options, sort_keys=True, default=str))
240249

241250

242251
def get_transform_method(value: str, transform: TransformOptions = None) -> Callable[..., Any]:

‎aws_lambda_powertools/utilities/parameters/secrets.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,12 @@ def get_multiple( # type: ignore[override] # ty: ignore[invalid-method-overrid
247247
# Create a unique cache key for this batch of secrets
248248
# Use sorted names to ensure consistent caching regardless of order
249249
cache_key_name = "|".join(sorted(names))
250-
key = self._build_cache_key(name=cache_key_name, transform=transform, is_nested=True)
250+
key = self._build_cache_key(
251+
name=cache_key_name,
252+
transform=transform,
253+
is_nested=True,
254+
sdk_options=sdk_options,
255+
)
251256

252257
# If max_age is not set, resolve it from the environment variable, defaulting to DEFAULT_MAX_AGE_SECS
253258
max_age = resolve_max_age(env=os.getenv(constants.PARAMETERS_MAX_AGE_ENV, DEFAULT_MAX_AGE_SECS), choice=max_age)

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

Lines changed: 149 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,8 @@ def test_ssm_provider_get_cached(mock_name, mock_value, config):
10051005
provider = parameters.SSMProvider(boto_config=config)
10061006

10071007
# Inject value in the internal store
1008-
cache_key = provider._build_cache_key(name=mock_name)
1008+
# SSMProvider.get() always passes decrypt to the SDK call, so it is part of the key
1009+
cache_key = provider._build_cache_key(name=mock_name, sdk_options={"decrypt": False})
10091010
provider.add_to_cache(key=cache_key, value=mock_value, max_age=60)
10101011

10111012
# Stub the boto3 client
@@ -1393,6 +1394,153 @@ def test_appconf_provider_clear_cache(mock_name, config):
13931394
assert provider.store == {}
13941395

13951396

1397+
def _ssm_get_parameter_response(name: str, value: str, version: int) -> dict:
1398+
return {
1399+
"Parameter": {
1400+
"Name": name,
1401+
"Type": "SecureString",
1402+
"Value": value,
1403+
"Version": version,
1404+
"Selector": f"{name}:{version}",
1405+
"SourceResult": "string",
1406+
"LastModifiedDate": datetime(2015, 1, 1),
1407+
"ARN": f"arn:aws:ssm:us-east-2:111122223333:parameter/{name}",
1408+
},
1409+
}
1410+
1411+
1412+
def test_ssm_provider_get_decrypt_does_not_reuse_encrypted_cache(mock_name, mock_version, config):
1413+
# GIVEN a parameter fetched without decryption and cached
1414+
provider = parameters.SSMProvider(boto_config=config)
1415+
stubber = stub.Stubber(provider.client)
1416+
stubber.add_response(
1417+
"get_parameter",
1418+
_ssm_get_parameter_response(mock_name, "ciphertext", mock_version),
1419+
{"Name": mock_name, "WithDecryption": False},
1420+
)
1421+
stubber.add_response(
1422+
"get_parameter",
1423+
_ssm_get_parameter_response(mock_name, "plaintext", mock_version),
1424+
{"Name": mock_name, "WithDecryption": True},
1425+
)
1426+
stubber.activate()
1427+
1428+
try:
1429+
encrypted = provider.get(mock_name)
1430+
1431+
# WHEN the same parameter is fetched with decrypt=True
1432+
decrypted = provider.get(mock_name, decrypt=True)
1433+
1434+
# THEN the second call reaches SSM instead of returning the cached ciphertext
1435+
assert encrypted == "ciphertext"
1436+
assert decrypted == "plaintext"
1437+
stubber.assert_no_pending_responses()
1438+
finally:
1439+
stubber.deactivate()
1440+
1441+
1442+
def test_ssm_provider_get_multiple_recursive_does_not_reuse_non_recursive_cache(mock_name, mock_version, config):
1443+
# GIVEN a path fetched non-recursively and cached
1444+
provider = parameters.SSMProvider(boto_config=config)
1445+
1446+
def _response(names: list[str]) -> dict:
1447+
return {
1448+
"Parameters": [
1449+
{
1450+
"Name": f"{mock_name}/{name}",
1451+
"Type": "String",
1452+
"Value": name,
1453+
"Version": mock_version,
1454+
"LastModifiedDate": datetime(2015, 1, 1),
1455+
"ARN": f"arn:aws:ssm:us-east-2:111122223333:parameter/{mock_name}/{name}",
1456+
}
1457+
for name in names
1458+
],
1459+
}
1460+
1461+
stubber = stub.Stubber(provider.client)
1462+
stubber.add_response(
1463+
"get_parameters_by_path",
1464+
_response(["A"]),
1465+
{"Path": mock_name, "Recursive": False, "WithDecryption": False},
1466+
)
1467+
stubber.add_response(
1468+
"get_parameters_by_path",
1469+
_response(["A", "B/C"]),
1470+
{"Path": mock_name, "Recursive": True, "WithDecryption": False},
1471+
)
1472+
stubber.activate()
1473+
1474+
try:
1475+
shallow = provider.get_multiple(mock_name)
1476+
1477+
# WHEN the same path is fetched recursively
1478+
deep = provider.get_multiple(mock_name, recursive=True)
1479+
1480+
# THEN the recursive call gets the nested parameter too
1481+
assert shallow == {"A": "A"}
1482+
assert deep == {"A": "A", "B/C": "B/C"}
1483+
stubber.assert_no_pending_responses()
1484+
finally:
1485+
stubber.deactivate()
1486+
1487+
1488+
def test_secrets_provider_get_different_sdk_options_are_cached_separately(mock_name, config):
1489+
# GIVEN a secret fetched at one version stage and cached
1490+
provider = parameters.SecretsProvider(boto_config=config)
1491+
stubber = stub.Stubber(provider.client)
1492+
stubber.add_response(
1493+
"get_secret_value",
1494+
{"Name": mock_name, "SecretString": "previous-value"},
1495+
{"SecretId": mock_name, "VersionStage": "AWSPREVIOUS"},
1496+
)
1497+
stubber.add_response(
1498+
"get_secret_value",
1499+
{"Name": mock_name, "SecretString": "current-value"},
1500+
{"SecretId": mock_name, "VersionStage": "AWSCURRENT"},
1501+
)
1502+
stubber.activate()
1503+
1504+
try:
1505+
previous = provider.get(mock_name, VersionStage="AWSPREVIOUS")
1506+
1507+
# WHEN it is fetched at another version stage, and again at the first one
1508+
current = provider.get(mock_name, VersionStage="AWSCURRENT")
1509+
previous_cached = provider.get(mock_name, VersionStage="AWSPREVIOUS")
1510+
1511+
# THEN each stage has its own value and repeated calls are still served from the cache
1512+
assert previous == "previous-value"
1513+
assert current == "current-value"
1514+
assert previous_cached == "previous-value"
1515+
stubber.assert_no_pending_responses()
1516+
finally:
1517+
stubber.deactivate()
1518+
1519+
1520+
def test_base_provider_cache_key_includes_unhashable_sdk_options(mock_name):
1521+
# GIVEN a provider whose SDK options hold an unhashable value
1522+
calls: list[dict] = []
1523+
1524+
class TestProvider(BaseProvider):
1525+
def _get(self, name: str, **sdk_options) -> str:
1526+
calls.append(sdk_options)
1527+
return f"value-{len(calls)}"
1528+
1529+
def _get_multiple(self, path: str, **sdk_options) -> dict[str, str]:
1530+
raise NotImplementedError()
1531+
1532+
provider = TestProvider()
1533+
1534+
# WHEN the same name is fetched with different and then repeated options
1535+
first = provider.get(mock_name, Filters=[{"Key": "a"}])
1536+
second = provider.get(mock_name, Filters=[{"Key": "b"}])
1537+
first_again = provider.get(mock_name, Filters=[{"Key": "a"}])
1538+
1539+
# THEN only the distinct options reach the underlying store
1540+
assert (first, second, first_again) == ("value-1", "value-2", "value-1")
1541+
assert len(calls) == 2
1542+
1543+
13961544
def test_ssm_provider_get_expired(mock_name, mock_value, mock_version, config):
13971545
"""
13981546
Test SSMProvider.get() with a cached but expired value

0 commit comments

Comments
 (0)