Skip to content

Commit 222ecc0

Browse files
jleboncgwalters
authored andcommitted
ostree-ext/store: Relabel layer commits to avoid duplicate objects
When importing a non-ostree container (i.e. without `/ostree`), layer commits may use the SELinux policy of the booted commit if available. We need to handle the case where there isn't one, and the case where there is one, but the booted policy has drifted from the target policy. In either case, layer commits may reference objects which don't exist in the merge commit, effectively inflating (up to doubling) the number of commit objects in the repo associated with that import. Fix this by relabeling each layer commit after the merge commit is written. Each layer is checked out, recommitted with the same SELinux policy used for the merge commit, and its ref updated to the new commit. Since the correctly-labeled objects already exist in the repo from the merge commit, most writes are no-ops. A subsequent prune removes the orphaned pre-relabel objects. Closes: #1637 Assisted-by: OpenCode (Claude Opus 4.6) Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
1 parent 13cb153 commit 222ecc0

File tree

2 files changed

+203
-9
lines changed

2 files changed

+203
-9
lines changed

crates/ostree-ext/ci/priv-integration.sh

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,78 @@ echo "ok deploy derived container from local dir"
135135
ostree container image remove --repo "${repo}" "${derived_img_dir}"
136136
rm -rf /var/tmp/derived.dir
137137

138+
# Test: non-ostree container import with SELinux relabeling
139+
# Converts the FCOS image to a plain (non-ostree) image using chunkah,
140+
# then deploys it and verifies the relabeling optimization ran.
141+
# See https://github.com/bootc-dev/bootc/issues/1637
142+
143+
# Clean state
144+
ostree --repo="${repo}" refs ostree/container/image --delete
145+
ostree container image prune-images --full --sysroot="${sysroot}"
146+
147+
# Convert FCOS to non-ostree image using chunkah
148+
# --prune /sysroot/ strips the ostree deployment data
149+
# --label KEY- removes ostree-specific labels
150+
# See also https://github.com/coreos/chunkah?tab=readme-ov-file#compatibility-with-bootable-bootc-images
151+
nonostree_archive=/var/tmp/nonostree.ociarchive
152+
chunkah_config="$(podman inspect ${image})"
153+
systemd-run -dP --wait podman run --rm \
154+
--mount=type=image,src=${image},dst=/chunkah \
155+
-v /var/tmp:/output:z \
156+
-e CHUNKAH_CONFIG_STR="${chunkah_config}" \
157+
quay.io/coreos/chunkah build \
158+
--prune /sysroot/ \
159+
--label ostree.commit- \
160+
--label ostree.final-diffid- \
161+
-o /output/nonostree.ociarchive
162+
163+
# Deploy the non-ostree image with debug logging to capture relabeling messages
164+
RUST_LOG=ostree_ext=debug ostree container image deploy \
165+
--sysroot "${sysroot}" \
166+
--stateroot "${stateroot}" \
167+
--imgref ostree-unverified-image:oci-archive:${nonostree_archive} 2>deploy-nonostree.txt
168+
169+
# Verify relabeling occurred (N > 0 layers were relabeled)
170+
if ! grep -qE 'relabeled [1-9][0-9]* layer commits' deploy-nonostree.txt; then
171+
echo "Relabeling did not occur or relabeled 0 layers" 1>&2
172+
cat deploy-nonostree.txt
173+
exit 1
174+
fi
175+
176+
# Verify orphaned pre-relabel objects were pruned
177+
if ! grep -qE 'pruned [1-9][0-9]* orphaned objects after relabeling' deploy-nonostree.txt; then
178+
echo "Post-relabel prune did not remove any objects" 1>&2
179+
cat deploy-nonostree.txt
180+
exit 1
181+
fi
182+
183+
# Verify that layer and merge commit share the same file objects after relabeling.
184+
# Find the layer containing the bootc binary via chunkah's manifest annotation.
185+
ostree container image metadata --repo "${repo}" oci-archive:${nonostree_archive} > nonostree-manifest.json
186+
layer_digest=$(jq -r '.layers[] | select(.annotations["org.chunkah.component"] | test("rpm/bootc")) | .digest' nonostree-manifest.json | head -1)
187+
layer_ref="ostree/container/blob/$(echo ${layer_digest} | sed 's/:/_3A_/')"
188+
189+
# Get the checksum of /usr/bin/bootc from the layer commit
190+
layer_bootc_csum=$(ostree --repo="${repo}" ls -RC "${layer_ref}" /usr/bin/bootc | awk '{print $5}')
191+
192+
# Get the checksum of /usr/bin/bootc from the merge commit
193+
img_ref=$(ostree --repo="${repo}" refs ostree/container/image | head -1)
194+
merge_bootc_csum=$(ostree --repo="${repo}" ls -RC "ostree/container/image/${img_ref}" /usr/bin/bootc | awk '{print $5}')
195+
196+
# Sanity check: ostree checksums are 64 hex chars
197+
test ${#layer_bootc_csum} = 64
198+
199+
# If relabeling worked, both should have the same SELinux-labeled objects
200+
test "${layer_bootc_csum}" = "${merge_bootc_csum}"
201+
echo "ok layer and merge commit share objects after relabeling"
202+
203+
# Cleanup
204+
ostree admin --sysroot="${sysroot}" undeploy 0
205+
ostree container image prune-images --full --sysroot="${sysroot}"
206+
rm -f ${nonostree_archive}
207+
208+
echo "ok non-ostree container import with SELinux relabeling"
209+
138210
# Verify policy
139211

140212
mkdir -p /etc/pki/containers

crates/ostree-ext/src/container/store.rs

Lines changed: 131 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,12 @@ impl CachedImageUpdate {
292292
}
293293
}
294294

