diff --git a/openstackclient/compute/v2/security_group.py b/openstackclient/compute/v2/security_group.py index f378af14cb..042e5caf3d 100644 --- a/openstackclient/compute/v2/security_group.py +++ b/openstackclient/compute/v2/security_group.py @@ -56,23 +56,6 @@ def _xform_security_group_rule(sgroup): return info -def _xform_and_trim_security_group_rule(sgroup): - info = _xform_security_group_rule(sgroup) - # Trim parent security group ID since caller has this information. - info.pop('parent_group_id', None) - # Trim keys with empty string values. - keys_to_trim = [ - 'ip_protocol', - 'ip_range', - 'port_range', - 'remote_security_group', - ] - for key in keys_to_trim: - if key in info and not info[key]: - info.pop(key) - return info - - class CreateSecurityGroup(command.ShowOne): """Create a new security group""" @@ -215,40 +198,3 @@ class ListSecurityGroupRule(command.Lister): (utils.get_item_properties( s, columns, ) for s in rules)) - - -class ShowSecurityGroup(command.ShowOne): - """Display security group details""" - - def get_parser(self, prog_name): - parser = super(ShowSecurityGroup, self).get_parser(prog_name) - parser.add_argument( - 'group', - metavar='', - help='Security group to display (name or ID)', - ) - return parser - - def take_action(self, parsed_args): - - compute_client = self.app.client_manager.compute - info = {} - info.update(utils.find_resource( - compute_client.security_groups, - parsed_args.group, - )._info) - rules = [] - for r in info['rules']: - formatted_rule = _xform_and_trim_security_group_rule(r) - rules.append(utils.format_dict(formatted_rule)) - - # Format rules into a list of strings - info.update( - {'rules': utils.format_list(rules, separator='\n')} - ) - # Map 'tenant_id' column to 'project_id' - info.update( - {'project_id': info.pop('tenant_id')} - ) - - return zip(*sorted(six.iteritems(info))) diff --git a/openstackclient/network/utils.py b/openstackclient/network/utils.py new file mode 100644 index 0000000000..287f027163 --- /dev/null +++ b/openstackclient/network/utils.py @@ -0,0 +1,41 @@ +# 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. +# + + +# Transform compute security group rule for display. +def transform_compute_security_group_rule(sg_rule): + info = {} + info.update(sg_rule) + from_port = info.pop('from_port') + to_port = info.pop('to_port') + if isinstance(from_port, int) and isinstance(to_port, int): + port_range = {'port_range': "%u:%u" % (from_port, to_port)} + elif from_port is None and to_port is None: + port_range = {'port_range': ""} + else: + port_range = {'port_range': "%s:%s" % (from_port, to_port)} + info.update(port_range) + if 'cidr' in info['ip_range']: + info['ip_range'] = info['ip_range']['cidr'] + else: + info['ip_range'] = '' + if info['ip_protocol'] is None: + info['ip_protocol'] = '' + elif info['ip_protocol'].lower() == 'icmp': + info['port_range'] = '' + group = info.pop('group') + if 'name' in group: + info['remote_security_group'] = group['name'] + else: + info['remote_security_group'] = '' + return info diff --git a/openstackclient/network/v2/security_group.py b/openstackclient/network/v2/security_group.py index 62699ffd3d..64717945f3 100644 --- a/openstackclient/network/v2/security_group.py +++ b/openstackclient/network/v2/security_group.py @@ -14,9 +14,81 @@ """Security Group action implementations""" import argparse +import six from openstackclient.common import utils from openstackclient.network import common +from openstackclient.network import utils as network_utils + + +def _format_network_security_group_rules(sg_rules): + # For readability and to align with formatting compute security group + # rules, trim keys with caller known (e.g. security group and tenant ID) + # or empty values. + for sg_rule in sg_rules: + empty_keys = [k for k, v in six.iteritems(sg_rule) if not v] + for key in empty_keys: + sg_rule.pop(key) + sg_rule.pop('security_group_id', None) + sg_rule.pop('tenant_id', None) + return utils.format_list_of_dicts(sg_rules) + + +def _format_compute_security_group_rule(sg_rule): + info = network_utils.transform_compute_security_group_rule(sg_rule) + # Trim parent security group ID since caller has this information. + info.pop('parent_group_id', None) + # Trim keys with empty string values. + keys_to_trim = [ + 'ip_protocol', + 'ip_range', + 'port_range', + 'remote_security_group', + ] + for key in keys_to_trim: + if key in info and not info[key]: + info.pop(key) + return utils.format_dict(info) + + +def _format_compute_security_group_rules(sg_rules): + rules = [] + for sg_rule in sg_rules: + rules.append(_format_compute_security_group_rule(sg_rule)) + return utils.format_list(rules, separator='\n') + + +_formatters_network = { + 'security_group_rules': _format_network_security_group_rules, +} + + +_formatters_compute = { + 'rules': _format_compute_security_group_rules, +} + + +def _get_columns(item): + # Build the display columns and a list of the property columns + # that need to be mapped (display column name, property name). + columns = list(item.keys()) + property_column_mappings = [] + if 'security_group_rules' in columns: + columns.append('rules') + columns.remove('security_group_rules') + property_column_mappings.append(('rules', 'security_group_rules')) + if 'tenant_id' in columns: + columns.append('project_id') + columns.remove('tenant_id') + property_column_mappings.append(('project_id', 'tenant_id')) + display_columns = sorted(columns) + + # Build the property columns and apply any column mappings. + property_columns = sorted(columns) + for property_column_mapping in property_column_mappings: + property_index = property_columns.index(property_column_mapping[0]) + property_columns[property_index] = property_column_mapping[1] + return tuple(display_columns), property_columns class DeleteSecurityGroup(common.NetworkAndComputeCommand): @@ -143,3 +215,39 @@ class SetSecurityGroup(common.NetworkAndComputeCommand): data.name, data.description, ) + + +class ShowSecurityGroup(common.NetworkAndComputeShowOne): + """Display security group details""" + + def update_parser_common(self, parser): + parser.add_argument( + 'group', + metavar='', + help='Security group to display (name or ID)', + ) + return parser + + def take_action_network(self, client, parsed_args): + obj = client.find_security_group(parsed_args.group, + ignore_missing=False) + 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): + obj = utils.find_resource( + client.security_groups, + parsed_args.group, + ) + display_columns, property_columns = _get_columns(obj._info) + data = utils.get_dict_properties( + obj._info, + property_columns, + formatters=_formatters_compute + ) + return (display_columns, data) diff --git a/openstackclient/network/v2/security_group_rule.py b/openstackclient/network/v2/security_group_rule.py index a61e3233df..92f28cce77 100644 --- a/openstackclient/network/v2/security_group_rule.py +++ b/openstackclient/network/v2/security_group_rule.py @@ -18,38 +18,11 @@ import six from openstackclient.common import exceptions from openstackclient.common import utils from openstackclient.network import common - - -def _xform_security_group_rule(sgroup): - info = {} - info.update(sgroup) - from_port = info.pop('from_port') - to_port = info.pop('to_port') - if isinstance(from_port, int) and isinstance(to_port, int): - port_range = {'port_range': "%u:%u" % (from_port, to_port)} - elif from_port is None and to_port is None: - port_range = {'port_range': ""} - else: - port_range = {'port_range': "%s:%s" % (from_port, to_port)} - info.update(port_range) - if 'cidr' in info['ip_range']: - info['ip_range'] = info['ip_range']['cidr'] - else: - info['ip_range'] = '' - if info['ip_protocol'] is None: - info['ip_protocol'] = '' - elif info['ip_protocol'].lower() == 'icmp': - info['port_range'] = '' - group = info.pop('group') - if 'name' in group: - info['remote_security_group'] = group['name'] - else: - info['remote_security_group'] = '' - return info +from openstackclient.network import utils as network_utils def _format_security_group_rule_show(obj): - data = _xform_security_group_rule(obj) + data = network_utils.transform_compute_security_group_rule(obj) return zip(*sorted(six.iteritems(data))) diff --git a/openstackclient/tests/compute/v2/fakes.py b/openstackclient/tests/compute/v2/fakes.py index 11d9ff1b46..26d3a28309 100644 --- a/openstackclient/tests/compute/v2/fakes.py +++ b/openstackclient/tests/compute/v2/fakes.py @@ -333,7 +333,9 @@ class FakeSecurityGroup(object): security_group_attrs.update(attrs) # Set default methods. - security_group_methods = {} + security_group_methods = { + 'keys': ['id', 'name', 'description', 'tenant_id', 'rules'], + } # Overwrite default methods. security_group_methods.update(methods) @@ -369,7 +371,7 @@ class FakeSecurityGroupRule(object): """Fake one or more security group rules.""" @staticmethod - def create_one_security_group_rule(attrs={}, methods={}): + def create_one_security_group_rule(attrs=None, methods=None): """Create a fake security group rule. :param Dictionary attrs: @@ -379,6 +381,11 @@ class FakeSecurityGroupRule(object): :return: A FakeResource object, with id, etc. """ + if attrs is None: + attrs = {} + if methods is None: + methods = {} + # Set default attributes. security_group_rule_attrs = { 'from_port': -1, @@ -406,7 +413,7 @@ class FakeSecurityGroupRule(object): return security_group_rule @staticmethod - def create_security_group_rules(attrs={}, methods={}, count=2): + def create_security_group_rules(attrs=None, methods=None, count=2): """Create multiple fake security group rules. :param Dictionary attrs: diff --git a/openstackclient/tests/network/v2/fakes.py b/openstackclient/tests/network/v2/fakes.py index 9e6bf97f65..6cb7e997da 100644 --- a/openstackclient/tests/network/v2/fakes.py +++ b/openstackclient/tests/network/v2/fakes.py @@ -419,7 +419,7 @@ class FakeSecurityGroup(object): """Fake one or more security groups.""" @staticmethod - def create_one_security_group(attrs={}, methods={}): + def create_one_security_group(attrs=None, methods=None): """Create a fake security group. :param Dictionary attrs: @@ -429,6 +429,11 @@ class FakeSecurityGroup(object): :return: A FakeResource object, with id, name, etc. """ + if attrs is None: + attrs = {} + if methods is None: + methods = {} + # Set default attributes. security_group_attrs = { 'id': 'security-group-id-' + uuid.uuid4().hex, @@ -442,7 +447,10 @@ class FakeSecurityGroup(object): security_group_attrs.update(attrs) # Set default methods. - security_group_methods = {} + security_group_methods = { + 'keys': ['id', 'name', 'description', 'tenant_id', + 'security_group_rules'], + } # Overwrite default methods. security_group_methods.update(methods) @@ -451,10 +459,14 @@ class FakeSecurityGroup(object): info=copy.deepcopy(security_group_attrs), methods=copy.deepcopy(security_group_methods), loaded=True) + + # Set attributes with special mapping in OpenStack SDK. + security_group.project_id = security_group_attrs['tenant_id'] + return security_group @staticmethod - def create_security_groups(attrs={}, methods={}, count=2): + def create_security_groups(attrs=None, methods=None, count=2): """Create multiple fake security groups. :param Dictionary attrs: @@ -478,7 +490,7 @@ class FakeSecurityGroupRule(object): """Fake one or more security group rules.""" @staticmethod - def create_one_security_group_rule(attrs={}, methods={}): + def create_one_security_group_rule(attrs=None, methods=None): """Create a fake security group rule. :param Dictionary attrs: @@ -488,6 +500,11 @@ class FakeSecurityGroupRule(object): :return: A FakeResource object, with id, etc. """ + if attrs is None: + attrs = {} + if methods is None: + methods = {} + # Set default attributes. security_group_rule_attrs = { 'direction': 'ingress', @@ -520,13 +537,13 @@ class FakeSecurityGroupRule(object): methods=copy.deepcopy(security_group_rule_methods), loaded=True) - # Set attributes with special mappings. + # Set attributes with special mapping in OpenStack SDK. security_group_rule.project_id = security_group_rule_attrs['tenant_id'] return security_group_rule @staticmethod - def create_security_group_rules(attrs={}, methods={}, count=2): + def create_security_group_rules(attrs=None, methods=None, count=2): """Create multiple fake security group rules. :param Dictionary attrs: diff --git a/openstackclient/tests/network/v2/test_security_group.py b/openstackclient/tests/network/v2/test_security_group.py index b8114cbcf0..a66b7b0061 100644 --- a/openstackclient/tests/network/v2/test_security_group.py +++ b/openstackclient/tests/network/v2/test_security_group.py @@ -363,3 +363,122 @@ class TestSetSecurityGroupCompute(TestSecurityGroupCompute): new_description ) self.assertIsNone(result) + + +class TestShowSecurityGroupNetwork(TestSecurityGroupNetwork): + + # The security group rule to be shown with the group. + _security_group_rule = \ + network_fakes.FakeSecurityGroupRule.create_one_security_group_rule() + + # The security group to be shown. + _security_group = \ + network_fakes.FakeSecurityGroup.create_one_security_group( + attrs={'security_group_rules': [_security_group_rule._info]} + ) + + columns = ( + 'description', + 'id', + 'name', + 'project_id', + 'rules', + ) + + data = ( + _security_group.description, + _security_group.id, + _security_group.name, + _security_group.project_id, + security_group._format_network_security_group_rules( + [_security_group_rule._info]), + ) + + def setUp(self): + super(TestShowSecurityGroupNetwork, self).setUp() + + self.network.find_security_group = mock.Mock( + return_value=self._security_group) + + # Get the command object to test + self.cmd = security_group.ShowSecurityGroup(self.app, self.namespace) + + def test_show_no_options(self): + self.assertRaises(tests_utils.ParserException, + self.check_parser, self.cmd, [], []) + + def test_show_all_options(self): + arglist = [ + self._security_group.id, + ] + verifylist = [ + ('group', self._security_group.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.network.find_security_group.assert_called_once_with( + self._security_group.id, ignore_missing=False) + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) + + +class TestShowSecurityGroupCompute(TestSecurityGroupCompute): + + # The security group rule to be shown with the group. + _security_group_rule = \ + compute_fakes.FakeSecurityGroupRule.create_one_security_group_rule() + + # The security group to be shown. + _security_group = \ + compute_fakes.FakeSecurityGroup.create_one_security_group( + attrs={'rules': [_security_group_rule._info]} + ) + + columns = ( + 'description', + 'id', + 'name', + 'project_id', + 'rules', + ) + + data = ( + _security_group.description, + _security_group.id, + _security_group.name, + _security_group.tenant_id, + security_group._format_compute_security_group_rules( + [_security_group_rule._info]), + ) + + def setUp(self): + super(TestShowSecurityGroupCompute, self).setUp() + + self.app.client_manager.network_endpoint_enabled = False + + self.compute.security_groups.get.return_value = self._security_group + + # Get the command object to test + self.cmd = security_group.ShowSecurityGroup(self.app, None) + + def test_show_no_options(self): + self.assertRaises(tests_utils.ParserException, + self.check_parser, self.cmd, [], []) + + def test_show_all_options(self): + arglist = [ + self._security_group.id, + ] + verifylist = [ + ('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_groups.get.assert_called_once_with( + self._security_group.id) + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) diff --git a/releasenotes/notes/bug-1519511-68ca30ad5519d3d8.yaml b/releasenotes/notes/bug-1519511-68ca30ad5519d3d8.yaml new file mode 100644 index 0000000000..d69244e6c2 --- /dev/null +++ b/releasenotes/notes/bug-1519511-68ca30ad5519d3d8.yaml @@ -0,0 +1,6 @@ +--- +features: + - The ``security group show`` command now uses Network v2 when + enabled which results in a more detailed output for network + security group rules. + [Bug `1519511 `_] diff --git a/setup.cfg b/setup.cfg index 981d2fef49..4809cd2283 100644 --- a/setup.cfg +++ b/setup.cfg @@ -100,7 +100,6 @@ openstack.compute.v2 = keypair_show = openstackclient.compute.v2.keypair:ShowKeypair security_group_create = openstackclient.compute.v2.security_group:CreateSecurityGroup - security_group_show = openstackclient.compute.v2.security_group:ShowSecurityGroup security_group_rule_create = openstackclient.compute.v2.security_group:CreateSecurityGroupRule security_group_rule_list = openstackclient.compute.v2.security_group:ListSecurityGroupRule @@ -348,6 +347,7 @@ openstack.network.v2 = 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 + security_group_show = openstackclient.network.v2.security_group:ShowSecurityGroup security_group_rule_delete = openstackclient.network.v2.security_group_rule:DeleteSecurityGroupRule security_group_rule_show = openstackclient.network.v2.security_group_rule:ShowSecurityGroupRule