Bash script prepares virtual environment

Question

I have a few Flask projects that I want to install, each in their own virtual environment. The approach is derived from this tutorial. The steps to create the VE's are very similar, so they're easy to automate. So I wrote this, as a first step. It seems to work OK:

#! /bin/bash  

if [ $# -lt 2 ]  
  then  
    echo -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name"  
    exit  
fi  

echo project name: $1  
echo project user: $2  

# create the webapp user and its home directory, and give it ownership  
sudo useradd --system --gid webapps --shell /bin/bash --home /mnt/u1/$1 $2  
sudo mkdir -p /mnt/u1/$1  
sudo chown $2 /mnt/u1/$1  

# become the webapp user, create a virtual environment, start it, and install gunicorn and flask  
sudo su $2 -c "cd ~; virtualenv ./venv"  
sudo su $2 -c "cd ~;. venv/bin/activate; pip install gunicorn flask"

echo -e "Done! Start with:\n$ sudo su - $2\n$ . venv/bin/activate"  
echo -e "Or undo the whole kaboodle with:\n$ sudo userdel -r $2"  

# at this point, manual configuration is necessary, which is different for each project.  

I'm fairly new to bash programming, so I'm open to suggestions.
Is this good practice at all?
Any serious pitfalls I have overlooked?
Any room for improvement?

Answer 1

There are a number of good practice items you are missing here, that would benefit your script.

First up, you should name your variables/parameters. The check you have to ensure that the project name and user are specified is OK, but you should rather pull the $1 and $2 values in to their own variables, say pname and puser.

pname=$1
puser=$2

Then it becomes easier to read the code... $pname is a lot easier to understand than $1.

The second issue I see is that you don't check for error conditions. A relatively easy handler is to add the exit-on-error flag to your shell script. Consider adding set -e at the beginning of the script (or a more elaborate system, but your script should be fine with set -e. For more information see: What does set -e mean in a bash script?

The third issue is a small one - why are you using both sudo and su in the same command..... sudo su .... seems rhetorical. Just use sudo --user $puser ....

Another little issue is that you should exit with a non-zero if the parameters are wrong (use exit 1 instead of exit)...

echo -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name"  
exit

should be:

echo -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name"  
exit 1 

Yet another small issue is that it's normal to not have .sh as the extension for exectuable shell scripts. Why call your script mkve.sh instead of mkve? Also, you call it two different things in the error message, mkve.sh and mkve.bash.... which is right?

Finally, it is unconventional to put home directories in the /mnt folder. That folder has a special purpose when mounting file systems that are not normally mounted (things like CD's, USB drives, etc.). Using that folder for these home directories will probably lead to unexpected issues if/when someone mounts a drove over /mnt and all your home folders disappear.

The end code would be something like (untested):

#! /bin/bash  

# exit if there's a failed command
set -e

if [ $# -lt 2 ]  
  then  
    echo -e "mkve - make virtual environment\nUsage: mkve project_name user_name"  
    exit 2
fi

pname=$1
puser=$2

echo project name: $pname
echo project user: $puser

# create the webapp user and its home directory, and give it ownership  
sudo useradd --system --gid webapps --shell /bin/bash --home /ves/$pname $puser  
sudo mkdir -p /mnt/u1/$pname
sudo chown $puser /mnt/u1/$pname

# become the webapp user, create a virtual environment, start it, and install gunicorn and flask  
sudo --user $puser bash -c "cd ~; virtualenv ./venv"  
sudo --user $puser bash -c "cd ~;. venv/bin/activate; pip install gunicorn flask"

echo -e "Done! Start with:\n$ sudo -u $puser -i\n$ . venv/bin/activate"  
echo -e "Or undo the whole kaboodle with:\n$ sudo userdel -r $puser"  

# at this point, manual configuration is necessary, which is different for each project.