cors.set_defaults does not have real test coverage in placement and probably not nova either

Bug #1784663 reported by Chris Dent on 2018-07-31
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Low
Unassigned

Bug Description

Both the placement and nova apis allow oslo_middleware.cors in their WSGI middleware stacks.

Placement has some gabbi functional tests which test that the middleware is present and does the right thing when using the middleware's own configuration defaults. Both when it is on or off in cors.yaml and non-cors.yaml.

However, the WSGI application that is actually used in a deployment, the one created by nova/api/openstack/placement/wsgi.py is not used in those functional tests. That code calls the set_defaults method on the cors middleware to change and define those HTTP headers and request methods which will be allowed without further configuration.

As far as I know, nothing (such as a tempest test) confirms those headers in either placement or nova, and it's relatively certain they are incomplete with regard to microversions (as OpenStack-API-Version and X-OpenStack-Nova-API-Version).

This bug is the result of discussion on https://review.openstack.org/#/c/587183/2/nova/api/openstack/placement/wsgi.py

The gabbi tests show the kinds of requests that can be done to confirm the right headers are generated:

https://github.com/openstack/nova/blob/master/nova/tests/functional/api/openstack/placement/gabbits/cors.yaml

https://github.com/openstack/nova/blob/master/nova/tests/functional/api/openstack/placement/gabbits/non-cors.yaml

Matt Riedemann (mriedem) wrote :

I'm confused, if there are gabbi tests testing this, then there is coverage, yeah? I don't think this is really something that tempest would likely need to test if we can just test it in-tree.

Changed in nova:
status: New → Incomplete
Matt Riedemann (mriedem) wrote :

Oh I see:

> However, the WSGI application that is actually used in a deployment, the one created by nova/api/openstack/placement/wsgi.py is not used in those functional tests.

Changed in nova:
status: Incomplete → Confirmed
importance: Undecided → Low
tags: added: testing
tags: added: cors

Reviewed: https://review.openstack.org/590445
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=94617a71761710544cf9505ffda5b6403a645a7e
Submitter: Zuul
Branch: master

commit 94617a71761710544cf9505ffda5b6403a645a7e
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 9 13:32:10 2018 -0400

    placement: ignore policy scope check failures if not enforcing scope

    Rather than spam the placement API logs with scope check
    warnings from oslo.policy when placement isn't configured
    for scope type enforcement, ignore those warnings.

    Note that the same warnings filter in the placement WarningsFixture
    is left intact because the configuration defaults setting code in
    wsgi.py which allows the warning filter to be set in deploy.py,
    is not run by the functional tests. In future changes this will
    be resolved by unifying configuration handling.

    Closes-Bug: #1786498
    Related-Bug: #1784663
    Change-Id: I34e4e550c9c31a654308e555210588156418f9e3

Reviewed: https://review.openstack.org/591872
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=97e7087ac329c5b6fc77ddd76d2007e03866e00f
Submitter: Zuul
Branch: stable/rocky

commit 97e7087ac329c5b6fc77ddd76d2007e03866e00f
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 9 13:32:10 2018 -0400

    placement: ignore policy scope check failures if not enforcing scope

    Rather than spam the placement API logs with scope check
    warnings from oslo.policy when placement isn't configured
    for scope type enforcement, ignore those warnings.

    Note that the same warnings filter in the placement WarningsFixture
    is left intact because the configuration defaults setting code in
    wsgi.py which allows the warning filter to be set in deploy.py,
    is not run by the functional tests. In future changes this will
    be resolved by unifying configuration handling.

    Closes-Bug: #1786498
    Related-Bug: #1784663
    Change-Id: I34e4e550c9c31a654308e555210588156418f9e3
    (cherry picked from commit 94617a71761710544cf9505ffda5b6403a645a7e)

tags: added: in-stable-rocky
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