From 221b7052abcefa56213a6516cecc6de9c73026d1 Mon Sep 17 00:00:00 2001 From: gvrangan Date: Thu, 21 Sep 2017 06:37:10 +0530 Subject: [PATCH] Support icmp-type and icmp-code to be set as zero When icmp-type or icmp-code are set to 0, the current implementation ignores the value, this fix will allow the value to be copied and displayed Change-Id: I96133a57883d22e98fcbb9fe0328d9e050472469 Signed-off-by: gvrangan --- .../network/v2/security_group_rule.py | 4 +- .../v2/test_security_group_rule_network.py | 170 ++++++++++++++++++ ...-icmp-type-code-zero-cbef0a36db2b8123.yaml | 5 + 3 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/support-icmp-type-code-zero-cbef0a36db2b8123.yaml diff --git a/openstackclient/network/v2/security_group_rule.py b/openstackclient/network/v2/security_group_rule.py index ad685cf8ae..06d467254b 100644 --- a/openstackclient/network/v2/security_group_rule.py +++ b/openstackclient/network/v2/security_group_rule.py @@ -294,9 +294,9 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne): 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] - if parsed_args.icmp_type: + if parsed_args.icmp_type is not None and parsed_args.icmp_type >= 0: attrs['port_range_min'] = parsed_args.icmp_type - if parsed_args.icmp_code: + if parsed_args.icmp_code is not None and parsed_args.icmp_code >= 0: attrs['port_range_max'] = parsed_args.icmp_code # NOTE(dtroyer): --src-ip and --src-group were deprecated in Nov 2016. diff --git a/openstackclient/tests/unit/network/v2/test_security_group_rule_network.py b/openstackclient/tests/unit/network/v2/test_security_group_rule_network.py index 5d9d03e9c7..36add8ca80 100644 --- a/openstackclient/tests/unit/network/v2/test_security_group_rule_network.py +++ b/openstackclient/tests/unit/network/v2/test_security_group_rule_network.py @@ -407,6 +407,78 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork): self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) + def test_create_icmp_code_zero(self): + self._setup_security_group_rule({ + 'port_range_min': 15, + 'port_range_max': 0, + 'protocol': 'icmp', + 'remote_ip_prefix': '0.0.0.0/0', + }) + arglist = [ + '--protocol', self._security_group_rule.protocol, + '--icmp-type', str(self._security_group_rule.port_range_min), + '--icmp-code', str(self._security_group_rule.port_range_max), + self._security_group.id, + ] + verifylist = [ + ('protocol', self._security_group_rule.protocol), + ('icmp_code', self._security_group_rule.port_range_max), + ('icmp_type', self._security_group_rule.port_range_min), + ('group', self._security_group.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + self.assertEqual(self.expected_columns, columns) + self.assertEqual(self.expected_data, data) + + def test_create_icmp_code_greater_than_zero(self): + self._setup_security_group_rule({ + 'port_range_min': 15, + 'port_range_max': 18, + 'protocol': 'icmp', + 'remote_ip_prefix': '0.0.0.0/0', + }) + arglist = [ + '--protocol', self._security_group_rule.protocol, + '--icmp-type', str(self._security_group_rule.port_range_min), + '--icmp-code', str(self._security_group_rule.port_range_max), + self._security_group.id, + ] + verifylist = [ + ('protocol', self._security_group_rule.protocol), + ('icmp_type', self._security_group_rule.port_range_min), + ('icmp_code', self._security_group_rule.port_range_max), + ('group', self._security_group.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + self.assertEqual(self.expected_columns, columns) + self.assertEqual(self.expected_data, data) + + def test_create_icmp_code_negative_value(self): + self._setup_security_group_rule({ + 'port_range_min': 15, + 'port_range_max': None, + 'protocol': 'icmp', + 'remote_ip_prefix': '0.0.0.0/0', + }) + arglist = [ + '--protocol', self._security_group_rule.protocol, + '--icmp-type', str(self._security_group_rule.port_range_min), + '--icmp-code', '-2', + self._security_group.id, + ] + verifylist = [ + ('protocol', self._security_group_rule.protocol), + ('icmp_type', self._security_group_rule.port_range_min), + ('icmp_code', -2), + ('group', self._security_group.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + self.assertEqual(self.expected_columns, columns) + self.assertEqual(self.expected_data, data) + def test_create_icmp_type(self): self._setup_security_group_rule({ 'port_range_min': 15, @@ -440,6 +512,104 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork): self.assertEqual(self.expected_columns, columns) self.assertEqual(self.expected_data, data) + def test_create_icmp_type_zero(self): + self._setup_security_group_rule({ + 'port_range_min': 0, + '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.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) + self.assertEqual(self.expected_data, data) + + def test_create_icmp_type_greater_than_zero(self): + self._setup_security_group_rule({ + 'port_range_min': 13, # timestamp + '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.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) + self.assertEqual(self.expected_data, data) + + def test_create_icmp_type_negative_value(self): + self._setup_security_group_rule({ + 'port_range_min': None, # timestamp + 'protocol': 'icmp', + 'remote_ip_prefix': '0.0.0.0/0', + }) + arglist = [ + '--icmp-type', '-13', + '--protocol', self._security_group_rule.protocol, + self._security_group.id, + ] + verifylist = [ + ('dst_port', None), + ('icmp_type', -13), + ('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.ether_type, + '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({ 'ether_type': 'IPv6', diff --git a/releasenotes/notes/support-icmp-type-code-zero-cbef0a36db2b8123.yaml b/releasenotes/notes/support-icmp-type-code-zero-cbef0a36db2b8123.yaml new file mode 100644 index 0000000000..c5910ddb39 --- /dev/null +++ b/releasenotes/notes/support-icmp-type-code-zero-cbef0a36db2b8123.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Add support to set ``--icmp-type`` and ``--icmp-code`` to 0 in the + ``security group rule`` command. + [Bug `1703704 `_]