From bfc27c9abaf366b6534ccb9e032cc26fd3ad3b65 Mon Sep 17 00:00:00 2001
From: Yikun Jiang <yikunkero@gmail.com>
Date: Thu, 18 Oct 2018 11:47:29 +0800
Subject: [PATCH] Forbidden to revert volume to a different size snapshot

As mentioned from original design:
"As we support extend volumes at present, we could meet the
case that snapshot is smaller than volume when reverting to
snapshot and it's obviously safe to revert in that case. But we
will still restrict to revert the volume which current volume size
is equal to snapshot, cause we don't support shrink volume yet
(that ability will be used in the generic method, if the driver
don't support revert to snapshot)."

So, we need add a size check before we revert a volume to a
snapshot. If the volume size is not equal to its latest
snapshot size, it will be raise a HTTPBadRequest(400).

Change-Id: Ib3cc95a10870822b9f597d66007699fb40908b61
Closes-bug: #1798503
---
 cinder/api/v3/volumes.py                 |  6 +++++
 cinder/tests/unit/api/v3/test_volumes.py | 28 ++++++++++++++++++++----
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/cinder/api/v3/volumes.py b/cinder/api/v3/volumes.py
index 0986a33194f..b2e51e9d713 100644
--- a/cinder/api/v3/volumes.py
+++ b/cinder/api/v3/volumes.py
@@ -190,6 +190,12 @@ class VolumeController(volumes_v2.VolumeController):
                     "the latest one of volume %(v_id)s.")
             raise exc.HTTPBadRequest(explanation=msg % {'s_id': snapshot_id,
                                                         'v_id': volume.id})
+        if volume.size != l_snap.volume_size:
+            msg = _("Can't revert volume %(v_id)s to its latest snapshot "
+                    "%(s_id)s. The volume size must be equal to the snapshot "
+                    "size.")
+            raise exc.HTTPBadRequest(explanation=msg % {'s_id': snapshot_id,
+                                                        'v_id': volume.id})
         try:
             msg = 'Reverting volume %(v_id)s to snapshot %(s_id)s.'
             LOG.info(msg, {'v_id': volume.id,
diff --git a/cinder/tests/unit/api/v3/test_volumes.py b/cinder/tests/unit/api/v3/test_volumes.py
index 615c6586e67..94fcdd85b05 100644
--- a/cinder/tests/unit/api/v3/test_volumes.py
+++ b/cinder/tests/unit/api/v3/test_volumes.py
@@ -786,20 +786,22 @@ class VolumeApiTest(test.TestCase):
         else:
             self.assertNotIn('provider_id', res_dict['volume'])
 
-    def _fake_create_volume(self):
+    def _fake_create_volume(self, size=1):
         vol = {
             'display_name': 'fake_volume1',
-            'status': 'available'
+            'status': 'available',
+            'size': size
         }
         volume = objects.Volume(context=self.ctxt, **vol)
         volume.create()
         return volume
 
-    def _fake_create_snapshot(self, volume_id):
+    def _fake_create_snapshot(self, volume_id, volume_size=1):
         snap = {
             'display_name': 'fake_snapshot1',
             'status': 'available',
-            'volume_id': volume_id
+            'volume_id': volume_id,
+            'volume_size': volume_size
         }
         snapshot = objects.Snapshot(context=self.ctxt, **snap)
         snapshot.create()
@@ -870,6 +872,24 @@ class VolumeApiTest(test.TestCase):
                           req, fake_volume['id'], {'revert': {'snapshot_id':
                                                    fake_snapshot['id']}})
 
+    @mock.patch.object(objects.Volume, 'get_latest_snapshot')
+    @mock.patch.object(volume_api.API, 'get_volume')
+    def test_volume_revert_with_not_equal_size(self, mock_volume,
+                                               mock_latest):
+        fake_volume = self._fake_create_volume(size=2)
+        fake_snapshot = self._fake_create_snapshot(fake_volume['id'],
+                                                   volume_size=1)
+        mock_volume.return_value = fake_volume
+        mock_latest.return_value = fake_snapshot
+        req = fakes.HTTPRequest.blank('/v3/volumes/%s/revert'
+                                      % fake_volume['id'])
+        req.headers = mv.get_mv_header(mv.VOLUME_REVERT)
+        req.api_version_request = mv.get_api_version(
+            mv.VOLUME_REVERT)
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.revert,
+                          req, fake_volume['id'],
+                          {'revert': {'snapshot_id': fake_snapshot['id']}})
+
     def test_view_get_attachments(self):
         fake_volume = self._fake_create_volume()
         fake_volume['attach_status'] = fields.VolumeAttachStatus.ATTACHING