Add measurements to blueprints #9718
Conversation
labbott
force-pushed
the
labbott/measurement_blueprints
branch
from
03bf716 to
e6a722e
Compare
labbott
changed the title
labbott
force-pushed
the
labbott/measurement_blueprints
branch
from
e6a722e to
9d27ff4
Compare
We don't actually update the measuremenst right now, that will come in follow on work.
|
|
||
| #[derive(Queryable, Clone, Debug, Selectable, Insertable)] | ||
| #[diesel(table_name = bp_single_measurements)] | ||
| //#[diesel(check_for_backend(diesel::pg::Pg))] |
| }, | ||
| hash: *self | ||
| .image_artifact_sha256 | ||
| .expect("this should always be set"), |
There was a problem hiding this comment.
Can the field be non-optional (and the corresponding db row have a NOT NULL constraint)?
| pub id: DbTypedUuid<MeasurementKind>, | ||
|
|
||
| pub image_artifact_sha256: Option<ArtifactHash>, | ||
| pub prune: bool, |
There was a problem hiding this comment.
Discussed in chat but noting for completeness - I think we can drop the prune field since the planner has access to both the current and previous TUF repo.
| } | ||
| } | ||
|
|
||
| impl From<BTreeSet<BlueprintSingleMeasurement>> for BlueprintMeasurements { |
There was a problem hiding this comment.
These Froms seem fine-ish but surprising - could wherever we're currently using BTreeSet<BlueprintSingleMeasurement> use BlueprintMeasurements instead?
| self.measurements.insert(single); | ||
| } | ||
|
|
||
| // An empty measurement set here corresponds to the install dataset |
There was a problem hiding this comment.
Is this property guaranteed? Wondering if there are edge cases where we could have an empty set of measurements but that didn't imply "use the install dataset" - maybe dev/test systems? Or if this is a surprising security-related property in the future sometime if someone assumes "empty set" means "won't be able to talk" but it actually means "falls back to whatever was most recently written to the install dataset (possibly months/years ago)".
There was a problem hiding this comment.
Yeah this is a bit of a strange edge case. I did a first pass with this being an enum like BlueprintZoneImageSource but I'm trying to remember why doing that here was ugly. It's possible I went a layer too far in getting rid of indirection. I'll play around with this more.
That said, there eventually should be no case in the future when this is empty past the initial install. As we get further along and more confident I expect we'll want to make that a stronger assertion.
No description provided.