Open Stack

-----BEGIN PGP SIGNED MESSAGE-----
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]


== 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
-----BEGIN PGP SIGNATURE-----
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
-----END PGP SIGNATURE-----