Fix service role support

Turns out the service role support doesn't quite work,
because you could not enumerate nodes regardless of node
owner or lessee in order to enable services like Nova to
enumerate nodes to be able to schedule upon them, or
networking-baremetal to enumerate ports in update mapping
in Neutron.

So this change enables permissions to be modified to allow
service project users with the service role to enumerate the
list of resources, and grants rights similar to "system scoped
members" to the service project's users with the "service" role
which aligns with update actions to provision/unprovision nodes.

Adds some additional rbac testing to ensure we appropriately
covered these access rights.

Closes-Bug: 2051592
Change-Id: I2b4bcc748b6e43e4215dc45137becce301349032

ironic/common/context.py

@@ -12,9 +12,13 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from oslo_config import cfg
 from oslo_context import context
 
 
+CONF = cfg.CONF
+
+
 class RequestContext(context.RequestContext):
     """Extends security contexts from the oslo.context library."""
 
@@ -44,6 +48,9 @@ class RequestContext(context.RequestContext):
             'project_name': self.project_name,
             'is_public_api': self.is_public_api,
         })
+        if (CONF.rbac_service_role_elevated_access
+                and CONF.rbac_service_project_name is not None):
+            policy_values['config.service_project_name'] = CONF.rbac_service_project_name  # noqa
         return policy_values
 
     def ensure_thread_contain_context(self):

ironic/common/policy.py

@@ -51,15 +51,22 @@ SYSTEM_ADMIN = 'role:admin and system_scope:all'
 # authorization that system administrators typically have. This persona, or
 # check string, typically isn't used by default, but it's existence it useful
 # in the event a deployment wants to offload some administrative action from
-# system administrator to system members
+# system administrator to system members.
-SYSTEM_MEMBER = 'role:member and system_scope:all'
+# The rule:service_role match here is to enable an elevated level of API
+# access for a specialized service role and users with appropriate
+# service role access.
+SYSTEM_MEMBER = '(role:member and system_scope:all) or rule:service_role'  # noqa
 
-# Generic policy check string for read-only access to system-level resources.
+# Generic policy check string for read-only access to system-level
-# This persona is useful for someone who needs access for auditing or even
+# resources. This persona is useful for someone who needs access
-# support. These uses are also able to view project-specific resources where
+# for auditing or even support. These uses are also able to view
-# applicable (e.g., listing all volumes in the deployment, regardless of the
+# project-specific resources where applicable (e.g., listing all
-# project they belong to).
+# volumes in the deployment, regardless of the project they belong to).
-SYSTEM_READER = '(role:reader and system_scope:all) or (role:service and system_scope:all)'  # noqa
+# The rule:service_role match here is to enable an elevated level of API
+# access for a specialized service role and users with appropriate
+# role access, specifically because 'service" role is outside of the RBAC
+# model defaults and does not imply reader access.
+SYSTEM_READER = '(role:reader and system_scope:all) or (role:service and system_scope:all) or rule:service_role'  # noqa
 
 # This check string is reserved for actions that require the highest level of
 # authorization on a project or resources within the project (e.g., setting the
