Browse code

Merge pull request #31352 from dnephin/allow-arg-in-fromt

[dockerfile] Allow ARG in FROM

Daniel Nephin authored on 2017/04/11 02:23:40
Showing 12 changed files
1 1
new file mode 100644
... ...
@@ -0,0 +1,124 @@
0
+package dockerfile
1
+
2
+// builtinAllowedBuildArgs is list of built-in allowed build args
3
+// these args are considered transparent and are excluded from the image history.
4
+// Filtering from history is implemented in dispatchers.go
5
+var builtinAllowedBuildArgs = map[string]bool{
6
+	"HTTP_PROXY":  true,
7
+	"http_proxy":  true,
8
+	"HTTPS_PROXY": true,
9
+	"https_proxy": true,
10
+	"FTP_PROXY":   true,
11
+	"ftp_proxy":   true,
12
+	"NO_PROXY":    true,
13
+	"no_proxy":    true,
14
+}
15
+
16
+// buildArgs manages arguments used by the builder
17
+type buildArgs struct {
18
+	// args that are allowed for expansion/substitution and passing to commands in 'run'.
19
+	allowedBuildArgs map[string]*string
20
+	// args defined before the first `FROM` in a Dockerfile
21
+	allowedMetaArgs map[string]*string
22
+	// args referenced by the Dockerfile
23
+	referencedArgs map[string]struct{}
24
+	// args provided by the user on the command line
25
+	argsFromOptions map[string]*string
26
+}
27
+
28
+func newBuildArgs(argsFromOptions map[string]*string) *buildArgs {
29
+	return &buildArgs{
30
+		allowedBuildArgs: make(map[string]*string),
31
+		allowedMetaArgs:  make(map[string]*string),
32
+		referencedArgs:   make(map[string]struct{}),
33
+		argsFromOptions:  argsFromOptions,
34
+	}
35
+}
36
+
37
+// UnreferencedOptionArgs returns the list of args that were set from options but
38
+// were never referenced from the Dockerfile
39
+func (b *buildArgs) UnreferencedOptionArgs() []string {
40
+	leftoverArgs := []string{}
41
+	for arg := range b.argsFromOptions {
42
+		if _, ok := b.referencedArgs[arg]; !ok {
43
+			leftoverArgs = append(leftoverArgs, arg)
44
+		}
45
+	}
46
+	return leftoverArgs
47
+}
48
+
49
+// ResetAllowed clears the list of args that are allowed to be used by a
50
+// directive
51
+func (b *buildArgs) ResetAllowed() {
52
+	b.allowedBuildArgs = make(map[string]*string)
53
+}
54
+
55
+// AddMetaArg adds a new meta arg that can be used by FROM directives
56
+func (b *buildArgs) AddMetaArg(key string, value *string) {
57
+	b.allowedMetaArgs[key] = value
58
+}
59
+
60
+// AddArg adds a new arg that can be used by directives
61
+func (b *buildArgs) AddArg(key string, value *string) {
62
+	b.allowedBuildArgs[key] = value
63
+	b.referencedArgs[key] = struct{}{}
64
+}
65
+
66
+// IsUnreferencedBuiltin checks if the key is a built-in arg, or if it has been
67
+// referenced by the Dockerfile. Returns true if the arg is a builtin that has
68
+// not been referenced in the Dockerfile.
69
+func (b *buildArgs) IsUnreferencedBuiltin(key string) bool {
70
+	_, isBuiltin := builtinAllowedBuildArgs[key]
71
+	_, isAllowed := b.allowedBuildArgs[key]
72
+	return isBuiltin && !isAllowed
73
+}
74
+
75
+// GetAllAllowed returns a mapping with all the allowed args
76
+func (b *buildArgs) GetAllAllowed() map[string]string {
77
+	return b.getAllFromMapping(b.allowedBuildArgs)
78
+}
79
+
80
+// GetAllMeta returns a mapping with all the meta meta args
81
+func (b *buildArgs) GetAllMeta() map[string]string {
82
+	return b.getAllFromMapping(b.allowedMetaArgs)
83
+}
84
+
85
+func (b *buildArgs) getAllFromMapping(source map[string]*string) map[string]string {
86
+	m := make(map[string]string)
87
+
88
+	keys := keysFromMaps(source, builtinAllowedBuildArgs)
89
+	for _, key := range keys {
90
+		v, ok := b.getBuildArg(key, source)
91
+		if ok {
92
+			m[key] = v
93
+		}
94
+	}
95
+	return m
96
+}
97
+
98
+func (b *buildArgs) getBuildArg(key string, mapping map[string]*string) (string, bool) {
99
+	defaultValue, exists := mapping[key]
100
+	// Return override from options if one is defined
101
+	if v, ok := b.argsFromOptions[key]; ok && v != nil {
102
+		return *v, ok
103
+	}
104
+
105
+	if defaultValue == nil {
106
+		if v, ok := b.allowedMetaArgs[key]; ok && v != nil {
107
+			return *v, ok
108
+		}
109
+		return "", false
110
+	}
111
+	return *defaultValue, exists
112
+}
113
+
114
+func keysFromMaps(source map[string]*string, builtin map[string]bool) []string {
115
+	keys := []string{}
116
+	for key := range source {
117
+		keys = append(keys, key)
118
+	}
119
+	for key := range builtin {
120
+		keys = append(keys, key)
121
+	}
122
+	return keys
123
+}
0 124
new file mode 100644
... ...
@@ -0,0 +1,63 @@
0
+package dockerfile
1
+
2
+import (
3
+	"github.com/docker/docker/pkg/testutil/assert"
4
+	"testing"
5
+)
6
+
7
+func strPtr(source string) *string {
8
+	return &source
9
+}
10
+
11
+func TestGetAllAllowed(t *testing.T) {
12
+	buildArgs := newBuildArgs(map[string]*string{
13
+		"ArgNotUsedInDockerfile":              strPtr("fromopt1"),
14
+		"ArgOverriddenByOptions":              strPtr("fromopt2"),
15
+		"ArgNoDefaultInDockerfileFromOptions": strPtr("fromopt3"),
16
+		"HTTP_PROXY":                          strPtr("theproxy"),
17
+	})
18
+
19
+	buildArgs.AddMetaArg("ArgFromMeta", strPtr("frommeta1"))
20
+	buildArgs.AddMetaArg("ArgFromMetaOverriden", strPtr("frommeta2"))
21
+	buildArgs.AddMetaArg("ArgFromMetaNotUsed", strPtr("frommeta3"))
22
+
23
+	buildArgs.AddArg("ArgOverriddenByOptions", strPtr("fromdockerfile2"))
24
+	buildArgs.AddArg("ArgWithDefaultInDockerfile", strPtr("fromdockerfile1"))
25
+	buildArgs.AddArg("ArgNoDefaultInDockerfile", nil)
26
+	buildArgs.AddArg("ArgNoDefaultInDockerfileFromOptions", nil)
27
+	buildArgs.AddArg("ArgFromMeta", nil)
28
+	buildArgs.AddArg("ArgFromMetaOverriden", strPtr("fromdockerfile3"))
29
+
30
+	all := buildArgs.GetAllAllowed()
31
+	expected := map[string]string{
32
+		"HTTP_PROXY":                          "theproxy",
33
+		"ArgOverriddenByOptions":              "fromopt2",
34
+		"ArgWithDefaultInDockerfile":          "fromdockerfile1",
35
+		"ArgNoDefaultInDockerfileFromOptions": "fromopt3",
36
+		"ArgFromMeta":                         "frommeta1",
37
+		"ArgFromMetaOverriden":                "fromdockerfile3",
38
+	}
39
+	assert.DeepEqual(t, all, expected)
40
+}
41
+
42
+func TestGetAllMeta(t *testing.T) {
43
+	buildArgs := newBuildArgs(map[string]*string{
44
+		"ArgNotUsedInDockerfile":        strPtr("fromopt1"),
45
+		"ArgOverriddenByOptions":        strPtr("fromopt2"),
46
+		"ArgNoDefaultInMetaFromOptions": strPtr("fromopt3"),
47
+		"HTTP_PROXY":                    strPtr("theproxy"),
48
+	})
49
+
50
+	buildArgs.AddMetaArg("ArgFromMeta", strPtr("frommeta1"))
51
+	buildArgs.AddMetaArg("ArgOverriddenByOptions", strPtr("frommeta2"))
52
+	buildArgs.AddMetaArg("ArgNoDefaultInMetaFromOptions", nil)
53
+
54
+	all := buildArgs.GetAllMeta()
55
+	expected := map[string]string{
56
+		"HTTP_PROXY":                    "theproxy",
57
+		"ArgFromMeta":                   "frommeta1",
58
+		"ArgOverriddenByOptions":        "fromopt2",
59
+		"ArgNoDefaultInMetaFromOptions": "fromopt3",
60
+	}
61
+	assert.DeepEqual(t, all, expected)
62
+}
... ...
@@ -36,20 +36,6 @@ var validCommitCommands = map[string]bool{
36 36
 	"workdir":     true,
37 37
 }
