resolve status of policy target deprecated in stein

Bug #1873110 reported by Brian Rosmaita
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
Medium
Brian Rosmaita

Bug Description

Change Iba58e785df934d1c4175c0877d266193ac0167b7, "Add policy granularity to the encryption API", deprecated the policy target "volume_extension:volume_type_encryption" in the Stein release for removal later [0]. It also left a TODO [1] to remove some special purpose code used for the deprecation. The deprecation period for removal of an item deprecated in Stein has passed, and this deprecation needs to be resolved.

[0] https://opendev.org/openstack/cinder/src/commit/edd5bcace2a31325fd3dce47cc615059fdeb295d/cinder/policies/volume_type.py#L75-L77

[1] https://opendev.org/openstack/cinder/src/commit/edd5bcace2a31325fd3dce47cc615059fdeb295d/cinder/api/contrib/volume_type_encryption.py#L62-L65

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Download full text (3.2 KiB)

This is the deprecated policy. It has always had this default value. Originally, it covered all of the get, post, put, delete operations:
#"volume_extension:volume_type_encryption": "rule:admin_api"

These are the fine-grained policies introduced by Change Iba58e785df934d1c4175c0877d266193ac0167b7:
#"volume_extension:volume_type_encryption:create": "rule:volume_extension:volume_type_encryption"
#"volume_extension:volume_type_encryption:get": "rule:volume_extension:volume_type_encryption"
#"volume_extension:volume_type_encryption:update": "rule:volume_extension:volume_type_encryption"
#"volume_extension:volume_type_encryption:delete": "rule:volume_extension:volume_type_encryption"

So the way this is set up is:
- if you did nothing, you get the default of "rule:admin_api" for all operations
- if you modified the original policy, you get that same behavior for all the fine-grained policies
- if you modify the fine-grained policies, you can fine-tune their behavior

I suggest we do nothing, that is, we should un-deprecate the original policy.
- for current operators who have modified their policy config, there will be no effect
- for "new" operators who want to modify their config, they have the option of modifying one policy to affect all four classes of action, or they can use the fine-grained policies

There were comments on the review introducing the new policies that suggested that it was pretty unlikely people needed this level of control. My proposal respects that: operators have an easy way to adjust all policies at once, but also have the option for fine-grained control should they want it.

The other aspect of Change Iba58e785df934d1c4175c0877d266193ac0167b7 was some elaborate code [0,1] to check whether the deprecated policy was set to a non-default value, and if so, log a warning telling the operator to migrate to the fine-grained policy appropriate to the action. I believe this code has a subtle bug, namely, if an operator decided to customize the deprecated policy to control 3 of the actions and only wanted to use fine-grained control on the remaining action, the fine-grained policy would be ignored and the deprecated value would be used instead for that action, despite what was defined in the policy configuration file. Policy configuration is difficult enough for operators without this additional weirdness.

So to be clear about what I'm proposing:
(1) Keep the "volume_extension:volume_type_encryption" policy target, keep its current default value, and un-deprecate it
(2) Leave the default values as they are for the fine-grained policies
(3) Rip out the policy-deprecation code (not just the stuff in volume_type.py, but also the infrastructure for this in policy.py)

Finally, this is consistent with a current proposal to introduce some fine-grained policies for a different set of actions. See https://launchpad.net/cinder/+bug/1841587 and the discussion on the patch https://review.opendev.org/#/c/678799/

[0] https://opendev.org/openstack/cinder/src/commit/edd5bcace2a31325fd3dce47cc615059fdeb295d/cinder/api/contrib/volume_type_encryption.py#L61-L74
[1] https://opendev.org/openstack/cinder/src/commit/edd5bcace2a31325fd3dce47cc6...

Read more...

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

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

Changed in cinder:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.opendev.org/720502
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=65604daae00fd742783e8d17b41b879d7daa1144
Submitter: Zuul
Branch: master

commit 65604daae00fd742783e8d17b41b879d7daa1144
Author: Brian Rosmaita <email address hidden>
Date: Thu Apr 16 09:14:38 2020 -0400

    Resolve deprecation of encryption policy target

    Commit ebc9a12a19bff61bd6101def5cc997513d329bc2 in Stein deprecated
    the "volume_extension:volume_type_encryption" policy target for later
    removal. Instead of removing the target, this patch removes the
    deprecation notice and retains the target as a base policy that can
    be used to set the four finer-grained policies in one place. Also
    removes the supporting code that was logging a warning about the
    deprecated policy.

    See Bug #1873110 for a more thorough discussion of why the
    deprecation is being resolved in this way.

    Change-Id: I24fe5cedea9384d8708d44efb2f70a9cabfab6ca
    Closes-bug: #1873110

Changed in cinder:
status: In Progress → Fix Released
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.