Browse code

Reimplementing more builder integration tests as unit tests

Signed-off-by: Tomasz Kopczynski <tomek@kopczynski.net.pl>

Tomasz Kopczynski authored on 2016/05/22 01:33:16
Showing 4 changed files
... ...
@@ -3,6 +3,7 @@ package dockerfile
3 3
 import (
4 4
 	"io/ioutil"
5 5
 	"os"
6
+	"path/filepath"
6 7
 	"strings"
7 8
 	"testing"
8 9
 
... ...
@@ -16,6 +17,7 @@ import (
16 16
 
17 17
 type dispatchTestCase struct {
18 18
 	name, dockerfile, expectedError string
19
+	files                           map[string]string
19 20
 }
20 21
 
21 22
 func init() {
... ...
@@ -34,21 +36,97 @@ func initDispatchTestCases() []dispatchTestCase {
34 34
 			name:          "ONBUILD forbidden FROM",
35 35
 			dockerfile:    "ONBUILD FROM scratch",
36 36
 			expectedError: "FROM isn't allowed as an ONBUILD trigger",
37
+			files:         nil,
37 38
 		},
38 39
 		{
39 40
 			name:          "ONBUILD forbidden MAINTAINER",
40 41
 			dockerfile:    "ONBUILD MAINTAINER docker.io",
41 42
 			expectedError: "MAINTAINER isn't allowed as an ONBUILD trigger",
43
+			files:         nil,
42 44
 		},
43 45
 		{
44 46
 			name:          "ARG two arguments",
45 47
 			dockerfile:    "ARG foo bar",
46 48
 			expectedError: "ARG requires exactly one argument definition",
49
+			files:         nil,
47 50
 		},
48 51
 		{
49 52
 			name:          "MAINTAINER unknown flag",
50 53
 			dockerfile:    "MAINTAINER --boo joe@example.com",
51 54
 			expectedError: "Unknown flag: boo",
55
+			files:         nil,
56
+		},
57
+		{
58
+			name:          "ADD multiple files to file",
59
+			dockerfile:    "ADD file1.txt file2.txt test",
60
+			expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /",
61
+			files:         map[string]string{"file1.txt": "test1", "file2.txt": "test2"},
62
+		},
63
+		{
64
+			name:          "JSON ADD multiple files to file",
65
+			dockerfile:    `ADD ["file1.txt", "file2.txt", "test"]`,
66
+			expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /",
67
+			files:         map[string]string{"file1.txt": "test1", "file2.txt": "test2"},
68
+		},
69
+		{
70
+			name:          "Wiildcard ADD multiple files to file",
71
+			dockerfile:    "ADD file*.txt test",
72
+			expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /",
73
+			files:         map[string]string{"file1.txt": "test1", "file2.txt": "test2"},
74
+		},
75
+		{
76
+			name:          "Wiildcard JSON ADD multiple files to file",
77
+			dockerfile:    `ADD ["file*.txt", "test"]`,
78
+			expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /",
79
+			files:         map[string]string{"file1.txt": "test1", "file2.txt": "test2"},
80
+		},
81
+		{
82
+			name:          "COPY multiple files to file",
83
+			dockerfile:    "COPY file1.txt file2.txt test",
84
+			expectedError: "When using COPY with more than one source file, the destination must be a directory and end with a /",
85
+			files:         map[string]string{"file1.txt": "test1", "file2.txt": "test2"},
86
+		},
87
+		{
88
+			name:          "JSON COPY multiple files to file",
89
+			dockerfile:    `COPY ["file1.txt", "file2.txt", "test"]`,
90
+			expectedError: "When using COPY with more than one source file, the destination must be a directory and end with a /",
91
+			files:         map[string]string{"file1.txt": "test1", "file2.txt": "test2"},
92
+		},
93
+		{
94
+			name:          "ADD multiple files to file with whitespace",
95
+			dockerfile:    `ADD [ "test file1.txt", "test file2.txt", "test" ]`,
96
+			expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /",
97
+			files:         map[string]string{"test file1.txt": "test1", "test file2.txt": "test2"},
98
+		},
99
+		{
100
+			name:          "COPY multiple files to file with whitespace",
101
+			dockerfile:    `COPY [ "test file1.txt", "test file2.txt", "test" ]`,
102
+			expectedError: "When using COPY with more than one source file, the destination must be a directory and end with a /",
103
+			files:         map[string]string{"test file1.txt": "test1", "test file2.txt": "test2"},
104
+		},
105
+		{
106
+			name:          "COPY wildcard no files",
107
+			dockerfile:    `COPY file*.txt /tmp/`,
108
+			expectedError: "No source files were specified",
109
+			files:         nil,
110
+		},
111
+		{
112
+			name:          "COPY url",
113
+			dockerfile:    `COPY https://index.docker.io/robots.txt /`,
114
+			expectedError: "Source can't be a URL for COPY",
115
+			files:         nil,
116
+		},
117
+		{
118
+			name:          "Chaining ONBUILD",
119
+			dockerfile:    `ONBUILD ONBUILD RUN touch foobar`,
120
+			expectedError: "Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed",
121
+			files:         nil,
122
+		},
123
+		{
124
+			name:          "Invalid instruction",
125
+			dockerfile:    `foo bar`,
126
+			expectedError: "Unknown instruction: FOO",
127
+			files:         nil,
52 128
 		}}
53 129
 
54 130
 	return dispatchTestCases
... ...
@@ -66,6 +144,10 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) {
66 66
 	contextDir, cleanup := createTestTempDir(t, "", "builder-dockerfile-test")
67 67
 	defer cleanup()
68 68
 
69
+	for filename, content := range testCase.files {
70
+		createTestTempFile(t, contextDir, filename, content, 0777)
71
+	}
72
+
69 73
 	tarStream, err := archive.Tar(contextDir, archive.Uncompressed)
70 74
 
71 75
 	if err != nil {
... ...
@@ -132,3 +214,16 @@ func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) {
132 132
 		}
133 133
 	}
134 134
 }
