[SRU] Fails to list security groups if one or more exists without rules

Bug #1840465 reported by Tobias Urdin
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
OpenStack Dashboard (Horizon)
Medium
Akihiro Motoki
Ubuntu Cloud Archive
Medium
Unassigned
Queens
Medium
Unassigned
Rocky
Medium
Unassigned
Stein
Medium
Unassigned
Train
Medium
Unassigned
horizon (Ubuntu)
Medium
Unassigned
Bionic
Medium
Unassigned
Disco
Medium
Unassigned
Eoan
Medium
Unassigned

Bug Description

Horizon 14.0.2 (rocky)
If a security group without any rules exists the listing of security groups fails with a KeyError.

Traceback (most recent call last):
  File "/usr/share/openstack-dashboard/openstack_dashboard/api/rest/utils.py", line 127, in _wrapped
    data = function(self, request, *args, **kw)
  File "/usr/share/openstack-dashboard/openstack_dashboard/api/rest/network.py", line 44, in get
    security_groups = api.neutron.security_group_list(request)
  File "/usr/lib/python2.7/site-packages/horizon/utils/memoized.py", line 95, in wrapped
    value = cache[key] = func(*args, **kwargs)
  File "/usr/share/openstack-dashboard/openstack_dashboard/api/neutron.py", line 1641, in security_group_list
    return SecurityGroupManager(request).list(**params)
  File "/usr/share/openstack-dashboard/openstack_dashboard/api/neutron.py", line 372, in list
    return self._list(**params)
  File "/usr/share/openstack-dashboard/openstack_dashboard/api/neutron.py", line 359, in _list
    return [SecurityGroup(sg) for sg in secgroups.get('security_groups')]
  File "/usr/share/openstack-dashboard/openstack_dashboard/api/neutron.py", line 240, in __init__
    for rule in sg['security_group_rules']]
KeyError: 'security_group_rules'

=======================================================================

[Impact]

By default, new security groups created through horizon or CLI include 2 default security rules. Upon managing those rules and removing them (to perhaps add others or limit traffic completely), the security group page errors out and prevents listing of *all* security groups if the empty security group is within the list to be displayed. Therefore, not only is the empty security group affected, but all others as well, as they cannot be listed. The root cause of the bug is that the payload does not include the expected key "security_group_rules" for that security group when there are no rules.

A fix has been implemented for Train (from master), Stein, Rocky and Queens releases and should be backported so the issue is addressed on previous those releases. The fix prevents the crash by ensuring the key "security_group_rules" is present with an empty list in case it was not included in the payload.

[Test Case]

1. Reproducing the issue

1a. Go to the Security Group section at Project > Network > Security Groups
1b. Create a security group
1c. Click the Manage Rules button for that security group you just created
1d. Delete the two default rules
1e. Go back to the Security Group section at Project > Network > Security Groups
1f. Security groups are no longer being listed and there will be an error popup: "Error: Unable to retrieve security groups.".

2. Install the package with the fixed code

3. Confirm bug has been fixed

3a. Repeat step 1a, you will notice that now security groups can be listed, including the empty one you had previously created

3b. Repeat steps 1b through 1e, the newly created and emptied security group can also be listed along the others.

[Regression Potential]

Given the following indicators:

a. Upstream CI passed, for all releases the fix is being backported.
b. The small scope of the problem: a key error prevents the page from being rendered. The fix is to make sure the key is always present, since it is always expected.
c. The fix is very simple, and during tests we can see that the other pages handle an empty list properly as the value for the "security_group_rules" key.

We can safely state that the regression potential is negligible.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (master)

Fix proposed to branch: master
Review: https://review.opendev.org/676950

Changed in horizon:
assignee: nobody → Tobias Urdin (tobias-urdin)
status: New → In Progress
Akihiro Motoki (amotoki)
tags: added: neutron
Changed in horizon:
importance: Undecided → Medium
tags: added: stein-backport-potential
Changed in horizon:
assignee: Tobias Urdin (tobias-urdin) → Akihiro Motoki (amotoki)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (master)

Reviewed: https://review.opendev.org/676950
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=cdb191ec83f86dffade56be07ca53077d7c78b14
Submitter: Zuul
Branch: master

commit cdb191ec83f86dffade56be07ca53077d7c78b14
Author: Akihiro Motoki <email address hidden>
Date: Mon Aug 19 16:35:42 2019 +0900

    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 <email address hidden>
    Change-Id: I6e05a7dc6b6655514ee2bff6bd327da86f13900a

Changed in horizon:
status: In Progress → Fix Released
Revision history for this message
Tobias Urdin (tobias-urdin) wrote : Re: Fails to list security groups if one or more exists without rules

Can we backport back to Rocky if possible?

tags: added: rocky-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (stable/stein)

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/677399

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (stable/rocky)

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/677400

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (stable/stein)

