13:01:01 #startmeeting nova api 13:01:02 Minutes (text): http://eavesdrop.openstack.org/meetings/nova_api_startmeeting_nova_api/2017/nova_api_startmeeting_nova_api.2017-03-22-13.00.txt 13:01:03 Log: http://eavesdrop.openstack.org/meetings/nova_api_startmeeting_nova_api/2017/nova_api_startmeeting_nova_api.2017-03-22-13.00.log.html 13:01:04 Meeting started Wed Mar 22 13:01:01 2017 UTC and is due to finish in 60 minutes. The chair is alex_xu. Information about MeetBot at http://wiki.debian.org/MeetBot. 13:01:05 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 13:01:07 The meeting name has been set to 'nova_api' 13:01:14 o/ 13:01:22 o/ 13:01:23 mriedem: are you everywhere now because of your working-from-home-ness? 13:01:26 o/ 13:01:31 cdent: yes 13:02:02 mriedem: you worked at office before, i thought no-one will work at office in the US 13:02:16 alex_xu: the local ibm people do 13:02:17 #topic priorities 13:02:36 #link https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/policy-docs 13:02:51 the spec for policy docs merged, and already have a lot of patches ready for review \o/ 13:03:33 want to ask how to show policy for deprecated APIs - #link https://review.openstack.org/#/c/448267/ 13:03:39 as test line ok? 13:03:42 o/ 13:03:48 text 13:03:56 so deprecated APIs are still APIs 13:04:06 thats different to deprecated policy rules that are about to die 13:04:12 yea 13:04:16 yea 13:04:40 we only to note deprecated API in the api-ref. 13:04:50 s/only/only need/ 13:05:12 I think we should get the basics in there, and see how we look once thats done 13:05:42 its a shame to bloat loads saying that the API is deprecated, if we don't need to. 13:05:42 yea 13:06:01 #link https://docs.openstack.org/developer/nova/sample_policy.html 13:06:08 its starting to look much better already ^ 13:06:48 yeah it is 13:07:07 yea, so we do show note about deprecated API like volume API in that sample. 13:07:28 I thought we said about we probably don't need to mention it 13:08:18 we might deprecated a policy rule, but thats different 13:08:31 yea that is separate. 13:08:31 johnthetubaguy: do we need a space line between the doc and rule? a little hard to figure which one is rule. 13:09:16 alex_xu: sure, thats an oslo.policy generator change though 13:09:51 sneti or aunnam might fancy fixing that up ^ 13:10:10 johnthetubaguy: ah yes, that true. I thought we can add a '\n', but you are right, that should be done by oslo.policy 13:10:12 oh I quick note on "os_compute_api:os-attach-interfaces:discoverable" and the like 13:10:45 I asked them not to document those, because I think we can remove those 13:11:20 +1. 13:11:26 yea, hope we can remove those. really needn't waiting for one more release 13:11:41 patrole project want to test discover policy which they should not right? 13:11:59 that in the follow up spec, removing those 13:12:11 gmann: correct, its not worth it 13:12:22 only thing we did document that those are going to remove and they thought its valid policy 13:12:26 johnthetubaguy: or just create a separated bp for it, then people can begin to kill those directly? 13:12:27 gmann: I still need to work out what that thing does at some point, probably before we approve the next spec 13:12:36 alex_xu: can do 13:12:38 +1, ll convey that 13:13:35 gmann: I hadn't spotted they still existed till we started this, there are quite a few rules sneti and aunnam have found that actually are unused 13:14:05 I am sure, just like with the config, there is a bunch of stuff that will need tidying up 13:14:13 yea 13:14:36 i feel its good to have patrole gate job on nova side ? 13:14:41 anyways, not sure there is much else I wanted to discuss on that one (I am keen to talk about the other two specs I guess) 13:15:22 johnthetubaguy: would OSIC guys want to create a bp for removing discovery entry? 13:15:49 alex_xu: its already on their TODO list, I can ask them to create a separate specless BP for that 13:16:02 johnthetubaguy: thanks 13:16:20 gmann: I think we need to work out where it fits, I was really wanting in tree functional tests for the policy stuff 13:16:38 thanks OSIC guys, thanks sneti and aunnam 13:16:58 +1 great to see us making progress on this stuff 13:17:14 #link https://review.openstack.org/#/c/433037/ 13:17:17 johnthetubaguy: yea functional tests always nice. i was thinking we can use patrole effort also for more testing 13:17:30 johnthetubaguy: ^ you want to talk that more 13:17:32 but not sure if sometime that can delay the changes etc 13:18:04 so the spec 13:18:14 really I am wondering whats needed to get that approved / agreed 13:18:39 had some great comments from alex_xu sneti and aunnam to clean up the text in there 13:19:08 gmann: having both might be correct, I just don't know right now 13:19:23 I should copy the summary I guess 13:19:36 sure 13:19:39 Here we separate the concerns of what resources user has access to and 13:19:39 what actions the user is allowed to do. 13:19:44 thats basically it 13:19:57 remove scope checks from policy.json 13:20:58 yea, mix the role check and scope checks into one rule, that makes people hard to understand 13:21:16 also makes it easy to break Nova's API in the policy file 13:21:37 now we are not breaking the whole user_id thing just yet, while we get hierarchical quotas implemented 13:21:40 and scope check isn't something people need to change 13:22:00 alex_xu: +1 thats the hope, well its something we don't want them to change 13:22:18 doing that makes this one much easier 13:22:20 #link https://review.openstack.org/#/c/427872 13:22:43 basically having some proper default roles for various use cases, beyond user vs global admin 13:23:21 mriedem: do you think you will get chance to hit those two remaining policy specs? 13:23:51 johnthetubaguy: umm 13:23:54 today? 13:23:57 I didn't read that spec again. the orignal one feel mix too much thing: more role, deprecated policy..etc 13:24:15 mriedem: sometime this week would be cool, not worried about it being today 13:24:40 honestly i didn't grok those very well the last time i looked 13:24:44 alex_xu: yeah, the key bit is the new role, the deprecated policy is needed to implement that 13:24:44 around the time of the ptg 13:24:53 mriedem: that first one reads much better now I hope 13:25:11 ok i guess if i still don't understand the problem i'll yell about it 13:25:17 mriedem: totally 13:25:20 i think it was missing some clear examples before 13:25:20 johnthetubaguy: ok, i will try to read that again 13:26:14 #link https://review.openstack.org/435485 13:26:25 mriedem: johnthetubaguy also have a poc ^ 13:26:42 yeah, its just for that first one 13:27:20 gmann: thats the functional tests I was on about earlier ^ 13:27:47 now there are some fun details here, between 404 errors and 403 errors 13:27:56 johnthetubaguy: i see, ll look into those tomorrow. 13:28:08 DB checks triggering 404s and policy triggering (sometimes) 403 errors 13:29:04 but the basic idea is to have zero API impact, its just there are some inconsistencies that need to be ironed out here and there 13:29:55 for the policy check we return 403, for scope check we return 400, i thought 13:30:19 well, its more complicated I think 13:30:20 404 on policy failure ? 13:30:41 johnthetubaguy: the 404 and 403 both are expected return code, so i think it's fine. we won't impact the API 13:30:49 alex_xu: agreed 13:31:00 my worry is information disclosure 13:31:11 someone with no access to a server, gets a 404 on that URL today 13:31:22 so you can't tell if you guessed the instance uuid correctly 13:31:27 (for example) 13:31:41 I vote we keep that behaviour the same, even when the check is no longer in the DB code 13:32:19 guessing uuids is a new olympic sport 13:32:27 but yeah, it is a tricky issue 13:32:47 johnthetubaguy: you already separate the scope check out of policy rule. that means the policy rule is only about whether you have permission to access the API 13:32:49 johnthetubaguy: you mean 404 on scope check right so people would not know anything about internal restriction etc 13:33:00 so we return 403 for polciy check is right at here. The API endpoint existed, you are just not allowed to access 13:33:40 for scope check, for a uuid, we will return 404. you can't see that instance. and we didn't leak something 13:34:14 yeah, something like that 13:34:48 if you got 403 for all valid and invalid uuids, that would be fine, for example 13:35:05 its the difference in return value that means you can probe for information 13:35:32 anyways, thats all implementation detail I think, because it exact circumstances are different for different APIs 13:35:49 the functional tests we create will help discover these things 13:36:01 (at least, thats how I found out there was a problem, when I wrote the POC) 13:39:14 i need check the spec about the functional tests 13:39:33 I think I said tests that pass before and after the change. 13:39:46 from first look those are really useful and ll find many issue as johnthetubaguy mentioned 13:39:51 i.e. we move the check from the DB into the new logic, and the same tests should check its all good 13:40:02 +1 13:40:16 gmann: it shocked me how interesting they will be :) 13:40:51 ok, anything more about policy? 13:41:12 johnthetubaguy: but we need to complete all those tests before new policy way 13:41:51 i think those will be huge with all negative combination for all current policy 13:42:01 well, need to add the test in each code path, before converting that code path 13:42:20 we certainly shouldn't add them all first 13:42:35 yea that's much feasible 13:43:03 most of that test I added is common boilerplate stuff, I think 13:43:18 the per API stuff is quite minimal I hope 13:43:29 but time will tell 13:43:43 anyways, feedback very welcome on all that stuff 13:44:00 I would love for us to make some progress on at least the first of those two specs this cycle 13:44:12 +1 13:44:17 \o/ 13:44:18 granted, the third spec should not be started until the first two are completed 13:44:41 yea 13:45:04 cool, seems like we are roughly agreed on the approach, which is all good 13:45:26 yes 13:46:20 I didn't get progress for the poc of remove stevedore, hope I bring news in next week 13:46:52 we have three items in the open, if nothing more for priorities, let us jump to open 13:46:52 alex_xu++ on that stuff. 13:46:58 cdent: thanks 13:47:00 (the removal of stevedore) 13:47:16 yeah, that tottally needs removing 13:47:25 #topic open 13:47:31 mriedem: your turn 13:47:34 yay 13:47:43 i just replied to comments in https://review.openstack.org/#/c/447149/ 13:47:53 (FWIW, we hit some strangeness around limits and used limits looking at that policy stuff, so the collapse of those would be nice at some point) 13:47:53 alex_xu: some clarifying questions for you 13:47:59 mriedem: thanks 13:48:05 ken'ichi also brought up the os-hosts API 13:48:08 which i didn't even think of, 13:48:23 but now that i look at it, is basically the same as the os-services API, except it has some additional actions, like stop/start/reboot 13:48:30 so that's pretty ugly 13:48:40 oh, most of those are broken too 13:48:42 i don't know why we have 2 APIs for the same resource, but i've questioned this over my tenure in nova 13:48:59 johnthetubaguy: i haven't looked at what supports those APIs, but assume it's xen 13:49:08 mriedem: quite the opposite I think 13:49:09 because xen is always the oddball in the API 13:49:10 oh 13:49:34 I think its more about clustered hypervisors where that distinction becomes important 13:49:52 quite how you start a host that is shutdown and no nova-compute service running on it, for example 13:50:11 it might be left over from the old pre-ironic stuff, I haven't dug to check that 13:50:33 i feel we add new 'uuid' field explicitly better than change the id field's value to a uuid 13:50:53 mriedem: so I guess we need to stage this work somehow, we could go with the more obvious fixes first 13:51:09 looks like xen and hyperv support host_power_action 13:51:11 but vcenter doesn't 13:51:15 nor libvirt or ironic 13:51:34 mriedem: hmm, must have been baremetal than, I know its mostly broken for xen 13:51:52 probably worth deprecating that API I guess 13:51:55 maybe first step is opening a bug about actually documenting wtf os-hosts does and what supports it :) 13:52:04 yeah 13:52:09 then deprecating it 13:52:19 ok so i think i basicaly have two open questions for the spec, 13:52:47 1. do we change the os-services action APIs to take a service_id rather than rely on looking up the service by hostname/binary in the body? 13:52:57 e.g. PUT /os-services/{service_id}/disable 13:53:16 PUT /os-services/disable-log-reason 13:53:17 { 13:53:17 'disabled_reason': 'host os upgrade maintenance' 13:53:17 } 13:53:34 I am +1 that myself 13:53:36 err PUT /os-services/{service_id}/disable-log-reason 13:53:41 yeah, that 13:53:59 alex_xu: gmann: cdent: opinions on that? 13:54:06 * cdent is thinking 13:54:21 feel free to just chime in on the spec when you have time 13:54:23 so we want to introduce that new action way? 13:54:28 i want dansmith to review it before i make changes 13:54:36 not /os-services/{service_id}/action, anymore? 13:54:54 ack 13:55:04 i'll give the review a real review later today 13:55:04 alex_xu: we already have PUT /os-services/disable today 13:55:11 and user will get service_id from list 13:55:13 with a body like: 13:55:13 { "host": "host1", "binary": "nova-compute" } 13:55:26 mriedem: at least we consistent with other existed apis i think 13:55:30 so that becomes just PUT /os-services/{service_id}/disable 13:55:32 with no body 13:55:45 * edleafe wakes up. Long night 13:55:45 because we don't need the host and binary to identify the service if we have the id 13:56:00 in general using identifiers in the URL is good 13:56:15 * alex_xu reminds that we only left 4 mins 13:56:16 IMO /disable explicitly much cleaner than /action 13:56:18 yeah honestly i had assumed this was how things already worked in here 13:56:22 cdent: is that the correct URL for an action, rather than our silly /action catch all thing? 13:56:24 yea +1 on those 13:56:27 ken'ichi didn't want /action 13:56:29 which i'm fine with 13:56:53 johnthetubaguy: unresolved huge debate that was so huge people gave up 13:56:55 yeah, I think avoiding the catch all /action is probably good, I can't remember if there is already an API WG thingy for this 13:57:01 cdent: ah, OK 13:57:02 it was abandoned 13:57:09 the other thing, 13:57:18 because it looked like soap 13:57:21 johnthetubaguy: cdent: usually POST is more forgiving, no? 13:57:23 2. do we also want to do this for os-hosts if we're doing it for os-services (since they are basically the same api)? 13:57:26 i'd think yes 13:57:38 "yes" to also doing os-hosts 13:57:42 well, the option is to just deprecate os-hosts 13:57:50 me also not comfirtable for /action 13:58:01 and roll any remaining things of interest into os-services 13:58:01 johnthetubaguy: if we deprecate os-hosts, 13:58:09 then we have to move the stop/start/reboot actions from os-hosts to os-services 13:58:17 which we could do 13:58:22 I think we could argue they shouldn't live in our API 13:58:25 it just starts to become a hell of a big spec 13:58:25 so there is no replacement 13:59:06 with 1 minute left, move discussion to the spec? 13:59:15 yes 13:59:16 i also wanted sdague's thoughts on all of this too 13:59:16 yea 13:59:21 yes 13:59:28 anyway, thanks 13:59:32 edleafe: sorry, we didn't have enough time for your item 13:59:44 np 13:59:51 I slept in anyway :) 13:59:58 +1 on wanting sdague's view on this 14:00:02 ok, it's time to close the meeting, thanks all, let us back to the nova channel 14:00:04 #endmeeting