4
\$\begingroup\$

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?

\$\endgroup\$
2
\$\begingroup\$

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. 
\$\endgroup\$
  • \$\begingroup\$ Many thanks! I've copied most of your changes. Any reason for exit 2, rather than exit 1? To answer your questions, I'm using .sh because I get syntax highlighting in my editor. Second, I'm using /mnt/u1 because I'm putting those apps on a USB-stick in a Raspberry Pi. Its system disk is an SD-card and I want to avoid frequent writes to it, see here. Had an SD-card bricked mid-project twice, so far. Very inconvenient. A USB stick is a bit easier to backup and replace (althought I admit that's debatable.) \$\endgroup\$ – RolfBly Jan 4 '17 at 19:54
  • \$\begingroup\$ echo -e is also an antipattern, maybe switch to printf instead. \$\endgroup\$ – tripleee Feb 9 '17 at 13:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.