Reviewed: https://review.opendev.org/677399
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=5cf44f0e1282a042625dd02b3a053a8545529e6e
Submitter: Zuul
Branch: stable/stein

commit 5cf44f0e1282a042625dd02b3a053a8545529e6e
Author: Akihiro Motoki <email address hidden>
Date: Mon Aug 19 16:35:42 2019 +0900

    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 <email address hidden>
    Change-Id: I6e05a7dc6b6655514ee2bff6bd327da86f13900a
    (cherry picked from commit cdb191ec83f86dffade56be07ca53077d7c78b14)

tags: added: in-stable-stein
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/horizon 16.0.0.0b2

This issue was fixed in the openstack/horizon 16.0.0.0b2 development milestone.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (stable/rocky)

Reviewed: https://review.opendev.org/677400
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=205c4465cdf8112779e8eb5f1618dcfc1b11e305
Submitter: Zuul
Branch: stable/rocky

commit 205c4465cdf8112779e8eb5f1618dcfc1b11e305
Author: Akihiro Motoki <email address hidden>
Date: Mon Aug 19 16:35:42 2019 +0900

    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 <email address hidden>
    Change-Id: I6e05a7dc6b6655514ee2bff6bd327da86f13900a
    (cherry picked from commit cdb191ec83f86dffade56be07ca53077d7c78b14)

tags: added: in-stable-rocky
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (stable/queens)

Fix proposed to branch: stable/queens
Review: https://review.opendev.org/686485

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (stable/queens)

Reviewed: https://review.opendev.org/686485
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=9d73ebee7a1ce442f57fad319d6ca6b354ffe171
Submitter: Zuul
Branch: stable/queens

commit 9d73ebee7a1ce442f57fad319d6ca6b354ffe171
Author: Akihiro Motoki <email address hidden>
Date: Mon Aug 19 16:35:42 2019 +0900

    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 <email address hidden>
    Change-Id: I6e05a7dc6b6655514ee2bff6bd327da86f13900a
    (cherry picked from commit cdb191ec83f86dffade56be07ca53077d7c78b14)
    (cherry picked from commit 205c4465cdf8112779e8eb5f1618dcfc1b11e305)

tags: added: in-stable-queens
summary: - Fails to list security groups if one or more exists without rules
+ [SRU] Fails to list security groups if one or more exists without rules
Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :
description: updated
description: updated
description: updated
Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :
Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :
Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :

Attached debdiffs for disco, cosmic and bionic.

Changed in horizon (Ubuntu Bionic):
status: New → In Progress
Changed in horizon (Ubuntu Disco):
status: New → In Progress
Changed in horizon (Ubuntu Eoan):
status: New → In Progress
tags: added: sts-sru-needed
Changed in horizon (Ubuntu Eoan):
status: In Progress → Fix Released
importance: Undecided → Medium
Changed in horizon (Ubuntu Disco):
importance: Undecided → Medium
Changed in horizon (Ubuntu Bionic):
importance: Undecided → Medium
Changed in horizon (Ubuntu):
importance: Undecided → Medium
status: New → Fix Released
Revision history for this message
Corey Bryant (corey.bryant) wrote : Please test proposed package

Hello Tobias, or anyone else affected,

Accepted horizon into rocky-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:rocky-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-rocky-needed to verification-rocky-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-rocky-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-rocky-needed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/horizon 13.0.3

This issue was fixed in the openstack/horizon 13.0.3 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/horizon 14.0.4

This issue was fixed in the openstack/horizon 14.0.4 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/horizon 15.1.1

This issue was fixed in the openstack/horizon 15.1.1 release.

Akihiro Motoki (amotoki)
tags: removed: rocky-backport-potential stein-backport-potential
Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :

While performing validation, I was unable to reproduce the issue in bionic-rocky deployment. Inspecting the code, the fix was not present. I added logging, and confirmed that neutron is now including the key "security_group_rules" in the response, therefore the error is no longer triggered. I added logging to confirm that it was indeed the case, and got:

[Tue Nov 12 22:32:51.766942 2019] [wsgi:error] [pid 26153:tid 139930429884160] [remote 10.5.0.43:40358] sg: {'id': '40ca8baa-a3a5-412d-b955-7f9fb3e86e26', 'name': 'test3', 'tenant_id': 'a3ef2c1a2a8e4b628f592e0f87ef3c11', 'description': '', 'security_group_rules': [], 'tags': [], 'created_at': '2019-11-12T22:11:56Z', 'updated_at': '2019-11-12T22:12:03Z', 'revision_number': 3, 'project_id': 'a3ef2c1a2a8e4b628f592e0f87ef3c11'}

I also re-tested this with a limited user account, same results.

