diff --git a/deckhand/barbican/cache.py b/deckhand/barbican/cache.py new file mode 100644 index 00000000..d357eeb1 --- /dev/null +++ b/deckhand/barbican/cache.py @@ -0,0 +1,80 @@ +# Copyright 2018 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License 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. + +from beaker.cache import CacheManager +from beaker.util import parse_cache_config_options +from oslo_log import log as logging + +from deckhand.conf import config + +CONF = config.CONF +LOG = logging.getLogger(__name__) + +_CACHE_OPTS = { + 'cache.type': 'memory', + 'expire': CONF.barbican.cache_timeout, +} +_CACHE = CacheManager(**parse_cache_config_options(_CACHE_OPTS)) +_BARBICAN_CACHE = _CACHE.get_cache('barbican_cache') + + +# NOTE(felipemonteiro): The functions below realize a lookup and reverse-lookup +# to allow for much faster retrieval of encrypted data from Barbican, which +# doesn't currently support batched requests in its Secrets API. This behavior +# is necessary since Deckhand has to potentially retrieve and store up to +# dozens of secrets per request. Note that data for both lookup functions +# below are invalidated together, as they are tied to the same cache. + +def lookup_by_ref(barbicanclient, secret_ref): + """Look up secret object using secret reference. + + Allows for quick lookup of secret payloads using ``secret_ref`` via + caching. + """ + def do_lookup(): + """Returns secret object stored in Barbican.""" + return barbicanclient.call("secrets.get", secret_ref) + + if CONF.barbican.enable_cache: + return _BARBICAN_CACHE.get(key=secret_ref, createfunc=do_lookup) + else: + return do_lookup() + + +def lookup_by_payload(barbicanclient, **kwargs): + """Look up secret reference using the secret payload. + + Allows for quick lookup of secret references using ``secret_payload`` via + caching (essentially a reverse-lookup). + + Useful for ensuring that documents with the same secret payload (which + occurs when the same document is recreated across different revisions) + persist the same secret reference in the database -- and thus quicker + future ``lookup_by_ref`` lookups. + """ + def do_lookup(): + """Returns secret Barbican reference.""" + secret = barbicanclient.call("secrets.create", **kwargs) + return secret.store() + + secret_payload = kwargs['payload'] + + if CONF.barbican.enable_cache: + return _BARBICAN_CACHE.get(key=secret_payload, createfunc=do_lookup) + else: + return do_lookup() + + +def invalidate(): + _BARBICAN_CACHE.clear() diff --git a/deckhand/barbican/driver.py b/deckhand/barbican/driver.py index 82d77009..03afc63d 100644 --- a/deckhand/barbican/driver.py +++ b/deckhand/barbican/driver.py @@ -20,6 +20,7 @@ from oslo_serialization import base64 from oslo_utils import excutils import six +from deckhand.barbican import cache from deckhand.barbican import client_wrapper from deckhand import errors from deckhand import types @@ -145,8 +146,7 @@ class BarbicanDriver(object): LOG.info('Storing encrypted document data in Barbican.') try: - secret = self.barbicanclient.call("secrets.create", **kwargs) - secret_ref = secret.store() + secret_ref = cache.lookup_by_payload(self.barbicanclient, **kwargs) except (barbicanclient.exceptions.HTTPAuthError, barbicanclient.exceptions.HTTPClientError) as e: LOG.exception(str(e)) @@ -180,7 +180,7 @@ class BarbicanDriver(object): def get_secret(self, secret_ref, src_doc): """Get a secret.""" try: - secret = self.barbicanclient.call("secrets.get", secret_ref) + secret = cache.lookup_by_ref(self.barbicanclient, secret_ref) except (barbicanclient.exceptions.HTTPAuthError, barbicanclient.exceptions.HTTPClientError) as e: LOG.exception(str(e)) @@ -204,6 +204,9 @@ class BarbicanDriver(object): def delete_secret(self, secret_ref): """Delete a secret.""" try: + # NOTE(felipemonteiro): No cache invalidation is performed here + # as the only API that invokes this method is DELETE /revisions + # which also invalidates the entire Barbican cache. return self.barbicanclient.call("secrets.delete", secret_ref) except (barbicanclient.exceptions.HTTPAuthError, barbicanclient.exceptions.HTTPServerError) as e: diff --git a/deckhand/conf/config.py b/deckhand/conf/config.py index 1a43c89c..17835ced 100644 --- a/deckhand/conf/config.py +++ b/deckhand/conf/config.py @@ -26,8 +26,8 @@ barbican_group = cfg.OptGroup( barbican_opts = [ - # TODO(fmontei): Drop these options and related group once Keystone - # endpoint lookup is used instead. + # TODO(felipemonteiro): Drop this option once Keystone endpoint lookup is + # implemented. cfg.StrOpt( 'api_endpoint', sample_default='http://barbican.example.org:9311/', @@ -35,7 +35,17 @@ barbican_opts = [ cfg.IntOpt( 'max_workers', default=10, help='Maximum number of threads used to call secret storage service ' - 'concurrently.') + 'concurrently.'), + # TODO(felipemonteiro): This is better off being removed because the same + # effect can be achieved through per-test gabbi fixtures that clean up + # the cache between tests. + cfg.BoolOpt('enable_cache', default=True, + help="Whether to enable Barbican secret caching. Useful " + "for testing to avoid cross-test caching conflicts."), + cfg.StrOpt( + 'cache_timeout', default=3600, + help="How long (in seconds) Barbican secret reference/payload lookup " + "results should remain cached in memory.") ] diff --git a/deckhand/control/common.py b/deckhand/control/common.py index 38a16834..9168ecaf 100644 --- a/deckhand/control/common.py +++ b/deckhand/control/common.py @@ -16,6 +16,7 @@ import functools import falcon +from deckhand.barbican import cache as barbican_cache from deckhand.engine import cache as engine_cache @@ -125,4 +126,5 @@ def sanitize_params(allowed_params): def invalidate_cache_data(): """Invalidate all data associated with document rendering.""" + barbican_cache.invalidate() engine_cache.invalidate() diff --git a/deckhand/tests/deckhand.conf.test b/deckhand/tests/deckhand.conf.test index f04331c5..3540ca9f 100644 --- a/deckhand/tests/deckhand.conf.test +++ b/deckhand/tests/deckhand.conf.test @@ -8,6 +8,7 @@ development_mode = false policy_file = policy.yaml [barbican] +enable_cache = false [database] connection = ${AIRSHIP_DECKHAND_DATABASE_URL} diff --git a/deckhand/tests/unit/barbican/__init__.py b/deckhand/tests/unit/barbican/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/deckhand/tests/unit/barbican/test_cache.py b/deckhand/tests/unit/barbican/test_cache.py new file mode 100644 index 00000000..1321aaea --- /dev/null +++ b/deckhand/tests/unit/barbican/test_cache.py @@ -0,0 +1,120 @@ +# Copyright 2018 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License 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. + +import mock +import testtools + +from deckhand.barbican import cache +from deckhand.tests import test_utils +from deckhand.tests.unit import base as test_base + + +class BarbicanCacheTest(test_base.DeckhandTestCase): + + def setUp(self): + super(BarbicanCacheTest, self).setUp() + self.secret_ref = test_utils.rand_barbican_ref() + self.secret_payload = 'very-secret-payload' + # Clear the cache between tests. + cache.invalidate() + + def _mock_barbicanclient(self): + def call_barbican(action, *args, **kwargs): + if action == "secrets.create": + return mock.Mock(**{'store.return_value': self.secret_ref}) + elif action == "secrets.get": + return mock.Mock(payload=self.secret_payload) + + mock_barbicanclient = mock.Mock() + mock_barbicanclient.call.side_effect = call_barbican + + return mock_barbicanclient + + @property + def barbicanclient(self): + return self._mock_barbicanclient() + + def test_lookup_by_ref_cache(self): + """Validate ``lookup_by_ref`` caching works. + + Passing in None in lieu of an actual barbican client (or mock object) + proves that: + + * if the payload is in the cache, then no error is thrown since the + cache is hit so no further processing is performed, where otherwise a + method would be called on `None` + * if the payload is not in the cache, then following logic above, + method is called on `None`, raising AttributeError + """ + + # Validate that caching the ref returns expected payload. + secret = cache.lookup_by_ref(self.barbicanclient, self.secret_ref) + self.assertEqual(self.secret_payload, secret.payload) + + # Validate that the cache actually works. + next_secret = cache.lookup_by_ref(None, self.secret_ref) + self.assertEqual(secret.payload, next_secret.payload) + + # Validate that the reverse cache works. + kwargs = {'payload': secret.payload} + secret_ref = cache.lookup_by_payload(self.barbicanclient, **kwargs) + self.assertEqual(self.secret_ref, secret_ref) + + # Different ref isn't in cache - expect AttributeError. + with testtools.ExpectedException(AttributeError): + cache.lookup_by_ref(None, secret_ref='uh-oh') + + # Invalidate the cache and ensure the original data isn't there. + cache.invalidate() + + # The cache won't be hit this time - expect AttributeError. + with testtools.ExpectedException(AttributeError): + cache.lookup_by_ref(None, self.secret_ref) + + def test_lookup_by_payload_cache(self): + """Validate ``lookup_by_payload`` caching works. + + Passing in None in lieu of an actual barbican client (or mock object) + proves that: + + * if the payload is in the cache, then no error is thrown since the + cache is hit so no further processing is performed, where otherwise a + method would be called on `None` + * if the payload is not in the cache, then following logic above, + method is called on `None`, raising AttributeError + """ + + # Validate that caching the payload returns expected ref. + kwargs = {'payload': self.secret_payload} + secret_ref = cache.lookup_by_payload(self.barbicanclient, **kwargs) + self.assertEqual(self.secret_ref, secret_ref) + + # Validate that the cache actually works. + next_secret_ref = cache.lookup_by_payload(None, **kwargs) + self.assertEqual(secret_ref, next_secret_ref) + + # Validate that the reverse cache works. + secret = cache.lookup_by_ref(self.barbicanclient, secret_ref) + self.assertEqual(self.secret_payload, secret.payload) + + # Different payload isn't in cache - expect AttributeError. + with testtools.ExpectedException(AttributeError): + cache.lookup_by_payload(None, payload='uh-oh') + + # Invalidate the cache and ensure the original data isn't there. + cache.invalidate() + + # The cache won't be hit this time - expect AttributeError. + with testtools.ExpectedException(AttributeError): + cache.lookup_by_payload(None, **kwargs)