From 50d26d2fd8deb86675b9a3e8a519cfd5a1e28190 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Wed, 8 Jul 2026 21:23:32 +0900 Subject: [PATCH] exporter/containerimage: support unpack for rewrite-timestamp mode ImageWriter.Commit now returns the committed remotes (the rewritten layers when rewrite-timestamp is set) as Result[Remote], and unpackImage consumes them instead of re-fetching the original (non-rewritten) layers from the source ref. The provider of a rewritten remote preserves the Unlazier of the original provider, so that non-rewritten lazy layers can still be fetched before unpacking. Fixes issue 4230 Co-Authored-By: Claude Fable 5 Signed-off-by: Akihiro Suda --- client/client_export_image_test.go | 93 ++++++++++++++++++++++++++++ client/client_test.go | 1 + exporter/containerimage/export.go | 27 +++----- exporter/containerimage/writer.go | 98 +++++++++++++++++++----------- exporter/oci/export.go | 2 +- 5 files changed, 166 insertions(+), 55 deletions(-) diff --git a/client/client_export_image_test.go b/client/client_export_image_test.go index 902e1dfaecc6..e9ba8183278d 100644 --- a/client/client_export_image_test.go +++ b/client/client_export_image_test.go @@ -35,6 +35,7 @@ import ( "github.com/moby/buildkit/util/testutil/integration" "github.com/moby/buildkit/util/testutil/workers" digest "github.com/opencontainers/go-digest" + imagespecidentity "github.com/opencontainers/image-spec/identity" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/stretchr/testify/require" @@ -175,6 +176,98 @@ func testBuildExportScratch(t *testing.T, sb integration.Sandbox) { require.Equal(t, "i am platform linux/arm64", img.Img.Config.Labels["foo"]) } +// testBuildExportUnpackWithRewriteTimestamp verifies that the "unpack" exporter option +// can be used together with "rewrite-timestamp", and that the image is unpacked to the +// snapshot that matches the rewritten layers. +// https://github.com/moby/buildkit/issues/4230 +func testBuildExportUnpackWithRewriteTimestamp(t *testing.T, sb integration.Sandbox) { + workers.CheckFeatureCompat(t, sb, workers.FeatureImageExporter, workers.FeatureSourceDateEpoch) + requiresLinux(t) + + cdAddress := sb.ContainerdAddress() + if cdAddress == "" { + t.Skip("test requires containerd worker") + } + + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + busybox := llb.Image("busybox:latest") + cmd := `sh -e -c "echo -n foo > data"` + + st := llb.Scratch() + st = busybox.Run(llb.Shlex(cmd), llb.Dir("/wd")).AddMount("/wd", st) + + def, err := st.Marshal(sb.Context()) + require.NoError(t, err) + + tm := time.Unix(1700000000, 0).UTC() + target := "docker.io/buildkit/build/exporter:unpack-rewrite-timestamp" + + _, err = c.Solve(sb.Context(), def, SolveOpt{ + Exports: []ExportEntry{ + { + Type: ExporterImage, + Attrs: map[string]string{ + "name": target, + "unpack": "true", + "rewrite-timestamp": "true", + "source-date-epoch": fmt.Sprintf("%d", tm.Unix()), + }, + }, + }, + }, nil) + require.NoError(t, err) + + client, err := newContainerd(cdAddress) + require.NoError(t, err) + defer client.Close() + + ctx := namespaces.WithNamespace(sb.Context(), "buildkit") + + img, err := client.ImageService().Get(ctx, target) + require.NoError(t, err) + + contentStore := client.ContentStore() + + manifest, err := images.Manifest(ctx, contentStore, img.Target, platforms.Default()) + require.NoError(t, err) + + // all the layers must be rewritten to match the epoch + require.NotEmpty(t, manifest.Layers) + for _, l := range manifest.Layers { + require.Equal(t, fmt.Sprintf("%d", tm.Unix()), l.Annotations["buildkit/rewritten-timestamp"]) + } + + // the timestamps in the layer blobs must be clamped to the epoch + dt, err := content.ReadBlob(ctx, contentStore, manifest.Layers[0]) + require.NoError(t, err) + m, err := testutil.ReadTarToMap(dt, true) + require.NoError(t, err) + require.Contains(t, m, "data") + for name, item := range m { + require.False(t, item.Header.ModTime.After(tm), "file %q has timestamp %v, expected not after %v", name, item.Header.ModTime, tm) + } + + // the image must be unpacked to the snapshot that matches the rewritten diffIDs + dt, err = content.ReadBlob(ctx, contentStore, manifest.Config) + require.NoError(t, err) + var ociimg ocispecs.Image + require.NoError(t, json.Unmarshal(dt, &ociimg)) + require.NotEmpty(t, ociimg.RootFS.DiffIDs) + + chainID := imagespecidentity.ChainID(ociimg.RootFS.DiffIDs) + snapshotService := client.SnapshotService(sb.Snapshotter()) + _, err = snapshotService.Stat(ctx, chainID.String()) + require.NoError(t, err) + + err = client.ImageService().Delete(ctx, target, images.SynchronousDelete()) + require.NoError(t, err) + + checkAllReleasable(t, c, sb, true) +} + // testBuildExportWithForeignLayer verifies foreign (non-distributable) layer handling during // image export. propagate=1 preserves foreign media type and URLs; propagate=0 converts to // regular layers. Skipped on Windows: the test image is Linux-only and BuildKit's Windows diff --git a/client/client_test.go b/client/client_test.go index 2f3fda52e4e8..1970c7045581 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -63,6 +63,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){ // client_export_image_test.go testBuildExportScratch, + testBuildExportUnpackWithRewriteTimestamp, testBuildExportWithForeignLayer, testBuildExportWithUncompressed, testBuildExportZstd, diff --git a/exporter/containerimage/export.go b/exporter/containerimage/export.go index 2db7c487c056..3303d5b62f25 100644 --- a/exporter/containerimage/export.go +++ b/exporter/containerimage/export.go @@ -27,7 +27,9 @@ import ( "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/session" "github.com/moby/buildkit/snapshot" + "github.com/moby/buildkit/solver" "github.com/moby/buildkit/solver/llbsolver/compat" + "github.com/moby/buildkit/solver/result" "github.com/moby/buildkit/util/compression" "github.com/moby/buildkit/util/contentutil" "github.com/moby/buildkit/util/errutil" @@ -252,7 +254,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source } }() - desc, err := e.opt.ImageWriter.Commit(ctx, src, buildInfo.SessionID, buildInfo.InlineCache, &opts, buildInfo.CompatibilityVersion, e.Type()) + desc, committedRemotes, err := e.opt.ImageWriter.Commit(ctx, src, buildInfo.SessionID, buildInfo.InlineCache, &opts, buildInfo.CompatibilityVersion, e.Type()) if err != nil { return nil, nil, nil, err } @@ -323,14 +325,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source tagDone(nil) if e.unpack { - if opts.RewriteTimestamp { - // e.unpackImage cannot be used because src ref does not point to the rewritten image - // / - // TODO: change e.unpackImage so that it takes Result[Remote] as parameter. - // https://github.com/moby/buildkit/pull/4057#discussion_r1324106088 - return nil, nil, nil, errors.New("exporter option \"rewrite-timestamp\" conflicts with \"unpack\"") - } - if err := e.unpackImage(ctx, img, src, session.NewGroup(buildInfo.SessionID)); err != nil { + if err := e.unpackImage(ctx, img, src, committedRemotes); err != nil { return nil, nil, nil, err } } @@ -441,7 +436,7 @@ func (e *imageExporterInstance) pushImage(ctx context.Context, src *exporter.Sou return push.Push(ctx, e.opt.SessionManager, sessionID, mprovider, e.opt.ImageWriter.ContentStore(), dgst, targetName, e.insecure, e.opt.RegistryHosts, e.pushByDigest, annotations) } -func (e *imageExporterInstance) unpackImage(ctx context.Context, img images.Image, src *exporter.Source, s session.Group) (err0 error) { +func (e *imageExporterInstance) unpackImage(ctx context.Context, img images.Image, src *exporter.Source, remotes *result.Result[*solver.Remote]) (err0 error) { matcher := platforms.Only(platforms.Normalize(platforms.DefaultSpec())) ps, err := exptypes.ParsePlatforms(src.Metadata) @@ -462,9 +457,9 @@ func (e *imageExporterInstance) unpackImage(ctx context.Context, img images.Imag return matcher.Less(matching[i].Platform, matching[j].Platform) }) - ref, _ := src.FindRef(matching[0].ID) - if ref == nil { - // ref has no layers, so nothing to unpack + remote, _ := remotes.FindRef(matching[0].ID) + if remote == nil || len(remote.Descriptors) == 0 { + // image has no layers, so nothing to unpack return nil } @@ -485,12 +480,6 @@ func (e *imageExporterInstance) unpackImage(ctx context.Context, img images.Imag return err } - remotes, err := ref.GetRemotes(ctx, true, e.opts.RefCfg, false, s) - if err != nil { - return err - } - remote := remotes[0] - // ensure the content for each layer exists locally in case any are lazy if unlazier, ok := remote.Provider.(cache.Unlazier); ok { if err := unlazier.Unlazy(ctx); err != nil { diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index 9214f8e0957d..c47a1aa991c5 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -65,23 +65,27 @@ type ImageWriter struct { opt WriterOpt } -func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, sessionID string, inlineCache exptypes.InlineCache, opts *ImageCommitOpts, compatibilityVersion int, exporterType string) (*ocispecs.Descriptor, error) { +// Commit commits the image to the content store and returns the descriptor of +// the manifest (or the manifest list) and the remotes that were committed for +// each platform. The returned remotes contain the rewritten layers when +// opts.RewriteTimestamp is set. +func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, sessionID string, inlineCache exptypes.InlineCache, opts *ImageCommitOpts, compatibilityVersion int, exporterType string) (*ocispecs.Descriptor, *result.Result[*solver.Remote], error) { if _, ok := inp.Metadata[exptypes.ExporterPlatformsKey]; len(inp.Refs) > 0 && !ok { - return nil, errors.Errorf("unable to export multiple refs, missing platforms mapping") + return nil, nil, errors.Errorf("unable to export multiple refs, missing platforms mapping") } if err := opts.Validate(); err != nil { - return nil, err + return nil, nil, err } if compatibilityVersion == compat.CompatibilityVersion013 && opts.RefCfg.Compression.Type == compression.Zstd { feature := fmt.Sprintf("%s exporter compression=%s", exporterType, opts.RefCfg.Compression.Type.String()) - return nil, solvererrdefs.NewUnsupportedCompatibilityFeatureError(compatibilityVersion, feature) + return nil, nil, solvererrdefs.NewUnsupportedCompatibilityFeatureError(compatibilityVersion, feature) } isMap := len(inp.Refs) > 0 ps, err := exptypes.ParsePlatforms(inp.Metadata) if err != nil { - return nil, err + return nil, nil, err } if !isMap { @@ -104,7 +108,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session } if opts.Epoch == nil { if tm, err := epoch.ParseSource(inp, nil); err != nil { - return nil, err + return nil, nil, err } else if tm != nil { opts.Epoch = &epoch.Epoch{Value: tm} } @@ -113,17 +117,19 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session for pk, a := range opts.Annotations { if pk != "" { if _, ok := inp.FindRef(pk); !ok { - return nil, errors.Errorf("invalid annotation: no platform %s found in source", pk) + return nil, nil, errors.Errorf("invalid annotation: no platform %s found in source", pk) } } if !opts.OCITypesEnabled() && len(a.Index)+len(a.IndexDescriptor)+len(a.ManifestDescriptor) > 0 { - return nil, errors.New("cannot export annotations with \"oci-mediatypes=false\"") + return nil, nil, errors.New("cannot export annotations with \"oci-mediatypes=false\"") } } + committedRemotes := &result.Result[*solver.Remote]{} + if !isMap { if len(ps.Platforms) > 1 { - return nil, errors.New("cannot export multiple platforms without multi-platform enabled") + return nil, nil, errors.New("cannot export multiple platforms without multi-platform enabled") } var ref cache.ImmutableRef @@ -137,7 +143,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session if opts.Epoch == nil { expEpoch, err = epoch.ParseSource(inp, p) if err != nil { - return nil, err + return nil, nil, err } } } else { @@ -152,33 +158,38 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session if len(baseImgConfig) > 0 { var baseImgX dockerspec.DockerOCIImage if err := json.Unmarshal(baseImgConfig, &baseImgX); err != nil { - return nil, errors.Wrap(err, "failed to unmarshal base image config") + return nil, nil, errors.Wrap(err, "failed to unmarshal base image config") } baseImg = &baseImgX } remotes, err := ic.exportLayers(ctx, opts.RefCfg, session.NewGroup(sessionID), ref) if err != nil { - return nil, err + return nil, nil, err } remote := &remotes[0] if opts.RewriteTimestamp { remote, err = ic.rewriteRemoteWithEpoch(ctx, opts, remote, baseImg, expEpoch) if err != nil { - return nil, err + return nil, nil, err } } + if p != nil { + committedRemotes.AddRef(p.ID, remote) + } else { + committedRemotes.SetRef(remote) + } annotations := opts.Annotations.Platform(nil) if len(annotations.Index) > 0 || len(annotations.IndexDescriptor) > 0 { - return nil, errors.Errorf("index annotations not supported for single platform export") + return nil, nil, errors.Errorf("index annotations not supported for single platform export") } var inlineCacheEntry *exptypes.InlineCacheEntry if inlineCache != nil { inlineCacheResult, err := inlineCache(ctx) if err != nil { - return nil, err + return nil, nil, err } if inlineCacheResult != nil { if p != nil { @@ -191,7 +202,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session mfstDesc, configDesc, err := ic.commitDistributionManifest(ctx, opts, ref, config, remote, annotations, inlineCacheEntry, expEpoch, session.NewGroup(sessionID), baseImg) if err != nil { - return nil, err + return nil, nil, err } if mfstDesc.Annotations == nil { mfstDesc.Annotations = make(map[string]string) @@ -201,11 +212,11 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session } mfstDesc.Annotations[exptypes.ExporterConfigDigestKey] = configDesc.Digest.String() - return mfstDesc, nil + return mfstDesc, committedRemotes, nil } if !opts.OCITypesEnabled() && len(inp.Attestations) > 0 { - return nil, errors.New("cannot export attestations with \"oci-mediatypes=false\"") + return nil, nil, errors.New("cannot export attestations with \"oci-mediatypes=false\"") } refs := make([]cache.ImmutableRef, 0, len(inp.Refs)) @@ -213,7 +224,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session for _, p := range ps.Platforms { r, ok := inp.FindRef(p.ID) if !ok { - return nil, errors.Errorf("failed to find ref for ID %s", p.ID) + return nil, nil, errors.Errorf("failed to find ref for ID %s", p.ID) } remotesMap[p.ID] = len(refs) refs = append(refs, r) @@ -221,14 +232,14 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session remotes, err := ic.exportLayers(ctx, opts.RefCfg, session.NewGroup(sessionID), refs...) if err != nil { - return nil, err + return nil, nil, err } var inlineCacheResult *result.Result[*exptypes.InlineCacheEntry] if inlineCache != nil { inlineCacheResult, err = inlineCache(ctx) if err != nil { - return nil, err + return nil, nil, err } } @@ -251,7 +262,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session for i, p := range ps.Platforms { r, ok := inp.FindRef(p.ID) if !ok { - return nil, errors.Errorf("failed to find ref for ID %s", p.ID) + return nil, nil, errors.Errorf("failed to find ref for ID %s", p.ID) } config := exptypes.ParseKey(inp.Metadata, exptypes.ExporterImageConfigKey, &p) baseImgConfig := exptypes.ParseKey(inp.Metadata, exptypes.ExporterImageBaseConfigKey, &p) @@ -259,7 +270,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session if len(baseImgConfig) > 0 { var baseImgX dockerspec.DockerOCIImage if err := json.Unmarshal(baseImgConfig, &baseImgX); err != nil { - return nil, errors.Wrap(err, "failed to unmarshal base image config") + return nil, nil, errors.Wrap(err, "failed to unmarshal base image config") } baseImg = &baseImgX } @@ -268,7 +279,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session if opts.Epoch == nil { expEpoch, err = epoch.ParseSource(inp, &p) if err != nil { - return nil, err + return nil, nil, err } } else { expEpoch = opts.Epoch.Value @@ -283,9 +294,10 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session if opts.RewriteTimestamp { remote, err = ic.rewriteRemoteWithEpoch(ctx, opts, remote, baseImg, expEpoch) if err != nil { - return nil, err + return nil, nil, err } } + committedRemotes.AddRef(p.ID, remote) var inlineCacheEntry *exptypes.InlineCacheEntry if inlineCacheResult != nil { @@ -294,7 +306,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session desc, _, err := ic.commitDistributionManifest(ctx, opts, r, config, remote, opts.Annotations.Platform(&p.Platform), inlineCacheEntry, expEpoch, session.NewGroup(sessionID), baseImg) if err != nil { - return nil, err + return nil, nil, err } dp := p.Platform desc.Platform = &dp @@ -305,7 +317,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session if attestations, ok := inp.Attestations[p.ID]; ok { attestations, err := attestation.Unbundle(ctx, session.NewGroup(sessionID), attestations) if err != nil { - return nil, err + return nil, nil, err } eg, ctx2 := errgroup.WithContext(ctx) @@ -320,7 +332,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session }) } if err := eg.Wait(); err != nil { - return nil, err + return nil, nil, err } var defaultSubjects []intoto.Subject @@ -330,7 +342,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session } pl, err := purl.RefToPURL(packageurl.TypeDocker, name, &p.Platform) if err != nil { - return nil, err + return nil, nil, err } defaultSubjects = append(defaultSubjects, intoto.Subject{ Name: pl, @@ -339,12 +351,12 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session } stmts, err := attestation.MakeInTotoStatements(ctx, session.NewGroup(sessionID), attestations, defaultSubjects) if err != nil { - return nil, err + return nil, nil, err } desc, err := ic.commitAttestationsManifest(ctx, opts, *desc, stmts, opts.OCIArtifactEnabled()) if err != nil { - return nil, err + return nil, nil, err } desc.Platform = &intotoPlatform attestationManifests = append(attestationManifests, *desc) @@ -358,7 +370,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session idxBytes, err := json.MarshalIndent(idx, "", " ") if err != nil { - return nil, errors.Wrap(err, "failed to marshal index") + return nil, nil, errors.Wrap(err, "failed to marshal index") } idxDigest := digest.FromBytes(idxBytes) @@ -371,11 +383,11 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session idxDone := progress.OneOff(ctx, "exporting manifest list "+idxDigest.String()) if err := content.WriteBlob(ctx, ic.opt.ContentStore, idxDigest.String(), bytes.NewReader(idxBytes), idxDesc, content.WithLabels(labels)); err != nil { - return nil, idxDone(errors.Wrapf(err, "error writing manifest list blob %s", idxDigest)) + return nil, nil, idxDone(errors.Wrapf(err, "error writing manifest list blob %s", idxDigest)) } idxDone(nil) - return &idxDesc, nil + return &idxDesc, committedRemotes, nil } func (ic *ImageWriter) exportLayers(ctx context.Context, refCfg cacheconfig.RefConfig, s session.Group, refs ...cache.ImmutableRef) ([]solver.Remote, error) { @@ -480,12 +492,28 @@ func (ic *ImageWriter) rewriteRemoteWithEpoch(ctx context.Context, opts *ImageCo if err := rewriteDone(eg.Wait()); err != nil { return nil, err } + var provider content.InfoReaderProvider = cs + if unlazier, ok := remote.Provider.(cache.Unlazier); ok { + // Preserve the unlazier of the original provider, as the layers that + // were not rewritten (e.g., the layers of the base image) may still be + // lazy and need to be fetched before unpacking the image. + provider = &unlazyProvider{Store: cs, unlazier: unlazier} + } return &solver.Remote{ - Provider: cs, + Provider: provider, Descriptors: remoteDescriptors, }, nil } +type unlazyProvider struct { + content.Store + unlazier cache.Unlazier +} + +func (p *unlazyProvider) Unlazy(ctx context.Context) error { + return p.unlazier.Unlazy(ctx) +} + func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, opts *ImageCommitOpts, ref cache.ImmutableRef, config []byte, remote *solver.Remote, annotations *Annotations, inlineCache *exptypes.InlineCacheEntry, epoch *time.Time, sg session.Group, baseImg *dockerspec.DockerOCIImage) (*ocispecs.Descriptor, *ocispecs.Descriptor, error) { if len(config) == 0 { var err error diff --git a/exporter/oci/export.go b/exporter/oci/export.go index 950e43a756e7..baed31d26124 100644 --- a/exporter/oci/export.go +++ b/exporter/oci/export.go @@ -163,7 +163,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source } }() - desc, err := e.opt.ImageWriter.Commit(ctx, src, buildInfo.SessionID, buildInfo.InlineCache, &opts, buildInfo.CompatibilityVersion, e.Type()) + desc, _, err := e.opt.ImageWriter.Commit(ctx, src, buildInfo.SessionID, buildInfo.InlineCache, &opts, buildInfo.CompatibilityVersion, e.Type()) if err != nil { return nil, nil, nil, err }