Skip to content

Commit

Permalink
contenthash: include basename in content checksum for wildcards
Browse files Browse the repository at this point in the history
While we generally ignore the basename in this layer, for wildcards
there in no other place to add the basename to the checksum as they
can not be resolved earlier. Before the basename that was in the
checksum was the wildcard itself, so if the wildcard remained same,
content remained same but the file where wildcard pointed to was
renamed, the cache was not invalidated.

Unfortunately, this change breaks cache for all copy commands that
use a wildcard.

Signed-off-by: Tonis Tiigi <[email protected]>
(cherry picked from commit 1982e1e)
  • Loading branch information
tonistiigi committed Apr 22, 2021
1 parent 56a99f4 commit a1094c6
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 13 deletions.
20 changes: 11 additions & 9 deletions cache/contenthash/checksum.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 6 additions & 4 deletions cache/contenthash/checksum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,25 +179,27 @@ 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)
require.Equal(t, expFoos, dgst)

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)
Expand Down
47 changes: 47 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ var allTests = []integration.Test{
testDockefileCheckHostname,
testDefaultShellAndPath,
testDockerfileLowercase,
testWildcardRenameCache,
}

var fileOpTests = []integration.Test{
Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit a1094c6

Please sign in to comment.