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
This commit is contained in:
judy-yu 2016-11-04 17:23:46 +08:00
parent 970b0e0005
commit 3a915b5b5f
4 changed files with 190 additions and 22 deletions

View File

@ -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>

View File

@ -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)

View File

@ -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',

View File

@ -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>`_]