38 38
 
39
-// BuiltinAllowedBuildArgs is list of built-in allowed build args
40
-// these args are considered transparent and are excluded from the image history.
41
-// Filtering from history is implemented in dispatchers.go
42
-var BuiltinAllowedBuildArgs = map[string]bool{
43
-	"HTTP_PROXY":  true,
44
-	"http_proxy":  true,
45
-	"HTTPS_PROXY": true,
46
-	"https_proxy": true,
47
-	"FTP_PROXY":   true,
48
-	"ftp_proxy":   true,
49
-	"NO_PROXY":    true,
50
-	"no_proxy":    true,
51
-}
52
-
53 39
 var defaultLogConfig = container.LogConfig{Type: "none"}
54 40
 
55 41
 // Builder is a Dockerfile builder
... ...
@@ -66,20 +52,19 @@ type Builder struct {
66 66
 	clientCtx context.Context
67 67
 	cancel    context.CancelFunc
68 68
 
69
-	dockerfile       *parser.Node
70
-	runConfig        *container.Config // runconfig for cmd, run, entrypoint etc.
71
-	flags            *BFlags
72
-	tmpContainers    map[string]struct{}
73
-	image            string         // imageID
74
-	imageContexts    *imageContexts // helper for storing contexts from builds
75
-	noBaseImage      bool
76
-	maintainer       string
77
-	cmdSet           bool
78
-	disableCommit    bool
79
-	cacheBusted      bool
80
-	allowedBuildArgs map[string]*string  // list of build-time args that are allowed for expansion/substitution and passing to commands in 'run'.
81
-	allBuildArgs     map[string]struct{} // list of all build-time args found during parsing of the Dockerfile
82
-	directive        parser.Directive
69
+	dockerfile    *parser.Node
70
+	runConfig     *container.Config // runconfig for cmd, run, entrypoint etc.
71
+	flags         *BFlags
72
+	tmpContainers map[string]struct{}
73
+	image         string         // imageID
74
+	imageContexts *imageContexts // helper for storing contexts from builds
75
+	noBaseImage   bool           // A flag to track the use of `scratch` as the base image
76
+	maintainer    string
77
+	cmdSet        bool
78
+	disableCommit bool
79
+	cacheBusted   bool
80
+	buildArgs     *buildArgs
81
+	directive     parser.Directive
83 82
 
84 83
 	// TODO: remove once docker.Commit can receive a tag
85 84
 	id string
... ...
@@ -134,18 +119,17 @@ func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, back
134 134
 	}
