Partial fix for bug 919051

Test instance and volume IDs

Change-Id: Icbdeb3a082e3e85f7d1698875529bfffbf6fa5f2

4 files changed, 217 insertions, 5 deletions

diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py
index 937e1ed80..15501f4ff 100644
--- a/nova/api/ec2/__init__.py
+++ b/nova/api/ec2/__init__.py
@@ -626,6 +626,12 @@ class Executor(wsgi.Application):
             LOG.debug(_('InvalidRequest raised: %s'), unicode(ex),
                      context=context)
             return ec2_error(req, request_id, type(ex).__name__, unicode(ex))
+        except exception.InvalidInstanceIDMalformed as ex:
+            LOG.debug(_('ValidatorError raised: %s'), unicode(ex),
+                     context=context)
+            #EC2 Compatibility
+            return self._error(req, context, "InvalidInstanceID.Malformed",
+                unicode(ex))
         except Exception as ex:
             env = req.environ.copy()
             for k in env.keys():
diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 560a2d0dd..f99cbdaf7 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -43,6 +43,7 @@ from nova import network
 from nova import rpc
 from nova import utils
 from nova import volume
+from nova.api import validator
 
 
 FLAGS = flags.FLAGS
@@ -51,6 +52,15 @@ flags.DECLARE('dhcp_domain', 'nova.network.manager')
 LOG = logging.getLogger(__name__)
 
 
+def validate_ec2_id(val):
+    if not validator.validate_str()(val):
+        raise exception.InvalidInstanceIDMalformed(val)
+    try:
+        ec2utils.ec2_id_to_id(val)
+    except exception.InvalidEc2Id:
+        raise exception.InvalidInstanceIDMalformed(val)
+
+
 def _gen_key(context, user_id, key_name):
     """Generate a key
 
@@ -322,6 +332,7 @@ class CloudController(object):
         return s
 
     def create_snapshot(self, context, volume_id, **kwargs):
+        validate_ec2_id(volume_id)
         LOG.audit(_("Create snapshot of volume %s"), volume_id,
                   context=context)
         volume_id = ec2utils.ec2_id_to_id(volume_id)
@@ -775,6 +786,7 @@ class CloudController(object):
             ec2_id = instance_id[0]
         else:
             ec2_id = instance_id
+        validate_ec2_id(ec2_id)
         instance_id = ec2utils.ec2_id_to_id(ec2_id)
         instance = self.compute_api.get(context, instance_id)
         output = self.compute_api.get_console_output(context, instance)
@@ -787,6 +799,7 @@ class CloudController(object):
         if volume_id:
             volumes = []
             for ec2_id in volume_id:
+                validate_ec2_id(ec2_id)
                 internal_id = ec2utils.ec2_id_to_id(ec2_id)
                 volume = self.volume_api.get(context, internal_id)
                 volumes.append(volume)
@@ -857,12 +870,15 @@ class CloudController(object):
         return self._format_volume(context, dict(volume))
 
     def delete_volume(self, context, volume_id, **kwargs):
+        validate_ec2_id(volume_id)
         volume_id = ec2utils.ec2_id_to_id(volume_id)
         volume = self.volume_api.get(context, volume_id)
         self.volume_api.delete(context, volume)
         return True
 
     def attach_volume(self, context, volume_id, instance_id, device, **kwargs):
+        validate_ec2_id(instance_id)
+        validate_ec2_id(volume_id)
         volume_id = ec2utils.ec2_id_to_id(volume_id)
         instance_id = ec2utils.ec2_id_to_id(instance_id)
         instance = self.compute_api.get(context, instance_id)
@@ -879,6 +895,7 @@ class CloudController(object):
                 'volumeId': ec2utils.id_to_ec2_vol_id(volume_id)}
 
     def detach_volume(self, context, volume_id, **kwargs):
+        validate_ec2_id(volume_id)
         volume_id = ec2utils.ec2_id_to_id(volume_id)
         LOG.audit(_("Detach volume %s"), volume_id, context=context)
         volume = self.volume_api.get(context, volume_id)
@@ -967,6 +984,7 @@ class CloudController(object):
                 _('attribute not supported: %s') % attribute)
 
         ec2_instance_id = instance_id
+        validate_ec2_id(instance_id)
         instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
         instance = self.compute_api.get(context, instance_id)
         result = {'instance_id': ec2_instance_id}
@@ -1250,6 +1268,7 @@ class CloudController(object):
         LOG.debug(_("Going to start terminating instances"))
         previous_states = []
         for ec2_id in instance_id:
+            validate_ec2_id(ec2_id)
             _instance_id = ec2utils.ec2_id_to_id(ec2_id)
             instance = self.compute_api.get(context, _instance_id)
             previous_states.append(instance)
@@ -1262,6 +1281,7 @@ class CloudController(object):
         """instance_id is a list of instance ids"""
         LOG.audit(_("Reboot instance %r"), instance_id, context=context)
         for ec2_id in instance_id:
+            validate_ec2_id(ec2_id)
             _instance_id = ec2utils.ec2_id_to_id(ec2_id)
             instance = self.compute_api.get(context, _instance_id)
             self.compute_api.reboot(context, instance, 'HARD')
@@ -1272,6 +1292,7 @@ class CloudController(object):
         Here instance_id is a list of instance ids"""
         LOG.debug(_("Going to stop instances"))
         for ec2_id in instance_id:
