diff --git a/openstackclient/compute/v2/security_group.py b/openstackclient/compute/v2/security_group.py index 042e5caf..734bd78e 100644 --- a/openstackclient/compute/v2/security_group.py +++ b/openstackclient/compute/v2/security_group.py @@ -56,38 +56,6 @@ def _xform_security_group_rule(sgroup): return info -class CreateSecurityGroup(command.ShowOne): - """Create a new security group""" - - def get_parser(self, prog_name): - parser = super(CreateSecurityGroup, self).get_parser(prog_name) - parser.add_argument( - "name", - metavar="", - help="New security group name", - ) - parser.add_argument( - "--description", - metavar="", - help="Security group description", - ) - return parser - - def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - - description = parsed_args.description or parsed_args.name - - data = compute_client.security_groups.create( - parsed_args.name, - description, - ) - - info = {} - info.update(data._info) - return zip(*sorted(six.iteritems(info))) - - class CreateSecurityGroupRule(command.ShowOne): """Create a new security group rule""" diff --git a/openstackclient/network/v2/security_group.py b/openstackclient/network/v2/security_group.py index 64717945..f8162477 100644 --- a/openstackclient/network/v2/security_group.py +++ b/openstackclient/network/v2/security_group.py @@ -91,6 +91,56 @@ def _get_columns(item): return tuple(display_columns), property_columns +class CreateSecurityGroup(common.NetworkAndComputeShowOne): + """Create a new security group""" + + def update_parser_common(self, parser): + parser.add_argument( + "name", + metavar="", + help="New security group name", + ) + parser.add_argument( + "--description", + metavar="", + help="Security group description", + ) + return parser + + def _get_description(self, parsed_args): + if parsed_args.description is not None: + return parsed_args.description + else: + return parsed_args.name + + def take_action_network(self, client, parsed_args): + attrs = {} + attrs['name'] = parsed_args.name + attrs['description'] = self._get_description(parsed_args) + obj = client.create_security_group(**attrs) + display_columns, property_columns = _get_columns(obj) + data = utils.get_item_properties( + obj, + property_columns, + formatters=_formatters_network + ) + return (display_columns, data) + + def take_action_compute(self, client, parsed_args): + description = self._get_description(parsed_args) + obj = client.security_groups.create( + parsed_args.name, + description, + ) + display_columns, property_columns = _get_columns(obj._info) + data = utils.get_dict_properties( + obj._info, + property_columns, + formatters=_formatters_compute + ) + return (display_columns, data) + + class DeleteSecurityGroup(common.NetworkAndComputeCommand): """Delete a security group""" diff --git a/openstackclient/tests/compute/v2/test_security_group.py b/openstackclient/tests/compute/v2/test_security_group.py deleted file mode 100644 index 01e27b82..00000000 --- a/openstackclient/tests/compute/v2/test_security_group.py +++ /dev/null @@ -1,127 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -# - -import copy - -from openstackclient.compute.v2 import security_group -from openstackclient.tests.compute.v2 import fakes as compute_fakes -from openstackclient.tests import fakes -from openstackclient.tests.identity.v2_0 import fakes as identity_fakes - - -security_group_id = '11' -security_group_name = 'wide-open' -security_group_description = 'nothing but net' - -SECURITY_GROUP = { - 'id': security_group_id, - 'name': security_group_name, - 'description': security_group_description, - 'tenant_id': identity_fakes.project_id, -} - - -class FakeSecurityGroupResource(fakes.FakeResource): - - def get_keys(self): - return {'property': 'value'} - - -class TestSecurityGroup(compute_fakes.TestComputev2): - - def setUp(self): - super(TestSecurityGroup, self).setUp() - - # Get a shortcut compute client security_groups mock - self.secgroups_mock = self.app.client_manager.compute.security_groups - self.secgroups_mock.reset_mock() - - # Get a shortcut identity client projects mock - self.projects_mock = self.app.client_manager.identity.projects - self.projects_mock.reset_mock() - - -class TestSecurityGroupCreate(TestSecurityGroup): - - columns = ( - 'description', - 'id', - 'name', - 'tenant_id', - ) - data = ( - security_group_description, - security_group_id, - security_group_name, - identity_fakes.project_id, - ) - - def setUp(self): - super(TestSecurityGroupCreate, self).setUp() - - self.secgroups_mock.create.return_value = FakeSecurityGroupResource( - None, - copy.deepcopy(SECURITY_GROUP), - loaded=True, - ) - - # Get the command object to test - self.cmd = security_group.CreateSecurityGroup(self.app, None) - - def test_security_group_create_no_options(self): - arglist = [ - security_group_name, - ] - verifylist = [ - ('name', security_group_name), - ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - - # In base command class ShowOne in cliff, abstract method take_action() - # returns a two-part tuple with a tuple of column names and a tuple of - # data to be shown. - columns, data = self.cmd.take_action(parsed_args) - - # SecurityGroupManager.create(name, description) - self.secgroups_mock.create.assert_called_with( - security_group_name, - security_group_name, - ) - - self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) - - def test_security_group_create_description(self): - arglist = [ - security_group_name, - '--description', security_group_description, - ] - verifylist = [ - ('name', security_group_name), - ('description', security_group_description), - ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - - # In base command class ShowOne in cliff, abstract method take_action() - # returns a two-part tuple with a tuple of column names and a tuple of - # data to be shown. - columns, data = self.cmd.take_action(parsed_args) - - # SecurityGroupManager.create(name, description) - self.secgroups_mock.create.assert_called_with( - security_group_name, - security_group_description, - ) - - self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) diff --git a/openstackclient/tests/network/v2/test_security_group.py b/openstackclient/tests/network/v2/test_security_group.py index a66b7b00..2d43d872 100644 --- a/openstackclient/tests/network/v2/test_security_group.py +++ b/openstackclient/tests/network/v2/test_security_group.py @@ -37,6 +37,153 @@ class TestSecurityGroupCompute(compute_fakes.TestComputev2): self.compute = self.app.client_manager.compute +class TestCreateSecurityGroupNetwork(TestSecurityGroupNetwork): + + # The security group to be created. + _security_group = \ + network_fakes.FakeSecurityGroup.create_one_security_group() + + columns = ( + 'description', + 'id', + 'name', + 'project_id', + 'rules', + ) + + data = ( + _security_group.description, + _security_group.id, + _security_group.name, + _security_group.project_id, + '', + ) + + def setUp(self): + super(TestCreateSecurityGroupNetwork, self).setUp() + + self.network.create_security_group = mock.Mock( + return_value=self._security_group) + + # Get the command object to test + self.cmd = security_group.CreateSecurityGroup(self.app, self.namespace) + + def test_create_no_options(self): + self.assertRaises(tests_utils.ParserException, + self.check_parser, self.cmd, [], []) + + def test_create_min_options(self): + arglist = [ + self._security_group.name, + ] + verifylist = [ + ('name', self._security_group.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.network.create_security_group.assert_called_once_with(**{ + 'description': self._security_group.name, + 'name': self._security_group.name, + }) + self.assertEqual(tuple(self.columns), columns) + self.assertEqual(self.data, data) + + def test_create_all_options(self): + arglist = [ + '--description', self._security_group.description, + self._security_group.name, + ] + verifylist = [ + ('description', self._security_group.description), + ('name', self._security_group.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.network.create_security_group.assert_called_once_with(**{ + 'description': self._security_group.description, + 'name': self._security_group.name, + }) + self.assertEqual(tuple(self.columns), columns) + self.assertEqual(self.data, data) + + +class TestCreateSecurityGroupCompute(TestSecurityGroupCompute): + + # The security group to be shown. + _security_group = \ + compute_fakes.FakeSecurityGroup.create_one_security_group() + + columns = ( + 'description', + 'id', + 'name', + 'project_id', + 'rules', + ) + + data = ( + _security_group.description, + _security_group.id, + _security_group.name, + _security_group.tenant_id, + '', + ) + + def setUp(self): + super(TestCreateSecurityGroupCompute, self).setUp() + + self.app.client_manager.network_endpoint_enabled = False + + self.compute.security_groups.create.return_value = self._security_group + + # Get the command object to test + self.cmd = security_group.CreateSecurityGroup(self.app, None) + + def test_create_no_options(self): + self.assertRaises(tests_utils.ParserException, + self.check_parser, self.cmd, [], []) + + def test_create_min_options(self): + arglist = [ + self._security_group.name, + ] + verifylist = [ + ('name', self._security_group.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.compute.security_groups.create.assert_called_once_with( + self._security_group.name, + self._security_group.name) + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) + + def test_create_all_options(self): + arglist = [ + '--description', self._security_group.description, + self._security_group.name, + ] + verifylist = [ + ('description', self._security_group.description), + ('name', self._security_group.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.compute.security_groups.create.assert_called_once_with( + self._security_group.name, + self._security_group.description) + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) + + class TestDeleteSecurityGroupNetwork(TestSecurityGroupNetwork): # The security group to be deleted. @@ -65,7 +212,7 @@ class TestDeleteSecurityGroupNetwork(TestSecurityGroupNetwork): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.network.delete_security_group.assert_called_with( + self.network.delete_security_group.assert_called_once_with( self._security_group) self.assertIsNone(result) @@ -100,7 +247,7 @@ class TestDeleteSecurityGroupCompute(TestSecurityGroupCompute): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.compute.security_groups.delete.assert_called_with( + self.compute.security_groups.delete.assert_called_once_with( self._security_group.id) self.assertIsNone(result) @@ -143,7 +290,7 @@ class TestListSecurityGroupNetwork(TestSecurityGroupNetwork): columns, data = self.cmd.take_action(parsed_args) - self.network.security_groups.assert_called_with() + self.network.security_groups.assert_called_once_with() self.assertEqual(self.expected_columns, columns) self.assertEqual(self.expected_data, tuple(data)) @@ -158,7 +305,7 @@ class TestListSecurityGroupNetwork(TestSecurityGroupNetwork): columns, data = self.cmd.take_action(parsed_args) - self.network.security_groups.assert_called_with() + self.network.security_groups.assert_called_once_with() self.assertEqual(self.expected_columns, columns) self.assertEqual(self.expected_data, tuple(data)) @@ -212,7 +359,7 @@ class TestListSecurityGroupCompute(TestSecurityGroupCompute): columns, data = self.cmd.take_action(parsed_args) kwargs = {'search_opts': {'all_tenants': False}} - self.compute.security_groups.list.assert_called_with(**kwargs) + self.compute.security_groups.list.assert_called_once_with(**kwargs) self.assertEqual(self.expected_columns, columns) self.assertEqual(self.expected_data, tuple(data)) @@ -228,7 +375,7 @@ class TestListSecurityGroupCompute(TestSecurityGroupCompute): columns, data = self.cmd.take_action(parsed_args) kwargs = {'search_opts': {'all_tenants': True}} - self.compute.security_groups.list.assert_called_with(**kwargs) + self.compute.security_groups.list.assert_called_once_with(**kwargs) self.assertEqual(self.expected_columns_all_projects, columns) self.assertEqual(self.expected_data_all_projects, tuple(data)) diff --git a/releasenotes/notes/bug-1519511-65b8901ae6ea2e63.yaml b/releasenotes/notes/bug-1519511-65b8901ae6ea2e63.yaml new file mode 100644 index 00000000..843c6caf --- /dev/null +++ b/releasenotes/notes/bug-1519511-65b8901ae6ea2e63.yaml @@ -0,0 +1,13 @@ +--- +features: + - The ``security group create`` command now uses Network v2 when + enabled which results in a more detailed output for network + security group rules. + [Bug `1519511 `_] +fixes: + - The ``security group create`` command now uses Network v2 when + enabled which allows the security group description to be created + with an empty value. In addition, the ``tenant_id`` field changed + to ``project_id`` to match the ``security group show`` command + output. + [Bug `1519511 `_] diff --git a/setup.cfg b/setup.cfg index b514057b..a05a79d8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -99,7 +99,6 @@ openstack.compute.v2 = keypair_list = openstackclient.compute.v2.keypair:ListKeypair keypair_show = openstackclient.compute.v2.keypair:ShowKeypair - security_group_create = openstackclient.compute.v2.security_group:CreateSecurityGroup security_group_rule_create = openstackclient.compute.v2.security_group:CreateSecurityGroupRule security_group_rule_list = openstackclient.compute.v2.security_group:ListSecurityGroupRule @@ -344,6 +343,7 @@ openstack.network.v2 = router_set = openstackclient.network.v2.router:SetRouter router_show = openstackclient.network.v2.router:ShowRouter + security_group_create = openstackclient.network.v2.security_group:CreateSecurityGroup security_group_delete = openstackclient.network.v2.security_group:DeleteSecurityGroup security_group_list = openstackclient.network.v2.security_group:ListSecurityGroup security_group_set = openstackclient.network.v2.security_group:SetSecurityGroup