Support IPv6 addresses better

When adding a security group rule, if no IP address is given
we will use '0.0.0.0/0', but if the ethertype is IPv6 we will
leave it as None.  Change this to be '::/0' to match what we
do for IPv4 - use the "any" address.  The neutron server
treats them both the same when checking for duplicates.

Because there are most likely entries in the DB using None
for the IP, print them as '0.0.0.0/0' or '::/0' so it is more
obvious what address they are actually referring to.

Also change to display the Ethertype column by default
instead of with --long, since easily knowing IPv4 or IPv6
is useful.

Change-Id: Ic396fc23caa66b6b0034c5d30b27c6ed499de5a6
Closes-bug: #1735575

Author: Brian Haley

doc/source/cli/command-objects/security-group-rule.rst

@@ -27,8 +27,9 @@ Create a new security group rule
 
 .. option:: --remote-ip <ip-address>
 
-    Remote IP address block
-    (may use CIDR notation; default for IPv4 rule: 0.0.0.0/0)
+    Remote IP address block (may use CIDR notation;
+    default for IPv4 rule: 0.0.0.0/0,
+    default for IPv6 rule: ::/0)
 
 .. option:: --remote-group <group>
 
@@ -134,6 +135,7 @@ List security group rules
     openstack security group rule list
         [--all-projects]
         [--protocol <protocol>]
+        [--ethertype <ethertype>]
         [--ingress | --egress]
         [--long]
         [<group>]
@@ -151,7 +153,6 @@ List security group rules
 
     *Compute version 2 does not have additional fields to display.*
 
