Merge "Support icmp-type and icmp-code to be set as zero"

2 files changed, 172 insertions, 2 deletions

diff --git a/openstackclient/network/v2/security_group_rule.py b/openstackclient/network/v2/security_group_rule.py
index ad685cf8..06d46725 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 5d9d03e9..36add8ca 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',