Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP/DNM: Use 2024 edition #1130

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
429 changes: 229 additions & 200 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion blockdev/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
description = "Internal blockdev code"
# Should never be published to crates.io
publish = false
edition = "2021"
edition = "2024"
license = "MIT OR Apache-2.0"
name = "bootc-blockdev"
repository = "https://github.com/containers/bootc"
Expand Down
2 changes: 1 addition & 1 deletion blockdev/src/blockdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::path::Path;
use std::process::Command;
use std::sync::OnceLock;

use anyhow::{anyhow, Context, Result};
use anyhow::{Context, Result, anyhow};
use camino::Utf8Path;
use camino::Utf8PathBuf;
use fn_error_context::context;
Expand Down
2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "bootc"
version = "0.1.9"
edition = "2021"
edition = "2024"
license = "MIT OR Apache-2.0"
repository = "https://github.com/containers/bootc"
readme = "README.md"
Expand Down
4 changes: 2 additions & 2 deletions lib/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
[package]
description = "bootc implementation"
edition = "2021"
edition = "2024"
license = "MIT OR Apache-2.0"
name = "bootc-lib"
readme = "README.md"
repository = "https://github.com/containers/bootc"
version = "1.1.5"
# In general we try to keep this pinned to what's in the latest RHEL9.
# However right now, we bumped to 1.82 as that's what composefs-rs uses.
rust-version = "1.82.0"
rust-version = "1.85.0"

include = ["/src", "LICENSE-APACHE", "LICENSE-MIT"]

Expand Down
2 changes: 1 addition & 1 deletion lib/src/bootloader.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::{anyhow, bail, Context, Result};
use anyhow::{Context, Result, anyhow, bail};
use camino::{Utf8Path, Utf8PathBuf};
use fn_error_context::context;

Expand Down
45 changes: 25 additions & 20 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::io::Seek;
use std::os::unix::process::CommandExt;
use std::process::Command;