-
 .. option:: --protocol
 
     List rules by the IP protocol (ah, dhcp, egp, esp, gre, icmp, igmp,
@@ -161,6 +162,12 @@ List security group rules
 
     *Network version 2*
 
+.. option:: --ethertype
+
+    List rules by the Ethertype (IPv4 or IPv6)
+
+    *Network version 2*
+
 .. option:: --ingress
 
     List rules applied to incoming network traffic

openstackclient/network/v2/security_group_rule.py

@@ -62,6 +62,17 @@ def _format_network_port_range(rule):
     return port_range
 
 
+def _format_remote_ip_prefix(rule):
+    remote_ip_prefix = rule['remote_ip_prefix']
+    if remote_ip_prefix is None:
+        ethertype = rule['ether_type']
+        if ethertype == 'IPv4':
+            remote_ip_prefix = '0.0.0.0/0'
+        elif ethertype == 'IPv6':
+            remote_ip_prefix = '::/0'
+    return remote_ip_prefix
+
+
 def _get_columns(item):
     column_map = {
         'tenant_id': 'project_id',
@@ -108,7 +119,8 @@ def update_parser_common(self, parser):
             "--remote-ip",
             metavar="<ip-address>",
             help=_("Remote IP address block (may use CIDR notation; "
-                   "default for IPv4 rule: 0.0.0.0/0)"),
+                   "default for IPv4 rule: 0.0.0.0/0, "
+                   "default for IPv6 rule: ::/0)"),
         )
         remote_group.add_argument(
             "--remote-group",
@@ -230,6 +242,14 @@ def _get_protocol(self, parsed_args, default_protocol='any'):
             protocol = None
         return protocol
 
+    def _get_ethertype(self, parsed_args, protocol):
+        ethertype = 'IPv4'
+        if parsed_args.ethertype is not None:
+            ethertype = parsed_args.ethertype
+        elif self._is_ipv6_protocol(protocol):
+            ethertype = 'IPv6'
+        return ethertype
+
     def _is_ipv6_protocol(self, protocol):
         # NOTE(rtheis): Neutron has deprecated protocol icmpv6.
         # However, while the OSC CLI doesn't document the protocol,
@@ -264,12 +284,8 @@ def take_action_network(self, client, parsed_args):
 
         # NOTE(rtheis): Use ethertype specified else default based
         # on IP protocol.
-        if parsed_args.ethertype:
-            attrs['ethertype'] = parsed_args.ethertype
-        elif self._is_ipv6_protocol(attrs['protocol']):
-            attrs['ethertype'] = 'IPv6'
-        else:
-            attrs['ethertype'] = 'IPv4'
+        attrs['ethertype'] = self._get_ethertype(parsed_args,
+                                                 attrs['protocol'])
 
         # NOTE(rtheis): Validate the port range and ICMP type and code.
         # It would be ideal if argparse could do this.
@@ -306,6 +322,8 @@ def take_action_network(self, client, parsed_args):
             attrs['remote_ip_prefix'] = parsed_args.remote_ip
         elif attrs['ethertype'] == 'IPv4':
             attrs['remote_ip_prefix'] = '0.0.0.0/0'
+        elif attrs['ethertype'] == 'IPv6':
+            attrs['remote_ip_prefix'] = '::/0'
         attrs['security_group_id'] = security_group_id
         if parsed_args.project is not None:
             identity_client = self.app.client_manager.identity
@@ -387,6 +405,7 @@ def _format_network_security_group_rule(self, rule):
         """
         rule = rule.to_dict()
         rule['port_range'] = _format_network_port_range(rule)
+        rule['remote_ip_prefix'] = _format_remote_ip_prefix(rule)
         return rule
 
     def update_parser_common(self, parser):
@@ -418,6 +437,12 @@ def update_parser_network(self, parser):
                    "udp, udplite, vrrp and integer representations [0-255] "
                    "or any; default: any (all protocols))")
         )
+        parser.add_argument(
+            '--ethertype',
+            metavar='<ethertype>',
+            type=_convert_to_lowercase,
+            help=_("List rules by the Ethertype (IPv4 or IPv6)")
+        )
         direction_group = parser.add_mutually_exclusive_group()
         direction_group.add_argument(
             '--ingress',
@@ -458,11 +483,12 @@ def _get_column_headers(self, parsed_args):
         column_headers = (
             'ID',
             'IP Protocol',
+            'Ethertype',
             'IP Range',
             'Port Range',
         )
         if parsed_args.long:
-            column_headers = column_headers + ('Direction', 'Ethertype',)
+            column_headers = column_headers + ('Direction',)
         column_headers = column_headers + ('Remote Security Group',)
         if parsed_args.group is None:
             column_headers = column_headers + ('Security Group',)
@@ -473,11 +499,12 @@ def take_action_network(self, client, parsed_args):
         columns = (
             'id',
             'protocol',
+            'ether_type',
             'remote_ip_prefix',
             'port_range',
         )
         if parsed_args.long:
-            columns = columns + ('direction', 'ether_type',)
+            columns = columns + ('direction',)
         columns = columns + ('remote_group_id',)
 
         # Get the security group rules using the requested query.
@@ -516,6 +543,7 @@ def take_action_compute(self, client, parsed_args):
         columns = (
             "ID",
             "IP Protocol",
+            "Ethertype",
             "IP Range",
             "Port Range",
             "Remote Security Group",
@@ -564,6 +592,9 @@ def update_parser_common(self, parser):
     def take_action_network(self, client, parsed_args):
         obj = client.find_security_group_rule(parsed_args.rule,
                                               ignore_missing=False)
+        # necessary for old rules that have None in this field
+        if not obj['remote_ip_prefix']:
+            obj['remote_ip_prefix'] = _format_remote_ip_prefix(obj)
         display_columns, columns = _get_columns(obj)
         data = utils.get_item_properties(obj, columns)
         return (display_columns, data)

openstackclient/tests/unit/network/v2/test_security_group_rule_compute.py

@@ -337,13 +337,15 @@ class TestListSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
     _security_group_rule_tcp = \
         compute_fakes.FakeSecurityGroupRule.create_one_security_group_rule({
             'ip_protocol': 'tcp',
+            'ethertype': 'IPv4',
             'from_port': 80,
             'to_port': 80,
             'group': {'name': _security_group['name']},
         })
     _security_group_rule_icmp = \
         compute_fakes.FakeSecurityGroupRule.create_one_security_group_rule({
             'ip_protocol': 'icmp',
+            'ethertype': 'IPv4',
             'from_port': -1,
             'to_port': -1,
             'ip_range': {'cidr': '10.0.2.0/24'},
@@ -357,6 +359,7 @@ class TestListSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
     expected_columns_with_group = (
         'ID',
         'IP Protocol',
+        'Ethertype',
         'IP Range',
         'Port Range',
         'Remote Security Group',
@@ -373,6 +376,7 @@ class TestListSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
         expected_rule_with_group = (
             rule['id'],
             rule['ip_protocol'],
+            rule['ethertype'],
             rule['ip_range'],
             rule['port_range'],
             rule['remote_security_group'],

openstackclient/tests/unit/network/v2/test_security_group_rule_network.py

@@ -388,7 +388,7 @@ def test_create_network_options(self):
             'port_range_min': 443,
             'protocol': '6',
             'remote_group_id': None,
-            'remote_ip_prefix': None,
+            'remote_ip_prefix': '::/0',
         })
         arglist = [
             '--dst-port', str(self._security_group_rule.port_range_min),
@@ -419,6 +419,7 @@ def test_create_network_options(self):
             'port_range_max': self._security_group_rule.port_range_max,
             'port_range_min': self._security_group_rule.port_range_min,
             'protocol': self._security_group_rule.protocol,
+            'remote_ip_prefix': self._security_group_rule.remote_ip_prefix,
             'security_group_id': self._security_group.id,
             'tenant_id': self.project.id,
         })
@@ -664,6 +665,7 @@ def test_create_ipv6_icmp_type_code(self):
             'port_range_min': 139,
             'port_range_max': 2,
             'protocol': 'ipv6-icmp',
+            'remote_ip_prefix': '::/0',
         })
         arglist = [
             '--icmp-type', str(self._security_group_rule.port_range_min),
@@ -688,6 +690,7 @@ def test_create_ipv6_icmp_type_code(self):
             'port_range_min': self._security_group_rule.port_range_min,
             'port_range_max': self._security_group_rule.port_range_max,
             'protocol': self._security_group_rule.protocol,
+            'remote_ip_prefix': self._security_group_rule.remote_ip_prefix,
             'security_group_id': self._security_group.id,
         })
         self.assertEqual(self.expected_columns, columns)
@@ -698,6 +701,7 @@ def test_create_icmpv6_type(self):
             'ether_type': 'IPv6',
             'port_range_min': 139,
             'protocol': 'icmpv6',
+            'remote_ip_prefix': '::/0',
         })
         arglist = [
             '--icmp-type', str(self._security_group_rule.port_range_min),
@@ -720,6 +724,7 @@ def test_create_icmpv6_type(self):
             'ethertype': self._security_group_rule.ether_type,
             'port_range_min': self._security_group_rule.port_range_min,
             'protocol': self._security_group_rule.protocol,
+            'remote_ip_prefix': self._security_group_rule.remote_ip_prefix,
             'security_group_id': self._security_group.id,
         })
         self.assertEqual(self.expected_columns, columns)
@@ -868,15 +873,16 @@ class TestListSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
     expected_columns_with_group_and_long = (
         'ID',
         'IP Protocol',
+        'Ethertype',
         'IP Range',
         'Port Range',
         'Direction',
-        'Ethertype',
         'Remote Security Group',
     )
     expected_columns_no_group = (
         'ID',
         'IP Protocol',
+        'Ethertype',
         'IP Range',
         'Port Range',
         'Remote Security Group',
@@ -889,16 +895,17 @@ class TestListSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         expected_data_with_group_and_long.append((
             _security_group_rule.id,
             _security_group_rule.protocol,
+            _security_group_rule.ether_type,
             _security_group_rule.remote_ip_prefix,
             security_group_rule._format_network_port_range(
                 _security_group_rule),
             _security_group_rule.direction,
-            _security_group_rule.ether_type,
             _security_group_rule.remote_group_id,
         ))
         expected_data_no_group.append((
             _security_group_rule.id,
             _security_group_rule.protocol,
+            _security_group_rule.ether_type,
             _security_group_rule.remote_ip_prefix,
             security_group_rule._format_network_port_range(
                 _security_group_rule),

releasenotes/notes/better-ipv6-address-suppport-security-group-rules-95272847349982e5.yaml

@@ -0,0 +1,15 @@
+---
+features:
+  - |
+    Security group rules can now be filtered by Ethertype in
+    ``security group rule list`` using ``--ethertype`` with either
+    ``ipv4`` or ``ipv6`` as an argument.
+upgrade:
+  - |
+    Security group rule listings now have the ``Ethertype`` field displayed
+    by default to more easily differentiate between IPv4 and IPv6 rules.
+    In addition, the ``IP Range`` field of a security group will be
+    changed to ``0.0.0.0/0`` for IPv4 and ``::/0`` for IPv6 if no
+    value is returned for the address, based on the Ethertype field of
+    the rule. For further information see
+    [Bug `1735575 <https://bugs.launchpad.net/bugs/1735575>`_]