yahoo-eng-team team mailing list archive

[Ben Nemec <openstack@xxxxxxxxxxxx>]

Public bug reported:

If I create a config file named fake.conf that looks like this:

[oslo_policy]
policy_file = /home/fedora/keystone/keystone.policy.yaml

and put a redundant rule in the referenced policy file, that rule should
get printed out when I run:

oslopolicy-list-redundant --config-file ./fake.conf --namespace keystone

Unfortunately it's not. I believe the reason is this line in the
enforcer code in keystone:
https://github.com/openstack/keystone/blob/b56af0b207748afb6a67d2bb77425028b8d98882/keystone/common/rbac_enforcer/policy.py#L43

That causes the config object to drop all cli arguments, so the
--config-file is ignored and the default search path is used. In my
case, I don't actually have a file at the default search path so the
tool silently* falls back to the default policy-in-code. This means the
tool is essentially doing nothing, but behaving as if it verified
everything.

*: Silent because it uses the default value for policy_file, and since
policy files are optional now we don't warn if the default one is
missing.

I am able to fix this by making the following changes:

diff --git a/keystone/common/rbac_enforcer/policy.py b/keystone/common/rbac_enforcer/policy.py
index 52cc86c36..3e503e59c 100644
--- a/keystone/common/rbac_enforcer/policy.py
+++ b/keystone/common/rbac_enforcer/policy.py
@@ -40,7 +40,7 @@ def get_enforcer():
     # from the CONF object. This makes things easier here because we don't have
     # to parse arguments passed in from the command line and remove unexpected
     # arguments before building a Config object.
-    CONF([], project='keystone')
+    #CONF([], project='keystone')
     return _ENFORCER._enforcer

and

diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py
index bd75389..ab12931 100644
--- a/oslo_policy/generator.py
+++ b/oslo_policy/generator.py
@@ -391,7 +391,7 @@ def upgrade_policy(args=None):
 
 def list_redundant(args=None):
     logging.basicConfig(level=logging.WARN)
-    conf = cfg.ConfigOpts()
+    conf = cfg.CONF
     conf.register_cli_opts(ENFORCER_OPTS)
     conf.register_opts(ENFORCER_OPTS)
     conf(args)

I'm not entirely sure why the CONF object was getting re-initialized in
the enforcer code in Keystone. IMHO that's the job of the entry point,
which in this case is oslo.policy. I believe Keystone is already
initializing the object itself in
https://github.com/openstack/keystone/blob/b56af0b207748afb6a67d2bb77425028b8d98882/keystone/server/__init__.py#L32
so I'm guessing it was added exclusively for oslo.policy?

** Affects: keystone
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Identity (keystone).
https://bugs.launchpad.net/bugs/1843925

Title:
  Keystone's policy enforcer breaks oslopolicy-list-redundant

Status in OpenStack Identity (keystone):
  New

Bug description:
  If I create a config file named fake.conf that looks like this:

  [oslo_policy]
  policy_file = /home/fedora/keystone/keystone.policy.yaml

  and put a redundant rule in the referenced policy file, that rule
  should get printed out when I run:

  oslopolicy-list-redundant --config-file ./fake.conf --namespace
  keystone

  Unfortunately it's not. I believe the reason is this line in the
  enforcer code in keystone:
  https://github.com/openstack/keystone/blob/b56af0b207748afb6a67d2bb77425028b8d98882/keystone/common/rbac_enforcer/policy.py#L43

  That causes the config object to drop all cli arguments, so the
  --config-file is ignored and the default search path is used. In my
  case, I don't actually have a file at the default search path so the
  tool silently* falls back to the default policy-in-code. This means
  the tool is essentially doing nothing, but behaving as if it verified
  everything.

  *: Silent because it uses the default value for policy_file, and since
  policy files are optional now we don't warn if the default one is
  missing.

  I am able to fix this by making the following changes:

  diff --git a/keystone/common/rbac_enforcer/policy.py b/keystone/common/rbac_enforcer/policy.py
  index 52cc86c36..3e503e59c 100644
  --- a/keystone/common/rbac_enforcer/policy.py
  +++ b/keystone/common/rbac_enforcer/policy.py
  @@ -40,7 +40,7 @@ def get_enforcer():
       # from the CONF object. This makes things easier here because we don't have
       # to parse arguments passed in from the command line and remove unexpected
       # arguments before building a Config object.
  -    CONF([], project='keystone')
  +    #CONF([], project='keystone')
       return _ENFORCER._enforcer

  and

  diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py
  index bd75389..ab12931 100644
  --- a/oslo_policy/generator.py
  +++ b/oslo_policy/generator.py
  @@ -391,7 +391,7 @@ def upgrade_policy(args=None):
   
   def list_redundant(args=None):
       logging.basicConfig(level=logging.WARN)
  -    conf = cfg.ConfigOpts()
  +    conf = cfg.CONF
       conf.register_cli_opts(ENFORCER_OPTS)
       conf.register_opts(ENFORCER_OPTS)
       conf(args)

  I'm not entirely sure why the CONF object was getting re-initialized
  in the enforcer code in Keystone. IMHO that's the job of the entry
  point, which in this case is oslo.policy. I believe Keystone is
  already initializing the object itself in
  https://github.com/openstack/keystone/blob/b56af0b207748afb6a67d2bb77425028b8d98882/keystone/server/__init__.py#L32
  so I'm guessing it was added exclusively for oslo.policy?

To manage notifications about this bug go to:
https://bugs.launchpad.net/keystone/+bug/1843925/+subscriptions