diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0d5009cf..e4476b60 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,14 @@ Changelog ********* +1.0.7 -- 2018-01-xx +=================== + +Bugfixes +-------- +* Fix :class:`MostRecentProvider` cache reuse bug. + `#105 `_ + 1.0.6 -- 2018-01-15 =================== diff --git a/src/dynamodb_encryption_sdk/encrypted/item.py b/src/dynamodb_encryption_sdk/encrypted/item.py index 8ef19631..25f1ef50 100644 --- a/src/dynamodb_encryption_sdk/encrypted/item.py +++ b/src/dynamodb_encryption_sdk/encrypted/item.py @@ -70,7 +70,6 @@ def encrypt_dynamodb_item(item, crypto_config): 'Reserved attribute name "{}" is not allowed in plaintext item.'.format(reserved_name.value) ) - crypto_config.materials_provider.refresh() encryption_materials = crypto_config.encryption_materials() inner_material_description = encryption_materials.material_description.copy() diff --git a/src/dynamodb_encryption_sdk/internal/utils.py b/src/dynamodb_encryption_sdk/internal/utils.py index 6b1ee127..bb133766 100644 --- a/src/dynamodb_encryption_sdk/internal/utils.py +++ b/src/dynamodb_encryption_sdk/internal/utils.py @@ -59,10 +59,8 @@ class TableInfoCache(object): _client = attr.ib(validator=attr.validators.instance_of(botocore.client.BaseClient)) _auto_refresh_table_indexes = attr.ib(validator=attr.validators.instance_of(bool)) - def __init__( - self, client, auto_refresh_table_indexes # type: botocore.client.BaseClient # type: bool - ): # noqa=D107 - # type: (...) -> None + def __init__(self, client, auto_refresh_table_indexes): # noqa=D107 + # type: (botocore.client.BaseClient, bool) -> None # Workaround pending resolution of attrs/mypy interaction. # https://github.com/python/mypy/issues/2088 # https://github.com/python-attrs/attrs/issues/215 diff --git a/src/dynamodb_encryption_sdk/material_providers/most_recent.py b/src/dynamodb_encryption_sdk/material_providers/most_recent.py index 01c38607..80892882 100644 --- a/src/dynamodb_encryption_sdk/material_providers/most_recent.py +++ b/src/dynamodb_encryption_sdk/material_providers/most_recent.py @@ -112,6 +112,7 @@ def get(self, name): def clear(self): # type: () -> None """Clear the cache.""" + _LOGGER.debug("Clearing cache") with self._cache_lock: self._cache = OrderedDict() # type: OrderedDict[Any, Any] # pylint: disable=attribute-defined-outside-init @@ -163,8 +164,10 @@ def decryption_materials(self, encryption_context): """ version = self._provider_store.version_from_material_description(encryption_context.material_description) try: + _LOGGER.debug("Looking in cache for decryption materials provider version %d", version) provider = self._cache.get(version) except KeyError: + _LOGGER.debug("Decryption materials provider not found in cache") try: provider = self._provider_store.provider(self._material_name, version) except InvalidVersionError: @@ -183,6 +186,7 @@ def _ttl_action(self): :rtype: TtlActions """ if self._version is None: + _LOGGER.debug("TTL Expired because no version is known") return TtlActions.EXPIRED time_since_updated = time.time() - self._last_updated @@ -193,6 +197,7 @@ def _ttl_action(self): elif time_since_updated < self._version_ttl + _GRACE_PERIOD: return TtlActions.GRACE_PERIOD + _LOGGER.debug("TTL Expired because known version has expired") return TtlActions.EXPIRED def _get_max_version(self): @@ -260,6 +265,8 @@ def _get_most_recent_version(self, allow_local): finally: self._lock.release() + _LOGGER.debug("New latest version is %d", self._version) + return provider def encryption_materials(self, encryption_context): @@ -271,14 +278,16 @@ def encryption_materials(self, encryption_context): """ ttl_action = self._ttl_action() + _LOGGER.debug('TTL Action "%s" when getting encryption materials', ttl_action.name) + provider = None if ttl_action is TtlActions.LIVE: try: - _LOGGER.debug("Looking in cache for materials provider version %d", self._version) + _LOGGER.debug("Looking in cache for encryption materials provider version %d", self._version) provider = self._cache.get(self._version) except KeyError: - _LOGGER.debug("Provider not found in cache") + _LOGGER.debug("Encryption materials provider not found in cache") ttl_action = TtlActions.EXPIRED if provider is None: @@ -286,6 +295,7 @@ def encryption_materials(self, encryption_context): # Otherwise, block until we can acquire the lock. allow_local = bool(ttl_action is TtlActions.GRACE_PERIOD) + _LOGGER.debug("Getting most recent materials provider version") provider = self._get_most_recent_version(allow_local) return provider.encryption_materials(encryption_context) @@ -293,6 +303,7 @@ def encryption_materials(self, encryption_context): def refresh(self): # type: () -> None """Clear all local caches for this provider.""" + _LOGGER.debug("Refreshing MostRecentProvider instance.") with self._lock: self._cache.clear() self._version = None # type: int # pylint: disable=attribute-defined-outside-init diff --git a/src/dynamodb_encryption_sdk/material_providers/store/meta.py b/src/dynamodb_encryption_sdk/material_providers/store/meta.py index 2dbfe227..c16cd5e7 100644 --- a/src/dynamodb_encryption_sdk/material_providers/store/meta.py +++ b/src/dynamodb_encryption_sdk/material_providers/store/meta.py @@ -11,6 +11,7 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. """Meta cryptographic provider store.""" +import logging from enum import Enum import attr @@ -22,7 +23,7 @@ from dynamodb_encryption_sdk.delegated_keys.jce import JceNameLocalDelegatedKey from dynamodb_encryption_sdk.encrypted.table import EncryptedTable from dynamodb_encryption_sdk.exceptions import InvalidVersionError, NoKnownVersionError, VersionAlreadyExistsError -from dynamodb_encryption_sdk.identifiers import EncryptionKeyType, KeyEncodingType +from dynamodb_encryption_sdk.identifiers import LOGGER_NAME, EncryptionKeyType, KeyEncodingType from dynamodb_encryption_sdk.material_providers import CryptographicMaterialsProvider from dynamodb_encryption_sdk.material_providers.wrapped import WrappedCryptographicMaterialsProvider @@ -36,6 +37,7 @@ __all__ = ("MetaStore",) +_LOGGER = logging.getLogger(LOGGER_NAME) class MetaStoreAttributeNames(Enum): @@ -104,6 +106,7 @@ def create_table(cls, client, table_name, read_units, write_units): :param int read_units: Read capacity units to provision :param int write_units: Write capacity units to provision """ + _LOGGER.debug("Creating MetaStore table") try: client.create_table( TableName=table_name, @@ -127,6 +130,7 @@ def _load_materials(self, material_name, version): :returns: Materials loaded into delegated keys :rtype: tuple(JceNameLocalDelegatedKey) """ + _LOGGER.debug('Loading material "%s" version %d from MetaStore table', material_name, version) key = {MetaStoreAttributeNames.PARTITION.value: material_name, MetaStoreAttributeNames.SORT.value: version} response = self._encrypted_table.get_item(Key=key) try: @@ -168,6 +172,7 @@ def _save_materials(self, material_name, version, encryption_key, signing_key): :param int version: Version of material to locate :raises VersionAlreadyExistsError: if the specified version already exists """ + _LOGGER.debug('Saving material "%s" version %d to MetaStore table', material_name, version) item = { MetaStoreAttributeNames.PARTITION.value: material_name, MetaStoreAttributeNames.SORT.value: version, diff --git a/test/functional/functional_test_utils.py b/test/functional/functional_test_utils.py index 202b0e02..9353408a 100644 --- a/test/functional/functional_test_utils.py +++ b/test/functional/functional_test_utils.py @@ -16,15 +16,14 @@ import base64 import copy import itertools -import logging import os from collections import defaultdict from decimal import Decimal import boto3 import pytest -import six from boto3.dynamodb.types import Binary +from botocore.exceptions import NoRegionError from moto import mock_dynamodb2 from dynamodb_encryption_sdk.delegated_keys.jce import JceNameLocalDelegatedKey @@ -34,6 +33,7 @@ from dynamodb_encryption_sdk.encrypted.table import EncryptedTable from dynamodb_encryption_sdk.identifiers import CryptoAction from dynamodb_encryption_sdk.internal.identifiers import ReservedAttributes +from dynamodb_encryption_sdk.material_providers.most_recent import MostRecentProvider from dynamodb_encryption_sdk.material_providers.static import StaticCryptographicMaterialsProvider from dynamodb_encryption_sdk.material_providers.store.meta import MetaStore from dynamodb_encryption_sdk.material_providers.wrapped import WrappedCryptographicMaterialsProvider @@ -44,11 +44,12 @@ RUNNING_IN_TRAVIS = "TRAVIS" in os.environ _DELEGATED_KEY_CACHE = defaultdict(lambda: defaultdict(dict)) TEST_TABLE_NAME = "my_table" +TEST_REGION_NAME = "us-west-2" TEST_INDEX = { "partition_attribute": {"type": "S", "value": "test_value"}, "sort_attribute": {"type": "N", "value": Decimal("99.233")}, } -SECONARY_INDEX = { +SECONDARY_INDEX = { "secondary_index_1": {"type": "B", "value": Binary(b"\x00\x01\x02")}, "secondary_index_2": {"type": "S", "value": "another_value"}, } @@ -76,8 +77,8 @@ @pytest.fixture def example_table(): - mock_dynamodb2().start() - ddb = boto3.client("dynamodb", region_name="us-west-2") + mock_dynamodb2().start(reset=False) + ddb = boto3.client("dynamodb", region_name=TEST_REGION_NAME) ddb.create_table( TableName=TEST_TABLE_NAME, KeySchema=[ @@ -95,9 +96,9 @@ def example_table(): @pytest.fixture -def table_with_local_seconary_indexes(): - mock_dynamodb2().start() - ddb = boto3.client("dynamodb", region_name="us-west-2") +def table_with_local_secondary_indexes(): + mock_dynamodb2().start(reset=False) + ddb = boto3.client("dynamodb", region_name=TEST_REGION_NAME) ddb.create_table( TableName=TEST_TABLE_NAME, KeySchema=[ @@ -118,7 +119,7 @@ def table_with_local_seconary_indexes(): ], AttributeDefinitions=[ {"AttributeName": name, "AttributeType": value["type"]} - for name, value in list(TEST_INDEX.items()) + list(SECONARY_INDEX.items()) + for name, value in list(TEST_INDEX.items()) + list(SECONDARY_INDEX.items()) ], ProvisionedThroughput={"ReadCapacityUnits": 100, "WriteCapacityUnits": 100}, ) @@ -128,9 +129,9 @@ def table_with_local_seconary_indexes(): @pytest.fixture -def table_with_global_seconary_indexes(): - mock_dynamodb2().start() - ddb = boto3.client("dynamodb", region_name="us-west-2") +def table_with_global_secondary_indexes(): + mock_dynamodb2().start(reset=False) + ddb = boto3.client("dynamodb", region_name=TEST_REGION_NAME) ddb.create_table( TableName=TEST_TABLE_NAME, KeySchema=[ @@ -153,7 +154,7 @@ def table_with_global_seconary_indexes(): ], AttributeDefinitions=[ {"AttributeName": name, "AttributeType": value["type"]} - for name, value in list(TEST_INDEX.items()) + list(SECONARY_INDEX.items()) + for name, value in list(TEST_INDEX.items()) + list(SECONDARY_INDEX.items()) ], ProvisionedThroughput={"ReadCapacityUnits": 100, "WriteCapacityUnits": 100}, ) @@ -650,22 +651,100 @@ def client_cycle_batch_items_check_paginators( def build_metastore(): - client = boto3.client("dynamodb", region_name="us-west-2") + client = boto3.client("dynamodb", region_name=TEST_REGION_NAME) table_name = base64.urlsafe_b64encode(os.urandom(32)).decode("utf-8").replace("=", ".") MetaStore.create_table(client, table_name, 1, 1) waiter = client.get_waiter("table_exists") waiter.wait(TableName=table_name) - table = boto3.resource("dynamodb", region_name="us-west-2").Table(table_name) - yield MetaStore(table, build_static_jce_cmp("AES", 256, "HmacSHA256", 256)) + table = boto3.resource("dynamodb", region_name=TEST_REGION_NAME).Table(table_name) + return MetaStore(table, build_static_jce_cmp("AES", 256, "HmacSHA256", 256)), table_name + +def delete_metastore(table_name): + client = boto3.client("dynamodb", region_name=TEST_REGION_NAME) client.delete_table(TableName=table_name) - waiter = client.get_waiter("table_not_exists") - waiter.wait(TableName=table_name) + # It sometimes takes a long time to delete a table. + # If hanging, asynchronously deleting tables becomes an issue, + # come back to this. + # Otherwise, let's just let them take care of themselves. + # waiter = client.get_waiter("table_not_exists") + # waiter.wait(TableName=table_name) @pytest.fixture def mock_metastore(): with mock_dynamodb2(): - yield next(build_metastore()) + metastore, table_name = build_metastore() + yield metastore + delete_metastore(table_name) + + +def _count_entries(records, *messages): + count = 0 + + for record in records: + if all((message in record.getMessage() for message in messages)): + count += 1 + + return count + + +def _count_puts(records, table_name): + return _count_entries(records, '"TableName": "{}"'.format(table_name), "OperationModel(name=PutItem)") + + +def _count_gets(records, table_name): + return _count_entries(records, '"TableName": "{}"'.format(table_name), "OperationModel(name=GetItem)") + + +def check_metastore_cache_use_encrypt(metastore, table_name, log_capture): + try: + table = boto3.resource("dynamodb").Table(table_name) + except NoRegionError: + table = boto3.resource("dynamodb", region_name=TEST_REGION_NAME).Table(table_name) + + most_recent_provider = MostRecentProvider(provider_store=metastore, material_name="test", version_ttl=600.0) + e_table = EncryptedTable(table=table, materials_provider=most_recent_provider) + + item = diverse_item() + item.update(TEST_KEY) + e_table.put_item(Item=item) + e_table.put_item(Item=item) + e_table.put_item(Item=item) + e_table.put_item(Item=item) + + try: + primary_puts = _count_puts(log_capture.records, e_table.name) + metastore_puts = _count_puts(log_capture.records, metastore._table.name) + + assert primary_puts == 4 + assert metastore_puts == 1 + + e_table.get_item(Key=TEST_KEY) + e_table.get_item(Key=TEST_KEY) + e_table.get_item(Key=TEST_KEY) + + primary_gets = _count_gets(log_capture.records, e_table.name) + metastore_gets = _count_gets(log_capture.records, metastore._table.name) + metastore_puts = _count_puts(log_capture.records, metastore._table.name) + + assert primary_gets == 3 + assert metastore_gets == 0 + assert metastore_puts == 1 + + most_recent_provider.refresh() + + e_table.get_item(Key=TEST_KEY) + e_table.get_item(Key=TEST_KEY) + e_table.get_item(Key=TEST_KEY) + + primary_gets = _count_gets(log_capture.records, e_table.name) + metastore_gets = _count_gets(log_capture.records, metastore._table.name) + + assert primary_gets == 6 + assert metastore_gets == 1 + + finally: + e_table.delete_item(Key=TEST_KEY) diff --git a/test/functional/material_providers/test_most_recent.py b/test/functional/material_providers/test_most_recent.py index 4d20727a..1d41ef83 100644 --- a/test/functional/material_providers/test_most_recent.py +++ b/test/functional/material_providers/test_most_recent.py @@ -21,6 +21,13 @@ from dynamodb_encryption_sdk.material_providers.most_recent import MostRecentProvider from dynamodb_encryption_sdk.material_providers.store import ProviderStore +from ..functional_test_utils import ( # pylint: disable=unused-import + TEST_TABLE_NAME, + check_metastore_cache_use_encrypt, + example_table, + mock_metastore, +) + pytestmark = [pytest.mark.functional, pytest.mark.local] @@ -130,3 +137,7 @@ def test_decryption_materials_cache_use(): expected_calls.append(("version_from_material_description", 0)) assert store.provider_calls == expected_calls + + +def test_cache_use_encrypt(mock_metastore, example_table, caplog): + check_metastore_cache_use_encrypt(mock_metastore, TEST_TABLE_NAME, caplog) diff --git a/test/functional/test_structures.py b/test/functional/test_structures.py index afe5c0f2..0c180fe9 100644 --- a/test/functional/test_structures.py +++ b/test/functional/test_structures.py @@ -21,8 +21,8 @@ from .functional_test_utils import ( TEST_TABLE_NAME, example_table, - table_with_global_seconary_indexes, - table_with_local_seconary_indexes, + table_with_global_secondary_indexes, + table_with_local_secondary_indexes, ) pytestmark = [pytest.mark.functional, pytest.mark.local] @@ -37,14 +37,14 @@ def test_tableinfo_refresh_indexes_no_secondary_indexes(example_table): table.refresh_indexed_attributes(client) -def test_tableinfo_refresh_indexes_with_gsis(table_with_global_seconary_indexes): +def test_tableinfo_refresh_indexes_with_gsis(table_with_global_secondary_indexes): client = boto3.client("dynamodb", region_name="us-west-2") table = TableInfo(name=TEST_TABLE_NAME) table.refresh_indexed_attributes(client) -def test_tableinfo_refresh_indexes_with_lsis(table_with_local_seconary_indexes): +def test_tableinfo_refresh_indexes_with_lsis(table_with_local_secondary_indexes): client = boto3.client("dynamodb", region_name="us-west-2") table = TableInfo(name=TEST_TABLE_NAME) diff --git a/test/integration/integration_test_utils.py b/test/integration/integration_test_utils.py index 522cc130..c5b24ea2 100644 --- a/test/integration/integration_test_utils.py +++ b/test/integration/integration_test_utils.py @@ -69,4 +69,6 @@ def ddb_table_name(): @pytest.fixture def temp_metastore(): - yield next(functional_test_utils.build_metastore()) + metastore, table_name = functional_test_utils.build_metastore() + yield metastore + functional_test_utils.delete_metastore(table_name) diff --git a/test/integration/material_providers/store/test_meta.py b/test/integration/material_providers/store/test_meta.py index 50dadc71..c092ba21 100644 --- a/test/integration/material_providers/store/test_meta.py +++ b/test/integration/material_providers/store/test_meta.py @@ -11,9 +11,6 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. """Integration tests for ``dynamodb_encryption_sdk.material_providers.store.meta``.""" -import os - -import boto3 import pytest from dynamodb_encryption_sdk.exceptions import NoKnownVersionError diff --git a/test/integration/material_providers/test_most_recent.py b/test/integration/material_providers/test_most_recent.py new file mode 100644 index 00000000..90c2ccf8 --- /dev/null +++ b/test/integration/material_providers/test_most_recent.py @@ -0,0 +1,26 @@ +# Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. +"""Load testing using MostRecentProvider and MetaStore.""" +import pytest + +from ..integration_test_utils import ( # pylint: disable=unused-import + ddb_table_name, + functional_test_utils, + temp_metastore, +) + +pytestmark = [pytest.mark.integ, pytest.mark.ddb_integ] + + +def test_cache_use_encrypt(temp_metastore, ddb_table_name, caplog): + functional_test_utils.check_metastore_cache_use_encrypt(temp_metastore, ddb_table_name, caplog)