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

rewrite-timestamp: fix incompatibility with COPY --link #4804

Merged
merged 1 commit into from
Apr 1, 2024
Merged

rewrite-timestamp: fix incompatibility with COPY --link #4804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 9 additions & 4 deletions exporter/containerimage/writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,11 +419,16 @@ func (ic *ImageWriter) rewriteRemoteWithEpoch(ctx context.Context, opts *ImageCo
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
// Usually we get non-empty diffID here, but if the content was ingested via a third-party containerd client,
// diffID here can be empty, and will be computed by the converter.
diffID := digest.Digest(desc.Annotations[labels.LabelUncompressed])
if diffID == "" {
info, err := cs.Info(ctx, desc.Digest)
if err != nil {
return nil, err
}
diffID = digest.Digest(info.Labels[labels.LabelUncompressed]) // can be still empty
AkihiroSuda marked this conversation as resolved.
Show resolved Hide resolved
}
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]
Expand Down
186 changes: 113 additions & 73 deletions frontend/dockerfile/dockerfile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6945,7 +6945,16 @@ func testReproSourceDateEpoch(t *testing.T, sb integration.Sandbox) {
tm := time.Date(2023, time.January, 10, 12, 34, 56, 0, time.UTC) // 1673354096
t.Logf("SOURCE_DATE_EPOCH=%d", tm.Unix())

dockerfile := []byte(`# The base image cannot be busybox, due to https://github.com/moby/buildkit/issues/3455
type testCase struct {
name string
dockerfile string
files []fstest.Applier
expectedDigest string
}
testCases := []testCase{
{
name: "Basic",
dockerfile: `# The base image could not be busybox, due to https://github.com/moby/buildkit/issues/3455
FROM amd64/debian:bullseye-20230109-slim
RUN touch /foo
RUN touch /foo.1
Expand All @@ -6956,7 +6965,20 @@ RUN touch -d '2030-01-01 12:34:56' /foo-2030.1
RUN rm -f /foo.1
RUN rm -f /foo-2010.1
RUN rm -f /foo-2030.1
`)
`,
expectedDigest: "sha256:04e5d0cbee3317c79f50494cfeb4d8a728402a970ef32582ee47c62050037e3f",
},
{
// https://github.com/moby/buildkit/issues/4746
name: "CopyLink",
dockerfile: `FROM amd64/debian:bullseye-20230109-slim
COPY --link foo foo
`,
files: []fstest.Applier{fstest.CreateFile("foo", []byte("foo"), 0600)},
expectedDigest: "sha256:9f75e4bdbf3d825acb36bb603ddef4a25742afb8ccb674763ffc611ae047d8a6",
},
}

// https://explore.ggcr.dev/?image=amd64%2Fdebian%3Abullseye-20230109-slim
baseImageLayers := []digest.Digest{
"sha256:8740c948ffd4c816ea7ca963f99ca52f4788baa23f228da9581a9ea2edd3fcd7",
Expand All @@ -6966,88 +6988,98 @@ RUN rm -f /foo-2030.1
timeMustParse(t, time.RFC3339Nano, "2023-01-11T02:34:44.829692296Z"),
}

const expectedDigest = "sha256:04e5d0cbee3317c79f50494cfeb4d8a728402a970ef32582ee47c62050037e3f"

dir := integration.Tmpdir(
t,
fstest.CreateFile("Dockerfile", dockerfile, 0600),
)

ctx := sb.Context()
c, err := client.New(ctx, sb.Address())
require.NoError(t, err)
defer c.Close()

