set_defaults() doesn't change default policy_file value when used with the config generator

Bug #1807184 reported by Brian Rosmaita
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
oslo.policy
Fix Released
Medium
Brian Rosmaita

Bug Description

Here's a patch for Cinder where I want to have the default policy_file value that Cinder uses ("policy.yaml") show up in the [oslo_policy] section of the generated config file.

https://review.openstack.org/#/c/623126/2

Cinder uses set_defaults() when it's running [1] and the "policy.yaml" file is the one that's picked up if there's no value set in the config file. So it looks like it's just a problem with the config generation part.

Ben Nemec (bnemec)
Changed in oslo.policy:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to oslo.policy (master)

Fix proposed to branch: master
Review: https://review.openstack.org/623292

Changed in oslo.policy:
assignee: nobody → Ben Nemec (bnemec)
status: Confirmed → In Progress
Changed in oslo.policy:
assignee: Ben Nemec (bnemec) → Brian Rosmaita (brian-rosmaita)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to oslo.policy (master)

Reviewed: https://review.openstack.org/623292
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=3d85afb24a014f43e961887c4e5b679e7eb7dec8
Submitter: Zuul
Branch: master

commit 3d85afb24a014f43e961887c4e5b679e7eb7dec8
Author: Ben Nemec <email address hidden>
Date: Thu Dec 6 19:13:15 2018 +0000

    Fix sample config value when set_defaults is used

    By calling set_default[1] on a conf object it only applies to opts
    registered to that object. This causes an incorrect value to appear in
    the generated sample config because it deals with a list of raw opts,
    not a conf object.

    To fix this, we can call the global set_defaults[2] on the cfg module
    which alters the opts directly. This is the method used in the cors
    middleware[3] and works as expected there.

    This does complicate the unit tests, however. Because we're altering
    global state we need to save the original opts and restore them after
    the test. Furthermore, the conf.reset() call in the config fixture
    doesn't sufficiently reset the conf object to allow it to recognize
    the replaced opts. For the purposes of this test we can just create
    a standalone conf object though, which gets past that problem.

    It's possible that we should fix reset() so it actually removes opts
    in groups completely, but I'm unsure what implications that might
    have for other users of the function.

    1: https://github.com/openstack/oslo.config/blob/b5df53543fd3edbc369cacbdd1c3038bdce9085e/oslo_config/cfg.py#L2433
    2: https://github.com/openstack/oslo.config/blob/b5df53543fd3edbc369cacbdd1c3038bdce9085e/oslo_config/cfg.py#L391
    3: https://github.com/openstack/oslo.middleware/blob/8c7fa5bb105cdfd15376c6b1f42ef1383b7cb3eb/oslo_middleware/cors.py#L88

    Change-Id: I3af9de1b39b6360ecfcb448d8c37b463e1a42ca7
    Closes-Bug: 1807184

Changed in oslo.policy:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/oslo.policy 1.44.1

This issue was fixed in the openstack/oslo.policy 1.44.1 release.

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

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.