diff --git a/cache/contenthash/checksum.go b/cache/contenthash/checksum.go index a8335eaa9c0a..ac9d3ec36a27 100644 --- a/cache/contenthash/checksum.go +++ b/cache/contenthash/checksum.go @@ -406,17 +406,19 @@ func (cc *cacheContext) ChecksumWildcard(ctx context.Context, mountable cache.Mo return digest.FromBytes([]byte{}), nil } - if len(wildcards) > 1 { - digester := digest.Canonical.Digester() - for i, w := range wildcards { - if i != 0 { - digester.Hash().Write([]byte{0}) - } - digester.Hash().Write([]byte(w.Record.Digest)) + if len(wildcards) == 1 && path.Base(p) == path.Base(wildcards[0].Path) { + return wildcards[0].Record.Digest, nil + } + + digester := digest.Canonical.Digester() + for i, w := range wildcards { + if i != 0 { + digester.Hash().Write([]byte{0}) } - return digester.Digest(), nil + digester.Hash().Write([]byte(path.Base(w.Path))) + digester.Hash().Write([]byte(w.Record.Digest)) } - return wildcards[0].Record.Digest, nil + return digester.Digest(), nil } func (cc *cacheContext) Checksum(ctx context.Context, mountable cache.Mountable, p string, followLinks bool, s session.Group) (digest.Digest, error) { diff --git a/cache/contenthash/checksum_test.go b/cache/contenthash/checksum_test.go index a75f507659ac..0c377707ee26 100644 --- a/cache/contenthash/checksum_test.go +++ b/cache/contenthash/checksum_test.go @@ -179,9 +179,9 @@ func TestChecksumWildcard(t *testing.T) { dgst, err := cc.ChecksumWildcard(context.TODO(), ref, "f*o", false, nil) require.NoError(t, err) - require.Equal(t, dgstFileData0, dgst) + require.Equal(t, digest.FromBytes(append([]byte("foo"), []byte(dgstFileData0)...)), dgst) - expFoos := digest.Digest("sha256:c9f914ad7ad8fe6092ce67484b43ca39c2087aabf9e4a1b223249b0f8b09b9f2") + expFoos := digest.Digest("sha256:7f51c821895cfc116d3f64231dfb438e87a237ecbbe027cd96b7ee5e763cc569") dgst, err = cc.ChecksumWildcard(context.TODO(), ref, "f*", false, nil) require.NoError(t, err) @@ -189,15 +189,17 @@ func TestChecksumWildcard(t *testing.T) { dgst, err = cc.ChecksumWildcard(context.TODO(), ref, "x/d?", false, nil) require.NoError(t, err) - require.Equal(t, dgstDirD0, dgst) + require.Equal(t, digest.FromBytes(append([]byte("d0"), []byte(dgstDirD0)...)), dgst) dgst, err = cc.ChecksumWildcard(context.TODO(), ref, "x/d?/def", true, nil) require.NoError(t, err) require.Equal(t, dgstFileData0, dgst) + expFoos2 := digest.Digest("sha256:8afc09c7018d65d5eb318a9ef55cb704dec1f06d288181d913fc27a571aa042d") + dgst, err = cc.ChecksumWildcard(context.TODO(), ref, "y*", true, nil) require.NoError(t, err) - require.Equal(t, expFoos, dgst) + require.Equal(t, expFoos2, dgst) err = ref.Release(context.TODO()) require.NoError(t, err) diff --git a/client/client_test.go b/client/client_test.go index e7807d08a9bd..60c5aacf9bb6 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -98,6 +98,7 @@ func TestIntegration(t *testing.T) { testSharedCacheMounts, testLockedCacheMounts, testDuplicateCacheMount, + testRunCacheWithMounts, testParallelLocalBuilds, testSecretMounts, testExtraHosts, @@ -2759,6 +2760,33 @@ func testDuplicateCacheMount(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) } +func testRunCacheWithMounts(t *testing.T, sb integration.Sandbox) { + requiresLinux(t) + c, err := New(context.TODO(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + busybox := llb.Image("busybox:latest") + + out := busybox.Run(llb.Shlex(`sh -e -c "[[ -f /m1/sbin/apk ]]"`)) + out.AddMount("/m1", llb.Image("alpine:latest"), llb.Readonly) + + def, err := out.Marshal(context.TODO()) + require.NoError(t, err) + + _, err = c.Solve(context.TODO(), def, SolveOpt{}, nil) + require.NoError(t, err) + + out = busybox.Run(llb.Shlex(`sh -e -c "[[ -f /m1/sbin/apk ]]"`)) + out.AddMount("/m1", llb.Image("busybox:latest"), llb.Readonly) + + def, err = out.Marshal(context.TODO()) + require.NoError(t, err) + + _, err = c.Solve(context.TODO(), def, SolveOpt{}, nil) + require.Error(t, err) +} + func testCacheMountNoCache(t *testing.T, sb integration.Sandbox) { requiresLinux(t) c, err := New(context.TODO(), sb.Address()) diff --git a/codecov.yml b/codecov.yml index f8111de9a427..48a7cfb621ce 100644 --- a/codecov.yml +++ b/codecov.yml @@ -6,3 +6,4 @@ coverage: default: target: auto # auto % coverage target threshold: 1% # allow for 1% reduction of coverage without failing + patch: off diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 326cef5e55c6..882679f9376f 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -108,6 +108,7 @@ var allTests = []integration.Test{ testDockefileCheckHostname, testDefaultShellAndPath, testDockerfileLowercase, + testWildcardRenameCache, } var fileOpTests = []integration.Test{ @@ -3752,6 +3753,52 @@ LABEL foo=bar require.Equal(t, "baz", v) } +// #2008 +func testWildcardRenameCache(t *testing.T, sb integration.Sandbox) { + skipDockerd(t, sb) + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM alpine +COPY file* /files/ +RUN ls /files/file1 +`) + dir, err := tmpdir( + fstest.CreateFile("Dockerfile", dockerfile, 0600), + fstest.CreateFile("file1", []byte("foo"), 0600), + ) + require.NoError(t, err) + defer os.RemoveAll(dir) + + c, err := client.New(context.TODO(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + destDir, err := ioutil.TempDir("", "buildkit") + require.NoError(t, err) + defer os.RemoveAll(destDir) + + _, err = f.Solve(context.TODO(), c, client.SolveOpt{ + LocalDirs: map[string]string{ + builder.DefaultLocalNameDockerfile: dir, + builder.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + err = os.Rename(filepath.Join(dir, "file1"), filepath.Join(dir, "file2")) + require.NoError(t, err) + + // cache should be invalidated and build should fail + _, err = f.Solve(context.TODO(), c, client.SolveOpt{ + LocalDirs: map[string]string{ + builder.DefaultLocalNameDockerfile: dir, + builder.DefaultLocalNameContext: dir, + }, + }, nil) + require.Error(t, err) +} + func testOnBuildCleared(t *testing.T, sb integration.Sandbox) { f := getFrontend(t, sb) diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 24aa46aa9fb1..37d64ed0ef93 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -69,8 +69,8 @@ func cloneExecOp(old *pb.ExecOp) pb.ExecOp { } n.Meta = &meta n.Mounts = nil - for i := range n.Mounts { - m := *n.Mounts[i] + for i := range old.Mounts { + m := *old.Mounts[i] n.Mounts = append(n.Mounts, &m) } return n @@ -97,6 +97,22 @@ func (e *execOp) CacheMap(ctx context.Context, g session.Group, index int) (*sol } } + // Special case for cache compatibility with buggy versions that wrongly + // excluded Exec.Mounts: for the default case of one root mount (i.e. RUN + // inside a Dockerfile), do not include the mount when generating the cache + // map. + if len(op.Mounts) == 1 && + op.Mounts[0].Dest == "/" && + op.Mounts[0].Selector == "" && + !op.Mounts[0].Readonly && + op.Mounts[0].MountType == pb.MountType_BIND && + op.Mounts[0].CacheOpt == nil && + op.Mounts[0].SSHOpt == nil && + op.Mounts[0].SecretOpt == nil && + op.Mounts[0].ResultID == "" { + op.Mounts = nil + } + dt, err := json.Marshal(struct { Type string Exec *pb.ExecOp diff --git a/util/contentutil/copy.go b/util/contentutil/copy.go index 08c604730061..b471d8b94850 100644 --- a/util/contentutil/copy.go +++ b/util/contentutil/copy.go @@ -65,7 +65,7 @@ func CopyChain(ctx context.Context, ingester content.Ingester, provider content. handlers := []images.Handler{ images.ChildrenHandler(provider), filterHandler, - retryhandler.New(remotes.FetchHandler(ingester, &localFetcher{provider}), nil), + retryhandler.New(remotes.FetchHandler(ingester, &localFetcher{provider}), func(_ []byte) {}), } if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil { diff --git a/util/imageutil/config.go b/util/imageutil/config.go index c1ea0214523e..0be587058a6c 100644 --- a/util/imageutil/config.go +++ b/util/imageutil/config.go @@ -101,7 +101,7 @@ func Config(ctx context.Context, str string, resolver remotes.Resolver, cache Co children := childrenConfigHandler(cache, platform) handlers := []images.Handler{ - retryhandler.New(remotes.FetchHandler(cache, fetcher), nil), + retryhandler.New(remotes.FetchHandler(cache, fetcher), func(_ []byte) {}), children, } if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil { diff --git a/util/progress/logs/logs.go b/util/progress/logs/logs.go index da82c6923eba..43408b8a8c80 100644 --- a/util/progress/logs/logs.go +++ b/util/progress/logs/logs.go @@ -132,6 +132,7 @@ func (sw *streamWriter) Close() error { func LoggerFromContext(ctx context.Context) func([]byte) { return func(dt []byte) { pw, _, _ := progress.FromContext(ctx) + defer pw.Close() pw.Write(identity.NewID(), client.VertexLog{ Stream: stderr, Data: []byte(dt), diff --git a/util/resolver/retryhandler/retry.go b/util/resolver/retryhandler/retry.go index bddcfb8488cd..3d2b797778ac 100644 --- a/util/resolver/retryhandler/retry.go +++ b/util/resolver/retryhandler/retry.go @@ -9,6 +9,7 @@ import ( "time" "github.com/containerd/containerd/images" + remoteserrors "github.com/containerd/containerd/remotes/errors" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) @@ -47,6 +48,14 @@ func New(f images.HandlerFunc, logger func([]byte)) images.HandlerFunc { } func retryError(err error) bool { + // Retry on 5xx errors + var errUnexpectedStatus remoteserrors.ErrUnexpectedStatus + if errors.As(err, &errUnexpectedStatus) && + errUnexpectedStatus.StatusCode >= 500 && + errUnexpectedStatus.StatusCode <= 599 { + return true + } + if errors.Is(err, io.EOF) || errors.Is(err, syscall.ECONNRESET) || errors.Is(err, syscall.EPIPE) { return true }