Log warning for redundant file rules3.4.0

If any rules present in policy file is exactly same as
defaults then operators do not need to keep these
redundant rules in files. 'oslopolicy-list-redundant' tool
is to detects such rule but we can log warnings also for
such rule to communicate it to the deployer in strong way.

Partial implement blueprint policy-json-to-yaml

Change-Id: Ie31ea13e8ea62bc495ceb1c1694407539e2cab8d

2 files changed, 37 insertions, 1 deletions

diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py
index d808b37..781502c 100644
--- a/oslo_policy/policy.py
+++ b/oslo_policy/policy.py
@@ -833,8 +833,23 @@ class Enforcer(object):
         if overwrite:
             self.file_rules = {}
         parsed_file = parse_file_contents(data)
+        redundant_file_rules = []
         for name, check_str in parsed_file.items():
-            self.file_rules[name] = RuleDefault(name, check_str)
+            file_rule = RuleDefault(name, check_str)
+            self.file_rules[name] = file_rule
+            reg_rule = self.registered_rules.get(name)
+            if (reg_rule and (file_rule == reg_rule)):
+                redundant_file_rules.append(name)
+        if redundant_file_rules:
+            # NOTE(gmann): Log warning for redundant file rules which
+            # can be detected via 'oslopolicy-list-redundant' tool too.
+            LOG.warning("Policy Rules %(names)s specified in policy files "
+                        "are the same as the defaults provided by the "
+                        "service. You can remove these rules from policy "
+                        "files which will make maintenance easier. You can "
+                        "detect these redundant rules by "
+                        "``oslopolicy-list-redundant`` tool also.",
+                        {'names': redundant_file_rules})
 
     def _load_policy_file(self, path, force_reload, overwrite=True):
         """Load policy rules from the specified policy file.
diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py
index 8444379..7b05362 100644
--- a/oslo_policy/tests/test_policy.py
+++ b/oslo_policy/tests/test_policy.py
@@ -17,6 +17,7 @@
 
 import os
 from unittest import mock
+import yaml
 
 from oslo_config import cfg
 from oslo_context import context
@@ -421,6 +422,26 @@ class EnforcerTest(base.PolicyBaseTestCase):
 
         mock_log.warning.assert_any_call(policy.WARN_JSON)
 
+    @mock.patch.object(policy, 'LOG')
+    def test_warning_on_redundant_file_rules(self, mock_log):
+        rules = yaml.dump({'admin': 'is_admin:True'})
+        self.create_config_file('policy.yaml', rules)
+        path = self.get_config_file_fullname('policy.yaml')
+        enforcer = policy.Enforcer(self.conf, policy_file=path)
+        # register same rule in default as present in file.
+        enforcer.register_default(policy.RuleDefault(name='admin',
+                                  check_str='is_admin:True'))
+
+        enforcer.load_rules(True)
+        warn_msg = ("Policy Rules %(names)s specified in policy files "
+                    "are the same as the defaults provided by the service. "
+                    "You can remove these rules from policy files which "
+                    "will make maintenance easier. You can detect these "
+                    "redundant rules by ``oslopolicy-list-redundant`` tool "
+                    "also.")
+
+        mock_log.warning.assert_any_call(warn_msg, {'names': ['admin']})
+
     def test_load_multiple_directories(self):
         self.create_config_file(
             os.path.join('policy.d', 'a.conf'), POLICY_A_CONTENTS)