13:00:04 <alex_xu> #startmeeting nova api
13:00:04 <openstack> Meeting started Wed Nov 23 13:00:04 2016 UTC and is due to finish in 60 minutes.  The chair is alex_xu. Information about MeetBot at http://wiki.debian.org/MeetBot.
13:00:05 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
13:00:07 <openstack> The meeting name has been set to 'nova_api'
13:00:11 <alex_xu> who is here today?
13:01:18 <cdent> o/
13:01:45 <alex_xu> cdent: good to see you get well
13:02:17 <cdent> Thanks. I'm not 100% yet but bored, so getting on irc is a thing to do :)
13:03:32 <alex_xu> emmm....holiday weeks
13:03:49 <cdent> indeed
13:04:49 <alex_xu> actually only thing want to talk about is about spec https://review.openstack.org/393205, people get different opionion, probably i will reach johnthetubaguy later to ensure what we should now
13:05:21 <cdent> I tend to agree with jay: remove the vectors for bad
13:05:32 * johnthetubaguy wonders in late
13:06:03 <alex_xu> johnthetubaguy: ah, just talk about https://review.openstack.org/#/c/393205/
13:06:22 <alex_xu> cdent also tend to jay
13:06:25 <johnthetubaguy> I think my problem is that "bad" is relative, in small deployments those queries are quite quick, and folks might be counting on them
13:06:56 <alex_xu> yea, i'm more afraid to break the api, so tend to johnthetubaguy
13:07:03 <johnthetubaguy> I wonder if we just make it safe by default, i.e. policy stops them by default?
13:07:18 <johnthetubaguy> but folks that rely on them can restore the API, if needed
13:07:26 <alex_xu> johnthetubaguy: after we have capabilities discovery?
13:07:29 <johnthetubaguy> although, that feels a bit like over-engineering
13:07:37 <cdent> i was just gonna say that
13:07:48 <johnthetubaguy> alex_xu: I am thinking before we have it, just afterwards it could be discoverable
13:08:29 <johnthetubaguy> we got direct feedback on this at the summit, from operators saying don't break my searching
13:08:57 <johnthetubaguy> thats what really worry, feels like we are just ignoring them
13:09:34 <johnthetubaguy> ... at the same time, we should protect our users from cutting their legs off
13:10:12 <alex_xu> is it worth an email to get board discussion?
13:10:20 <cdent> I can't remember we were debating the user query or just the admin query
13:10:33 <cdent> it seems that on the admin side, perhaps enough rope to hang yourself is okay?
13:10:51 <johnthetubaguy> thats where I my thinking about policy comes in really
13:11:11 <johnthetubaguy> I am OK restricting the user query by default (and I think the operators where too)
13:11:44 <johnthetubaguy> I am tempted to just restrict everyone by default, but let you relax it, if you want to take that risk
13:11:54 <johnthetubaguy> at least for the admins
13:12:06 <johnthetubaguy> but again, feels overcomplicated, so I don't feel happy with that direction as such
13:12:47 <johnthetubaguy> but not breaking users can really suck sometimes, maybe this is just one of those time
13:12:48 <johnthetubaguy> s
13:14:02 <johnthetubaguy> hmm, we got network delays again, or is everyone just very quiet today?
13:14:24 <alex_xu> i'm jus thinking
13:14:51 <johnthetubaguy> thats allowed I guess :)
13:15:01 <alex_xu> so....those query parameters already being here for very long time, people should cut their leg many times....
13:15:14 <johnthetubaguy> cdent: do you think if its clear we protect people by default, that spec seems more reasonable?
13:15:17 * cdent has slow brain
13:16:02 <cdent> Yeah, if the default is protection, but it can be turned off, that seems likely to make folk happy without totally rejecting the input of operators
13:16:41 <alex_xu> turned off by policy?
13:16:56 <johnthetubaguy> yeah, off by default in the policy
13:17:21 <alex_xu> and only off the admin part?
13:17:21 <johnthetubaguy> it does mean if we missed some things, we can tweak the groups of queries linked to each policy
13:19:10 <alex_xu> filters won't break the user, it only ignore those filters. it just didn't return expected sliently
13:19:15 <johnthetubaguy> I duno, I was thinking off by default for everyone
13:19:30 <alex_xu> sort will break user, the 400 returned
13:20:00 <johnthetubaguy> hmm, true, I guess this should be ignore using policy
13:20:05 <johnthetubaguy> based on the overall plan
13:20:15 <johnthetubaguy> a new microversion could add in the errors to help folks later
13:20:42 <alex_xu> how about we ignore for invalid sort key?
13:21:01 <alex_xu> at least it won't break the user.
13:22:03 <johnthetubaguy> yeah, I think we should ignore the not allowed ones, to be consistent
13:22:06 <johnthetubaguy> filters and sorts
13:23:14 <alex_xu> or return 403 is enough, most of client should process the case when the request return 403
13:24:23 <alex_xu> s/403/401
13:25:57 <johnthetubaguy> unsure
13:26:05 <johnthetubaguy> that feels inconsistent
13:26:28 <johnthetubaguy> in the future, we can have a microversion that does what we want, and rejects all input thats not acepted/acceptable
13:26:44 <alex_xu> johnthetubaguy: so break them vs. ignore silently
13:27:10 <alex_xu> johnthetubaguy: +1 microversion in the future to rejects all and return 400
13:28:03 <johnthetubaguy> yeah, for now, I think we are choosing silently ignore invalid input, unless its an allowed key that has an invalid value
13:28:22 <johnthetubaguy> to mirror the old behaviour, as close as we can, in a general way
13:30:42 <cdent> the twists and turns we take "to mirror the old behaviour, as close as we can, in a general way" throughout all this stuff are both admirable and depressing
13:30:54 <cdent> it's a nice quotable phrase too
13:32:53 <alex_xu> I'm a little tent to the 401 returned, the 401 should be processed by client in general. I'm just afraid we return the result the client didn't expected, then just return a fault result siliently
13:32:58 <johnthetubaguy> yeah, its hard to describe how technical debt mounts up, thats sure a big cause of it
13:35:10 <johnthetubaguy> you mean 403?
13:35:27 <alex_xu> emm....what we return when policy failed?
13:35:35 <johnthetubaguy> I think 403
13:35:51 <johnthetubaguy> 401 suggests you need a new token I guess?
13:36:40 <johnthetubaguy> authenticated vs authorised I am assuming?
13:37:19 <johnthetubaguy> so what do we want to do next?
13:37:32 <johnthetubaguy> draft a new version of the spec, and see how that looks?
13:37:37 <alex_xu> yea
13:37:46 <cdent> yeah 401 is auth req, 403 is you got auth but it ain't no good
13:37:52 <alex_xu> choice in the main, and write others in the alternative section
13:38:17 <alex_xu> s/choice/choose one/
13:38:36 <johnthetubaguy> yeah
13:38:50 <johnthetubaguy> thats fair, feels like a discussion in the alternatives section
13:39:24 <alex_xu> #action alex_xu draft a new version of https://review.openstack.org/#/c/393205
13:39:47 <johnthetubaguy> oh, one clarification...
13:40:13 <johnthetubaguy> I think we agreed that all joined fields and anything that isn't using he same format as returned in the API, can be rejected without policy?
13:40:35 <alex_xu> I think yes
13:40:53 <alex_xu> joined fields means 'system_metadata', 'info_cache'...etc
13:41:04 <alex_xu> the tags isn't in the list
13:41:04 <johnthetubaguy> yeah, stuff not in the instance table directly
13:41:15 <johnthetubaguy> tags are indexed OK, so that should be fine
13:41:29 <johnthetubaguy> well, tags are intentionally there, put it that way
13:41:44 <alex_xu> what about metadata one, it works now, and metadata is the normal user faced data.
13:42:08 <johnthetubaguy> yeah, that one feels like we could stop it via policy for now, as its not very efficient
13:42:19 <alex_xu> ok, got it
13:43:18 <johnthetubaguy> (1) stuff we block that may have worked or caused 500 (silently) (2) stuff we block with policy (silently) (3) stuff we allow for admins only by default (4) stuff we allow for users by default
13:43:28 <johnthetubaguy> I think thats the 4 camps they need splitting into
13:43:46 <johnthetubaguy> I guess there is (5) stuff not on any of those lists that is ignored silently
13:44:31 <johnthetubaguy> man this we managed to make this really complicated by taking so long to clear up this mess
13:45:13 <alex_xu> emm...we block with policy and sliently ignore the invalid parameters. actually the policy equal to a config option
13:45:33 <alex_xu> policy means it will return 403
13:46:01 <johnthetubaguy> it doesn't have to be, in this case its not going to trigger 403, it can in the new microversion
13:46:19 <johnthetubaguy> now in theory, all the stuff that causes 500s, could trigger 403, but that seems like an even worse mess
13:47:19 <alex_xu> yea
13:47:28 <cdent> what's an even worse mess?
13:47:53 <johnthetubaguy> having lists of all the things we should reject differently because of what they return today
13:48:02 <johnthetubaguy> oslo.db blocks a bunch of stuff that triggers 500s
13:48:26 <johnthetubaguy> its a recent-ish change we didn't notice happen
13:48:54 * cdent nods
13:49:16 <cdent> It sort of read like you were saying 403s were messier than 500s, so I was confused
13:51:24 <alex_xu> so...we have plan now
13:51:28 <alex_xu> update the spec
13:51:45 <johnthetubaguy> cdent: ah, oops, true
13:52:12 <johnthetubaguy> alex_xu: sounds good, thank you!
13:52:25 <alex_xu> johnthetubaguy: np
13:52:54 <alex_xu> anything more? otherwise let us close the meeting
13:53:34 * cdent nods
13:54:08 <alex_xu> ok, i think we back to work now
13:54:11 <alex_xu> thanks all!
13:54:17 <alex_xu> cdent: have a good rest
13:54:20 <alex_xu> #endmeeting