Skip to content

Commit 70fb3d0

Browse files
committed
fix: wcow: add powershell.exe dir to default PATH
The current default `PATH` has been set to the most basic subset for both `nanoserver` and `servercore`. ```go const DefaultPathEnvWindows = "c:\\Windows\\System32;c:\\Windows; ``` The path to `powershell.exe` is conspicuously missing hence breaking the developer experience for most workloads that depend on PS. This fix proposes to append `;C:\\Windows\\System32\\WindowsPowerShell\\v1.0` to that list to support PS scenarios, that make a big chunk of workloads, especially legacy ones, migrating from on-prem to cloud. Fixes moby#4901, microsoft/Windows-Containers#500 Ref moby#3158 Implication for nanoserver: =========================== This does not any way break the experiences for those using `nanoserver` base image, as much as PS and the `C:\\Windows\\System32\\WindowsPowerShell\\v1.0` path is not present on `nanoserver`. Transition users to using `ENV`: ============================== Users coming from classic `docker build` will need to transition from using `RUN setx /M` to `ENV` as a way of persisting environment variables in the images. We can take a hit on not supporting backward compatibility for `RUN setx /M`, as opposed to not supporting both `RUN setx /M` and `RUN powershell` as it is the case right now. Alternative considered: ======================= We thought of an option to ask the Windows platform team to add a default `Config.Env.PATH` in the config file for the base image. However, this causes a regression for `RUN setx /M` on the classic `docker build`. There could be a couple of more people too that might be depending on `Config.Env = null` as it is currently. See `docker inspect mcr.microsoft.com/windows/servercore:ltsc2022`. Here is the regression details when we set `ENV` in the base image. ``` PS> type .\Dockerfile FROM mcr.microsoft.com/windows/servercore:ltsc2022 AS newbase ENV PATH="C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps" FROM newbase RUN setx /M PATH "C:\my\path;%PATH%" RUN echo %PATH% PS> docker build --no-cache -t profnandaa/servercore-path-tests . Sending build context to Docker daemon 2.048kB Step 1/5 : FROM mcr.microsoft.com/windows/servercore:ltsc2022 AS newbase ---> e64ba0f4256b Step 2/5 : ENV PATH="C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps" ---> Running in ab3dc4921d7d ---> Removed intermediate container ab3dc4921d7d ---> f57b0a2d0e28 Step 3/5 : FROM newbase ---> f57b0a2d0e28 Step 4/5 : RUN setx /M PATH "%PATH%;C:\my\path;" ---> Running in 6ca6171334a0 SUCCESS: Specified value was saved. ---> Removed intermediate container 6ca6171334a0 ---> 6d2870e2f91d Step 5/5 : RUN echo %PATH% ---> Running in 785633e2a31c C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps ^ | ~~~~~~~~~ notice "C:\my\path" is missing, meaning `setx /M` is not persisting as before. ---> Removed intermediate container 785633e2a31c ---> ed490181b903 Successfully built ed490181b903 Successfully tagged profnandaa/servercore-path-tests:latest ``` Current `RUN setx /M` behavior: ``` PS> type .\Dockerfile FROM mcr.microsoft.com/windows/servercore:ltsc2022 RUN setx /M PATH "%PATH%;C:\my\path;" RUN echo %PATH% PS> docker build --no-cache -t profnandaa/servercore-path-tests . Sending build context to Docker daemon 2.048kB Step 1/3 : FROM mcr.microsoft.com/windows/servercore:ltsc2022 ---> e64ba0f4256b Step 2/3 : RUN setx /M PATH "C:\my\path;%PATH%" ---> Running in 5502dd679495 SUCCESS: Specified value was saved. ---> Removed intermediate container 5502dd679495 ---> 0b59f38e2da4 Step 3/3 : RUN echo %PATH% ---> Running in 3bacb0b27bad C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps;C:\my\path; ^ |~~~~ `setx /M` persists. ---> Removed intermediate container 3bacb0b27bad ---> cda1b4cd27ff Successfully built cda1b4cd27ff Successfully tagged profnandaa/servercore-path-tests:latest ``` `setx` vs. `ENV` has been discussed in details in moby#5445 Signed-off-by: Anthony Nandaa <[email protected]>
1 parent f118814 commit 70fb3d0

