openstack/nova
Code Issues Proposed changes

Merge "Separate flavor extra specs policy for server APIs"

Browse Source
This commit is contained in:
Zuul
2022-02-26 11:44:35 +00:00
committed by Gerrit Code Review
parent 731668ebae 1be007243b
commit 50f1ba0cf5
9 changed files with 169 additions and 159 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
nova/api/openstack/compute/views/servers.py
View File

@@ -32,7 +32,6 @@ from nova import objects
from nova.objects import fields
from nova.objects import virtual_interface
from nova.policies import extended_server_attributes as esa_policies
from nova.policies import flavor_extra_specs as fes_policies
from nova.policies import servers as servers_policies
from nova import utils
@@ -234,7 +233,9 @@ class ViewBuilder(common.ViewBuilder):
if api_version_request.is_supported(request, min_version='2.47'):
context = request.environ['nova.context']
show_extra_specs = context.can(
fes_policies.POLICY_ROOT % 'index', fatal=False)
servers_policies.SERVERS % 'show:flavor-extra-specs',
fatal=False,
target={'project_id': instance.project_id})
if cell_down_support and 'display_name' not in instance:
# NOTE(tssurya): If the microversion is >= 2.69, this boolean will
@@ -437,8 +438,9 @@ class ViewBuilder(common.ViewBuilder):
if api_version_request.is_supported(request, min_version='2.47'):
# Determine if we should show extra_specs in the inlined flavor
# once before we iterate the list of instances
show_extra_specs = context.can(fes_policies.POLICY_ROOT % 'index',
fatal=False)
show_extra_specs = context.can(
servers_policies.SERVERS % 'show:flavor-extra-specs',
fatal=False)
else:
show_extra_specs = False
show_extended_attr = context.can(

6
nova/policies/base.py
View File

@@ -56,6 +56,7 @@ PROJECT_MEMBER = 'rule:project_member_api'
PROJECT_READER = 'rule:project_reader_api'
PROJECT_MEMBER_OR_SYSTEM_ADMIN = 'rule:system_admin_or_owner'
PROJECT_READER_OR_SYSTEM_READER = 'rule:system_or_project_reader'
PROJECT_READER_OR_ADMIN = 'rule:project_reader_or_admin'
ADMIN = 'rule:context_is_admin'
# NOTE(gmann): Below is the mapping of new roles and scope_types
@@ -139,6 +140,11 @@ rules = [
"system_or_project_reader",
"rule:system_reader_api or rule:project_reader_api",
"Default rule for System+Project read only APIs.",
deprecated_rule=DEPRECATED_ADMIN_OR_OWNER_POLICY),
policy.RuleDefault(
"project_reader_or_admin",
"rule:project_reader_api or rule:context_is_admin",
"Default rule for Project reader and admin APIs.",
deprecated_rule=DEPRECATED_ADMIN_OR_OWNER_POLICY)
]

29
nova/policies/flavor_extra_specs.py
View File

