[openstack-dev] [oslo][policy][neutron] oslo.policy API is not powerful enough to switch Neutron to it

Adam Young ayoung at redhat.com
Mon Apr 20 20:19:54 UTC 2015

On 04/17/2015 08:45 AM, Ihar Hrachyshka wrote:
> Hash: SHA1
> Hi,
> tl;dr neutron has special semantics for policy targets that relies on
> private symbols from oslo.policy, and it's impossible to introduce
> this semantics into oslo.policy itself due to backwards compatibility
> concerns, meaning we need to expose some more symbols as part of
> public API for the library to facilitate neutron switch to it.
> ===
> oslo.policy was graduated during Kilo [1]. Neutron considered the
> switch to it [2], but failed to achieve it because some library
> symbols that were originally public (or at least looked like public)
> in policy.py from oslo-incubator, became private in oslo.policy.
> Specifically, Neutron policy code [3] relies on the following symbols
> that are now hidden inside oslo_policy._checks (note the underscore in
> the name of the module that suggests we cannot use the module directly):
> - - RoleCheck
> - - RuleCheck
> - - AndCheck
> Those symbols are used for the following matters:
> (all the relevant neutron code is in neutron/policy.py)
> 1. debug logging in case policy does not authorize an action
> (RuleCheck, AndCheck) [log_rule_list]
> 2. filling in admin context with admin roles (RoleCheck, RuleCheck,
> AndCheck/OrCheck internals) [get_admin_roles]
> 3. aggregating core, attribute and subattribute policies (RuleCheck,
> AndCheck) [_prepare_check]
If we make the checks public, is that sufficient?  I have no problem 
with most of these being public.
The only one that is Keystone specific is the Role check, which I would 
like to rework.  Instead of RoleCheck, can you build on top of  the 
newly submitted ability to do List checks?


Rule, And, Or, Not and Generic checks are building blocks for other 
logic, and should be public.

