Skip to content

Commit

Permalink
treefile: allow ${releasever} in more keys
Browse files Browse the repository at this point in the history
Besides allowing ${releasever}, only do the substitution as the final
pass after merging the treefiles for all the keys (currently ${basearch}
and ${releasever}) instead of doing it per parse. This way we have the
expected semantics where one could do:

```
include: "fedora-coreos.yaml"
releasever: "42"
```

and have that releasever used.

Fixes #1809

Signed-off-by: Rafael Fonseca <[email protected]>

Closes: #1848
Approved by: cgwalters
  • Loading branch information
r4f4 authored and rh-atomic-bot committed Jun 18, 2019
1 parent d0f90ca commit c94bd08
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 18 deletions.
10 changes: 10 additions & 0 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
87 changes: 69 additions & 18 deletions rust/src/treefile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,19 +130,6 @@ 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,
};

// Special handling for packages, since we allow whitespace within items.
// We also canonicalize bootstrap_packages to packages here so it's
// easier to append the basearch packages after.
Expand Down Expand Up @@ -410,7 +397,8 @@ impl Treefile {
basearch: Option<&str>,
workdir: openat::Dir,
) -> Fallible<Box<Treefile>> {
let parsed = treefile_parse_recurse(filename, basearch, 0)?;
let mut parsed = treefile_parse_recurse(filename, basearch, 0)?;
parsed.config = parsed.config.substitute_vars()?;
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() {
Expand Down Expand Up @@ -718,6 +706,39 @@ impl TreeComposeConfig {

Ok(self)
}

/// Look for use of ${variable} and replace it by its proper value
fn substitute_vars(mut self) -> Fallible<Self> {
let mut substvars: collections::HashMap<String, String> = collections::HashMap::new();
// Substitute ${basearch} and ${releasever}
if let Some(arch) = &self.basearch {
substvars.insert("basearch".to_string(), arch.clone());
}
if let Some(releasever) = &self.releasever {
substvars.insert("releasever".to_string(), releasever.clone());
}
envsubst::validate_vars(&substvars)?;

macro_rules! substitute_field {
( $field:ident ) => {{
if let Some(value) = self.$field.take() {
self.$field = if envsubst::is_templated(&value) {
match envsubst::substitute(value, &substvars) {
Ok(s) => Some(s),
Err(e) => return Err(e),
}
} else {
Some(value)
}
}
}};
};
substitute_field!(treeref);
substitute_field!(automatic_version_prefix);
substitute_field!(mutate_os_release);

Ok(self)
}
}

#[cfg(test)]
Expand Down Expand Up @@ -753,8 +774,9 @@ packages-s390x:
#[test]
fn basic_valid() {
let mut input = io::BufReader::new(VALID_PRELUDE.as_bytes());
let treefile =
let mut treefile =
treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)).unwrap();
treefile = treefile.substitute_vars().unwrap();
assert!(treefile.treeref.unwrap() == "exampleos/x86_64/blah");
assert!(treefile.packages.unwrap().len() == 5);
}
Expand Down Expand Up @@ -785,24 +807,28 @@ remove-files:
#[test]
fn basic_js_valid() {
let mut input = io::BufReader::new(VALID_PRELUDE_JS.as_bytes());
let treefile =
let mut treefile =
treefile_parse_stream(InputFormat::JSON, &mut input, Some(ARCH_X86_64)).unwrap();
treefile = treefile.substitute_vars().unwrap();
assert!(treefile.treeref.unwrap() == "exampleos/x86_64/blah");
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 mut treefile = treefile_parse_stream(InputFormat::YAML, &mut input, None).unwrap();
treefile = treefile.substitute_vars().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()
let treefile =
treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)).unwrap();
treefile.substitute_vars().unwrap()
}

fn test_invalid(data: &'static str) {
Expand All @@ -817,6 +843,31 @@ remove-files:
}
}

#[test]
fn basic_valid_releasever() {
let buf = r###"
ref: "exampleos/${basearch}/${releasever}"
releasever: 30
automatic-version-prefix: ${releasever}
mutate-os-release: ${releasever}
"###;
let mut input = io::BufReader::new(buf.as_bytes());
let mut treefile =
treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)).unwrap();
treefile = treefile.substitute_vars().unwrap();
assert!(treefile.treeref.unwrap() == "exampleos/x86_64/30");
assert!(treefile.releasever.unwrap() == "30");
assert!(treefile.automatic_version_prefix.unwrap() == "30");
assert!(treefile.mutate_os_release.unwrap() == "30");
}

#[test]
fn test_valid_no_releasever() {
let treefile = append_and_parse("automatic_version_prefix: ${releasever}");
assert!(treefile.releasever == None);
assert!(treefile.automatic_version_prefix.unwrap() == "${releasever}");
}

#[test]
fn basic_valid_legacy() {
let treefile = append_and_parse(
Expand Down

0 comments on commit c94bd08

Please sign in to comment.