Deprecating a policy is confusing

[Lance Bragstad]

Some oddities came up in IRC today as we were using DeprecatedRule to cover a test case in keystone [0].

The initial implementation of the DeprecatedRule object attempted to follow existing conventions used to deprecate configuration options in oslo.config, but there are some ways the we could improve the experience.

It can be confusing to see deprecated kwargs on the option that is not actually being deprecated. One way we could fix this would be to isolate all deprecated kwargs to the DeprecatedRule object instead of having to deal with setting deprecated kwargs on each object. So something like the following would be preferred:

deprecated_rule = policy.DeprecatedRule(
    name='foo:bar',
    check_str='role:bazz',
    deprecated_reason='foo:create_bar is more granular than foo:bar',
    deprecated_since='N'
)

policy.DocumentedRuleDefault(
    name='foo:create_bar',
    check_str='role:bang',
    description='Create a bar.',
    operations=[{'path': '/v1/bars', 'method': 'POST'}],
    deprecated_rule=deprecated_rule
)

instead of:

deprecated_rule = policy.DeprecatedRule(
    name='foo:bar',
    check_str='role:bazz'
)

policy.DocumentedRuleDefault(
    name='foo:create_bar',
    check_str='role:bang',
    description='Create a bar.',
    operations=[{'path': '/v1/bars', 'method': 'POST'}],
    deprecated_rule=deprecated_rule,
    deprecated_reason='foo:create_bar is more granular than foo:bar',
    deprecated_since='N'
)

[0] http://eavesdrop.openstack.org/irclogs/%23openstack-keystone/%23openstack-keystone.2018-06-27.log.html#t2018-06-27T15:46:58

[Lance Bragstad (lbragstad)]

I have a feeling even if we fix this bug, we'll have to support both ways of deprecating a policy for backwards compatibility. But we could just advertise the preferred way to deprecate in our documentation.

[Ben Nemec (bnemec)]

The new proposal seems reasonable. We have a similar feature in oslo.config with the DeprecatedOpt class: https://github.com/openstack/oslo.config/blob/9c71b0140931b51ffffc3bbdc1b655fdaccce45b/oslo_config/cfg.py#L1223

However, I'm not sure in that case if you can set things like deprecated_reason on the DeprecatedOpt or if you still have to set it on the main opt. In any case, this does seem like a better way to do it.