Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid applying SOURCE_DATE_EPOCH to base images #4663

Merged
merged 2 commits into from
Feb 24, 2024
Merged

Avoid applying SOURCE_DATE_EPOCH to base images #4663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1967bb
dockerfile2llb: emit base image config
AkihiroSuda Feb 19, 2024
cc14a06
Avoid applying `SOURCE_DATE_EPOCH` to base images
AkihiroSuda Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion examples/dockerfile2llb/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
type buildOpt struct {
target string
partialImageConfigFile string
baseImageConfigFile string
}

func main() {
Expand All @@ -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)
Expand All @@ -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{
Expand All @@ -63,6 +65,12 @@ func xmain() error {
return err
}
}
if opt.baseImageConfigFile != "" {
if err := writeJSON(opt.baseImageConfigFile, baseImg); err != nil {
return err
}
}

return nil
}

Expand Down
2 changes: 2 additions & 0 deletions exporter/containerimage/exptypes/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ const (
ExporterImageConfigKey = "containerimage.config"
ExporterImageConfigDigestKey = "containerimage.config.digest"
ExporterImageDescriptorKey = "containerimage.descriptor"
ExporterImageBaseConfigKey = "containerimage.base.config"
ExporterPlatformsKey = "refs.platforms"
)

// KnownRefMetadataKeys are the subset of exporter keys that can be suffixed by
// a platform to become platform specific
var KnownRefMetadataKeys = []string{
ExporterImageConfigKey,
ExporterImageBaseConfigKey,
}

type Platforms struct {
Expand Down
70 changes: 59 additions & 11 deletions exporter/containerimage/writer.go
View file
Open in desktop
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)
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 {
return nil, err
}
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
}
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), baseImg)
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)
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 {
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, baseImg)
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), baseImg)
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{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this construction in here. Why is a map with a single item passed? Instead of for example just setting epoch nil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is a map with a single item passed?

The converter pkg is designed to cover wider usecases that are not needed by exporter/containerimage.

Instead of for example just setting epoch nil?

Because the converter may not make the final decision until computing the diff ID (not always available in the label)

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, 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
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iiuc this looks for the index match for individual layers. But it should check that full baseImg roofs is a subset in the beginning of the exported image.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added var divergedFromBase bool to check this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a check before processing. Atm some layers can already be processed before the validation fails.

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 {
Expand All @@ -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()
Expand All @@ -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
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
Expand Down
14 changes: 7 additions & 7 deletions frontend/dockerfile/builder/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,34 +115,34 @@ 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{
Definition: def.ToPB(),
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()
Expand All @@ -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
Expand Down
30 changes: 10 additions & 20 deletions frontend/dockerfile/dockerfile2llb/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
}
Expand All @@ -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) {
Expand Down 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 @@ -445,6 +447,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 {
Expand Down Expand Up @@ -507,6 +510,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
Expand Down Expand Up @@ -834,6 +838,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
Expand Down Expand Up @@ -1882,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
Loading
Loading