From 6590dd2900b3e2166cef239e0b88c8245d83ce41 Mon Sep 17 00:00:00 2001
From: ghanshyam <ghanshyammann@gmail.com>
Date: Fri, 20 Oct 2017 11:48:00 +0300
Subject: [PATCH] Implement query param schema for delete assisted vol

DELETE assisted_volume_snapshots API accept 'delete_info'
query param.

This commit adds json schema to validate the valid
query parameters.

There is no change in API behaviour and additionalProperties
is kept True for backward compatibility.

Partially implements blueprint json-schema-validation-for-query-param

Change-Id: Ie1e81dfdb1e42af87725977c0f9504ea35ca9d70
---
 .../compute/assisted_volume_snapshots.py      |  1 +
 .../schemas/assisted_volume_snapshots.py      | 14 +++++++
 .../api/openstack/compute/test_volumes.py     | 38 +++++++++++++++++++
 3 files changed, 53 insertions(+)

diff --git a/nova/api/openstack/compute/assisted_volume_snapshots.py b/nova/api/openstack/compute/assisted_volume_snapshots.py
index b2f60933a4a0..a0e1614027c5 100644
--- a/nova/api/openstack/compute/assisted_volume_snapshots.py
+++ b/nova/api/openstack/compute/assisted_volume_snapshots.py
@@ -63,6 +63,7 @@ class AssistedVolumeSnapshotsController(wsgi.Controller):
             raise exc.HTTPBadRequest(explanation=e.format_message())
 
     @wsgi.response(204)
+    @validation.query_schema(assisted_volume_snapshots.delete_query)
     @extensions.expected_errors((400, 404))
     def delete(self, req, id):
         """Delete a snapshot."""
diff --git a/nova/api/openstack/compute/schemas/assisted_volume_snapshots.py b/nova/api/openstack/compute/schemas/assisted_volume_snapshots.py
index 0b7f3ba2522e..74e3da343471 100644
--- a/nova/api/openstack/compute/schemas/assisted_volume_snapshots.py
+++ b/nova/api/openstack/compute/schemas/assisted_volume_snapshots.py
@@ -12,6 +12,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from nova.api.validation import parameter_types
+
 snapshots_create = {
     'type': 'object',
     'properties': {
@@ -48,3 +50,15 @@ snapshots_create = {
     'required': ['snapshot'],
     'additionalProperties': False,
 }
+
+delete_query = {
+    'type': 'object',
+    'properties': {
+        'delete_info': parameter_types.multi_params({'type': 'string'})
+    },
+    # NOTE(gmann): This is kept True to keep backward compatibility.
+    # As of now Schema validation stripped out the additional parameters and
+    # does not raise 400. In the future, we may block the additional parameters
+    # by bump in Microversion.
+    'additionalProperties': True
+}
diff --git a/nova/tests/unit/api/openstack/compute/test_volumes.py b/nova/tests/unit/api/openstack/compute/test_volumes.py
index 84c69d11ddbd..b732e33ac5b6 100644
--- a/nova/tests/unit/api/openstack/compute/test_volumes.py
+++ b/nova/tests/unit/api/openstack/compute/test_volumes.py
@@ -19,6 +19,7 @@ import datetime
 import mock
 from oslo_serialization import jsonutils
 from oslo_utils import encodeutils
+import six
 from six.moves import urllib
 import webob
 from webob import exc
@@ -951,6 +952,43 @@ class AssistedSnapshotDeleteTestCaseV21(test.NoDBTestCase):
         api_error = exception.InstanceNotReady(instance_id=FAKE_UUID)
         self._test_assisted_delete_instance_conflict(api_error)
 
+    def test_delete_additional_query_parameters(self):
+        params = {
+            'delete_info': jsonutils.dumps({'volume_id': '1'}),
+            'additional': 123
+        }
+        req = fakes.HTTPRequest.blank(
+                '/v2/fake/os-assisted-volume-snapshots?%s' %
+                urllib.parse.urlencode(params))
+        req.method = 'DELETE'
+        self.controller.delete(req, '5')
+
+    def test_delete_duplicate_query_parameters_validation(self):
+        params = {
+            'delete_info': jsonutils.dumps({'volume_id': '1'}),
+            'delete_info': jsonutils.dumps({'volume_id': '2'})
+        }
+        req = fakes.HTTPRequest.blank(
+                '/v2/fake/os-assisted-volume-snapshots?%s' %
+                urllib.parse.urlencode(params))
+        req.method = 'DELETE'
+        self.controller.delete(req, '5')
+
+    def test_assisted_delete_missing_volume_id(self):
+        params = {
+            'delete_info': jsonutils.dumps({'something_else': '1'}),
+        }
+        req = fakes.HTTPRequest.blank(
+                '/v2/fake/os-assisted-volume-snapshots?%s' %
+                urllib.parse.urlencode(params))
+
+        req.method = 'DELETE'
+        ex = self.assertRaises(webob.exc.HTTPBadRequest,
+                               self.controller.delete, req, '5')
+        # This is the result of a KeyError but the only thing in the message
+        # is the missing key.
+        self.assertIn('volume_id', six.text_type(ex))
+
 
 class TestAssistedVolumeSnapshotsPolicyEnforcementV21(test.NoDBTestCase):