On Fri, Sep 28, 2018 at 2:54 PM Lance Bragstad <lbrags...@gmail.com> wrote: > > > On Fri, Sep 28, 2018 at 1:03 PM Harry Rybacki <hryba...@redhat.com> wrote: >> >> On Fri, Sep 28, 2018 at 1:57 PM Morgan Fainberg >> <morgan.fainb...@gmail.com> wrote: >> > >> > Ideally I would like to see it in the form of least specific to most >> > specific. But more importantly in a way that there is no additional >> > delimiters between the service type and the resource. Finally, I do not >> > like the change of plurality depending on action type. >> > >> > I propose we consider >> > >> > <service-type>:<resource>:<action>[:<subaction>] >> > >> > Example for keystone (note, action names below are strictly examples I am >> > fine with whatever form those actions take): >> > identity:projects:create >> > identity:projects:delete >> > identity:projects:list >> > identity:projects:get >> > >> > It keeps things simple and consistent when you're looking through >> > overrides / defaults. >> > --Morgan >> +1 -- I think the ordering if `resource` comes before >> `action|subaction` will be more clean. > > > ++ > > These are excellent points. I especially like being able to omit the > convention about plurality. Furthermore, I'd like to add that I think we > should make the resource singular (e.g., project instead or projects). For > example: > > compute:server:list > compute:server:update > compute:server:create > compute:server:delete > compute:server:action:reboot > compute:server:action:confirm_resize (or confirm-resize) > > Otherwise, someone might mistake compute:servers:get, as "list". This is > ultra-nick-picky, but something I thought of when seeing the usage of > "get_all" in policy names in favor of "list." > > In summary, the new convention based on the most recent feedback should be: > > <service-type>:<resource>:<action>[:<subaction>] > > Rules: > > service-type is always defined in the service types authority > resources are always singular > ++ plurality can be determined by related action. +++ for removing possible ambiguity.
> Thanks to all for sticking through this tedious discussion. I appreciate it. > Thanks for pushing the conversation, Lance! >> >> >> /R >> >> Harry >> > >> > On Fri, Sep 28, 2018 at 6:49 AM Lance Bragstad <lbrags...@gmail.com> wrote: >> >> >> >> Bumping this thread again and proposing two conventions based on the >> >> discussion here. I propose we decide on one of the two following >> >> conventions: >> >> >> >> <service-type>:<action>:<resource> >> >> >> >> or >> >> >> >> <service-type>:<action>_<resource> >> >> >> >> Where <service-type> is the corresponding service type of the project >> >> [0], and <action> is either create, get, list, update, or delete. I think >> >> decoupling the method from the policy name should aid in consistency, >> >> regardless of the underlying implementation. The HTTP method specifics >> >> can still be relayed using oslo.policy's DocumentedRuleDefault object [1]. >> >> >> >> I think the plurality of the resource should default to what makes sense >> >> for the operation being carried out (e.g., list:foobars, create:foobar). >> >> >> >> I don't mind the first one because it's clear about what the delimiter is >> >> and it doesn't look weird when projects have something like: >> >> >> >> <service-type>:<action>:<subaction>:<resource> >> >> >> >> If folks are ok with this, I can start working on some documentation that >> >> explains the motivation for this. Afterward, we can figure out how we >> >> want to track this work. >> >> >> >> What color do you want the shed to be? >> >> >> >> [0] https://service-types.openstack.org/service-types.json >> >> [1] >> >> https://docs.openstack.org/oslo.policy/latest/reference/api/oslo_policy.policy.html#default-rule >> >> >> >> On Fri, Sep 21, 2018 at 9:13 AM Lance Bragstad <lbrags...@gmail.com> >> >> wrote: >> >>> >> >>> >> >>> On Fri, Sep 21, 2018 at 2:10 AM Ghanshyam Mann <gm...@ghanshyammann.com> >> >>> wrote: >> >>>> >> >>>> ---- On Thu, 20 Sep 2018 18:43:00 +0900 John Garbutt >> >>>> <j...@johngarbutt.com> wrote ---- >> >>>> > tl;dr+1 consistent names >> >>>> > I would make the names mirror the API... because the Operator >> >>>> setting them knows the API, not the codeIgnore the crazy names in Nova, >> >>>> I certainly hate them >> >>>> >> >>>> Big +1 on consistent naming which will help operator as well as >> >>>> developer to maintain those. >> >>>> >> >>>> > >> >>>> > Lance Bragstad <lbrags...@gmail.com> wrote: >> >>>> > > I'm curious if anyone has context on the "os-" part of the format? >> >>>> > >> >>>> > My memory of the Nova policy mess...* Nova's policy rules >> >>>> traditionally followed the patterns of the code >> >>>> > ** Yes, horrible, but it happened.* The code used to have the >> >>>> OpenStack API and the EC2 API, hence the "os"* API used to expand with >> >>>> extensions, so the policy name is often based on extensions** note most >> >>>> of the extension code has now gone, including lots of related policies* >> >>>> Policy in code was focused on getting us to a place where we could >> >>>> rename policy** Whoop whoop by the way, it feels like we are really >> >>>> close to something sensible now! >> >>>> > Lance Bragstad <lbrags...@gmail.com> wrote: >> >>>> > Thoughts on using create, list, update, and delete as opposed to >> >>>> post, get, put, patch, and delete in the naming convention? >> >>>> > I could go either way as I think about "list servers" in the API.But >> >>>> my preference is for the URL stub and POST, GET, etc. >> >>>> > On Sun, Sep 16, 2018 at 9:47 PM Lance Bragstad >> >>>> <lbrags...@gmail.com> wrote:If we consider dropping "os", should we >> >>>> entertain dropping "api", too? Do we have a good reason to keep "api"?I >> >>>> wouldn't be opposed to simple service types (e.g "compute" or >> >>>> "loadbalancer"). >> >>>> > +1The API is known as "compute" in api-ref, so the policy should be >> >>>> for "compute", etc. >> >>>> >> >>>> Agree on mapping the policy name with api-ref as much as possible. >> >>>> Other than policy name having 'os-', we have 'os-' in resource name >> >>>> also in nova API url like /os-agents, /os-aggregates etc (almost every >> >>>> resource except servers , flavors). As we cannot get rid of those from >> >>>> API url, we need to keep the same in policy naming too? or we can have >> >>>> policy name like compute:agents:create/post but that mismatch from >> >>>> api-ref where agents resource url is os-agents. >> >>> >> >>> >> >>> Good question. I think this depends on how the service does policy >> >>> enforcement. >> >>> >> >>> I know we did something like this in keystone, which required policy >> >>> names and method names to be the same: >> >>> >> >>> "identity:list_users": "..." >> >>> >> >>> Because the initial implementation of policy enforcement used a >> >>> decorator like this: >> >>> >> >>> from keystone import controller >> >>> >> >>> @controller.protected >> >>> def list_users(self): >> >>> ... >> >>> >> >>> Having the policy name the same as the method name made it easier for >> >>> the decorator implementation to resolve the policy needed to protect the >> >>> API because it just looked at the name of the wrapped method. The >> >>> advantage was that it was easy to implement new APIs because you only >> >>> needed to add a policy, implement the method, and make sure you decorate >> >>> the implementation. >> >>> >> >>> While this worked, we are moving away from it entirely. The decorator >> >>> implementation was ridiculously complicated. Only a handful of keystone >> >>> developers understood it. With the addition of system-scope, it would >> >>> have only become more convoluted. It also enables a much more copy-paste >> >>> pattern (e.g., so long as I wrap my method with this decorator >> >>> implementation, things should work right?). Instead, we're calling >> >>> enforcement within the controller implementation to ensure things are >> >>> easier to understand. It requires developers to be cognizant of how >> >>> different token types affect the resources within an API. That said, >> >>> coupling the policy name to the method name is no longer a requirement >> >>> for keystone. >> >>> >> >>> Hopefully, that helps explain why we needed them to match. >> >>> >> >>>> >> >>>> >> >>>> Also we have action API (i know from nova not sure from other services) >> >>>> like POST /servers/{server_id}/action {addSecurityGroup} and their >> >>>> current policy name is all inconsistent. few have policy name >> >>>> including their resource name like >> >>>> "os_compute_api:os-flavor-access:add_tenant_access", few has 'action' >> >>>> in policy name like "os_compute_api:os-admin-actions:reset_state" and >> >>>> few has direct action name like "os_compute_api:os-console-output" >> >>> >> >>> >> >>> Since the actions API relies on the request body and uses a single HTTP >> >>> method, does it make sense to have the HTTP method in the policy name? >> >>> It feels redundant, and we might be able to establish a convention >> >>> that's more meaningful for things like action APIs. It looks like cinder >> >>> has a similar pattern [0]. >> >>> >> >>> [0] >> >>> https://developer.openstack.org/api-ref/block-storage/v3/index.html#volume-actions-volumes-action >> >>> >> >>>> >> >>>> >> >>>> May be we can make them consistent with >> >>>> <service-type>:<resource>:<action_with_snake_case> or any better >> >>>> opinion. >> >>>> >> >>>> > From: Lance Bragstad <lbrags...@gmail.com>> The topic of having >> >>>> consistent policy names has popped up a few times this week. >> >>>> > >> >>>> > I would love to have this nailed down before we go through all the >> >>>> policy rules again. In my head I hope in Nova we can go through each >> >>>> policy rule and do the following: >> >>>> > * move to new consistent policy name, deprecate existing name* >> >>>> hardcode scope check to project, system or user** (user, yes... >> >>>> keypairs, yuck, but its how they work)** deprecate in rule scope >> >>>> checks, which are largely bogus in Nova anyway* make read/write/admin >> >>>> distinction** therefore adding the "noop" role, amount other things >> >>>> >> >>>> + policy granularity. >> >>>> >> >>>> It is good idea to make the policy improvement all together and for all >> >>>> rules as you mentioned. But my worries is how much load it will be on >> >>>> operator side to migrate all policy rules at same time? What will be >> >>>> the deprecation period etc which i think we can discuss on proposed >> >>>> spec - https://review.openstack.org/#/c/547850 >> >>> >> >>> >> >>> Yeah, that's another valid concern. I know at least one operator has >> >>> weighed in already. I'm curious if operators have specific input here. >> >>> >> >>> It ultimately depends on if they override existing policies or not. If a >> >>> deployment doesn't have any overrides, it should be a relatively simple >> >>> change for operators to consume. >> >>> >> >>>> >> >>>> >> >>>> >> >>>> -gmann >> >>>> >> >>>> > Thanks,John >> >>>> __________________________________________________________________________ >> >>>> > 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 >> >> >> >> __________________________________________________________________________ >> >> 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 >> >> __________________________________________________________________________ >> 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 __________________________________________________________________________ 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
