-
Notifications
You must be signed in to change notification settings - Fork 191
install: Add --karg-delete
#2105
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
Changes from all commits
be58444
d44cb30
417551c
ddbbb3f
87f13a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -345,6 +345,12 @@ pub(crate) struct InstallConfigOpts { | |
| #[clap(long)] | ||
| pub(crate) karg: Option<Vec<CmdlineOwned>>, | ||
|
|
||
| /// Remove a kernel argument. This option can be provided multiple times. | ||
| /// | ||
| /// Example: --karg-delete=nosmt --karg=console=ttyS0,115200n8 | ||
| #[clap(long)] | ||
| pub(crate) karg_delete: Option<Vec<String>>, | ||
|
|
||
| /// The path to an `authorized_keys` that will be injected into the `root` account. | ||
| /// | ||
| /// The implementation of this uses systemd `tmpfiles.d`, writing to a file named | ||
|
|
@@ -1124,13 +1130,18 @@ async fn install_container( | |
|
|
||
| // Keep this in sync with install/completion.rs for the Anaconda fixups | ||
| let install_config_kargs = state.install_config.as_ref().and_then(|c| c.kargs.as_ref()); | ||
| let install_config_karg_deletes = state | ||
| .install_config | ||
| .as_ref() | ||
| .and_then(|c| c.karg_deletes.as_ref()); | ||
|
|
||
| // Final kargs, in order: | ||
| // - root filesystem kargs | ||
| // - install config kargs | ||
| // - kargs.d from container image | ||
| // - args specified on the CLI | ||
| let mut kargs = Cmdline::new(); | ||
| let mut karg_deletes = Vec::<&str>::new(); | ||
|
|
||
| kargs.extend(&root_setup.kargs); | ||
|
|
||
|
|
@@ -1142,6 +1153,19 @@ async fn install_container( | |
|
|
||
| kargs.extend(&kargsd); | ||
|
|
||
| // delete kargs before processing cli kargs, so cli kargs can override all other configs | ||
| if let Some(install_config_karg_deletes) = install_config_karg_deletes { | ||
| for karg_delete in install_config_karg_deletes { | ||
| karg_deletes.push(karg_delete); | ||
| } | ||
| } | ||
| if let Some(deletes) = state.config_opts.karg_delete.as_ref() { | ||
| for karg_delete in deletes { | ||
| karg_deletes.push(karg_delete); | ||
| } | ||
| } | ||
| delete_kargs(&mut kargs, &karg_deletes); | ||
|
|
||
| if let Some(cli_kargs) = state.config_opts.karg.as_ref() { | ||
| for karg in cli_kargs { | ||
| kargs.extend(karg); | ||
|
|
@@ -1224,6 +1248,18 @@ async fn install_container( | |
| Ok((deployment, aleph)) | ||
| } | ||
|
|
||
| pub(crate) fn delete_kargs(existing: &mut Cmdline, deletes: &Vec<&str>) { | ||
| for delete in deletes { | ||
| if let Some(param) = utf8::Parameter::parse(&delete) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm when would we get None from parsing here? Just when it's an empty string right? We should probably have filtered those out or errored if I'm right
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty strings or strings with only whitespace will return |
||
| if param.value().is_some() { | ||
| existing.remove_exact(¶m); | ||
| } else { | ||
| existing.remove(¶m.key()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Run a command in the host mount namespace | ||
| pub(crate) fn run_in_host_mountns(cmd: &str) -> Result<Command> { | ||
| let mut c = Command::new(bootc_utils::reexec::executable_path()?); | ||
|
|
@@ -3085,4 +3121,21 @@ UUID=boot-uuid /boot ext4 defaults 0 0 | |
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_delete_kargs() -> Result<()> { | ||
| let mut cmdline = Cmdline::from("console=tty0 quiet debug nosmt foo=bar foo=baz bar=baz"); | ||
|
|
||
| let deletions = vec!["foo=bar", "bar", "debug"]; | ||
|
|
||
| delete_kargs(&mut cmdline, &deletions); | ||
|
|
||
| let result = cmdline.to_string(); | ||
| assert!(!result.contains("foo=bar")); | ||
| assert!(!result.contains("bar")); | ||
| assert!(!result.contains("debug")); | ||
| assert!(result.contains("foo=baz")); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # number: 40 | ||
| # tmt: | ||
| # summary: Test bootc install --karg-delete | ||
| # duration: 30m | ||
| # extra: | ||
| # fixme_skip_if_composefs: true | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should work for composefs backend as well since the config gathering in the install path is pretty much the same for ostree and composefs. Were you getting some sort of error when testing this for composefs backend?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just that if you use I was looking through
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can only do this with type1 BLS though to be clear with composefs, not UKIs. We have to error out on any local karg manipulations there. |
||
| # | ||
| use std assert | ||
| use tap.nu | ||
| # | ||
| # Use an OS-matched target image to avoid version mismatches | ||
| let target_image = (tap get_target_image) | ||
|
|
||
| # setup filesystem | ||
| mkdir /var/mnt | ||
| truncate -s 10G disk.img | ||
| mkfs.ext4 disk.img | ||
| mount -o loop disk.img /var/mnt | ||
|
|
||
| # Mask off the bootupd state to reproduce https://github.com/bootc-dev/bootc/issues/1778 | ||
| # Also it turns out that installation outside of containers dies due to `error: Multiple commit objects found` | ||
| # so we mask off /sysroot/ostree | ||
| # And using systemd-run here breaks our install_t so we disable SELinux enforcement | ||
| setenforce 0 | ||
|
|
||
| mkdir /etc/bootc/install | ||
| { install: { kargs: ["foo=bar"] } } | to toml | save /etc/bootc/install/99-test.toml | ||
|
|
||
| tap run_install $"bootc install to-filesystem --disable-selinux --bootloader none --source-imgref ($target_image) --karg-delete localtestkarg --karg-delete foo /var/mnt" | ||
|
|
||
|
|
||
| # Verify the karg is gone from the bootloader entries | ||
| let entries = (glob /var/mnt/boot/loader/entries/*.conf | ||
| | each { open $in | lines } | ||
| | flatten) | ||
|
|
||
| let localtestkarg_found = ($entries | find "localtestkarg" | is-empty) | ||
| assert $localtestkarg_found "Found localtestkarg in bootloader entries, but it should have been deleted" | ||
|
|
||
| let foo_found = ($entries | find "foo" | is-empty) | ||
| assert $foo_found "Found foo in bootloader entries, but it should have been deleted" | ||
|
|
||
| # Clean up | ||
| umount /var/mnt | ||
| rm -rf disk.img | ||
|
|
||
| tap ok | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor style nit I think this could all be one combinator chain via
.chain().flatten().collect()