Skip to content
Open
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
50 changes: 29 additions & 21 deletions crates/lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,7 @@ async fn verify_target_fetch(
async fn prepare_install(
mut config_opts: InstallConfigOpts,
source_opts: InstallSourceOpts,
target_opts: InstallTargetOpts,
mut target_opts: InstallTargetOpts,
mut composefs_options: InstallComposefsOpts,
target_fs: Option<FilesystemEnum>,
) -> Result<Arc<State>> {
Expand Down Expand Up @@ -1543,6 +1543,34 @@ async fn prepare_install(
}
};

// Load install configuration from TOML drop-in files early, so that
// config values are available when constructing the target image reference.
let install_config = config::load_config()?;
if let Some(ref config) = install_config {
tracing::debug!("Loaded install configuration");
// Merge config file values into config_opts (CLI takes precedence)
// Only apply config file value if CLI didn't explicitly set it
if !config_opts.bootupd_skip_boot_uuid {
config_opts.bootupd_skip_boot_uuid = config
.bootupd
.as_ref()
.and_then(|b| b.skip_boot_uuid)
.unwrap_or(false);
}

if config_opts.bootloader.is_none() {
config_opts.bootloader = config.bootloader.clone();
}

if !target_opts.enforce_container_sigpolicy {
target_opts.enforce_container_sigpolicy = config
.enforce_container_sigpolicy
.unwrap_or(false);
}
Comment on lines +1553 to +1569
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The logic for merging boolean options from the configuration file can be made more explicit to improve readability and avoid potentially redundant assignments.

When a boolean configuration option is not present, the current implementation with unwrap_or(false) assigns false to a variable that is already known to be false inside the if !... block.

Using if let to check for the presence of the config value before assigning makes the intent clearer and avoids this no-op assignment.

        if !config_opts.bootupd_skip_boot_uuid {
            if let Some(skip) = config.bootupd.as_ref().and_then(|b| b.skip_boot_uuid) {
                config_opts.bootupd_skip_boot_uuid = skip;
            }
        }

        if config_opts.bootloader.is_none() {
            config_opts.bootloader = config.bootloader.clone();
        }

        if !target_opts.enforce_container_sigpolicy {
            if let Some(enforce) = config.enforce_container_sigpolicy {
                target_opts.enforce_container_sigpolicy = enforce;
            }
        }

} else {
tracing::debug!("No install configuration found");
}

// Parse the target CLI image reference options and create the *target* image
// reference, which defaults to pulling from a registry.
if target_opts.target_no_signature_verification {
Expand Down Expand Up @@ -1630,26 +1658,6 @@ async fn prepare_install(
println!("Digest: {digest}");
}

let install_config = config::load_config()?;
if let Some(ref config) = install_config {
tracing::debug!("Loaded install configuration");
// Merge config file values into config_opts (CLI takes precedence)
// Only apply config file value if CLI didn't explicitly set it
if !config_opts.bootupd_skip_boot_uuid {
config_opts.bootupd_skip_boot_uuid = config
.bootupd
.as_ref()
.and_then(|b| b.skip_boot_uuid)
.unwrap_or(false);
}

if config_opts.bootloader.is_none() {
config_opts.bootloader = config.bootloader.clone();
}
} else {
tracing::debug!("No install configuration found");
}

let root_filesystem = target_fs
.or(install_config
.as_ref()
Expand Down
51 changes: 51 additions & 0 deletions crates/lib/src/install/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ pub(crate) struct InstallConfiguration {
pub(crate) bootupd: Option<Bootupd>,
/// Bootloader to use (grub, systemd, none)
pub(crate) bootloader: Option<Bootloader>,
/// Enforce that the containers-storage stack has a non-default
/// (i.e. not `insecureAcceptAnything`) container image signature policy.
pub(crate) enforce_container_sigpolicy: Option<bool>,
}

fn merge_basic<T>(s: &mut Option<T>, o: Option<T>, _env: &EnvProperties) {
Expand Down Expand Up @@ -203,6 +206,11 @@ impl Mergeable for InstallConfiguration {
merge_basic(&mut self.boot_mount_spec, other.boot_mount_spec, env);
self.bootupd.merge(other.bootupd, env);
merge_basic(&mut self.bootloader, other.bootloader, env);
merge_basic(
&mut self.enforce_container_sigpolicy,
other.enforce_container_sigpolicy,
env,
);
if let Some(other_kargs) = other.kargs {
self.kargs
.get_or_insert_with(Default::default)
Expand Down Expand Up @@ -876,3 +884,46 @@ bootloader = "grub"
install.merge(other, &env);
assert_eq!(install.bootloader, Some(Bootloader::None));
}

#[test]
fn test_parse_enforce_container_sigpolicy() {
let env = EnvProperties {
sys_arch: "x86_64".to_string(),
};

// Test parsing true and false
for (input, expected) in [("true", true), ("false", false)] {
let toml_str = format!(
r#"[install]
enforce-container-sigpolicy = {input}
"#
);
let c: InstallConfigurationToplevel = toml::from_str(&toml_str).unwrap();
assert_eq!(
c.install.unwrap().enforce_container_sigpolicy.unwrap(),
expected
);
}

// Default (not specified) is None
let c: InstallConfigurationToplevel = toml::from_str(
r#"[install]
root-fs-type = "xfs"
"#,
)
.unwrap();
assert!(c.install.unwrap().enforce_container_sigpolicy.is_none());

// Test merging: last write wins
let mut install: InstallConfiguration = toml::from_str(
r#"enforce-container-sigpolicy = false
"#,
)
.unwrap();
let other = InstallConfiguration {
enforce_container_sigpolicy: Some(true),
..Default::default()
};
install.merge(other, &env);
assert_eq!(install.enforce_container_sigpolicy.unwrap(), true);
}
6 changes: 6 additions & 0 deletions docs/src/man/bootc-install-config.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ The `install` section supports these subfields:
- `boot-mount-spec`: A string specifying the /boot filesystem mount specification.
If not provided and /boot is a separate mount, its UUID will be used.
An empty string signals to omit boot mount kargs entirely.
- `enforce-container-sigpolicy`: A boolean that controls whether to enforce that
`/etc/containers/policy.json` includes a default policy which requires signatures.
When `true`, image pulls will be rejected if the policy file specifies
`insecureAcceptAnything` as the default. Defaults to `false`.
This is equivalent to the `--enforce-container-sigpolicy` CLI flag.

# filesystem

Expand Down Expand Up @@ -73,6 +78,7 @@ kargs = ["nosmt", "console=tty0"]
stateroot = "myos"
root-mount-spec = "LABEL=rootfs"
boot-mount-spec = "UUID=abcd-1234"
enforce-container-sigpolicy = true

[install.ostree]
bls-append-except-default = 'grub_users=""'
Expand Down
Loading