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

Haskell for non root #74

Merged

Conversation

DaneWeber
Copy link
Contributor

Fix

  • Temporarily change HOME to _REMOTE_USER_HOME for the execution of the install script.
  • Recursively chown back to the _REMOTE_USER.

I've also auto-formatted the file, but separated that into a different commit in case you don't want those changes.

Preexisting Behavior

The Haskell feature runs the install such that Cabal, Stack, etc. are installed in /root and are not useful when running the dev container in VS Code (or presumably other use cases).

The GHCup install instructions mention that the commands should be run as non-root:

These commands should be run as non-root/non-admin user.

Reproduction

The following .devcontainer.json illustrates the issue (and is what VS Code creates when selecting default values for an Ubuntu devcontainer with the Haskell feature added):

{
	"name": "Ubuntu",
	"image": "mcr.microsoft.com/devcontainers/base:jammy",
	"features": {
		"ghcr.io/devcontainers-contrib/features/haskell:1": {}
	}
}

After building and opening VS Code in the devcontainer, a new terminal produces the following:

vscode ➜ /workspaces/haskell_example $ ghcup --version
bash: ghcup: command not found
vscode ➜ /workspaces/haskell_example $ cabal --version
bash: cabal: command not found
vscode ➜ /workspaces/haskell_example $ stack --version
bash: stack: command not found
vscode ➜ /workspaces/haskell_example $ ls -AF ~/
.bash_logout  .bashrc  .config/  .gitconfig  .gnupg/  .oh-my-zsh/  .profile  .ssh/  .vscode-server/  .zshrc
vscode ➜ /workspaces/haskell_example $ sudo ls -AF /root
.bashrc  .cabal/  .cache/  .config/  .local/  .oh-my-zsh/  .profile  .stack/  .zshrc

Note the existence of .cabal and .stack in /root.

In the preexisting condition, you can verify that installing as the user succeeds by executing the following:

curl --proto '=https' --tlsv1.2 -sSf https://get-ghcup.haskell.org | BOOTSTRAP_HASKELL_NONINTERACTIVE=1 BOOTSTRAP_HASKELL_GHC_VERSION=latest BOOTSTRAP_HASKELL_CABAL_VERSION=latest BOOTSTRAP_HASKELL_INSTALL_STACK=1 BOOTSTRAP_HASKELL_INSTALL_HLS=1 BOOTSTRAP_HASKELL_ADJUST_BASHRC=P $SHELL

After everything is downloaded and installed, you'll have to source ~/.bashrc and then you can see the success:

vscode ➜ /workspaces/haskell_example $ ghcup --version
bash: ghcup: command not found
vscode ➜ /workspaces/haskell_example $ source ~/.bashrc 
vscode ➜ /workspaces/haskell_example $ ghcup --version
The GHCup Haskell installer, version v0.1.18.0
vscode ➜ /workspaces/haskell_example $ cabal --version
cabal-install version 3.8.1.0
compiled using version 3.8.1.0 of the Cabal library 
vscode ➜ /workspaces/haskell_example $ stack --version
Version 2.9.1, Git revision 409d56031b4240221d656db09b2ba476fe6bb5b1 x86_64 hpack-0.35.0
vscode ➜ /workspaces/haskell_example $ ls -AF ~/
.bash_logout  .bashrc  .cabal/  .config/  .ghcup/  .gitconfig  .gnupg/  .oh-my-zsh/  .profile  .ssh/  .stack/  .vscode-server/  .zshrc

@jcbhmr
Copy link
Contributor

jcbhmr commented Nov 24, 2022

👍 Thanks for the contribution! I think that this is a good fix that should be merged. I do have some nitpicks though:

Questions:

  1. Is this a change that warrants updating the version specifier in devcontainer-feature.json?
  2. Is there a reason you overwrite the $HOME variable instead of switching users with sudo?

@danielbraun89 What can be done to improve the tests? Anything? Is there a way to test that this doesn't regress?

I was able to use sudo which seems more "proper" than mutating the $HOME variable.

sudo -iu "$_REMOTE_USER" <<EOF
  # https://www.haskell.org/ghcup/
  curl --proto '=https' --tlsv1.2 -sSf https://get-ghcup.haskell.org | sh
EOF
{
  "image": "mcr.microsoft.com/devcontainers/universal:linux",
  "features": {
    "ghcr.io/jcbhmr/devcontainers-contrib-features/haskell:latest": {} // From pr74 branch
  },
}

image

Code changes
# The installation script is designed to be run by the non-root user
# The files need to be in the remote user's ~/ home directory
-ROOT_HOME="${HOME}"
-export HOME="${_REMOTE_USER_HOME}"
-
-curl --proto '=https' --tlsv1.2 -sSf https://get-ghcup.haskell.org | $SHELL
-
-export HOME="${ROOT_HOME}"
-chown -R "${_REMOTE_USER}:${_REMOTE_USER}" "${_REMOTE_USER_HOME}"
+# So, how do we switch users? We use 'sudo -iu <username>' to get a
+# login shell of another user! We use $_REMOTE_USER as defined in
+# a spec proposal (but still implemented in Codespaces): https://github.com/devcontainers/spec/blob/main/proposals/features-user-env-variables.md
+# Here's some more examples using it: https://github.com/search?q=org%3Adevcontainers+_REMOTE_USER&type=code
+# We also use /bin/sh as defined in the script hash-bang line instead of $SHELL.
+sudo -iu "$_REMOTE_USER" <<EOF
+  # https://www.haskell.org/ghcup/
+  curl --proto '=https' --tlsv1.2 -sSf https://get-ghcup.haskell.org | sh
+EOF

Relevant StackOverflow questions:

  1. How do I use su to execute the rest of the bash script as that user? -- where I got the sudo -iu <username> from
  2. sudo as another user with their environment -- verify that it sets the $HOME when you pass -i

@jcbhmr
Copy link
Contributor

jcbhmr commented Nov 26, 2022

A note: I added my own PR against DaneWeber's PR branch in DaneWeber#2 with my suggested changes.

The open question remains about the version bump.

@danielbraun89
Copy link
Member

danielbraun89 commented Nov 26, 2022

thanks @DaneWeber for finding this bug! seems like the testing method we inherited from the standard feature-starter is running with root user, Ill open another issue regarding improving the test to use non-root users as well. this will also catch additional issues as #80

@danielbraun89
Copy link
Member

@jcbhmr I agree sudo method is quite more elegant... how about ill merge this one as is, and then you PR the sudo implementation directly? Or do you prefer @DaneWeber approve it on his?

@jcbhmr
Copy link
Contributor

jcbhmr commented Nov 26, 2022

@danielbraun89 It doesn't really matter. Whatever works best for you 🤷‍♂️. I'll work with whatever.

@danielbraun89
Copy link
Member

Ok so lets go with the double merge in order to minimize the duration this bug stays open
btw this absolutely will require version update - as the release script is skipping the package publishing step if a package with the same version already exist

@danielbraun89 danielbraun89 merged commit f16649d into devcontainers-contrib:main Nov 26, 2022
@DaneWeber
Copy link
Contributor Author

Happy to see this merged! I'd like to mention that I agree with @jcbhmr's improvement (assuming it's fine for these install scripts) and that I was overly cautious about using su or sudo since I'm just learning about the dev container Features and wasn't sure exactly how things were working under the hood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants