From 221b7052abcefa56213a6516cecc6de9c73026d1 Mon Sep 17 00:00:00 2001
From: gvrangan <venkatrangang@hcl.com>
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 <venkatrangang@hcl.com>
---
.../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 <https://bugs.launchpad.net/python-openstackclient/+bug/1703704>`_]