Skip to content

Conversation

karencfv
Copy link
Contributor

@karencfv karencfv commented Jul 30, 2025

This commit introduces a new field in TufArtifactMeta that maps an RoT/RoT bootloader artifact with its associated sign. This will be necessary for the planner to support RoT and RoT bootloader updates.

NOTE: There is still a test failing. I'm going to investigate what's going on, but I'd like to set this PR as ready to review to get comments on the structure of the tables, etc.

@karencfv karencfv changed the title [tuf] Store rkth/sign hashes in artifact metadata [tuf] Store rkth/sign hashes in TUF repo description Aug 4, 2025
@karencfv karencfv marked this pull request as ready for review August 5, 2025 08:49
@karencfv karencfv requested review from iliana and plotnick August 5, 2025 08:49
Copy link
Contributor Author

@karencfv karencfv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look @iliana!

This makes me lean toward having a signed_by_rtkh: Option<Vec> on the external TufArtifactMeta. This is not something we have to do in the db-model structs; we can fill in the signed_by_rtkh field when we convert from the db-model crates to the external crates in the into_external function.

😄 , That's exactly what I had in the first iteration of this PR. Frankly, it'll be much easier to use the sign in the planner if it's contained in TufArtifactMeta so I'm all for keeping it this way. I've made the adaptations and I think this is ready for another review

@karencfv karencfv requested a review from iliana August 6, 2025 03:56
Copy link
Contributor

@plotnick plotnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the mechanism here is fine, and the implementation straightforward enough. My main comment is about the terminology, and so the field names.

The SIGN field of a Hubris archive intended to run on a RoT contains an LPC55 RKTH value. But those are separate concepts, and the caboose is independent of RoT/SP. I think the only real advantage of using the name sign over rkth is that it allows for the future possibility of signed SP images (which won't use a RKTH, they'll use something else). To support that future without coming back and renaming everything here, I think we should call this field just sign, and similarly drop the rot_ prefix from the names of all associated database tables, columns, fields, etc.

If we don't want to go in that direction, then I think we should call this field rot_rkth. But I think rot_sign isn't ideal.

@karencfv
Copy link
Contributor Author

karencfv commented Aug 6, 2025

Thanks for taking a look @plotnick! I've addressed your comments in bc394ba

@karencfv karencfv requested a review from plotnick August 6, 2025 22:08
Copy link
Contributor

@plotnick plotnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks great. Looks like it needs a cargo xtask openapi generate update, but otherwise I say 🚢

@karencfv karencfv enabled auto-merge (squash) August 6, 2025 23:17
@karencfv karencfv merged commit e15b5d9 into oxidecomputer:main Aug 7, 2025
17 checks passed
@karencfv karencfv deleted the include-rkth-in-artifact-metadata branch August 7, 2025 01:18
jgallagher added a commit that referenced this pull request Aug 22, 2025
…TufArtifactMeta::board`) (#8872)

When trying Reconfigurator-based updates on a racklette today with a
real TUF repo, the planner skipped all the RoT bootloader and RoT
updates because none of the artifact `name`s matched the `board`s
reported in inventory. Our tests were assuming these were the same (and
in prod they _are_ the same on the SP), but in real TUF repos these
don't match: all the RoT and bootloaders report a `board` of
`oxide-rot-1`, but the artifact names are pretty varied.

This PR adds a new `board` value to the `tuf_artifact` table
and the `TufArtifactMeta` struct. Outside of tests, the changes are
almost identical to those from #8729 that added a `sign` field in this
same spot. This allows us to stop assuming that `artifact.name` ==
`inventory_caboose.board`, and instead check `artifact.board` ==
`inventory_caboose.board`. It depends on
oxidecomputer/tufaceous#40, which changes the
way our fake data is generated to be more consistent with the state of
production devices and artifacts. This perturbs some of the
reconfiguartor-cli tests a bit, which significantly inflates the diff;
the actual code changes here are pretty small.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants