From bc8705c160234fec1af322ecbb5fe0ce5a0d35b8 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Wed, 10 Aug 2022 15:14:30 -0700 Subject: [PATCH] Allow project scoped admins to create/delete nodes Adds capabilites for a project scoped admin to create and delete nodes in Ironic's API. These nodes are automatically associated with the project of the requestor. Effectively, this does allow anyone with sufficient privilges, i.e. admin, in an OpenStack deployment to be able to create new baremetal nodes and delete those baremetal nodes. In this case, the user has the "owner" level of rights in the RBAC model. Change-Id: I3fd9ce5de0bc600275b5c4b7a95b0f9405342688 --- doc/source/admin/secure-rbac.rst | 13 +++++ .../contributor/webapi-version-history.rst | 7 +++ ironic/api/controllers/v1/node.py | 44 +++++++++++++- ironic/api/controllers/v1/versions.py | 5 +- ironic/common/policy.py | 19 +++++- ironic/common/release_mappings.py | 2 +- ironic/conf/api.py | 5 ++ .../unit/api/controllers/v1/test_node.py | 28 ++++++++- ironic/tests/unit/api/test_acl.py | 10 +++- .../unit/api/test_rbac_project_scoped.yaml | 58 ++++++++++++++++--- ...ger-scope-restricted-b455f66a751f10ec.yaml | 27 +++++++++ 11 files changed, 201 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/node-creation-no-longer-scope-restricted-b455f66a751f10ec.yaml diff --git a/doc/source/admin/secure-rbac.rst b/doc/source/admin/secure-rbac.rst index 639cfcb23f..7721211b69 100644 --- a/doc/source/admin/secure-rbac.rst +++ b/doc/source/admin/secure-rbac.rst @@ -267,3 +267,16 @@ restrictive and an ``owner`` may revoke access to ``lessee``. Access to the underlying baremetal node is not exclusive between the ``owner`` and ``lessee``, and this use model expects that some level of communication takes place between the appropriate parties. + +Can I, a project admin, create a node? +-------------------------------------- + +Starting in API version ``1.80``, the capability was added +to allow users with an ``admin`` role to be able to create and +delete their own nodes in Ironic. + +This functionality is enabled by default, and automatically +imparts ``owner`` privileges to the created Bare Metal node. + +This functionality can be disabled by setting +``[api]project_admin_can_manage_own_nodes`` to ``False``. diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index 58c0598eb6..c395bdcbe1 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,12 @@ REST API Version History ======================== +1.80 (Zed) +---------- + +This verison is a signifier of additional RBAC functionality allowing +a project scoped ``admin`` to create or delete nodes in Ironic. + 1.79 (Zed, 21.0) ---------------------- A node with the same name as the allocation ``name`` is moved to the @@ -9,6 +15,7 @@ start of the derived candidate list. 1.78 (Xena, 18.2) ---------------------- + Add endpoints to allow history events for nodes to be retrieved via the REST API. diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index dab1342588..59b166db4f 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -2462,7 +2462,15 @@ class NodesController(rest.RestController): raise exception.OperationNotPermitted() context = api.request.context - api_utils.check_policy('baremetal:node:create') + owned_node = False + if CONF.api.project_admin_can_manage_own_nodes: + owned_node = api_utils.check_policy_true( + 'baremetal:node:create:self_owned_node') + else: + owned_node = False + + if not owned_node: + api_utils.check_policy('baremetal:node:create') reject_fields_in_newer_versions(node) @@ -2486,6 +2494,28 @@ class NodesController(rest.RestController): if not node.get('resource_class'): node['resource_class'] = CONF.default_resource_class + cdict = context.to_policy_values() + if cdict.get('system_scope') != 'all' and owned_node: + # This only applies when the request is not system + # scoped. + + # First identify what was requested, and if there is + # a project ID to use. + project_id = None + requested_owner = node.get('owner', None) + if cdict.get('project_id', False): + project_id = cdict.get('project_id') + + if requested_owner and requested_owner != project_id: + # Translation: If project scoped, and an owner has been + # requested, and that owner does not match the requestor's + # project ID value. + msg = _("Cannot create a node as a project scoped admin " + "with an owner other than your own project.") + raise exception.Invalid(msg) + # Finally, note the project ID + node['owner'] = project_id + chassis = _replace_chassis_uuid_with_id(node) chassis_uuid = chassis and chassis.uuid or None @@ -2739,8 +2769,16 @@ class NodesController(rest.RestController): raise exception.OperationNotPermitted() context = api.request.context - rpc_node = api_utils.check_node_policy_and_retrieve( - 'baremetal:node:delete', node_ident, with_suffix=True) + try: + rpc_node = api_utils.check_node_policy_and_retrieve( + 'baremetal:node:delete', node_ident, with_suffix=True) + except exception.HTTPForbidden: + if not CONF.api.project_admin_can_manage_own_nodes: + raise + else: + rpc_node = api_utils.check_node_policy_and_retrieve( + 'baremetal:node:delete:self_owned_node', node_ident, + with_suffix=True) chassis_uuid = _get_chassis_uuid(rpc_node) notify.emit_start_notification(context, rpc_node, 'delete', diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 7fc80bc970..763d923897 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -117,7 +117,7 @@ BASE_VERSION = 1 # v1.77: Add fields selector to drivers list and driver detail. # v1.78: Add node history endpoint # v1.79: Change allocation behaviour to prefer node name match - +# v1.80: Marker to represent self service node creation/deletion MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 MINOR_2_AVAILABLE_STATE = 2 @@ -198,6 +198,7 @@ MINOR_76_NODE_CHANGE_BOOT_MODE = 76 MINOR_77_DRIVER_FIELDS_SELECTOR = 77 MINOR_78_NODE_HISTORY = 78 MINOR_79_ALLOCATION_NODE_NAME = 79 +MINOR_80_PROJECT_CREATE_DELETE_NODE = 80 # When adding another version, update: # - MINOR_MAX_VERSION @@ -205,7 +206,7 @@ MINOR_79_ALLOCATION_NODE_NAME = 79 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_79_ALLOCATION_NODE_NAME +MINOR_MAX_VERSION = MINOR_80_PROJECT_CREATE_DELETE_NODE # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/policy.py b/ironic/common/policy.py index a56257e0fc..7fdd398f99 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -437,11 +437,19 @@ node_policies = [ policy.DocumentedRuleDefault( name='baremetal:node:create', check_str=SYSTEM_ADMIN, - scope_types=['system'], + scope_types=['system', 'project'], description='Create Node records', operations=[{'path': '/nodes', 'method': 'POST'}], deprecated_rule=deprecated_node_create ), + policy.DocumentedRuleDefault( + name='baremetal:node:create:self_owned_node', + check_str=('role:admin'), + scope_types=['project'], + description='Create node records which will be tracked ' + 'as owned by the associated user project.', + operations=[{'path': '/nodes', 'method': 'POST'}], + ), policy.DocumentedRuleDefault( name='baremetal:node:list', check_str=API_READER, @@ -663,7 +671,14 @@ node_policies = [ operations=[{'path': '/nodes/{node_ident}', 'method': 'DELETE'}], deprecated_rule=deprecated_node_delete ), - + policy.DocumentedRuleDefault( + name='baremetal:node:delete:self_owned_node', + check_str=PROJECT_ADMIN, + scope_types=['project'], + description='Delete node records which are associated with ' + 'the requesting project.', + operations=[{'path': '/nodes/{node_ident}', 'method': 'DELETE'}], + ), policy.DocumentedRuleDefault( name='baremetal:node:validate', check_str=SYSTEM_OR_OWNER_MEMBER_AND_LESSEE_ADMIN, diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 9dfe864ee7..9403218702 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -491,7 +491,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.79', + 'api': '1.80', 'rpc': '1.55', 'objects': { 'Allocation': ['1.1'], diff --git a/ironic/conf/api.py b/ironic/conf/api.py index 2b0e9a824c..cf59fa0068 100644 --- a/ironic/conf/api.py +++ b/ironic/conf/api.py @@ -86,6 +86,11 @@ opts = [ 'network_data_schema', default='$pybasedir/api/controllers/v1/network-data-schema.json', help=_("Schema for network data used by this deployment.")), + cfg.BoolOpt('project_admin_can_manage_own_nodes', + default=True, + mutable=True, + help=_('If a project scoped administrative user is permitted ' + 'to create/delte baremetal nodes in their project.')), ] opt_group = cfg.OptGroup(name='api', diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index d7a3d474eb..6531f36e70 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -4898,13 +4898,39 @@ class TestPost(test_api_base.BaseApiTest): ndict = test_api_utils.post_get_test_node(owner='cowsay') response = self.post_json('/nodes', ndict, headers={api_base.Version.string: - str(api_v1.max_version())}) + str(api_v1.max_version()), + 'X-Project-Id': 'cowsay'}) self.assertEqual(http_client.CREATED, response.status_int) result = self.get_json('/nodes/%s' % ndict['uuid'], headers={api_base.Version.string: str(api_v1.max_version())}) self.assertEqual('cowsay', result['owner']) + def test_create_node_owner_system_scope(self): + ndict = test_api_utils.post_get_test_node(owner='catsay') + response = self.post_json('/nodes', ndict, + headers={api_base.Version.string: + str(api_v1.max_version()), + 'OpenStack-System-Scope': 'all', + 'X-Roles': 'admin'}) + self.assertEqual(http_client.CREATED, response.status_int) + result = self.get_json('/nodes/%s' % ndict['uuid'], + headers={api_base.Version.string: + str(api_v1.max_version())}) + self.assertEqual('catsay', result['owner']) + + def test_create_node_owner_recorded_project_scope(self): + ndict = test_api_utils.post_get_test_node() + response = self.post_json('/nodes', ndict, + headers={api_base.Version.string: + str(api_v1.max_version()), + 'X-Project-Id': 'ravensay'}) + self.assertEqual(http_client.CREATED, response.status_int) + result = self.get_json('/nodes/%s' % ndict['uuid'], + headers={api_base.Version.string: + str(api_v1.max_version())}) + self.assertEqual('ravensay', result['owner']) + def test_create_node_owner_old_api_version(self): headers = {api_base.Version.string: '1.32'} ndict = test_api_utils.post_get_test_node(owner='bob') diff --git a/ironic/tests/unit/api/test_acl.py b/ironic/tests/unit/api/test_acl.py index 5793e95a8a..cdc20d4770 100644 --- a/ironic/tests/unit/api/test_acl.py +++ b/ironic/tests/unit/api/test_acl.py @@ -81,10 +81,18 @@ class TestACLBase(base.BaseApiTest): body=None, assert_status=None, assert_dict_contains=None, assert_list_length=None, - deprecated=None): + deprecated=None, + self_manage_nodes=True): path = path.format(**self.format_data) self.mock_auth.side_effect = self._fake_process_request + # Set self management override + if not self_manage_nodes: + cfg.CONF.set_override( + 'project_admin_can_manage_own_nodes', + False, + 'api') + # always request the latest api version version = api_versions.max_version_string() rheaders = { diff --git a/ironic/tests/unit/api/test_rbac_project_scoped.yaml b/ironic/tests/unit/api/test_rbac_project_scoped.yaml index 802600703d..b55439ad1a 100644 --- a/ironic/tests/unit/api/test_rbac_project_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_project_scoped.yaml @@ -89,35 +89,71 @@ owner_admin_cannot_post_nodes: body: &node_post_body name: node driver: fake-driverz - assert_status: 500 + assert_status: 403 + self_manage_nodes: False + +owner_admin_can_post_nodes: + path: '/v1/nodes' + method: post + headers: *owner_admin_headers + body: *node_post_body + assert_status: 503 + self_manage_nodes: True owner_manager_cannot_post_nodes: path: '/v1/nodes' method: post headers: *owner_manager_headers body: *node_post_body - assert_status: 500 + assert_status: 403 lessee_admin_cannot_post_nodes: path: '/v1/nodes' method: post headers: *lessee_admin_headers body: *node_post_body - assert_status: 500 + assert_status: 403 + self_manage_nodes: False + +lessee_admin_can_post_nodes: + path: '/v1/nodes' + method: post + headers: *lessee_admin_headers + body: *node_post_body + assert_status: 403 + self_manage_nodes: False lessee_manager_cannot_post_nodes: path: '/v1/nodes' method: post headers: *lessee_manager_headers body: *node_post_body - assert_status: 500 + assert_status: 403 + self_manage_nodes: False + +lessee_manager_can_post_nodes: + path: '/v1/nodes' + method: post + headers: *lessee_manager_headers + body: *node_post_body + assert_status: 403 + self_manage_nodes: True third_party_admin_cannot_post_nodes: path: '/v1/nodes' method: post headers: *third_party_admin_headers body: *node_post_body - assert_status: 500 + assert_status: 403 + self_manage_nodes: False + +third_party_admin_can_post_nodes: + path: '/v1/nodes' + method: post + headers: *third_party_admin_headers + body: *node_post_body + assert_status: 503 + self_manage_nodes: True # Based on nodes_post_member owner_member_cannot_post_nodes: @@ -125,7 +161,7 @@ owner_member_cannot_post_nodes: method: post headers: *owner_member_headers body: *node_post_body - assert_status: 500 + assert_status: 403 # Based on nodes_post_reader owner_reader_cannot_post_reader: @@ -133,7 +169,7 @@ owner_reader_cannot_post_reader: method: post headers: *owner_reader_headers body: *node_post_body - assert_status: 500 + assert_status: 403 # Based on nodes_get_admin # TODO: Create 3 nodes, 2 owned, 1 leased where it is also owned. @@ -671,6 +707,14 @@ owner_admin_cannot_delete_nodes: method: delete headers: *owner_admin_headers assert_status: 403 + self_manage_nodes: False + +owner_admin_can_delete_nodes: + path: '/v1/nodes/{owner_node_ident}' + method: delete + headers: *owner_admin_headers + assert_status: 503 + self_manage_nodes: True owner_manager_cannot_delete_nodes: path: '/v1/nodes/{owner_node_ident}' diff --git a/releasenotes/notes/node-creation-no-longer-scope-restricted-b455f66a751f10ec.yaml b/releasenotes/notes/node-creation-no-longer-scope-restricted-b455f66a751f10ec.yaml new file mode 100644 index 0000000000..b405dddb36 --- /dev/null +++ b/releasenotes/notes/node-creation-no-longer-scope-restricted-b455f66a751f10ec.yaml @@ -0,0 +1,27 @@ +--- +features: + - | + Adds the capability for a project scoped ``admin`` user to be able to + create nodes in Ironic, which are then manageable by the project scoped + ``admin`` user. Effectively, this is self service Bare Metal as a Service, + however more advanced fields such as drivers, chassies, are not available + to these users. This is controlled through an auto-population of the + Node ``owner`` field, and can be controlled through the + ``[api]project_admin_can_manage_own_nodes`` setting, which defaults to + ``True``, and the new policy ``baremetal:node:create:self_owned_node``. + - | + Adds the capability for a project scoped ``admin`` user to be able to + delete nodes from Ironic which their `project` owns. This can be + contolled through the ``[api]project_admin_can_manage_own_nodes`` + setting, which defaults to ``True``, as well as the + ``baremetal:node:delete:self_owned_node`` policy. +security: + - | + This release contains an improvement which, by default, allows users to + create and delete baremetal nodes inside their own project. This can be + disabled using the ``[api]project_admin_can_manage_own_nodes`` setting. +upgrades: + - | + The API version has been increased to ``1.80`` in order to signify + the addition of additoinal Role Based Access Controls capabilities + around node creation and deletion. \ No newline at end of file