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

install: Check for a valid umask before running multi-user installer. #2643

Closed
wants to merge 2 commits into from

Conversation

buecking
Copy link

Fixes #2377 and #1560. I tested this on Linux (Ubuntu 16.04). I'm not sure if the arguments for sed on darwin are different, and I don't have a mac/bsd machine to test on.

@edolstra
Copy link
Member

Instead of checking for a correct umask, isn't it better to just set the correct umask (e.g. umask 0022)?

@buecking
Copy link
Author

buecking commented Feb 2, 2019

Instead of checking for a correct umask, isn't it better to just set the correct umask (e.g. umask 0022)?

There are potential consequences, side-affects, when setting your umask which was why I failed with an error.

I would be more inclined to just set the correct directory permissions on /nix rather than set the umask to 0022 and hope that those permissions are acceptable for all files/dirs touched by the installer.

@buecking
Copy link
Author

buecking commented Feb 6, 2019

I made changes to nix-install-matrix to check for regressions, but I wasn't able to run the tests on my laptop or devbox.

I attempted to extend the README to include my use case:

+## Pre-release
+
+In this example we will clone a pending PR, build a pre-release, then
+check this pre-release for regressions.
+
+We need to build a release tarball in order to run our tests. The
+release tarball contains both an installer and the required binary
+packages to install and run NixOS.
+
+```
+git clone git@github.com:contributor/nix.git --branch XXX
+cd nix && nix-build release.nix -A binaryTarball.x86_64-linux
+```
+
+
+If the build finishes successfully it should print the location of the
+tarball on the last line. Unpack the tarball and start a simple
+webserver to serve these data during our tests.
+
+```
+tar axf nix-2.3pre6621_92d08c0-x86_64-linux.tar.bz2 -C /tmp/
+cd /tmp/nix-2.3pre6621_92d08c0-x86_64-linux && python -m SimpleHTTPServer
+# Confirm the URL is working
+curl --output /dev/null --silent --head --fail \
+    "http://localhost:8000/install" || echo No Page Found!
+```
+
+Update `installUrl` in matrix.nix to point to our URL
+
+```
+  installScripts = let
+    installUrls = {
+      pre =  "http://localhost:8000/install";
+      stable = "https://nixos.org/nix/install";
+      "2.0.4" = "https://nixos.org/releases/nix/nix-2.0.4/install";
+    };
+    installUrl = installUrls.pre;
+```
+
+ XXX Write Me
+
+  1. Tweak test script to run preflight checks before creating a release tarball,
+    this step is very long so we want to get it right the first time. Suggest alternate methods
+   for saving time when building the tarball.
+ 
+ 2. Have the test script install the following packages (assume there is no bootstrap for using nixpkgs?) in which case these need to be installed with your distro's package manger.

+wget -q https://www.virtualbox.org/download/oracle_vbox_2016.asc -O- | sudo apt-key add -
+wget -q https://www.virtualbox.org/download/oracle_vbox.asc -O- | sudo apt-key add -
+sudo add-apt-repository "deb http://download.virtualbox.org/virtualbox/debian `lsb_release -cs` contrib"
+sudo apt-get update
+sudo apt-get install virtualbox-5.2
+    sudo apt install qemu-kvm libvirt-bin -- why can't we use nixos?
+    sudo apt install vagrant
+    sudo usermod -a -G libvirtd $USER
+
+  3. System set-up requirements
+    i. $USER belongs to libvirtd group
+    ii.  umask is 022
+

Here was the additional test case I added. I wasn't sure if I should check the umask change on every distro or.

I thought I would be able to use file:// protocol instead of http:// in maxtix.nix, but the individual tests could not access this url, and for good reason. So there's a requirement that one must have access to a webserver in order to host the release tarball.

diff --git a/matrix.nix b/matrix.nix
index b5cf920..d028da3 100644
--- a/matrix.nix
+++ b/matrix.nix
@@ -1,7 +1,7 @@
 {
   installScripts = let
     installUrls = {
-      pre = "https://nixos.org/releases/nix/nix-2.1pre6385_d16ff76c/install";
+      pre = "http://127.0.0.1:8000/install";
       stable = "https://nixos.org/nix/install";
       "2.0.4" = "https://nixos.org/releases/nix/nix-2.0.4/install";
     };
@@ -194,6 +194,15 @@
       '';
     };
 
+    "ubuntu-16-04-non-standard-umask" = {
+      image = "ubuntu/xenial64";
+      preInstall = ''
+        apt-get update
+        apt-get install -y curl
+        echo "umask 027 >> /etc/profile"
+      '';
+    };
+
     "ubuntu-14-04" = {
       image = "ubuntu/trusty64";
       preInstall = ''

@buecking buecking closed this Apr 6, 2020
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.

Installer assumes liberal umask
3 participants