135
+
136
+// createTestTempFile creates a temporary file within dir with specific contents and permissions.
137
+// When an error occurs, it terminates the test
138
+func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.FileMode) string {
139
+	filePath := filepath.Join(dir, filename)
140
+	err := ioutil.WriteFile(filePath, []byte(contents), perm)
141
+
142
+	if err != nil {
143
+		t.Fatalf("Error when creating %s file: %s", filename, err)
144
+	}
145
+
146
+	return filePath
147
+}
... ...
@@ -618,6 +618,27 @@ func (b *Builder) readDockerfile() error {
618 618
 		}
619 619
 	}
620 620
 
621
+	err := b.parseDockerfile()
622
+
623
+	if err != nil {
624
+		return err
625
+	}
626
+
627
+	// After the Dockerfile has been parsed, we need to check the .dockerignore
628
+	// file for either "Dockerfile" or ".dockerignore", and if either are
629
+	// present then erase them from the build context. These files should never
630
+	// have been sent from the client but we did send them to make sure that
631
+	// we had the Dockerfile to actually parse, and then we also need the
632
+	// .dockerignore file to know whether either file should be removed.
633
+	// Note that this assumes the Dockerfile has been read into memory and
634
+	// is now safe to be removed.
635
+	if dockerIgnore, ok := b.context.(builder.DockerIgnoreContext); ok {
636
+		dockerIgnore.Process([]string{b.options.Dockerfile})
637
+	}
638
+	return nil
639
+}
640
+
641
+func (b *Builder) parseDockerfile() error {
621 642
 	f, err := b.context.Open(b.options.Dockerfile)
622 643
 	if err != nil {
623 644
 		if os.IsNotExist(err) {
... ...
@@ -625,6 +646,7 @@ func (b *Builder) readDockerfile() error {
625 625
 		}
626 626
 		return err
627 627
 	}
628
+	defer f.Close()
628 629
 	if f, ok := f.(*os.File); ok {
629 630
 		// ignoring error because Open already succeeded
630 631
 		fi, err := f.Stat()
... ...
@@ -636,22 +658,10 @@ func (b *Builder) readDockerfile() error {
636 636
 		}
637 637
 	}
638 638
 	b.dockerfile, err = parser.Parse(f)
639
-	f.Close()
640 639
 	if err != nil {
641 640
 		return err
642 641
 	}
643 642
 
644
-	// After the Dockerfile has been parsed, we need to check the .dockerignore
645
-	// file for either "Dockerfile" or ".dockerignore", and if either are
646
-	// present then erase them from the build context. These files should never
647
-	// have been sent from the client but we did send them to make sure that
648
-	// we had the Dockerfile to actually parse, and then we also need the
649
-	// .dockerignore file to know whether either file should be removed.
650
-	// Note that this assumes the Dockerfile has been read into memory and
651
-	// is now safe to be removed.
652
-	if dockerIgnore, ok := b.context.(builder.DockerIgnoreContext); ok {
653
-		dockerIgnore.Process([]string{b.options.Dockerfile})
654
-	}
655 643
 	return nil
656 644
 }
657 645
 
658 646
new file mode 100644
... ...
@@ -0,0 +1,55 @@
0
+package dockerfile
1
+
2
+import (
3
+	"strings"
4
+	"testing"
5
+
6
+	"github.com/docker/docker/builder"
7
+	"github.com/docker/docker/pkg/archive"
8
+	"github.com/docker/engine-api/types"
9
+)
10
+
11
+func TestEmptyDockerfile(t *testing.T) {
12
+	contextDir, cleanup := createTestTempDir(t, "", "builder-dockerfile-test")
13
+	defer cleanup()
14
+
15
+	createTestTempFile(t, contextDir, builder.DefaultDockerfileName, "", 0777)
16
+
17
+	tarStream, err := archive.Tar(contextDir, archive.Uncompressed)
18
+
19
+	if err != nil {
20
+		t.Fatalf("Error when creating tar stream: %s", err)
21
+	}
22
+
23
+	defer func() {
24
+		if err = tarStream.Close(); err != nil {
25
+			t.Fatalf("Error when closing tar stream: %s", err)
26
+		}
27
+	}()
28
+
29
+	context, err := builder.MakeTarSumContext(tarStream)
30
+
31
+	if err != nil {
32
+		t.Fatalf("Error when creating tar context: %s", err)
33
+	}
34
+
35
+	defer func() {
36
+		if err = context.Close(); err != nil {
37
+			t.Fatalf("Error when closing tar context: %s", err)
38
+		}
39
+	}()
40
+
41
+	options := &types.ImageBuildOptions{}
42
+
43
+	b := &Builder{options: options, context: context}
44
+
45
+	err = b.readDockerfile()
46
+
47
+	if err == nil {
48
+		t.Fatalf("No error when executing test for empty Dockerfile")
49
+	}
50
+
51
+	if !strings.Contains(err.Error(), "The Dockerfile (Dockerfile) cannot be empty") {
52
+		t.Fatalf("Wrong error message. Should be \"%s\". Got \"%s\"", "The Dockerfile (Dockerfile) cannot be empty", err.Error())
53
+	}
54
+}
... ...
@@ -862,138 +862,6 @@ RUN [ $(ls -l / | grep new_dir | awk '{print $3":"$4}') = 'root:root' ]`, true);
862 862
 	}
863 863
 }
864 864
 
865
-func (s *DockerSuite) TestBuildAddMultipleFilesToFile(c *check.C) {
866
-	name := "testaddmultiplefilestofile"
867
-
868
-	ctx, err := fakeContext(`FROM `+minimalBaseImage()+`
869
-	ADD file1.txt file2.txt test
870
-	`,
871
-		map[string]string{
872
-			"file1.txt": "test1",
873
-			"file2.txt": "test1",
874
-		})
875
-	if err != nil {
876
-		c.Fatal(err)
877
-	}
878
-	defer ctx.Close()
879
-
880
-	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
881
-	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
882
-		c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
883
-	}
884
-
885
-}
886
-
887
-func (s *DockerSuite) TestBuildJSONAddMultipleFilesToFile(c *check.C) {
888
-	name := "testjsonaddmultiplefilestofile"
889
-
890
-	ctx, err := fakeContext(`FROM `+minimalBaseImage()+`
891
-	ADD ["file1.txt", "file2.txt", "test"]
892
-	`,
893
-		map[string]string{
894
-			"file1.txt": "test1",
895
-			"file2.txt": "test1",
896
-		})
897
-	if err != nil {
898
-		c.Fatal(err)
899
-	}
900
-	defer ctx.Close()
901
-
902
-	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
903
-	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
904
-		c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
905
-	}
906
-
907
-}
908
-
909
-func (s *DockerSuite) TestBuildAddMultipleFilesToFileWild(c *check.C) {
910
-	name := "testaddmultiplefilestofilewild"
911
-
912
-	ctx, err := fakeContext(`FROM `+minimalBaseImage()+`
913
-	ADD file*.txt test
914
-	`,
915
-		map[string]string{
916
-			"file1.txt": "test1",
917
-			"file2.txt": "test1",
918
-		})
919
-	if err != nil {
920
-		c.Fatal(err)
921
-	}
922
-	defer ctx.Close()
923
-
924
-	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
925
-	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
926
-		c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
927
-	}
928
-
929
-}
930
-
931
-func (s *DockerSuite) TestBuildJSONAddMultipleFilesToFileWild(c *check.C) {
932
-	name := "testjsonaddmultiplefilestofilewild"
933
-
934
-	ctx, err := fakeContext(`FROM `+minimalBaseImage()+`
935
-	ADD ["file*.txt", "test"]
936
-	`,
937
-		map[string]string{
938
-			"file1.txt": "test1",
939
-			"file2.txt": "test1",
940
-		})
941
-	if err != nil {
942
-		c.Fatal(err)
943
-	}
944
-	defer ctx.Close()
945
-
946
-	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
947
-	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
948
-		c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
949
-	}
950
-
951
-}
952
-
953
-func (s *DockerSuite) TestBuildCopyMultipleFilesToFile(c *check.C) {
954
-	name := "testcopymultiplefilestofile"
955
-
956
-	ctx, err := fakeContext(`FROM `+minimalBaseImage()+`
957
-	COPY file1.txt file2.txt test
958
-	`,
959
-		map[string]string{
960
-			"file1.txt": "test1",
961
-			"file2.txt": "test1",
962
-		})
963
-	if err != nil {
964
-		c.Fatal(err)
965
-	}
966
-	defer ctx.Close()
967
-
968
-	expected := "When using COPY with more than one source file, the destination must be a directory and end with a /"
969
-	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
970
-		c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
971
-	}
972
-
973
-}
974
-
975
-func (s *DockerSuite) TestBuildJSONCopyMultipleFilesToFile(c *check.C) {
976
-	name := "testjsoncopymultiplefilestofile"
977
-
978
-	ctx, err := fakeContext(`FROM `+minimalBaseImage()+`
979
-	COPY ["file1.txt", "file2.txt", "test"]
980
-	`,
981
-		map[string]string{
982
-			"file1.txt": "test1",
983
-			"file2.txt": "test1",
984
-		})
985
-	if err != nil {
986
-		c.Fatal(err)
987
-	}
988
-	defer ctx.Close()
989
-
990
-	expected := "When using COPY with more than one source file, the destination must be a directory and end with a /"
991
-	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
992
-		c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
993
-	}
994
-
995
-}
996
-
997 865
 func (s *DockerSuite) TestBuildAddFileWithWhitespace(c *check.C) {
998 866
 	testRequires(c, DaemonIsLinux) // Not currently passing on Windows
999 867
 	name := "testaddfilewithwhitespace"
... ...
@@ -1066,48 +934,6 @@ RUN [ $(cat "/test dir/test_file6") = 'test6' ]`,
1066 1066
 	}
