Skip to content

Commit

Permalink
treefile.rs: Deny unknown fields by default
Browse files Browse the repository at this point in the history
Let's not make the same mistake we did with JSON where typoing a
field means it's silently ignored.  This actually caught a bug
in a YAML usage we had:

```
error: Failed to load YAML treefile: unknown field `install_langs`, expected one of ... `install-langs` ...
```

Yes, this is a compatibility break with the feature we just announced
but...I seriously doubt anyone (that isn't known to me) has converted
yet, and if they are excited enough to start using a two-week-old feature
they can adjust.

Closes: #1459
Approved by: jlebon
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Jul 19, 2018
1 parent 95de58c commit 710aae0
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 18 deletions.
50 changes: 45 additions & 5 deletions rust/src/treefile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,8 @@ use serde_yaml;
use std::path::Path;
use std::{fs, io};

pub fn treefile_read_impl<W: io::Write>(filename: &Path, output: W) -> io::Result<()> {
let f = io::BufReader::new(fs::File::open(filename)?);

let mut treefile: TreeComposeConfig = match serde_yaml::from_reader(f) {
fn treefile_parse_yaml<R: io::Read>(input: R) -> io::Result<TreeComposeConfig> {
let mut treefile: TreeComposeConfig = match serde_yaml::from_reader(input) {
Ok(t) => t,
Err(e) => {
return Err(io::Error::new(
Expand All @@ -44,8 +42,13 @@ pub fn treefile_read_impl<W: io::Write>(filename: &Path, output: W) -> io::Resul
treefile.packages = Some(whitespace_split_packages(&pkgs));
}

serde_json::to_writer_pretty(output, &treefile)?;
Ok(treefile)
}

pub fn treefile_read_impl<W: io::Write>(filename: &Path, output: W) -> io::Result<()> {
let f = io::BufReader::new(fs::File::open(filename)?);
let treefile = treefile_parse_yaml(f)?;
serde_json::to_writer_pretty(output, &treefile)?;
Ok(())
}

Expand Down Expand Up @@ -99,6 +102,7 @@ fn serde_true() -> bool {
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(deny_unknown_fields)]
pub struct TreeComposeConfig {
// Compose controls
#[serde(rename = "ref")]
Expand Down Expand Up @@ -187,3 +191,39 @@ pub struct TreeComposeConfig {
#[serde(rename = "remove-from-packages")]
pub remove_from_packages: Option<Vec<Vec<String>>>,
}

#[cfg(test)]
mod tests {
use super::*;

static VALID_PRELUDE : &str = r###"
ref: "exampleos/x86_64/blah"
packages:
- foo bar
- baz
"###;

#[test]
fn basic_valid() {
let input = io::BufReader::new(VALID_PRELUDE.as_bytes());
let treefile = treefile_parse_yaml(input).unwrap();
assert!(treefile.treeref == "exampleos/x86_64/blah");
assert!(treefile.packages.unwrap().len() == 3);
}

#[test]
fn basic_invalid() {
let mut buf = VALID_PRELUDE.to_string();
buf.push_str(r###"install_langs:
- "klingon"
- "esperanto"
"###);
let buf = buf.as_bytes();
let input = io::BufReader::new(buf);
match treefile_parse_yaml(input) {
Err(ref e) if e.kind() == io::ErrorKind::InvalidInput => {},
Err(ref e) => panic!("Expected invalid treefile, not {}", e.to_string()),
_ => panic!("Expected invalid treefile"),
}
}
}
15 changes: 2 additions & 13 deletions tests/composedata/fedora-base.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,8 @@
"repos": ["fedora", "updates"],

"packages": ["kernel", "nss-altfiles", "systemd", "ostree", "selinux-policy-targeted", "chrony",
"tuned", "iputils", "fedora-release-atomichost", "docker", "container-selinux"],

"packages-aarch64": ["grub2-efi", "ostree-grub2",
"efibootmgr", "shim"],

"packages-armhfp": ["extlinux-bootloader"],

"packages-ppc64": ["grub2", "ostree-grub2"],

"packages-ppc64le": ["grub2", "ostree-grub2"],

"packages-x86_64": ["grub2", "grub2-efi", "ostree-grub2",
"efibootmgr", "shim"],
"tuned", "iputils", "fedora-release-atomichost", "docker", "container-selinux",
"grub2", "grub2-efi", "ostree-grub2", "efibootmgr", "shim"],

"ignore-removed-users": ["root"],
"ignore-removed-groups": ["root"],
Expand Down

0 comments on commit 710aae0

Please sign in to comment.