135 135
 	ctx, cancel := context.WithCancel(clientCtx)
136 136
 	b = &Builder{
137
-		clientCtx:        ctx,
138
-		cancel:           cancel,
139
-		options:          config,
140
-		Stdout:           os.Stdout,
141
-		Stderr:           os.Stderr,
142
-		docker:           backend,
143
-		context:          buildContext,
144
-		runConfig:        new(container.Config),
145
-		tmpContainers:    map[string]struct{}{},
146
-		id:               stringid.GenerateNonCryptoID(),
147
-		allowedBuildArgs: make(map[string]*string),
148
-		allBuildArgs:     make(map[string]struct{}),
137
+		clientCtx:     ctx,
138
+		cancel:        cancel,
139
+		options:       config,
140
+		Stdout:        os.Stdout,
141
+		Stderr:        os.Stderr,
142
+		docker:        backend,
143
+		context:       buildContext,
144
+		runConfig:     new(container.Config),
145
+		tmpContainers: map[string]struct{}{},
146
+		id:            stringid.GenerateNonCryptoID(),
147
+		buildArgs:     newBuildArgs(config.BuildArgs),
149 148
 		directive: parser.Directive{
150 149
 			EscapeSeen:           false,
151 150
 			LookingForDirectives: true,
... ...
@@ -316,18 +300,17 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
316 316
 // check if there are any leftover build-args that were passed but not
317 317
 // consumed during build. Print a warning, if there are any.
318 318
 func (b *Builder) warnOnUnusedBuildArgs() {
319
-	leftoverArgs := []string{}
320
-	for arg := range b.options.BuildArgs {
321
-		if _, ok := b.allBuildArgs[arg]; !ok {
322
-			leftoverArgs = append(leftoverArgs, arg)
323
-		}
324
-	}
325
-
319
+	leftoverArgs := b.buildArgs.UnreferencedOptionArgs()
326 320
 	if len(leftoverArgs) > 0 {
327 321
 		fmt.Fprintf(b.Stderr, "[Warning] One or more build-args %v were not consumed\n", leftoverArgs)
328 322
 	}
329 323
 }
330 324
 
325
+// hasFromImage returns true if the builder has processed a `FROM <image>` line
326
+func (b *Builder) hasFromImage() bool {
327
+	return b.image != "" || b.noBaseImage
328
+}
329
+
331 330
 // Cancel cancels an ongoing Dockerfile build.
332 331
 func (b *Builder) Cancel() {
333 332
 	b.cancel()
... ...
@@ -218,7 +218,15 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
218 218
 		return err
219 219
 	}
220 220
 
221
-	name := args[0]
221
+	substituionArgs := []string{}
222
+	for key, value := range b.buildArgs.GetAllMeta() {
223
+		substituionArgs = append(substituionArgs, key+"="+value)
224
+	}
225
+
226
+	name, err := ProcessWord(args[0], substituionArgs, b.directive.EscapeToken)
227
+	if err != nil {
228
+		return err
229
+	}
222 230
 
223 231
 	var image builder.Image
224 232
 
... ...
@@ -252,8 +260,7 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
252 252
 	}
253 253
 	b.from = image
254 254
 
255
-	b.allowedBuildArgs = make(map[string]*string)
256
-
255
+	b.buildArgs.ResetAllowed()
257 256
 	return b.processImageFrom(image)
258 257
 }
