Merge "Canonicalize IPv6 ICMP protocol name in security groups"

neutron/agent/linux/iptables_firewall.py

@@ -400,8 +400,8 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
             if rule.get('ethertype') == constants.IPv4:
                 ipv4_sg_rules.append(rule)
             elif rule.get('ethertype') == constants.IPv6:
-                if rule.get('protocol') == 'icmp':
+                if rule.get('protocol') in const.IPV6_ICMP_LEGACY_PROTO_LIST:
-                    rule['protocol'] = 'ipv6-icmp'
+                    rule['protocol'] = constants.PROTO_NAME_IPV6_ICMP
                 ipv6_sg_rules.append(rule)
         return ipv4_sg_rules, ipv6_sg_rules
 

neutron/common/_constants.py

@@ -41,6 +41,10 @@ IPTABLES_MULTIPORT_ONLY_PROTOCOLS = [
     constants.PROTO_NAME_UDPLITE
 ]
 
+# Legacy IPv6 ICMP protocol list
+IPV6_ICMP_LEGACY_PROTO_LIST = [constants.PROTO_NAME_ICMP,
+                               constants.PROTO_NAME_IPV6_ICMP_LEGACY]
+
 # Number of resources for neutron agent side functions to deal
 # with large sets.
 # Setting this value does not count on special conditions, it is just a human

neutron/db/securitygroups_db.py

@@ -447,10 +447,14 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
             protocol = constants.IP_PROTOCOL_NAME_ALIASES[protocol]
         return int(constants.IP_PROTOCOL_MAP.get(protocol, protocol))
 
-    def _get_ip_proto_name_and_num(self, protocol):
+    def _get_ip_proto_name_and_num(self, protocol, ethertype=None):
         if protocol is None:
             return
         protocol = str(protocol)
+        # Force all legacy IPv6 ICMP protocol names to be 'ipv6-icmp'
+        if (ethertype == constants.IPv6 and
+                protocol in const.IPV6_ICMP_LEGACY_PROTO_LIST):
+            protocol = constants.PROTO_NAME_IPV6_ICMP
         if protocol in constants.IP_PROTOCOL_MAP:
             return [protocol, str(constants.IP_PROTOCOL_MAP.get(protocol))]
         elif protocol in constants.IP_PROTOCOL_NUM_TO_NAME_MAP:
@@ -542,8 +546,14 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
                 raise ext_sg.SecurityGroupRulesNotSingleTenant()
         return sg_groups.pop()
 
+    def _make_canonical_ipv6_icmp_protocol(self, rule):
+        if (rule.get('ethertype') == constants.IPv6 and
+                rule.get('protocol') in const.IPV6_ICMP_LEGACY_PROTO_LIST):
+            rule['protocol'] = constants.PROTO_NAME_IPV6_ICMP
+
     def _validate_security_group_rule(self, context, security_group_rule):
         rule = security_group_rule['security_group_rule']
+        self._make_canonical_ipv6_icmp_protocol(rule)
         self._validate_port_range(rule)
         self._validate_ip_prefix(rule)
         self._validate_ethertype_and_protocol(rule)
@@ -599,7 +609,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
             elif value is None:
                 return none_char
             elif key == 'protocol':
-                return str(self._get_ip_proto_name_and_num(value))
+                return str(self._get_ip_proto_name_and_num(
+                               value, ethertype=rule.get('ethertype')))
             return str(value)
 
         comparison_keys = [

neutron/tests/unit/extensions/test_securitygroup.py

@@ -519,7 +519,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
         with self.security_group(name, description) as sg:
             security_group_id = sg['security_group']['id']
             rule = self._build_security_group_rule(
-                security_group_id, 'ingress', const.PROTO_NAME_IPV6_ICMP)
+                security_group_id, 'ingress', const.PROTO_NAME_IPV6_FRAG)
             res = self._create_security_group_rule(self.fmt, rule)
             self.deserialize(self.fmt, res)
             self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int)
@@ -1049,7 +1049,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                 for k, v, in keys:
                     self.assertEqual(rule['security_group_rule'][k], v)
 
-    def test_create_security_group_rule_icmpv6_legacy_protocol_name(self):
+    def _test_create_security_group_rule_legacy_protocol_name(self, protocol):
         name = 'webservers'
         description = 'my webservers'
         with self.security_group(name, description) as sg:
@@ -1057,7 +1057,6 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
             direction = "ingress"
             ethertype = const.IPv6
             remote_ip_prefix = "2001::f401:56ff:fefe:d3dc/128"
-            protocol = const.PROTO_NAME_IPV6_ICMP_LEGACY
             keys = [('remote_ip_prefix', remote_ip_prefix),
                     ('security_group_id', security_group_id),
                     ('direction', direction),
@@ -1069,8 +1068,19 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                                           None, None,
                                           ethertype) as rule:
                 for k, v, in keys:
+                    # IPv6 ICMP protocol will always be 'ipv6-icmp'
+                    if k == 'protocol':
+                        v = const.PROTO_NAME_IPV6_ICMP
                     self.assertEqual(rule['security_group_rule'][k], v)
 
+    def test_create_security_group_rule_ipv6_icmp_legacy_protocol_name(self):
+        protocol = const.PROTO_NAME_ICMP
+        self._test_create_security_group_rule_legacy_protocol_name(protocol)
+
+    def test_create_security_group_rule_icmpv6_legacy_protocol_name(self):
+        protocol = const.PROTO_NAME_IPV6_ICMP_LEGACY
+        self._test_create_security_group_rule_legacy_protocol_name(protocol)
+
     def test_create_security_group_source_group_ip_and_ip_prefix(self):
         security_group_id = "4cd70774-cc67-4a87-9b39-7d1db38eb087"
         direction = "ingress"

releasenotes/notes/security-group-ipv6-icmp-221c59dcaf2caa3c.yaml

@@ -0,0 +1,14 @@
+---
+upgrade:
+  - |
+    Existing IPv6 ICMP security group rules created by using legacy protocol
+    names ``icmpv6`` and ``icmp`` will now be returned as ``ipv6-icmp`` in
+    an API GET call.
+fixes:
+  - |
+    Security group rule code has been changed to better detect duplicate
+    rules by standardizing on ``ipv6-icmp`` as the protocol field value
+    for IPv6 ICMP rules.  The legacy names ``icmpv6`` and ``icmp`` can still
+    be used in API POST calls, but API GET calls will return ``ipv6-icmp``.
+    Partial fix for bug
+    `1582500 <https://bugs.launchpad.net/neutron/+bug/1582500>`_.