feat(boil): Support for per-image build arguments

rust/boil/README.md

@@ -63,6 +63,10 @@ images, and registry metadata.
 [metadata.registries]
 "oci.example.org" = { namespace = "my/namespace" } # Used for image checks
 
+[build-arguments]
+BAR = "baz"                                        # Specify build arguments which apply to all versions.
+                                                   # Can be overwritten on a per-version basis
+
 [versions."1.2.3".local-images]                    # Specify references to local images per version
 foo = "1.2.3"
 bar = "4.5.6"

rust/boil/src/config.rs

@@ -16,6 +16,8 @@ pub enum ConfigError {
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "kebab-case")]
 pub struct Config {
+    /// Global build arguments which apply to all images.
+    #[serde(default, deserialize_with = "docker::deserialize_args")]
     pub build_arguments: docker::BuildArguments,
     pub metadata: MetadataOptions,
 }

rust/boil/src/core/bakefile.rs

@@ -311,10 +311,12 @@ impl Bakefile {
         let date_time = Self::now()?;
 
         // Load build arguments from a file if the user requested it
-        let mut user_container_build_args = cli_args.build_arguments.clone();
+        let mut user_container_build_args: docker::BuildArguments =
+            docker::build_args_vec_to_btree_map(cli_args.build_arguments.clone());
+
         if let Some(path) = &cli_args.build_arguments_file {
             let build_arguments_from_file =
-                docker::BuildArguments::from_file(path).context(ParseBuildArgumentsSnafu)?;
+                docker::build_args_from_file(path).context(ParseBuildArgumentsSnafu)?;
             user_container_build_args.extend(build_arguments_from_file);
         }
 
@@ -385,42 +387,27 @@ impl Bakefile {
 
                 // TODO (@Techassi): Clean this up
                 // TODO (@Techassi): Move the arg formatting into functions
-                let mut build_arguments = docker::BuildArguments::new();
+                // Start off with the shared (across all versions of the same image) build arguments.
+                let mut build_arguments = image_config.build_arguments.clone();
 
-                let local_version_docker_args: Vec<_> = image_options
+                let local_version_docker_args: docker::BuildArguments = image_options
                     .local_images
                     .iter()
                     .map(|(image_name, image_version)| {
-                        docker::BuildArgument::local_image_version(
-                            image_name.to_string(),
-                            image_version.to_string(),
-                        )
+                        let key = docker::BuildArgumentKey::local_image_key(image_name);
+                        (key, image_version.to_owned())
                     })
                     .collect();
 
                 build_arguments.extend(image_options.build_arguments);
                 build_arguments.extend(local_version_docker_args);
+
                 // TODO (@Techassi): Rename this to IMAGE_VERSION
-                build_arguments.insert(docker::BuildArgument::new(
-                    "PRODUCT_VERSION".to_owned(),
-                    image_version.to_string(),
-                ));
-                build_arguments.insert(docker::BuildArgument::new(
-                    "IMAGE_REPOSITORY_URI".to_owned(),
-                    image_repository_uri.clone(),
-                ));
-                build_arguments.insert(docker::BuildArgument::new(
-                    "IMAGE_INDEX_MANIFEST_TAG".to_owned(),
-                    image_index_manifest_tag,
-                ));
-                build_arguments.insert(docker::BuildArgument::new(
-                    "IMAGE_MANIFEST_TAG".to_owned(),
-                    image_manifest_tag,
-                ));
-                build_arguments.insert(docker::BuildArgument::new(
-                    "IMAGE_MANIFEST_URI".to_owned(),
-                    image_manifest_uri.clone(),
-                ));
+                build_arguments.insert("PRODUCT_VERSION".into(), image_version.to_string());
+                build_arguments.insert("IMAGE_REPOSITORY_URI".into(), image_repository_uri.clone());
+                build_arguments.insert("IMAGE_INDEX_MANIFEST_TAG".into(), image_index_manifest_tag);
+                build_arguments.insert("IMAGE_MANIFEST_TAG".into(), image_manifest_tag);
+                build_arguments.insert("IMAGE_MANIFEST_URI".into(), image_manifest_uri.clone());
 
                 let tags = if let Some(floating_vendor_version) = floating_vendor_version.as_deref()
                 {
@@ -641,7 +628,7 @@ impl BakefileTarget {
         revision: String,
         release_version: String,
         container_build_args: docker::BuildArguments,
-        user_container_build_args: Vec<docker::BuildArgument>,
+        user_container_build_args: docker::BuildArguments,
         metadata: &MetadataOptions,
     ) -> Self {
         let config::MetadataOptions {
@@ -683,10 +670,7 @@ impl BakefileTarget {
 
         let mut arguments = container_build_args;
         arguments.extend(user_container_build_args);
-        arguments.insert(docker::BuildArgument::new(
-            "RELEASE_VERSION".to_owned(),
-            release_version,
-        ));
+        arguments.insert("RELEASE_VERSION".into(), release_version);
 
         // Labels describe Docker resources, and con be considered legacy. We
         // should use annotations instead. These labels are only added to be

rust/boil/src/core/docker.rs

@@ -1,12 +1,10 @@
 use std::{
-    collections::BTreeSet,
-    fmt::Display,
-    ops::{Deref, DerefMut},
+    collections::BTreeMap,
     path::{Path, PathBuf},
     str::FromStr,
 };
 
-use serde::{Deserialize, Serialize, de::Visitor, ser::SerializeMap};
+use serde::{Deserialize, Serialize, de::Visitor};
 use snafu::{OptionExt, ResultExt, Snafu, ensure};
 
 #[derive(Debug, Snafu)]
@@ -18,22 +16,22 @@ pub enum ParseBuildArgumentError {
     NonAscii,
 }
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
-pub struct BuildArgument((String, String));
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
+pub struct BuildArgumentKey(String);
 
-impl BuildArgument {
-    pub fn new(key: String, value: String) -> Self {
-        let key = Self::format_key(key);
-        Self((key, value))
-    }
-
-    pub fn local_image_version(image_name: String, image_version: String) -> Self {
-        Self::new(format!("{image_name}_VERSION"), image_version)
+impl<T> From<T> for BuildArgumentKey
+where
+    T: Into<String>,
+{
+    fn from(value: T) -> Self {
+        Self(value.into())
     }
+}
 
-    fn format_key(key: impl AsRef<str>) -> String {
-        key.as_ref().replace(['-', '/'], "_").to_uppercase()
-    }
+#[derive(Clone, Debug)]
+pub struct BuildArgument {
+    pub key: BuildArgumentKey,
+    pub value: String,
 }
 
 impl FromStr for BuildArgument {
@@ -43,42 +41,22 @@ impl FromStr for BuildArgument {
         ensure!(s.is_ascii(), NonAsciiSnafu);
 
         let (key, value) = s.split_once('=').context(InvalidFormatSnafu)?;
-        let key = Self::format_key(key);
+        let key = BuildArgumentKey::new(key);
 
-        Ok(Self((key, value.to_owned())))
+        Ok(Self {
+            key,
+            value: value.to_owned(),
+        })
     }
 }
 
-impl<'de> Deserialize<'de> for BuildArgument {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        struct BuildArgumentVisitor;
-
-        impl Visitor<'_> for BuildArgumentVisitor {
-            type Value = BuildArgument;
-
-            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-                write!(formatter, "a valid build argument")
-            }
-
-            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                BuildArgument::from_str(v).map_err(serde::de::Error::custom)
-            }
-        }
-
-        deserializer.deserialize_str(BuildArgumentVisitor)
+impl BuildArgumentKey {
+    pub fn new(name: impl AsRef<str>) -> Self {
+        Self(name.as_ref().replace(['-', '/'], "_").to_uppercase())
     }
-}
 
-impl Display for BuildArgument {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let (key, value) = &self.0;
-        write!(f, "{key}={value}")
+    pub fn local_image_key(image_name: &str) -> Self {
+        Self::new(format!("{image_name}_VERSION"))
     }
 }
 
@@ -94,107 +72,57 @@ pub enum ParseBuildArgumentsError {
     ParseBuildArgument { source: ParseBuildArgumentError },
 }
 
-#[derive(Clone, Debug, Default)]
-pub struct BuildArguments(BTreeSet<BuildArgument>);
-
-impl Deref for BuildArguments {
-    type Target = BTreeSet<BuildArgument>;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
+pub type BuildArguments = BTreeMap<BuildArgumentKey, String>;
 
-impl DerefMut for BuildArguments {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.0
-    }
-}
+/// Custom [`serde::Deserialize`] implementation to ensure we properly format the keys.
+pub fn deserialize_args<'de, D>(deserializer: D) -> Result<BuildArguments, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    struct BuildArgumentsVisitor;
 
-impl Extend<BuildArgument> for BuildArguments {
-    fn extend<T: IntoIterator<Item = BuildArgument>>(&mut self, iter: T) {
-        self.0.extend(iter);
-    }
-}
+    impl<'de> Visitor<'de> for BuildArgumentsVisitor {
+        type Value = BuildArguments;
 
-impl IntoIterator for BuildArguments {
-    type IntoIter = std::collections::btree_set::IntoIter<Self::Item>;
-    type Item = BuildArgument;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.0.into_iter()
-    }
-}
-
-impl<'de> Deserialize<'de> for BuildArguments {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        struct BuildArgumentsVisitor;
+        fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
+            write!(formatter, "a map of valid build arguments")
+        }
 
-        impl<'de> Visitor<'de> for BuildArgumentsVisitor {
-            type Value = BuildArguments;
+        fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
+        where
+            A: serde::de::MapAccess<'de>,
+        {
+            let mut args = BTreeMap::new();
 
-            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-                write!(formatter, "a map of valid build arguments")
+            while let Some((key, value)) = map.next_entry::<&str, _>()? {
+                args.insert(BuildArgumentKey::new(key), value);
             }
 
-            fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
-            where
-                A: serde::de::MapAccess<'de>,
-            {
-                let mut args = BTreeSet::new();
-
-                while let Some((key, value)) = map.next_entry()? {
-                    args.insert(BuildArgument::new(key, value));
-                }
-
-                Ok(BuildArguments(args))
-            }
+            Ok(args)
         }
-
-        deserializer.deserialize_map(BuildArgumentsVisitor)
     }
-}
 
-impl Serialize for BuildArguments {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        let mut map = serializer.serialize_map(Some(self.len()))?;
-
-        for BuildArgument((key, value)) in &self.0 {
-            map.serialize_entry(&key, &value)?;
-        }
-
-        map.end()
-    }
+    deserializer.deserialize_map(BuildArgumentsVisitor)
 }
 
-impl BuildArguments {
-    pub fn new() -> Self {
-        Self(BTreeSet::new())
-    }
+// We sadly cannot use a From<Vec<BuildArgument>> for BuildArguments impl here because of the
+// orphan rule.
+pub fn build_args_vec_to_btree_map(vec: Vec<BuildArgument>) -> BuildArguments {
+    vec.into_iter().map(|arg| (arg.key, arg.value)).collect()
+}
 
-    pub fn is_empty(&self) -> bool {
-        self.0.is_empty()
+pub fn build_args_from_file<P>(path: P) -> Result<BuildArguments, ParseBuildArgumentsError>
+where
+    P: AsRef<Path>,
+{
+    let path = path.as_ref();
+    let content = std::fs::read_to_string(path).context(ReadFileSnafu { path })?;
+    let mut args = BTreeMap::new();
+
+    for line in content.lines() {
+        let arg = BuildArgument::from_str(line).context(ParseBuildArgumentSnafu)?;
+        args.insert(arg.key, arg.value);
     }
 
-    pub fn from_file<P>(path: P) -> Result<Self, ParseBuildArgumentsError>
-    where
-        P: AsRef<Path>,
-    {
-        let path = path.as_ref();
-        let content = std::fs::read_to_string(path).context(ReadFileSnafu { path })?;
-        let mut args = Self::new();
-
-        for line in content.lines() {
-            let arg = BuildArgument::from_str(line).context(ParseBuildArgumentSnafu)?;
-            args.insert(arg);
-        }
-
-        Ok(args)
-    }
+    Ok(args)
 }

rust/boil/src/core/image.rs

@@ -9,7 +9,7 @@ use std::{
 use serde::Deserialize;
 use snafu::{ResultExt as _, Snafu, ensure};
 
-use crate::core::docker;
+use crate::core::docker::{self, BuildArguments};
 
 #[derive(Debug, PartialEq, Snafu)]
 pub enum ParseImageSelectorError {
@@ -111,10 +111,15 @@ pub enum ImageConfigError {
 }
 
 #[derive(Debug, Deserialize)]
+#[serde(rename_all = "kebab-case")]
 pub struct ImageConfig {
     #[serde(default)]
     pub metadata: ImageMetadata,
 
+    /// Shared build arguments across all versions, but can be overwritten on a image version level.
+    #[serde(default, deserialize_with = "docker::deserialize_args")]
+    pub build_arguments: BuildArguments,
+
     pub versions: ImageVersions,
 }
 
@@ -179,12 +184,12 @@ pub struct ImageVersionOptions {
 
     // NOTE (@Techassi): Potentially add a dependencies field here which will be automatically be
     // suffixed with _VERSION.
-    #[serde(default)]
+    #[serde(default, deserialize_with = "docker::deserialize_args")]
     pub build_arguments: docker::BuildArguments,
 
     /// A custom path to a Dockerfile/Containerfile for a particular version of an image.
     ///
-    /// This is usefull for cases where the same image is being built differently depending on it's
+    /// This is useful for cases where the same image is being built differently depending on it's
     /// version and it is too difficult/messy to do it the same Dockerfile/Containerfile.
     #[serde(alias = "containerfile")]
     pub dockerfile: Option<PathBuf>,