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

Features: "User" and Installs to User Home Directory #91

Closed
virtualstaticvoid opened this issue Sep 7, 2022 · 9 comments
Closed

Features: "User" and Installs to User Home Directory #91

virtualstaticvoid opened this issue Sep 7, 2022 · 9 comments
Assignees
Labels
finalization Proposal to be made part of the spec proposal Still under discussion, collecting feedback
Milestone

Comments

@virtualstaticvoid
Copy link

virtualstaticvoid commented Sep 7, 2022

Hello,

I'm writing a feature which requires writing configuration files to the home directory of the user within the container.

Looking at the source code of existing implemented features as a reference, such as the anaconda feature, I encountered code such as the following, which has specific logic to determine the user (and thus the home directory location).

https://github.com/devcontainers/features/blob/a5fbdab3c096152dd3a318c0c23da9465815aa69/src/anaconda/install.sh#L29-L44

I wasn't able to find specific documentation in the proposal on a "contract" for feature developers; with details on a USERNAME environment variable being provided when executing the install.sh script. Furthermore, use of "magical" users such as automatic, auto, vscode, node, codespace or other (via /etc/passwd) to aid in the determination behaviour is something I wouldn't want to have to repeat in authoring my features.

Furthermore, the reliance on having one of these user exist in a given base image is tenuous and makes this logic quite fragile (and smelly).

Therefore I propose a better and surprisingly much simpler solution, inspired in part by how the vagrant shell provisioner works, as follows:

  1. The process which is executing the install.sh script, has access to the given devcontainer.json file, which defines a remoteUser and thus can be used to provide the USERNAME environment variable to the script; sort of like a docker build argument, which would remove the need to have to infer the correct user.

  2. Since the feature install.sh script is typically executed as root, unless the base image switches the user, files installed to the home directory need to be chowned to make the files accessible to the user when the dev container is running. This can be solved by having an optional devcontainer-feature.json property, such as installAs, which is an enum type with values of root or remoteUser. When remoteUser is provided, install.sh is executed in the context of the user provided by the remoteUser property. This is similar in concept to how the privileged setting of the vagrant shell provisioner alters the context. This does place a limitation on what the install.sh script can do, so authors should include an assertion for root if required for proper execution.

    https://github.com/devcontainers/features/blob/a5fbdab3c096152dd3a318c0c23da9465815aa69/src/terraform/install.sh#L35-L38

  3. To aid troubleshooting, since the onus is on the devcontainer.json author to ensure alignment of the user(s) contained in the base image and the remoteUser provided, a check should be performed before installing features, to assert that the given remoteUser exists and a nice helpful message if it doesn't.

@virtualstaticvoid virtualstaticvoid changed the title Features: Installs to User Home Directory Features: "User" and Installs to User Home Directory Sep 7, 2022
@Chuxel
Copy link
Member

Chuxel commented Sep 8, 2022

Yeah agree with this - In fact, #75 proposes to bring in first class support for creating them as well. Part of that would be both the remoteUser and containerUser would be available as env vars in Features as well. That way you can su <user> -c to operate fully in that context where needed. e.g., your feature can have a install-user.sh and install.sh can su <user> -c ./install-user.sh.

Would that hit what you are getting at here?

A first step could just be to add in the env vars though, I agree - FYI @chrmarti since he's been talking about this one. I'd love to drop all of the "automatic" logic since it generally works, but is not foolproof since the UID of the user could be something other than 1000.

As a FYI, there's also groups created in some Features to help manage privs for some of this, but that's generally due to not having #25 for people running on Linux natively.

@virtualstaticvoid
Copy link
Author

A first step could just be to add in the env vars though

Yeah, that would work. Feature authors can then assume their install.sh will always be executed as root, and if need be su <user> ... can be used to execute as the given user, and remoteUser and containerUser should both be provided as environment variables to the install.sh script.

@chrmarti
Copy link
Contributor

chrmarti commented Sep 9, 2022

You could update the existing home folders and the /etc/skel folder that is used when users are added:

E.g., adding a ~/.bar config:

for home in /etc/skel /home/*
do
	su $(stat -c '%U' "$home") -c "echo foo >'$home/.bar'"
done

@Chuxel
Copy link
Member

Chuxel commented Sep 9, 2022

Yeah, that would work. Feature authors can then assume their install.sh will always be executed as root,

Yep, this is indeed the case! This is important to allow you to install any OS dependencies w/o passwordless sudo.

https://github.com/devcontainers/cli/blob/6278ab6033ddd056d5ab9923b1b9970575433d51/src/spec-configuration/containerFeaturesConfiguration.ts#L213

However, I'm noticing this is not called out in the spec which we should fix for sure.

One other tip is that typically the user has a UID of 1000 at the time of the Dockerfile build (since that won't be modified until the container actually starts), so that is a good initial proxy and many commands allow passing in a UID instead of a user name. For rc updates that should affect everyone, updating /etc/bash.bashrc or /etc/zsh/zshrc (with that location varying slightly depending on distro) covers things.

@chrmarti What you are suggesting is one possible solution as well! Though, you'd also need to update /root since it's not under /home. However, installers do update these locations themselves, so working around the lack of visibility to the ultimate user becomes tribal knowledge. In some cases you also don't want to add things to multiple home folders due to size - e.g., if you use nvm's defaults, you'd end up needing to install it repeatedly (and using a symlink may or may not work and can result in odd privs). So I think it's reasonable to expect better first class support in Features for users like what is proposed in #75. A REMOTE_USER and CONTAINER_USER env var would go a long ways to helping - heck in concept we may only need one USERNAME var that is the effective user... whether that's the image, container user, or remote user. Most of the scripts at https://github.com/devcontainers/features are already setup to accept one.

Perhaps we should leave this one open to capture that request independent of #75?

@chrmarti
Copy link
Contributor

@Chuxel Makes sense, let's track passing the effective username to the feature scripts here.

@chrmarti
Copy link
Contributor

chrmarti commented Oct 6, 2022

Goal

Feature scripts run as the root user and sometimes need to know which user account the dev container will be used with.

(The dev container user can be configured through the remoteUser property in the devcontainer.json. If that is not set, the container user will be used.)

Proposal

Pass _REMOTE_USER and _CONTAINER_USER environment variables to the features scripts with _CONTAINER_USER being the container's user and _REMOTE_USER being the configured remoteUser. If no remoteUser is configured, _REMOTE_USER is set to the same value as _CONTAINER_USER.

Additionally the home folders of the two users are passed to the feature scripts as _REMOTE_USER_HOME and _CONTAINER_USER_HOME environment variables.

Notes

  • The container user can be set with containerUser in the devcontainer.json and image metadata, user in the docker-compose.yml, USER in the Dockerfile and can be passed down from the base image.

@stuartleeks
Copy link

I imagine that a common use of these in feature installation scripts will be to write to the user home folder in the container.
Could we include a USER_HOME variable that is set to /root for root user and /home/$REMOTE_USER otherwise? (Just thinking that it would save multiple feature implementations from including that logic)

@chrmarti
Copy link
Contributor

chrmarti commented Oct 6, 2022

Makes sense. Some suggest using getent passwd $USER | cut -d: -f6 or eval echo "~$USER", but these are not obvious.

We should probably include the home folder for both users. Also prefixing the variables with _ will make sure they do not collide with feature options. Updating the proposal above.

@chrmarti
Copy link
Contributor

chrmarti commented Oct 7, 2022

Continuing in #110.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
finalization Proposal to be made part of the spec proposal Still under discussion, collecting feedback
Projects
None yet
Development

No branches or pull requests

5 participants