openstack
/
mistral
Code Issues Proposed changes

Refactor mistral context using oslo_context 

Also, using oslo.context coordinated with oslo.log will provide more
information in the log.

See more information here:
http://openstack.markmail.org/thread/kuvzwhtblwhoz6o5

Co-Authored-By: Lingxian Kong <anlin.kong@gmail.com>
Change-Id: I1f3f4c1546a85129af1bd58ead9132780909f0d3
This commit is contained in:
Lingxian Kong 2017-05-31 16:25:02 +12:00
parent f661fc7c6a
commit 14978c2352
18 changed files with 147 additions and 170 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@ -7,6 +7,7 @@
# Packages
*.egg
*.egg-info
.eggs/
dist
build
.venv

10
mistral/api/access_control.py
View File

@ -70,14 +70,20 @@ def enforce(action, context, target=None, do_raise=True,
'project_id': context.project_id,
'user_id': context.user_id,
}
target_obj.update(target or {})
policy_context = context.to_policy_values()
# Because policy.json example in Mistral repo still uses the rule
# 'is_admin: True', we insert 'is_admin' key to the default policy
# values.
policy_context['is_admin'] = context.is_admin
_ensure_enforcer_initialization()
return _ENFORCER.enforce(
action,
target_obj,
context.to_dict(),
policy_context,
do_raise=do_raise,
exc=exc
)

204
mistral/context.py
View File