+            validate_ec2_id(ec2_id)
             _instance_id = ec2utils.ec2_id_to_id(ec2_id)
             instance = self.compute_api.get(context, _instance_id)
             self.compute_api.stop(context, instance)
@@ -1282,6 +1303,7 @@ class CloudController(object):
         Here instance_id is a list of instance ids"""
         LOG.debug(_("Going to start instances"))
         for ec2_id in instance_id:
+            validate_ec2_id(ec2_id)
             _instance_id = ec2utils.ec2_id_to_id(ec2_id)
             instance = self.compute_api.get(context, _instance_id)
             self.compute_api.start(context, instance)
@@ -1476,7 +1498,7 @@ class CloudController(object):
         # NOTE(yamahata): name/description are ignored by register_image(),
         #                 do so here
         no_reboot = kwargs.get('no_reboot', False)
-
+        validate_ec2_id(instance_id)
         ec2_instance_id = instance_id
         instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
         instance = self.compute_api.get(context, instance_id)
diff --git a/nova/exception.py b/nova/exception.py
index 4f7cc5f33..518a95a62 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -404,10 +404,6 @@ class FlagNotSet(NotFound):
     message = _("Required flag %(flag)s not set.")
 
 
-class InstanceNotFound(NotFound):
-    message = _("Instance %(instance_id)s could not be found.")
-
-
 class VolumeNotFound(NotFound):
     message = _("Volume %(volume_id)s could not be found.")
 
@@ -985,3 +981,11 @@ class SolidFireAPIDataException(SolidFireAPIException):
 
 class DuplicateVlan(Duplicate):
     message = _("Detected existing vlan with id %(vlan)")
+
+
+class InstanceNotFound(NotFound):
+    message = _("Instance %(instance_id)s could not be found.")
+
+
+class InvalidInstanceIDMalformed(Invalid):
+        message = _("Invalid id: %(val) (expecting \"i-...\").")
diff --git a/nova/tests/api/ec2/test_ec2_validate.py b/nova/tests/api/ec2/test_ec2_validate.py
new file mode 100644
index 000000000..93ef7eb74
--- /dev/null
+++ b/nova/tests/api/ec2/test_ec2_validate.py
@@ -0,0 +1,180 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2012 Cloudscaling, Inc.
+# Author: Joe Gordon <jogo@cloudscaling.com>
+# 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.
+
+import webob.exc
+
+from nova.api.ec2 import cloud
+from nova.api.ec2 import ec2utils
+from nova.api.ec2 import inst_state
+from nova.compute import power_state
+from nova.compute import vm_states
+from nova import context
+from nova import db
+from nova import exception
+from nova import flags
+from nova.image import fake
+from nova import log as logging
+from nova import rpc
+from nova import test
+from nova import utils
+
+LOG = logging.getLogger('nova.tests.ec2_validate')
+FLAGS = flags.FLAGS
+
+
+class EC2ValidateTestCase(test.TestCase):
+    def setUp(self):
+        super(EC2ValidateTestCase, self).setUp()
+        self.flags(connection_type='fake',
+                   stub_network=True)
+
+        def dumb(*args, **kwargs):
+            pass
+
+        self.stubs.Set(utils, 'usage_from_instance', dumb)
+        # set up our cloud
+        self.cloud = cloud.CloudController()
+
+        # set up services
+        self.compute = self.start_service('compute')
+        self.scheduter = self.start_service('scheduler')
+        self.network = self.start_service('network')
+        self.volume = self.start_service('volume')
+        self.image_service = utils.import_object(FLAGS.image_service)
+
+        self.user_id = 'fake'
+        self.project_id = 'fake'
+        self.context = context.RequestContext(self.user_id,
+                                              self.project_id,
+                                              is_admin=True)
+
+        self.EC2_MALFORMED_IDS = ['foobar', '', 123]
+        self.EC2_VALID__IDS = ['i-284f3a41', 'i-001', 'i-deadbeef']
+
+        self.ec2_id_exception_map = [(x, exception.InvalidInstanceIDMalformed)
+                for x in self.EC2_MALFORMED_IDS]
+        self.ec2_id_exception_map.extend([(x, exception.InstanceNotFound)
+                for x in self.EC2_VALID__IDS])
+        self.volume_id_exception_map = [(x,
+                exception.InvalidInstanceIDMalformed)
+                for x in self.EC2_MALFORMED_IDS]
+        self.volume_id_exception_map.extend([(x, exception.VolumeNotFound)
+                for x in self.EC2_VALID__IDS])
+
+        def fake_show(meh, context, id):
+            return {'id': id,
+                    'container_format': 'ami',
+                    'properties': {
+                        'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                        'ramdisk_id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                        'type': 'machine',
+                        'image_state': 'available'}}
+
+        self.stubs.Set(fake._FakeImageService, 'show', fake_show)
+        self.stubs.Set(fake._FakeImageService, 'show_by_name', fake_show)
+
+        # NOTE(comstud): Make 'cast' behave like a 'call' which will
+        # ensure that operations complete
+        self.stubs.Set(rpc, 'cast', rpc.call)
+
+        # make sure we can map ami-00000001/2 to a uuid in FakeImageService
+        db.api.s3_image_create(self.context,
+                               'cedef40a-ed67-4d10-800e-17455edce175')
+        db.api.s3_image_create(self.context,
+                               '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6')
+
+    #EC2_API tests (InvalidInstanceID.Malformed)
+    def test_console_output(self):
+        for ec2_id, e in self.ec2_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.get_console_output,
+                              context=self.context,
+                              instance_id=[ec2_id])
+
+    def test_attach_volume(self):
+        for ec2_id, e in self.ec2_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.attach_volume,
+                              context=self.context,
+                              volume_id='i-1234',
+                              instance_id=ec2_id,
+                              device='/dev/vdc')
+        #missing instance error gets priority
+        for ec2_id, e in self.ec2_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.attach_volume,
+                              context=self.context,
+                              volume_id=ec2_id,
+                              instance_id='i-1234',
+                              device='/dev/vdc')
+
+    def test_describe_instance_ttribute(self):
+        for ec2_id, e in self.ec2_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.describe_instance_attribute,
+                              context=self.context,
+                              instance_id=ec2_id,
+                              attribute='kernel')
+
+    def test_instance_lifecycle(self):
+        lifecycle = [self.cloud.terminate_instances,
+                            self.cloud.reboot_instances,
+                            self.cloud.stop_instances,
+                            self.cloud.start_instances,
+                    ]
+        for cmd in lifecycle:
+            for ec2_id, e in self.ec2_id_exception_map:
+                self.assertRaises(e,
+                                  cmd,
+                                  context=self.context,
+                                  instance_id=[ec2_id])
+
+    def test_create_image(self):
+        for ec2_id, e in self.ec2_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.create_image,
+                              context=self.context,
+                              instance_id=ec2_id)
+
+    def test_create_snapshot(self):
+        for ec2_id, e in self.volume_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.create_snapshot,
+                              context=self.context,
+                              volume_id=ec2_id)
+
+    def test_describe_volumes(self):
+        for ec2_id, e in self.volume_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.describe_volumes,
+                              context=self.context,
+                              volume_id=[ec2_id])
+
+    def test_delete_volume(self):
+        for ec2_id, e in self.volume_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.delete_volume,
+                              context=self.context,
+                              volume_id=ec2_id)
+
+    def test_detach_volume(self):
+        for ec2_id, e in self.volume_id_exception_map:
+            self.assertRaises(e,
+                              self.cloud.detach_volume,
+                              context=self.context,
+                              volume_id=ec2_id)