Merge "Deprecate [placement]/policy_file config option"
This commit is contained in:
commit
83be3ee9bb
etc/placement
placement
releasenotes/notes
@ -5,6 +5,7 @@ namespace = placement.conf
|
||||
namespace = keystonemiddleware.auth_token
|
||||
namespace = oslo.log
|
||||
namespace = oslo.middleware.cors
|
||||
namespace = oslo.policy
|
||||
namespace = osprofiler
|
||||
# FIXME(mriedem): There are likely other missing 3rd party oslo library
|
||||
# options that should show up in the placement.conf docs, like oslo.policy.
|
||||
# options that should show up in the placement.conf docs, like oslo.concurrency
|
||||
|
@ -16,6 +16,7 @@ from __future__ import absolute_import
|
||||
|
||||
from oslo_log import log as logging
|
||||
from oslo_middleware import cors
|
||||
from oslo_policy import opts as policy_opts
|
||||
|
||||
from placement.conf import api
|
||||
from placement.conf import base
|
||||
@ -34,6 +35,7 @@ def register_opts(conf):
|
||||
paths.register_opts(conf)
|
||||
placement.register_opts(conf)
|
||||
logging.register_options(conf)
|
||||
policy_opts.set_defaults(conf)
|
||||
# The CORS middleware does not present a register_opts method, instead
|
||||
# it shares a list of available opts.
|
||||
conf.register_opts(cors.CORS_OPTS, 'cors')
|
||||
|
@ -40,6 +40,17 @@ is determined.
|
||||
# This default matches what is in
|
||||
# etc/nova/policy-generator.conf
|
||||
default='policy.yaml',
|
||||
deprecated_for_removal=True,
|
||||
deprecated_since='2.0.0',
|
||||
deprecated_reason="""
|
||||
This option was necessary when placement was part of nova but can now be
|
||||
replaced with the more standard usage of ``[oslo_policy]/policy_file`` which
|
||||
has the same semantics but a different default value. If you have a custom
|
||||
policy.yaml file and were not setting this option but just relying on the
|
||||
default value, you need to configure placement to use
|
||||
``[oslo_policy]/policy_file`` with policy.yaml specifically since otherwise
|
||||
that option defaults to policy.json.
|
||||
""",
|
||||
help='The file that defines placement policies. This can be an '
|
||||
'absolute path or relative to the configuration file.'),
|
||||
cfg.StrOpt(
|
||||
|
@ -13,6 +13,7 @@
|
||||
|
||||
from oslo_config import cfg
|
||||
from oslo_log import log as logging
|
||||
from oslo_policy import opts as policy_opts
|
||||
from oslo_policy import policy
|
||||
from oslo_utils import excutils
|
||||
|
||||
@ -36,13 +37,43 @@ def init(conf):
|
||||
"""Init an Enforcer class. Sets the _ENFORCER global."""
|
||||
global _ENFORCER
|
||||
if not _ENFORCER:
|
||||
# NOTE(mriedem): We have to explicitly pass in the
|
||||
# [placement]/policy_file path because otherwise oslo_policy defaults
|
||||
# to read the policy file from config option [oslo_policy]/policy_file
|
||||
# which is used by nova. In other words, to have separate policy files
|
||||
# for placement and nova, we have to use separate policy_file options.
|
||||
_enforcer = policy.Enforcer(
|
||||
conf, policy_file=conf.placement.policy_file)
|
||||
# TODO(mriedem): This compat code can be removed when the
|
||||
# [placement]/policy_file config option is removed.
|
||||
# First check to see if [oslo_policy]/policy_file exists since that's
|
||||
# what we want people using. That option defaults to policy.json while
|
||||
# [placement]/policy_file defaults to policy.yaml so if
|
||||
# [oslo_policy]/policy_file does not exist it means either someone with
|
||||
# custom policy has not migrated or they are using defaults in code.
|
||||
if conf.find_file(conf.oslo_policy.policy_file):
|
||||
# [oslo_policy]/policy_file exists so use it.
|
||||
policy_file = conf.oslo_policy.policy_file
|
||||
# Do a sanity check to see if [placement]/policy_file exists but
|
||||
# with a different name because if so we could be loading up the
|
||||
# wrong file. For example, maybe someone's packaging or deployment
|
||||
# tooling creates an empty policy.json but placement.conf is
|
||||
# actually configured to use [placement]/policy_file=policy.yaml
|
||||
# with custom rules.
|
||||
if (conf.placement.policy_file != conf.oslo_policy.policy_file and
|
||||
conf.find_file(conf.placement.policy_file)):
|
||||
LOG.error('Found [oslo_policy]/policy_file and '
|
||||
'[placement]/policy_file and not sure which to use. '
|
||||
'Using [oslo_policy]/policy_file since '
|
||||
'[placement]/policy_file is deprecated but you need '
|
||||
'to clean up your configuration file to stop using '
|
||||
'[placement]/policy_file.')
|
||||
else:
|
||||
# Check to see if a custom [placement]/policy_file is being used
|
||||
# and if so, log a warning to migrate to [oslo_policy]/policy_file.
|
||||
if conf.find_file(conf.placement.policy_file):
|
||||
LOG.warning('[placement]/policy_file is deprecated. Use '
|
||||
'[oslo_policy]/policy_file instead.')
|
||||
# For backward compatibility use [placement]/policy_file. Even if
|
||||
# the file does not exist we can specify this since we will load up
|
||||
# default rules from code. Once we remove the compat code we can
|
||||
# just stop passing the policy_file kwarg to Enforcer.
|
||||
policy_file = conf.placement.policy_file
|
||||
|
||||
_enforcer = policy.Enforcer(conf, policy_file=policy_file)
|
||||
_enforcer.register_defaults(policies.list_rules())
|
||||
_enforcer.load_rules()
|
||||
_ENFORCER = _enforcer
|
||||
@ -53,6 +84,7 @@ def get_enforcer():
|
||||
# files from overrides on disk and defaults in code. We can just pass an
|
||||
# empty list and let oslo do the config lifting for us.
|
||||
cfg.CONF([], project='placement')
|
||||
policy_opts.set_defaults(cfg.CONF)
|
||||
return _get_enforcer(cfg.CONF)
|
||||
|
||||
|
||||
|
@ -29,9 +29,8 @@ class PolicyFixture(fixtures.Fixture):
|
||||
"""Load the default placement policy for tests."""
|
||||
def setUp(self):
|
||||
super(PolicyFixture, self).setUp()
|
||||
policy_file = paths.state_path_def(
|
||||
'etc/placement/placement-policy.yaml')
|
||||
self.conf_fixture.config(group='placement', policy_file=policy_file)
|
||||
policy_file = paths.state_path_def('etc/placement/policy.yaml')
|
||||
self.conf_fixture.config(group='oslo_policy', policy_file=policy_file)
|
||||
placement_policy.reset()
|
||||
placement_policy.init(self.conf_fixture.conf)
|
||||
self.addCleanup(placement_policy.reset)
|
||||
|
@ -10,6 +10,7 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import mock
|
||||
import os
|
||||
|
||||
import fixtures
|
||||
@ -43,15 +44,22 @@ class PlacementPolicyTestCase(base.ContextTestCase):
|
||||
self.addCleanup(policy.reset)
|
||||
|
||||
def test_modified_policy_reloads(self):
|
||||
"""Creates a temporary placement-policy.yaml file and tests
|
||||
"""Creates a temporary policy.yaml file and tests
|
||||
authorizations against a fake rule between updates to the physical
|
||||
policy file.
|
||||
"""
|
||||
tempdir = self.useFixture(fixtures.TempDir())
|
||||
tmpfilename = os.path.join(tempdir.path, 'placement-policy.yaml')
|
||||
tmpfilename = os.path.join(tempdir.path, 'policy.yaml')
|
||||
|
||||
self.conf_fixture.config(
|
||||
group='placement', policy_file=tmpfilename)
|
||||
group='oslo_policy', policy_file=tmpfilename)
|
||||
|
||||
# We have to create the file before initializing the policy enforcer
|
||||
# otherwise it falls back to using CONF.placement.policy_file. This
|
||||
# can be removed when the deprecated CONF.placement.policy_file option
|
||||
# is removed.
|
||||
with open(tmpfilename, "w") as policyfile:
|
||||
policyfile.write('# The policy file is empty.')
|
||||
|
||||
action = 'placement:test'
|
||||
|
||||
@ -82,3 +90,122 @@ class PlacementPolicyTestCase(base.ContextTestCase):
|
||||
self.assertFalse(
|
||||
policy.authorize(
|
||||
self.ctxt, 'placement', self.target, do_raise=False))
|
||||
|
||||
@mock.patch('placement.policy.LOG.warning')
|
||||
def test_default_fallback_placement_policy_file_no_exist(self, mock_warn):
|
||||
"""Tests that by default the policy enforcer will fallback to the
|
||||
[placement]/policy_file when [oslo_policy]/policy_file does not
|
||||
exist. In this case the placement policy file does not exist so no
|
||||
warning about using it should be logged.
|
||||
"""
|
||||
# Make sure oslo_policy and placement use different policy_file
|
||||
# defaults (the former uses policy.json, the latter uses policy.yaml).
|
||||
config = self.conf_fixture.conf
|
||||
self.assertNotEqual(config.oslo_policy.policy_file,
|
||||
config.placement.policy_file)
|
||||
enforcer = policy._get_enforcer(config)
|
||||
self.assertEqual(config.placement.policy_file, enforcer.policy_file)
|
||||
# There should not be a warning logged since the policy file does not
|
||||
# actually exist.
|
||||
mock_warn.assert_not_called()
|
||||
|
||||
@mock.patch('placement.policy.LOG.warning')
|
||||
def test_default_fallback_placement_policy_file(self, mock_warn):
|
||||
"""Tests that by default the policy enforcer will fallback to the
|
||||
[placement]/policy_file when [oslo_policy]/policy_file does not
|
||||
exist. In this case the plcaement policy file exists, like in the case
|
||||
of using it to define custom rules, so a warning is logged.
|
||||
"""
|
||||
tempdir = self.useFixture(fixtures.TempDir())
|
||||
tmpfilename = os.path.join(tempdir.path, 'policy.yaml')
|
||||
self.conf_fixture.config(group='placement', policy_file=tmpfilename)
|
||||
# We have to create the file before initializing the policy enforcer
|
||||
# otherwise it falls back to using CONF.placement.policy_file. This
|
||||
# can be removed when the deprecated CONF.placement.policy_file option
|
||||
# is removed.
|
||||
with open(tmpfilename, "w") as policyfile:
|
||||
policyfile.write('# I would normally have custom rules in here.')
|
||||
config = self.conf_fixture.conf
|
||||
enforcer = policy._get_enforcer(config)
|
||||
self.assertEqual(config.placement.policy_file, enforcer.policy_file)
|
||||
# There should not be a warning logged since the policy file does not
|
||||
# actually exist.
|
||||
mock_warn.assert_called_once_with(
|
||||
'[placement]/policy_file is deprecated. Use '
|
||||
'[oslo_policy]/policy_file instead.')
|
||||
|
||||
@mock.patch('placement.policy.LOG.error')
|
||||
def test_init_from_oslo_policy_file_exists_same_policy_file_name(
|
||||
self, mock_log_error):
|
||||
"""Tests a scenario where the [oslo_policy]/policy_file exists and
|
||||
is the same name as the [placement]/policy_file so no error is logged
|
||||
since we'll use the file from oslo_policy config.
|
||||
"""
|
||||
# Configure [oslo_policy]/policy_file and [placement]/policy_file with
|
||||
# the same name.
|
||||
tempdir = self.useFixture(fixtures.TempDir())
|
||||
tmpfilename = os.path.join(tempdir.path, 'policy.yaml')
|
||||
self.conf_fixture.config(group='oslo_policy', policy_file=tmpfilename)
|
||||
self.conf_fixture.config(group='placement', policy_file=tmpfilename)
|
||||
# Create the [oslo_policy]/policy_file.
|
||||
with open(tmpfilename, "w") as policyfile:
|
||||
policyfile.write('# Assume upgrade with existing custom policy.')
|
||||
config = self.conf_fixture.conf
|
||||
policy._get_enforcer(config)
|
||||
# Checking what the Enforcer is using for a policy file does not really
|
||||
# matter too much since they are pointing at the same file, just make
|
||||
# sure we did not log an error.
|
||||
mock_log_error.assert_not_called()
|
||||
|
||||
@mock.patch('placement.policy.LOG.error')
|
||||
def test_init_from_oslo_file_exists_different_name_no_placement_file(
|
||||
self, mock_log_error):
|
||||
"""Tests a scenario where the [oslo_policy]/policy_file exists and
|
||||
has a different name from the [placement]/policy_file but the
|
||||
[placement]/policy_file does not exist so no error is logged.
|
||||
"""
|
||||
# Configure [oslo_policy]/policy_file and [placement]/policy_file with
|
||||
# different names.
|
||||
tempdir = self.useFixture(fixtures.TempDir())
|
||||
tmpfilename = os.path.join(tempdir.path, 'policy.yaml')
|
||||
self.conf_fixture.config(group='oslo_policy', policy_file=tmpfilename)
|
||||
self.conf_fixture.config(group='placement', policy_file='policy.json')
|
||||
# Create the [oslo_policy]/policy_file.
|
||||
with open(tmpfilename, "w") as policyfile:
|
||||
policyfile.write('# Assume upgrade with existing custom policy.')
|
||||
config = self.conf_fixture.conf
|
||||
enforcer = policy._get_enforcer(config)
|
||||
self.assertEqual(config.oslo_policy.policy_file, enforcer.policy_file)
|
||||
# Though the policy file names are different, the placement version
|
||||
# does not exist while the oslo policy one does so no error is logged.
|
||||
mock_log_error.assert_not_called()
|
||||
|
||||
@mock.patch('placement.policy.LOG.error')
|
||||
def test_init_from_oslo_file_exists_different_name_placement_file_exists(
|
||||
self, mock_log_error):
|
||||
"""Tests a scenario where the [oslo_policy]/policy_file exists and
|
||||
has a different name from the [placement]/policy_file and the
|
||||
[placement]/policy_file exists so an error is logged.
|
||||
"""
|
||||
# Configure [oslo_policy]/policy_file and [placement]/policy_file with
|
||||
# different names.
|
||||
tempdir = self.useFixture(fixtures.TempDir())
|
||||
oslo_name = os.path.join(tempdir.path, 'policy.yaml')
|
||||
self.conf_fixture.config(group='oslo_policy', policy_file=oslo_name)
|
||||
placement_name = os.path.join(tempdir.path, 'placement-policy.yaml')
|
||||
self.conf_fixture.config(group='placement', policy_file=placement_name)
|
||||
# Create the [oslo_policy]/policy_file.
|
||||
with open(oslo_name, "w") as oslo_policy_file:
|
||||
oslo_policy_file.write('# New oslo policy config.')
|
||||
# Create the [placement]/policy_file.
|
||||
with open(placement_name, "w") as placement_policy_file:
|
||||
placement_policy_file.write('# Old placement policy file.')
|
||||
config = self.conf_fixture.conf
|
||||
enforcer = policy._get_enforcer(config)
|
||||
self.assertEqual(config.oslo_policy.policy_file, enforcer.policy_file)
|
||||
# An error should be logged since we're going to use the oslo policy
|
||||
# file but there is a placement policy file with a different name that
|
||||
# also exists.
|
||||
mock_log_error.assert_called_once()
|
||||
self.assertIn('you need to clean up your configuration file',
|
||||
mock_log_error.call_args[0][0])
|
||||
|
@ -21,7 +21,6 @@ import os.path
|
||||
from oslo_config import cfg
|
||||
from oslo_log import log as logging
|
||||
from oslo_middleware import cors
|
||||
from oslo_policy import opts as policy_opts
|
||||
from oslo_utils import importutils
|
||||
import pbr.version
|
||||
|
||||
@ -79,10 +78,6 @@ def _parse_args(config, argv, default_config_files):
|
||||
|
||||
_set_middleware_defaults()
|
||||
|
||||
# This is needed so we can check [oslo_policy]/enforce_scope in the
|
||||
# deploy module.
|
||||
policy_opts.set_defaults(config)
|
||||
|
||||
config(argv[1:], project='placement',
|
||||
version=version_info.version_string(),
|
||||
default_config_files=default_config_files)
|
||||
|
@ -0,0 +1,11 @@
|
||||
---
|
||||
deprecations:
|
||||
- |
|
||||
The ``[placement]/policy_file`` configuration option is deprecated and its
|
||||
usage is being replaced with the more standard
|
||||
``[oslo_policy]/policy_file`` option. If you do not override policy with
|
||||
custom rules you will have nothing to do. If you do override the placement
|
||||
default policy then you will need to update your configuration to use the
|
||||
``[oslo_policy]/policy_file`` option. By default, the
|
||||
``[oslo_policy]/policy_file`` option will be used if the file it points at
|
||||
exists.
|
Loading…
x
Reference in New Issue
Block a user