New environment configuration file: /etc/devcontainer

[felipecrs]

A new option in devcontainer.json:

{
  "sourceEtcDevcontainer": true // defaults to true
}

This option makes the Remote - Containers extension always execute commands in the container using the following syntax:

$ docker exec ubuntu sh -c "test -f /etc/devcontainer && . /etc/devcontainer; echo 'hello world'"

Unless the user sets it to false.

This will not interfere in userEnvProbe behavior.

The /etc/devcontainer file must be a shell script compatible with Bourne Shell /bin/sh.

This would enable us to have a clean and unique environment configuration file that would work across all the users (root and non-root), and all the shells, without the caveats of /etc/profile or bash.bashrc.

So global environment variables can be set using scripts and not only through the Dockerfile's ENV statement, or the devcontainer.json's containerEnv, thus making the installation scripts easier to use since they would not require the user to manually inject some statements in the Dockerfile.

This would mainly benefit the scripts from script-library on vscode-dev-containers repository, which are used for doing common customizations on the containers. As explained here (bullet number 2).

[chrmarti]

Does "userEnvProbe" in the devcontainer.json not solve this?

[felipecrs]

I'll test it soon and let you know. Thanks.

[felipecrs]

@chrmarti Yes, this is what I was looking for, but with a caveat.

Without "userEnvProbe": "loginInteractiveShell" it does not work:

But with "userEnvProbe": "loginInteractiveShell" it works as expected:

However, I was actually looking for an option loginShell and not loginInteractiveShell, which translates to sh -l and sh -l -i respectively.

And, I'm proposing to make loginShell option as the default value instead of none. The reason is:

This would mainly benefit the scripts from script-library on vscode-dev-containers repository, which are used for doing common customizations on the containers. As explained here (bullet number 2).

This would enable us to switch from:

ARG PYTHON_VERSION="3.8.3"
ARG PYTHON_INSTALL_PATH="/usr/local/python${PYTHON_VERSION}"
ENV PIPX_HOME="/usr/local/py-utils"
ENV PATH="${PYTHON_INSTALL_PATH}/bin:${PATH}:${PIPX_HOME}/bin"

RUN bash -c "$(curl -fsSL "https://raw.githubusercontent.com/microsoft/vscode-dev-containers/master/script-library/python-debian.sh")" -- "${PYTHON_VERSION}" "${PYTHON_INSTALL_PATH}" "${PIPX_HOME}"

to:

RUN bash -c "$(curl -fsSL "https://raw.githubusercontent.com/microsoft/vscode-dev-containers/master/script-library/python-debian.sh")"

[chrmarti]

"userEnvProbe" doesn't affect the building of the Docker image. Couldn't you change your Dockerfile to use RUN bash -l -c "..." if using a login shell allows you to shorten it?

Taking this as the feature request to add "loginShell" for "userEnvProbe".

[PavelSosin-320]

Why it is better than entry-point script? Container is not multitenant. Repetitive login can create sibling processes that will be never killed by init during the termination and become ghost OS processes. This is very dangerous.

[felipecrs]

"userEnvProbe" doesn't affect the building of the Docker image. Couldn't you change your Dockerfile to use RUN bash -l -c "..." if using a login shell allows you to shorten it?

@chrmarti This is not about that, this is about setting up container wide environment not by using ENV. By the way, if someone wants to do what you said, it's better to use SHELL [ "bash", "-l", "-c" ].

Did you check my example? With "userEnvProbe": "loginInteractiveShell", VSCode process was able to find the custom Python executable even without setting the ENV PATH on the Dockerfile.

[felipecrs]

Why it is better than entry-point script?

I don't understand how this is related to the entry point script. Remember that entry point script is not run on docker exec. I actually want to assure that entry point is executed by default, but it's a totally different history.

Container is not multitenant. Repetitive login can create sibling processes that will be never killed by init during the termination and become ghost OS processes. This is very dangerous.

I use login shell in my Linux environments since ever, and I don't see anything dangerous in /etc/profile. Even if something could be found (such as starting a process), this block would probably check if there is a command already started. Such as the ssh-agent.

Also, the login environment would be only applied to the command which we are executing, it would not affect other running shells for example.

That said, do you have any reference for what you said? Or an example?

[Chuxel]

@chrmarti I'm actually a +1 on doing this by default. There's actually quite a bit of code in vscode-dev-containers that is working around this particular scenario. The big problem is scripts that must be sourced to get the path to work correctly. SDKMAN, nvm, rvm all are like this. I'm adding things the containers install to the PATH when possible, but this only covers a limited set. In the SDKMAN case there are a number of tools it can install. Worse yet, rvm throws up warnings if things are in the path and the targets are not easily predictable (e.g. the gem path). To your point, we could probably add SHELL to work around it, but I could see someone trying to create their own dev container hitting this and being confused.

