| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-11-11 | |||
| 12:46:33 | sean-k-mooney | and nova-compute.conf or similar was used for the compute service which would not read nova.conf | |
| 12:46:55 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.text_type (1/2) https://review.opendev.org/728109 | |
| 12:50:09 | sean-k-mooney | this was hit in kolla in queens https://github.com/openstack/kolla-ansible/commit/11b9eba2a48b793d00ba04d7ace8f644c2a14f97 and after devstack hit this in rocky | |
| 12:52:12 | sean-k-mooney | zigo: i think the fix for debian is to jsut continue to use the nova-compute.conf and not use nova.conf at all | |
| 12:52:36 | openstackgerrit | Lee Yarwood proposed openstack/nova-specs master: WIP - Image and flavor defined ephemeral storage encryption https://review.opendev.org/752284 | |
| 12:54:35 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Update contributor guide for Wallaby https://review.opendev.org/754427 | |
| 12:54:50 | openstackgerrit | Takashi Natsume proposed openstack/nova stable/victoria: Fix a hacking test https://review.opendev.org/758112 | |
| 12:54:58 | sean-k-mooney | zigo: by the way i consider having the db cred in the compute service to be a low severity securtiy issue which is why i have been pushing to get them removed for years | |
| 12:56:19 | sean-k-mooney | its low severity because you have to break out of the guest to get them and if you can get to the config you can get the rabbit creds anyway | |
| 13:02:07 | sean-k-mooney | zigo: just loking at https://salsa.debian.org/openstack-team/services/nova/-/blob/debian/victoria/debian/nova-compute.init.in i dont see nova.conf being included | |
| 13:03:47 | sean-k-mooney | owalsh: by the way we do not need a deprectation cycle for this | |
| 13:04:09 | sean-k-mooney | owalsh: we have not supported having the db cred in the compute agent since grizzly | |
| 13:04:47 | sean-k-mooney | we already had a dprecation cycle for this and it was raised a a relase blocker for our downstrem product for 16.1 | |
| 13:05:00 | owalsh | sean-k-mooney: may have been the intention but that's not what happend in the rpms/debs AFAICT | |
| 13:05:21 | sean-k-mooney | other tools have fixed it | |
| 13:05:30 | owalsh | sean-k-mooney: I can fix tripleo/16.1, it's puppet-nova and rpm installs that I'm concerned about | |
| 13:05:31 | sean-k-mooney | like kolla and devstack | |
| 13:06:00 | tkajinam | what would happen for example in the deployment where they use rpm and some other config management tools ? | |
| 13:06:07 | sean-k-mooney | well that can also be fix by using a nova-compute.conf | |
| 13:06:12 | tkajinam | for example if the deployment has ironic usually nova-compute runs on controller nodes | |
| 13:06:25 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.text_type (2/2) https://review.opendev.org/728117 | |
| 13:06:32 | sean-k-mooney | tkajinam: you use a spereate file that is all that is required | |
| 13:06:45 | owalsh | sean-k-mooney: the rpm systemd unit does not use a separate file | |
| 13:06:54 | sean-k-mooney | right that is a bug | |
| 13:07:20 | sean-k-mooney | and to be clear i first raised this as a potential security issues a few years ago | |
| 13:07:35 | sean-k-mooney | when i was still at intel | |
| 13:08:26 | sean-k-mooney | we had direct question form some telco partners we were workign with wanting to know if nova compute agtully need direct db access | |
| 13:08:58 | sean-k-mooney | we told them no it just need rabbmq because all db acces form the comptue is mideiated vai the conductor | |
| 13:10:07 | sean-k-mooney | the local conductor support was deprecated in 2015 https://github.com/openstack/nova/blob/master/releasenotes/notes/deprecate-local-conductor-9cb9f45728281eb0.yaml | |
| 13:10:44 | tkajinam | I understood that nova-compute doesn't need db access now, so these settings are indeed unnecessary and unnecessarily causing a security risk | |
| 13:11:02 | sean-k-mooney | tkajinam: it has not needed it for 10 release | |
| 13:11:51 | sean-k-mooney | tkajinam: all that is changing now is a condtional check for service version that happend when the rpc version was auto | |
| 13:11:57 | sean-k-mooney | is not done always | |
| 13:12:08 | sean-k-mooney | that check breaks if the db creds are in the config | |
| 13:12:47 | sean-k-mooney | we have patchs to work around it but we do not want to misslead people by allowing them to store db cred that will never be used in teh compute agent config | |
| 13:12:59 | tkajinam | yeah I see | |
| 13:13:52 | sean-k-mooney | if you had the rpc upgrade level set to auto you have not been able to have the api db creds in the db since i think rocky when cells v2 was added | |
| 13:13:54 | tkajinam | I do agree that we should go forward to remove unnecessary db creds, but at the same time we need some migration period so that we can prepare expected config structure | |
| 13:14:19 | tkajinam | at lease we need some changes in packaging (for default config files) and puppet-nova at least | |
| 13:14:36 | owalsh | and docs I expect | |
| 13:15:12 | owalsh | sean-k-mooney: can we use something other than CONF.api_database_connection to determine the cell scope? | |
| 13:15:36 | sean-k-mooney | not really | |
| 13:15:49 | sean-k-mooney | we could but i dont think we should | |
| 13:16:15 | sean-k-mooney | from a donwstream point of view we shoudl be fixing this in osp 13 as well | |
| 13:16:31 | sean-k-mooney | certenly in 16.1/16.2 | |
| 13:17:01 | owalsh | sean-k-mooney: yea, not concerned about this in tripleo, it's the other use cases | |
| 13:17:33 | sean-k-mooney | we had agreed not to block this in stable by the way | |
| 13:17:40 | tkajinam | my concern here would be that this is now breaking puppet jobs because we need new config file structure to start nova services in a single node | |
| 13:17:44 | sean-k-mooney | we are only going to make it a hard error on master/wallabyt | |
| 13:17:53 | sean-k-mooney | so packages have all of wallaby to update | |
| 13:18:21 | sean-k-mooney | we are going to backprot the patch below it which logs a warning to stable | |
| 13:20:01 | tkajinam | you mean that you will add some fall back mechanism ? | |
| 13:20:06 | sean-k-mooney | tkajinam: we could potentially delay merging the blocker patch a few week to give puppet-nova some time to update it be i think we should still merge it this cycle without a deprecation notice since it has not been supprot for a very long time but we can add an upgrade warning | |
| 13:20:14 | sean-k-mooney | its already proposed | |
| 13:20:50 | sean-k-mooney | https://review.opendev.org/#/c/762175/1/nova/utils.py | |
| 13:21:28 | sean-k-mooney | the bottom ptach catches the db not allowed excption and converts it to a warning. then does the local cell lookup | |
| 13:21:37 | tkajinam | from puppet perspective we need to know how distros change their default file structure | |
| 13:21:43 | sean-k-mooney | owalsh: tkajinam | |
| 13:21:52 | tkajinam | sean-k-mooney, yeah. that's what I exactly expected | |
| 13:22:09 | sean-k-mooney | right so that is goign to be backported to stable | |
| 13:22:16 | owalsh | sean-k-mooney: ack | |
| 13:22:21 | sean-k-mooney | ast patch will only be on master | |
| 13:22:33 | sean-k-mooney | * the last | |
| 13:22:52 | owalsh | sean-k-mooney: hard fail can't merge till rpms/debs/puppet are updated | |
| 13:23:09 | sean-k-mooney | well on the deb front its not using nova.conf | |
| 13:23:18 | sean-k-mooney | https://salsa.debian.org/openstack-team/services/nova/-/blob/debian/victoria/debian/nova-compute.init.in | |
| 13:23:33 | sean-k-mooney | the only deamon arge is nova-compute.conf | |
| 13:23:44 | sean-k-mooney | we wont use nova.conf if that is passed | |
| 13:23:50 | owalsh | but does it exists? | |
| 13:23:52 | tkajinam | what happens if /etc/nova/nova-compute.conf does not exist ? | |
| 13:23:57 | sean-k-mooney | it would only fall backk to nova.conf if its not there | |
| 13:24:05 | tkajinam | I guess nova picks up /etc/nova/nova.conf by default | |
| 13:24:31 | sean-k-mooney | so by default it will use policy in code, then nova.conf if nova-compute.conf is not there | |
| 13:24:44 | sean-k-mooney | * default in code not policy | |
| 13:25:01 | tkajinam | the question here would be whether nova-compute.conf is created by default when we install nova-compute package. | |
| 13:25:24 | tkajinam | sean-k-mooney, yeah I remember there is a logic to search some files... | |
| 13:25:44 | sean-k-mooney | not search | |
| 13:25:55 | tkajinam | oops, sorry | |
| 13:25:56 | sean-k-mooney | nova will use nova.conf if its in /etc/nova/ | |
| 13:26:08 | sean-k-mooney | if you pass a config file it only looks at that | |
| 13:26:30 | sean-k-mooney | so there is a default file location that we check otherwise you have to say where it is | |
| 13:26:56 | tkajinam | by --config-file option in the command line | |
| 13:27:02 | sean-k-mooney | yep | |
| 13:27:19 | gibi | sean-k-mooney: the default file search algo is in oslo.config https://github.com/openstack/oslo.config/blob/68cefad313bd03522e99b3de95f1786ebea45d4b/oslo_config/cfg.py#L281 | |
| 13:27:51 | sean-k-mooney | oh | |
| 13:27:53 | sean-k-mooney | so https://github.com/openstack/oslo.config/blob/68cefad313bd03522e99b3de95f1786ebea45d4b/oslo_config/cfg.py#L293-L298 | |
| 13:28:13 | sean-k-mooney | ok so it check a cople of other location byu default | |
| 13:28:18 | sean-k-mooney | for the nova.conf | |
| 13:28:19 | tkajinam | ahh yeah | |
| 13:28:34 | tkajinam | that's what I used to check | |
| 13:29:45 | sean-k-mooney | so i think the path forward is to merge and backport the first 2 patch and send a mail to the list that we intend to merge the last one but delay it till say m1 or m2 | |
| 13:30:09 | sean-k-mooney | to give packagers and installers time to catch up on there grizzly homework | |
| 13:30:43 | gibi | I can summarize this to an ML post | |
| 13:30:54 | tkajinam | lol, but indeed > grizzly homework | |
| 13:32:30 | tkajinam | once packaging team publishes the change for that then I'll update puppet-nova . | |
| 13:32:55 | tkajinam | but I'm a bit concerned with some feedback about this hard requirement since it can affect some existing deployments. | |
| 13:33:15 | sean-k-mooney | it should only affect them on upgrade though | |
| 13:33:42 | tkajinam | agreed | |
| 13:33:55 | sean-k-mooney | if they are using an installer i dont think most will see it | |
| 13:34:06 | sean-k-mooney | kolla has fixed this since queens | |