Skip to content

Commit

Permalink
lints: Rework to use linkme
Browse files Browse the repository at this point in the history
Two goals:

- The global static LINTS array is a conflict point
- It's easier to lay out the lint info when it's next to each
  function; prep for extending the lint data more.

Signed-off-by: Colin Walters <walters@verbum.org>
  • Loading branch information
cgwalters committed Feb 7, 2025
1 parent c23212a commit abc927f
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 76 deletions.
21 changes: 21 additions & 0 deletions 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 lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ indoc = { workspace = true }
libc = { workspace = true }
liboverdrop = "0.1.0"
libsystemd = "0.7"
linkme = "0.3"
openssl = { workspace = true }
regex = "1.10.4"
rustix = { workspace = true }
Expand Down
187 changes: 111 additions & 76 deletions lib/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
//!
//! This module implements `bootc container lint`.
// Unfortunately needed here to work with linkme
#![allow(unsafe_code)]

use std::collections::BTreeSet;
use std::env::consts::ARCH;
use std::os::unix::ffi::OsStrExt;
Expand All @@ -14,6 +17,7 @@ use cap_std_ext::cap_std::fs::MetadataExt;
use cap_std_ext::dirext::CapStdExtDirExt as _;
use fn_error_context::context;
use indoc::indoc;
use linkme::distributed_slice;
use ostree_ext::ostree_prepareroot;
use serde::Serialize;

Expand Down Expand Up @@ -52,6 +56,8 @@ impl LintError {
}

type LintFn = fn(&Dir) -> LintResult;
#[distributed_slice]
pub(crate) static LINTS: [Lint];

/// The classification of a lint type.
#[derive(Debug, Serialize)]
Expand Down Expand Up @@ -81,85 +87,37 @@ struct Lint {
description: &'static str,
}