target := registry + "/buildkit/testreprosourcedateepoch:" + fmt.Sprintf("%d", tm.Unix())
solveOpt := client.SolveOpt{
FrontendAttrs: map[string]string{
"build-arg:SOURCE_DATE_EPOCH": fmt.Sprintf("%d", tm.Unix()),
"platform": "linux/amd64",
},
LocalMounts: map[string]fsutil.FS{
dockerui.DefaultLocalNameDockerfile: dir,
dockerui.DefaultLocalNameContext: dir,
},
Exports: []client.ExportEntry{
{
Type: client.ExporterImage,
Attrs: map[string]string{
"name": target,
"push": "true",
"oci-mediatypes": "true",
"rewrite-timestamp": "true",
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
dir := integration.Tmpdir(
t,
append([]fstest.Applier{fstest.CreateFile("Dockerfile", []byte(tc.dockerfile), 0600)}, tc.files...)...,
)

target := registry + "/buildkit/testreprosourcedateepoch-" + strings.ToLower(tc.name) + ":" + fmt.Sprintf("%d", tm.Unix())
solveOpt := client.SolveOpt{
FrontendAttrs: map[string]string{
"build-arg:SOURCE_DATE_EPOCH": fmt.Sprintf("%d", tm.Unix()),
"platform": "linux/amd64",
},
},
},
CacheExports: []client.CacheOptionsEntry{
{
Type: "registry",
Attrs: map[string]string{
"ref": target + "-cache",
"oci-mediatypes": "true",
"image-manifest": "true",
LocalMounts: map[string]fsutil.FS{
dockerui.DefaultLocalNameDockerfile: dir,
dockerui.DefaultLocalNameContext: dir,
},
},
},
}
_, err = f.Solve(ctx, c, solveOpt, nil)
require.NoError(t, err)
Exports: []client.ExportEntry{
{
Type: client.ExporterImage,
Attrs: map[string]string{
"name": target,
"push": "true",
"oci-mediatypes": "true",
"rewrite-timestamp": "true",
},
},
},
CacheExports: []client.CacheOptionsEntry{
{
Type: "registry",
Attrs: map[string]string{
"ref": target + "-cache",
"oci-mediatypes": "true",
"image-manifest": "true",
},
},
},
}
_, err = f.Solve(ctx, c, solveOpt, nil)
require.NoError(t, err)

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())
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, tc.expectedDigest, desc.Digest.String())

// Image history from the base config must remain immutable
for i, tm := range baseImageHistoryTimestamps {
require.True(t, img.History[i].Created.Equal(tm))
}
// 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 {
require.Empty(t, l.Annotations["buildkit/rewritten-timestamp"])
}
// 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 {
require.Empty(t, l.Annotations["buildkit/rewritten-timestamp"])
}

// Build again, but without rewrite-timestamp
solveOpt2 := solveOpt
delete(solveOpt2.Exports[0].Attrs, "rewrite-timestamp")
_, err = f.Solve(ctx, c, solveOpt2, nil)
require.NoError(t, err)
_, 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"])
// Build again, after pruning the base image layer cache.
// For testing https://github.com/moby/buildkit/issues/4746
ensurePruneAll(t, c, sb)
_, err = f.Solve(ctx, c, solveOpt, nil)
require.NoError(t, err)
descAfterPrune, _, _ := readImage(t, ctx, target)
require.Equal(t, desc.Digest.String(), descAfterPrune.Digest.String())

// Build again, but without rewrite-timestamp
solveOpt2 := solveOpt
delete(solveOpt2.Exports[0].Attrs, "rewrite-timestamp")
_, err = f.Solve(ctx, c, solveOpt2, nil)
require.NoError(t, err)
_, 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"])
}
})
}
}

Expand Down Expand Up @@ -7141,6 +7173,14 @@ func readImage(t *testing.T, ctx context.Context, ref string) (ocispecs.Descript
require.NoError(t, json.Unmarshal(dt, &manifest))
imgDt, err := content.ReadBlob(ctx, provider, manifest.Config)
require.NoError(t, err)
// Verify that all the layer blobs are present
for _, layer := range manifest.Layers {
layerRA, err := provider.ReaderAt(ctx, layer)
require.NoError(t, err)
layerDigest, err := layer.Digest.Algorithm().FromReader(content.NewReader(layerRA))
require.NoError(t, err)
require.Equal(t, layer.Digest, layerDigest)
}
var img ocispecs.Image
require.NoError(t, json.Unmarshal(imgDt, &img))
return desc, manifest, img
Expand Down