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

Port to ocidir 0.3, containers-image-proxy 0.7 #663

Merged
merged 1 commit into from
Sep 19, 2024
Merged
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
4 changes: 2 additions & 2 deletions lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ rust-version = "1.74.0"
# Note that we re-export the oci-spec types
# that are exported by this crate, so when bumping
# semver here you must also bump our semver.
containers-image-proxy = "0.6.0"
containers-image-proxy = "0.7.0"
# We re-export this library too.
ostree = { features = ["v2022_6"], version = "0.19.0" }

Expand All @@ -36,7 +36,7 @@ once_cell = "1.9"
libc = "0.2.92"
libsystemd = "0.7.0"
openssl = "0.10.33"
ocidir = "0.2.0"
ocidir = "0.3.0"
pin-project = "1.0"
regex = "1.5.4"
rustix = { version = "0.38", features = ["fs", "process"] }
Expand Down
13 changes: 9 additions & 4 deletions lib/src/chunking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,12 @@ fn basic_packing<'a>(
mod test {
use super::*;

use oci_spec::image as oci_image;
use std::str::FromStr;

const FCOS_CONTENTMETA: &[u8] = include_bytes!("fixtures/fedora-coreos-contentmeta.json.gz");
const SHA256_EXAMPLE: &str =
"sha256:0000111122223333444455556666777788889999aaaabbbbccccddddeeeeffff";

#[test]
fn test_packing_basics() -> Result<()> {
Expand Down Expand Up @@ -838,8 +843,8 @@ mod test {

let config = oci_spec::image::DescriptorBuilder::default()
.media_type(oci_spec::image::MediaType::ImageConfig)
.size(7023)
.digest("sha256:imageconfig")
.size(7023_u64)
.digest(oci_image::Digest::from_str(SHA256_EXAMPLE).unwrap())
.build()
.expect("build config descriptor");

Expand All @@ -850,8 +855,8 @@ mod test {
let sep = COMPONENT_SEPARATOR.encode_utf8(&mut buf);
oci_spec::image::DescriptorBuilder::default()
.media_type(oci_spec::image::MediaType::ImageLayerGzip)
.size(100)
.digest(format!("sha256:{}", l.len()))
.size(100_u64)
.digest(oci_image::Digest::from_str(SHA256_EXAMPLE).unwrap())
.annotations(HashMap::from([(
CONTENT_ANNOTATION.to_string(),
l.join(sep),
Expand Down
13 changes: 9 additions & 4 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,9 +594,14 @@ pub fn layer_progress_format(p: &ImportProgress) -> String {
ImportProgress::DerivedLayerCompleted(v) => (false, "layer", v),
};
// podman outputs 12 characters of digest, let's add 7 for `sha256:`.
let short_digest = layer.digest().chars().take(12 + 7).collect::<String>();
let short_digest = layer
.digest()
.digest()
.chars()
.take(12 + 7)
.collect::<String>();
if starting {
let size = glib::format_size(layer.size() as u64);
let size = glib::format_size(layer.size());
format!("Fetching {s} {short_digest} ({size})")
} else {
format!("Fetched {s} {short_digest}")
Expand Down Expand Up @@ -934,13 +939,13 @@ async fn container_history(repo: &ostree::Repo, imgref: &ImageReference) -> Resu

let mut remaining = width;

let digest = layer.digest().as_str();
let digest = layer.digest().digest();
// Verify it's OK to slice, this should all be ASCII
assert!(digest.is_ascii());
let digest_max = columns[0].1;
let digest = &digest[0..digest_max as usize];
print_column(digest, digest_max, &mut remaining);
let size = glib::format_size(layer.size() as u64);
let size = glib::format_size(layer.size());
print_column(size.as_str(), columns[1].1, &mut remaining);
print_column(created_by, columns[2].1, &mut remaining);
println!();
Expand Down
9 changes: 6 additions & 3 deletions lib/src/container/encapsulate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,10 @@ pub(crate) fn export_chunked(

// This label (mentioned above) points to the last layer that is part of
// the ostree commit.
labels.insert(DIFFID_LABEL.into(), format!("sha256:{}", last_digest));
labels.insert(
DIFFID_LABEL.into(),
format!("sha256:{}", last_digest.digest()),
);
Ok(())
}

Expand Down Expand Up @@ -300,7 +303,7 @@ async fn build_impl(
config: &Config,
opts: Option<ExportOpts<'_, '_>>,
dest: &ImageReference,
) -> Result<String> {
) -> Result<oci_image::Digest> {
let mut opts = opts.unwrap_or_default();
if dest.transport == Transport::ContainerStorage {
opts.skip_compression = true;
Expand Down Expand Up @@ -407,7 +410,7 @@ pub async fn encapsulate<S: AsRef<str>>(
config: &Config,
opts: Option<ExportOpts<'_, '_>>,
dest: &ImageReference,
) -> Result<String> {
) -> Result<oci_image::Digest> {
build_impl(repo, ostree_ref.as_ref(), config, opts, dest).await
}

Expand Down
10 changes: 5 additions & 5 deletions lib/src/container/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,12 +341,12 @@ impl<'a> ManifestDiff<'a> {
let src_layers = src
.layers()
.iter()
.map(|l| (l.digest(), l))
.map(|l| (l.digest().digest(), l))
.collect::<HashMap<_, _>>();
let dest_layers = dest
.layers()
.iter()
.map(|l| (l.digest(), l))
.map(|l| (l.digest().digest(), l))
.collect::<HashMap<_, _>>();
let mut removed = Vec::new();
let mut added = Vec::new();
Expand All @@ -355,16 +355,16 @@ impl<'a> ManifestDiff<'a> {
removed.push(descriptor);
}
}
removed.sort_by(|a, b| a.digest().cmp(b.digest()));
removed.sort_by(|a, b| a.digest().digest().cmp(b.digest().digest()));
for (blobid, &descriptor) in dest_layers.iter() {
if !src_layers.contains_key(blobid) {
added.push(descriptor);
}
}
added.sort_by(|a, b| a.digest().cmp(b.digest()));
added.sort_by(|a, b| a.digest().digest().cmp(b.digest().digest()));

fn layersum<'a, I: Iterator<Item = &'a oci_spec::image::Descriptor>>(layers: I) -> u64 {
layers.map(|layer| layer.size() as u64).sum()
layers.map(|layer| layer.size()).sum()
}
let total = dest_layers.len() as u64;
let total_size = layersum(dest.layers().iter());
Expand Down
6 changes: 4 additions & 2 deletions lib/src/container/skopeo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
use super::ImageReference;
use anyhow::{Context, Result};
use cap_std_ext::cmdext::CapStdExtCommandExt;
use containers_image_proxy::oci_spec::image as oci_image;
use fn_error_context::context;
use io_lifetimes::OwnedFd;
use serde::Deserialize;
use std::io::Read;
use std::path::Path;
use std::process::Stdio;
use std::str::FromStr;
use tokio::process::Command;

// See `man containers-policy.json` and
Expand Down Expand Up @@ -68,7 +70,7 @@ pub(crate) async fn copy(
authfile: Option<&Path>,
add_fd: Option<(std::sync::Arc<OwnedFd>, i32)>,
progress: bool,
) -> Result<String> {
) -> Result<oci_image::Digest> {
let digestfile = tempfile::NamedTempFile::new()?;
let mut cmd = new_cmd();
cmd.arg("copy");
Expand Down Expand Up @@ -96,7 +98,7 @@ pub(crate) async fn copy(
let mut digestfile = digestfile.into_file();
let mut r = String::new();
digestfile.read_to_string(&mut r)?;
Ok(r.trim().to_string())
Ok(oci_image::Digest::from_str(r.trim())?)
}

#[cfg(test)]
Expand Down
16 changes: 8 additions & 8 deletions lib/src/container/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ fn ref_for_blob_digest(d: &str) -> Result<String> {

/// Convert e.g. sha256:12345... into `/ostree/container/blob/sha256_2B12345...`.
fn ref_for_layer(l: &oci_image::Descriptor) -> Result<String> {
ref_for_blob_digest(l.digest().as_str())
ref_for_blob_digest(l.digest().digest())
}

/// Convert e.g. sha256:12345... into `/ostree/container/blob/sha256_2B12345...`.
Expand Down Expand Up @@ -202,12 +202,12 @@ pub struct ManifestLayerState {
impl ManifestLayerState {
/// The cryptographic checksum.
pub fn digest(&self) -> &str {
self.layer.digest().as_str()
self.layer.digest().digest()
}

/// The (possibly compressed) size.
pub fn size(&self) -> u64 {
self.layer.size() as u64
self.layer.size()
}
}

Expand Down Expand Up @@ -638,7 +638,7 @@ impl ImageImporter {
}

let (manifest_digest, manifest) = self.proxy.fetch_manifest(&self.proxy_img).await?;
let new_imageid = manifest.config().digest().as_str();
let new_imageid = manifest.config().digest().digest();

// Query for previous stored state

Expand All @@ -649,7 +649,7 @@ impl ImageImporter {
return Ok(PrepareResult::AlreadyPresent(previous_state));
}
// Failing that, if they have the same imageID, we're also done.
let previous_imageid = previous_state.manifest.config().digest().as_str();
let previous_imageid = previous_state.manifest.config().digest().digest();
if previous_imageid == new_imageid {
return Ok(PrepareResult::AlreadyPresent(previous_state));
}
Expand Down Expand Up @@ -1355,7 +1355,7 @@ pub(crate) fn export_to_oci(
let new_config = dest_oci.write_config(new_config)?;
new_manifest.set_config(new_config);

dest_oci.insert_manifest(new_manifest, tag, oci_image::Platform::default())
Ok(dest_oci.insert_manifest(new_manifest, tag, oci_image::Platform::default())?)
}

/// Given a container image reference which is stored in `repo`, export it to the
Expand All @@ -1366,7 +1366,7 @@ pub async fn export(
src_imgref: &ImageReference,
dest_imgref: &ImageReference,
opts: Option<ExportToOCIOpts>,
) -> Result<String> {
) -> Result<oci_image::Digest> {
let opts = opts.unwrap_or_default();
let target_oci = dest_imgref.transport == Transport::OciDir;
let tempdir = if !target_oci {
Expand Down Expand Up @@ -1476,7 +1476,7 @@ fn gc_image_layers_impl(
let mut referenced_layers = BTreeSet::new();
for m in all_manifests.iter() {
for layer in m.layers() {
referenced_layers.insert(layer.digest().as_str());
referenced_layers.insert(layer.digest().digest());
}
}
tracing::debug!("Referenced layers: {}", referenced_layers.len());
Expand Down
25 changes: 12 additions & 13 deletions lib/src/container/unencapsulate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,18 +101,18 @@ impl<T: AsyncRead> AsyncRead for ProgressReader<T> {
async fn fetch_manifest_impl(
proxy: &mut ImageProxy,
imgref: &OstreeImageReference,
) -> Result<(oci_spec::image::ImageManifest, String)> {
) -> Result<(oci_image::ImageManifest, oci_image::Digest)> {
let oi = &proxy.open_image(&imgref.imgref.to_string()).await?;
let (digest, manifest) = proxy.fetch_manifest(oi).await?;
proxy.close_image(oi).await?;
Ok((manifest, digest))
Ok((manifest, oci_image::Digest::from_str(digest.as_str())?))
}

/// Download the manifest for a target image and its sha256 digest.
#[context("Fetching manifest")]
pub async fn fetch_manifest(
imgref: &OstreeImageReference,
) -> Result<(oci_spec::image::ImageManifest, String)> {
) -> Result<(oci_image::ImageManifest, oci_image::Digest)> {
let mut proxy = ImageProxy::new().await?;
fetch_manifest_impl(&mut proxy, imgref).await
}
Expand All @@ -122,13 +122,14 @@ pub async fn fetch_manifest(
pub async fn fetch_manifest_and_config(
imgref: &OstreeImageReference,
) -> Result<(
oci_spec::image::ImageManifest,
String,
oci_spec::image::ImageConfiguration,
oci_image::ImageManifest,
oci_image::Digest,
oci_image::ImageConfiguration,
)> {
let proxy = ImageProxy::new().await?;
let oi = &proxy.open_image(&imgref.imgref.to_string()).await?;
let (digest, manifest) = proxy.fetch_manifest(oi).await?;
let digest = oci_image::Digest::from_str(&digest)?;
let config = proxy.fetch_config(oi).await?;
Ok((manifest, digest, config))
}
Expand Down Expand Up @@ -289,19 +290,17 @@ pub(crate) async fn fetch_layer_decompress<'a>(
})?;
size = layer_blob.size;
media_type = &layer_blob.media_type;
(blob, driver) = proxy
.get_blob(img, layer_blob.digest.as_str(), size as u64)
.await?;
(blob, driver) = proxy.get_blob(img, &layer_blob.digest, size).await?;
}
_ => {
size = layer.size();
media_type = layer.media_type();
(blob, driver) = proxy
.get_blob(img, layer.digest().as_str(), size as u64)
.await?;
(blob, driver) = proxy.get_blob(img, layer.digest(), size).await?;
}
};

let driver = async { driver.await.map_err(Into::into) };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems to be enough to accomodate the thiserror change, took me longer than I would care to admit to understand the compiler error before.

Copy link
Member

Choose a reason for hiding this comment

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

I struggled a lot with this when I was learning Rust for sure. The thing that is subtle is that the ? operator automatically also does Into::into, which means in cases where you're changing the error type (like this) but not using ? (which happens in most cases) you suddenly have to think about these conversions.


if let Some(progress) = progress {
let (readprogress, mut readwatch) = ProgressReader::new(blob);
let readprogress = tokio::io::BufReader::new(readprogress);
Expand All @@ -311,7 +310,7 @@ pub(crate) async fn fetch_layer_decompress<'a>(
let status = LayerProgress {
layer_index,
fetched: *fetched,
total: size as u64,
total: size,
};
progress.send_replace(Some(status));
}
Expand Down
28 changes: 18 additions & 10 deletions lib/src/container/update_detachedmeta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use anyhow::{anyhow, Context, Result};
use camino::Utf8Path;
use cap_std::fs::Dir;
use cap_std_ext::cap_std;
use containers_image_proxy::oci_spec;
use containers_image_proxy::oci_spec::image as oci_image;
use std::io::{BufReader, BufWriter};

/// Given an OSTree container image reference, update the detached metadata (e.g. GPG signature)
Expand All @@ -16,7 +16,7 @@ pub async fn update_detached_metadata(
src: &ImageReference,
dest: &ImageReference,
detached_buf: Option<&[u8]>,
) -> Result<String> {
) -> Result<oci_image::Digest> {
// For now, convert the source to a temporary OCI directory, so we can directly
// parse and manipulate it. In the future this will be replaced by https://github.com/ostreedev/ostree-rs-ext/issues/153
// and other work to directly use the containers/image API via containers-image-proxy.
Expand All @@ -29,7 +29,7 @@ pub async fn update_detached_metadata(
};

// Full copy of the source image
let pulled_digest: String = skopeo::copy(src, &tempsrc_ref, None, None, false)
let pulled_digest = skopeo::copy(src, &tempsrc_ref, None, None, false)
.await
.context("Creating temporary copy to OCI dir")?;

Expand All @@ -44,16 +44,24 @@ pub async fn update_detached_metadata(
let tempsrc = ocidir::OciDir::open(&tempsrc)?;

// Load the manifest, platform, and config
let (mut manifest, manifest_descriptor) = tempsrc
.read_manifest_and_descriptor()
.context("Reading manifest from source")?;
anyhow::ensure!(manifest_descriptor.digest().as_str() == pulled_digest.as_str());
let idx = tempsrc
.read_index()?
.ok_or(anyhow!("Reading image index from source"))?;
let manifest_descriptor = idx
.manifests()
.first()
.ok_or(anyhow!("No manifests in index"))?;
let mut manifest: oci_image::ImageManifest = tempsrc
.read_json_blob(manifest_descriptor)
.context("Reading manifest json blob")?;

anyhow::ensure!(manifest_descriptor.digest().digest() == pulled_digest.digest());
let platform = manifest_descriptor
.platform()
.as_ref()
.cloned()
.unwrap_or_default();
let mut config: oci_spec::image::ImageConfiguration =
let mut config: oci_image::ImageConfiguration =
tempsrc.read_json_blob(manifest.config())?;
let mut ctrcfg = config
.config()
Expand Down Expand Up @@ -91,10 +99,10 @@ pub async fn update_detached_metadata(
.complete()?
};
// Get the diffid and descriptor for our new tar layer
let out_layer_diffid = format!("sha256:{}", out_layer.uncompressed_sha256);
let out_layer_diffid = format!("sha256:{}", out_layer.uncompressed_sha256.digest());
let out_layer_descriptor = out_layer
.descriptor()
.media_type(oci_spec::image::MediaType::ImageLayerGzip)
.media_type(oci_image::MediaType::ImageLayerGzip)
.build()
.unwrap(); // SAFETY: We pass all required fields

Expand Down
Loading
Loading