Merge "Fix service role support"

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.