259 258
 
... ...
@@ -360,7 +367,7 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
360 360
 // RUN [ "echo", "hi" ] # echo hi
361 361
 //
362 362
 func run(b *Builder, args []string, attributes map[string]bool, original string) error {
363
-	if b.image == "" && !b.noBaseImage {
363
+	if !b.hasFromImage() {
364 364
 		return errors.New("Please provide a source image with `from` prior to run")
365 365
 	}
366 366
 
... ...
@@ -438,7 +445,7 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
438 438
 	// properly match it.
439 439
 	b.runConfig.Env = env
440 440
 
441
-	// remove BuiltinAllowedBuildArgs (see: builder.go)  from the saveCmd
441
+	// remove builtinAllowedBuildArgs (see: builder.go)  from the saveCmd
442 442
 	// these args are transparent so resulting image should be the same regardless of the value
443 443
 	if len(cmdBuildEnv) > 0 {
444 444
 		saveCmd = config.Cmd
... ...
@@ -446,11 +453,8 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
446 446
 		copy(tmpBuildEnv, cmdBuildEnv)
447 447
 		for i, env := range tmpBuildEnv {
448 448
 			key := strings.SplitN(env, "=", 2)[0]
449
-			if _, ok := BuiltinAllowedBuildArgs[key]; ok {
450
-				// If an built-in arg is explicitly added in the Dockerfile, don't prune it
451
-				if _, ok := b.allowedBuildArgs[key]; !ok {
452
-					tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...)
453
-				}
449
+			if b.buildArgs.IsUnreferencedBuiltin(key) {
450
+				tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...)
454 451
 			}
455 452
 		}
456 453
 		sort.Strings(tmpBuildEnv)
... ...
@@ -781,15 +785,18 @@ func arg(b *Builder, args []string, attributes map[string]bool, original string)
781 781
 		name = arg
782 782
 		hasDefault = false
783 783
 	}
784
-	// add the arg to allowed list of build-time args from this step on.
785
-	b.allBuildArgs[name] = struct{}{}
786 784
 
787 785
 	var value *string
788 786
 	if hasDefault {
789 787
 		value = &newValue
790 788
 	}
791
-	b.allowedBuildArgs[name] = value
789
+	b.buildArgs.AddArg(name, value)
792 790
 
791
+	// Arg before FROM doesn't add a layer
792
+	if !b.hasFromImage() {
793
+		b.buildArgs.AddMetaArg(name, value)
794
+		return nil
795
+	}
793 796
 	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("ARG %s", arg))
794 797
 }
795 798
 
... ...
@@ -9,6 +9,8 @@ import (
9 9
 	"github.com/docker/docker/api/types"
10 10
 	"github.com/docker/docker/api/types/container"
11 11
 	"github.com/docker/docker/api/types/strslice"
12
+	"github.com/docker/docker/builder"
13
+	"github.com/docker/docker/pkg/testutil/assert"
12 14
 	"github.com/docker/go-connections/nat"
13 15
 )
14 16
 
... ...
@@ -189,35 +191,67 @@ func TestLabel(t *testing.T) {
189 189
 	}
190 190
 }
191 191
 
