openstack/cinder
Code Issues Proposed changes

Merge "Remove leading and trailing spaces from parameters"

Browse Source
This commit is contained in:
Zuul 2018-01-19 11:40:50 +00:00 committed by Gerrit Code Review
commit fa1cedbb2f
7 changed files with 156 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cinder/api/contrib/backups.py
View File

@ -156,6 +156,7 @@ class BackupsController(wsgi.Controller):
container = backup.get('container', None)
volume_id = backup['volume_id']
self.validate_name_and_description(backup, check_length=False)
name = backup.get('name', None)
description = backup.get('description', None)
incremental = strutils.bool_from_string(backup.get(

13
cinder/api/openstack/wsgi.py
View File

@ -1250,18 +1250,19 @@ class Controller(object):
raise webob.exc.HTTPBadRequest(explanation=fail_msg)
@staticmethod
def validate_name_and_description(body):
def validate_name_and_description(body, check_length=True):
for attribute in ['name', 'description',
'display_name', 'display_description']:
value = body.get(attribute)
if value is not None:
if isinstance(value, six.string_types):
body[attribute] = value.strip()
try:
utils.check_string_length(body[attribute], attribute,
min_length=0, max_length=255)
except exception.InvalidInput as error:
raise webob.exc.HTTPBadRequest(explanation=error.msg)
if check_length:
try:
utils.check_string_length(body[attribute], attribute,
min_length=0, max_length=255)
except exception.InvalidInput as error:
raise webob.exc.HTTPBadRequest(explanation=error.msg)
@staticmethod
def validate_string_length(value, entity_name, min_length=0,

2
cinder/api/v2/snapshots.py
View File

@ -121,6 +121,7 @@ class SnapshotsController(wsgi.Controller):
force = strutils.bool_from_string(force, strict=True)
LOG.info("Create snapshot from volume %s", volume_id)
self.validate_name_and_description(snapshot, check_length=False)
# NOTE(thingee): v2 API allows name instead of display_name
if 'name' in snapshot:
snapshot['display_name'] = snapshot.pop('name')
@ -148,6 +149,7 @@ class SnapshotsController(wsgi.Controller):
"""Update a snapshot."""
context = req.environ['cinder.context']
snapshot_body = body['snapshot']
self.validate_name_and_description(snapshot_body, check_length=False)
if 'name' in snapshot_body:
snapshot_body['display_name'] = snapshot_body.pop('name')

1
cinder/api/v3/backups.py
View File

@ -44,6 +44,7 @@ class BackupsController(backups_v2.BackupsController):
backup_update = body['backup']
self.validate_name_and_description(backup_update, check_length=False)
update_dict = {}
if 'name' in backup_update:
update_dict['display_name'] = backup_update.pop('name')

40
cinder/tests/unit/api/contrib/test_backups.py
View File

@ -515,6 +515,46 @@ class BackupsAPITestCase(test.TestCase):
volume.destroy()
@ddt.data({"backup": {"description": " sample description",
"name": " test name"}},
{"backup": {"description": "sample description ",
"name": "test "}},
{"backup": {"description": " sample description ",
"name": " test "}})
@mock.patch('cinder.db.service_get_all')
def test_create_backup_name_description_with_leading_trailing_spaces(
self, body, _mock_service_get_all):
_mock_service_get_all.return_value = [
{'availability_zone': 'fake_az', 'host': 'testhost',
'disabled': 0, 'updated_at': timeutils.utcnow(),
'uuid': fake.BACKUP_ID}]
volume = utils.create_volume(self.context, size=5)
body['backup']['volume_id'] = volume.id
req = webob.Request.blank('/v2/%s/backups' % fake.PROJECT_ID)
req.method = 'POST'
req.headers['Content-Type'] = 'application/json'
req.body = jsonutils.dump_as_bytes(body)
res = req.get_response(fakes.wsgi_app(
fake_auth_context=self.user_context))
res_dict = jsonutils.loads(res.body)
# create backup call doesn't return 'description' in response so get
# the created backup to assert name and description
req = webob.Request.blank('/v2/%s/backups/%s' % (
fake.PROJECT_ID, res_dict['backup']['id']))
req.method = 'GET'
req.headers['Content-Type'] = 'application/json'
res = req.get_response(fakes.wsgi_app(
fake_auth_context=self.user_context))
res_dict = jsonutils.loads(res.body)
self.assertEqual(body['backup']['name'].strip(),
res_dict['backup']['name'])
self.assertEqual(body['backup']['description'].strip(),
res_dict['backup']['description'])
volume.destroy()
@mock.patch('cinder.db.service_get_all')
def test_create_backup_with_metadata(self, _mock_service_get_all):
_mock_service_get_all.return_value = [

66
cinder/tests/unit/api/v2/test_snapshots.py
View File

@ -210,6 +210,24 @@ class SnapshotApiTest(test.TestCase):
self.assertRaises(exception.ValidationError,
self.controller.create, req, body=body)
@ddt.data({"snapshot": {"description": " sample description",
"name": " test"}},
{"snapshot": {"description": "sample description ",
"name": "test "}},
{"snapshot": {"description": " sample description ",
"name": " test name "}})
def test_snapshot_create_with_leading_trailing_spaces(self, body):
volume = utils.create_volume(self.ctx)
body['snapshot']['volume_id'] = volume.id
req = fakes.HTTPRequest.blank('/v2/snapshots')
resp_dict = self.controller.create(req, body=body)
self.assertEqual(body['snapshot']['display_name'].strip(),
resp_dict['snapshot']['name'])
self.assertEqual(body['snapshot']['description'].strip(),
resp_dict['snapshot']['description'])
db.volume_destroy(self.ctx, volume.id)
@mock.patch.object(volume.api.API, "update_snapshot",
side_effect=v2_fakes.fake_snapshot_update)
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
@ -316,6 +334,54 @@ class SnapshotApiTest(test.TestCase):
self.assertRaises(exception.SnapshotNotFound, self.controller.update,
req, 'not-the-uuid', body=body)
@mock.patch.object(volume.api.API, "update_snapshot",
side_effect=v2_fakes.fake_snapshot_update)
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
@mock.patch('cinder.db.volume_get')
@mock.patch('cinder.objects.Snapshot.get_by_id')
def test_snapshot_update_with_leading_trailing_spaces(
self, snapshot_get_by_id, volume_get,
snapshot_metadata_get, update_snapshot):
snapshot = {
'id': UUID,
'volume_id': fake.VOLUME_ID,
'status': fields.SnapshotStatus.AVAILABLE,
'created_at': "2018-01-14 00:00:00",
'volume_size': 100,
'display_name': 'Default name',
'display_description': 'Default description',
'expected_attrs': ['metadata'],
}
ctx = context.RequestContext(fake.PROJECT_ID, fake.USER_ID, True)
snapshot_obj = fake_snapshot.fake_snapshot_obj(ctx, **snapshot)
fake_volume_obj = fake_volume.fake_volume_obj(ctx)
snapshot_get_by_id.return_value = snapshot_obj
volume_get.return_value = fake_volume_obj
updates = {
"name": " test ",
"description": " test "
}
body = {"snapshot": updates}
req = fakes.HTTPRequest.blank('/v2/snapshots/%s' % UUID)
res_dict = self.controller.update(req, UUID, body=body)
expected = {
'snapshot': {
'id': UUID,
'volume_id': fake.VOLUME_ID,
'status': fields.SnapshotStatus.AVAILABLE,
'size': 100,
'created_at': datetime.datetime(2018, 1, 14, 0, 0, 0,
tzinfo=pytz.utc),
'updated_at': None,
'name': u'test',
'description': u'test',
'metadata': {},
}
}
self.assertEqual(expected, res_dict)
self.assertEqual(2, len(self.notifier.notifications))
@mock.patch.object(volume.api.API, "delete_snapshot",
side_effect=v2_fakes.fake_snapshot_update)
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())

39
cinder/tests/unit/api/v3/test_backups.py
View File

@ -15,9 +15,12 @@
"""The backups V3 api."""
import copy
import ddt
import mock
from oslo_serialization import jsonutils
from oslo_utils import strutils
import webob
from cinder.api import microversions as mv
from cinder.api.openstack import api_version_request as api_version
@ -44,6 +47,8 @@ class BackupsControllerAPITestCase(test.TestCase):
auth_token=True,
is_admin=True)
self.controller = backups.BackupsController()
self.user_context = context.RequestContext(
fake.USER_ID, fake.PROJECT_ID, auth_token=True)
def _fake_update_request(self, backup_id, version=mv.BACKUP_UPDATE):
req = fakes.HTTPRequest.blank('/v3/%s/backups/%s/update' %
@ -231,6 +236,40 @@ class BackupsControllerAPITestCase(test.TestCase):
self.assertEqual(new_description,
backup.display_description)
@ddt.data({"backup": {"description": " sample description",
"name": " test name"}},
{"backup": {"description": "sample description ",
"name": "test "}},
{"backup": {"description": " sample description ",
"name": " test "}})
def test_backup_update_name_description_with_leading_trailing_spaces(
self, body):
backup = test_utils.create_backup(
self.ctxt,
status=fields.BackupStatus.AVAILABLE)
req = self._fake_update_request(fake.BACKUP_ID)
expected_body = copy.deepcopy(body)
self.controller.update(req,
backup.id,
body=body)
backup.refresh()
# backup update call doesn't return 'description' in response so get
# the updated backup to assert name and description
req = webob.Request.blank('/v2/%s/backups/%s' % (
fake.PROJECT_ID, backup.id))
req.method = 'GET'
req.headers['Content-Type'] = 'application/json'
res = req.get_response(fakes.wsgi_app(
fake_auth_context=self.user_context))
res_dict = jsonutils.loads(res.body)
self.assertEqual(expected_body['backup']['name'].strip(),
res_dict['backup']['name'])
self.assertEqual(expected_body['backup']['description'].strip(),
res_dict['backup']['description'])
@ddt.data(mv.get_prior_version(mv.BACKUP_METADATA),
mv.BACKUP_METADATA)
def test_backup_show_with_metadata(self, version):