Skip to content

Commit

Permalink
Don't set a default PATH for Windows
Browse files Browse the repository at this point in the history
Commit ecf070a updated DefaultPathEnv to return
platform specific values, but also defined a default PATH for Windows.

However, Windows does not have a default PATH, and the PATH to use must be set
by the container image. Also see moby/moby@3c177dc,
which removed the default PATH on Windows;

> This 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.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed Nov 4, 2023
1 parent 6a0cd7c commit 7399b8d
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 22 deletions.
16 changes: 7 additions & 9 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1390,17 +1390,15 @@ func testClientGatewayContainerPlatformPATH(t *testing.T, sb integration.Sandbox
Platform *pb.Platform
Expected string
}{{
"default path",
nil,
utilsystem.DefaultPathEnvUnix,
Name: "default path",
Expected: utilsystem.DefaultPathEnvUnix,
}, {
"linux path",
&pb.Platform{OS: "linux"},
utilsystem.DefaultPathEnvUnix,
Name: "linux path",
Platform: &pb.Platform{OS: "linux"},
Expected: utilsystem.DefaultPathEnvUnix,
}, {
"windows path",
&pb.Platform{OS: "windows"},
utilsystem.DefaultPathEnvWindows,
Name: "windows path",
Platform: &pb.Platform{OS: "windows"},
}}

for _, tt := range tests {
Expand Down
4 changes: 3 additions & 1 deletion client/llb/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ func (e *ExecOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []
} else if e.constraints.Platform != nil {
os = e.constraints.Platform.OS
}
env = env.SetDefault("PATH", system.DefaultPathEnv(os))
if v, ok := system.DefaultPathEnv(os); ok {
env = env.SetDefault("PATH", v)
}
} else {
addCap(&e.constraints, pb.CapExecMetaSetsDefaultPath)
}
Expand Down
4 changes: 3 additions & 1 deletion exporter/containerimage/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,9 @@ func defaultImageConfig() ([]byte, error) {
img.Variant = pl.Variant
img.RootFS.Type = "layers"
img.Config.WorkingDir = "/"
img.Config.Env = []string{"PATH=" + system.DefaultPathEnv(pl.OS)}
if env, ok := system.DefaultPathEnv(pl.OS); ok {
img.Config.Env = []string{"PATH=" + env}
}
dt, err := json.Marshal(img)
return dt, errors.Wrap(err, "failed to create empty image config")
}
Expand Down
4 changes: 3 additions & 1 deletion frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,9 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS

// 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(d.platform.OS))
if env, ok := system.DefaultPathEnv(d.platform.OS); ok {
d.image.Config.Env = append(d.image.Config.Env, "PATH="+env)
}
}

// initialize base metadata from image conf
Expand Down
4 changes: 3 additions & 1 deletion frontend/dockerfile/dockerfile2llb/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ func emptyImage(platform ocispecs.Platform) image.Image {
img.Variant = platform.Variant
img.RootFS.Type = "layers"
img.Config.WorkingDir = "/"
img.Config.Env = []string{"PATH=" + system.DefaultPathEnv(platform.OS)}
if env, ok := system.DefaultPathEnv(platform.OS); ok {
img.Config.Env = []string{"PATH=" + env}
}
return img
}
4 changes: 3 additions & 1 deletion frontend/gateway/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,9 @@ 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(gwCtr.platform.OS))
if env, ok := utilsystem.DefaultPathEnv(gwCtr.platform.OS); ok {
procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "PATH", env)
}
if req.Tty {
procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "TERM", "xterm")
}
Expand Down
4 changes: 3 additions & 1 deletion solver/llbsolver/ops/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,9 @@ func (e *ExecOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu
if e.platform != nil {
currentOS = e.platform.OS
}
meta.Env = addDefaultEnvvar(meta.Env, "PATH", utilsystem.DefaultPathEnv(currentOS))
if env, ok := utilsystem.DefaultPathEnv(currentOS); ok {
meta.Env = addDefaultEnvvar(meta.Env, "PATH", env)
}

secretEnv, err := e.loadSecretEnv(ctx, g)
if err != nil {
Expand Down
34 changes: 27 additions & 7 deletions util/system/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,36 @@ import (
// ':' 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"
// DefaultPathEnvWindows is deliberately empty on Windows and the default path
// must be set by the image. BuildKit has no context of what the default
// path should be.
//
// Deprecated: Windows images must not have a default PATH set.
const DefaultPathEnvWindows = ""

func DefaultPathEnv(os string) string {
// DefaultPathEnv returns the default value for the PATH environment-variable
// if available for the given operating-system. The second return variable
// indicates whether a default exists.
//
// On Linux/Unix, this returns [DefaultPathEnvUnix]. On Windows, no default
// exists, and PATH must not be set as its image-dependent, and must be
// configured through other means.
//
// More details about default PATH values for Windows images can be found
// in the following GitHub discussions:
//
// - [moby/moby#13833]
// - [moby/buildkit#3158]
// - [containerd/containerd#9118]
//
// [moby/moby#13833]: https://github.com/moby/moby/pull/13833
// [moby/buildkit#3158]: https://github.com/moby/buildkit/pull/3158
// [containerd/containerd#9118]: https://github.com/containerd/containerd/pull/9118
func DefaultPathEnv(os string) (string, bool) {
if os == "windows" {
return DefaultPathEnvWindows
return "", false
}
return DefaultPathEnvUnix
return DefaultPathEnvUnix, true
}

// NormalizePath cleans the path based on the operating system the path is meant for.
Expand Down

0 comments on commit 7399b8d

Please sign in to comment.