From e1967bb9da3155384b32f1af4efc1c6f353a6614 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Mon, 19 Feb 2024 21:24:48 +0900 Subject: [PATCH 1/2] dockerfile2llb: emit base image config The base image config will be used later for avoiding applying `SOURCE_DATE_EPOCH` to the base image layers (issue 4614). The exporter stores this as the `ExporterImageBaseConfigKey` metadata. NOTE: For a multi-stage Dockerfile like below, the base image refers to `busybox`, not to `foo`: ```dockerfile FROM busybox AS foo FROM foo AS bar ``` Signed-off-by: Akihiro Suda --- examples/dockerfile2llb/main.go | 10 ++++- exporter/containerimage/exptypes/types.go | 2 + frontend/dockerfile/builder/build.go | 14 +++---- frontend/dockerfile/dockerfile2llb/convert.go | 11 ++++-- .../dockerfile/dockerfile2llb/convert_test.go | 38 +++++++++++++------ frontend/dockerfile/dockerfile2llb/image.go | 8 ++++ frontend/dockerui/build.go | 18 ++++++++- 7 files changed, 76 insertions(+), 25 deletions(-) diff --git a/examples/dockerfile2llb/main.go b/examples/dockerfile2llb/main.go index c44ceccdb602..fda4c61991f9 100644 --- a/examples/dockerfile2llb/main.go +++ b/examples/dockerfile2llb/main.go @@ -19,6 +19,7 @@ import ( type buildOpt struct { target string partialImageConfigFile string + baseImageConfigFile string } func main() { @@ -31,6 +32,7 @@ func xmain() error { var opt buildOpt flag.StringVar(&opt.target, "target", "", "target stage") flag.StringVar(&opt.partialImageConfigFile, "partial-image-config-file", "", "Output partial image config as a JSON file") + flag.StringVar(&opt.baseImageConfigFile, "base-image-config-file", "", "Output base image config as a JSON file") flag.Parse() df, err := io.ReadAll(os.Stdin) @@ -40,7 +42,7 @@ func xmain() error { caps := pb.Caps.CapSet(pb.Caps.All()) - state, img, _, err := dockerfile2llb.Dockerfile2LLB(appcontext.Context(), df, dockerfile2llb.ConvertOpt{ + state, img, baseImg, _, err := dockerfile2llb.Dockerfile2LLB(appcontext.Context(), df, dockerfile2llb.ConvertOpt{ MetaResolver: imagemetaresolver.Default(), LLBCaps: &caps, Config: dockerui.Config{ @@ -63,6 +65,12 @@ func xmain() error { return err } } + if opt.baseImageConfigFile != "" { + if err := writeJSON(opt.baseImageConfigFile, baseImg); err != nil { + return err + } + } + return nil } diff --git a/exporter/containerimage/exptypes/types.go b/exporter/containerimage/exptypes/types.go index 74465c858304..056485b66f12 100644 --- a/exporter/containerimage/exptypes/types.go +++ b/exporter/containerimage/exptypes/types.go @@ -13,6 +13,7 @@ const ( ExporterImageConfigKey = "containerimage.config" ExporterImageConfigDigestKey = "containerimage.config.digest" ExporterImageDescriptorKey = "containerimage.descriptor" + ExporterImageBaseConfigKey = "containerimage.base.config" ExporterPlatformsKey = "refs.platforms" ) @@ -20,6 +21,7 @@ const ( // a platform to become platform specific var KnownRefMetadataKeys = []string{ ExporterImageConfigKey, + ExporterImageBaseConfigKey, } type Platforms struct { diff --git a/frontend/dockerfile/builder/build.go b/frontend/dockerfile/builder/build.go index ed6fe09f22d6..fc6c29efb793 100644 --- a/frontend/dockerfile/builder/build.go +++ b/frontend/dockerfile/builder/build.go @@ -115,21 +115,21 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) { scanTargets := sync.Map{} - rb, err := bc.Build(ctx, func(ctx context.Context, platform *ocispecs.Platform, idx int) (client.Reference, *dockerspec.DockerOCIImage, error) { + rb, err := bc.Build(ctx, func(ctx context.Context, platform *ocispecs.Platform, idx int) (client.Reference, *dockerspec.DockerOCIImage, *dockerspec.DockerOCIImage, error) { opt := convertOpt opt.TargetPlatform = platform if idx != 0 { opt.Warn = nil } - st, img, scanTarget, err := dockerfile2llb.Dockerfile2LLB(ctx, src.Data, opt) + st, img, baseImg, scanTarget, err := dockerfile2llb.Dockerfile2LLB(ctx, src.Data, opt) if err != nil { - return nil, nil, err + return nil, nil, nil, err } def, err := st.Marshal(ctx) if err != nil { - return nil, nil, errors.Wrapf(err, "failed to marshal LLB definition") + return nil, nil, nil, errors.Wrapf(err, "failed to marshal LLB definition") } r, err := c.Solve(ctx, client.SolveRequest{ @@ -137,12 +137,12 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) { CacheImports: bc.CacheImports, }) if err != nil { - return nil, nil, err + return nil, nil, nil, err } ref, err := r.SingleRef() if err != nil { - return nil, nil, err + return nil, nil, nil, err } p := platforms.DefaultSpec() @@ -151,7 +151,7 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) { } scanTargets.Store(platforms.Format(platforms.Normalize(p)), scanTarget) - return ref, img, nil + return ref, img, baseImg, nil }) if err != nil { return nil, err diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 39399cdcb933..0b0af8a6add5 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -72,13 +72,13 @@ type SBOMTargets struct { IgnoreCache bool } -func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, *dockerspec.DockerOCIImage, *SBOMTargets, error) { +func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (st *llb.State, img, baseImg *dockerspec.DockerOCIImage, sbom *SBOMTargets, err error) { ds, err := toDispatchState(ctx, dt, opt) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } - sbom := SBOMTargets{ + sbom = &SBOMTargets{ Core: ds.state, Extras: map[string]llb.State{}, } @@ -97,7 +97,7 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, } } - return &ds.state, &ds.image, &sbom, nil + return &ds.state, &ds.image, ds.baseImg, sbom, nil } func Dockefile2Outline(ctx context.Context, dt []byte, opt ConvertOpt) (*outline.Outline, error) { @@ -445,6 +445,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS if err := json.Unmarshal(dt, &img); err != nil { return errors.Wrap(err, "failed to parse image config") } + d.baseImg = cloneX(&img) // immutable img.Created = nil // if there is no explicit target platform, try to match based on image config if d.platform == nil && platformOpt.implicitTarget { @@ -507,6 +508,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS d.state = d.base.state d.platform = d.base.platform d.image = clone(d.base.image) + d.baseImg = cloneX(d.base.baseImg) // Utilize the same path index as our base image so we propagate // the paths we use back to the base image. d.paths = d.base.paths @@ -834,6 +836,7 @@ type dispatchState struct { platform *ocispecs.Platform stage instructions.Stage base *dispatchState + baseImg *dockerspec.DockerOCIImage // immutable, unlike image noinit bool deps map[*dispatchState]instructions.Command buildArgs []instructions.KeyValuePairOptional diff --git a/frontend/dockerfile/dockerfile2llb/convert_test.go b/frontend/dockerfile/dockerfile2llb/convert_test.go index bdc58cb2c928..a4e0cd220ae4 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_test.go +++ b/frontend/dockerfile/dockerfile2llb/convert_test.go @@ -8,6 +8,7 @@ import ( "github.com/moby/buildkit/frontend/dockerfile/shell" "github.com/moby/buildkit/frontend/dockerui" "github.com/moby/buildkit/util/appcontext" + digest "github.com/opencontainers/go-digest" "github.com/stretchr/testify/assert" ) @@ -33,7 +34,7 @@ ENV FOO bar COPY f1 f2 /sub/ RUN ls -l ` - _, _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + _, _, _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.NoError(t, err) df = `FROM scratch AS foo @@ -42,7 +43,7 @@ FROM foo COPY --from=foo f1 / COPY --from=0 f2 / ` - _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + _, _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.NoError(t, err) df = `FROM scratch AS foo @@ -51,14 +52,14 @@ FROM foo COPY --from=foo f1 / COPY --from=0 f2 / ` - _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{ + _, _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{ Config: dockerui.Config{ Target: "Foo", }, }) assert.NoError(t, err) - _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{ + _, _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{ Config: dockerui.Config{ Target: "nosuch", }, @@ -68,21 +69,21 @@ COPY --from=0 f2 / df = `FROM scratch ADD http://github.com/moby/buildkit/blob/master/README.md / ` - _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + _, _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.NoError(t, err) df = `FROM scratch COPY http://github.com/moby/buildkit/blob/master/README.md / ` - _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + _, _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.EqualError(t, err, "source can't be a URL for COPY") df = `FROM "" AS foo` - _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + _, _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.Error(t, err) df = `FROM ${BLANK} AS foo` - _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + _, _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.Error(t, err) } @@ -93,7 +94,7 @@ ENV FOO bar COPY f1 f2 /sub/ RUN ls -l ` - state, _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + state, _, _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.NoError(t, err) _, err = state.Marshal(context.TODO()) @@ -194,7 +195,7 @@ func TestDockerfileCircularDependencies(t *testing.T) { df := `FROM busybox AS stage0 COPY --from=stage0 f1 /sub/ ` - _, _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + _, _, _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.EqualError(t, err, "circular dependency detected on stage: stage0") // multiple stages with circular dependency @@ -205,6 +206,21 @@ COPY --from=stage0 f2 /sub/ FROM busybox AS stage2 COPY --from=stage1 f2 /sub/ ` - _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + _, _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.EqualError(t, err, "circular dependency detected on stage: stage0") } + +func TestBaseImageConfig(t *testing.T) { + df := `FROM --platform=linux/amd64 busybox:1.36.1@sha256:6d9ac9237a84afe1516540f40a0fafdc86859b2141954b4d643af7066d598b74 AS foo +RUN echo foo + +# the source image of bar is busybox, not foo +FROM foo AS bar +RUN echo bar +` + _, _, baseImg, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + assert.NoError(t, err) + t.Logf("baseImg=%+v", baseImg) + assert.Equal(t, []digest.Digest{"sha256:2e112031b4b923a873c8b3d685d48037e4d5ccd967b658743d93a6e56c3064b9"}, baseImg.RootFS.DiffIDs) + assert.Equal(t, "2024-01-17 21:49:12 +0000 UTC", baseImg.Created.String()) +} diff --git a/frontend/dockerfile/dockerfile2llb/image.go b/frontend/dockerfile/dockerfile2llb/image.go index 1603bcacf56f..b6b589e77654 100644 --- a/frontend/dockerfile/dockerfile2llb/image.go +++ b/frontend/dockerfile/dockerfile2llb/image.go @@ -15,6 +15,14 @@ func clone(src dockerspec.DockerOCIImage) dockerspec.DockerOCIImage { return img } +func cloneX(src *dockerspec.DockerOCIImage) *dockerspec.DockerOCIImage { + if src == nil { + return nil + } + img := clone(*src) + return &img +} + func emptyImage(platform ocispecs.Platform) dockerspec.DockerOCIImage { img := dockerspec.DockerOCIImage{} img.Architecture = platform.Architecture diff --git a/frontend/dockerui/build.go b/frontend/dockerui/build.go index 960138cf87ec..c25640912067 100644 --- a/frontend/dockerui/build.go +++ b/frontend/dockerui/build.go @@ -14,7 +14,7 @@ import ( "golang.org/x/sync/errgroup" ) -type BuildFunc func(ctx context.Context, platform *ocispecs.Platform, idx int) (client.Reference, *dockerspec.DockerOCIImage, error) +type BuildFunc func(ctx context.Context, platform *ocispecs.Platform, idx int) (r client.Reference, img, baseImg *dockerspec.DockerOCIImage, err error) func (bc *Client) Build(ctx context.Context, fn BuildFunc) (*ResultBuilder, error) { res := client.NewResult() @@ -36,7 +36,7 @@ func (bc *Client) Build(ctx context.Context, fn BuildFunc) (*ResultBuilder, erro for i, tp := range targets { i, tp := i, tp eg.Go(func() error { - ref, img, err := fn(ctx, tp, i) + ref, img, baseImg, err := fn(ctx, tp, i) if err != nil { return err } @@ -46,6 +46,14 @@ func (bc *Client) Build(ctx context.Context, fn BuildFunc) (*ResultBuilder, erro return errors.Wrapf(err, "failed to marshal image config") } + var baseConfig []byte + if baseImg != nil { + baseConfig, err = json.Marshal(baseImg) + if err != nil { + return errors.Wrapf(err, "failed to marshal source image config") + } + } + p := platforms.DefaultSpec() if tp != nil { p = *tp @@ -67,9 +75,15 @@ func (bc *Client) Build(ctx context.Context, fn BuildFunc) (*ResultBuilder, erro if bc.MultiPlatformRequested { res.AddRef(k, ref) res.AddMeta(fmt.Sprintf("%s/%s", exptypes.ExporterImageConfigKey, k), config) + if len(baseConfig) > 0 { + res.AddMeta(fmt.Sprintf("%s/%s", exptypes.ExporterImageBaseConfigKey, k), baseConfig) + } } else { res.SetRef(ref) res.AddMeta(exptypes.ExporterImageConfigKey, config) + if len(baseConfig) > 0 { + res.AddMeta(exptypes.ExporterImageBaseConfigKey, baseConfig) + } } expPlatforms.Platforms[i] = exptypes.Platform{ ID: k, From cc14a06da5483b3a8f10ea068ad19b8008b255e8 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Mon, 19 Feb 2024 23:40:21 +0900 Subject: [PATCH 2/2] Avoid applying `SOURCE_DATE_EPOCH` to base images The exporter fetches the base image config via the `ExporterImageBaseConfigKey` metadata to detect the immutable layer diffIDs and the history objects. Fix issue 4614 Signed-off-by: Akihiro Suda --- exporter/containerimage/writer.go | 70 ++++++++++++++++--- frontend/dockerfile/dockerfile2llb/convert.go | 19 +---- frontend/dockerfile/dockerfile_test.go | 51 +++++++++++--- util/converter/converter.go | 14 +++- 4 files changed, 115 insertions(+), 39 deletions(-) diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index 303b9ff06580..822c250fc22b 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "reflect" "strconv" "strings" "time" @@ -34,6 +35,7 @@ import ( "github.com/moby/buildkit/util/purl" "github.com/moby/buildkit/util/system" "github.com/moby/buildkit/util/tracing" + dockerspec "github.com/moby/docker-image-spec/specs-go/v1" digest "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" @@ -124,6 +126,15 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session ref = inp.Ref } config := exptypes.ParseKey(inp.Metadata, exptypes.ExporterImageConfigKey, p) + baseImgConfig := exptypes.ParseKey(inp.Metadata, exptypes.ExporterImageBaseConfigKey, p) + var baseImg *dockerspec.DockerOCIImage + 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") + } + baseImg = &baseImgX + } remotes, err := ic.exportLayers(ctx, opts.RefCfg, session.NewGroup(sessionID), ref) if err != nil { @@ -131,7 +142,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session } remote := &remotes[0] if opts.RewriteTimestamp { - remote, err = ic.rewriteRemoteWithEpoch(ctx, opts, remote) + remote, err = ic.rewriteRemoteWithEpoch(ctx, opts, remote, baseImg) if err != nil { return nil, err } @@ -157,7 +168,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session } } - mfstDesc, configDesc, err := ic.commitDistributionManifest(ctx, opts, ref, config, remote, annotations, inlineCacheEntry, opts.Epoch, session.NewGroup(sessionID)) + mfstDesc, configDesc, err := ic.commitDistributionManifest(ctx, opts, ref, config, remote, annotations, inlineCacheEntry, opts.Epoch, session.NewGroup(sessionID), baseImg) if err != nil { return nil, err } @@ -222,6 +233,15 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session return 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) + var baseImg *dockerspec.DockerOCIImage + 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") + } + baseImg = &baseImgX + } remote := &remotes[remotesMap[p.ID]] if remote == nil { @@ -230,7 +250,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session } } if opts.RewriteTimestamp { - remote, err = ic.rewriteRemoteWithEpoch(ctx, opts, remote) + remote, err = ic.rewriteRemoteWithEpoch(ctx, opts, remote, baseImg) if err != nil { return nil, err } @@ -241,7 +261,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session inlineCacheEntry, _ = inlineCacheResult.FindRef(p.ID) } - desc, _, err := ic.commitDistributionManifest(ctx, opts, r, config, remote, opts.Annotations.Platform(&p.Platform), inlineCacheEntry, opts.Epoch, session.NewGroup(sessionID)) + desc, _, err := ic.commitDistributionManifest(ctx, opts, r, config, remote, opts.Annotations.Platform(&p.Platform), inlineCacheEntry, opts.Epoch, session.NewGroup(sessionID), baseImg) if err != nil { return nil, err } @@ -369,8 +389,14 @@ func (ic *ImageWriter) exportLayers(ctx context.Context, refCfg cacheconfig.RefC // the new blob. // // If no conversion is needed, this returns nil without error. -func rewriteImageLayerWithEpoch(ctx context.Context, cs content.Store, desc ocispecs.Descriptor, comp compression.Config, epoch *time.Time) (*ocispecs.Descriptor, error) { - converterFn, err := converter.NewWithRewriteTimestamp(ctx, cs, desc, comp, epoch) +func rewriteImageLayerWithEpoch(ctx context.Context, cs content.Store, desc ocispecs.Descriptor, comp compression.Config, epoch *time.Time, immDiffID digest.Digest) (*ocispecs.Descriptor, error) { + var immDiffIDs map[digest.Digest]struct{} + if immDiffID != "" { + immDiffIDs = map[digest.Digest]struct{}{ + immDiffID: {}, + } + } + converterFn, err := converter.NewWithRewriteTimestamp(ctx, cs, desc, comp, epoch, immDiffIDs) if err != nil { return nil, err } @@ -380,7 +406,7 @@ func rewriteImageLayerWithEpoch(ctx context.Context, cs content.Store, desc ocis return converterFn(ctx, cs, desc) } -func (ic *ImageWriter) rewriteRemoteWithEpoch(ctx context.Context, opts *ImageCommitOpts, remote *solver.Remote) (*solver.Remote, error) { +func (ic *ImageWriter) rewriteRemoteWithEpoch(ctx context.Context, opts *ImageCommitOpts, remote *solver.Remote, baseImg *dockerspec.DockerOCIImage) (*solver.Remote, error) { if opts.Epoch == nil { bklog.G(ctx).Warn("rewrite-timestamp is specified, but no source-date-epoch was found") return remote, nil @@ -390,10 +416,25 @@ func (ic *ImageWriter) rewriteRemoteWithEpoch(ctx context.Context, opts *ImageCo eg, ctx := errgroup.WithContext(ctx) rewriteDone := progress.OneOff(ctx, fmt.Sprintf("rewriting layers with source-date-epoch %d (%s)", opts.Epoch.Unix(), opts.Epoch.String())) + var divergedFromBase bool for i, desc := range remoteDescriptors { i, desc := i, desc + info, err := cs.Info(ctx, desc.Digest) + if err != nil { + return nil, err + } + diffID := digest.Digest(info.Labels[labels.LabelUncompressed]) // can be empty + var immDiffID digest.Digest + if !divergedFromBase && baseImg != nil && i < len(baseImg.RootFS.DiffIDs) { + immDiffID = baseImg.RootFS.DiffIDs[i] + if immDiffID == diffID { + bklog.G(ctx).WithField("blob", desc).Debugf("Not rewriting to apply epoch (immutable diffID %q)", diffID) + continue + } + divergedFromBase = true + } eg.Go(func() error { - if rewrittenDesc, err := rewriteImageLayerWithEpoch(ctx, cs, desc, opts.RefCfg.Compression, opts.Epoch); err != nil { + if rewrittenDesc, err := rewriteImageLayerWithEpoch(ctx, cs, desc, opts.RefCfg.Compression, opts.Epoch, immDiffID); err != nil { bklog.G(ctx).WithError(err).Warnf("failed to rewrite layer %d/%d to match source-date-epoch %d (%s)", i+1, len(remoteDescriptors), opts.Epoch.Unix(), opts.Epoch.String()) } else if rewrittenDesc != nil { @@ -411,7 +452,7 @@ func (ic *ImageWriter) rewriteRemoteWithEpoch(ctx context.Context, opts *ImageCo }, nil } -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) (*ocispecs.Descriptor, *ocispecs.Descriptor, error) { +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 config, err = defaultImageConfig() @@ -430,7 +471,7 @@ func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, opts *Ima return nil, nil, err } - config, err = patchImageConfig(config, remote.Descriptors, history, inlineCache, epoch) + config, err = patchImageConfig(config, remote.Descriptors, history, inlineCache, epoch, baseImg) if err != nil { return nil, nil, err } @@ -658,7 +699,7 @@ func parseHistoryFromConfig(dt []byte) ([]ocispecs.History, error) { return config.History, nil } -func patchImageConfig(dt []byte, descs []ocispecs.Descriptor, history []ocispecs.History, cache *exptypes.InlineCacheEntry, epoch *time.Time) ([]byte, error) { +func patchImageConfig(dt []byte, descs []ocispecs.Descriptor, history []ocispecs.History, cache *exptypes.InlineCacheEntry, epoch *time.Time, baseImg *dockerspec.DockerOCIImage) ([]byte, error) { var img ocispecs.Image if err := json.Unmarshal(dt, &img); err != nil { return nil, errors.Wrap(err, "invalid image config for export") @@ -692,7 +733,14 @@ func patchImageConfig(dt []byte, descs []ocispecs.Descriptor, history []ocispecs m["rootfs"] = dt if epoch != nil { + var divergedFromBase bool for i, h := range history { + if !divergedFromBase && baseImg != nil && i < len(baseImg.History) && reflect.DeepEqual(h, baseImg.History[i]) { + // Retain the timestamp for the base image layers + // https://github.com/moby/buildkit/issues/4614 + continue + } + divergedFromBase = true if h.Created == nil || h.Created.After(*epoch) { history[i].Created = epoch } diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 0b0af8a6add5..754b14e7b31c 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -270,7 +270,9 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS ds.noinit = true ds.state = *s if img != nil { - ds.image = clampTimes(*img, opt.Epoch) + // timestamps are inherited as-is, regardless to SOURCE_DATE_EPOCH + // https://github.com/moby/buildkit/issues/4614 + ds.image = *img if img.Architecture != "" && img.OS != "" { ds.platform = &ocispecs.Platform{ OS: img.OS, @@ -1885,21 +1887,6 @@ func commonImageNames() []string { return out } -func clampTimes(img dockerspec.DockerOCIImage, tm *time.Time) dockerspec.DockerOCIImage { - if tm == nil { - return img - } - for i, h := range img.History { - if h.Created == nil || h.Created.After(*tm) { - img.History[i].Created = tm - } - } - if img.Created != nil && img.Created.After(*tm) { - img.Created = tm - } - return img -} - func isHTTPSource(src string) bool { return strings.HasPrefix(src, "http://") || strings.HasPrefix(src, "https://") } diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index e49d85b392aa..dc9ba225932a 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -53,6 +53,7 @@ import ( "github.com/moby/buildkit/util/testutil/httpserver" "github.com/moby/buildkit/util/testutil/integration" "github.com/moby/buildkit/util/testutil/workers" + digest "github.com/opencontainers/go-digest" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/stretchr/testify/require" @@ -6866,8 +6867,16 @@ RUN rm -f /foo.1 RUN rm -f /foo-2010.1 RUN rm -f /foo-2030.1 `) + // https://explore.ggcr.dev/?image=amd64%2Fdebian%3Abullseye-20230109-slim + baseImageLayers := []digest.Digest{ + "sha256:8740c948ffd4c816ea7ca963f99ca52f4788baa23f228da9581a9ea2edd3fcd7", + } + baseImageHistoryTimestamps := []time.Time{ + timeMustParse(t, time.RFC3339Nano, "2023-01-11T02:34:44.402266175Z"), + timeMustParse(t, time.RFC3339Nano, "2023-01-11T02:34:44.829692296Z"), + } - const expectedDigest = "sha256:3eb3c164e3420bbfcf52c34f1e40ee66631d69445e934175b779551c729f80df" + const expectedDigest = "sha256:04e5d0cbee3317c79f50494cfeb4d8a728402a970ef32582ee47c62050037e3f" dir := integration.Tmpdir( t, @@ -6914,13 +6923,24 @@ RUN rm -f /foo-2030.1 _, err = f.Solve(ctx, c, solveOpt, nil) require.NoError(t, err) - desc, manifest := readImage(t, ctx, target) - _, cacheManifest := readImage(t, ctx, target+"-cache") + desc, manifest, img := readImage(t, ctx, target) + _, cacheManifest, _ := readImage(t, ctx, target+"-cache") t.Log("The digest may change depending on the BuildKit version, the snapshotter configuration, etc.") require.Equal(t, expectedDigest, desc.Digest.String()) - // Image layers must have rewritten-timestamp - for _, l := range manifest.Layers { - require.Equal(t, fmt.Sprintf("%d", tm.Unix()), l.Annotations["buildkit/rewritten-timestamp"]) + + // Image history from the base config must remain immutable + for i, tm := range baseImageHistoryTimestamps { + require.True(t, img.History[i].Created.Equal(tm)) + } + + // Image layers, *except the base layers*, must have rewritten-timestamp + for i, l := range manifest.Layers { + if i < len(baseImageLayers) { + require.Empty(t, l.Annotations["buildkit/rewritten-timestamp"]) + require.Equal(t, baseImageLayers[i], l.Digest) + } else { + require.Equal(t, fmt.Sprintf("%d", tm.Unix()), l.Annotations["buildkit/rewritten-timestamp"]) + } } // Cache layers must *not* have rewritten-timestamp for _, l := range cacheManifest.Layers { @@ -6932,21 +6952,34 @@ RUN rm -f /foo-2030.1 delete(solveOpt2.Exports[0].Attrs, "rewrite-timestamp") _, err = f.Solve(ctx, c, solveOpt2, nil) require.NoError(t, err) - _, manifest2 := readImage(t, ctx, target) + _, manifest2, img2 := readImage(t, ctx, target) + for i, tm := range baseImageHistoryTimestamps { + require.True(t, img2.History[i].Created.Equal(tm)) + } for _, l := range manifest2.Layers { require.Empty(t, l.Annotations["buildkit/rewritten-timestamp"]) } } +func timeMustParse(t *testing.T, layout, value string) time.Time { + tm, err := time.Parse(layout, value) + require.NoError(t, err) + return tm +} + //nolint:revive // context-as-argument: context.Context should be the first parameter of a function -func readImage(t *testing.T, ctx context.Context, ref string) (ocispecs.Descriptor, ocispecs.Manifest) { +func readImage(t *testing.T, ctx context.Context, ref string) (ocispecs.Descriptor, ocispecs.Manifest, ocispecs.Image) { desc, provider, err := contentutil.ProviderFromRef(ref) require.NoError(t, err) dt, err := content.ReadBlob(ctx, provider, desc) require.NoError(t, err) var manifest ocispecs.Manifest require.NoError(t, json.Unmarshal(dt, &manifest)) - return desc, manifest + imgDt, err := content.ReadBlob(ctx, provider, manifest.Config) + require.NoError(t, err) + var img ocispecs.Image + require.NoError(t, json.Unmarshal(imgDt, &img)) + return desc, manifest, img } func testNilContextInSolveGateway(t *testing.T, sb integration.Sandbox) { diff --git a/util/converter/converter.go b/util/converter/converter.go index 6bb8aa858e9c..4b956f02f106 100644 --- a/util/converter/converter.go +++ b/util/converter/converter.go @@ -26,12 +26,12 @@ import ( // New returns converter function according to the specified compression type. // If no conversion is needed, this returns nil without error. func New(ctx context.Context, cs content.Store, desc ocispecs.Descriptor, comp compression.Config) (converter.ConvertFunc, error) { - return NewWithRewriteTimestamp(ctx, cs, desc, comp, nil) + return NewWithRewriteTimestamp(ctx, cs, desc, comp, nil, nil) } // NewWithRewriteTimestamp returns converter function according to the specified compression type and the epoch. // If no conversion is needed, this returns nil without error. -func NewWithRewriteTimestamp(ctx context.Context, cs content.Store, desc ocispecs.Descriptor, comp compression.Config, rewriteTimestamp *time.Time) (converter.ConvertFunc, error) { +func NewWithRewriteTimestamp(ctx context.Context, cs content.Store, desc ocispecs.Descriptor, comp compression.Config, rewriteTimestamp *time.Time, immDiffIDs map[digest.Digest]struct{}) (converter.ConvertFunc, error) { needs, err := comp.Type.NeedsConversion(ctx, cs, desc) if err != nil { return nil, errors.Wrapf(err, "failed to determine conversion needs") @@ -53,6 +53,7 @@ func NewWithRewriteTimestamp(ctx context.Context, cs content.Store, desc ocispec c.compress, c.finalize = comp.Type.Compress(ctx, comp) c.decompress = from.Decompress c.rewriteTimestamp = rewriteTimestamp + c.immDiffIDs = immDiffIDs return (&c).convert, nil } @@ -63,6 +64,7 @@ type conversion struct { compress compression.Compressor finalize compression.Finalizer rewriteTimestamp *time.Time + immDiffIDs map[digest.Digest]struct{} // diffIDs of immutable layers } var bufioPool = sync.Pool{ @@ -116,6 +118,7 @@ func (c *conversion) convert(ctx context.Context, cs content.Store, desc ocispec // convert this layer diffID := digest.Canonical.Digester() + origDiffID := digest.Canonical.Digester() decR, err := c.decompress(ctx, cs, desc) if err != nil { return nil, err @@ -123,7 +126,7 @@ func (c *conversion) convert(ctx context.Context, cs content.Store, desc ocispec defer decR.Close() rdr := decR if c.rewriteTimestamp != nil { - tcR := tarconverter.NewReader(decR, rewriteTimestampInTarHeader(*c.rewriteTimestamp)) + tcR := tarconverter.NewReader(io.TeeReader(decR, origDiffID.Hash()), rewriteTimestampInTarHeader(*c.rewriteTimestamp)) defer tcR.Close() rdr = tcR } @@ -136,6 +139,11 @@ func (c *conversion) convert(ctx context.Context, cs content.Store, desc ocispec if err := bufW.Flush(); err != nil { // Flush the buffer return nil, errors.Wrap(err, "failed to flush diff during conversion") } + origDiffIDVal := origDiffID.Digest() + if _, ok := c.immDiffIDs[origDiffIDVal]; ok { + bklog.G(ctx).WithField("blob", desc).Debugf("Not rewriting to apply epoch (immutable diffID %q, computed during conversion)", origDiffIDVal) + return &desc, nil + } labelz[labels.LabelUncompressed] = diffID.Digest().String() // update diffID label if c.rewriteTimestamp != nil { labelz[labelRewrittenTimestamp] = fmt.Sprintf("%d", c.rewriteTimestamp.UTC().Unix())