| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-12 | |||
| 13:40:31 | mriedem | what i don't understand is this part from the config option help: | |
| 13:40:32 | mriedem | "During image signature verification nova will first verify the validity of the | |
| 13:40:32 | mriedem | image's signing certificate using the set of trusted certificates associated | |
| 13:40:32 | mriedem | with the instance." | |
| 13:40:43 | mriedem | at this point in the series, the instance doesn't have trusted certs | |
| 13:40:46 | mriedem | that comes later in the api change | |
| 13:41:19 | bauzas | yeah I saw | |
| 13:41:45 | bauzas | mriedem: in https://github.com/openstack/nova/commit/05bf7269 L113, they explain what they want | |
| 13:42:07 | mriedem | right that's what i asked about here https://review.openstack.org/#/c/479949/76/nova/image/glance.py@385 | |
| 13:42:42 | mriedem | this is the API change https://review.openstack.org/#/c/486204/109/nova/compute/api.py@970 | |
| 13:42:54 | mriedem | elif (CONF.glance.verify_glance_signatures and | |
| 13:42:54 | mriedem | CONF.glance.enable_certificate_validation and | |
| 13:42:54 | mriedem | CONF.glance.default_trusted_certificate_ids): | |
| 13:42:54 | mriedem | certs_to_return = objects.TrustedCerts( | |
| 13:42:54 | mriedem | ids=CONF.glance.default_trusted_certificate_ids) | |
| 13:43:15 | mriedem | ^ is essentially this glance download() logic except it doesn't use default_trusted_certificate_ids | |
| 13:45:01 | bauzas | mriedem: mmm, I don't like the API magically guessing which certs IDs to use | |
| 13:46:09 | bauzas | mriedem: so, IIUC, we store the certs in the instance ? | |
| 13:46:37 | bauzas | mriedem: and then, later on, the virt driver passes the certs to the glance API | |
| 13:47:03 | mriedem | if the user provides trusted cert IDs, we use those, | |
| 13:47:31 | mriedem | if the user does not provide trusted cert IDs, but image sig verification is enabled, and default_trusted_certificate_ids is set, we use those | |
| 13:47:34 | bauzas | oh, and if it doesn't but nova is configured, we store the default ones | |
| 13:47:37 | mriedem | yes they are persisted with the instance | |
| 13:47:45 | mriedem | by default, there are no default ones, but yes | |
| 13:47:59 | bauzas | ok, so, follow my logic about why I don't like us persisting the defaults | |
| 13:48:19 | bauzas | say I'm an operator and I did set some default certs | |
| 13:48:36 | bauzas | some users went by, and some instances now persisted my default certs | |
| 13:48:40 | bauzas | that's awesome | |
| 13:49:02 | bauzas | but then, I need to remove those default certs because of any security concern | |
| 13:49:10 | bauzas | revoke it, if you prefer | |
| 13:49:14 | bauzas | but then I'm stuck | |
| 13:49:27 | mriedem | you can rebuild those instances | |
| 13:49:41 | bauzas | because I don't know whether the cert is in the instance was persisted because the user asked for it, or because we defaulted it | |
| 13:50:04 | mriedem | well, they are IDs | |
| 13:50:06 | bauzas | of course, yeah you can rebuild | |
| 13:50:20 | mriedem | so if you were looking at the db, then you know which cert IDs were your defaults that you're revoking | |
| 13:50:41 | bauzas | but from a guessing logic, I have no idea whether my IDs that are in the instance are those given by the user or by a very old configuration | |
| 13:51:24 | mriedem | i'm not sure how much this matters, | |
| 13:51:49 | mriedem | if the instance is storing cert IDs for defaults that you've revoked, those cert IDs aren't getting used - they were used when spawning the instance | |
| 13:52:10 | mriedem | they would fail if you tried to rebuild the instance w/o providing new trusted certs | |
| 13:52:18 | sahid | dansmith, jaypipes, you might have already think about that but... what about to add a 'version' attribute for our objects fields | |
| 13:52:27 | mriedem | or rebuilding and specifying trusted_certs=None to get the new defaults i guess | |
| 13:52:33 | sahid | something like fields.IntegerField(version=2.0, nullable=True) | |
| 13:52:55 | sahid | so we could remove that obj_make_compatible method from all of our objects | |
| 13:52:58 | mriedem | sahid: there is already a version on the object | |
| 13:53:00 | sahid | to have it only in the base class | |
| 13:53:08 | mriedem | oh version per field.... | |
| 13:53:12 | sahid | yes | |
| 13:53:33 | dansmith | sahid: removing the field isn't the only action we need to take during a backport, | |
| 13:53:40 | dansmith | it's just a common one | |
| 13:54:08 | sahid | but that seems a bit hacky to have that obj_make_compatible() method everywhere, no? | |
| 13:54:29 | sahid | we could probably provide something smart to avoid that change each time we add new field | |
| 13:55:05 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove network info stubbing in functional test https://review.openstack.org/570018 | |
| 13:55:06 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add request_spec.RequestGroup versioned object https://review.openstack.org/568840 | |
| 13:55:07 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add requested_resources field to RequestSpec https://review.openstack.org/567267 | |
| 13:55:08 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add bandwidth related standard resource classes https://review.openstack.org/570847 | |
| 13:55:09 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transfer port.resource_request to the scheduler https://review.openstack.org/567268 | |
| 13:55:10 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Send resource allocations in the port binding https://review.openstack.org/569459 | |
| 13:55:11 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Test boot with more ports with bandwidth request https://review.openstack.org/573317 | |
| 13:55:33 | dansmith | I dunno, it's two lines of code per thing we add, and it makes people think about what needs to happen to backport the thing | |
| 13:56:18 | dansmith | we used to have a mapping for backporting nested objects, and maintaining that was not free either | |
| 13:56:22 | sahid | yes i don't know eother, it's just a suggestion | |
| 13:57:03 | dansmith | version per field kinda adds noise to the definition too, which isn't great | |
| 13:58:08 | sahid | yep true, but beside of that we could avoid to have the obj_make_compatible() method + unit test related | |
| 13:58:32 | dansmith | sahid: you could add to our base object an implementation of make compatible that, if field in self.obj_field_history: delete if < self.obj_field_history[field] | |
| 13:58:45 | dansmith | it'd still be one line per change though | |
| 14:00:35 | sahid | what this obj_field_history is supposed to carry? | |
| 14:00:44 | dansmith | sahid: hang on I'll show you what I mean | |
| 14:01:09 | mriedem | bauzas: http://lists.openstack.org/pipermail/openstack-dev/2018-June/131410.html | |
| 14:01:40 | mriedem | guess i can go back and read the giant spec on this | |
| 14:02:10 | openstackgerrit | Simon Dodsley proposed openstack/nova master: Add enhanced KVM storage QoS quotas https://review.openstack.org/558530 | |
| 14:02:13 | dansmith | sahid: something like this: https://pastebin.com/waWNUM3U | |
| 14:02:27 | dansmith | sahid: (that is in base.NovaObject btw) | |
| 14:03:10 | sahid | dansmith: oh i see | |
| 14:03:38 | sahid | not a big fan :) but yes it's a way to achieve the same thing | |
| 14:04:44 | mriedem | bauzas: relevant parts of the spec https://specs.openstack.org/openstack/nova-specs/specs/rocky/approved/nova-validate-certificates.html#proposed-change | |
| 14:04:51 | mriedem | https://specs.openstack.org/openstack/nova-specs/specs/rocky/approved/nova-validate-certificates.html#other-deployer-impact | |
| 14:05:30 | mriedem | i think it just means, enable_certificate_validation=True means enforce cert validation and default_trusted_certificate_ids is if you enforce and don't want to kill your users if they don't provide trusted certs in the server create/rebuild requests | |
| 14:06:04 | mriedem | both get used in the API changes, and you wouldn't want to set enable_certificate_validation=True UNTIL the api changes are in place to use default_trusted_certificate_ids | |
| 14:06:09 | mriedem | at least as things are written today | |
| 14:07:03 | mriedem | since enable_certificate_validation is deprecated, i'm not sure at what point it is removed, | |
| 14:07:09 | mriedem | nor do i understand how it's a transition to anything | |
| 14:07:41 | mriedem | i guess both options together are meant to enable the transition to cert validation of images | |
| 14:07:48 | mriedem | before forcing it on users | |
| 14:11:38 | bauzas | mriedem: sorry was afk | |
| 14:13:56 | bauzas | mriedem: " all prior usage of the server create/rebuild API when booting signed images will now fail if trusted certificates cannot be located." | |
| 14:15:21 | bauzas | mriedem: okay, so when the API change is merged, I understand that things will continue to work as defaults will be seen as user-provided | |
| 14:15:49 | simondodsley_ | mreidem: referencing the Cinder QoS change... the QoS values you see are the capacity based QoS options. These are for things like Ceph and ScaleIO (I think). The external disk QoS spec is separate to these per_gb values. The spec for external disk QoS is here: https://github.com/openstack/cinder/blob/master/cinder/volume/qos_specs.py. This doesn't appear to have any documentation, so I will create these once the | |
| 14:15:49 | simondodsley_ | Nova piece has been merged there are no Cinder changes required to add new QoS values | |
| 14:15:54 | bauzas | mriedem: later, when they want to drop the defaults and force the user to ask, then it would pop up an exception | |
| 14:16:39 | simondodsley_ | mriedem: ^ | |
| 14:17:33 | bauzas | mriedem: unrelated how can we know that vdi_remote_stream from https://review.openstack.org/#/c/486475/ is going to be CI-tested by Xen ? | |
| 14:17:49 | bauzas | because the 3rd-party CI says cool, but I don't know if it checked it | |
| 14:18:21 | mriedem | bauzas: that patch isn't using vdi_remote_stream | |
| 14:18:26 | mriedem | that's why i have https://review.openstack.org/#/c/574318/ | |
| 14:18:32 | mriedem | which hasn't had xenserver ci run on it | |
| 14:18:54 | mriedem | "Matt, thanks for this DNM patch. But it looks like the Citrix CI is out of service now. Naichuan and Bob will help to fix it." | |
| 14:19:37 | mriedem | looks like https://review.openstack.org/#/c/486446/ does it though... | |
| 14:19:50 | bauzas | mriedem: but AFAICU, your DNM change only checks UTs | |
| 14:19:56 | mriedem | no | |