Add project and silo ids to VM attestation#1114
Conversation
|
💯 agree w/ policy element here. In your case you're looking for something akin to a group membership instead of a unique identifier for the instance. These are the only notion of "group" that we have currently but, as you note, a generic
|
totally agree that setting up an initial
I'm pretty sure we just picked the name |
|
Thanks! I'll take another pass here and cleanup that interface. |
ed15cd0 to
23ce881
Compare
|
Turning this into a real PR now. I refactored the interface and renamed |
219782d to
6ff504a
Compare
|
Updated dependency version pins and rebased against main. |
iximeow
left a comment
There was a problem hiding this comment.
I'd missed that the dice-util side of things is landed now, sorry about that - one naming thought which I'm not deeply wedded to..
merge if you want, if you leave the name as-is I'll probably take a swing at it later.
| } | ||
|
|
||
| pub fn prepare_instance_conf( | ||
| pub fn prepare_init_state( |
There was a problem hiding this comment.
not to quibble on names too much but the idea was that this is where we can kick off whatever async work needs to happen to go from the initial derivable-from-instance-spec information to the actual conf we'd use for attestation.
maybe measure_instance is a better name? init state is kind of just an implementation detail tracking in getting the instance information through to the attestation side of things.
There was a problem hiding this comment.
Happy to rename this, I definitely do not know much of the larger context here and have no attachment to it :)
There was a problem hiding this comment.
Updated this fn name and fixed the related failure message so that it matches. If we are good otherwise I'll get this merged tomorrow.
Update Crucible from `7103cd3a` to `bd9a0e2a`, picking up the following PRs: - Use an explicit rev for oxidecomputer git deps (oxidecomputer/crucible#1936) - Add Clone and Deserialize to VolumeInfo et al (oxidecomputer/crucible#1935) - Update omicron/oximeter (oxidecomputer/crucible#1933) - [meta] update to drift 0.1.4 (oxidecomputer/crucible#1932) - Don't log if there is nothing to log (oxidecomputer/crucible#1930) - Add VolumeInfo (oxidecomputer/crucible#1928) - Remove bonus Volume layer (oxidecomputer/crucible#1927) - Add session and client id to panic messages (oxidecomputer/crucible#1926) - [crucible-agent-types] migrate to RFD 619 pattern (oxidecomputer/crucible#1899) - Background read-only region creation (oxidecomputer/crucible#1919) - [crucible-downstairs-repair] switch to RFD 619 pattern (oxidecomputer/crucible#1901) - [crucible-pantry] switch to RFD 619 pattern (oxidecomputer/crucible#1900) - Use separate in-memory types (oxidecomputer/crucible#1913) - Remove old field from dtrace action script (oxidecomputer/crucible#1917) - Retry data writes that return an IO error (oxidecomputer/crucible#1915) - Bump dropshot to 0.17.0 (oxidecomputer/crucible#1909) - Reject snapshot requests when read-only (oxidecomputer/crucible#1914) - update ringbuf method, fix clippy lint (oxidecomputer/crucible#1904) - bump vergen-v9 version too (oxidecomputer/crucible#1903) - update dropshot to 0.16.7, dropshot-api-manager to 0.5.2 (oxidecomputer/crucible#1851) - perf-vol.d updates (oxidecomputer/crucible#1898) - upgrade progenitor to 0.13, reqwest to 0.13 (oxidecomputer/crucible#1854) - Remove cargo nextest from github workflow, out of space (oxidecomputer/crucible#1846) - Add a test for VCR serialize/deserialize (oxidecomputer/crucible#1843) Update Propolis from `bc489ddf` to `58ab73bd`, picking up the following PRs: - Bump crucible to latest, update Omicron, use explicit revs (oxidecomputer/propolis#1141) - Add project and silo ids to VM attestation (oxidecomputer/propolis#1114) - Update escargot (oxidecomputer/propolis#1139) - Prefix shebang and mark D scripts as executable (oxidecomputer/propolis#1140) - Fix error in propolis-server README (oxidecomputer/propolis#1138) - [meta] update to drift 0.1.4 (oxidecomputer/propolis#1137) - Fix Intel CPUID leaf 4 cache topology for SMT (oxidecomputer/propolis#1002) - support NVMe Deallocate (oxidecomputer/propolis#1105) - viona: do not lose used/avail indices (oxidecomputer/propolis#1135) - viona: multiqueue device should stay multiqueue across migration (oxidecomputer/propolis#1121) - Bump crucible rev to latest (oxidecomputer/propolis#1132) - expand zerocopy IntoBytes/FromByes use in guest memory accesses (oxidecomputer/propolis#1130) - dropshot-api-manager 0.7.1 (oxidecomputer/propolis#1129) - improve slog component setting (oxidecomputer/propolis#1124) - wait for viona Poller to run before declaring device running (oxidecomputer/propolis#1118) - virtio: tolerate importing queues with adjusted size (oxidecomputer/propolis#1117) - Run viona unit tests in CI (oxidecomputer/propolis#1120) - feature gate Crucible-specific boot digest code (oxidecomputer/propolis#1119) Also: - ran `cargo update -p vergen` - removed the `reqwest012` dependency - removed `reqwest012_client` from Nexus - ran `cargo hakari generate` and `cargo hakari manage-deps` - replace use of `ProgenitorOperationRetry` with `retry_operation_while_indefinitely` - during the region replacement drive saga, consume the new `VolumeInfo` from Propolis and use that to determine when to consider a replacement done
This relies on vm-attest#68 to add support for including the project and silo ids in the VM instance configuration data provided by attestations. These PRs are meant as discussion points, and not intended to be merged yet. This change allows upstream callers to make richer policy decisions as project and silo ids are likely to be much more stable than VM ids. For instance if we are rolling deployments we may bring up multiple VMs and move IPs between them to roll a release forwards or backwards. It would be a lot of coordination to propagate that value to all of the policy deciders during a deploy when what we would really want is to specify a policy for all VMs in a project (or possibly with a tag in a future world).
There is not much interesting going on outside of piping the ids through. That said, it makes the API for
prepare_instance_confpretty nasty as we are passing three opaque Uuids that rely on their order to be correct. Happy to change this to something like a builder or some other pattern to avoid errors.The primary focus of this PR though is to discuss if this the correct path to passing the data to the attestation. Notably we do not provide any access to sled metadata so as to split this conversion from concerns about exposing placement data.