Merge pull request #1256 from ckyrouac/run-fetch-check

install: Deprecate skip-fetch-check, replace with run-fetch-check

Author: cgwalters

docs/src/bootc-install.md

@@ -93,11 +93,9 @@ installation in a manufacturing environment from a mirrored registry.
 By default, the installation process will verify that the container (representing the target OS)
 can fetch its own updates.
 
-Additionally note that to perform an install with a target image reference set to an
+Additionally note that to perform an upgrade with a target image reference set to an
 authenticated registry, you must provide a pull secret.  One path is to embed the pull secret into
 the image in `/etc/ostree/auth.json`.
-Alternatively, the secret can be added after an installation process completes and managed separately;
-in that case you will need to specify `--skip-fetch-check`.
 
 ### Configuring the default root filesystem type
 

docs/src/man/bootc-install-to-disk.md

@@ -7,7 +7,8 @@ bootc-install-to-disk - Install to the target block device
 **bootc install to-disk** \[**\--wipe**\] \[**\--block-setup**\]
 \[**\--filesystem**\] \[**\--root-size**\] \[**\--source-imgref**\]
 \[**\--target-transport**\] \[**\--target-imgref**\]
-\[**\--enforce-container-sigpolicy**\] \[**\--skip-fetch-check**\]
+\[**\--enforce-container-sigpolicy**\]
+\[**\--skip-fetch-check**\] \[**\--run-fetch-check**\]
 \[**\--disable-selinux**\] \[**\--karg**\]
 \[**\--root-ssh-authorized-keys**\] \[**\--generic-image**\]
 \[**\--bound-images**\] \[**\--stateroot**\] \[**\--via-loopback**\]
