diff --git a/mistral/actions/openstack/actions.py b/mistral/actions/openstack/actions.py index 85ff0d81..1b5c07fd 100644 --- a/mistral/actions/openstack/actions.py +++ b/mistral/actions/openstack/actions.py @@ -52,7 +52,7 @@ CONF = cfg.CONF class NovaAction(base.OpenStackAction): - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Nova action security context: %s" % ctx) @@ -90,7 +90,7 @@ class GlanceAction(base.OpenStackAction): def _get_client_class(cls): return glanceclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Glance action security context: %s" % ctx) @@ -114,7 +114,7 @@ class KeystoneAction(base.OpenStackAction): def _get_client_class(cls): return keystoneclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Keystone action security context: %s" % ctx) @@ -160,7 +160,7 @@ class CeilometerAction(base.OpenStackAction): def _get_client_class(cls): return ceilometerclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Ceilometer action security context: %s" % ctx) @@ -192,7 +192,7 @@ class HeatAction(base.OpenStackAction): def _get_client_class(cls): return heatclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Heat action security context: %s" % ctx) @@ -225,7 +225,7 @@ class NeutronAction(base.OpenStackAction): def _get_client_class(cls): return neutronclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Neutron action security context: %s" % ctx) @@ -246,7 +246,7 @@ class CinderAction(base.OpenStackAction): def _get_client_class(cls): return cinderclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Cinder action security context: %s" % ctx) @@ -287,7 +287,7 @@ class MistralAction(base.OpenStackAction): def _get_client_class(cls): return mistralclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Mistral action security context: %s" % ctx) @@ -324,7 +324,7 @@ class TroveAction(base.OpenStackAction): def _get_client_class(cls): return troveclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Trove action security context: %s" % ctx) @@ -362,7 +362,7 @@ class IronicAction(base.OpenStackAction): def _get_client_class(cls): return ironicclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Ironic action security context: %s" % ctx) @@ -408,7 +408,7 @@ class BaremetalIntrospectionAction(base.OpenStackAction): return cls._get_client_class()() - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Baremetal introspection action security context: %s" % ctx) @@ -430,7 +430,7 @@ class SwiftAction(base.OpenStackAction): def _get_client_class(cls): return swift_client.Connection - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Swift action security context: %s" % ctx) @@ -451,7 +451,7 @@ class ZaqarAction(base.OpenStackAction): def _get_client_class(cls): return zaqarclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Zaqar action security context: %s" % ctx) @@ -547,7 +547,7 @@ class BarbicanAction(base.OpenStackAction): def _get_client_class(cls): return barbicanclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Barbican action security context: %s" % ctx) @@ -649,7 +649,7 @@ class DesignateAction(base.OpenStackAction): def _get_client_class(cls): return designateclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Designate action security context: %s" % ctx) @@ -687,7 +687,7 @@ class MagnumAction(base.OpenStackAction): def _get_client_class(cls): return magnumclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Magnum action security context: %s" % ctx) @@ -715,7 +715,7 @@ class MuranoAction(base.OpenStackAction): def _get_client_class(cls): return muranoclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Murano action security context: %s" % ctx) @@ -742,7 +742,7 @@ class TackerAction(base.OpenStackAction): def _get_client_class(cls): return tackerclient.Client - def _get_client(self): + def _create_client(self): ctx = context.ctx() LOG.debug("Tacker action security context: %s" % ctx) diff --git a/mistral/actions/openstack/base.py b/mistral/actions/openstack/base.py index 76d6604b..12b7f55d 100644 --- a/mistral/actions/openstack/base.py +++ b/mistral/actions/openstack/base.py @@ -16,10 +16,16 @@ import abc import inspect import traceback +from cachetools import LRUCache + from oslo_log import log from mistral.actions import base +from mistral import context from mistral import exceptions as exc +from mistral.utils.openstack import keystone as keystone_utils + +from threading import Lock LOG = log.getLogger(__name__) @@ -32,18 +38,15 @@ class OpenStackAction(base.Action): """ _kwargs_for_run = {} client_method_name = None + _clients = LRUCache(100) + _lock = Lock() def __init__(self, **kwargs): self._kwargs_for_run = kwargs @abc.abstractmethod - def _get_client(self): - """Returns python-client instance - - Gets client instance according to specific OpenStack Service - (e.g. Nova, Glance, Heat, Keystone etc) - - """ + def _create_client(self): + """Creates client required for action operation""" pass @classmethod @@ -74,6 +77,42 @@ class OpenStackAction(base.Action): def get_fake_client_method(cls): return cls._get_client_method(cls._get_fake_client()) + def _get_client(self): + """Returns python-client instance via cache or creation + + Gets client instance according to specific OpenStack Service + (e.g. Nova, Glance, Heat, Keystone etc) + + """ + ctx = context.ctx() + client_class = self.__class__.__name__ + # Colon character is reserved (rfc3986) which avoids key collisions. + key = client_class + ':' + ctx.project_name + ':' + ctx.project_id + + def create_cached_client(): + new_client = self._create_client() + new_client._mistral_ctx_expires_at = ctx.expires_at + + with self._lock: + self._clients[key] = new_client + + return new_client + + with self._lock: + client = self._clients.get(key) + + if client is None: + return create_cached_client() + + if keystone_utils.will_expire_soon(client._mistral_ctx_expires_at): + LOG.debug("cache expiring soon, will refresh client") + + return create_cached_client() + + LOG.debug("cache not expiring soon, will return cached client") + + return client + def run(self): try: method = self._get_client_method(self._get_client()) diff --git a/mistral/config.py b/mistral/config.py index 17ecc9e5..013c9bb0 100644 --- a/mistral/config.py +++ b/mistral/config.py @@ -104,6 +104,13 @@ os_endpoint_type = cfg.StrOpt( ' communication with OpenStack services.' ) +expiration_token_duration = cfg.IntOpt( + 'expiration_token_duration', + default=30, + help='Window of seconds to determine whether the given token is about' + ' to expire.' +) + pecan_opts = [ cfg.StrOpt( 'root', @@ -267,6 +274,7 @@ CONF.register_opt(rpc_impl_opt) CONF.register_opt(rpc_response_timeout_opt) CONF.register_opts(keycloak_oidc_opts, group=KEYCLOAK_OIDC_GROUP) CONF.register_opt(os_endpoint_type) +CONF.register_opt(expiration_token_duration) CLI_OPTS = [ use_debugger_opt, @@ -276,7 +284,7 @@ CLI_OPTS = [ default_group_opts = itertools.chain( CLI_OPTS, [wf_trace_log_name_opt, auth_type_opt, rpc_impl_opt, os_endpoint_type, - rpc_response_timeout_opt] + rpc_response_timeout_opt, expiration_token_duration] ) CONF.register_cli_opts(CLI_OPTS) diff --git a/mistral/context.py b/mistral/context.py index 999a6f5b..f5b337c6 100644 --- a/mistral/context.py +++ b/mistral/context.py @@ -85,7 +85,8 @@ class MistralContext(BaseContext): "roles", "is_admin", "is_trust_scoped", - "redelivered" + "redelivered", + "expires_at", ]) def __repr__(self): @@ -127,7 +128,7 @@ def spawn(thread_description, func, *args, **kwargs): None, func, *args, **kwargs) -def context_from_headers(headers): +def context_from_headers_and_env(headers, env): params = _extract_auth_params_from_headers(headers) auth_cacert = params['auth_cacert'] auth_token = params['auth_token'] @@ -147,6 +148,7 @@ def context_from_headers(headers): project_name=headers.get('X-Project-Name'), roles=headers.get('X-Roles', "").split(","), is_trust_scoped=False, + expires_at=env.get('keystone.token_info')['token']['expires_at'], ) @@ -319,7 +321,10 @@ def authenticate_with_keycloak(req): class ContextHook(hooks.PecanHook): def before(self, state): - set_ctx(context_from_headers(state.request.headers)) + set_ctx(context_from_headers_and_env( + state.request.headers, + state.request.environ + )) def after(self, state): set_ctx(None) diff --git a/mistral/tests/unit/actions/openstack/test_openstack_actions.py b/mistral/tests/unit/actions/openstack/test_openstack_actions.py index a11084db..e4e1600b 100644 --- a/mistral/tests/unit/actions/openstack/test_openstack_actions.py +++ b/mistral/tests/unit/actions/openstack/test_openstack_actions.py @@ -55,7 +55,9 @@ class OpenStackActionTest(base.BaseTestCase): project_id='1234', project_name='admin', auth_token=None, - is_admin=False + is_admin=False, + # set year to 3016 in order for token to always be valid + expires_at='3016-07-13T18:34:22.000000Z' ) ctx.set_ctx(test_ctx) @@ -117,6 +119,37 @@ class OpenStackActionTest(base.BaseTestCase): mock_novaclient.Client().servers.get.assert_called_once_with( server="1234-abcd") + # Repeat test in order to validate cache. + mock_novaclient.reset_mock() + action.run() + + mock_novaclient.Client.assert_not_called() + mock_novaclient.Client().servers.get.assert_called_with( + server="1234-abcd") + + # Repeat again with different context for cache testing. + test_ctx.project_name = 'service' + test_ctx.project_id = '1235' + ctx.set_ctx(test_ctx) + + mock_novaclient.reset_mock() + action.run() + mock_novaclient.Client.assert_called_once_with( + 2, + username=None, + api_key=None, + endpoint_type='publicURL', + service_type='compute', + auth_token=test_ctx.auth_token, + tenant_id=test_ctx.project_id, + region_name=mock_ks_endpoint_v2().region, + auth_url=mock_ks_endpoint_v2().url + ) + + self.assertTrue(mock_novaclient.Client().servers.get.called) + mock_novaclient.Client().servers.get.assert_called_once_with( + server="1234-abcd") + @mock.patch.object(actions.GlanceAction, '_get_client') def test_glance_action(self, mocked): method_name = "images.delete" diff --git a/mistral/tests/unit/api/base.py b/mistral/tests/unit/api/base.py index 04c703f1..e9ba8bc1 100644 --- a/mistral/tests/unit/api/base.py +++ b/mistral/tests/unit/api/base.py @@ -57,7 +57,9 @@ class APITest(base.DbTestCase): self.addCleanup(periodic.stop_all_periodic_tasks) # Make sure the api get the correct context. - self.patch_ctx = mock.patch('mistral.context.context_from_headers') + self.patch_ctx = mock.patch( + 'mistral.context.context_from_headers_and_env' + ) self.mock_ctx = self.patch_ctx.start() self.mock_ctx.return_value = self.ctx self.addCleanup(self.patch_ctx.stop) diff --git a/mistral/tests/unit/api/v2/test_keycloak_auth.py b/mistral/tests/unit/api/v2/test_keycloak_auth.py index e099f3a6..6edbb5f2 100644 --- a/mistral/tests/unit/api/v2/test_keycloak_auth.py +++ b/mistral/tests/unit/api/v2/test_keycloak_auth.py @@ -117,7 +117,9 @@ class TestKeyCloakOIDCAuth(base.DbTestCase): self.addCleanup(periodic.stop_all_periodic_tasks) # Make sure the api get the correct context. - self.patch_ctx = mock.patch('mistral.context.context_from_headers') + self.patch_ctx = mock.patch( + 'mistral.context.context_from_headers_and_env' + ) self.mock_ctx = self.patch_ctx.start() self.mock_ctx.return_value = self.ctx self.addCleanup(self.patch_ctx.stop) diff --git a/mistral/utils/openstack/keystone.py b/mistral/utils/openstack/keystone.py index c8057bf5..45466aba 100644 --- a/mistral/utils/openstack/keystone.py +++ b/mistral/utils/openstack/keystone.py @@ -18,6 +18,7 @@ from keystoneauth1 import session as ks_session from keystoneclient.v3 import client as ks_client from keystoneclient.v3 import endpoints as enp from oslo_config import cfg +from oslo_utils import timeutils from mistral import context @@ -148,3 +149,10 @@ def get_admin_session(): project_domain_name='Default') return ks_session.Session(auth=auth) + + +def will_expire_soon(expires_at): + stale_duration = CONF.expiration_token_duration + assert stale_duration, "expiration_token_duration must be specified" + expires = timeutils.parse_isotime(expires_at) + return timeutils.is_soon(expires, stale_duration)