openstack
/
oslo.policy
Code Issues Proposed changes

Don't parse cli args on the global object in sphinxpolicygen 

sphinxpolicygen is calling the generate_sample cli entrypoint when
we aren't actually the command being run. This can cause problems
if the consuming project has cli args that get registered on import
of their modules because we may have parsed args before those modules
get imported. This results in an exception because oslo.config won't
allow cli args to be registered after they've been parsed once.

This change makes use of the existing parameter to generate_sample
that allows us to pass in a local config object on which to register
the cli args. This way we can parse them without affecting the
global config object.

This was the only place I could find that we were doing something
like this so I believe it should eliminate the problem.

Change-Id: I8e9f28b0a15d1ed092d72b983be74fe281708fbe
This commit is contained in:
Ben Nemec 2020-03-20 16:04:05 +00:00
parent c386837121
commit a0d99e1046
2 changed files with 16 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
oslo_policy/sphinxpolicygen.py
View File

@ -17,6 +17,7 @@
import os
from oslo_config import cfg
from sphinx.util import logging
from oslo_policy import generator
@ -77,8 +78,14 @@ def _generate_sample(app, policy_file, base_name):
out_file = os.path.join(app.srcdir, file_name)
info('writing sample policy to %s' % out_file)
# NOTE(bnemec): We don't want to do cli parsing on the global object here
# because that can break consumers who do cli arg registration on import
# 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])
'--output-file', out_file],
conf=conf)
def setup(app):

12
oslo_policy/tests/test_sphinxpolicygen.py
View File

@ -32,7 +32,8 @@ class SingleSampleGenerationTest(base.BaseTestCase):
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'],
conf=mock.ANY)
@mock.patch('os.path.isdir')
@mock.patch('os.path.isfile')
@ -49,7 +50,8 @@ class SingleSampleGenerationTest(base.BaseTestCase):
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'],
conf=mock.ANY)
@mock.patch('os.path.isdir')
@mock.patch('os.path.isfile')
@ -70,7 +72,9 @@ class SingleSampleGenerationTest(base.BaseTestCase):
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'],
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'],
conf=mock.ANY)])