[chrmarti]

The issue with /etc/profile is that in some images (e.g., Alpine and Debian) it overwrites PATH (investigation https://github.com/microsoft/vscode-remote-release/issues/3224#issuecomment-650034161), so additions to PATH via ENV are lost (bug https://github.com/microsoft/vscode-remote-release/issues/544). Due to that we changed to run with no startup scripts sourced.

One option is to probe the environment with an interactive non-login shell ("userEnvProbe": "interactiveShell"), but that adds to the startup time (numbers on my machine https://github.com/microsoft/vscode-remote-release/issues/3224#issuecomment-656032893) so we decided to not do that by default.

[chrmarti]

@Chuxel Are there any workarounds that cannot be replaced with the recently added "userEnvProbe"?

[Chuxel]

@chrmarti I don't believe so. I need to do a more complete test. This last release there was enough change already that I opted to keep things as they were.

[chrmarti]

@Chuxel If it turns out that you have to use "userEnvProbe": "interactiveShell" on almost all definitions we should reconsider the default.

I expect "loginShell" "loginInteractiveShell" to be used less often due to some /etc/profile overwriting PATH. If those would be better choices, we could think about somehow merging the two PATHs. That is not trivial because the order of PATH entries matters and we might need another property for the user to configure (or turn off) that - so we need good reasons to (also) take that approach.

[felipecrs]

The problem of using "interactiveShell" is (as the name says) that the shell is interactive, which means that at any point, bash or zsh can prompt the user for something. Not ideal in this case.

@chrmarti merging the two PATHs seems to be an interesting approach. The order would need to be respected as it is today: Docker ENVs comes before the /etc/profile ones.

[Chuxel]

@felipecrs Typically CLIs are smart enough to determine when they should prompt even in an interactive shell due to piping. As @chrmarti mentions, .profile can do all sorts of crazy things and typically also fires .bashrc under the hood, so of the two, interactive generally gives the expected results.

[felipecrs]

@Chuxel 'typically' is not a rule, right? The rule is: interactive terminals may ask the user for input. But it will probably work, as you suggest.

And the /etc/bash.bashrc is only sourced by /etc/profile if running in an interactive shell (that's why it checks for $PS1).

By the way, you're talking about .profile and .bashrc, but I suppose you are referring to /etc/profile and /etc/bash.bashrc and not ~/.profile and ~/.bashrc, right?

In the end, I [again] suggest using the standard way to customize the system's environment on Linux: adding as many .sh files you want in the /etc/profile.d/ rather than editing /etc/bash.bashrc or even /etc/profile directly.

[felipecrs]

I also think that the all sorts of crazy things are not so crazy:

$ docker run --rm alpine sh -c 'cat /etc/profile; cat /etc/profile.d/*.sh'
cat: can't open '/etc/profile.d/*.sh': No such file or directory
export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
export PAGER=less
export PS1='\h:\w\$ '
umask 022

for script in /etc/profile.d/*.sh ; do
        if [ -r $script ] ; then
                . $script
        fi
done

$ docker run --rm debian sh -c 'cat /etc/profile; cat /etc/profile.d/*.sh'
# /etc/profile: system-wide .profile file for the Bourne shell (sh(1))
# and Bourne compatible shells (bash(1), ksh(1), ash(1), ...).

if [ "`id -u`" -eq 0 ]; then
  PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
else
  PATH="/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games"
fi
export PATH

if [ "${PS1-}" ]; then
  if [ "${BASH-}" ] && [ "$BASH" != "/bin/sh" ]; then
    # The file bash.bashrc already sets the default PS1.
    # PS1='\h:\w\$ '
    if [ -f /etc/bash.bashrc ]; then
      . /etc/bash.bashrc
    fi
  else
    if [ "`id -u`" -eq 0 ]; then
      PS1='# '
    else
      PS1='$ '
    fi
  fi
fi

if [ -d /etc/profile.d ]; then
  for i in /etc/profile.d/*.sh; do
    if [ -r $i ]; then
      . $i
    fi
  done
  unset i
fi
cat: '/etc/profile.d/*.sh': No such file or directory

$ docker run --rm ubuntu sh -c 'cat /etc/profile; cat /etc/profile.d/*.sh'
# /etc/profile: system-wide .profile file for the Bourne shell (sh(1))
# and Bourne compatible shells (bash(1), ksh(1), ash(1), ...).

if [ "${PS1-}" ]; then
  if [ "${BASH-}" ] && [ "$BASH" != "/bin/sh" ]; then
    # The file bash.bashrc already sets the default PS1.
    # PS1='\h:\w\$ '
    if [ -f /etc/bash.bashrc ]; then
      . /etc/bash.bashrc
    fi
  else
    if [ "`id -u`" -eq 0 ]; then
      PS1='# '
    else
      PS1='$ '
    fi
  fi
fi

if [ -d /etc/profile.d ]; then
  for i in /etc/profile.d/*.sh; do
    if [ -r $i ]; then
      . $i
    fi
  done
  unset i
fi
# Make sure the locale variables are set to valid values.
eval $(/usr/bin/locale-check C.UTF-8)

It seems that the only problem is thePATH.

[Chuxel]

@Chuxel 'typically' is not a rule, right?

@felipecrs Yep! That's why the property exists - so you can adapt to your situation.

[felipecrs]

@Chuxel Yeah, this is true. But the point of this issue is to come up with a better default approach, not only for my situation.

The concerns raised about relying on the /etc/profile (loginShell) are valid. Fixing up PATH variable does not seem right to me, too intrusive, which makes this option not feasible.

And as I said, relying on interactiveShell isn't a good idea as well, as I said previously. This would also make bashrc to be sourced twice if the user opens a new bash under VSCode. Besides that, bash.bashrc and zshrc can also do all sorts of crazy things, and we don't want that, we just want to load our environment customizations.

So I think I have a better proposal:

A new option in devcontainer.json:

{
  "sourceEtcDevcontainer": true // defaults to true
}

This option makes the Remote - Containers extension always execute commands in the container using the following syntax:

$ docker exec ubuntu sh -c "test -f /etc/devcontainer && . /etc/devcontainer; echo 'hello world'"

Unless the user sets it to false.

This will not interfere in userEnvProbe behavior.

The /etc/devcontainer file must be a shell script compatible with Bourne Shell /bin/sh.

This would enable us to have a clean and unique environment configuration file that would work across all the users (root and non-root), and all the shells, without the caveats of /etc/profile or bash.bashrc.

@Chuxel @chrmarti could you please review this new proposal?

[chrmarti]

Note that the 'userEnvProbe' only starts a corresponding shell and uses printenv to extract the environment variables, it does not use that shell for anything else. So bashrc is sourced twice, but not in the same shell or a subshell. That should not cause the trouble you anticipate above. VS Code is using the same approach locally (except there it is always an interactive login shell and you cannot disable it).

We would probably use the same approach (printenv) to extract the environment variables if we added support for /etc/devcontainer, so we only source it once and are not forced to always use a shell.

[smhc]

Looks like a duplicate of https://github.com/microsoft/vscode-remote-release/issues/3585

[felipecrs]

Looks like a duplicate of https://github.com/microsoft/vscode-remote-release/issues/3585

True, pretty much the same. I'll update the PR title and description to match https://github.com/microsoft/vscode-remote-release/issues/3593#issuecomment-690826874, so they'll be different.

[felipecrs]

I rewrote this Issue.

[felipecrs]

Another thing, BASH_ENV environment variable can be used to specify bash rc files that should run if not in interactive or login mode. So, one could set in Dockerfile like:

ENV BASH_ENV="/etc/devcontainer"
SHELL ["/bin/bash", "-c"]
RUN echo "/etc/devcontainer will be sourced"

As it's an ENV, it will remain in the container and it will be used even when "envProbe": "none" (which is the default). It will not affect the terminals opened inside of VS Code, as BASH_ENV does not get executed when in interactive mode. This means that it's still needed to do something like:

RUN printf '%s\n' '' 'test -f /etc/devcontainer && . /etc/devcontainer' | sudo tee /etc/bash.bashrc /etc/zsh/zshrc

[QAston]

One problem with userEnvProbe is that it's not available for containers you attach to using attach to existing container option https://github.com/microsoft/vscode-remote-release/issues/6226 , /etc/devcontainer config file would potentially solve that.

[chrmarti]

@QAston You can open the config file associated with the 'attach' container using F1 > Remote-Containers: Open Configuration File and add "userEnvProbe" to that. The default is "loginInteractiveShell".

[QAston]

@chrmarti thanks, I didn't notice it because it's not documented in the reference: https://code.visualstudio.com/docs/remote/devcontainerjson-reference#_attached-container-configuration-reference

[chrmarti]

@QAston Thanks, opened a PR for the documentation. (https://github.com/microsoft/vscode-docs/pull/5119)