192
-func TestFrom(t *testing.T) {
193
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
192
+func newBuilderWithMockBackend() *Builder {
193
+	b := &Builder{
194
+		flags:     &BFlags{},
195
+		runConfig: &container.Config{},
196
+		options:   &types.ImageBuildOptions{},
197
+		docker:    &MockBackend{},
198
+		buildArgs: newBuildArgs(make(map[string]*string)),
199
+	}
194 200
 	b.imageContexts = &imageContexts{b: b}
201
+	return b
202
+}
203
+
204
+func TestFromScratch(t *testing.T) {
205
+	b := newBuilderWithMockBackend()
195 206
 
196 207
 	err := from(b, []string{"scratch"}, nil, "")
197 208
 
198 209
 	if runtime.GOOS == "windows" {
199
-		if err == nil {
200
-			t.Fatal("Error not set on Windows")
201
-		}
210
+		assert.Error(t, err, "Windows does not support FROM scratch")
211
+		return
212
+	}
202 213
 
203
-		expectedError := "Windows does not support FROM scratch"
214
+	assert.NilError(t, err)
215
+	assert.Equal(t, b.image, "")
216
+	assert.Equal(t, b.noBaseImage, true)
217
+}
204 218
 
205
-		if !strings.Contains(err.Error(), expectedError) {
206
-			t.Fatalf("Error message not correct on Windows. Should be: %s, got: %s", expectedError, err.Error())
207
-		}
208
-	} else {
209
-		if err != nil {
210
-			t.Fatalf("Error when executing from: %s", err.Error())
211
-		}
219
+func TestFromWithArg(t *testing.T) {
220
+	tag, expected := ":sometag", "expectedthisid"
212 221
 
213
-		if b.image != "" {
214
-			t.Fatalf("Image should be empty, got: %s", b.image)
215
-		}
222
+	getImage := func(name string) (builder.Image, error) {
223
+		assert.Equal(t, name, "alpine"+tag)
224
+		return &mockImage{id: "expectedthisid"}, nil
225
+	}
226
+	b := newBuilderWithMockBackend()
227
+	b.docker.(*MockBackend).getImageOnBuildFunc = getImage
216 228
 
217
-		if b.noBaseImage != true {
218
-			t.Fatalf("Image should not have any base image, got: %v", b.noBaseImage)
219
-		}
229
+	assert.NilError(t, arg(b, []string{"THETAG=" + tag}, nil, ""))
230
+	err := from(b, []string{"alpine${THETAG}"}, nil, "")
231
+
232
+	assert.NilError(t, err)
233
+	assert.Equal(t, b.image, expected)
234
+	assert.Equal(t, b.from.ImageID(), expected)
235
+	assert.Equal(t, len(b.buildArgs.GetAllAllowed()), 0)
236
+	assert.Equal(t, len(b.buildArgs.GetAllMeta()), 1)
237
+}
238
+
239
+func TestFromWithUndefinedArg(t *testing.T) {
240
+	tag, expected := "sometag", "expectedthisid"
241
+
242
+	getImage := func(name string) (builder.Image, error) {
243
+		assert.Equal(t, name, "alpine")
244
+		return &mockImage{id: "expectedthisid"}, nil
220 245
 	}
246
+	b := newBuilderWithMockBackend()
247
+	b.docker.(*MockBackend).getImageOnBuildFunc = getImage
248
+	b.options.BuildArgs = map[string]*string{"THETAG": &tag}
249
+
250
+	err := from(b, []string{"alpine${THETAG}"}, nil, "")
251
+	assert.NilError(t, err)
252
+	assert.Equal(t, b.image, expected)
221 253
 }
222 254
 
223 255
 func TestOnbuildIllegalTriggers(t *testing.T) {
... ...
@@ -461,29 +495,18 @@ func TestStopSignal(t *testing.T) {
461 461
 }
462 462
 
463 463
 func TestArg(t *testing.T) {
464
-	// This is a bad test that tests implementation details and not at
465
-	// any features of the builder. Replace or remove.
466
-	buildOptions := &types.ImageBuildOptions{BuildArgs: make(map[string]*string)}
467
-
468
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true, allowedBuildArgs: make(map[string]*string), allBuildArgs: make(map[string]struct{}), options: buildOptions}
464
+	b := newBuilderWithMockBackend()
469 465
 
470 466
 	argName := "foo"
471 467
 	argVal := "bar"
472 468
 	argDef := fmt.Sprintf("%s=%s", argName, argVal)
473 469
 
474
-	if err := arg(b, []string{argDef}, nil, ""); err != nil {
475
-		t.Fatalf("Error should be empty, got: %s", err.Error())
476
-	}
477
-
478
-	value, ok := b.getBuildArg(argName)
470
+	err := arg(b, []string{argDef}, nil, "")
471
+	assert.NilError(t, err)
479 472
 
480
-	if !ok {
481
-		t.Fatalf("%s argument should be a build arg", argName)
482
-	}
483
-
484
-	if value != "bar" {
485
-		t.Fatalf("%s argument should have default value 'bar', got %s", argName, value)
486
-	}
473
+	expected := map[string]string{argName: argVal}
474
+	allowed := b.buildArgs.GetAllAllowed()
475
+	assert.DeepEqual(t, allowed, expected)
487 476
 }
488 477
 
489 478
 func TestShell(t *testing.T) {
... ...
@@ -175,7 +175,10 @@ func (b *Builder) evaluateEnv(cmd string, str string, envs []string) ([]string,
175 175
 	if allowWordExpansion[cmd] {
176 176
 		processFunc = ProcessWords
177 177
 	} else {
178
-		processFunc = ProcessWord
178
+		processFunc = func(word string, envs []string, escape rune) ([]string, error) {
179
+			word, err := ProcessWord(word, envs, escape)
180
+			return []string{word}, err
181
+		}
179 182
 	}
180 183
 	return processFunc(str, envs, b.directive.EscapeToken)
181 184
 }
... ...
@@ -186,7 +189,7 @@ func (b *Builder) buildArgsWithoutConfigEnv() []string {
186 186
 	envs := []string{}
187 187
 	configEnv := runconfigopts.ConvertKVStringsToMap(b.runConfig.Env)
188 188
 
189
-	for key, val := range b.getBuildArgs() {
189
+	for key, val := range b.buildArgs.GetAllAllowed() {
190 190
 		if _, ok := configEnv[key]; !ok {
191 191
 			envs = append(envs, fmt.Sprintf("%s=%s", key, val))
192 192
 		}
... ...
@@ -180,9 +180,17 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) {
180 180
 	}
181 181
 
182 182
 	config := &container.Config{}
183
-	options := &types.ImageBuildOptions{}
183
+	options := &types.ImageBuildOptions{
184
+		BuildArgs: make(map[string]*string),
185
+	}
184 186
 
185
-	b := &Builder{runConfig: config, options: options, Stdout: ioutil.Discard, context: context}
187
+	b := &Builder{
188
+		runConfig: config,
189
+		options:   options,
190
+		Stdout:    ioutil.Discard,
191
+		context:   context,
192
+		buildArgs: newBuildArgs(options.BuildArgs),
193
+	}
186 194
 
187 195
 	err = b.dispatch(0, len(n.Children), n.Children[0])
188 196
 
... ...
@@ -43,7 +43,7 @@ func (b *Builder) commit(id string, autoCmd strslice.StrSlice, comment string) e
43 43
 	if b.disableCommit {
44 44
 		return nil
45 45
 	}
46
-	if b.image == "" && !b.noBaseImage {
46
+	if !b.hasFromImage() {
47 47
 		return errors.New("Please provide a source image with `from` prior to commit")
48 48
 	}
49 49
 	b.runConfig.Image = b.image
... ...
@@ -516,7 +516,7 @@ func (b *Builder) probeCache() (bool, error) {
516 516
 }
517 517
 
518 518
 func (b *Builder) create() (string, error) {
519
-	if b.image == "" && !b.noBaseImage {
519
+	if !b.hasFromImage() {
520 520
 		return "", errors.New("Please provide a source image with `from` prior to run")
521 521
 	}
522 522
 	b.runConfig.Image = b.image
... ...
@@ -710,36 +710,3 @@ func (b *Builder) parseDockerfile() error {
710 710
 
711 711
 	return nil
712 712
 }
713
-
714
-func (b *Builder) getBuildArg(arg string) (string, bool) {
715
-	defaultValue, defined := b.allowedBuildArgs[arg]
716
-	_, builtin := BuiltinAllowedBuildArgs[arg]
717
-	if defined || builtin {
718
-		if v, ok := b.options.BuildArgs[arg]; ok && v != nil {
719
-			return *v, ok
720
-		}
721
-	}
722
-	if defaultValue == nil {
723
-		return "", false
724
-	}
725
-	return *defaultValue, defined
726
-}
727
-
728
-func (b *Builder) getBuildArgs() map[string]string {
729
-	m := make(map[string]string)
730
-	for arg := range b.options.BuildArgs {
731
-		v, ok := b.getBuildArg(arg)
732
-		if ok {
733
-			m[arg] = v
734
-		}
735
-	}
736
-	for arg := range b.allowedBuildArgs {
737
-		if _, ok := m[arg]; !ok {
738
-			v, ok := b.getBuildArg(arg)
739
-			if ok {
740
-				m[arg] = v
741
-			}
742
-		}
743
-	}
744
-	return m
745
-}
746 713
new file mode 100644
... ...
@@ -0,0 +1,99 @@
0
+package dockerfile
1
+
2
+import (
3
+	"io"
4
+	"time"
5
+
6
+	"github.com/docker/distribution/reference"
7
+	"github.com/docker/docker/api/types"
8
+	"github.com/docker/docker/api/types/backend"
9
+	"github.com/docker/docker/api/types/container"
10
+	"github.com/docker/docker/builder"
11
+	"github.com/docker/docker/image"
12
+	"golang.org/x/net/context"
13
+)
14
+
15
+// MockBackend implements the builder.Backend interface for unit testing
16
+type MockBackend struct {
17
+	getImageOnBuildFunc func(string) (builder.Image, error)
18
+}
19
+
20
+func (m *MockBackend) GetImageOnBuild(name string) (builder.Image, error) {
21
+	if m.getImageOnBuildFunc != nil {
22
+		return m.getImageOnBuildFunc(name)
23
+	}
24
+	return &mockImage{id: "theid"}, nil
25
+}
26
+
27
+func (m *MockBackend) TagImageWithReference(image.ID, reference.Named) error {
28
+	return nil
29
+}
30
+
31
+func (m *MockBackend) PullOnBuild(ctx context.Context, name string, authConfigs map[string]types.AuthConfig, output io.Writer) (builder.Image, error) {
32
+	return nil, nil
33
+}
34
+
35
+func (m *MockBackend) ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool) error {
36
+	return nil
37
+}
38
+
39
+func (m *MockBackend) ContainerCreate(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error) {
40
+	return container.ContainerCreateCreatedBody{}, nil
41
+}
42
+
43
+func (m *MockBackend) ContainerRm(name string, config *types.ContainerRmConfig) error {
44
+	return nil
45
+}
46
+
47
+func (m *MockBackend) Commit(string, *backend.ContainerCommitConfig) (string, error) {
48
+	return "", nil
49
+}
50
+
51
+func (m *MockBackend) ContainerKill(containerID string, sig uint64) error {
52
+	return nil
53
+}
54
+
55
+func (m *MockBackend) ContainerStart(containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error {
56
+	return nil
57
+}
58
+
59
+func (m *MockBackend) ContainerWait(containerID string, timeout time.Duration) (int, error) {
60
+	return 0, nil
61
+}
62
+
63
+func (m *MockBackend) ContainerUpdateCmdOnBuild(containerID string, cmd []string) error {
64
+	return nil
65
+}
66
+
67
+func (m *MockBackend) ContainerCreateWorkdir(containerID string) error {
68
+	return nil
69
+}
70
+
71
+func (m *MockBackend) CopyOnBuild(containerID string, destPath string, src builder.FileInfo, decompress bool) error {
72
+	return nil
73
+}
74
+
75
+func (m *MockBackend) HasExperimental() bool {
76
+	return false
77
+}
78
+
79
+func (m *MockBackend) SquashImage(from string, to string) (string, error) {
80
+	return "", nil
81
+}
82
+
83
+func (m *MockBackend) MountImage(name string) (string, func() error, error) {
84
+	return "", func() error { return nil }, nil
85
+}
86
+
87
+type mockImage struct {
88
+	id     string
89
+	config *container.Config
90
+}
91
+
92
+func (i *mockImage) ImageID() string {
93
+	return i.id
94
+}
95
+
96
+func (i *mockImage) RunConfig() *container.Config {
97
+	return i.config
98
+}
... ...
@@ -24,9 +24,9 @@ type shellWord struct {
24 24
 
25 25
 // ProcessWord will use the 'env' list of environment variables,
26 26
 // and replace any env var references in 'word'.
27
-func ProcessWord(word string, env []string, escapeToken rune) ([]string, error) {
27
+func ProcessWord(word string, env []string, escapeToken rune) (string, error) {
28 28
 	word, _, err := process(word, env, escapeToken)
29
-	return []string{word}, err
29
+	return word, err
30 30
 }
31 31
 
32 32
 // ProcessWords will use the 'env' list of environment variables,
... ...
@@ -54,7 +54,7 @@ func TestShellParser4EnvVars(t *testing.T) {
54 54
 				assert.Error(t, err, "")
55 55
 			} else {
56 56
 				assert.NilError(t, err)
57
-				assert.DeepEqual(t, newWord, []string{expected})
57
+				assert.Equal(t, newWord, expected)
58 58
 			}
59 59
 		}
60 60
 	}
... ...
@@ -4728,7 +4728,7 @@ func (s *DockerSuite) TestBuildBuildTimeArgDefintionWithNoEnvInjection(c *check.
4728 4728
 		ARG %s
4729 4729
 		RUN env`, envKey)
4730 4730
 
4731
-	result := buildImage(imgName, build.WithDockerfile(dockerfile))
4731
+	result := cli.BuildCmd(c, imgName, build.WithDockerfile(dockerfile))
4732 4732
 	result.Assert(c, icmd.Success)
4733 4733
 	if strings.Count(result.Combined(), envKey) != 1 {
4734 4734
 		c.Fatalf("unexpected number of occurrences of the arg in output: %q expected: 1", result.Combined())
... ...
@@ -4745,29 +4745,45 @@ func (s *DockerSuite) TestBuildBuildTimeArgMultipleFrom(c *check.C) {
4745 4745
     ARG bar=def
4746 4746
     RUN env > /out`
4747 4747
 
4748
-	result := buildImage(imgName, build.WithDockerfile(dockerfile))
4748
+	result := cli.BuildCmd(c, imgName, build.WithDockerfile(dockerfile))
4749 4749
 	result.Assert(c, icmd.Success)
4750 4750
 
4751
-	result = icmd.RunCmd(icmd.Cmd{
4752
-		Command: []string{dockerBinary, "images", "-q", "-f", "label=multifromtest=1"},
4753
-	})
4754
-	result.Assert(c, icmd.Success)
4751
+	result = cli.DockerCmd(c, "images", "-q", "-f", "label=multifromtest=1")
4755 4752
 	parentID := strings.TrimSpace(result.Stdout())
4756 4753
 
4757
-	result = icmd.RunCmd(icmd.Cmd{
4758
-		Command: []string{dockerBinary, "run", "--rm", parentID, "cat", "/out"},
4759
-	})
4760
-	result.Assert(c, icmd.Success)
4754
+	result = cli.DockerCmd(c, "run", "--rm", parentID, "cat", "/out")
4761 4755
 	c.Assert(result.Stdout(), checker.Contains, "foo=abc")
4762 4756
 
4763
-	result = icmd.RunCmd(icmd.Cmd{
4764
-		Command: []string{dockerBinary, "run", "--rm", imgName, "cat", "/out"},
4765
-	})
4766
-	result.Assert(c, icmd.Success)
4757
+	result = cli.DockerCmd(c, "run", "--rm", imgName, "cat", "/out")
4767 4758
 	c.Assert(result.Stdout(), checker.Not(checker.Contains), "foo")
4768 4759
 	c.Assert(result.Stdout(), checker.Contains, "bar=def")
4769 4760
 }
4770 4761
 
4762
+func (s *DockerSuite) TestBuildBuildTimeFromArgMultipleFrom(c *check.C) {
4763
+	imgName := "multifrombldargtest"
4764
+	dockerfile := `ARG tag=nosuchtag
4765
+     FROM busybox:${tag}
4766
+     LABEL multifromtest=1
4767
+     RUN env > /out
4768
+     FROM busybox:${tag}
4769
+     ARG tag
4770
+     RUN env > /out`
4771
+
4772
+	result := cli.BuildCmd(c, imgName,
4773
+		build.WithDockerfile(dockerfile),
4774
+		cli.WithFlags("--build-arg", fmt.Sprintf("tag=latest")))
4775
+	result.Assert(c, icmd.Success)
4776
+
4777
+	result = cli.DockerCmd(c, "images", "-q", "-f", "label=multifromtest=1")
4778
+	parentID := strings.TrimSpace(result.Stdout())
4779
+
4780
+	result = cli.DockerCmd(c, "run", "--rm", parentID, "cat", "/out")
4781
+	c.Assert(result.Stdout(), checker.Not(checker.Contains), "tag")
4782
+
4783
+	result = cli.DockerCmd(c, "run", "--rm", imgName, "cat", "/out")
4784
+	c.Assert(result.Stdout(), checker.Contains, "tag=latest")
4785
+}
4786
+
4771 4787
 func (s *DockerSuite) TestBuildBuildTimeUnusedArgMultipleFrom(c *check.C) {
4772 4788
 	imgName := "multifromunusedarg"
4773 4789
 	dockerfile := `FROM busybox
... ...
@@ -4776,16 +4792,14 @@ func (s *DockerSuite) TestBuildBuildTimeUnusedArgMultipleFrom(c *check.C) {
4776 4776
     ARG bar
4777 4777
     RUN env > /out`
4778 4778
 
4779
-	result := buildImage(imgName, build.WithDockerfile(dockerfile), cli.WithFlags(
4780
-		"--build-arg", fmt.Sprintf("baz=abc")))
4779
+	result := cli.BuildCmd(c, imgName,
4780
+		build.WithDockerfile(dockerfile),
4781
+		cli.WithFlags("--build-arg", fmt.Sprintf("baz=abc")))
4781 4782
 	result.Assert(c, icmd.Success)
4782 4783
 	c.Assert(result.Combined(), checker.Contains, "[Warning]")
4783 4784
 	c.Assert(result.Combined(), checker.Contains, "[baz] were not consumed")
4784 4785
 
4785
-	result = icmd.RunCmd(icmd.Cmd{
4786
-		Command: []string{dockerBinary, "run", "--rm", imgName, "cat", "/out"},
4787
-	})
4788
-	result.Assert(c, icmd.Success)
4786
+	result = cli.DockerCmd(c, "run", "--rm", imgName, "cat", "/out")
4789 4787
 	c.Assert(result.Stdout(), checker.Not(checker.Contains), "bar")
4790 4788
 	c.Assert(result.Stdout(), checker.Not(checker.Contains), "baz")
4791 4789
 }