> == 1. debug logging in case policy does not authorize an action ==
> Neutron logs rules that failed to match if policy module does not
> authorize an action. Not sure whether Neutron developers really want
> to have those debug logs, and whether we cannot just kill them to
> avoid this specific usage of private symbols; though it also seems
> that we could easily use __str__ that is present for all types of
> Checks instead. So it does not look like a blocker for the switch.
> == 2. filling in admin context with admin roles ==
> Admin context object is filled with .roles attribute that is a list of
> roles considered granting admin permissions [4]. The attribute would
> then be used by plugins that would like to do explicit policy checks.
> As per Salvatore, this attribute can probably be dropped now that all
> plugins and services don't rely on it (Salvatore mentioned lbaas
> mixins as the ones that previously relied on it, but are now not doing
> it since service split from neutron tree (?)).
> The problem with dropping the .roles attribute from context object in
> Liberty is that we, as a responsible upstream with lots of plugins
> maintained out-of-tree (see the ongoing vendor decomposition effort)
> would need to support the attribute while it's marked as deprecated
> for at least one cycle, meaning that if we don't get those oslo.policy
> internals we rely on in Liberty, we would need to postpone the switch
> till Mizzle, or rely on private symbols during the switch (while a new
> release of oslo.policy can easily break us).
> (BTW the code to extract admin roles is not really robust and has
> bugs, f.e. it does not handle AndChecks that could be used in
> context_is_admin. In theory, 'and' syntax would mean that both roles
> are needed to claim someone is an admin, while the code to extract
> admin roles handles 'and' the same way as 'or'. For the deprecation
> time being, we may need to document this limitation.)
> == 3. aggregating core, attribute and subattribute policies ==
> That's the most interesting issue.
> For oslo.policy, policies are described as "target: rule", where rule
> is interpreted as per registered checks, while target is opaque to the
> library.
> Neutron extended the syntax for target as:
> target[:attribute[:subattribute]].
> If attribute is present in a policy entry, it applies to target iff
> attribute is set, and 'enforce_policy' is set in attribute map for the
> attribute in question, and target is not read-only (=its name does not
> start from "get_").
> If subattribute is present, the rule applies to target if 'validate'
> is set in attribute map for the attribute, and its type is dict, plus
> all the requirements for :attribute described above.
> Note that those rules are *aggregated* into single matching rule with
> AndCheck, so f.e. if action is create_network, and provider is set in
> target, then the actual rule validated would be all the rules for
> create_network, create_network:provider, and e.g.
> create_network:provider:physical_network, joined into single rule with
> AndCheck (meaning, target should conform to all of those requirements).
> This stands for a significant extension of original oslo.policy intent.
> Originally, I thought that we would be able to introduce neutron
> policy semantics into oslo.policy, and just switch to it once it's
> there. But there is a problem with that approach. Other projects (like
> nova [5]) already use similar syntax for their policy targets, while
> not putting such semantics on top of what oslo.policy provides (which
> is basically nothing, for target is not interpreted in any special
> way). AFAIU the way those projects use this syntax does not introduce
> any new *meaning*, so it's used for mere convenience to split policy
> file into logical parts (or namespaces). It means that if we would
> introduce neutron semantics into oslo.policy, that would interfere
> with other consumers of the library that rely on the fact that target
> is opaque for policy mechanism, probably breaking their policies,
> requiring users to switch to new syntax, or even silently exposing
> actions to unauthorized users.
> AFAIU that leaves Neutron with its custom crafted semantics to
> maintain outside oslo.policy. On the other side, we cannot just drop
> this semantics from Neutron, otherwise we would break existing setups
> that may rely on it. Meaning, we still need access to RuleCheck and
> AndCheck to switch to the library.
> So the question to oslo.policy maintainers is: whether all that is
> said above makes sense, and if so, whether we may now consider
> exposing those private symbols (+ maybe OrCheck, NotCheck, and other
> primitives that are logically bound to AndCheck) as part of public
> API. If community agrees with my analysis and justification for the
> change, I am happy to propose a patch that would do just that.
> PS: one more question to oslo.policy maintainers is whether consuming
> projects may rely on Enforcer.rules attribute being present, and
> following dict semantics, as it currently does. Neutron and other
> projects (like Nova) relies on the attribute, while it's not really
> documented in official library documentation. We may need
> clarification on whether its usage is supported, and document it
> appropriately.
> [1]: https://review.openstack.org/#/c/140161/
> [2]:
> http://lists.openstack.org/pipermail/openstack-dev/2015-January/054753.h
> tml
> [3]:
> http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py
> [4]:
> http://git.openstack.org/cgit/openstack/neutron/tree/neutron/context.py#
> n71
> [5]:
> http://git.openstack.org/cgit/openstack/nova/tree/etc/nova/policy.json#n
> 33
> Thanks,
> /Ihar
> Version: GnuPG v2
> iQEcBAEBAgAGBQJVMQCGAAoJEC5aWaUY1u57pGMH/j2dq6kqFlofrvz8uDpiv8Gr
> O3YhzbhfO/zkNkfS/N/bS48QmxsVOY6mAvSZ/lJwMteGU2EDipP8zVnSKIwnk9rX
> Wh2NWiPOT+dRkC/a8XCtT10n9gdz2yOcenB2ezEvRHNmKNjM76hSVT0RO8tk05+R
> K/n2ANQcRTY9q1WzC+w8xTSc1CpvbFY+1k0DqQqKZW1JInl0kFyxzTggtUxuIz4o
> j4epSiFL8Z4GkXJ51GVpDj5vQA+pgvYfieSnrF23VwF6gV1BD4uxccOtegrDZx+C
> b2L/9dQsAexBxp8x9Xm798lkhMjKE52GV9PrtMr0zLRMlJFDZSPbmramlthUh+I=
> =cr7A
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

More information about the OpenStack-dev mailing list