| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-27 | |||
| 15:07:25 | openstack | Launchpad bug 1875418 in OpenStack Compute (nova) "Generated policy.json in Ussuri is broken by default" [Undecided,New] | |
| 15:07:25 | zigo | Bug filled: https://bugs.launchpad.net/nova/+bug/1875418 | |
| 15:07:31 | stephenfin | zigo: why can't we just stop including a generated policy file in the package? | |
| 15:07:49 | dansmith | he can, he said that | |
| 15:07:50 | zigo | stephenfin: How are operators supposed to double-guess what they can use? | |
| 15:07:55 | gmann | dansmith: i mean current defaults are "new default + old deprecated defaults" and file generated was half bald with 'new defaults' only | |
| 15:08:21 | zigo | stephenfin: That's actually exactly what I'll be doing: provide an empty policy.json. But that's really not user friendly. | |
| 15:08:29 | stephenfin | zigo: You have a openstack-nova-doc package, yeah? It's documented in there | |
| 15:08:41 | zigo | stephenfin: I'd very much prefer having a policy.json that reflects what's currently enforced in Nova. | |
| 15:08:46 | gmann | i might be wrong but re-generating the policy file is something you are intentionally changing so adopt all change instead of half | |
| 15:08:53 | dansmith | stephenfin: he's saying that's a sucky user experience, and he's right despite how clean it seems to us | |
| 15:10:13 | gmann | and this is always a problem from starting if deployer rely on re-generated file which does not include the deprecated-but-supported rule | |
| 15:10:13 | dansmith | gmann: before this, generating the file and tweaking the rules you want ended up with all things at the same generation. If you tweaked a rule that we removed, then sure it's broken, but now people will be introducing old "syntax" to a generated file of new "syntax", and also not realize that as soon as they generate the file, they need scoped tokens right? | |
| 15:10:16 | zigo | stephenfin: Just did that and uploaded the package: https://salsa.debian.org/openstack-team/services/nova/-/commit/48bc8889ae8a787104b76e95c3e1dfc5893d146b | |
| 15:11:20 | zigo | Though really, that's really not user friendly to do that. | |
| 15:11:24 | gmann | dansmith: right. | |
| 15:11:51 | dansmith | gmann: so another question.. if I don't take the generated file, continue to run with the deprecated defaults, but need to tweak something.. how do I see the generated old defaults file? is there a flag to the tool? or do I have to look at train docs? | |
| 15:11:59 | gmann | but how to fix all those script to generate file, we can do something on oslopolicy-sample-generator | |
| 15:12:11 | stephenfin | zigo: Forgive me but is that not the normal way config files work? They're used for overrides, not defaults | |
| 15:12:51 | dansmith | stephenfin: but they're usually fully commented-out so you can see all the options in place while you're overriding | |
| 15:12:54 | zigo | stephenfin: If I'm listening to you, then my Nova package should ship an empty /etc/nova/nova.conf? Are you serious ?!? | |
| 15:13:02 | zigo | :) | |
| 15:13:06 | gmann | dansmith: train doc, or nova policy reference file. | |
| 15:13:16 | dansmith | gmann: that sucks | |
| 15:13:30 | gmann | yeah, tool is not adding them | |
| 15:13:43 | zigo | I think I'll give another try with the yaml thing, see if that works, and if I can ship a fully commented out one. | |
| 15:14:10 | zigo | That's still not nice, because it'd be supposed to work if all comments get removed ... If you know what I mean. | |
| 15:14:13 | dansmith | zigo: do you treat policy like config or what? what if they've modified their policy file? | |
| 15:14:20 | stephenfin | zigo: Not empty, but IMO we shouldn't be including values with defaults ¯\_(ツ)_/¯ | |
| 15:14:25 | zigo | Just same as in for nova.conf, where commented out stuff are supposed to be the default. | |
| 15:14:37 | stephenfin | I mean, that's how other config files works | |
| 15:14:42 | dansmith | stephenfin: that's the developer-focused "look at how clean this is" approach, but it sucks for admins | |
| 15:14:57 | stephenfin | znc.conf jumps to mind, since I was hacking on over the weekend | |
| 15:15:04 | stephenfin | ditto for sssd.conf | |
| 15:15:09 | stephenfin | or krb5.conf | |
| 15:15:11 | zigo | dansmith: For most packages, I don't have them as CONFFILES (these files, marked by dpkg as "prompt user if there's a change on upgrade...). | |
| 15:15:30 | gmann | stephenfin: if oslopolicy-sample-generator add complete default (new + deprecated) then config case is same otherwise it is issue | |
| 15:15:55 | stephenfin | dansmith: yeah, maybe. I just figured everyone was doing 'man [app].conf' | |
| 15:15:57 | zigo | So policy.json files live in /usr/share/FOO-common/policy.json and are copied to /etc/FOO only if /etc/FOO doesn't have a policy.json file. | |
| 15:16:05 | bnemec | I'm still confused why this would be failing on scope. enforce_scope is false by default. | |
| 15:16:06 | zigo | This way, no prompt on upgrade, and the old version is kept. | |
| 15:16:16 | zigo | Except I didn't do that for Nova, I don't know why ... | |
| 15:16:29 | zigo | So in the Nova case, /etc/nova/policy.json *IS* a CONFFILE. | |
| 15:16:34 | gmann | bnemec: we have 'system:all' string in check_str for new defaults of system scope role | |
| 15:16:42 | dansmith | gmann: so it sounds like we need a big warning reno about this at the very least | |
| 15:16:45 | zigo | (and then dpkg will prompt on upgrade if there's some diff) | |
| 15:17:04 | dansmith | gmann: we probably also should switch to yaml by default, and make sure our CI jobs are using them that way | |
| 15:17:21 | gmann | bnemec: https://github.com/openstack/nova/blob/347d656c35fdf0c309039a7c1f352f82c6950868/nova/policies/base.py#L104 | |
| 15:17:22 | stephenfin | bnemec: I suspect the oslo-generate-policy command is using the scoped policies, but nova is still defaulting to non-scoped (to avoid breaking upgrades, funnily enough) | |
| 15:17:30 | dansmith | the yaml is better in every respect, except for compatibility | |
| 15:17:33 | gmann | https://github.com/openstack/nova/blob/347d656c35fdf0c309039a7c1f352f82c6950868/nova/policies/base.py#L36 | |
| 15:17:38 | stephenfin | yaml++ | |
| 15:17:47 | bnemec | gmann: Why? Isn't the scope check built-in to the policy enough? | |
| 15:17:52 | zigo | stephenfin: Looks like you're right yeah. | |
| 15:18:07 | gibi | dansmith: I need to read back after my current call | |
| 15:18:20 | gmann | bnemec: when enforce_scope is true then yes otherwise we need to differentiate the system vs project - https://github.com/openstack/nova/blob/347d656c35fdf0c309039a7c1f352f82c6950868/nova/policies/base.py#L36 | |
| 15:18:24 | zigo | stephenfin: How would we make oslo-generate-policy to use non-scoped policies then? | |
| 15:18:26 | dansmith | gibi: definitely needs your review | |
| 15:18:50 | bnemec | That seems like it's completely defeating the purpose of enforce_scope. | |
| 15:18:57 | stephenfin | zigo: not sure you want to do that | |
| 15:19:07 | stephenfin | you'd be generated deprecated configuration | |
| 15:19:11 | stephenfin | *generating | |
| 15:19:22 | dansmith | stephenfin: the deprecated form is supposed to be the default we assume if no policy file | |
| 15:19:34 | zigo | stephenfin: If nova.conf defaults to non-scoped, but policy.json to scoped, then we do have a problem. | |
| 15:19:41 | zigo | Choose your side comrade ! :) | |
| 15:20:07 | gmann | zigo: yeah, agree. | |
| 15:20:27 | stephenfin | dansmith: Yes, because we care about upgrades. New deployments would ideally be overriding nova's defaults though | |
| 15:20:41 | stephenfin | zigo: I assume there's no way to distinguish between new installs and upgrades? | |
| 15:20:47 | dansmith | stephenfin: he generates those for upgrades too he just said | |
| 15:20:59 | zigo | stephenfin: There is, if you're talking about packaging. | |
| 15:21:06 | stephenfin | I am | |
| 15:21:08 | dansmith | stephenfin: and, unless we default the enforce_scope on, and detail the differences between scoped tokens for users of new deployments, it's not that cut and dried | |
| 15:21:18 | zigo | That's an argument given to the .postinst script of the package. | |
| 15:22:18 | zigo | It's defined here: https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html#summary-of-ways-maintainer-scripts-are-called | |
| 15:22:18 | stephenfin | dansmith: it sounds like we can do that for a new installation (default enforce_scope to on) | |
| 15:22:36 | dansmith | stephenfin: we don't now though, AFAIK | |
| 15:22:57 | stephenfin | we wouldn't do it - the package would | |
| 15:23:04 | stephenfin | it would override the nova default | |
| 15:23:06 | zigo | I'd very much you give operators at least one more cycle to enforce this. | |
| 15:23:32 | zigo | Then just set enforce_scope to True by default in Victoria ... | |
| 15:23:52 | stephenfin | zigo: I'd like to know if the following combination is possible/makes sense | |
| 15:23:54 | dansmith | stephenfin: not sure how you could coordinate that across every deployment tool | |
| 15:24:25 | stephenfin | new installation: enforce_scope = True (override), use Ussuri policy.json | |
| 15:24:43 | stephenfin | upgrade: enforce_scope = False (nova default), use Train policy.json | |
| 15:24:44 | stephenfin | ? | |
| 15:25:06 | stephenfin | dansmith: we do that kind of stuff in TripleO, albeit higher than the package level | |
| 15:25:29 | dansmith | stephenfin: right but everyone needs to do that.. tripleo, kolla, debian, ubuntu, rdo, $mycustomthing | |
| 15:27:05 | zigo | stephenfin: This is going to be horrible to manage with puppet-nova... | |
| 15:27:06 | stephenfin | I didn't think we generated policy.json for RDO/OSP, and I assume Ubuntu will take whatever Debian does. I can't argue with $mycustomthing though, no | |
| 15:27:21 | zigo | stephenfin: You assume wrong ! :) | |
| 15:27:28 | zigo | Ubuntu do their own crap ... | |
| 15:27:34 | stephenfin | \o/ | |
| 15:27:44 | gmann | I was checking to remove 'system:all' from new default but that leads to over-permission issue | |
| 15:27:57 | zigo | I tried for years to fight this, it never worked, because of marketting reasons. | |
| 15:28:19 | zigo | And there's all sorts of issues because of this. :) | |
| 15:28:53 | zigo | Like, people trying to use whatever horizon plugin that I was packaging but they didn't, and it broke on Ubuntu, but they don't care because "it's not in main" ... | |
| 15:29:02 | zigo | The usual thing with Ubuntu... :) | |
| 15:29:28 | gmann | i thought policy-in-code was the time when we asked (or should) deployer to not to re-generate the complete policy file instead keep override rule only | |
| 15:29:49 | stephenfin | gmann: Yeah, I think that's the big disconnect here | |
| 15:30:19 | stephenfin | so doing different things for new installation/upgrade probably isn't an option | |