@@ -98,7 +105,7 @@ PROJECT_SERVICE = ('role:service and project_id:%(node.owner)s')
 # administrator should be able to delete any baremetal host in the deployment,
 # a project member should only be able to delete hosts in their project).
 SYSTEM_OR_PROJECT_MEMBER = (
-    '(' + SYSTEM_MEMBER + ') or (' + PROJECT_MEMBER + ')'
+    '(' + SYSTEM_MEMBER + ') or (' + PROJECT_MEMBER + ') or (' + SYSTEM_SERVICE + ')'  # noqa
 )
 SYSTEM_OR_PROJECT_READER = (
     '(' + SYSTEM_READER + ') or (' + PROJECT_READER + ') or (' + PROJECT_SERVICE + ')'  # noqa
@@ -210,6 +217,13 @@ default_policies = [
     policy.RuleDefault('show_instance_secrets',
                        '!',
                        description='Show or mask secrets within instance information in API responses'),  # noqa
+    # NOTE(TheJulia): This is a special rule to allow customization of the
+    # service role check. The config.service_project_name is a reserved
+    # target check field which is loaded from configuration to the
+    # check context in ironic/common/context.py.
+    policy.RuleDefault('service_role',
+                       'role:service and project_name:%(config.service_project_name)s',  # noqa
+                       description='Rule to match service role usage with a service project, delineated as a separate rule to enable customization.'),  # noqa
     # Roles likely to be overridden by operator
     # TODO(TheJulia): Lets nuke demo from high orbit.
     policy.RuleDefault('is_member',
@@ -484,7 +498,7 @@ node_policies = [
     policy.DocumentedRuleDefault(
         name='baremetal:node:list_all',
         check_str=SYSTEM_READER,
-        scope_types=['system'],
+        scope_types=['system', 'project'],
         description='Retrieve multiple Node records',
         operations=[{'path': '/nodes', 'method': 'GET'},
                     {'path': '/nodes/detail', 'method': 'GET'}],

ironic/conf/default.py

@@ -445,6 +445,40 @@ webserver_opts = [
                       'being accessed.')),
 ]
 
+rbac_opts = [
+    cfg.BoolOpt('rbac_service_role_elevated_access',
+                default=False,
+                help=_('Enable elevated access for users with service role '
+                       'belonging to the \'rbac_service_project_name\' '
+                       'project when using default policy. The default '
+                       'setting of disabled causes all service role '
+                       'requests to be scoped to the project the service '
+                       'account belongs to.')),
+    cfg.StrOpt('rbac_service_project_name',
+               default='service',
+               help=_('The project name utilized for Role Based Access '
+                      'Control checks for the reserved `service` project. '
+                      'This project is utilized for services to have '
+                      'accounts for cross-service communication. Often '
+                      'these accounts require higher levels of access, and '
+                      'effectively this permits accounts from the service '
+                      'to not be restricted to project scoping '
+                      'of responses. i.e. The service project user with a '
+                      '`service` role will be able to see nodes across all '
+                      'projects, similar to System scoped access. If not '
+                      'set to a value, and all service role access will '
+                      'be filtered matching an `owner` or `lessee`, if '
+                      'applicable. If an operator wishes to make behavior '
+                      'visible for all service role users across '
+                      'all projects, then a custom policy must be used '
+                      'to override the default "service_role" rule. '
+                      'It should be noted that the value of "service" '
+                      'is a default convention for OpenStack deployments, '
+                      'but the requsite access and details around '
+                      'end configuration are largely up to an operator '
+                      'if they are doing an OpenStack deployment manually.')),
+]
+
 
 def list_opts():
     _default_opt_lists = [
@@ -461,6 +495,7 @@ def list_opts():
         service_opts,
         utils_opts,
         webserver_opts,
+        rbac_opts
     ]
     full_opt_list = []
     for options in _default_opt_lists:
@@ -482,3 +517,4 @@ def register_opts(conf):
     conf.register_opts(service_opts)
     conf.register_opts(utils_opts)
     conf.register_opts(webserver_opts)
+    conf.register_opts(rbac_opts)

ironic/tests/unit/api/test_acl.py

@@ -77,12 +77,14 @@ class TestACLBase(base.BaseApiTest):
     def _fake_process_request(self, request, auth_token_request):
         pass
 
-    def _test_request(self, path, params=None, headers=None, method='get',
+    def _test_request(self, path, params=None, headers=None, method='get',  # noqa: C901, E501
                       body=None, assert_status=None,
                       assert_dict_contains=None,
                       assert_list_length=None,
                       deprecated=None,
-                      self_manage_nodes=True):
+                      self_manage_nodes=True,
+                      enable_service_project=False,
+                      service_project='service'):
         path = path.format(**self.format_data)
         self.mock_auth.side_effect = self._fake_process_request
 
@@ -92,6 +94,13 @@ class TestACLBase(base.BaseApiTest):
                 'project_admin_can_manage_own_nodes',
                 False,
                 'api')
+        if enable_service_project:
+            cfg.CONF.set_override('rbac_service_role_elevated_access', True)
+        if service_project != 'service':
+            # Enable us to sort of gracefully test a name variation
+            # with existing ddt test modeling.
+            cfg.CONF.set_override('rbac_service_project_name',
+                                  service_project)
 
         # always request the latest api version
         version = api_versions.max_version_string()

ironic/tests/unit/api/test_rbac_system_scoped.yaml

@@ -27,6 +27,11 @@ values:
     X-Auth-Token: 'baremetal-service-token'
     X-Roles: service
     OpenStack-System-Scope: all
+  service_project_headers: &service_project_headers
+    X-Auth-Toke: 'service-project-token'
+    X-Roles: 'service'
+    X-Project-Name: 'service'
+    X-Project-ID: c11111111111111111111111111111
   owner_project_id: &owner_project_id '{owner_project_id}'
   other_project_id: &other_project_id '{other_project_id}'
   node_ident: &node_ident '{node_ident}'
@@ -112,6 +117,34 @@ nodes_get_service:
     nodes: 3
   assert_status: 200
 
+nodes_get_service_project:
+  path: '/v1/nodes'
+  method: get
+  headers: *service_project_headers
+  assert_list_length:
+    nodes: 3
+  assert_status: 200
+  enable_service_project: true
+
+nodes_get_service_project_disabled:
+  path: '/v1/nodes'
+  method: get
+  headers: *service_project_headers
+  assert_list_length:
+    nodes: 0
+  assert_status: 200
+  enable_service_project: false
+
+nodes_get_service_project_admin:
+  path: '/v1/nodes'
+  method: get
+  headers: *service_project_headers
+  assert_list_length:
+    nodes: 0
+  assert_status: 200
+  service_project: admin
+  enable_service_project: true
+
 nodes_get_other_admin:
   path: '/v1/nodes'
   method: get
@@ -200,6 +233,21 @@ nodes_node_ident_patch_member:
   body: *extra_patch
   assert_status: 503
 
+nodes_node_ident_patch_service:
+  path: '/v1/nodes/{node_ident}'
+  method: patch
+  headers: *service_headers
+  body: *extra_patch
+  assert_status: 503
+
+nodes_node_ident_patch_service_project:
+  path: '/v1/nodes/{node_ident}'
+  method: patch
+  headers: *service_project_headers
+  body: *extra_patch
+  assert_status: 503
+  enable_service_project: true
+
 nodes_node_ident_patch_reader:
   path: '/v1/nodes/{node_ident}'
   method: patch
@@ -248,6 +296,19 @@ nodes_validate_get_member:
   headers: *scoped_member_headers
   assert_status: 503
 
+nodes_validate_get_service:
+  path: '/v1/nodes/{node_ident}/validate'
+  method: get
+  headers: *service_headers
+  assert_status: 503
+
+nodes_validate_get_service_project:
+  path: '/v1/nodes/{node_ident}/validate'
+  method: get
+  headers: *service_project_headers
+  assert_status: 503
+  enable_service_project: true
+
 nodes_validate_get_reader:
   path: '/v1/nodes/{node_ident}/validate'
   method: get
@@ -815,6 +876,14 @@ nodes_vifs_post_service:
   assert_status: 503
   body: *vif_body
 
+nodes_vifs_post_service_project:
+  path: '/v1/nodes/{node_ident}/vifs'
+  method: post
+  headers: *service_project_headers
+  assert_status: 503
+  body: *vif_body
+  enable_service_project: true
+
 # This calls the conductor, hence not status 403.
 nodes_vifs_node_vif_ident_delete_admin:
   path: '/v1/nodes/{node_ident}/vifs/{vif_ident}'
@@ -1004,6 +1073,26 @@ nodes_portgroups_get_reader:
   headers: *reader_headers
   assert_status: 200
 
+nodes_portgroups_get_service:
+  path: '/v1/nodes/{node_ident}/portgroups'
+  method: get
+  headers: *service_headers
+  assert_status: 200
+
+nodes_portgroups_get_service_project:
+  path: '/v1/nodes/{node_ident}/portgroups'
+  method: get
+  headers: *service_project_headers
+  assert_status: 200
+  enable_service_project: true
+
+nodes_portgroups_get_service_project:
+  path: '/v1/nodes/{node_ident}/portgroups'
+  method: get
+  headers: *service_project_headers
+  assert_status: 404
+  enable_service_project: false
+
 nodes_portgroups_detail_get_admin:
   path: '/v1/nodes/{node_ident}/portgroups/detail'
   method: get
@@ -1022,6 +1111,25 @@ nodes_portgroups_detail_get_reader:
   headers: *reader_headers
   assert_status: 200
 
+nodes_portgroups_detail_get_service:
+  path: '/v1/nodes/{node_ident}/portgroups/detail'
+  method: get
+  headers: *service_headers
+  assert_status: 200
+
+nodes_portgroups_detail_get_service_project:
+  path: '/v1/nodes/{node_ident}/portgroups/detail'
+  method: get
+  headers: *service_project_headers
+  assert_status: 200
+  enable_service_project: true
+
+nodes_portgroups_detail_get_service_project:
+  path: '/v1/nodes/{node_ident}/portgroups/detail'
+  method: get
+  headers: *service_project_headers
+  assert_status: 404
+
 # Ports - https://docs.openstack.org/api-ref/baremetal/#ports-ports
 
 ports_get_admin:
@@ -1029,6 +1137,33 @@ ports_get_admin:
   method: get
   headers: *admin_headers
   assert_status: 200
+  assert_list_length:
+    ports: 1
+
+ports_get_service:
+  path: '/v1/ports'
+  method: get
+  headers: *service_headers
+  assert_status: 200
+  assert_list_length:
+    ports: 1
+
+ports_get_service_project:
+  path: '/v1/ports'
+  method: get
+  headers: *service_project_headers
+  assert_status: 200
+  assert_list_length:
+    ports: 1
+  enable_service_project: true
+
+ports_get_service_project_disabled:
+  path: '/v1/ports'
+  method: get
+  headers: *service_project_headers
+  assert_status: 200
+  assert_list_length:
+    ports: 0
 
 ports_get_member:
   path: '/v1/ports'
@@ -1258,6 +1393,14 @@ volume_get_service:
   headers: *service_headers
   assert_status: 200
 
+volume_get_service_project:
+  path: '/v1/volume'
+  method: get
+  headers: *service_project_headers
+  assert_status: 200
+  assert_list_length:
+  enable_service_project: true
+
 # Volume connectors
 
 volume_connectors_get_admin:
@@ -1284,6 +1427,24 @@ volume_connectors_get_service:
   headers: *service_headers
   assert_status: 200
 
+volume_connectors_get_service_project:
+  path: '/v1/volume/connectors'
+  method: get
+  headers: *service_project_headers
+  assert_status: 200
+  assert_list_length:
+    connectors: 1
+  enable_service_project: true
+
+volume_connectors_get_service_project_disable:
+  path: '/v1/volume/connectors'
+  method: get
+  headers: *service_project_headers
+  assert_status: 200
+  assert_list_length:
+    connectors: 0
+  enable_service_project: false
+
 # NOTE(TheJulia): This ends up returning a 400 due to the
 # UUID not already being in ironic.
 volume_connectors_post_admin:
@@ -1319,6 +1480,14 @@ volume_connectors_post_service:
   assert_status: 201
   body: *volume_connector_body
 
+volume_connectors_post_service_project:
+  path: '/v1/volume/connectors'
+  method: post
+  headers: *service_project_headers
+  assert_status: 201
+  body: *volume_connector_body
+  enable_service_project: true
+
 volume_volume_connector_id_get_admin:
   path: '/v1/volume/connectors/{volume_connector_ident}'
   method: get
@@ -1443,6 +1612,53 @@ volume_targets_post_member:
     boot_index: 2
     volume_id: 'test-id2'
 
+volume_targets_post_service:
+  path: '/v1/volume/targets'
+  method: post
+  headers: *service_headers
+  assert_status: 201
+  body:
+    node_uuid: 1be26c0b-03f2-4d2e-ae87-c02d7f33c123
+    volume_type: iscsi
+    boot_index: 2
+    volume_id: 'test-id2'
+
+volume_targets_post_service_project:
+  path: '/v1/volume/targets'
+  method: post
+  headers: *service_project_headers
+  assert_status: 201
+  body:
+    node_uuid: 1be26c0b-03f2-4d2e-ae87-c02d7f33c123
+    volume_type: iscsi
+    boot_index: 2
+    volume_id: 'test-id2'
+  enable_service_project: true
+
+volume_targets_post_service_project_disabled:
+  path: '/v1/volume/targets'
+  method: post
+  headers: *service_project_headers
+  assert_status: 403
+  body:
+    node_uuid: 1be26c0b-03f2-4d2e-ae87-c02d7f33c123
+    volume_type: iscsi
+    boot_index: 2
+    volume_id: 'test-id2'
+
+volume_targets_post_service_project_admin:
+  path: '/v1/volume/targets'
+  method: post
+  headers: *service_project_headers
+  assert_status: 403
+  body:
+    node_uuid: 1be26c0b-03f2-4d2e-ae87-c02d7f33c123
+    volume_type: iscsi
+    boot_index: 2
+    volume_id: 'test-id2'
+  service_project: admin
+  enable_service_project: true
+
 volume_targets_post_reader:
   path: '/v1/volume/targets'
   method: post
@@ -1507,6 +1723,14 @@ volume_volume_target_id_patch_service:
   headers: *service_headers
   assert_status: 503
 
+volume_volume_target_id_patch_service:
+  path: '/v1/volume/targets/{volume_target_ident}'
+  method: patch
+  body: *volume_target_patch
+  headers: *service_project_headers
+  assert_status: 503
+  enable_service_project: true
+
 volume_volume_target_id_delete_admin:
   path: '/v1/volume/targets/{volume_target_ident}'
   method: delete

releasenotes/notes/service-project-service-role-fix-e4d1a8c23856926a.yaml

@@ -0,0 +1,41 @@
+---
+fixes:
+  - |
+    Provides a fix for ``service`` role support to enable the use
+    case where a dedicated service project is used for cloud service
+    operation to facilitate actions as part of the operation of the
+    cloud infrastructure.
+
+    OpenStack clouds can take a variety of configuration models
+    for service accounts. It is now possible to utilize the
+    ``[DEFAULT] rbac_service_role_elevated_access`` setting to
+    enable users with a ``service`` role in a dedicated ``service``
+    project to act upon the API similar to a "System" scoped
+    "Member" where resources regardless of ``owner`` or ``lessee``
+    settings are available. This is needed to enable synchronization
+    processes, such as ``nova-compute`` or the ``networking-baremetal``
+    ML2 plugin to perform actions across the whole of an Ironic
+    deployment, if desirable where a "System" scoped user is also
+    undesirable.
+
+    This functionality can be tuned to utilize a customized project
+    name aside from the default convention ``service``, for example
+    ``baremetal`` or ``admin``, utilizing the
+    ``[DEFAULT] rbac_service_project_name`` setting.
+
+    Operators can alternatively entirely override the
+    ``service_role`` RBAC policy rule, if so desired, however
+    Ironic feels the default is both reasonable and delineates
+    sufficiently for the variety of Role Based Access Control
+    usage cases which can exist with a running Ironic deployment.
+upgrades:
+  - |
+    This version of ironic includes an opt-in fix to the Role Based Access
+    Control logic where the "service" role in a "service" project is
+    able to be granted elevated access to the API surface of Ironic such that
+    all baremetal nodes are visible to that API consumer. This is for
+    deployments which have not moved to a "System" scoped user for connecting
+    to ironic for services like ``nova-compute`` and the
+    ``networking-baremetal`` Neutron plugin, and where it is desirable for
+    those services to be able to operate across the whole of the Ironic
+    deployment.