From 49335c362806b666adec74fa91543143cd6264f0 Mon Sep 17 00:00:00 2001 From: Rain Date: Fri, 20 Dec 2024 10:27:28 +0000 Subject: [PATCH] [spr] changes to main this commit is based on Created using spr 1.3.6-beta.1 [skip ci] --- .github/workflows/rust.yml | 13 ++- .gitignore | 2 +- Cargo.toml | 6 +- src/config/identifier.rs | 168 +++++++++++++++++++++++++++++++ src/{config.rs => config/imp.rs} | 45 +++++---- src/config/mod.rs | 9 ++ src/package.rs | 85 +++++++++------- src/target.rs | 23 +++-- tests/mod.rs | 60 +++++------ 9 files changed, 306 insertions(+), 105 deletions(-) create mode 100644 src/config/identifier.rs rename src/{config.rs => config/imp.rs} (85%) create mode 100644 src/config/mod.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 8adca91..04e2f97 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -3,11 +3,15 @@ # name: Rust +env: + RUSTFLAGS: -D warnings + on: push: branches: [ main ] pull_request: - branches: [ main ] + # Allow CI to run on all PRs, not just ones targeting main. This allows + # stacked PRs to be tested. jobs: check-style: @@ -22,15 +26,22 @@ jobs: components: rustfmt - name: Check style run: cargo fmt -- --check + - name: Check clippy + run: cargo clippy --all-targets --all-features build-and-test: runs-on: ${{ matrix.os }} strategy: matrix: + # 1.81 is the MSRV. + toolchain: [ stable, "1.81" ] os: [ ubuntu-latest, macos-latest ] steps: # actions/checkout@v2 - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 + - uses: dtolnay/rust-toolchain@v1 + with: + toolchain: ${{ matrix.toolchain }} - name: Build run: cargo build --tests --verbose - name: Run tests diff --git a/.gitignore b/.gitignore index 088ba6b..321eccd 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,6 @@ # Generated by Cargo # will have compiled files and executables -/target/ +/target # Remove Cargo.lock from gitignore if creating an executable, leave it for libraries # More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html diff --git a/Cargo.toml b/Cargo.toml index a6f130e..5f94e34 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,11 +3,7 @@ name = "omicron-zone-package" version = "0.11.1" authors = ["Sean Klein "] edition = "2021" -# -# Report a specific error in the case that the toolchain is too old for -# let-else: -# -rust-version = "1.65.0" +rust-version = "1.81.0" license = "MPL-2.0" repository = "https://github.com/oxidecomputer/omicron-package" description = "Packaging tools for Oxide's control plane software" diff --git a/src/config/identifier.rs b/src/config/identifier.rs new file mode 100644 index 0000000..cb2edc2 --- /dev/null +++ b/src/config/identifier.rs @@ -0,0 +1,168 @@ +// 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 std::{borrow::Cow, fmt, str::FromStr}; + +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +/// A unique identifier for a configuration parameter. +/// +/// Config identifiers must be: +/// +/// * non-empty +/// * ASCII printable +/// * first character must be a letter +/// * contain only letters, numbers, underscores, and hyphens +/// +/// In general, config identifiers represent Rust package and Oxide service names. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize)] +#[serde(transparent)] +pub struct ConfigIdent(Cow<'static, str>); + +impl ConfigIdent { + /// Creates a new config identifier at runtime. + pub fn new>(s: S) -> Result { + let s = s.into(); + Self::validate(&s)?; + Ok(Self(Cow::Owned(s))) + } + + /// Creates a new config identifier from a static string. + pub fn new_static(s: &'static str) -> Result { + Self::validate(s)?; + Ok(Self(Cow::Borrowed(s))) + } + + /// Creates a new config identifier at compile time, panicking if the + /// identifier is invalid. + pub const fn new_const(s: &'static str) -> Self { + match Self::validate(s) { + Ok(_) => Self(Cow::Borrowed(s)), + Err(error) => panic!("{}", error.as_static_str()), + } + } + + const fn validate(id: &str) -> Result<(), InvalidConfigIdent> { + if id.is_empty() { + return Err(InvalidConfigIdent::Empty); + } + + let bytes = id.as_bytes(); + if !bytes[0].is_ascii_alphabetic() { + return Err(InvalidConfigIdent::StartsWithNonLetter); + } + + let mut bytes = match bytes { + [_, rest @ ..] => rest, + [] => panic!("already checked that it's non-empty"), + }; + while let [next, rest @ ..] = &bytes { + if !(next.is_ascii_alphanumeric() || *next == b'_' || *next == b'-') { + break; + } + bytes = rest; + } + + if !bytes.is_empty() { + return Err(InvalidConfigIdent::ContainsInvalidCharacters); + } + + Ok(()) + } + + /// Returns the identifier as a string. + #[inline] + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl FromStr for ConfigIdent { + type Err = InvalidConfigIdent; + + fn from_str(s: &str) -> Result { + Self::new(s) + } +} + +impl<'de> Deserialize<'de> for ConfigIdent { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + Self::new(s).map_err(serde::de::Error::custom) + } +} + +impl AsRef for ConfigIdent { + #[inline] + fn as_ref(&self) -> &str { + &self.0 + } +} + +impl std::fmt::Display for ConfigIdent { + #[inline] + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + self.0.fmt(f) + } +} + +/// Errors that can occur when creating a `ConfigIdent`. +#[derive(Clone, Debug, Error)] +pub enum InvalidConfigIdent { + Empty, + NonAsciiPrintable, + StartsWithNonLetter, + ContainsInvalidCharacters, +} + +impl InvalidConfigIdent { + pub const fn as_static_str(&self) -> &'static str { + match self { + Self::Empty => "config identifier must be non-empty", + Self::NonAsciiPrintable => "config identifier must be ASCII printable", + Self::StartsWithNonLetter => "config identifier must start with a letter", + Self::ContainsInvalidCharacters => { + "config identifier must contain only letters, numbers, underscores, and hyphens" + } + } + } +} + +impl fmt::Display for InvalidConfigIdent { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.as_static_str().fmt(f) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn valid_identifiers() { + let valid = [ + "a", "ab", "a1", "a_", "a-", "a_b", "a-b", "a1_", "a1-", "a1_b", "a1-b", + ]; + for &id in &valid { + ConfigIdent::new(id).unwrap_or_else(|error| { + panic!("{} should have succeeded, but failed with: {:?}", id, error); + }); + } + } + + #[test] + fn invalid_identifiers() { + let invalid = [ + "", "1", "_", "-", "1_", "-a", "_a", "a!", "a ", "a\n", "a\t", "a\r", "a\x7F", "aɑ", + ]; + for &id in &invalid { + ConfigIdent::new(id).expect_err(&format!("{} should have failed", id)); + } + } +} diff --git a/src/config.rs b/src/config/imp.rs similarity index 85% rename from src/config.rs rename to src/config/imp.rs index 041d52c..4cf83bb 100644 --- a/src/config.rs +++ b/src/config/imp.rs @@ -5,17 +5,19 @@ //! Configuration for a package. use crate::package::{Package, PackageOutput, PackageSource}; -use crate::target::Target; +use crate::target::TargetMap; use serde_derive::Deserialize; use std::collections::BTreeMap; use std::path::Path; use thiserror::Error; use topological_sort::TopologicalSort; +use super::ConfigIdent; + /// Describes a set of packages to act upon. /// /// This structure maps "package name" to "package" -pub struct PackageMap<'a>(pub BTreeMap<&'a String, &'a Package>); +pub struct PackageMap<'a>(pub BTreeMap<&'a ConfigIdent, &'a Package>); // The name of a file which should be created by building a package. #[derive(Clone, Eq, Hash, Ord, PartialEq, PartialOrd)] @@ -68,12 +70,12 @@ impl<'a> PackageMap<'a> { /// /// Returns packages in batches that may be built concurrently. pub struct PackageDependencyIter<'a> { - lookup_by_output: BTreeMap, + lookup_by_output: BTreeMap, outputs: TopologicalSort, } impl<'a> Iterator for PackageDependencyIter<'a> { - type Item = Vec<(&'a String, &'a Package)>; + type Item = Vec<(&'a ConfigIdent, &'a Package)>; fn next(&mut self) -> Option { if self.outputs.is_empty() { @@ -99,27 +101,26 @@ impl<'a> Iterator for PackageDependencyIter<'a> { } /// Describes the configuration for a set of packages. -#[derive(Deserialize, Debug)] +#[derive(Clone, Deserialize, Debug)] pub struct Config { /// Packages to be built and installed. #[serde(default, rename = "package")] - pub packages: BTreeMap, + pub packages: BTreeMap, } impl Config { /// Returns target packages to be assembled on the builder machine. - pub fn packages_to_build(&self, target: &Target) -> PackageMap<'_> { + pub fn packages_to_build(&self, target: &TargetMap) -> PackageMap<'_> { PackageMap( self.packages .iter() .filter(|(_, pkg)| target.includes_package(pkg)) - .map(|(name, pkg)| (name, pkg)) .collect(), ) } /// Returns target packages which should execute on the deployment machine. - pub fn packages_to_deploy(&self, target: &Target) -> PackageMap<'_> { + pub fn packages_to_deploy(&self, target: &TargetMap) -> PackageMap<'_> { let all_packages = self.packages_to_build(target).0; PackageMap( all_packages @@ -159,18 +160,18 @@ mod test { #[test] fn test_order() { - let pkg_a_name = String::from("pkg-a"); + let pkg_a_name = ConfigIdent::new_const("pkg-a"); let pkg_a = Package { - service_name: String::from("a"), + service_name: ConfigIdent::new_const("a"), source: PackageSource::Manual, output: PackageOutput::Tarball, only_for_targets: None, setup_hint: None, }; - let pkg_b_name = String::from("pkg-b"); + let pkg_b_name = ConfigIdent::new_const("pkg-b"); let pkg_b = Package { - service_name: String::from("b"), + service_name: ConfigIdent::new_const("b"), source: PackageSource::Composite { packages: vec![pkg_a.get_output_file(&pkg_a_name)], }, @@ -186,7 +187,7 @@ mod test { ]), }; - let mut order = cfg.packages_to_build(&Target::default()).build_order(); + let mut order = cfg.packages_to_build(&TargetMap::default()).build_order(); // "pkg-a" comes first, because "pkg-b" depends on it. assert_eq!(order.next(), Some(vec![(&pkg_a_name, &pkg_a)])); assert_eq!(order.next(), Some(vec![(&pkg_b_name, &pkg_b)])); @@ -199,10 +200,10 @@ mod test { #[test] #[should_panic(expected = "cyclic dependency in package manifest")] fn test_cyclic_dependency() { - let pkg_a_name = String::from("pkg-a"); - let pkg_b_name = String::from("pkg-b"); + let pkg_a_name = ConfigIdent::new_const("pkg-a"); + let pkg_b_name = ConfigIdent::new_const("pkg-b"); let pkg_a = Package { - service_name: String::from("a"), + service_name: ConfigIdent::new_const("a"), source: PackageSource::Composite { packages: vec![String::from("pkg-b.tar")], }, @@ -211,7 +212,7 @@ mod test { setup_hint: None, }; let pkg_b = Package { - service_name: String::from("b"), + service_name: ConfigIdent::new_const("b"), source: PackageSource::Composite { packages: vec![String::from("pkg-a.tar")], }, @@ -227,7 +228,7 @@ mod test { ]), }; - let mut order = cfg.packages_to_build(&Target::default()).build_order(); + let mut order = cfg.packages_to_build(&TargetMap::default()).build_order(); order.next(); } @@ -237,9 +238,9 @@ mod test { #[test] #[should_panic(expected = "Could not find a package which creates 'pkg-b.tar'")] fn test_missing_dependency() { - let pkg_a_name = String::from("pkg-a"); + let pkg_a_name = ConfigIdent::new_const("pkg-a"); let pkg_a = Package { - service_name: String::from("a"), + service_name: ConfigIdent::new_const("a"), source: PackageSource::Composite { packages: vec![String::from("pkg-b.tar")], }, @@ -252,7 +253,7 @@ mod test { packages: BTreeMap::from([(pkg_a_name.clone(), pkg_a.clone())]), }; - let mut order = cfg.packages_to_build(&Target::default()).build_order(); + let mut order = cfg.packages_to_build(&TargetMap::default()).build_order(); order.next(); } } diff --git a/src/config/mod.rs b/src/config/mod.rs new file mode 100644 index 0000000..de4ca23 --- /dev/null +++ b/src/config/mod.rs @@ -0,0 +1,9 @@ +// 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/. + +mod identifier; +mod imp; + +pub use identifier::*; +pub use imp::*; diff --git a/src/package.rs b/src/package.rs index c148076..88ba399 100644 --- a/src/package.rs +++ b/src/package.rs @@ -10,9 +10,10 @@ use crate::archive::{ }; use crate::blob::{self, BLOB}; use crate::cache::{Cache, CacheError}; +use crate::config::ConfigIdent; use crate::input::{BuildInput, BuildInputs, MappedPath, TargetDirectory, TargetPackage}; use crate::progress::{NoProgress, Progress}; -use crate::target::Target; +use crate::target::TargetMap; use crate::timer::BuildTimer; use anyhow::{anyhow, bail, Context, Result}; @@ -168,7 +169,7 @@ pub enum PackageOutput { #[derive(Clone, Deserialize, Debug, PartialEq)] pub struct Package { /// The name of the service name to be used on the target OS. - pub service_name: String, + pub service_name: ConfigIdent, /// Identifies from where the package originates. /// @@ -182,7 +183,7 @@ pub struct Package { /// Identifies the targets for which the package should be included. /// /// If ommitted, the package is assumed to be included for all targets. - pub only_for_targets: Option>, + pub only_for_targets: Option, /// A human-readable string with suggestions for setup if packaging fails. #[serde(default)] @@ -193,7 +194,7 @@ pub struct Package { const DEFAULT_VERSION: semver::Version = semver::Version::new(0, 0, 0); async fn new_zone_archive_builder( - package_name: &str, + package_name: &ConfigIdent, output_directory: &Utf8Path, ) -> Result>> { let tarfile = output_directory.join(format!("{}.tar.gz", package_name)); @@ -203,7 +204,7 @@ async fn new_zone_archive_builder( /// Configuration that can modify how a package is built. pub struct BuildConfig<'a> { /// Describes the [Target] to build the package for. - pub target: &'a Target, + pub target: &'a TargetMap, /// Describes how progress will be communicated back to the caller. pub progress: &'a dyn Progress, @@ -212,10 +213,10 @@ pub struct BuildConfig<'a> { pub cache_disabled: bool, } -static DEFAULT_TARGET: Target = Target(BTreeMap::new()); +static DEFAULT_TARGET: TargetMap = TargetMap(BTreeMap::new()); static DEFAULT_PROGRESS: NoProgress = NoProgress::new(); -impl<'a> Default for BuildConfig<'a> { +impl Default for BuildConfig<'_> { fn default() -> Self { Self { target: &DEFAULT_TARGET, @@ -227,19 +228,23 @@ impl<'a> Default for BuildConfig<'a> { impl Package { /// The path of a package once it is built. - pub fn get_output_path(&self, name: &str, output_directory: &Utf8Path) -> Utf8PathBuf { - output_directory.join(self.get_output_file(name)) + pub fn get_output_path(&self, id: &ConfigIdent, output_directory: &Utf8Path) -> Utf8PathBuf { + output_directory.join(self.get_output_file(id)) } /// The path of a package after it has been "stamped" with a version. - pub fn get_stamped_output_path(&self, name: &str, output_directory: &Utf8Path) -> Utf8PathBuf { + pub fn get_stamped_output_path( + &self, + name: &ConfigIdent, + output_directory: &Utf8Path, + ) -> Utf8PathBuf { output_directory .join("versioned") .join(self.get_output_file(name)) } /// The filename of a package once it is built. - pub fn get_output_file(&self, name: &str) -> String { + pub fn get_output_file(&self, name: &ConfigIdent) -> String { match self.output { PackageOutput::Zone { .. } => format!("{}.tar.gz", name), PackageOutput::Tarball => format!("{}.tar", name), @@ -249,8 +254,8 @@ impl Package { #[deprecated = "Use 'Package::create', which now takes a 'BuildConfig', and implements 'Default'"] pub async fn create_for_target( &self, - target: &Target, - name: &str, + target: &TargetMap, + name: &ConfigIdent, output_directory: &Utf8Path, ) -> Result { let build_config = BuildConfig { @@ -263,7 +268,7 @@ impl Package { pub async fn create( &self, - name: &str, + name: &ConfigIdent, output_directory: &Utf8Path, build_config: &BuildConfig<'_>, ) -> Result { @@ -273,7 +278,7 @@ impl Package { pub async fn stamp( &self, - name: &str, + name: &ConfigIdent, output_directory: &Utf8Path, version: &semver::Version, ) -> Result { @@ -345,8 +350,8 @@ impl Package { pub async fn create_with_progress_for_target( &self, progress: &impl Progress, - target: &Target, - name: &str, + target: &TargetMap, + name: &ConfigIdent, output_directory: &Utf8Path, ) -> Result { let config = BuildConfig { @@ -359,7 +364,7 @@ impl Package { async fn create_internal( &self, - name: &str, + name: &ConfigIdent, output_directory: &Utf8Path, config: &BuildConfig<'_>, ) -> Result { @@ -382,7 +387,7 @@ impl Package { // Adds the version file to the archive fn get_version_input( &self, - package_name: &str, + package_name: &ConfigIdent, version: Option<&semver::Version>, ) -> BuildInput { match &self.output { @@ -397,7 +402,7 @@ impl Package { let kvs = vec![ ("v", "1"), ("t", "layer"), - ("pkg", package_name), + ("pkg", package_name.as_ref()), ("version", version), ]; @@ -427,7 +432,7 @@ impl Package { fn get_paths_inputs( &self, - target: &Target, + target: &TargetMap, paths: &Vec, ) -> Result { let mut inputs = BuildInputs::new(); @@ -514,8 +519,8 @@ impl Package { fn get_all_inputs( &self, - package_name: &str, - target: &Target, + package_name: &ConfigIdent, + target: &TargetMap, output_directory: &Utf8Path, zoned: bool, version: Option<&semver::Version>, @@ -559,7 +564,7 @@ impl Package { let dst_directory = match self.output { PackageOutput::Zone { .. } => { let dst = Utf8Path::new("/opt/oxide") - .join(&self.service_name) + .join(self.service_name.as_str()) .join("bin"); inputs.0.extend( zone_get_all_parent_inputs(&dst)? @@ -589,7 +594,7 @@ impl Package { let destination_path = if zoned { zone_archive_path( &Utf8Path::new("/opt/oxide") - .join(&self.service_name) + .join(self.service_name.as_str()) .join(BLOB), )? } else { @@ -597,7 +602,9 @@ impl Package { }; if let Some(s3_blobs) = self.source.blobs() { inputs.0.extend(s3_blobs.iter().map(|blob| { - let from = download_directory.join(&self.service_name).join(blob); + let from = download_directory + .join(self.service_name.as_str()) + .join(blob); let to = destination_path.join(blob); BuildInput::AddBlob { path: MappedPath { from, to }, @@ -608,7 +615,7 @@ impl Package { if let Some(buildomat_blobs) = self.source.buildomat_blobs() { inputs.0.extend(buildomat_blobs.iter().map(|blob| { let from = download_directory - .join(&self.service_name) + .join(self.service_name.as_str()) .join(&blob.artifact); let to = destination_path.join(&blob.artifact); BuildInput::AddBlob { @@ -623,7 +630,7 @@ impl Package { async fn create_zone_package( &self, timer: &mut BuildTimer, - name: &str, + name: &ConfigIdent, output_directory: &Utf8Path, config: &BuildConfig<'_>, ) -> Result { @@ -762,7 +769,7 @@ impl Package { async fn create_tarball_package( &self, - name: &str, + name: &ConfigIdent, output_directory: &Utf8Path, config: &BuildConfig<'_>, ) -> Result { @@ -851,7 +858,7 @@ pub struct InterpolatedString(String); impl InterpolatedString { // Interpret the string for the specified target. // Substitutes key/value pairs as necessary. - pub fn interpolate(&self, target: &Target) -> Result { + pub fn interpolate(&self, target: &TargetMap) -> Result { let mut input = self.0.as_str(); let mut output = String::new(); @@ -893,7 +900,7 @@ pub struct InterpolatedMappedPath { } impl InterpolatedMappedPath { - fn interpolate(&self, target: &Target) -> Result { + fn interpolate(&self, target: &TargetMap) -> Result { Ok(MappedPath { from: Utf8PathBuf::from(self.from.interpolate(target)?), to: Utf8PathBuf::from(self.to.interpolate(target)?), @@ -907,7 +914,7 @@ mod test { #[test] fn interpolate_noop() { - let target = Target(BTreeMap::new()); + let target = TargetMap(BTreeMap::new()); let is = InterpolatedString(String::from("nothing to change")); let s = is.interpolate(&target).unwrap(); @@ -916,7 +923,7 @@ mod test { #[test] fn interpolate_single() { - let mut target = Target(BTreeMap::new()); + let mut target = TargetMap(BTreeMap::new()); target.0.insert("key1".to_string(), "value1".to_string()); let is = InterpolatedString(String::from("{{key1}}")); @@ -926,7 +933,7 @@ mod test { #[test] fn interpolate_single_with_prefix() { - let mut target = Target(BTreeMap::new()); + let mut target = TargetMap(BTreeMap::new()); target.0.insert("key1".to_string(), "value1".to_string()); let is = InterpolatedString(String::from("prefix-{{key1}}")); @@ -936,7 +943,7 @@ mod test { #[test] fn interpolate_single_with_suffix() { - let mut target = Target(BTreeMap::new()); + let mut target = TargetMap(BTreeMap::new()); target.0.insert("key1".to_string(), "value1".to_string()); let is = InterpolatedString(String::from("{{key1}}-suffix")); @@ -946,7 +953,7 @@ mod test { #[test] fn interpolate_multiple() { - let mut target = Target(BTreeMap::new()); + let mut target = TargetMap(BTreeMap::new()); target.0.insert("key1".to_string(), "value1".to_string()); target.0.insert("key2".to_string(), "value2".to_string()); let is = InterpolatedString(String::from("{{key1}}-{{key2}}")); @@ -957,7 +964,7 @@ mod test { #[test] fn interpolate_missing_key() { - let mut target = Target(BTreeMap::new()); + let mut target = TargetMap(BTreeMap::new()); target.0.insert("key1".to_string(), "value1".to_string()); let is = InterpolatedString(String::from("{{key3}}")); @@ -972,7 +979,7 @@ mod test { #[test] fn interpolate_missing_closing() { - let mut target = Target(BTreeMap::new()); + let mut target = TargetMap(BTreeMap::new()); target.0.insert("key1".to_string(), "value1".to_string()); let is = InterpolatedString(String::from("{{key1")); @@ -992,7 +999,7 @@ mod test { // as part of they key -- INCLUDING other "{{" characters. #[test] fn interpolate_key_as_literal() { - let mut target = Target(BTreeMap::new()); + let mut target = TargetMap(BTreeMap::new()); target.0.insert("oh{{no".to_string(), "value".to_string()); let is = InterpolatedString(String::from("{{oh{{no}}")); diff --git a/src/target.rs b/src/target.rs index d803525..4b19c32 100644 --- a/src/target.rs +++ b/src/target.rs @@ -3,14 +3,19 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use crate::package::Package; +use serde::Deserialize; use std::collections::BTreeMap; -/// A target describes what platform and configuration we're trying -/// to deploy on. -#[derive(Clone, Debug, Default)] -pub struct Target(pub BTreeMap); +/// Describes what platform and configuration we're trying to deploy on. +/// +/// For flexibility, this is an arbitrary key-value map without any attached +/// semantics to particular keys. Those semantics are provided by the consumers +/// of this tooling within omicron. +#[derive(Clone, Debug, Default, PartialEq, Deserialize)] +#[serde(transparent)] +pub struct TargetMap(pub BTreeMap); -impl Target { +impl TargetMap { // Returns true if this target should include the package. pub(crate) fn includes_package(&self, pkg: &Package) -> bool { let valid_targets = if let Some(targets) = &pkg.only_for_targets { @@ -24,7 +29,7 @@ impl Target { // For each of the targets permitted by the package, check if // the current target matches. - for (k, v) in valid_targets { + for (k, v) in &valid_targets.0 { let target_value = if let Some(target_value) = self.0.get(k) { target_value } else { @@ -39,7 +44,7 @@ impl Target { } } -impl std::fmt::Display for Target { +impl std::fmt::Display for TargetMap { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { for (key, value) in &self.0 { write!(f, "{}={} ", key, value)?; @@ -54,7 +59,7 @@ pub enum TargetParseError { MissingEquals(String), } -impl std::str::FromStr for Target { +impl std::str::FromStr for TargetMap { type Err = TargetParseError; fn from_str(s: &str) -> Result { @@ -66,6 +71,6 @@ impl std::str::FromStr for Target { .map(|(k, v)| (k.to_string(), v.to_string())) }) .collect::, _>>()?; - Ok(Target(kvs)) + Ok(TargetMap(kvs)) } } diff --git a/tests/mod.rs b/tests/mod.rs index c961bf7..f16db4a 100644 --- a/tests/mod.rs +++ b/tests/mod.rs @@ -12,10 +12,13 @@ mod test { use tar::Archive; use omicron_zone_package::blob::download; - use omicron_zone_package::config; + use omicron_zone_package::config::{self, ConfigIdent}; use omicron_zone_package::package::BuildConfig; use omicron_zone_package::progress::NoProgress; - use omicron_zone_package::target::Target; + use omicron_zone_package::target::TargetMap; + + const MY_PACKAGE: ConfigIdent = ConfigIdent::new_const("my-package"); + const MY_SERVICE: ConfigIdent = ConfigIdent::new_const("my-service"); fn entry_path<'a, R>(entry: &tar::Entry<'a, R>) -> Utf8PathBuf where @@ -58,19 +61,18 @@ mod test { async fn test_package_as_zone() { // Parse the configuration let cfg = config::parse("tests/service-a/cfg.toml").unwrap(); - let package_name = "my-service"; - let package = cfg.packages.get(package_name).unwrap(); + let package = cfg.packages.get(&MY_SERVICE).unwrap(); // Create the packaged file let out = camino_tempfile::tempdir().unwrap(); let build_config = BuildConfig::default(); package - .create(package_name, out.path(), &build_config) + .create(&MY_SERVICE, out.path(), &build_config) .await .unwrap(); // Verify the contents - let path = package.get_output_path(package_name, out.path()); + let path = package.get_output_path(&MY_SERVICE, out.path()); assert!(path.exists()); let gzr = flate2::read::GzDecoder::new(File::open(path).unwrap()); let mut archive = Archive::new(gzr); @@ -97,19 +99,18 @@ mod test { async fn test_rust_package_as_zone() { // Parse the configuration let cfg = config::parse("tests/service-b/cfg.toml").unwrap(); - let package_name = "my-service"; - let package = cfg.packages.get(package_name).unwrap(); + let package = cfg.packages.get(&MY_SERVICE).unwrap(); // Create the packaged file let out = camino_tempfile::tempdir().unwrap(); let build_config = BuildConfig::default(); package - .create(package_name, out.path(), &build_config) + .create(&MY_SERVICE, out.path(), &build_config) .await .unwrap(); // Verify the contents - let path = package.get_output_path(package_name, out.path()); + let path = package.get_output_path(&MY_SERVICE, out.path()); assert!(path.exists()); let gzr = flate2::read::GzDecoder::new(File::open(path).unwrap()); let mut archive = Archive::new(gzr); @@ -140,19 +141,18 @@ mod test { async fn test_rust_package_as_tarball() { // Parse the configuration let cfg = config::parse("tests/service-c/cfg.toml").unwrap(); - let package_name = "my-service"; - let package = cfg.packages.get(package_name).unwrap(); + let package = cfg.packages.get(&MY_SERVICE).unwrap(); // Create the packaged file let out = camino_tempfile::tempdir().unwrap(); let build_config = BuildConfig::default(); package - .create(package_name, out.path(), &build_config) + .create(&MY_SERVICE, out.path(), &build_config) .await .unwrap(); // Verify the contents - let path = package.get_output_path(package_name, out.path()); + let path = package.get_output_path(&MY_SERVICE, out.path()); assert!(path.exists()); let mut archive = Archive::new(File::open(path).unwrap()); let mut ents = archive.entries().unwrap(); @@ -168,7 +168,7 @@ mod test { // Try stamping it, verify the contents again let expected_semver = semver::Version::new(3, 3, 3); let path = package - .stamp(package_name, out.path(), &expected_semver) + .stamp(&MY_SERVICE, out.path(), &expected_semver) .await .unwrap(); assert!(path.exists()); @@ -192,22 +192,20 @@ mod test { async fn test_rust_package_with_distinct_service_name() { // Parse the configuration let cfg = config::parse("tests/service-d/cfg.toml").unwrap(); - let package_name = "my-package"; - let service_name = "my-service"; - let package = cfg.packages.get(package_name).unwrap(); + let package = cfg.packages.get(&MY_PACKAGE).unwrap(); - assert_eq!(package.service_name, service_name); + assert_eq!(package.service_name, MY_SERVICE); // Create the packaged file let out = camino_tempfile::tempdir().unwrap(); let build_config = BuildConfig::default(); package - .create(package_name, out.path(), &build_config) + .create(&MY_PACKAGE, out.path(), &build_config) .await .unwrap(); // Verify the contents - let path = package.get_output_path(package_name, out.path()); + let path = package.get_output_path(&MY_PACKAGE, out.path()); assert!(path.exists()); let mut archive = Archive::new(File::open(path).unwrap()); let mut ents = archive.entries().unwrap(); @@ -223,14 +221,20 @@ mod test { let out = camino_tempfile::tempdir().unwrap(); // Ask for the order of packages to-be-built - let packages = cfg.packages_to_build(&Target::default()); + let packages = cfg.packages_to_build(&TargetMap::default()); let mut build_order = packages.build_order(); // Build the dependencies first. let batch = build_order.next().expect("Missing dependency batch"); let mut batch_pkg_names: Vec<_> = batch.iter().map(|(name, _)| *name).collect(); batch_pkg_names.sort(); - assert_eq!(batch_pkg_names, vec!["pkg-1", "pkg-2"]); + assert_eq!( + batch_pkg_names, + vec![ + &ConfigIdent::new_const("pkg-1"), + &ConfigIdent::new_const("pkg-2"), + ] + ); let build_config = BuildConfig::default(); for (package_name, package) in batch { // Create the packaged file @@ -243,17 +247,17 @@ mod test { // Build the composite package let batch = build_order.next().expect("Missing dependency batch"); let batch_pkg_names: Vec<_> = batch.iter().map(|(name, _)| *name).collect(); - let package_name = "pkg-3"; - assert_eq!(batch_pkg_names, vec![package_name]); - let package = cfg.packages.get(package_name).unwrap(); + let package_name = ConfigIdent::new_const("pkg-3"); + assert_eq!(batch_pkg_names, vec![&package_name]); + let package = cfg.packages.get(&package_name).unwrap(); let build_config = BuildConfig::default(); package - .create(package_name, out.path(), &build_config) + .create(&package_name, out.path(), &build_config) .await .unwrap(); // Verify the contents - let path = package.get_output_path(package_name, out.path()); + let path = package.get_output_path(&package_name, out.path()); assert!(path.exists()); let gzr = flate2::read::GzDecoder::new(File::open(path).unwrap()); let mut archive = Archive::new(gzr);