Skip to content

Commit 8e8e2a0

Browse files
committed
install: Drop support for old skopeo
Let's just hard require a skopeo that can fetch from `containers-storage`. Motivated by #263 which was moving this code around. Signed-off-by: Colin Walters <[email protected]>
1 parent 7c6b282 commit 8e8e2a0

File tree

1 file changed

+41
-82
lines changed

1 file changed

+41
-82
lines changed

lib/src/install.rs

Lines changed: 41 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,7 @@ pub(crate) struct SourceInfo {
245245
/// Whether or not SELinux appears to be enabled in the source commit
246246
pub(crate) selinux: bool,
247247
/// Whether the source is available in the host mount namespace
248-
pub(crate) in_host_mountns: Option<HostMountnsInfo>,
249-
}
250-
251-
/// Information about the host mount namespace
252-
#[derive(Debug, Clone)]
253-
pub(crate) struct HostMountnsInfo {
254-
/// True if the skoepo on host supports containers-storage:
255-
pub(crate) skopeo_supports_containers_storage: bool,
248+
pub(crate) in_host_mountns: bool,
256249
}
257250

258251
// Shared read-only global state
@@ -395,27 +388,23 @@ impl SourceInfo {
395388
};
396389
let digest = crate::podman::imageid_to_digest(&container_info.imageid)?;
397390

398-
let skopeo_supports_containers_storage = skopeo_supports_containers_storage()
399-
.context("Failed to run skopeo (it currently must be installed in the host root)")?;
400-
Self::from(
401-
imageref,
402-
Some(digest),
403-
Some(HostMountnsInfo {
404-
skopeo_supports_containers_storage,
405-
}),
406-
)
391+
// Verify up front we can do the fetch
392+
require_skopeo_with_containers_storage()?;
393+
394+
Self::new(imageref, Some(digest), true)
407395
}
408396

409397
#[context("Creating source info from a given imageref")]
410398
pub(crate) fn from_imageref(imageref: &str) -> Result<Self> {
411399
let imageref = ostree_container::ImageReference::try_from(imageref)?;
412-
Self::from(imageref, None, None)
400+
Self::new(imageref, None, false)
413401
}
414402

