From cdb191ec83f86dffade56be07ca53077d7c78b14 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Mon, 19 Aug 2019 16:35:42 +0900 Subject: [PATCH] Fix listing security groups when no rules When listing security groups in the dashboard and one or more security groups had no rules it failed because python throws a KeyError. This commit changes the neutron API wrapper in horizon to ensure ensure rule information in SG always exists. Closes-Bug: #1840465 Co-Authored-By: Tobias Urdin Change-Id: I6e05a7dc6b6655514ee2bff6bd327da86f13900a --- openstack_dashboard/api/neutron.py | 2 ++ openstack_dashboard/test/test_data/neutron_data.py | 12 +++++++++--- openstack_dashboard/test/unit/api/test_neutron.py | 4 +++- ...-group-no-rules-list-bugfix-b77ab5aff1d3e45e.yaml | 5 +++++ 4 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/security-group-no-rules-list-bugfix-b77ab5aff1d3e45e.yaml diff --git a/openstack_dashboard/api/neutron.py b/openstack_dashboard/api/neutron.py index 7cdb0e16bc..186447eec9 100644 --- a/openstack_dashboard/api/neutron.py +++ b/openstack_dashboard/api/neutron.py @@ -235,6 +235,8 @@ class SecurityGroup(NeutronAPIDictWrapper): def __init__(self, sg, sg_dict=None): if sg_dict is None: sg_dict = {sg['id']: sg['name']} + if 'security_group_rules' not in sg: + sg['security_group_rules'] = [] sg['rules'] = [SecurityGroupRule(rule, sg_dict) for rule in sg['security_group_rules']] super(SecurityGroup, self).__init__(sg) diff --git a/openstack_dashboard/test/test_data/neutron_data.py b/openstack_dashboard/test/test_data/neutron_data.py index f558174b3c..5c2fc0d2f8 100644 --- a/openstack_dashboard/test/test_data/neutron_data.py +++ b/openstack_dashboard/test/test_data/neutron_data.py @@ -498,6 +498,10 @@ def data(TEST): 'description': 'NotDefault', 'id': '443a4d7a-4bd2-4474-9a77-02b35c9f8c95', 'name': 'another_group'} + sec_group_empty = {'tenant_id': '1', + 'description': 'SG without rules', + 'id': 'f205f3bc-d402-4e40-b004-c62401e19b4b', + 'name': 'empty_group'} def add_rule_to_group(secgroup, default_only=True): rule_egress_ipv4 = { @@ -581,18 +585,20 @@ def data(TEST): add_rule_to_group(sec_group_1, default_only=False) add_rule_to_group(sec_group_2) add_rule_to_group(sec_group_3) + # NOTE: sec_group_empty is a SG without rules, + # so we don't call add_rule_to_group. - groups = [sec_group_1, sec_group_2, sec_group_3] + groups = [sec_group_1, sec_group_2, sec_group_3, sec_group_empty] sg_name_dict = dict([(sg['id'], sg['name']) for sg in groups]) for sg in groups: # Neutron API. TEST.api_security_groups.add(sg) - for rule in sg['security_group_rules']: + for rule in sg.get('security_group_rules', []): TEST.api_security_group_rules.add(copy.copy(rule)) # OpenStack Dashboard internaly API. TEST.security_groups.add( neutron.SecurityGroup(copy.deepcopy(sg), sg_name_dict)) - for rule in sg['security_group_rules']: + for rule in sg.get('security_group_rules', []): TEST.security_group_rules.add( neutron.SecurityGroupRule(copy.copy(rule), sg_name_dict)) diff --git a/openstack_dashboard/test/unit/api/test_neutron.py b/openstack_dashboard/test/unit/api/test_neutron.py index 75672abcb5..5b122a479f 100644 --- a/openstack_dashboard/test/unit/api/test_neutron.py +++ b/openstack_dashboard/test/unit/api/test_neutron.py @@ -1150,7 +1150,9 @@ class NeutronApiSecurityGroupTests(test.APIMockTestCase): def _cmp_sg(self, exp_sg, ret_sg): self.assertEqual(exp_sg['id'], ret_sg.id) self.assertEqual(exp_sg['name'], ret_sg.name) - exp_rules = exp_sg['security_group_rules'] + # When a SG has no rules, neutron API does not contain + # 'security_group_rules' field, so .get() method needs to be used. + exp_rules = exp_sg.get('security_group_rules', []) self.assertEqual(len(exp_rules), len(ret_sg.rules)) for (exprule, retrule) in six.moves.zip(exp_rules, ret_sg.rules): self._cmp_sg_rule(exprule, retrule) diff --git a/releasenotes/notes/security-group-no-rules-list-bugfix-b77ab5aff1d3e45e.yaml b/releasenotes/notes/security-group-no-rules-list-bugfix-b77ab5aff1d3e45e.yaml new file mode 100644 index 0000000000..b98292cf71 --- /dev/null +++ b/releasenotes/notes/security-group-no-rules-list-bugfix-b77ab5aff1d3e45e.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + [:bug:`1840465`] Fixed a bug where listing security groups did not work + if one or more security groups had no rules in them.