@@ -83,17 +84,16 @@ more complex such as RAID, LVM, LUKS etc.
     Enabling this option enforces that \`/etc/containers/policy.json\`
     includes a default policy which requires signatures
 
-**\--skip-fetch-check**
+**\--skip-fetch-check (deprecated, see --run-fetch-check)**
 
-:   By default, the accessiblity of the target image will be verified
-    (just the manifest will be fetched). Specifying this option
-    suppresses the check; use this when you know the issues it might
-    find are addressed.
+:   This is now the default and has no effect.
 
-    A common reason this may fail is when one is using an image which
-    requires registry authentication, but not embedding the pull secret
-    in the image so that updates can be fetched by the installed OS
-    \"day 2\".
+**\--run-fetch-check **
+
+:   Verify the target image can be pulled using the bootc image.
+
+    This will ensure the bootc system can be upgraded,
+    i.e. the registry credentials are available on the bootc image.
 
 **\--disable-selinux**
 

docs/src/man/bootc-install-to-existing-root.md

@@ -7,7 +7,7 @@ bootc-install-to-existing-root - Install to the host root filesystem
 **bootc install to-existing-root** \[**\--replace**\]
 \[**\--source-imgref**\] \[**\--target-transport**\]
 \[**\--target-imgref**\] \[**\--enforce-container-sigpolicy**\]
-\[**\--skip-fetch-check**\] \[**\--disable-selinux**\] \[**\--karg**\]
+\[**\--skip-fetch-check**\] \[**\--run-fetch-check**\]
 \[**\--root-ssh-authorized-keys**\] \[**\--generic-image**\]
 \[**\--bound-images**\] \[**\--stateroot**\]
 \[**\--acknowledge-destructive**\] \[**-h**\|**\--help**\]
@@ -68,17 +68,16 @@ cleaned up if desired when rebooted into the new root.
     Enabling this option enforces that \`/etc/containers/policy.json\`
     includes a default policy which requires signatures
 
-**\--skip-fetch-check**
+**\--skip-fetch-check (deprecated, see --run-fetch-check)**
 
-:   By default, the accessiblity of the target image will be verified
-    (just the manifest will be fetched). Specifying this option
-    suppresses the check; use this when you know the issues it might
-    find are addressed.
+:   This is now the default and has no effect.
 
-    A common reason this may fail is when one is using an image which
-    requires registry authentication, but not embedding the pull secret
-    in the image so that updates can be fetched by the installed OS
-    \"day 2\".
+**\--run-fetch-check**
+
+:   Verify the target image can be pulled using the bootc image.
+
+    This will ensure the bootc system can be upgraded,
+    i.e. the registry credentials are available on the bootc image.
 
 **\--disable-selinux**
 

docs/src/man/bootc-install-to-filesystem.md

@@ -10,7 +10,8 @@ filesystem structure
 \[**\--acknowledge-destructive**\] \[**\--skip-finalize**\]
 \[**\--source-imgref**\] \[**\--target-transport**\]
 \[**\--target-imgref**\] \[**\--enforce-container-sigpolicy**\]
-\[**\--skip-fetch-check**\] \[**\--disable-selinux**\] \[**\--karg**\]
+\[**\--skip-fetch-check**\] \[**\--run-fetch-check**\]
+\[**\--disable-selinux**\] \[**\--karg**\]
 \[**\--root-ssh-authorized-keys**\] \[**\--generic-image**\]
 \[**\--bound-images**\] \[**\--stateroot**\] \[**-h**\|**\--help**\]
 \<*ROOT_PATH*\>
@@ -97,17 +98,16 @@ is currently expected to be empty by default.
     Enabling this option enforces that \`/etc/containers/policy.json\`
     includes a default policy which requires signatures
 
-**\--skip-fetch-check**
+**\--skip-fetch-check (deprecated, see --run-fetch-check)**
 
-:   By default, the accessiblity of the target image will be verified
-    (just the manifest will be fetched). Specifying this option
-    suppresses the check; use this when you know the issues it might
-    find are addressed.
+:   This is now the default and has no effect.
 
-    A common reason this may fail is when one is using an image which
-    requires registry authentication, but not embedding the pull secret
-    in the image so that updates can be fetched by the installed OS
-    \"day 2\".
+**\--run-fetch-check**
+
+:   Verify the target image can be pulled using the bootc image.
+
+    This will ensure the bootc system can be upgraded,
+    i.e. the registry credentials are available on the bootc image.
 
 **\--disable-selinux**
 

hack/lldb/deploy.sh

@@ -11,7 +11,7 @@ sudo podman build --build-arg "sshpubkey=$(cat ~/.ssh/id_rsa.pub)" -f Containerf
 mkdir -p ~/.cache/bootc-dev/disks
 rm -f ~/.cache/bootc-dev/disks/lldb.raw
 truncate -s 10G ~/.cache/bootc-dev/disks/lldb.raw
-sudo podman run --pid=host --network=host --privileged --security-opt label=type:unconfined_t -v ~/.cache/bootc-dev/disks:/output localhost/bootc-lldb bootc install to-disk --via-loopback --generic-image --skip-fetch-check /output/lldb.raw
+sudo podman run --pid=host --network=host --privileged --security-opt label=type:unconfined_t -v ~/.cache/bootc-dev/disks:/output localhost/bootc-lldb bootc install to-disk --via-loopback --generic-image /output/lldb.raw
 
 # create a new VM in libvirt
 set +e

lib/src/install.rs

@@ -124,12 +124,14 @@ pub(crate) struct InstallTargetOpts {
     #[serde(default)]
     pub(crate) enforce_container_sigpolicy: bool,
 
-    /// By default, the accessiblity of the target image will be verified (just the manifest will be fetched).
-    /// Specifying this option suppresses the check; use this when you know the issues it might find
-    /// are addressed.
-    ///
-    /// A common reason this may fail is when one is using an image which requires registry authentication,
-    /// but not embedding the pull secret in the image so that updates can be fetched by the installed OS "day 2".
+    /// Verify the image can be fetched from the bootc image. Updates may fail when the installation
+    /// host is authenticated with the registry but the pull secret is not in the bootc image.
+    #[clap(long)]
+    #[serde(default)]
+    pub(crate) run_fetch_check: bool,
+
+    /// Verify the image can be fetched from the bootc image. Updates may fail when the installation
+    /// host is authenticated with the registry but the pull secret is not in the bootc image.
     #[clap(long)]
     #[serde(default)]
     pub(crate) skip_fetch_check: bool,
@@ -1287,7 +1289,7 @@ async fn prepare_install(
     // And continue to init global state
     osbuild::adjust_for_bootc_image_builder(&rootfs, &tempdir)?;
 
-    if !target_opts.skip_fetch_check {
+    if target_opts.run_fetch_check {
         verify_target_fetch(&tempdir, &target_imgref).await?;
     }
 

tests-integration/src/hostpriv.rs

@@ -23,7 +23,11 @@ fn test_loopback_install(image: &'static str) -> Result<()> {
     tmpdisk.as_file_mut().set_len(size)?;
     let tmpdisk = tmpdisk.into_temp_path();
     let tmpdisk = tmpdisk.to_str().unwrap();
-    cmd!(sh, "sudo {base_args...} -v {tmpdisk}:/disk {image} bootc install to-disk --via-loopback --skip-fetch-check /disk").run()?;
+    cmd!(
+        sh,
+        "sudo {base_args...} -v {tmpdisk}:/disk {image} bootc install to-disk --via-loopback /disk"
+    )
+    .run()?;
     Ok(())
 }
 

tests-integration/src/install.rs

@@ -76,8 +76,6 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
     // Handy defaults
 
     let target_args = &["-v", "/:/target"];
-    // We always need this as we assume we're operating on a local image
-    let generic_inst_args = ["--skip-fetch-check"];
 
     let tests = [
         Trial::test("loopback install", move || {
@@ -88,7 +86,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
             tmpdisk.as_file_mut().set_len(size)?;
             let tmpdisk = tmpdisk.into_temp_path();
             let tmpdisk = tmpdisk.to_str().unwrap();
-            cmd!(sh, "sudo {BASE_ARGS...} -v {tmpdisk}:/disk {image} bootc install to-disk --via-loopback {generic_inst_args...} /disk").run()?;
+            cmd!(sh, "sudo {BASE_ARGS...} -v {tmpdisk}:/disk {image} bootc install to-disk --via-loopback /disk").run()?;
             Ok(())
         }),
         Trial::test(
@@ -100,7 +98,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
                 let tmp_keys = tmpd.path().join("test_authorized_keys");
                 let tmp_keys = tmp_keys.to_str().unwrap();
                 std::fs::write(&tmp_keys, b"ssh-ed25519 ABC0123 [email protected]")?;
-                cmd!(sh, "sudo {BASE_ARGS...} {target_args...} -v {tmp_keys}:/test_authorized_keys {image} bootc install to-filesystem {generic_inst_args...} --acknowledge-destructive --karg=foo=bar --replace=alongside --root-ssh-authorized-keys=/test_authorized_keys /target").run()?;
+                cmd!(sh, "sudo {BASE_ARGS...} {target_args...} -v {tmp_keys}:/test_authorized_keys {image} bootc install to-filesystem --acknowledge-destructive --karg=foo=bar --replace=alongside --root-ssh-authorized-keys=/test_authorized_keys /target").run()?;
 
                 // Also test install finalize here
                 cmd!(
@@ -142,7 +140,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
         Trial::test("Install and verify selinux state", move || {
             let sh = &xshell::Shell::new()?;
             reset_root(sh, image)?;
-            cmd!(sh, "sudo {BASE_ARGS...} {image} bootc install to-existing-root --acknowledge-destructive {generic_inst_args...}").run()?;
+            cmd!(sh, "sudo {BASE_ARGS...} {image} bootc install to-existing-root --acknowledge-destructive").run()?;
             generic_post_install_verification()?;
             let root = &Dir::open_ambient_dir("/ostree", cap_std::ambient_authority()).unwrap();
             crate::selinux::verify_selinux_recurse(root, false)?;
@@ -151,7 +149,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
         Trial::test("Install to non-default stateroot", move || {
             let sh = &xshell::Shell::new()?;
             reset_root(sh, image)?;
-            cmd!(sh, "sudo {BASE_ARGS...} {image} bootc install to-existing-root --stateroot {NON_DEFAULT_STATEROOT} --acknowledge-destructive {generic_inst_args...}").run()?;
+            cmd!(sh, "sudo {BASE_ARGS...} {image} bootc install to-existing-root --stateroot {NON_DEFAULT_STATEROOT} --acknowledge-destructive").run()?;
             generic_post_install_verification()?;
             assert!(
                 Utf8Path::new(&format!("/ostree/deploy/{NON_DEFAULT_STATEROOT}")).try_exists()?
@@ -163,7 +161,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
             reset_root(sh, image)?;
             let empty = sh.create_temp_dir()?;
             let empty = empty.path().to_str().unwrap();
-            cmd!(sh, "sudo {BASE_ARGS...} -v {empty}:/usr/lib/bootc/install {image} bootc install to-existing-root {generic_inst_args...}").run()?;
+            cmd!(sh, "sudo {BASE_ARGS...} -v {empty}:/usr/lib/bootc/install {image} bootc install to-existing-root").run()?;
             generic_post_install_verification()?;
             Ok(())
         }),