openstack/cinder
Code Issues Proposed changes
919d054980
cinder/cinder/tests/unit/policies/test_volume.py
Sean McGinnis 3eb9b422f4
Introduce flake8-import-order extension 
This adds usage of the flake8-import-order extension to our flake8
checks to enforce consistency on our import ordering to follow the
overall OpenStack code guidelines.

Since we have now dropped Python 2, this also cleans up a few cases for
things that were third party libs but became part of the standard
library such as mock, which is now a standard part of unittest.

Some questions, in order of importance:

Q: Are you insane?
A: Potentially.

Q: Why should we touch all of these files?
A: This adds consistency to our imports. The extension makes sure that
   all imports follow our published guidelines of having imports ordered
   by standard lib, third party, and local. This will be a one time
   churn, then we can ensure consistency over time.

Q: Why bother. this doesn't really matter?
A: I agree - but...

We have the issue that we have less people actively involved and less
time to perform thorough code reviews. This will make it objective and
automated to catch these kinds of issues.

But part of this, even though it maybe seems a little annoying, is for
making it easier for contributors. Right now, we may or may not notice
if something is following the guidelines or not. And we may or may not
comment in a review to ask for a contributor to make adjustments to
follow the guidelines.

But then further along into the review process, someone decides to be
thorough, and after the contributor feels like they've had to deal with
other change requests and things are in really good shape, they get a -1
on something mostly meaningless as far as the functionality of their
code. It can be a frustrating and disheartening thing.

I believe this actually helps avoid that by making it an objective thing
that they find out right away up front - either the code is following
the guidelines and everything is happy, or it's not and running local
jobs or the pep8 CI job will let them know right away and they can fix
it. No guessing on whether or not someone is going to take a stand on
following the guidelines or not.

This will also make it easier on the code reviewers. The more we can
automate, the more time we can spend in code reviews making sure the
logic of the change is correct and less time looking at trivial coding
and style things.

Q: Should we use our hacking extensions for this?
A: Hacking has had to keep back linter requirements for a long time now.
   Current versions of the linters actually don't work with the way
   we've been hooking into them for our hacking checks. We will likely
   need to do away with those at some point so we can move on to the
   current linter releases. This will help ensure we have something in
   place when that time comes to make sure some checks are automated.

Q: Didn't you spend more time on this than the benefit we'll get from
   it?
A: Yeah, probably.

Change-Id: Ic13ba238a4a45c6219f4de131cfe0366219d722f
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
2020-01-06 09:59:35 -06:00

492 lines
20 KiB
Python
Raw Blame History

