openstack/deb-murano
Code Issues Proposed changes

Fixes to unit tests around policies

Browse Source 
* Change to policy check signature to default to an empty target
* Tests for policy checks were very inflexible. Changed now so that
  for each API test policy rules must be explicitly provided, and
  tests must record which, how many, and in what order policy checks
  should be expected
* Test requests look more like actual requests; rather than calling the
  controller functions directly, tests go through the WSGI mapper/router
  infrastructure and all input/output must be valid as far as the wsgi
  is concerned.

Change-Id: I1dbf35249b4efd6ae9c85b08139e3a627fff6fc5
This commit is contained in:
Steve McLellan 2014-07-23 18:03:02 -05:00 committed by Ruslan Kamaldinov
parent d794d08302
commit 87e137e998
5 changed files with 121 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
murano/api/v1/environments.py
View File

@ -37,7 +37,7 @@ class Controller(object):
@request_statistics.stats_count(API_NAME, 'Index')
def index(self, request):
LOG.debug('Environments:List')
policy.check('list_environments', request.context, {})
policy.check('list_environments', request.context)
#Only environments from same tenant as user should be returned
filters = {'tenant_id': request.context.tenant}
@ -49,7 +49,7 @@ class Controller(object):
@request_statistics.stats_count(API_NAME, 'Create')
def create(self, request, body):
LOG.debug('Environments:Create <Body {0}>'.format(body))
policy.check('create_environment', request.context, {})
policy.check('create_environment', request.context)
try:
environment = envs.EnvironmentServices.create(

4
murano/common/policy.py
View File

@ -54,11 +54,11 @@ def set_rules(data, default_rule=None, overwrite=True):
_ENFORCER.set_rules(rules, overwrite=overwrite)
def init(default_rule=None):
def init(default_rule=None, use_conf=True):
global _ENFORCER
if not _ENFORCER:
LOG.debug("Enforcer is not present, recreating.")
_ENFORCER = policy.Enforcer()
_ENFORCER = policy.Enforcer(use_conf=use_conf)
_ENFORCER.load_rules()

81
murano/tests/unit/api/base.py
View File

@ -16,10 +16,13 @@
import fixtures
import logging
import mock
import routes
import urllib
import webob
from murano.api.v1 import request_statistics
from murano.api.v1 import router
from murano.common import policy
from murano.common import rpc
from murano.openstack.common import timeutils
from murano.openstack.common import wsgi
@ -105,13 +108,37 @@ class ControllerTest(object):
self.api_version = '1.0'
self.tenant = 'test_tenant'
self.mock_policy_check = None
self.mapper = routes.Mapper()
self.api = router.API(self.mapper)
request_statistics.init_stats()
def setUp(self):
super(ControllerTest, self).setUp()
self.is_admin = False
policy.init(use_conf=False)
real_policy_check = policy.check
self._policy_check_expectations = []
self._actual_policy_checks = []
def wrap_policy_check(rule, ctxt, target={}, **kwargs):
self._actual_policy_checks.append((rule, target))
return real_policy_check(rule, ctxt, target=target, **kwargs)
mock.patch('murano.common.policy.check',
side_effect=wrap_policy_check).start()
# Deny everything
self._set_policy_rules({"default": "!"})
def _environ(self, path):
return {
'SERVER_NAME': 'server.test',
'SERVER_PORT': 8082,
'SERVER_PORT': '8082',
'SERVER_PROTOCOL': 'http',
'SCRIPT_NAME': '/v1',
'PATH_INFO': path,
'wsgi.url_scheme': 'http',
@ -130,7 +157,9 @@ class ControllerTest(object):
environ['QUERY_STRING'] = qs
req = wsgi.Request(environ)
req.context = utils.dummy_context('api_test_user', self.tenant)
req.context = utils.dummy_context('api_test_user',
self.tenant,
is_admin=self.is_admin)
self.context = req.context
return req
@ -141,46 +170,40 @@ class ControllerTest(object):
return self._simple_request(path, method='DELETE')
def _data_request(self, path, data, content_type='application/json',
method='POST'):
method='POST', params={}):
environ = self._environ(path)
environ['REQUEST_METHOD'] = method
req = wsgi.Request(environ)
req.context = utils.dummy_context('api_test_user', self.tenant)
self.context = req.context
req.content_type = content_type
req.body = data
if params:
qs = urllib.urlencode(params)
environ['QUERY_STRING'] = qs
return req
def _post(self, path, data, content_type='application/json'):
return self._data_request(path, data, content_type)
def _post(self, path, data, content_type='application/json', params={}):
return self._data_request(path, data, content_type, params=params)
def _put(self, path, data, content_type='application/json'):
return self._data_request(path, data, content_type, method='PUT')
def _put(self, path, data, content_type='application/json', params={}):
return self._data_request(path, data, content_type, method='PUT',
params=params)
def _mock_policy_setup(self, mocker, action, allowed=True,
target=None, expected_request_count=1):
if self.mock_policy_check is not None:
# Test existing policy check record
self._check_policy()
self.mock_policy_check.reset_mock()
def _set_policy_rules(self, rules):
policy.set_rules(rules)
self.mock_policy_check = mocker
self.policy_action = action
self.mock_policy_check.return_value = allowed
self.policy_target = target
self.expected_request_count = expected_request_count
def expect_policy_check(self, action, target={}):
self._policy_check_expectations.append((action, target))
def _check_policy(self):
"""Assert policy checks called as expected"""
if self.mock_policy_check:
# Check that policy enforcement got called as expected
self.mock_policy_check.assert_called_with(
self.policy_action,
self.context,
self.policy_target or {})
self.assertEqual(self.expected_request_count,
len(self.mock_policy_check.call_args_list))
def _assert_policy_checks(self):
self.assertEqual(self._policy_check_expectations,
self._actual_policy_checks)
def tearDown(self):
self._check_policy()
self._assert_policy_checks()
policy.reset()
super(ControllerTest, self).tearDown()

103
murano/tests/unit/api/v1/test_environments.py
View File

@ -14,18 +14,14 @@
# limitations under the License.
import json
import mock
from webob import exc
from murano.api.v1 import environments
from murano.common import policy
from murano.db import models
from murano.openstack.common import timeutils
import murano.tests.unit.api.base as tb
import murano.tests.unit.utils as test_utils
@mock.patch.object(policy, 'check')
class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase):
RPC_IMPORT = 'murano.db.services.environments.rpc'
@ -33,17 +29,25 @@ class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase):
super(TestEnvironmentApi, self).setUp()
self.controller = environments.Controller()
def test_list_empty_environments(self, mock_policy_check):
def test_list_empty_environments(self):
"""Check that with no environments an empty list is returned"""
self._mock_policy_setup(mock_policy_check, 'list_environments')
self._set_policy_rules(
{'list_environments': '@'}
)
self.expect_policy_check('list_environments')
req = self._get('/environments')
result = self.controller.index(req)
self.assertEqual({'environments': []}, result)
result = req.get_response(self.api)
self.assertEqual({'environments': []}, json.loads(result.body))
def test_create_environment(self, mock_policy_check):
def test_create_environment(self):
"""Create an environment, test environment.show()"""
self._mock_policy_setup(mock_policy_check, 'create_environment')
self._set_policy_rules(
{'list_environments': '@',
'create_environment': '@',
'show_environment': '@'}
)
self.expect_policy_check('create_environment')
fake_now = timeutils.utcnow()
timeutils.utcnow.override_time = fake_now
@ -56,49 +60,57 @@ class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase):
'name': 'my_env',
'networking': {},
'version': 0,
'created': fake_now,
'updated': fake_now}
# TODO(sjmc7) - bug 1347298
'created': timeutils.isotime(fake_now)[:-1],
'updated': timeutils.isotime(fake_now)[:-1]}
body = {'name': 'my_env'}
req = self._post('/environments', json.dumps(body))
result = self.controller.create(req, body)
self.assertEqual(expected, result)
result = req.get_response(self.api)
self.assertEqual(expected, json.loads(result.body))
expected['status'] = 'ready'
# Reset the policy expectation
self._mock_policy_setup(mock_policy_check, 'list_environments')
self.expect_policy_check('list_environments')
req = self._get('/environments')
result = self.controller.index(req)
self.assertEqual({'environments': [expected]}, result)
result = req.get_response(self.api)
self.assertEqual(200, result.status_code)
self.assertEqual({'environments': [expected]}, json.loads(result.body))
expected['services'] = []
# Reset the policy expectation
self._mock_policy_setup(mock_policy_check, 'show_environment',
target={'environment_id': uuids[-1]})
self.expect_policy_check('show_environment',
{'environment_id': uuids[-1]})
req = self._get('/environments/%s' % uuids[-1])
result = self.controller.show(req, uuids[-1])
result = req.get_response(self.api)
self.assertEqual(expected, result)
self.assertEqual(expected, json.loads(result.body))
self.assertEqual(3, mock_uuid.call_count)
def test_missing_environment(self, mock_policy_check):
def test_missing_environment(self):
"""Check that a missing environment results in an HTTPNotFound"""
self._mock_policy_setup(mock_policy_check, 'show_environment',
target={'environment_id': 'no-such-id'})
self._set_policy_rules(
{'show_environment': '@'}
)
self.expect_policy_check('show_environment',
{'environment_id': 'no-such-id'})
req = self._get('/environments/no-such-id')
self.assertRaises(exc.HTTPNotFound, self.controller.show,
req, 'no-such-id')
result = req.get_response(self.api)
self.assertEqual(404, result.status_code)
def test_update_environment(self, mock_policy_check):
def test_update_environment(self):
"""Check that environment rename works"""
self._mock_policy_setup(mock_policy_check, 'update_environment',
target={'environment_id': '12345'})
self._set_policy_rules(
{'show_environment': '@',
'update_environment': '@'}
)
self.expect_policy_check('update_environment',
{'environment_id': '12345'})
fake_now = timeutils.utcnow()
timeutils.utcnow.override_time = fake_now
@ -133,20 +145,28 @@ class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase):
body = {
'name': 'renamed env'
}
req = self._post('/environments/12345', json.dumps(body))
result = self.controller.update(req, '12345', body)
req = self._put('/environments/12345', json.dumps(body))
result = req.get_response(self.api)
self.assertEqual(200, result.status_code)
self._mock_policy_setup(mock_policy_check, 'show_environment',
target={'environment_id': '12345'})
self.expect_policy_check('show_environment',
{'environment_id': '12345'})
req = self._get('/environments/12345')
result = self.controller.show(req, '12345')
result = req.get_response(self.api)
self.assertEqual(200, result.status_code)
self.assertEqual(expected, result)
expected['created'] = timeutils.isotime(expected['created'])[:-1]
expected['updated'] = timeutils.isotime(expected['updated'])[:-1]
def test_delete_environment(self, mock_policy_check):
self.assertEqual(expected, json.loads(result.body))
def test_delete_environment(self):
"""Test that environment deletion results in the correct rpc call"""
self._mock_policy_setup(mock_policy_check, 'delete_environment',
target={'environment_id': '12345'})
self._set_policy_rules(
{'delete_environment': '@'}
)
self.expect_policy_check('delete_environment',
{'environment_id': '12345'})
fake_now = timeutils.utcnow()
expected = dict(
@ -174,9 +194,10 @@ class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase):
}
req = self._delete('/environments/12345')
result = self.controller.delete(req, '12345')
result = req.get_response(self.api)
self.mock_engine_rpc.handle_task.assert_called_once_with(rpc_task)
# Should this be expected behavior?
self.assertEqual(None, result)
self.assertEqual('', result.body)
self.assertEqual(200, result.status_code)

5
murano/tests/unit/utils.py
View File

@ -30,12 +30,13 @@ def reset_dummy_db():
def dummy_context(user='test_username', tenant_id='test_tenant_id',
password='password', roles=[], user_id=None):
password='password', roles=[], user_id=None,
is_admin=False):
return context.RequestContext.from_dict({
'tenant': tenant_id,
'user': user,
#'roles': roles, # Commented until policy check changes land
'is_admin': False,
'is_admin': is_admin,
})