16:00:00 #startmeeting policy 16:00:01 Meeting started Wed Mar 1 16:00:00 2017 UTC and is due to finish in 60 minutes. The chair is lbragstad. Information about MeetBot at http://wiki.debian.org/MeetBot. 16:00:03 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 16:00:06 The meeting name has been set to 'policy' 16:00:09 ping antwash, raildo, ktychkova, dolphm, dstanek, rderose, htruta, atrmr, gagehugo, lamt, thinrichs, edmondsw, ruan, ayoung, stevemar, ravelar, morgan, raj_singh, johnthetubeguy 16:00:14 agenda #link https://etherpad.openstack.org/p/keystone-policy-meeting 16:00:16 o/ 16:00:20 o/ 16:00:22 o/ 16:00:24 o/ 16:00:31 o/ 16:00:34 o/ 16:00:54 o/ 16:01:09 Hey 16:01:23 o/ 16:01:28 o/ 16:01:46 we have a pretty good crowd today 16:02:17 o/ 16:02:50 #topic Recap policy discussions from the PTG 16:02:52 ravelar, can you maybe come up with a more descriptive name than Policy in code (part 5) 16:03:04 * johnthetubaguy lurks 16:03:08 johnthetubaguy o/ 16:03:26 ayoung are you talking about all the parts or just that one specifically? 16:03:30 ravelar, all of them 16:03:40 like...are they by subunit, I assume? 16:03:52 o/ 16:03:54 ayoung what do you prefer? They are just redundantly adding default groups 16:03:57 johnthetubaguy I totally misspelled your nick in the ping (sorry!) 16:04:08 lbragstad: heh, no worries 16:04:18 ayoung and I am just following novas convention when they made the patches on their end 16:04:20 ravelar, why are they different reviews? Is there some ordering there? 16:04:27 johnthetubaguy I pinged you in nova IRC to look at a nova policy-related patch that needs your quick review... other reviewers are holding it up waiting for you 16:04:33 lbragstad: that must be my plumber cousin 16:04:43 johnthetubaguy that's what I was thinking 16:04:49 edmondsw: yeah, stuck trying to rework some critical specs at the moment, its open in a tab 16:04:54 ayoung no - they are just broken up so the changes aren't so big 16:04:58 ayoung there are many default groups and rather than putting them in one huge patch change, I simply chained them together. 16:04:59 johnthetubaguy tx 16:05:05 and why are they going into common instead of the sub projects? 16:05:14 ayoung like here https://blueprints.launchpad.net/nova/+spec/policy-in-code 16:05:22 I'd expect to see identity/policy assignment/policy etc 16:05:37 the only stuff in common should be actual common stuff 16:05:50 ayoung isn't policy common? it was a recommendation made by dolphm 16:06:04 ravelar ++ 16:06:10 ayoung and it is in one central place like you would expect to find policy.json 16:06:13 ravelar didn't you both refactor that last friday? 16:06:23 ayoung: i think he's just asking why the individual policies are not in their respective modules 16:06:31 err, ravelar 16:06:34 ravelar, no, the policy changes you made look like each is specific to a certain submodule 16:06:35 having it in a central place makes it easier to find... which is a big help to folks that want to customize policy 16:06:44 right 16:06:56 nova's done the same with conf, incidentally 16:06:59 lbragstad yes 16:07:07 edmondsw by central you mean in keystone/common/policy/ ? 16:07:07 which is also nice 16:07:18 edmondsw, yeah, don' 16:07:21 t do that 16:07:44 edmondsw, the whole point of the submodules is to group like functionality together in a way that things can grow 16:07:50 lbragstad yes... or I'd prefer to be like nova and keystone/policies 16:08:03 FWIW, this is how ironic did it: https://github.com/openstack/ironic/blob/8db68fef4e97b2ed6552b80215ab03093f18e615/ironic/common/policy.py 16:08:04 put things from submoduiles into common means that everything should be in common 16:08:12 edmondsw: we also have keystone/policy 16:08:13 edmondsw: for /v3/policies 16:08:20 edmondsw it was originally setup that way, however when looking at it, the feedback was that we also have keystone/policy 16:08:34 I meant keystone/policies would be a directory, not a file 16:08:41 heh 16:08:46 this is similar to our conf structure though. 16:08:50 and i think that was the goal. 16:09:01 e.g. https://github.com/openstack/nova/tree/master/nova/policies 16:09:06 keystone/policy was just the datastore, though. It was agnostic of content. We wanted to kill that at one point 16:09:10 ravelar: maybe base.py should be in keystone/common, and each individual policy file should be in keystone/identity keystone/catalog etc 16:09:26 dolphm, ++ 16:09:30 that could work 16:09:39 FWIW, that caused a mess for our config 16:09:41 think in terms of people that have to trace from API calls to policy 16:09:46 dolphm wouldn't that scatter a lot of the files unnecessarily? 16:09:54 the API call starts at the router, then goes to the controller. 16:10:10 if you put them next to each other, in a central place, you can consider them as a single operator "API", but each to their own really, there are trade offs both ways 16:10:13 although, nova did nova/policies/ for everything just like nova/conf and keystone/conf 16:10:14 the logical place to look for the policy for a specific controller would be in a file right next to it 16:10:24 ravelar: it puts the actual policies closer to the code their protecting 16:10:35 yeah but controller that calls policies is in common 16:10:41 not in the respective folder 16:10:42 similar to the schema.py files of each submidle 16:10:45 ayoung dolphm we have common conf directory... why not same for policies? 16:10:46 module* 16:10:47 https://github.com/openstack/keystone/tree/master/keystone/conf 16:10:56 ravelar: it's just two different philosophies; i don't think one is necessarily better than the other (other than i'll maintain that having both keystone/policy and keystone/policies would suck) 16:11:04 policy is just a different type of conf, really 16:11:09 ... but if you want to curate the policy as a single interface the operators need to understand, you might want it in one folder 16:11:25 johnthetubaguy ++ 16:11:38 edmondsw, because with conf you tend to go from the file to the repo to find what implements that. With policy you tend to go from the router to the controller to the policy to see what is going to happen 16:11:39 dolphm: that sounds like the worst of both choices policy and policies 16:11:52 johnthetubaguy: right 16:11:53 ayoung not me... 16:12:01 ayoung I got from the file to the repo, just like conf 16:12:16 edmondsw, yes, but with policy in code, there will be no file 16:12:25 ayoung not true 16:12:29 I'm leaning towards keeping it in one folder 16:12:30 ayoung you'll generate the file 16:12:50 the policy is being called from common controller protected though right? 16:12:51 edmondsw you can't expect operators to understand routers and such to make those connections 16:13:05 and I should talk to ayoung, not myself... 16:13:05 so, one folder is fine. just name the files the same as the modules they protect 16:13:16 identity assignment and so on 16:13:36 I don't mind having them named after the resources they protect 16:13:42 me too 16:13:43 but I could go either way here 16:13:45 I mean, its wrong, but so much of Keystone is wrong, this is not even something that registers. 16:13:47 FWIW, we were aiming more towards the URL structure than the file structure, but the fact they aren't the same is a totally different problem we have 16:13:58 johnthetubaguy, +++++++ 16:14:26 that's a reason to fix the file structure, no? :) 16:14:47 I still struggle to figure out where to look in keystone's code structure for differnt things... 16:14:48 johnthetubaguy: i hate the fact that we have to consider URL structure at all 16:15:15 so, the alternative setup we could go with is to merge the routers and maybe even the controllers into a single place, and name them accordingly 16:15:59 dstanek, so I was looking at how Kubernetes does that. With Kubernets, you have a single discovery page that lists the entities, and then from each entity you get the URL. Then the URL structures are forced to be very regular 16:16:04 ayoung: you mean out of common? 16:16:09 so user, group, role, etc.... 16:16:31 move = move and have the old location import the new one with a deprecation warning. 16:16:35 rderose, I'd probable go keystone/routers 16:16:46 common is unnecessary naming for most things 16:16:55 true 16:17:29 dstanek: I am thinking about the operator, what they care about when setting things is the URL structure, at least I do 16:17:31 So...I opened with criticism, but ravelar I should have opened with a compliement on the nice work you are doing there...this is all just detail stuff 16:17:42 ++ 16:17:45 ++ 16:17:48 ++ 16:17:53 ++ 16:17:55 :) 16:17:55 johnthetubaguy, hence http://specs.openstack.org/openstack/keystone-specs/specs/keystone/ongoing/role-check-from-middleware.html 16:18:05 ravelar those details aside - i think those changes are close to merging 16:18:13 ayoung: ++ :) 16:18:16 ayoung thanks! I like the input in general 16:18:40 it would be nice to get those issues resolved in review sometime today so that ravelar and antwash can address 16:18:49 policy etherpad from PTG #link https://etherpad.openstack.org/p/pike-ptg-keystone-policy 16:18:53 ayoung: true, but then we end up with access checks in too many places, well in the Nova case at least, but thats a different discussion 16:19:07 these are the patches we have in keystone #link https://review.openstack.org/#/q/topic:bp/policy-in-code+status:open+project:openstack/keystone 16:19:26 lbragstad, I really don't care where it lands, so long as there is a well thought out rationale. If it is functional, we can also refactor in the future 16:19:39 ayoung ++ exactly, 16:19:43 johnthetubaguy: i'd love to try to figure out exactly why url structure matters so much. maybe we can have a side conversation later 16:20:22 johnthetubaguy, the RBAC check and the scope check are fundamentally different things, with different requirements. I want to make sure we keep that in mind, and don't hard code the role check into the deep policy-in-code impl 16:20:23 dstanek: operators shouldn't need to read the code, only our policy docs and the api-ref, its all stemming from that view of the world really 16:20:39 code should not know about any actual roles...that should be a configuration/operator decision 16:20:46 dstanek because the url structure is everything from the operator perspective. And as nice as comments are, I'm going to want to double-check things, which means going into the code. I need to know where to do that, and what I know is the url structure 16:20:56 ayoung: yeah, I think we have exactly the same goals 16:21:29 Of course, the horrible Nova-everything-in-one-post-api-cuz-we-love-SOAP-API approach does make it a little diffiduclt to convince people 16:21:34 but, hey 16:21:48 johnthetubaguy: edmondsw: you guys have the same opinion for the exact opposite reason. 16:21:54 :) 16:22:02 #progress 16:22:13 johnthetubaguy: i've always thought the way we document api is wrong :-) 16:23:07 dstanek: but it IS documented :P 16:23:30 for some definition of documented :P 16:23:36 ++ 16:23:50 so - after ravelar and antwash get through moving policy into code we have the documentation bits - #link https://review.openstack.org/#/c/435078/ 16:24:15 which will have to be dependent on #link https://review.openstack.org/#/c/439070/ 16:24:31 cc johnthetubaguy ^ that's the interface the sdague was talking about 16:24:49 ah, yeah, thats it 16:25:07 didn't we say we needed multiple url and verb pairs? 16:25:17 johnthetubaguy yeah - probably ? 16:25:18 BTW, ravelar have you looked at my refactorings to support is_admin? 16:25:39 johnthetubaguy antwash just posted that yesterday to start getting feedback on it 16:26:11 seee the three reviews stacked up here https://review.openstack.org/#/c/387710/ 16:26:30 johnthetubaguy any feedback nova has on that interface would be a huge help 16:27:02 sneti and aunnam should totally take a look at that ^ 16:27:33 once that is released in a new version of oslo.policy, we'll be able to tackle the documentation bits 16:27:34 I'm totes going to block anything that prevents those from merging...wrote them too long ago 16:27:47 Oct 17... 16:28:13 johnthetubaguy I just -1'ed with that comment 16:28:29 and to remove scope and access 16:28:43 and only SteveMar has reviewed them, so please just merge them 16:29:02 https://review.openstack.org/#/c/387161/12 16:29:08 https://review.openstack.org/#/c/387710/13 16:29:18 and https://review.openstack.org/#/c/257636/18 16:29:24 ayoung ah looking at that now 16:29:42 edmondsw: oops, me too, but thats cool 16:29:55 johnthetubaguy edmondsw perfect 16:30:11 johnthetubaguy is there anything else we came up with during last weeks session that needs to be in that patch that you remember? 16:30:12 ravelar, thanks, but also need people with +2 to just pull the trigger. I'm not able to actively work on this any more, and its hurting keystone, and all of openstack, to not have is_admin support in policy 16:30:47 ayoung there's a -1 from henrynash on https://review.openstack.org/#/c/257636/18 16:30:53 edmondsw, and he is still wrong 16:31:07 edmondsw, that is cuz he's thinking in terms of the cloudsample 16:31:19 ayoung cool... I didn't read his comment yet 16:31:22 we discussed, and he never re-addressed it 16:31:34 you just said nobody had reviewed them but stevemar :) 16:31:45 ayoung: as previously said, let me know when you want me to pick something up which you can't work on. 16:32:04 though i'm still unsure what the consensus for moving forward with role check in middleware is 16:32:05 edmondsw, yeah, and Sam looked at that one, too, but only steve looked at the prior 16:32:12 https://review.openstack.org/#/c/387161/12 16:32:33 its a straight refactoring, pulling code together for the is_admin check to be done in once place 16:32:48 knikolla ayoung we agreed at the PTG that role check in middleware was essentially dead now 16:33:11 edmondsw, that is stupid, but irrelevant to this. 16:33:37 ayoung I was responding to knikolla's question about that 16:33:54 edmondsw, you can;'t just kill the idea without replacign it with something else that solves the problem. until we have the replacement, it merely pining for the Fjords 16:34:19 edmondsw, so, what did you say was a better approach? 16:34:38 ayoung consensus was that the approach the nova team is taking is better 16:34:48 #link https://review.openstack.org/#/q/topic:bp/policy-remove-scope-checks 16:34:54 cc johnthetubaguy ^ 16:34:57 ayoung don't shoot the messenger :) 16:35:23 so we have me too spec on the docs, but this is the more interesting one 16:35:37 so this is largely splitting our existing policy checks into a policy check and a scope check 16:35:37 edmondsw: actually i think we said we need to go down the current path and get what we committed to completed 16:35:44 then revisit under the new context 16:36:04 dstanek specifically moving policy into code and documenting it 16:36:12 lbragstad: ++ 16:36:22 dstanek that's essentially common ground work we need regardless of the direction we take 16:36:26 so where today we have a policy check, nova is going to have two checks, one checking policy and another checking the context scope 16:36:28 we were trying to not boil the ocean just yet 16:36:56 but... moving policy into code and documenting it has immediate positive effects on operators 16:37:09 lbragstad ++ 16:37:18 Wonderful. Is Nova going to go around and fix this in every other project? Is it going to provide a way to map from URL to required role? 16:38:03 ayoung essentially yes 16:38:10 I like maping a role to a URL, but doing that means breaking so much backwards compatibility, we just can't do that yet 16:38:36 johnthetubaguy, we can for everything but that Bulk api 16:39:02 the docs will tell operators which policy rules go with which URLs, and the default policy settings, which include role 16:39:05 So there, yeah, you would need something like a faked-out-url-string that does the same kind of role check 16:39:07 what I have right now is quick fixes for the problems operators are facing today, in a way that works across upgrade without changing their existing policy files 16:39:20 but then that would be the one off...and you would still need a top level role to be able to execute that API at all 16:39:35 johnthetubaguy, what I have is a fix for the whole damn problem, for all services 16:39:45 that works across upgrade without changing their existing policy files 16:39:57 ayoung: except it doesn't deal with upgrade 16:40:03 yes it does 16:40:23 so it seems like folks with modified policy files get a broken system across upgrade 16:40:39 and we still need to write all the docs on what each of the rules mean, and what the sensible defaults are 16:41:30 the problem is history has taught me its impossible to have this conversation on IRC and specs, needs video and voice / high bandwidth 16:42:04 johnthetubaguy, the problem is that when we have cross project meetings about this in the past, the only people there were Keystone people. I 16:42:08 've given up 16:42:20 johnthetubaguy ++ hence the PTG 16:43:23 if I had seen the policy ones, I would have gone, I just didn't sadly 16:44:11 we have the same issue in reverse with capabilities API 16:44:44 had no API wg or keystone folks when we did that in cross project session before, it happens, PTG is making that a little better and worse at the same time 16:44:55 johnthetubaguy, so, the Nova work should probably be orthoganal, but the RBAC check from middleware needs to be kept alive, and implemented. Otherwise, we don't really have a solution. OpenStack is more than just Nova, and a solution tthat ignores the other services is not a solution 16:45:32 doing a smarter scope check in Nova is awesome 16:45:38 Nova is nothing without the rest of OpenStack, I am totally sold on that 16:45:41 its the right thing to do, and the other project should follow suit 16:45:49 it feels like all this stuff is needed before we could move to anything new anyways 16:45:52 johnthetubaguy you were in the main policy one :) 16:46:09 johnthetubaguy, have you read that spec? 16:46:24 I am pretty sure you hhave, we discuessd in previous meetings 16:46:32 ayoung: I did, I think I read read that when you pointed to that last time 16:46:40 re-read^ 16:47:01 ayoung the role check in middleware spec had a few reviews from contributors representing other projects 16:47:23 lbragstad So, who decided it was dead? What superceded it? 16:47:45 ayoung we decided to put it on hold until we do policy in code and solid documentation 16:47:51 ayoung: i think just getting some of the foundational work done 16:48:00 I think I remember starting to add comments on it actually, but it had merged already, but I could be miss-remembering that 16:48:15 lbragstad, the two are orthogonal, though 16:48:17 ayoung and the *main* reason why I wanted to do that was because it will force our developers through and exercise of understand the warts in our existing policy implementation 16:48:35 ayoung that way we can get more people up to speed on the approach 16:48:55 while the is_admin check should be a pre-req for the policy-in-code work 16:49:22 lbragstad, who do you mean by "our developers" ? I was working on this, heads down, for years. 16:49:29 ayoung exactly 16:49:47 ayoung just you though, and you undestand a *lot* about policy 16:49:48 So who are you planning on taking this on? 16:50:00 antwash and ravelar are started on it already 16:50:39 lbragstad, wonderful. ravelar the policy in code stuff is, I would say, third priority. 16:50:45 First is bug 968696 16:50:45 bug 968696 in OpenStack Identity (keystone) ""admin"-ness not properly scoped" [High,In progress] https://launchpad.net/bugs/968696 - Assigned to Adam Young (ayoung) 16:50:50 Second is policy in middleware 16:50:57 policy in code is third 16:51:02 now... 16:51:17 for me this is about the operators, if we can do something to get them into a happier place, and it doesn't move us further away from the existing plans, thats all good 16:51:17 I don't say that to belittle your efforts, and it can go in as soon as it is ready 16:51:25 I disagree because the policy in code has zero impact 16:51:33 lbragstad, and has 0 value 16:51:38 busineess valuie 16:51:45 it can be done the way things are now 16:52:00 ayoung: all the operators want the policy docs yesterday, this gets them some docs 16:52:03 policy in code is a good idea, but it does not solve the limitations of keystone 16:52:05 i also disagree with that because it allows better management of policy files 16:52:15 policy in code is going to make fixing 968696 easier 16:52:27 ayoung i won't say that policy in code is going to be the silver bullet 16:52:38 edmondsw, not if it does not take the is_admin fixes into account 16:52:44 which were written 6 months ago 16:52:58 ayoung have you read john's specs? 16:52:59 but I do understand the value of implementing it because it will force developers *across* openstack to understand the policy checks they provide 16:53:07 lbragstad, its a good idea, its just not nearly as important as understanding where keystone is falling down 16:53:54 "falling down"? 16:54:05 johnthetubaguy, yes 16:54:18 great movie? 16:54:44 johnthetubaguy, I have ahad to stop thinking about the security aspects of opolicy enforcement becauser it was keeping me up nights 16:55:16 dstanek, Meh...good movie, not quite great...only watched once 16:55:54 So, please get the is_admin stuff in *before* policy in code 16:56:15 the policy in code stuff looks well enough self contained that it should follow in pretty soon there after 16:56:37 then, please either go with RBAC in middleware, or come up with some other approach that actually solves the problems 16:56:41 I see no technical reason why policy in code has to be dependent on the is_admin stuff 16:56:55 they seem in parallel to me 16:57:04 lbragstad, look closer 16:57:16 is_admin needs to be enforced by the the code 16:57:29 * lbragstad squints 16:57:29 otherwise, both those patches will need to be rewritten 16:57:55 I will review those three patches today 16:57:58 Its going to be a rebase nightmare, and I am not around to do it 16:58:03 which means it is not going to happen 16:58:37 lbragstad, understand that I don't have time to work on them, so if you have changes you want made, you need to find someone else to make them 16:59:04 ack 16:59:28 alright - we're about out of time 16:59:29 there are tweaks to the default policy.json there is a little clash there 16:59:44 johnthetubaguy, yep. 16:59:48 honestly, people are setting the rules with no idea what they mean right now, that seems really bad 16:59:56 johnthetubaguy ++ 17:00:03 they are both things we need to fix, so there is certainly agreement there 17:01:21 alright - thanks for coming everyone! i appreciate the discussion :) 17:01:27 see everyone next week 17:01:30 johnthetubaguy, so lets make it so they never have to touche policy.jhson again 17:01:34 #endmeeting