From 21a94fe2e8e66df20620c04c88c8b3973420200a Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 31 Jul 2025 03:05:50 +0000 Subject: [PATCH 1/3] [spr] initial version Created using spr 1.3.6-beta.1 --- Cargo.lock | 8 +- Cargo.toml | 7 +- dev-tools/releng/src/tuf.rs | 2 + installinator-common/src/progress.rs | 3 + installinator/src/artifact.rs | 98 ++++++++-- installinator/src/dispatch.rs | 167 +++++++++++++++--- nexus/tests/integration_tests/updates.rs | 2 + .../src/artifacts/artifacts_with_plan.rs | 52 +++--- update-common/src/artifacts/update_plan.rs | 45 ++++- update-common/src/errors.rs | 4 + wicketd/src/update_tracker.rs | 38 +++- wicketd/tests/integration_tests/updates.rs | 132 ++++++++++++-- 12 files changed, 465 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f2abe1f8721..d3fadda15ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14129,7 +14129,7 @@ dependencies = [ [[package]] name = "tufaceous" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/tufaceous?branch=main#c269f5704d39eb4234e67e18be10d424ef23ac77" +source = "git+https://github.com//oxidecomputer/tufaceous?branch=sunshowers%2Fspr%2Fadd-support-for-an-installinator-document#69264b956872266e15508ffe5f4252a7fa3156a9" dependencies = [ "anyhow", "camino", @@ -14150,7 +14150,7 @@ dependencies = [ [[package]] name = "tufaceous-artifact" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/tufaceous?branch=main#c269f5704d39eb4234e67e18be10d424ef23ac77" +source = "git+https://github.com//oxidecomputer/tufaceous?branch=sunshowers%2Fspr%2Fadd-support-for-an-installinator-document#69264b956872266e15508ffe5f4252a7fa3156a9" dependencies = [ "daft", "hex", @@ -14167,7 +14167,7 @@ dependencies = [ [[package]] name = "tufaceous-brand-metadata" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/tufaceous?branch=main#c269f5704d39eb4234e67e18be10d424ef23ac77" +source = "git+https://github.com//oxidecomputer/tufaceous?branch=sunshowers%2Fspr%2Fadd-support-for-an-installinator-document#69264b956872266e15508ffe5f4252a7fa3156a9" dependencies = [ "semver 1.0.26", "serde", @@ -14179,7 +14179,7 @@ dependencies = [ [[package]] name = "tufaceous-lib" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/tufaceous?branch=main#c269f5704d39eb4234e67e18be10d424ef23ac77" +source = "git+https://github.com//oxidecomputer/tufaceous?branch=sunshowers%2Fspr%2Fadd-support-for-an-installinator-document#69264b956872266e15508ffe5f4252a7fa3156a9" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index f09163a5aa2..ace8829f070 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -964,11 +964,16 @@ opt-level = 3 # propolis-client = { path = "../propolis/lib/propolis-client" } # propolis-mock-server = { path = "../propolis/bin/mock-server" } -# [patch."https://github.com/oxidecomputer/tufaceous"] +[patch."https://github.com/oxidecomputer/tufaceous"] # tufaceous = { path = "../tufaceous/bin" } # tufaceous-artifact = { path = "../tufaceous/artifact" } # tufaceous-brand-metadata = { path = "../tufaceous/brand-metadata" } # tufaceous-lib = { path = "../tufaceous/lib" } +# Extra slash here to pretend to be a different source. +tufaceous = { git = "https://github.com//oxidecomputer/tufaceous", branch = "sunshowers/spr/add-support-for-an-installinator-document" } +tufaceous-artifact = { git = "https://github.com//oxidecomputer/tufaceous", branch = "sunshowers/spr/add-support-for-an-installinator-document" } +tufaceous-brand-metadata = { git = "https://github.com//oxidecomputer/tufaceous", branch = "sunshowers/spr/add-support-for-an-installinator-document" } +tufaceous-lib = { git = "https://github.com//oxidecomputer/tufaceous", branch = "sunshowers/spr/add-support-for-an-installinator-document" } # [patch."https://github.com/oxidecomputer/typify"] # typify = { path = "../typify/typify" } diff --git a/dev-tools/releng/src/tuf.rs b/dev-tools/releng/src/tuf.rs index 403392dfd78..5875f43e30e 100644 --- a/dev-tools/releng/src/tuf.rs +++ b/dev-tools/releng/src/tuf.rs @@ -21,6 +21,7 @@ use tokio::io::AsyncReadExt; use tufaceous_artifact::ArtifactHash; use tufaceous_artifact::ArtifactVersion; use tufaceous_artifact::KnownArtifactKind; +use tufaceous_lib::IncludeInstallinatorDocument; use tufaceous_lib::Key; use tufaceous_lib::assemble::ArtifactManifest; use tufaceous_lib::assemble::DeserializedArtifactData; @@ -143,6 +144,7 @@ pub(crate) async fn build_tuf_repo( manifest, keys, expiry, + IncludeInstallinatorDocument::Yes, output_dir.join("repo.zip"), ) .build() diff --git a/installinator-common/src/progress.rs b/installinator-common/src/progress.rs index 3a3b9c316ae..43188b321e4 100644 --- a/installinator-common/src/progress.rs +++ b/installinator-common/src/progress.rs @@ -43,6 +43,9 @@ impl StepSpec for InstallinatorSpec { )] #[serde(rename_all = "snake_case")] pub enum InstallinatorComponent { + /// The installinator document. + InstallinatorDocument, + /// The host phase 2 component. HostPhase2, diff --git a/installinator/src/artifact.rs b/installinator/src/artifact.rs index 695e395c1bb..405159ebabc 100644 --- a/installinator/src/artifact.rs +++ b/installinator/src/artifact.rs @@ -12,14 +12,19 @@ use installinator_common::EventReport; use ipcc::{InstallinatorImageId, Ipcc}; use omicron_uuid_kinds::MupdateUuid; use tokio::sync::mpsc; -use tufaceous_artifact::{ArtifactHash, ArtifactHashId}; +use tufaceous_artifact::{ + ArtifactHash, ArtifactHashId, ArtifactKind, KnownArtifactKind, +}; use crate::{errors::HttpError, fetch::FetchReceiver}; #[derive(Clone, Debug, Eq, PartialEq, Args)] pub(crate) struct ArtifactIdOpts { /// Retrieve artifact ID from IPCC - #[clap(long, required_unless_present_any = ["update_id", "host_phase_2", "control_plane"])] + #[clap( + long, + required_unless_present_any = ["update_id", "host_phase_2", "control_plane", "installinator_doc"] + )] from_ipcc: bool, #[clap( @@ -31,31 +36,100 @@ pub(crate) struct ArtifactIdOpts { #[clap( long, - conflicts_with = "from_ipcc", - required_unless_present = "from_ipcc" + conflicts_with_all = ["from_ipcc", "installinator_doc"], + required_unless_present_any = ["from_ipcc", "installinator_doc"], )] host_phase_2: Option, #[clap( long, - conflicts_with = "from_ipcc", - required_unless_present = "from_ipcc" + conflicts_with_all = ["from_ipcc", "installinator_doc"], + required_unless_present_any = ["from_ipcc", "installinator_doc"], )] control_plane: Option, + + #[clap( + long, + conflicts_with_all = ["from_ipcc", "host_phase_2", "control_plane"], + required_unless_present_any = ["from_ipcc", "host_phase_2", "control_plane"], + )] + installinator_doc: Option, } impl ArtifactIdOpts { - pub(crate) fn resolve(&self) -> Result { + pub(crate) fn resolve(&self) -> Result { if self.from_ipcc { let ipcc = Ipcc::new().context("error opening IPCC")?; - ipcc.installinator_image_id() - .context("error retrieving installinator image ID") + let image_id = ipcc + .installinator_image_id() + .context("error retrieving installinator image ID")?; + Ok(LookupId::from_image_id(&image_id)) } else { let update_id = self.update_id.unwrap(); - let host_phase_2 = self.host_phase_2.unwrap(); - let control_plane = self.control_plane.unwrap(); + let kind = + if let Some(installinator_doc_hash) = self.installinator_doc { + LookupIdKind::Document(installinator_doc_hash) + } else { + LookupIdKind::Hashes { + host_phase_2: self.host_phase_2.unwrap(), + control_plane: self.control_plane.unwrap(), + } + }; + + Ok(LookupId { update_id, kind }) + } + } +} + +/// Identifiers used by installinator to retrieve artifacts. +pub(crate) struct LookupId { + pub(crate) update_id: MupdateUuid, + pub(crate) kind: LookupIdKind, +} + +impl LookupId { + fn from_image_id(image_id: &InstallinatorImageId) -> Self { + // This sentinel hash is used to indicate that the host phase 2 hash is + // actually the hash to the installinator document. + let kind = if image_id.control_plane == ArtifactHash([0; 32]) { + LookupIdKind::Document(image_id.host_phase_2) + } else { + LookupIdKind::Hashes { + host_phase_2: image_id.host_phase_2, + control_plane: image_id.control_plane, + } + }; + + Self { update_id: image_id.update_id, kind } + } +} + +/// Either an installinator document hash, or host phase 2 and control plane +/// hashes. +pub(crate) enum LookupIdKind { + Document(ArtifactHash), + Hashes { host_phase_2: ArtifactHash, control_plane: ArtifactHash }, +} + +/// The host phase 2 and control plane hashes to download. +#[derive(Clone, Debug)] +pub(crate) struct ArtifactsToDownload { + pub(crate) host_phase_2: ArtifactHash, + pub(crate) control_plane: ArtifactHash, +} + +impl ArtifactsToDownload { + pub(crate) fn host_phase_2_id(&self) -> ArtifactHashId { + ArtifactHashId { + kind: ArtifactKind::HOST_PHASE_2, + hash: self.host_phase_2, + } + } - Ok(InstallinatorImageId { update_id, host_phase_2, control_plane }) + pub(crate) fn control_plane_id(&self) -> ArtifactHashId { + ArtifactHashId { + kind: KnownArtifactKind::ControlPlane.into(), + hash: self.control_plane, } } } diff --git a/installinator/src/dispatch.rs b/installinator/src/dispatch.rs index f3504d94c65..346c267c72e 100644 --- a/installinator/src/dispatch.rs +++ b/installinator/src/dispatch.rs @@ -17,14 +17,15 @@ use omicron_common::FileKv; use sha2::{Digest, Sha256}; use slog::{Drain, error, warn}; use tufaceous_artifact::{ - ArtifactHash, ArtifactHashId, ArtifactKind, KnownArtifactKind, + ArtifactHash, ArtifactHashId, ArtifactKind, InstallinatorArtifactKind, + InstallinatorDocument, }; use tufaceous_lib::ControlPlaneZoneImages; use update_engine::StepResult; use crate::{ ArtifactWriter, WriteDestination, - artifact::ArtifactIdOpts, + artifact::{ArtifactIdOpts, ArtifactsToDownload, LookupIdKind}, fetch::{FetchArtifactBackend, FetchedArtifact, HttpFetchBackend}, peers::DiscoveryMechanism, reporter::{HttpProgressBackend, ProgressReporter, ReportProgressBackend}, @@ -192,12 +193,12 @@ impl InstallOpts { crate::bootstrap::bootstrap_sled(&data_links, log.clone()).await?; } - let image_id = self.artifact_ids.resolve()?; + let lookup_id = self.artifact_ids.resolve()?; let discovery = self.discover_opts.mechanism.clone(); let (progress_reporter, event_sender) = ProgressReporter::new( log, - image_id.update_id, + lookup_id.update_id, ReportProgressBackend::new( log, HttpProgressBackend::new(log, discovery), @@ -208,33 +209,140 @@ impl InstallOpts { let engine = UpdateEngine::new(log, event_sender); - let host_phase_2_id = ArtifactHashId { - // TODO: currently we're assuming that wicketd will unpack the host - // phase 2 image. We may instead have the installinator do it. - kind: ArtifactKind::HOST_PHASE_2, - hash: image_id.host_phase_2, - }; - let host_2_phase_id_2 = host_phase_2_id.clone(); + let to_download = match lookup_id.kind { + LookupIdKind::Document(hash) => { + engine + .new_step( + InstallinatorComponent::InstallinatorDocument, + InstallinatorStepId::Download, + "Downloading installinator document", + async move |cx| { + let installinator_doc_id = ArtifactHashId { + kind: ArtifactKind::INSTALLINATOR_DOCUMENT, + hash, + }; + let installinator_doc = fetch_artifact( + &cx, + &installinator_doc_id, + discovery, + log, + ) + .await?; + + // Check that the sha256 of the data we got from + // wicket matches the data we asked for. + // + // If this fails, we fail the entire installation + // rather than trying to fetch the artifact again, + // because we're fetching data from wicketd (cached + // in a temp dir) over TCP to ourselves (in memory), + // so the only cases where this could fail are + // disturbing enough (memory corruption, corruption + // under TCP, or wicketd gave us something other + // than what we requested) we want to know + // immediately and not retry: it's likely an + // operator could miss any warnings we emit if a + // retry succeeds. + check_downloaded_artifact_hash( + "installinator document", + installinator_doc.artifact.clone(), + installinator_doc_id.hash, + ) + .await?; + + // Read the document as JSON. + // + // Going via the reader is slightly less efficient + // than operating purely in-memory, but serde_json + // doesn't have a good way to pass in a BufList + // directly. + let json: InstallinatorDocument = + serde_json::from_reader(buf_list::Cursor::new( + &installinator_doc.artifact, + )) + .context( + "error deserializing \ + installinator document", + )?; + + // Every valid installinator document must have the + // host phase 2 and control plane hashes. + let mut host_phase_2_hash = None; + let mut control_plane_hash = None; + for artifact in &json.artifacts { + match artifact.kind { + InstallinatorArtifactKind::HostPhase2 => { + host_phase_2_hash = Some(artifact.hash); + } + InstallinatorArtifactKind::ControlPlane => { + control_plane_hash = + Some(artifact.hash); + } + } + } + + // Return an error if either the host phase 2 or the + // control plane artifacts are missing. + let mut missing = Vec::new(); + if host_phase_2_hash.is_none() { + missing.push( + InstallinatorArtifactKind::HostPhase2, + ); + } + if control_plane_hash.is_none() { + missing.push( + InstallinatorArtifactKind::ControlPlane, + ); + } + if !missing.is_empty() { + bail!( + "installinator document missing \ + required artifacts: {:?}", + missing + ); + } + + StepSuccess::new(ArtifactsToDownload { + host_phase_2: host_phase_2_hash.expect( + "host phase 2 is Some, checked above", + ), + control_plane: control_plane_hash.expect( + "control plane is Some, checked above", + ), + }) + .into() + }, + ) + .register() + } + LookupIdKind::Hashes { host_phase_2, control_plane } => { + StepHandle::ready(ArtifactsToDownload { + host_phase_2, + control_plane, + }) + } + } + .into_shared(); + + let to_download_2 = to_download.clone(); + let to_download_3 = to_download_2.clone(); let host_phase_2_artifact = engine .new_step( InstallinatorComponent::HostPhase2, InstallinatorStepId::Download, "Downloading host phase 2 artifact", async move |cx| { + let to_download = to_download.into_value(cx.token()).await; + let host_phase_2_id = to_download.host_phase_2_id(); + let host_phase_2_artifact = fetch_artifact(&cx, &host_phase_2_id, discovery, log) .await?; // Check that the sha256 of the data we got from wicket - // matches the data we asked for. If this fails, we fail the - // entire installation rather than trying to fetch the - // artifact again, because we're fetching data from wicketd - // (in memory) over TCP to ourselves (in memory), so the - // only cases where this could fail are disturbing enough - // (memory corruption, corruption under TCP, or wicketd gave - // us something other than what we requested) we want to - // know immediately and not retry: it's likely an operator - // could miss any warnings we emit if a retry succeeds. + // matches the data we asked for. We do not retry this for + // the same reasons described above when checking the + // installinator document. check_downloaded_artifact_hash( "host phase 2", host_phase_2_artifact.artifact.clone(), @@ -255,17 +363,16 @@ impl InstallOpts { ) .register(); - let control_plane_id = ArtifactHashId { - kind: KnownArtifactKind::ControlPlane.into(), - hash: image_id.control_plane, - }; - let control_plane_id_2 = control_plane_id.clone(); let control_plane_artifact = engine .new_step( InstallinatorComponent::ControlPlane, InstallinatorStepId::Download, "Downloading control plane artifact", async move |cx| { + let to_download = + to_download_2.into_value(cx.token()).await; + let control_plane_id = to_download.control_plane_id(); + let control_plane_artifact = fetch_artifact(&cx, &control_plane_id, discovery, log) .await?; @@ -345,16 +452,20 @@ impl InstallOpts { "Writing host and control plane artifacts", async move |cx| { let destination = destination.into_value(cx.token()).await; + let to_download = + to_download_3.into_value(cx.token()).await; + let host_phase_2_id = to_download.host_phase_2_id(); + let control_plane_id = to_download.control_plane_id(); let host_phase_2_artifact = host_phase_2_artifact.into_value(cx.token()).await; let control_plane_zones = control_plane_zones.into_value(cx.token()).await; let mut writer = ArtifactWriter::new( - image_id.update_id, - &host_2_phase_id_2, + lookup_id.update_id, + &host_phase_2_id, &host_phase_2_artifact.artifact, - &control_plane_id_2, + &control_plane_id, &control_plane_zones, destination, ); diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 4391fd3fd3e..f6ba8bfd773 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -26,6 +26,7 @@ use std::fmt::Debug; use tough::editor::signed::SignedRole; use tough::schema::Root; use tufaceous_artifact::KnownArtifactKind; +use tufaceous_lib::IncludeInstallinatorDocument; use tufaceous_lib::Key; use tufaceous_lib::assemble::{ArtifactManifest, OmicronRepoAssembler}; use tufaceous_lib::assemble::{DeserializedManifest, ManifestTweak}; @@ -86,6 +87,7 @@ impl TestTrustRoot { manifest, vec![self.key.clone()], self.expiry, + IncludeInstallinatorDocument::Yes, archive_path.to_path_buf(), ); assembler.set_root_role(self.root_role.clone()); diff --git a/update-common/src/artifacts/artifacts_with_plan.rs b/update-common/src/artifacts/artifacts_with_plan.rs index f358b88d69d..8667ab863cb 100644 --- a/update-common/src/artifacts/artifacts_with_plan.rs +++ b/update-common/src/artifacts/artifacts_with_plan.rs @@ -223,26 +223,8 @@ impl ArtifactsWithPlan { error: Box::new(error), })?; - let target_hash = repository - .repo() - .targets() - .signed - .find_target(&target_name, false) - .map_err(|error| RepositoryError::TargetHashRead { - target: artifact.target.clone(), - error: Box::new(error), - })? - .hashes - .sha256 - .clone() - .into_vec(); - - // The target hash is SHA-256, which is 32 bytes long. - let artifact_hash = ArtifactHash( - target_hash - .try_into() - .map_err(RepositoryError::TargetHashLength)?, - ); + let artifact_hash = + lookup_artifact_hash(&repository, artifact, &target_name)?; let stream = repository .repo() @@ -335,6 +317,30 @@ impl ArtifactsWithPlan { } } +fn lookup_artifact_hash( + repository: &OmicronRepo, + artifact: &tufaceous_artifact::Artifact, + target_name: &TargetName, +) -> Result { + let target_hash = repository + .repo() + .targets() + .signed + .find_target(target_name, false) + .map_err(|error| RepositoryError::TargetHashRead { + target: artifact.target.clone(), + error: Box::new(error), + })? + .hashes + .sha256 + .clone() + .into_vec(); + let artifact_hash = ArtifactHash( + target_hash.try_into().map_err(RepositoryError::TargetHashLength)?, + ); + Ok(artifact_hash) +} + #[derive(Debug, Clone, Copy)] pub enum ControlPlaneZonesMode { /// Ensure the control plane zones are combined into a single composite @@ -422,10 +428,11 @@ mod tests { .collect(); // `by_id` should contain one entry for every `KnownArtifactKind` - // (except `Zone`)... + // (except `Zone`), as well as `installinator_document`. let mut expected_kinds: BTreeSet<_> = KnownArtifactKind::iter() .filter(|k| !matches!(k, KnownArtifactKind::Zone)) .map(ArtifactKind::from) + .chain(std::iter::once(ArtifactKind::INSTALLINATOR_DOCUMENT)) .collect(); assert_eq!( expected_kinds, by_id_kinds, @@ -503,12 +510,13 @@ mod tests { .await?; // `by_id` should contain one entry for every `KnownArtifactKind` - // (except `ControlPlane`). + // (except `Zone`), as well as `installinator_document`. let by_id_kinds: BTreeSet<_> = plan.by_id().keys().map(|id| id.kind.clone()).collect(); let expected_kinds: BTreeSet<_> = KnownArtifactKind::iter() .filter(|k| !matches!(k, KnownArtifactKind::ControlPlane)) .map(ArtifactKind::from) + .chain(std::iter::once(ArtifactKind::INSTALLINATOR_DOCUMENT)) .collect(); assert_eq!( expected_kinds, by_id_kinds, diff --git a/update-common/src/artifacts/update_plan.rs b/update-common/src/artifacts/update_plan.rs index 53d68828600..e1c5779ed1e 100644 --- a/update-common/src/artifacts/update_plan.rs +++ b/update-common/src/artifacts/update_plan.rs @@ -72,13 +72,18 @@ pub struct UpdatePlan { pub trampoline_phase_1: ArtifactIdData, pub trampoline_phase_2: ArtifactIdData, - // We need to send installinator the hash of the host_phase_2 data it should - // fetch from us; we compute it while generating the plan. + // We need to send installinator either the hash of the installinator + // document (for newer TUF repos), or the host phase 2 and control plane + // hashes (for older TUF repos). + // + // TODO-cleanup: After r16, installinator_doc_hash will always be present. + pub installinator_doc_hash: Option, + + // We compute this while generating the plan. pub host_phase_2_hash: ArtifactHash, - // We also need to send installinator the hash of the control_plane image it - // should fetch from us. This is already present in the TUF repository, but - // we record it here for use by the update process. + // This is already present in the TUF repository, but we record it here + // for use by the update process. // // When built with `ControlPlaneZonesMode::Split`, this hash does not // reference any artifacts in our corresponding `ArtifactsWithPlan`. @@ -136,9 +141,15 @@ pub struct UpdatePlanBuilder<'a> { // In contrast to the trampoline phase 2 image, the host phase 2 image and // the control plane are fetched by installinator from us over the bootstrap - // network. The only information we have to send to the SP via MGS is the - // hash of these two artifacts; we still hold the data in our `by_hash` map - // we build below, but we don't need the data when driving an update. + // network. + // + // For newer TUF repos, this information is contained within the + // installinator document. We have to send this data to the SP via MGS. + installinator_doc_hash: Option, + + // For older TUF repos, the information we have to send to the SP via MGS is + // the hash of these two artifacts; we still hold the data in our `by_hash` + // map we build below, but we don't need the data when driving an update. host_phase_2_hash: Option, control_plane_hash: Option, @@ -181,9 +192,9 @@ impl<'a> UpdatePlanBuilder<'a> { host_phase_1: None, trampoline_phase_1: None, trampoline_phase_2: None, + installinator_doc_hash: None, host_phase_2_hash: None, control_plane_hash: None, - by_id: BTreeMap::new(), by_hash: HashMap::new(), rot_by_sign: HashMap::new(), @@ -202,6 +213,8 @@ impl<'a> UpdatePlanBuilder<'a> { artifact_hash: ArtifactHash, stream: impl Stream> + Send, ) -> Result<(), RepositoryError> { + if artifact_id.kind == ArtifactKind::INSTALLINATOR_DOCUMENT {} + // If we don't know this artifact kind, we'll still serve it up by hash, // but we don't do any further processing on it. let Some(artifact_kind) = artifact_id.kind.to_known() else { @@ -772,6 +785,17 @@ impl<'a> UpdatePlanBuilder<'a> { let data = self.extracted_artifacts.store(artifact_hash_id, stream).await?; + // The installinator document is of course known to us, but it's not + // really an artifact in the usual sense -- in fact, we want to treat it + // as an opaque blob. It's most convenient to treat it as unknown + // artifact except for recording its hash if found. + if artifact_kind == ArtifactKind::INSTALLINATOR_DOCUMENT { + if self.installinator_doc_hash.is_some() { + return Err(RepositoryError::DuplicateInstallinatorDocument); + } + self.installinator_doc_hash = Some(data.hash()); + } + self.record_extracted_artifact( artifact_id, data, @@ -1124,6 +1148,9 @@ impl<'a> UpdatePlanBuilder<'a> { KnownArtifactKind::Trampoline, ), )?, + // For backwards compatibility, installinator_doc_hash is currently + // optional. + installinator_doc_hash: self.installinator_doc_hash, host_phase_2_hash: self.host_phase_2_hash.ok_or( RepositoryError::MissingArtifactKind(KnownArtifactKind::Host), )?, diff --git a/update-common/src/errors.rs b/update-common/src/errors.rs index 2c5671642aa..bed5ae70c20 100644 --- a/update-common/src/errors.rs +++ b/update-common/src/errors.rs @@ -96,6 +96,9 @@ pub enum RepositoryError { #[error("multiple artifacts found for kind `{0:?}`")] DuplicateArtifactKind(KnownArtifactKind), + #[error("multiple installinator documents found")] + DuplicateInstallinatorDocument, + #[error("duplicate board found for kind `{kind:?}`: `{board}`")] DuplicateBoardEntry { board: String, kind: KnownArtifactKind }, @@ -196,6 +199,7 @@ impl RepositoryError { // Errors that are definitely caused by bad repository contents. RepositoryError::DuplicateArtifactKind(_) + | RepositoryError::DuplicateInstallinatorDocument | RepositoryError::LocateTarget { .. } | RepositoryError::TargetHashLength(_) | RepositoryError::MissingArtifactKind(_) diff --git a/wicketd/src/update_tracker.rs b/wicketd/src/update_tracker.rs index 3a6c9b6e494..f1ab0bc0da2 100644 --- a/wicketd/src/update_tracker.rs +++ b/wicketd/src/update_tracker.rs @@ -65,6 +65,7 @@ use tufaceous_artifact::ArtifactVersion; use update_common::artifacts::ArtifactIdData; use update_common::artifacts::ArtifactsWithPlan; use update_common::artifacts::ControlPlaneZonesMode; + use update_common::artifacts::UpdatePlan; use update_common::artifacts::VerificationMode; use update_engine::AbortHandle; @@ -1367,11 +1368,38 @@ impl UpdateDriver { UpdateStepId::SettingInstallinatorImageId, "Setting installinator image ID", async move |_cx| { - let installinator_image_id = InstallinatorImageId { - control_plane: plan.control_plane_hash.to_string(), - host_phase_2: plan.host_phase_2_hash.to_string(), - update_id: update_cx.update_id, - }; + let installinator_image_id = + match plan.installinator_doc_hash { + Some(hash) => { + // In this case (for newer TUF repos), we set the + // host phase 2 hash to the document hash, and the + // control plane hash to all zeroes. The latter acts + // as an indication to installinator that the former + // is actually a document hash. + InstallinatorImageId { + host_phase_2: hash.to_string(), + control_plane: ArtifactHash([0; 32]) + .to_string(), + update_id: update_cx.update_id, + } + } + None => { + // For older TUF repos, we follow the previous + // logic. + // + // TODO-cleanup: Once we no longer support older TUF + // repos, we can remove this logic. + InstallinatorImageId { + host_phase_2: plan + .host_phase_2_hash + .to_string(), + control_plane: plan + .control_plane_hash + .to_string(), + update_id: update_cx.update_id, + } + } + }; update_cx .mgs_client .sp_installinator_image_id_set( diff --git a/wicketd/tests/integration_tests/updates.rs b/wicketd/tests/integration_tests/updates.rs index 715d50deaa0..c27925483f4 100644 --- a/wicketd/tests/integration_tests/updates.rs +++ b/wicketd/tests/integration_tests/updates.rs @@ -29,6 +29,7 @@ use sled_agent_zone_images::ZoneImageSourceResolver; use sled_storage::config::MountConfig; use tokio::sync::oneshot; use tufaceous_artifact::{ArtifactHashId, ArtifactKind, KnownArtifactKind}; +use update_common::artifacts::UpdatePlan; use update_engine::NestedError; use wicket::OutputKind; use wicket_common::{ @@ -95,10 +96,11 @@ async fn test_updates() { .into_inner(); // We should have an artifact for every known artifact kind (except - // `Zone`)... + // `Zone`), as well as the installinator document... let expected_kinds: BTreeSet<_> = KnownArtifactKind::iter() .filter(|k| !matches!(k, KnownArtifactKind::Zone)) .map(ArtifactKind::from) + .chain(std::iter::once(ArtifactKind::INSTALLINATOR_DOCUMENT)) .collect(); // ... and installable artifacts that replace the top level host, @@ -298,7 +300,65 @@ async fn test_updates() { // multi_thread is required. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_installinator_fetch() { - let gateway = gateway_setup::test_setup("test_updates", SpPort::One).await; + let gateway = gateway_setup::test_setup( + "test_installinator_fetch_no_installinator_document", + SpPort::One, + ) + .await; + let wicketd_testctx = WicketdTestContext::setup(gateway).await; + let log = wicketd_testctx.log(); + + let temp_dir = Utf8TempDir::new().expect("temp dir created"); + let archive_path = temp_dir.path().join("archive.zip"); + + // Test ingestion of an artifact with non-semver versions. This ensures that + // wicketd for v14 and above can handle non-semver versions. + // + // --allow-non-semver can be removed once customer systems are updated to + // v14 and above. + let args = tufaceous::Args::try_parse_from([ + "tufaceous", + "assemble", + "../update-common/manifests/fake-non-semver.toml", + "--allow-non-semver", + archive_path.as_str(), + ]) + .expect("args parsed correctly"); + + args.exec(log).await.expect("assemble command completed successfully"); + + // Read the archive and upload it to the server. + let zip_bytes = + fs_err::read(&archive_path).expect("archive read correctly"); + wicketd_testctx + .wicketd_client + .put_repository(zip_bytes) + .await + .expect("bytes read and archived"); + + let update_plan = wicketd_testctx + .server + .artifact_store + .current_plan() + .expect("we just uploaded a repository, so there should be a plan"); + + installinator_fetch_impl(&wicketd_testctx, &temp_dir, &update_plan, true) + .await; + + wicketd_testctx.teardown().await; +} + +// See documentation for extract_nested_artifact_pair in update_plan.rs for why +// multi_thread is required. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_installinator_fetch_no_installinator_document() { + // TODO-cleanup: future TUF repos will always have an installinator + // document, and this test can be removed. + let gateway = gateway_setup::test_setup( + "test_installinator_fetch_no_installinator_document", + SpPort::One, + ) + .await; let wicketd_testctx = WicketdTestContext::setup(gateway).await; let log = wicketd_testctx.log(); @@ -315,6 +375,7 @@ async fn test_installinator_fetch() { "assemble", "../update-common/manifests/fake-non-semver.toml", "--allow-non-semver", + "--no-installinator-document", archive_path.as_str(), ]) .expect("args parsed correctly"); @@ -335,10 +396,33 @@ async fn test_installinator_fetch() { .artifact_store .current_plan() .expect("we just uploaded a repository, so there should be a plan"); + + installinator_fetch_impl( + &wicketd_testctx, + &temp_dir, + &update_plan, + /* has_installinator_doc */ false, + ) + .await; + + wicketd_testctx.teardown().await; +} + +async fn installinator_fetch_impl( + wicketd_testctx: &WicketdTestContext, + temp_dir: &Utf8TempDir, + update_plan: &UpdatePlan, + // TODO-cleanup: in the future, all TUF repos will have installinator + // documents. + has_installinator_doc: bool, +) { + let log = wicketd_testctx.log(); + let host_phase_2_hash = update_plan.host_phase_2_hash.to_string(); let control_plane_hash = update_plan.control_plane_hash.to_string(); - // Are the artifacts available when looked up by hash? + // Are the host phase 2 and control plane artifacts available when looked up + // by hash? let host_phase_2_id = ArtifactHashId { kind: ArtifactKind::HOST_PHASE_2, hash: update_plan.host_phase_2_hash, @@ -362,6 +446,24 @@ async fn test_installinator_fetch() { .contains_by_hash(&control_plane_id), "control plane ID found by hash" ); + // Is the installinator document available, if it exists in the update plan? + let installinator_doc_hash = has_installinator_doc.then(|| { + let installinator_doc_hash = update_plan + .installinator_doc_hash + .expect("expected installinator document to be present"); + let installinator_doc_id = ArtifactHashId { + kind: ArtifactKind::INSTALLINATOR_DOCUMENT, + hash: installinator_doc_hash, + }; + assert!( + wicketd_testctx + .server + .artifact_store + .contains_by_hash(&installinator_doc_id), + "installinator document ID found by hash" + ); + installinator_doc_hash.to_string() + }); // Tell the installinator to download artifacts from that location. let peers_list = format!( @@ -394,25 +496,33 @@ async fn test_installinator_fetch() { temp_dir.path().join("pool/int").join(non_boot_zpool.to_string()); let update_id_str = mupdate_id.to_string(); - let args = installinator::InstallinatorApp::try_parse_from([ + + let mut args = vec![ "installinator", "install", "--mechanism", peers_list.as_str(), "--update-id", update_id_str.as_str(), - "--host-phase-2", - host_phase_2_hash.as_str(), - "--control-plane", - control_plane_hash.as_str(), a_path.as_str(), b_path.as_str(), "--data-link0", "cxgbe0", "--data-link1", "cxgbe1", - ]) - .expect("installinator args parsed successfully"); + ]; + + // Pass in the installinator document hash, or the host phase 2 and control + // plane hashes, based on what's available. + if let Some(installinator_doc_hash) = &installinator_doc_hash { + args.extend(["--installinator-doc", installinator_doc_hash.as_str()]); + } else { + args.extend(["--host-phase-2", host_phase_2_hash.as_str()]); + args.extend(["--control-plane", control_plane_hash.as_str()]); + } + + let args = installinator::InstallinatorApp::try_parse_from(args) + .expect("installinator args parsed successfully"); args.exec(&log.new(slog::o!("crate" => "installinator"))) .await @@ -574,8 +684,6 @@ async fn test_installinator_fetch() { ); recv_handle.await.expect("recv_handle succeeded"); - - wicketd_testctx.teardown().await; } // See documentation for extract_nested_artifact_pair in update_plan.rs for why From d55916b92d969a951d216ee9d4c0947068a0f5ad Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 31 Jul 2025 03:15:40 +0000 Subject: [PATCH 2/3] clippy Created using spr 1.3.6-beta.1 --- installinator/src/dispatch.rs | 2 +- update-common/src/artifacts/update_plan.rs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/installinator/src/dispatch.rs b/installinator/src/dispatch.rs index 346c267c72e..8c46a3df0bb 100644 --- a/installinator/src/dispatch.rs +++ b/installinator/src/dispatch.rs @@ -79,7 +79,7 @@ enum InstallinatorCommand { /// Scan hardware to find the target M.2 device. DebugHardwareScan(DebugHardwareScan), /// Perform the installation. - Install(InstallOpts), + Install(Box), } /// Perform discovery of peers. diff --git a/update-common/src/artifacts/update_plan.rs b/update-common/src/artifacts/update_plan.rs index e1c5779ed1e..a77c38ea31b 100644 --- a/update-common/src/artifacts/update_plan.rs +++ b/update-common/src/artifacts/update_plan.rs @@ -213,8 +213,6 @@ impl<'a> UpdatePlanBuilder<'a> { artifact_hash: ArtifactHash, stream: impl Stream> + Send, ) -> Result<(), RepositoryError> { - if artifact_id.kind == ArtifactKind::INSTALLINATOR_DOCUMENT {} - // If we don't know this artifact kind, we'll still serve it up by hash, // but we don't do any further processing on it. let Some(artifact_kind) = artifact_id.kind.to_known() else { From 543390e299c5b7a2817d5c62ffa8a2915a520347 Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 31 Jul 2025 20:10:44 +0000 Subject: [PATCH 3/3] update Created using spr 1.3.6-beta.1 --- Cargo.lock | 8 +-- Cargo.toml | 8 +-- .../output/cmds-mupdate-update-flow-stdout | 1 + .../output/cmds-noop-image-source-stdout | 2 + .../tests/output/cmds-target-release-stdout | 3 + dev-tools/releng/src/tuf.rs | 3 +- installinator/src/dispatch.rs | 7 ++- .../planning/src/mgs_updates/mod.rs | 1 + nexus/tests/integration_tests/updates.rs | 63 +++++++++++++++---- openapi/installinator.json | 7 +++ .../src/artifacts/artifacts_with_plan.rs | 4 +- update-common/src/artifacts/update_plan.rs | 61 ++++++++++++++---- wicketd/tests/integration_tests/updates.rs | 3 +- 13 files changed, 130 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d3fadda15ce..9f5d46779a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14129,7 +14129,7 @@ dependencies = [ [[package]] name = "tufaceous" version = "0.1.0" -source = "git+https://github.com//oxidecomputer/tufaceous?branch=sunshowers%2Fspr%2Fadd-support-for-an-installinator-document#69264b956872266e15508ffe5f4252a7fa3156a9" +source = "git+https://github.com//oxidecomputer/tufaceous?rev=63eb87709a4854b131212fe0e539776415946bb6#63eb87709a4854b131212fe0e539776415946bb6" dependencies = [ "anyhow", "camino", @@ -14150,7 +14150,7 @@ dependencies = [ [[package]] name = "tufaceous-artifact" version = "0.1.0" -source = "git+https://github.com//oxidecomputer/tufaceous?branch=sunshowers%2Fspr%2Fadd-support-for-an-installinator-document#69264b956872266e15508ffe5f4252a7fa3156a9" +source = "git+https://github.com//oxidecomputer/tufaceous?rev=63eb87709a4854b131212fe0e539776415946bb6#63eb87709a4854b131212fe0e539776415946bb6" dependencies = [ "daft", "hex", @@ -14167,7 +14167,7 @@ dependencies = [ [[package]] name = "tufaceous-brand-metadata" version = "0.1.0" -source = "git+https://github.com//oxidecomputer/tufaceous?branch=sunshowers%2Fspr%2Fadd-support-for-an-installinator-document#69264b956872266e15508ffe5f4252a7fa3156a9" +source = "git+https://github.com//oxidecomputer/tufaceous?rev=63eb87709a4854b131212fe0e539776415946bb6#63eb87709a4854b131212fe0e539776415946bb6" dependencies = [ "semver 1.0.26", "serde", @@ -14179,7 +14179,7 @@ dependencies = [ [[package]] name = "tufaceous-lib" version = "0.1.0" -source = "git+https://github.com//oxidecomputer/tufaceous?branch=sunshowers%2Fspr%2Fadd-support-for-an-installinator-document#69264b956872266e15508ffe5f4252a7fa3156a9" +source = "git+https://github.com//oxidecomputer/tufaceous?rev=63eb87709a4854b131212fe0e539776415946bb6#63eb87709a4854b131212fe0e539776415946bb6" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index ace8829f070..b5f282e61dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -970,10 +970,10 @@ opt-level = 3 # tufaceous-brand-metadata = { path = "../tufaceous/brand-metadata" } # tufaceous-lib = { path = "../tufaceous/lib" } # Extra slash here to pretend to be a different source. -tufaceous = { git = "https://github.com//oxidecomputer/tufaceous", branch = "sunshowers/spr/add-support-for-an-installinator-document" } -tufaceous-artifact = { git = "https://github.com//oxidecomputer/tufaceous", branch = "sunshowers/spr/add-support-for-an-installinator-document" } -tufaceous-brand-metadata = { git = "https://github.com//oxidecomputer/tufaceous", branch = "sunshowers/spr/add-support-for-an-installinator-document" } -tufaceous-lib = { git = "https://github.com//oxidecomputer/tufaceous", branch = "sunshowers/spr/add-support-for-an-installinator-document" } +tufaceous = { git = "https://github.com//oxidecomputer/tufaceous", rev = "63eb87709a4854b131212fe0e539776415946bb6" } +tufaceous-artifact = { git = "https://github.com//oxidecomputer/tufaceous", rev = "63eb87709a4854b131212fe0e539776415946bb6" } +tufaceous-brand-metadata = { git = "https://github.com//oxidecomputer/tufaceous", rev = "63eb87709a4854b131212fe0e539776415946bb6" } +tufaceous-lib = { git = "https://github.com//oxidecomputer/tufaceous", rev = "63eb87709a4854b131212fe0e539776415946bb6" } # [patch."https://github.com/oxidecomputer/typify"] # typify = { path = "../typify/typify" } diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout index 77b502ab240..979287bf84e 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout @@ -17,6 +17,7 @@ created repo-1.0.0.zip for system version 1.0.0 > set target-release repo-1.0.0.zip INFO extracting uploaded archive to INFO created directory to store extracted artifacts, path: +INFO added artifact, name: installinator_document, kind: installinator_document, version: 1.0.0, hash: c6ae866031d1183094c92cde9d9d1fd5f18356abc81a842ce31471b473fd5582, length: 367 INFO added artifact, name: SimGimletSp, kind: gimlet_sp, version: 1.0.0, hash: 7e6667e646ad001b54c8365a3d309c03f89c59102723d38d01697ee8079fe670, length: 747 INFO added artifact, name: fake-gimlet-rot, kind: gimlet_rot_image_a, version: 1.0.0, hash: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a, length: 735 INFO added artifact, name: fake-gimlet-rot, kind: gimlet_rot_image_b, version: 1.0.0, hash: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a, length: 735 diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-noop-image-source-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-noop-image-source-stdout index bd344107868..0ce1db3d0ae 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-noop-image-source-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-noop-image-source-stdout @@ -39,6 +39,7 @@ created repo-2.0.0.zip for system version 2.0.0 > set target-release repo-1.0.0.zip INFO extracting uploaded archive to INFO created directory to store extracted artifacts, path: +INFO added artifact, name: installinator_document, kind: installinator_document, version: 1.0.0, hash: c6ae866031d1183094c92cde9d9d1fd5f18356abc81a842ce31471b473fd5582, length: 367 INFO added artifact, name: SimGimletSp, kind: gimlet_sp, version: 1.0.0, hash: 7e6667e646ad001b54c8365a3d309c03f89c59102723d38d01697ee8079fe670, length: 747 INFO added artifact, name: fake-gimlet-rot, kind: gimlet_rot_image_a, version: 1.0.0, hash: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a, length: 735 INFO added artifact, name: fake-gimlet-rot, kind: gimlet_rot_image_b, version: 1.0.0, hash: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a, length: 735 @@ -101,6 +102,7 @@ sled aff6c093-197d-42c5-ad80-9f10ba051a34: install dataset updated: to target re > sled-update-install-dataset serial4 --from-repo repo-2.0.0.zip INFO extracting uploaded archive to INFO created directory to store extracted artifacts, path: +INFO added artifact, name: installinator_document, kind: installinator_document, version: 2.0.0, hash: 065b2b7cde09474c9a1bcf98a8f9c93129f617e074404c6aa76164f440a5627f, length: 367 INFO added artifact, name: fake-gimlet-sp, kind: gimlet_sp, version: 2.0.0, hash: ce1e98a8a9ae541654508f101d59a3ddeba3d28177f1d42d5614248eef0b820b, length: 751 INFO added artifact, name: fake-gimlet-rot, kind: gimlet_rot_image_a, version: 2.0.0, hash: e7047f500a5391e22cd8e6a8d3ae66c9d9de7a8d021e6e9a10e05bb6d554da77, length: 743 INFO added artifact, name: fake-gimlet-rot, kind: gimlet_rot_image_b, version: 2.0.0, hash: e7047f500a5391e22cd8e6a8d3ae66c9d9de7a8d021e6e9a10e05bb6d554da77, length: 743 diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-target-release-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-target-release-stdout index 51bc5998419..34b390dffd2 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-target-release-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-target-release-stdout @@ -32,6 +32,7 @@ created repo-1.0.0.zip for system version 1.0.0 > set target-release repo-1.0.0.zip INFO extracting uploaded archive to INFO created directory to store extracted artifacts, path: +INFO added artifact, name: installinator_document, kind: installinator_document, version: 1.0.0, hash: c6ae866031d1183094c92cde9d9d1fd5f18356abc81a842ce31471b473fd5582, length: 367 INFO added artifact, name: SimGimletSp, kind: gimlet_sp, version: 1.0.0, hash: 7e6667e646ad001b54c8365a3d309c03f89c59102723d38d01697ee8079fe670, length: 747 INFO added artifact, name: fake-gimlet-rot, kind: gimlet_rot_image_a, version: 1.0.0, hash: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a, length: 735 INFO added artifact, name: fake-gimlet-rot, kind: gimlet_rot_image_b, version: 1.0.0, hash: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a, length: 735 @@ -70,6 +71,7 @@ internal DNS generations: 1 external DNS generations: 1 target number of Nexus instances: default target release (generation 2): 1.0.0 (system-update-v1.0.0.zip) + artifact: c6ae866031d1183094c92cde9d9d1fd5f18356abc81a842ce31471b473fd5582 installinator_document (installinator_document version 1.0.0) artifact: 7e6667e646ad001b54c8365a3d309c03f89c59102723d38d01697ee8079fe670 gimlet_sp (SimGimletSp version 1.0.0) artifact: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a gimlet_rot_image_a (fake-gimlet-rot version 1.0.0) artifact: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a gimlet_rot_image_b (fake-gimlet-rot version 1.0.0) @@ -147,6 +149,7 @@ internal DNS generations: 1 external DNS generations: 1 target number of Nexus instances: default target release (generation 2): 1.0.0 (system-update-v1.0.0.zip) + artifact: c6ae866031d1183094c92cde9d9d1fd5f18356abc81a842ce31471b473fd5582 installinator_document (installinator_document version 1.0.0) artifact: 7e6667e646ad001b54c8365a3d309c03f89c59102723d38d01697ee8079fe670 gimlet_sp (SimGimletSp version 1.0.0) artifact: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a gimlet_rot_image_a (fake-gimlet-rot version 1.0.0) artifact: 04e4a7fdb84acca92c8fd3235e26d64ea61bef8a5f98202589fd346989c5720a gimlet_rot_image_b (fake-gimlet-rot version 1.0.0) diff --git a/dev-tools/releng/src/tuf.rs b/dev-tools/releng/src/tuf.rs index 5875f43e30e..2b568c35dd1 100644 --- a/dev-tools/releng/src/tuf.rs +++ b/dev-tools/releng/src/tuf.rs @@ -21,7 +21,6 @@ use tokio::io::AsyncReadExt; use tufaceous_artifact::ArtifactHash; use tufaceous_artifact::ArtifactVersion; use tufaceous_artifact::KnownArtifactKind; -use tufaceous_lib::IncludeInstallinatorDocument; use tufaceous_lib::Key; use tufaceous_lib::assemble::ArtifactManifest; use tufaceous_lib::assemble::DeserializedArtifactData; @@ -144,7 +143,7 @@ pub(crate) async fn build_tuf_repo( manifest, keys, expiry, - IncludeInstallinatorDocument::Yes, + true, output_dir.join("repo.zip"), ) .build() diff --git a/installinator/src/dispatch.rs b/installinator/src/dispatch.rs index 8c46a3df0bb..b3e1dca7c4b 100644 --- a/installinator/src/dispatch.rs +++ b/installinator/src/dispatch.rs @@ -17,8 +17,8 @@ use omicron_common::FileKv; use sha2::{Digest, Sha256}; use slog::{Drain, error, warn}; use tufaceous_artifact::{ - ArtifactHash, ArtifactHashId, ArtifactKind, InstallinatorArtifactKind, - InstallinatorDocument, + ArtifactHash, ArtifactHashId, InstallinatorArtifactKind, + InstallinatorDocument, KnownArtifactKind, }; use tufaceous_lib::ControlPlaneZoneImages; use update_engine::StepResult; @@ -218,7 +218,8 @@ impl InstallOpts { "Downloading installinator document", async move |cx| { let installinator_doc_id = ArtifactHashId { - kind: ArtifactKind::INSTALLINATOR_DOCUMENT, + kind: KnownArtifactKind::InstallinatorDocument + .into(), hash, }; let installinator_doc = fetch_artifact( diff --git a/nexus/reconfigurator/planning/src/mgs_updates/mod.rs b/nexus/reconfigurator/planning/src/mgs_updates/mod.rs index 3c3192c2071..f5f4ef97cdd 100644 --- a/nexus/reconfigurator/planning/src/mgs_updates/mod.rs +++ b/nexus/reconfigurator/planning/src/mgs_updates/mod.rs @@ -412,6 +412,7 @@ fn try_make_update_sp( KnownArtifactKind::GimletRot | KnownArtifactKind::Host | KnownArtifactKind::Trampoline + | KnownArtifactKind::InstallinatorDocument | KnownArtifactKind::ControlPlane | KnownArtifactKind::Zone | KnownArtifactKind::PscRot diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index f6ba8bfd773..619636b7c3e 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -26,7 +26,6 @@ use std::fmt::Debug; use tough::editor::signed::SignedRole; use tough::schema::Root; use tufaceous_artifact::KnownArtifactKind; -use tufaceous_lib::IncludeInstallinatorDocument; use tufaceous_lib::Key; use tufaceous_lib::assemble::{ArtifactManifest, OmicronRepoAssembler}; use tufaceous_lib::assemble::{DeserializedManifest, ManifestTweak}; @@ -87,7 +86,7 @@ impl TestTrustRoot { manifest, vec![self.key.clone()], self.expiry, - IncludeInstallinatorDocument::Yes, + true, archive_path.to_path_buf(), ); assembler.set_root_role(self.root_role.clone()); @@ -447,9 +446,49 @@ async fn test_repo_upload() -> Result<()> { let mut description = response.recorded; description.sort_artifacts(); - // The artifacts should be exactly the same as the 1.0.0 repo we uploaded. + // The artifacts should be exactly the same as the 1.0.0 repo we + // uploaded, other than the installinator document (which will have + // system version 2.0.0). + let mut installinator_doc_1 = None; + let filtered_artifacts_1 = initial_description + .artifacts + .iter() + .filter(|artifact| { + if artifact.id.kind + == KnownArtifactKind::InstallinatorDocument.into() + { + installinator_doc_1 = Some(*artifact); + false + } else { + true + } + }) + .collect::>(); + let mut installinator_doc_2 = None; + let filtered_artifacts_2 = description + .artifacts + .iter() + .filter(|artifact| { + if artifact.id.kind + == KnownArtifactKind::InstallinatorDocument.into() + { + installinator_doc_2 = Some(*artifact); + false + } else { + true + } + }) + .collect::>(); + + let installinator_doc_1 = installinator_doc_1 + .expect("should have found installinator document in 1.0.0"); + assert_eq!(installinator_doc_1.id.version, "1.0.0".parse().unwrap()); + let installinator_doc_2 = installinator_doc_2 + .expect("should have found installinator document in 2.0.0"); + assert_eq!(installinator_doc_2.id.version, "2.0.0".parse().unwrap()); + assert_eq!( - initial_description.artifacts, description.artifacts, + filtered_artifacts_1, filtered_artifacts_2, "artifacts for 1.0.0 and 2.0.0 should match" ); @@ -469,21 +508,21 @@ async fn test_repo_upload() -> Result<()> { "initial description matches fetched description" ); } - // No artifacts changed, so the generation number should still be 2... + // The installinator document changed, so the generation number is bumped to + // 3. assert_eq!( datastore.tuf_get_generation(&opctx).await.unwrap(), - 2u32.into() + 3u32.into() ); - // ... and the task should have nothing to do and should immediately - // delete the local artifacts. + // ... and the task will have one artifact to replicate. let status = wait_tuf_artifact_replication_step(&cptestctx.internal_client).await; eprintln!("{status:?}"); - assert_eq!(status.generation, 2u32.into()); + assert_eq!(status.generation, 3u32.into()); assert_eq!(status.last_run_counters.list_ok, 4); - assert_eq!(status.last_run_counters.put_ok, 0); - assert_eq!(status.last_run_counters.copy_ok, 0); - assert_eq!(status.local_repos, 0); + assert_eq!(status.last_run_counters.put_ok, 3); + assert_eq!(status.last_run_counters.copy_ok, 1); + assert_eq!(status.local_repos, 1); cptestctx.teardown().await; Ok(()) diff --git a/openapi/installinator.json b/openapi/installinator.json index 3dd7cecee78..65ec7f40017 100644 --- a/openapi/installinator.json +++ b/openapi/installinator.json @@ -276,6 +276,13 @@ "InstallinatorComponent": { "description": "Installinator components.", "oneOf": [ + { + "description": "The installinator document.", + "type": "string", + "enum": [ + "installinator_document" + ] + }, { "description": "The host phase 2 component.", "type": "string", diff --git a/update-common/src/artifacts/artifacts_with_plan.rs b/update-common/src/artifacts/artifacts_with_plan.rs index 8667ab863cb..843b5ad4fa9 100644 --- a/update-common/src/artifacts/artifacts_with_plan.rs +++ b/update-common/src/artifacts/artifacts_with_plan.rs @@ -428,11 +428,10 @@ mod tests { .collect(); // `by_id` should contain one entry for every `KnownArtifactKind` - // (except `Zone`), as well as `installinator_document`. + // (except `Zone`). let mut expected_kinds: BTreeSet<_> = KnownArtifactKind::iter() .filter(|k| !matches!(k, KnownArtifactKind::Zone)) .map(ArtifactKind::from) - .chain(std::iter::once(ArtifactKind::INSTALLINATOR_DOCUMENT)) .collect(); assert_eq!( expected_kinds, by_id_kinds, @@ -516,7 +515,6 @@ mod tests { let expected_kinds: BTreeSet<_> = KnownArtifactKind::iter() .filter(|k| !matches!(k, KnownArtifactKind::ControlPlane)) .map(ArtifactKind::from) - .chain(std::iter::once(ArtifactKind::INSTALLINATOR_DOCUMENT)) .collect(); assert_eq!( expected_kinds, by_id_kinds, diff --git a/update-common/src/artifacts/update_plan.rs b/update-common/src/artifacts/update_plan.rs index a77c38ea31b..d6b030f6dcf 100644 --- a/update-common/src/artifacts/update_plan.rs +++ b/update-common/src/artifacts/update_plan.rs @@ -259,6 +259,14 @@ impl<'a> UpdatePlanBuilder<'a> { KnownArtifactKind::Trampoline => { self.add_trampoline_artifact(artifact_id, stream) } + KnownArtifactKind::InstallinatorDocument => { + self.add_installinator_document_artifact( + artifact_id, + artifact_hash, + stream, + ) + .await + } KnownArtifactKind::ControlPlane => { self.add_control_plane_artifact( artifact_id, @@ -291,6 +299,7 @@ impl<'a> UpdatePlanBuilder<'a> { KnownArtifactKind::GimletRot | KnownArtifactKind::Host | KnownArtifactKind::Trampoline + | KnownArtifactKind::InstallinatorDocument | KnownArtifactKind::ControlPlane | KnownArtifactKind::Zone | KnownArtifactKind::PscRot @@ -384,6 +393,7 @@ impl<'a> UpdatePlanBuilder<'a> { KnownArtifactKind::GimletRot | KnownArtifactKind::Host | KnownArtifactKind::Trampoline + | KnownArtifactKind::InstallinatorDocument | KnownArtifactKind::ControlPlane | KnownArtifactKind::Zone | KnownArtifactKind::PscRot @@ -479,6 +489,7 @@ impl<'a> UpdatePlanBuilder<'a> { KnownArtifactKind::GimletSp | KnownArtifactKind::Host | KnownArtifactKind::Trampoline + | KnownArtifactKind::InstallinatorDocument | KnownArtifactKind::ControlPlane | KnownArtifactKind::Zone | KnownArtifactKind::PscSp @@ -726,6 +737,38 @@ impl<'a> UpdatePlanBuilder<'a> { Ok(()) } + async fn add_installinator_document_artifact( + &mut self, + artifact_id: ArtifactId, + artifact_hash: ArtifactHash, + stream: impl Stream> + Send, + ) -> Result<(), RepositoryError> { + // The installinator document is treated as an opaque single-unit + // artifact by update-common, so that older versions of update-common + // can handle newer versions of this artifact. + if self.installinator_doc_hash.is_some() { + return Err(RepositoryError::DuplicateInstallinatorDocument); + } + + let artifact_kind = artifact_id.kind.clone(); + let artifact_hash_id = + ArtifactHashId { kind: artifact_kind.clone(), hash: artifact_hash }; + + let data = + self.extracted_artifacts.store(artifact_hash_id, stream).await?; + + self.record_extracted_artifact( + artifact_id, + data, + artifact_kind, + self.log, + )?; + + self.installinator_doc_hash = Some(artifact_hash); + + Ok(()) + } + async fn add_control_plane_artifact( &mut self, artifact_id: ArtifactId, @@ -783,17 +826,6 @@ impl<'a> UpdatePlanBuilder<'a> { let data = self.extracted_artifacts.store(artifact_hash_id, stream).await?; - // The installinator document is of course known to us, but it's not - // really an artifact in the usual sense -- in fact, we want to treat it - // as an opaque blob. It's most convenient to treat it as unknown - // artifact except for recording its hash if found. - if artifact_kind == ArtifactKind::INSTALLINATOR_DOCUMENT { - if self.installinator_doc_hash.is_some() { - return Err(RepositoryError::DuplicateInstallinatorDocument); - } - self.installinator_doc_hash = Some(data.hash()); - } - self.record_extracted_artifact( artifact_id, data, @@ -2061,6 +2093,13 @@ mod tests { assert_eq!(hash_ids.len(), 1); assert_eq!(plan.control_plane_hash, hash_ids[0].hash); } + KnownArtifactKind::InstallinatorDocument => { + assert_eq!(hash_ids.len(), 1); + assert_eq!( + plan.installinator_doc_hash, + Some(hash_ids[0].hash) + ); + } KnownArtifactKind::Zone => { unreachable!( "tufaceous does not yet generate repos \ diff --git a/wicketd/tests/integration_tests/updates.rs b/wicketd/tests/integration_tests/updates.rs index c27925483f4..aad2504d00c 100644 --- a/wicketd/tests/integration_tests/updates.rs +++ b/wicketd/tests/integration_tests/updates.rs @@ -100,7 +100,6 @@ async fn test_updates() { let expected_kinds: BTreeSet<_> = KnownArtifactKind::iter() .filter(|k| !matches!(k, KnownArtifactKind::Zone)) .map(ArtifactKind::from) - .chain(std::iter::once(ArtifactKind::INSTALLINATOR_DOCUMENT)) .collect(); // ... and installable artifacts that replace the top level host, @@ -452,7 +451,7 @@ async fn installinator_fetch_impl( .installinator_doc_hash .expect("expected installinator document to be present"); let installinator_doc_id = ArtifactHashId { - kind: ArtifactKind::INSTALLINATOR_DOCUMENT, + kind: KnownArtifactKind::InstallinatorDocument.into(), hash: installinator_doc_hash, }; assert!(