From fd5fd924d152338204fcf69673fedd31a3904977 Mon Sep 17 00:00:00 2001 From: Richard Theis Date: Fri, 15 Apr 2016 07:36:43 -0500 Subject: [PATCH] Additional network protocol support Add the following network protocol support to the "os security group rule create" command: - Add "--icmp-type" and "--icmp-code" options for Network v2 only. These options can be used to set the ICMP type and code for ICMP IP protocols. - Change the "--proto" option to "--protocol". Using the "--proto" option is still supported, but is no longer documented and may be deprecated in a future release. - Add the following Network v2 IP protocols to the "--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]. The "os security group rule list" command now supports displaying the ICMP type and code for security group rules with the ICMP IP protocols. Change-Id: Ic84bc92bc7aa5ac08f6ef91660eb6c125a200eb3 Closes-Bug: #1519512 Implements: blueprint neutron-client --- .../command-objects/security-group-rule.rst | 42 ++- .../network/v2/test_security_group_rule.py | 2 +- .../network/v2/security_group_rule.py | 184 ++++++++++--- openstackclient/tests/network/v2/fakes.py | 4 +- .../network/v2/test_security_group_rule.py | 250 +++++++++++++++++- .../notes/bug-1519512-dbf4368fe10dc495.yaml | 24 ++ 6 files changed, 448 insertions(+), 58 deletions(-) create mode 100644 releasenotes/notes/bug-1519512-dbf4368fe10dc495.yaml diff --git a/doc/source/command-objects/security-group-rule.rst b/doc/source/command-objects/security-group-rule.rst index b0ac3c944..97cce35cf 100644 --- a/doc/source/command-objects/security-group-rule.rst +++ b/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 ] [--src-ip | --src-group ] - [--dst-port ] + [--dst-port | [--icmp-type [--icmp-code ]]] + [--protocol ] [--ingress | --egress] [--ethertype ] [--project [--project-domain ]] -.. option:: --proto - - IP protocol (icmp, tcp, udp; default: tcp) - .. option:: --src-ip Source IP address block @@ -39,8 +35,35 @@ Create a new security group rule .. option:: --dst-port - Destination port, may be a single port or port range: 137:139 - (only required for IP protocols tcp and udp) + 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. + +.. option:: --icmp-type + + ICMP type for ICMP IP protocols + + *Network version 2 only* + +.. option:: --icmp-code + + ICMP code for ICMP IP protocols + + *Network version 2 only* + +.. option:: --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 of network traffic (IPv4, IPv6; default: IPv4) + Ethertype of network traffic + (IPv4, IPv6; default: based on IP protocol) *Network version 2 only* diff --git a/functional/tests/network/v2/test_security_group_rule.py b/functional/tests/network/v2/test_security_group_rule.py index 26e6e0e45..64e1fcdf6 100644 --- a/functional/tests/network/v2/test_security_group_rule.py +++ b/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') diff --git a/openstackclient/network/v2/security_group_rule.py b/openstackclient/network/v2/security_group_rule.py index 5b22a0dd8..92dd1e5a2 100644 --- a/openstackclient/network/v2/security_group_rule.py +++ b/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 - (rule.port_range_min or rule.port_range_max)): + if _is_icmp_protocol(rule.protocol): + 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='', - 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="", - default="tcp", - choices=['icmp', 'tcp', 'udp'], - type=_convert_to_lowercase, - help=_("IP protocol (icmp, tcp, udp; default: tcp)") + help=_("Create rule in this security group (name or ID)") ) source_group = parser.add_mutually_exclusive_group() source_group.add_argument( @@ -94,17 +105,49 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne): metavar="", help=_("Source security group (name or ID)") ) - parser.add_argument( - "--dst-port", - metavar="", - 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='', + 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='', + type=int, + help=_("ICMP type for ICMP IP protocols") + ) + parser.add_argument( + '--icmp-code', + metavar='', + 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='', + 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='', + 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='', 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='', + 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='', + choices=['icmp', 'tcp', 'udp'], + type=_convert_to_lowercase, + help=_("IP protocol (icmp, tcp, udp; default: tcp)") + ) + protocol_group.add_argument( + '--proto', + metavar='', + 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. - if parsed_args.proto != 'icmp': + + # NOTE(rtheis): Validate the port range and ICMP type and code. + # 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, diff --git a/openstackclient/tests/network/v2/fakes.py b/openstackclient/tests/network/v2/fakes.py index 7c4604bd8..a0baf7847 100644 --- a/openstackclient/tests/network/v2/fakes.py +++ b/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_min': 0, + 'port_range_max': None, + 'port_range_min': None, 'protocol': 'tcp', 'remote_group_id': None, 'remote_ip_prefix': '0.0.0.0/0', diff --git a/openstackclient/tests/network/v2/test_security_group_rule.py b/openstackclient/tests/network/v2/test_security_group_rule.py index bd903f9e7..2a64b8844 100644 --- a/openstackclient/tests/network/v2/test_security_group_rule.py +++ b/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, }) diff --git a/releasenotes/notes/bug-1519512-dbf4368fe10dc495.yaml b/releasenotes/notes/bug-1519512-dbf4368fe10dc495.yaml new file mode 100644 index 000000000..f8f2387fe --- /dev/null +++ b/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 `_] + - 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 `_] + - The ``security group rule list`` command supports displaying + the ICMP type and code for security group rules with the + ICMP IP protocols. + [Bug `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 `_]