From c3868371212597069e4614d9ae05fe7cd0358ca1 Mon Sep 17 00:00:00 2001 From: Ben Nemec Date: Mon, 17 Feb 2020 16:17:34 +0000 Subject: [PATCH] Temporarily make namespace arg optional In order to fix the referenced bug, we need to register cli args on the global config object. Unfortunately, that causes issues because our consumers are re-calling the conf object in their enforcers due to the way we used to handle cli args. Specifically, the conf call in the consumer fails because the namespace arg from oslo.policy is registered as required, but they don't pass it to the conf call. Long-term we want to stop having consumers call the conf object at all, but in the meantime we need to provide a migration path that doesn't break them. This change registers the namespace arg as optional on the conf object and temporarily moves the required check to oslo.policy. This will allow us to maintain the existing behavior for our cli tools while not breaking consumers who haven't migrated to the new cli arg behavior. Note that we do have unit test coverage of this behavior[0], so we can be reasonably confident the explicit check is maintaining compatibility. Change-Id: I34ce1dd15c464bec319e51d3e217e26554f1a944 Closes-Bug: 1863637 Related-Bug: 1849518 0: https://github.com/openstack/oslo.policy/blob/6e2fe3857367eb2b3e2d2e92121a408e1ff89ea4/oslo_policy/tests/test_generator.py#L500 --- oslo_policy/generator.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py index fad156e8..fd9f1591 100644 --- a/oslo_policy/generator.py +++ b/oslo_policy/generator.py @@ -31,7 +31,6 @@ GENERATOR_OPTS = [ RULE_OPTS = [ cfg.MultiStrOpt('namespace', - required=True, help='Option namespace(s) under "oslo.policy.policies" in ' 'which to query for options.'), cfg.StrOpt('format', @@ -42,7 +41,6 @@ RULE_OPTS = [ ENFORCER_OPTS = [ cfg.StrOpt('namespace', - required=True, help='Option namespace under "oslo.policy.enforcer" in ' 'which to look for a policy.Enforcer.'), ] @@ -334,6 +332,17 @@ def on_load_failure_callback(*args, **kwargs): raise +def _check_for_namespace_opt(conf): + # NOTE(bnemec): This opt is required, but due to lp#1849518 we need to + # make it optional while our consumers migrate to the new method of + # parsing cli args. Making the arg itself optional and explicitly checking + # for it in the tools will allow us to migrate projects without breaking + # anything. Once everyone has migrated, we can make the arg required again + # and remove this check. + if conf.namespace is None: + raise cfg.RequiredOptError('namespace', 'DEFAULT') + + def generate_sample(args=None, conf=None): logging.basicConfig(level=logging.WARN) # Allow the caller to pass in a local conf object for unit testing @@ -342,6 +351,7 @@ def generate_sample(args=None, conf=None): conf.register_cli_opts(GENERATOR_OPTS + RULE_OPTS) conf.register_opts(GENERATOR_OPTS + RULE_OPTS) conf(args) + _check_for_namespace_opt(conf) _generate_sample(conf.namespace, conf.output_file, conf.format) @@ -351,6 +361,7 @@ def generate_policy(args=None): conf.register_cli_opts(GENERATOR_OPTS + ENFORCER_OPTS) conf.register_opts(GENERATOR_OPTS + ENFORCER_OPTS) conf(args) + _check_for_namespace_opt(conf) _generate_policy(conf.namespace, conf.output_file) @@ -377,6 +388,7 @@ def upgrade_policy(args=None, conf=None): conf.register_cli_opts(GENERATOR_OPTS + RULE_OPTS + UPGRADE_OPTS) conf.register_opts(GENERATOR_OPTS + RULE_OPTS + UPGRADE_OPTS) conf(args) + _check_for_namespace_opt(conf) with open(conf.policy, 'r') as input_data: policies = policy.parse_file_contents(input_data.read()) default_policies = get_policies_dict(conf.namespace) @@ -404,4 +416,5 @@ def list_redundant(args=None): conf.register_cli_opts(ENFORCER_OPTS) conf.register_opts(ENFORCER_OPTS) conf(args) + _check_for_namespace_opt(conf) _list_redundant(conf.namespace)