Merge "Additional network protocol support"

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

@@ -16,18 +16,14 @@ Create a new security group rule
 .. code:: bash
 
     os security group rule create
-        [--proto <proto>]
         [--src-ip <ip-address> | --src-group <group>]
-        [--dst-port <port-range>]
+        [--dst-port <port-range> | [--icmp-type <icmp-type> [--icmp-code <icmp-code>]]]
+        [--protocol <protocol>]
         [--ingress | --egress]
         [--ethertype <ethertype>]
         [--project <project> [--project-domain <project-domain>]]
         <group>
 
-.. option:: --proto <proto>
-
-    IP protocol (icmp, tcp, udp; default: tcp)
-
 .. option:: --src-ip <ip-address>
 
     Source IP address block
@@ -39,8 +35,35 @@ Create a new security group rule
 
 .. option:: --dst-port <port-range>
 
-    Destination port, may be a single port or port range: 137:139
+    Destination port, may be a single port or a starting and
-    (only required for IP protocols tcp and udp)
+    ending port range: 137:139. Required for IP protocols TCP
+    and UDP. Ignored for ICMP IP protocols.
+
+.. option:: --icmp-type <icmp-type>
+
+    ICMP type for ICMP IP protocols
+
+    *Network version 2 only*
+
+.. option:: --icmp-code <icmp-code>
+
+    ICMP code for ICMP IP protocols
+
+    *Network version 2 only*
+
+.. option:: --protocol <protocol>
+
+    IP protocol (icmp, tcp, udp; default: tcp)
+
+    *Compute version 2*
+
+    IP protocol (ah, dccp, egp, esp, gre, icmp, igmp,
+    ipv6-encap, ipv6-frag, ipv6-icmp, ipv6-nonxt,
+    ipv6-opts, ipv6-route, ospf, pgm, rsvp, sctp, tcp,
+    udp, udplite, vrrp and integer representations [0-255];
+    default: tcp)
+
+    *Network version 2*
 
 .. option:: --ingress
 
@@ -56,7 +79,8 @@ Create a new security group rule
 
 .. option:: --ethertype <ethertype>
 
-    Ethertype of network traffic (IPv4, IPv6; default: IPv4)
+    Ethertype of network traffic
+    (IPv4, IPv6; default: based on IP protocol)
 
     *Network version 2 only*
 

functional/tests/network/v2/test_security_group_rule.py

@@ -37,7 +37,7 @@ class SecurityGroupRuleTests(test.TestCase):
         opts = cls.get_show_opts(cls.ID_FIELD)
         raw_output = cls.openstack('security group rule create ' +
                                    cls.SECURITY_GROUP_NAME +
-                                   ' --proto tcp --dst-port 80:80' +
+                                   ' --protocol tcp --dst-port 80:80' +
                                    ' --ingress --ethertype IPv4' +
                                    opts)
         cls.SECURITY_GROUP_RULE_ID = raw_output.strip('\n')

openstackclient/network/v2/security_group_rule.py

@@ -36,9 +36,21 @@ def _format_security_group_rule_show(obj):
 
 
 def _format_network_port_range(rule):
+    # Display port range or ICMP type and code. For example:
+    # - ICMP type: 'type=3'
+    # - ICMP type and code: 'type=3:code=0'
+    # - ICMP code: Not supported
+    # - Matching port range: '443:443'
+    # - Different port range: '22:24'
+    # - Single port: '80:80'
+    # - No port range: ''
     port_range = ''
-    if (rule.protocol != 'icmp' and
+    if _is_icmp_protocol(rule.protocol):
-            (rule.port_range_min or rule.port_range_max)):
+        if rule.port_range_min:
+            port_range += 'type=' + str(rule.port_range_min)
+        if rule.port_range_max:
+            port_range += ':code=' + str(rule.port_range_max)
+    elif rule.port_range_min or rule.port_range_max:
         port_range_min = str(rule.port_range_min)
         port_range_max = str(rule.port_range_max)
         if rule.port_range_min is None:
@@ -61,6 +73,17 @@ def _convert_to_lowercase(string):
     return string.lower()
 
 
