Earlier  
Posted Nick Remark
#openstack-nova - 2020-11-11
12:29:01 gibi zigo: thanks
12:29:04 gibi zigo: yes it does
12:29:15 zigo Oh, I see, it's about https://review.opendev.org/#/c/762176/
12:29:30 zigo Well, indeed, -1 for me as it would prevent an all-in-one setup.
12:29:31 gibi zigo: yes
12:29:40 gibi zigo: I guess that patch breaks debian then too
12:30:26 gibi in the all-in-one-case
12:30:45 gibi as nova.conf will have a [database]connection config but
12:30:50 zigo It'd be ok if nova-compute was using a different filename, in which case we should just not add the [database] section for nova-compute.
12:31:17 gibi zigo: owalsh figured out the nova-compute binary reads nova.conf by default
12:31:49 gibi so if there is no --config-file added to the binary then it reads the same file as other nova binaries in an all-in-one deployment
12:31:50 zigo Please don't attempt to use /etc/nova/nova-compute.conf, as this is already in use in Debian.
12:32:34 zigo The Debian package already does:
12:32:34 zigo nova-compute --config-file=/etc/nova/nova.conf --config-file=/etc/nova/nova-compute.conf --log-file=/var/log/nova/nova-compute.log
12:32:34 zigo to startup nova-compute.
12:33:12 zigo Though, feel free to make it use something else, like /etc/nova/nova-compute-service.conf, then nova-compute can get rid of reading /etc/nova/nova.conf reading...
12:33:26 zigo (in fact, whatever, I'll adapt the packaging anyways...)
12:33:53 gibi zigo: thanks for the input
12:34:11 gibi I will try to get oppinions from the rest of the core team before moving forward
12:43:42 sean-k-mooney zigo: nova-compute should not need to read nova.conf today
12:44:03 sean-k-mooney zigo: what you could do is remove all the db creds form nova.conf and put them in nova-db.conf
12:44:15 sean-k-mooney then put common config in nova.conf
12:44:28 sean-k-mooney and start nova compute with -config-file=/etc/nova/nova.conf --config-file=/etc/nova/nova-compute.conf
12:44:41 sean-k-mooney and nova-api with -config-file=/etc/nova/nova.conf --config-file=/etc/nova/nova-db.conf
12:44:48 sean-k-mooney or similar
12:45:34 sean-k-mooney i really dont think we need to add a new file
12:46:09 sean-k-mooney we could but the expectation was for a very long time that nova.conf is used by the contol plane service only
12:46:31 openstackgerrit Takashi Natsume proposed openstack/nova master: Remove six.binary_type/integer_types/string_types https://review.opendev.org/728094
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

Earlier   Later