openstack/cinder
Code Issues Proposed changes

V3 jsonschema validation: Backups

Browse Source 
This patch adds jsonschema validation for below Backups API's
* POST /v3/{project_id}/backups
* PUT  /v3/{project_id}/backups/{backup_id}
* POST /v3/{project_id}/backups/{backup_id}/restore
* POST /v3/{project_id}/backups/{backup_id}/import_record

Made changes to unit tests to pass body as keyword argument as wsgi
calls action method [1] and passes body as keyword argument.

[1] https://github.com/openstack/cinder/blob/master/cinder/api/openstack/wsgi.py#L997

Change-Id: Idd6c6be1c8bdf4dcf730f67e75a58a0329fe5259
Partial-Implements: bp json-schema-validation
This commit is contained in:
pooja jadhav 2017-10-30 12:08:17 +05:30
parent b6d2ec994c
commit 1dc6fd93c2
8 changed files with 249 additions and 104 deletions

40
cinder/api/contrib/backups.py

@ -18,6 +18,7 @@
"""The backups api."""
from oslo_log import log as logging
from oslo_utils import strutils
from six.moves import http_client
from webob import exc
@ -25,10 +26,11 @@ from cinder.api import common
from cinder.api import extensions
from cinder.api import microversions as mv
from cinder.api.openstack import wsgi
from cinder.api.schemas import backups as backup
from cinder.api import validation
from cinder.api.views import backups as backup_views
from cinder import backup as backupAPI
from cinder import exception
from cinder.i18n import _
from cinder import utils
from cinder import volume as volumeAPI
@ -141,29 +143,25 @@ class BackupsController(wsgi.Controller):
# immediately
# - maybe also do validation of swift container name
@wsgi.response(http_client.ACCEPTED)
@validation.schema(backup.create, '2.0', '3.42')
@validation.schema(backup.create_backup_v343, '3.43')
def create(self, req, body):
"""Create a new backup."""
LOG.debug('Creating new backup %s', body)
self.assert_valid_body(body, 'backup')
context = req.environ['cinder.context']
backup = body['backup']
req_version = req.api_version_request
try:
volume_id = backup['volume_id']
except KeyError:
msg = _("Incorrect request body format")
raise exc.HTTPBadRequest(explanation=msg)
backup = body['backup']
container = backup.get('container', None)
if container:
utils.check_string_length(container, 'Backup container',
min_length=0, max_length=255)
self.validate_name_and_description(backup)
volume_id = backup['volume_id']
name = backup.get('name', None)
description = backup.get('description', None)
incremental = backup.get('incremental', False)
force = backup.get('force', False)
incremental = strutils.bool_from_string(backup.get(
'incremental', False), strict=True)
force = strutils.bool_from_string(backup.get(
'force', False), strict=True)
snapshot_id = backup.get('snapshot_id', None)
metadata = backup.get('metadata', None) if req_version.matches(
mv.BACKUP_METADATA) else None
@ -190,11 +188,11 @@ class BackupsController(wsgi.Controller):
return retval
@wsgi.response(http_client.ACCEPTED)
@validation.schema(backup.restore)
def restore(self, req, id, body):
"""Restore an existing backup to a volume."""
LOG.debug('Restoring backup %(backup_id)s (%(body)s)',
{'backup_id': id, 'body': body})
self.assert_valid_body(body, 'restore')
context = req.environ['cinder.context']
restore = body['restore']
@ -241,19 +239,15 @@ class BackupsController(wsgi.Controller):
return retval
@wsgi.response(http_client.CREATED)
@validation.schema(backup.import_record)
def import_record(self, req, body):
"""Import a backup."""
LOG.debug('Importing record from %s.', body)
self.assert_valid_body(body, 'backup-record')
context = req.environ['cinder.context']
import_data = body['backup-record']
# Verify that body elements are provided
try:
backup_service = import_data['backup_service']
backup_url = import_data['backup_url']
except KeyError:
msg = _("Incorrect request body format.")
raise exc.HTTPBadRequest(explanation=msg)
backup_service = import_data['backup_service']
backup_url = import_data['backup_url']
LOG.debug('Importing backup using %(service)s and url %(url)s.',
{'service': backup_service, 'url': backup_url})

108
cinder/api/schemas/backups.py Normal file

