Skip to content

Commit

Permalink
Avoid applying SOURCE_DATE_EPOCH to source images
Browse files Browse the repository at this point in the history 
The exporter fetches the source image config via the `ExporterImageSourceConfigKey`
metadata to detect the immutable layer diffIDs and the history objects.

Fix issue 4614

Signed-off-by: Akihiro Suda <[email protected]>
  • Loading branch information
@AkihiroSuda
AkihiroSuda committed Feb 23, 2024
1 parent 2586152 commit acaafbf
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 39 deletions.
66 changes: 55 additions & 11 deletions exporter/containerimage/writer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"fmt"
"reflect"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -124,14 +126,23 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session
ref = inp.Ref
}
config := exptypes.ParseKey(inp.Metadata, exptypes.ExporterImageConfigKey, p)
srcImgConfig := exptypes.ParseKey(inp.Metadata, exptypes.ExporterImageSourceConfigKey, p)
var srcImg *dockerspec.DockerOCIImage
if len(srcImgConfig) > 0 {
var srcImgX dockerspec.DockerOCIImage
if err := json.Unmarshal(srcImgConfig, &srcImgX); err != nil {
return nil, errors.Wrap(err, "failed to unmarshal source image config")
}
srcImg = &srcImgX
}

remotes, err := ic.exportLayers(ctx, opts.RefCfg, session.NewGroup(sessionID), ref)
if err != nil {
return nil, err
}
remote := &remotes[0]
if opts.RewriteTimestamp {
remote, err = ic.rewriteRemoteWithEpoch(ctx, opts, remote)
remote, err = ic.rewriteRemoteWithEpoch(ctx, opts, remote, srcImg)
if err != nil {
return nil, err
}
Expand All @@ -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), srcImg)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -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)
srcImgConfig := exptypes.ParseKey(inp.Metadata, exptypes.ExporterImageSourceConfigKey, &p)
var srcImg *dockerspec.DockerOCIImage
if len(srcImgConfig) > 0 {
var srcImgX dockerspec.DockerOCIImage
if err := json.Unmarshal(srcImgConfig, &srcImgX); err != nil {
return nil, errors.Wrap(err, "failed to unmarshal source image config")
}
srcImg = &srcImgX
}

remote := &remotes[remotesMap[p.ID]]
if remote == nil {
Expand All @@ -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, srcImg)
if err != nil {
return nil, err
}
Expand All @@ -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), srcImg)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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, srcImg *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
Expand All @@ -392,8 +418,21 @@ func (ic *ImageWriter) rewriteRemoteWithEpoch(ctx context.Context, opts *ImageCo
fmt.Sprintf("rewriting layers with source-date-epoch %d (%s)", opts.Epoch.Unix(), opts.Epoch.String()))
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 srcImg != nil && i < len(srcImg.RootFS.DiffIDs) {
immDiffID = srcImg.RootFS.DiffIDs[i]
if immDiffID == diffID {
bklog.G(ctx).WithField("blob", desc).Debugf("Not rewriting to apply epoch (immutable diffID %q)", diffID)
continue
}
}
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 {
Expand All @@ -411,7 +450,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, srcImg *dockerspec.DockerOCIImage) (*ocispecs.Descriptor, *ocispecs.Descriptor, error) {
if len(config) == 0 {
var err error
config, err = defaultImageConfig()
Expand All @@ -430,7 +469,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, srcImg)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -658,7 +697,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, srcImg *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")
Expand Down Expand Up @@ -693,6 +732,11 @@ func patchImageConfig(dt []byte, descs []ocispecs.Descriptor, history []ocispecs

if epoch != nil {
for i, h := range history {
if srcImg != nil && i < len(srcImg.History) && reflect.DeepEqual(h, srcImg.History[i]) {
// Retain the timestamp for the source image layers
// https://github.com/moby/buildkit/issues/4614
continue
}
if h.Created == nil || h.Created.After(*epoch) {
history[i].Created = epoch
}
Expand Down
19 changes: 3 additions & 16 deletions frontend/dockerfile/dockerfile2llb/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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://")
}
Expand Down
51 changes: 42 additions & 9 deletions frontend/dockerfile/dockerfile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
sourceImageLayers := []digest.Digest{
"sha256:8740c948ffd4c816ea7ca963f99ca52f4788baa23f228da9581a9ea2edd3fcd7",
}
sourceImageHistoryTimestamps := []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,
Expand Down Expand Up @@ -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 source config must remain immutable
for i, tm := range sourceImageHistoryTimestamps {
require.True(t, img.History[i].Created.Equal(tm))
}

// Image layers, *except the source layers*, must have rewritten-timestamp
for i, l := range manifest.Layers {
if i < len(sourceImageLayers) {
require.Empty(t, l.Annotations["buildkit/rewritten-timestamp"])
require.Equal(t, sourceImageLayers[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 {
Expand All @@ -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 sourceImageHistoryTimestamps {
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) {
Expand Down
14 changes: 11 additions & 3 deletions util/converter/converter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
}
Expand All @@ -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{
Expand Down Expand Up @@ -116,14 +118,15 @@ 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
}
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
}
Expand All @@ -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())
Expand Down

0 comments on commit acaafbf

Please sign in to comment.