415-
fn from(
403+
/// Construct a new source information structure
404+
fn new(
416405
imageref: ostree_container::ImageReference,
417406
digest: Option<String>,
418-
in_host_mountns: Option<HostMountnsInfo>,
407+
in_host_mountns: bool,
419408
) -> Result<Self> {
420409
let cancellable = ostree::gio::Cancellable::NONE;
421410
let commit = Task::new("Reading ostree commit", "ostree")
@@ -612,39 +601,31 @@ async fn initialize_ostree_root_from_self(
612601
let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(rootfs)));
613602
sysroot.load(cancellable)?;
614603

615-
let mut temporary_dir = None;
616-
let (src_imageref, proxy_cfg) = match &state.source.in_host_mountns {
617-
None => (state.source.imageref.clone(), None),
618-
Some(host_mountns_info) => {
619-
let src_imageref = if host_mountns_info.skopeo_supports_containers_storage {
620-
// We always use exactly the digest of the running image to ensure predictability.
621-
let digest = state
622-
.source
623-
.digest
624-
.as_ref()
625-
.ok_or_else(|| anyhow::anyhow!("Missing container image digest"))?;
626-
let spec = crate::utils::digested_pullspec(&state.source.imageref.name, digest);
627-
ostree_container::ImageReference {
628-
transport: ostree_container::Transport::ContainerStorage,
629-
name: spec,
630-
}
631-
} else {
632-
let td = tempfile::tempdir_in("/var/tmp")?;
633-
let path: &Utf8Path = td.path().try_into().unwrap();
634-
let r = copy_to_oci(&state.source.imageref, path)?;
635-
temporary_dir = Some(td);
636-
r
637-
};
604+
let (src_imageref, proxy_cfg) = if !state.source.in_host_mountns {
605+
(state.source.imageref.clone(), None)
606+
} else {
607+
let src_imageref = {
608+
// We always use exactly the digest of the running image to ensure predictability.
609+
let digest = state
610+
.source
611+
.digest
612+
.as_ref()
613+
.ok_or_else(|| anyhow::anyhow!("Missing container image digest"))?;
614+
let spec = crate::utils::digested_pullspec(&state.source.imageref.name, digest);
615+
ostree_container::ImageReference {
616+
transport: ostree_container::Transport::ContainerStorage,
617+
name: spec,
618+
}
619+
};
638620

639-
// We need to fetch the container image from the root mount namespace
640-
let skopeo_cmd = run_in_host_mountns("skopeo");
641-
let proxy_cfg = ostree_container::store::ImageProxyConfig {
642-
skopeo_cmd: Some(skopeo_cmd),
643-
..Default::default()
644-
};
621+
// We need to fetch the container image from the root mount namespace
622+
let skopeo_cmd = run_in_host_mountns("skopeo");
623+
let proxy_cfg = ostree_container::store::ImageProxyConfig {
624+
skopeo_cmd: Some(skopeo_cmd),
625+
..Default::default()
626+
};
645627

646-
(src_imageref, Some(proxy_cfg))
647-
}
628+
(src_imageref, Some(proxy_cfg))
648629
};
649630
let src_imageref = ostree_container::OstreeImageReference {
650631
// There are no signatures to verify since we're fetching the already
@@ -671,8 +652,6 @@ async fn initialize_ostree_root_from_self(
671652
println!("Installed: {target_image}");
672653
println!(" Digest: {digest}");
673654

674-
drop(temporary_dir);
675-
676655
// Write the entry for /boot to /etc/fstab. TODO: Encourage OSes to use the karg?
677656
// Or better bind this with the grub data.
678657
sysroot.load(cancellable)?;
@@ -717,32 +696,6 @@ async fn initialize_ostree_root_from_self(
717696
Ok(aleph)
718697
}
719698

720-
#[context("Copying to oci")]
721-
fn copy_to_oci(
722-
src_imageref: &ostree_container::ImageReference,
723-
dir: &Utf8Path,
724-
) -> Result<ostree_container::ImageReference> {
725-
tracing::debug!("Copying {src_imageref}");
726-
let src_imageref = src_imageref.to_string();
727-
let dest_imageref = ostree_container::ImageReference {
728-
transport: ostree_container::Transport::OciDir,
729-
name: dir.to_string(),
730-
};
731-
let dest_imageref_str = dest_imageref.to_string();
732-
Task::new_cmd(
733-
"Copying to temporary OCI (skopeo is too old)",
734-
run_in_host_mountns("skopeo"),
735-
)
736-
.args([
737-
"copy",
738-
// TODO: enable this once ostree is fixed "--dest-oci-accept-uncompressed-layers",
739-
src_imageref.as_str(),
740-
dest_imageref_str.as_str(),
741-
])
742-
.run()?;
743-
Ok(dest_imageref)
744-
}
745-
746699
/// Run a command in the host mount namespace
747700
pub(crate) fn run_in_host_mountns(cmd: &str) -> Command {
748701
let mut c = Command::new("/proc/self/exe");
@@ -769,11 +722,12 @@ pub(crate) fn exec_in_host_mountns(args: &[std::ffi::OsString]) -> Result<()> {
769722
}
770723

771724
#[context("Querying skopeo version")]
772-
fn skopeo_supports_containers_storage() -> Result<bool> {
725+
fn require_skopeo_with_containers_storage() -> Result<()> {
773726
let out = Task::new_cmd("skopeo --version", run_in_host_mountns("skopeo"))
774727
.args(["--version"])
775728
.quiet()
776-
.read()?;
729+
.read()
730+
.context("Failed to run skopeo (it currently must be installed in the host root)")?;
777731
let mut v = out
778732
.strip_prefix("skopeo version ")
779733
.map(|v| v.split('.'))
@@ -785,7 +739,12 @@ fn skopeo_supports_containers_storage() -> Result<bool> {
785739
.next()
786740
.ok_or_else(|| anyhow::anyhow!("Missing minor version"))?;
787741
let (major, minor) = (major.parse::<u64>()?, minor.parse::<u64>()?);
788-
Ok(major > 1 || minor > 10)
742+
let supported = major > 1 || minor > 10;
743+
if supported {
744+
Ok(())
745+
} else {
746+
anyhow::bail!("skopeo >= 1.11 is required on host")
747+
}
789748
}
790749

791750
pub(crate) struct RootSetup {

0 commit comments

Comments
 (0)