@ -0,0 +1,108 @@
# Copyright (C) 2017 NTT DATA
# All Rights Reserved.
#
# 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.
"""
Schema for V3 Backups API.
"""
import copy
from cinder.api.validation import parameter_types
create = {
'type': 'object',
'properties': {
'type': 'object',
'backup': {
'type': 'object',
'properties': {
'volume_id': parameter_types.uuid,
'container': parameter_types.container,
'description': parameter_types.description,
'incremental': parameter_types.boolean,
'force': parameter_types.boolean,
'name': parameter_types.name_allow_zero_min_length,
'snapshot_id': parameter_types.uuid,
},
'required': ['volume_id'],
'additionalProperties': False,
},
},
'required': ['backup'],
'additionalProperties': False,
}
create_backup_v343 = copy.deepcopy(create)
create_backup_v343['properties']['backup']['properties'][
'metadata'] = parameter_types.extra_specs
update = {
'type': 'object',
'properties': {
'type': 'object',
'backup': {
'type': ['object', 'null'],
'properties': {
'name': parameter_types.name_allow_zero_min_length,
'description': parameter_types.description,
},
'additionalProperties': False,
},
},
'required': ['backup'],
'additionalProperties': False,
}
update_backup_v343 = copy.deepcopy(update)
update_backup_v343['properties']['backup']['properties'][
'metadata'] = parameter_types.extra_specs
restore = {
'type': 'object',
'properties': {
'type': 'object',
'restore': {
'type': ['object', 'null'],
'properties': {
'name': parameter_types.name_allow_zero_min_length,
'volume_id': parameter_types.uuid
},
'additionalProperties': False,
},
},
'required': ['restore'],
'additionalProperties': False,
}
import_record = {
'type': 'object',
'properties': {
'type': 'object',
'backup-record': {
'type': 'object',
'properties': {
'backup_service': parameter_types.backup_service,
'backup_url': parameter_types.backup_url
},
'required': ['backup_service', 'backup_url'],
'additionalProperties': False,
},
},
'required': ['backup-record'],
'additionalProperties': False,
}

12
cinder/api/v3/backups.py

@ -16,13 +16,13 @@
"""The backups V3 API."""
from oslo_log import log as logging
from webob import exc
from cinder.api.contrib import backups as backups_v2
from cinder.api import microversions as mv
from cinder.api.openstack import wsgi
from cinder.api.schemas import backups as backup
from cinder.api.v3.views import backups as backup_views
from cinder.i18n import _
from cinder.api import validation
from cinder.policies import backups as policy
@ -35,15 +35,15 @@ class BackupsController(backups_v2.BackupsController):
_view_builder_class = backup_views.ViewBuilder
@wsgi.Controller.api_version(mv.BACKUP_UPDATE)
@validation.schema(backup.update, '3.9', '3.42')
@validation.schema(backup.update_backup_v343, '3.43')
def update(self, req, id, body):
"""Update a backup."""
context = req.environ['cinder.context']
self.assert_valid_body(body, 'backup')
req_version = req.api_version_request
backup_update = body['backup']
self.validate_name_and_description(backup_update)
update_dict = {}
if 'name' in backup_update:
update_dict['display_name'] = backup_update.pop('name')
@ -53,10 +53,6 @@ class BackupsController(backups_v2.BackupsController):
if (req_version.matches(
mv.BACKUP_METADATA) and 'metadata' in backup_update):
update_dict['metadata'] = backup_update.pop('metadata')
# Check no unsupported fields.
if backup_update:
msg = _("Unsupported fields %s.") % (", ".join(backup_update))
raise exc.HTTPBadRequest(explanation=msg)
new_backup = self.backup_api.update(context, id, update_dict)

10
cinder/api/validation/parameter_types.py

