diff --git a/Cargo.lock b/Cargo.lock index 759fbb317..28fe09d67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2707,6 +2707,7 @@ dependencies = [ "backoff", "base64 0.21.4", "bhyve_api", + "camino", "cfg-if", "crucible-client-types", "errno 0.2.8", @@ -2740,6 +2741,7 @@ version = "0.1.0" dependencies = [ "anyhow", "backtrace", + "camino", "clap 4.4.0", "phd-framework", "phd-tests", diff --git a/Cargo.toml b/Cargo.toml index c01029f95..e32306364 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,6 +88,7 @@ bitstruct = "0.1" bitvec = "1.0" byteorder = "1" bytes = "1.1" +camino = "1.1.6" cc = "1.0.73" cfg-if = "1.0.0" chrono = "0.4.19" diff --git a/phd-tests/README.md b/phd-tests/README.md index 7fb1477e6..1b02f6cc8 100644 --- a/phd-tests/README.md +++ b/phd-tests/README.md @@ -71,55 +71,67 @@ Other options are described in the runner's help text (`cargo run -- --help`). ### Specifying artifacts -The runner requires a TOML file that specifies what guest OS and firmware -artifacts are available in a given test run. This file has the following -entries: - -- `local_root`: The path to the local directory in which artifacts are stored. - PHD requires read/write access to this directory. -- One or more `guest_images` tables, written as `[guest_images.$KEY]`. The - runner uses the value of its `--default-guest-artifact` parameter to choose a - guest image to use for tests that don't attach their own disks. - - These tables have the following fields: - - `guest_os_kind`: Supplies the "kind" of guest OS this is. PHD uses this to - create an adapter that tells the rest of the framework how to interact with - this guest OS--how to log in, what command prompt to expect, etc. The list - of kinds is given by the `framework::guest_os::artifacts::GuestOsKind` enum. - - Note that PHD expects images to conform to the per-OS-kind behaviors encoded - in its guest OS adapters. That is, if the PHD code's adapter for - `GuestOsKind::Foo` says that FooOS is expected to have a shell prompt of - `user@foo$`, and instead it's `user@bar$`, tests using this artifact will - fail. - - `metadata.relative_local_path`: The path to this artifact relative to the - `local_root` in this artifact file. - - `metadata.expected_digest`: Optional. A string containing the expected - SHA256 digest of this artifact. - - At the start of each test, PHD will check the integrity of artifacts with - expected digests against this digest. If the computed and expected digests - don't match, PHD will either attempt to reacquire the artifact or abort - testing. - - If not specified, PHD will skip pre-test integrity checks for this artifact. - Note that this can cause changes to a disk image to persist between test - cases! - - `metadata.remote_uri`: Optional. A URI from which to try to download this - artifact. - - If an artifact is not present on disk or has the wrong SHA256 digest, the - runner will try to redownload the artifact from this path. - - If this is omitted for an artifact, the runner will abort testing if it - encounters a scenario where it needs to reacquire the artifact. -- One or more `[bootroms]` tables, written as `[bootroms.$KEY]`. The runner uses - the value of its `--default-bootrom-artifact` parameter to choose a bootrom to - use for tests that don't select their own bootrom. - - The fields in these tables are `relative_local_path`, `expected_digest`, and - `remote_uri`, with the same semantics as for guest images (just without the - `metadata.` prefix). +The runner requires a TOML file that specifies the guest OS and firmware images +that are available for a test run to use. It has the following format: + +```toml +# An array of URIs from which to try to fetch artifacts with the "remote_server" +# source type. The runner appends "/filename" to each of these URIs to generate +# a download URI for each such artifact. +remote_server_uris = ["http://foo.com", "http://bar.net"] + +# Every artifact has a named entry in the "artifacts" table. The runner's +# `default_guest_artifact` and `default_bootrom_artifact` parameters name the +# guest OS and bootrom artifacts that will be used for a given test run. +# +# Every artifact has a kind, which is one of `guest_os`, `bootrom`, or +# `propolis_server`. +# +# Every artifact also has a source, which is one of `remote_server`, +# `local_path`, or `buildomat`. +# +# The following entry specifies a guest OS named "alpine" that searches the +# remote URI list for files named "alpine.iso": +[artifacts.alpine] +filename = "alpine.iso" + +# Bootrom and Propolis server artifacts can put a `kind = "foo"` entry inline, +# but guest OSes need to use the structured data syntax to specify the guest OS +# adapter to use when booting a guest from this artifact. +[artifacts.alpine.kind] +guest_os = "alpine" + +# Remote artifacts are required to specify an expected SHA256 digest as a +# string. +[artifacts.alpine.source.remote_server] +sha256 = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +# The following entry specifies a debug bootrom pulled from Buildomat. Buildomat +# outputs are associated with a single repo and a commit therein; the jobs that +# create them also specify a 'series' that identifies the task that created the +# collateral. +[artifacts.bootrom] +filename = "OVMF_CODE.fd" +kind = "bootrom" + +[artifacts.bootrom.source.buildomat] +repo = "oxidecomputer/edk2" +series = "image_debug" +commit = "commit_sha" +sha256 = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + +# This entry specifies a local directory in which an artifact can be found. +# SHA256 digests are optional for local artifacts. This allows you to create +# an entry for a local artifact that changes frequently (e.g. a Propolis build) +# without having to edit the digest every time it changes. +[artifacts.propolis] +filename = "propolis-server" +kind = "propolis_server" + +[artifacts.propolis.source.local_path] +path = "/home/oxide/propolis/target/debug" +# sha256 = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" +``` ## Authoring tests diff --git a/phd-tests/artifacts.toml b/phd-tests/artifacts.toml index 2862c33bc..9598a9ce4 100644 --- a/phd-tests/artifacts.toml +++ b/phd-tests/artifacts.toml @@ -1,11 +1,17 @@ -[guest_images.alpine] -guest_os_kind = "alpine" -metadata.relative_local_path = "alpine.iso" -metadata.expected_digest = "ba8007f74f9b54fbae3b2520da577831b4834778a498d732f091260c61aa7ca1" -metadata.remote_uri = "https://oxide-omicron-build.s3.amazonaws.com/alpine.iso" +remote_server_uris = ["https://oxide-omicron-build.s3.amazonaws.com"] -[bootroms.ovmf] -relative_local_path = "OVMF_CODE.fd" -expected_digest = "29813374b58e3b77fb665f2d95cb3bab37d44fdd2c4fce2a70de9d76a3512a4f" -remote_uri = "https://buildomat.eng.oxide.computer/public/file/oxidecomputer/edk2/image_debug/6d92acf0a22718dd4175d7c64dbcf7aaec3740bd/OVMF_CODE.fd" +[artifacts.alpine] +filename = "alpine.iso" +[artifacts.alpine.kind] +guest_os = "alpine" +[artifacts.alpine.source.remote_server] +sha256 = "ba8007f74f9b54fbae3b2520da577831b4834778a498d732f091260c61aa7ca1" +[artifacts.ovmf] +filename = "OVMF_CODE.fd" +kind = "bootrom" +[artifacts.ovmf.source.buildomat] +repo = "oxidecomputer/edk2" +series = "image_debug" +commit = "6d92acf0a22718dd4175d7c64dbcf7aaec3740bd" +sha256 = "29813374b58e3b77fb665f2d95cb3bab37d44fdd2c4fce2a70de9d76a3512a4f" diff --git a/phd-tests/framework/Cargo.toml b/phd-tests/framework/Cargo.toml index 4b500467f..9b4316a67 100644 --- a/phd-tests/framework/Cargo.toml +++ b/phd-tests/framework/Cargo.toml @@ -11,6 +11,7 @@ doctest = false anyhow.workspace = true backoff.workspace = true bhyve_api.workspace = true +camino = { workspace = true, features = ["serde1"] } cfg-if.workspace = true errno.workspace = true futures.workspace = true diff --git a/phd-tests/framework/src/artifacts.rs b/phd-tests/framework/src/artifacts.rs deleted file mode 100644 index e5b6f46ea..000000000 --- a/phd-tests/framework/src/artifacts.rs +++ /dev/null @@ -1,431 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Management of artifacts: guest OS and guest firmware images that can be -//! attached to guests. -//! -//! The runner requires a path to a TOML file defining a set of artifacts. This -//! file defines -//! -//! - an optional remote URI from which to download resources that are missing, -//! - a table of guest OS images, specifying the -//! [`crate::guest_os::GuestOsKind`] of each image and its metadata, and -//! - a table of guest firmware images (bootroms), specifying metadata for each -//! one. -//! -//! Artifact metadata includes -//! -//! - the path to the artifact relative to the local root directory, -//! - an optional SHA256 digest against which to compare the local artifact, and -//! - an optional path to the artifact relative to the remote URI from which the -//! artifact can be reacquired if it is missing or corrupted. - -use std::{ - collections::BTreeMap, - fs::File, - io::{BufReader, Read, Seek, SeekFrom, Write}, - path::{Path, PathBuf}, - time::Duration, -}; - -use anyhow::{anyhow, bail, Result}; -use ring::digest::{Digest, SHA256}; -use serde::{Deserialize, Serialize}; -use thiserror::Error; -use tracing::{error, info, info_span, instrument}; - -use crate::guest_os::GuestOsKind; - -/// Errors that can arise while loading or interacting with an artifact store. -#[derive(Debug, Error)] -pub enum ArtifactStoreError { - /// Raised when the local artifact root specified in the artifact TOML - /// doesn't appear to exist. - #[error("The local root directory {0} does not exist")] - LocalRootNotFound(PathBuf), - - /// Raised when the local artifact root specified in the artifact TOML - /// exists but doesn't appear to be a directory. - #[error("The local root {0} is inaccessible or not a directory")] - LocalRootNotDirectory(PathBuf), - - /// Raised by [`ArtifactStore::check_local_copies`] when an artifact is not - /// usable (e.g. because it is missing or has an invalid digest) and cannot - /// be fixed (because no remote path to it is present). - #[error( - "One or more artifacts had invalid contents; check logs for details" - )] - ArtifactContentsInvalid(), -} - -/// A single artifact. -#[derive(Debug, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -struct ArtifactMetadata { - /// The path to the artifact relative to the root directory specified in - /// this artifact's store. - relative_local_path: PathBuf, - - /// An optional SHA256 digest for this artifact. If present, the artifact - /// store will use this digest to determine whether the artifact can be used - /// or should be replaced, possibly with an artifact in remote storage. - expected_digest: Option, - - /// An optional path to this artifact relative to the file server root - /// stored in the artifact store. If present, the store will use this to - /// replace the artifact at startup if it appears to be corrupted. - remote_uri: Option, -} - -impl ArtifactMetadata { - /// Determine whether a local artifact is present and usable under the terms - /// specified in its metadata. - fn check_local_artifact( - &self, - local_root: &Path, - remote_uri: Option<&str>, - ) -> Result<()> { - let mut local_path = PathBuf::new(); - local_path.push(local_root); - local_path.push(&self.relative_local_path); - - // There are four possibilities: - // - // 1. The artifact doesn't exist at the expected path. - // 2. The artifact exists, but has no digest recorded in the store. - // 3. The artifact exists and has a digest recorded in the store, - // but the digest on disk doesn't match it. - // 4. The artifact exists and has digest that matches what's in the - // store. - // - // In cases 1 and 3, try to redownload the artifact. In cases 2 and - // 4, accept the artifact as-is and continue. - let exists = local_path.exists(); - if exists { - match &self.expected_digest { - None => { - info!("Artifact exists but has no digest in its metadata"); - return Ok(()); - } - Some(digest) => match hash_equals(&local_path, digest) { - Ok(()) => { - info!("Artifact digest OK"); - return Ok(()); - } - Err(_) => { - info!("Artifact digest mismatched, will replace it"); - } - }, - } - } else { - info!("Artifact does not exist, will download it"); - } - - // The artifact is not usable as-is. See if it can be reacquired from - // the remote source. - let remote_uri = remote_uri - .ok_or_else(|| anyhow!("Can't download artifact: no remote URI"))?; - if exists { - info!(?local_path, "Removing mismatched artifact before replacing"); - std::fs::remove_file(&local_path)?; - } - - let download_timeout = Duration::from_secs(600); - info!( - ?local_path, - ?remote_uri, - "Downloading artifact with timeout {:?}", - download_timeout, - ); - - let client = reqwest::blocking::ClientBuilder::new() - .timeout(download_timeout) - .build()?; - let request = client.get(remote_uri).build()?; - let response = client.execute(request)?; - let mut new_file = std::fs::File::create(&local_path)?; - new_file.write_all(&response.bytes()?)?; - if let Some(digest) = &self.expected_digest { - hash_equals(&local_path, digest)?; - } - - // Make the artifact read-only to try to ensure tests can't change it. - // Disks copied from this artifact will be edited to be writable. - let mut permissions = new_file.metadata()?.permissions(); - permissions.set_readonly(true); - new_file.set_permissions(permissions)?; - - Ok(()) - } -} - -/// A wrapper for guest OS artifacts that includes their OS kind. -#[derive(Debug, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -struct GuestOsArtifact { - guest_os_kind: GuestOsKind, - metadata: ArtifactMetadata, -} - -/// A collection of artifacts that can be loaded by test VMs. -#[derive(Debug, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct ArtifactStoreConfig { - /// A map from names to guest OS artifacts. - guest_images: BTreeMap, - - /// A map from names to bootrom artifact metadata. - bootroms: BTreeMap, -} - -impl ArtifactStoreConfig { - fn from_toml(raw_toml: &str) -> Result { - let config = toml::de::from_str(raw_toml)?; - info!(?config, "Parsed artifact store configuration"); - Ok(config) - } -} - -#[derive(Debug)] -pub struct ArtifactStore { - /// The local directory into which to save artifacts. - local_root: PathBuf, - config: ArtifactStoreConfig, -} - -impl ArtifactStore { - /// Opens the supplied file, reads its contents, and uses - /// [`ArtifactStore::from_toml`] to try to interpret those contents as TOML - /// describing an artifact store. - pub fn from_file( - config_path: impl AsRef, - local_root: PathBuf, - ) -> Result { - info!(path = ?config_path.as_ref(), - "Reading artifact store configuration from file"); - let contents = std::fs::read_to_string(config_path.as_ref())?; - Self::from_toml(&contents, local_root) - } - - /// Interprets the supplied string as TOML and attempts to deserialize it as - /// an artifact store. - pub fn from_toml(raw_toml: &str, local_root: PathBuf) -> Result { - info!(?local_root, "Initializing artifact store"); - let config = ArtifactStoreConfig::from_toml(raw_toml)?; - let store = Self { local_root, config }; - store.verify()?; - Ok(store) - } - - /// Retrieves this store's local root directory. - pub fn get_local_root(&self) -> &Path { - &self.local_root - } - - /// Given a guest OS artifact name, attempts to retrieve the corresponding - /// artifact and return a path to its contents and its guest OS kind. - pub fn get_guest_artifact_info_by_name( - &self, - artifact: &str, - ) -> Option<(PathBuf, GuestOsKind)> { - self.config.guest_images.get(artifact).map(|a| { - ( - self.construct_full_path(&a.metadata.relative_local_path), - a.guest_os_kind, - ) - }) - } - - /// Given an artifact name, attempts to retrieve the guest firmware artifact - /// with that name and returns the local path to that artifact. - pub fn get_bootrom_by_name(&self, artifact: &str) -> Option { - self.config - .bootroms - .get(artifact) - .map(|a| self.construct_full_path(&a.relative_local_path)) - } - - fn construct_full_path(&self, relative_path: &Path) -> PathBuf { - let mut full = PathBuf::new(); - full.push(&self.local_root); - full.push(relative_path); - full - } - - fn verify(&self) -> Result<()> { - if !self.local_root.exists() { - return Err(ArtifactStoreError::LocalRootNotFound( - self.local_root.clone(), - ) - .into()); - } - if !self.local_root.is_dir() { - return Err(ArtifactStoreError::LocalRootNotDirectory( - self.local_root.clone(), - ) - .into()); - } - - Ok(()) - } - - /// Verifies the existence and integrity of the local on-disk artifacts - /// described by the store. - /// - /// Note: This routine may mutate artifacts on disk. This struct makes no - /// attempt to synchronize these accesses between multiple threads. The - /// caller is responsible for ensuring that it only checks local copies when - /// no artifacts are otherwise in use. - /// - /// # Return value - /// - /// - `Ok` if all the artifacts exist and all the artifacts with digests in - /// store have matching digests on disk. - /// - `Err(ArtifactStoreError::ArtifactContentsInvalid)` if one or more - /// artifacts could not be obtained or verified. The process logs contain - /// more information about the specific artifacts that failed and the - /// errors that caused those failures. Note that this routine checks all - /// artifacts in the store even if one fails. - #[instrument(skip_all)] - pub fn check_local_copies(&self) -> Result<()> { - let mut all_ok = true; - - let iter = self - .config - .guest_images - .iter() - .map(|(k, v)| (k, &v.metadata)) - .chain(self.config.bootroms.iter()); - - for (name, metadata) in iter { - info!(?name, ?metadata, "Checking artifact"); - let span = info_span!("Artifact", ?name); - let _guard = span.enter(); - if let Err(e) = metadata.check_local_artifact( - &self.local_root, - metadata.remote_uri.as_deref(), - ) { - error!(?e, "Metadata check failed"); - all_ok = false; - } - } - - all_ok - .then_some(()) - .ok_or_else(|| ArtifactStoreError::ArtifactContentsInvalid().into()) - } -} - -fn sha256_digest(file: &mut File) -> Result { - file.seek(SeekFrom::Start(0))?; - let mut reader = BufReader::new(file); - let mut context = ring::digest::Context::new(&SHA256); - let mut buffer = [0; 1024]; - - loop { - let count = reader.read(&mut buffer)?; - if count == 0 { - break; - } - context.update(&buffer[..count]); - } - - Ok(context.finish()) -} - -fn hash_equals(path: impl AsRef, expected_digest: &str) -> Result<()> { - let mut file = File::open(path.as_ref())?; - let digest = hex::encode(sha256_digest(&mut file)?.as_ref()); - if digest != expected_digest { - bail!( - "Digest of {:#?} was {}, expected {}", - path.as_ref(), - digest, - expected_digest - ); - } - Ok(()) -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn store_to_from_toml() { - let guest_artifact = GuestOsArtifact { - guest_os_kind: GuestOsKind::Alpine, - metadata: ArtifactMetadata { - relative_local_path: "alpine.raw".into(), - expected_digest: Some("abcd1234".to_string()), - remote_uri: Some("https://127.0.0.1/alpine.raw".to_string()), - }, - }; - - let bootrom_artifact = ArtifactMetadata { - relative_local_path: "OVMF_CODE.fd".into(), - expected_digest: None, - remote_uri: Some("https://127.0.0.1/OVMF_CODE.fd".to_string()), - }; - - let config = ArtifactStoreConfig { - guest_images: BTreeMap::from([( - "alpine".to_string(), - guest_artifact, - )]), - bootroms: BTreeMap::from([( - "bootrom".to_string(), - bootrom_artifact, - )]), - }; - - let out = toml::ser::to_string(&config).unwrap(); - println!("TOML serialization output: {}", out); - let _: ArtifactStoreConfig = toml::de::from_str(&out).unwrap(); - } - - #[test] - fn verify_raw_toml() { - let raw = r#" - [guest_images.alpine] - guest_os_kind = "alpine" - metadata.relative_local_path = "alpine.raw" - metadata.expected_digest = "abcd1234" - metadata.remote_uri = "https://127.0.0.1/alpine.raw" - - [bootroms.bootrom] - relative_local_path = "OVMF_CODE.fd" - remote_uri = "https://127.0.0.1/OVMF_CODE.fd" - "#; - - let store = ArtifactStoreConfig::from_toml(raw).unwrap(); - println!("Generated store: {:#?}", store); - - let guest_image = store.guest_images.get("alpine").unwrap(); - assert!(matches!(guest_image.guest_os_kind, GuestOsKind::Alpine)); - assert_eq!( - guest_image.metadata.relative_local_path.to_string_lossy(), - "alpine.raw" - ); - assert_eq!( - guest_image.metadata.expected_digest.as_ref().unwrap(), - "abcd1234" - ); - assert_eq!( - guest_image.metadata.remote_uri.as_ref().unwrap(), - "https://127.0.0.1/alpine.raw" - ); - - let bootrom = store.bootroms.get("bootrom").unwrap(); - assert_eq!( - bootrom.relative_local_path.to_string_lossy(), - "OVMF_CODE.fd" - ); - assert!(bootrom.expected_digest.is_none()); - assert_eq!( - bootrom.remote_uri.as_ref().unwrap(), - "https://127.0.0.1/OVMF_CODE.fd" - ); - } -} diff --git a/phd-tests/framework/src/artifacts/manifest.rs b/phd-tests/framework/src/artifacts/manifest.rs new file mode 100644 index 000000000..7f90770f7 --- /dev/null +++ b/phd-tests/framework/src/artifacts/manifest.rs @@ -0,0 +1,21 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use anyhow::Result; +use serde::Deserialize; +use std::collections::BTreeMap; + +#[derive(Clone, Debug, Deserialize)] +pub(super) struct Manifest { + pub(super) remote_server_uris: Vec, + pub(super) artifacts: BTreeMap, +} + +impl Manifest { + pub(super) fn from_toml_path(toml_path: &camino::Utf8Path) -> Result { + let contents = std::fs::read(toml_path.as_str())?; + let toml_contents = String::from_utf8_lossy(&contents); + Ok(toml::from_str(&toml_contents)?) + } +} diff --git a/phd-tests/framework/src/artifacts/mod.rs b/phd-tests/framework/src/artifacts/mod.rs new file mode 100644 index 000000000..36603b5fd --- /dev/null +++ b/phd-tests/framework/src/artifacts/mod.rs @@ -0,0 +1,49 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Support for working with files consumed by PHD test runs. + +use serde::{Deserialize, Serialize}; + +mod manifest; +mod store; + +pub use store::Store as ArtifactStore; + +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +enum ArtifactKind { + GuestOs(crate::guest_os::GuestOsKind), + Bootrom, + PropolisServer, +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +enum ArtifactSource { + /// Get the artifact from Buildomat. This downloads from + /// https://buildomat.eng.oxide.computer/public/file/REPO/SERIES/COMMIT. + Buildomat { repo: String, series: String, commit: String, sha256: String }, + + /// Get the artifact from the manifest's list of remote artifact servers. + RemoteServer { sha256: String }, + + /// Get the artifact from the local file system. + LocalPath { path: camino::Utf8PathBuf, sha256: Option }, +} + +/// An individual artifact. +#[derive(Clone, Debug, Serialize, Deserialize)] +struct Artifact { + /// The artifact file's name. When reacquiring an artifact from its source, + /// this filename is appended to the URI generated from that source. + filename: String, + + /// The kind of artifact this is. + kind: ArtifactKind, + + /// The source to use to obtain this artifact if it's not present on the + /// host system. + source: ArtifactSource, +} diff --git a/phd-tests/framework/src/artifacts/store.rs b/phd-tests/framework/src/artifacts/store.rs new file mode 100644 index 000000000..adae978ee --- /dev/null +++ b/phd-tests/framework/src/artifacts/store.rs @@ -0,0 +1,290 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use crate::{ + artifacts::{manifest::Manifest, ArtifactSource}, + guest_os::GuestOsKind, +}; + +use anyhow::bail; +use camino::{Utf8Path, Utf8PathBuf}; +use ring::digest::{Digest, SHA256}; +use std::collections::BTreeMap; +use std::fs::File; +use std::io::{BufReader, Read, Seek, SeekFrom, Write}; +use std::sync::Mutex; +use std::time::Duration; +use tracing::info; + +#[derive(Debug)] +struct StoredArtifact { + description: super::Artifact, + cached_path: Option, +} + +impl StoredArtifact { + fn ensure( + &mut self, + local_dir: &Utf8Path, + remote_uris: &[String], + ) -> anyhow::Result { + // If the artifact already exists and has been verified, return the path + // to it straightaway. + if let Some(path) = &self.cached_path { + info!(%path, "Verified artifact already exists"); + return Ok(path.clone()); + } + + // If the manifest says to look for a local copy of the file, see if it + // exists in the expected location and use it if it is. + if let ArtifactSource::LocalPath { path, sha256 } = + &self.description.source + { + let mut path = path.clone(); + path.push(self.description.filename.as_str()); + info!(%path, ?sha256, "Examining locally-sourced artifact"); + + // Local files can have a digest but aren't required to have one. + // This facilitates the use of local build outputs whose hashes + // frequently change. If a digest was passed, make sure it matches. + if let Some(digest) = sha256 { + hash_equals(&path, digest)?; + } else if !path.is_file() { + anyhow::bail!("artifact path {} is not a file", path); + } + + // The file is in the right place and has the right hash (if that + // was checked), so mark it as cached and return the cached path. + info!(%path, "Locally-sourced artifact is valid, caching its path"); + self.cached_path = Some(path.clone()); + return Ok(path.clone()); + } + + let expected_digest = match &self.description.source { + ArtifactSource::Buildomat { sha256, .. } => sha256, + ArtifactSource::RemoteServer { sha256 } => sha256, + ArtifactSource::LocalPath { .. } => { + unreachable!("local path case handled above") + } + }; + + // See if the artifact already exists in the expected location in the + // local artifact storage directory. If it does and it has the correct + // digest, mark the artifact as present. + let mut maybe_path = local_dir.to_path_buf(); + maybe_path + .push(format!("{}/{}", expected_digest, self.description.filename)); + + info!(%maybe_path, "checking for existing copy of artifact"); + if maybe_path.is_file() { + if hash_equals(&maybe_path, expected_digest).is_ok() { + info!(%maybe_path, + "Valid artifact already exists, caching its path"); + + self.cached_path = Some(maybe_path.clone()); + return Ok(maybe_path); + } else { + info!(%maybe_path, "Existing artifact is invalid, deleting it"); + std::fs::remove_file(&maybe_path)?; + } + } else if maybe_path.exists() { + anyhow::bail!( + "artifact path {} already exists but isn't a file", + maybe_path + ); + } + + // The artifact is not in the expected place or has the wrong digest, so + // reacquire it. First, construct the set of source URIs from which the + // artifact can be reacquired. + let source_uris = match &self.description.source { + ArtifactSource::Buildomat { repo, series, commit, .. } => { + let buildomat_uri = format!( + "https://buildomat.eng.oxide.computer/public/file\ + /{}/{}/{}/{}", + repo, series, commit, self.description.filename + ); + + vec![buildomat_uri] + } + ArtifactSource::RemoteServer { .. } => { + if remote_uris.is_empty() { + anyhow::bail!( + "can't acquire artifact from remote server with no \ + remote URIs" + ); + } + + remote_uris + .iter() + .map(|uri| format!("{}/{}", uri, self.description.filename)) + .collect() + } + ArtifactSource::LocalPath { .. } => { + unreachable!("local path case handled above") + } + }; + + // There is at least one plausible place from which to try to obtain the + // artifact. Create the directory that will hold it. + std::fs::create_dir_all(maybe_path.parent().unwrap())?; + let download_timeout = Duration::from_secs(600); + for uri in &source_uris { + info!(%maybe_path, + uri, + "Downloading artifact with timeout {:?}", + download_timeout); + + let client = reqwest::blocking::ClientBuilder::new() + .timeout(download_timeout) + .build()?; + + let request = client.get(uri).build()?; + let response = match client.execute(request) { + Ok(resp) => resp, + Err(e) => { + info!(?e, uri, "Error obtaining artifact from source"); + continue; + } + }; + + let mut new_file = std::fs::File::create(&maybe_path)?; + new_file.write_all(&response.bytes()?)?; + match hash_equals(&maybe_path, expected_digest) { + Ok(_) => { + // Make the newly-downloaded artifact read-only to try to + // ensure tests won't change it. Disks created from an + // artifact can be edited to be writable. + let mut permissions = new_file.metadata()?.permissions(); + permissions.set_readonly(true); + new_file.set_permissions(permissions)?; + self.cached_path = Some(maybe_path.clone()); + return Ok(maybe_path); + } + Err(e) => { + info!(?e, uri, "Failed to verify digest for artifact"); + } + } + } + + Err(anyhow::anyhow!( + "failed to locate or obtain artifact at path {}", + maybe_path + )) + } +} + +#[derive(Debug)] +pub struct Store { + local_dir: Utf8PathBuf, + artifacts: BTreeMap>, + remote_server_uris: Vec, +} + +impl Store { + pub fn from_toml_path( + local_dir: Utf8PathBuf, + toml_path: &Utf8Path, + ) -> anyhow::Result { + Ok(Self::from_manifest(local_dir, Manifest::from_toml_path(toml_path)?)) + } + + fn from_manifest(local_dir: Utf8PathBuf, manifest: Manifest) -> Self { + let Manifest { artifacts, remote_server_uris } = manifest; + let artifacts = artifacts + .into_iter() + .map(|(k, v)| { + ( + k, + Mutex::new(StoredArtifact { + description: v, + cached_path: None, + }), + ) + }) + .collect(); + + let store = Self { local_dir, artifacts, remote_server_uris }; + info!(?store, "Created new artifact store from manifest"); + store + } + + pub fn get_guest_os_image( + &self, + artifact_name: &str, + ) -> anyhow::Result<(Utf8PathBuf, GuestOsKind)> { + let entry = if let Some(e) = self.artifacts.get(artifact_name) { + e + } else { + anyhow::bail!("artifact {} not found in store", artifact_name); + }; + + let mut guard = entry.lock().unwrap(); + match guard.description.kind { + super::ArtifactKind::GuestOs(kind) => { + let path = + guard.ensure(&self.local_dir, &self.remote_server_uris)?; + Ok((path, kind)) + } + _ => Err(anyhow::anyhow!( + "artifact {} is not a guest OS image", + artifact_name + )), + } + } + + pub fn get_bootrom( + &self, + artifact_name: &str, + ) -> anyhow::Result { + let entry = if let Some(e) = self.artifacts.get(artifact_name) { + e + } else { + anyhow::bail!("artifact {} not found in store", artifact_name); + }; + + let mut guard = entry.lock().unwrap(); + match guard.description.kind { + super::ArtifactKind::Bootrom => { + guard.ensure(&self.local_dir, &self.remote_server_uris) + } + _ => Err(anyhow::anyhow!( + "artifact {} is not a bootrom", + artifact_name + )), + } + } +} + +fn sha256_digest(file: &mut File) -> anyhow::Result { + file.seek(SeekFrom::Start(0))?; + let mut reader = BufReader::new(file); + let mut context = ring::digest::Context::new(&SHA256); + let mut buffer = [0; 1024]; + + loop { + let count = reader.read(&mut buffer)?; + if count == 0 { + break; + } + context.update(&buffer[..count]); + } + + Ok(context.finish()) +} + +fn hash_equals(path: &Utf8Path, expected_digest: &str) -> anyhow::Result<()> { + let mut file = File::open(path)?; + let digest = hex::encode(sha256_digest(&mut file)?.as_ref()); + if digest != expected_digest { + bail!( + "Digest of {} was {}, expected {}", + path, + digest, + expected_digest + ); + } + + Ok(()) +} diff --git a/phd-tests/framework/src/disk/mod.rs b/phd-tests/framework/src/disk/mod.rs index 5a2c55bdc..a75163e13 100644 --- a/phd-tests/framework/src/disk/mod.rs +++ b/phd-tests/framework/src/disk/mod.rs @@ -13,6 +13,7 @@ use std::{ sync::Arc, }; +use anyhow::Context; use propolis_client::instance_spec::v0::StorageBackendV0; use thiserror::Error; @@ -31,9 +32,6 @@ mod file; /// Errors that can arise while working with disks. #[derive(Debug, Error)] pub enum DiskError { - #[error("Could not find source artifact {0}")] - ArtifactNotFound(String), - #[error("Disk factory has no Crucible downstairs path")] NoCrucibleDownstairsPath, @@ -150,10 +148,12 @@ impl DiskFactory<'_> { artifact_name: &str, ) -> Result<(PathBuf, GuestOsKind), DiskError> { self.artifact_store - .get_guest_artifact_info_by_name(artifact_name) - .ok_or_else(|| { - DiskError::ArtifactNotFound(artifact_name.to_string()) + .get_guest_os_image(artifact_name) + .map(|(utf8, kind)| (utf8.into_std_path_buf(), kind)) + .with_context(|| { + format!("failed to get guest OS artifact '{}'", artifact_name) }) + .map_err(Into::into) } /// Creates a new disk backed by a file whose initial contents are specified diff --git a/phd-tests/framework/src/test_vm/factory.rs b/phd-tests/framework/src/test_vm/factory.rs index f2151b898..0cf608615 100644 --- a/phd-tests/framework/src/test_vm/factory.rs +++ b/phd-tests/framework/src/test_vm/factory.rs @@ -9,7 +9,8 @@ use std::{ path::PathBuf, }; -use anyhow::Result; +use anyhow::{Context, Result}; +use camino::Utf8PathBuf; use thiserror::Error; use tracing::info; @@ -26,11 +27,6 @@ use super::{ /// Errors that can arise while creating a VM factory. #[derive(Debug, Error)] pub enum FactoryConstructionError { - /// Raised if the default bootrom key in the [`FactoryOptions`] does not - /// yield a valid bootrom from the artifact store. - #[error("Default bootrom {0} not in artifact store")] - DefaultBootromMissing(String), - /// Raised if the default guest image key in the [`FactoryOptions`] does not /// yield a valid image from the artifact store. #[error("Default guest image {0} not in artifact store")] @@ -47,11 +43,11 @@ pub enum FactoryConstructionError { pub struct FactoryOptions { /// The path to the Propolis server binary to use for VMs created by this /// factory. - pub propolis_server_path: String, + pub propolis_server_path: Utf8PathBuf, /// The directory to use as a temporary directory for config TOML files, /// server logs, and the like. - pub tmp_directory: PathBuf, + pub tmp_directory: Utf8PathBuf, /// The logging discipline to use for this factory's Propolis servers. pub server_log_mode: ServerLogMode, @@ -86,16 +82,17 @@ impl<'a> VmFactory<'a> { ) -> Result { info!(?opts, "Building VM factory"); let bootrom_path = store - .get_bootrom_by_name(&opts.default_bootrom_artifact) - .ok_or_else(|| { - FactoryConstructionError::DefaultBootromMissing( - opts.default_bootrom_artifact.clone(), + .get_bootrom(&opts.default_bootrom_artifact) + .with_context(|| { + format!( + "failed to get path to bootrom artifact '{}'", + &opts.default_bootrom_artifact ) })?; Ok(Self { opts, - default_bootrom_path: bootrom_path.to_string_lossy().to_string(), + default_bootrom_path: bootrom_path.into_string(), port_allocator, }) } @@ -174,7 +171,7 @@ impl VmFactory<'_> { let server_port = self.port_allocator.next()?; let vnc_port = self.port_allocator.next()?; let server_params = ServerProcessParameters { - server_path: &self.opts.propolis_server_path, + server_path: self.opts.propolis_server_path.as_str(), config_toml_path: vm_config.server_toml_path().clone(), server_addr: SocketAddrV4::new( Ipv4Addr::new(127, 0, 0, 1), diff --git a/phd-tests/runner/Cargo.toml b/phd-tests/runner/Cargo.toml index 6a01aa412..c54db0c55 100644 --- a/phd-tests/runner/Cargo.toml +++ b/phd-tests/runner/Cargo.toml @@ -12,6 +12,7 @@ doctest = false [dependencies] anyhow.workspace = true backtrace.workspace = true +camino.workspace = true clap = { workspace = true, features = ["derive"] } phd-framework.workspace = true phd-tests.workspace = true diff --git a/phd-tests/runner/src/config.rs b/phd-tests/runner/src/config.rs index ebf74cd37..6446f9000 100644 --- a/phd-tests/runner/src/config.rs +++ b/phd-tests/runner/src/config.rs @@ -2,8 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::path::PathBuf; - +use camino::Utf8PathBuf; use clap::{Args, Parser, Subcommand}; use phd_framework::server_log_mode::ServerLogMode; @@ -34,21 +33,21 @@ pub struct ProcessArgs { pub struct RunOptions { /// The command to use to launch the Propolis server. #[clap(long, value_parser)] - pub propolis_server_cmd: PathBuf, + pub propolis_server_cmd: Utf8PathBuf, /// The command to use to launch Crucible downstairs servers. #[clap(long, value_parser)] - pub crucible_downstairs_cmd: Option, + pub crucible_downstairs_cmd: Option, /// The directory into which to write temporary files (config TOMLs, log /// files, etc.) generated during test execution. #[clap(long, value_parser)] - pub tmp_directory: PathBuf, + pub tmp_directory: Utf8PathBuf, /// The directory in which artifacts (guest OS images, bootroms, etc.) /// are to be stored. #[clap(long, value_parser)] - pub artifact_directory: PathBuf, + pub artifact_directory: Utf8PathBuf, /// If true, direct Propolis servers created by the runner to log to /// stdout/stderr handles inherited from the runner. @@ -76,7 +75,7 @@ pub struct RunOptions { /// The path to a TOML file describing the artifact store to use for this /// run. #[clap(long, value_parser)] - pub artifact_toml_path: PathBuf, + pub artifact_toml_path: Utf8PathBuf, /// The default artifact store key to use to load a guest OS image in tests /// that do not explicitly specify one. diff --git a/phd-tests/runner/src/fixtures.rs b/phd-tests/runner/src/fixtures.rs index 69486cef7..9b02b1854 100644 --- a/phd-tests/runner/src/fixtures.rs +++ b/phd-tests/runner/src/fixtures.rs @@ -3,31 +3,26 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use anyhow::Result; -use phd_framework::artifacts::ArtifactStore; use tracing::instrument; use crate::TestContext; /// A wrapper containing the objects needed to run the executor's test fixtures. pub struct TestFixtures<'a> { - artifact_store: &'a ArtifactStore, test_context: &'a TestContext<'a>, } impl<'a> TestFixtures<'a> { /// Creates a new set of test fixtures using the supplied command-line /// parameters and artifact store. - pub fn new( - artifact_store: &'a ArtifactStore, - test_context: &'a TestContext, - ) -> Result { - Ok(Self { artifact_store, test_context }) + pub fn new(test_context: &'a TestContext) -> Result { + Ok(Self { test_context }) } /// Calls fixture routines that need to run before any tests run. #[instrument(skip_all)] pub fn execution_setup(&mut self) -> Result<()> { - self.artifact_store.check_local_copies() + Ok(()) } /// Calls fixture routines that need to run after all tests run. diff --git a/phd-tests/runner/src/main.rs b/phd-tests/runner/src/main.rs index 02f5c1bda..46f491e39 100644 --- a/phd-tests/runner/src/main.rs +++ b/phd-tests/runner/src/main.rs @@ -44,9 +44,9 @@ fn main() { } fn run_tests(run_opts: &RunOptions) -> ExecutionStats { - let artifact_store = ArtifactStore::from_file( - &run_opts.artifact_toml_path, + let artifact_store = ArtifactStore::from_toml_path( run_opts.artifact_directory.clone(), + &run_opts.artifact_toml_path, ) .unwrap(); @@ -57,10 +57,7 @@ fn run_tests(run_opts: &RunOptions) -> ExecutionStats { let mut config_toml_path = run_opts.tmp_directory.clone(); config_toml_path.push("vm_config.toml"); let factory_config = phd_framework::test_vm::factory::FactoryOptions { - propolis_server_path: run_opts - .propolis_server_cmd - .to_string_lossy() - .to_string(), + propolis_server_path: run_opts.propolis_server_cmd.clone(), tmp_directory: run_opts.tmp_directory.clone(), server_log_mode: run_opts.server_logging_mode, default_bootrom_artifact: run_opts.default_bootrom_artifact.clone(), @@ -86,7 +83,7 @@ fn run_tests(run_opts: &RunOptions) -> ExecutionStats { run_opts.server_logging_mode, ), }; - let fixtures = TestFixtures::new(&artifact_store, &ctx).unwrap(); + let fixtures = TestFixtures::new(&ctx).unwrap(); // Run the tests and print results. let execution_stats = execute::run_tests_with_ctx(&ctx, fixtures, run_opts);