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 <akihiro.suda.cz@hco.ntt.co.jp>
  • Loading branch information
@AkihiroSuda
AkihiroSuda committed Feb 19, 2024
1 parent 0027594 commit 3c33c2e
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 39 deletions.
51 changes: 40 additions & 11 deletions exporter/containerimage/writer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,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 *ocispecs.Image
if len(srcImgConfig) > 0 {
var srcImgX ocispecs.Image
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 +166,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 +231,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 *ocispecs.Image
if len(srcImgConfig) > 0 {
var srcImgX ocispecs.Image
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 +248,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 +259,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 +387,8 @@ 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, immDiffIDs map[digest.Digest]struct{}) (*ocispecs.Descriptor, error) {
converterFn, err := converter.NewWithRewriteTimestamp(ctx, cs, desc, comp, epoch, immDiffIDs)
if err != nil {
return nil, err
}
Expand All @@ -380,11 +398,17 @@ 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 *ocispecs.Image) (*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
}
immDiffIDs := make(map[digest.Digest]struct{})
if srcImg != nil {
for _, d := range srcImg.RootFS.DiffIDs {
immDiffIDs[d] = struct{}{}
}
}
remoteDescriptors := remote.Descriptors
cs := contentutil.NewStoreWithProvider(ic.opt.ContentStore, remote.Provider)
eg, ctx := errgroup.WithContext(ctx)
Expand All @@ -393,7 +417,7 @@ func (ic *ImageWriter) rewriteRemoteWithEpoch(ctx context.Context, opts *ImageCo
for i, desc := range remoteDescriptors {
i, desc := i, desc
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, immDiffIDs); 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 +435,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 *ocispecs.Image) (*ocispecs.Descriptor, *ocispecs.Descriptor, error) {
if len(config) == 0 {
var err error
config, err = defaultImageConfig()
Expand All @@ -430,7 +454,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 +682,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 *ocispecs.Image) ([]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 +717,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) {
// 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 image.Image, tm *time.Time) image.Image {
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
24 changes: 21 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 All @@ -86,6 +88,16 @@ func rewriteTimestampInTarHeader(epoch time.Time) tarconverter.HeaderConverter {
}

func (c *conversion) convert(ctx context.Context, cs content.Store, desc ocispecs.Descriptor) (*ocispecs.Descriptor, error) {
origInfo, err := cs.Info(ctx, desc.Digest)
if err != nil {
return nil, err
}
foreknownDiffID := digest.Digest(origInfo.Labels[labels.LabelUncompressed]) // can be empty
if _, ok := c.immDiffIDs[foreknownDiffID]; ok {
bklog.G(ctx).WithField("blob", desc).Debugf("Not rewriting to apply epoch (immutable diffID %q, foreknown)", foreknownDiffID)
return &desc, nil
}

bklog.G(ctx).WithField("blob", desc).WithField("target", c.target).Debugf("converting blob to the target compression")
// prepare the source and destination
labelz := make(map[string]string)
Expand Down Expand Up @@ -116,14 +128,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 +149,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)", 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 3c33c2e

Please sign in to comment.