use anyhow::{ensure, Context, Result};
use anyhow::{Context, Result, ensure};
use camino::Utf8PathBuf;
use cap_std_ext::cap_std;
use cap_std_ext::cap_std::fs::Dir;
Expand Down Expand Up @@ -976,7 +976,11 @@ pub fn global_init() -> Result<()> {
// Setting the environment is thread-unsafe, but we ask calling code
// to invoke this as early as possible. (In practice, that's just the cli's `main.rs`)
// xref https://internals.rust-lang.org/t/synchronized-ffi-access-to-posix-environment-variable-functions/15475
std::env::set_var("HOME", "/root");
// TODO: Audit that the environment access only happens in single-threaded code.
#[allow(unsafe_code)]
unsafe {
std::env::set_var("HOME", "/root")
};
}
Ok(())
}
Expand Down Expand Up @@ -1012,26 +1016,27 @@ impl Opt {
I::Item: Into<OsString> + Clone,
{
let mut args = args.into_iter();
let first = if let Some(first) = args.next() {
let first: OsString = first.into();
let argv0 = callname_from_argv0(&first);
tracing::debug!("argv0={argv0:?}");
let mapped = match argv0 {
InternalsOpts::GENERATOR_BIN => {
Some(["bootc", "internals", "systemd-generator"].as_slice())
}
"ostree-container" | "ostree-ima-sign" | "ostree-provisional-repair" => {
Some(["bootc", "internals", "ostree-ext"].as_slice())
let first = match args.next() {
Some(first) => {
let first: OsString = first.into();
let argv0 = callname_from_argv0(&first);
tracing::debug!("argv0={argv0:?}");
let mapped = match argv0 {
InternalsOpts::GENERATOR_BIN => {
Some(["bootc", "internals", "systemd-generator"].as_slice())
}
"ostree-container" | "ostree-ima-sign" | "ostree-provisional-repair" => {
Some(["bootc", "internals", "ostree-ext"].as_slice())
}
_ => None,
};
if let Some(base_args) = mapped {
let base_args = base_args.iter().map(OsString::from);
return Opt::parse_from(base_args.chain(args.map(|i| i.into())));
}
_ => None,
};
if let Some(base_args) = mapped {
let base_args = base_args.iter().map(OsString::from);
return Opt::parse_from(base_args.chain(args.map(|i| i.into())));
Some(first)
}
Some(first)
} else {
None
_ => None,
};
Opt::parse_from(first.into_iter().chain(args.map(|i| i.into())))
}
Expand Down
29 changes: 16 additions & 13 deletions lib/src/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::collections::HashSet;
use std::io::{BufRead, Write};

use anyhow::Ok;
use anyhow::{anyhow, Context, Result};
use anyhow::{Context, Result, anyhow};
use cap_std::fs::{Dir, MetadataExt};
use cap_std_ext::cap_std;
use cap_std_ext::dirext::CapStdExtDirExt;
Expand Down Expand Up @@ -783,20 +783,23 @@ pub(crate) fn switch_origin_inplace(root: &Dir, imgref: &ImageReference) -> Resu

let mut ostree_deploys = root.open_dir("sysroot/ostree/deploy")?.entries()?;
let deploydir = loop {
if let Some(ent) = ostree_deploys.next() {
let ent = ent?;
if !ent.file_type()?.is_dir() {
continue;
match ostree_deploys.next() {
Some(ent) => {
let ent = ent?;
if !ent.file_type()?.is_dir() {
continue;
}
tracing::debug!("Checking {:?}", ent.file_name());
let child_dir = ent
.open_dir()
.with_context(|| format!("Opening dir {:?}", ent.file_name()))?;
if let Some(d) = child_dir.open_dir_optional("deploy")? {
break d;
}
}
tracing::debug!("Checking {:?}", ent.file_name());
let child_dir = ent
.open_dir()
.with_context(|| format!("Opening dir {:?}", ent.file_name()))?;
if let Some(d) = child_dir.open_dir_optional("deploy")? {
break d;
_ => {
anyhow::bail!("Failed to find a deployment");
}
} else {
anyhow::bail!("Failed to find a deployment");
}
};
let newest_deployment = find_newest_deployment_name(&deploydir)?;
Expand Down
4 changes: 2 additions & 2 deletions lib/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
//!
//! APIs for operating on container images in the bootc storage.

use anyhow::{bail, Context, Result};
use anyhow::{Context, Result, bail};
use bootc_utils::CommandRunExt;
use cap_std_ext::cap_std::{self, fs::Dir};
use clap::ValueEnum;
use comfy_table::{presets::NOTHING, Table};
use comfy_table::{Table, presets::NOTHING};
use fn_error_context::context;
use ostree_ext::container::{ImageReference, Transport};
use serde::Serialize;
Expand Down
53 changes: 35 additions & 18 deletions lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use std::str::FromStr;
use std::sync::Arc;
use std::time::Duration;

use anyhow::{anyhow, ensure, Context, Result};
use anyhow::{Context, Result, anyhow, ensure};
use bootc_utils::CommandRunExt;
use camino::Utf8Path;
use camino::Utf8PathBuf;
Expand Down Expand Up @@ -826,11 +826,14 @@ async fn install_container(
.with_context(|| format!("Recursive SELinux relabeling of {d}"))?;
}

if let Some(cfs_super) = root.open_optional(OSTREE_COMPOSEFS_SUPER)? {
let label = crate::lsm::require_label(policy, "/usr".into(), 0o644)?;
crate::lsm::set_security_selinux(cfs_super.as_fd(), label.as_bytes())?;
} else {
tracing::warn!("Missing {OSTREE_COMPOSEFS_SUPER}; composefs is not enabled?");
match root.open_optional(OSTREE_COMPOSEFS_SUPER)? {
Some(cfs_super) => {
let label = crate::lsm::require_label(policy, "/usr".into(), 0o644)?;
crate::lsm::set_security_selinux(cfs_super.as_fd(), label.as_bytes())?;
}
_ => {
tracing::warn!("Missing {OSTREE_COMPOSEFS_SUPER}; composefs is not enabled?");
}
}
}

Expand Down Expand Up @@ -1036,7 +1039,10 @@ fn require_host_userns() -> Result<()> {
.uid();
// We must really be in a rootless container, or in some way
// we're not part of the host user namespace.
ensure!(pid1_uid == 0, "{proc1} is owned by {pid1_uid}, not zero; this command must be run in the root user namespace (e.g. not rootless podman)");
ensure!(
pid1_uid == 0,
"{proc1} is owned by {pid1_uid}, not zero; this command must be run in the root user namespace (e.g. not rootless podman)"
);
tracing::trace!("OK: we're in a matching user namespace with pid1");
Ok(())
}
Expand Down Expand Up @@ -1176,7 +1182,10 @@ async fn prepare_install(
let external_source = source_opts.source_imgref.is_some();
let source = match source_opts.source_imgref {
None => {
ensure!(host_is_container, "Either --source-imgref must be defined or this command must be executed inside a podman container.");
ensure!(
host_is_container,
"Either --source-imgref must be defined or this command must be executed inside a podman container."
);

crate::cli::require_root(true)?;

Expand Down Expand Up @@ -1531,11 +1540,14 @@ pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> {
}

// At this point, all other threads should be gone.
if let Some(state) = Arc::into_inner(state) {
state.consume()?;
} else {
// This shouldn't happen...but we will make it not fatal right now
tracing::warn!("Failed to consume state Arc");
match Arc::into_inner(state) {
Some(state) => {
state.consume()?;
}
_ => {
// This shouldn't happen...but we will make it not fatal right now
tracing::warn!("Failed to consume state Arc");
}
}

installation_complete();
Expand Down Expand Up @@ -1579,11 +1591,16 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> {
let name = entry.file_name();
let etype = entry.file_type()?;
if etype == FileType::dir() {
if let Some(subdir) = d.open_dir_noxdev(&name)? {
remove_all_in_dir_no_xdev(&subdir, mount_err)?;
d.remove_dir(&name)?;
} else if mount_err {
anyhow::bail!("Found unexpected mount point {name:?}");
match d.open_dir_noxdev(&name)? {
Some(subdir) => {
remove_all_in_dir_no_xdev(&subdir, mount_err)?;
d.remove_dir(&name)?;
}
_ => {
if mount_err {
anyhow::bail!("Found unexpected mount point {name:?}");
}
}
}
} else {
d.remove_file_optional(&name)?;
Expand Down
6 changes: 3 additions & 3 deletions lib/src/install/baseline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ use clap::ValueEnum;
use fn_error_context::context;
use serde::{Deserialize, Serialize};

use super::config::Filesystem;
use super::MountSpec;
use super::RootSetup;
use super::State;
use super::RUN_BOOTC;
use super::RW_KARG;
use super::RootSetup;
use super::State;
use super::config::Filesystem;
use crate::mount;
#[cfg(feature = "install-to-disk")]
use crate::mount::is_mounted_in_pid1_mountns;
Expand Down
12 changes: 6 additions & 6 deletions lib/src/install/osconfig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ mod tests {

let content = root.read_to_string(format!("etc/tmpfiles.d/{ROOT_SSH_TMPFILE}"))?;
assert_eq!(
content,
"f~ /var/roothome/.ssh/authorized_keys 600 root root - c3NoLWVkMjU1MTkgQUJDREUgZXhhbXBsZUBkZW1vCg==\n"
);
content,
"f~ /var/roothome/.ssh/authorized_keys 600 root root - c3NoLWVkMjU1MTkgQUJDREUgZXhhbXBsZUBkZW1vCg==\n"
);

Ok(())
}
Expand All @@ -83,9 +83,9 @@ mod tests {

let content = root.read_to_string(format!("etc/tmpfiles.d/{ROOT_SSH_TMPFILE}"))?;
assert_eq!(
content,
"f~ /root/.ssh/authorized_keys 600 root root - c3NoLWVkMjU1MTkgQUJDREUgZXhhbXBsZUBkZW1vCg==\n"
);
content,
"f~ /root/.ssh/authorized_keys 600 root root - c3NoLWVkMjU1MTkgQUJDREUgZXhhbXBsZUBkZW1vCg==\n"
);
Ok(())
}
}
6 changes: 5 additions & 1 deletion lib/src/kargs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ match-architectures = ["x86_64", "aarch64"]
.to_string();
let parsed_kargs = parse_kargs_toml(&file_content, sys_arch).unwrap();
assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]);
std::env::set_var("ARCH", "aarch64");
// TODO: Audit that the environment access only happens in single-threaded code.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It definitely doesn't, rust runs unit tests in multiple threads by default. We need to stop setting this environment variable in the tests at all. At a quick look it's not clear we need this line of code, shouldn't we be just doing let sys_arch = "aarch64"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it doesn't even reference the environment variable anywhere. No idea what that was ever about. I'll go open a separate PR against main to fix just that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it doesn't even reference the environment variable anywhere. No idea what that was ever about. I'll go open a separate PR against main to fix just that.

#1132

#[allow(unsafe_code)]
unsafe {
std::env::set_var("ARCH", "aarch64")
};
let parsed_kargs = parse_kargs_toml(&file_content, sys_arch).unwrap();
assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]);
}
Expand Down
6 changes: 4 additions & 2 deletions lib/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,9 @@ fn check_buildah_injected(root: &Dir) -> LintResult {
for ent in RUNTIME_INJECTED {
if let Some(meta) = root.symlink_metadata_optional(ent)? {
if meta.is_file() && meta.size() == 0 {
return lint_err(format!("/{ent} is an empty file; this may have been synthesized by a container runtime."));
return lint_err(format!(
"/{ent} is an empty file; this may have been synthesized by a container runtime."
));
}
}
}
Expand All @@ -287,7 +289,7 @@ fn check_usretc(root: &Dir) -> LintResult {
// But having both /etc and /usr/etc is not something we want to support.
if root.symlink_metadata_optional("usr/etc")?.is_some() {
return lint_err(
"Found /usr/etc - this is a bootc implementation detail and not supported to use in containers"
"Found /usr/etc - this is a bootc implementation detail and not supported to use in containers",
);
}
lint_ok()
Expand Down
17 changes: 11 additions & 6 deletions lib/src/lsm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ pub(crate) fn selinux_ensure_install_or_setenforce() -> Result<Option<SetEnforce
Some(SetEnforceGuard::new())
} else {
let current = get_current_security_context()?;
anyhow::bail!("Failed to enter install_t (running as {current}) - use BOOTC_SETENFORCE0_FALLBACK=1 to override");
anyhow::bail!(
"Failed to enter install_t (running as {current}) - use BOOTC_SETENFORCE0_FALLBACK=1 to override"
);
};
Ok(g)
}
Expand Down Expand Up @@ -395,11 +397,14 @@ where
// Apply the target mode bits
rustix::fs::fchmod(fd, mode).context("fchmod")?;
// If we have a label, apply it
if let Some(label) = label {
tracing::debug!("Setting label for {destname} to {label}");
set_security_selinux(fd, label.as_bytes())?;
} else {
tracing::debug!("No label for {destname}");
match label {
Some(label) => {
tracing::debug!("Setting label for {destname} to {label}");
set_security_selinux(fd, label.as_bytes())?;
}
_ => {
tracing::debug!("No label for {destname}");
}
}
// Finally call the underlying writer function
f(w)
Expand Down
2 changes: 1 addition & 1 deletion lib/src/mount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
process::Command,
};

use anyhow::{anyhow, Context, Result};
use anyhow::{Context, Result, anyhow};
use bootc_utils::CommandRunExt;
use camino::Utf8Path;
use fn_error_context::context;
Expand Down
Loading
Loading