+def _is_icmp_protocol(protocol):
+    # NOTE(rtheis): Neutron has deprecated protocol icmpv6.
+    # However, while the OSC CLI doesn't document the protocol,
+    # the code must still handle it. In addition, handle both
+    # protocol names and numbers.
+    if protocol in ['icmp', 'icmpv6', 'ipv6-icmp', '1', '58']:
+        return True
+    else:
+        return False
+
+
 class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
     """Create a new security group rule"""
 
@@ -68,19 +91,7 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
         parser.add_argument(
             'group',
             metavar='<group>',
-            help='Create rule in this security group (name or ID)',
+            help=_("Create rule in this security group (name or ID)")
-        )
-        # TODO(rtheis): Add support for additional protocols for network.
-        # Until then, continue enforcing the compute choices. When additional
-        # protocols are added, the default ethertype must be determined
-        # based on the protocol.
-        parser.add_argument(
-            "--proto",
-            metavar="<proto>",
-            default="tcp",
-            choices=['icmp', 'tcp', 'udp'],
-            type=_convert_to_lowercase,
-            help=_("IP protocol (icmp, tcp, udp; default: tcp)")
         )
         source_group = parser.add_mutually_exclusive_group()
         source_group.add_argument(
@@ -94,17 +105,49 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
             metavar="<group>",
             help=_("Source security group (name or ID)")
         )
-        parser.add_argument(
-            "--dst-port",
-            metavar="<port-range>",
-            default=(0, 0),
-            action=parseractions.RangeAction,
-            help=_("Destination port, may be a single port or port range: "
-                   "137:139 (only required for IP protocols tcp and udp)")
-        )
         return parser
 
     def update_parser_network(self, parser):