1067 1067
 }
1068 1068
 
1069
-func (s *DockerSuite) TestBuildAddMultipleFilesToFileWithWhitespace(c *check.C) {
1070
-	name := "testaddmultiplefilestofilewithwhitespace"
1071
-	ctx, err := fakeContext(`FROM busybox
1072
-	ADD [ "test file1", "test file2", "test" ]
1073
-    `,
1074
-		map[string]string{
1075
-			"test file1": "test1",
1076
-			"test file2": "test2",
1077
-		})
1078
-	if err != nil {
1079
-		c.Fatal(err)
1080
-	}
1081
-	defer ctx.Close()
1082
-
1083
-	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
1084
-	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
1085
-		c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
1086
-	}
1087
-
1088
-}
1089
-
1090
-func (s *DockerSuite) TestBuildCopyMultipleFilesToFileWithWhitespace(c *check.C) {
1091
-	name := "testcopymultiplefilestofilewithwhitespace"
1092
-	ctx, err := fakeContext(`FROM busybox
1093
-	COPY [ "test file1", "test file2", "test" ]
1094
-        `,
1095
-		map[string]string{
1096
-			"test file1": "test1",
1097
-			"test file2": "test2",
1098
-		})
1099
-	if err != nil {
1100
-		c.Fatal(err)
1101
-	}
1102
-	defer ctx.Close()
1103
-
1104
-	expected := "When using COPY with more than one source file, the destination must be a directory and end with a /"
1105
-	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
1106
-		c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
1107
-	}
1108
-
1109
-}
1110
-
1111 1069
 func (s *DockerSuite) TestBuildCopyWildcard(c *check.C) {
1112 1070
 	testRequires(c, DaemonIsLinux) // Windows doesn't have httpserver image yet
1113 1071
 	name := "testcopywildcard"
... ...
@@ -1159,26 +985,6 @@ func (s *DockerSuite) TestBuildCopyWildcard(c *check.C) {
1159 1159
 
1160 1160
 }
1161 1161
 
1162
-func (s *DockerSuite) TestBuildCopyWildcardNoFind(c *check.C) {
1163
-	name := "testcopywildcardnofind"
1164
-	ctx, err := fakeContext(`FROM busybox
1165
-	COPY file*.txt /tmp/
1166
-	`, nil)
1167
-	if err != nil {
1168
-		c.Fatal(err)
1169
-	}
1170
-	defer ctx.Close()
1171
-
1172
-	_, err = buildImageFromContext(name, ctx, true)
1173
-	if err == nil {
1174
-		c.Fatal("should have failed to find a file")
1175
-	}
1176
-	if !strings.Contains(err.Error(), "No source files were specified") {
1177
-		c.Fatalf("Wrong error %v, must be about no source files", err)
1178
-	}
1179
-
1180
-}
1181
-
1182 1162
 func (s *DockerSuite) TestBuildCopyWildcardInName(c *check.C) {
1183 1163
 	name := "testcopywildcardinname"
1184 1164
 	ctx, err := fakeContext(`FROM busybox
... ...
@@ -1580,17 +1386,6 @@ COPY . /`,
1580 1580
 	}
1581 1581
 }
1582 1582
 
1583
-func (s *DockerSuite) TestBuildCopyDisallowRemote(c *check.C) {
1584
-	name := "testcopydisallowremote"
1585
-
1586
-	_, out, err := buildImageWithOut(name, `FROM `+minimalBaseImage()+`
1587
-COPY https://index.docker.io/robots.txt /`,
1588
-		true)
1589
-	if err == nil || !strings.Contains(out, "Source can't be a URL for COPY") {
1590
-		c.Fatalf("Error should be about disallowed remote source, got err: %s, out: %q", err, out)
1591
-	}
1592
-}
1593
-
1594 1583
 func (s *DockerSuite) TestBuildAddBadLinks(c *check.C) {
1595 1584
 	testRequires(c, DaemonIsLinux) // Not currently working on Windows
1596 1585
 
... ...
@@ -3289,18 +3084,6 @@ func (s *DockerSuite) TestBuildFails(c *check.C) {
3289 3289
 	}
3290 3290
 }
3291 3291
 
3292
-func (s *DockerSuite) TestBuildFailsDockerfileEmpty(c *check.C) {
3293
-	name := "testbuildfails"
3294
-	_, err := buildImage(name, ``, true)
3295
-	if err != nil {
3296
-		if !strings.Contains(err.Error(), "The Dockerfile (Dockerfile) cannot be empty") {
3297
-			c.Fatalf("Wrong error %v, must be about empty Dockerfile", err)
3298
-		}
3299
-	} else {
3300
-		c.Fatal("Error must not be nil")
3301
-	}
3302
-}
3303
-
3304 3292
 func (s *DockerSuite) TestBuildOnBuild(c *check.C) {
3305 3293
 	name := "testbuildonbuild"
3306 3294
 	_, err := buildImage(name,
... ...
@@ -3319,21 +3102,6 @@ func (s *DockerSuite) TestBuildOnBuild(c *check.C) {
3319 3319
 	}
3320 3320
 }
3321 3321
 
3322
-func (s *DockerSuite) TestBuildOnBuildForbiddenChained(c *check.C) {
3323
-	name := "testbuildonbuildforbiddenchained"
3324
-	_, err := buildImage(name,
3325
-		`FROM busybox
3326
-		ONBUILD ONBUILD RUN touch foobar`,
3327
-		true)
3328
-	if err != nil {
3329
-		if !strings.Contains(err.Error(), "Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed") {
3330
-			c.Fatalf("Wrong error %v, must be about chaining ONBUILD", err)
3331
-		}
3332
-	} else {
3333
-		c.Fatal("Error must not be nil")
3334
-	}
3335
-}
3336
-
3337 3322
 // gh #2446
3338 3323
 func (s *DockerSuite) TestBuildAddToSymlinkDest(c *check.C) {
3339 3324
 	testRequires(c, DaemonIsLinux)
... ...
@@ -4564,16 +4332,6 @@ func (s *DockerSuite) TestBuildCmdJSONNoShDashC(c *check.C) {
4564 4564
 
4565 4565
 }
4566 4566
 
4567
-func (s *DockerSuite) TestBuildErrorInvalidInstruction(c *check.C) {
4568
-	name := "testbuildignoreinvalidinstruction"
4569
-
4570
-	out, _, err := buildImageWithOut(name, "FROM busybox\nfoo bar", true)
4571
-	if err == nil {
4572
-		c.Fatalf("Should have failed: %s", out)
4573
-	}
4574
-
4575
-}
4576
-
4577 4567
 func (s *DockerSuite) TestBuildEntrypointInheritance(c *check.C) {
4578 4568
 
4579 4569
 	if _, err := buildImage("parent", `