Fix os-detach attachment_id schema

The os-detach attachment_id schema is more restrictive
in a non-backwards compatible way in that it was previously
possible to do:

POST /volume/v3/{project_id}/volumes/{volume_id}/action
{
    "os-detach": {
        "attachment_id": null
    }
}

With the schema change I39ede009d5e909a076860df7305865286caa5352
attachment_id is still optional but if specified, it must be
a non-null UUID string, which is not backward compatible and
can break old client code.

This change makes the attachment_id parameter value optional
again and also fixes the "uuid_allow_null" parameter type
definition which previously allowed non-uuid format strings.

Change-Id: Ifb97457a03795b84287922a5389fab91c402380f
Closes-Bug: #1768650

cinder/api/schemas/volume_actions.py

@@ -73,7 +73,8 @@ detach = {
         'os-detach': {
             'type': ['object', 'null'],
             'properties': {
-                'attachment_id': parameter_types.uuid,
+                # NOTE(mriedem): This allows null for backward compatibility.
+                'attachment_id': parameter_types.uuid_allow_null,
             },
             'additionalProperties': False,
         },

cinder/api/validation/parameter_types.py

@@ -182,7 +182,7 @@ name_allow_zero_min_length = {
 
 
 uuid_allow_null = {
-    'type': ['string', 'null']
+    'oneOf': [uuid, {'type': 'null'}]
 }
 
 

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

@@ -988,6 +988,25 @@ class BackupsAPITestCase(test.TestCase):
                          fake.WILL_NOT_BE_FOUND_ID,
                          res_dict['itemNotFound']['message'])
 
+    def test_create_backup_with_invalid_volume_id_format(self):
+        body = {"backup": {"name": "nightly001",
+                           "description":
+                           "Nightly Backup 03-Sep-2012",
+                           "volume_id": 'not a uuid',
+                           "container": "nightlybackups",
+                           }
+                }
+        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)
+        self.assertEqual(http_client.BAD_REQUEST, res.status_int)
+        self.assertIn("'not a uuid' is not a 'uuid'",
+                      res_dict['badRequest']['message'])
+
     def test_create_backup_with_InvalidVolume(self):
         # need to create the volume referenced below first
         volume = utils.create_volume(self.context, size=5, status='restoring')

cinder/tests/unit/api/contrib/test_snapshot_manage.py

@@ -253,10 +253,12 @@ class SnapshotManageTest(test.TestCase):
         self.assertEqual(http_client.BAD_REQUEST, res.status_int)
 
     def test_manage_snapshot_error_volume_id(self):
-        """Test correct failure when volume can't be found."""
+        """Test correct failure when volume id is invalid format."""
         body = {'snapshot': {'volume_id': 'error_volume_id', 'ref': {}}}
         res = self._get_resp_post(body)
         self.assertEqual(http_client.BAD_REQUEST, res.status_int)
+        self.assertIn("'error_volume_id' is not a 'uuid'",
+                      jsonutils.loads(res.body)['badRequest']['message'])
 
     def _get_resp_get(self, host, detailed, paging, admin=True):
         """Helper to execute a GET os-snapshot-manage API call."""

cinder/tests/unit/api/contrib/test_volume_actions.py

@@ -299,6 +299,18 @@ class VolumeActionsTest(test.TestCase):
             fake_auth_context=self.context))
         self.assertEqual(http_client.ACCEPTED, res.status_int)
 
+    def test_detach_null_attachment_id(self):
+        body = {'os-detach': {'attachment_id': None}}
+        req = webob.Request.blank('/v2/%s/volumes/%s/action' %
+                                  (fake.PROJECT_ID, fake.VOLUME_ID))
+        req.method = "POST"
+        req.body = jsonutils.dump_as_bytes(body)
+        req.headers["content-type"] = "application/json"
+
+        res = req.get_response(fakes.wsgi_app(
+            fake_auth_context=self.context))
+        self.assertEqual(http_client.ACCEPTED, res.status_int)
+
     def test_volume_detach_raises_remote_error(self):
         volume_remote_error = \
             messaging.RemoteError(exc_type='VolumeAttachmentNotFound')