Fixed validation check for ICMP rules

Fixed the code that checks the ICMP type/code
as from/to ports. Also fixed the CSS for the
security rules form, and the script to change
the form labels in case of form reload due to
an error.

Fixes bug 997669

Change-Id: I3b9c0ba3f46b91a311f887560dae5e9399961b84
(cherry picked from commit 7b565fc)

horizon/dashboards/nova/access_and_security/security_groups/forms.py

@@ -108,18 +108,33 @@ def clean(self):
         from_port = cleaned_data.get("from_port", None)
         to_port = cleaned_data.get("to_port", None)
         cidr = cleaned_data.get("cidr", None)
+        ip_proto = cleaned_data.get('ip_protocol', None)
         source_group = cleaned_data.get("source_group", None)
 
-        if from_port == None:
-            msg = _('The "from" port number is invalid.')
-            raise ValidationError(msg)
-        if to_port == None:
-            msg = _('The "to" port number is invalid.')
-            raise ValidationError(msg)
-        if to_port < from_port:
-            msg = _('The "to" port number must be greater than or equal to '
-                    'the "from" port number.')
-            raise ValidationError(msg)
+        if ip_proto == 'icmp':
+            if from_port == None:
+                msg = _('The ICMP type is invalid.')
+                raise ValidationError(msg)
+            if to_port == None:
+                msg = _('The ICMP code is invalid.')
+                raise ValidationError(msg)
+            if from_port not in xrange(-1, 256):
+                msg = _('The ICMP type not in range (-1, 255)')
+                raise ValidationError(msg)
+            if to_port not in xrange(-1, 256):
+                msg = _('The ICMP code not in range (-1, 255)')
+                raise ValidationError(msg)
+        else:
+            if from_port == None:
+                msg = _('The "from" port number is invalid.')
+                raise ValidationError(msg)
+            if to_port == None:
+                msg = _('The "to" port number is invalid.')
+                raise ValidationError(msg)
+            if to_port < from_port:
+                msg = _('The "to" port number must be greater than '
+                        'or equal to the "from" port number.')
+                raise ValidationError(msg)
 
         if source_group and cidr != self.fields['cidr'].initial:
             # Specifying a source group *and* a custom CIDR is invalid.

horizon/dashboards/nova/access_and_security/security_groups/tests.py

@@ -194,6 +194,76 @@ def test_edit_rules_invalid_port_range(self):
         self.assertNoMessages()
         self.assertContains(res, "greater than or equal to")
 
+    def test_edit_rules_invalid_icmp_rule(self):
+        self.mox.StubOutWithMock(api, 'security_group_get')
+        self.mox.StubOutWithMock(api, 'security_group_list')
+
+        sec_group = self.security_groups.first()
+        sec_group_list = self.security_groups.list()
+        icmp_rule = self.security_group_rules.list()[1]
+
+        api.security_group_get(IsA(http.HttpRequest),
+                               sec_group.id).AndReturn(sec_group)
+        api.security_group_list(
+                        IsA(http.HttpRequest)).AndReturn(sec_group_list)
+        api.security_group_get(IsA(http.HttpRequest),
+                               sec_group.id).AndReturn(sec_group)
+        api.security_group_list(
+                        IsA(http.HttpRequest)).AndReturn(sec_group_list)
+        api.security_group_get(IsA(http.HttpRequest),
+                               sec_group.id).AndReturn(sec_group)
+        api.security_group_list(
+                        IsA(http.HttpRequest)).AndReturn(sec_group_list)
+        api.security_group_get(IsA(http.HttpRequest),
+                               sec_group.id).AndReturn(sec_group)
+        api.security_group_list(
+                        IsA(http.HttpRequest)).AndReturn(sec_group_list)
+        self.mox.ReplayAll()
+
+        formData = {'method': 'AddRule',
+                    'security_group_id': sec_group.id,
+                    'from_port': 256,
+                    'to_port': icmp_rule.to_port,
+                    'ip_protocol': icmp_rule.ip_protocol,
+                    'cidr': icmp_rule.ip_range['cidr'],
+                    'source_group': ''}
+        res = self.client.post(self.edit_url, formData)
+        self.assertNoMessages()
+        self.assertContains(res, "The ICMP type not in range (-1, 255)")
+
+        formData = {'method': 'AddRule',
+                    'security_group_id': sec_group.id,
+                    'from_port': icmp_rule.from_port,
+                    'to_port': 256,
+                    'ip_protocol': icmp_rule.ip_protocol,
+                    'cidr': icmp_rule.ip_range['cidr'],
+                    'source_group': ''}
+        res = self.client.post(self.edit_url, formData)
+        self.assertNoMessages()
+        self.assertContains(res, "The ICMP code not in range (-1, 255)")
+
+        formData = {'method': 'AddRule',
+                    'security_group_id': sec_group.id,
+                    'from_port': icmp_rule.from_port,
+                    'to_port': None,
+                    'ip_protocol': icmp_rule.ip_protocol,
+                    'cidr': icmp_rule.ip_range['cidr'],
+                    'source_group': ''}
+        res = self.client.post(self.edit_url, formData)
+        self.assertNoMessages()
+        self.assertContains(res, "The ICMP code is invalid")
+
+        formData = {'method': 'AddRule',
+                    'security_group_id': sec_group.id,
+                    'from_port': None,
+                    'to_port': icmp_rule.to_port,
+                    'ip_protocol': icmp_rule.ip_protocol,
+                    'cidr': icmp_rule.ip_range['cidr'],
+                    'source_group': ''}
+        res = self.client.post(self.edit_url, formData)
+        self.assertNoMessages()
+        self.assertContains(res, "The ICMP type is invalid")
+
     def test_edit_rules_add_rule_exception(self):
         sec_group = self.security_groups.first()
         sec_group_list = self.security_groups.list()

horizon/static/horizon/js/forms.js

@@ -58,7 +58,11 @@ horizon.addInitFunction(function () {
          return true;
       $('label[for='+ $(obj).attr('id') + ']').html(label_val);
       });
-    })).change();
+    }));
+    $('select.switchable').trigger('change');
+    $('body').on('shown', '.modal', function(evt) {
+      $('select.switchable').trigger('change');
+    });
 
   /* Twipsy tooltips */
 

horizon/tests/test_data/nova_data.py

@@ -196,9 +196,19 @@ def data(TEST):
             'to_port': u"80",
             'parent_group_id': 1,
             'ip_range': {'cidr': u"0.0.0.0/32"}}
+
+    icmp_rule = {'id': 2,
+            'ip_protocol': u"icmp",
+            'from_port': u"9",
+            'to_port': u"5",
+            'parent_group_id': 1,
+            'ip_range': {'cidr': u"0.0.0.0/32"}}
     rule_obj = rules.SecurityGroupRule(rules.SecurityGroupRuleManager(None),
                                        rule)
+    rule_obj2 = rules.SecurityGroupRule(rules.SecurityGroupRuleManager(None),
+                                       icmp_rule)
     TEST.security_group_rules.add(rule_obj)
+    TEST.security_group_rules.add(rule_obj2)
 
     sec_group_1.rules = [rule_obj]
     sec_group_2.rules = [rule_obj]

openstack_dashboard/static/dashboard/css/style.css

@@ -1006,3 +1006,9 @@ label.log-length {
   line-height: 28px;
   margin-right: 10px;
 }
+
+.split_five div.control-group input[type="text"],
+.split_five div.control-group select {
+  width: 120px;
+}
+