diff --git a/doc/source/command-objects/security-group-rule.rst b/doc/source/command-objects/security-group-rule.rst index 5284b2dc2..5bd144e98 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 | --src-group ] + [--remote-ip | --remote-group ] [--dst-port | [--icmp-type [--icmp-code ]]] [--protocol ] [--ingress | --egress] @@ -24,14 +24,14 @@ Create a new security group rule [--project [--project-domain ]] -.. option:: --src-ip +.. option:: --remote-ip - Source IP address block + Remote IP address block (may use CIDR notation; default for IPv4 rule: 0.0.0.0/0) -.. option:: --src-group +.. option:: --remote-group - Source security group (name or ID) + Remote security group (name or ID) .. option:: --dst-port diff --git a/openstackclient/network/v2/security_group_rule.py b/openstackclient/network/v2/security_group_rule.py index e3be44ece..49435db67 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='', 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="", + 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="", 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="", + help=_("Remote security group (name or ID)") + ) + remote_group.add_argument( "--src-group", metavar="", 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 96d58e5c0..a6093ffe0 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 000000000..2b8e6c16b --- /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 `_]