Merge "Populate existing policy tests"
This commit is contained in:
Zuul
Gerrit Code Review
commit
b22429fa8b
@ -32,6 +32,7 @@ primarily for developers.
|
||||
Developing New Notifications <notifications>
|
||||
OSProfiler Tracing <osprofiler-support>
|
||||
Rolling Upgrades <rolling-upgrades>
|
||||
Role Based Access Control Testing <rbac-testing>
|
||||
|
||||
These pages contain information for PTLs, cross-project liaisons, and core
|
||||
reviewers.
|
||||
|
||||
119
doc/source/contributor/rbac-testing.rst
Normal file
119
doc/source/contributor/rbac-testing.rst
Normal file
@ -0,0 +1,119 @@
|
||||
==================================
|
||||
Role Based Acces Control - Testing
|
||||
==================================
|
||||
|
||||
.. todo: This entire file is being added in to provide context for
|
||||
reviewers so we can keep in-line comments to the necessary points
|
||||
in the yaml files. It *IS* written with a forward awareness of the
|
||||
later patches, but it is also broad in nature attempting to provide
|
||||
context to aid in review.
|
||||
|
||||
The Role Based Access control testing is a minor departure from the ironic
|
||||
standard pattern of entirely python based unit testing. In part this was done
|
||||
for purposes of speed and to keep the declaration of the test context.
|
||||
|
||||
This also lended itself to be very useful due to the nature of A/B testing
|
||||
which is requried to properly migrate the Ironic project from a project
|
||||
scoped universe where an ``admin project`` is utilized as the authenticating
|
||||
factor coupled with two custom roles, ``baremetal_admin``, and
|
||||
``baremetal_observer``.
|
||||
|
||||
As a contributor looking back after getting a over a thousand additional tests
|
||||
in place using this method, it definitely helped the speed at which these
|
||||
were created, and then ported to support additional.
|
||||
|
||||
How these tests work
|
||||
====================
|
||||
|
||||
These tests execute API calls through the API layer, using the appropriate
|
||||
verb and header, which settings to prevent the ``keystonemiddleware`` from
|
||||
intercepting and replacing the headers we're passing. Ultimately this is a
|
||||
feature, and it helps quite a bit.
|
||||
|
||||
The second aspect of how this works is we're mocking the conductor RPC
|
||||
``get_topic_for`` and ``get_random_topic_for`` methods. These calls raise
|
||||
Temporary Unavailable, since trying to execute the entire interaction into
|
||||
the conductor is moderately pointless because all policy enforement is
|
||||
located with-in the API layer.
|
||||
|
||||
At the same time wiring everything up to go from API to conductor code and
|
||||
back would have been a heavier lift. As such, the tests largely look for
|
||||
one of the following error codes.
|
||||
|
||||
* 200 - Got the item from the API - This is an database driven interaction.
|
||||
* 201 - Created - This is databaes driven interaction. These are rare.
|
||||
* 204 - Accepted - This is a database driven interaction. These are rare.
|
||||
* 403 - Forbidden - This tells us the policy worked as expected where
|
||||
access was denied.
|
||||
* 404 - NotFound - This is typically when objects were not found. Before
|
||||
ironic becomes scope aware, these are generally only in the drivers
|
||||
API endpoint's behavior. In System scope aware Project scoped
|
||||
configuration, i.e. later RBAC tests, this will become the dominant
|
||||
response for project scoped users as responding with a 403 if they
|
||||
could be an owner or lessee would provide insight into the existence
|
||||
of a node.
|
||||
* 503 - Service Unavailable - In the context of our tests, we expect this
|
||||
when a request *has* been successfully authenticated and would have
|
||||
been sent along to the conductor.
|
||||
|
||||
How to make changes or review these tests?
|
||||
==========================================
|
||||
|
||||
The tests cycle through the various endpoints, and repeating patterns
|
||||
are clearly visible. Typically this means a given endpoint is cycled
|
||||
through with the same basic test using slightly different parameters
|
||||
such as different authentication parameters. When it comes to system
|
||||
scope aware tests supporting node ``owners`` and ``lessee``, these
|
||||
tests will cycle a little more with slightly different attributes
|
||||
as the operation is not general against a shared common node, but
|
||||
different nodes.
|
||||
|
||||
Some tests will test body contents, or attributes. some will validate
|
||||
the number of records returned. This is important later with ``owner``
|
||||
and ``lessee`` having slightly different views of the universe.
|
||||
|
||||
Some general rules apply
|
||||
|
||||
* Admins can do things
|
||||
* Members can do some things, but not everything
|
||||
* Readers can always read, but as we get into sensitive data later on
|
||||
such as fields containing infrastucture internal addresses, these values
|
||||
will become hidden and additional tests will examine this.
|
||||
* Third party, or external/other Admins will find nothing but sadness
|
||||
in empty lists, 403, 404, or even 500 errors.
|
||||
|
||||
What is/will be tested?
|
||||
=======================
|
||||
|
||||
The idea is to in essence test as much as possible, however as these
|
||||
tests Role Based Access Control related capabilities will come in a
|
||||
series of phases, styles vary a little.
|
||||
|
||||
The first phase is ``"legacy"``. In essence these are partially
|
||||
programatically generated and then human reviewed and values populated
|
||||
with expected values.
|
||||
|
||||
The second phase is remarkably similar to ``legacy``. It is the safety net
|
||||
where we execute the ``legacy`` tests with the updated ``oslo.policy``
|
||||
configuration to help enforce scopes. These tests will intentionally begin to
|
||||
fail in phase three.
|
||||
|
||||
The third phase is the implementation of System scope awareness for the
|
||||
API. In this process, as various portions of the API are made system scope
|
||||
aware. The ``legacy`` tests are marked as ``deprecated`` which signals to
|
||||
the second phase test sequences that they are **expected** to fail. New
|
||||
``system scoped`` tests are also implemented which are matched up by name
|
||||
to the ``legacy`` tests. The major difference being some header values,
|
||||
and a user with a ``member`` role in the ``system`` scope now has some
|
||||
rights.
|
||||
|
||||
The forth phase, is implementaiton of ``owner`` and ``lessee`` aware
|
||||
project scoping. The testing approach is similar, however it is much more of
|
||||
a "shotgun" approach. We test what we know should work, and what know should
|
||||
not work, but we do not have redundant testing for each role as ``admin``
|
||||
users are also ``members``, and since the policy rules are designed around
|
||||
thresholds of access, it just made no sense to run the same test for admin
|
||||
and members, where member was the threshold. These thresholds will vary with
|
||||
the proposed default policy. The forth scope also tests a third party external
|
||||
admin as a negative test to ensure that we are also denying access to
|
||||
resources appropriately.
|
||||
@ -112,7 +112,7 @@ class BaseApiTest(db_base.DbTestCase):
|
||||
return response
|
||||
|
||||
def put_json(self, path, params, expect_errors=False, headers=None,
|
||||
extra_environ=None, status=None):
|
||||
extra_environ=None, status=None, path_prefix=PATH_PREFIX):
|
||||
"""Sends simulated HTTP PUT request to Pecan test app.
|
||||
|
||||
:param path: url path of target service
|
||||
@ -127,10 +127,11 @@ class BaseApiTest(db_base.DbTestCase):
|
||||
return self._request_json(path=path, params=params,
|
||||
expect_errors=expect_errors,
|
||||
headers=headers, extra_environ=extra_environ,
|
||||
status=status, method="put")
|
||||
status=status, method="put",
|
||||
path_prefix=path_prefix)
|
||||
|
||||
def post_json(self, path, params, expect_errors=False, headers=None,
|
||||
extra_environ=None, status=None):
|
||||
extra_environ=None, status=None, path_prefix=PATH_PREFIX):
|
||||
"""Sends simulated HTTP POST request to Pecan test app.
|
||||
|
||||
:param path: url path of target service
|
||||
@ -145,10 +146,11 @@ class BaseApiTest(db_base.DbTestCase):
|
||||
return self._request_json(path=path, params=params,
|
||||
expect_errors=expect_errors,
|
||||
headers=headers, extra_environ=extra_environ,
|
||||
status=status, method="post")
|
||||
status=status, method="post",
|
||||
path_prefix=path_prefix)
|
||||
|
||||
def patch_json(self, path, params, expect_errors=False, headers=None,
|
||||
extra_environ=None, status=None):
|
||||
extra_environ=None, status=None, path_prefix=PATH_PREFIX):
|
||||
"""Sends simulated HTTP PATCH request to Pecan test app.
|
||||
|
||||
:param path: url path of target service
|
||||
@ -163,7 +165,8 @@ class BaseApiTest(db_base.DbTestCase):
|
||||
return self._request_json(path=path, params=params,
|
||||
expect_errors=expect_errors,
|
||||
headers=headers, extra_environ=extra_environ,
|
||||
status=status, method="patch")
|
||||
status=status, method="patch",
|
||||
path_prefix=path_prefix)
|
||||
|
||||
def delete(self, path, expect_errors=False, headers=None,
|
||||
extra_environ=None, status=None, path_prefix=PATH_PREFIX):
|
||||
|
||||
@ -23,6 +23,9 @@ import ddt
|
||||
from keystonemiddleware import auth_token
|
||||
from oslo_config import cfg
|
||||
|
||||
from ironic.api.controllers.v1 import versions as api_versions
|
||||
from ironic.common import exception
|
||||
from ironic.conductor import rpcapi
|
||||
from ironic.tests.unit.api import base
|
||||
from ironic.tests.unit.db import utils as db_utils
|
||||
|
||||
@ -42,6 +45,17 @@ class TestACLBase(base.BaseApiTest):
|
||||
self.mock_auth = mock_auth.start()
|
||||
self.addCleanup(mock_auth.stop)
|
||||
|
||||
topic = mock.patch.object(
|
||||
rpcapi.ConductorAPI, 'get_topic_for', autospec=True)
|
||||
self.mock_topic = topic.start()
|
||||
self.mock_topic.side_effect = exception.TemporaryFailure
|
||||
self.addCleanup(topic.stop)
|
||||
rtopic = mock.patch.object(rpcapi.ConductorAPI, 'get_random_topic',
|
||||
autospec=True)
|
||||
self.mock_random_topic = rtopic.start()
|
||||
self.mock_random_topic.side_effect = exception.TemporaryFailure
|
||||
self.addCleanup(rtopic.stop)
|
||||
|
||||
def _make_app(self):
|
||||
cfg.CONF.set_override('auth_strategy', 'keystone')
|
||||
return super(TestACLBase, self)._make_app()
|
||||
@ -53,23 +67,70 @@ class TestACLBase(base.BaseApiTest):
|
||||
def _check_skip(self, **kwargs):
|
||||
if kwargs.get('skip_reason'):
|
||||
self.skipTest(kwargs.get('skip_reason'))
|
||||
# Remove ASAP, but as a few hundred tests use this, we can
|
||||
# rip it out later.
|
||||
if kwargs.get('skip'):
|
||||
self.skipTest(kwargs.get('skip_reason', 'Not implemented'))
|
||||
|
||||
def _fake_process_request(self, request, auth_token_request):
|
||||
pass
|
||||
|
||||
def _test_request(self, path, params=None, headers=None, method='get',
|
||||
assert_status=None, assert_dict_contains=None):
|
||||
body=None, assert_status=None,
|
||||
assert_dict_contains=None,
|
||||
assert_list_length=None):
|
||||
path = path.format(**self.format_data)
|
||||
self.mock_auth.side_effect = self._fake_process_request
|
||||
|
||||
# always request the latest api version
|
||||
version = api_versions.max_version_string()
|
||||
rheaders = {
|
||||
'X-OpenStack-Ironic-API-Version': version
|
||||
}
|
||||
# NOTE(TheJulia): Logging the test request to aid
|
||||
# in troubleshooting ACL testing. This is a pattern
|
||||
# followed in API unit testing in ironic, and
|
||||
# really does help.
|
||||
print('API ACL Testing Path %s %s' % (method, path))
|
||||
if headers:
|
||||
for k, v in headers.items():
|
||||
rheaders[k] = v.format(**self.format_data)
|
||||
|
||||
if method == 'get':
|
||||
response = self.get_json(
|
||||
path,
|
||||
headers=headers,
|
||||
headers=rheaders,
|
||||
expect_errors=True,
|
||||
extra_environ=self.environ,
|
||||
path_prefix=''
|
||||
)
|
||||
elif method == 'put':
|
||||
response = self.put_json(
|
||||
path,
|
||||
headers=rheaders,
|
||||
expect_errors=True,
|
||||
extra_environ=self.environ,
|
||||
path_prefix='',
|
||||
params=body
|
||||
)
|
||||
elif method == 'post':
|
||||
response = self.post_json(
|
||||
path,
|
||||
headers=rheaders,
|
||||
expect_errors=True,
|
||||
extra_environ=self.environ,
|
||||
path_prefix='',
|
||||
params=body
|
||||
)
|
||||
elif method == 'patch':
|
||||
response = self.patch_json(
|
||||
path,
|
||||
params=body,
|
||||
headers=rheaders,
|
||||
expect_errors=True,
|
||||
extra_environ=self.environ,
|
||||
path_prefix=''
|
||||
)
|
||||
elif method == 'delete':
|
||||
response = self.delete(
|
||||
path,
|
||||
headers=rheaders,
|
||||
expect_errors=True,
|
||||
extra_environ=self.environ,
|
||||
path_prefix=''
|
||||
@ -77,12 +138,10 @@ class TestACLBase(base.BaseApiTest):
|
||||
else:
|
||||
assert False, 'Unimplemented test method: %s' % method
|
||||
|
||||
other_asserts = bool(assert_dict_contains)
|
||||
|
||||
if assert_status:
|
||||
self.assertEqual(assert_status, response.status_int)
|
||||
else:
|
||||
self.assertIsNotNone(other_asserts,
|
||||
self.assertIsNotNone(assert_status,
|
||||
'Tests must include an assert_status')
|
||||
|
||||
if assert_dict_contains:
|
||||
@ -91,6 +150,22 @@ class TestACLBase(base.BaseApiTest):
|
||||
self.assertEqual(v.format(**self.format_data),
|
||||
response.json[k])
|
||||
|
||||
if assert_list_length:
|
||||
for root, length in assert_list_length.items():
|
||||
# root - object to look inside
|
||||
# length - number of expected elements which will be
|
||||
# important for owner/lessee testing.
|
||||
items = response.json[root]
|
||||
self.assertIsInstance(items, list)
|
||||
self.assertEqual(length, len(items))
|
||||
|
||||
# NOTE(TheJulia): API tests in Ironic tend to have a pattern
|
||||
# to print request and response data to aid in development
|
||||
# and troubleshooting. As such the prints should remain,
|
||||
# at least until we are through primary development of the
|
||||
# this test suite.
|
||||
print('ACL Test GOT %s' % response)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class TestRBACBasic(TestACLBase):
|
||||
@ -110,8 +185,61 @@ class TestRBACBasic(TestACLBase):
|
||||
class TestRBACModelBeforeScopes(TestACLBase):
|
||||
|
||||
def _create_test_data(self):
|
||||
fake_db_node = db_utils.create_test_node(chassis_id=None)
|
||||
self.format_data['node_ident'] = fake_db_node['uuid']
|
||||
allocated_node_id = 31
|
||||
fake_db_allocation = db_utils.create_test_allocation(
|
||||
node_id=allocated_node_id,
|
||||
resource_class="CUSTOM_TEST")
|
||||
fake_db_node = db_utils.create_test_node(
|
||||
chassis_id=None,
|
||||
driver='fake-driverz')
|
||||
fake_db_node_alloced = db_utils.create_test_node(
|
||||
id=allocated_node_id,
|
||||
chassis_id=None,
|
||||
allocation_id=fake_db_allocation['id'],
|
||||
uuid='22e26c0b-03f2-4d2e-ae87-c02d7f33c000',
|
||||
driver='fake-driverz')
|
||||
fake_vif_port_id = "ee21d58f-5de2-4956-85ff-33935ea1ca00"
|
||||
fake_db_port = db_utils.create_test_port(
|
||||
node_id=fake_db_node['id'],
|
||||
internal_info={'tenant_vif_port_id': fake_vif_port_id})
|
||||
fake_db_portgroup = db_utils.create_test_portgroup(
|
||||
node_id=fake_db_node['id'])
|
||||
fake_db_chassis = db_utils.create_test_chassis()
|
||||
fake_db_deploy_template = db_utils.create_test_deploy_template()
|
||||
fake_db_conductor = db_utils.create_test_conductor()
|
||||
fake_db_volume_target = db_utils.create_test_volume_target(
|
||||
node_id=fake_db_allocation['id'])
|
||||
fake_db_volume_connector = db_utils.create_test_volume_connector(
|
||||
node_id=fake_db_allocation['id'])
|
||||
# Trait name aligns with create_test_node_trait.
|
||||
fake_trait = 'trait'
|
||||
fake_setting = 'FAKE_SETTING'
|
||||
db_utils.create_test_bios_setting(
|
||||
node_id=fake_db_node['id'],
|
||||
name=fake_setting,
|
||||
value=fake_setting)
|
||||
db_utils.create_test_node_trait(
|
||||
node_id=fake_db_node['id'])
|
||||
|
||||
self.format_data.update({
|
||||
'node_ident': fake_db_node['uuid'],
|
||||
'allocated_node_ident': fake_db_node_alloced['uuid'],
|
||||
'port_ident': fake_db_port['uuid'],
|
||||
'portgroup_ident': fake_db_portgroup['uuid'],
|
||||
'chassis_ident': fake_db_chassis['uuid'],
|
||||
'deploy_template_ident': fake_db_deploy_template['uuid'],
|
||||
'allocation_ident': fake_db_allocation['uuid'],
|
||||
'conductor_ident': fake_db_conductor['hostname'],
|
||||
'vif_ident': fake_vif_port_id,
|
||||
# Can't use the same fake-driver as other tests can
|
||||
# pollute a global method cache in the API that is in the
|
||||
# test runner, resulting in false positives.
|
||||
'driver_name': 'fake-driverz',
|
||||
'bios_setting': fake_setting,
|
||||
'trait': fake_trait,
|
||||
'volume_target_ident': fake_db_volume_target['uuid'],
|
||||
'volume_connector_ident': fake_db_volume_connector['uuid'],
|
||||
})
|
||||
|
||||
@ddt.file_data('test_rbac_legacy.yaml')
|
||||
@ddt.unpack
|
||||
|
||||
@ -19,6 +19,7 @@ project_admin_can_get_node:
|
||||
assert_dict_contains:
|
||||
uuid: '{node_uuid}'
|
||||
driver: 'fake-hardware'
|
||||
assert_status: 200
|
||||
|
||||
project_member_cannot_get_node:
|
||||
path: *node_path
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Loading…
x
Reference in New Issue
Block a user