Merge "make deprecated rule examples explicit"3.12.0

7 files changed, 102 insertions, 25 deletions

diff --git a/doc/source/cli/common/generator-opts.rst b/doc/source/cli/common/generator-opts.rst
index b88549f..afa7d16 100644
--- a/doc/source/cli/common/generator-opts.rst
+++ b/doc/source/cli/common/generator-opts.rst
@@ -1,3 +1,8 @@
 .. option:: --output-file OUTPUT_FILE
 
     Path of the file to write to. Defaults to stdout.
+
+.. option:: --exclude-deprecated True
+
+   Option allowing the rendered output to be generated *without* deprecated
+   policy information.
diff --git a/doc/source/user/sphinxpolicygen.rst b/doc/source/user/sphinxpolicygen.rst
index 227dbad..4aae146 100644
--- a/doc/source/user/sphinxpolicygen.rst
+++ b/doc/source/user/sphinxpolicygen.rst
@@ -40,6 +40,11 @@ where:
   ``_static/nova.policy.yaml.sample``. If this option is not specified, the
   file will be output to ``sample.policy.yaml``.
 
+``exclude_deprecated``
+  Boolean value, default False, controls if the output should include deprecated
+  policy information or values, as these can be confusing and misleading
+  in some cases.
+
 Once configured, you can include this configuration file in your source:
 
 .. code:: reST
diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py
index f2b6027..deedcd7 100644
--- a/oslo_policy/generator.py
+++ b/oslo_policy/generator.py
@@ -27,6 +27,10 @@ LOG = logging.getLogger(__name__)
 GENERATOR_OPTS = [
     cfg.StrOpt('output-file',
                help='Path of the file to write to. Defaults to stdout.'),
+    cfg.BoolOpt('exclude-deprecated',
+                default=False,
+                help='If True, exclude deprecated entries from the generated '
+                     'output.'),
 ]
 
 RULE_OPTS = [
@@ -232,7 +236,16 @@ def _format_rule_default_yaml(default, include_help=True, comment_rule=True,
         }
 
         if default.name != default.deprecated_rule.name:
-            text += ('"%(old_name)s": "rule:%(name)s"\n' %
+            text += ('# WARNING: A rule name change has been identified.\n'
+                     '#          This may be an artifact of new rules being\n'
+                     '#          included which require legacy fallback\n'
+                     '#          rules to ensure proper policy behavior.\n'
+                     '#          Alternatively, this may just be an alias.\n'
+                     '#          Please evaluate on a case by case basis\n'
+                     '#          keeping in mind the format for aliased\n'
+                     '#          rules is:\n'
+                     '#          "old_rule_name": "new_rule_name".\n')
+            text += ('# "%(old_name)s": "rule:%(name)s"\n' %
                      {'old_name': default.deprecated_rule.name,
                       'name': default.name})
         text += '\n'
@@ -252,7 +265,7 @@ def _format_rule_default_json(default):
 
 
 def _sort_and_format_by_section(policies, output_format='yaml',
-                                include_help=True):
+                                include_help=True, exclude_deprecated=False):
     """Generate a list of policy section texts
 
     The text for a section will be created and returned one at a time. The
@@ -264,20 +277,24 @@ def _sort_and_format_by_section(policies, output_format='yaml',
     :param policies: A dict of {section1: [rule_default_1, rule_default_2],
                                 section2: [rule_default_3]}
     :param output_format: The format of the file to output to.
+    :param exclude_deprecated: If to exclude deprecated policy rule entries,
+                               defaults to False.
     """
     for section in sorted(policies.keys()):
         rule_defaults = policies[section]
         for rule_default in rule_defaults:
             if output_format == 'yaml':
-                yield _format_rule_default_yaml(rule_default,
-                                                include_help=include_help)
+                yield _format_rule_default_yaml(
+                    rule_default,
+                    include_help=include_help,
+                    add_deprecated_rules=not exclude_deprecated)
             elif output_format == 'json':
                 LOG.warning(policy.WARN_JSON)
                 yield _format_rule_default_json(rule_default)
 
 
 def _generate_sample(namespaces, output_file=None, output_format='yaml',
-                     include_help=True):
+                     include_help=True, exclude_deprecated=False):
     """Generate a sample policy file.
 
     List all of the policies available via the namespace specified in the
@@ -291,6 +308,8 @@ def _generate_sample(namespaces, output_file=None, output_format='yaml',
     :param include_help: True, generates a sample-policy file with help text
                          along with rules in which everything is commented out.
                          False, generates a sample-policy file with only rules.
+    :param exclude_deprecated: If to exclude deprecated policy rule entries,
+                               defaults to False.
     """
     policies = get_policies_dict(namespaces)
 
@@ -298,8 +317,10 @@ def _generate_sample(namespaces, output_file=None, output_format='yaml',
                    else sys.stdout)
 
     sections_text = []
