From 580b0aff8810f234adcf364b0fe151b6e3c58182 Mon Sep 17 00:00:00 2001 From: Richard Theis Date: Mon, 14 Dec 2015 13:29:43 -0600 Subject: [PATCH] Refactor security group delete to use SDK Refactored the 'os security group delete' command to use the SDK when neutron is enabled, but continue to use the nova client when nova network is enabled. This patch set introduces a new NetworkAndComputeCommand class to be used for commands that must support neutron and nova network. The new class allows both the parser and actions to be unique. The current DeleteSecurityGroup class is now a subclass of this new class and has moved under the network v2 commands. This patch set also introduces a new FakeSecurityGroup class for testing security groups. And finally, this patch set updates the command documentation for security group and security group rule to indicate that Network v2 is also used. Change-Id: Ic21376b86b40cc6d97f360f3760ba5beed154537 Partial-Bug: #1519511 Related-to: blueprint neutron-client --- .../command-objects/security-group-rule.rst | 2 +- doc/source/command-objects/security-group.rst | 2 +- openstackclient/compute/v2/security_group.py | 22 ---- openstackclient/network/common.py | 62 +++++++++++ openstackclient/network/v2/security_group.py | 40 +++++++ openstackclient/tests/network/test_common.py | 103 ++++++++++++++++++ openstackclient/tests/network/v2/fakes.py | 78 +++++++++++++ .../tests/network/v2/test_security_group.py | 99 +++++++++++++++++ setup.cfg | 2 +- 9 files changed, 385 insertions(+), 25 deletions(-) create mode 100644 openstackclient/network/common.py create mode 100644 openstackclient/network/v2/security_group.py create mode 100644 openstackclient/tests/network/test_common.py create mode 100644 openstackclient/tests/network/v2/test_security_group.py diff --git a/doc/source/command-objects/security-group-rule.rst b/doc/source/command-objects/security-group-rule.rst index ec03644e9..50bc64aa8 100644 --- a/doc/source/command-objects/security-group-rule.rst +++ b/doc/source/command-objects/security-group-rule.rst @@ -2,7 +2,7 @@ security group rule =================== -Compute v2 +Compute v2, Network v2 security group rule create -------------------------- diff --git a/doc/source/command-objects/security-group.rst b/doc/source/command-objects/security-group.rst index 60de41d82..cf86bda6b 100644 --- a/doc/source/command-objects/security-group.rst +++ b/doc/source/command-objects/security-group.rst @@ -2,7 +2,7 @@ security group ============== -Compute v2 +Compute v2, Network v2 security group create --------------------- diff --git a/openstackclient/compute/v2/security_group.py b/openstackclient/compute/v2/security_group.py index b975a5053..2a8908d75 100644 --- a/openstackclient/compute/v2/security_group.py +++ b/openstackclient/compute/v2/security_group.py @@ -169,28 +169,6 @@ class CreateSecurityGroupRule(command.ShowOne): return zip(*sorted(six.iteritems(info))) -class DeleteSecurityGroup(command.Command): - """Delete a security group""" - - def get_parser(self, prog_name): - parser = super(DeleteSecurityGroup, self).get_parser(prog_name) - parser.add_argument( - 'group', - metavar='', - help='Security group to delete (name or ID)', - ) - return parser - - def take_action(self, parsed_args): - - compute_client = self.app.client_manager.compute - data = utils.find_resource( - compute_client.security_groups, - parsed_args.group, - ) - compute_client.security_groups.delete(data.id) - - class DeleteSecurityGroupRule(command.Command): """Delete a security group rule""" diff --git a/openstackclient/network/common.py b/openstackclient/network/common.py new file mode 100644 index 000000000..c539dd052 --- /dev/null +++ b/openstackclient/network/common.py @@ -0,0 +1,62 @@ +# 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 abc +import six + +from openstackclient.common import command + + +@six.add_metaclass(abc.ABCMeta) +class NetworkAndComputeCommand(command.Command): + """Network and Compute Command""" + + def take_action(self, parsed_args): + if self.app.client_manager.is_network_endpoint_enabled(): + return self.take_action_network(self.app.client_manager.network, + parsed_args) + else: + return self.take_action_compute(self.app.client_manager.compute, + parsed_args) + + def get_parser(self, prog_name): + self.log.debug('get_parser(%s)', prog_name) + parser = super(NetworkAndComputeCommand, self).get_parser(prog_name) + parser = self.update_parser_common(parser) + self.log.debug('common parser: %s', parser) + if self.app.client_manager.is_network_endpoint_enabled(): + return self.update_parser_network(parser) + else: + return self.update_parser_compute(parser) + + def update_parser_common(self, parser): + """Default is no updates to parser.""" + return parser + + def update_parser_network(self, parser): + """Default is no updates to parser.""" + return parser + + def update_parser_compute(self, parser): + """Default is no updates to parser.""" + return parser + + @abc.abstractmethod + def take_action_network(self, client, parsed_args): + """Override to do something useful.""" + pass + + @abc.abstractmethod + def take_action_compute(self, client, parsed_args): + """Override to do something useful.""" + pass diff --git a/openstackclient/network/v2/security_group.py b/openstackclient/network/v2/security_group.py new file mode 100644 index 000000000..4e122f21a --- /dev/null +++ b/openstackclient/network/v2/security_group.py @@ -0,0 +1,40 @@ +# 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. +# + +"""Security Group action implementations""" + +from openstackclient.common import utils +from openstackclient.network import common + + +class DeleteSecurityGroup(common.NetworkAndComputeCommand): + """Delete a security group""" + + def update_parser_common(self, parser): + parser.add_argument( + 'group', + metavar='', + help='Security group to delete (name or ID)', + ) + return parser + + def take_action_network(self, client, parsed_args): + obj = client.find_security_group(parsed_args.group) + client.delete_security_group(obj) + + def take_action_compute(self, client, parsed_args): + data = utils.find_resource( + client.security_groups, + parsed_args.group, + ) + client.security_groups.delete(data.id) diff --git a/openstackclient/tests/network/test_common.py b/openstackclient/tests/network/test_common.py new file mode 100644 index 000000000..a3396b9da --- /dev/null +++ b/openstackclient/tests/network/test_common.py @@ -0,0 +1,103 @@ +# 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 argparse +import mock + +from openstackclient.network import common +from openstackclient.tests import utils + + +class FakeNetworkAndComputeCommand(common.NetworkAndComputeCommand): + def update_parser_common(self, parser): + parser.add_argument( + 'common', + metavar='', + help='Common argument', + ) + return parser + + def update_parser_network(self, parser): + parser.add_argument( + 'network', + metavar='', + help='Network argument', + ) + return parser + + def update_parser_compute(self, parser): + parser.add_argument( + 'compute', + metavar='', + help='Compute argument', + ) + return parser + + def take_action_network(self, client, parsed_args): + client.network_action(parsed_args) + return 'take_action_network' + + def take_action_compute(self, client, parsed_args): + client.compute_action(parsed_args) + return 'take_action_compute' + + +class TestNetworkAndComputeCommand(utils.TestCommand): + def setUp(self): + super(TestNetworkAndComputeCommand, self).setUp() + + self.namespace = argparse.Namespace() + + # Create network client mocks. + self.app.client_manager.network = mock.Mock() + self.network = self.app.client_manager.network + self.network.network_action = mock.Mock(return_value=None) + + # Create compute client mocks. + self.app.client_manager.compute = mock.Mock() + self.compute = self.app.client_manager.compute + self.compute.compute_action = mock.Mock(return_value=None) + + # Get the command object to test + self.cmd = FakeNetworkAndComputeCommand(self.app, self.namespace) + + def test_take_action_network(self): + arglist = [ + 'common', + 'network' + ] + verifylist = [ + ('common', 'common'), + ('network', 'network') + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + self.network.network_action.assert_called_with(parsed_args) + self.assertEqual('take_action_network', result) + + def test_take_action_compute(self): + arglist = [ + 'common', + 'compute' + ] + verifylist = [ + ('common', 'common'), + ('compute', 'compute') + ] + + self.app.client_manager.network_endpoint_enabled = False + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + self.compute.compute_action.assert_called_with(parsed_args) + self.assertEqual('take_action_compute', result) diff --git a/openstackclient/tests/network/v2/fakes.py b/openstackclient/tests/network/v2/fakes.py index 4c862bd33..26a8664af 100644 --- a/openstackclient/tests/network/v2/fakes.py +++ b/openstackclient/tests/network/v2/fakes.py @@ -321,6 +321,84 @@ class FakeRouter(object): return mock.MagicMock(side_effect=routers) +class FakeSecurityGroup(object): + """Fake one or more security groups.""" + + @staticmethod + def create_one_security_group(attrs={}, methods={}): + """Create a fake security group. + + :param Dictionary attrs: + A dictionary with all attributes + :param Dictionary methods: + A dictionary with all methods + :return: + A FakeResource object, with id, name, etc. + """ + # Set default attributes. + security_group_attrs = { + 'id': 'security-group-id-' + uuid.uuid4().hex, + 'name': 'security-group-name-' + uuid.uuid4().hex, + 'description': 'security-group-description-' + uuid.uuid4().hex, + 'tenant_id': 'project-id-' + uuid.uuid4().hex, + 'security_group_rules': [], + } + + # Overwrite default attributes. + security_group_attrs.update(attrs) + + # Set default methods. + security_group_methods = {} + + # Overwrite default methods. + security_group_methods.update(methods) + + security_group = fakes.FakeResource( + info=copy.deepcopy(security_group_attrs), + methods=copy.deepcopy(security_group_methods), + loaded=True) + return security_group + + @staticmethod + def create_security_groups(attrs={}, methods={}, count=2): + """Create multiple fake security groups. + + :param Dictionary attrs: + A dictionary with all attributes + :param Dictionary methods: + A dictionary with all methods + :param int count: + The number of security groups to fake + :return: + A list of FakeResource objects faking the security groups + """ + security_groups = [] + for i in range(0, count): + security_groups.append( + FakeRouter.create_one_security_group(attrs, methods)) + + return security_groups + + @staticmethod + def get_security_groups(security_groups=None, count=2): + """Get an iterable MagicMock object with a list of faked security groups. + + If security group list is provided, then initialize the Mock object + with the list. Otherwise create one. + + :param List security groups: + A list of FakeResource objects faking security groups + :param int count: + The number of security groups to fake + :return: + An iterable Mock object with side_effect set to a list of faked + security groups + """ + if security_groups is None: + security_groups = FakeRouter.create_security_groups(count) + return mock.MagicMock(side_effect=security_groups) + + class FakeSubnet(object): """Fake one or more subnets.""" diff --git a/openstackclient/tests/network/v2/test_security_group.py b/openstackclient/tests/network/v2/test_security_group.py new file mode 100644 index 000000000..98388ec7c --- /dev/null +++ b/openstackclient/tests/network/v2/test_security_group.py @@ -0,0 +1,99 @@ +# 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 mock + +from openstackclient.network.v2 import security_group +from openstackclient.tests.network.v2 import fakes as network_fakes + + +class TestSecurityGroup(network_fakes.TestNetworkV2): + + def setUp(self): + super(TestSecurityGroup, self).setUp() + + # Get a shortcut to the network client + self.network = self.app.client_manager.network + + # Create compute client mocks. + self.app.client_manager.compute = mock.Mock() + self.compute = self.app.client_manager.compute + self.compute.security_groups = mock.Mock() + + +class TestDeleteSecurityGroupNetwork(TestSecurityGroup): + + # The security group to be deleted. + _security_group = \ + network_fakes.FakeSecurityGroup.create_one_security_group() + + def setUp(self): + super(TestDeleteSecurityGroupNetwork, self).setUp() + + self.network.delete_security_group = mock.Mock(return_value=None) + + self.network.find_security_group = mock.Mock( + return_value=self._security_group) + + # Get the command object to test + self.cmd = security_group.DeleteSecurityGroup(self.app, self.namespace) + + def test_security_group_delete(self): + arglist = [ + self._security_group.name, + ] + verifylist = [ + ('group', self._security_group.name), + ] + + 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._security_group) + self.assertEqual(None, result) + + +class TestDeleteSecurityGroupCompute(TestSecurityGroup): + + # The security group to be deleted. + _security_group = \ + network_fakes.FakeSecurityGroup.create_one_security_group() + + def setUp(self): + super(TestDeleteSecurityGroupCompute, self).setUp() + + self.app.client_manager.network_endpoint_enabled = False + + self.compute.security_groups.delete = mock.Mock(return_value=None) + + self.compute.security_groups.get = mock.Mock( + return_value=self._security_group) + + # Get the command object to test + self.cmd = security_group.DeleteSecurityGroup(self.app, self.namespace) + + def test_security_group_delete(self): + arglist = [ + self._security_group.name, + ] + verifylist = [ + ('group', self._security_group.name), + ] + + 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._security_group.id) + self.assertEqual(None, result) diff --git a/setup.cfg b/setup.cfg index 2751eb317..8e1e3e5ab 100644 --- a/setup.cfg +++ b/setup.cfg @@ -103,7 +103,6 @@ openstack.compute.v2 = keypair_show = openstackclient.compute.v2.keypair:ShowKeypair security_group_create = openstackclient.compute.v2.security_group:CreateSecurityGroup - security_group_delete = openstackclient.compute.v2.security_group:DeleteSecurityGroup security_group_list = openstackclient.compute.v2.security_group:ListSecurityGroup security_group_set = openstackclient.compute.v2.security_group:SetSecurityGroup security_group_show = openstackclient.compute.v2.security_group:ShowSecurityGroup @@ -338,6 +337,7 @@ openstack.network.v2 = router_list = openstackclient.network.v2.router:ListRouter router_set = openstackclient.network.v2.router:SetRouter router_show = openstackclient.network.v2.router:ShowRouter + security_group_delete = openstackclient.network.v2.security_group:DeleteSecurityGroup subnet_list = openstackclient.network.v2.subnet:ListSubnet openstack.object_store.v1 =