diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 52c4a4064d..ba98c636e9 100644 --- a/builder/dockerfile/dispatchers.go +++ b/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 } diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index 6b9b538102..119d5e904b 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/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) { diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index 08c05daf8f..78cda4bb92 100644 --- a/builder/dockerfile/evaluator.go +++ b/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) } diff --git a/builder/dockerfile/mockbackend_test.go b/builder/dockerfile/mockbackend_test.go new file mode 100644 index 0000000000..4c03569678 --- /dev/null +++ b/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 +} diff --git a/builder/dockerfile/shell_parser.go b/builder/dockerfile/shell_parser.go index 2a8b441134..8e210ad6a9 100644 --- a/builder/dockerfile/shell_parser.go +++ b/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, diff --git a/builder/dockerfile/shell_parser_test.go b/builder/dockerfile/shell_parser_test.go index 0712fde675..6c3fff5b66 100644 --- a/builder/dockerfile/shell_parser_test.go +++ b/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) } } }