@ -175,3 +175,13 @@ uuid_allow_null = {
metadata_allows_null = copy.deepcopy(extra_specs)
metadata_allows_null['type'] = ['object', 'null']
container = {
'type': ['string', 'null'], 'minLength': 0, 'maxLength': 255}
backup_url = {'type': 'string', 'minLength': 1, 'format': 'base64'}
backup_service = {'type': 'string', 'minLength': 0, 'maxLength': 255}

17
cinder/api/validation/validators.py

@ -18,6 +18,7 @@ Internal implementation of request Body validating middleware.
"""
import base64
import re
import jsonschema
@ -124,6 +125,22 @@ def _validate_status(param_value):
return True
@jsonschema.FormatChecker.cls_checks('base64')
def _validate_base64_format(instance):
try:
if isinstance(instance, six.text_type):
instance = instance.encode('utf-8')
base64.decodestring(instance)
except base64.binascii.Error:
return False
except TypeError:
# The name must be string type. If instance isn't string type, the
# TypeError will be raised at here.
return False
return True
class FormatChecker(jsonschema.FormatChecker):
"""A FormatChecker can output the message from cause exception

2
cinder/backup/api.py

@ -40,7 +40,6 @@ from cinder.policies import backups as policy
import cinder.policy
from cinder import quota
from cinder import quota_utils
from cinder import utils
import cinder.volume
from cinder.volume import utils as volume_utils
@ -200,7 +199,6 @@ class API(base.Base):
force=False, snapshot_id=None, metadata=None):
"""Make the RPC call to create a volume backup."""
context.authorize(policy.CREATE_POLICY)
utils.check_metadata_properties(metadata)
volume = self.volume_api.get(context, volume_id)
snapshot = None
if snapshot_id:

151
cinder/tests/unit/api/contrib/test_backups.py

