From 9ca72c296b64bbe678dfa9c65039f40f23eac290 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Fri, 12 Jun 2020 10:01:20 -0500 Subject: [PATCH] Cache auth token in keyring keystoneauth has support for caching auth tokens. Plumb it through so that we use it in openstacksdk if keyring is installed. Make it an opt-in for now via config option 'cache.auth' which will at the very least help us test it. Change-Id: Ia71d128afd628ed264bcc0d8d61c421861df459f --- doc/source/user/config/configuration.rst | 11 ++++ openstack/config/cloud_region.py | 44 +++++++++++++++- openstack/config/loader.py | 22 ++++++-- openstack/connection.py | 5 ++ openstack/tests/unit/config/base.py | 2 +- openstack/tests/unit/config/test_config.py | 50 +++++++++++++++++++ ...ache-auth-in-keyring-773dd5f682cd1610.yaml | 5 ++ 7 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/cache-auth-in-keyring-773dd5f682cd1610.yaml diff --git a/doc/source/user/config/configuration.rst b/doc/source/user/config/configuration.rst index 36fbf3bb2..221af4705 100644 --- a/doc/source/user/config/configuration.rst +++ b/doc/source/user/config/configuration.rst @@ -250,6 +250,17 @@ are connecting to OpenStack can share a cache should you desire. region_name: ca-ymq-1 dns_api_version: 1 +`openstacksdk` can also cache authorization state (token) in the keyring. +That allow the consequent connections to the same cloud to skip fetching new +token. When the token gets expired or gets invalid `openstacksdk` will +establish new connection. + + +.. code-block:: yaml + + cache: + auth: true + IPv6 ---- diff --git a/openstack/config/cloud_region.py b/openstack/config/cloud_region.py index 346d38f4c..79b5003cf 100644 --- a/openstack/config/cloud_region.py +++ b/openstack/config/cloud_region.py @@ -17,6 +17,11 @@ import os.path import warnings import urllib +try: + import keyring +except ImportError: + keyring = None + from keystoneauth1 import discover import keystoneauth1.exceptions.catalog from keystoneauth1.loading import adapter as ks_load_adap @@ -239,7 +244,8 @@ class CloudRegion: cache_arguments=None, password_callback=None, statsd_host=None, statsd_port=None, statsd_prefix=None, influxdb_config=None, - collector_registry=None): + collector_registry=None, + cache_auth=False): self._name = name self.config = _util.normalize_keys(config) # NOTE(efried): For backward compatibility: a) continue to accept the @@ -251,6 +257,8 @@ class CloudRegion: self.log = _log.setup_logging('openstack.config') self._force_ipv4 = force_ipv4 self._auth = auth_plugin + self._cache_auth = cache_auth + self.load_auth_from_cache() self._openstack_config = openstack_config self._keystone_session = session self._session_constructor = session_constructor or ks_session.Session @@ -557,6 +565,40 @@ class CloudRegion: """Return a keystoneauth plugin from the auth credentials.""" return self._auth + def skip_auth_cache(self): + return not keyring or not self._auth or not self._cache_auth + + def load_auth_from_cache(self): + if self.skip_auth_cache(): + return + + cache_id = self._auth.get_cache_id() + + # skip if the plugin does not support caching + if not cache_id: + return + + try: + state = keyring.get_password('openstacksdk', cache_id) + except RuntimeError: # the fail backend raises this + self.log.debug('Failed to fetch auth from keyring') + return + + self.log.debug('Reusing authentication from keyring') + self._auth.set_auth_state(state) + + def set_auth_cache(self): + if self.skip_auth_cache(): + return + + cache_id = self._auth.get_cache_id() + state = self._auth.get_auth_state() + + try: + keyring.set_password('openstacksdk', cache_id, state) + except RuntimeError: # the fail backend raises this + self.log.debug('Failed to set auth into keyring') + def insert_user_agent(self): """Set sdk information into the user agent of the Session. diff --git a/openstack/config/loader.py b/openstack/config/loader.py index 79bc14741..3267a9967 100644 --- a/openstack/config/loader.py +++ b/openstack/config/loader.py @@ -160,9 +160,19 @@ class OpenStackConfig: self._load_envvars = load_envvars if load_yaml_config: - self._config_files = config_files or CONFIG_FILES - self._secure_files = secure_files or SECURE_FILES - self._vendor_files = vendor_files or VENDOR_FILES + # "if config_files" is not sufficient to process empty list + if config_files is not None: + self._config_files = config_files + else: + self._config_files = CONFIG_FILES + if secure_files is not None: + self._secure_files = secure_files + else: + self._secure_files = SECURE_FILES + if vendor_files is not None: + self._vendor_files = vendor_files + else: + self._vendor_files = VENDOR_FILES else: self._config_files = [] self._secure_files = [] @@ -259,6 +269,7 @@ class OpenStackConfig: clouds=dict(defaults=dict(self.defaults))) self.default_cloud = 'defaults' + self._cache_auth = False self._cache_expiration_time = 0 self._cache_path = CACHE_PATH self._cache_class = 'dogpile.cache.null' @@ -268,6 +279,9 @@ class OpenStackConfig: if 'cache' in self.cloud_config: cache_settings = _util.normalize_keys(self.cloud_config['cache']) + self._cache_auth = get_boolean( + cache_settings.get('auth', self._cache_auth)) + # expiration_time used to be 'max_age' but the dogpile setting # is expiration_time. Support max_age for backwards compat. self._cache_expiration_time = cache_settings.get( @@ -1146,6 +1160,7 @@ class OpenStackConfig: session_constructor=self._session_constructor, app_name=self._app_name, app_version=self._app_version, + cache_auth=self._cache_auth, cache_expiration_time=self._cache_expiration_time, cache_expirations=self._cache_expirations, cache_path=self._cache_path, @@ -1251,6 +1266,7 @@ class OpenStackConfig: force_ipv4=force_ipv4, auth_plugin=auth_plugin, openstack_config=self, + cache_auth=self._cache_auth, cache_expiration_time=self._cache_expiration_time, cache_expirations=self._cache_expirations, cache_path=self._cache_path, diff --git a/openstack/connection.py b/openstack/connection.py index 8b8994941..43cce4787 100644 --- a/openstack/connection.py +++ b/openstack/connection.py @@ -455,6 +455,10 @@ class Connection( self.config._influxdb_config['additional_metric_tags'] = \ self.config.config['additional_metric_tags'] + def __del__(self): + # try to force release of resources and save authorization + self.close() + @property def session(self): if not self._session: @@ -531,6 +535,7 @@ class Connection( """Release any resources held open.""" if self.__pool_executor: self.__pool_executor.shutdown() + self.config.set_auth_cache() def set_global_request_id(self, global_request_id): self._global_request_id = global_request_id diff --git a/openstack/tests/unit/config/base.py b/openstack/tests/unit/config/base.py index 471cb5a6b..81102cea0 100644 --- a/openstack/tests/unit/config/base.py +++ b/openstack/tests/unit/config/base.py @@ -194,7 +194,7 @@ NO_CONF = { def _write_yaml(obj): # Assume NestedTempfile so we don't have to cleanup - with tempfile.NamedTemporaryFile(delete=False) as obj_yaml: + with tempfile.NamedTemporaryFile(delete=False, suffix='.yaml') as obj_yaml: obj_yaml.write(yaml.safe_dump(obj).encode('utf-8')) return obj_yaml.name diff --git a/openstack/tests/unit/config/test_config.py b/openstack/tests/unit/config/test_config.py index e0fb827cc..f697c03da 100644 --- a/openstack/tests/unit/config/test_config.py +++ b/openstack/tests/unit/config/test_config.py @@ -15,6 +15,7 @@ import argparse import copy import os +from unittest import mock import fixtures import testtools @@ -91,6 +92,7 @@ class TestConfig(base.TestCase): } }) c = config.OpenStackConfig(config_files=[single_conf], + secure_files=[], vendor_files=[self.vendor_yaml]) cc = c.get_one() self.assertEqual(cc.name, 'single') @@ -180,6 +182,7 @@ class TestConfig(base.TestCase): } }) c = config.OpenStackConfig(config_files=[single_conf], + secure_files=[], vendor_files=[self.vendor_yaml]) cc = c.get_one() self.assertEqual('http://example.com/v2', cc.get_endpoint('identity')) @@ -463,6 +466,53 @@ class TestConfig(base.TestCase): exceptions.ConfigException, c._get_region, cloud='_test_cloud', region_name='region1') + @mock.patch('openstack.config.cloud_region.keyring') + @mock.patch( + 'keystoneauth1.identity.base.BaseIdentityPlugin.set_auth_state') + def test_load_auth_cache_not_found(self, ks_mock, kr_mock): + c = config.OpenStackConfig( + config_files=[self.cloud_yaml], secure_files=[]) + c._cache_auth = True + + kr_mock.get_password = mock.Mock(side_effect=[RuntimeError]) + + region = c.get_one('_test-cloud_') + kr_mock.get_password.assert_called_with( + 'openstacksdk', region._auth.get_cache_id()) + ks_mock.assert_not_called() + + @mock.patch('openstack.config.cloud_region.keyring') + @mock.patch( + 'keystoneauth1.identity.base.BaseIdentityPlugin.set_auth_state') + def test_load_auth_cache_found(self, ks_mock, kr_mock): + c = config.OpenStackConfig( + config_files=[self.cloud_yaml], secure_files=[]) + c._cache_auth = True + fake_auth = {'a': 'b'} + + kr_mock.get_password = mock.Mock(return_value=fake_auth) + + region = c.get_one('_test-cloud_') + kr_mock.get_password.assert_called_with( + 'openstacksdk', region._auth.get_cache_id()) + ks_mock.assert_called_with(fake_auth) + + @mock.patch('openstack.config.cloud_region.keyring') + def test_set_auth_cache(self, kr_mock): + c = config.OpenStackConfig( + config_files=[self.cloud_yaml], secure_files=[]) + c._cache_auth = True + + kr_mock.get_password = mock.Mock(side_effect=[RuntimeError]) + kr_mock.set_password = mock.Mock() + + region = c.get_one('_test-cloud_') + + region.set_auth_cache() + kr_mock.set_password.assert_called_with( + 'openstacksdk', region._auth.get_cache_id(), + region._auth.get_auth_state()) + class TestExcludedFormattedConfigValue(base.TestCase): # verify https://storyboard.openstack.org/#!/story/1635696 diff --git a/releasenotes/notes/cache-auth-in-keyring-773dd5f682cd1610.yaml b/releasenotes/notes/cache-auth-in-keyring-773dd5f682cd1610.yaml new file mode 100644 index 000000000..8e35c048a --- /dev/null +++ b/releasenotes/notes/cache-auth-in-keyring-773dd5f682cd1610.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Added support for optionally caching auth information int the local + keyring. Requires the installation of the python ``keyring`` package.