Merge "Refactor security group create to use SDK"

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="<name>",
-            help="New security group name",
-        )
-        parser.add_argument(
-            "--description",
-            metavar="<description>",
-            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"""
 

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="<name>",
+            help="New security group name",
+        )
+        parser.add_argument(
+            "--description",
+            metavar="<description>",
+            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"""
 

openstackclient/tests/compute/v2/test_security_group.py

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

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

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 <https://bugs.launchpad.net/bugs/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 <https://bugs.launchpad.net/bugs/1519511>`_]

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