@@ -17,14 +17,12 @@ from oslo_policy import policy
from nova.policies import base
POLICY_ROOT = 'os_compute_api:os-flavor-extra-specs:%s'
flavor_extra_specs_policies = [
policy.DocumentedRuleDefault(
name=POLICY_ROOT % 'show',
check_str=base.PROJECT_READER_OR_SYSTEM_READER,
check_str=base.PROJECT_READER_OR_ADMIN,
description="Show an extra spec for a flavor",
operations=[
{
@@ -75,34 +73,15 @@ flavor_extra_specs_policies = [
),
policy.DocumentedRuleDefault(
name=POLICY_ROOT % 'index',
check_str=base.PROJECT_READER,
check_str=base.PROJECT_READER_OR_ADMIN,
description="List extra specs for a flavor. Starting with "
"microversion 2.47, the flavor used for a server is also returned "
"in the response when showing server details, updating a server or "
"rebuilding a server. Starting with microversion 2.61, extra specs "
"may be returned in responses for the flavor resource.",
"microversion 2.61, extra specs may be returned in responses "
"for the flavor resource.",
operations=[
{
'path': '/flavors/{flavor_id}/os-extra_specs/',
'method': 'GET'
},
# Microversion 2.47 operations for servers:
{
'path': '/servers/detail',
'method': 'GET'
},
{
'path': '/servers/{server_id}',
'method': 'GET'
},
{
'path': '/servers/{server_id}',
'method': 'PUT'
},
{
'path': '/servers/{server_id}/action (rebuild)',
'method': 'POST'
},
# Microversion 2.61 operations for flavors:
{
'path': '/flavors',

41
nova/policies/servers.py
View File

@@ -22,6 +22,17 @@ ZERO_DISK_FLAVOR = SERVERS % 'create:zero_disk_flavor'
REQUESTED_DESTINATION = 'compute:servers:create:requested_destination'
CROSS_CELL_RESIZE = 'compute:servers:resize:cross_cell'
DEPRECATED_POLICY = policy.DeprecatedRule(
'os_compute_api:os-flavor-extra-specs:index',
base.RULE_ADMIN_OR_OWNER,
)
DEPRECATED_REASON = """
Policies for showing flavor extra specs in server APIs response is
seprated as new policy. This policy is deprecated only for that but
not for list extra specs and showing it in flavor API response.
"""
rules = [
policy.DocumentedRuleDefault(
name=SERVERS % 'index',
@@ -95,6 +106,36 @@ rules = [
}
],
scope_types=['project']),
policy.DocumentedRuleDefault(
name=SERVERS % 'show:flavor-extra-specs',
check_str=base.PROJECT_READER,
description="Starting with microversion 2.47, the flavor and its "
"extra specs used for a server is also returned in the response "
"when showing server details, updating a server or rebuilding a "
"server.",
operations=[
# Microversion 2.47 operations for servers:
{
'path': '/servers/detail',
'method': 'GET'
},
{
'path': '/servers/{server_id}',
'method': 'GET'
},
{
'path': '/servers/{server_id}',
'method': 'PUT'
},
{
'path': '/servers/{server_id}/action (rebuild)',
'method': 'POST'
},
],
scope_types=['project'],
deprecated_rule=DEPRECATED_POLICY,
deprecated_reason=DEPRECATED_REASON,
deprecated_since='25.0.0'),
# the details in host_status are pretty sensitive, only admins
# should do that by default.
policy.DocumentedRuleDefault(

1
nova/tests/unit/fake_policy.py
View File

@@ -44,6 +44,7 @@ policy_data = """
"os_compute_api:servers:trigger_crash_dump": "",
"os_compute_api:servers:show:host_status": "",
"os_compute_api:servers:show": "",
"os_compute_api:servers:show:flavor-extra-specs" : "",
"os_compute_api:servers:show:host_status:unknown-only": "",
"os_compute_api:servers:allow_all_filters": "",
"os_compute_api:servers:migrations:force_complete": "",

2
nova/tests/unit/policies/base.py
View File

@@ -149,6 +149,8 @@ class BasePolicyTest(test.TestCase):
"role:admin and system_scope:all",
"system_reader_api":
"role:reader and system_scope:all",
"project_reader_or_admin":
"rule:project_reader_api or rule:context_is_admin",
"project_admin_api":
"role:admin and project_id:%(project_id)s",
"project_member_api":

128
nova/tests/unit/policies/test_flavor_extra_specs.py
View File

@@ -10,22 +10,16 @@
# License for the specific language governing permissions and limitations
# under the License.
import fixtures
import mock
from oslo_utils.fixture import uuidsentinel as uuids
from nova.api.openstack.compute import flavor_manage
from nova.api.openstack.compute import flavors
from nova.api.openstack.compute import flavors_extraspecs
from nova.api.openstack.compute import servers
from nova.compute import vm_states
from nova import objects
from nova.policies import flavor_extra_specs as policies
from nova.policies import flavor_manage as fm_policies
from nova.policies import servers as s_policies
from nova.tests.unit.api.openstack import fakes
from nova.tests.unit import fake_flavor
from nova.tests.unit import fake_instance
from nova.tests.unit.policies import base
@@ -42,30 +36,7 @@ class FlavorExtraSpecsPolicyTest(base.BasePolicyTest):
self.controller = flavors_extraspecs.FlavorExtraSpecsController()
self.flavor_ctrl = flavors.FlavorsController()
self.fm_ctrl = flavor_manage.FlavorManageController()
self.server_ctrl = servers.ServersController()
self.req = fakes.HTTPRequest.blank('')
self.server_ctrl._view_builder._add_security_grps = mock.MagicMock()
self.server_ctrl._view_builder._get_metadata = mock.MagicMock()
self.server_ctrl._view_builder._get_addresses = mock.MagicMock()
self.server_ctrl._view_builder._get_host_id = mock.MagicMock()
self.server_ctrl._view_builder._get_fault = mock.MagicMock()
self.server_ctrl._view_builder._add_host_status = mock.MagicMock()
self.instance = fake_instance.fake_instance_obj(
self.project_member_context,
id=1, uuid=uuids.fake_id, project_id=self.project_id,
vm_state=vm_states.ACTIVE)
self.mock_get = self.useFixture(
fixtures.MockPatch('nova.api.openstack.common.get_instance')).mock
self.mock_get.return_value = self.instance
fakes.stub_out_secgroup_api(
self, security_groups=[{'name': 'default'}])
self.mock_get_all = self.useFixture(fixtures.MockPatchObject(
self.server_ctrl.compute_api, 'get_all')).mock
self.mock_get_all.return_value = objects.InstanceList(
objects=[self.instance])
def get_flavor_extra_specs(context, flavor_id):
return fake_flavor.fake_flavor_obj(
@@ -87,8 +58,6 @@ class FlavorExtraSpecsPolicyTest(base.BasePolicyTest):
# scopes, since scope checking is disabled.
self.all_system_authorized_contexts = (self.all_project_contexts |
self.all_system_contexts)
self.all_project_authorized_contexts = (self.all_project_contexts |
self.all_system_contexts)
# In the base/legacy case, any admin is an admin.
self.admin_authorized_contexts = set([self.project_admin_context,
@@ -226,85 +195,6 @@ class FlavorExtraSpecsPolicyTest(base.BasePolicyTest):
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp['flavor'])
def test_server_detail_with_extra_specs_policy(self):
rule = s_policies.SERVERS % 'detail'
# server 'detail' policy is checked before flavor extra specs 'index'
# policy so we have to allow it for everyone otherwise it will fail
# first for unauthorized contexts.
self.policy.set_rules({rule: "@"}, overwrite=False)
req = fakes.HTTPRequest.blank('', version='2.47')
rule_name = policies.POLICY_ROOT % 'index'
authorize_res, unauthorize_res = self.common_policy_auth(
self.all_project_authorized_contexts,
rule_name, self.server_ctrl.detail, req,
fatal=False)
for resp in authorize_res:
self.assertIn('extra_specs', resp['servers'][0]['flavor'])
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp['servers'][0]['flavor'])
@mock.patch('nova.objects.BlockDeviceMappingList.bdms_by_instance_uuid')
@mock.patch('nova.compute.api.API.get_instance_host_status')
def test_server_show_with_extra_specs_policy(self, mock_get, mock_block):
rule = s_policies.SERVERS % 'show'
# server 'show' policy is checked before flavor extra specs 'index'
# policy so we have to allow it for everyone otherwise it will fail
# first for unauthorized contexts.
self.policy.set_rules({rule: "@"}, overwrite=False)
req = fakes.HTTPRequest.blank('', version='2.47')
rule_name = policies.POLICY_ROOT % 'index'
authorize_res, unauthorize_res = self.common_policy_auth(
self.all_project_authorized_contexts,
rule_name, self.server_ctrl.show, req, 'fake',
fatal=False)
for resp in authorize_res:
self.assertIn('extra_specs', resp['server']['flavor'])
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp['server']['flavor'])
@mock.patch('nova.objects.BlockDeviceMappingList.bdms_by_instance_uuid')
@mock.patch('nova.compute.api.API.get_instance_host_status')
@mock.patch('nova.compute.api.API.rebuild')
def test_server_rebuild_with_extra_specs_policy(self, mock_rebuild,
mock_get, mock_bdm):
rule = s_policies.SERVERS % 'rebuild'
# server 'rebuild' policy is checked before flavor extra specs 'index'
# policy so we have to allow it for everyone otherwise it will fail
# first for unauthorized contexts.
self.policy.set_rules({rule: "@"}, overwrite=False)
req = fakes.HTTPRequest.blank('', version='2.47')
rule_name = policies.POLICY_ROOT % 'index'
authorize_res, unauthorize_res = self.common_policy_auth(
self.all_project_authorized_contexts,
rule_name, self.server_ctrl._action_rebuild,
req, self.instance.uuid,
body={'rebuild': {"imageRef": uuids.fake_id}},
fatal=False)
for resp in authorize_res:
self.assertIn('extra_specs', resp.obj['server']['flavor'])
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp.obj['server']['flavor'])
@mock.patch('nova.compute.api.API.update_instance')
def test_server_update_with_extra_specs_policy(self, mock_update):
rule = s_policies.SERVERS % 'update'
# server 'update' policy is checked before flavor extra specs 'index'
# policy so we have to allow it for everyone otherwise it will fail
# first for unauthorized contexts.
self.policy.set_rules({rule: "@"}, overwrite=False)
req = fakes.HTTPRequest.blank('', version='2.47')
rule_name = policies.POLICY_ROOT % 'index'
authorize_res, unauthorize_res = self.common_policy_auth(
self.all_project_authorized_contexts,
rule_name, self.server_ctrl.update,
req, self.instance.uuid,
body={'server': {'name': 'test'}},
fatal=False)
for resp in authorize_res:
self.assertIn('extra_specs', resp['server']['flavor'])
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp['server']['flavor'])
class FlavorExtraSpecsScopeTypePolicyTest(FlavorExtraSpecsPolicyTest):
"""Test Flavor Extra Specs APIs policies with system scope enabled.
@@ -326,17 +216,6 @@ class FlavorExtraSpecsScopeTypePolicyTest(FlavorExtraSpecsPolicyTest):
# Only system_admin can do system admin things
self.admin_authorized_contexts = [self.system_admin_context]
# Scope checking is in effect, so break apart project/system
# authorization. Note that even for the server tests above, we
# are technically authorizing against a server-embedded flavor
# (which has no project affiliation like the actual flavor it
# came from) and thus the other_project_* contexts are
# technically valid here. In reality, failure for
# other_project_* to get the server itself would prevent those
# projects from seeing the flavor extra_specs for it.
self.all_project_authorized_contexts = self.all_project_contexts
self.all_system_authorized_contexts = self.all_system_contexts
class FlavorExtraSpecsNoLegacyNoScopeTest(FlavorExtraSpecsPolicyTest):
"""Test Flavor Extra Specs API policies with deprecated rules
@@ -355,15 +234,13 @@ class FlavorExtraSpecsNoLegacyNoScopeTest(FlavorExtraSpecsPolicyTest):
self.system_foo_context,
self.project_foo_context,
])
self.reduce_set('all_project_authorized', everything_but_foo)
self.reduce_set('all_system_authorized', everything_but_foo)
self.reduce_set('all_authorized', everything_but_foo)
class FlavorExtraSpecsNoLegacyPolicyTest(FlavorExtraSpecsScopeTypePolicyTest):
"""Test Flavor Extra Specs APIs policies with system scope enabled,
and no more deprecated rules that allow the legacy admin API to
access system_admin_or_owner APIs.
and no more deprecated rules.
"""
without_deprecated_rules = True
@@ -373,9 +250,6 @@ class FlavorExtraSpecsNoLegacyPolicyTest(FlavorExtraSpecsScopeTypePolicyTest):
# access. Same note as above, regarding other_project_*
# contexts. With scope checking enabled, project and system
# contexts stay separate.
self.reduce_set(
'all_project_authorized',
self.all_project_contexts - set([self.project_foo_context]))
self.reduce_set(
'all_system_authorized',
self.all_system_contexts - set([self.system_foo_context]))

108
nova/tests/unit/policies/test_servers.py
View File

@@ -29,6 +29,7 @@ from nova.network import neutron
from nova import objects
from nova.objects import fields
from nova.objects.instance_group import InstanceGroup
from nova.policies import base as base_policy
from nova.policies import extended_server_attributes as ea_policies
from nova.policies import servers as policies
from nova.tests.unit.api.openstack import fakes
@@ -324,6 +325,102 @@ class ServersPolicyTest(base.BasePolicyTest):
self.controller.show,
self.req, self.instance.uuid)
@mock.patch('nova.objects.BlockDeviceMappingList.bdms_by_instance_uuid')
@mock.patch('nova.compute.api.API.get_instance_host_status')
def test_server_show_with_extra_specs_policy(self, mock_get, mock_block):
rule = policies.SERVERS % 'show'
# server 'show' policy is checked before flavor extra specs
# policy so we have to allow it for everyone otherwise it will fail
# first for unauthorized contexts.
self.policy.set_rules({rule: "@"}, overwrite=False)
req = fakes.HTTPRequest.blank('', version='2.47')
rule_name = policies.SERVERS % 'show:flavor-extra-specs'
authorize_res, unauthorize_res = self.common_policy_auth(
self.project_reader_authorized_contexts,
rule_name, self.controller.show, req,
self.instance.uuid, fatal=False)
for resp in authorize_res:
self.assertIn('extra_specs', resp['server']['flavor'])
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp['server']['flavor'])
@mock.patch('nova.compute.api.API.get_all')
def test_server_detail_with_extra_specs_policy(self, mock_get):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
if 'project_id' in search_opts or 'user_id' in search_opts:
return objects.InstanceList(objects=self.servers)
else:
raise
self.mock_get_all.side_effect = fake_get_all
rule = policies.SERVERS % 'detail'
# server 'detail' policy is checked before flavor extra specs
# policy so we have to allow it for everyone otherwise it will fail
# first for unauthorized contexts.
self.policy.set_rules({rule: "@"}, overwrite=False)
req = fakes.HTTPRequest.blank('', version='2.47')
rule_name = policies.SERVERS % 'show:flavor-extra-specs'
authorize_res, unauthorize_res = self.common_policy_auth(
self.everyone_authorized_contexts,
rule_name, self.controller.detail, req,
fatal=False)
for resp in authorize_res:
self.assertIn('extra_specs', resp['servers'][0]['flavor'])
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp['servers'][0]['flavor'])
@mock.patch('nova.objects.BlockDeviceMappingList.bdms_by_instance_uuid')
@mock.patch('nova.compute.api.API.get_instance_host_status')
@mock.patch('nova.compute.api.API.rebuild')
def test_server_rebuild_with_extra_specs_policy(self, mock_rebuild,
mock_get, mock_bdm):
rule = policies.SERVERS % 'rebuild'
# server 'rebuild' policy is checked before flavor extra specs
# policy so we have to allow it for everyone otherwise it will fail
# first for unauthorized contexts.
self.policy.set_rules({rule: "@"}, overwrite=False)
req = fakes.HTTPRequest.blank('', version='2.47')
rule_name = policies.SERVERS % 'show:flavor-extra-specs'
authorize_res, unauthorize_res = self.common_policy_auth(
self.project_reader_authorized_contexts,
rule_name, self.controller._action_rebuild,
req, self.instance.uuid,
body={'rebuild': {"imageRef": uuids.fake_id}},
fatal=False)
for resp in authorize_res:
self.assertIn('extra_specs', resp.obj['server']['flavor'])
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp.obj['server']['flavor'])
@mock.patch('nova.objects.BlockDeviceMappingList.bdms_by_instance_uuid')
@mock.patch.object(InstanceGroup, 'get_by_instance_uuid')
@mock.patch('nova.compute.api.API.update_instance')
def test_server_update_with_extra_specs_policy(self,
mock_update, mock_group, mock_bdm):
mock_update.return_value = self.instance
rule = policies.SERVERS % 'update'
# server 'update' policy is checked before flavor extra specs
# policy so we have to allow it for everyone otherwise it will fail
# first for unauthorized contexts.
self.policy.set_rules({rule: "@"}, overwrite=False)
req = fakes.HTTPRequest.blank('', version='2.47')
rule_name = policies.SERVERS % 'show:flavor-extra-specs'
authorize_res, unauthorize_res = self.common_policy_auth(
self.project_reader_authorized_contexts,
rule_name, self.controller.update,
req, self.instance.uuid,
body={'server': {'name': 'test'}},
fatal=False)
for resp in authorize_res:
self.assertIn('extra_specs', resp['server']['flavor'])
for resp in unauthorize_res:
self.assertNotIn('extra_specs', resp['server']['flavor'])
@mock.patch('nova.compute.api.API.create')
def test_create_server_policy(self, mock_create):
mock_create.return_value = ([self.instance], '')
@@ -1228,6 +1325,10 @@ class ServersNoLegacyNoScopeTest(ServersPolicyTest):
checking still disabled.
"""
without_deprecated_rules = True
rules_without_deprecation = {
policies.SERVERS % 'show:flavor-extra-specs':
base_policy.PROJECT_READER,
}
def setUp(self):
super(ServersNoLegacyNoScopeTest, self).setUp()
@@ -1335,10 +1436,13 @@ class ServersScopeTypePolicyTest(ServersPolicyTest):
class ServersNoLegacyPolicyTest(ServersScopeTypePolicyTest):
"""Test Servers APIs policies with system scope enabled,
and no more deprecated rules that allow the legacy admin API to
access system_admin_or_owner APIs.
and no more deprecated rules.
"""
without_deprecated_rules = True
rules_without_deprecation = {
policies.SERVERS % 'show:flavor-extra-specs':
base_policy.PROJECT_READER,
}
def setUp(self):
super(ServersNoLegacyPolicyTest, self).setUp()

3
nova/tests/unit/test_policy.py
View File

@@ -409,6 +409,7 @@ class RealRolePolicyTestCase(test.NoDBTestCase):
"os_compute_api:servers:resize",
"os_compute_api:servers:revert_resize",
"os_compute_api:servers:show",
"os_compute_api:servers:show:flavor-extra-specs",
"os_compute_api:servers:update",
"os_compute_api:servers:create_image:allow_volume_backed",
"os_compute_api:os-admin-password",
@@ -560,7 +561,7 @@ class RealRolePolicyTestCase(test.NoDBTestCase):
'system_admin_api', 'system_reader_api',
'project_admin_api', 'project_member_api',
'project_reader_api', 'system_admin_or_owner',
'system_or_project_reader')
'system_or_project_reader', 'project_reader_or_admin')
result = set(rules.keys()) - set(self.admin_only_rules +
self.admin_or_owner_rules +
self.allow_all_rules + self.system_reader_rules +