Posted	Nick	Remark
#openstack-nova - 2020-01-29
15:49:36	stephenfin	right, but is that behaviour ever desirable though?
15:49:38	sean-k-mooney	so if im using a newer micoverion for the flavor endpoint that add something not in the old one i can still turn off the vlidation
15:49:42	efried	stephenfin: We forgot or messed up an in-tree one, and don't want to wait for the fix. Could flip to the prior microversion, but the warnings would give me a way to sort of manually make sure everything else is kosher.
15:49:46	sean-k-mooney	stephenfin: yes
15:50:13	sean-k-mooney	stephenfin: there was a spec for composable flavor recently
15:50:16	stephenfin	efried: wdym by "everything else"
15:50:28	efried	the remainder of the extra specs in my request.
15:50:34	sean-k-mooney	if that was approved and landed after this feature we might want to use that without validation
15:50:57	stephenfin	oh, yes, you can set multiple extra specs at once. duh
15:50:57	efried	I try with strict/permissive, I get a bounce on extra spec X, I redrive with 'off' and make sure the only warning I get is on X.
15:51:04	sean-k-mooney	efried: im more or less ok with your deffintions by the way your off just does a littel more then i expected
15:51:13	efried	we can call it 'warn'
15:51:22	stephenfin	efried: vs. just retrying one by one?
15:51:26	efried	yes
15:51:44	efried	or "I want to test drive this feature"
15:51:45	sean-k-mooney	i suggested permissive to mirror selinux but warn would be fine
15:51:54	efried	"but still create my flavor"
15:51:56	sean-k-mooney	although that implice that strict should be error
15:52:01	efried	really for completeness
15:52:31	stephenfin	Okay. You're aware the warnings are only going to be in the logs though, yeah?
15:52:37	stephenfin	So I'm not sure how much feedback you're going to be getting
15:52:53	efried	It would be nice to find a way to send them back to the CLI, but yeah, I get it.
15:53:04	stephenfin	compared to just trying one by one 'til you find the erroneous extra spec
15:53:06	sean-k-mooney	well for errors we can put it in the resonce body
15:53:20	stephenfin	sean-k-mooney: right, but not for warnings
15:53:21	efried	I think that ^ would be nice for a future microversion, don't need to do it now.
15:53:22	sean-k-mooney	for wraning maybe we could alter the respocne to include them too
15:53:37	stephenfin	for those we already have a body - the updated flavor (I think)
15:53:53	sean-k-mooney	ya it should be the full flavor object
15:54:05	sean-k-mooney	but we could extend it from the mircoverion on
15:54:06	stephenfin	yup, so no way to get back that warning via the API
15:54:11	sean-k-mooney	that said its a nice to have
15:54:36	efried	so then yeah, call it 'off' and the warnings are in the logs
15:54:43	sean-k-mooney	sure there is we just change the api respoce to also have validtion warning but lets leave that out of scope for now
15:54:53	stephenfin	efried: ack
15:55:22	efried	(note that we technically wouldn't need 'strict', since that's the default, but again for completeness it would be nice to support it)
15:55:32	stephenfin	agreed
15:55:39	sean-k-mooney	yep
15:56:05	sean-k-mooney	i like the implcit state to be setable explictly even if its the default if you say nothing
15:56:13	sean-k-mooney	it makes testing nicer imo
15:56:33	sean-k-mooney	and it make documting the behavior simpler since you have a name for each mode
15:56:55	efried	could call 'permissive' 'values-only' or something, since 'permissive' isn't particularly descriptive
15:57:01	efried	bikeshed away.
15:57:58	sean-k-mooney	ya maybe again i just suggeste permissive since selinux in permissve mode doese policy validateion and log any violation but allow whatever the process was doing to continue
15:58:23	sean-k-mooney	it seamed like a good analog but not many peopel might make that connection
15:59:20	efried	'ignore-unknown-keys'
15:59:45	efried	'ignore-(well-okay-log-a-warning)-unknown-keys'
16:00:02	dansmith	so,
16:00:10	dansmith	I had to deal with something and lost track here
16:00:51	dansmith	one reason to not allow the client to choose more than strict or not-strict is that if you have permission to create flavors within your tenant, and you can set validation=warn (in the logs), then you can spam the server-side logs with warnings
16:00:59	dansmith	which would be considered somewhat of a DoS
16:02:39	stephenfin	we're adding a policy for this though, right?
16:03:10	dansmith	a policy for what? each of the values of the validation=(on|off|warn) ?
16:03:38	stephenfin	<dansmith> stephenfin: but I think it's legit to have a validation=no per-request, but that can return 403 either because of policy or config if it's documented when you add it, no? so later we could turn that off or default it off
16:03:44	stephenfin	what did you mean by that? ^
16:04:08	stephenfin	I assumed you meant for the ability to set '?validation=<anything>'
16:04:49	dansmith	stephenfin: I meant state that validation=no might be disabled (i.e. always required) in the docs, so that we can allow that to be turned off in config or policy in the future if the op wants to require all flavors be validated
16:05:05	dansmith	I'm just arguing against having a client-controlled way to emit WARNING messages in the server-side logs
16:05:44	sean-k-mooney	or i guess restircted to a group e.g. admins although it is an admin only api already by default
16:06:20	stephenfin	I think WARNING might be a bit much, tbh. INFO seems appropriate if the user has explicitly requested no/limited validation
16:06:24	sean-k-mooney	the policy.json seam like a better approch if we were to make it configurable
16:06:39	stephenfin	If it was INFO, we'd be okay, right?
16:07:06	stephenfin	since no operator will get paged for INFO logs
16:07:10	sean-k-mooney	i think dansmith was concerned about ddos risks
16:07:13	dansmith	I dunno, I just don't like it in general
16:07:14	dansmith	right
16:07:18	sean-k-mooney	infor does not really help with hat
16:07:21	sean-k-mooney	*that
16:07:21	dansmith	but it's not a sticking point
16:07:25	dansmith	sean-k-mooney: correct
16:07:43	stephenfin	they could already ddos things but just repeatedly hitting the API with working extra specs
16:07:50	stephenfin	we log each request iirc
16:07:56	stephenfin	*by just
16:08:05	stephenfin	that's what API rate limiting is for
16:08:27	sean-k-mooney	well i suspect the error could be long if you add a bunch of invalide extra specs so its a larger risk
16:08:42	sean-k-mooney	but again its an admin only api by default
16:08:46	dansmith	and logging something that is a warning as info doesn't make it info
16:08:58	sean-k-mooney	so the request will get rejected by a normal user well before it hit your code
16:09:00	dansmith	but anyway, like I say, not a sticking point
16:09:37	stephenfin	Is it a warning though? Per above, info does seem appropriate if the user has explicitly requested no/limited validation
16:09:58	stephenfin	It's a warning if they didn't, which they'll see in their HTTP 4xx response
16:10:07	dansmith	seems like a warning to me :)
16:10:08	dansmith	anyway
16:10:16	dansmith	we needn't argue about it
16:10:27	dansmith	I will just toldjaso if you get CVE paperwork over it :)
16:10:29	stephenfin	fine by me :)
16:16:01	bauzas	mmm, what the heck is this ?
16:16:04	bauzas	stestr: error: unrecognized arguments: --test-path=./nova/tests/functional run test_nova_manage
16:16:09	bauzas	holy shit
16:16:53	bauzas	I probably need to upgrade stestr by hand
16:16:59	bauzas	but I did recreated my venc
16:17:02	bauzas	venv*
16:17:09	stephenfin	tox -e function --recreate ?
16:17:17	stephenfin	*al
16:18:32	stephenfin	unless you're using stestr without tox, which is odd
16:18:57	stephenfin	seeing as you can do stuff like 'tox -e functional -- -n path_to_functional_test.py::TestClass.test_method'
16:22:34	bauzas	stephenfin: that's what I did
16:22:51	bauzas	and I ended up with stestr==2.6.0
16:23:24	bauzas	I always asked to run a subset of tests by doing tox -efunc <my_regex>
16:23:27	bauzas	and it worked

Earlier   Later