From d3ea7e80e879b506bddffd51c3ab65b8078a34f4 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Sun, 3 Jan 2016 19:45:06 -0800 Subject: [PATCH] Add default PATH to 'scratch' images Closes #19012 Signed-off-by: Doug Davis --- builder/dockerfile/dispatchers.go | 33 ++++++++++++------------ builder/dockerfile/internals.go | 28 +++++++++++++++----- integration-cli/docker_cli_build_test.go | 33 ++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 22 deletions(-) diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index c32834ba88..53e6322bfb 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -193,6 +193,11 @@ func from(b *Builder, args []string, attributes map[string]bool, original string name := args[0] + var ( + image builder.Image + err error + ) + // Windows cannot support a container with no base image. if name == api.NoBaseImageSpecifier { if runtime.GOOS == "windows" { @@ -200,24 +205,20 @@ func from(b *Builder, args []string, attributes map[string]bool, original string } b.image = "" b.noBaseImage = true - return nil - } - - var ( - image builder.Image - err error - ) - // TODO: don't use `name`, instead resolve it to a digest - if !b.options.PullParent { - image, err = b.docker.GetImage(name) - // TODO: shouldn't we error out if error is different from "not found" ? - } - if image == nil { - image, err = b.docker.Pull(name) - if err != nil { - return err + } else { + // TODO: don't use `name`, instead resolve it to a digest + if !b.options.PullParent { + image, err = b.docker.GetImage(name) + // TODO: shouldn't we error out if error is different from "not found" ? + } + if image == nil { + image, err = b.docker.Pull(name) + if err != nil { + return err + } } } + return b.processImageFrom(image) } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 6f0c34c0e8..24c9087eae 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -35,6 +35,7 @@ import ( "github.com/docker/docker/pkg/system" "github.com/docker/docker/pkg/tarsum" "github.com/docker/docker/pkg/urlutil" + "github.com/docker/docker/runconfig/opts" ) func (b *Builder) commit(id string, autoCmd *strslice.StrSlice, comment string) error { @@ -394,15 +395,30 @@ func containsWildcards(name string) bool { } func (b *Builder) processImageFrom(img builder.Image) error { - b.image = img.ID() + if img != nil { + b.image = img.ID() - if img.Config() != nil { - b.runConfig = img.Config() + if img.Config() != nil { + b.runConfig = img.Config() + } } - // The default path will be blank on Windows (set by HCS) - if len(b.runConfig.Env) == 0 && system.DefaultPathEnv != "" { - b.runConfig.Env = append(b.runConfig.Env, "PATH="+system.DefaultPathEnv) + // Check to see if we have a default PATH, note that windows won't + // have one as its set by HCS + if system.DefaultPathEnv != "" { + // Convert the slice of strings that represent the current list + // of env vars into a map so we can see if PATH is already set. + // If its not set then go ahead and give it our default value + configEnv := opts.ConvertKVStringsToMap(b.runConfig.Env) + if _, ok := configEnv["PATH"]; !ok { + b.runConfig.Env = append(b.runConfig.Env, + "PATH="+system.DefaultPathEnv) + } + } + + if img == nil { + // Typically this means they used "FROM scratch" + return nil } // Process ONBUILD triggers if they exist diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 207082f201..a8fd723848 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -2143,6 +2143,39 @@ func (s *DockerSuite) TestBuildEnv(c *check.C) { } } +func (s *DockerSuite) TestBuildPATH(c *check.C) { + testRequires(c, DaemonIsLinux) + + defPath := "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + + fn := func(dockerfile string, exp string) { + _, err := buildImage("testbldpath", dockerfile, true) + c.Assert(err, check.IsNil) + + res, err := inspectField("testbldpath", "Config.Env") + c.Assert(err, check.IsNil) + + if res != exp { + c.Fatalf("Env %q, expected %q for dockerfile:%q", res, exp, dockerfile) + } + } + + tests := []struct{ dockerfile, exp string }{ + {"FROM scratch\nMAINTAINER me", "[PATH=" + defPath + "]"}, + {"FROM busybox\nMAINTAINER me", "[PATH=" + defPath + "]"}, + {"FROM scratch\nENV FOO=bar", "[PATH=" + defPath + " FOO=bar]"}, + {"FROM busybox\nENV FOO=bar", "[PATH=" + defPath + " FOO=bar]"}, + {"FROM scratch\nENV PATH=/test", "[PATH=/test]"}, + {"FROM busybox\nENV PATH=/test", "[PATH=/test]"}, + {"FROM scratch\nENV PATH=''", "[PATH=]"}, + {"FROM busybox\nENV PATH=''", "[PATH=]"}, + } + + for _, test := range tests { + fn(test.dockerfile, test.exp) + } +} + func (s *DockerSuite) TestBuildContextCleanup(c *check.C) { testRequires(c, DaemonIsLinux) testRequires(c, SameHostDaemon)