295+
/// A layer in the ostree repo, identified by its ref and commit checksum.
296+
struct LayerRef {
297+
ostree_ref: String,
298+
commit: String,
299+
}
300+
295301
/// Context for importing a container image.
296302
#[derive(Debug)]
297303
pub struct ImageImporter {
@@ -1200,7 +1206,7 @@ impl ImageImporter {
12001206
fn write_merge_commit_impl(
12011207
repo: &ostree::Repo,
12021208
base_commit: Option<&str>,
1203-
layer_commits: &[String],
1209+
layer_commits: &[LayerRef],
12041210
have_derived_layers: bool,
12051211
metadata: glib::Variant,
12061212
timestamp: u64,
@@ -1246,13 +1252,14 @@ impl ImageImporter {
12461252

12471253
// Layer all subsequent commits
12481254
checkout_opts.process_whiteouts = true;
1249-
for commit in layer_commits {
1255+
for lc in layer_commits {
1256+
let commit = &lc.commit;
12501257
tracing::debug!("Unpacking {commit}");
12511258
repo.checkout_at(
12521259
Some(&checkout_opts),
12531260
(*td).as_raw_fd(),
12541261
rootpath,
1255-
&commit,
1262+
commit,
12561263
cancellable,
12571264
)
12581265
.with_context(|| format!("Checking out layer {commit}"))?;
@@ -1265,13 +1272,22 @@ impl ImageImporter {
12651272
modifier.set_devino_cache(&devino);
12661273
// If we have derived layers, then we need to handle the case where
12671274
// the derived layers include custom policy. Just relabel everything
1268-
// in this case.
1275+
// in this case. Note "derived layers" also include the case where the
1276+
// image has no OSTree repo at all.
1277+
//
1278+
// Track whether we need to relabel layer commits afterwards. Which is
1279+
// only relevant if they're derived layers.
1280+
let should_relabel;
12691281
if have_derived_layers {
12701282
let sepolicy = ostree::SePolicy::new_at(root_dir.as_raw_fd(), cancellable)?;
1271-
tracing::debug!("labeling from merged tree");
1272-
modifier.set_sepolicy(Some(&sepolicy));
1283+
should_relabel = sepolicy.name().map_or(false, |s| !s.is_empty());
1284+
if should_relabel {
1285+
tracing::debug!("labeling from merged tree");
1286+
modifier.set_sepolicy(Some(&sepolicy));
1287+
}
12731288
} else if let Some(base) = base_commit.as_ref() {
12741289
tracing::debug!("labeling from base tree");
1290+
should_relabel = false;
12751291
// TODO: We can likely drop this; we know all labels should be pre-computed.
12761292
modifier.set_sepolicy_from_commit(repo, &base, cancellable)?;
12771293
} else {
@@ -1311,11 +1327,32 @@ impl ImageImporter {
13111327
if !no_imgref {
13121328
repo.transaction_set_ref(None, ostree_ref, Some(merged_commit.as_str()));
13131329
}
1330+
1331+
// Relabel layer commits with the SELinux policy from the merged tree.
1332+
// Since the merge commit already wrote correctly-labeled objects, most
1333+
// writes here are no-ops (objects already exist in the repo). This
1334+
// ensures layer and merge commits share the same objects, avoiding
1335+
// duplicate storage.
1336+
let n_relabeled_layers = if should_relabel {
1337+
let n =
1338+
Self::relabel_layers(repo, layer_commits, &modifier, checkout_mode, cancellable)?;
1339+
tracing::debug!("relabeled {n} layer commits");
1340+
n
1341+
} else {
1342+
0
1343+
};
1344+
13141345
txn.commit(cancellable)?;
13151346

13161347
if !disable_gc {
13171348
let n: u32 = gc_image_layers_impl(repo, cancellable)?;
13181349
tracing::debug!("pruned {n} layers");
1350+
// Prune orphaned objects from the old (pre-relabel) layer commits.
1351+
if n_relabeled_layers > 0 {
1352+
let (_, n_pruned, _) =
1353+
repo.prune(ostree::RepoPruneFlags::REFS_ONLY, 0, cancellable)?;
1354+
tracing::debug!("pruned {n_pruned} orphaned objects after relabeling");
1355+
}
13191356
}
13201357

13211358
// Here we re-query state just to run through the same code path,
@@ -1324,6 +1361,85 @@ impl ImageImporter {
13241361
Ok(state)
13251362
}
13261363

1364+
/// Relabel layer commits with the given commit modifier (which carries
1365+
/// an SELinux policy). Each layer is checked out, recommitted with the
1366+
/// policy applied, and its ref is updated to point at the new commit.
1367+
/// Returns the number of layers that were actually relabeled (i.e. whose
1368+
/// commit changed).
1369+
fn relabel_layers(
1370+
repo: &ostree::Repo,
1371+
layer_commits: &[LayerRef],
1372+
modifier: &ostree::RepoCommitModifier,
1373+
checkout_mode: ostree::RepoCheckoutMode,
1374+
cancellable: Option<&gio::Cancellable>,
1375+
) -> Result<u32> {
1376+
use rustix::fd::AsRawFd;
1377+
1378+
let repodir = Dir::reopen_dir(&repo.dfd_borrow())?;
1379+
let repo_tmp = repodir.open_dir("tmp")?;
1380+
let rootpath = "root";
1381+
let mut n_relabeled = 0u32;
1382+
let checkout_opts = ostree::RepoCheckoutAtOptions {
1383+
mode: checkout_mode,
1384+
no_copy_fallback: true,
1385+
force_copy_zerosized: true,
1386+
..Default::default()
1387+
};
1388+
1389+
for lc in layer_commits {
1390+
let (layer_ref, old_commit) = (&lc.ostree_ref, &lc.commit);
1391+
let td = cap_std_ext::cap_tempfile::TempDir::new_in(&repo_tmp)?;
1392+
repo.checkout_at(
1393+
Some(&checkout_opts),
1394+
(*td).as_raw_fd(),
1395+
rootpath,
1396+
old_commit,
1397+
cancellable,
1398+
)
1399+
.with_context(|| format!("Checking out layer {old_commit} for relabeling"))?;
1400+
1401+
let mt = ostree::MutableTree::new();
1402+
repo.write_dfd_to_mtree(
1403+
(*td).as_raw_fd(),
1404+
rootpath,
1405+
&mt,
1406+
Some(modifier),
1407+
cancellable,
1408+
)
1409+
.with_context(|| format!("Writing relabeled layer {old_commit} to mtree"))?;
1410+
1411+
let root = repo
1412+
.write_mtree(&mt, cancellable)
1413+
.context("Writing mtree")?;
1414+
let root = root.downcast::<ostree::RepoFile>().unwrap();
1415+
1416+
// Preserve the original commit's metadata and timestamp
1417+
let (commit_v, _) = repo.load_commit(old_commit)?;
1418+
let old_metadata = commit_v.child_value(0);
1419+
let old_timestamp = ostree::commit_get_timestamp(&commit_v);
1420+
1421+
let new_commit = repo
1422+
.write_commit_with_time(
1423+
None,
1424+
None,
1425+
None,
1426+
Some(&old_metadata),
1427+
&root,
1428+
old_timestamp,
1429+
cancellable,
1430+
)
1431+
.with_context(|| format!("Writing relabeled commit for layer {layer_ref}"))?;
1432+
1433+
if new_commit != *old_commit {
1434+
repo.transaction_set_ref(None, layer_ref, Some(new_commit.as_str()));
1435+
n_relabeled += 1;
1436+
tracing::debug!("Relabeled layer {layer_ref}: {old_commit} -> {new_commit}");
1437+
}
1438+
}
1439+
1440+
Ok(n_relabeled)
1441+
}
1442+
13271443
/// Import a layered container image.
13281444
///
13291445
/// If enabled, this will also prune unused container image layers.
@@ -1358,7 +1474,7 @@ impl ImageImporter {
13581474

13591475
let ostree_ref = ref_for_image(&target_imgref.imgref)?;
13601476

1361-
let mut layer_commits = Vec::new();
1477+
let mut layer_commits: Vec<LayerRef> = Vec::new();
13621478
let mut layer_filtered_content: Option<MetaFilteredData> = None;
13631479
let have_derived_layers = !import.layers.is_empty();
13641480
tracing::debug!("Processing layers: {}", import.layers.len());
@@ -1367,7 +1483,10 @@ impl ImageImporter {
13671483
tracing::debug!("Reusing fetched commit {}", c);
13681484
Self::ensure_ref_for_layer(&self.repo, &layer.ostree_ref, &c)?;
13691485

1370-
layer_commits.push(c.to_string());
1486+
layer_commits.push(LayerRef {
1487+
ostree_ref: layer.ostree_ref.clone(),
1488+
commit: c.to_string(),
1489+
});
13711490
} else {
13721491
if let Some(p) = self.layer_progress.as_ref() {
13731492
p.send(ImportProgress::DerivedLayerStarted(layer.layer.clone()))
@@ -1403,7 +1522,10 @@ impl ImageImporter {
14031522
.await
14041523
.with_context(|| format!("Parsing layer blob {}", layer.layer.digest()))?;
14051524
tracing::debug!("Imported layer: {}", r.commit.as_str());
1406-
layer_commits.push(r.commit);
1525+
layer_commits.push(LayerRef {
1526+
ostree_ref: layer.ostree_ref.clone(),
1527+
commit: r.commit,
1528+
});
14071529
let filtered_owned = HashMap::from_iter(r.filtered.clone());
14081530
if let Some((filtered, n_rest)) = bootc_utils::collect_until(
14091531
r.filtered.iter(),

0 commit comments

Comments
 (0)