Merge pull request moby#23322 from tkopczynski/20784-builder-dockerfile-symlink

vdemeester · web-flow · commit e08aed59dfd1 · 2016-06-15T10:52:49.000+02:00
Reimplement integration test for symlink Dockerfile as a unit test
diff --git a/builder/dockerfile/evaluator_test.go b/builder/dockerfile/evaluator_test.go
@@ -2,8 +2,6 @@ package dockerfile
 
 import (
 	"io/ioutil"
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 
@@ -195,35 +193,3 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) {
 	}
 
 }
-
-// createTestTempDir creates a temporary directory for testing.
-// It returns the created path and a cleanup function which is meant to be used as deferred call.
-// When an error occurs, it terminates the test.
-func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) {
-	path, err := ioutil.TempDir(dir, prefix)
-
-	if err != nil {
-		t.Fatalf("Error when creating directory %s with prefix %s: %s", dir, prefix, err)
-	}
-
-	return path, func() {
-		err = os.RemoveAll(path)
-
-		if err != nil {
-			t.Fatalf("Error when removing directory %s: %s", path, err)
-		}
-	}
-}
-
-// createTestTempFile creates a temporary file within dir with specific contents and permissions.
-// When an error occurs, it terminates the test
-func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.FileMode) string {
-	filePath := filepath.Join(dir, filename)
-	err := ioutil.WriteFile(filePath, []byte(contents), perm)
-
-	if err != nil {
-		t.Fatalf("Error when creating %s file: %s", filename, err)
-	}
-
-	return filePath
-}
diff --git a/builder/dockerfile/internals_test.go b/builder/dockerfile/internals_test.go
@@ -1,6 +1,7 @@
 package dockerfile
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -15,6 +16,25 @@ func TestEmptyDockerfile(t *testing.T) {
 
 	createTestTempFile(t, contextDir, builder.DefaultDockerfileName, "", 0777)
 
+	readAndCheckDockerfile(t, "emptyDockefile", contextDir, "", "The Dockerfile (Dockerfile) cannot be empty")
+}
+
+func TestSymlinkDockerfile(t *testing.T) {
+	contextDir, cleanup := createTestTempDir(t, "", "builder-dockerfile-test")
+	defer cleanup()
+
+	createTestSymlink(t, contextDir, builder.DefaultDockerfileName, "/etc/passwd")
+
+	// The reason the error is "Cannot locate specified Dockerfile" is because
+	// in the builder, the symlink is resolved within the context, therefore
+	// Dockerfile -> /etc/passwd becomes etc/passwd from the context which is
+	// a nonexistent file.
+	expectedError := fmt.Sprintf("Cannot locate specified Dockerfile: %s", builder.DefaultDockerfileName)
+
+	readAndCheckDockerfile(t, "symlinkDockerfile", contextDir, builder.DefaultDockerfileName, expectedError)
+}
+
+func readAndCheckDockerfile(t *testing.T, testName, contextDir, dockerfilePath, expectedError string) {
 	tarStream, err := archive.Tar(contextDir, archive.Uncompressed)
 
 	if err != nil {
@@ -39,18 +59,20 @@ func TestEmptyDockerfile(t *testing.T) {
 		}
 	}()
 
-	options := &types.ImageBuildOptions{}
+	options := &types.ImageBuildOptions{
+		Dockerfile: dockerfilePath,
+	}
 
 	b := &Builder{options: options, context: context}
 
 	err = b.readDockerfile()
 
 	if err == nil {
-		t.Fatalf("No error when executing test for empty Dockerfile")
+		t.Fatalf("No error when executing test: %s", testName)
 	}
 
-	if !strings.Contains(err.Error(), "The Dockerfile (Dockerfile) cannot be empty") {
-		t.Fatalf("Wrong error message. Should be \"%s\". Got \"%s\"", "The Dockerfile (Dockerfile) cannot be empty", err.Error())
+	if !strings.Contains(err.Error(), expectedError) {
+		t.Fatalf("Wrong error message. Should be \"%s\". Got \"%s\"", expectedError, err.Error())
 	}
 }
 
diff --git a/builder/dockerfile/utils_test.go b/builder/dockerfile/utils_test.go
@@ -0,0 +1,50 @@
+package dockerfile
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// createTestTempDir creates a temporary directory for testing.
+// It returns the created path and a cleanup function which is meant to be used as deferred call.
+// When an error occurs, it terminates the test.
+func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) {
+	path, err := ioutil.TempDir(dir, prefix)
+
+	if err != nil {
+		t.Fatalf("Error when creating directory %s with prefix %s: %s", dir, prefix, err)
+	}
+
+	return path, func() {
+		err = os.RemoveAll(path)
+
+		if err != nil {
+			t.Fatalf("Error when removing directory %s: %s", path, err)
+		}
+	}
+}
+
+// createTestTempFile creates a temporary file within dir with specific contents and permissions.
+// When an error occurs, it terminates the test
+func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.FileMode) string {
+	filePath := filepath.Join(dir, filename)
+	err := ioutil.WriteFile(filePath, []byte(contents), perm)
+
+	if err != nil {
+		t.Fatalf("Error when creating %s file: %s", filename, err)
+	}
+
+	return filePath
+}
+
+// createTestSymlink creates a symlink file within dir which points to oldname
+func createTestSymlink(t *testing.T, dir, filename, oldname string) string {
+	filePath := filepath.Join(dir, filename)
+	if err := os.Symlink(oldname, filePath); err != nil {
+		t.Fatalf("Error when creating %s symlink to %s: %s", filename, oldname, err)
+	}
+
+	return filePath
+}
diff --git a/integration-cli/docker_api_build_test.go b/integration-cli/docker_api_build_test.go
@@ -193,38 +193,6 @@ RUN echo from dockerfile`,
 	c.Assert(out, checker.Contains, "from Dockerfile")
 }
 
-func (s *DockerSuite) TestBuildApiDockerfileSymlink(c *check.C) {
-	// Test to make sure we stop people from trying to leave the
-	// build context when specifying a symlink as the path to the dockerfile
-	buffer := new(bytes.Buffer)
-	tw := tar.NewWriter(buffer)
-	defer tw.Close()
-
-	err := tw.WriteHeader(&tar.Header{
-		Name:     "Dockerfile",
-		Typeflag: tar.TypeSymlink,
-		Linkname: "/etc/passwd",
-	})
-	// failed to write tar file header
-	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.StatusInternalServerError)
-
-	out, err := readBody(body)
-	c.Assert(err, checker.IsNil)
-
-	// The reason the error is "Cannot locate specified Dockerfile" is because
-	// in the builder, the symlink is resolved within the context, therefore
-	// Dockerfile -> /etc/passwd becomes etc/passwd from the context which is
-	// a nonexistent file.
-	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.