const LINTS: &[Lint] = &[
Lint {
name: "var-run",
ty: LintType::Fatal,
f: check_var_run,
description: "Check for /var/run being a physical directory; this is always a bug.",
},
Lint {
name: "kernel",
ty: LintType::Fatal,
f: check_kernel,
description: indoc! { r#"
Check for multiple kernels, i.e. multiple directories of the form /usr/lib/modules/$kver.
Only one kernel is supported in an image.
"# },
},
Lint {
name: "bootc-kargs",
ty: LintType::Fatal,
f: check_parse_kargs,
description: "Verify syntax of /usr/lib/bootc/kargs.d.",
},
Lint {
name: "etc-usretc",
ty: LintType::Fatal,
f: check_usretc,
description: indoc! { r#"
Verify that only one of /etc or /usr/etc exist. You should only have /etc
in a container image. It will cause undefined behavior to have both /etc
and /usr/etc.
"#},
},
Lint {
// This one can be lifted in the future, see https://github.com/containers/bootc/issues/975
name: "utf8",
ty: LintType::Fatal,
f: check_utf8,
description: indoc! { r#"
Check for non-UTF8 filenames. Currently, the ostree backend of bootc only supports
UTF-8 filenames. Non-UTF8 filenames will cause a fatal error.
"#},
},
Lint {
name: "baseimage-root",
ty: LintType::Fatal,
f: check_baseimage_root,
description: indoc! { r#"
Check that expected files are present in the root of the filesystem; such
as /sysroot and a composefs configuration for ostree. More in
<https://containers.github.io/bootc/bootc-images.html#standard-image-content>.
"#},
},
Lint {
name: "var-log",
ty: LintType::Warning,
f: check_varlog,
description: indoc! { r#"
Check for non-empty regular files in `/var/log`. It is often undesired
to ship log files in container images. Log files in general are usually
per-machine state in `/var`. Additionally, log files often include
timestamps, causing unreproducible container images, and may contain
sensitive build system information.
"#},
},
Lint {
name: "nonempty-boot",
ty: LintType::Warning,
f: check_boot,
description: indoc! { r#"
The `/boot` directory should be present, but empty. The kernel
content should be in /usr/lib/modules instead in the container image.
Any content here in the container image will be masked at runtime.
"#},
},
];
impl Lint {
pub(crate) const fn new_fatal(
name: &'static str,
description: &'static str,
f: LintFn,
) -> Self {
Lint {
name: name,
ty: LintType::Fatal,
f: f,
description: description,
}
}

pub(crate) const fn new_warning(
name: &'static str,
description: &'static str,
f: LintFn,
) -> Self {
Lint {
name: name,
ty: LintType::Warning,
f: f,
description: description,
}
}
}

pub(crate) fn lint_list(output: impl std::io::Write) -> Result<()> {
// Dump in yaml format by default, it's readable enough
serde_yaml::to_writer(output, LINTS)?;
serde_yaml::to_writer(output, &*LINTS)?;
Ok(())
}

Expand Down Expand Up @@ -214,6 +172,13 @@ pub(crate) fn lint(
Ok(())
}

#[allow(unsafe_code)]
#[distributed_slice(LINTS)]
static LINT_VAR_RUN: Lint = Lint::new_fatal(
"var-run",
"Check for /var/run being a physical directory; this is always a bug.",
check_var_run,
);
fn check_var_run(root: &Dir) -> LintResult {
if let Some(meta) = root.symlink_metadata_optional("var/run")? {
if !meta.is_symlink() {
Expand All @@ -223,6 +188,17 @@ fn check_var_run(root: &Dir) -> LintResult {
lint_ok()
}

#[allow(unsafe_code)]
#[distributed_slice(LINTS)]
static LINT_ETC_USRUSETC: Lint = Lint::new_fatal(
"etc-usretc",
indoc! { r#"
Verify that only one of /etc or /usr/etc exist. You should only have /etc
in a container image. It will cause undefined behavior to have both /etc
and /usr/etc.
"# },
check_usretc,
);
fn check_usretc(root: &Dir) -> LintResult {
let etc_exists = root.symlink_metadata_optional("etc")?.is_some();
// For compatibility/conservatism don't bomb out if there's no /etc.
Expand All @@ -239,18 +215,45 @@ fn check_usretc(root: &Dir) -> LintResult {
}

/// Validate that we can parse the /usr/lib/bootc/kargs.d files.
#[allow(unsafe_code)]
#[distributed_slice(LINTS)]
static LINT_KARGS: Lint = Lint::new_fatal(
"bootc-kargs",
"Verify syntax of /usr/lib/bootc/kargs.d.",
check_parse_kargs,
);
fn check_parse_kargs(root: &Dir) -> LintResult {
let args = crate::kargs::get_kargs_in_root(root, ARCH)?;
tracing::debug!("found kargs: {args:?}");
lint_ok()
}

#[allow(unsafe_code)]
#[distributed_slice(LINTS)]
static LINT_KERNEL: Lint = Lint::new_fatal(
"kernel",
indoc! { r#"
Check for multiple kernels, i.e. multiple directories of the form /usr/lib/modules/$kver.
Only one kernel is supported in an image.
"# },
check_kernel,
);
fn check_kernel(root: &Dir) -> LintResult {
let result = ostree_ext::bootabletree::find_kernel_dir_fs(&root)?;
tracing::debug!("Found kernel: {:?}", result);
lint_ok()
}

// This one can be lifted in the future, see https://github.com/containers/bootc/issues/975
#[distributed_slice(LINTS)]
static LINT_UTF8: Lint = Lint::new_fatal(
"utf8",
indoc! { r#"
Check for non-UTF8 filenames. Currently, the ostree backend of bootc only supports
UTF-8 filenames. Non-UTF8 filenames will cause a fatal error.
"#},
check_utf8,
);
fn check_utf8(dir: &Dir) -> LintResult {
for entry in dir.entries()? {
let entry = entry?;
Expand Down Expand Up @@ -312,6 +315,16 @@ fn check_baseimage_root_norecurse(dir: &Dir) -> LintResult {
}

/// Check ostree-related base image content.
#[distributed_slice(LINTS)]
static LINT_BASEIMAGE_ROOT: Lint = Lint::new_fatal(
"baseimage-root",
indoc! { r#"
Check that expected files are present in the root of the filesystem; such
as /sysroot and a composefs configuration for ostree. More in
<https://containers.github.io/bootc/bootc-images.html#standard-image-content>.
"#},
check_baseimage_root,
);
fn check_baseimage_root(dir: &Dir) -> LintResult {
if let Err(e) = check_baseimage_root_norecurse(dir)? {
return Ok(Err(e));
Expand Down Expand Up @@ -348,6 +361,18 @@ fn collect_nonempty_regfiles(
Ok(())
}

#[distributed_slice(LINTS)]
static LINT_VARLOG: Lint = Lint::new_warning(
"var-log",
indoc! { r#"
Check for non-empty regular files in `/var/log`. It is often undesired
to ship log files in container images. Log files in general are usually
per-machine state in `/var`. Additionally, log files often include
timestamps, causing unreproducible container images, and may contain
sensitive build system information.
"#},
check_varlog,
);
fn check_varlog(root: &Dir) -> LintResult {
let Some(d) = root.open_dir_optional("var/log")? else {
return lint_ok();
Expand All @@ -367,6 +392,16 @@ fn check_varlog(root: &Dir) -> LintResult {
lint_err(format!("Found non-empty logfile: {first}{others}"))
}

#[distributed_slice(LINTS)]
static LINT_NONEMPTY_BOOT: Lint = Lint::new_warning(
"nonempty-boot",
indoc! { r#"
The `/boot` directory should be present, but empty. The kernel
content should be in /usr/lib/modules instead in the container image.
Any content here in the container image will be masked at runtime.
"#},
check_boot,
);
fn check_boot(root: &Dir) -> LintResult {
let Some(d) = root.open_dir_optional("boot")? else {
return lint_err(format!("Missing /boot directory"));
Expand Down

0 comments on commit abc927f

Please sign in to comment.