From 3a915b5b5fc8f7f5dc2618eeedbef7517f1557c3 Mon Sep 17 00:00:00 2001
From: judy-yu <yujie@cmss.chinamobile.com>
Date: Fri, 4 Nov 2016 17:23:46 +0800
Subject: [PATCH] Not appropriate name sg rule attribute
For ingress rules set ip-prefix means src-ip-
prefix, but for egress rules set ip-prefix means
dst-ip-prefix. It is not appropriate to name
src-ip directly. So as to src-group.
Change-Id: I03fd0e14e470e7272930ac2651e73263b83bd4e1
Closes-bug: #1637365
---
.../command-objects/security-group-rule.rst | 10 +-
.../network/v2/security_group_rule.py | 54 +++++--
.../network/v2/test_security_group_rule.py | 141 +++++++++++++++++-
.../notes/bug-1637365-b90cdcc05ffc7d3a.yaml | 7 +
4 files changed, 190 insertions(+), 22 deletions(-)
create mode 100644 releasenotes/notes/bug-1637365-b90cdcc05ffc7d3a.yaml
diff --git a/doc/source/command-objects/security-group-rule.rst b/doc/source/command-objects/security-group-rule.rst
index 5284b2dc22..5bd144e988 100644
--- a/doc/source/command-objects/security-group-rule.rst
+++ b/doc/source/command-objects/security-group-rule.rst
@@ -16,7 +16,7 @@ Create a new security group rule
.. code:: bash
os security group rule create
- [--src-ip <ip-address> | --src-group <group>]
+ [--remote-ip <ip-address> | --remote-group <group>]
[--dst-port <port-range> | [--icmp-type <icmp-type> [--icmp-code <icmp-code>]]]
[--protocol <protocol>]
[--ingress | --egress]
@@ -24,14 +24,14 @@ Create a new security group rule
[--project <project> [--project-domain <project-domain>]]
<group>
-.. option:: --src-ip <ip-address>
+.. option:: --remote-ip <ip-address>
- Source IP address block
+ Remote IP address block
(may use CIDR notation; default for IPv4 rule: 0.0.0.0/0)
-.. option:: --src-group <group>
+.. option:: --remote-group <group>
- Source security group (name or ID)
+ Remote security group (name or ID)
.. option:: --dst-port <port-range>
diff --git a/openstackclient/network/v2/security_group_rule.py b/openstackclient/network/v2/security_group_rule.py
index e3be44ece7..49435db673 100644
--- a/openstackclient/network/v2/security_group_rule.py
+++ b/openstackclient/network/v2/security_group_rule.py
@@ -94,14 +94,31 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
metavar='<group>',
help=_("Create rule in this security group (name or ID)")
)
- source_group = parser.add_mutually_exclusive_group()
- source_group.add_argument(
+ # NOTE(yujie): Support either remote-ip option name for now.
+ # However, consider deprecating and then removing --src-ip in
+ # a future release.
+ remote_group = parser.add_mutually_exclusive_group()
+ remote_group.add_argument(
+ "--remote-ip",
+ metavar="<ip-address>",
+ help=_("Remote IP address block (may use CIDR notation; "
+ "default for IPv4 rule: 0.0.0.0/0)")
+ )
+ remote_group.add_argument(
"--src-ip",
metavar="<ip-address>",
help=_("Source IP address block (may use CIDR notation; "
"default for IPv4 rule: 0.0.0.0/0)")
)
- source_group.add_argument(
+ # NOTE(yujie): Support either remote-group option name for now.
+ # However, consider deprecating and then removing --src-group in
+ # a future release.
+ remote_group.add_argument(
+ "--remote-group",
+ metavar="<group>",
+ help=_("Remote security group (name or ID)")
+ )
+ remote_group.add_argument(
"--src-group",
metavar="<group>",
help=_("Source security group (name or ID)")
@@ -277,13 +294,16 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
if parsed_args.icmp_code:
attrs['port_range_max'] = parsed_args.icmp_code
- if parsed_args.src_group is not None:
+ if not (parsed_args.remote_group is None and
+ parsed_args.src_group is None):
attrs['remote_group_id'] = client.find_security_group(
- parsed_args.src_group,
+ parsed_args.remote_group or parsed_args.src_group,
ignore_missing=False
).id
- elif parsed_args.src_ip is not None:
- attrs['remote_ip_prefix'] = parsed_args.src_ip
+ elif not (parsed_args.remote_ip is None and
+ parsed_args.src_ip is None):
+ attrs['remote_ip_prefix'] = (parsed_args.remote_ip or
+ parsed_args.src_ip)
elif attrs['ethertype'] == 'IPv4':
attrs['remote_ip_prefix'] = '0.0.0.0/0'
attrs['security_group_id'] = security_group_id
@@ -312,23 +332,25 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
from_port, to_port = -1, -1
else:
from_port, to_port = parsed_args.dst_port
- src_ip = None
- if parsed_args.src_group is not None:
- parsed_args.src_group = utils.find_resource(
+ remote_ip = None
+ if not (parsed_args.remote_group is None and
+ parsed_args.src_group is None):
+ parsed_args.remote_group = utils.find_resource(
client.security_groups,
- parsed_args.src_group,
+ parsed_args.remote_group or parsed_args.src_group,
).id
- if parsed_args.src_ip is not None:
- src_ip = parsed_args.src_ip
+ if not (parsed_args.remote_ip is None and
+ parsed_args.src_ip is None):
+ remote_ip = parsed_args.remote_ip or parsed_args.src_ip
else:
- src_ip = '0.0.0.0/0'
+ remote_ip = '0.0.0.0/0'
obj = client.security_group_rules.create(
group.id,
protocol,
from_port,
to_port,
- src_ip,
- parsed_args.src_group,
+ remote_ip,
+ parsed_args.remote_group,
)
return _format_security_group_rule_show(obj._info)
diff --git a/openstackclient/tests/unit/network/v2/test_security_group_rule.py b/openstackclient/tests/unit/network/v2/test_security_group_rule.py
index 96d58e5c00..a6093ffe03 100644
--- a/openstackclient/tests/unit/network/v2/test_security_group_rule.py
+++ b/openstackclient/tests/unit/network/v2/test_security_group_rule.py
@@ -119,6 +119,15 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
self.assertRaises(tests_utils.ParserException,
self.check_parser, self.cmd, arglist, [])
+ def test_create_all_remote_options(self):
+ arglist = [
+ '--remote-ip', '10.10.0.0/24',
+ '--remote-group', self._security_group.id,
+ self._security_group.id,
+ ]
+ self.assertRaises(tests_utils.ParserException,
+ self.check_parser, self.cmd, arglist, [])
+
def test_create_bad_ethertype(self):
arglist = [
'--ethertype', 'foo',
@@ -213,7 +222,7 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
self.assertEqual(self.expected_columns, columns)
self.assertEqual(self.expected_data, data)
- def test_create_source_group(self):
+ def test_create_remote_group(self):
self._setup_security_group_rule({
'port_range_max': 22,
'port_range_min': 22,
@@ -248,6 +257,34 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
self.assertEqual(self.expected_columns, columns)
self.assertEqual(self.expected_data, data)
+ def test_create_source_group(self):
+ self._setup_security_group_rule({
+ 'remote_group_id': self._security_group.id,
+ })
+ arglist = [
+ '--ingress',
+ '--src-group', self._security_group.name,
+ self._security_group.id,
+ ]
+ verifylist = [
+ ('ingress', True),
+ ('src_group', self._security_group.name),
+ ('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_group_id': self._security_group_rule.remote_group_id,
+ 'security_group_id': self._security_group.id,
+ })
+ self.assertEqual(self.expected_columns, columns)
+ self.assertEqual(self.expected_data, data)
+
def test_create_source_ip(self):
self._setup_security_group_rule({
'protocol': 'icmp',
@@ -277,6 +314,35 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
self.assertEqual(self.expected_columns, columns)
self.assertEqual(self.expected_data, data)
+ def test_create_remote_ip(self):
+ self._setup_security_group_rule({
+ 'protocol': 'icmp',
+ 'remote_ip_prefix': '10.0.2.0/24',
+ })
+ arglist = [
+ '--protocol', self._security_group_rule.protocol,
+ '--remote-ip', self._security_group_rule.remote_ip_prefix,
+ self._security_group.id,
+ ]
+ verifylist = [
+ ('protocol', self._security_group_rule.protocol),
+ ('remote_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_network_options(self):
self._setup_security_group_rule({
'direction': 'egress',
@@ -498,6 +564,15 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
self.assertRaises(tests_utils.ParserException,
self.check_parser, self.cmd, arglist, [])
+ def test_create_all_remote_options(self):
+ arglist = [
+ '--remote-ip', '10.10.0.0/24',
+ '--remote-group', self._security_group.id,
+ self._security_group.id,
+ ]
+ self.assertRaises(tests_utils.ParserException,
+ self.check_parser, self.cmd, arglist, [])
+
def test_create_bad_protocol(self):
arglist = [
'--protocol', 'foo',
@@ -588,6 +663,38 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
self.assertEqual(expected_columns, columns)
self.assertEqual(expected_data, data)
+ def test_create_remote_group(self):
+ expected_columns, expected_data = self._setup_security_group_rule({
+ 'from_port': 22,
+ 'to_port': 22,
+ 'group': {'name': self._security_group.name},
+ })
+ arglist = [
+ '--dst-port', str(self._security_group_rule.from_port),
+ '--remote-group', self._security_group.name,
+ self._security_group.id,
+ ]
+ verifylist = [
+ ('dst_port', (self._security_group_rule.from_port,
+ self._security_group_rule.to_port)),
+ ('remote_group', self._security_group.name),
+ ('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'],
+ self._security_group.id,
+ )
+ self.assertEqual(expected_columns, columns)
+ self.assertEqual(expected_data, data)
+
def test_create_source_ip(self):
expected_columns, expected_data = self._setup_security_group_rule({
'ip_protocol': 'icmp',
@@ -620,6 +727,38 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
self.assertEqual(expected_columns, columns)
self.assertEqual(expected_data, data)
+ def test_create_remote_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,
+ '--remote-ip', self._security_group_rule.ip_range['cidr'],
+ self._security_group.id,
+ ]
+ verifylist = [
+ ('protocol', self._security_group_rule.ip_protocol),
+ ('remote_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',
diff --git a/releasenotes/notes/bug-1637365-b90cdcc05ffc7d3a.yaml b/releasenotes/notes/bug-1637365-b90cdcc05ffc7d3a.yaml
new file mode 100644
index 0000000000..2b8e6c16bc
--- /dev/null
+++ b/releasenotes/notes/bug-1637365-b90cdcc05ffc7d3a.yaml
@@ -0,0 +1,7 @@
+upgrade:
+ -
+ Changed the ``security group rule create`` command ``--src-ip``
+ option to ``--remote-ip``, ``--src-group`` option to ``--remote-group``.
+ Using the ``--src-ip`` ``--src-group`` option is still supported, but
+ is no longer documented and may be deprecated in a future release.
+ [Bug `1637365 <https://bugs.launchpad.net/python-openstackclient/+bug/1637365>`_]