Fix arg in from when arg is not defined

Add mock builder backend
Add tests for ARG in FROM

Signed-off-by: Daniel Nephin <dnephin@docker.com>

builder/dockerfile/dispatchers.go

@@ -22,7 +22,6 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/docker/builder"
-	"github.com/docker/docker/pkg/shellvar"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/go-connections/nat"
 	"github.com/pkg/errors"
@@ -219,14 +218,8 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
 		return err
 	}
 
-	getBuildArg := func(key string) (string, bool) {
-		value, ok := b.options.BuildArgs[key]
-		if value != nil {
-			return *value, ok
-		}
-		return "", ok
-	}
-	name, err := shellvar.Substitute(args[0], getBuildArg)
+	substituionArgs := b.buildArgsWithoutConfigEnv()
+	name, err := ProcessWord(args[0], substituionArgs, b.directive.EscapeToken)
 	if err != nil {
 		return err
 	}

builder/dockerfile/dispatchers_test.go

@@ -9,6 +9,8 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
+	"github.com/docker/docker/builder"
+	"github.com/docker/docker/pkg/testutil/assert"
 	"github.com/docker/go-connections/nat"
 )
 
@@ -189,35 +191,68 @@ func TestLabel(t *testing.T) {
 	}
 }
 
-func TestFrom(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
+func newBuilderWithMockBackend() *Builder {
+	b := &Builder{
+		flags:            &BFlags{},
+		runConfig:        &container.Config{},
+		options:          &types.ImageBuildOptions{},
+		docker:           &MockBackend{},
+		allowedBuildArgs: make(map[string]*string),
+		allBuildArgs:     make(map[string]struct{}),
+	}
 	b.imageContexts = &imageContexts{b: b}
+	return b
+}
+
+func TestFromScratch(t *testing.T) {
+	b := newBuilderWithMockBackend()
 
 	err := from(b, []string{"scratch"}, nil, "")
 
 	if runtime.GOOS == "windows" {
-		if err == nil {
-			t.Fatal("Error not set on Windows")
-		}
-
-		expectedError := "Windows does not support FROM scratch"
-
-		if !strings.Contains(err.Error(), expectedError) {
-			t.Fatalf("Error message not correct on Windows. Should be: %s, got: %s", expectedError, err.Error())
-		}
-	} else {
-		if err != nil {
-			t.Fatalf("Error when executing from: %s", err.Error())
-		}
-
-		if b.image != "" {
-			t.Fatalf("Image should be empty, got: %s", b.image)
-		}
-
-		if b.noBaseImage != true {
-			t.Fatalf("Image should not have any base image, got: %v", b.noBaseImage)
-		}
+		assert.Error(t, err, "Windows does not support FROM scratch")
+		return
 	}
+
+	assert.NilError(t, err)
+	assert.Equal(t, b.image, "")
+	assert.Equal(t, b.noBaseImage, true)
+}
+
+func TestFromWithArg(t *testing.T) {
+	tag, expected := ":sometag", "expectedthisid"
+
+	getImage := func(name string) (builder.Image, error) {
+		assert.Equal(t, name, "alpine"+tag)
+		return &mockImage{id: "expectedthisid"}, nil
+	}
+	b := newBuilderWithMockBackend()
+	b.docker.(*MockBackend).getImageOnBuildFunc = getImage
+
+	assert.NilError(t, arg(b, []string{"THETAG=" + tag}, nil, ""))
+	err := from(b, []string{"alpine${THETAG}"}, nil, "")
+
+	assert.NilError(t, err)
+	assert.Equal(t, b.image, expected)
+	assert.Equal(t, b.from.ImageID(), expected)
+	assert.NotNil(t, b.allowedBuildArgs)
+	assert.Equal(t, len(b.allowedBuildArgs), 0)
+}
+
+func TestFromWithUndefinedArg(t *testing.T) {
+	tag, expected := "sometag", "expectedthisid"
+
+	getImage := func(name string) (builder.Image, error) {
+		assert.Equal(t, name, "alpine")
+		return &mockImage{id: "expectedthisid"}, nil
+	}
+	b := newBuilderWithMockBackend()
+	b.docker.(*MockBackend).getImageOnBuildFunc = getImage
+	b.options.BuildArgs = map[string]*string{"THETAG": &tag}
+
+	err := from(b, []string{"alpine${THETAG}"}, nil, "")
+	assert.NilError(t, err)
+	assert.Equal(t, b.image, expected)
 }
 
 func TestOnbuildIllegalTriggers(t *testing.T) {

builder/dockerfile/evaluator.go

@@ -175,7 +175,10 @@ func (b *Builder) evaluateEnv(cmd string, str string, envs []string) ([]string,
 	if allowWordExpansion[cmd] {
 		processFunc = ProcessWords
 	} else {
-		processFunc = ProcessWord
+		processFunc = func(word string, envs []string, escape rune) ([]string, error) {
+			word, err := ProcessWord(word, envs, escape)
+			return []string{word}, err
+		}
 	}
 	return processFunc(str, envs, b.directive.EscapeToken)
 }

builder/dockerfile/mockbackend_test.go

@@ -0,0 +1,99 @@
+package dockerfile
+
+import (
+	"io"
+	"time"
+
+	"github.com/docker/distribution/reference"
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/backend"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/builder"
+	"github.com/docker/docker/image"
+	"golang.org/x/net/context"
+)
+
+// MockBackend implements the builder.Backend interface for unit testing
+type MockBackend struct {
+	getImageOnBuildFunc func(string) (builder.Image, error)
+}
+
+func (m *MockBackend) GetImageOnBuild(name string) (builder.Image, error) {
+	if m.getImageOnBuildFunc != nil {
+		return m.getImageOnBuildFunc(name)
+	}
+	return &mockImage{id: "theid"}, nil
+}
+
+func (m *MockBackend) TagImageWithReference(image.ID, reference.Named) error {
+	return nil
+}
+
+func (m *MockBackend) PullOnBuild(ctx context.Context, name string, authConfigs map[string]types.AuthConfig, output io.Writer) (builder.Image, error) {
+	return nil, nil
+}
+
+func (m *MockBackend) ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool) error {
+	return nil
+}
+
+func (m *MockBackend) ContainerCreate(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error) {
+	return container.ContainerCreateCreatedBody{}, nil
+}
+
+func (m *MockBackend) ContainerRm(name string, config *types.ContainerRmConfig) error {
+	return nil
+}
+
+func (m *MockBackend) Commit(string, *backend.ContainerCommitConfig) (string, error) {
+	return "", nil
+}
+
+func (m *MockBackend) ContainerKill(containerID string, sig uint64) error {
+	return nil
+}
+
+func (m *MockBackend) ContainerStart(containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error {
+	return nil
+}
+
+func (m *MockBackend) ContainerWait(containerID string, timeout time.Duration) (int, error) {
+	return 0, nil
+}
+
+func (m *MockBackend) ContainerUpdateCmdOnBuild(containerID string, cmd []string) error {
+	return nil
+}
+
+func (m *MockBackend) ContainerCreateWorkdir(containerID string) error {
+	return nil
+}
+
+func (m *MockBackend) CopyOnBuild(containerID string, destPath string, src builder.FileInfo, decompress bool) error {
+	return nil
+}
+
+func (m *MockBackend) HasExperimental() bool {
+	return false
+}
+
+func (m *MockBackend) SquashImage(from string, to string) (string, error) {
+	return "", nil
+}
+
+func (m *MockBackend) MountImage(name string) (string, func() error, error) {
+	return "", func() error { return nil }, nil
+}
+
+type mockImage struct {
+	id     string
+	config *container.Config
+}
+
+func (i *mockImage) ImageID() string {
+	return i.id
+}
+
+func (i *mockImage) RunConfig() *container.Config {
+	return i.config
+}

builder/dockerfile/shell_parser.go

@@ -24,9 +24,9 @@ type shellWord struct {
 
 // ProcessWord will use the 'env' list of environment variables,
 // and replace any env var references in 'word'.
-func ProcessWord(word string, env []string, escapeToken rune) ([]string, error) {
+func ProcessWord(word string, env []string, escapeToken rune) (string, error) {
 	word, _, err := process(word, env, escapeToken)
-	return []string{word}, err
+	return word, err
 }
 
 // ProcessWords will use the 'env' list of environment variables,

builder/dockerfile/shell_parser_test.go

@@ -54,7 +54,7 @@ func TestShellParser4EnvVars(t *testing.T) {
 				assert.Error(t, err, "")
 			} else {
 				assert.NilError(t, err)
-				assert.DeepEqual(t, newWord, []string{expected})
+				assert.Equal(t, newWord, expected)
 			}
 		}
 	}