WIP: treefile: allow ${releasever} in more keys

Fixes #1809 Signed-off-by: Rafael Fonseca <[email protected]>
coreos · Jun 6, 2019 · f470738 · f470738
1 parent 68c416f
commit f470738
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 36 deletions.
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
@@ -23,6 +23,7 @@ c_utf8 = "0.1.0"
 systemd = "0.4.0"
 indicatif = "0.11.0"
 lazy_static = "1.1.0"
+envsubst = "0.1.0"
 
 [lib]
 name = "rpmostree_rust"

diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs
@@ -21,7 +21,7 @@
  * */
 
 use c_utf8::CUtf8Buf;
-use failure::Fallible;
+use failure::{bail, Fallible};
 use failure::ResultExt;
 use openat;
 use serde_derive::{Deserialize, Serialize};
@@ -32,8 +32,6 @@ use std::io::prelude::*;
 use std::path::Path;
 use std::{collections, fs, io};
 
-use crate::utils;
-
 const INCLUDE_MAXDEPTH: u32 = 50;
 
 /// This struct holds file descriptors for any external files/data referenced by
@@ -77,6 +75,7 @@ fn treefile_parse_stream<R: io::Read>(
     fmt: InputFormat,
     input: &mut R,
     basearch: Option<&str>,
+    releasever: Option<&str>,
 ) -> Fallible<TreeComposeConfig> {
     let mut treefile: TreeComposeConfig = match fmt {
         InputFormat::YAML => {
@@ -99,6 +98,8 @@ fn treefile_parse_stream<R: io::Read>(
         }
     };
 
+    let mut substvars: collections::HashMap<String, String> = collections::HashMap::new();
+
     treefile.basearch = match (treefile.basearch, basearch) {
         (Some(treearch), Some(arch)) => {
             if treearch != arch {
@@ -118,6 +119,25 @@ fn treefile_parse_stream<R: io::Read>(
         // really, only for tests do we not specify a basearch. let's just canonicalize to None
         (_, None) => None,
     };
+    if let Some(arch) = &treefile.basearch {
+        substvars.insert("basearch".to_string(), arch.clone());
+    }
+
+    treefile.releasever = match (treefile.releasever, releasever) {
+        (Some(treerelease), Some(release)) => {
+            if treerelease != release {
+                bail!("Invalid releasever \"{}\"", treerelease);
+            } else {
+                Some(treerelease)
+            }
+        }
+        (None, Some(release)) => Some(release.into()),
+        // really, only for tests do we not specify a releasever. let's just canonicalize to None
+        (_, None) => None,
+    };
+    if let Some(releasever) = &treefile.releasever {
+        substvars.insert("releasever".to_string(), releasever.clone());
+    }
 
     // remove from packages-${arch} keys from the extra keys
     let mut archful_pkgs: Option<Vec<String>> = take_archful_pkgs(basearch, &mut treefile)?;
@@ -131,18 +151,31 @@ fn treefile_parse_stream<R: io::Read>(
         .into());
     }
 
-    // Substitute ${basearch}
-    treefile.treeref = match (basearch, treefile.treeref.take()) {
-        (Some(basearch), Some(treeref)) => {
-            let mut varsubsts = HashMap::new();
-            varsubsts.insert("basearch".to_string(), basearch.to_string());
-            Some(
-                utils::varsubst(&treeref, &varsubsts)
-                    .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e.to_string()))?,
-            )
-        }
-        (_, v) => v,
+    // Substitute ${basearch} and ${releasever}
+    macro_rules! substitute_vars {
+        ( $($field:ident),* ) => {{
+            $(
+                assert!(envsubst::validate_vars(&substvars).is_ok());
+                //let value = treefile.$field.take().unwrap();
+                if let Some(value) = treefile.$field.take() {
+                    treefile.$field = if envsubst::is_templated(&value) {
+                        match envsubst::substitute(value.clone(), &substvars) {
+                            Ok(s) => Some(s),
+                            Err(_) => Some(value), // leave field untouched
+                        }
+                        //Some(envsubst::substitute(value, &substvars)?)
+                    } else {
+                        Some(value)
+                    }
+                }
+            )*
+        }};
     };
+    substitute_vars!(
+        treeref,
+        automatic_version_prefix,
+        mutate_os_release
+    );
 
     // Special handling for packages, since we allow whitespace within items.
     // We also canonicalize bootstrap_packages to packages here so it's
@@ -236,6 +269,7 @@ fn load_passwd_file<P: AsRef<Path>>(
 fn treefile_parse<P: AsRef<Path>>(
     filename: P,
     basearch: Option<&str>,
+    releasever: Option<&str>,
 ) -> Fallible<ConfigAndExternals> {
     let filename = filename.as_ref();
     let mut f = io::BufReader::new(open_file(filename)?);
@@ -248,7 +282,7 @@ fn treefile_parse<P: AsRef<Path>>(
     } else {
         InputFormat::JSON
     };
-    let tf = treefile_parse_stream(fmt, &mut f, basearch).map_err(|e| {
+    let tf = treefile_parse_stream(fmt, &mut f, basearch, releasever).map_err(|e| {
         io::Error::new(
             io::ErrorKind::InvalidInput,
             format!("Parsing {}: {}", filename.to_string_lossy(), e.to_string()),
@@ -374,10 +408,11 @@ fn treefile_merge_externals(dest: &mut TreefileExternals, src: &mut TreefileExte
 fn treefile_parse_recurse<P: AsRef<Path>>(
     filename: P,
     basearch: Option<&str>,
+    releasever: Option<&str>,
     depth: u32,
 ) -> Fallible<ConfigAndExternals> {
     let filename = filename.as_ref();
-    let mut parsed = treefile_parse(filename, basearch)?;
+    let mut parsed = treefile_parse(filename, basearch, releasever)?;
     let include_path = parsed.config.include.take();
     if let &Some(ref include_path) = &include_path {
         if depth == INCLUDE_MAXDEPTH {
@@ -389,7 +424,7 @@ fn treefile_parse_recurse<P: AsRef<Path>>(
         }
         let parent = filename.parent().unwrap();
         let include_path = parent.join(include_path);
-        let mut included = treefile_parse_recurse(include_path, basearch, depth + 1)?;
+        let mut included = treefile_parse_recurse(include_path, basearch, releasever, depth + 1)?;
         treefile_merge(&mut parsed.config, &mut included.config);
         treefile_merge_externals(&mut parsed.externals, &mut included.externals);
     }
@@ -413,9 +448,10 @@ impl Treefile {
     fn new_boxed(
         filename: &Path,
         basearch: Option<&str>,
+        releasever: Option<&str>,
         workdir: openat::Dir,
     ) -> Fallible<Box<Treefile>> {
-        let parsed = treefile_parse_recurse(filename, basearch, 0)?;
+        let parsed = treefile_parse_recurse(filename, basearch, releasever, 0)?;
         Treefile::validate_config(&parsed.config)?;
         let dfd = openat::Dir::open(filename.parent().unwrap())?;
         let (rojig_name, rojig_spec) = if let Some(rojig) = parsed.config.rojig.as_ref() {
@@ -725,6 +761,7 @@ mod tests {
     use tempfile;
 
     static ARCH_X86_64: &str = "x86_64";
+    static RELEASE_VER: &str = "30";
 
     static VALID_PRELUDE: &str = r###"
 ref: "exampleos/x86_64/blah"
@@ -740,7 +777,7 @@ packages-s390x:
     // This one has "comments" (hence unknown keys)
     static VALID_PRELUDE_JS: &str = r###"
 {
- "ref": "exampleos/${basearch}/blah",
+ "ref": "exampleos/${basearch}/${releasever}",
  "comment-packages": "We want baz to enable frobnication",
  "packages": ["foo", "bar", "baz"],
  "packages-x86_64": ["grub2", "grub2-tools"],
@@ -753,7 +790,7 @@ packages-s390x:
     fn basic_valid() {
         let mut input = io::BufReader::new(VALID_PRELUDE.as_bytes());
         let treefile =
-            treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)).unwrap();
+            treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64), None).unwrap();
         assert!(treefile.treeref.unwrap() == "exampleos/x86_64/blah");
         assert!(treefile.packages.unwrap().len() == 5);
     }
@@ -776,7 +813,7 @@ remove-files:
         let buf = buf.as_bytes();
         let mut input = io::BufReader::new(buf);
         let treefile =
-            treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)).unwrap();
+            treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64), None).unwrap();
         assert!(treefile.add_files.unwrap().len() == 2);
         assert!(treefile.remove_files.unwrap().len() == 2);
     }
@@ -785,29 +822,29 @@ remove-files:
     fn basic_js_valid() {
         let mut input = io::BufReader::new(VALID_PRELUDE_JS.as_bytes());
         let treefile =
-            treefile_parse_stream(InputFormat::JSON, &mut input, Some(ARCH_X86_64)).unwrap();
-        assert!(treefile.treeref.unwrap() == "exampleos/x86_64/blah");
+            treefile_parse_stream(InputFormat::JSON, &mut input, Some(ARCH_X86_64), Some(RELEASE_VER)).unwrap();
+        assert!(treefile.treeref.unwrap() == "exampleos/x86_64/30");
         assert!(treefile.packages.unwrap().len() == 5);
     }
 
     #[test]
     fn basic_valid_noarch() {
         let mut input = io::BufReader::new(VALID_PRELUDE.as_bytes());
-        let treefile = treefile_parse_stream(InputFormat::YAML, &mut input, None).unwrap();
+        let treefile = treefile_parse_stream(InputFormat::YAML, &mut input, None, None).unwrap();
         assert!(treefile.treeref.unwrap() == "exampleos/x86_64/blah");
         assert!(treefile.packages.unwrap().len() == 3);
     }
 
     fn append_and_parse(append: &'static str) -> TreeComposeConfig {
         let buf = VALID_PRELUDE.to_string() + append;
         let mut input = io::BufReader::new(buf.as_bytes());
-        treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)).unwrap()
+        treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64), None).unwrap()
     }
 
     fn test_invalid(data: &'static str) {
         let buf = VALID_PRELUDE.to_string() + data;
         let mut input = io::BufReader::new(buf.as_bytes());
-        match treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)) {
+        match treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64), None) {
             Err(ref e) => match e.downcast_ref::<io::Error>() {
                 Some(ref ioe) if ioe.kind() == io::ErrorKind::InvalidInput => {}
                 _ => panic!("Expected invalid treefile, not {}", e.to_string()),
@@ -904,6 +941,20 @@ automatic_version_prefix: bar
         );
     }
 
+    #[test]
+    fn test_invalid_release_ver() {
+        let buf = VALID_PRELUDE.to_string() + r###"releasever: blah"###;
+        let mut input = io::BufReader::new(buf.as_bytes());
+        match treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64), Some(RELEASE_VER)) {
+            Err(ref e) => { println!("{}", e.to_string()); assert!(e.to_string() == "Invalid releasever \"blah\"");
+                //match e.kind() {
+                //&ErrorKind::Msg(_) => {},
+                //_ => panic!("Expected invalid treefile, not {}", e.to_string()),
+            },
+            Ok(_) => panic!("Expected invalid treefile"),
+        }
+    }
+
     struct TreefileTest {
         tf: Box<Treefile>,
         #[allow(dead_code)]
@@ -921,6 +972,7 @@ automatic_version_prefix: bar
             let tf = Treefile::new_boxed(
                 tf_path.as_path(),
                 basearch,
+                None,
                 openat::Dir::open(workdir.path())?,
             )?;
             Ok(TreefileTest { tf, workdir })
@@ -960,17 +1012,17 @@ rojig:
     fn test_treefile_merge() {
         let basearch = Some(ARCH_X86_64);
         let mut base_input = io::BufReader::new(VALID_PRELUDE.as_bytes());
-        let mut base = treefile_parse_stream(InputFormat::YAML, &mut base_input, basearch).unwrap();
+        let mut base = treefile_parse_stream(InputFormat::YAML, &mut base_input, basearch, None).unwrap();
         let mut mid_input = io::BufReader::new(
             r###"
 packages:
   - some layered packages
 "###
             .as_bytes(),
         );
-        let mut mid = treefile_parse_stream(InputFormat::YAML, &mut mid_input, basearch).unwrap();
+        let mut mid = treefile_parse_stream(InputFormat::YAML, &mut mid_input, basearch, None).unwrap();
         let mut top_input = io::BufReader::new(ROJIG_YAML.as_bytes());
-        let mut top = treefile_parse_stream(InputFormat::YAML, &mut top_input, basearch).unwrap();
+        let mut top = treefile_parse_stream(InputFormat::YAML, &mut top_input, basearch, None).unwrap();
         treefile_merge(&mut mid, &mut base);
         treefile_merge(&mut top, &mut mid);
         let tf = &top;
@@ -982,7 +1034,7 @@ packages:
     #[test]
     fn test_open_file_nonexistent() {
         let path = "/usr/share/empty/manifest.yaml";
-        match treefile_parse(path, None) {
+        match treefile_parse(path, None, None) {
             Err(ref e) => assert!(e
                 .to_string()
                 .starts_with(format!("Can't open file {:?}:", path).as_str())),
@@ -1014,17 +1066,19 @@ mod ffi {
     pub extern "C" fn ror_treefile_new(
         filename: *const libc::c_char,
         basearch: *const libc::c_char,
+        releasever: *const libc::c_char,
         workdir_dfd: libc::c_int,
         gerror: *mut *mut glib_sys::GError,
     ) -> *mut Treefile {
         // Convert arguments
         let filename = ffi_view_os_str(filename);
         let basearch = ffi_view_nullable_str(basearch);
+        let releasever = ffi_view_nullable_str(releasever);
         let workdir = ffi_view_openat_dir(workdir_dfd);
         // Run code, map error if any, otherwise extract raw pointer, passing
         // ownership back to C.
         ptr_glib_error(
-            Treefile::new_boxed(filename.as_ref(), basearch, workdir),
+            Treefile::new_boxed(filename.as_ref(), basearch, releasever, workdir),
             gerror,
         )
     }

diff --git a/src/app/rpmostree-compose-builtin-rojig.c b/src/app/rpmostree-compose-builtin-rojig.c
@@ -284,7 +284,8 @@ rpm_ostree_rojig_compose_new (const char    *treefile_path,
     return FALSE;
 
   const char *arch = dnf_context_get_base_arch (rpmostree_context_get_dnf (self->corectx));
-  self->treefile_rs = ror_treefile_new (treefile_path, arch, self->workdir_dfd, error);
+  const char *release = dnf_context_get_release_ver (rpmostree_context_get_dnf (self->corectx));
+  self->treefile_rs = ror_treefile_new (treefile_path, arch, release, self->workdir_dfd, error);
   if (!self->treefile_rs)
     return glnx_prefix_error (error, "Failed to load YAML treefile");
 

diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c
@@ -466,12 +466,15 @@ static gboolean
 parse_treefile_to_json (const char    *treefile_path,
                         int            workdir_dfd,
                         const char    *arch,
+                        const char    *release,
                         RORTreefile  **out_treefile_rs,
                         JsonParser   **out_parser,
                         GError       **error)
 {
   g_autoptr(JsonParser) parser = json_parser_new ();
-  g_autoptr(RORTreefile) treefile_rs = ror_treefile_new (treefile_path, arch, workdir_dfd, error);
+  g_autoptr(RORTreefile) treefile_rs = ror_treefile_new (treefile_path,
+                                                         arch, release,
+                                                         workdir_dfd, error);
   if (!treefile_rs)
     return glnx_prefix_error (error, "Failed to load YAML treefile");
 
@@ -683,8 +686,9 @@ rpm_ostree_compose_context_new (const char    *treefile_pathstr,
     return FALSE;
 
   const char *arch = dnf_context_get_base_arch (rpmostree_context_get_dnf (self->corectx));
+  const char *release = dnf_context_get_release_ver (rpmostree_context_get_dnf (self->corectx));
   if (!parse_treefile_to_json (gs_file_get_path_cached (self->treefile_path),
-                               self->workdir_dfd, arch,
+                               self->workdir_dfd, arch, release,
                                &self->treefile_rs, &self->treefile_parser,
                                error))
     return FALSE;
@@ -1173,7 +1177,7 @@ rpmostree_compose_builtin_postprocess (int             argc,
     {
       if (!glnx_mkdtempat (AT_FDCWD, "/var/tmp/rpm-ostree.XXXXXX", 0700, &workdir_tmp, error))
         return FALSE;
-      if (!parse_treefile_to_json (treefile_path, workdir_tmp.fd, NULL,
+      if (!parse_treefile_to_json (treefile_path, workdir_tmp.fd, NULL, NULL,
                                    &treefile_rs, &treefile_parser, error))
         return FALSE;
 

diff --git a/src/app/rpmostree-ex-builtin-commit2rojig.c b/src/app/rpmostree-ex-builtin-commit2rojig.c
@@ -99,7 +99,7 @@ rpmostree_ex_builtin_commit2rojig (int             argc,
   if (!glnx_mkdtemp ("rpmostree-commit2rojig-XXXXXX", 0700, &tmpd, error))
     return FALSE;
 
-  g_autoptr(RORTreefile) treefile_rs = ror_treefile_new (treefile, NULL, tmpd.fd, error);
+  g_autoptr(RORTreefile) treefile_rs = ror_treefile_new (treefile, NULL, NULL, tmpd.fd, error);
   const char *rojig_spec_path = ror_treefile_get_rojig_spec_path (treefile_rs);
   if (!rpmostree_commit2rojig (repo, pkgcache_repo, rev, tmpd.fd, rojig_spec_path, outputdir,
                                cancellable, error))