I proceeded to upgrade the package, confirmed the code has been changed and includes that fix. Ran the test case and saw that it does not cause any problems. Added additional logging to confirm the new condition (the fix) is not invoked as it is not necessary and indeed it was not being invoked.

I briefly checked neutron, python-neutronclient and horizon code, but I haven't been able to track down the change that caused the key 'security_group_rules' to no longer be missing.

I proceeded to reproduce the problem in xenial-queens to re-confirm the test case, and I couldn't.

Therefore my conclusion is that the test case needs to be improved, as it seems to be missing details that are required to reproduce the problem.

Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :

I tried to reproduce again in a different deployment unsuccessfully. I don't know how (and if) it differs from the previous instance I was able to reproduce it.

Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :

Since the system behaves as expected with the updated package installed, passing the test case, I will mark it as verified.

tags: added: verification-rocky-done
removed: verification-rocky-needed
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Please test proposed package

Hello Tobias, or anyone else affected,

Accepted horizon into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/horizon/3:15.1.0-0ubuntu1.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in horizon (Ubuntu Disco):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-disco
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Hello Tobias, or anyone else affected,

Accepted horizon into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/horizon/3:13.0.2-0ubuntu2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in horizon (Ubuntu Bionic):
status: In Progress → Fix Committed
tags: added: verification-needed-bionic
Revision history for this message
Corey Bryant (corey.bryant) wrote :

Hello Tobias, or anyone else affected,

Accepted horizon into queens-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:queens-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-queens-needed to verification-queens-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-queens-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-queens-needed
Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :

verified on bionic-queens

tags: added: verification-done-bionic verification-queens-done
removed: verification-needed-bionic verification-queens-needed
Revision history for this message
Rodrigo Barbieri (rodrigo-barbieri2010) wrote :

verified on disco-stein

tags: added: verification-done-disco
removed: verification-needed-disco
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package horizon - 3:15.1.0-0ubuntu1.2

---------------
horizon (3:15.1.0-0ubuntu1.2) disco; urgency=medium

  * d/p/lp1840465.patch: Fix failing to list security groups
    with no rules (LP: #1840465).

 -- Rodrigo Barbieri <email address hidden> Fri, 18 Oct 2019 19:13:41 +0000

Changed in horizon (Ubuntu Disco):
status: Fix Committed → Fix Released
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for horizon has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package horizon - 3:13.0.2-0ubuntu2

---------------
horizon (3:13.0.2-0ubuntu2) bionic; urgency=medium

  * d/p/lp1840465.patch: Fix failing to list security groups
    with no rules (LP: #1840465).

 -- Rodrigo Barbieri <email address hidden> Fri, 18 Oct 2019 20:27:28 +0000

Changed in horizon (Ubuntu Bionic):
status: Fix Committed → Fix Released
Revision history for this message
Corey Bryant (corey.bryant) wrote : Please test proposed package

Hello Tobias, or anyone else affected,

Accepted horizon into stein-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:stein-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-stein-needed to verification-stein-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-stein-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-stein-needed
Revision history for this message
Hemanth Nakkina (hemanth-n) wrote :

The test case mentioned in the bug description have been verified on bionic-stein

tags: added: verification-stein-done
removed: verification-stein-needed
tags: added: verification-done
removed: verification-needed
Revision history for this message
Corey Bryant (corey.bryant) wrote : Update Released

The verification of the Stable Release Update for horizon has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Corey Bryant (corey.bryant) wrote :

This bug was fixed in the package horizon - 3:15.1.0-0ubuntu1.2~cloud0
---------------

 horizon (3:15.1.0-0ubuntu1.2~cloud0) bionic-stein; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 horizon (3:15.1.0-0ubuntu1.2) disco; urgency=medium
 .
   * d/p/lp1840465.patch: Fix failing to list security groups
     with no rules (LP: #1840465).

Revision history for this message
Corey Bryant (corey.bryant) wrote :

The verification of the Stable Release Update for horizon has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Corey Bryant (corey.bryant) wrote :

This bug was fixed in the package horizon - 3:14.0.3-0ubuntu1~cloud1
---------------

 horizon (3:14.0.3-0ubuntu1~cloud1) bionic-rocky; urgency=medium
 .
   * d/p/lp1840465.patch: Fix failing to list security groups
     with no rules (LP: #1840465).

Revision history for this message
Corey Bryant (corey.bryant) wrote :

The verification of the Stable Release Update for horizon has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Corey Bryant (corey.bryant) wrote :

This bug was fixed in the package horizon - 3:13.0.2-0ubuntu2~cloud0
---------------

 horizon (3:13.0.2-0ubuntu2~cloud0) xenial-queens; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 horizon (3:13.0.2-0ubuntu2) bionic; urgency=medium
 .
   * d/p/lp1840465.patch: Fix failing to list security groups
     with no rules (LP: #1840465).

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers