deprecated_for_removal generated sample policy has uncommented rule entry

Bug #1771442 reported by Matt Riedemann on 2018-05-15
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
oslo.policy
Undecided
Lance Bragstad

Bug Description

I have code like this:

https://review.openstack.org/#/c/524425/11/nova/api/openstack/placement/policies/base.py

rules = [
    # "placement" is the default rule (action) used for all routes that do
    # not yet have granular policy rules. It is used in
    # PlacementHandler.__call__ and can be dropped once all routes have
    # granular policy handling.
    policy.RuleDefault(
        "placement",
        "role:admin",
        description="This rule is used for all routes that do not yet "
                    "have granular policy rules. It will be replaced "
                    "with rule:admin_api.",
        # TODO(mriedem): Figure out how to use deprecated_rule to alias
        # this for rule:admin_api since it's basically a rename.
        deprecated_for_removal=True,
        deprecated_reason="This was a catch-all rule hard-coded into "
                          "the placement service and has been superseded by "
                          "granular policy rules per operation.",
        deprecated_since="18.0.0"),
    policy.RuleDefault(
        "admin_api",
        "role:admin",
        description="Default rule for most placement APIs."),
]

def list_rules():
    return rules

--

The sample policy file generated from this has the deprecated_for_removal rule uncommented for some reason:

http://logs.openstack.org/25/524425/11/check/build-openstack-sphinx-docs/39855e8/html/configuration/sample-placement-policy.html

# DEPRECATED
# "placement" has been deprecated since 18.0.0.
# This was a catch-all rule hard-coded into the placement service and
# has been superseded by granular policy rules per operation.
# This rule is used for all routes that do not yet have granular
# policy rules. It will be replaced with rule:admin_api.
#"placement": "role:admin"

"placement": "role:admin"# Default rule for most placement APIs.
#"admin_api": "role:admin"

It seems there is a missing # to comment out the %(text)s line here:

https://github.com/openstack/oslo.policy/blob/1.35.0/oslo_policy/generator.py#L139
This clearly looks wrong and there is at least a missing newline.

Riccardo Pittau (rpittau) wrote :

I believe since the rule is deprecated for removal in the future, the entry should not be commented out.
If the rule is already deprecated, you should use the class DeprecatedRule and not RuleDefault.

On the other hand, it does look like it's missing a newline.

Changed in oslo.policy:
status: New → Confirmed

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

Changed in oslo.policy:
assignee: nobody → Lance Bragstad (lbragstad)
status: Confirmed → In Progress

Reviewed: https://review.openstack.org/571830
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=0f31938dd720015444e03f0056c0cfc0e4b8e932
Submitter: Zuul
Branch: master

commit 0f31938dd720015444e03f0056c0cfc0e4b8e932
Author: Lance Bragstad <email address hidden>
Date: Fri Jun 1 21:25:19 2018 +0000

    Remove erroneous newline in sample generation

    The sample generation code for policies has a couple different cases
    that make sure deprecated rules have descriptions and reasoning
    formatted in the comment section. One of the cases that handles
    policies deprecated for removal was injecting an extra newline in the
    comment text that threw off the formatting of the sample, leaving
    the subsequent policy uncommented, and visually unpleasing.

    This commit removes the extra newline in the formatting logic for
    deprecated policies and adds a test case for the behavior.

    Change-Id: I76338d2fbaccf3b43e0da04732fd9df3c05dfbda
    Closes-Bug: 1771442

Changed in oslo.policy:
status: In Progress → Fix Released

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

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

Other bug subscribers