From b67e3c71a042719a6814621dd1c00c2e1818d2b1 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 22 Feb 2022 11:08:56 -0800 Subject: make deprecated rule examples explicit Deprecated rules can be confusing and downright unfriendly when evaluating a generated sample output and seeing legacy rules being aliased to new rules. Technically this is also invalid and results in a broken sample file with overriding behavior. Under normal circumstances, this wouldn't be a big deal, but with the Secure RBAC effort, projects also performed some further delineation of RBAC policies instead of performing a 1:1 mapping. As a result of the policy enforcement model, a prior deprecated rule was required, which meant the prior deprecated rule would be reported multiple times in the output. Since we don't have an extra flag in the policy-in-code definitions of policies, all we can *really* do is both clarify the purpose and meaning of the entry, not enable the alias by default in sample output (as it is a sample! not an override of code!), and provide projects as well as operators with a knob to exclude deprecated policy inclusion into examples and sample output. Closes-Bug: #1945336 Change-Id: I6d02eb4d8f94323a806fab991ba2f1c3bbf71d04 --- oslo_policy/generator.py | 49 ++++++++++++++++++++++++------- oslo_policy/sphinxpolicygen.py | 17 +++++++---- oslo_policy/tests/test_generator.py | 11 ++++++- oslo_policy/tests/test_sphinxpolicygen.py | 21 ++++++++----- 4 files changed, 73 insertions(+), 25 deletions(-) (limited to 'oslo_policy') 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)]) -- cgit v1.2.1