diff --git a/doc/source/configuration/policy-concepts.rst b/doc/source/configuration/policy-concepts.rst index ec52077fa844..89bdc259ddf4 100644 --- a/doc/source/configuration/policy-concepts.rst +++ b/doc/source/configuration/policy-concepts.rst @@ -33,8 +33,8 @@ issues that had been identified: component than API in Nova that did not honor changes to policy. As a result, policy could not override hard-coded in-project checks. -Keystone comes with ``admin``, ``member`` and ``reader`` roles by default. -Please refer to :keystone-doc:`this document ` +Keystone comes with ``admin``, ``manager``, ``member`` and ``reader`` roles +by default. Please refer to :keystone-doc:`this document ` for more information about these new defaults. In addition, keystone supports a new "system scope" concept that makes it easier to protect deployment level resources from project or system level resources. Please refer to @@ -215,6 +215,44 @@ OR 'project_id:%(project_id)s' in the check_str is important to restrict the access within the requested project. +.. rubric:: ``manager`` + +``project_manager`` is denoted by someone with the manager role on a project. +It is intended to be used in project-level management APIs and perform more +privileged operations than ``project_member`` on its project resources. It +inherits all the permissions of a ``project_member`` and ``project_reader``. +For example, ``project_manager`` can migrate (cold or live) their server +without specifying the host. Further, the ``project_manager`` will be able +to list their own project migrations. + +``project_manager`` persona in Nova policy rule (it is defined as +``project_manager_api`` in policy yaml) looks like: + +'project_id:%(project_id)s' in the check_str is important to restrict the +access within the requested project. + +.. code-block:: yaml + + # Default rule for Project level management APIs. + "project_manager_api": "role:manager and project_id:%(project_id)s" + +To keep the legacy ``admin`` behavior unchanged, Nova allow ``admin`` +also to access the project level management APIs: + +.. code-block:: yaml + + # Default rule for Project level management APIs. + "project_manager_or_admin": "rule:project_manager_api or rule:context_is_admin" + +The above base rule are used for specific API access: + +.. code-block:: yaml + + # Cold migrate a server without specifying a host + # POST /servers/{server_id}/action (migrate) + # Intended scope(s): project + "os_compute_api:os-migrate-server:migrate": "rule:project_manager_or_admin" + .. rubric:: ``admin`` This role is to perform the admin level write operations. Nova policies are @@ -248,16 +286,26 @@ overridden in the policy.yaml file but scope is not override-able. perform the admin level operations. Example: enable/disable compute service, Live migrate server etc. +#. PROJECT_MANAGER: ``manager`` role on ``project`` scope. This is used to + perform project management operations within project. For example: migrate + a server. + #. PROJECT_MEMBER: ``member`` role on ``project`` scope. This is used to perform resource owner level operation within project. For example: Pause a server. #. PROJECT_READER: ``reader`` role on ``project`` scope. This is used to perform read-only operation within project. For example: Get server. -#. PROJECT_MEMBER_OR_ADMIN: ``admin`` or ``member`` role on ``project`` scope. Such policy rules are default to most of the owner level APIs and align +#. PROJECT_MANAGER_OR_ADMIN: ``admin`` or ``manager`` role on ``project`` scope. + Such policy rules are default to project management level APIs and along + with ``manager`` role, legacy admin can continue to access those APIs. + +#. PROJECT_MEMBER_OR_ADMIN: ``admin`` or ``member`` role on ``project`` scope. + Such policy rules are default to most of the owner level APIs and align with ``member`` role legacy admin can continue to access those APIs. -#. PROJECT_READER_OR_ADMIN: ``admin`` or ``reader`` role on ``project`` scope. Such policy rules are default to most of the read only APIs so that legacy +#. PROJECT_READER_OR_ADMIN: ``admin`` or ``reader`` role on ``project`` scope. + Such policy rules are default to most of the read only APIs so that legacy admin can continue to access those APIs. Backward Compatibility @@ -336,21 +384,31 @@ NOTE:: Below table show how legacy rules are mapped to new rules: -+--------------------+---------------------------+----------------+-----------+ -| Legacy Rule | New Rules |Operation |scope_type | -+====================+===========================+================+===========+ -| RULE_ADMIN_API |-> ADMIN |Global resource | [project] | -| | |Write & Read | | -+--------------------+---------------------------+----------------+-----------+ -| |-> ADMIN |Project admin | [project] | -| | |level operation | | -| +---------------------------+----------------+-----------+ -| RULE_ADMIN_OR_OWNER|-> PROJECT_MEMBER_OR_ADMIN |Project resource| [project] | -| | |Write | | -| +---------------------------+----------------+-----------+ -| |-> PROJECT_READER_OR_ADMIN |Project resource| [project] | -| | |Read | | -+--------------------+---------------------------+----------------+-----------+ +.. list-table:: + :widths: 25 45 15 15 + :header-rows: 1 + + * - Legacy Rule + - New Rule + - Operation + - Scope + * - RULE_ADMIN_API + - ADMIN + - Global resource Write & Read + - project + * - RULE_ADMIN_API + - PROJECT_MANAGER_OR_ADMIN + - Project management level + - project + * - RULE_ADMIN_OR_OWNER + - PROJECT_MEMBER_OR_ADMIN + - Project resource write + - project + * - RULE_ADMIN_OR_OWNER + - PROJECT_READER_OR_ADMIN + - Project resource read + - project We expect all deployments to migrate to the new policy by OpenStack 2023.1 -(Nova 27.0.0) release so that we can remove the support of old policies. +(Nova 27.0.0) release (``project_manager`` role is available from Nova 32.0.0) +so that we can remove the support of old policies. diff --git a/nova/api/openstack/compute/migrate_server.py b/nova/api/openstack/compute/migrate_server.py index 270a95005617..8ab2c6c5dd66 100644 --- a/nova/api/openstack/compute/migrate_server.py +++ b/nova/api/openstack/compute/migrate_server.py @@ -108,10 +108,14 @@ class MigrateServerController(wsgi.Controller): instance = common.get_instance(self.compute_api, context, id, expected_attrs=['numa_topology']) - context.can(ms_policies.POLICY_ROOT % 'migrate_live', - target={'project_id': instance.project_id}) - host = body["os-migrateLive"]["host"] + if host: + context.can(ms_policies.POLICY_ROOT % 'migrate_live:host', + target={'project_id': instance.project_id}) + else: + context.can(ms_policies.POLICY_ROOT % 'migrate_live', + target={'project_id': instance.project_id}) + block_migration = body["os-migrateLive"]["block_migration"] force = None async_ = api_version_request.is_supported(req, '2.34') diff --git a/nova/api/openstack/compute/migrations.py b/nova/api/openstack/compute/migrations.py index 2cbf2849c70c..6baf8447bc0e 100644 --- a/nova/api/openstack/compute/migrations.py +++ b/nova/api/openstack/compute/migrations.py @@ -38,7 +38,8 @@ class MigrationsController(wsgi.Controller): self.compute_api = compute.API() def _output(self, req, migrations_obj, add_link=False, - add_uuid=False, add_user_project=False): + add_uuid=False, add_user_project=False, + add_host=True): """Returns the desired output of the API from an object. From a MigrationsList's object this method returns a list of @@ -71,6 +72,25 @@ class MigrationsController(wsgi.Controller): del obj['user_id'] if 'project_id' in obj: del obj['project_id'] + # TODO(gmaan): This API (and list/show server migrations) does not + # return the 'server_host', which is strange and not consistent + # with the info returned for the destination host. This needs to + # be fixed with microversion bump. When we do that, there are some + # more improvement can be done in this and list/show server + # migrations API. It makes sense to do all those improvements + # in a single microversion: + # - Non-admin user can get their own project migrations if the + # policy does not permit listing all projects migrations (for more + # details, refer to the comment in _index() method). + # - Check the comments in the file + # api/openstack/compute/server_migrations.py for more possible + # improvement in the list server migration API. + if not add_host: + obj['dest_compute'] = None + obj['dest_host'] = None + obj['dest_node'] = None + obj['source_compute'] = None + obj['source_node'] = None # NOTE(Shaohe Feng) above version 2.23, add migration_type for all # kinds of migration, but we only add links just for in-progress # live-migration. @@ -93,6 +113,19 @@ class MigrationsController(wsgi.Controller): context.can(migrations_policies.POLICY_ROOT % 'index') search_opts = {} search_opts.update(req.GET) + project_id = search_opts.get('project_id') + # TODO(gmaan): If the user request all or cross project migrations + # (passing other project id or not passing project id itself) then + # policy needs to permit the same otherwise it will raise 403 error. + # This behavior can be improved by returning their own project + # migrations if the policy does not permit to list all or cross + # project migrations but that will be API behavior change and + # needs to be done with microversion bump. + if not project_id or project_id != context.project_id: + context.can(migrations_policies.POLICY_ROOT % 'index:all_projects') + add_host = context.can(migrations_policies.POLICY_ROOT % 'index:host', + fatal=False) + if 'changes-since' in search_opts: if allow_changes_since: search_opts['changes-since'] = timeutils.parse_isotime( @@ -133,10 +166,9 @@ class MigrationsController(wsgi.Controller): else: migrations = self.compute_api.get_migrations( context, search_opts) - add_user_project = api_version_request.is_supported(req, '2.80') migrations = self._output(req, migrations, add_link, - add_uuid, add_user_project) + add_uuid, add_user_project, add_host) migrations_dict = {'migrations': migrations} if next_link: diff --git a/nova/api/openstack/compute/server_migrations.py b/nova/api/openstack/compute/server_migrations.py index c084be150724..fa687f6ea24f 100644 --- a/nova/api/openstack/compute/server_migrations.py +++ b/nova/api/openstack/compute/server_migrations.py @@ -26,17 +26,29 @@ from nova.i18n import _ from nova.policies import servers_migrations as sm_policies -def output(migration, include_uuid=False, include_user_project=False): +def output( + migration, + include_uuid=False, + include_user_project=False, + include_host=True, +): """Returns the desired output of the API from an object. From a Migrations's object this method returns the primitive object with the only necessary and expected fields. """ + # TODO(gmaan): We have a separate policy to show the host related info. + # For backward compatibility, we always return the host fields in response + # but policy controls the value. To be consistent to other APIs, if the + # policy does not permit then we should not include these fields in + # response but that needs to be done with a new microversion. There + # are more related improvements to be done in list migrations APIs, + # refer to the comments in api/openstack/compute/migrations.py. result = { "created_at": migration.created_at, - "dest_compute": migration.dest_compute, - "dest_host": migration.dest_host, - "dest_node": migration.dest_node, + "dest_compute": None, + "dest_host": None, + "dest_node": None, "disk_processed_bytes": migration.disk_processed, "disk_remaining_bytes": migration.disk_remaining, "disk_total_bytes": migration.disk_total, @@ -45,8 +57,8 @@ def output(migration, include_uuid=False, include_user_project=False): "memory_remaining_bytes": migration.memory_remaining, "memory_total_bytes": migration.memory_total, "server_uuid": migration.instance_uuid, - "source_compute": migration.source_compute, - "source_node": migration.source_node, + "source_compute": None, + "source_node": None, "status": migration.status, "updated_at": migration.updated_at } @@ -55,6 +67,19 @@ def output(migration, include_uuid=False, include_user_project=False): if include_user_project: result['user_id'] = migration.user_id result['project_id'] = migration.project_id + # TODO(gmaan): This API (and list os-migrations) does not + # return the 'server_host', which is strange and not consistent with + # the info returned for the destination host. This needs to be fixed + # with microversion bump. There are more related improvements to be + # done in list migrations APIs, refer to the comments in + # api/openstack/compute/migrations.py + if include_host: + result['dest_compute'] = migration.dest_compute + result['dest_host'] = migration.dest_host + result['dest_node'] = migration.dest_node + result['source_compute'] = migration.source_compute + result['source_node'] = migration.source_node + return result @@ -103,6 +128,9 @@ class ServerMigrationsController(wsgi.Controller): context.can(sm_policies.POLICY_ROOT % 'index', target={'project_id': instance.project_id}) + include_host = context.can(sm_policies.POLICY_ROOT % 'index:host', + fatal=False, + target={'project_id': instance.project_id}) migrations = self.compute_api.get_migrations_in_progress_by_instance( context, server_id, 'live-migration') @@ -111,7 +139,7 @@ class ServerMigrationsController(wsgi.Controller): include_user_project = api_version_request.is_supported(req, '2.80') return {'migrations': [ - output(migration, include_uuid, include_user_project) + output(migration, include_uuid, include_user_project, include_host) for migration in migrations]} @wsgi.api_version("2.23") diff --git a/nova/policies/base.py b/nova/policies/base.py index ab0c319cdfff..0d4c3ac658d8 100644 --- a/nova/policies/base.py +++ b/nova/policies/base.py @@ -38,24 +38,26 @@ DEPRECATED_ADMIN_OR_OWNER_POLICY = policy.DeprecatedRule( ) ADMIN = 'rule:context_is_admin' +PROJECT_MEMBER = 'rule:project_manager_api' PROJECT_MEMBER = 'rule:project_member_api' PROJECT_READER = 'rule:project_reader_api' +PROJECT_MANAGER_OR_ADMIN = 'rule:project_manager_or_admin' PROJECT_MEMBER_OR_ADMIN = 'rule:project_member_or_admin' PROJECT_READER_OR_ADMIN = 'rule:project_reader_or_admin' # NOTE(gmann): Below is the mapping of new roles with legacy roles:: -# Legacy Rule | New Rules |Operation |scope_type| -# -------------------+---------------------------+----------------+----------- -# RULE_ADMIN_API |-> ADMIN |Global resource | [project] -# | |Write & Read | -# -------------------+---------------------------+----------------+----------- -# |-> ADMIN |Project admin | [project] -# | |level operation | -# RULE_ADMIN_OR_OWNER|-> PROJECT_MEMBER_OR_ADMIN |Project resource| [project] -# | |Write | -# |-> PROJECT_READER_OR_ADMIN |Project resource| [project] -# | |Read | +# Legacy Rule | New Rules |Operation |scope_type| +# -------------------+----------------------------+----------------+----------- +# RULE_ADMIN_API |-> ADMIN |Global resource | [project] +# |-> PROJECT_MANAGER_OR_ADMIN |Write & Read | +# -------------------+----------------------------+----------------+----------- +# |-> ADMIN |Project admin | [project] +# | |level operation | +# RULE_ADMIN_OR_OWNER|-> PROJECT_MEMBER_OR_ADMIN |Project resource| [project] +# | |Write | +# |-> PROJECT_READER_OR_ADMIN |Project resource| [project] +# | |Read | # NOTE(johngarbutt) The base rules here affect so many APIs the list # of related API operations has not been populated. It would be @@ -89,6 +91,11 @@ rules = [ deprecated_for_removal=True, deprecated_reason=DEPRECATED_REASON, deprecated_since='21.0.0'), + policy.RuleDefault( + "project_manager_api", + "role:manager and project_id:%(project_id)s", + "Default rule for Project level management APIs.", + deprecated_rule=DEPRECATED_ADMIN_POLICY), policy.RuleDefault( "project_member_api", "role:member and project_id:%(project_id)s", @@ -99,6 +106,11 @@ rules = [ "role:reader and project_id:%(project_id)s", "Default rule for Project level read only APIs.", deprecated_rule=DEPRECATED_ADMIN_OR_OWNER_POLICY), + policy.RuleDefault( + "project_manager_or_admin", + "rule:project_manager_api or rule:context_is_admin", + "Default rule for Project Manager or admin APIs.", + deprecated_rule=DEPRECATED_ADMIN_POLICY), policy.RuleDefault( "project_member_or_admin", "rule:project_member_api or rule:context_is_admin", diff --git a/nova/policies/migrate_server.py b/nova/policies/migrate_server.py index 55c54a72e1cf..ea875f34c93e 100644 --- a/nova/policies/migrate_server.py +++ b/nova/policies/migrate_server.py @@ -20,11 +20,26 @@ from nova.policies import base POLICY_ROOT = 'os_compute_api:os-migrate-server:%s' +DEPRECATED_REASON = """\ +Nova introduces one more policy to live migration API. The original policy is +not deprecated and used to allow live migration without requesting a specific +host. A new policy is added to control the live migration requesting a +specific host. If you have overridden the original policy in your deployment, +you must also add the new policy to keep the same permissions for live +migration to a specific host. +""" + +DEPRECATED_POLICY = policy.DeprecatedRule( + POLICY_ROOT % 'migrate_live', + base.ADMIN, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='32.0.0' +) migrate_server_policies = [ policy.DocumentedRuleDefault( name=POLICY_ROOT % 'migrate', - check_str=base.ADMIN, + check_str=base.PROJECT_MANAGER_OR_ADMIN, description="Cold migrate a server without specifying a host", operations=[ { @@ -44,10 +59,20 @@ migrate_server_policies = [ } ], scope_types=['project']), + + # NOTE(gmaan): You might see this policy as deprecated in the new policy + # 'migrate_live:host' but it is not deprecated and still be used for + # the live migration without specifying a host. By adding this existing + # policy in new policy deprecated field, oslo.policy will handle the policy + # overridden case. In that case, oslo.policy will pick the existing policy + # overridden value from policy.yaml file and apply the same to the new + # policy. This way existing deployment (for default as well as custom + # policy case) will not break. policy.DocumentedRuleDefault( name=POLICY_ROOT % 'migrate_live', - check_str=base.ADMIN, - description="Live migrate a server to a new host without a reboot", + check_str=base.PROJECT_MANAGER_OR_ADMIN, + description="Live migrate a server to a new host without a reboot " + "without specifying a host.", operations=[ { 'method': 'POST', @@ -55,6 +80,24 @@ migrate_server_policies = [ } ], scope_types=['project']), + policy.DocumentedRuleDefault( + name=POLICY_ROOT % 'migrate_live:host', + check_str=base.ADMIN, + description="Live migrate a server to a specified host without " + "a reboot.", + operations=[ + { + 'method': 'POST', + 'path': '/servers/{server_id}/action (os-migrateLive)' + } + ], + scope_types=['project'], + # TODO(gmaan): We can remove this after the next SLURP release + # (after 2026.1 release). We need to keep this deprecated rule + # for the case where operator has overridden the old policy + # 'migrate_live' in policy.yaml. For details, refer to the above + # comment in the 'migrate_live' policy rule. + deprecated_rule=DEPRECATED_POLICY), ] diff --git a/nova/policies/migrations.py b/nova/policies/migrations.py index ce2aeaa564fe..86feb3fe3e2c 100644 --- a/nova/policies/migrations.py +++ b/nova/policies/migrations.py @@ -20,12 +20,36 @@ from nova.policies import base POLICY_ROOT = 'os_compute_api:os-migrations:%s' +DEPRECATED_REASON = """\ +Nova introduces two new policies to list migrations. The original policy +is not deprecated and used to list the live migration without host info. +Two new policies are added to list migration with host info and cross +projects migrations. If you have overridden the original policy in your +deployment, you must also update the new policy to keep the same +permissions. +""" + +DEPRECATED_POLICY = policy.DeprecatedRule( + POLICY_ROOT % 'index', + base.ADMIN, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='32.0.0' +) + migrations_policies = [ + # NOTE(gmaan): You might see this policy as deprecated in the new policies + # 'index:all_projects' and 'index:host' but it is not deprecated and still + # be used to list the migration without host info. By adding this existing + # policy in new policies deprecated field, oslo.policy will handle the + # policy overridden case. In that case, oslo.policy will pick the existing + # policy overridden value from policy.yaml file and apply the same to the + # new policy. This way existing deployment (for default as well as custom + # policy case) will not break. policy.DocumentedRuleDefault( name=POLICY_ROOT % 'index', - check_str=base.ADMIN, - description="List migrations", + check_str=base.PROJECT_MANAGER_OR_ADMIN, + description="List migrations without host info", operations=[ { 'method': 'GET', @@ -33,6 +57,40 @@ migrations_policies = [ } ], scope_types=['project']), + policy.DocumentedRuleDefault( + name=POLICY_ROOT % 'index:all_projects', + check_str=base.ADMIN, + description="List migrations for all or cross projects", + operations=[ + { + 'method': 'GET', + 'path': '/os-migrations' + } + ], + scope_types=['project'], + # TODO(gmaan): We can remove this after the next SLURP release + # (after 2026.1 release). We need to keep this deprecated rule + # for the case where operator has overridden the old policy + # 'index' in policy.yaml. For details, refer to the above + # comment in the 'index' policy rule. + deprecated_rule=DEPRECATED_POLICY), + policy.DocumentedRuleDefault( + name=POLICY_ROOT % 'index:host', + check_str=base.ADMIN, + description="List migrations with host info", + operations=[ + { + 'method': 'GET', + 'path': '/os-migrations' + } + ], + scope_types=['project'], + # TODO(gmaan): We can remove this after the next SLURP release + # (after 2026.1 release). We need to keep this deprecated rule + # for the case where operator has overridden the old policy + # 'index' in policy.yaml. For details, refer to the above + # comment in the 'index' policy rule. + deprecated_rule=DEPRECATED_POLICY), ] diff --git a/nova/policies/servers_migrations.py b/nova/policies/servers_migrations.py index 21762fc57595..7674daaf7894 100644 --- a/nova/policies/servers_migrations.py +++ b/nova/policies/servers_migrations.py @@ -20,6 +20,20 @@ from nova.policies import base POLICY_ROOT = 'os_compute_api:servers:migrations:%s' +DEPRECATED_REASON = """\ +Nova introduces one new policy to list live migrations. The original policy +is not deprecated and used to list the in-progress live migration without +host info. A new policy is added to list live migration with host info. +If you have overridden the original policy in your deployment, you must +also update the new policy to keep the same permissions. +""" + +DEPRECATED_POLICY = policy.DeprecatedRule( + POLICY_ROOT % 'index', + base.ADMIN, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='32.0.0' +) servers_migrations_policies = [ policy.DocumentedRuleDefault( @@ -36,7 +50,7 @@ servers_migrations_policies = [ scope_types=['project']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'force_complete', - check_str=base.ADMIN, + check_str=base.PROJECT_MANAGER_OR_ADMIN, description="Force an in-progress live migration for a given server " "to complete", operations=[ @@ -49,7 +63,7 @@ servers_migrations_policies = [ scope_types=['project']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'delete', - check_str=base.ADMIN, + check_str=base.PROJECT_MANAGER_OR_ADMIN, description="Delete(Abort) an in-progress live migration", operations=[ { @@ -58,10 +72,20 @@ servers_migrations_policies = [ } ], scope_types=['project']), + + # NOTE(gmaan): You might see this policy as deprecated in the new policy + # 'index:host' but it is not deprecated and still be used to list the live + # migration without host info. By adding this existing policy in new + # policy deprecated field, oslo.policy will handle the policy overridden + # case. In that case, oslo.policy will pick the existing policy overridden + # value from policy.yaml file and apply the same to the new policy. This + # way existing deployment (for default as well as custom policy case) will + # not break. policy.DocumentedRuleDefault( name=POLICY_ROOT % 'index', - check_str=base.ADMIN, - description="Lists in-progress live migrations for a given server", + check_str=base.PROJECT_MANAGER_OR_ADMIN, + description="Lists in-progress live migrations for a given server " + "without host info.", operations=[ { 'method': 'GET', @@ -69,6 +93,24 @@ servers_migrations_policies = [ } ], scope_types=['project']), + policy.DocumentedRuleDefault( + name=POLICY_ROOT % 'index:host', + check_str=base.ADMIN, + description="Lists in-progress live migrations for a given server " + "with host info.", + operations=[ + { + 'method': 'GET', + 'path': '/servers/{server_id}/migrations' + } + ], + scope_types=['project'], + # TODO(gmaan): We can remove this after the next SLURP release + # (after 2026.1 release). We need to keep this deprecated rule + # for the case where operator has overridden the old policy + # 'index' in policy.yaml. For details, refer to the above + # comment in the 'index' policy rule. + deprecated_rule=DEPRECATED_POLICY), ] diff --git a/nova/tests/functional/test_servers_resource_request.py b/nova/tests/functional/test_servers_resource_request.py index 9c91af72186b..c4bb8485987b 100644 --- a/nova/tests/functional/test_servers_resource_request.py +++ b/nova/tests/functional/test_servers_resource_request.py @@ -1521,6 +1521,7 @@ class NonAdminPortResourceRequestTests( 'os_compute_api:os-server-external-events:create': '@', 'os_compute_api:os-hypervisors:list': '@', 'os_compute_api:os-migrations:index': '@', + 'os_compute_api:os-migrations:index:all_projects': '@', 'os_compute_api:os-services:list': '@', }) @@ -1668,6 +1669,7 @@ class NonAdminMultiGroupResReqTests( 'os_compute_api:os-server-external-events:create': '@', 'os_compute_api:os-hypervisors:list': '@', 'os_compute_api:os-migrations:index': '@', + 'os_compute_api:os-migrations:index:all_projects': '@', 'os_compute_api:os-services:list': '@', }) @@ -2625,12 +2627,14 @@ class NonAdminServerMoveWithPortResourceRequestTests( 'os_compute_api:os-shelve:unshelve': '@', 'os_compute_api:os-migrate-server:migrate': '@', 'os_compute_api:os-migrate-server:migrate_live': '@', + 'os_compute_api:os-migrate-server:migrate_live:host': '@', 'os_compute_api:servers:resize': '@', 'os_compute_api:servers:confirm_resize': '@', 'os_compute_api:servers:revert_resize': '@', 'os_compute_api:os-evacuate': '@', 'os_compute_api:os-hypervisors:list': '@', 'os_compute_api:os-migrations:index': '@', + 'os_compute_api:os-migrations:index:all_projects': '@', 'os_compute_api:os-services:list': '@', 'compute:servers:create:requested_destination': '@', 'os_compute_api:os-instance-actions:show': '@', @@ -2675,12 +2679,14 @@ class NonAdminServerMoveWithMultiGroupResReqTests( 'os_compute_api:os-shelve:unshelve': '@', 'os_compute_api:os-migrate-server:migrate': '@', 'os_compute_api:os-migrate-server:migrate_live': '@', + 'os_compute_api:os-migrate-server:migrate_live:host': '@', 'os_compute_api:servers:resize': '@', 'os_compute_api:servers:confirm_resize': '@', 'os_compute_api:servers:revert_resize': '@', 'os_compute_api:os-evacuate': '@', 'os_compute_api:os-hypervisors:list': '@', 'os_compute_api:os-migrations:index': '@', + 'os_compute_api:os-migrations:index:all_projects': '@', 'os_compute_api:os-services:list': '@', 'compute:servers:create:requested_destination': '@', 'os_compute_api:os-instance-actions:show': '@', diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index 1423a426355a..e0c057e6bb41 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -49,6 +49,7 @@ policy_data = """ "os_compute_api:servers:allow_all_filters": "", "os_compute_api:servers:migrations:force_complete": "", "os_compute_api:servers:migrations:index": "", + "os_compute_api:servers:migrations:index:host": "", "os_compute_api:servers:migrations:show": "", "os_compute_api:servers:migrations:delete": "", "os_compute_api:os-admin-actions:inject_network_info": "", @@ -127,7 +128,10 @@ policy_data = """ "os_compute_api:os-migrate-server:migrate": "", "os_compute_api:os-migrate-server:migrate:host": "", "os_compute_api:os-migrate-server:migrate_live": "", + "os_compute_api:os-migrate-server:migrate_live:host": "", "os_compute_api:os-migrations:index": "", + "os_compute_api:os-migrations:index:all_projects": "", + "os_compute_api:os-migrations:index:host": "", "os_compute_api:os-multinic:add": "", "os_compute_api:os-multinic:remove": "", "os_compute_api:os-networks:list": "", diff --git a/nova/tests/unit/policies/base.py b/nova/tests/unit/policies/base.py index 6094a10c5f9a..2d1c0d68e717 100644 --- a/nova/tests/unit/policies/base.py +++ b/nova/tests/unit/policies/base.py @@ -168,16 +168,32 @@ class BasePolicyTest(test.TestCase): self.project_admin_context, self.project_manager_context, self.project_member_context, ]) - # With scope enable and legacy rule, only project scoped admin - # and any role in that project will have access. + # With scope disable and no legacy rule, any admin, + # project managers have access. No other role in that project + # will have access. + self.project_manager_or_admin_with_no_scope_no_legacy = set([ + self.legacy_admin_context, self.system_admin_context, + self.project_admin_context, self.project_manager_context, + ]) + # With scope enable and legacy rule, only admin with a project + # scope token and any role in that project will have access. self.project_m_r_or_admin_with_scope_and_legacy = set([ self.legacy_admin_context, self.project_admin_context, self.project_manager_context, self.project_member_context, self.project_reader_context, self.project_foo_context ]) - # With scope enable and no legacy rule, only project scoped admin - # and project members have access. No other role in that project - # or system scoped token will have access. + # With scope enable and no legacy rule, only admin with a + # project scope token and project managers have access. No + # other role in that project or system scoped token will + # have access. + self.project_manager_or_admin_with_scope_no_legacy = set([ + self.legacy_admin_context, self.project_admin_context, + self.project_manager_context + ]) + # With scope enable and no legacy rule, only admin with a + # project scope token and project members have access. No + # other role in that project or system scoped token will + # have access. self.project_member_or_admin_with_scope_no_legacy = set([ self.legacy_admin_context, self.project_admin_context, self.project_manager_context, self.project_member_context @@ -190,9 +206,10 @@ class BasePolicyTest(test.TestCase): self.project_admin_context, self.project_manager_context, self.project_member_context, self.project_reader_context ]) - # With scope enable and no legacy rule, only project scoped admin, - # project members, and project reader have access. No other role - # in that project or system scoped token will have access. + # With scope enable and no legacy rule, only admin with a + # project scope token, project members, and project reader + # have access. No other role in that project or system scoped + # token will have access. self.project_reader_or_admin_with_scope_no_legacy = set([ self.legacy_admin_context, self.project_admin_context, self.project_manager_context, self.project_member_context, @@ -280,6 +297,7 @@ class BasePolicyTest(test.TestCase): # At this time, we can call ensure_return() to assert the func's # response to ensure that changes are right. fatal = kwarg.pop('fatal', True) + fatal_auth = kwarg.pop('fatal_auth', True) authorized_response = [] unauthorize_response = [] @@ -321,7 +339,7 @@ class BasePolicyTest(test.TestCase): req.environ['nova.context'] = context args1 = copy.deepcopy(arg) kwargs1 = copy.deepcopy(kwarg) - if not fatal: + if not (fatal and fatal_auth): authorized_response.append( ensure_return(req, *args1, **kwargs1)) else: diff --git a/nova/tests/unit/policies/test_migrate_server.py b/nova/tests/unit/policies/test_migrate_server.py index 51ebcb28e8c8..cf728a2e3bbf 100644 --- a/nova/tests/unit/policies/test_migrate_server.py +++ b/nova/tests/unit/policies/test_migrate_server.py @@ -54,10 +54,14 @@ class MigrateServerPolicyTest(base.BasePolicyTest): self.legacy_admin_context, self.system_admin_context, self.project_admin_context] + self.project_manager_authorized_contexts = [ + self.legacy_admin_context, self.system_admin_context, + self.project_admin_context, self.project_manager_context] + @mock.patch('nova.compute.api.API.resize') def test_migrate_server_policy(self, mock_resize): rule_name = ms_policies.POLICY_ROOT % 'migrate' - self.common_policy_auth(self.project_admin_authorized_contexts, + self.common_policy_auth(self.project_manager_authorized_contexts, rule_name, self.controller._migrate, self.req, self.instance.uuid, body={'migrate': None}) @@ -75,6 +79,19 @@ class MigrateServerPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.API.live_migrate') def test_migrate_live_server_policy(self, mock_live_migrate): rule_name = ms_policies.POLICY_ROOT % 'migrate_live' + body = {'os-migrateLive': { + 'host': None, + 'block_migration': "False", + 'disk_over_commit': "False"} + } + self.common_policy_auth(self.project_manager_authorized_contexts, + rule_name, self.controller._migrate_live, + self.req, self.instance.uuid, + body=body) + + @mock.patch('nova.compute.api.API.live_migrate') + def test_migrate_live_server_host_policy(self, mock_live_migrate): + rule_name = ms_policies.POLICY_ROOT % 'migrate_live:host' body = {'os-migrateLive': { 'host': 'hostname', 'block_migration': "False", @@ -92,6 +109,18 @@ class MigrateServerNoLegacyNoScopeTest(MigrateServerPolicyTest): """ without_deprecated_rules = True + rules_without_deprecation = { + ms_policies.POLICY_ROOT % 'migrate': + base_policy.PROJECT_MANAGER_OR_ADMIN, + ms_policies.POLICY_ROOT % 'migrate_live': + base_policy.PROJECT_MANAGER_OR_ADMIN, + } + + def setUp(self): + super(MigrateServerNoLegacyNoScopeTest, self).setUp() + + self.project_manager_authorized_contexts = ( + self.project_manager_or_admin_with_no_scope_no_legacy) class MigrateServerScopeTypePolicyTest(MigrateServerPolicyTest): @@ -111,6 +140,9 @@ class MigrateServerScopeTypePolicyTest(MigrateServerPolicyTest): # With scope enabled, system admin is not allowed. self.project_admin_authorized_contexts = [ self.legacy_admin_context, self.project_admin_context] + self.project_manager_authorized_contexts = [ + self.legacy_admin_context, self.project_admin_context, + self.project_manager_context] class MigrateServerScopeTypeNoLegacyPolicyTest( @@ -119,6 +151,17 @@ class MigrateServerScopeTypeNoLegacyPolicyTest( and no more deprecated rules. """ without_deprecated_rules = True + rules_without_deprecation = { + ms_policies.POLICY_ROOT % 'migrate': + base_policy.PROJECT_MANAGER_OR_ADMIN, + ms_policies.POLICY_ROOT % 'migrate_live': + base_policy.PROJECT_MANAGER_OR_ADMIN, + } + + def setUp(self): + super(MigrateServerScopeTypeNoLegacyPolicyTest, self).setUp() + self.project_manager_authorized_contexts = ( + self.project_manager_or_admin_with_scope_no_legacy) class MigrateServerOverridePolicyTest( @@ -134,12 +177,14 @@ class MigrateServerOverridePolicyTest( rule_migrate = ms_policies.POLICY_ROOT % 'migrate' rule_migrate_host = ms_policies.POLICY_ROOT % 'migrate:host' rule_live_migrate = ms_policies.POLICY_ROOT % 'migrate_live' + rule_live_migrate_host = ms_policies.POLICY_ROOT % 'migrate_live:host' # NOTE(gmann): override the rule to project member and verify it # work as policy is system and project scoped. self.policy.set_rules({ rule_migrate: base_policy.PROJECT_MEMBER, rule_migrate_host: base_policy.PROJECT_MEMBER, - rule_live_migrate: base_policy.PROJECT_MEMBER}, + rule_live_migrate: base_policy.PROJECT_MEMBER, + rule_live_migrate_host: base_policy.PROJECT_MEMBER}, overwrite=False) # Check that project member role as override above @@ -147,3 +192,6 @@ class MigrateServerOverridePolicyTest( self.project_admin_authorized_contexts = [ self.project_admin_context, self.project_manager_context, self.project_member_context] + + self.project_manager_authorized_contexts = ( + self.project_admin_authorized_contexts) diff --git a/nova/tests/unit/policies/test_migrations.py b/nova/tests/unit/policies/test_migrations.py index 25cd75a12529..b2cea1f46397 100644 --- a/nova/tests/unit/policies/test_migrations.py +++ b/nova/tests/unit/policies/test_migrations.py @@ -10,13 +10,24 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime +import functools from unittest import mock +from oslo_utils.fixture import uuidsentinel as uuids + from nova.api.openstack.compute import migrations +import nova.conf +from nova import exception +from nova import objects +from nova.objects import base as obj_base +from nova.policies import base as base_policy from nova.policies import migrations as migrations_policies from nova.tests.unit.api.openstack import fakes from nova.tests.unit.policies import base +CONF = nova.conf.CONF + class MigrationsPolicyTest(base.BasePolicyTest): """Test Migrations APIs policies with all possible context. @@ -37,13 +48,225 @@ class MigrationsPolicyTest(base.BasePolicyTest): self.legacy_admin_context, self.system_admin_context, self.project_admin_context] + self.project_manager_authorized_contexts = [ + self.legacy_admin_context, self.system_admin_context, + self.project_admin_context, self.project_manager_context, + self.other_project_manager_context] + + def fake_migrations(self, project_id=None, same_project=None): + migration = [ + { + 'id': 1234, + 'source_node': 'node1', + 'dest_node': 'node2', + 'dest_compute_id': 123, + 'source_compute': 'compute1', + 'dest_compute': 'compute2', + 'dest_host': '1.2.3.4', + 'status': 'running', + 'instance_uuid': 1234, + 'old_instance_type_id': 1, + 'new_instance_type_id': 2, + 'migration_type': 'migration', + 'hidden': False, + 'memory_total': 123456, + 'memory_processed': 12345, + 'memory_remaining': 111111, + 'disk_total': 234567, + 'disk_processed': 23456, + 'disk_remaining': 211111, + 'created_at': datetime.datetime(2025, 1, 1), + 'updated_at': datetime.datetime(2025, 1, 1), + 'deleted_at': None, + 'deleted': False, + 'uuid': uuids.migration1, + 'cross_cell_move': False, + 'user_id': None, + 'project_id': 'other_project' + } + ] + if project_id is None: + return [migration[0], migration[0]] + if same_project: + migration[0]['project_id'] = project_id + return migration + @mock.patch('nova.compute.api.API.get_migrations') def test_list_migrations_policy(self, mock_migration): + rule = migrations_policies.POLICY_ROOT % 'index:all_projects' + # Migration 'index:all_projects' policy is checked after 'index' + # policy so we have to allow it to everyone so that we can test + # 'index' policy properly. + self.policy.set_rules({rule: "@"}, overwrite=False) + rule_name = migrations_policies.POLICY_ROOT % 'index' - self.common_policy_auth(self.project_admin_authorized_contexts, + self.common_policy_auth(self.project_manager_authorized_contexts, rule_name, self.controller.index, self.req) + @mock.patch('nova.compute.api.API.get_migrations') + def test_list_migrations_host_policy(self, mock_get): + fake_migration = self.fake_migrations() + mock_get.return_value = obj_base.obj_make_list( + 'fake-context', + objects.MigrationList(), + objects.Migration, + fake_migration) + + rule_1 = migrations_policies.POLICY_ROOT % 'index' + rule_2 = migrations_policies.POLICY_ROOT % 'index:all_projects' + # Migration 'index' and 'index:all_projects' policy are checked + # before 'index:host' policy so we have to allow those to everyone + # otherwise it will fail first for unauthorized contexts. + self.policy.set_rules({rule_1: "@", rule_2: "@"}, overwrite=False) + rule_name = migrations_policies.POLICY_ROOT % 'index:host' + authorize_res, unauthorize_res = self.common_policy_auth( + self.project_admin_authorized_contexts, + rule_name, self.controller.index, self.req, + fatal=False) + self.assertNotEqual(0, len(authorize_res)) + self.assertNotEqual(0, len(unauthorize_res)) + # NOTE(gmaan): Check host info is returned only in authorized + # context response. + for resp in authorize_res: + self.assertIn('compute2', resp['migrations'][0]['dest_compute']) + self.assertIn('1.2.3.4', resp['migrations'][0]['dest_host']) + self.assertIn('node2', resp['migrations'][0]['dest_node']) + self.assertIn('compute1', resp['migrations'][0]['source_compute']) + self.assertIn('node1', resp['migrations'][0]['source_node']) + self.assertIn('compute2', resp['migrations'][1]['dest_compute']) + self.assertIn('1.2.3.4', resp['migrations'][1]['dest_host']) + self.assertIn('node2', resp['migrations'][1]['dest_node']) + self.assertIn('compute1', resp['migrations'][1]['source_compute']) + self.assertIn('node1', resp['migrations'][1]['source_node']) + for resp in unauthorize_res: + self.assertIsNone(resp['migrations'][0]['dest_compute']) + self.assertIsNone(resp['migrations'][0]['dest_host']) + self.assertIsNone(resp['migrations'][0]['dest_node']) + self.assertIsNone(resp['migrations'][0]['source_compute']) + self.assertIsNone(resp['migrations'][0]['source_node']) + self.assertIsNone(resp['migrations'][1]['dest_compute']) + self.assertIsNone(resp['migrations'][1]['dest_host']) + self.assertIsNone(resp['migrations'][1]['dest_node']) + self.assertIsNone(resp['migrations'][1]['source_compute']) + self.assertIsNone(resp['migrations'][1]['source_node']) + + def test_list_migrations_check_primary_policy(self): + rule = migrations_policies.POLICY_ROOT % 'index' + # Migration 'index' policy is the primary policy and checked before + # 'index:host' and 'host:all_projects' policies so if 'index' policy + # is not allowed and even other policies are allowed then server + # migration list will be denied. + self.policy.set_rules({rule: "!"}, overwrite=False) + self.common_policy_auth( + set([]), + rule, self.controller.index, self.req) + + @mock.patch('nova.compute.api.API.get_migrations_sorted') + def test_list_all_projects_migrations_policy(self, mock_get): + fake_migration = self.fake_migrations() + mock_get.return_value = obj_base.obj_make_list( + 'fake-context', + objects.MigrationList(), + objects.Migration, + fake_migration) + + rule = migrations_policies.POLICY_ROOT % 'index' + # Migration 'index' policy is checked before 'index:all_projects' + # policy so we have to allow it for everyone otherwise it + # will fail first for unauthorized contexts. + self.policy.set_rules({rule: "@"}, overwrite=False) + rule_name = migrations_policies.POLICY_ROOT % 'index:all_projects' + if not CONF.oslo_policy.enforce_scope: + check_rule = rule_name + else: + check_rule = functools.partial( + base.rule_if_system, rule, rule_name) + req = fakes.HTTPRequest.blank('/os-migrations', version='2.80') + authorize_res, _ = self.common_policy_auth( + self.project_admin_authorized_contexts, + check_rule, self.controller.index, req, + fatal_auth=False) + # NOTE(gmaan): Make sure there are authorize context with response. + self.assertNotEqual(0, len(authorize_res)) + # NOTE(gmaan): Check all project migrations are returned + for resp in authorize_res: + self.assertEqual(2, len(resp['migrations'])) + + def prepare_microversion_request(self, cxtx, mock_get, project_id=None): + if not cxtx.project_id: + cxtx.project_id = 'fake' + project_id = project_id or cxtx.project_id + req = fakes.HTTPRequest.blank( + '/os-migrations?project_id=%s' % project_id, + version='2.80') + req.environ['nova.context'] = cxtx + fake_migration = self.fake_migrations( + project_id, same_project=(cxtx.project_id == project_id)) + mock_get.return_value = obj_base.obj_make_list( + 'fake-context', + objects.MigrationList(), + objects.Migration, + fake_migration) + return req, project_id + + @mock.patch('nova.compute.api.API.get_migrations_sorted') + def test_list_same_project_migrations_policy(self, mock_get): + rule_name = migrations_policies.POLICY_ROOT % 'index' + # NOTE(gmaan): Project manager should be able to get their + # own project migrations. + for auth_cxtx in self.project_manager_authorized_contexts: + req, project_id = self.prepare_microversion_request( + auth_cxtx, mock_get) + resp = self.controller.index(req) + # NOTE(gmaan): Check their own project migrations are returned + self.assertEqual(1, len(resp['migrations'])) + self.assertEqual(project_id, + resp['migrations'][0]['project_id']) + for unauth_cxtx in (self.all_contexts - + set(self.project_manager_authorized_contexts)): + req, project_id = self.prepare_microversion_request( + unauth_cxtx, mock_get) + exc = self.assertRaises( + exception.PolicyNotAuthorized, self.controller.index, req) + self.assertEqual( + "Policy doesn't allow %s to be performed." % rule_name, + exc.format_message()) + + @mock.patch('nova.compute.api.API.get_migrations_sorted') + def test_list_other_project_migrations_policy(self, mock_get): + rule_name = migrations_policies.POLICY_ROOT % 'index' + # NOTE(gmaan): Only Admin (not Project manager) can list the + # other project migrations. + for auth_cxtx in self.project_admin_authorized_contexts: + project_id = 'other_project' + req, _ = self.prepare_microversion_request( + auth_cxtx, mock_get, project_id=project_id) + resp = self.controller.index(req) + # NOTE(gmaan): Check their own project migrations are returned + self.assertEqual(1, len(resp['migrations'])) + self.assertEqual('other_project', + resp['migrations'][0]['project_id']) + for unauth_cxtx in (self.all_contexts - + set(self.project_admin_authorized_contexts)): + project_id = 'other_project' + req, _ = self.prepare_microversion_request( + unauth_cxtx, mock_get, project_id=project_id) + exc = self.assertRaises( + exception.PolicyNotAuthorized, self.controller.index, req) + rule_name = migrations_policies.POLICY_ROOT % 'index' + # NOTE(gmaan): Except system user, 'index' policy will allow + # 'manager' to access list migration. 'index:all_projects' policy + # will control if they can get their own or other project + # migrations. + if ('manager' in unauth_cxtx.roles and + unauth_cxtx.system_scope != 'all'): + rule_name = (migrations_policies.POLICY_ROOT + % 'index:all_projects') + self.assertEqual( + "Policy doesn't allow %s to be performed." % rule_name, + exc.format_message()) + class MigrationsNoLegacyNoScopeTest(MigrationsPolicyTest): """Test Migrations API policies with deprecated rules @@ -51,6 +274,22 @@ class MigrationsNoLegacyNoScopeTest(MigrationsPolicyTest): """ without_deprecated_rules = True + rules_without_deprecation = { + migrations_policies.POLICY_ROOT % 'index': + base_policy.PROJECT_MANAGER_OR_ADMIN, + } + + def setUp(self): + super(MigrationsNoLegacyNoScopeTest, self).setUp() + + # NOTE(gmaan): self.other_project_manager_context is in authorized + # context to list their own project migration only. They will not + # be able to get other project migration or host info in migrations. + self.project_manager_authorized_contexts = set([ + self.legacy_admin_context, self.system_admin_context, + self.project_admin_context, self.project_manager_context, + self.other_project_manager_context + ]) class MigrationsScopeTypePolicyTest(MigrationsPolicyTest): @@ -71,6 +310,10 @@ class MigrationsScopeTypePolicyTest(MigrationsPolicyTest): # With scope enabled, system admin is not allowed. self.project_admin_authorized_contexts = [ self.legacy_admin_context, self.project_admin_context] + self.project_manager_authorized_contexts = [ + self.legacy_admin_context, self.project_admin_context, + self.project_manager_context, + self.other_project_manager_context] class MigrationsScopeTypeNoLegacyPolicyTest( @@ -79,3 +322,18 @@ class MigrationsScopeTypeNoLegacyPolicyTest( and no more deprecated rules. """ without_deprecated_rules = True + rules_without_deprecation = { + migrations_policies.POLICY_ROOT % 'index': + base_policy.PROJECT_MANAGER_OR_ADMIN, + } + + def setUp(self): + super(MigrationsScopeTypeNoLegacyPolicyTest, self).setUp() + # NOTE(gmaan): This is end goal of new defaults. Only admin can get + # all project migrations with host info and manager role can only get + # their own project migrations but without host info. + self.project_manager_authorized_contexts = set([ + self.legacy_admin_context, self.project_admin_context, + self.project_manager_context, + self.other_project_manager_context + ]) diff --git a/nova/tests/unit/policies/test_server_migrations.py b/nova/tests/unit/policies/test_server_migrations.py index 05c0abde446a..dd9083fe30a7 100644 --- a/nova/tests/unit/policies/test_server_migrations.py +++ b/nova/tests/unit/policies/test_server_migrations.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime from unittest import mock import fixtures @@ -18,6 +19,7 @@ from oslo_utils.fixture import uuidsentinel as uuids from nova.api.openstack.compute import server_migrations from nova.compute import vm_states from nova import objects +from nova.objects import base as obj_base from nova.policies import base as base_policy from nova.policies import servers_migrations as policies from nova.tests.unit.api.openstack import fakes @@ -50,14 +52,94 @@ class ServerMigrationsPolicyTest(base.BasePolicyTest): self.project_admin_authorized_contexts = [ self.legacy_admin_context, self.system_admin_context, self.project_admin_context] + self.project_manager_authorized_contexts = [ + self.legacy_admin_context, self.system_admin_context, + self.project_admin_context, self.project_manager_context] @mock.patch('nova.compute.api.API.get_migrations_in_progress_by_instance') def test_list_server_migrations_policy(self, mock_get): rule_name = policies.POLICY_ROOT % 'index' - self.common_policy_auth(self.project_admin_authorized_contexts, + self.common_policy_auth(self.project_manager_authorized_contexts, rule_name, self.controller.index, self.req, self.instance.uuid) + @mock.patch('nova.compute.api.API.get_migrations_in_progress_by_instance') + def test_list_server_migrations_host_policy(self, mock_get): + fake_migrations = [ + { + 'id': 1234, + 'source_node': 'node1', + 'dest_node': 'node2', + 'dest_compute_id': 123, + 'source_compute': 'compute1', + 'dest_compute': 'compute2', + 'dest_host': '1.2.3.4', + 'status': 'running', + 'instance_uuid': self.instance.uuid, + 'old_instance_type_id': 1, + 'new_instance_type_id': 2, + 'migration_type': 'live-migration', + 'hidden': False, + 'memory_total': 123456, + 'memory_processed': 12345, + 'memory_remaining': 111111, + 'disk_total': 234567, + 'disk_processed': 23456, + 'disk_remaining': 211111, + 'created_at': datetime.datetime(2025, 1, 1), + 'updated_at': datetime.datetime(2025, 1, 1), + 'deleted_at': None, + 'deleted': False, + 'uuid': uuids.migration1, + 'cross_cell_move': False, + 'user_id': None, + 'project_id': None + }, + ] + + mock_get.return_value = obj_base.obj_make_list( + 'fake-context', + objects.MigrationList(), + objects.Migration, + fake_migrations) + + rule = policies.POLICY_ROOT % 'index' + # Migration 'index' policy is checked before 'index:host' + # policy so we have to allow it for everyone otherwise it + # will fail first for unauthorized contexts. + self.policy.set_rules({rule: "@"}, overwrite=False) + rule_name = policies.POLICY_ROOT % 'index:host' + authorize_res, unauthorize_res = self.common_policy_auth( + self.project_admin_authorized_contexts, + rule_name, self.controller.index, self.req, + self.instance.uuid, fatal=False) + # NOTE(gmaan): Check host info is returned only in authorized + # context response. + for resp in authorize_res: + self.assertIn('compute2', resp['migrations'][0]['dest_compute']) + self.assertIn('1.2.3.4', resp['migrations'][0]['dest_host']) + self.assertIn('node2', resp['migrations'][0]['dest_node']) + self.assertIn('compute1', resp['migrations'][0]['source_compute']) + self.assertIn('node1', resp['migrations'][0]['source_node']) + for resp in unauthorize_res: + self.assertIsNone(resp['migrations'][0]['dest_compute']) + self.assertIsNone(resp['migrations'][0]['dest_host']) + self.assertIsNone(resp['migrations'][0]['dest_node']) + self.assertIsNone(resp['migrations'][0]['source_compute']) + self.assertIsNone(resp['migrations'][0]['source_node']) + + def test_list_server_migrations_check_primary_policy(self): + rule = policies.POLICY_ROOT % 'index' + # Migration 'index' policy is the primary policy and checked before + # 'index:host' policy so if 'index' policy is not allowed and even + # 'index:host' policy is allowed then server migration list will be + # denied. + self.policy.set_rules({rule: "!"}, overwrite=False) + self.common_policy_auth( + set([]), + rule, self.controller.index, self.req, + self.instance.uuid) + @mock.patch('nova.api.openstack.compute.server_migrations.output') @mock.patch('nova.compute.api.API.get_migration_by_id_and_instance') def test_show_server_migrations_policy(self, mock_show, mock_output): @@ -73,14 +155,14 @@ class ServerMigrationsPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.API.live_migrate_abort') def test_delete_server_migrations_policy(self, mock_delete): rule_name = policies.POLICY_ROOT % 'delete' - self.common_policy_auth(self.project_admin_authorized_contexts, + self.common_policy_auth(self.project_manager_authorized_contexts, rule_name, self.controller.delete, self.req, self.instance.uuid, 11111) @mock.patch('nova.compute.api.API.live_migrate_force_complete') def test_force_delete_server_migrations_policy(self, mock_force): rule_name = policies.POLICY_ROOT % 'force_complete' - self.common_policy_auth(self.project_admin_authorized_contexts, + self.common_policy_auth(self.project_manager_authorized_contexts, rule_name, self.controller._force_complete, self.req, self.instance.uuid, 11111, body={"force_complete": None}) @@ -92,6 +174,19 @@ class ServerMigrationsNoLegacyNoScopeTest(ServerMigrationsPolicyTest): """ without_deprecated_rules = True + rules_without_deprecation = { + policies.POLICY_ROOT % 'force_complete': + base_policy.PROJECT_MANAGER_OR_ADMIN, + policies.POLICY_ROOT % 'delete': + base_policy.PROJECT_MANAGER_OR_ADMIN, + policies.POLICY_ROOT % 'index': + base_policy.PROJECT_MANAGER_OR_ADMIN, + } + + def setUp(self): + super(ServerMigrationsNoLegacyNoScopeTest, self).setUp() + self.project_manager_authorized_contexts = ( + self.project_manager_or_admin_with_no_scope_no_legacy) class ServerMigrationsScopeTypePolicyTest(ServerMigrationsPolicyTest): @@ -110,6 +205,9 @@ class ServerMigrationsScopeTypePolicyTest(ServerMigrationsPolicyTest): # With scope enabled, system admin is not allowed. self.project_admin_authorized_contexts = [ self.legacy_admin_context, self.project_admin_context] + self.project_manager_authorized_contexts = [ + self.legacy_admin_context, self.project_admin_context, + self.project_manager_context] class ServerMigrationsScopeTypeNoLegacyPolicyTest( @@ -118,6 +216,19 @@ class ServerMigrationsScopeTypeNoLegacyPolicyTest( and no more deprecated rules. """ without_deprecated_rules = True + rules_without_deprecation = { + policies.POLICY_ROOT % 'force_complete': + base_policy.PROJECT_MANAGER_OR_ADMIN, + policies.POLICY_ROOT % 'delete': + base_policy.PROJECT_MANAGER_OR_ADMIN, + policies.POLICY_ROOT % 'index': + base_policy.PROJECT_MANAGER_OR_ADMIN, + } + + def setUp(self): + super(ServerMigrationsScopeTypeNoLegacyPolicyTest, self).setUp() + self.project_manager_authorized_contexts = ( + self.project_manager_or_admin_with_scope_no_legacy) class ServerMigrationsOverridePolicyTest( @@ -132,6 +243,7 @@ class ServerMigrationsOverridePolicyTest( super(ServerMigrationsOverridePolicyTest, self).setUp() rule_show = policies.POLICY_ROOT % 'show' rule_list = policies.POLICY_ROOT % 'index' + rule_list_host = policies.POLICY_ROOT % 'index:host' rule_force = policies.POLICY_ROOT % 'force_complete' rule_delete = policies.POLICY_ROOT % 'delete' # NOTE(gmann): override the rule to project member and verify it @@ -139,6 +251,7 @@ class ServerMigrationsOverridePolicyTest( self.policy.set_rules({ rule_show: base_policy.PROJECT_READER, rule_list: base_policy.PROJECT_READER, + rule_list_host: base_policy.PROJECT_READER, rule_force: base_policy.PROJECT_READER, rule_delete: base_policy.PROJECT_READER}, overwrite=False) @@ -148,3 +261,5 @@ class ServerMigrationsOverridePolicyTest( self.project_admin_authorized_contexts = [ self.project_admin_context, self.project_manager_context, self.project_member_context, self.project_reader_context] + self.project_manager_authorized_contexts = ( + self.project_admin_authorized_contexts) diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 6511a9f5ba00..b74b96940370 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -304,9 +304,10 @@ class RealRolePolicyTestCase(test.NoDBTestCase): super(RealRolePolicyTestCase, self).setUp() self.policy = self.useFixture(nova_fixtures.RealPolicyFixture()) self.non_admin_context = context.RequestContext( - 'fake', 'fake', roles=['member', 'reader']) + 'fake', 'fake', roles=['manager', 'member', 'reader']) self.admin_context = context.RequestContext( - 'fake', 'fake', True, roles=['admin', 'member', 'reader']) + 'fake', 'fake', True, roles=[ + 'admin', 'manager', 'member', 'reader']) self.target = {} self.fake_policy = jsonutils.loads(fake_policy.policy_data) @@ -321,8 +322,6 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:servers:allow_all_filters", "os_compute_api:servers:show:host_status", "os_compute_api:servers:show:host_status:unknown-only", - "os_compute_api:servers:migrations:force_complete", - "os_compute_api:servers:migrations:delete", "os_compute_api:os-admin-actions:inject_network_info", "os_compute_api:os-admin-actions:reset_state", "os_compute_api:os-aggregates:index", @@ -349,9 +348,8 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:os-hosts:start", "os_compute_api:os-instance-actions:events", "os_compute_api:os-lock-server:unlock:unlock_override", - "os_compute_api:os-migrate-server:migrate", "os_compute_api:os-migrate-server:migrate:host", - "os_compute_api:os-migrate-server:migrate_live", + "os_compute_api:os-migrate-server:migrate_live:host", "os_compute_api:os-quota-sets:update", "os_compute_api:os-quota-sets:delete", "os_compute_api:os-server-diagnostics", @@ -370,10 +368,11 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:servers:create:zero_disk_flavor", "os_compute_api:os-baremetal-nodes:list", "os_compute_api:os-baremetal-nodes:show", - "os_compute_api:servers:migrations:index", + "os_compute_api:servers:migrations:index:host", "os_compute_api:servers:migrations:show", "os_compute_api:os-simple-tenant-usage:list", - "os_compute_api:os-migrations:index", + "os_compute_api:os-migrations:index:all_projects", + "os_compute_api:os-migrations:index:host", "os_compute_api:os-services:list", "os_compute_api:os-instance-actions:events:details", "os_compute_api:os-instance-usage-audit-log:list", @@ -426,6 +425,9 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:servers:delete", "os_compute_api:servers:detail", "os_compute_api:servers:index", + "os_compute_api:servers:migrations:force_complete", + "os_compute_api:servers:migrations:delete", + "os_compute_api:servers:migrations:index", "os_compute_api:servers:reboot", "os_compute_api:servers:rebuild", "os_compute_api:servers:rebuild:trusted_certs", @@ -448,6 +450,9 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:os-floating-ips:remove", "os_compute_api:os-floating-ips:create", "os_compute_api:os-floating-ips:delete", + "os_compute_api:os-migrate-server:migrate", + "os_compute_api:os-migrate-server:migrate_live", + "os_compute_api:os-migrations:index", "os_compute_api:os-multinic:add", "os_compute_api:os-multinic:remove", "os_compute_api:os-rescue", @@ -558,8 +563,10 @@ class RealRolePolicyTestCase(test.NoDBTestCase): # admin_only, non_admin, admin_or_user, empty_rule special_rules = ('admin_api', 'admin_or_owner', 'context_is_admin', 'os_compute_api:os-quota-class-sets:show', - 'project_admin_api', 'project_member_api', - 'project_reader_api', 'project_member_or_admin', + 'project_admin_api', 'project_manager_api', + 'project_member_api', 'project_reader_api', + 'project_manager_or_admin', + 'project_member_or_admin', 'project_reader_or_admin') result = set(rules.keys()) - set(self.admin_only_rules + self.admin_or_owner_rules + diff --git a/releasenotes/notes/add-policy-manager-role-e245ba669eb88b26.yaml b/releasenotes/notes/add-policy-manager-role-e245ba669eb88b26.yaml new file mode 100644 index 000000000000..214b4c932921 --- /dev/null +++ b/releasenotes/notes/add-policy-manager-role-e245ba669eb88b26.yaml @@ -0,0 +1,108 @@ +--- +features: + - | + The Nova policies introduce ``manager`` default roles provided by + keystone. A ``project_manager`` denoted by someone with the ``manager`` + role on a project. It is intended to perform more privileged operations + than ``project_member`` on its project resources. To avoid any change in + ``admin`` permissions, Nova use ``PROJECT_MANAGER_OR_ADMIN`` as default. + + Currently, nova supports: + + * ``admin`` + * ``project_manager`` + * ``project_member`` + * ``project_reader`` + + Currently, scope checks and new defaults are enabled by default. It is + recommended to use new defaults but if your deployment need more time + then you can disable them by switching the below config option in + ``nova.conf`` file.: + + [oslo_policy] + enforce_new_defaults=False + enforce_scope=False + + Please refer `Policy New Defaults`_ for detail about policy new defaults. + + In this release, the below APIs policy are default to + ``PROJECT_MANAGER_OR_ADMIN``: + + - ``os_compute_api:os-migrate-server:migrate`` ("Cold migrate a server + without specifying a host") + - ``os_compute_api:os-migrate-server:migrate_live`` (live migrate server + without specifying host) + - ``os_compute_api:os-migrations:index`` (List migrations without host + info) + - ``os_compute_api:servers:migrations:index`` (Lists in-progress live + migrations for a given server without host info) + - ``os_compute_api:servers:migrations:force_complete`` (Force an + in-progress live migration for a given server) + - ``os_compute_api:servers:migrations:delete`` (Delete(Abort) an + in-progress live migration) + + To introduced ``project_manager`` in migration APIs, we need to add a few + new policies. + + * Live migrate: + + - Existing policy is used when live migrate server without specifying + host: + + - ``os_compute_api:os-migrate-server:migrate_live`` (live migrate + server without specifying host) + - Default: ``PROJECT_MANAGER_OR_ADMIN`` + - New policy is used when live migrate server to a specific host: + + - ``os_compute_api:os-migrate-server:migrate_live:host`` (live migrate + server to a specific host) + - Default: ``ADMIN`` + + * List server migration: + + - Existing policy is used to list live migrations without host info: + + - ``os_compute_api:servers:migrations:index`` (Lists in-progress live + migrations for a given server) + - Default: ``PROJECT_MANAGER_OR_ADMIN`` + - New policy is used to host info in live migrations list: + + - ``os_compute_api:servers:migrations:index:host`` (Lists in-progress + live migrations for a given server with host info) + - Default: ``ADMIN`` + + * List migration: + + - Existing policy is used to list live migrations without host info: + + - ``os_compute_api:os-migrations:index`` (List migrations without + host info) + - Default: ``PROJECT_MANAGER_OR_ADMIN`` + - New policy is used to host info in live migrations list: + + - ``os_compute_api:os-migrations:index:all_projects`` (List migrations + for all or cross projects) + - Default: ``ADMIN`` + - ``os_compute_api:os-migrations:index:host`` (List migrations + with host info) + - Default: ``ADMIN`` +upgrade: + - | + New policies are added to the live migration APIs with the same default. + If you are using default policy, then no action is needed, but if you have + overridden the existing live migration policies in your deployment, you + must include the new policy with the same permissions. + + - Existing policy: + + - ``os_compute_api:os-migrate-server:migrate_live`` + - ``os_compute_api:servers:migrations::index`` + - ``os_compute_api:os-migrations:index`` + - New policy: + + - ``os_compute_api:os-migrate-server:migrate_live:host`` + - ``os_compute_api:servers:migrations:index:host`` + - ``os_compute_api:os-migrations:index:all_projects`` + - ``os_compute_api:os-migrations:index:host`` + + .. _Policy New Defaults: https://docs.openstack.org/nova/latest/configuration/policy-concepts.html