-    for section in _sort_and_format_by_section(policies, output_format,
-                                               include_help=include_help):
+    for section in _sort_and_format_by_section(
+            policies, output_format,
+            include_help=include_help,
+            exclude_deprecated=exclude_deprecated):
         sections_text.append(section)
 
     if output_format == 'yaml':
@@ -315,7 +336,7 @@ def _generate_sample(namespaces, output_file=None, output_format='yaml',
         output_file.close()
 
 
-def _generate_policy(namespace, output_file=None):
+def _generate_policy(namespace, output_file=None, exclude_deprecated=False):
     """Generate a policy file showing what will be used.
 
     This takes all registered policies and merges them with what's defined in
@@ -323,6 +344,8 @@ def _generate_policy(namespace, output_file=None):
     that will be honored by policy checks.
 
     :param output_file: The path of a file to output to. stdout used if None.
+    :param exclude_deprecated: If to exclude deprecated policy rule entries,
+                               defaults to False.
     """
     enforcer = _get_enforcer(namespace)
     # Ensure that files have been parsed
@@ -338,7 +361,9 @@ def _generate_policy(namespace, output_file=None):
     output_file = (open(output_file, 'w') if output_file
                    else sys.stdout)
 
-    for section in _sort_and_format_by_section(policies, include_help=False):
+    for section in _sort_and_format_by_section(
+            policies, include_help=False,
+            exclude_deprecated=exclude_deprecated):
         output_file.write(section)
 
     if output_file != sys.stdout:
@@ -520,7 +545,8 @@ def generate_sample(args=None, conf=None):
     conf.register_opts(GENERATOR_OPTS + RULE_OPTS)
     conf(args)
     _check_for_namespace_opt(conf)
-    _generate_sample(conf.namespace, conf.output_file, conf.format)
+    _generate_sample(conf.namespace, conf.output_file, conf.format,
+                     conf.exclude_deprecated)
 
 
 def generate_policy(args=None):
@@ -530,7 +556,8 @@ def generate_policy(args=None):
     conf.register_opts(GENERATOR_OPTS + ENFORCER_OPTS)
     conf(args)
     _check_for_namespace_opt(conf)
-    _generate_policy(conf.namespace, conf.output_file)
+    _generate_policy(conf.namespace, conf.output_file,
+                     conf.exclude_deprecated)
 
 
 def _upgrade_policies(policies, default_policies):
diff --git a/oslo_policy/sphinxpolicygen.py b/oslo_policy/sphinxpolicygen.py
index 1aef057..3987d04 100644
--- a/oslo_policy/sphinxpolicygen.py
+++ b/oslo_policy/sphinxpolicygen.py
@@ -37,18 +37,20 @@ def generate_sample(app):
         for config_file, base_name in app.config.policy_generator_config_file:
             if base_name is None:
                 base_name = _get_default_basename(config_file)
-            _generate_sample(app, config_file, base_name)
+            _generate_sample(app, config_file, base_name,
+                             app.config.exclude_deprecated)
     else:
         _generate_sample(app,
                          app.config.policy_generator_config_file,
-                         app.config.sample_policy_basename)
+                         app.config.sample_policy_basename,
+                         app.config.exclude_deprecated)
 
 
 def _get_default_basename(config_file):
     return os.path.splitext(os.path.basename(config_file))[0]
 
 
-def _generate_sample(app, policy_file, base_name):
+def _generate_sample(app, policy_file, base_name, exclude_deprecated):
 
     def info(msg):
         LOG.info('[%s] %s' % (__name__, msg))
@@ -83,14 +85,17 @@ def _generate_sample(app, policy_file, base_name):
     # in their documented modules. It's not allowed to register a cli arg after
     # the args have been parsed once.
     conf = cfg.ConfigOpts()
-    generator.generate_sample(args=['--config-file', config_path,
-                                    '--output-file', out_file],
-                              conf=conf)
+    generator.generate_sample(
+        args=['--config-file', config_path,
+              '--output-file', out_file,
+              '--exclude-deprecated', exclude_deprecated],
+        conf=conf)
 
 
 def setup(app):
     app.add_config_value('policy_generator_config_file', None, 'env')
     app.add_config_value('sample_policy_basename', None, 'env')
