Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-01-29
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 dansmith but it's not a sticking point
16:07:21 sean-k-mooney *that
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
16:24:57 stephenfin bauzas: That's what I have too. Working fine here
16:25:17 bauzas checking with stestr==2.5.0
16:25:38 bauzas functional runtests: commands[0] | stestr --test-path=./nova/tests/functional run test_nova_manage
16:25:42 bauzas mmmmm
16:26:21 stephenfin bauzas: '$ tox -e functional -- test_nova_manage' wfm
16:27:05 bauzas I never had to use the positional markers, but whatever, gonna try
16:27:41 bauzas crazy, still same issue
16:28:07 stephenfin want to dump the complete output to paste.o.o
16:28:19 stephenfin it's something simple, I'd suspect
16:30:06 bauzas yeah me too, but can't see the problem
16:30:22 bauzas it's just stestr which doesn't sound to accept --test-path
16:30:24 bauzas http://paste.openstack.org/show/788935/
16:31:11 bauzas actually, nope
16:33:59 bauzas well, I'm puzzled
16:34:37 stephenfin ah, wait, have you added additional tests for nova-manage?
16:35:13 stephenfin There's a bug with oslo.config whereby the CLI parser is global'ish
16:35:49 stephenfin Yeah, '--remote_debug-port' is a nova-manage option. You're not mocking stuff properly
16:38:06 stephenfin bauzas: mriedem saw the issue pop up in a unit test at https://review.opendev.org/#/c/694806/2/ and it's currently causing an issue with glance and the latest version of cliff
16:40:19 bauzas stephenfin: interesting, if I use the functional-py36 target, it does work
16:40:49 stephenfin even if you rebuild the venv?
16:41:03 bauzas it was created, I never used the target yet
16:41:13 stephenfin gotcha
16:41:20 stephenfin weird
16:41:24 stephenfin then I'm not sure :(
16:41:25 bauzas to make it clear, the functional-py36 target works, but not the standard one
16:41:53 bauzas either way, I know what to do, but I have to look at the target differences in tox.ini
16:42:32 efried bauzas: would it be productive for me to read the current PS of the numa topo spec, or wait til you rev it?
16:43:06 bauzas efried: I'm wraping my head around the group_policy issue, but your thoughts could be helpful
16:43:59 bauzas efried: and we have a disagreement on the memory modeling with NUMA with sean-k-mooney, your opinion could help us finding a consensus
16:44:25 bauzas efried: to answer your question, yeah comments would be appreciated on the current rev
16:44:32 efried okay, I'll give it a read. My view on group_policy is that we should be ignoring it. It doesn't really have a place with granular groups and the other knobs we put in in recent microversions.
16:44:59 efried I at least proposed (though I don't remember if we actually pulled the trigger on this) making it no longer required in a recent microversion.

Earlier   Later