Add nova-status upgrade check and reno for policy new defaults

There are cases where policy file is re-generated freshly
and end up having the new defaults only but expectation is that
old deprecated rule keep working.

If a rule is present in policy file then, that has priority over
its defaults so either rules should not be present in policy file
or users need to update their token to match the overridden rule
permission.

This issue was always present when any policy defaults were changed
with old defaults being supported as deprecated. This is we have
changed all the policy for new defaults so it came up as broken case.

Adding nova-status upgrade check also to detect such policy file.

Related-Bug: #1875418

Change-Id: Id9cd65877e53577bff22e408ca07bbeec4407f6e

Author: gmaanos

doc/source/cli/nova-status.rst

@@ -143,6 +143,11 @@ Upgrade
   **21.0.0 (Ussuri)**
   * Checks for the Placement API are modified to require version 1.35.
 
+  **21.0.0 (Ussuri)**
+
+  * Checks for the policy files are not automatically overwritten with
+    new defaults.
+
 See Also
 ========
 

doc/source/install/verify.rst

@@ -123,3 +123,7 @@ Verify operation of the Compute service.
       | Result: Success                                                    |
       | Details: None                                                      |
       +--------------------------------------------------------------------+
+      | Check: Policy Scope-based Defaults                                 |
+      | Result: Success                                                    |
+      | Details: None                                                      |
+      +--------------------------------------------------------------------+

nova/cmd/status.py

@@ -40,6 +40,7 @@
 from nova import exception
 from nova.i18n import _
 from nova.objects import cell_mapping as cell_mapping_obj
+from nova import policy
 from nova import utils
 from nova import version
 from nova.volume import cinder
@@ -346,6 +347,70 @@ def _check_cinder(self):
                 six.text_type(ex))
         return upgradecheck.Result(upgradecheck.Code.SUCCESS)
 
+    def _check_policy(self):
+        """Checks to see if policy file is overwritten with the new
+        defaults.
+        """
+        msg = _("Your policy file contains rules which examine token scope, "
+                "which may be due to generation with the new defaults. "
+                "If that is done intentionally to migrate to the new rule "
+                "format, then you are required to enable the flag "
+                "'oslo_policy.enforce_scope=True' and educate end users on "
+                "how to request scoped tokens from Keystone. Another easy "
+                "and recommended way for you to achieve the same is via two "
+                "flags, 'oslo_policy.enforce_scope=True' and "
+                "'oslo_policy.enforce_new_defaults=True' and avoid "
+                "overwriting the file. Please refer to this document to "
+                "know the complete migration steps: "
+                "https://docs.openstack.org/nova/latest/configuration"
+                "/policy-concepts.html. If you did not intend to migrate "
+                "to new defaults in this upgrade, then with your current "
+                "policy file the scope checking rule will fail. A possible "
+                "reason for such a policy file is that you generated it with "
+                "'oslopolicy-sample-generator' in json format. "
+                "Three ways to fix this until you are ready to migrate to "
+                "scoped policies: 1. Generate the policy file with "
+                "'oslopolicy-sample-generator' in yaml format, keep "
+                "the generated content commented out, and update "
+                "the generated policy.yaml location in "
+                "``oslo_policy.policy_file``. "
+                "2. Use a pre-existing sample config file from the Train "
+                "release. 3. Use an empty or non-existent file to take all "
+                "the defaults.")
+        rule = "system_admin_api"
+        rule_new_default = "role:admin and system_scope:all"
+        status = upgradecheck.Result(upgradecheck.Code.SUCCESS)
+        # NOTE(gmann): Initialise the policy if it not initialized.
+        # We need policy enforcer with all the rules loaded to check
+        # their value with defaults.
+        try:
+            if policy._ENFORCER is None:
+                policy.init(suppress_deprecation_warnings=True)
+
+            # For safer side, recheck that the enforcer is available before
+            # upgrade checks. If something is wrong on oslo side and enforcer
+            # is still not available the return warning to avoid any false
+            # result.
+            if policy._ENFORCER is not None:
+                current_rule = str(policy._ENFORCER.rules[rule]).strip("()")
+                if (current_rule == rule_new_default and
+                    not CONF.oslo_policy.enforce_scope):
+                    status = upgradecheck.Result(upgradecheck.Code.WARNING,
+                                                 msg)
+            else:
+                status = upgradecheck.Result(
+                    upgradecheck.Code.WARNING,
+                    _('Policy is not initialized to check the policy rules'))
+        except Exception as ex:
+            status = upgradecheck.Result(
+                upgradecheck.Code.WARNING,
+                _('Unable to perform policy checks due to error: %s') %
+                six.text_type(ex))
+        # reset the policy state so that it can be initialized from fresh if
+        # operator changes policy file after running this upgrade checks.
+        policy.reset()
+        return status
+
     # The format of the check functions is to return an upgradecheck.Result
     # object with the appropriate upgradecheck.Code and details set. If the
     # check hits warnings or failures then those should be stored in the
