diff --git a/client/llb/exec.go b/client/llb/exec.go index c1c82aa756da..decc0d7407f7 100644 --- a/client/llb/exec.go +++ b/client/llb/exec.go @@ -153,7 +153,13 @@ func (e *ExecOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] } if c.Caps != nil { if err := c.Caps.Supports(pb.CapExecMetaSetsDefaultPath); err != nil { - env = env.SetDefault("PATH", system.DefaultPathEnv) + os := "linux" + if c.Platform != nil { + os = c.Platform.OS + } else if e.constraints.Platform != nil { + os = e.constraints.Platform.OS + } + env = env.SetDefault("PATH", system.DefaultPathEnv(os)) } else { addCap(&e.constraints, pb.CapExecMetaSetsDefaultPath) } diff --git a/client/llb/llbtest/platform_test.go b/client/llb/llbtest/platform_test.go index 5b88bdb3b822..82bf946031d8 100644 --- a/client/llb/llbtest/platform_test.go +++ b/client/llb/llbtest/platform_test.go @@ -143,7 +143,7 @@ func TestFallbackPath(t *testing.T) { require.False(t, def.Metadata[e.Vertex.Digest()].Caps[pb.CapExecMetaSetsDefaultPath]) v, ok := getenv(e, "PATH") require.True(t, ok) - require.Equal(t, system.DefaultPathEnv, v) + require.Equal(t, system.DefaultPathEnvUnix, v) // All capabilities, including pb.CapExecMetaSetsDefaultPath, // so should get no PATH (not present at all, rather than diff --git a/examples/buildkit0/buildkit.go b/examples/buildkit0/buildkit.go index 66fb750a5a97..dd094d15ec59 100644 --- a/examples/buildkit0/buildkit.go +++ b/examples/buildkit0/buildkit.go @@ -35,7 +35,7 @@ func main() { func goBuildBase() llb.State { goAlpine := llb.Image("docker.io/library/golang:1.13-alpine") return goAlpine. - AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnv). + AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnvUnix). AddEnv("GOPATH", "/go"). Run(llb.Shlex("apk add --no-cache g++ linux-headers")). Run(llb.Shlex("apk add --no-cache git libseccomp-dev make")).Root() diff --git a/examples/buildkit1/buildkit.go b/examples/buildkit1/buildkit.go index c6dad534816c..ea3e558e1809 100644 --- a/examples/buildkit1/buildkit.go +++ b/examples/buildkit1/buildkit.go @@ -35,7 +35,7 @@ func main() { func goBuildBase() llb.State { goAlpine := llb.Image("docker.io/library/golang:1.13-alpine") return goAlpine. - AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnv). + AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnvUnix). AddEnv("GOPATH", "/go"). Run(llb.Shlex("apk add --no-cache g++ linux-headers")). Run(llb.Shlex("apk add --no-cache git libseccomp-dev make")).Root() diff --git a/examples/buildkit2/buildkit.go b/examples/buildkit2/buildkit.go index 4bd55bef93a1..9e82d3adee0a 100644 --- a/examples/buildkit2/buildkit.go +++ b/examples/buildkit2/buildkit.go @@ -35,7 +35,7 @@ func main() { func goBuildBase() llb.State { goAlpine := llb.Image("docker.io/library/golang:1.13-alpine") return goAlpine. - AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnv). + AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnvUnix). AddEnv("GOPATH", "/go"). Run(llb.Shlex("apk add --no-cache g++ linux-headers libseccomp-dev make")).Root() } diff --git a/examples/buildkit3/buildkit.go b/examples/buildkit3/buildkit.go index 4ddcf59ef829..633891483d19 100644 --- a/examples/buildkit3/buildkit.go +++ b/examples/buildkit3/buildkit.go @@ -36,7 +36,7 @@ func main() { func goBuildBase() llb.State { goAlpine := llb.Image("docker.io/library/golang:1.13-alpine") return goAlpine. - AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnv). + AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnvUnix). AddEnv("GOPATH", "/go"). Run(llb.Shlex("apk add --no-cache g++ linux-headers libseccomp-dev make")).Root() } diff --git a/examples/nested-llb/main.go b/examples/nested-llb/main.go index 1c15934091eb..d53aa57cb43f 100644 --- a/examples/nested-llb/main.go +++ b/examples/nested-llb/main.go @@ -34,7 +34,7 @@ func main() { func goBuildBase() llb.State { goAlpine := llb.Image("docker.io/library/golang:1.13-alpine") return goAlpine. - AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnv). + AddEnv("PATH", "/usr/local/go/bin:"+system.DefaultPathEnvUnix). AddEnv("GOPATH", "/go"). Run(llb.Shlex("apk add --no-cache g++ linux-headers make")).Root() } diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index a0858801c1b2..bfee4d15801a 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -315,7 +315,7 @@ func emptyImageConfig() ([]byte, error) { } img.RootFS.Type = "layers" img.Config.WorkingDir = "/" - img.Config.Env = []string{"PATH=" + system.DefaultPathEnv} + img.Config.Env = []string{"PATH=" + system.DefaultPathEnv(pl.OS)} dt, err := json.Marshal(img) return dt, errors.Wrap(err, "failed to create empty image config") } diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index ca5d9c5b49bc..5b3d52a6366a 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -317,7 +317,11 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, // make sure that PATH is always set if _, ok := shell.BuildEnvs(d.image.Config.Env)["PATH"]; !ok { - d.image.Config.Env = append(d.image.Config.Env, "PATH="+system.DefaultPathEnv) + var os string + if d.platform != nil { + os = d.platform.OS + } + d.image.Config.Env = append(d.image.Config.Env, "PATH="+system.DefaultPathEnv(os)) } // initialize base metadata from image conf @@ -1349,7 +1353,7 @@ func withShell(img Image, args []string) []string { if len(img.Config.Shell) > 0 { shell = append([]string{}, img.Config.Shell...) } else { - shell = defaultShell() + shell = defaultShell(img.OS) } return append(shell, strings.Join(args, " ")) } diff --git a/frontend/dockerfile/dockerfile2llb/defaultshell.go b/frontend/dockerfile/dockerfile2llb/defaultshell.go new file mode 100644 index 000000000000..58b17372bd44 --- /dev/null +++ b/frontend/dockerfile/dockerfile2llb/defaultshell.go @@ -0,0 +1,8 @@ +package dockerfile2llb + +func defaultShell(os string) []string { + if os == "windows" { + return []string{"cmd", "/S", "/C"} + } + return []string{"/bin/sh", "-c"} +} diff --git a/frontend/dockerfile/dockerfile2llb/defaultshell_unix.go b/frontend/dockerfile/dockerfile2llb/defaultshell_unix.go deleted file mode 100644 index b5d541d1f5db..000000000000 --- a/frontend/dockerfile/dockerfile2llb/defaultshell_unix.go +++ /dev/null @@ -1,7 +0,0 @@ -// +build !windows - -package dockerfile2llb - -func defaultShell() []string { - return []string{"/bin/sh", "-c"} -} diff --git a/frontend/dockerfile/dockerfile2llb/defaultshell_windows.go b/frontend/dockerfile/dockerfile2llb/defaultshell_windows.go deleted file mode 100644 index 7693e050863a..000000000000 --- a/frontend/dockerfile/dockerfile2llb/defaultshell_windows.go +++ /dev/null @@ -1,7 +0,0 @@ -// +build windows - -package dockerfile2llb - -func defaultShell() []string { - return []string{"cmd", "/S", "/C"} -} diff --git a/frontend/dockerfile/dockerfile2llb/image.go b/frontend/dockerfile/dockerfile2llb/image.go index 55e9add23363..6eb4cf6e7ae0 100644 --- a/frontend/dockerfile/dockerfile2llb/image.go +++ b/frontend/dockerfile/dockerfile2llb/image.go @@ -74,6 +74,6 @@ func emptyImage(platform specs.Platform) Image { } img.RootFS.Type = "layers" img.Config.WorkingDir = "/" - img.Config.Env = []string{"PATH=" + system.DefaultPathEnv} + img.Config.Env = []string{"PATH=" + system.DefaultPathEnv(platform.OS)} return img } diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index a137fdf8562f..614c653bb8d5 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -106,6 +106,7 @@ var allTests = []integration.Test{ testMultiArgs, testFrontendSubrequests, testDockefileCheckHostname, + testDefaultShellAndPath, } var fileOpTests = []integration.Test{ @@ -1209,6 +1210,96 @@ COPY --from=build /out . require.Equal(t, "foo bar:box-foo:123 456", string(dt)) } +func testDefaultShellAndPath(t *testing.T, sb integration.Sandbox) { + skipDockerd(t, sb) + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM scratch +ENTRYPOINT foo bar +COPY Dockerfile . +`) + + dir, err := tmpdir( + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + require.NoError(t, err) + defer os.RemoveAll(dir) + + c, err := client.New(context.TODO(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + destDir, err := ioutil.TempDir("", "buildkit") + require.NoError(t, err) + defer os.RemoveAll(destDir) + + out := filepath.Join(destDir, "out.tar") + outW, err := os.Create(out) + require.NoError(t, err) + + _, err = f.Solve(context.TODO(), c, client.SolveOpt{ + LocalDirs: map[string]string{ + builder.DefaultLocalNameDockerfile: dir, + builder.DefaultLocalNameContext: dir, + }, + FrontendAttrs: map[string]string{ + "platform": "windows/amd64,linux/amd64", + }, + Exports: []client.ExportEntry{ + { + Type: client.ExporterOCI, + Output: fixedWriteCloser(outW), + }, + }, + }, nil) + require.NoError(t, err) + + dt, err := ioutil.ReadFile(filepath.Join(destDir, "out.tar")) + require.NoError(t, err) + + m, err := testutil.ReadTarToMap(dt, false) + require.NoError(t, err) + + var idx ocispec.Index + err = json.Unmarshal(m["index.json"].Data, &idx) + require.NoError(t, err) + + mlistHex := idx.Manifests[0].Digest.Hex() + + idx = ocispec.Index{} + err = json.Unmarshal(m["blobs/sha256/"+mlistHex].Data, &idx) + require.NoError(t, err) + + require.Equal(t, 2, len(idx.Manifests)) + + for i, exp := range []struct { + p string + entrypoint []string + env []string + }{ + {p: "windows/amd64", entrypoint: []string{"cmd", "/S", "/C", "foo bar"}, env: []string{"PATH=c:\\Windows\\System32;c:\\Windows"}}, + {p: "linux/amd64", entrypoint: []string{"/bin/sh", "-c", "foo bar"}, env: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}}, + } { + t.Run(exp.p, func(t *testing.T) { + require.Equal(t, exp.p, platforms.Format(*idx.Manifests[i].Platform)) + + var mfst ocispec.Manifest + err = json.Unmarshal(m["blobs/sha256/"+idx.Manifests[i].Digest.Hex()].Data, &mfst) + require.NoError(t, err) + + require.Equal(t, 1, len(mfst.Layers)) + + var img ocispec.Image + err = json.Unmarshal(m["blobs/sha256/"+mfst.Config.Digest.Hex()].Data, &img) + require.NoError(t, err) + + require.Equal(t, exp.entrypoint, img.Config.Entrypoint) + require.Equal(t, exp.env, img.Config.Env) + }) + } +} + func testExportMultiPlatform(t *testing.T, sb integration.Sandbox) { skipDockerd(t, sb) f := getFrontend(t, sb) diff --git a/frontend/gateway/container.go b/frontend/gateway/container.go index 9981124c40f4..99a3640ddd53 100644 --- a/frontend/gateway/container.go +++ b/frontend/gateway/container.go @@ -227,7 +227,7 @@ func (gwCtr *gatewayContainer) Start(ctx context.Context, req client.StartReques if procInfo.Meta.Cwd == "" { procInfo.Meta.Cwd = "/" } - procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "PATH", utilsystem.DefaultPathEnv) + procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "PATH", utilsystem.DefaultPathEnvUnix) // support windows? if req.Tty { procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "TERM", "xterm") } diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index e5aeeea12b8f..5516376fd8ae 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -380,7 +380,11 @@ func (e *execOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu if e.op.Meta.ProxyEnv != nil { meta.Env = append(meta.Env, proxyEnvList(e.op.Meta.ProxyEnv)...) } - meta.Env = addDefaultEnvvar(meta.Env, "PATH", utilsystem.DefaultPathEnv) + var currentOS string + if e.platform != nil { + currentOS = e.platform.OS + } + meta.Env = addDefaultEnvvar(meta.Env, "PATH", utilsystem.DefaultPathEnv(currentOS)) stdout, stderr := logs.NewLogStreams(ctx, os.Getenv("BUILDKIT_DEBUG_EXEC_OUTPUT") == "1") defer stdout.Close() diff --git a/util/system/path.go b/util/system/path.go new file mode 100644 index 000000000000..f6dc70dc8dd5 --- /dev/null +++ b/util/system/path.go @@ -0,0 +1,18 @@ +package system + +// DefaultPathEnvUnix is unix style list of directories to search for +// executables. Each directory is separated from the next by a colon +// ':' character . +const DefaultPathEnvUnix = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + +// DefaultPathEnvWindows is windows style list of directories to search for +// executables. Each directory is separated from the next by a colon +// ';' character . +const DefaultPathEnvWindows = "c:\\Windows\\System32;c:\\Windows" + +func DefaultPathEnv(os string) string { + if os == "windows" { + return DefaultPathEnvWindows + } + return DefaultPathEnvUnix +} diff --git a/util/system/path_unix.go b/util/system/path_unix.go index c607c4db09f2..f3762e69d36a 100644 --- a/util/system/path_unix.go +++ b/util/system/path_unix.go @@ -2,11 +2,6 @@ package system -// DefaultPathEnv is unix style list of directories to search for -// executables. Each directory is separated from the next by a colon -// ':' character . -const DefaultPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" - // CheckSystemDriveAndRemoveDriveLetter verifies that a path, if it includes a drive letter, // is the system drive. This is a no-op on Linux. func CheckSystemDriveAndRemoveDriveLetter(path string) (string, error) { diff --git a/util/system/path_windows.go b/util/system/path_windows.go index cbfe2c1576ce..3fc47449484e 100644 --- a/util/system/path_windows.go +++ b/util/system/path_windows.go @@ -8,10 +8,6 @@ import ( "strings" ) -// DefaultPathEnv is deliberately empty on Windows as the default path will be set by -// the container. Docker has no context of what the default path should be. -const DefaultPathEnv = "" - // CheckSystemDriveAndRemoveDriveLetter verifies and manipulates a Windows path. // This is used, for example, when validating a user provided path in docker cp. // If a drive letter is supplied, it must be the system drive. The drive letter