From 01f3b1210017f3093c3d02a97648a1dad93e94e3 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:37:18 +0000 Subject: [PATCH 01/13] Compose VM rootfs from shared layer blobs --- lib/images/compose.go | 44 ++++++ lib/images/compose_test.go | 204 +++++++++++++++++++++++++ lib/images/oci.go | 5 +- lib/images/recovery_regression_test.go | 2 +- lib/images/testlayers_test.go | 52 +++++++ 5 files changed, 304 insertions(+), 3 deletions(-) create mode 100644 lib/images/compose.go create mode 100644 lib/images/compose_test.go create mode 100644 lib/images/testlayers_test.go diff --git a/lib/images/compose.go b/lib/images/compose.go new file mode 100644 index 000000000..3567644e1 --- /dev/null +++ b/lib/images/compose.go @@ -0,0 +1,44 @@ +package images + +import ( + "fmt" + "os" +) + +// composeRootfs validates the persisted model and merges its layers into dest +// in manifest order, reading each layer blob from the shared OCI cache. +// Whiteout and opaque-directory markers are interpreted as each layer is +// applied. +func (c *ociClient) composeRootfs(dest, layoutTag string, model *imageManifestModel) error { + if err := validateManifestModel(layoutTag, model); err != nil { + return fmt.Errorf("validate manifest model: %w", err) + } + if len(model.Layers) == 0 { + return fmt.Errorf("image has no layers") + } + if err := os.MkdirAll(dest, 0755); err != nil { + return fmt.Errorf("create compose directory: %w", err) + } + for i, desc := range model.Layers { + if err := c.applyLayerToDir(dest, desc); err != nil { + return fmt.Errorf("apply layer %d (%s): %w", i, desc.Digest, err) + } + } + return nil +} + +func (c *ociClient) applyLayerToDir(dest string, desc layerDescriptor) error { + layerDir, err := os.MkdirTemp("", "hypeman-layer-*") + if err != nil { + return fmt.Errorf("create layer staging directory: %w", err) + } + defer os.RemoveAll(layerDir) + + if _, err := unpackCachedLayer(c.cacheDir, desc, layerDir); err != nil { + return err + } + if err := applyLayerTree(layerDir, dest); err != nil { + return fmt.Errorf("apply layer tree: %w", err) + } + return nil +} diff --git a/lib/images/compose_test.go b/lib/images/compose_test.go new file mode 100644 index 000000000..b67c8dee8 --- /dev/null +++ b/lib/images/compose_test.go @@ -0,0 +1,204 @@ +package images + +import ( + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + gcr "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +// composeTestImage builds the standard two-layer fixture: a base layer with +// content the top layer deletes, masks, replaces, and extends. +func composeTestImage(t *testing.T) gcr.Image { + t.Helper() + + base := specLayer(t, []tarEntrySpec{ + {name: "etc/", isDir: true, mode: 0755}, + {name: "etc/config.txt", content: "original", mode: 0644}, + {name: "app/", isDir: true, mode: 0755}, + {name: "app/main.txt", content: "v1", mode: 0644}, + {name: "data/", isDir: true, mode: 0755}, + {name: "data/old.txt", content: "stale", mode: 0644}, + {name: "replacedir/", isDir: true, mode: 0755}, + {name: "replacedir/inner.txt", content: "inner", mode: 0644}, + }) + top := specLayer(t, []tarEntrySpec{ + {name: "etc/.wh.config.txt", content: "", mode: 0644}, + {name: "app/main.txt", content: "v2", mode: 0644}, + {name: "data/.wh..wh..opq", content: "", mode: 0644}, + {name: "data/new.txt", content: "new", mode: 0644}, + {name: "bin/", isDir: true, mode: 0755}, + {name: "bin/tool", content: "tool", mode: 0755}, + {name: "replacedir", content: "now a file", mode: 0644}, + }) + + img, err := mutate.AppendLayers(empty.Image, base, top) + require.NoError(t, err) + return img +} + +// composeFixture composes the standard fixture image into the shared OCI cache +// and returns a client plus its validated manifest model. +func composeFixture(t *testing.T, p *paths.Paths) (*ociClient, string, *imageManifestModel) { + t.Helper() + + img := composeTestImage(t) + writeLayerTestLayout(t, p, img) + + client, err := newOCIClient(p.SystemOCICache()) + require.NoError(t, err) + digest, err := img.Digest() + require.NoError(t, err) + tag := digestToLayoutTag(digest.String()) + bundle, err := client.extractOCIImageBundle(tag) + require.NoError(t, err) + return client, tag, bundle.Model +} + +func TestComposeRootfsWhiteoutsAndOrdering(t *testing.T) { + p := paths.New(t.TempDir()) + client, tag, model := composeFixture(t, p) + require.Len(t, model.Layers, 2) + + dest := filepath.Join(t.TempDir(), "rootfs") + require.NoError(t, client.composeRootfs(dest, tag, model)) + + // Whiteout removed the base entry. + _, err := os.Lstat(filepath.Join(dest, "etc", "config.txt")) + require.True(t, os.IsNotExist(err), "whiteout must delete the base entry") + + // Plain replacement. + data, err := os.ReadFile(filepath.Join(dest, "app", "main.txt")) + require.NoError(t, err) + require.Equal(t, "v2", string(data)) + + // Opaque directory masked the base content. + _, err = os.Lstat(filepath.Join(dest, "data", "old.txt")) + require.True(t, os.IsNotExist(err), "opaque marker must mask base contents") + data, err = os.ReadFile(filepath.Join(dest, "data", "new.txt")) + require.NoError(t, err) + require.Equal(t, "new", string(data)) + + // Directory replaced by a regular file. + info, err := os.Lstat(filepath.Join(dest, "replacedir")) + require.NoError(t, err) + require.False(t, info.IsDir()) + data, err = os.ReadFile(filepath.Join(dest, "replacedir")) + require.NoError(t, err) + require.Equal(t, "now a file", string(data)) + + // New entry present with its mode. + info, err = os.Stat(filepath.Join(dest, "bin", "tool")) + require.NoError(t, err) + require.Equal(t, os.FileMode(0755), info.Mode().Perm()) + + // No whiteout markers survive composition. + require.NoError(t, filepath.Walk(dest, func(path string, info os.FileInfo, err error) error { + require.NoError(t, err) + require.NotContains(t, info.Name(), whiteoutPrefix, "whiteout marker leaked into composed rootfs") + return nil + })) +} + +// zeroLayerModel returns a schema-valid manifest model with no layers. +func zeroLayerModel() *imageManifestModel { + return &imageManifestModel{ + SchemaVersion: manifestModelSchemaVersion, + Digest: "sha256:" + strings.Repeat("ab", 32), + Config: manifestConfigRef{Digest: "sha256:" + strings.Repeat("cd", 32)}, + Layers: make([]layerDescriptor, 0), + } +} + +func TestComposeRootfsEmptyLayers(t *testing.T) { + p := paths.New(t.TempDir()) + client, err := newOCIClient(p.SystemOCICache()) + require.NoError(t, err) + model := zeroLayerModel() + err = client.composeRootfs(t.TempDir(), model.Digest, model) + require.ErrorContains(t, err, "no layers") +} + +func TestComposeRootfsInvalidModel(t *testing.T) { + p := paths.New(t.TempDir()) + client, tag, model := composeFixture(t, p) + + model.Config.DiffIDs = model.Config.DiffIDs[:1] + err := client.composeRootfs(filepath.Join(t.TempDir(), "rootfs"), tag, model) + require.ErrorContains(t, err, "1 diff ids for 2 layers") +} + +func TestComposeRootfsMissingBlob(t *testing.T) { + p := paths.New(t.TempDir()) + client, err := newOCIClient(p.SystemOCICache()) + require.NoError(t, err) + + digestHex := "sha256:" + strings.Repeat("ab", 32) + model := &imageManifestModel{ + SchemaVersion: manifestModelSchemaVersion, + Digest: digestHex, + Config: manifestConfigRef{ + Digest: "sha256:" + strings.Repeat("cd", 32), + DiffIDs: []string{"sha256:" + strings.Repeat("ef", 32)}, + }, + Layers: []layerDescriptor{{ + Digest: "sha256:" + strings.Repeat("01", 32), + MediaType: "application/vnd.oci.image.layer.v1.tar+gzip", + DiffID: "sha256:" + strings.Repeat("ef", 32), + }}, + } + err = client.composeRootfs(t.TempDir(), digestHex, model) + require.ErrorContains(t, err, "missing from oci cache") +} + +func TestComposeRootfsDiffIDMismatch(t *testing.T) { + p := paths.New(t.TempDir()) + client, tag, model := composeFixture(t, p) + + // Replace the top layer's cached blob with different content so the + // unpacked diff id no longer matches the descriptor. + other := specLayer(t, []tarEntrySpec{{name: "other.txt", content: "other", mode: 0644}}) + otherBlob, err := other.Compressed() + require.NoError(t, err) + data, err := io.ReadAll(otherBlob) + require.NoError(t, err) + topHex := strings.TrimPrefix(model.Layers[1].Digest, "sha256:") + require.NoError(t, os.WriteFile(p.OCICacheBlob(topHex), data, 0644)) + + err = client.composeRootfs(filepath.Join(t.TempDir(), "rootfs"), tag, model) + require.ErrorContains(t, err, "diff id mismatch") +} + +// TestComposeRootfsExportsValidErofs composes the fixture image and exports it +// to erofs, then verifies the filesystem is intact and its contents match the +// composed tree. +func TestComposeRootfsExportsValidErofs(t *testing.T) { + if _, err := exec.LookPath("mkfs.erofs"); err != nil { + t.Skip("mkfs.erofs not available") + } + if _, err := exec.LookPath("fsck.erofs"); err != nil { + t.Skip("fsck.erofs not available") + } + + p := paths.New(t.TempDir()) + client, tag, model := composeFixture(t, p) + + staging := filepath.Join(t.TempDir(), "rootfs") + require.NoError(t, client.composeRootfs(staging, tag, model)) + + diskPath := filepath.Join(t.TempDir(), "rootfs.erofs") + size, err := ExportRootfs(staging, diskPath, FormatErofs) + require.NoError(t, err) + require.Greater(t, size, int64(0)) + + output, err := exec.Command("fsck.erofs", "--extract", diskPath).CombinedOutput() + require.NoError(t, err, "fsck.erofs failed: %s", output) +} diff --git a/lib/images/oci.go b/lib/images/oci.go index 36684cacd..5cf18daff 100644 --- a/lib/images/oci.go +++ b/lib/images/oci.go @@ -283,9 +283,10 @@ func (c *ociClient) pullAndExportWithPlatformAuth(ctx context.Context, imageRef, result.LayerCount = bundle.LayerCount result.CompressedBytes = bundle.CompressedBytes - // Unpack layers to the export directory + // Compose the rootfs from the shared layer blobs in manifest order. + // composeRootfs validates the model and rejects zero-layer manifests. if err := result.measure("layer_unpack", func() error { - return c.unpackLayers(ctx, layoutTag, exportDir) + return c.composeRootfs(exportDir, layoutTag, bundle.Model) }); err != nil { return result, fmt.Errorf("unpack layers: %w", err) } diff --git a/lib/images/recovery_regression_test.go b/lib/images/recovery_regression_test.go index b979939a9..ad3398316 100644 --- a/lib/images/recovery_regression_test.go +++ b/lib/images/recovery_regression_test.go @@ -65,7 +65,7 @@ func TestRecoverInterruptedBuildsCapturedFixtureMarksBuildFailed(t *testing.T) { require.NotNil(t, meta.Error) assert.Equal(t, recoveryFixtureDigest, meta.Digest) assert.Equal(t, StatusFailed, meta.Status) - assert.Contains(t, *meta.Error, "config rootfs.diff_ids has 0 entries but manifest has 1 layers") + assert.Contains(t, *meta.Error, "manifest model has 0 diff ids for 1 layers") } func copyRecoveryFixture(t *testing.T) string { diff --git a/lib/images/testlayers_test.go b/lib/images/testlayers_test.go new file mode 100644 index 000000000..d56cf1015 --- /dev/null +++ b/lib/images/testlayers_test.go @@ -0,0 +1,52 @@ +package images + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "io" + "testing" + + gcr "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/tarball" + "github.com/stretchr/testify/require" +) + +type tarEntrySpec struct { + name string + content string + isDir bool + mode int64 +} + +// specLayer builds a gzipped tar layer from entry specs in order. +func specLayer(t *testing.T, entries []tarEntrySpec) gcr.Layer { + t.Helper() + + var buf bytes.Buffer + gzw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gzw) + for _, entry := range entries { + if entry.isDir { + require.NoError(t, tw.WriteHeader(&tar.Header{Name: entry.name, Typeflag: tar.TypeDir, Mode: entry.mode})) + continue + } + require.NoError(t, tw.WriteHeader(&tar.Header{ + Name: entry.name, + Typeflag: tar.TypeReg, + Mode: entry.mode, + Size: int64(len(entry.content)), + })) + _, err := tw.Write([]byte(entry.content)) + require.NoError(t, err) + } + require.NoError(t, tw.Close()) + require.NoError(t, gzw.Close()) + + data := buf.Bytes() + layer, err := tarball.LayerFromOpener(func() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(data)), nil + }) + require.NoError(t, err) + return layer +} From 878b5d81fa0fc333cee867f1152fb8233d826af1 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:37:50 +0000 Subject: [PATCH 02/13] Make rootfs composition safe and cancellable --- lib/images/compose.go | 29 ++++++++++++++++++++++------- lib/images/compose_test.go | 7 +++++-- lib/images/oci.go | 3 +-- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/lib/images/compose.go b/lib/images/compose.go index 3567644e1..dea6555a0 100644 --- a/lib/images/compose.go +++ b/lib/images/compose.go @@ -1,8 +1,10 @@ package images import ( + "context" "fmt" "os" + "path/filepath" ) // composeRootfs validates the persisted model and merges its layers into dest @@ -10,31 +12,44 @@ import ( // Whiteout and opaque-directory markers are interpreted as each layer is // applied. func (c *ociClient) composeRootfs(dest, layoutTag string, model *imageManifestModel) error { + return c.composeRootfsContext(context.Background(), dest, layoutTag, model) +} + +func (c *ociClient) composeRootfsContext(ctx context.Context, dest, layoutTag string, model *imageManifestModel) error { if err := validateManifestModel(layoutTag, model); err != nil { return fmt.Errorf("validate manifest model: %w", err) } - if len(model.Layers) == 0 { - return fmt.Errorf("image has no layers") + if err := os.MkdirAll(filepath.Dir(dest), 0755); err != nil { + return fmt.Errorf("create compose parent: %w", err) } - if err := os.MkdirAll(dest, 0755); err != nil { + staging, err := os.MkdirTemp(filepath.Dir(dest), ".compose-*") + if err != nil { return fmt.Errorf("create compose directory: %w", err) } + defer os.RemoveAll(staging) + for i, desc := range model.Layers { - if err := c.applyLayerToDir(dest, desc); err != nil { + if err := c.applyLayerToDir(ctx, staging, desc); err != nil { return fmt.Errorf("apply layer %d (%s): %w", i, desc.Digest, err) } } + if err := os.RemoveAll(dest); err != nil { + return fmt.Errorf("replace compose directory: %w", err) + } + if err := os.Rename(staging, dest); err != nil { + return fmt.Errorf("install compose directory: %w", err) + } return nil } -func (c *ociClient) applyLayerToDir(dest string, desc layerDescriptor) error { - layerDir, err := os.MkdirTemp("", "hypeman-layer-*") +func (c *ociClient) applyLayerToDir(ctx context.Context, dest string, desc layerDescriptor) error { + layerDir, err := os.MkdirTemp(filepath.Dir(dest), ".layer-*") if err != nil { return fmt.Errorf("create layer staging directory: %w", err) } defer os.RemoveAll(layerDir) - if _, err := unpackCachedLayer(c.cacheDir, desc, layerDir); err != nil { + if _, err := unpackCachedLayerContext(ctx, c.cacheDir, desc, layerDir); err != nil { return err } if err := applyLayerTree(layerDir, dest); err != nil { diff --git a/lib/images/compose_test.go b/lib/images/compose_test.go index b67c8dee8..3e0dea77d 100644 --- a/lib/images/compose_test.go +++ b/lib/images/compose_test.go @@ -123,8 +123,11 @@ func TestComposeRootfsEmptyLayers(t *testing.T) { client, err := newOCIClient(p.SystemOCICache()) require.NoError(t, err) model := zeroLayerModel() - err = client.composeRootfs(t.TempDir(), model.Digest, model) - require.ErrorContains(t, err, "no layers") + dest := filepath.Join(t.TempDir(), "rootfs") + require.NoError(t, client.composeRootfs(dest, model.Digest, model)) + entries, err := os.ReadDir(dest) + require.NoError(t, err) + require.Empty(t, entries) } func TestComposeRootfsInvalidModel(t *testing.T) { diff --git a/lib/images/oci.go b/lib/images/oci.go index 5cf18daff..0d208a891 100644 --- a/lib/images/oci.go +++ b/lib/images/oci.go @@ -284,9 +284,8 @@ func (c *ociClient) pullAndExportWithPlatformAuth(ctx context.Context, imageRef, result.CompressedBytes = bundle.CompressedBytes // Compose the rootfs from the shared layer blobs in manifest order. - // composeRootfs validates the model and rejects zero-layer manifests. if err := result.measure("layer_unpack", func() error { - return c.composeRootfs(exportDir, layoutTag, bundle.Model) + return c.composeRootfsContext(ctx, exportDir, layoutTag, bundle.Model) }); err != nil { return result, fmt.Errorf("unpack layers: %w", err) } From 6069e997b817c526defcabb1804e8a02401c7cc8 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:01:31 +0000 Subject: [PATCH 03/13] Track explicit layer directories during composition --- lib/images/compose.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/images/compose.go b/lib/images/compose.go index dea6555a0..88e6619df 100644 --- a/lib/images/compose.go +++ b/lib/images/compose.go @@ -49,10 +49,11 @@ func (c *ociClient) applyLayerToDir(ctx context.Context, dest string, desc layer } defer os.RemoveAll(layerDir) - if _, err := unpackCachedLayerContext(ctx, c.cacheDir, desc, layerDir); err != nil { + stats, err := unpackCachedLayerContext(ctx, c.cacheDir, desc, layerDir) + if err != nil { return err } - if err := applyLayerTree(layerDir, dest); err != nil { + if err := applyLayerTreeWithExplicitDirs(layerDir, dest, stats.explicitDirs); err != nil { return fmt.Errorf("apply layer tree: %w", err) } return nil From 74f3ee6b7861933ba8b3ba562d0b7d6de374e310 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:30:27 +0000 Subject: [PATCH 04/13] Use renamed layer unpack and apply helpers --- lib/images/compose.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/images/compose.go b/lib/images/compose.go index 88e6619df..82d4a2419 100644 --- a/lib/images/compose.go +++ b/lib/images/compose.go @@ -49,11 +49,11 @@ func (c *ociClient) applyLayerToDir(ctx context.Context, dest string, desc layer } defer os.RemoveAll(layerDir) - stats, err := unpackCachedLayerContext(ctx, c.cacheDir, desc, layerDir) + stats, err := unpackCachedLayer(ctx, c.cacheDir, desc, layerDir) if err != nil { return err } - if err := applyLayerTreeWithExplicitDirs(layerDir, dest, stats.explicitDirs); err != nil { + if err := applyLayerTree(layerDir, dest, stats.explicitDirs); err != nil { return fmt.Errorf("apply layer tree: %w", err) } return nil From 6ac8432bdaf9c045ba9a94c79be1619c6a2378d5 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:56:03 +0000 Subject: [PATCH 05/13] Apply layers onto the staging tree directly with umoci --- lib/images/compose.go | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/lib/images/compose.go b/lib/images/compose.go index 82d4a2419..304732ca0 100644 --- a/lib/images/compose.go +++ b/lib/images/compose.go @@ -29,7 +29,7 @@ func (c *ociClient) composeRootfsContext(ctx context.Context, dest, layoutTag st defer os.RemoveAll(staging) for i, desc := range model.Layers { - if err := c.applyLayerToDir(ctx, staging, desc); err != nil { + if _, err := unpackCachedLayer(ctx, c.cacheDir, desc, staging, composeOnDiskFormat()); err != nil { return fmt.Errorf("apply layer %d (%s): %w", i, desc.Digest, err) } } @@ -41,20 +41,3 @@ func (c *ociClient) composeRootfsContext(ctx context.Context, dest, layoutTag st } return nil } - -func (c *ociClient) applyLayerToDir(ctx context.Context, dest string, desc layerDescriptor) error { - layerDir, err := os.MkdirTemp(filepath.Dir(dest), ".layer-*") - if err != nil { - return fmt.Errorf("create layer staging directory: %w", err) - } - defer os.RemoveAll(layerDir) - - stats, err := unpackCachedLayer(ctx, c.cacheDir, desc, layerDir) - if err != nil { - return err - } - if err := applyLayerTree(layerDir, dest, stats.explicitDirs); err != nil { - return fmt.Errorf("apply layer tree: %w", err) - } - return nil -} From 1fbb25a0b55b23e260e2f33cc499fc46e6038506 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:50:26 +0000 Subject: [PATCH 06/13] Compose rootfs always from cache blobs and drop dead unpack path composition is the only unpack path now: move composeOnDiskFormat into production, point unpackCachedLayer at the cache blob directory directly, and delete the unused umoci unpackLayers path and its helpers. Also: drop the unused composeRootfs wrapper, restore the 0755 export directory mode, reuse removePath for staging and destination cleanup, migrate the unpackLayers tests to the compose path, fix the diff id mismatch test to exercise the diff id check, share the tar layer builders, and require a layered rootfs in manifest model validation. --- lib/images/compose.go | 32 +++--- lib/images/compose_test.go | 31 +++--- lib/images/layer_artifact.go | 20 ++-- lib/images/manifest_model.go | 2 +- lib/images/manifest_model_test.go | 42 ++------ lib/images/oci.go | 142 ------------------------- lib/images/oci_test.go | 46 ++++---- lib/images/recovery_regression_test.go | 13 ++- 8 files changed, 88 insertions(+), 240 deletions(-) diff --git a/lib/images/compose.go b/lib/images/compose.go index 304732ca0..3f3432a2d 100644 --- a/lib/images/compose.go +++ b/lib/images/compose.go @@ -3,37 +3,43 @@ package images import ( "context" "fmt" + "log/slog" "os" "path/filepath" ) -// composeRootfs validates the persisted model and merges its layers into dest -// in manifest order, reading each layer blob from the shared OCI cache. -// Whiteout and opaque-directory markers are interpreted as each layer is -// applied. -func (c *ociClient) composeRootfs(dest, layoutTag string, model *imageManifestModel) error { - return c.composeRootfsContext(context.Background(), dest, layoutTag, model) -} - +// composeRootfsContext validates the persisted model and merges its layers +// into dest in manifest order, reading each layer blob from the shared OCI +// cache. Whiteout and opaque-directory markers are interpreted as each layer +// is applied. func (c *ociClient) composeRootfsContext(ctx context.Context, dest, layoutTag string, model *imageManifestModel) error { if err := validateManifestModel(layoutTag, model); err != nil { return fmt.Errorf("validate manifest model: %w", err) } - if err := os.MkdirAll(filepath.Dir(dest), 0755); err != nil { + parent := filepath.Dir(dest) + if err := os.MkdirAll(parent, 0755); err != nil { return fmt.Errorf("create compose parent: %w", err) } - staging, err := os.MkdirTemp(filepath.Dir(dest), ".compose-*") + staging, err := os.MkdirTemp(parent, ".compose-*") if err != nil { return fmt.Errorf("create compose directory: %w", err) } - defer os.RemoveAll(staging) + defer func() { + if err := removePath(staging); err != nil { + slog.Warn("failed to remove compose staging directory", "dir", staging, "error", err) + } + }() for i, desc := range model.Layers { - if _, err := unpackCachedLayer(ctx, c.cacheDir, desc, staging, composeOnDiskFormat()); err != nil { + if _, err := unpackCachedLayer(ctx, filepath.Join(c.cacheDir, "blobs", "sha256"), desc, staging, composeOnDiskFormat()); err != nil { return fmt.Errorf("apply layer %d (%s): %w", i, desc.Digest, err) } } - if err := os.RemoveAll(dest); err != nil { + // Match the export-directory mode the unpack path used; MkdirTemp is 0700. + if err := os.Chmod(staging, 0755); err != nil { + return fmt.Errorf("set compose directory mode: %w", err) + } + if err := removePath(dest); err != nil { return fmt.Errorf("replace compose directory: %w", err) } if err := os.Rename(staging, dest); err != nil { diff --git a/lib/images/compose_test.go b/lib/images/compose_test.go index 3e0dea77d..bf9f637f7 100644 --- a/lib/images/compose_test.go +++ b/lib/images/compose_test.go @@ -1,7 +1,7 @@ package images import ( - "io" + "context" "os" "os/exec" "path/filepath" @@ -69,7 +69,7 @@ func TestComposeRootfsWhiteoutsAndOrdering(t *testing.T) { require.Len(t, model.Layers, 2) dest := filepath.Join(t.TempDir(), "rootfs") - require.NoError(t, client.composeRootfs(dest, tag, model)) + require.NoError(t, client.composeRootfsContext(context.Background(), dest, tag, model)) // Whiteout removed the base entry. _, err := os.Lstat(filepath.Join(dest, "etc", "config.txt")) @@ -113,6 +113,7 @@ func zeroLayerModel() *imageManifestModel { return &imageManifestModel{ SchemaVersion: manifestModelSchemaVersion, Digest: "sha256:" + strings.Repeat("ab", 32), + RootFSType: "layers", Config: manifestConfigRef{Digest: "sha256:" + strings.Repeat("cd", 32)}, Layers: make([]layerDescriptor, 0), } @@ -124,7 +125,7 @@ func TestComposeRootfsEmptyLayers(t *testing.T) { require.NoError(t, err) model := zeroLayerModel() dest := filepath.Join(t.TempDir(), "rootfs") - require.NoError(t, client.composeRootfs(dest, model.Digest, model)) + require.NoError(t, client.composeRootfsContext(context.Background(), dest, model.Digest, model)) entries, err := os.ReadDir(dest) require.NoError(t, err) require.Empty(t, entries) @@ -135,7 +136,7 @@ func TestComposeRootfsInvalidModel(t *testing.T) { client, tag, model := composeFixture(t, p) model.Config.DiffIDs = model.Config.DiffIDs[:1] - err := client.composeRootfs(filepath.Join(t.TempDir(), "rootfs"), tag, model) + err := client.composeRootfsContext(context.Background(), filepath.Join(t.TempDir(), "rootfs"), tag, model) require.ErrorContains(t, err, "1 diff ids for 2 layers") } @@ -148,6 +149,7 @@ func TestComposeRootfsMissingBlob(t *testing.T) { model := &imageManifestModel{ SchemaVersion: manifestModelSchemaVersion, Digest: digestHex, + RootFSType: "layers", Config: manifestConfigRef{ Digest: "sha256:" + strings.Repeat("cd", 32), DiffIDs: []string{"sha256:" + strings.Repeat("ef", 32)}, @@ -158,7 +160,7 @@ func TestComposeRootfsMissingBlob(t *testing.T) { DiffID: "sha256:" + strings.Repeat("ef", 32), }}, } - err = client.composeRootfs(t.TempDir(), digestHex, model) + err = client.composeRootfsContext(context.Background(), t.TempDir(), digestHex, model) require.ErrorContains(t, err, "missing from oci cache") } @@ -166,17 +168,14 @@ func TestComposeRootfsDiffIDMismatch(t *testing.T) { p := paths.New(t.TempDir()) client, tag, model := composeFixture(t, p) - // Replace the top layer's cached blob with different content so the - // unpacked diff id no longer matches the descriptor. - other := specLayer(t, []tarEntrySpec{{name: "other.txt", content: "other", mode: 0644}}) - otherBlob, err := other.Compressed() - require.NoError(t, err) - data, err := io.ReadAll(otherBlob) - require.NoError(t, err) - topHex := strings.TrimPrefix(model.Layers[1].Digest, "sha256:") - require.NoError(t, os.WriteFile(p.OCICacheBlob(topHex), data, 0644)) + // Desynchronize the top layer's diff id from its content while keeping the + // model internally consistent, so validation passes and the mismatch is + // caught against the unpacked stream instead. + forged := "sha256:" + strings.Repeat("ff", 32) + model.Layers[1].DiffID = forged + model.Config.DiffIDs[1] = forged - err = client.composeRootfs(filepath.Join(t.TempDir(), "rootfs"), tag, model) + err := client.composeRootfsContext(context.Background(), filepath.Join(t.TempDir(), "rootfs"), tag, model) require.ErrorContains(t, err, "diff id mismatch") } @@ -195,7 +194,7 @@ func TestComposeRootfsExportsValidErofs(t *testing.T) { client, tag, model := composeFixture(t, p) staging := filepath.Join(t.TempDir(), "rootfs") - require.NoError(t, client.composeRootfs(staging, tag, model)) + require.NoError(t, client.composeRootfsContext(context.Background(), staging, tag, model)) diskPath := filepath.Join(t.TempDir(), "rootfs.erofs") size, err := ExportRootfs(staging, diskPath, FormatErofs) diff --git a/lib/images/layer_artifact.go b/lib/images/layer_artifact.go index fb13f1b48..a7b511775 100644 --- a/lib/images/layer_artifact.go +++ b/lib/images/layer_artifact.go @@ -126,9 +126,8 @@ func layerDigestHex(value string) (string, error) { // layerMapOptions preserves tar ownership when running as root. Otherwise // umoci's rootless mode skips chown and stands in empty files for device nodes. -// Unlike unpackLayers in oci.go, which maps container root to the current -// user, this deliberately leaves ownership untouched as root: artifacts must -// keep the layer's on-disk ownership for later stacking. +// As root this deliberately leaves ownership untouched: artifacts must keep +// the layer's on-disk ownership for later stacking. func layerMapOptions() layer.MapOptions { return layer.MapOptions{Rootless: os.Geteuid() != 0} } @@ -179,6 +178,14 @@ func layerArtifactOnDiskFormat() layer.OnDiskFormat { return layer.OverlayfsRootfs{MapOptions: layerMapOptions()} } +// composeOnDiskFormat applies whiteouts against the tree being composed: +// deletions execute immediately on the destination instead of becoming +// overlayfs whiteout inodes, since the composed tree is mounted as a single +// lower filesystem rather than stacked. +func composeOnDiskFormat() layer.OnDiskFormat { + return layer.DirRootfs{MapOptions: layerMapOptions()} +} + // readLayerRecord loads the artifact record for a layer digest, if present. // A missing record returns (nil, nil): the layer simply was never // materialized. @@ -413,9 +420,10 @@ func (r contextReader) Read(p []byte) (int, error) { return r.reader.Read(p) } -// unpackCachedLayer locates desc's blob in the shared OCI cache, unpacks it -// into dest, and verifies both the blob digest and the diff ID when the -// descriptor carries one. The caller must have validated desc.Digest. +// unpackCachedLayer locates desc's blob under blobDir (the OCI layout's +// blobs/sha256 directory), unpacks it into dest, and verifies both the blob +// digest and the diff ID when the descriptor carries one. The caller must +// have validated desc.Digest. func unpackCachedLayer(ctx context.Context, p *paths.Paths, desc layerDescriptor, dest string, onDisk layer.OnDiskFormat) (*unpackStats, error) { blobPath := p.OCICacheBlob(strings.TrimPrefix(desc.Digest, "sha256:")) if _, err := os.Stat(blobPath); err != nil { diff --git a/lib/images/manifest_model.go b/lib/images/manifest_model.go index 2e4fc3e09..f5b91d8e9 100644 --- a/lib/images/manifest_model.go +++ b/lib/images/manifest_model.go @@ -76,7 +76,7 @@ func validateManifestModel(digestHex string, model *imageManifestModel) error { if model.Digest != digestFromHex(digestHex) { return fmt.Errorf("manifest model digest %q does not match %q", model.Digest, digestFromHex(digestHex)) } - if model.RootFSType != "" && model.RootFSType != "layers" { + if model.RootFSType != "layers" { return fmt.Errorf("unsupported manifest rootfs type: %q", model.RootFSType) } if err := validateManifestConfig(model); err != nil { diff --git a/lib/images/manifest_model_test.go b/lib/images/manifest_model_test.go index 1137e3912..fef2e0e80 100644 --- a/lib/images/manifest_model_test.go +++ b/lib/images/manifest_model_test.go @@ -1,11 +1,7 @@ package images import ( - "archive/tar" - "bytes" - "compress/gzip" "context" - "io" "os" "strings" "testing" @@ -15,7 +11,6 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/mutate" - "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/google/go-containerregistry/pkg/v1/types" "github.com/kernel/hypeman/lib/paths" "github.com/stretchr/testify/require" @@ -24,27 +19,7 @@ import ( // syntheticLayer builds a gzipped tar layer containing one file. func syntheticLayer(t *testing.T, name, content string) gcr.Layer { t.Helper() - - var buf bytes.Buffer - gzw := gzip.NewWriter(&buf) - tw := tar.NewWriter(gzw) - require.NoError(t, tw.WriteHeader(&tar.Header{ - Name: name, - Size: int64(len(content)), - Typeflag: tar.TypeReg, - Mode: 0644, - })) - _, err := tw.Write([]byte(content)) - require.NoError(t, err) - require.NoError(t, tw.Close()) - require.NoError(t, gzw.Close()) - - data := buf.Bytes() - layer, err := tarball.LayerFromOpener(func() (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader(data)), nil - }) - require.NoError(t, err) - return layer + return specLayer(t, []tarEntrySpec{{name: name, content: content, mode: 0644}}) } // writeSyntheticLayout writes img into a fresh OCI layout cache tagged with the @@ -52,16 +27,13 @@ func syntheticLayer(t *testing.T, name, content string) gcr.Layer { func writeSyntheticLayout(t *testing.T, img gcr.Image) (*ociClient, string) { t.Helper() - client, err := newOCIClient(t.TempDir()) - require.NoError(t, err) + p := paths.New(t.TempDir()) + writeLayerTestLayout(t, p, img) - digest, err := img.Digest() + client, err := newOCIClient(p.SystemOCICache()) require.NoError(t, err) - layoutPath, err := layout.Write(client.cacheDir, empty.Index) + digest, err := img.Digest() require.NoError(t, err) - require.NoError(t, layoutPath.AppendImage(img, layout.WithAnnotations(map[string]string{ - "org.opencontainers.image.ref.name": digestToLayoutTag(digest.String()), - }))) return client, digestToLayoutTag(digest.String()) } @@ -130,6 +102,7 @@ func TestManifestModelWriteReadRoundtrip(t *testing.T) { model := &imageManifestModel{ SchemaVersion: manifestModelSchemaVersion, Digest: "sha256:" + digestHex, + RootFSType: "layers", Platform: "linux/amd64", Config: manifestConfigRef{ Digest: "sha256:" + strings.Repeat("c", 64), @@ -157,7 +130,8 @@ func TestReadManifestModelRejectsInvalidSchema(t *testing.T) { p := paths.New(t.TempDir()) digestHex := strings.Repeat("a", 64) require.NoError(t, os.MkdirAll(p.ImageContentDir(digestHex), 0755)) - require.NoError(t, os.WriteFile(p.ImageContentManifestModel(digestHex), []byte(`{"schema_version":1,"digest":"sha256:`+digestHex+`"}`), 0644)) + require.NoError(t, os.WriteFile(p.ImageContentManifestModel(digestHex), + []byte(`{"schema_version":1,"digest":"sha256:`+digestHex+`","rootfs_type":"layers"}`), 0644)) _, err := readManifestModel(p, digestHex) require.ErrorContains(t, err, "config digest is empty") } diff --git a/lib/images/oci.go b/lib/images/oci.go index 0d208a891..8cab6f630 100644 --- a/lib/images/oci.go +++ b/lib/images/oci.go @@ -13,13 +13,9 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/remote" - digest "github.com/opencontainers/go-digest" - "github.com/opencontainers/image-spec/specs-go" v1 "github.com/opencontainers/image-spec/specs-go/v1" - rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/umoci/oci/cas/dir" "github.com/opencontainers/umoci/oci/casext" - "github.com/opencontainers/umoci/oci/layer" ) // ociClient handles OCI image operations without requiring Docker daemon @@ -497,139 +493,6 @@ func manifestModelFromImage(layoutTag string, configFile *gcr.ConfigFile, manife return model, compressedBytes } -// unpackLayers unpacks all OCI layers to a target directory using umoci -// Uses go-containerregistry to get the manifest (handles both Docker v2 and OCI v1) -// then converts it to OCI v1 format for umoci's layer unpacker. -func (c *ociClient) unpackLayers(ctx context.Context, layoutTag, targetDir string) error { - // Open OCI layout using go-containerregistry (handles Docker v2 and OCI v1) - path, err := layout.FromPath(c.cacheDir) - if err != nil { - return fmt.Errorf("open oci layout: %w", err) - } - - // Get the image by annotation tag from the layout - img, err := imageByAnnotation(path, layoutTag) - if err != nil { - return fmt.Errorf("find image by tag %s: %w", layoutTag, err) - } - - // Get manifest from go-containerregistry - gcrManifest, err := img.Manifest() - if err != nil { - return fmt.Errorf("get manifest: %w", err) - } - - configFile, err := img.ConfigFile() - if err != nil { - return fmt.Errorf("get config file: %w", err) - } - if err := validateConfigFileForUnpack(layoutTag, gcrManifest, configFile); err != nil { - return err - } - - // Convert go-containerregistry manifest to OCI v1.Manifest for umoci - ociManifest := convertToOCIManifest(gcrManifest) - - // Open the shared OCI layout with umoci for layer unpacking - casEngine, err := dir.Open(c.cacheDir) - if err != nil { - return fmt.Errorf("open oci layout for unpacking: %w", err) - } - defer casEngine.Close() - - // Pre-create target directory (umoci needs it to exist) - if err := os.MkdirAll(targetDir, 0755); err != nil { - return fmt.Errorf("create target dir: %w", err) - } - - // Unpack layers using umoci's layer package with rootless mode - // Map container UIDs to current user's UID (identity mapping) - uid := uint32(os.Getuid()) - gid := uint32(os.Getgid()) - - unpackOpts := &layer.UnpackOptions{ - OnDiskFormat: layer.DirRootfs{ - MapOptions: layer.MapOptions{ - Rootless: true, // Don't fail on chown errors - UIDMappings: []rspec.LinuxIDMapping{ - {HostID: uid, ContainerID: 0, Size: 1}, // Map container root to current user - }, - GIDMappings: []rspec.LinuxIDMapping{ - {HostID: gid, ContainerID: 0, Size: 1}, // Map container root group to current user group - }, - }, - }, - } - - err = layer.UnpackRootfs(ctx, casEngine, targetDir, ociManifest, unpackOpts) - if err != nil { - return fmt.Errorf("unpack rootfs: %w", err) - } - - return nil -} - -// validateConfigFileForUnpack rejects malformed image configs before calling -// umoci. In particular, we verify that the config blob resolves to a real OCI -// image config, that it declares a layered rootfs, and that rootfs.diff_ids has -// one entry per manifest layer so umoci won't index past the end of the slice. -func validateConfigFileForUnpack(layoutTag string, manifest *gcr.Manifest, configFile *gcr.ConfigFile) error { - if convertToOCIMediaType(string(manifest.Config.MediaType)) != v1.MediaTypeImageConfig { - return fmt.Errorf( - "unpack rootfs: config blob is not correct mediatype %s: %s", - v1.MediaTypeImageConfig, - manifest.Config.MediaType, - ) - } - if configFile.RootFS.Type != "layers" { - return fmt.Errorf("unpack rootfs: config: unsupported rootfs.type: %s", configFile.RootFS.Type) - } - if len(configFile.RootFS.DiffIDs) != len(manifest.Layers) { - return fmt.Errorf( - "unpack rootfs: config rootfs.diff_ids has %d entries but manifest has %d layers for %s", - len(configFile.RootFS.DiffIDs), - len(manifest.Layers), - layoutTag, - ) - } - return nil -} - -// convertToOCIManifest converts a go-containerregistry manifest to OCI v1.Manifest -// This allows us to use go-containerregistry (which handles both Docker v2 and OCI v1) -// for manifest parsing, while still using umoci for layer unpacking. -// Docker v2 mediatypes are converted to OCI equivalents since umoci expects OCI format. -func convertToOCIManifest(gcrManifest *gcr.Manifest) v1.Manifest { - // Convert config descriptor with mediatype conversion - configDesc := v1.Descriptor{ - MediaType: convertToOCIMediaType(string(gcrManifest.Config.MediaType)), - Digest: gcrDigestToOCI(gcrManifest.Config.Digest), - Size: gcrManifest.Config.Size, - Annotations: gcrManifest.Config.Annotations, - } - - // Convert layer descriptors with mediatype conversion - layers := make([]v1.Descriptor, len(gcrManifest.Layers)) - for i, layer := range gcrManifest.Layers { - layers[i] = v1.Descriptor{ - MediaType: convertToOCIMediaType(string(layer.MediaType)), - Digest: gcrDigestToOCI(layer.Digest), - Size: layer.Size, - Annotations: layer.Annotations, - } - } - - return v1.Manifest{ - Versioned: specs.Versioned{ - SchemaVersion: int(gcrManifest.SchemaVersion), - }, - MediaType: convertToOCIMediaType(string(gcrManifest.MediaType)), - Config: configDesc, - Layers: layers, - Annotations: gcrManifest.Annotations, - } -} - // convertToOCIMediaType converts Docker v2 media types to OCI equivalents. // Images from Docker Hub often use Docker-specific mediatypes, but umoci // requires OCI-standard mediatypes for layer unpacking. @@ -651,11 +514,6 @@ func convertToOCIMediaType(mediaType string) string { } } -// gcrDigestToOCI converts a go-containerregistry digest to OCI digest -func gcrDigestToOCI(d gcr.Hash) digest.Digest { - return digest.NewDigestFromEncoded(digest.Algorithm(d.Algorithm), d.Hex) -} - type containerMetadata struct { OS string Architecture string diff --git a/lib/images/oci_test.go b/lib/images/oci_test.go index 2cb765cc5..9febe59c8 100644 --- a/lib/images/oci_test.go +++ b/lib/images/oci_test.go @@ -42,17 +42,16 @@ const testImageKernelVersion = "ch-6.12.8-kernel-1.6-202603301" // cache with image-manifest=true const buildKitCacheConfigMediaType = "application/vnd.buildkit.cacheconfig.v0" -// TestUnpackLayersFailsOnBuildKitCacheMediatype verifies that hypeman's image -// unpacker fails when encountering BuildKit cache images. This reproduces the -// production issue where global cache images exported by BuildKit cannot be -// pre-pulled by hypeman because they use a non-standard config mediatype. +// TestComposeRootfsFailsOnBuildKitCacheMediatype verifies that rootfs +// composition fails when encountering BuildKit cache images. This reproduces +// the production issue where global cache images exported by BuildKit cannot +// be pre-pulled by hypeman because they use a non-standard config mediatype. // // The error occurs because: // 1. BuildKit exports cache with --export-cache type=registry,image-manifest=true // 2. The exported manifest uses "application/vnd.buildkit.cacheconfig.v0" as config mediatype -// 3. hypeman's unpackLayers expects "application/vnd.oci.image.config.v1+json" -// 4. umoci.UnpackRootfs fails with "config blob is not correct mediatype" -func TestUnpackLayersFailsOnBuildKitCacheMediatype(t *testing.T) { +// 3. hypeman's manifest validation expects "application/vnd.oci.image.config.v1+json" +func TestComposeRootfsFailsOnBuildKitCacheMediatype(t *testing.T) { // Create a temp directory for the OCI layout cacheDir := t.TempDir() @@ -60,24 +59,27 @@ func TestUnpackLayersFailsOnBuildKitCacheMediatype(t *testing.T) { err := createBuildKitCacheLayout(cacheDir, "test-cache") require.NoError(t, err, "failed to create mock BuildKit cache layout") - // Create OCI client and try to unpack + // Create OCI client and extract the bundle client, err := newOCIClient(cacheDir) require.NoError(t, err) + bundle, err := client.extractOCIImageBundle("test-cache") + require.NoError(t, err) - targetDir := t.TempDir() - err = client.unpackLayers(context.Background(), "test-cache", targetDir) + err = client.composeRootfsContext(context.Background(), filepath.Join(t.TempDir(), "rootfs"), "test-cache", bundle.Model) - // This should fail with a mediatype error - require.Error(t, err, "unpackLayers should fail on BuildKit cache mediatype") - assert.Contains(t, err.Error(), "config", "error should mention config") + // The rejection happens during manifest-model validation: the cacheconfig + // mediatype is not an OCI image config, and the config blob declares no + // layered rootfs, so composition refuses the image. + require.Error(t, err, "compose should fail on BuildKit cache mediatype") + assert.Contains(t, err.Error(), "manifest", "error should come from manifest model validation") t.Logf("Got expected error: %v", err) } -// TestExtractMetadataSucceedsOnBuildKitCache verifies that extractOCIMetadata +// TestExtractMetadataSucceedsOnBuildKitCache verifies that extractOCIImageBundle // does NOT fail on BuildKit cache images - it's go-containerregistry which is -// lenient about mediatypes. The failure only happens during unpackLayers when -// umoci tries to unpack the rootfs. +// lenient about mediatypes. The failure only happens during composition when +// the manifest model is validated. func TestExtractMetadataSucceedsOnBuildKitCache(t *testing.T) { cacheDir := t.TempDir() @@ -88,7 +90,6 @@ func TestExtractMetadataSucceedsOnBuildKitCache(t *testing.T) { require.NoError(t, err) // This succeeds because go-containerregistry doesn't validate config mediatype - // The failure only happens in unpackLayers when umoci validates the config bundle, err := client.extractOCIImageBundle("test-cache") require.NoError(t, err, "extractOCIImageBundle succeeds - go-containerregistry is lenient") @@ -292,7 +293,7 @@ func createTestDockerImage(t *testing.T) v1.Image { // TestDockerSaveTarballToOCILayoutRoundtrip tests the exact pipeline used by // buildBuilderFromDockerfile: docker save tarball → load via go-containerregistry -// → write to OCI layout cache → verify existsInLayout + extractMetadata + unpackLayers. +// → write to OCI layout cache → verify existsInLayout + extractMetadata + composeRootfsContext. // // This simulates: // 1. docker build → docker save (we use go-containerregistry to create the tarball) @@ -300,7 +301,7 @@ func createTestDockerImage(t *testing.T) v1.Image { // 3. layout.AppendImage with digest annotation (write to OCI cache) // 4. existsInLayout (cache hit detection) // 5. extractOCIMetadata (read config from cache) -// 6. unpackLayers (unpack rootfs from cache) +// 6. composeRootfsContext (compose rootfs from cache blobs) func TestDockerSaveTarballToOCILayoutRoundtrip(t *testing.T) { // Step 1: Create a synthetic Docker image (simulates docker build output) img := createTestDockerImage(t) @@ -347,10 +348,9 @@ func TestDockerSaveTarballToOCILayoutRoundtrip(t *testing.T) { assert.Equal(t, testImageKernelVersion, meta.Labels["io.kernel.kernel-version"]) assert.Equal(t, "6.12.8+", meta.Labels["io.kernel.kernel-release"]) - // Step 7: Verify unpackLayers produces correct rootfs - // umoci's UnpackRootfs extracts directly into the target directory + // Step 7: Verify composeRootfsContext produces correct rootfs unpackDir := filepath.Join(t.TempDir(), "unpack") - err = client.unpackLayers(context.Background(), layoutTag, unpackDir) + err = client.composeRootfsContext(context.Background(), unpackDir, layoutTag, bundle.Model) require.NoError(t, err) // Verify expected files exist in unpacked rootfs @@ -369,7 +369,7 @@ func TestDockerSaveTarballToOCILayoutRoundtrip(t *testing.T) { require.NoError(t, err, "/app directory should exist") assert.True(t, stat.IsDir()) - t.Log("Full roundtrip verified: docker save tarball → OCI layout → existsInLayout → extractMetadata → unpackLayers") + t.Log("Full roundtrip verified: docker save tarball → OCI layout → existsInLayout → extractMetadata → composeRootfsContext") } // TestDockerSaveToOCILayoutCacheHit verifies that pullAndExport correctly diff --git a/lib/images/recovery_regression_test.go b/lib/images/recovery_regression_test.go index ad3398316..0a33393e7 100644 --- a/lib/images/recovery_regression_test.go +++ b/lib/images/recovery_regression_test.go @@ -21,19 +21,22 @@ const ( recoveryFixtureDigestHex = "073e2a02f0df492def76940a909b6b79b896fc8907cceeb03452b250697d98fa" ) -func TestUnpackLayersCapturedFixtureReturnsErrorInsteadOfPanicking(t *testing.T) { +func TestComposeRootfsCapturedFixtureReturnsErrorInsteadOfPanicking(t *testing.T) { dataDir := copyRecoveryFixture(t) client, err := newOCIClient(filepath.Join(dataDir, "system", "oci-cache")) require.NoError(t, err) - var unpackErr error + bundle, err := client.extractOCIImageBundle(recoveryFixtureTag) + require.NoError(t, err) + + var composeErr error require.NotPanics(t, func() { - unpackErr = client.unpackLayers(context.Background(), recoveryFixtureTag, filepath.Join(t.TempDir(), "rootfs")) + composeErr = client.composeRootfsContext(context.Background(), filepath.Join(t.TempDir(), "rootfs"), recoveryFixtureTag, bundle.Model) }) - require.Error(t, unpackErr) - assert.Contains(t, unpackErr.Error(), "config rootfs.diff_ids has 0 entries but manifest has 1 layers") + require.Error(t, composeErr) + assert.Contains(t, composeErr.Error(), "manifest model has 0 diff ids for 1 layers") } func TestRecoverInterruptedBuildsCapturedFixtureMarksBuildFailed(t *testing.T) { From a434f0a62c82a57859a149e549920646188d9b30 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 3 Sep 2026 20:35:22 +0000 Subject: [PATCH 07/13] Review cleanups for the composition path Name the compose entry point composeRootfs like the other ctx-taking methods, derive the cache blob directory in one place, and describe the replace-on-compose semantics in the doc comment. Tighten the BuildKit cache test assertion to the actual rejection, drop the rootfs_type omitempty now that validation requires it, and fix the compose rootfs error wrap. --- lib/images/compose.go | 17 +++++++------ lib/images/compose_test.go | 34 +++++++++++--------------- lib/images/manifest_model.go | 2 +- lib/images/oci.go | 10 ++++++-- lib/images/oci_test.go | 19 +++++++------- lib/images/recovery_regression_test.go | 2 +- 6 files changed, 43 insertions(+), 41 deletions(-) diff --git a/lib/images/compose.go b/lib/images/compose.go index 3f3432a2d..a7efead86 100644 --- a/lib/images/compose.go +++ b/lib/images/compose.go @@ -8,11 +8,13 @@ import ( "path/filepath" ) -// composeRootfsContext validates the persisted model and merges its layers -// into dest in manifest order, reading each layer blob from the shared OCI -// cache. Whiteout and opaque-directory markers are interpreted as each layer -// is applied. -func (c *ociClient) composeRootfsContext(ctx context.Context, dest, layoutTag string, model *imageManifestModel) error { +// composeRootfs validates the persisted model and merges its layers into +// dest in manifest order, reading each layer blob from the shared OCI cache. +// Whiteout and opaque-directory markers are interpreted as each layer is +// applied. Any previous tree at dest is replaced: callers must not read dest +// concurrently, and a failure between the remove and the rename leaves dest +// absent. +func (c *ociClient) composeRootfs(ctx context.Context, dest, layoutTag string, model *imageManifestModel) error { if err := validateManifestModel(layoutTag, model); err != nil { return fmt.Errorf("validate manifest model: %w", err) } @@ -31,11 +33,12 @@ func (c *ociClient) composeRootfsContext(ctx context.Context, dest, layoutTag st }() for i, desc := range model.Layers { - if _, err := unpackCachedLayer(ctx, filepath.Join(c.cacheDir, "blobs", "sha256"), desc, staging, composeOnDiskFormat()); err != nil { + if _, err := unpackCachedLayer(ctx, c.cacheBlobDir(), desc, staging, composeOnDiskFormat()); err != nil { return fmt.Errorf("apply layer %d (%s): %w", i, desc.Digest, err) } } - // Match the export-directory mode the unpack path used; MkdirTemp is 0700. + // The export directory must stay traversable by other readers; MkdirTemp + // creates it 0700. if err := os.Chmod(staging, 0755); err != nil { return fmt.Errorf("set compose directory mode: %w", err) } diff --git a/lib/images/compose_test.go b/lib/images/compose_test.go index bf9f637f7..71307e579 100644 --- a/lib/images/compose_test.go +++ b/lib/images/compose_test.go @@ -69,7 +69,7 @@ func TestComposeRootfsWhiteoutsAndOrdering(t *testing.T) { require.Len(t, model.Layers, 2) dest := filepath.Join(t.TempDir(), "rootfs") - require.NoError(t, client.composeRootfsContext(context.Background(), dest, tag, model)) + require.NoError(t, client.composeRootfs(context.Background(), dest, tag, model)) // Whiteout removed the base entry. _, err := os.Lstat(filepath.Join(dest, "etc", "config.txt")) @@ -108,24 +108,19 @@ func TestComposeRootfsWhiteoutsAndOrdering(t *testing.T) { })) } -// zeroLayerModel returns a schema-valid manifest model with no layers. -func zeroLayerModel() *imageManifestModel { - return &imageManifestModel{ +func TestComposeRootfsEmptyLayers(t *testing.T) { + p := paths.New(t.TempDir()) + client, err := newOCIClient(p.SystemOCICache()) + require.NoError(t, err) + model := &imageManifestModel{ SchemaVersion: manifestModelSchemaVersion, Digest: "sha256:" + strings.Repeat("ab", 32), RootFSType: "layers", Config: manifestConfigRef{Digest: "sha256:" + strings.Repeat("cd", 32)}, Layers: make([]layerDescriptor, 0), } -} - -func TestComposeRootfsEmptyLayers(t *testing.T) { - p := paths.New(t.TempDir()) - client, err := newOCIClient(p.SystemOCICache()) - require.NoError(t, err) - model := zeroLayerModel() dest := filepath.Join(t.TempDir(), "rootfs") - require.NoError(t, client.composeRootfsContext(context.Background(), dest, model.Digest, model)) + require.NoError(t, client.composeRootfs(context.Background(), dest, model.Digest, model)) entries, err := os.ReadDir(dest) require.NoError(t, err) require.Empty(t, entries) @@ -136,7 +131,7 @@ func TestComposeRootfsInvalidModel(t *testing.T) { client, tag, model := composeFixture(t, p) model.Config.DiffIDs = model.Config.DiffIDs[:1] - err := client.composeRootfsContext(context.Background(), filepath.Join(t.TempDir(), "rootfs"), tag, model) + err := client.composeRootfs(context.Background(), filepath.Join(t.TempDir(), "rootfs"), tag, model) require.ErrorContains(t, err, "1 diff ids for 2 layers") } @@ -160,7 +155,7 @@ func TestComposeRootfsMissingBlob(t *testing.T) { DiffID: "sha256:" + strings.Repeat("ef", 32), }}, } - err = client.composeRootfsContext(context.Background(), t.TempDir(), digestHex, model) + err = client.composeRootfs(context.Background(), t.TempDir(), digestHex, model) require.ErrorContains(t, err, "missing from oci cache") } @@ -175,13 +170,12 @@ func TestComposeRootfsDiffIDMismatch(t *testing.T) { model.Layers[1].DiffID = forged model.Config.DiffIDs[1] = forged - err := client.composeRootfsContext(context.Background(), filepath.Join(t.TempDir(), "rootfs"), tag, model) + err := client.composeRootfs(context.Background(), filepath.Join(t.TempDir(), "rootfs"), tag, model) require.ErrorContains(t, err, "diff id mismatch") } // TestComposeRootfsExportsValidErofs composes the fixture image and exports it -// to erofs, then verifies the filesystem is intact and its contents match the -// composed tree. +// to erofs, then verifies the filesystem passes fsck. func TestComposeRootfsExportsValidErofs(t *testing.T) { if _, err := exec.LookPath("mkfs.erofs"); err != nil { t.Skip("mkfs.erofs not available") @@ -193,11 +187,11 @@ func TestComposeRootfsExportsValidErofs(t *testing.T) { p := paths.New(t.TempDir()) client, tag, model := composeFixture(t, p) - staging := filepath.Join(t.TempDir(), "rootfs") - require.NoError(t, client.composeRootfsContext(context.Background(), staging, tag, model)) + dest := filepath.Join(t.TempDir(), "rootfs") + require.NoError(t, client.composeRootfs(context.Background(), dest, tag, model)) diskPath := filepath.Join(t.TempDir(), "rootfs.erofs") - size, err := ExportRootfs(staging, diskPath, FormatErofs) + size, err := ExportRootfs(dest, diskPath, FormatErofs) require.NoError(t, err) require.Greater(t, size, int64(0)) diff --git a/lib/images/manifest_model.go b/lib/images/manifest_model.go index f5b91d8e9..1c4cad1ab 100644 --- a/lib/images/manifest_model.go +++ b/lib/images/manifest_model.go @@ -23,7 +23,7 @@ type imageManifestModel struct { MediaType string `json:"media_type,omitempty"` Platform string `json:"platform"` // os/arch[/variant] Config manifestConfigRef `json:"config"` - RootFSType string `json:"rootfs_type,omitempty"` + RootFSType string `json:"rootfs_type"` Layers []layerDescriptor `json:"layers"` // manifest order, base layer first } diff --git a/lib/images/oci.go b/lib/images/oci.go index 8cab6f630..f70ee65dc 100644 --- a/lib/images/oci.go +++ b/lib/images/oci.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path/filepath" "strings" "time" @@ -73,6 +74,11 @@ func newOCIClient(cacheDir string) (*ociClient, error) { return &ociClient{cacheDir: cacheDir}, nil } +// cacheBlobDir returns the OCI layout's blob directory under the cache root. +func (c *ociClient) cacheBlobDir() string { + return filepath.Join(c.cacheDir, "blobs", "sha256") +} + // vmPlatform returns the target platform for VM images: a Linux guest on the // host architecture. Hypeman VMs are always Linux regardless of host OS. func vmPlatform() gcr.Platform { @@ -281,9 +287,9 @@ func (c *ociClient) pullAndExportWithPlatformAuth(ctx context.Context, imageRef, // Compose the rootfs from the shared layer blobs in manifest order. if err := result.measure("layer_unpack", func() error { - return c.composeRootfsContext(ctx, exportDir, layoutTag, bundle.Model) + return c.composeRootfs(ctx, exportDir, layoutTag, bundle.Model) }); err != nil { - return result, fmt.Errorf("unpack layers: %w", err) + return result, fmt.Errorf("compose rootfs: %w", err) } return result, nil diff --git a/lib/images/oci_test.go b/lib/images/oci_test.go index 9febe59c8..4c2badfcd 100644 --- a/lib/images/oci_test.go +++ b/lib/images/oci_test.go @@ -65,13 +65,12 @@ func TestComposeRootfsFailsOnBuildKitCacheMediatype(t *testing.T) { bundle, err := client.extractOCIImageBundle("test-cache") require.NoError(t, err) - err = client.composeRootfsContext(context.Background(), filepath.Join(t.TempDir(), "rootfs"), "test-cache", bundle.Model) + err = client.composeRootfs(context.Background(), filepath.Join(t.TempDir(), "rootfs"), "test-cache", bundle.Model) - // The rejection happens during manifest-model validation: the cacheconfig - // mediatype is not an OCI image config, and the config blob declares no - // layered rootfs, so composition refuses the image. + // The cacheconfig config blob declares no layered rootfs, so manifest model + // validation rejects it before any blob is read. require.Error(t, err, "compose should fail on BuildKit cache mediatype") - assert.Contains(t, err.Error(), "manifest", "error should come from manifest model validation") + assert.Contains(t, err.Error(), "rootfs type", "error should be the rootfs type rejection") t.Logf("Got expected error: %v", err) } @@ -293,7 +292,7 @@ func createTestDockerImage(t *testing.T) v1.Image { // TestDockerSaveTarballToOCILayoutRoundtrip tests the exact pipeline used by // buildBuilderFromDockerfile: docker save tarball → load via go-containerregistry -// → write to OCI layout cache → verify existsInLayout + extractMetadata + composeRootfsContext. +// → write to OCI layout cache → verify existsInLayout + extractMetadata + composeRootfs. // // This simulates: // 1. docker build → docker save (we use go-containerregistry to create the tarball) @@ -301,7 +300,7 @@ func createTestDockerImage(t *testing.T) v1.Image { // 3. layout.AppendImage with digest annotation (write to OCI cache) // 4. existsInLayout (cache hit detection) // 5. extractOCIMetadata (read config from cache) -// 6. composeRootfsContext (compose rootfs from cache blobs) +// 6. composeRootfs (compose rootfs from cache blobs) func TestDockerSaveTarballToOCILayoutRoundtrip(t *testing.T) { // Step 1: Create a synthetic Docker image (simulates docker build output) img := createTestDockerImage(t) @@ -348,9 +347,9 @@ func TestDockerSaveTarballToOCILayoutRoundtrip(t *testing.T) { assert.Equal(t, testImageKernelVersion, meta.Labels["io.kernel.kernel-version"]) assert.Equal(t, "6.12.8+", meta.Labels["io.kernel.kernel-release"]) - // Step 7: Verify composeRootfsContext produces correct rootfs + // Step 7: Verify composeRootfs produces correct rootfs unpackDir := filepath.Join(t.TempDir(), "unpack") - err = client.composeRootfsContext(context.Background(), unpackDir, layoutTag, bundle.Model) + err = client.composeRootfs(context.Background(), unpackDir, layoutTag, bundle.Model) require.NoError(t, err) // Verify expected files exist in unpacked rootfs @@ -369,7 +368,7 @@ func TestDockerSaveTarballToOCILayoutRoundtrip(t *testing.T) { require.NoError(t, err, "/app directory should exist") assert.True(t, stat.IsDir()) - t.Log("Full roundtrip verified: docker save tarball → OCI layout → existsInLayout → extractMetadata → composeRootfsContext") + t.Log("Full roundtrip verified: docker save tarball → OCI layout → existsInLayout → extractMetadata → composeRootfs") } // TestDockerSaveToOCILayoutCacheHit verifies that pullAndExport correctly diff --git a/lib/images/recovery_regression_test.go b/lib/images/recovery_regression_test.go index 0a33393e7..489925dee 100644 --- a/lib/images/recovery_regression_test.go +++ b/lib/images/recovery_regression_test.go @@ -32,7 +32,7 @@ func TestComposeRootfsCapturedFixtureReturnsErrorInsteadOfPanicking(t *testing.T var composeErr error require.NotPanics(t, func() { - composeErr = client.composeRootfsContext(context.Background(), filepath.Join(t.TempDir(), "rootfs"), recoveryFixtureTag, bundle.Model) + composeErr = client.composeRootfs(context.Background(), filepath.Join(t.TempDir(), "rootfs"), recoveryFixtureTag, bundle.Model) }) require.Error(t, composeErr) From e5dcf39a22fef4aeee10cf941dbbdcf6c9c58db9 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 3 Sep 2026 20:58:14 +0000 Subject: [PATCH 08/13] Pin compose validation rejections and replacement semantics in tests Test the config mediatype rejection for BuildKit cache images separately from the rootfs type rejection, cover composition into a pre-populated export directory and the restored 0755 mode, assert the rootfs type guard directly, drop the dead diff_id omitempty tag, and deduplicate the layer digest in the apply-layer error wrap. --- lib/images/compose.go | 5 ++- lib/images/compose_test.go | 15 ++++++- lib/images/manifest_model.go | 2 +- lib/images/manifest_model_test.go | 19 ++++++++- lib/images/oci_test.go | 70 +++++++++++++++++++++++++++++-- 5 files changed, 100 insertions(+), 11 deletions(-) diff --git a/lib/images/compose.go b/lib/images/compose.go index a7efead86..a44ba08b7 100644 --- a/lib/images/compose.go +++ b/lib/images/compose.go @@ -13,7 +13,8 @@ import ( // Whiteout and opaque-directory markers are interpreted as each layer is // applied. Any previous tree at dest is replaced: callers must not read dest // concurrently, and a failure between the remove and the rename leaves dest -// absent. +// absent. A crash can also strand .compose-* staging directories in dest's +// parent, the same way .unpack-* directories can strand under layer builds. func (c *ociClient) composeRootfs(ctx context.Context, dest, layoutTag string, model *imageManifestModel) error { if err := validateManifestModel(layoutTag, model); err != nil { return fmt.Errorf("validate manifest model: %w", err) @@ -34,7 +35,7 @@ func (c *ociClient) composeRootfs(ctx context.Context, dest, layoutTag string, m for i, desc := range model.Layers { if _, err := unpackCachedLayer(ctx, c.cacheBlobDir(), desc, staging, composeOnDiskFormat()); err != nil { - return fmt.Errorf("apply layer %d (%s): %w", i, desc.Digest, err) + return fmt.Errorf("apply layer %d: %w", i, err) } } // The export directory must stay traversable by other readers; MkdirTemp diff --git a/lib/images/compose_test.go b/lib/images/compose_test.go index 71307e579..5771183a9 100644 --- a/lib/images/compose_test.go +++ b/lib/images/compose_test.go @@ -69,10 +69,21 @@ func TestComposeRootfsWhiteoutsAndOrdering(t *testing.T) { require.Len(t, model.Layers, 2) dest := filepath.Join(t.TempDir(), "rootfs") + // Pre-populate dest and weaken its mode so composition must replace the + // whole tree and restore the 0755 export-directory mode. + require.NoError(t, os.MkdirAll(filepath.Join(dest, "junk"), 0700)) + require.NoError(t, os.WriteFile(filepath.Join(dest, "junk", "stale.txt"), []byte("stale"), 0644)) require.NoError(t, client.composeRootfs(context.Background(), dest, tag, model)) + // Stale content is gone and the export directory mode is restored. + _, err := os.Lstat(filepath.Join(dest, "junk")) + require.True(t, os.IsNotExist(err), "composition must replace the previous tree") + info, err := os.Stat(dest) + require.NoError(t, err) + require.Equal(t, os.FileMode(0755), info.Mode().Perm()) + // Whiteout removed the base entry. - _, err := os.Lstat(filepath.Join(dest, "etc", "config.txt")) + _, err = os.Lstat(filepath.Join(dest, "etc", "config.txt")) require.True(t, os.IsNotExist(err), "whiteout must delete the base entry") // Plain replacement. @@ -88,7 +99,7 @@ func TestComposeRootfsWhiteoutsAndOrdering(t *testing.T) { require.Equal(t, "new", string(data)) // Directory replaced by a regular file. - info, err := os.Lstat(filepath.Join(dest, "replacedir")) + info, err = os.Lstat(filepath.Join(dest, "replacedir")) require.NoError(t, err) require.False(t, info.IsDir()) data, err = os.ReadFile(filepath.Join(dest, "replacedir")) diff --git a/lib/images/manifest_model.go b/lib/images/manifest_model.go index 1c4cad1ab..5fcaf7151 100644 --- a/lib/images/manifest_model.go +++ b/lib/images/manifest_model.go @@ -41,7 +41,7 @@ type layerDescriptor struct { Digest string `json:"digest"` // compressed blob digest, sha256:... Size int64 `json:"size"` // compressed bytes MediaType string `json:"media_type,omitempty"` - DiffID string `json:"diff_id,omitempty"` // uncompressed diff id from the image config + DiffID string `json:"diff_id"` // uncompressed diff id from the image config } // digestFromHex returns the full sha256 digest string for a bare hex value. diff --git a/lib/images/manifest_model_test.go b/lib/images/manifest_model_test.go index fef2e0e80..9f757bfd8 100644 --- a/lib/images/manifest_model_test.go +++ b/lib/images/manifest_model_test.go @@ -22,8 +22,8 @@ func syntheticLayer(t *testing.T, name, content string) gcr.Layer { return specLayer(t, []tarEntrySpec{{name: name, content: content, mode: 0644}}) } -// writeSyntheticLayout writes img into a fresh OCI layout cache tagged with the -// image's own digest, mirroring pullToOCILayout. +// writeSyntheticLayout writes img into a temp-dir-backed paths root and +// returns a client for its cache plus the image's layout tag. func writeSyntheticLayout(t *testing.T, img gcr.Image) (*ociClient, string) { t.Helper() @@ -136,6 +136,21 @@ func TestReadManifestModelRejectsInvalidSchema(t *testing.T) { require.ErrorContains(t, err, "config digest is empty") } +func TestValidateManifestModelRejectsNonLayeredRootFS(t *testing.T) { + model := &imageManifestModel{ + SchemaVersion: manifestModelSchemaVersion, + Digest: "sha256:" + strings.Repeat("ab", 32), + RootFSType: "", + Config: manifestConfigRef{Digest: "sha256:" + strings.Repeat("cd", 32)}, + } + err := validateManifestModel(model.Digest, model) + require.ErrorContains(t, err, "rootfs type") + + model.RootFSType = "flattened" + err = validateManifestModel(model.Digest, model) + require.ErrorContains(t, err, "rootfs type") +} + func TestReadManifestModelMissing(t *testing.T) { p := paths.New(t.TempDir()) model, err := readManifestModel(p, "deadbeef") diff --git a/lib/images/oci_test.go b/lib/images/oci_test.go index 4c2badfcd..56510c314 100644 --- a/lib/images/oci_test.go +++ b/lib/images/oci_test.go @@ -47,10 +47,8 @@ const buildKitCacheConfigMediaType = "application/vnd.buildkit.cacheconfig.v0" // the production issue where global cache images exported by BuildKit cannot // be pre-pulled by hypeman because they use a non-standard config mediatype. // -// The error occurs because: -// 1. BuildKit exports cache with --export-cache type=registry,image-manifest=true -// 2. The exported manifest uses "application/vnd.buildkit.cacheconfig.v0" as config mediatype -// 3. hypeman's manifest validation expects "application/vnd.oci.image.config.v1+json" +// The fixture's cacheconfig blob declares no layered rootfs, so manifest +// model validation rejects the image before any blob is read. func TestComposeRootfsFailsOnBuildKitCacheMediatype(t *testing.T) { // Create a temp directory for the OCI layout cacheDir := t.TempDir() @@ -75,6 +73,70 @@ func TestComposeRootfsFailsOnBuildKitCacheMediatype(t *testing.T) { t.Logf("Got expected error: %v", err) } +// TestComposeRootfsFailsOnBuildKitCacheConfigMediatype pins the config +// mediatype rejection specifically: with a layered rootfs declared, the +// cacheconfig config mediatype itself is what fails validation. +func TestComposeRootfsFailsOnBuildKitCacheConfigMediatype(t *testing.T) { + cacheDir := t.TempDir() + blobsDir := filepath.Join(cacheDir, "blobs", "sha256") + require.NoError(t, os.MkdirAll(blobsDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(cacheDir, "oci-layout"), []byte(`{"imageLayoutVersion":"1.0.0"}`), 0644)) + + layerContent := []byte{ + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, // gzip header + 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // empty tar + } + layerDigest := sha256Hash(layerContent) + require.NoError(t, os.WriteFile(filepath.Join(blobsDir, layerDigest), layerContent, 0644)) + + // BuildKit cacheconfig-shaped config that nonetheless declares a layered + // rootfs, so the config mediatype check is the one that fires. + configJSON := []byte(`{"rootfs":{"type":"layers","diff_ids":["sha256:` + layerDigest + `"]}}`) + configDigest := sha256Hash(configJSON) + require.NoError(t, os.WriteFile(filepath.Join(blobsDir, configDigest), configJSON, 0644)) + + manifest := map[string]interface{}{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": map[string]interface{}{ + "mediaType": buildKitCacheConfigMediaType, + "digest": "sha256:" + configDigest, + "size": len(configJSON), + }, + "layers": []map[string]interface{}{{ + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "digest": "sha256:" + layerDigest, + "size": len(layerContent), + }}, + } + manifestBytes, err := json.Marshal(manifest) + require.NoError(t, err) + manifestDigest := sha256Hash(manifestBytes) + require.NoError(t, os.WriteFile(filepath.Join(blobsDir, manifestDigest), manifestBytes, 0644)) + + index := map[string]interface{}{ + "schemaVersion": 2, + "manifests": []map[string]interface{}{{ + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:" + manifestDigest, + "size": len(manifestBytes), + "annotations": map[string]string{"org.opencontainers.image.ref.name": "test-cache"}, + }}, + } + indexBytes, err := json.Marshal(index) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(cacheDir, "index.json"), indexBytes, 0644)) + + client, err := newOCIClient(cacheDir) + require.NoError(t, err) + bundle, err := client.extractOCIImageBundle("test-cache") + require.NoError(t, err) + + err = client.composeRootfs(context.Background(), filepath.Join(t.TempDir(), "rootfs"), "test-cache", bundle.Model) + require.Error(t, err) + assert.Contains(t, err.Error(), "config media type", "error should be the config mediatype rejection") +} + // TestExtractMetadataSucceedsOnBuildKitCache verifies that extractOCIImageBundle // does NOT fail on BuildKit cache images - it's go-containerregistry which is // lenient about mediatypes. The failure only happens during composition when From e2cdd1af2adb1b963c067073743f476c298c9f0a Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:49:09 +0000 Subject: [PATCH 09/13] Restore empty config mediatype rejection and document export root mode validateManifestModel now rejects an empty config media type, matching the old unpack path's rejection surface; hand-built test models declare the OCI config mediatype so each test isolates the check it targets. Document that the export root is always 0755 and dedupe a duplicated test comment. --- lib/images/compose.go | 6 ++++-- lib/images/compose_test.go | 12 ++++++++---- lib/images/manifest_model.go | 2 +- lib/images/manifest_model_test.go | 10 +++++++--- lib/images/oci_test.go | 3 --- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/images/compose.go b/lib/images/compose.go index a44ba08b7..9df8c6175 100644 --- a/lib/images/compose.go +++ b/lib/images/compose.go @@ -13,8 +13,10 @@ import ( // Whiteout and opaque-directory markers are interpreted as each layer is // applied. Any previous tree at dest is replaced: callers must not read dest // concurrently, and a failure between the remove and the rename leaves dest -// absent. A crash can also strand .compose-* staging directories in dest's -// parent, the same way .unpack-* directories can strand under layer builds. +// absent. The export root is always 0755 regardless of the last layer's tar +// root entry, matching the mode the previous unpack path created. A crash +// can also strand .compose-* staging directories in dest's parent, the same +// way .unpack-* directories can strand under layer builds. func (c *ociClient) composeRootfs(ctx context.Context, dest, layoutTag string, model *imageManifestModel) error { if err := validateManifestModel(layoutTag, model); err != nil { return fmt.Errorf("validate manifest model: %w", err) diff --git a/lib/images/compose_test.go b/lib/images/compose_test.go index 5771183a9..d6768de62 100644 --- a/lib/images/compose_test.go +++ b/lib/images/compose_test.go @@ -127,8 +127,11 @@ func TestComposeRootfsEmptyLayers(t *testing.T) { SchemaVersion: manifestModelSchemaVersion, Digest: "sha256:" + strings.Repeat("ab", 32), RootFSType: "layers", - Config: manifestConfigRef{Digest: "sha256:" + strings.Repeat("cd", 32)}, - Layers: make([]layerDescriptor, 0), + Config: manifestConfigRef{ + Digest: "sha256:" + strings.Repeat("cd", 32), + MediaType: "application/vnd.oci.image.config.v1+json", + }, + Layers: make([]layerDescriptor, 0), } dest := filepath.Join(t.TempDir(), "rootfs") require.NoError(t, client.composeRootfs(context.Background(), dest, model.Digest, model)) @@ -157,8 +160,9 @@ func TestComposeRootfsMissingBlob(t *testing.T) { Digest: digestHex, RootFSType: "layers", Config: manifestConfigRef{ - Digest: "sha256:" + strings.Repeat("cd", 32), - DiffIDs: []string{"sha256:" + strings.Repeat("ef", 32)}, + Digest: "sha256:" + strings.Repeat("cd", 32), + MediaType: "application/vnd.oci.image.config.v1+json", + DiffIDs: []string{"sha256:" + strings.Repeat("ef", 32)}, }, Layers: []layerDescriptor{{ Digest: "sha256:" + strings.Repeat("01", 32), diff --git a/lib/images/manifest_model.go b/lib/images/manifest_model.go index 5fcaf7151..a945b9f1c 100644 --- a/lib/images/manifest_model.go +++ b/lib/images/manifest_model.go @@ -92,7 +92,7 @@ func validateManifestConfig(model *imageManifestModel) error { if _, err := parseSHA256Digest(model.Config.Digest); err != nil { return fmt.Errorf("invalid manifest model config digest: %q", model.Config.Digest) } - if model.Config.MediaType != "" && convertToOCIMediaType(model.Config.MediaType) != v1.MediaTypeImageConfig { + if convertToOCIMediaType(model.Config.MediaType) != v1.MediaTypeImageConfig { return fmt.Errorf("invalid manifest model config media type: %q", model.Config.MediaType) } if len(model.Config.DiffIDs) != len(model.Layers) { diff --git a/lib/images/manifest_model_test.go b/lib/images/manifest_model_test.go index 9f757bfd8..bb0ebf722 100644 --- a/lib/images/manifest_model_test.go +++ b/lib/images/manifest_model_test.go @@ -105,8 +105,9 @@ func TestManifestModelWriteReadRoundtrip(t *testing.T) { RootFSType: "layers", Platform: "linux/amd64", Config: manifestConfigRef{ - Digest: "sha256:" + strings.Repeat("c", 64), - DiffIDs: []string{"sha256:" + strings.Repeat("d", 64), "sha256:" + strings.Repeat("e", 64)}, + Digest: "sha256:" + strings.Repeat("c", 64), + MediaType: "application/vnd.oci.image.config.v1+json", + DiffIDs: []string{"sha256:" + strings.Repeat("d", 64), "sha256:" + strings.Repeat("e", 64)}, }, Layers: []layerDescriptor{ {Digest: "sha256:" + strings.Repeat("f", 64), Size: 10, DiffID: "sha256:" + strings.Repeat("d", 64)}, @@ -141,7 +142,10 @@ func TestValidateManifestModelRejectsNonLayeredRootFS(t *testing.T) { SchemaVersion: manifestModelSchemaVersion, Digest: "sha256:" + strings.Repeat("ab", 32), RootFSType: "", - Config: manifestConfigRef{Digest: "sha256:" + strings.Repeat("cd", 32)}, + Config: manifestConfigRef{ + Digest: "sha256:" + strings.Repeat("cd", 32), + MediaType: "application/vnd.oci.image.config.v1+json", + }, } err := validateManifestModel(model.Digest, model) require.ErrorContains(t, err, "rootfs type") diff --git a/lib/images/oci_test.go b/lib/images/oci_test.go index 56510c314..bed89ca2c 100644 --- a/lib/images/oci_test.go +++ b/lib/images/oci_test.go @@ -64,9 +64,6 @@ func TestComposeRootfsFailsOnBuildKitCacheMediatype(t *testing.T) { require.NoError(t, err) err = client.composeRootfs(context.Background(), filepath.Join(t.TempDir(), "rootfs"), "test-cache", bundle.Model) - - // The cacheconfig config blob declares no layered rootfs, so manifest model - // validation rejects it before any blob is read. require.Error(t, err, "compose should fail on BuildKit cache mediatype") assert.Contains(t, err.Error(), "rootfs type", "error should be the rootfs type rejection") From c57b92c590dcd48cc5abd1d8891cfa3ec606338d Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:57:23 +0000 Subject: [PATCH 10/13] Key build dirs by digest and restore prior error strings Two pending builds of the same ref with different digests could share one build directory and delete each other's rootfs mid-build; key the build directory by the resolved digest to match the queue's deduplication. Keep the observable error strings from the previous unpack path (the layer_unpack wrap and the config rootfs.diff_ids message) so anything keyed on them keeps matching. --- lib/images/compose_test.go | 2 +- lib/images/manager.go | 5 ++++- lib/images/manifest_model.go | 7 ++++++- lib/images/oci.go | 2 +- lib/images/recovery_regression_test.go | 4 ++-- 5 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/images/compose_test.go b/lib/images/compose_test.go index d6768de62..7e58a0b00 100644 --- a/lib/images/compose_test.go +++ b/lib/images/compose_test.go @@ -146,7 +146,7 @@ func TestComposeRootfsInvalidModel(t *testing.T) { model.Config.DiffIDs = model.Config.DiffIDs[:1] err := client.composeRootfs(context.Background(), filepath.Join(t.TempDir(), "rootfs"), tag, model) - require.ErrorContains(t, err, "1 diff ids for 2 layers") + require.ErrorContains(t, err, "config rootfs.diff_ids has 1 entries but manifest has 2 layers") } func TestComposeRootfsMissingBlob(t *testing.T) { diff --git a/lib/images/manager.go b/lib/images/manager.go index 6ebc221f6..84b6282ef 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -458,7 +458,10 @@ func (m *manager) newPendingImageMetadata(ref *ResolvedRef, req CreateImageReque func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials *authn.AuthConfig, buildID string) { buildStart := time.Now() buildStatus := "failed" - buildDir := m.paths.SystemBuild(ref.String()) + // Key the build directory by digest so two pending builds of the same + // ref with different digests never compose into (and delete) the same + // rootfs. This matches the queue's digest-based deduplication. + buildDir := m.paths.SystemBuild(ref.DigestHex()) tempDir := filepath.Join(buildDir, "rootfs") defer func() { m.recordBuildMetrics(ctx, buildStart, buildStatus) diff --git a/lib/images/manifest_model.go b/lib/images/manifest_model.go index a945b9f1c..24450f2ad 100644 --- a/lib/images/manifest_model.go +++ b/lib/images/manifest_model.go @@ -96,7 +96,12 @@ func validateManifestConfig(model *imageManifestModel) error { return fmt.Errorf("invalid manifest model config media type: %q", model.Config.MediaType) } if len(model.Config.DiffIDs) != len(model.Layers) { - return fmt.Errorf("manifest model has %d diff ids for %d layers", len(model.Config.DiffIDs), len(model.Layers)) + return fmt.Errorf( + "config rootfs.diff_ids has %d entries but manifest has %d layers for %s", + len(model.Config.DiffIDs), + len(model.Layers), + model.Digest, + ) } return nil } diff --git a/lib/images/oci.go b/lib/images/oci.go index f70ee65dc..9fd191024 100644 --- a/lib/images/oci.go +++ b/lib/images/oci.go @@ -289,7 +289,7 @@ func (c *ociClient) pullAndExportWithPlatformAuth(ctx context.Context, imageRef, if err := result.measure("layer_unpack", func() error { return c.composeRootfs(ctx, exportDir, layoutTag, bundle.Model) }); err != nil { - return result, fmt.Errorf("compose rootfs: %w", err) + return result, fmt.Errorf("unpack layers: %w", err) } return result, nil diff --git a/lib/images/recovery_regression_test.go b/lib/images/recovery_regression_test.go index 489925dee..cd7d2e7ed 100644 --- a/lib/images/recovery_regression_test.go +++ b/lib/images/recovery_regression_test.go @@ -36,7 +36,7 @@ func TestComposeRootfsCapturedFixtureReturnsErrorInsteadOfPanicking(t *testing.T }) require.Error(t, composeErr) - assert.Contains(t, composeErr.Error(), "manifest model has 0 diff ids for 1 layers") + assert.Contains(t, composeErr.Error(), "config rootfs.diff_ids has 0 entries but manifest has 1 layers") } func TestRecoverInterruptedBuildsCapturedFixtureMarksBuildFailed(t *testing.T) { @@ -68,7 +68,7 @@ func TestRecoverInterruptedBuildsCapturedFixtureMarksBuildFailed(t *testing.T) { require.NotNil(t, meta.Error) assert.Equal(t, recoveryFixtureDigest, meta.Digest) assert.Equal(t, StatusFailed, meta.Status) - assert.Contains(t, *meta.Error, "manifest model has 0 diff ids for 1 layers") + assert.Contains(t, *meta.Error, "config rootfs.diff_ids has 0 entries but manifest has 1 layers") } func copyRecoveryFixture(t *testing.T) string { From 13ccbd5e61972aa61a70cb8966ddd4ec0cd5d89d Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:10:46 +0000 Subject: [PATCH 11/13] Match the legacy error suffix and guard empty digest in builds Pass the layout digest hex into the config validation so the rootfs.diff_ids error ends with the same bare-hex suffix the old unpack path emitted, fail builds whose ref resolved without a digest before they can derive a build directory from an empty key, and rename the SystemBuild parameter to reflect what callers now pass. --- lib/images/layer_artifact_test.go | 7 ------- lib/images/manager.go | 7 ++++++- lib/images/manifest_model.go | 6 +++--- lib/paths/paths.go | 7 ++++--- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/images/layer_artifact_test.go b/lib/images/layer_artifact_test.go index 596c44196..cad19c8e5 100644 --- a/lib/images/layer_artifact_test.go +++ b/lib/images/layer_artifact_test.go @@ -21,7 +21,6 @@ import ( "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/kernel/hypeman/lib/paths" "github.com/klauspost/compress/zstd" - "github.com/opencontainers/umoci/oci/layer" "github.com/stretchr/testify/require" "golang.org/x/sys/unix" ) @@ -33,12 +32,6 @@ import ( // later be stacked. const whiteoutPrefix = ".wh." -// composeOnDiskFormat applies whiteouts against the tree being composed. It -// belongs to the composition flow and moves to production with that change. -func composeOnDiskFormat() layer.OnDiskFormat { - return layer.DirRootfs{MapOptions: layerMapOptions()} -} - const testTarGzMediaType = "application/vnd.oci.image.layer.v1.tar+gzip" // writeLayerTestLayout writes img into the shared OCI cache of p tagged with diff --git a/lib/images/manager.go b/lib/images/manager.go index 84b6282ef..534a9e4dc 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -461,7 +461,12 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials // Key the build directory by digest so two pending builds of the same // ref with different digests never compose into (and delete) the same // rootfs. This matches the queue's digest-based deduplication. - buildDir := m.paths.SystemBuild(ref.DigestHex()) + digestHex := ref.DigestHex() + if digestHex == "" { + m.updateStatusByDigest(ref, StatusFailed, fmt.Errorf("missing resolved digest"), buildID) + return + } + buildDir := m.paths.SystemBuild(digestHex) tempDir := filepath.Join(buildDir, "rootfs") defer func() { m.recordBuildMetrics(ctx, buildStart, buildStatus) diff --git a/lib/images/manifest_model.go b/lib/images/manifest_model.go index 24450f2ad..16a913ce4 100644 --- a/lib/images/manifest_model.go +++ b/lib/images/manifest_model.go @@ -79,13 +79,13 @@ func validateManifestModel(digestHex string, model *imageManifestModel) error { if model.RootFSType != "layers" { return fmt.Errorf("unsupported manifest rootfs type: %q", model.RootFSType) } - if err := validateManifestConfig(model); err != nil { + if err := validateManifestConfig(digestHex, model); err != nil { return err } return validateManifestLayers(model) } -func validateManifestConfig(model *imageManifestModel) error { +func validateManifestConfig(digestHex string, model *imageManifestModel) error { if model.Config.Digest == "" { return fmt.Errorf("manifest model config digest is empty") } @@ -100,7 +100,7 @@ func validateManifestConfig(model *imageManifestModel) error { "config rootfs.diff_ids has %d entries but manifest has %d layers for %s", len(model.Config.DiffIDs), len(model.Layers), - model.Digest, + digestHex, ) } return nil diff --git a/lib/paths/paths.go b/lib/paths/paths.go index b294f2417..9086242e6 100644 --- a/lib/paths/paths.go +++ b/lib/paths/paths.go @@ -100,9 +100,10 @@ func (p *Paths) OCICacheLayout() string { return filepath.Join(p.SystemOCICache(), "oci-layout") } -// SystemBuild returns the path to a system build directory. -func (p *Paths) SystemBuild(ref string) string { - return filepath.Join(p.dataDir, "system", "builds", ref) +// SystemBuild returns the path to the system build directory for one +// manifest digest hex. +func (p *Paths) SystemBuild(digestHex string) string { + return filepath.Join(p.dataDir, "system", "builds", digestHex) } // SystemBinary returns the path to a VMM binary. From dcff16f7464974d2465c020eeb7dbcccaeb74c27 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:36:33 +0000 Subject: [PATCH 12/13] Validate layer media types --- lib/images/manifest_model.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/images/manifest_model.go b/lib/images/manifest_model.go index 16a913ce4..63e45decf 100644 --- a/lib/images/manifest_model.go +++ b/lib/images/manifest_model.go @@ -111,6 +111,13 @@ func validateManifestLayers(model *imageManifestModel) error { if _, err := parseSHA256Digest(layer.Digest); err != nil { return fmt.Errorf("invalid manifest model layer %d digest: %q", i, layer.Digest) } + if layer.MediaType != "" { + switch convertToOCIMediaType(layer.MediaType) { + case v1.MediaTypeImageLayer, v1.MediaTypeImageLayerGzip, v1.MediaTypeImageLayerZstd: + default: + return fmt.Errorf("invalid manifest model layer %d media type: %q", i, layer.MediaType) + } + } diffID, err := parseSHA256Digest(model.Config.DiffIDs[i]) if err != nil || layer.DiffID != diffID.String() { return fmt.Errorf("invalid manifest model layer %d diff id", i) From 1e5ceb8322d33fae7216bbaa7fcf3a7cdde44406 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:39:21 +0000 Subject: [PATCH 13/13] Keep cache blob lookup reusable --- lib/images/layer_artifact.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/images/layer_artifact.go b/lib/images/layer_artifact.go index a7b511775..da1413ce4 100644 --- a/lib/images/layer_artifact.go +++ b/lib/images/layer_artifact.go @@ -339,7 +339,7 @@ func (s *layerStore) materializeLayerArtifactOnce(ctx context.Context, desc laye s.endLayerBuild() }() - stats, err := unpackCachedLayer(ctx, s.paths, desc, unpackDir, layerArtifactOnDiskFormat()) + stats, err := unpackCachedLayer(ctx, s.paths.OCICacheBlobDir(), desc, unpackDir, layerArtifactOnDiskFormat()) if err != nil { return nil, err } @@ -424,8 +424,8 @@ func (r contextReader) Read(p []byte) (int, error) { // blobs/sha256 directory), unpacks it into dest, and verifies both the blob // digest and the diff ID when the descriptor carries one. The caller must // have validated desc.Digest. -func unpackCachedLayer(ctx context.Context, p *paths.Paths, desc layerDescriptor, dest string, onDisk layer.OnDiskFormat) (*unpackStats, error) { - blobPath := p.OCICacheBlob(strings.TrimPrefix(desc.Digest, "sha256:")) +func unpackCachedLayer(ctx context.Context, blobDir string, desc layerDescriptor, dest string, onDisk layer.OnDiskFormat) (*unpackStats, error) { + blobPath := filepath.Join(blobDir, strings.TrimPrefix(desc.Digest, "sha256:")) if _, err := os.Stat(blobPath); err != nil { if os.IsNotExist(err) { return nil, fmt.Errorf("layer blob missing from oci cache: %s", desc.Digest)