oslopolicy-list-redundant loses cli args when used with keystone

[Ben Nemec]

There is an issue with the configuration handling in oslo.policy and keystone that causes cli args like --config-file to be ignored in the keystone enforcer when running oslopolicy-list-redundant. Specifically, because keystone re-initializes the global config object when creating the enforcer[0], and doesn't pass any cli args to it, those cli args get ignored. This can cause problems if, for example, the policy file is not in the default location and is instead specified in the config file passed via --config-file. Since --config-file gets ignored by the enforcer, it just looks in the default location and doesn't find a file.

One solution would be to have oslo.policy initialize the global config object itself (switching [1] to use the global object instead of a local one) and remove the initialization from the enforcer entirely. One potential downside of this is that if a project's enforcer needs project-specific config setup it wouldn't be possible for that to happen (oslo.policy wouldn't know about it), but since that doesn't apply to keystone and would only really be an issue if a project's enforcer had a dependency on a cli arg (cli args are the only thing that need to be registered before calling the conf object), I think it's a worthwhile tradeoff.

0: https://github.com/openstack/keystone/blob/1ef56e58ec63f19eff25a1044c8831ba8f97e26a/keystone/common/rbac_enforcer/policy.py#L43
1: https://github.com/openstack/oslo.policy/blob/0f7e144d013155f27f74b0eb91b7ae0f1530a86b/oslo_policy/generator.py#L399

[Ben Nemec (bnemec)]

Oh yuck, it looks like this is happening in all projects. Here's Nova's call: https://github.com/openstack/nova/blob/2718de6ed7c21f8ff8cf74164ae5054531fdbc30/nova/policy.py#L224

That's going to make this a real joy to fix. :-/

Hopefully we can switch oslo.policy to use the global conf object and not break everyone, then migrate projects away from their own calls over time.

[Ben Nemec (bnemec)]

And I'm wrong about the project-specific part too. My proposed solution will break the default case because oslo.policy won't pass the project name to the conf initialization. Maybe we need to update the calls in all of the projects to include cli args instead of removing them completely? Still need to use the global conf object to register the options on the oslo.policy side so things like --namespace will be registered. I'll have to test that.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/690628
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=686aa238f921e8b6dff814d001690e15fa8ccea6
Submitter: Zuul
Branch: master

commit 686aa238f921e8b6dff814d001690e15fa8ccea6
Author: Ben Nemec <email address hidden>
Date: Wed Oct 23 15:36:42 2019 +0000

    Initialize global config object in cli tools

    Currently, passing --config-file to a tool like oslopolicy-list-redundant
    is ineffective because the projects pass an empty cli arg list to the
    conf object when they initialize it. By registering our cli args on the
    global conf object, the projects can safely parse cli args in their
    call to the conf object so things like --config-file won't be ignored.
    This didn't work before because oslo.policy recognizes cli args like
    --namespace that aren't recognized by the consuming projects.

    This will require followup changes in each project to stop passing an
    empty cli arg list to the conf object initialization. In the meantime,
    everything should continue to work as it did before.

    Change-Id: Iacd257fc6c351582de45476768e3fd1775317d3c
    Closes-Bug: 1849518

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/708212

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/708212
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=c3868371212597069e4614d9ae05fe7cd0358ca1
Submitter: Zuul
Branch: master

commit c3868371212597069e4614d9ae05fe7cd0358ca1
Author: Ben Nemec <email address hidden>
Date: Mon Feb 17 16:17:34 2020 +0000

    Temporarily make namespace arg optional

    In order to fix the referenced bug, we need to register cli args on
    the global config object. Unfortunately, that causes issues because
    our consumers are re-calling the conf object in their enforcers due
    to the way we used to handle cli args. Specifically, the conf call
    in the consumer fails because the namespace arg from oslo.policy is
    registered as required, but they don't pass it to the conf call.

    Long-term we want to stop having consumers call the conf object at
    all, but in the meantime we need to provide a migration path that
    doesn't break them. This change registers the namespace arg as
    optional on the conf object and temporarily moves the required check
    to oslo.policy. This will allow us to maintain the existing behavior
    for our cli tools while not breaking consumers who haven't migrated
    to the new cli arg behavior.

    Note that we do have unit test coverage of this behavior[0], so we
    can be reasonably confident the explicit check is maintaining
    compatibility.

    Change-Id: I34ce1dd15c464bec319e51d3e217e26554f1a944
    Closes-Bug: 1863637
    Related-Bug: 1849518
    0: https://github.com/openstack/oslo.policy/blob/6e2fe3857367eb2b3e2d2e92121a408e1ff89ea4/oslo_policy/tests/test_generator.py#L500

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/690630
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=ba8dd06e123adb353c5bb71d75c345cf3e463ba8
Submitter: Zuul
Branch: master

commit ba8dd06e123adb353c5bb71d75c345cf3e463ba8
Author: Ben Nemec <email address hidden>
Date: Wed Oct 23 16:11:35 2019 +0000

    Parse cli args in get_enforcer

    Previously this call to the conf object couldn't parse cli args
    because the oslo.policy tool was registering its cli opts on a
    private conf object, so attempting to parse them on the global
    object would fail. The dependency makes oslo.policy use the global
    object instead so cli arg parsing works correctly.

    This is important because ignoring cli args as this was previously
    doing caused things like --config-file to be dropped, which meant
    that running the tool with that option specified did not work as
    expected.

    Depends-On: https://review.opendev.org/690628
    Change-Id: Id553743277a35660a40d6b3b02847d7a35abbfb9
    Closes-Bug: 1849518