diff --git a/nova/api/openstack/compute/flavor_manage.py b/nova/api/openstack/compute/flavor_manage.py index 1e8dc5c87795..d03071fc657f 100644 --- a/nova/api/openstack/compute/flavor_manage.py +++ b/nova/api/openstack/compute/flavor_manage.py @@ -12,6 +12,8 @@ import webob +from oslo_log import log as logging + from nova.api.openstack.compute.schemas import flavor_manage from nova.api.openstack.compute.views import flavors as flavors_view from nova.api.openstack import extensions @@ -21,8 +23,12 @@ from nova.compute import flavors from nova import exception from nova.i18n import _ from nova import objects +from nova.policies import base from nova.policies import flavor_manage as fm_policies +from nova import policy + +LOG = logging.getLogger(__name__) ALIAS = "os-flavor-manage" @@ -41,7 +47,15 @@ class FlavorManageController(wsgi.Controller): @wsgi.action("delete") def _delete(self, req, id): context = req.environ['nova.context'] - context.can(fm_policies.BASE_POLICY_NAME) + # TODO(rb560u): remove this check in future release + using_old_action = \ + policy.verify_deprecated_policy(fm_policies.BASE_POLICY_NAME, + fm_policies.POLICY_ROOT % 'delete', + base.RULE_ADMIN_API, + context) + + if not using_old_action: + context.can(fm_policies.POLICY_ROOT % 'delete') flavor = objects.Flavor(context=context, flavorid=id) try: @@ -57,7 +71,15 @@ class FlavorManageController(wsgi.Controller): @validation.schema(flavor_manage.create, '2.1') def _create(self, req, body): context = req.environ['nova.context'] - context.can(fm_policies.BASE_POLICY_NAME) + # TODO(rb560u): remove this check in future release + using_old_action = \ + policy.verify_deprecated_policy(fm_policies.BASE_POLICY_NAME, + fm_policies.POLICY_ROOT % 'create', + base.RULE_ADMIN_API, + context) + + if not using_old_action: + context.can(fm_policies.POLICY_ROOT % 'create') vals = body['flavor'] diff --git a/nova/policies/flavor_manage.py b/nova/policies/flavor_manage.py index 255df7d97bb9..ba12143e58ce 100644 --- a/nova/policies/flavor_manage.py +++ b/nova/policies/flavor_manage.py @@ -20,13 +20,17 @@ from nova.policies import base BASE_POLICY_NAME = 'os_compute_api:os-flavor-manage' +POLICY_ROOT = 'os_compute_api:os-flavor-manage:%s' +BASE_POLICY_RULE = 'rule:%s' % BASE_POLICY_NAME flavor_manage_policies = [ + # TODO(rb560u): remove this rule in future release policy.DocumentedRuleDefault( BASE_POLICY_NAME, base.RULE_ADMIN_API, - "Create and delete Flavors", + "Create and delete Flavors. Deprecated in Pike and will be " + "removed in future release", [ { 'method': 'POST', @@ -38,6 +42,26 @@ flavor_manage_policies = [ }, ]), + policy.DocumentedRuleDefault( + POLICY_ROOT % 'create', + BASE_POLICY_RULE, + "Create a flavor", + [ + { + 'method': 'POST', + 'path': '/flavors' + } + ]), + policy.DocumentedRuleDefault( + POLICY_ROOT % 'delete', + BASE_POLICY_RULE, + "Delete a flavor", + [ + { + 'method': 'DELETE', + 'path': '/flavors/{flavor_id}' + } + ]), ] diff --git a/nova/policy.py b/nova/policy.py index 843c4268988f..4324fbc61c07 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -225,3 +225,33 @@ def get_enforcer(): cfg.CONF(conf_args, project='nova') init() return _ENFORCER + + +def verify_deprecated_policy(old_policy, new_policy, default_rule, context): + """Check the rule of the deprecated policy action + + If the current rule of the deprecated policy action is set to a non-default + value, then a warning message is logged stating that the new policy + action should be used to dictate permissions as the old policy action is + being deprecated. + + :param old_policy: policy action that is being deprecated + :param new_policy: policy action that is replacing old_policy + :param default_rule: the old_policy action default rule value + :param context: the nova context + """ + + if _ENFORCER: + current_rule = str(_ENFORCER.rules[old_policy]) + else: + current_rule = None + + if current_rule != default_rule: + LOG.warning("Start using the new action '{0}'. The existing " + "action '{1}' is being deprecated and will be " + "removed in future release.".format(new_policy, + old_policy)) + context.can(old_policy) + return True + else: + return False diff --git a/nova/tests/unit/api/openstack/compute/test_flavor_manage.py b/nova/tests/unit/api/openstack/compute/test_flavor_manage.py index 3dbc58f88f01..c4aae37bc649 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavor_manage.py +++ b/nova/tests/unit/api/openstack/compute/test_flavor_manage.py @@ -23,6 +23,7 @@ from nova.api.openstack.compute import flavor_manage as flavormanage_v21 from nova.compute import flavors from nova import db from nova import exception +from nova import policy from nova import test from nova.tests.unit.api.openstack import fakes @@ -373,19 +374,20 @@ class PrivateFlavorManageTestV21(test.TestCase): self.assertEqual(body["flavor"][key], self.expected["flavor"][key]) -class FlavorManagerPolicyEnforcementV21(test.NoDBTestCase): +class FlavorManagerPolicyEnforcementV21(test.TestCase): def setUp(self): super(FlavorManagerPolicyEnforcementV21, self).setUp() self.controller = flavormanage_v21.FlavorManageController() + self.adm_req = fakes.HTTPRequest.blank('', use_admin_context=True) + self.req = fakes.HTTPRequest.blank('') def test_create_policy_failed(self): rule_name = "os_compute_api:os-flavor-manage" self.policy.set_rules({rule_name: "project:non_fake"}) - req = fakes.HTTPRequest.blank('') exc = self.assertRaises( exception.PolicyNotAuthorized, - self.controller._create, req, + self.controller._create, self.req, body={"flavor": { "name": "test", "ram": 512, @@ -394,6 +396,8 @@ class FlavorManagerPolicyEnforcementV21(test.NoDBTestCase): "swap": 512, "rxtx_factor": 1, }}) + # The deprecated action is being enforced since the rule that is + # configured is different than the default rule self.assertEqual( "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) @@ -401,11 +405,172 @@ class FlavorManagerPolicyEnforcementV21(test.NoDBTestCase): def test_delete_policy_failed(self): rule_name = "os_compute_api:os-flavor-manage" self.policy.set_rules({rule_name: "project:non_fake"}) - req = fakes.HTTPRequest.blank('') exc = self.assertRaises( exception.PolicyNotAuthorized, - self.controller._delete, req, + self.controller._delete, self.req, fakes.FAKE_UUID) + # The deprecated action is being enforced since the rule that is + # configured is different than the default rule self.assertEqual( "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) + + @mock.patch.object(policy.LOG, 'warning') + def test_create_policy_rbac_inherit_default(self, mock_warning): + """Test to verify inherited rule is working. The rule of the + deprecated action is not set to the default, so the deprecated + action is being enforced + """ + + default_flavor_policy = "os_compute_api:os-flavor-manage" + create_flavor_policy = "os_compute_api:os-flavor-manage:create" + rules = {default_flavor_policy: 'is_admin:True', + create_flavor_policy: 'rule:%s' % default_flavor_policy} + self.policy.set_rules(rules) + body = { + "flavor": { + "name": "azAZ09. -_", + "ram": 512, + "vcpus": 2, + "disk": 1, + "OS-FLV-EXT-DATA:ephemeral": 1, + "id": six.text_type('1234'), + "swap": 512, + "rxtx_factor": 1, + "os-flavor-access:is_public": True, + } + } + # check for success as admin + self.controller._create(self.adm_req, body=body) + # check for failure as non-admin + exc = self.assertRaises(exception.PolicyNotAuthorized, + self.controller._create, self.req, + body=body) + # The deprecated action is being enforced since the rule that is + # configured is different than the default rule + self.assertEqual( + "Policy doesn't allow %s to be performed." % default_flavor_policy, + exc.format_message()) + mock_warning.assert_called_with("Start using the new " + "action '{0}'. The existing action '{1}' is being deprecated and " + "will be removed in future release.".format(create_flavor_policy, + default_flavor_policy)) + + @mock.patch.object(policy.LOG, 'warning') + def test_delete_policy_rbac_inherit_default(self, mock_warning): + """Test to verify inherited rule is working. The rule of the + deprecated action is not set to the default, so the deprecated + action is being enforced + """ + + default_flavor_policy = "os_compute_api:os-flavor-manage" + create_flavor_policy = "os_compute_api:os-flavor-manage:create" + delete_flavor_policy = "os_compute_api:os-flavor-manage:delete" + rules = {default_flavor_policy: 'is_admin:True', + create_flavor_policy: 'rule:%s' % default_flavor_policy, + delete_flavor_policy: 'rule:%s' % default_flavor_policy} + self.policy.set_rules(rules) + body = { + "flavor": { + "name": "azAZ09. -_", + "ram": 512, + "vcpus": 2, + "disk": 1, + "OS-FLV-EXT-DATA:ephemeral": 1, + "id": six.text_type('1234'), + "swap": 512, + "rxtx_factor": 1, + "os-flavor-access:is_public": True, + } + } + self.flavor = self.controller._create(self.adm_req, body=body) + mock_warning.assert_called_once_with("Start using the new " + "action '{0}'. The existing action '{1}' is being deprecated and " + "will be removed in future release.".format(create_flavor_policy, + default_flavor_policy)) + # check for success as admin + flavor = self.flavor + self.controller._delete(self.adm_req, flavor['flavor']['id']) + # check for failure as non-admin + flavor = self.flavor + exc = self.assertRaises(exception.PolicyNotAuthorized, + self.controller._delete, self.req, + flavor['flavor']['id']) + # The deprecated action is being enforced since the rule that is + # configured is different than the default rule + self.assertEqual( + "Policy doesn't allow %s to be performed." % default_flavor_policy, + exc.format_message()) + mock_warning.assert_called_with("Start using the new " + "action '{0}'. The existing action '{1}' is being deprecated and " + "will be removed in future release.".format(delete_flavor_policy, + default_flavor_policy)) + + def test_create_policy_rbac_no_change_to_default_action_rule(self): + """Test to verify the correct action is being enforced. When the + rule configured for the deprecated action is the same as the + default, the new action should be enforced. + """ + + default_flavor_policy = "os_compute_api:os-flavor-manage" + create_flavor_policy = "os_compute_api:os-flavor-manage:create" + # The default rule of the deprecated action is admin_api + rules = {default_flavor_policy: 'rule:admin_api', + create_flavor_policy: 'rule:%s' % default_flavor_policy} + self.policy.set_rules(rules) + body = { + "flavor": { + "name": "azAZ09. -_", + "ram": 512, + "vcpus": 2, + "disk": 1, + "OS-FLV-EXT-DATA:ephemeral": 1, + "id": six.text_type('1234'), + "swap": 512, + "rxtx_factor": 1, + "os-flavor-access:is_public": True, + } + } + exc = self.assertRaises(exception.PolicyNotAuthorized, + self.controller._create, self.req, + body=body) + self.assertEqual( + "Policy doesn't allow %s to be performed." % create_flavor_policy, + exc.format_message()) + + def test_delete_policy_rbac_change_to_default_action_rule(self): + """Test to verify the correct action is being enforced. When the + rule configured for the deprecated action is the same as the + default, the new action should be enforced. + """ + + default_flavor_policy = "os_compute_api:os-flavor-manage" + create_flavor_policy = "os_compute_api:os-flavor-manage:create" + delete_flavor_policy = "os_compute_api:os-flavor-manage:delete" + # The default rule of the deprecated action is admin_api + # Set the rule of the create flavor action to is_admin:True so that + # admin context can be used to create a flavor + rules = {default_flavor_policy: 'rule:admin_api', + create_flavor_policy: 'is_admin:True', + delete_flavor_policy: 'rule:%s' % default_flavor_policy} + self.policy.set_rules(rules) + body = { + "flavor": { + "name": "azAZ09. -_", + "ram": 512, + "vcpus": 2, + "disk": 1, + "OS-FLV-EXT-DATA:ephemeral": 1, + "id": six.text_type('1234'), + "swap": 512, + "rxtx_factor": 1, + "os-flavor-access:is_public": True, + } + } + flavor = self.controller._create(self.adm_req, body=body) + exc = self.assertRaises(exception.PolicyNotAuthorized, + self.controller._delete, self.req, + flavor['flavor']['id']) + self.assertEqual( + "Policy doesn't allow %s to be performed." % delete_flavor_policy, + exc.format_message()) diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index 11729fe82c5e..920ed06e15c8 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -51,6 +51,8 @@ policy_data = """ "os_compute_api:os-flavor-extra-specs:index": "", "os_compute_api:os-flavor-extra-specs:show": "", "os_compute_api:os-flavor-manage": "", + "os_compute_api:os-flavor-manage:create": "", + "os_compute_api:os-flavor-manage:delete": "", "os_compute_api:os-floating-ip-dns": "", "os_compute_api:os-floating-ip-dns:domain:update": "", "os_compute_api:os-floating-ip-dns:domain:delete": "", diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 00d08982a34d..0d2914fcc44b 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -72,6 +72,9 @@ class PolicyTestCase(test.NoDBTestCase): oslo_policy.RuleDefault("true", '@'), oslo_policy.RuleDefault("example:allowed", '@'), oslo_policy.RuleDefault("example:denied", "!"), + oslo_policy.RuleDefault("old_action_not_default", "@"), + oslo_policy.RuleDefault("new_action", "@"), + oslo_policy.RuleDefault("old_action_default", "rule:admin_api"), oslo_policy.RuleDefault("example:get_http", "http://www.example.com"), oslo_policy.RuleDefault("example:my_file", @@ -181,6 +184,32 @@ class PolicyTestCase(test.NoDBTestCase): "project_id:%(project_id)s")]) mock_warning.assert_not_called() + @mock.patch.object(policy.LOG, 'warning') + def test_verify_deprecated_policy_using_old_action(self, mock_warning): + + old_policy = "old_action_not_default" + new_policy = "new_action" + default_rule = "rule:admin_api" + + using_old_action = policy.verify_deprecated_policy( + old_policy, new_policy, default_rule, self.context) + + mock_warning.assert_called_once_with("Start using the new " + "action '{0}'. The existing action '{1}' is being deprecated and " + "will be removed in future release.".format(new_policy, + old_policy)) + self.assertTrue(using_old_action) + + def test_verify_deprecated_policy_using_new_action(self): + old_policy = "old_action_default" + new_policy = "new_action" + default_rule = "rule:admin_api" + + using_old_action = policy.verify_deprecated_policy( + old_policy, new_policy, default_rule, self.context) + + self.assertFalse(using_old_action) + class IsAdminCheckTestCase(test.NoDBTestCase): def setUp(self): @@ -283,6 +312,8 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:os-flavor-extra-specs:update", "os_compute_api:os-flavor-extra-specs:delete", "os_compute_api:os-flavor-manage", +"os_compute_api:os-flavor-manage:create", +"os_compute_api:os-flavor-manage:delete", "os_compute_api:os-floating-ips-bulk", "os_compute_api:os-floating-ip-dns:domain:delete", "os_compute_api:os-floating-ip-dns:domain:update", diff --git a/releasenotes/notes/flavor-api-policy-granularity-f563d621c615fd64.yaml b/releasenotes/notes/flavor-api-policy-granularity-f563d621c615fd64.yaml new file mode 100644 index 000000000000..3d10f60bcbf1 --- /dev/null +++ b/releasenotes/notes/flavor-api-policy-granularity-f563d621c615fd64.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + Add granularity to the ``os_compute_api:os-flavor-manage`` policy + with the addition of distinct actions for create and delete: + + - ``os_compute_api:os-flavor-manage:create`` + - ``os_compute_api:os-flavor-manage:delete`` + + To address backwards compatibility, the new rules added to the + flavor_manage.py policy file, default to the existing rule, + ``os_compute_api:os-flavor-manage``, if it is set to a non-default + value.