-----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----- __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
