From 76d3457bd9fd7ec597fd0fdc6cff8fdbdcec8aca Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Sat, 18 Apr 2020 13:02:55 -0500 Subject: [PATCH] Add new default roles in servers attributes policies This adds new defaults roles in servers extended attributes and host_status policies as SYSTEM_ADMIN but passing project_id of server as these policies are scoped with system and project. Also add tests to simulates the future where we drop the deprecation fall back in the policy by overriding the rules with a version where there are no deprecated rule options. Operators can do the same by adding overrides in their policy files that match the default but stop the rule deprecation fallback from happening. Partial implement blueprint policy-defaults-refresh Change-Id: I55a47d447f5f5b2d18754027f69e05e729d40338 --- nova/api/openstack/compute/views/servers.py | 19 ++++++++++++++----- nova/policies/extended_server_attributes.py | 2 +- nova/policies/servers.py | 4 ++-- nova/tests/unit/policies/test_servers.py | 14 ++++++++++++++ 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/nova/api/openstack/compute/views/servers.py b/nova/api/openstack/compute/views/servers.py index 9747cfa249b5..6c9f1006e653 100644 --- a/nova/api/openstack/compute/views/servers.py +++ b/nova/api/openstack/compute/views/servers.py @@ -184,7 +184,7 @@ class ViewBuilder(common.ViewBuilder): return ret @staticmethod - def _get_host_status_unknown_only(context): + def _get_host_status_unknown_only(context, instance=None): """We will use the unknown_only variable to tell us what host status we can show, if any: * unknown_only = False means we can show any host status. @@ -199,16 +199,23 @@ class ViewBuilder(common.ViewBuilder): # Check show:host_status policy first because if it passes, we know we # can show any host status and need not check the more restrictive # show:host_status:unknown-only policy. + # Keeping target as None (which means policy will default these target + # to context.project_id) for now which is case of 'detail' API which + # policy is default to system and project reader. + target = None + if instance is not None: + target = {'project_id': instance.project_id} if context.can( servers_policies.SERVERS % 'show:host_status', - fatal=False): + fatal=False, target=target): unknown_only = False # If we are not allowed to show any/all host status, check if we can at # least show only the host status: UNKNOWN. elif context.can( servers_policies.SERVERS % 'show:host_status:unknown-only', - fatal=False): + fatal=False, + target=target): unknown_only = True return unknown_only @@ -304,7 +311,8 @@ class ViewBuilder(common.ViewBuilder): if show_extended_attr is None: show_extended_attr = context.can( - esa_policies.BASE_POLICY_NAME, fatal=False) + esa_policies.BASE_POLICY_NAME, fatal=False, + target={'project_id': instance.project_id}) if show_extended_attr: properties = ['host', 'name', 'node'] if api_version_request.is_supported(request, min_version='2.3'): @@ -358,7 +366,8 @@ class ViewBuilder(common.ViewBuilder): add_delete_on_termination) if (api_version_request.is_supported(request, min_version='2.16')): if show_host_status is None: - unknown_only = self._get_host_status_unknown_only(context) + unknown_only = self._get_host_status_unknown_only( + context, instance) # If we're not allowed by policy to show host status at all, # don't bother requesting instance host status from the compute # API. diff --git a/nova/policies/extended_server_attributes.py b/nova/policies/extended_server_attributes.py index ab925f7c440e..9a01285980b0 100644 --- a/nova/policies/extended_server_attributes.py +++ b/nova/policies/extended_server_attributes.py @@ -24,7 +24,7 @@ BASE_POLICY_NAME = 'os_compute_api:os-extended-server-attributes' extended_server_attributes_policies = [ policy.DocumentedRuleDefault( name=BASE_POLICY_NAME, - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="""Return extended attributes for server. This rule will control the visibility for a set of servers attributes: diff --git a/nova/policies/servers.py b/nova/policies/servers.py index 5c3b4078e8fe..92a0ed7ff9f7 100644 --- a/nova/policies/servers.py +++ b/nova/policies/servers.py @@ -99,7 +99,7 @@ rules = [ # should do that by default. policy.DocumentedRuleDefault( name=SERVERS % 'show:host_status', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description=""" Show a server with additional host status information. @@ -133,7 +133,7 @@ API responses which are also controlled by this policy rule, like the scope_types=['system', 'project']), policy.DocumentedRuleDefault( name=SERVERS % 'show:host_status:unknown-only', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description=""" Show a server with additional host status information, only if host status is UNKNOWN. diff --git a/nova/tests/unit/policies/test_servers.py b/nova/tests/unit/policies/test_servers.py index 57409fd451fd..10be3130283b 100644 --- a/nova/tests/unit/policies/test_servers.py +++ b/nova/tests/unit/policies/test_servers.py @@ -1309,3 +1309,17 @@ class ServersNoLegacyPolicyTest(ServersScopeTypePolicyTest): self.project_foo_context, self.other_project_reader_context, self.system_reader_context, self.system_foo_context ] + # Check that system admin is able to get server extended attributes + # or host status. + self.server_attr_admin_authorized_contexts = [ + self.system_admin_context] + # Check that non-system admin is not able to get server extended + # attributes or host status. + self.server_attr_admin_unauthorized_contexts = [ + self.legacy_admin_context, self.project_admin_context, + self.system_member_context, self.system_reader_context, + self.system_foo_context, self.project_member_context, + self.project_reader_context, self.project_foo_context, + self.other_project_member_context, + self.other_project_reader_context + ]