On 20 April 2015 at 10:03, Ihar Hrachyshka <ihrac...@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 04/17/2015 07:49 PM, Salvatore Orlando wrote: > > == 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.) > > > > > >> Roles are normally populated by the keystone middleware. I'm not > >> sure whether we want to drop them altogether from the context, > >> but I would expect the policy engine to use them even after > >> switching to oslo.policy - Plugins should never leverage this > >> kind of information. I am indeede tempted to drop roles and other > >> AAA-related info from the context when it's dispatched to the > >> plugin. This should be done carefully - beyond breaking some > >> plugins it might also impact the notifiers we use to communicate > >> with nova. > > > >> The function which artificially populates roles in the context > >> was built for artificial contextes, which in some cases were > >> created by plugins performing db operations at startup. I would > >> check if we still have this requirement, and if not remove the > >> function. > > > > Ouch, I failed in wording above (ETOOMANYWORDS?). I only meant > dropping explicit admin role population from the context object. If > there are any reasons to drop the whole attribute, they are irrelevant > to oslo.policy adoption discussion, and are worth a separate thread, > if at all. Thanks for keeping me honest on the non-sense above! > Nah it was me being ambiguous in my reply. We need roles in the context. Otherwise most policy checks would not be applicable anymore. I was making a point that a function that loads and stores them in context generated with operation like get_admin_context or ContextBase.elevated is not needed anymore. Indeed there is a note in the code pointing that out [1]. And here's the patch for doing it [2] (needs some more work) [1] http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py#n472 [2] https://review.openstack.org/#/c/175238/1 > > > > > > > == 3. aggregating core, attribute and subattribute policies == > > > > [...] > > >> Policies on subattributes are an abomination of nature and they > >> should go. > > Not sure they can easily go now, without breaking existing setups. I > wouldn't require existing deployments to convert their policies unless > we are completely locked otherwise. > Sure. That was just the personal opinion of the developer who was asked to introduce them. > >> The problem however is that this needs first a rethink about > >> some API extensions - namely the one for external gateway modes. > >> However, as you say we can't reablockedlly do without policies on > >> attributes at the moment. Policies like the following: > > > >> "create_subnetpool:shared": "rule:admin_only" > > > >> Led us to implement [1], which uses the "symbols" which were now > >> made private. > > You probably forgot to define [1] in your email. At least [1] in the > original email seems irrelevant to attribute matching in neutron. > I was pointing out this (no reference to avoid confusion): http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py#n152 > > >> That logic is specific for Neutron, which adds semantic value to > >> the policy target. As Ihar says, imposing this on all the other > >> projects might not be welcome and in some cases break the project > >> themselves. > > > > > > 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. > > > > > >> Making this possilbe would be the quickest path for Neutron. > >> However if the oslo_policy team took this decision it must have > >> been for a solid design reasoning. It is tough to ask to revise a > >> design decision for a single user of a library. Unfortunately a > >> particular Neutron developer took the liberty of playing with > >> authZ policies - I bet he was even proud of what he did: leaving > >> the project with more technical debt. That particular developer > >> should be dealt with appropriately, but this is another story. > > > > Do we need a separate neutron-troika gerrit group to handle those cases? > > >> I think that the only alternative to making those symbols public > >> in oslo_policy is for Neutron to perform attribute and > >> sub-attribute authZ checks in a different fashion (perhaps an > >> additional engine). This will be a backward incompatible > >> configuration change as deployers will have to replace their > >> policy.json file. Probably scripts might be provided to ease the > >> transition, but it's not going to be simple. > > > > Not sure why you think that it would require policy.json conversion. > We could just validate actions against their basic rules (like > 'create_network: ...') via oslo.policy, and add a separate > neutron-specific matching just for attribute and sub-attribute matching. > This is what I meant. policy.json is a configuration file, and it would be changed in a backward incompatible way. If any operator did any change to attribute-level policies, this change is likely to cause a breakage for them. > > If we get to that point of despair, we are maybe better off just > dropping oslo.policy engine completely, now that we would need to > maintain our own anyway. > > That said, I really hope we don't consider this option outside this > thread, and instead claim everyone proposing things like that being a > war criminal and a traitor (again, neutron-troika would be of help). > Are you suggesting I am a criminal and a traitor? I'm off to Mexico then ;) > > /Ihar > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2 > > iQEcBAEBAgAGBQJVNLLnAAoJEC5aWaUY1u57RhAH/2vzlaojHk/89NE+t92IETOx > OcRdheOyuwzCNxQaYip+BqWVLgjuLo7OHuirtPdDjhpBptNxEy+EwpPY8VG65bEm > YQhCIegTZbB0RqrosUMP6RRvBraxJhyPS+Uvona2HVOaqiFoh7TBT2JO2mZY/Ohu > dJ3TNqXDF5o0tez60XfWSJ58SGklqoFdypy703ZPaODJ8pWlCpb9oo79r8+Dgqzk > tI7Zp3RDZ8ZuHsVSoaIF/eS30+UZV2riBZ9AFiJ80mON7mDuVD1rOz5CuWaM6WPD > tkPKlPFKN0zmcswb1NZRtR17U3Ao7ajysj/D7VNARQX7HFNntjRjPzGL3OiyExI= > =7hmF > -----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 >
__________________________________________________________________________ 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