16:00:16 <cmurphy> #startmeeting keystone
16:00:18 <openstack> Meeting started Tue Sep 17 16:00:16 2019 UTC and is due to finish in 60 minutes.  The chair is cmurphy. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:19 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:21 <openstack> The meeting name has been set to 'keystone'
16:00:28 <cmurphy> #link https://etherpad.openstack.org/p/keystone-weekly-meeting agenda
16:00:34 <lbragstad> o/
16:02:22 <bnemec> o/
16:02:59 <gagehugo> o/
16:03:56 <cmurphy> light attendance today
16:04:00 <cmurphy> #topic announcements
16:04:48 <cmurphy> feature freeze was last week, i put procedural -2s on some unfinished spec implementations and in general no new features should be approved
16:05:16 <cmurphy> soft string freeze also in effect, i always have a hard time remembering that one
16:05:43 <cmurphy> requirements freeze also in effect so don't approve changes to requirements.txt
16:06:22 <bnemec> I'm not sure how many strings are still getting translated outside of horizon at this point.
16:06:58 <cmurphy> bnemec: interesting, i guess i haven't seen any translation bot proposals in a long time
16:08:42 <cmurphy> lastly wanted to put out a call again for ptg topics, i haven't done any arrangements since last week so the schedule is still open to expand if there are topics to discuss
16:08:54 <cmurphy> #link https://etherpad.openstack.org/p/keystone-shanghai-ptg keystone virtual ptg
16:09:42 <cmurphy> #topic RC1
16:10:42 <cmurphy> we targeted all the policy migrations for feature freeze and we mostly completed everything we had bug reports for but lbragstad pointed out that some slipped through the cracks
16:11:10 <cmurphy> i think it's not unreasonable to get those in before the stable branch is cut next week
16:11:19 <cmurphy> thoughts?
16:11:29 <lbragstad> i think that sounds good - it'll be tight
16:11:51 <lbragstad> i think it boils down to four APIs
16:11:56 <cmurphy> i think the gate is less congested than last week at least
16:12:27 <lbragstad> 1. project tags (done) 2. project endpoints (relatively simple) 3. limits (underway but needs help) and 4. role assignments for project subtrees
16:13:05 <bnemec> Check queue is long right now. :-/
16:13:49 <lbragstad> i think we can address those four areas - we will no longer have rule:admin_required used anywhere
16:14:20 <lbragstad> we can also completely remove the policy.v3cloudsample.json file along with the related testing we have for it
16:14:27 <cmurphy> lbragstad: what help do you need with limits?
16:14:38 <lbragstad> so - i was digging into a big yesterday
16:14:47 <lbragstad> i think it just needs to be consolidated into a single check string
16:14:59 <lbragstad> i'm not sure why i decided to write it into three check strings
16:15:36 <lbragstad> outside of that - i think 95% of the testing for that patch is done, it just needs a little cleanup in the APi
16:15:55 <lbragstad> cmurphy i think you had a couple comments on additional test cases that would be good to add
16:17:06 <lbragstad> so - 1. consolidate check strings and policies 2. add the missing tests 3. squash series into a single commit (including removing the limit policies from policy.v3cloudsample.json)
16:17:22 <cmurphy> sounds good
16:17:36 <cmurphy> i can take a look at the role assignments one
16:17:46 <cmurphy> i feel like i conquered the grants api last week
16:17:52 <lbragstad> ++
16:18:06 <lbragstad> all i did was grep the code base for base.RULE_ADMIN_REQUIRED
16:18:24 <cmurphy> gagehugo: any chance you have time for project endpoints?
16:18:26 <lbragstad> which returns a bunch of stuff because most hits are false positives since they're deprecated
16:19:33 <cmurphy> lbragstad: i had a comment on the project tags one, domain admins can't create/update tags for projects in their domain?
16:19:37 <lbragstad> project-endpoints are strictly system admin, member, and reader, so it should be straight forward
16:19:48 <lbragstad> cmurphy they cannot
16:19:57 <cmurphy> lbragstad: why?
16:20:07 <lbragstad> we had a design discussion during the specification that project tags are "admin-only"
16:20:28 <lbragstad> e.g., a deployment could use them for billing codes or service memberships
16:20:34 <lbragstad> (even though we don't recommend that)
16:20:51 <cmurphy> huh okay
16:20:56 <lbragstad> and giving a domain admin the ability to modify those would allow them to bump their memberships
16:21:36 <lbragstad> http://specs.openstack.org/openstack/keystone-specs/specs/keystone/queens/project-tags.html#security-impact
16:22:13 <cmurphy> "Typically, only the project admin should be able to create/edit the tags for a project"
16:22:16 <lbragstad> ^ technically - that means it shouldn't be visible to anyone but system users, but it was implemented in a way that allows domain and project users to query for tag information
16:22:18 <cmurphy> that seems like the opposite of what you said
16:22:23 <lbragstad> oh
16:22:43 <cmurphy> gagehugo: can you clarify ^
16:22:43 <lbragstad> i glossed over project and assumed system
16:23:26 <gagehugo> sorry stepped away
16:24:40 <lbragstad> policy.v3cloudsample.json only appears to let project users view tags, not edit them https://opendev.org/openstack/keystone/src/branch/master/etc/policy.v3cloudsample.json#L16-L17
16:24:51 <lbragstad> i guess that's what i was going off of ^
16:25:03 <lbragstad> but maybe i misinterpreted it
16:26:01 <cmurphy> i think that policy is in contradiction with the default policy base.RULE_ADMIN_OR_TARGET_PROJECT
16:26:08 <gagehugo> tags are an attribute of a project, so a project's users should be able to view them the same as other attributes
16:26:24 <cmurphy> what about modifying them?
16:26:32 <gagehugo> yeah that sample for get/list should be RULE_ADMIN_OR_TARGET_PROJECT
16:26:47 <cmurphy> the change in https://review.opendev.org/#/c/682503/3/keystone/common/policies/project.py would have only system admins allowed to modify
16:26:57 <cmurphy> but the spec makes it seem like project admins should be able to
16:27:05 <gagehugo> well "admin" of the project can change them already without hitting the /tags url
16:27:43 <cmurphy> ?
16:27:46 <lbragstad> because they're using old/deprecated policies?
16:27:56 <cmurphy> oh
16:27:57 <gagehugo> I mean they can do an update on the project itself
16:28:06 <gagehugo> but only that one, not "all" projects
16:28:14 <lbragstad> but - that's in tandem with old and deprecated policies?
16:28:42 <cmurphy> in a world where 968696 is closed a project admin shouldn't be able to do a PATCH /v3/projects
16:29:05 <cmurphy> i don't think?
16:29:09 <lbragstad> yeah - update_project is rule:admin_required
16:29:19 <lbragstad> er - that's the deprecated value)
16:29:36 <lbragstad> then we fixed it to be SYSTEM_ADMIN_OR_DOMAIN_ADMIN
16:29:59 <lbragstad> project users have no authorization on the PATCH /v3/project/{project_id} api
16:30:02 <lbragstad> in the new world
16:30:11 <gagehugo> yes
16:30:33 <lbragstad> and in the old world of policy, only "project administrators" can update projects (because it's misinterpreted as a cloud admin)
16:31:42 <gagehugo> yes, any projects
16:31:54 <cmurphy> so my new-world reading of http://specs.openstack.org/openstack/keystone-specs/specs/keystone/queens/project-tags.html#security-impact is that project admins can update project tags (but not projects themselves) and implicitly domain admins and system admins should as well
16:32:13 <lbragstad> so - i guess we either need to update project tag functionality to all domain and project users to modify tags, or we need to update the specification to be inline with the implementation
16:32:19 <lbragstad> cmurphy i agree with that view
16:32:31 <lbragstad> if we want the implementation to match the spec
16:32:36 <gagehugo> a project admin can update the project tags of projects that they have "admin" on, not all projects
16:33:15 <lbragstad> gagehugo i don't see how the code allows that
16:33:46 <gagehugo> PATCH /v3/projects/{project_id}
16:33:47 <lbragstad> the deprecated policies for project tags use base.RULE_ADMIN_OR_TARGET_PROJECT for only list_project_tags and get_project_tag
16:34:10 <lbragstad> oh - but that's because of bug 968696
16:34:11 <openstack> bug 968696 in OpenStack Identity (keystone) ""admin"-ness not properly scoped" [High,In progress] https://launchpad.net/bugs/968696 - Assigned to Colleen Murphy (krinkle)
16:34:11 <cmurphy> gagehugo: we're talking about once the deprecated policies are removed and the default policies don't allow that
16:34:48 <gagehugo> I mean PATCH /v3/projects/{project_id}, not PUT /v3/projects/{project_id}/tags
16:35:09 <gagehugo> a "project admin" cannot use PUT /v3/projects/{project_id}/tags on just any project anymore though
16:35:18 <cmurphy> yes the default policies minus deprecations do not allow a user with role:admin on a project to do PATCH /v3/projects/{project_id}
16:35:34 <gagehugo> oh ok
16:35:43 <lbragstad> we fixed that in stein i believe
16:35:45 <gagehugo> I misunderstood
16:36:03 <lbragstad> in the future - should project and domain users be able to modify tags to projects they're authorized on?
16:36:21 <cmurphy> ie should they be allowed to PATCH /v3/projects/{project_id}
16:36:26 <cmurphy> er
16:36:39 <cmurphy> i meant should they be allowed to PUT /v3/projects/{project_id}/tags
16:36:46 <lbragstad> ^ yes
16:36:59 <lbragstad> should they be allowed to modify tags via the tags api directly
16:37:16 <gagehugo> that was the point of adding a separate API iirc
16:37:17 <lbragstad> because i wrote https://review.opendev.org/#/c/682503/ such that they can only view tags, not modify them
16:37:31 <gagehugo> one of the points*
16:37:56 <cmurphy> good point
16:39:43 <cmurphy> lbragstad: so i think 682503 needs to be updated to allow project and domain admins to modify tags
16:39:53 <lbragstad> ok
16:39:59 <gagehugo> yeah
16:40:03 * lbragstad is digging for something quick
16:40:22 <lbragstad> do we have project tag documentation?
16:40:25 * gagehugo has slept many time since we last discussed tags
16:40:28 <gagehugo> times*
16:40:46 <gagehugo> https://docs.openstack.org/api-ref/identity/v3/#project-tags ?
16:41:01 <lbragstad> hmm
16:41:11 <lbragstad> i swear you had a patch that added security concerns somewhere
16:41:25 <gagehugo> I can look
16:41:37 <lbragstad> e.g., it was essentially a warning to operators that they shouldn't use tags for important stuff like billing codes or accounting information
16:42:21 <lbragstad> because we didn't want to have them use them for that use case since it creates a conflict of interest for domain and project users
16:43:44 <gagehugo> yeah, I remember that discussion
16:44:15 <lbragstad> i can't find that information in the spec or the api reference
16:44:27 <lbragstad> i'm not sure where it went, but i thought we merged it
16:44:33 <lbragstad> regardless, i think we have consensus?
16:44:45 <cmurphy> i think so
16:44:56 <gagehugo> how are server tags handled?
16:45:18 * lbragstad isn't sure
16:45:34 <gagehugo> think neutron has network tags too
16:46:28 <gagehugo> I think we have consensus though
16:46:39 <lbragstad> https://github.com/openstack/nova/blob/master/nova/policies/server_tags.py
16:47:05 <lbragstad> admin or owner - so it looks like they grant write operations for tags to non-admins
16:50:29 <cmurphy> okay to close out this topic i had suggested in last week's newsletter that we use today's office hours to prioritize rc1 bugs, does that sounds like a good idea or should we do that asynchronously?
16:51:20 <lbragstad> i can help
16:51:33 <gagehugo> that sounds good
16:51:41 <lbragstad> do you expect it to take a while
16:51:42 <lbragstad> ?
16:52:05 <cmurphy> i would timebox it at no more than an hour
16:52:10 <cmurphy> would hope for less
16:52:35 <lbragstad> ++
16:53:00 <cmurphy> #topic review requests
16:53:35 <cmurphy> lbragstad: you had some reviews ^
16:53:39 <lbragstad> i did
16:53:40 <lbragstad> #link https://review.opendev.org/#/c/682266/
16:54:04 <lbragstad> ^ isn't ready yet, but i plan on ripping out the policy.v3cloudsample.json file asap (pending the other things we've discussed landing first)
16:54:10 <lbragstad> #link https://review.opendev.org/#/q/I8f0f7a623a1741f461493d872849fae7ef3e8077
16:54:25 <cmurphy> will keep an eye on it
16:54:31 <lbragstad> ^ those fix system-scope when using domain-specific backends, otherwise system-scope tokens completely fail
16:54:55 <cmurphy> i approved the one on master, i don't think we can merge the stable backports until that one is merged
16:55:33 <gagehugo> looking
16:55:41 <lbragstad> ++
16:55:49 <cmurphy> I'd like reviews on https://review.opendev.org/680788 actually i think it would be best to merge that asap before we get too deep into the rest of the policy fixes
16:55:52 <cmurphy> #link https://review.opendev.org/680788
16:56:13 <lbragstad> sounds good
16:56:20 <cmurphy> #link https://review.opendev.org/682447 fixes CI for OSA
16:57:41 <lbragstad> +2 on https://review.opendev.org/#/c/680788/3
16:57:42 * gagehugo likes seeing the test times go from 50 min to 17 with the protection job
16:57:48 <lbragstad> ++
16:57:56 <cmurphy> lol
16:58:07 <cmurphy> #topic open floor
16:58:17 <cmurphy> anything else for the next two minutes?
16:59:31 <gagehugo> +2/+A on https://review.opendev.org/#/c/680788/3
16:59:46 <cmurphy> ty
17:00:06 <cmurphy> thanks guys
17:00:08 <cmurphy> #endmeeting