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

Author: Zuul

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.