@ -21,6 +21,7 @@ import ddt
import mock
from oslo_serialization import jsonutils
from oslo_utils import timeutils
import six
from six.moves import http_client
import webob
@ -482,10 +483,7 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
@mock.patch('cinder.db.service_get_all')
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_create_backup_json(self, mock_validate,
_mock_service_get_all):
def test_create_backup_json(self, _mock_service_get_all):
_mock_service_get_all.return_value = [
{'availability_zone': 'fake_az', 'host': 'testhost',
'disabled': 0, 'updated_at': timeutils.utcnow(),
@ -493,8 +491,8 @@ class BackupsAPITestCase(test.TestCase):
volume = utils.create_volume(self.context, size=5)
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
@ -514,15 +512,11 @@ class BackupsAPITestCase(test.TestCase):
_mock_service_get_all.assert_called_once_with(mock.ANY,
disabled=False,
topic='cinder-backup')
self.assertTrue(mock_validate.called)
volume.destroy()
@mock.patch('cinder.db.service_get_all')
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_create_backup_with_metadata(self, mock_validate,
_mock_service_get_all):
def test_create_backup_with_metadata(self, _mock_service_get_all):
_mock_service_get_all.return_value = [
{'availability_zone': 'fake_az', 'host': 'testhost',
'disabled': 0, 'updated_at': timeutils.utcnow(),
@ -530,8 +524,8 @@ class BackupsAPITestCase(test.TestCase):
volume = utils.create_volume(self.context, size=1)
# Create a backup with metadata
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
@ -606,8 +600,8 @@ class BackupsAPITestCase(test.TestCase):
status=fields.BackupStatus.AVAILABLE,
size=1, availability_zone='az1',
host='testhost')
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
@ -633,10 +627,7 @@ class BackupsAPITestCase(test.TestCase):
volume.destroy()
@mock.patch('cinder.db.service_get_all')
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_create_backup_snapshot_json(self, mock_validate,
_mock_service_get_all):
def test_create_backup_snapshot_json(self, _mock_service_get_all):
_mock_service_get_all.return_value = [
{'availability_zone': 'fake_az', 'host': 'testhost',
'disabled': 0, 'updated_at': timeutils.utcnow(),
@ -644,8 +635,8 @@ class BackupsAPITestCase(test.TestCase):
volume = utils.create_volume(self.context, size=5, status='available')
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
@ -664,7 +655,6 @@ class BackupsAPITestCase(test.TestCase):
_mock_service_get_all.assert_called_once_with(mock.ANY,
disabled=False,
topic='cinder-backup')
self.assertTrue(mock_validate.called)
volume.destroy()
@ -727,8 +717,8 @@ class BackupsAPITestCase(test.TestCase):
def test_create_backup_with_non_existent_snapshot(self):
volume = utils.create_volume(self.context, size=5, status='restoring')
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"snapshot_id": fake.SNAPSHOT_ID,
"volume_id": volume.id,
@ -761,17 +751,15 @@ class BackupsAPITestCase(test.TestCase):
req.method = 'POST'
req.environ['cinder.context'] = self.context
req.api_version_request = api_version.APIVersionRequest()
self.assertRaises(exception.InvalidInput,
req.api_version_request = api_version.APIVersionRequest("2.0")
self.assertRaises(exception.ValidationError,
self.controller.create,
req,
body)
body=body)
@mock.patch('cinder.db.service_get_all')
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
@ddt.data(False, True)
def test_create_backup_delta(self, backup_from_snapshot,
mock_validate,
_mock_service_get_all):
_mock_service_get_all.return_value = [
{'availability_zone': 'fake_az', 'host': 'testhost',
@ -780,26 +768,35 @@ class BackupsAPITestCase(test.TestCase):
volume = utils.create_volume(self.context, size=5)
snapshot = None
snapshot_id = None
if backup_from_snapshot:
snapshot = utils.create_snapshot(self.context,
volume.id,
status=
fields.SnapshotStatus.AVAILABLE)
snapshot_id = snapshot.id
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
"incremental": True,
"snapshot_id": snapshot_id,
}
}
else:
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
"incremental": True,
}
}
backup = utils.create_backup(self.context, volume.id,
status=fields.BackupStatus.AVAILABLE,
size=1, availability_zone='az1',
host='testhost')
body = {"backup": {"display_name": "nightly001",
"display_description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
"incremental": True,
"snapshot_id": snapshot_id,
}
}
req = webob.Request.blank('/v2/%s/backups' % fake.PROJECT_ID)
req.method = 'POST'
req.headers['Content-Type'] = 'application/json'
@ -813,7 +810,6 @@ class BackupsAPITestCase(test.TestCase):
_mock_service_get_all.assert_called_once_with(mock.ANY,
disabled=False,
topic='cinder-backup')
self.assertTrue(mock_validate.called)
backup.destroy()
if snapshot:
@ -833,8 +829,8 @@ class BackupsAPITestCase(test.TestCase):
backup = utils.create_backup(self.context, volume.id,
availability_zone='az1', size=1,
host='testhost')
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
@ -872,13 +868,13 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
self.assertEqual(http_client.BAD_REQUEST,
res_dict['badRequest']['code'])
self.assertEqual("Missing required element 'backup' in request body.",
self.assertEqual("None is not of type 'object'",
res_dict['badRequest']['message'])
def test_create_backup_with_body_KeyError(self):
# omit volume_id from body
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"container": "nightlybackups",
}
@ -894,12 +890,12 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
self.assertEqual(http_client.BAD_REQUEST,
res_dict['badRequest']['code'])
self.assertEqual('Incorrect request body format',
res_dict['badRequest']['message'])
self.assertIn("'volume_id' is a required property",
res_dict['badRequest']['message'])
def test_create_backup_with_VolumeNotFound(self):
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": fake.WILL_NOT_BE_FOUND_ID,
"container": "nightlybackups",
@ -951,8 +947,8 @@ class BackupsAPITestCase(test.TestCase):
volume = utils.create_volume(self.context, size=2)
req = webob.Request.blank('/v2/%s/backups' % fake.PROJECT_ID)
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
@ -985,8 +981,8 @@ class BackupsAPITestCase(test.TestCase):
volume = utils.create_volume(self.context, size=5, status='available')
body = {"backup": {"display_name": "nightly001",
"display_description":
body = {"backup": {"name": "nightly001",
"description":
"Nightly Backup 03-Sep-2012",
"volume_id": volume.id,
"container": "nightlybackups",
@ -1334,7 +1330,7 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
self.assertEqual(http_client.BAD_REQUEST,
res_dict['badRequest']['code'])
self.assertEqual("Missing required element 'restore' in request body.",
self.assertEqual("None is not of type 'object'",
res_dict['badRequest']['message'])
backup.destroy()
@ -1359,8 +1355,14 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
self.assertEqual(http_client.BAD_REQUEST,
res_dict['badRequest']['code'])
self.assertEqual("Missing required element 'restore' in request body.",
res_dict['badRequest']['message'])
if six.PY3:
self.assertEqual("Additional properties are not allowed "
"('' was unexpected)",
res_dict['badRequest']['message'])
else:
self.assertEqual("Additional properties are not allowed "
"(u'' was unexpected)",
res_dict['badRequest']['message'])
@mock.patch('cinder.db.service_get_all')
@mock.patch('cinder.volume.api.API.create')
@ -2088,8 +2090,18 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
self.assertEqual(http_client.BAD_REQUEST,
res_dict['badRequest']['code'])
self.assertEqual('Incorrect request body format.',
res_dict['badRequest']['message'])
if six.PY3:
self.assertEqual(
"Invalid input for field/attribute backup-record. "
"Value: {'backup_url': 'fake'}. 'backup_service' "
"is a required property",
res_dict['badRequest']['message'])
else:
self.assertEqual(
"Invalid input for field/attribute backup-record. "
"Value: {u'backup_url': u'fake'}. 'backup_service' "
"is a required property",
res_dict['badRequest']['message'])
# test with no backup_url
req = webob.Request.blank('/v2/%s/backups/import_record' %
@ -2104,8 +2116,18 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
self.assertEqual(http_client.BAD_REQUEST,
res_dict['badRequest']['code'])
self.assertEqual('Incorrect request body format.',
res_dict['badRequest']['message'])
if six.PY3:
self.assertEqual(
"Invalid input for field/attribute backup-record. "
"Value: {'backup_service': 'fake'}. 'backup_url' "
"is a required property",
res_dict['badRequest']['message'])
else:
self.assertEqual(
"Invalid input for field/attribute backup-record. "
"Value: {u'backup_service': u'fake'}. 'backup_url' "
"is a required property",
res_dict['badRequest']['message'])
# test with no backup_url and backup_url
req = webob.Request.blank('/v2/%s/backups/import_record' %
@ -2120,8 +2142,10 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
self.assertEqual(http_client.BAD_REQUEST,
res_dict['badRequest']['code'])
self.assertEqual('Incorrect request body format.',
res_dict['badRequest']['message'])
self.assertEqual(
"Invalid input for field/attribute backup-record. "
"Value: {}. 'backup_service' is a required property",
res_dict['badRequest']['message'])
def test_import_record_with_no_body(self):
ctx = context.RequestContext(fake.USER_ID, fake.PROJECT_ID,
@ -2139,8 +2163,7 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(http_client.BAD_REQUEST, res.status_int)
self.assertEqual(http_client.BAD_REQUEST,
res_dict['badRequest']['code'])
self.assertEqual("Missing required element 'backup-record' in "
"request body.",
self.assertEqual("None is not of type 'object'",
res_dict['badRequest']['message'])
@mock.patch('cinder.backup.rpcapi.BackupAPI.check_support_to_force_delete',

13
cinder/tests/unit/api/v3/test_backups.py

@ -18,7 +18,6 @@
import ddt
import mock
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
@ -66,17 +65,17 @@ class BackupsControllerAPITestCase(test.TestCase):
def test_backup_update_with_no_body(self):
# omit body from the request
req = self._fake_update_request(fake.BACKUP_ID)
self.assertRaises(webob.exc.HTTPBadRequest,
self.assertRaises(exception.ValidationError,
self.controller.update,
req, fake.BACKUP_ID, None)
req, fake.BACKUP_ID, body=None)
def test_backup_update_with_unsupported_field(self):
req = self._fake_update_request(fake.BACKUP_ID)
body = {"backup": {"id": fake.BACKUP2_ID,
"description": "", }}
self.assertRaises(webob.exc.HTTPBadRequest,
self.assertRaises(exception.ValidationError,
self.controller.update,
req, fake.BACKUP_ID, body)
req, fake.BACKUP_ID, body=body)
def test_backup_update_with_backup_not_found(self):
req = self._fake_update_request(fake.BACKUP_ID)
@ -87,7 +86,7 @@ class BackupsControllerAPITestCase(test.TestCase):
body = {"backup": updates}
self.assertRaises(exception.NotFound,
self.controller.update,
req, fake.BACKUP_ID, body)
req, fake.BACKUP_ID, body=body)
def _create_multiple_backups_with_different_project(self):
test_utils.create_backup(
@ -225,7 +224,7 @@ class BackupsControllerAPITestCase(test.TestCase):
body = {"backup": updates}
self.controller.update(req,
backup.id,
body)
body=body)
backup.refresh()
self.assertEqual(new_name, backup.display_name)