@@ -362,6 +427,8 @@ def _check_cinder(self):
         (_('Ironic Flavor Migration'), _check_ironic_flavor_migration),
         # Added in Train
         (_('Cinder API'), _check_cinder),
+        # Added in Ussuri
+        (_('Policy Scope-based Defaults'), _check_policy),
     )
 
 

nova/tests/unit/cmd/test_status.py

@@ -39,8 +39,10 @@
 # NOTE(mriedem): We only use objects as a convenience to populate the database
 # in the tests, we don't use them in the actual CLI.
 from nova import objects
+from nova import policy
 from nova import test
 from nova.tests import fixtures as nova_fixtures
+from nova.tests.unit import policy_fixture
 
 
 CONF = nova.conf.CONF
@@ -546,3 +548,57 @@ def test_microversion_available(self, mock_version_check):
         result = self.cmd._check_cinder()
         mock_version_check.assert_called_once()
         self.assertEqual(upgradecheck.Code.SUCCESS, result.code)
+
+
+class TestUpgradeCheckPolicy(test.NoDBTestCase):
+
+    new_default_status = upgradecheck.Code.WARNING
+
+    def setUp(self):
+        super(TestUpgradeCheckPolicy, self).setUp()
+        self.cmd = status.UpgradeCommands()
+        self.rule_name = "system_admin_api"
+
+    def tearDown(self):
+        super(TestUpgradeCheckPolicy, self).tearDown()
+        # Check if policy is reset back after the upgrade check
+        self.assertIsNone(policy._ENFORCER)
+
+    def test_policy_rule_with_new_defaults(self):
+        new_default = "role:admin and system_scope:all"
+        rule = {self.rule_name: new_default}
+        self.policy.set_rules(rule, overwrite=False)
+        self.assertEqual(self.new_default_status,
+                         self.cmd._check_policy().code)
+
+    def test_policy_rule_with_old_defaults(self):
+        new_default = "is_admin:True"
+        rule = {self.rule_name: new_default}
+        self.policy.set_rules(rule, overwrite=False)
+
+        self.assertEqual(upgradecheck.Code.SUCCESS,
+                         self.cmd._check_policy().code)
+
+    def test_policy_rule_with_both_defaults(self):
+        new_default = "(role:admin and system_scope:all) or is_admin:True"
+        rule = {self.rule_name: new_default}
+        self.policy.set_rules(rule, overwrite=False)
+
+        self.assertEqual(upgradecheck.Code.SUCCESS,
+                         self.cmd._check_policy().code)
+
+    def test_policy_checks_with_fresh_init_and_no_policy_override(self):
+        self.policy = self.useFixture(policy_fixture.OverridePolicyFixture(
+                                      rules_in_file={}))
+        policy.reset()
+        self.assertEqual(upgradecheck.Code.SUCCESS,
+                         self.cmd._check_policy().code)
+
+
+class TestUpgradeCheckPolicyEnableScope(TestUpgradeCheckPolicy):
+
+    new_default_status = upgradecheck.Code.SUCCESS
+
+    def setUp(self):
+        super(TestUpgradeCheckPolicyEnableScope, self).setUp()
+        self.flags(enforce_scope=True, group="oslo_policy")

releasenotes/notes/bug-1875418-0df3198e36530ec7.yaml

@@ -0,0 +1,31 @@
+---
+upgrade:
+  - |
+    Nova policies implemented the ``scope_type`` and new defaults
+    provided by keystone. Old defaults are deprecated and still work
+    if rules are not overridden in the policy file. If you don't override
+    any policies at all, then you don't need to do anything different until the
+    W release when old deprecated rules are removed and tokens need to be
+    scoped to work with new defaults and scope of policies. For migration
+    to new policies you can refer to `this document
+    <https://docs.openstack.org/nova/latest/configuration/policy-concepts.html#migration-plan>`_.
+
+    If you are overwriting the policy rules (all or some of them) in the policy
+    file with new default values or any new value that requires scoped tokens,
+    then non-scoped tokens will not work. Also if you generate the policy
+    file with 'oslopolicy-sample-generator' json format or any other tool,
+    you will get rules defaulted in the new format, which examines the token
+    scope. Unless you turn on ``oslo_policy.enforce_scope``, scope-checking
+    rules will fail. Thus, be sure to enable ``oslo_policy.enforce_scope`` and
+    `educate <https://docs.openstack.org/nova/latest/configuration/policy-concepts.html>`_
+    end users on how to request scoped tokens from Keystone, or
+    use a pre-existing sample config file from the Train release until you are
+    ready to migrate to scoped policies. Another way is to generate the policy
+    file in yaml format as described `here
+    <https://docs.openstack.org/oslo.policy/latest/cli/index.html#oslopolicy-policy-generator>`_
+    and update the policy.yaml location in ``oslo_policy.policy_file``.
+
+    For more background about the possible problem, check `this bug
+    <https://bugs.launchpad.net/nova/+bug/1875418>`_.
+    A upgrade check has been added to the ``nova-status upgrade check``
+    command for this.