use path defaulting code (#39)

Metaswitch · Aug 15, 2023 · 7a50168 · 7a50168
1 parent a95744f
commit 7a50168
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 80 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 ### Added
 ### Fixed
 
+## 0.2.8 - 2023-08-15
+
+### Fixed
+Set a sensible default for the PATH variable correctly.
 
 ## 0.2.7 - 2023-08-15
 

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "rpmoci"
-version = "0.2.7"
+version = "0.2.8"
 edition = "2021"
 description = "Build container images from RPMs"
 # rpmoci uses DNF (via pyo3) which is GPLV2+ licensed,

diff --git a/src/config.rs b/src/config.rs
@@ -16,7 +16,7 @@
 //! along with this program.  If not, see <https://www.gnu.org/licenses/>.
 use anyhow::Result;
 use oci_spec::{
-    image::{Arch, ConfigBuilder, ImageConfigurationBuilder, Os},
+    image::{Arch, ConfigBuilder, ImageConfiguration, ImageConfigurationBuilder, Os},
     OciSpecError,
 };
 use serde::{Deserialize, Serialize};
@@ -130,8 +130,11 @@ impl Repository {
     }
 }
 
-impl TryFrom<&Config> for oci_spec::image::ImageConfiguration {
-    fn try_from(cfg: &Config) -> Result<Self, Self::Error> {
+impl ImageConfig {
+    pub(crate) fn to_oci_image_configuration(
+        &self,
+        cli_labels: HashMap<String, String>,
+    ) -> Result<ImageConfiguration, OciSpecError> {
         let ImageConfig {
             user,
             exposed_ports,
@@ -144,8 +147,10 @@ impl TryFrom<&Config> for oci_spec::image::ImageConfiguration {
             stopsignal,
             author,
             ..
-        } = &cfg.image;
+        } = &self;
         let mut builder = ConfigBuilder::default();
+        let mut merged_labels = labels.clone();
+        merged_labels.extend(cli_labels);
 
         // default the PATH variable to /usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin
         let mut envs = envs.clone();
@@ -184,12 +189,14 @@ impl TryFrom<&Config> for oci_spec::image::ImageConfiguration {
         }
         builder.build()
     }
-
-    type Error = OciSpecError;
 }
 
 #[cfg(test)]
 mod tests {
+    use std::collections::HashMap;
+
+    use crate::config::ImageConfig;
+
     use super::Config;
 
     #[test]
@@ -266,29 +273,25 @@ mod tests {
 
     #[test]
     fn path_env_defaulting() {
-        let config_with_path = r#"[contents]
-        packages = ["foo"]
-        repositories = ["https://packages.microsoft.com/cbl-mariner/2.0/prod/base/x86_64"]
-        [image]
+        let config_with_path = r#"
         envs = { PATH = "/usr/bin"}
         "#;
         let config: oci_spec::image::ImageConfiguration =
-            (&toml::from_str::<Config>(config_with_path).unwrap())
-                .try_into()
+            toml::from_str::<ImageConfig>(config_with_path)
+                .unwrap()
+                .to_oci_image_configuration(HashMap::new())
                 .unwrap();
         let envs = config.config().as_ref().unwrap().env().as_ref().unwrap();
         assert!(envs.iter().any(|e| e == "PATH=/usr/bin"));
         assert_eq!(envs.len(), 1);
 
-        let config_without_path = r#"[contents]
-        packages = ["foo"]
-        repositories = ["https://packages.microsoft.com/cbl-mariner/2.0/prod/base/x86_64"]
-        [image]
+        let config_without_path = r#"
         envs = { FOO = "bar"}
         "#;
         let config: oci_spec::image::ImageConfiguration =
-            (&toml::from_str::<Config>(config_without_path).unwrap())
-                .try_into()
+            toml::from_str::<ImageConfig>(config_without_path)
+                .unwrap()
+                .to_oci_image_configuration(HashMap::new())
                 .unwrap();
         let envs = config.config().as_ref().unwrap().env().as_ref().unwrap();
         assert!(envs

diff --git a/src/lockfile/build.rs b/src/lockfile/build.rs
@@ -19,15 +19,10 @@ use std::{fs, path::PathBuf, process::Command};
 
 use anyhow::{bail, Context, Result};
 use glob::glob;
-use oci_spec::image::{
-    Arch, ConfigBuilder, ImageConfiguration, ImageConfigurationBuilder, ImageIndex,
-    ImageManifestBuilder, MediaType, Os, RootFsBuilder,
-};
-use oci_spec::OciSpecError;
+use oci_spec::image::{ImageIndex, ImageManifestBuilder, MediaType, RootFsBuilder};
 use tempfile::TempDir;
 
 use super::Lockfile;
-use crate::config::ImageConfig;
 use crate::write;
 use crate::{config::Config, oci};
 
@@ -126,7 +121,7 @@ impl Lockfile {
 
         // Create the image configuration blob
         write::ok("Writing", "image configuration blob")?;
-        let mut image_config = build_image_configuration(cfg, labels)?;
+        let mut image_config = cfg.image.to_oci_image_configuration(labels)?;
         let rootfs = RootFsBuilder::default().diff_ids(vec![diff_id]).build()?;
         image_config.set_rootfs(rootfs);
         let config = oci::write_json_blob(&image_config, MediaType::ImageConfig, image)?;
@@ -187,56 +182,3 @@ impl Lockfile {
         Ok(())
     }
 }
-
-fn build_image_configuration(
-    cfg: &Config,
-    cli_labels: HashMap<String, String>,
-) -> Result<ImageConfiguration, OciSpecError> {
-    let ImageConfig {
-        user,
-        exposed_ports,
-        envs,
-        entrypoint,
-        cmd,
-        volumes,
-        labels,
-        workingdir,
-        stopsignal,
-        author,
-        ..
-    } = &cfg.image;
-    let mut builder = ConfigBuilder::default();
-    let mut merged_labels = labels.clone();
-    merged_labels.extend(cli_labels);
-    builder = builder
-        .cmd(cmd.clone())
-        .volumes(volumes.clone())
-        .entrypoint(entrypoint.clone())
-        .env(
-            envs.iter()
-                .map(|(k, v)| format!("{}={}", k, v))
-                .collect::<Vec<_>>(),
-        )
-        .exposed_ports(exposed_ports.clone())
-        .labels(merged_labels);
-    if let Some(user) = user {
-        builder = builder.user(user);
-    }
-    if let Some(stopsignal) = stopsignal {
-        builder = builder.stop_signal(stopsignal);
-    }
-    if let Some(workingdir) = workingdir {
-        builder = builder.working_dir(workingdir);
-    }
-    let config = builder.build()?;
-
-    let mut builder = ImageConfigurationBuilder::default()
-        .config(config)
-        .architecture(Arch::Amd64)
-        .os(Os::Linux)
-        .created(chrono::Utc::now().to_rfc3339());
-    if let Some(author) = author {
-        builder = builder.author(author);
-    }
-    builder.build()
-}