@ -15,8 +15,8 @@
import base64
from keystoneclient.v3 import client as keystone_client
from oslo_config import cfg
from oslo_context import context as oslo_context
import oslo_messaging as messaging
from oslo_serialization import jsonutils
from osprofiler import profiler
@ -33,63 +33,83 @@ _CTX_THREAD_LOCAL_NAME = "MISTRAL_APP_CTX_THREAD_LOCAL"
ALLOWED_WITHOUT_AUTH = ['/', '/v2/']
class BaseContext(object):
"""Container for context variables."""
class MistralContext(oslo_context.RequestContext):
def __init__(self, auth_uri=None, auth_cacert=None, insecure=False,
service_catalog=None, region_name=None, is_trust_scoped=False,
redelivered=False, expires_at=None, trust_id=None,
is_target=False, **kwargs):
self.auth_uri = auth_uri
self.auth_cacert = auth_cacert
self.insecure = insecure
self.service_catalog = service_catalog
self.region_name = region_name
self.is_trust_scoped = is_trust_scoped
self.redelivered = redelivered
self.expires_at = expires_at
self.trust_id = trust_id
self.is_target = is_target
_elements = set()
# We still use Mistral thread local variable. Maybe could consider
# using the variable provided by oslo_context in future.
super(MistralContext, self).__init__(overwrite=False, **kwargs)
def __init__(self, __mapping=None, **kwargs):
if __mapping is None:
self.__values = dict(**kwargs)
else:
if isinstance(__mapping, BaseContext):
__mapping = __mapping.__values
self.__values = dict(__mapping)
self.__values.update(**kwargs)
def convert_to_dict(self):
"""Return a dictionary of context attributes.
bad_keys = set(self.__values) - self._elements
Use get_logging_values() instead of to_dict() from parent class to get
more information from the context. This method is not named "to_dict"
to avoid recursive call.
"""
ctx_dict = self.get_logging_values()
ctx_dict.update(
{
'auth_uri': self.auth_uri,
'auth_cacert': self.auth_cacert,
'insecure': self.insecure,
'service_catalog': self.service_catalog,
'region_name': self.region_name,
'is_trust_scoped': self.is_trust_scoped,
'redelivered': self.redelivered,
'expires_at': self.expires_at,
'trust_id': self.trust_id,
'is_target': self.is_target,
}
)
if bad_keys:
raise TypeError("Only %s keys are supported. %s given" %
(tuple(self._elements), tuple(bad_keys)))
return ctx_dict
def __getattr__(self, name):
try:
return self.__values[name]
except KeyError:
if name in self._elements:
return None
else:
raise AttributeError(name)
@classmethod
def from_dict(cls, values, **kwargs):
"""Construct a context object from a provided dictionary."""
kwargs.setdefault('auth_uri', values.get('auth_uri'))
kwargs.setdefault('auth_cacert', values.get('auth_cacert'))
kwargs.setdefault('insecure', values.get('insecure', False))
kwargs.setdefault('service_catalog', values.get('service_catalog'))
kwargs.setdefault('region_name', values.get('region_name'))
kwargs.setdefault(
'is_trust_scoped', values.get('is_trust_scoped', False)
)
kwargs.setdefault('redelivered', values.get('redelivered', False))
kwargs.setdefault('expires_at', values.get('expires_at'))
kwargs.setdefault('trust_id', values.get('trust_id'))
kwargs.setdefault('is_target', values.get('is_target', False))
def to_dict(self):
return self.__values
return super(MistralContext, cls).from_dict(values, **kwargs)
@classmethod
def from_environ(cls, headers, env):
kwargs = _extract_mistral_auth_params(headers)
class MistralContext(BaseContext):
# Use set([...]) since set literals are not supported in Python 2.6.
_elements = set([
"auth_uri",
"auth_cacert",
"insecure",
"user_id",
"project_id",
"auth_token",
"service_catalog",
"user_name",
"region_name",
"project_name",
"roles",
"is_admin",
"is_trust_scoped",
"redelivered",
"expires_at",
"trust_id",
"is_target",
])
token_info = env.get('keystone.token_info', {})
if not kwargs['is_target']:
kwargs['service_catalog'] = token_info.get('token', {})
kwargs['expires_at'] = (token_info['token']['expires_at']
if token_info else None)
def __repr__(self):
return "MistralContext %s" % self.to_dict()
context = super(MistralContext, cls).from_environ(env, **kwargs)
context.is_admin = True if 'admin' in context.roles else False
return context
def has_ctx():
@ -107,47 +127,7 @@ def set_ctx(new_ctx):
utils.set_thread_local(_CTX_THREAD_LOCAL_NAME, new_ctx)
def context_from_headers_and_env(headers, env):
params = _extract_auth_params_from_headers(headers)
auth_cacert = params['auth_cacert']
insecure = params['insecure']
auth_token = params['auth_token']
auth_uri = params['auth_uri']
project_id = params['project_id']
region_name = params['region_name']
user_id = params['user_id']
user_name = params['user_name']
is_target = params['is_target']
token_info = env.get('keystone.token_info', {})
service_catalog = (params['service_catalog'] if is_target
else token_info.get('token', {}))
roles = headers.get('X-Roles', "").split(",")
is_admin = True if 'admin' in roles else False
return MistralContext(
auth_uri=auth_uri,
auth_cacert=auth_cacert,
insecure=insecure,
user_id=user_id,
project_id=project_id,
auth_token=auth_token,
is_target=is_target,
service_catalog=service_catalog,
user_name=user_name,
region_name=region_name,
project_name=headers.get('X-Project-Name'),
roles=roles,
is_trust_scoped=False,
expires_at=token_info['token']['expires_at'] if token_info else None,
is_admin=is_admin
)
def _extract_auth_params_from_headers(headers):
def _extract_mistral_auth_params(headers):
service_catalog = None
if headers.get("X-Target-Auth-Uri"):
@ -157,8 +137,8 @@ def _extract_auth_params_from_headers(headers):
'insecure': headers.get('X-Target-Insecure', False),
'auth_token': headers.get('X-Target-Auth-Token'),
'auth_uri': headers.get('X-Target-Auth-Uri'),
'project_id': headers.get('X-Target-Project-Id'),
'user_id': headers.get('X-Target-User-Id'),
'tenant': headers.get('X-Target-Project-Id'),
'user': headers.get('X-Target-User-Id'),
'user_name': headers.get('X-Target-User-Name'),
'region_name': headers.get('X-Target-Region-Name'),
'is_target': True
@ -176,13 +156,9 @@ def _extract_auth_params_from_headers(headers):
)
else:
params = {
'auth_uri': CONF.keystone_authtoken.auth_uri,
'auth_cacert': CONF.keystone_authtoken.cafile,
'insecure': False,
'auth_token': headers.get('X-Auth-Token'),
'auth_uri': CONF.keystone_authtoken.auth_uri,
'project_id': headers.get('X-Project-Id'),
'user_id': headers.get('X-User-Id'),
'user_name': headers.get('X-User-Name'),
'region_name': headers.get('X-Region-Name'),
'is_target': False
}
@ -203,27 +179,6 @@ def _extract_service_catalog_from_headers(headers):
return None
def context_from_config():
keystone = keystone_client.Client(
username=CONF.keystone_authtoken.admin_user,
password=CONF.keystone_authtoken.admin_password,
tenant_name=CONF.keystone_authtoken.admin_tenant_name,
auth_url=CONF.keystone_authtoken.auth_uri,
is_trust_scoped=False,
)
keystone.authenticate()
return MistralContext(
user_id=keystone.user_id,
project_id=keystone.project_id,
auth_token=keystone.auth_token,
project_name=CONF.keystone_authtoken.admin_tenant_name,
user_name=CONF.keystone_authtoken.admin_user,
is_trust_scoped=False,
)
class RpcContextSerializer(messaging.Serializer):
def __init__(self, entity_serializer=None):
self.entity_serializer = (
@ -243,7 +198,7 @@ class RpcContextSerializer(messaging.Serializer):
return self.entity_serializer.deserialize(entity)
def serialize_context(self, context):
ctx = context.to_dict()
ctx = context.convert_to_dict()
pfr = profiler.get()
@ -262,7 +217,7 @@ class RpcContextSerializer(messaging.Serializer):
if trace_info:
profiler.init(**trace_info)
ctx = MistralContext(**context)
ctx = MistralContext.from_dict(context)
set_ctx(ctx)
return ctx
@ -291,10 +246,11 @@ class AuthHook(hooks.PecanHook):
class ContextHook(hooks.PecanHook):
def before(self, state):
set_ctx(context_from_headers_and_env(
state.request.headers,
state.request.environ
))
context = MistralContext.from_environ(
state.request.headers, state.request.environ
)
set_ctx(context)
def after(self, state):
set_ctx(None)

2
mistral/rpc/kombu/kombu_client.py
View File

@ -120,7 +120,7 @@ class KombuRPCClient(rpc_base.RPCClient, kombu_base.Base):
correlation_id = utils.generate_unicode_uuid()
body = {
'rpc_ctx': ctx.to_dict(),
'rpc_ctx': ctx.convert_to_dict(),
'rpc_method': method,
'arguments': self._serialize_message(kwargs),
'async': async_

2
mistral/rpc/kombu/kombu_server.py
View File

@ -182,7 +182,7 @@ class KombuRPCServer(rpc_base.RPCServer, kombu_base.Base):
if not isinstance(ctx, dict):
return
context = auth_ctx.MistralContext(**ctx)
context = auth_ctx.MistralContext.from_dict(ctx)
auth_ctx.set_ctx(context)
return context

8
mistral/services/expiration_policy.py
View File

@ -83,8 +83,8 @@ def _delete(executions):
# TODO(tuan_luong): Manipulation with auth_ctx should be
# out of db transaction scope.
ctx = auth_ctx.MistralContext(
user_id=None,
project_id=execution.project_id,
user=None,
tenant=execution.project_id,
auth_token=None,
is_admin=True
)
@ -126,8 +126,8 @@ def setup():
pt = ExecutionExpirationPolicy(CONF)
ctx = auth_ctx.MistralContext(
user_id=None,
project_id=None,
user=None,
tenant=None,
auth_token=None,
is_admin=True
)

4
mistral/services/periodic.py
View File

@ -119,8 +119,8 @@ def setup():
pt = MistralPeriodicTasks(CONF)
ctx = auth_ctx.MistralContext(
user_id=None,
project_id=None,
user=None,
tenant=None,
auth_token=None,
is_admin=True
)

8
mistral/services/security.py
View File

@ -66,16 +66,16 @@ def create_context(trust_id, project_id):
client = keystone.client_for_trusts(trust_id)
return auth_ctx.MistralContext(
user_id=client.user_id,
project_id=project_id,
user=client.user_id,
tenant=project_id,
auth_token=client.auth_token,
is_trust_scoped=True,
trust_id=trust_id,
)
return auth_ctx.MistralContext(
user_id=None,
project_id=None,
user=None,
tenant=None,
auth_token=None,
is_admin=True
)

4
mistral/tests/unit/actions/openstack/test_openstack_actions.py
View File

@ -48,8 +48,8 @@ class OpenStackActionTest(base.BaseTestCase):
mock_ks_endpoint_v2):
test_ctx = ctx.MistralContext(
user_id=None,
project_id='1234',
user=None,
tenant='1234',
project_name='admin',
auth_token=None,
is_admin=False,

2
mistral/tests/unit/api/base.py
View File

@ -56,7 +56,7 @@ class APITest(base.DbTestCase):
# Make sure the api get the correct context.
self.patch_ctx = mock.patch(
'mistral.context.context_from_headers_and_env'
'mistral.context.MistralContext.from_environ'
)
self.mock_ctx = self.patch_ctx.start()
self.mock_ctx.return_value = self.ctx

4
mistral/tests/unit/api/v2/test_executions.py
View File

@ -638,7 +638,7 @@ class TestExecutionsController(base.APITest):
self.assertIsNone(resource_function)
@mock.patch('mistral.db.v2.api.get_workflow_executions')
@mock.patch('mistral.context.context_from_headers_and_env')
@mock.patch('mistral.context.MistralContext.from_environ')
def test_get_all_projects_admin(self, mock_context, mock_get_execs):
admin_ctx = unit_base.get_context(admin=True)
mock_context.return_value = admin_ctx
@ -658,7 +658,7 @@ class TestExecutionsController(base.APITest):
self.assertEqual(403, resp.status_int)
@mock.patch('mistral.db.v2.api.get_workflow_executions')
@mock.patch('mistral.context.context_from_headers_and_env')
@mock.patch('mistral.context.MistralContext.from_environ')
def test_get_all_filter_by_project_id(self, mock_context, mock_get_execs):
admin_ctx = unit_base.get_context(admin=True)
mock_context.return_value = admin_ctx

2
mistral/tests/unit/api/v2/test_keycloak_auth.py
View File

@ -116,7 +116,7 @@ class TestKeyCloakOIDCAuth(base.DbTestCase):
# Make sure the api get the correct context.
self.patch_ctx = mock.patch(
'mistral.context.context_from_headers_and_env'
'mistral.context.MistralContext.from_environ'
)
self.mock_ctx = self.patch_ctx.start()
self.mock_ctx.return_value = self.ctx

2
mistral/tests/unit/api/v2/test_workflows.py
View File

@ -426,7 +426,7 @@ class TestWorkflowsController(base.APITest):
self.assertEqual(0, len(resp.json['workflows']))
@mock.patch('mistral.db.v2.api.get_workflow_definitions')
@mock.patch('mistral.context.context_from_headers_and_env')
@mock.patch('mistral.context.MistralContext.from_environ')
def test_get_all_projects_admin(self, mock_context, mock_get_wf_defs):
admin_ctx = unit_base.get_context(admin=True)
mock_context.return_value = admin_ctx

29
mistral/tests/unit/base.py
View File

@ -53,21 +53,21 @@ def get_resource(resource_name):
def get_context(default=True, admin=False):
if default:
return auth_context.MistralContext(
user_id='1-2-3-4',
project_id=security.DEFAULT_PROJECT_ID,
user_name='test-user',
project_name='test-project',
is_admin=admin
)
return auth_context.MistralContext.from_dict({
'user_name': 'test-user',
'user': '1-2-3-4',
'tenant': security.DEFAULT_PROJECT_ID,
'project_name': 'test-project',
'is_admin': admin
})
else:
return auth_context.MistralContext(
user_id='9-0-44-5',
project_id='99-88-33',
user_name='test-user',
project_name='test-another',
is_admin=admin
)
return auth_context.MistralContext.from_dict({
'user_name': 'test-user',
'user': '9-0-44-5',
'tenant': '99-88-33',
'project_name': 'test-another',
'is_admin': admin
})
def register_action_class(name, cls, attributes=None, desc=None):
@ -101,7 +101,6 @@ class FakeHTTPResponse(object):
class BaseTest(base.BaseTestCase):
def setUp(self):
super(BaseTest, self).setUp()
self.addCleanup(spec_parser.clear_caches)
def register_action_class(self, name, cls, attributes=None, desc=None):

6
mistral/tests/unit/rpc/kombu/test_kombu_client.py
View File

@ -48,7 +48,11 @@ class KombuClientTestCase(base.KombuTestCase):
self.addCleanup(restore_listener)
self.client = kombu_client.KombuRPCClient(conf)
self.ctx = type('context', (object,), {'to_dict': lambda self: {}})()
self.ctx = type(
'context',
(object,),
{'convert_to_dict': lambda self: {}}
)()
def test_sync_call_result_get(self):
self.client._listener.get_result = mock.MagicMock(

21
mistral/tests/unit/rpc/kombu/test_kombu_server.py
View File

@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
from mistral import context
from mistral import exceptions as exc
from mistral.tests.unit.rpc.kombu import base
from mistral.tests.unit.rpc.kombu import fake_kombu
@ -197,8 +198,8 @@ class KombuServerTestCase(base.KombuTestCase):
@mock.patch.object(kombu_server.KombuRPCServer, 'publish_message')
@mock.patch.object(kombu_server.KombuRPCServer, '_get_rpc_method')
@mock.patch('mistral.context.MistralContext')
def test__on_message_is_async(self, mistral_context, get_rpc_method,
@mock.patch('mistral.context.MistralContext.from_dict')
def test__on_message_is_async(self, mock_get_context, get_rpc_method,
publish_message):
result = 'result'
request = {
@ -221,9 +222,13 @@ class KombuServerTestCase(base.KombuTestCase):
rpc_method = mock.MagicMock(return_value=result)
get_rpc_method.return_value = rpc_method
ctx = context.MistralContext()
mock_get_context.return_value = ctx
self.server._on_message(request, message)
rpc_method.assert_called_once_with(
rpc_ctx=mistral_context(),
rpc_ctx=ctx,
a=1,
b=2
)
@ -231,8 +236,8 @@ class KombuServerTestCase(base.KombuTestCase):
@mock.patch.object(kombu_server.KombuRPCServer, 'publish_message')
@mock.patch.object(kombu_server.KombuRPCServer, '_get_rpc_method')
@mock.patch('mistral.context.MistralContext')
def test__on_message_is_sync(self, mistral_context, get_rpc_method,
@mock.patch('mistral.context.MistralContext.from_dict')
def test__on_message_is_sync(self, mock_get_context, get_rpc_method,
publish_message):
result = 'result'
request = {
@ -257,9 +262,13 @@ class KombuServerTestCase(base.KombuTestCase):
rpc_method = mock.MagicMock(return_value=result)
get_rpc_method.return_value = rpc_method
ctx = context.MistralContext()
mock_get_context.return_value = ctx
self.server._on_message(request, message)
rpc_method.assert_called_once_with(
rpc_ctx=mistral_context(),
rpc_ctx=ctx,
a=1,
b=2
)

7
mistral/tests/unit/services/test_scheduler.py
View File

@ -62,7 +62,7 @@ def compare_context_values(expected, actual):
def target_check_context_method(expected_project_id):
actual_project_id = auth_context.ctx()._BaseContext__values['project_id']
actual_project_id = auth_context.ctx().project_id
compare_context_values(expected_project_id, actual_project_id)
@ -152,8 +152,9 @@ class SchedulerServiceTest(base.DbTestCase):
default_context = base.get_context(default=True)
auth_context.set_ctx(default_context)
default_project_id = (
default_context._BaseContext__values['project_id']
default_context.project_id
)
method_args1 = {'expected_project_id': default_project_id}
scheduler.schedule_call(
@ -166,7 +167,7 @@ class SchedulerServiceTest(base.DbTestCase):
second_context = base.get_context(default=False)
auth_context.set_ctx(second_context)
second_project_id = (
second_context._BaseContext__values['project_id']
second_context.project_id
)
method_args2 = {'expected_project_id': second_project_id}

1
requirements.txt
View File

@ -16,6 +16,7 @@ mistral-lib>=0.2.0 # Apache-2.0
networkx>=1.10 # BSD
oslo.concurrency>=3.8.0 # Apache-2.0
oslo.config!=4.3.0,!=4.4.0,>=4.0.0 # Apache-2.0
oslo.context>=2.14.0 # Apache-2.0
oslo.db>=4.23.0 # Apache-2.0
oslo.i18n!=3.15.2,>=2.1.0 # Apache-2.0
oslo.messaging!=5.25.0,>=5.24.2 # Apache-2.0