diff options
| -rw-r--r-- | HACKING.rst | 1 | ||||
| -rw-r--r-- | nova/hacking/checks.py | 20 | ||||
| -rw-r--r-- | nova/tests/unit/test_hacking.py | 19 |
3 files changed, 40 insertions, 0 deletions
diff --git a/HACKING.rst b/HACKING.rst index 632e9ad0e0..721326db4e 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -61,6 +61,7 @@ Nova Specific Commandments - [N348] Deprecated library function os.popen() - [N349] Check for closures in tests which are not used - [N350] Policy registration should be in the central location ``nova/policies/`` +- [N351] Do not use the oslo_policy.policy.Enforcer.enforce() method. Creating Unit Tests ------------------- diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index f4493c4bde..f850991b1e 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -40,6 +40,7 @@ cfg_re = re.compile(r".*\scfg\.") # Excludes oslo.config OptGroup objects cfg_opt_re = re.compile(r".*[\s\[]cfg\.[a-zA-Z]*Opt\(") rule_default_re = re.compile(r".*RuleDefault\(") +policy_enforce_re = re.compile(r".*_ENFORCER\.enforce\(") vi_header_re = re.compile(r"^#\s+vim?:.+") virt_file_re = re.compile(r"\./nova/(?:tests/)?virt/(\w+)/") virt_import_re = re.compile( @@ -666,6 +667,24 @@ def check_policy_registration_in_central_place(logical_line, filename): yield(0, msg) +def check_policy_enforce(logical_line, filename): + """Look for uses of nova.policy._ENFORCER.enforce() + + Now that policy defaults are registered in code the _ENFORCER.authorize + method should be used. That ensures that only registered policies are used. + Uses of _ENFORCER.enforce could allow unregistered policies to be used, so + this check looks for uses of that method. + + N351 + """ + + msg = ('N351: nova.policy._ENFORCER.enforce() should not be used. ' + 'Use the authorize() method instead.') + + if policy_enforce_re.match(logical_line): + yield(0, msg) + + def check_doubled_words(physical_line, filename): """Check for the common doubled-word typos @@ -813,6 +832,7 @@ def factory(register): register(check_greenthread_spawns) register(check_config_option_in_central_place) register(check_policy_registration_in_central_place) + register(check_policy_enforce) register(check_doubled_words) register(check_python3_no_iteritems) register(check_python3_no_iterkeys) diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index 8598d08852..f7cc118090 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -790,3 +790,22 @@ class HackingTestCase(test.NoDBTestCase): code, checks.check_policy_registration_in_central_place, filename="nova/api/openstack/compute/non_existent.py", expected_errors=errors) + + def test_check_policy_enforce(self): + errors = [(3, 0, "N351")] + code = """ + from nova import policy + + policy._ENFORCER.enforce('context_is_admin', target, credentials) + """ + self._assert_has_errors(code, checks.check_policy_enforce, + expected_errors=errors) + + def test_check_policy_enforce_does_not_catch_other_enforce(self): + # Simulate a different enforce method defined in Nova + code = """ + from nova import foo + + foo.enforce() + """ + self._assert_has_no_errors(code, checks.check_policy_enforce) |