openstack/oslo.policy
Code Issues Proposed changes

make deprecated rule examples explicit

Browse Source 
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
This commit is contained in:
Julia Kreger 2022-02-22 11:08:56 -08:00
parent b48b711b09
commit b67e3c71a0
7 changed files with 102 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
doc/source/cli/common/generator-opts.rst
View File

@ -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.

5
doc/source/user/sphinxpolicygen.rst
View File

@ -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

49
oslo_policy/generator.py
View File

@ -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):

17
oslo_policy/sphinxpolicygen.py
View File

@ -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,

11
oslo_policy/tests/test_generator.py
View File

@ -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()

21
oslo_policy/tests/test_sphinxpolicygen.py
View File

@ -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)])

19
releasenotes/notes/fix-deprecated-rule-handling-c6fe321fce6293a9.yaml Normal file
View File

@ -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.