From f470738ce8def132f8fc53818712f6d834748bb4 Mon Sep 17 00:00:00 2001 From: Rafael Fonseca Date: Tue, 4 Jun 2019 11:09:35 +0200 Subject: [PATCH] WIP: treefile: allow ${releasever} in more keys Fixes #1809 Signed-off-by: Rafael Fonseca --- rust/Cargo.lock | 10 ++ rust/Cargo.toml | 1 + rust/src/treefile.rs | 116 ++++++++++++++------ src/app/rpmostree-compose-builtin-rojig.c | 3 +- src/app/rpmostree-compose-builtin-tree.c | 10 +- src/app/rpmostree-ex-builtin-commit2rojig.c | 2 +- 6 files changed, 106 insertions(+), 36 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 00912c1e08..c3c5b61c7d 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -216,6 +216,14 @@ name = "encode_unicode" version = "0.3.5" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "envsubst" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "failure 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "failure" version = "0.1.5" @@ -631,6 +639,7 @@ dependencies = [ "c_utf8 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "clap 2.32.0 (registry+https://github.com/rust-lang/crates.io-index)", "curl 0.4.20 (registry+https://github.com/rust-lang/crates.io-index)", + "envsubst 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "failure 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", "gio-sys 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "glib 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -937,6 +946,7 @@ dependencies = [ "checksum dtoa 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "6d301140eb411af13d3115f9a562c85cc6b541ade9dfa314132244aaee7489dd" "checksum either 1.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = "5527cfe0d098f36e3f8839852688e63c8fff1c90b2b405aef730615f9a7bcf7b" "checksum encode_unicode 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)" = "90b2c9496c001e8cb61827acdefad780795c42264c137744cae6f7d9e3450abd" +"checksum envsubst 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "13cf19a8d534c83456ea13365a1935810a39f0e43bf1ec9371077e1966da396a" "checksum failure 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "795bd83d3abeb9220f257e597aa0080a508b27533824adf336529648f6abf7e2" "checksum failure_derive 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "ea1063915fd7ef4309e222a5a07cf9c319fb9c7836b1f89b85458672dbb127e1" "checksum fuchsia-zircon 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "2e9763c69ebaae630ba35f74888db465e49e259ba1bc0eda7d06f4a067615d82" diff --git a/rust/Cargo.toml b/rust/Cargo.toml index a45178306a..c6cc943d2d 100644 --- 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 index 926d4c4670..474ae01afe 100644 --- 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( fmt: InputFormat, input: &mut R, basearch: Option<&str>, + releasever: Option<&str>, ) -> Fallible { let mut treefile: TreeComposeConfig = match fmt { InputFormat::YAML => { @@ -99,6 +98,8 @@ fn treefile_parse_stream( } }; + let mut substvars: collections::HashMap = collections::HashMap::new(); + treefile.basearch = match (treefile.basearch, basearch) { (Some(treearch), Some(arch)) => { if treearch != arch { @@ -118,6 +119,25 @@ fn treefile_parse_stream( // 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> = take_archful_pkgs(basearch, &mut treefile)?; @@ -131,18 +151,31 @@ fn treefile_parse_stream( .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>( fn treefile_parse>( filename: P, basearch: Option<&str>, + releasever: Option<&str>, ) -> Fallible { let filename = filename.as_ref(); let mut f = io::BufReader::new(open_file(filename)?); @@ -248,7 +282,7 @@ fn treefile_parse>( } 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>( filename: P, basearch: Option<&str>, + releasever: Option<&str>, depth: u32, ) -> Fallible { 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>( } 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> { - 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,15 +822,15 @@ 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); } @@ -801,13 +838,13 @@ remove-files: 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::() { 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, #[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,7 +1012,7 @@ 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: @@ -968,9 +1020,9 @@ 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 = ⊤ @@ -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 index 02f0e03e32..8e89aa2dea 100644 --- 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 index fd27a0dedb..1a219acd7e 100644 --- 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 index c79ad5976b..91a26a433c 100644 --- 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))