Skip to content

Commit 1770055

Browse files
committed
store: Support importing images without /ostree
A sticking point keeping ostree in the picture here for containers was SELinux handling. When we started this effort I'd feared rewriting. But recently we changed things such that we label derived images using the policy from the final root. This is a relatively small change in code size and complexity, that allows us to import images that don't have "ostree stuff" in them at all, i.e. there's no `/ostree/repo/objects`. The advantage here is that this significantly simplifies constructing base images. The main disadvantage today for people who build images this way is that we end up re-labeling and re-checksumming all objects. But, the real fix for that in the future will be for us to rework things such that we support `security.selinux` for example as native xattrs in the tar stream. Signed-off-by: Colin Walters <[email protected]>
1 parent 680b29f commit 1770055

File tree

4 files changed

+242
-61
lines changed

4 files changed

+242
-61
lines changed

ostree-ext/src/container/store.rs

Lines changed: 77 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -227,15 +227,16 @@ pub struct PreparedImport {
227227
/// The layers containing split objects
228228
pub ostree_layers: Vec<ManifestLayerState>,
229229
/// The layer for the ostree commit.
230-
pub ostree_commit_layer: ManifestLayerState,
230+
pub ostree_commit_layer: Option<ManifestLayerState>,
231231
/// Any further non-ostree (derived) layers.
232232
pub layers: Vec<ManifestLayerState>,
233233
}
234234

235235
impl PreparedImport {
236236
/// Iterate over all layers; the commit layer, the ostree split object layers, and any non-ostree layers.
237237
pub fn all_layers(&self) -> impl Iterator<Item = &ManifestLayerState> {
238-
std::iter::once(&self.ostree_commit_layer)
238+
self.ostree_commit_layer
239+
.iter()
239240
.chain(self.ostree_layers.iter())
240241
.chain(self.layers.iter())
241242
}
@@ -376,21 +377,20 @@ fn layer_from_diffid<'a>(
376377
pub(crate) fn parse_manifest_layout<'a>(
377378
manifest: &'a ImageManifest,
378379
config: &ImageConfiguration,
379-
) -> Result<(&'a Descriptor, Vec<&'a Descriptor>, Vec<&'a Descriptor>)> {
380+
) -> Result<(
381+
Option<&'a Descriptor>,
382+
Vec<&'a Descriptor>,
383+
Vec<&'a Descriptor>,
384+
)> {
380385
let config_labels = super::labels_of(config);
381386

382387
let first_layer = manifest
383388
.layers()
384389
.first()
385390
.ok_or_else(|| anyhow!("No layers in manifest"))?;
386-
let target_diffid = config_labels
387-
.and_then(|labels| labels.get(DIFFID_LABEL))
388-
.ok_or_else(|| {
389-
anyhow!(
390-
"No {} label found, not an ostree encapsulated container",
391-
DIFFID_LABEL
392-
)
393-
})?;
391+
let Some(target_diffid) = config_labels.and_then(|labels| labels.get(DIFFID_LABEL)) else {
392+
return Ok((None, Vec::new(), manifest.layers().iter().collect()));
393+
};
394394

395395
let target_layer = layer_from_diffid(manifest, config, target_diffid.as_str())?;
396396
let mut chunk_layers = Vec::new();
@@ -416,7 +416,20 @@ pub(crate) fn parse_manifest_layout<'a>(
416416
}
417417
}
418418

419-
Ok((ostree_layer, chunk_layers, derived_layers))
419+
Ok((Some(ostree_layer), chunk_layers, derived_layers))
420+
}
421+
422+
/// Like [`parse_manifest_layout`] but requires the image has an ostree base.
423+
#[context("Parsing manifest layout")]
424+
pub(crate) fn parse_ostree_manifest_layout<'a>(
425+
manifest: &'a ImageManifest,
426+
config: &ImageConfiguration,
427+
) -> Result<(&'a Descriptor, Vec<&'a Descriptor>, Vec<&'a Descriptor>)> {
428+
let (ostree_layer, component_layers, derived_layers) = parse_manifest_layout(manifest, config)?;
429+
let ostree_layer = ostree_layer.ok_or_else(|| {
430+
anyhow!("No {DIFFID_LABEL} label found, not an ostree encapsulated container")
431+
})?;
432+
Ok((ostree_layer, component_layers, derived_layers))
420433
}
421434

