websockify-0.9.0 breaks tempest tests

Bug #1840788 reported by Matthew Thode on 2019-08-20
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
melanie witt
Ghanshyam Mann

Bug Description

see https://review.opendev.org/677479 for a test review

Matt Riedemann (mriedem) on 2019-08-21
Changed in nova:
status: New → Confirmed
Eric Fried (efried) wrote :

Fix [1], being worked via related bug [2], should resolve this bug.

Not sure whether to mark as dup, or what...

[1] https://review.opendev.org/#/c/674364/
[2] https://bugs.launchpad.net/tempest/+bug/1838777

Change abandoned by Eric Fried (<email address hidden>) on branch: master
Review: https://review.opendev.org/677483
Reason: no longer needed

Eric Fried (efried) wrote :

Fix [1], being worked via related bug [2], resolve that bug but not this one, as can be seen from [3] where I've tested [1] against [4].

The _validate_websocket_upgrade bit succeeds, but the subsequent bit, _validate_rfb_negotiation, fails [5].

[1] https://review.opendev.org/#/c/674364/
[2] https://bugs.launchpad.net/tempest/+bug/1838777
[3] https://review.opendev.org/#/c/677798/1
[4] https://review.opendev.org/677479
[5] https://storage.gra1.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/logs_98/677798/1/check/tempest-full/604808b/testr_results.html.gz

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

Changed in nova:
assignee: nobody → melanie witt (melwitt)
status: Confirmed → In Progress
melanie witt (melwitt) wrote :
Changed in tempest:
status: New → In Progress

Reviewed: https://review.opendev.org/677856
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=791fa595e6f1eb2447a4bebf4ecb390c85de0b44
Submitter: Zuul
Branch: master

commit 791fa595e6f1eb2447a4bebf4ecb390c85de0b44
Author: melanie witt <email address hidden>
Date: Wed Aug 21 23:52:52 2019 +0000

    Handle websockify v0.9.0 in console proxy

    In websockify v0.9.0, the 'socket' staticmethod moved from the
    websockfiy.websocket.WebSocketServer class to the
    websockify.websockifyserver.WebSockifyServer class [1][2], so our
    import of the top-level module is no longer sufficient for accessing
    the 'socket' method [3] when using v0.9.0:

     AttributeError: module 'websockify' has no attribute 'WebSocketServer'

    This adds a try_import from the v0.9.0 location and uses
    WebSockifyServer.socket if the module is present. Otherwise, it
    will fall back on the old location. This way, we are able to run with
    websockify v0.9.0 and earlier versions with the same code.

    Partial-Bug: #1840788

    [1] https://github.com/novnc/websockify/commit/8a697622495fd319582cd1c604e7eb2cc0ac0ef6
    [2] https://github.com/novnc/websockify/commit/e47591f4aaa0221a187d3ea2f61c7ab5bb93ed54
    [3] https://github.com/novnc/websockify/blob/v0.9.0/websockify/__init__.py

    Change-Id: I4a50e2f772101315140df43910be2e3f69a63b73

Changed in tempest:
assignee: nobody → Ghanshyam Mann (ghanshyammann)

Reviewed: https://review.opendev.org/674364
Committed: https://git.openstack.org/cgit/openstack/tempest/commit/?id=fd01d15d144caa4d5a482301d05cf724c75c4500
Submitter: Zuul
Branch: master

commit fd01d15d144caa4d5a482301d05cf724c75c4500
Author: Leo Henken <email address hidden>
Date: Fri Aug 2 11:42:52 2019 -0500

    Fix test_novnc to adequately validate websocket upgrade

    Currently, test_novnc validates the websocket upgrade by verifying
    that the websocket response reports a protocol switch and that the
    response includes a server name specified in the configuration
    field vnc_server_header. This explicit server name configuration
    field introduces a security concern and convolutes the code base.

    HTTP RFC7231 (https://tools.ietf.org/html/rfc7231) section 6.2.2
    says that when switching protocols, the response "MUST generate
    an Upgrade header field that indicates which protocols will be
    switched to".

    This patchset uses this required Upgrade field to validate the
    websocket upgrade instead of an environment-based configuration
    field, making the code base cleaner, safer, and more reliable.

    vnc_server_header is deprecated and necessary release notes are

    Change-Id: I5d3c9bdd0d20a15ade672f276dd0f24b654e3de5
    Closes-bug: #1838777
    Closes-bug: #1840788

Changed in tempest:
status: In Progress → Fix Released

This issue was fixed in the openstack/tempest 22.0.0 release.

melanie witt (melwitt) wrote :

I had used Partial-Bug on my patch because the bug still needed a change in tempest to be fully fixed. But I had forgotten how launchpad works where each component/project in a bug needs to "Closes-Bug" for their own part. Marking this as "Fix Released" accordingly.

Changed in nova:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers