From 00806b02e1e5f31268b58ac647ae4b476d5579fd Mon Sep 17 00:00:00 2001 From: Gerald Pinder Date: Fri, 9 May 2025 15:53:59 -0400 Subject: [PATCH] chore: Simplify opts using new ImageRef type --- process/drivers/buildah_driver.rs | 4 +- process/drivers/docker_driver.rs | 28 +++++----- process/drivers/opts/build.rs | 15 ++---- process/drivers/podman_driver.rs | 4 +- process/drivers/traits.rs | 88 +++++++++++++++--------------- process/drivers/types.rs | 90 ++++++++++++++++++++++++++++++- src/commands/build.rs | 2 +- 7 files changed, 153 insertions(+), 78 deletions(-) diff --git a/process/drivers/buildah_driver.rs b/process/drivers/buildah_driver.rs index 319eb4b..5ff11bc 100644 --- a/process/drivers/buildah_driver.rs +++ b/process/drivers/buildah_driver.rs @@ -84,12 +84,12 @@ impl BuildDriver for BuildahDriver { "-f", &*opts.containerfile, "-t", - &*opts.image, + opts.image.to_string(), ); trace!("{command:?}"); let status = command - .build_status(&opts.image, "Building Image") + .build_status(opts.image.to_string(), "Building Image") .into_diagnostic()?; if status.success() { diff --git a/process/drivers/docker_driver.rs b/process/drivers/docker_driver.rs index 6aea40b..aec3a62 100644 --- a/process/drivers/docker_driver.rs +++ b/process/drivers/docker_driver.rs @@ -29,7 +29,7 @@ use crate::{ RunOptsVolume, TagOpts, }, traits::{BuildDriver, DriverVersion, InspectDriver, RunDriver}, - types::{ContainerId, ImageMetadata, Platform}, + types::{ContainerId, ImageMetadata, ImageRef, Platform}, }, logging::CommandLogging, signal_handler::{ContainerRuntime, ContainerSignalId, add_cid, remove_cid}, @@ -207,7 +207,7 @@ impl BuildDriver for DockerDriver { opts.platform.to_string(), ], "-t", - &*opts.image, + opts.image.to_string(), "-f", if let Some(cache_from) = opts.cache_from.as_ref() => [ "--cache-from", @@ -388,7 +388,7 @@ impl BuildDriver for DockerDriver { Self::setup()?; - let final_images = get_final_images(opts)?; + let final_images = get_final_images(opts); let first_image = final_images.first().unwrap(); @@ -418,22 +418,22 @@ fn build_tag_push_cmd(opts: &BuildTagPushOpts<'_>, first_image: &str) -> Command format!("--builder={BLUE_BUILD}"), "build", ".", - match (opts.image, opts.archive_path.as_deref()) { - (Some(_), None) if opts.push => [ + match &opts.image { + ImageRef::Remote(_remote) if opts.push => [ "--output", format!( "type=image,name={first_image},push=true,compression={},oci-mediatypes=true", opts.compression ), ], - (Some(_), None) if env::var(GITHUB_ACTIONS).is_err() => "--load", - (None, Some(archive_path)) => [ + ImageRef::Remote(_remote) if env::var(GITHUB_ACTIONS).is_err() => "--load", + ImageRef::LocalTar(archive_path) => [ "--output", format!("type=oci,dest={}", archive_path.display()), ], _ => [], }, - for opts.image.as_ref().map_or_else(Vec::new, |image| { + for opts.image.remote_ref().map_or_else(Vec::new, |image| { opts.tags.iter().flat_map(|tag| { vec![ "-t".to_string(), @@ -465,9 +465,9 @@ fn build_tag_push_cmd(opts: &BuildTagPushOpts<'_>, first_image: &str) -> Command c } -fn get_final_images(opts: &BuildTagPushOpts<'_>) -> Result> { - Ok(match (opts.image, opts.archive_path.as_deref()) { - (Some(image), None) => { +fn get_final_images(opts: &BuildTagPushOpts<'_>) -> Vec { + match &opts.image { + ImageRef::Remote(image) => { let images = if opts.tags.is_empty() { let image = image.to_string(); string_vec![image] @@ -480,12 +480,10 @@ fn get_final_images(opts: &BuildTagPushOpts<'_>) -> Result> { images } - (None, Some(archive_path)) => { + ImageRef::LocalTar(archive_path) => { string_vec![archive_path.display().to_string()] } - (Some(_), Some(_)) => bail!("Cannot use both image and archive path"), - (None, None) => bail!("Need either the image or archive path set"), - }) + } } impl InspectDriver for DockerDriver { diff --git a/process/drivers/opts/build.rs b/process/drivers/opts/build.rs index b789655..e668386 100644 --- a/process/drivers/opts/build.rs +++ b/process/drivers/opts/build.rs @@ -3,7 +3,7 @@ use std::{borrow::Cow, path::Path}; use bon::Builder; use oci_distribution::Reference; -use crate::drivers::types::Platform; +use crate::drivers::types::{ImageRef, Platform}; use super::CompressionType; @@ -11,7 +11,7 @@ use super::CompressionType; #[derive(Debug, Clone, Builder)] pub struct BuildOpts<'scope> { #[builder(into)] - pub image: Cow<'scope, str>, + pub image: ImageRef<'scope>, #[builder(default)] pub squash: bool, @@ -65,17 +65,8 @@ pub struct PruneOpts { #[derive(Debug, Clone, Builder)] pub struct BuildTagPushOpts<'scope> { /// The base image name. - /// - /// NOTE: This SHOULD NOT contain the tag of the image. - /// - /// NOTE: You cannot have this set with `archive_path` set. - pub image: Option<&'scope Reference>, - - /// The path to the archive file. - /// - /// NOTE: You cannot have this set with image set. #[builder(into)] - pub archive_path: Option>, + pub image: ImageRef<'scope>, /// The path to the Containerfile to build. #[builder(into)] diff --git a/process/drivers/podman_driver.rs b/process/drivers/podman_driver.rs index b7fec79..ffb5942 100644 --- a/process/drivers/podman_driver.rs +++ b/process/drivers/podman_driver.rs @@ -179,13 +179,13 @@ impl BuildDriver for PodmanDriver { "-f", &*opts.containerfile, "-t", - &*opts.image, + opts.image.to_string(), ".", ); trace!("{command:?}"); let status = command - .build_status(&opts.image, "Building Image") + .build_status(opts.image.to_string(), "Building Image") .into_diagnostic()?; if status.success() { diff --git a/process/drivers/traits.rs b/process/drivers/traits.rs index d142a99..eeb01d4 100644 --- a/process/drivers/traits.rs +++ b/process/drivers/traits.rs @@ -10,7 +10,11 @@ use miette::{Context, IntoDiagnostic, Result, bail}; use oci_distribution::Reference; use semver::VersionReq; -use crate::drivers::{Driver, functions::get_private_key, types::CiDriverType}; +use crate::drivers::{ + Driver, + functions::get_private_key, + types::{CiDriverType, ImageRef}, +}; #[cfg(feature = "sigstore")] use super::sigstore_driver::SigstoreDriver; @@ -122,17 +126,8 @@ pub trait BuildDriver: PrivateDriver { fn build_tag_push(opts: &BuildTagPushOpts) -> Result> { trace!("BuildDriver::build_tag_push({opts:#?})"); - let full_image = match (opts.archive_path.as_ref(), opts.image.as_ref()) { - (Some(archive_path), None) => { - format!("oci-archive:{}", archive_path.display()) - } - (None, Some(image)) => image.to_string(), - (Some(_), Some(_)) => bail!("Cannot use both image and archive path"), - (None, None) => bail!("Need either the image or archive path set"), - }; - let build_opts = BuildOpts::builder() - .image(&full_image) + .image(&opts.image) .containerfile(opts.containerfile.as_ref()) .platform(opts.platform) .squash(opts.squash) @@ -140,51 +135,54 @@ pub trait BuildDriver: PrivateDriver { .maybe_cache_to(opts.cache_to) .build(); - info!("Building image {full_image}"); + info!("Building image {}", opts.image); Self::build(&build_opts)?; - let image_list: Vec = if !opts.tags.is_empty() && opts.archive_path.is_none() { - let image = opts.image.unwrap(); - debug!("Tagging all images"); + let image_list: Vec = match &opts.image { + ImageRef::Remote(image) if !opts.tags.is_empty() => { + debug!("Tagging all images"); - let mut image_list = Vec::with_capacity(opts.tags.len()); + let mut image_list = Vec::with_capacity(opts.tags.len()); - for tag in &opts.tags { - debug!("Tagging {} with {tag}", &full_image); - let tagged_image: Reference = - format!("{}/{}:{tag}", image.resolve_registry(), image.repository()) - .parse() - .into_diagnostic()?; + for tag in &opts.tags { + debug!("Tagging {} with {tag}", &image); + let tagged_image = Reference::with_tag( + image.registry().into(), + image.repository().into(), + tag.to_string(), + ); - let tag_opts = TagOpts::builder() - .src_image(image) - .dest_image(&tagged_image) - .build(); + let tag_opts = TagOpts::builder() + .src_image(image.as_ref()) + .dest_image(&tagged_image) + .build(); - Self::tag(&tag_opts)?; - image_list.push(tagged_image.to_string()); + Self::tag(&tag_opts)?; + image_list.push(tagged_image.to_string()); - if opts.push { - let retry_count = if opts.retry_push { opts.retry_count } else { 0 }; + if opts.push { + let retry_count = if opts.retry_push { opts.retry_count } else { 0 }; - debug!("Pushing all images"); - // Push images with retries (1s delay between retries) - blue_build_utils::retry(retry_count, 5, || { - debug!("Pushing image {tagged_image}"); + debug!("Pushing all images"); + // Push images with retries (1s delay between retries) + blue_build_utils::retry(retry_count, 5, || { + debug!("Pushing image {tagged_image}"); - let push_opts = PushOpts::builder() - .image(&tagged_image) - .compression_type(opts.compression) - .build(); + let push_opts = PushOpts::builder() + .image(&tagged_image) + .compression_type(opts.compression) + .build(); - Self::push(&push_opts) - })?; + Self::push(&push_opts) + })?; + } } - } - image_list - } else { - string_vec![&full_image] + image_list + } + _ => { + string_vec![&opts.image] + } }; Ok(image_list) @@ -293,7 +291,7 @@ pub trait RechunkDriver: RunDriver + BuildDriver + ContainerMountDriver { Self::build( &BuildOpts::builder() - .image(raw_image.to_string()) + .image(raw_image) .containerfile(&*opts.containerfile) .platform(opts.platform) .privileged(true) diff --git a/process/drivers/types.rs b/process/drivers/types.rs index 0e7cbc6..b5a9207 100644 --- a/process/drivers/types.rs +++ b/process/drivers/types.rs @@ -1,4 +1,9 @@ -use std::{collections::HashMap, env}; +use std::{ + borrow::Cow, + collections::HashMap, + env, + path::{Path, PathBuf}, +}; use blue_build_utils::{ constants::{GITHUB_ACTIONS, GITLAB_CI, IMAGE_VERSION_LABEL}, @@ -6,6 +11,7 @@ use blue_build_utils::{ }; use clap::ValueEnum; use log::trace; +use oci_distribution::Reference; use serde::Deserialize; use serde_json::Value; @@ -313,3 +319,85 @@ impl TryFrom for OciDir { Ok(Self(format!("oci:{}", value.display()))) } } + +/// An image ref that could reference +/// a remote registry or a local tarball. +#[derive(Debug, Clone)] +pub enum ImageRef<'scope> { + Remote(Cow<'scope, Reference>), + LocalTar(Cow<'scope, Path>), +} + +impl ImageRef<'_> { + #[must_use] + pub fn remote_ref(&self) -> Option<&Reference> { + match self { + Self::Remote(remote) => Some(remote.as_ref()), + Self::LocalTar(_) => None, + } + } +} + +impl<'scope> From<&'scope Self> for ImageRef<'scope> { + fn from(value: &'scope ImageRef) -> Self { + match value { + Self::Remote(remote) => Self::Remote(Cow::Borrowed(remote.as_ref())), + Self::LocalTar(path) => Self::LocalTar(Cow::Borrowed(path.as_ref())), + } + } +} + +impl<'scope> From<&'scope Reference> for ImageRef<'scope> { + fn from(value: &'scope Reference) -> Self { + Self::Remote(Cow::Borrowed(value)) + } +} + +impl From for ImageRef<'_> { + fn from(value: Reference) -> Self { + Self::Remote(Cow::Owned(value)) + } +} + +impl<'scope> From<&'scope Path> for ImageRef<'scope> { + fn from(value: &'scope Path) -> Self { + Self::LocalTar(Cow::Borrowed(value)) + } +} + +impl<'scope> From<&'scope PathBuf> for ImageRef<'scope> { + fn from(value: &'scope PathBuf) -> Self { + Self::from(value.as_path()) + } +} + +impl From for ImageRef<'_> { + fn from(value: PathBuf) -> Self { + Self::LocalTar(Cow::Owned(value)) + } +} + +impl From> for String { + fn from(value: ImageRef<'_>) -> Self { + Self::from(&value) + } +} + +impl From<&ImageRef<'_>> for String { + fn from(value: &ImageRef<'_>) -> Self { + format!("{value}") + } +} + +impl std::fmt::Display for ImageRef<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + match self { + Self::Remote(remote) => remote.whole(), + Self::LocalTar(path) => format!("oci-archive:{}", path.display()), + } + ) + } +} diff --git a/src/commands/build.rs b/src/commands/build.rs index 90a98d1..e8f3e26 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -339,7 +339,7 @@ impl BuildCommand { BuildTagPushOpts::builder() .containerfile(containerfile) .platform(self.platform) - .archive_path(PathBuf::from(format!( + .image(PathBuf::from(format!( "{}/{}.{ARCHIVE_SUFFIX}", archive_dir.to_string_lossy().trim_end_matches('/'), recipe.name.to_lowercase().replace('/', "_"),