diff --git a/nova/api/openstack/compute/volumes.py b/nova/api/openstack/compute/volumes.py index eae566a429c0..406b4933f8a0 100644 --- a/nova/api/openstack/compute/volumes.py +++ b/nova/api/openstack/compute/volumes.py @@ -266,14 +266,27 @@ class VolumeAttachmentController(wsgi.Controller): """Returns the list of volume attachments for a given instance.""" context = req.environ['nova.context'] context.can(va_policies.POLICY_ROOT % 'index') - return self._items(req, server_id, - entity_maker=_translate_attachment_summary_view) + + instance = common.get_instance(self.compute_api, context, server_id) + + bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( + context, instance.uuid) + limited_list = common.limited(bdms, req) + + results = [] + for bdm in limited_list: + if bdm.volume_id: + va = _translate_attachment_summary_view(bdm.volume_id, + bdm.instance_uuid, + bdm.device_name) + results.append(va) + + return {'volumeAttachments': results} @extensions.expected_errors(404) def show(self, req, server_id, id): """Return data about the given volume attachment.""" context = req.environ['nova.context'] - context.can(vol_policies.BASE_POLICY_NAME) context.can(va_policies.POLICY_ROOT % 'show') volume_id = id @@ -308,7 +321,6 @@ class VolumeAttachmentController(wsgi.Controller): def create(self, req, server_id, body): """Attach a volume to an instance.""" context = req.environ['nova.context'] - context.can(vol_policies.BASE_POLICY_NAME) context.can(va_policies.POLICY_ROOT % 'create') volume_id = body['volumeAttachment']['volumeId'] @@ -362,7 +374,6 @@ class VolumeAttachmentController(wsgi.Controller): @validation.schema(volumes_schema.update_volume_attachment) def update(self, req, server_id, id, body): context = req.environ['nova.context'] - context.can(vol_policies.BASE_POLICY_NAME) context.can(va_policies.POLICY_ROOT % 'update') old_volume_id = id @@ -420,7 +431,6 @@ class VolumeAttachmentController(wsgi.Controller): def delete(self, req, server_id, id): """Detach a volume from an instance.""" context = req.environ['nova.context'] - context.can(vol_policies.BASE_POLICY_NAME) context.can(va_policies.POLICY_ROOT % 'delete') volume_id = id @@ -474,26 +484,6 @@ class VolumeAttachmentController(wsgi.Controller): msg = _("volume_id not found: %s") % volume_id raise exc.HTTPNotFound(explanation=msg) - def _items(self, req, server_id, entity_maker): - """Returns a list of attachments, transformed through entity_maker.""" - context = req.environ['nova.context'] - context.can(vol_policies.BASE_POLICY_NAME) - - instance = common.get_instance(self.compute_api, context, server_id) - - bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( - context, instance.uuid) - limited_list = common.limited(bdms, req) - results = [] - - for bdm in limited_list: - if bdm.volume_id: - results.append(entity_maker(bdm.volume_id, - bdm.instance_uuid, - bdm.device_name)) - - return {'volumeAttachments': results} - def _translate_snapshot_detail_view(context, vol): """Maps keys for snapshots details view.""" diff --git a/nova/tests/unit/api/openstack/compute/test_volumes.py b/nova/tests/unit/api/openstack/compute/test_volumes.py index ad73c7ecd1ee..419935398084 100644 --- a/nova/tests/unit/api/openstack/compute/test_volumes.py +++ b/nova/tests/unit/api/openstack/compute/test_volumes.py @@ -886,57 +886,29 @@ class TestVolumeAttachPolicyEnforcementV21(test.NoDBTestCase): self.controller.index, self.req, FAKE_UUID) def test_show_volume_attach_policy_failed(self): - rule_name = "os_compute_api:os-volumes" - rules = {"os_compute_api:os-volumes-attachments:show": "@", - rule_name: "project:non_fake"} - self._common_policy_check(rules, rule_name, self.controller.show, - self.req, FAKE_UUID, FAKE_UUID_A) - rule_name = "os_compute_api:os-volumes-attachments:show" - rules = {"os_compute_api:os-volumes": "@", - rule_name: "project:non_fake"} + rules = {rule_name: "project:non_fake"} self._common_policy_check(rules, rule_name, self.controller.show, self.req, FAKE_UUID, FAKE_UUID_A) def test_create_volume_attach_policy_failed(self): - rule_name = "os_compute_api:os-volumes" - rules = {"os_compute_api:os-volumes-attachments:create": "@", - rule_name: "project:non_fake"} - body = {'volumeAttachment': {'volumeId': FAKE_UUID_A, - 'device': '/dev/fake'}} - self._common_policy_check(rules, rule_name, self.controller.create, - self.req, FAKE_UUID, body=body) - rule_name = "os_compute_api:os-volumes-attachments:create" - rules = {"os_compute_api:os-volumes": "@", - rule_name: "project:non_fake"} + rules = {rule_name: "project:non_fake"} + body = {'volumeAttachment': {'volumeId': FAKE_UUID_A, + 'device': '/dev/fake'}} self._common_policy_check(rules, rule_name, self.controller.create, self.req, FAKE_UUID, body=body) def test_update_volume_attach_policy_failed(self): - rule_name = "os_compute_api:os-volumes" - rules = {"os_compute_api:os-volumes-attachments:update": "@", - rule_name: "project:non_fake"} + rule_name = "os_compute_api:os-volumes-attachments:update" + rules = {rule_name: "project:non_fake"} body = {'volumeAttachment': {'volumeId': FAKE_UUID_B}} self._common_policy_check(rules, rule_name, self.controller.update, self.req, FAKE_UUID, FAKE_UUID_A, body=body) - rule_name = "os_compute_api:os-volumes-attachments:update" - rules = {"os_compute_api:os-volumes": "@", - rule_name: "project:non_fake"} - self._common_policy_check(rules, rule_name, self.controller.update, - self.req, FAKE_UUID, FAKE_UUID_A, body=body) - def test_delete_volume_attach_policy_failed(self): - rule_name = "os_compute_api:os-volumes" - rules = {"os_compute_api:os-volumes-attachments:delete": "@", - rule_name: "project:non_fake"} - self._common_policy_check(rules, rule_name, self.controller.delete, - self.req, FAKE_UUID, FAKE_UUID_A) - rule_name = "os_compute_api:os-volumes-attachments:delete" - rules = {"os_compute_api:os-volumes": "@", - rule_name: "project:non_fake"} + rules = {rule_name: "project:non_fake"} self._common_policy_check(rules, rule_name, self.controller.delete, self.req, FAKE_UUID, FAKE_UUID_A) diff --git a/releasenotes/notes/bug-volume-attach-policy-1635358-671ce4d4ee8c211b.yaml b/releasenotes/notes/bug-volume-attach-policy-1635358-671ce4d4ee8c211b.yaml new file mode 100644 index 000000000000..89c83ccd48d3 --- /dev/null +++ b/releasenotes/notes/bug-volume-attach-policy-1635358-671ce4d4ee8c211b.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The os-volume_attachments APIs no longer check + ``os_compute_api:os-volumes`` policy. They do still check + ``os_compute_api:os-volumes-attachments`` policy rules. Deployers + who have customized policy should confirm that their settings for + os-volume_attachments policy checks are sufficient.