| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-01-29 | |||
| 15:41:58 | sean-k-mooney | incidentally if you felt like writhing a blog on it or example plugin you could provide a yaml one | |
| 15:42:06 | stephenfin | he wanted the qparam too, I think, but also suggested we use YAML for everything if we were using it | |
| 15:42:25 | stephenfin | so we didn't have two different representations of the same thing | |
| 15:42:41 | efried | so now we have one representation | |
| 15:42:45 | efried | it's python. | |
| 15:42:57 | sean-k-mooney | yes | |
| 15:42:59 | stephenfin | if everything's an object, that concern's resolved. gibi will let me know if he doesn't agree, I'm sure | |
| 15:43:06 | stephenfin | efried: Don't you love it | |
| 15:43:30 | sean-k-mooney | we can even assume its python 3 at long last | |
| 15:43:32 | dansmith | cool | |
| 15:43:34 | efried | If we're providing the qparam so I can push through my snowflake without writing python code for it, I'm okay. | |
| 15:43:46 | stephenfin | \o/ sweet | |
| 15:44:14 | efried | I still think it would be a good idea to have a 'permissive' mode on that qparam | |
| 15:44:43 | stephenfin | efried: what's the difference vs. enable/disable? | |
| 15:44:59 | efried | If I'm constructing my flavor all at once, rather than one extra spec at a time, it gives me the advantage of validating the known/in-tree things while ignoring the snowflakes. | |
| 15:45:09 | sean-k-mooney | permissive mode i guess would log a warning or let you know it failed validation | |
| 15:45:18 | stephenfin | oh, that's what I was going to do for off | |
| 15:45:19 | efried | using two separate calls is a workaround to that | |
| 15:45:30 | efried | yeah, 'off' is 'warn'. | |
| 15:45:41 | efried | but that's for values too | |
| 15:45:46 | sean-k-mooney | oh i was thing off ment you know off as in dont even check | |
| 15:45:47 | stephenfin | no, I was thinking off means you'll never be able to disable validation for the in-tree stuff | |
| 15:45:52 | stephenfin | sean-k-mooney: nope | |
| 15:45:53 | efried | 'permissive' means don't restrict to known keys. | |
| 15:46:05 | stephenfin | wait, that sentence doesn't make sense | |
| 15:46:31 | stephenfin | no, I was thinking off means don't worry about unrecognised keys but do validate values for the known ones | |
| 15:46:35 | stephenfin | so you'll never be able to disable validation for the in-tree stuff | |
| 15:46:45 | stephenfin | better. | |
| 15:47:11 | sean-k-mooney | in my head 'stirct' = know keys only, 'permissive' mean allow unkonw keys but warn, off mean dont do validation at all | |
| 15:47:47 | stephenfin | would you ever want to turn it off for *everything*? | |
| 15:47:58 | efried | I'm not married to the names, but I thought three modes: | |
| 15:47:58 | efried | - off: we don't 4xx for anything. We can still run the validators, but just warn if we find something awry. | |
| 15:47:58 | efried | - permissive: only validate values, only for keys we recognize. Ignore unrecognized keys. | |
| 15:47:58 | efried | - strict: above, plus fail on unrecognized keys. | |
| 15:48:01 | sean-k-mooney | its what you will get for the old microversion | |
| 15:48:13 | sean-k-mooney | so i thnk it makes sense to represent that code path as an option | |
| 15:48:36 | stephenfin | efried: That mostly makes sense but I'm wondering what off is useful for | |
| 15:49:07 | sean-k-mooney | stephenfin: off is basicaly whatever behavior you get with the old microverison | |
| 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? | |