#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from unittest import mock
from six.moves import http_client
from cinder.tests.unit import fake_constants
from cinder.tests.unit.policies import test_base
from cinder.volume import api as volume_api
# TODO(yikun): The below policy test cases should be added:
# * HOST_ATTRIBUTE_POLICY
# * MIG_ATTRIBUTE_POLICY
# * ENCRYPTION_METADATA_POLICY
# * MULTIATTACH_POLICY
class VolumePolicyTests(test_base.CinderPolicyTests):
def test_admin_can_create_volume(self):
admin_context = self.admin_context
path = '/v3/%(project_id)s/volumes' % {
'project_id': admin_context.project_id
}
body = {"volume": {"size": 1}}
response = self._get_request_response(admin_context, path, 'POST',
body=body)
self.assertEqual(http_client.ACCEPTED, response.status_int)
def test_nonadmin_user_can_create_volume(self):
user_context = self.user_context
path = '/v3/%(project_id)s/volumes' % {
'project_id': user_context.project_id
}
body = {"volume": {"size": 1}}
response = self._get_request_response(user_context, path, 'POST',
body=body)
self.assertEqual(http_client.ACCEPTED, response.status_int)
def test_admin_can_create_volume_from_image(self):
admin_context = self.admin_context
path = '/v3/%(project_id)s/volumes' % {
'project_id': admin_context.project_id
}
body = {"volume": {"size": 1, "image_id": fake_constants.IMAGE_ID}}
response = self._get_request_response(admin_context, path, 'POST',
body=body)
self.assertEqual(http_client.ACCEPTED, response.status_int)
def test_nonadmin_user_can_create_volume_from_image(self):
user_context = self.user_context
path = '/v3/%(project_id)s/volumes' % {
'project_id': user_context.project_id
}
body = {"volume": {"size": 1, "image_id": fake_constants.IMAGE_ID}}
response = self._get_request_response(user_context, path, 'POST',
body=body)
self.assertEqual(http_client.ACCEPTED, response.status_int)
@mock.patch.object(volume_api.API, 'get_volume')
def test_admin_can_show_volumes(self, mock_volume):
# Make sure administrators are authorized to list volumes
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(admin_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
self.assertEqual(response.json_body['volume']['id'], volume.id)
@mock.patch.object(volume_api.API, 'get_volume')
def test_owner_can_show_volumes(self, mock_volume):
# Make sure owners are authorized to list their volumes
user_context = self.user_context
volume = self._create_fake_volume(user_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(user_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
self.assertEqual(response.json_body['volume']['id'], volume.id)
@mock.patch.object(volume_api.API, 'get_volume')
def test_owner_cannot_show_volumes_for_others(self, mock_volume):
# Make sure volumes are only exposed to their owners
owner_context = self.user_context
non_owner_context = self.other_user_context
volume = self._create_fake_volume(owner_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': non_owner_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(non_owner_context, path, 'GET')
# NOTE(lbragstad): Technically, this user isn't supposed to see this
# volume, because they didn't create it and it lives in a different
# project. Does cinder return a 404 in cases like this? Or is a 403
# expected?
self.assertEqual(http_client.NOT_FOUND, response.status_int)
def test_admin_can_get_all_volumes_detail(self):
# Make sure administrators are authorized to list volumes
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context)
path = '/v3/%(project_id)s/volumes/detail' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(admin_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
res_vol = response.json_body['volumes'][0]
self.assertEqual(volume.id, res_vol['id'])
def test_owner_can_get_all_volumes_detail(self):
# Make sure owners are authorized to list volumes
user_context = self.user_context
volume = self._create_fake_volume(user_context)
path = '/v3/%(project_id)s/volumes/detail' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(user_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
res_vol = response.json_body['volumes'][0]
self.assertEqual(volume.id, res_vol['id'])
@mock.patch.object(volume_api.API, 'get')
def test_admin_can_update_volumes(self, mock_volume):
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
body = {"volume": {"name": "update_name"}}
response = self._get_request_response(admin_context, path, 'PUT',
body=body)
self.assertEqual(http_client.OK, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_owner_can_update_volumes(self, mock_volume):
user_context = self.user_context
volume = self._create_fake_volume(user_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
body = {"volume": {"name": "update_name"}}
response = self._get_request_response(user_context, path, 'PUT',
body=body)
self.assertEqual(http_client.OK, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_owner_cannot_update_volumes_for_others(self, mock_volume):
owner_context = self.user_context
non_owner_context = self.other_user_context
volume = self._create_fake_volume(owner_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': non_owner_context.project_id, 'volume_id': volume.id
}
body = {"volume": {"name": "update_name"}}
response = self._get_request_response(non_owner_context, path, 'PUT',
body=body)
self.assertEqual(http_client.FORBIDDEN, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_owner_can_delete_volumes(self, mock_volume):
user_context = self.user_context
volume = self._create_fake_volume(user_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(user_context, path, 'DELETE')
self.assertEqual(http_client.ACCEPTED, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_admin_can_delete_volumes(self, mock_volume):
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(admin_context, path, 'DELETE')
self.assertEqual(http_client.ACCEPTED, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_owner_cannot_delete_volumes_for_others(self, mock_volume):
owner_context = self.user_context
non_owner_context = self.other_user_context
volume = self._create_fake_volume(owner_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': non_owner_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(non_owner_context, path,
'DELETE')
self.assertEqual(http_client.FORBIDDEN, response.status_int)
@mock.patch.object(volume_api.API, 'get_volume')
def test_admin_can_show_tenant_id_in_volume(self, mock_volume):
# Make sure administrators are authorized to show tenant_id
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(admin_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
res_vol = response.json_body['volume']
self.assertEqual(admin_context.project_id,
res_vol['os-vol-tenant-attr:tenant_id'])
@mock.patch.object(volume_api.API, 'get_volume')
def test_owner_can_show_tenant_id_in_volume(self, mock_volume):
# Make sure owners are authorized to show tenant_id in volume
user_context = self.user_context
volume = self._create_fake_volume(user_context)
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(user_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
res_vol = response.json_body['volume']
self.assertEqual(user_context.project_id,
res_vol['os-vol-tenant-attr:tenant_id'])
def test_admin_can_show_tenant_id_in_volume_detail(self):
# Make sure admins are authorized to show tenant_id in volume detail
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context)
path = '/v3/%(project_id)s/volumes/detail' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(admin_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
res_vol = response.json_body['volumes'][0]
# Make sure owners are authorized to show tenant_id
self.assertEqual(admin_context.project_id,
res_vol['os-vol-tenant-attr:tenant_id'])
def test_owner_can_show_tenant_id_in_volume_detail(self):
# Make sure owners are authorized to show tenant_id in volume detail
user_context = self.user_context
volume = self._create_fake_volume(user_context)
path = '/v3/%(project_id)s/volumes/detail' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(user_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
res_vol = response.json_body['volumes'][0]
# Make sure owners are authorized to show tenant_id
self.assertEqual(user_context.project_id,
res_vol['os-vol-tenant-attr:tenant_id'])
def test_admin_can_create_metadata(self):
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context, metadata={"k": "v"})
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
body = {"metadata": {"k1": "v1"}}
response = self._get_request_response(admin_context, path, 'POST',
body=body)
self.assertEqual(http_client.OK, response.status_int)
def test_admin_can_get_metadata(self):
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context, metadata={"k": "v"})
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(admin_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
res_meta = response.json_body['metadata']
self.assertIn('k', res_meta)
self.assertEqual('v', res_meta['k'])
def test_admin_can_update_metadata(self):
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context, metadata={"k": "v"})
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': admin_context.project_id, 'volume_id': volume.id
}
body = {"metadata": {"k": "v2"}}
response = self._get_request_response(admin_context, path, 'PUT',
body=body)
self.assertEqual(http_client.OK, response.status_int)
res_meta = response.json_body['metadata']
self.assertIn('k', res_meta)
self.assertEqual('v2', res_meta['k'])
def test_admin_can_delete_metadata(self):
admin_context = self.admin_context
volume = self._create_fake_volume(admin_context, metadata={"k": "v"})
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata/%(key)s' % {
'project_id': admin_context.project_id, 'volume_id': volume.id,
'key': 'k'
}
response = self._get_request_response(admin_context, path, 'DELETE')
self.assertEqual(http_client.OK, response.status_int)
def test_owner_can_create_metadata(self):
user_context = self.user_context
volume = self._create_fake_volume(user_context, metadata={"k": "v"})
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
body = {"metadata": {"k1": "v1"}}
response = self._get_request_response(user_context, path, 'POST',
body=body)
self.assertEqual(http_client.OK, response.status_int)
def test_owner_can_get_metadata(self):
user_context = self.user_context
volume = self._create_fake_volume(user_context, metadata={"k": "v"})
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(user_context, path, 'GET')
self.assertEqual(http_client.OK, response.status_int)
res_meta = response.json_body['metadata']
self.assertIn('k', res_meta)
self.assertEqual('v', res_meta['k'])
def test_owner_can_update_metadata(self):
user_context = self.user_context
volume = self._create_fake_volume(user_context, metadata={"k": "v"})
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': user_context.project_id, 'volume_id': volume.id
}
body = {"metadata": {"k": "v2"}}
response = self._get_request_response(user_context, path, 'PUT',
body=body)
self.assertEqual(http_client.OK, response.status_int)
res_meta = response.json_body['metadata']
self.assertIn('k', res_meta)
self.assertEqual('v2', res_meta['k'])
def test_owner_can_delete_metadata(self):
user_context = self.user_context
volume = self._create_fake_volume(user_context, metadata={"k": "v"})
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata/%(key)s' % {
'project_id': user_context.project_id, 'volume_id': volume.id,
'key': 'k'
}
response = self._get_request_response(user_context, path, 'DELETE')
self.assertEqual(http_client.OK, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_owner_cannot_create_metadata_for_others(self, mock_volume):
owner_context = self.user_context
non_owner_context = self.other_user_context
volume = self._create_fake_volume(owner_context, metadata={"k": "v"})
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': non_owner_context.project_id, 'volume_id': volume.id
}
body = {"metadata": {"k1": "v1"}}
response = self._get_request_response(non_owner_context, path, 'POST',
body=body)
self.assertEqual(http_client.FORBIDDEN, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_owner_cannot_get_metadata_for_others(self, mock_volume):
owner_context = self.user_context
non_owner_context = self.other_user_context
volume = self._create_fake_volume(owner_context, metadata={"k": "v"})
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': non_owner_context.project_id, 'volume_id': volume.id
}
response = self._get_request_response(non_owner_context, path, 'GET')
self.assertEqual(http_client.FORBIDDEN, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_owner_cannot_update_metadata_for_others(self, mock_volume):
owner_context = self.user_context
non_owner_context = self.other_user_context
volume = self._create_fake_volume(owner_context, metadata={"k": "v"})
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % {
'project_id': non_owner_context.project_id, 'volume_id': volume.id
}
body = {"metadata": {"k": "v2"}}
response = self._get_request_response(non_owner_context, path, 'PUT',
body=body)
self.assertEqual(http_client.FORBIDDEN, response.status_int)
@mock.patch.object(volume_api.API, 'get')
def test_owner_cannot_delete_metadata_for_others(self, mock_volume):
owner_context = self.user_context
non_owner_context = self.other_user_context
volume = self._create_fake_volume(owner_context, metadata={"k": "v"})
mock_volume.return_value = volume
path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata/%(key)s' % {
'project_id': non_owner_context.project_id,
'volume_id': volume.id,
'key': 'k'
}
response = self._get_request_response(non_owner_context, path,
'DELETE')
self.assertEqual(http_client.FORBIDDEN, response.status_int)
View Git Blame Copy Permalink