Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: WCOW: add powershell.exe dir to default PATH for backward compatibiliy #5446

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Commits on Nov 6, 2024

  1. 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]>
    profnandaa committed Nov 6, 2024
    Configuration menu
    Copy the full SHA
    af5e909 View commit details
    Browse the repository at this point in the history