Merge "Add client caching for OpenStack actions"
This commit is contained in:
commit
2f6d1e6c72
@ -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)
|
||||
|
@ -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())
|
||||
|
@ -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)
|
||||
|
@ -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)
|
||||
|
@ -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"
|
||||
|
@ -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)
|
||||
|
@ -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)
|
||||
|
@ -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)
|
||||
|
Loading…
Reference in New Issue
Block a user