+        parser.add_argument(
+            '--dst-port',
+            metavar='<port-range>',
+            action=parseractions.RangeAction,
+            help=_("Destination port, may be a single port or a starting and "
+                   "ending port range: 137:139. Required for IP protocols TCP "
+                   "and UDP. Ignored for ICMP IP protocols.")
+        )
+        parser.add_argument(
+            '--icmp-type',
+            metavar='<icmp-type>',
+            type=int,
+            help=_("ICMP type for ICMP IP protocols")
+        )
+        parser.add_argument(
+            '--icmp-code',
+            metavar='<icmp-code>',
+            type=int,
+            help=_("ICMP code for ICMP IP protocols")
+        )
+        # NOTE(rtheis): Support either protocol option name for now.
+        # However, consider deprecating and then removing --proto in
+        # a future release.
+        protocol_group = parser.add_mutually_exclusive_group()
+        protocol_group.add_argument(
+            '--protocol',
+            metavar='<protocol>',
+            type=_convert_to_lowercase,
+            help=_("IP protocol (ah, dccp, egp, esp, gre, icmp, igmp, "
+                   "ipv6-encap, ipv6-frag, ipv6-icmp, ipv6-nonxt, "
+                   "ipv6-opts, ipv6-route, ospf, pgm, rsvp, sctp, tcp, "
+                   "udp, udplite, vrrp and integer representations [0-255]; "
+                   "default: tcp)")
+        )
+        protocol_group.add_argument(
+            '--proto',
+            metavar='<proto>',
+            type=_convert_to_lowercase,
+            help=argparse.SUPPRESS
+        )
         direction_group = parser.add_mutually_exclusive_group()
         direction_group.add_argument(
             '--ingress',
@@ -120,7 +163,8 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
             '--ethertype',
             metavar='<ethertype>',
             choices=['IPv4', 'IPv6'],
-            help=_("Ethertype of network traffic (IPv4, IPv6; default: IPv4)")
+            help=_("Ethertype of network traffic "
+                   "(IPv4, IPv6; default: based on IP protocol)")
         )
         parser.add_argument(
             '--project',
@@ -130,6 +174,55 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
         identity_common.add_project_domain_option_to_parser(parser)
         return parser
 
+    def update_parser_compute(self, parser):
+        parser.add_argument(
+            '--dst-port',
+            metavar='<port-range>',
+            default=(0, 0),
+            action=parseractions.RangeAction,
+            help=_("Destination port, may be a single port or a starting and "
+                   "ending port range: 137:139. Required for IP protocols TCP "
+                   "and UDP. Ignored for ICMP IP protocols.")
+        )
+        # NOTE(rtheis): Support either protocol option name for now.
+        # However, consider deprecating and then removing --proto in
+        # a future release.
+        protocol_group = parser.add_mutually_exclusive_group()
+        protocol_group.add_argument(
+            '--protocol',
+            metavar='<protocol>',
+            choices=['icmp', 'tcp', 'udp'],
+            type=_convert_to_lowercase,
+            help=_("IP protocol (icmp, tcp, udp; default: tcp)")
+        )
+        protocol_group.add_argument(
+            '--proto',
+            metavar='<proto>',
+            choices=['icmp', 'tcp', 'udp'],
+            type=_convert_to_lowercase,
+            help=argparse.SUPPRESS
+        )
+        return parser
+
+    def _get_protocol(self, parsed_args):
+        protocol = 'tcp'
+        if parsed_args.protocol is not None:
+            protocol = parsed_args.protocol
+        if parsed_args.proto is not None:
+            protocol = parsed_args.proto
+        return protocol
+
+    def _is_ipv6_protocol(self, protocol):
+        # NOTE(rtheis): Neutron has deprecated protocol icmpv6.
+        # However, while the OSC CLI doesn't document the protocol,
+        # the code must still handle it. In addition, handle both
+        # protocol names and numbers.
+        if (protocol.startswith('ipv6-') or
+                protocol in ['icmpv6', '41', '43', '44', '58', '59', '60']):
+            return True
+        else:
+            return False
+
     def take_action_network(self, client, parsed_args):
         # Get the security group ID to hold the rule.
         security_group_id = client.find_security_group(
@@ -139,24 +232,50 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
 
         # Build the create attributes.
         attrs = {}
+        attrs['protocol'] = self._get_protocol(parsed_args)
+
         # NOTE(rtheis): A direction must be specified and ingress
         # is the default.
         if parsed_args.ingress or not parsed_args.egress:
             attrs['direction'] = 'ingress'
         if parsed_args.egress:
             attrs['direction'] = 'egress'
+
+        # 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:
-            # NOTE(rtheis): Default based on protocol is IPv4 for now.
-            # Once IPv6 protocols are added, this will need to be updated.
             attrs['ethertype'] = 'IPv4'
-        # TODO(rtheis): Add port range support (type and code) for icmp
+
-        # protocol. Until then, continue ignoring the port range.
+        # NOTE(rtheis): Validate the port range and ICMP type and code.
-        if parsed_args.proto != 'icmp':
+        # It would be ideal if argparse could do this.
+        if parsed_args.dst_port and (parsed_args.icmp_type or
+                                     parsed_args.icmp_code):
+            msg = _('Argument --dst-port not allowed with arguments '
+                    '--icmp-type and --icmp-code')
+            raise exceptions.CommandError(msg)
+        if parsed_args.icmp_type is None and parsed_args.icmp_code is not None:
+            msg = _('Argument --icmp-type required with argument --icmp-code')
+            raise exceptions.CommandError(msg)
+        is_icmp_protocol = _is_icmp_protocol(attrs['protocol'])
+        if not is_icmp_protocol and (parsed_args.icmp_type or
+                                     parsed_args.icmp_code):
+            msg = _('ICMP IP protocol required with arguments '
+                    '--icmp-type and --icmp-code')
+            raise exceptions.CommandError(msg)
+        # NOTE(rtheis): For backwards compatibility, continue ignoring
+        # the destination port range when an ICMP IP protocol is specified.
+        if parsed_args.dst_port and not is_icmp_protocol:
             attrs['port_range_min'] = parsed_args.dst_port[0]
             attrs['port_range_max'] = parsed_args.dst_port[1]
-        attrs['protocol'] = parsed_args.proto
+        if parsed_args.icmp_type:
+            attrs['port_range_min'] = parsed_args.icmp_type
+        if parsed_args.icmp_code:
+            attrs['port_range_max'] = parsed_args.icmp_code
+
         if parsed_args.src_group is not None:
             attrs['remote_group_id'] = client.find_security_group(
                 parsed_args.src_group,
@@ -187,7 +306,8 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
             client.security_groups,
             parsed_args.group,
         )
-        if parsed_args.proto == 'icmp':
+        protocol = self._get_protocol(parsed_args)
+        if protocol == 'icmp':
             from_port, to_port = -1, -1
         else:
             from_port, to_port = parsed_args.dst_port
@@ -203,7 +323,7 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
             src_ip = '0.0.0.0/0'
         obj = client.security_group_rules.create(
             group.id,
-            parsed_args.proto,
+            protocol,
             from_port,
             to_port,
             src_ip,

openstackclient/tests/network/v2/fakes.py

@@ -496,8 +496,8 @@ class FakeSecurityGroupRule(object):
             'direction': 'ingress',
             'ethertype': 'IPv4',
             'id': 'security-group-rule-id-' + uuid.uuid4().hex,
-            'port_range_max': 0,
+            'port_range_max': None,
-            'port_range_min': 0,
+            'port_range_min': None,
             'protocol': 'tcp',
             'remote_group_id': None,
             'remote_ip_prefix': '0.0.0.0/0',

openstackclient/tests/network/v2/test_security_group_rule.py

@@ -14,6 +14,7 @@
 import copy
 import mock
 
+from openstackclient.common import exceptions
 from openstackclient.network import utils as network_utils
 from openstackclient.network.v2 import security_group_rule
 from openstackclient.tests.compute.v2 import fakes as compute_fakes
@@ -131,14 +132,6 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         self.assertRaises(tests_utils.ParserException,
                           self.check_parser, self.cmd, arglist, [])
 
-    def test_create_bad_protocol(self):
-        arglist = [
-            '--protocol', 'foo',
-            self._security_group.id,
-        ]
-        self.assertRaises(tests_utils.ParserException,
-                          self.check_parser, self.cmd, arglist, [])
-
     def test_create_bad_ethertype(self):
         arglist = [
             '--ethertype', 'foo',
@@ -147,6 +140,32 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         self.assertRaises(tests_utils.ParserException,
                           self.check_parser, self.cmd, arglist, [])
 
+    def test_create_all_protocol_options(self):
+        arglist = [
+            '--protocol', 'tcp',
+            '--proto', 'tcp',
+            self._security_group.id,
+        ]
+        self.assertRaises(tests_utils.ParserException,
+                          self.check_parser, self.cmd, arglist, [])
+
+    def test_create_all_port_range_options(self):
+        arglist = [
+            '--dst-port', '80:80',
+            '--icmp-type', '3',
+            '--icmp-code', '1',
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('dst_port', (80, 80)),
+            ('icmp_type', 3),
+            ('icmp_code', 1),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+                          parsed_args)
+
     def test_create_default_rule(self):
         self._setup_security_group_rule({
             'port_range_max': 443,
@@ -177,6 +196,36 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         self.assertEqual(self.expected_columns, columns)
         self.assertEqual(self.expected_data, data)
 
+    def test_create_proto_option(self):
+        self._setup_security_group_rule({
+            'protocol': 'icmp',
+            'remote_ip_prefix': '10.0.2.0/24',
+        })
+        arglist = [
+            '--proto', self._security_group_rule.protocol,
+            '--src-ip', self._security_group_rule.remote_ip_prefix,
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('proto', self._security_group_rule.protocol),
+            ('protocol', None),
+            ('src_ip', self._security_group_rule.remote_ip_prefix),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.create_security_group_rule.assert_called_once_with(**{
+            'direction': self._security_group_rule.direction,
+            'ethertype': self._security_group_rule.ethertype,
+            '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)
+        self.assertEqual(self.expected_data, data)
+
     def test_create_source_group(self):
         self._setup_security_group_rule({
             'port_range_max': 22,
@@ -215,17 +264,15 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
     def test_create_source_ip(self):
         self._setup_security_group_rule({
             'protocol': 'icmp',
-            'port_range_max': -1,
-            'port_range_min': -1,
             'remote_ip_prefix': '10.0.2.0/24',
         })
         arglist = [
-            '--proto', self._security_group_rule.protocol,
+            '--protocol', self._security_group_rule.protocol,
             '--src-ip', self._security_group_rule.remote_ip_prefix,
             self._security_group.id,
         ]
         verifylist = [
-            ('proto', self._security_group_rule.protocol),
+            ('protocol', self._security_group_rule.protocol),
             ('src_ip', self._security_group_rule.remote_ip_prefix),
             ('group', self._security_group.id),
         ]
@@ -249,6 +296,7 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
             'ethertype': 'IPv6',
             'port_range_max': 443,
             'port_range_min': 443,
+            'protocol': '6',
             'remote_group_id': None,
             'remote_ip_prefix': None,
         })
@@ -258,6 +306,7 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
             '--ethertype', self._security_group_rule.ethertype,
             '--project', identity_fakes.project_name,
             '--project-domain', identity_fakes.domain_name,
+            '--protocol', self._security_group_rule.protocol,
             self._security_group.id,
         ]
         verifylist = [
@@ -267,6 +316,7 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
             ('ethertype', self._security_group_rule.ethertype),
             ('project', identity_fakes.project_name),
             ('project_domain', identity_fakes.domain_name),
+            ('protocol', self._security_group_rule.protocol),
             ('group', self._security_group.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -285,6 +335,136 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         self.assertEqual(self.expected_columns, columns)
         self.assertEqual(self.expected_data, data)
 
+    def test_create_tcp_with_icmp_type(self):
+        arglist = [
+            '--protocol', 'tcp',
+            '--icmp-type', '15',
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('protocol', 'tcp'),
+            ('icmp_type', 15),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+                          parsed_args)
+
+    def test_create_icmp_code(self):
+        arglist = [
+            '--protocol', '1',
+            '--icmp-code', '1',
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('protocol', '1'),
+            ('icmp_code', 1),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+                          parsed_args)
+
+    def test_create_icmp_type(self):
+        self._setup_security_group_rule({
+            'port_range_min': 15,
+            'protocol': 'icmp',
+            'remote_ip_prefix': '0.0.0.0/0',
+        })
+        arglist = [
+            '--icmp-type', str(self._security_group_rule.port_range_min),
+            '--protocol', self._security_group_rule.protocol,
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('dst_port', None),
+            ('icmp_type', self._security_group_rule.port_range_min),
+            ('icmp_code', None),
+            ('protocol', self._security_group_rule.protocol),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.create_security_group_rule.assert_called_once_with(**{
+            'direction': self._security_group_rule.direction,
+            'ethertype': self._security_group_rule.ethertype,
+            '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)
+        self.assertEqual(self.expected_data, data)
+
+    def test_create_ipv6_icmp_type_code(self):
+        self._setup_security_group_rule({
+            'ethertype': 'IPv6',
+            'port_range_min': 139,
+            'port_range_max': 2,
+            'protocol': 'ipv6-icmp',
+        })
+        arglist = [
+            '--icmp-type', str(self._security_group_rule.port_range_min),
+            '--icmp-code', str(self._security_group_rule.port_range_max),
+            '--protocol', self._security_group_rule.protocol,
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('dst_port', None),
+            ('icmp_type', self._security_group_rule.port_range_min),
+            ('icmp_code', self._security_group_rule.port_range_max),
+            ('protocol', self._security_group_rule.protocol),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.create_security_group_rule.assert_called_once_with(**{
+            'direction': self._security_group_rule.direction,
+            'ethertype': self._security_group_rule.ethertype,
+            '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,
+            'security_group_id': self._security_group.id,
+        })
+        self.assertEqual(self.expected_columns, columns)
+        self.assertEqual(self.expected_data, data)
+
+    def test_create_icmpv6_type(self):
+        self._setup_security_group_rule({
+            'ethertype': 'IPv6',
+            'port_range_min': 139,
+            'protocol': 'icmpv6',
+        })
+        arglist = [
+            '--icmp-type', str(self._security_group_rule.port_range_min),
+            '--protocol', self._security_group_rule.protocol,
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('dst_port', None),
+            ('icmp_type', self._security_group_rule.port_range_min),
+            ('icmp_code', None),
+            ('protocol', self._security_group_rule.protocol),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.create_security_group_rule.assert_called_once_with(**{
+            'direction': self._security_group_rule.direction,
+            'ethertype': self._security_group_rule.ethertype,
+            'port_range_min': self._security_group_rule.port_range_min,
+            'protocol': self._security_group_rule.protocol,
+            'security_group_id': self._security_group.id,
+        })
+        self.assertEqual(self.expected_columns, columns)
+        self.assertEqual(self.expected_data, data)
+
 
 class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
 
@@ -337,10 +517,21 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
         self.assertRaises(tests_utils.ParserException,
                           self.check_parser, self.cmd, arglist, [])
 
+    def test_create_all_protocol_options(self):
+        arglist = [
+            '--protocol', 'tcp',
+            '--proto', 'tcp',
+            self._security_group.id,
+        ]
+        self.assertRaises(tests_utils.ParserException,
+                          self.check_parser, self.cmd, arglist, [])
+
     def test_create_network_options(self):
         arglist = [
             '--ingress',
             '--ethertype', 'IPv4',
+            '--icmp-type', '3',
+            '--icmp-code', '11',
             '--project', identity_fakes.project_name,
             '--project-domain', identity_fakes.domain_name,
             self._security_group.id,
@@ -409,6 +600,38 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
         self.assertEqual(expected_data, data)
 
     def test_create_source_ip(self):
+        expected_columns, expected_data = self._setup_security_group_rule({
+            'ip_protocol': 'icmp',
+            'from_port': -1,
+            'to_port': -1,
+            'ip_range': {'cidr': '10.0.2.0/24'},
+        })
+        arglist = [
+            '--protocol', self._security_group_rule.ip_protocol,
+            '--src-ip', self._security_group_rule.ip_range['cidr'],
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('protocol', self._security_group_rule.ip_protocol),
+            ('src_ip', self._security_group_rule.ip_range['cidr']),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.compute.security_group_rules.create.assert_called_once_with(
+            self._security_group.id,
+            self._security_group_rule.ip_protocol,
+            self._security_group_rule.from_port,
+            self._security_group_rule.to_port,
+            self._security_group_rule.ip_range['cidr'],
+            None,
+        )
+        self.assertEqual(expected_columns, columns)
+        self.assertEqual(expected_data, data)
+
+    def test_create_proto_option(self):
         expected_columns, expected_data = self._setup_security_group_rule({
             'ip_protocol': 'icmp',
             'from_port': -1,
@@ -422,6 +645,7 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
         ]
         verifylist = [
             ('proto', self._security_group_rule.ip_protocol),
+            ('protocol', None),
             ('src_ip', self._security_group_rule.ip_range['cidr']),
             ('group', self._security_group.id),
         ]
@@ -522,8 +746,6 @@ class TestListSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
     _security_group_rule_icmp = \
         network_fakes.FakeSecurityGroupRule.create_one_security_group_rule({
             'protocol': 'icmp',
-            'port_range_max': -1,
-            'port_range_min': -1,
             'remote_ip_prefix': '10.0.2.0/24',
             'security_group_id': _security_group.id,
         })

releasenotes/notes/bug-1519512-dbf4368fe10dc495.yaml

@@ -0,0 +1,24 @@
+---
+features:
+  - Add ``--icmp-type`` and ``--icmp-code`` options to the
+    ``security group rule create`` command for Network v2 only.
+    These options can be used to set ICMP type and code for
+    ICMP IP protocols.
+    [Bug `1519512 <https://bugs.launchpad.net/bugs/1519512>`_]
+  - The following Network v2 IP protocols are supported by the
+    ``security group rule create`` command ``--protocol`` option,
+    ``ah``, ``dccp``, ``egp``, ``esp``, ``gre``, ``igmp``,
+    ``ipv6-encap``, ``ipv6-frag``, ``ipv6-icmp``, ``ipv6-nonxt``,
+    ``ipv6-opts``, ``ipv6-route``, ``ospf``, ``pgm``, ``rsvp``, ``sctp``,
+    ``udplite``, ``vrrp`` and integer representations [0-255].
+    [Bug `1519512 <https://bugs.launchpad.net/bugs/1519512>`_]
+  - The ``security group rule list`` command supports displaying
+    the ICMP type and code for security group rules with the
+    ICMP IP protocols.
+    [Bug `1519512 <https://bugs.launchpad.net/bugs/1519512>`_]
+upgrade:
+  - Changed the ``security group rule create`` command ``--proto``
+    option to ``--protocol``. Using the ``--proto`` option is still
+    supported, but is no longer documented and may be deprecated in
+    a future release.
+    [Bug `1519512 <https://bugs.launchpad.net/bugs/1519512>`_]