+    app.add_config_value('exclude_deprecated', False, 'env')
     app.connect('builder-inited', generate_sample)
     return {
         'parallel_read_safe': True,
diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py
index 5dbb4ed..9bdf2ee 100644
--- a/oslo_policy/tests/test_generator.py
+++ b/oslo_policy/tests/test_generator.py
@@ -223,7 +223,16 @@ class GenerateSampleYAMLTestCase(base.PolicyBaseTestCase):
 # "foo:post_bar":"role:fizz" has been deprecated since N in favor of
 # "foo:create_bar":"role:fizz".
 # foo:post_bar is being removed in favor of foo:create_bar
-"foo:post_bar": "rule:foo:create_bar"
+# WARNING: A rule name change has been identified.
+#          This may be an artifact of new rules being
+#          included which require legacy fallback
+#          rules to ensure proper policy behavior.
+#          Alternatively, this may just be an alias.
+#          Please evaluate on a case by case basis
+#          keeping in mind the format for aliased
+#          rules is:
+#          "old_rule_name": "new_rule_name".
+# "foo:post_bar": "rule:foo:create_bar"
 
 '''
         stdout = self._capture_stdout()
diff --git a/oslo_policy/tests/test_sphinxpolicygen.py b/oslo_policy/tests/test_sphinxpolicygen.py
index 461d6c5..4d4fb68 100644
--- a/oslo_policy/tests/test_sphinxpolicygen.py
+++ b/oslo_policy/tests/test_sphinxpolicygen.py
@@ -27,13 +27,15 @@ class SingleSampleGenerationTest(base.BaseTestCase):
         isdir.return_value = True
 
         config = mock.Mock(policy_generator_config_file='nova.conf',
-                           sample_policy_basename='nova')
+                           sample_policy_basename='nova',
+                           exclude_deprecated=False)
         app = mock.Mock(srcdir='/opt/nova', config=config)
         sphinxpolicygen.generate_sample(app)
 
         sample.assert_called_once_with(args=[
             '--config-file', '/opt/nova/nova.conf',
-            '--output-file', '/opt/nova/nova.policy.yaml.sample'],
+            '--output-file', '/opt/nova/nova.policy.yaml.sample',
+            '--exclude-deprecated', False],
             conf=mock.ANY)
 
     @mock.patch('os.path.isdir')
@@ -45,13 +47,15 @@ class SingleSampleGenerationTest(base.BaseTestCase):
         isdir.return_value = True
 
         config = mock.Mock(policy_generator_config_file='nova.conf',
-                           sample_policy_basename=None)
+                           sample_policy_basename=None,
+                           exclude_deprecated=True)
         app = mock.Mock(srcdir='/opt/nova', config=config)
         sphinxpolicygen.generate_sample(app)
 
         sample.assert_called_once_with(args=[
             '--config-file', '/opt/nova/nova.conf',
-            '--output-file', '/opt/nova/sample.policy.yaml'],
+            '--output-file', '/opt/nova/sample.policy.yaml',
+            '--exclude-deprecated', True],
             conf=mock.ANY)
 
     @mock.patch('os.path.isdir')
@@ -66,16 +70,19 @@ class SingleSampleGenerationTest(base.BaseTestCase):
 
         config = mock.Mock(policy_generator_config_file=[
             ('nova.conf', 'nova'),
-            ('placement.conf', 'placement')])
+            ('placement.conf', 'placement')],
+            exclude_deprecated=False)
         app = mock.Mock(srcdir='/opt/nova', config=config)
         sphinxpolicygen.generate_sample(app)
 
         sample.assert_has_calls([
             mock.call(args=[
                 '--config-file', '/opt/nova/nova.conf',
-                '--output-file', '/opt/nova/nova.policy.yaml.sample'],
+                '--output-file', '/opt/nova/nova.policy.yaml.sample',
+                '--exclude-deprecated', False],
                 conf=mock.ANY),
             mock.call(args=[
                 '--config-file', '/opt/nova/placement.conf',
-                '--output-file', '/opt/nova/placement.policy.yaml.sample'],
+                '--output-file', '/opt/nova/placement.policy.yaml.sample',
+                '--exclude-deprecated', False],
                 conf=mock.ANY)])
diff --git a/releasenotes/notes/fix-deprecated-rule-handling-c6fe321fce6293a9.yaml b/releasenotes/notes/fix-deprecated-rule-handling-c6fe321fce6293a9.yaml
new file mode 100644
index 0000000..359b867
--- /dev/null
+++ b/releasenotes/notes/fix-deprecated-rule-handling-c6fe321fce6293a9.yaml
@@ -0,0 +1,19 @@
+---
+fixes:
+  - |
+    Fixes handling of deprecated rules when generating sample policy files
+    such that legacy rules are no longer automatically aliased in the
+    resulting output. Previously, the behavior led to operator confusion when
+    attempting to evaluate the output to determine if customized rules were
+    required, as the aliases were always added as active rules. A warning
+    is now also added to the generated output.
+    For more information, please see `launchpad bug #1945336 <https://bugs.launchpad.net/oslo.policy/+bug/1945336>`_.
+features:
+  - Adds the ability to exclude deprecated policies from generated samples by
+    utilizing the ``--exclude-deprecated`` setting when generating YAML
+    example files. The Spinx generator can also be controlled using the
+    ``exclude_deprecated`` environment variable. By default, these rules
+    will be included, but operators and projects may not desire these
+    deprecated rules to exist in latest documentation, espescially when
+    considering the number of policy rules projects have made in the
+    Secure RBAC effort.