Merge "[PATCH] Allow [:print:] chars for security group names"

nova/api/ec2/__init__.py

@@ -61,6 +61,10 @@ ec2_opts = [
                 default=False,
                 help='Return the IP address as private dns hostname in '
                      'describe instances'),
+    cfg.BoolOpt('ec2_strict_validation',
+                default=True,
+                help='Validate security group names'
+                     ' according to EC2 specification'),
     ]
 
 FLAGS = flags.FLAGS

nova/api/ec2/cloud.py

@@ -759,18 +759,36 @@ class CloudController(object):
         return source_project_id
 
     def create_security_group(self, context, group_name, group_description):
-        if not re.match('^[a-zA-Z0-9_\- ]+$', str(group_name)):
-            # Some validation to ensure that values match API spec.
-            # - Alphanumeric characters, spaces, dashes, and underscores.
-            # TODO(Daviey): LP: #813685 extend beyond group_name checking, and
-            #  probably create a param validator that can be used elsewhere.
-            err = _("Value (%s) for parameter GroupName is invalid."
-                    " Content limited to Alphanumeric characters, "
-                    "spaces, dashes, and underscores.") % group_name
-            # err not that of master ec2 implementation, as they fail to raise.
-            raise exception.InvalidParameterValue(err=err)
+        if isinstance(group_name, unicode):
+            group_name = group_name.encode('utf-8')
+        # TODO(Daviey): LP: #813685 extend beyond group_name checking, and
+        # probably create a param validator that can be used elsewhere.
+        if FLAGS.ec2_strict_validation:
+            # EC2 specification gives constraints for name and description:
+            # Accepts alphanumeric characters, spaces, dashes, and underscores
+            err = _("Value (%(value)s) for parameter %(param)s is invalid."
+                    " Content limited to Alphanumeric characters,"
+                    " spaces, dashes, and underscores.")
+            if not re.match('^[a-zA-Z0-9_\- ]+$', group_name):
+                raise exception.InvalidParameterValue(
+                    err=err % {"value": group_name,
+                               "param": "GroupName"})
+            if not re.match('^[a-zA-Z0-9_\- ]+$', group_description):
+                raise exception.InvalidParameterValue(
+                    err=err % {"value": group_description,
+                               "param": "GroupDescription"})
+        else:
+            # Amazon accepts more symbols.
+            # So, allow POSIX [:print:] characters.
+            if not re.match(r'^[\x20-\x7E]+$', group_name):
+                err = _("Value (%(value)s) for parameter %(param)s is invalid."
+                        " Content is limited to characters"
+                        " from the [:print:] class.")
+                raise exception.InvalidParameterValue(
+                    err=err % {"value": group_name,
+                               "param": "GroupName"})
 
-        if len(str(group_name)) > 255:
+        if len(group_name) > 255:
             err = _("Value (%s) for parameter GroupName is invalid."
                     " Length exceeds maximum of 255.") % group_name
             raise exception.InvalidParameterValue(err=err)

nova/tests/test_api.py

@@ -34,10 +34,14 @@ from nova.api.ec2 import ec2utils
 from nova import block_device
 from nova import context
 from nova import exception
+from nova import flags
 from nova import test
 from nova import utils
 
 
+FLAGS = flags.FLAGS
+
+
 class FakeHttplibSocket(object):
     """a fake socket implementation for httplib.HTTPResponse, trivial"""
     def __init__(self, response_string):
@@ -344,19 +348,37 @@ class ApiEc2TestCase(test.TestCase):
 
     def test_group_name_valid_chars_security_group(self):
         """ Test that we sanely handle invalid security group names.
-         API Spec states we should only accept alphanumeric characters,
-         spaces, dashes, and underscores. """
-        self.expect_http()
-        self.mox.ReplayAll()
+         EC2 API Spec states we should only accept alphanumeric characters,
+         spaces, dashes, and underscores. Amazon implementation
+         accepts more characters - so, [:print:] is ok. """
 
-        # Test block group_name of non alphanumeric characters, spaces,
-        # dashes, and underscores.
-        security_group_name = "aa #^% -=99"
-
-        self.assertRaises(boto_exc.EC2ResponseError,
-                self.ec2.create_security_group,
-                security_group_name,
-                'test group')
+        bad_strict_ec2 = "aa \t\x01\x02\x7f"
+        bad_amazon_ec2 = "aa #^% -=99"
+        test_raise = [
+            (True, bad_amazon_ec2, "test desc"),
+            (True, "test name", bad_amazon_ec2),
+            (False, bad_strict_ec2, "test desc"),
+        ]
+        for test in test_raise:
+            self.expect_http()
+            self.mox.ReplayAll()
+            FLAGS.ec2_strict_validation = test[0]
+            self.assertRaises(boto_exc.EC2ResponseError,
+                              self.ec2.create_security_group,
+                              test[1],
+                              test[2])
+        test_accept = [
+            (False, bad_amazon_ec2, "test desc"),
+            (False, "test name", bad_amazon_ec2),
+        ]
+        for test in test_accept:
+            self.expect_http()
+            self.mox.ReplayAll()
+            FLAGS.ec2_strict_validation = test[0]
+            self.ec2.create_security_group(test[1], test[2])
+            self.expect_http()
+            self.mox.ReplayAll()
+            self.ec2.delete_security_group(test[1])
 
     def test_group_name_valid_length_security_group(self):
         """Test that we sanely handle invalid security group names.