From 183aff322568e38ee5b6348907b7b83563f24fcd Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Thu, 9 Jan 2020 01:22:22 +0000 Subject: [PATCH] Add new default roles in os-agents policies This adds new defaults roles in os-agents API policies. This policy is default to system admin role. Policy rules are made more granular to adopt the new defaults. Partial implement blueprint policy-defaults-refresh Change-Id: I9b094ecd3c5ff4f56ecfafb72c2a660b4146035f --- nova/api/openstack/compute/agents.py | 8 +-- nova/policies/agents.py | 64 +++++++++++++++-- nova/tests/unit/fake_policy.py | 5 +- nova/tests/unit/policies/test_agents.py | 95 +++++++++++++++++++++++-- nova/tests/unit/test_policy.py | 5 +- 5 files changed, 159 insertions(+), 18 deletions(-) diff --git a/nova/api/openstack/compute/agents.py b/nova/api/openstack/compute/agents.py index c2578126c384..8f6a9c1bf861 100644 --- a/nova/api/openstack/compute/agents.py +++ b/nova/api/openstack/compute/agents.py @@ -52,7 +52,7 @@ class AgentController(wsgi.Controller): def index(self, req): """Return a list of all agent builds. Filter by hypervisor.""" context = req.environ['nova.context'] - context.can(agents_policies.BASE_POLICY_NAME) + context.can(agents_policies.BASE_POLICY_NAME % 'list') hypervisor = None agents = [] if 'hypervisor' in req.GET: @@ -75,7 +75,7 @@ class AgentController(wsgi.Controller): def update(self, req, id, body): """Update an existing agent build.""" context = req.environ['nova.context'] - context.can(agents_policies.BASE_POLICY_NAME) + context.can(agents_policies.BASE_POLICY_NAME % 'update') # TODO(oomichi): This parameter name "para" is different from the ones # of the other APIs. Most other names are resource names like "server" @@ -118,7 +118,7 @@ class AgentController(wsgi.Controller): def delete(self, req, id): """Deletes an existing agent build.""" context = req.environ['nova.context'] - context.can(agents_policies.BASE_POLICY_NAME) + context.can(agents_policies.BASE_POLICY_NAME % 'delete') try: utils.validate_integer(id, 'id') @@ -140,7 +140,7 @@ class AgentController(wsgi.Controller): def create(self, req, body): """Creates a new agent build.""" context = req.environ['nova.context'] - context.can(agents_policies.BASE_POLICY_NAME) + context.can(agents_policies.BASE_POLICY_NAME % 'create') agent = body['agent'] hypervisor = agent['hypervisor'] diff --git a/nova/policies/agents.py b/nova/policies/agents.py index cc6dc7441a28..eb420b8dca1b 100644 --- a/nova/policies/agents.py +++ b/nova/policies/agents.py @@ -18,15 +18,24 @@ from oslo_policy import policy from nova.policies import base -BASE_POLICY_NAME = 'os_compute_api:os-agents' +BASE_POLICY_NAME = 'os_compute_api:os-agents:%s' +DEPRECATED_AGENTS_POLICY = policy.DeprecatedRule( + 'os_compute_api:os-agents', + base.RULE_ADMIN_API, +) + +DEPRECATED_REASON = """ +Nova API policies are introducing new default roles with scope_type +capabilities. Old policies are deprecated and silently going to be ignored +in nova 23.0.0 release. +""" agents_policies = [ policy.DocumentedRuleDefault( - name=BASE_POLICY_NAME, - check_str=base.RULE_ADMIN_API, - description="""Create, list, update, and delete guest agent builds - + name=BASE_POLICY_NAME % 'list', + check_str=base.SYSTEM_READER, + description="""List guest agent builds This is XenAPI driver specific. It is used to force the upgrade of the XenAPI guest agent on instance boot. """, @@ -35,20 +44,63 @@ It is used to force the upgrade of the XenAPI guest agent on instance boot. 'path': '/os-agents', 'method': 'GET' }, + ], + scope_types=['system'], + deprecated_rule=DEPRECATED_AGENTS_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'create', + check_str=base.SYSTEM_ADMIN, + description="""Create guest agent builds +This is XenAPI driver specific. +It is used to force the upgrade of the XenAPI guest agent on instance boot. +""", + operations=[ + { 'path': '/os-agents', 'method': 'POST' }, + ], + scope_types=['system'], + deprecated_rule=DEPRECATED_AGENTS_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'update', + check_str=base.SYSTEM_ADMIN, + description="""Update guest agent builds +This is XenAPI driver specific. +It is used to force the upgrade of the XenAPI guest agent on instance boot. +""", + operations=[ { 'path': '/os-agents/{agent_build_id}', 'method': 'PUT' }, + ], + scope_types=['system'], + deprecated_rule=DEPRECATED_AGENTS_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'delete', + check_str=base.SYSTEM_ADMIN, + description="""Delete guest agent builds +This is XenAPI driver specific. +It is used to force the upgrade of the XenAPI guest agent on instance boot. +""", + operations=[ { 'path': '/os-agents/{agent_build_id}', 'method': 'DELETE' } ], - scope_types=['system']), + scope_types=['system'], + deprecated_rule=DEPRECATED_AGENTS_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), ] diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index f9bc531c5803..e508a9a4c14a 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -25,7 +25,10 @@ policy_data = """ "os_compute_api:os-admin-actions:reset_network": "", "os_compute_api:os-admin-actions:reset_state": "", "os_compute_api:os-admin-password": "", - "os_compute_api:os-agents": "", + "os_compute_api:os-agents:list": "", + "os_compute_api:os-agents:create": "", + "os_compute_api:os-agents:update": "", + "os_compute_api:os-agents:delete": "", "os_compute_api:os-attach-interfaces:list": "", "os_compute_api:os-attach-interfaces:show": "", "os_compute_api:os-attach-interfaces:create": "", diff --git a/nova/tests/unit/policies/test_agents.py b/nova/tests/unit/policies/test_agents.py index 0905f109a426..47db55c543da 100644 --- a/nova/tests/unit/policies/test_agents.py +++ b/nova/tests/unit/policies/test_agents.py @@ -14,8 +14,11 @@ import mock from nova.api.openstack.compute import agents from nova.db.sqlalchemy import models +from nova import exception +from nova.policies import base as base_policy from nova.tests.unit.api.openstack import fakes from nova.tests.unit.policies import base +from nova.tests.unit import policy_fixture class AgentsPolicyTest(base.BasePolicyTest): @@ -44,9 +47,27 @@ class AgentsPolicyTest(base.BasePolicyTest): self.project_foo_context, self.project_reader_context ] + # Check that system scoped admin, member and reader are able to + # read the agent data. + # NOTE(gmann): Until old default rule which is admin_api is + # deprecated and not removed, project admin and legacy admin + # will be able to read the agent data. This make sure that existing + # tokens will keep working even we have changed this policy defaults + # to reader role. + self.reader_authorized_contexts = [ + self.system_admin_context, self.system_member_context, + self.system_reader_context, self.legacy_admin_context, + self.project_admin_context] + # Check that non-system-reader are not able to read the agent + # data + self.reader_unauthorized_contexts = [ + self.system_foo_context, self.other_project_member_context, + self.project_foo_context, self.project_member_context, + self.project_reader_context] + @mock.patch('nova.db.api.agent_build_destroy') def test_delete_agent_policy(self, mock_delete): - rule_name = "os_compute_api:os-agents" + rule_name = "os_compute_api:os-agents:delete" self.common_policy_check(self.admin_authorized_contexts, self.admin_unauthorized_contexts, rule_name, self.controller.delete, @@ -54,15 +75,15 @@ class AgentsPolicyTest(base.BasePolicyTest): @mock.patch('nova.db.api.agent_build_get_all') def test_index_agents_policy(self, mock_get): - rule_name = "os_compute_api:os-agents" - self.common_policy_check(self.admin_authorized_contexts, - self.admin_unauthorized_contexts, + rule_name = "os_compute_api:os-agents:list" + self.common_policy_check(self.reader_authorized_contexts, + self.reader_unauthorized_contexts, rule_name, self.controller.index, self.req) @mock.patch('nova.db.api.agent_build_update') def test_update_agent_policy(self, mock_update): - rule_name = "os_compute_api:os-agents" + rule_name = "os_compute_api:os-agents:update" body = {'para': {'version': '7.0', 'url': 'http://example.com/path/to/resource', 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} @@ -73,7 +94,7 @@ class AgentsPolicyTest(base.BasePolicyTest): self.req, 1, body=body) def test_create_agent_policy(self): - rule_name = "os_compute_api:os-agents" + rule_name = "os_compute_api:os-agents:create" body = {'agent': {'hypervisor': 'kvm', 'os': 'win', 'architecture': 'x86', @@ -123,3 +144,65 @@ class AgentsScopeTypePolicyTest(AgentsPolicyTest): self.other_project_member_context, self.project_foo_context, self.project_reader_context ] + + # Check that system admin, member and reader are able to read the + # agent data + self.reader_authorized_contexts = [ + self.system_admin_context, self.system_member_context, + self.system_reader_context] + # Check that non-system or non-reader are not able to read the agent + # data + self.reader_unauthorized_contexts = [ + self.system_foo_context, self.legacy_admin_context, + self.project_admin_context, self.project_member_context, + self.other_project_member_context, + self.project_foo_context, self.project_reader_context + ] + + +class AgentsDeprecatedPolicyTest(base.BasePolicyTest): + """Test os-agents APIs Deprecated policies. + This class checks if deprecated policy rules are + overridden by user on policy.json file then they + still work because oslo.policy add deprecated rules + in logical OR condition and enforce them for policy + checks if overridden. + """ + + def setUp(self): + super(AgentsDeprecatedPolicyTest, self).setUp() + self.controller = agents.AgentController() + self.admin_req = fakes.HTTPRequest.blank('') + self.admin_req.environ['nova.context'] = self.project_admin_context + self.reader_req = fakes.HTTPRequest.blank('') + self.reader_req.environ['nova.context'] = self.project_reader_context + self.deprecated_policy = "os_compute_api:os-agents" + # Overridde rule with different checks than defaults so that we can + # verify the rule overridden case. + override_rules = {self.deprecated_policy: base_policy.RULE_ADMIN_API} + # NOTE(gmann): Only override the deprecated rule in policy file so + # that we can verify if overridden checks are considered by + # oslo.policy. Oslo.policy will consider the overridden rules if: + # 1. overridden deprecated rule's checks are different than defaults + # 2. new rules are not present in policy file + self.policy = self.useFixture(policy_fixture.OverridePolicyFixture( + rules_in_file=override_rules)) + + def test_deprecated_policy_overridden_rule_is_checked(self): + # Test to verify if deprecatd overridden policy is working. + + # check for success as admin role. Deprecated rule + # has been overridden with admin checks in policy.json + # If admin role pass it means overridden rule is enforced by + # olso.policy because new default is system reader and the old + # default is admin. + with mock.patch('nova.db.api.agent_build_get_all'): + self.controller.index(self.admin_req) + + # check for failure with reader context. + exc = self.assertRaises(exception.PolicyNotAuthorized, + self.controller.index, self.reader_req) + self.assertEqual( + "Policy doesn't allow os_compute_api:os-agents:list to be" + " performed.", + exc.format_message()) diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 83409c3c3515..268a150a285b 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -328,7 +328,9 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:os-aggregates:add_host", "os_compute_api:os-aggregates:remove_host", "os_compute_api:os-aggregates:set_metadata", -"os_compute_api:os-agents", +"os_compute_api:os-agents:create", +"os_compute_api:os-agents:update", +"os_compute_api:os-agents:delete", "os_compute_api:os-baremetal-nodes", "os_compute_api:os-evacuate", "os_compute_api:os-extended-server-attributes", @@ -458,6 +460,7 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:os-instance-actions:events:details", "os_compute_api:os-instance-usage-audit-log:list", "os_compute_api:os-instance-usage-audit-log:show", +"os_compute_api:os-agents:list", ) self.system_reader_or_owner_rules = (