422435
/// Find the timestamp of the manifest (or config), ignoring errors.
@@ -601,10 +614,10 @@ impl ImageImporter {
601614
parse_manifest_layout(&manifest, &config)?;
602615

603616
let query = |l: &Descriptor| query_layer(&self.repo, l.clone());
604-
let commit_layer = query(commit_layer)?;
617+
let commit_layer = commit_layer.map(query).transpose()?;
605618
let component_layers = component_layers
606619
.into_iter()
607-
.map(query)
620+
.map(|l| query(l))
608621
.collect::<Result<Vec<_>>>()?;
609622
let remaining_layers = remaining_layers
610623
.into_iter()
@@ -693,6 +706,7 @@ impl ImageImporter {
693706
pub(crate) async fn unencapsulate_base(
694707
&mut self,
695708
import: &mut store::PreparedImport,
709+
require_ostree: bool,
696710
write_refs: bool,
697711
) -> Result<()> {
698712
tracing::debug!("Fetching base");
@@ -707,6 +721,14 @@ impl ImageImporter {
707721
None
708722
}
709723
};
724+
let Some(commit_layer) = import.ostree_commit_layer.as_mut() else {
725+
if require_ostree {
726+
anyhow::bail!(
727+
"No {DIFFID_LABEL} label found, not an ostree encapsulated container"
728+
);
729+
}
730+
return Ok(());
731+
};
710732
let des_layers = self.proxy.get_layer_info(&self.proxy_img).await?;
711733
for layer in import.ostree_layers.iter_mut() {
712734
if layer.commit.is_some() {
@@ -755,25 +777,25 @@ impl ImageImporter {
755777
.await?;
756778
}
757779
}
758-
if import.ostree_commit_layer.commit.is_none() {
780+
if commit_layer.commit.is_none() {
759781
if let Some(p) = self.layer_progress.as_ref() {
760782
p.send(ImportProgress::OstreeChunkStarted(
761-
import.ostree_commit_layer.layer.clone(),
783+
commit_layer.layer.clone(),
762784
))
763785
.await?;
764786
}
765787
let (blob, driver, media_type) = fetch_layer(
766788
&self.proxy,
767789
&self.proxy_img,
768790
&import.manifest,
769-
&import.ostree_commit_layer.layer,
791+
&commit_layer.layer,
770792
self.layer_byte_progress.as_ref(),
771793
des_layers.as_ref(),
772794
self.imgref.imgref.transport,
773795
)
774796
.await?;
775797
let repo = self.repo.clone();
776-
let target_ref = import.ostree_commit_layer.ostree_ref.clone();
798+
let target_ref = commit_layer.ostree_ref.clone();
777799
let import_task =
778800
crate::tokio_util::spawn_blocking_cancellable_flatten(move |cancellable| {
779801
let txn = repo.auto_transaction(Some(cancellable))?;
@@ -792,10 +814,10 @@ impl ImageImporter {
792814
Ok::<_, anyhow::Error>(commit)
793815
});
794816
let commit = super::unencapsulate::join_fetch(import_task, driver).await?;
795-
import.ostree_commit_layer.commit = Some(commit);
817+
commit_layer.commit = Some(commit);
796818
if let Some(p) = self.layer_progress.as_ref() {
797819
p.send(ImportProgress::OstreeChunkCompleted(
798-
import.ostree_commit_layer.layer.clone(),
820+
commit_layer.layer.clone(),
799821
))
800822
.await?;
801823
}
@@ -818,11 +840,12 @@ impl ImageImporter {
818840
anyhow::bail!("Image has {} non-ostree layers", prep.layers.len());
819841
}
820842
let deprecated_warning = prep.deprecated_warning().map(ToOwned::to_owned);
821-
self.unencapsulate_base(&mut prep, false).await?;
843+
self.unencapsulate_base(&mut prep, true, false).await?;
822844
// TODO change the imageproxy API to ensure this happens automatically when
823845
// the image reference is dropped
824846
self.proxy.close_image(&self.proxy_img).await?;
825-
let ostree_commit = prep.ostree_commit_layer.commit.unwrap();
847+
// SAFETY: We know we have a commit
848+
let ostree_commit = prep.ostree_commit_layer.unwrap().commit.unwrap();
826849
let image_digest = prep.manifest_digest;
827850
Ok(Import {
828851
ostree_commit,
@@ -844,20 +867,23 @@ impl ImageImporter {
844867
}
845868
// First download all layers for the base image (if necessary) - we need the SELinux policy
846869
// there to label all following layers.
847-
self.unencapsulate_base(&mut import, true).await?;
870+
self.unencapsulate_base(&mut import, false, true).await?;
848871
let des_layers = self.proxy.get_layer_info(&self.proxy_img).await?;
849872
let proxy = self.proxy;
850873
let proxy_img = self.proxy_img;
851874
let target_imgref = self.target_imgref.as_ref().unwrap_or(&self.imgref);
852-
let base_commit = import.ostree_commit_layer.commit.clone().unwrap();
875+
let base_commit = import
876+
.ostree_commit_layer
877+
.as_ref()
878+
.map(|c| c.commit.clone().unwrap());
853879

854-
let root_is_transient = {
855-
let rootf = self
856-
.repo
857-
.read_commit(&base_commit, gio::Cancellable::NONE)?
858-
.0;
880+
let root_is_transient = if let Some(base) = base_commit.as_ref() {
881+
let rootf = self.repo.read_commit(&base, gio::Cancellable::NONE)?.0;
859882
let rootf = rootf.downcast_ref::<ostree::RepoFile>().unwrap();
860883
crate::ostree_prepareroot::overlayfs_root_enabled(rootf)?
884+
} else {
885+
// For generic images we assume they're using composefs
886+
true
861887
};
862888
tracing::debug!("Base rootfs is transient: {root_is_transient}");
863889

@@ -888,7 +914,7 @@ impl ImageImporter {
888914
// An important aspect of this is that we SELinux label the derived layers using
889915
// the base policy.
890916
let opts = crate::tar::WriteTarOptions {
891-
base: Some(base_commit.clone()),
917+
base: base_commit.clone(),
892918
selinux: true,
893919
allow_nonusr: root_is_transient,
894920
retain_var: self.ostree_v2024_3,
@@ -975,14 +1001,16 @@ impl ImageImporter {
9751001
process_whiteouts: false,
9761002
..Default::default()
9771003
};
978-
repo.checkout_at(
979-
Some(&checkout_opts),
980-
(*td).as_raw_fd(),
981-
rootpath,
982-
&base_commit,
983-
cancellable,
984-
)
985-
.context("Checking out base commit")?;
1004+
if let Some(base) = base_commit.as_ref() {
1005+
repo.checkout_at(
1006+
Some(&checkout_opts),
1007+
(*td).as_raw_fd(),
1008+
rootpath,
1009+
&base,
1010+
cancellable,
1011+
)
1012+
.context("Checking out base commit")?;
1013+
}
9861014

9871015
// Layer all subsequent commits
9881016
checkout_opts.process_whiteouts = true;
@@ -1008,10 +1036,12 @@ impl ImageImporter {
10081036
let sepolicy = ostree::SePolicy::new_at(rootpath.as_raw_fd(), cancellable)?;
10091037
tracing::debug!("labeling from merged tree");
10101038
modifier.set_sepolicy(Some(&sepolicy));
1011-
} else {
1039+
} else if let Some(base) = base_commit.as_ref() {
10121040
tracing::debug!("labeling from base tree");
10131041
// TODO: We can likely drop this; we know all labels should be pre-computed.
1014-
modifier.set_sepolicy_from_commit(repo, &base_commit, cancellable)?;
1042+
modifier.set_sepolicy_from_commit(repo, &base, cancellable)?;
1043+
} else {
1044+
unreachable!()
10151045
}
10161046

10171047
let mt = ostree::MutableTree::new();
@@ -1303,6 +1333,7 @@ pub(crate) fn export_to_oci(
13031333
let srcinfo = query_image(repo, imgref)?.ok_or_else(|| anyhow!("No such image"))?;
13041334
let (commit_layer, component_layers, remaining_layers) =
13051335
parse_manifest_layout(&srcinfo.manifest, &srcinfo.configuration)?;
1336+
let commit_layer = commit_layer.ok_or_else(|| anyhow!("Missing {DIFFID_LABEL}"))?;
13061337
let commit_chunk_ref = ref_for_layer(commit_layer)?;
13071338
let commit_chunk_rev = repo.require_rev(&commit_chunk_ref)?;
13081339
let mut chunking = chunking::Chunking::new(repo, &commit_chunk_rev)?;
@@ -1768,10 +1799,12 @@ pub(crate) fn verify_container_image(
17681799
.0
17691800
.downcast::<ostree::RepoFile>()
17701801
.expect("downcast");
1771-
println!(
1772-
"Verifying with base ostree layer {}",
1773-
ref_for_layer(commit_layer)?
1774-
);
1802+
if let Some(commit_layer) = commit_layer {
1803+
println!(
1804+
"Verifying with base ostree layer {}",
1805+
ref_for_layer(commit_layer)?
1806+
);
1807+
}
17751808
compare_commit_trees(
17761809
repo,
17771810
"/".into(),

ostree-ext/src/container/update_detachedmeta.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ pub async fn update_detached_metadata(
7070
.ok_or_else(|| anyhow!("Image is missing container configuration"))?;
7171

7272
// Find the OSTree commit layer we want to replace
73-
let (commit_layer, _, _) = container_store::parse_manifest_layout(&manifest, &config)?;
73+
let (commit_layer, _, _) =
74+
container_store::parse_ostree_manifest_layout(&manifest, &config)?;
7475
let commit_layer_idx = manifest
7576
.layers()
7677
.iter()

0 commit comments

Comments
 (0)