فهرست منبع

Fix build cache false positives when build context tar contains unnormalized paths

If a build context tar has path names of the form 'x/./y', they will be
stored in this unnormalized form internally by tarsum. When the builder
walks the untarred directory tree and queries hashes for each relative
path, it will query paths of the form 'x/y', and they will not be found.

To correct this, have tarsum normalize path names by calling Clean.

Add a test to detect this caching false positive.

Fixes #21715

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Aaron Lehmann 9 سال پیش
والد
کامیت
8691a77e44
3فایلهای تغییر یافته به همراه62 افزوده شده و 2 حذف شده
  1. 1 1
      builder/tarsum.go
  2. 59 0
      integration-cli/docker_api_build_test.go
  3. 2 1
      pkg/tarsum/tarsum.go

+ 1 - 1
builder/tarsum.go

@@ -138,7 +138,7 @@ func (c *tarSumContext) Walk(root string, walkFn WalkFunc) error {
 		}
 		}
 
 
 		sum := rel
 		sum := rel
-		if tsInfo := c.sums.GetFile(rel); tsInfo != nil {
+		if tsInfo := c.sums.GetFile(filepath.ToSlash(rel)); tsInfo != nil {
 			sum = tsInfo.Sum()
 			sum = tsInfo.Sum()
 		}
 		}
 		fi := &HashedFileInfo{PathFileInfo{FileInfo: info, FilePath: fullpath}, sum}
 		fi := &HashedFileInfo{PathFileInfo{FileInfo: info, FilePath: fullpath}, sum}

+ 59 - 0
integration-cli/docker_api_build_test.go

@@ -4,6 +4,8 @@ import (
 	"archive/tar"
 	"archive/tar"
 	"bytes"
 	"bytes"
 	"net/http"
 	"net/http"
+	"regexp"
+	"strings"
 
 
 	"github.com/docker/docker/pkg/integration/checker"
 	"github.com/docker/docker/pkg/integration/checker"
 	"github.com/go-check/check"
 	"github.com/go-check/check"
@@ -255,3 +257,60 @@ func (s *DockerSuite) TestBuildApiDockerfileSymlink(c *check.C) {
 	// a nonexistent file.
 	// a nonexistent file.
 	c.Assert(string(out), checker.Contains, "Cannot locate specified Dockerfile: Dockerfile", check.Commentf("Didn't complain about leaving build context"))
 	c.Assert(string(out), checker.Contains, "Cannot locate specified Dockerfile: Dockerfile", check.Commentf("Didn't complain about leaving build context"))
 }
 }
+
+func (s *DockerSuite) TestBuildApiUnnormalizedTarPaths(c *check.C) {
+	// Make sure that build context tars with entries of the form
+	// x/./y don't cause caching false positives.
+
+	buildFromTarContext := func(fileContents []byte) string {
+		buffer := new(bytes.Buffer)
+		tw := tar.NewWriter(buffer)
+		defer tw.Close()
+
+		dockerfile := []byte(`FROM busybox
+	COPY dir /dir/`)
+		err := tw.WriteHeader(&tar.Header{
+			Name: "Dockerfile",
+			Size: int64(len(dockerfile)),
+		})
+		//failed to write tar file header
+		c.Assert(err, checker.IsNil)
+
+		_, err = tw.Write(dockerfile)
+		// failed to write Dockerfile in tar file content
+		c.Assert(err, checker.IsNil)
+
+		err = tw.WriteHeader(&tar.Header{
+			Name: "dir/./file",
+			Size: int64(len(fileContents)),
+		})
+		//failed to write tar file header
+		c.Assert(err, checker.IsNil)
+
+		_, err = tw.Write(fileContents)
+		// failed to write file contents in tar file content
+		c.Assert(err, checker.IsNil)
+
+		// failed to close tar archive
+		c.Assert(tw.Close(), checker.IsNil)
+
+		res, body, err := sockRequestRaw("POST", "/build", buffer, "application/x-tar")
+		c.Assert(err, checker.IsNil)
+		c.Assert(res.StatusCode, checker.Equals, http.StatusOK)
+
+		out, err := readBody(body)
+		c.Assert(err, checker.IsNil)
+		lines := strings.Split(string(out), "\n")
+		c.Assert(len(lines), checker.GreaterThan, 1)
+		c.Assert(lines[len(lines)-2], checker.Matches, ".*Successfully built [0-9a-f]{12}.*")
+
+		re := regexp.MustCompile("Successfully built ([0-9a-f]{12})")
+		matches := re.FindStringSubmatch(lines[len(lines)-2])
+		return matches[1]
+	}
+
+	imageA := buildFromTarContext([]byte("abc"))
+	imageB := buildFromTarContext([]byte("def"))
+
+	c.Assert(imageA, checker.Not(checker.Equals), imageB)
+}

+ 2 - 1
pkg/tarsum/tarsum.go

@@ -28,6 +28,7 @@ import (
 	"fmt"
 	"fmt"
 	"hash"
 	"hash"
 	"io"
 	"io"
+	"path"
 	"strings"
 	"strings"
 )
 )
 
 
@@ -235,7 +236,7 @@ func (ts *tarSum) Read(buf []byte) (int, error) {
 				}
 				}
 				return n, err
 				return n, err
 			}
 			}
-			ts.currentFile = strings.TrimSuffix(strings.TrimPrefix(currentHeader.Name, "./"), "/")
+			ts.currentFile = path.Clean(currentHeader.Name)
 			if err := ts.encodeHeader(currentHeader); err != nil {
 			if err := ts.encodeHeader(currentHeader); err != nil {
 				return 0, err
 				return 0, err
 			}
 			}