diff --git a/docs/macos-experimental.md b/docs/macos-experimental.md index 57e9f396d..187542f00 100644 --- a/docs/macos-experimental.md +++ b/docs/macos-experimental.md @@ -20,9 +20,37 @@ go run -tags containers_image_openpgp ./cmd/import-macos \ --name localhost/macos:spike ``` -The data directory needs APFS clonefile support. Images are `darwin/arm64`, not -OCI macOS containers. Registry pulls, manifest resolution and tagging/promotion -of macOS images are unsupported. Do not mutate the imported image files. +The local importer needs APFS clonefile support. Images are `darwin/arm64` +machine images, not OCI macOS containers. Registry pulls also support experimental +complete-machine bundles described below. Additional tags within the same +repository are supported; cross-repository promotion remains unsupported. +Do not mutate the imported image files. + +## Experimental OCI bundles + +The normal image API accepts a `darwin/arm64` OCI manifest whose configuration +labels describe a complete installed machine: + +| Label | Value | +|---|---| +| `io.hypeman.machine-image.version` | `1` | +| `io.hypeman.machine-image.kind` | `macos-image` | +| `io.hypeman.machine-image.disk-format` | `raw` | +| `io.hypeman.machine-image.disk-path` | Relative installed boot disk path | +| `io.hypeman.machine-image.aux-path` | Relative auxiliary storage path | +| `io.hypeman.machine-image.platform-path` | Relative macOS platform configuration JSON path | + +All three files must be distinct, nonempty regular files inside the unpacked +image. Platform configuration uses the local bundle's `MacOSImage` schema and +is limited to 64 KiB. Absolute paths, traversal and symlink escapes are rejected. +The installed disk is materialized directly rather than converted to a Linux +root filesystem. The matching auxiliary storage and platform identity are +retained. Do not package live mutable guest storage. + +This schema is experimental pending reconciliation with other machine-image +platforms. Complete bundles only: no base/delta chain, identity rebinding, or +concurrent-template guarantee. Ordinary OCI transport does not imply sparse +distribution efficiency, boot portability, or container execution semantics. ## API diff --git a/lib/images/finalization_rollback_test.go b/lib/images/finalization_rollback_test.go new file mode 100644 index 000000000..65d936b22 --- /dev/null +++ b/lib/images/finalization_rollback_test.go @@ -0,0 +1,31 @@ +package images + +import ( + "os" + "path/filepath" + "testing" + + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func TestFinalizationRollbackPreservesUninstalledManifest(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + ref, err := ParseNormalizedRef("localhost/qa@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + require.NoError(t, err) + resolved := NewResolvedRef(ref, ref.Digest()) + require.NoError(t, writeMetadata(p, ref.Repository(), ref.DigestHex(), &imageMetadata{BuildID: "qa-build", Status: StatusPending})) + layout := resolveImageLayout(p, ref.Repository(), ref.DigestHex()) + modelPath := manifestModelPath(p, layout, ref.DigestHex()) + require.NoError(t, os.MkdirAll(filepath.Dir(modelPath), 0700)) + require.NoError(t, os.WriteFile(modelPath, []byte("preexisting-manifest"), 0600)) + stagedDisk := filepath.Join(layout.dir, "qa-staged-disk") + require.NoError(t, os.WriteFile(stagedDisk, []byte("synthetic-disk"), 0600)) + // Manifest validation fails before any model file is installed. + err = m.finalizeImage(resolved, &pullResult{Metadata: &containerMetadata{OS: "linux", Architecture: "arm64"}, Manifest: &imageManifestModel{}}, "qa-build", stagedImageFiles{disk: stagedDisk, sizeBytes: 14}) + require.ErrorContains(t, err, "write manifest model") + data, err := os.ReadFile(modelPath) + require.NoError(t, err, "rollback must not remove a manifest that this attempt did not install") + require.Equal(t, "preexisting-manifest", string(data)) +} diff --git a/lib/images/macos_import_darwin_test.go b/lib/images/macos_import_darwin_test.go index 8518fbab2..f8cb4b660 100644 --- a/lib/images/macos_import_darwin_test.go +++ b/lib/images/macos_import_darwin_test.go @@ -40,12 +40,13 @@ func TestMacOSOfflineImport(t *testing.T) { _, err = ImportMacOSImage(ctx, p, "localhost/macos:cancelled", source) require.Error(t, err) } -func TestMacOSPlatformLocalOnly(t *testing.T) { +func TestMacOSPlatform(t *testing.T) { p, err := ParsePlatform("darwin/arm64") require.NoError(t, err) require.Equal(t, "darwin", p.OS) _, err = ParsePlatform("darwin/amd64") require.Error(t, err) - _, err = resolveManifestPlatform(&containerMetadata{OS: "darwin", Architecture: "arm64"}, "") - require.ErrorIs(t, err, ErrInvalidPlatform) + manifest, err := resolveManifestPlatform(&containerMetadata{OS: "darwin", Architecture: "arm64"}, "") + require.NoError(t, err) + require.Equal(t, "darwin/arm64", manifest.String()) } diff --git a/lib/images/macos_machine.go b/lib/images/macos_machine.go new file mode 100644 index 000000000..ed95911dc --- /dev/null +++ b/lib/images/macos_machine.go @@ -0,0 +1,64 @@ +package images + +import ( + "fmt" + "os" + + "github.com/kernel/hypeman/lib/forkvm" +) + +const ( + MacOSMachineVersionLabel = "io.hypeman.machine-image.version" + MacOSMachineKindLabel = "io.hypeman.machine-image.kind" + MacOSMachineDiskLabel = "io.hypeman.machine-image.disk-path" + MacOSMachineFormatLabel = "io.hypeman.machine-image.disk-format" + MacOSMachineAuxLabel = "io.hypeman.machine-image.aux-path" + MacOSMachinePlatformLabel = "io.hypeman.machine-image.platform-path" +) + +// stageMacOSMachine copies a validated bundle's boot disk and auxiliary storage to +// build-private paths, outside the manager lock. It returns the bytes both files +// occupy, which is the size recorded for accounting. +func stageMacOSMachine(payload *macOSMachinePayload, diskTemp, auxTemp string) (stagedImageFiles, error) { + diskSize, err := stageMachineFile(payload.Disk, diskTemp) + if err != nil { + return stagedImageFiles{}, fmt.Errorf("stage boot disk: %w", err) + } + auxSize, err := stageMachineFile(payload.Aux, auxTemp) + if err != nil { + return stagedImageFiles{}, fmt.Errorf("stage auxiliary storage: %w", err) + } + return stagedImageFiles{disk: diskTemp, aux: auxTemp, macos: payload.Platform, sizeBytes: diskSize + auxSize}, nil +} + +func stageMachineFile(src, dst string) (int64, error) { + if err := forkvm.CopyRegularFile(src, dst); err != nil { + return 0, err + } + // Registry-supplied modes must not expose the canonical files to other local users. + if err := os.Chmod(dst, 0600); err != nil { + return 0, err + } + info, err := os.Stat(dst) + if err != nil { + return 0, err + } + return info.Size(), nil +} + +func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePayload, error) { + // Normalize as resolveManifestPlatform does, so aliases such as aarch64 and + // case variants such as Darwin select the machine path rather than rootfs. + normalized := Platform{OS: meta.OS, Architecture: meta.Architecture, Variant: meta.Variant}.Normalize() + if normalized.OS != "darwin" { + return nil, nil + } + if normalized.Architecture != "arm64" || normalized.Variant != "" { + return nil, fmt.Errorf("macOS machine requires darwin/arm64") + } + labels := meta.Labels + if labels[MacOSMachineVersionLabel] != "1" || labels[MacOSMachineKindLabel] != "macos-image" || labels[MacOSMachineFormatLabel] != "raw" { + return nil, fmt.Errorf("darwin artifact requires version 1 macos-image with raw disk, not an ordinary container image") + } + return readMacOSMachineBundle(root, labels[MacOSMachineDiskLabel], labels[MacOSMachineAuxLabel], labels[MacOSMachinePlatformLabel], false) +} diff --git a/lib/images/macos_machine_test.go b/lib/images/macos_machine_test.go new file mode 100644 index 000000000..7261ee0ca --- /dev/null +++ b/lib/images/macos_machine_test.go @@ -0,0 +1,230 @@ +package images + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/json" + "fmt" + "io" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/tarball" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func macOSFixture(t *testing.T) string { + t.Helper() + root := t.TempDir() + c := MacOSImage{HardwareModel: []byte{1}, MachineIdentifier: []byte{2}, MAC: "02:00:00:00:00:01", CPUs: 4, Memory: 8 << 30} + b, err := json.Marshal(c) + require.NoError(t, err) + for f, data := range map[string][]byte{"config.json": b, "disk.img": []byte("not a real boot disk: synthetic OCI transport fixture"), "aux.img": []byte("synthetic aux")} { + require.NoError(t, os.WriteFile(filepath.Join(root, f), data, 0600)) + } + return root +} + +func macOSFixtureMetadata() *containerMetadata { + return &containerMetadata{OS: "darwin", Architecture: "arm64", Labels: map[string]string{ + MacOSMachineVersionLabel: "1", MacOSMachineKindLabel: "macos-image", MacOSMachineFormatLabel: "raw", + MacOSMachineDiskLabel: "disk.img", MacOSMachineAuxLabel: "aux.img", MacOSMachinePlatformLabel: "config.json", + }} +} + +func TestMacOSMachineValidation(t *testing.T) { + root := macOSFixture(t) + payload, err := parseMacOSMachine(root, macOSFixtureMetadata()) + require.NoError(t, err) + require.NotNil(t, payload) + for _, tc := range []struct{ name, key, value string }{ + {"unsupported version", MacOSMachineVersionLabel, "2"}, {"container not machine", MacOSMachineKindLabel, ""}, + {"wrong format", MacOSMachineFormatLabel, "qcow2"}, {"absolute path", MacOSMachineDiskLabel, "/etc/passwd"}, + {"traversal", MacOSMachineDiskLabel, "../disk.img"}, {"missing aux", MacOSMachineAuxLabel, "missing.img"}, + {"duplicate file", MacOSMachineAuxLabel, "disk.img"}, + } { + t.Run(tc.name, func(t *testing.T) { + meta := macOSFixtureMetadata() + meta.Labels[tc.key] = tc.value + _, err := parseMacOSMachine(root, meta) + require.Error(t, err) + }) + } + outside := filepath.Join(t.TempDir(), "outside") + require.NoError(t, os.WriteFile(outside, []byte("outside"), 0600)) + require.NoError(t, os.Symlink(outside, filepath.Join(root, "escape"))) + meta := macOSFixtureMetadata() + meta.Labels[MacOSMachineDiskLabel] = "escape" + _, err = parseMacOSMachine(root, meta) + require.Error(t, err) + meta = macOSFixtureMetadata() + meta.Architecture = "amd64" + _, err = parseMacOSMachine(root, meta) + require.Error(t, err) + + // Platform aliases normalize before the machine check, as they do for manifest matching. + for _, alias := range []struct{ os, arch string }{{"darwin", "aarch64"}, {"Darwin", "arm64"}} { + meta = macOSFixtureMetadata() + meta.OS, meta.Architecture = alias.os, alias.arch + payload, err := parseMacOSMachine(root, meta) + require.NoError(t, err, alias) + require.NotNil(t, payload, alias) + } + require.NoError(t, os.WriteFile(filepath.Join(root, "config.json"), []byte(`{}`), 0600)) + _, err = parseMacOSMachine(root, macOSFixtureMetadata()) + require.Error(t, err) + + // EUI-64 and 20-octet InfiniBand addresses parse with net.ParseMAC but are not Ethernet. + for _, mac := range []string{"02:00:00:00:00:00:00:01", strings.Repeat("00:", 19) + "01"} { + b, err := json.Marshal(MacOSImage{HardwareModel: []byte{1}, MachineIdentifier: []byte{2}, MAC: mac, CPUs: 4, Memory: 8 << 30}) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(root, "config.json"), b, 0600)) + _, err = parseMacOSMachine(root, macOSFixtureMetadata()) + require.Error(t, err, mac) + } + + // A hardlink with a different name is the same file and must not satisfy distinctness. + linked := macOSFixture(t) + require.NoError(t, os.Link(filepath.Join(linked, "disk.img"), filepath.Join(linked, "alias.img"))) + meta = macOSFixtureMetadata() + meta.Labels[MacOSMachineAuxLabel] = "alias.img" + _, err = parseMacOSMachine(linked, meta) + require.ErrorContains(t, err, "distinct") +} + +// Optional real-bundle run only reads a stopped source, never the live benchmark disk. +// It proves normal manager pull/materialization, not VZ boot or API server deployment. +func TestMacOSMachineOCIRoundTrip(t *testing.T) { + source := macOSFixture(t) + work := t.TempDir() + _, err := parseMacOSMachine(source, macOSFixtureMetadata()) + require.NoError(t, err) + layerPath := filepath.Join(work, "bundle.tar.gz") + file, err := os.OpenFile(layerPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) + require.NoError(t, err) + gz, err := gzip.NewWriterLevel(file, gzip.BestSpeed) + require.NoError(t, err) + tw := tar.NewWriter(gz) + hashes := map[string]string{} + t.Log("packing stopped bundle", source) + for _, f := range []string{"disk.img", "aux.img", "config.json"} { + input, err := os.Open(filepath.Join(source, f)) + require.NoError(t, err) + info, err := input.Stat() + require.NoError(t, err) + // The registry controls modes; a permissive disk must not become the canonical image mode. + mode := int64(0600) + if f == "disk.img" { + mode = 0666 + } + require.NoError(t, tw.WriteHeader(&tar.Header{Name: f, Mode: mode, Size: info.Size(), Typeflag: tar.TypeReg})) + h := sha256.New() + _, err = io.Copy(io.MultiWriter(tw, h), input) + require.NoError(t, err) + require.NoError(t, input.Close()) + hashes[f] = fmt.Sprintf("%x", h.Sum(nil)) + } + require.NoError(t, tw.Close()) + require.NoError(t, gz.Close()) + require.NoError(t, file.Close()) + t.Log("packed; streaming blobs into loopback registry storage") + layer, err := tarball.LayerFromFile(layerPath) + require.NoError(t, err) + image, err := mutate.AppendLayers(empty.Image, layer) + require.NoError(t, err) + cfg, err := image.ConfigFile() + require.NoError(t, err) + cfg.OS = "darwin" + cfg.Architecture = "arm64" + cfg.Config.Labels = macOSFixtureMetadata().Labels + image, err = mutate.ConfigFile(image, cfg) + require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(work, "registry-blobs"), 0700)) + // The test registry buffers PATCH uploads in memory even with a disk blob + // handler. Seed blobs through the streaming handler so real VM layers do + // not consume tens of GiB of RAM; remote.Write still publishes the manifest. + blobs := registry.NewDiskBlobHandler(filepath.Join(work, "registry-blobs")) + writer := blobs.(registry.BlobPutHandler) + layerDigest, err := layer.Digest() + require.NoError(t, err) + compressed, err := layer.Compressed() + require.NoError(t, err) + err = writer.Put(context.Background(), "macos", layerDigest, compressed) + closeErr := compressed.Close() + require.NoError(t, err) + require.NoError(t, closeErr) + configDigest, err := image.ConfigName() + require.NoError(t, err) + rawConfig, err := image.RawConfigFile() + require.NoError(t, err) + require.NoError(t, writer.Put(context.Background(), "macos", configDigest, io.NopCloser(bytes.NewReader(rawConfig)))) + server := httptest.NewServer(registry.New(registry.WithBlobHandler(blobs))) + defer server.Close() + ref, err := name.ParseReference(strings.TrimPrefix(server.URL, "http://") + "/macos:spike") + require.NoError(t, err) + require.NoError(t, remote.Write(ref, image)) + t.Log("published; pulling through normal image manager") + p := paths.New(filepath.Join(work, "hypeman-data")) + manager, err := NewManager(p, 1, nil) + require.NoError(t, err) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) + defer cancel() + pulled, err := manager.CreateImage(ctx, CreateImageRequest{Name: ref.Name(), Platform: "darwin/arm64"}) + require.NoError(t, err) + require.NoError(t, manager.WaitForReady(ctx, pulled.Name)) + pulled, err = manager.GetImage(ctx, pulled.Name) + require.NoError(t, err) + require.Equal(t, StatusReady, pulled.Status) + require.Equal(t, "darwin/arm64", pulled.Platform) + require.NotNil(t, pulled.MacOS) + disk, err := GetDiskPath(p, pulled.Name, pulled.Digest) + require.NoError(t, err) + for f, path := range map[string]string{"disk.img": disk, "aux.img": filepath.Join(filepath.Dir(disk), "aux.img")} { + input, err := os.Open(path) + require.NoError(t, err) + h := sha256.New() + _, err = io.Copy(h, input) + require.NoError(t, err) + require.NoError(t, input.Close()) + require.Equal(t, hashes[f], fmt.Sprintf("%x", h.Sum(nil)), f) + info, err := os.Stat(path) + require.NoError(t, err) + require.Equal(t, os.FileMode(0600), info.Mode().Perm(), f) + } + expected, err := os.ReadFile(filepath.Join(source, "config.json")) + require.NoError(t, err) + var config MacOSImage + require.NoError(t, json.Unmarshal(expected, &config)) + require.Equal(t, config, *pulled.MacOS) + digest, err := image.Digest() + require.NoError(t, err) + require.Equal(t, digest.String(), pulled.Digest) + reused, err := manager.CreateImage(ctx, CreateImageRequest{Name: ref.Name(), Platform: "darwin/arm64"}) + require.NoError(t, err) + require.Equal(t, StatusReady, reused.Status) + tagged, err := manager.TagImage(ctx, ref.Name(), ref.Context().Name()+":stable") + require.NoError(t, err) + require.Equal(t, pulled.Digest, tagged.Digest) + require.Equal(t, pulled.MacOS, tagged.MacOS) + tagDisk, err := GetDiskPath(p, tagged.Name, tagged.Digest) + require.NoError(t, err) + require.Equal(t, disk, tagDisk) + _, err = manager.TagImage(ctx, ref.Name(), strings.TrimPrefix(server.URL, "http://")+"/other:stable") + require.ErrorIs(t, err, ErrInvalidPlatform) + _, err = manager.CreateImage(ctx, CreateImageRequest{Name: ref.Name(), Platform: "linux/arm64"}) + require.Error(t, err) + t.Log("round-trip verified", pulled.Digest) +} diff --git a/lib/images/manager.go b/lib/images/manager.go index 5ccd7b7e7..58922fb0d 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -174,10 +174,6 @@ func (m *manager) CreateImage(ctx context.Context, req CreateImageRequest) (*Ima return nil, err } - if platform.OS == "darwin" { - return nil, fmt.Errorf("%w: macOS images must be imported locally with import-macos, not pulled as Linux containers", ErrInvalidPlatform) - } - // Parse and normalize normalized, err := ParseNormalizedRef(req.Name) if err != nil { @@ -223,7 +219,11 @@ func (m *manager) CreateImage(ctx context.Context, req CreateImageRequest) (*Ima m.createMu.Lock() defer m.createMu.Unlock() - if img, found, err := m.reuseExistingImage(ref, req.Credentials, req.Tags); found || err != nil { + var requested *Platform + if req.Platform != "" { + requested = &platform + } + if img, found, err := m.reuseExistingImage(ref, req.Credentials, req.Tags, requested); found || err != nil { return img, err } return m.createAndQueueImage(ref, req, platform) @@ -255,13 +255,15 @@ func (m *manager) ImportLocalImage(ctx context.Context, repo, reference, digest m.createMu.Lock() defer m.createMu.Unlock() - if img, found, err := m.reuseExistingImage(ref, nil, nil); found || err != nil { + if img, found, err := m.reuseExistingImage(ref, nil, nil, nil); found || err != nil { return img, err } return m.createAndQueueImage(ref, CreateImageRequest{Name: imageRef}, hostPlatform()) } -func (m *manager) reuseExistingImage(ref *ResolvedRef, credentials *authn.AuthConfig, resourceTags tags.Tags) (*Image, bool, error) { +// reuseExistingImage returns a record already on disk for ref. requested is the +// caller's explicit platform, or nil when the request named none. +func (m *manager) reuseExistingImage(ref *ResolvedRef, credentials *authn.AuthConfig, resourceTags tags.Tags, requested *Platform) (*Image, bool, error) { meta, err := readMetadata(m.paths, ref.Repository(), ref.DigestHex()) if err != nil { return nil, false, nil @@ -279,6 +281,13 @@ func (m *manager) reuseExistingImage(ref *ResolvedRef, credentials *authn.AuthCo return nil, true, fmt.Errorf("%w: retry after the current pull completes", ErrCredentialConflict) } } + // Only ready macOS records carry a platform the request can contradict. + if requested != nil && meta.Status == StatusReady && meta.MacOS != nil { + actual, err := ParsePlatform(meta.Platform) + if err != nil || !requested.Matches(actual) { + return nil, true, fmt.Errorf("%w: requested %s but cached manifest is %s", ErrInvalidPlatform, *requested, meta.Platform) + } + } if resourceTags != nil { meta.Tags = tags.Clone(resourceTags) if err := writeMetadata(m.paths, ref.Repository(), ref.DigestHex(), meta); err != nil { @@ -509,14 +518,27 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials m.updateStatusByDigest(ref, StatusConverting, nil, buildID) - diskPath := resolveImageLayout(m.paths, ref.Repository(), ref.DigestHex()).disk + layout := resolveImageLayout(m.paths, ref.Repository(), ref.DigestHex()) + diskPath := layout.disk // Keep the temporary filesystem beside its final path so finalization stays // atomic even when system/builds and images are on different filesystems. diskTempPath := diskPath + ".tmp-" + buildID defer os.Remove(diskTempPath) - // Use default image format (erofs on Linux, ext4 on Darwin) + payload, err := parseMacOSMachine(tempDir, result.Metadata) + if err != nil { + m.updateStatusByDigest(ref, StatusFailed, err, buildID) + return + } convertStart := time.Now() - diskSize, err := ExportRootfs(tempDir, diskTempPath, DefaultImageFormat) + var staged stagedImageFiles + if payload != nil { + auxTemp := filepath.Join(layout.dir, "aux.img.tmp-"+buildID) + defer os.Remove(auxTemp) + staged, err = stageMacOSMachine(payload, diskTempPath, auxTemp) + } else { + staged = stagedImageFiles{disk: diskTempPath} + staged.sizeBytes, err = ExportRootfs(tempDir, diskTempPath, DefaultImageFormat) + } m.recordImageBuildPhase(ctx, ref.Digest(), "filesystem_export", time.Since(convertStart), phaseStatus(err), "not_applicable") if err != nil { m.updateStatusByDigest(ref, StatusFailed, fmt.Errorf("convert to %s: %w", DefaultImageFormat, err), buildID) @@ -524,7 +546,7 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials } finalizeStart := time.Now() - err = m.finalizeImage(ref, result, diskSize, buildID, diskTempPath) + err = m.finalizeImage(ref, result, buildID, staged) m.recordImageBuildPhase(ctx, ref.Digest(), "finalize", time.Since(finalizeStart), phaseStatus(err), "not_applicable") if err != nil { if errors.Is(err, errStaleBuild) { @@ -537,7 +559,16 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials buildStatus = "success" } -func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize int64, buildID, diskTempPath string) error { +// stagedImageFiles are the build outputs written before finalization takes createMu. +// Finalization only renames them into place and commits metadata. +type stagedImageFiles struct { + disk string // staged disk, beside its final path + aux string // staged auxiliary storage; macOS machine images only + macos *MacOSImage // platform of a macOS machine image; nil for rootfs images + sizeBytes int64 // bytes the staged files occupy, recorded for accounting +} + +func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID string, staged stagedImageFiles) error { m.createMu.Lock() defer m.createMu.Unlock() @@ -559,16 +590,25 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i return err } - modelPath := manifestModelPath(m.paths, layout, ref.DigestHex()) - diskInstalled := false - modelWritten := false - + if actualPlatform.OS == "darwin" && staged.macos == nil { + return fmt.Errorf("macOS image requires a validated machine bundle") + } + // Files installed before the metadata commit. Finalization failure removes them. + var installed []string + if staged.macos != nil { + auxPath := filepath.Join(layout.dir, "aux.img") + if err := os.Rename(staged.aux, auxPath); err != nil { + return rollbackFinalization(installed, err) + } + installed = append(installed, auxPath) + meta.MacOS = staged.macos + } if err := installAtomically(layout.disk, func(path string) error { - return os.Rename(diskTempPath, path) + return os.Rename(staged.disk, path) }); err != nil { - return fmt.Errorf("install image disk: %w", err) + return rollbackFinalization(installed, fmt.Errorf("install image disk: %w", err)) } - diskInstalled = true + installed = append(installed, layout.disk) // Persist the manifest content model beside the shared content so later // stages can recompose the image from per-layer artifacts and GC can tell @@ -576,16 +616,17 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i if result.Manifest != nil { model := *result.Manifest model.Platform = actualPlatform.String() + modelPath := manifestModelPath(m.paths, layout, ref.DigestHex()) if err := writeManifestModelAt(modelPath, ref.DigestHex(), &model); err != nil { - return rollbackFinalization(layout, modelPath, diskInstalled, modelWritten, fmt.Errorf("write manifest model: %w", err)) + return rollbackFinalization(installed, fmt.Errorf("write manifest model: %w", err)) } - modelWritten = true + installed = append(installed, modelPath) } meta.Status = StatusReady meta.Error = nil meta.Platform = actualPlatform.String() - meta.SizeBytes = diskSize + meta.SizeBytes = staged.sizeBytes meta.Entrypoint = result.Metadata.Entrypoint meta.Cmd = result.Metadata.Cmd meta.Env = result.Metadata.Env @@ -593,7 +634,7 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i meta.WorkingDir = result.Metadata.WorkingDir if err := writeMetadataFile(layout.metadata, meta); err != nil { - return rollbackFinalization(layout, modelPath, diskInstalled, modelWritten, fmt.Errorf("write final metadata: %w", err)) + return rollbackFinalization(installed, fmt.Errorf("write final metadata: %w", err)) } m.notifyReady(ref.DigestHex(), StatusReady, nil) @@ -611,13 +652,11 @@ func manifestModelPath(p *paths.Paths, layout imageLayout, digestHex string) str return filepath.Join(layout.dir, "manifest.json") } -func rollbackFinalization(layout imageLayout, modelPath string, diskInstalled, modelWritten bool, cause error) error { +// rollbackFinalization removes the files finalization installed before it failed. +func rollbackFinalization(installed []string, cause error) error { var rollbackErr error - if modelWritten { - rollbackErr = errors.Join(rollbackErr, os.Remove(modelPath)) - } - if diskInstalled { - rollbackErr = errors.Join(rollbackErr, os.Remove(layout.disk)) + for _, path := range installed { + rollbackErr = errors.Join(rollbackErr, os.Remove(path)) } if rollbackErr != nil { return errors.Join(cause, fmt.Errorf("rollback finalization: %w", rollbackErr)) diff --git a/lib/images/manager_test.go b/lib/images/manager_test.go index 13a6835e2..23404d5a8 100644 --- a/lib/images/manager_test.go +++ b/lib/images/manager_test.go @@ -736,7 +736,7 @@ func TestDeleteAndRecreateDuringBuildTail(t *testing.T) { m.updateStatusByDigest(staleRef, StatusFailed, errors.New("stale build"), firstMeta.BuildID) staleBundle, err := m.ociClient.extractOCIImageBundle(digestHex) require.NoError(t, err) - require.ErrorIs(t, m.finalizeImage(staleRef, &pullResult{Metadata: staleBundle.Meta}, 1, firstMeta.BuildID, ""), errStaleBuild) + require.ErrorIs(t, m.finalizeImage(staleRef, &pullResult{Metadata: staleBundle.Meta}, firstMeta.BuildID, stagedImageFiles{sizeBytes: 1}), errStaleBuild) currentMeta, err = readMetadata(p, repo, digestHex) require.NoError(t, err) require.Equal(t, StatusPending, currentMeta.Status) diff --git a/lib/images/platform.go b/lib/images/platform.go index 28e788b26..3ff14acf6 100644 --- a/lib/images/platform.go +++ b/lib/images/platform.go @@ -167,10 +167,6 @@ func resolveManifestPlatform(meta *containerMetadata, requested string) (Platfor Variant: meta.Variant, }.Normalize() - if actual.OS == "darwin" { - return Platform{}, fmt.Errorf("%w: macOS must be imported as a local disk bundle", ErrInvalidPlatform) - } - // An explicit request is authoritative for the match check and, when the // manifest omits its own platform, for the recorded value too. if strings.TrimSpace(requested) != "" { @@ -184,8 +180,8 @@ func resolveManifestPlatform(meta *containerMetadata, requested string) (Platfor if err := actual.validate(); err != nil { return Platform{}, fmt.Errorf("image platform: %w", err) } - if !want.Matches(actual) { - return Platform{}, fmt.Errorf("%w: requested %s but manifest is %s", ErrInvalidPlatform, want, actual) + if err := validateDigestPlatform(requested, want, actual); err != nil { + return Platform{}, err } return actual, nil } diff --git a/lib/images/tag.go b/lib/images/tag.go index 3a497a6a9..614ac4f14 100644 --- a/lib/images/tag.go +++ b/lib/images/tag.go @@ -28,8 +28,8 @@ func (m *manager) TagImage(ctx context.Context, source, target string) (*Image, if err != nil { return nil, err } - if meta.MacOS != nil { - return nil, fmt.Errorf("%w: tagging macOS bundles is not implemented", ErrInvalidPlatform) + if meta.MacOS != nil && sourceRef.Repository() != targetRef.Repository() { + return nil, fmt.Errorf("%w: cross-repository promotion of macOS bundles is not implemented", ErrInvalidPlatform) } if err := m.cancelPendingTag(targetRef.Repository(), targetRef.Tag()); err != nil { return nil, fmt.Errorf("cancel pending image tag: %w", err) diff --git a/lib/images/tag_test.go b/lib/images/tag_test.go index 9a6d6a605..5475c33d7 100644 --- a/lib/images/tag_test.go +++ b/lib/images/tag_test.go @@ -297,7 +297,7 @@ func TestReuseExistingImageUpdatesResourceTags(t *testing.T) { ref, err := ParseNormalizedRef(repository + "@sha256:" + digest) require.NoError(t, err) resolved := NewResolvedRef(ref, "sha256:"+digest) - img, found, err := m.reuseExistingImage(resolved, nil, tags.Tags{"team": "payments"}) + img, found, err := m.reuseExistingImage(resolved, nil, tags.Tags{"team": "payments"}, nil) require.NoError(t, err) require.True(t, found) require.Equal(t, "payments", img.Tags["team"])