File tree

2 files changed

+46
-2
lines changed

2 files changed

+46
-2
lines changed

frontend/dockerfile/dockerfile_test.go

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ var allTests = integration.TestFuncs(
212212
testLocalCustomSessionID,
213213
testTargetStageNameArg,
214214
testStepNames,
215+
testPowershellInDefaultPathOnWindows,
215216
)
216217

217218
// Tests that depend on the `security.*` entitlements
@@ -1812,7 +1813,7 @@ COPY Dockerfile .
18121813
entrypoint []string
18131814
env []string
18141815
}{
1815-
{p: "windows/amd64", entrypoint: []string{"cmd", "/S", "/C", "foo bar"}, env: []string{"PATH=c:\\Windows\\System32;c:\\Windows"}},
1816+
{p: "windows/amd64", entrypoint: []string{"cmd", "/S", "/C", "foo bar"}, env: []string{"PATH=c:\\Windows\\System32;c:\\Windows;C:\\Windows\\System32\\WindowsPowerShell\\v1.0"}},
18161817
{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"}},
18171818
} {
18181819
t.Run(exp.p, func(t *testing.T) {
@@ -1949,6 +1950,49 @@ COPY --from=base /out/ /
19491950
require.Equal(t, "value:final", string(dt))
19501951
}
19511952

1953+
func testPowershellInDefaultPathOnWindows(t *testing.T, sb integration.Sandbox) {
1954+
integration.SkipOnPlatform(t, "!windows")
1955+
1956+
f := getFrontend(t, sb)
1957+
1958+
// just testing that the powershell path is in PATH
1959+
// but not testing powershell itself since it will need
1960+
// servercore image that is too bulky for just one single test.
1961+
dockerfile := []byte(`
1962+
FROM nanoserver
1963+
USER ContainerAdministrator
1964+
RUN echo %PATH% > env_path.txt
1965+
`)
1966+
dir := integration.Tmpdir(
1967+
t,
1968+
fstest.CreateFile("Dockerfile", dockerfile, 0600),
1969+
)
1970+
c, err := client.New(sb.Context(), sb.Address())
1971+
require.NoError(t, err)
1972+
defer c.Close()
1973+
1974+
destDir := t.TempDir()
1975+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
1976+
Exports: []client.ExportEntry{
1977+
{
1978+
Type: client.ExporterLocal,
1979+
OutputDir: destDir,
1980+
},
1981+
},
1982+
LocalMounts: map[string]fsutil.FS{
1983+
dockerui.DefaultLocalNameDockerfile: dir,
1984+
dockerui.DefaultLocalNameContext: dir,
1985+
},
1986+
}, nil)
1987+
require.NoError(t, err)
1988+
1989+
dt, err := os.ReadFile(filepath.Join(destDir, "env_path.txt"))
1990+
require.NoError(t, err)
1991+
1992+
envPath := string(dt)
1993+
require.Contains(t, envPath, "C:\\Windows\\System32\\WindowsPowerShell\\v1.0")
1994+
}
1995+
19521996
func testExportMultiPlatform(t *testing.T, sb integration.Sandbox) {
19531997
integration.SkipOnPlatform(t, "windows")
19541998
workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureMultiPlatform)

util/system/path.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ const DefaultPathEnvUnix = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/s
1616
// DefaultPathEnvWindows is windows style list of directories to search for
1717
// executables. Each directory is separated from the next by a colon
1818
// ';' character .
19-
const DefaultPathEnvWindows = "c:\\Windows\\System32;c:\\Windows"
19+
const DefaultPathEnvWindows = "c:\\Windows\\System32;c:\\Windows;C:\\Windows\\System32\\WindowsPowerShell\\v1.0"
2020

2121
func DefaultPathEnv(os string) string {
2222
if os == "windows" {

0 commit comments

Comments
 (0)