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

@@ -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
 ----

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.
 

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
+            # "if config_files" is not sufficient to process empty list
-            self._secure_files = secure_files or SECURE_FILES
+            if config_files is not None:
-            self._vendor_files = vendor_files or VENDOR_FILES
+                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,

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

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
 

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

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.