From f8f83b4751f5671cbb8cdddb9a7a7230d6cb916d Mon Sep 17 00:00:00 2001
From: Andrew Laski <andrew@lascii.com>
Date: Fri, 1 Jul 2016 11:30:58 -0400
Subject: [PATCH] Hacking check for policy registration

Ensure that policy registration happens in the centralized
nova/policies/ directory. There is an exception for the test_policy unit
tests because some of them register rules for testing.

Change-Id: Ia51eeb09eff86f82528b27bdc11a71762dfaed1a
Partially-Implements: bp policy-in-code
---
 HACKING.rst                     |  1 +
 nova/hacking/checks.py          | 16 ++++++++++++++++
 nova/tests/unit/test_hacking.py | 17 +++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/HACKING.rst b/HACKING.rst
index 4898451f36..632e9ad0e0 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -60,6 +60,7 @@ Nova Specific Commandments
 - [N347] Provide enough help text for config options
 - [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/``
 
 Creating Unit Tests
 -------------------
diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py
index 43c0388bca..f4493c4bde 100644
--- a/nova/hacking/checks.py
+++ b/nova/hacking/checks.py
@@ -39,6 +39,7 @@ session_check = re.compile(r"\w*def [a-zA-Z0-9].*[(].*session.*[)]")
 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\(")
 vi_header_re = re.compile(r"^#\s+vim?:.+")
 virt_file_re = re.compile(r"\./nova/(?:tests/)?virt/(\w+)/")
 virt_import_re = re.compile(
@@ -651,6 +652,20 @@ def check_config_option_in_central_place(logical_line, filename):
         yield(0, msg)
 
 
+def check_policy_registration_in_central_place(logical_line, filename):
+    msg = ('N350: Policy registration should be in the central location '
+           '"/nova/policies/*".')
+    # This is where registration should happen
+    if "nova/policies/" in filename:
+        return
+    # A couple of policy tests register rules
+    if "nova/tests/unit/test_policy.py" in filename:
+        return
+
+    if rule_default_re.match(logical_line):
+        yield(0, msg)
+
+
 def check_doubled_words(physical_line, filename):
     """Check for the common doubled-word typos
 
@@ -797,6 +812,7 @@ def factory(register):
     register(check_no_contextlib_nested)
     register(check_greenthread_spawns)
     register(check_config_option_in_central_place)
+    register(check_policy_registration_in_central_place)
     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 380a34bd65..8598d08852 100644
--- a/nova/tests/unit/test_hacking.py
+++ b/nova/tests/unit/test_hacking.py
@@ -773,3 +773,20 @@ class HackingTestCase(test.NoDBTestCase):
                    self.assertRaises(FakeExcepion, _test)
                """
         self._assert_has_no_errors(code, checker)
+
+    def test_check_policy_registration_in_central_place(self):
+        errors = [(3, 0, "N350")]
+        code = """
+        from nova import policy
+
+        policy.RuleDefault('context_is_admin', 'role:admin')
+        """
+        # registration in the proper place
+        self._assert_has_no_errors(
+            code, checks.check_policy_registration_in_central_place,
+            filename="nova/policies/base.py")
+        # option at a location which is not in scope right now
+        self._assert_has_errors(
+            code, checks.check_policy_registration_in_central_place,
+            filename="nova/api/openstack/compute/non_existent.py",
+            expected_errors=errors)