Bash Database Backup to AWS S3

Question

I've written a bash script that backs up databases and uploads the backups to an S3 bucket. I have the bucket configured to delete backups that are older than 90 days.

Is there anything that can be improved in this script?

#!/usr/bin/env bash
# Backup MySQL databases to S3
# Follow the installation instructions listed on the user guid doc:
# http://docs.aws.amazon.com/cli/latest/userguide/installing.html
# Note backups will be deleted after 90 days. This is setup in the management tab:
# https://s3.console.aws.amazon.com/s3/buckets/backups.databases/?tab=management
# DB details
DB_USER=""
DB_PASSWORD=""
DB_HOST=""
DB_IGNORED=(Database mysql information_schema performance_schema cond_instances)
# S3 details
S3_BUCKET=backups.databases
S3_FOLDER=backups/databases
# Temp storage location
TMP_DIR=/tmp
# Check if dependencies exist
dependencies=(aws mysql mysqldump tar rm echo)
for dependency in "${dependencies[@]}"
do
 if [[ $(command -v "$dependency") = "" ]]
 then
 # Tell the user that the dependency is not installed, then exit the script
 $(which echo) "Dependency $dependency isn't installed. Install this dependency to continue."
 exit 0
 fi
done
DB_DATABASES=($($(which echo) "show databases;" | $(which mysql) -h ${DB_HOST} -p${DB_PASSWORD} -u ${DB_USER} 2> /dev/null))
# Filename for the backups
time=$($(which date) +%b-%d-%y-%H%M)
filename="backup-$time.tar.gz"
for database in "${DB_DATABASES[@]}"
do
 # Skip ignored databases
 if [[ "${DB_IGNORED[@]}" =~ "$database" ]]
 then
 continue
 fi
 # Dump database SQL schema to an SQL file stored in the temp directory
 $(which echo) "Backing up $database..."
 $(which mysqldump) -h ${DB_HOST} -u ${DB_USER} -p${DB_PASSWORD} ${database} > "$TMP_DIR/$database.sql" 2> /dev/null
 $(which tar) -cpzf "$TMP_DIR/$database-$filename" "$TMP_DIR/$database.sql" 2> /dev/null
 # Upload the exported SQL file to S3
 $(which echo) "Uploading to S3..."
 $(which aws) s3 cp "$TMP_DIR/$database-$filename" "s3://$S3_BUCKET/$S3_FOLDER/$database-$filename"
 $(which echo) "Database $database uploaded to S3"
 # Remove the temporary files
 $(which echo) "Cleaning up..."
 $(which rm) -f "$TMP_DIR/$database-$filename"
 $(which rm) -f "$TMP_DIR/$database.sql"
done

I had to add 2> /dev/null to the end of the mysql and mysqldump commands due to a warning message showing.

Warning: Using a password on the command line interface can be insecure.

Is there a better way of handling this?

Any feedback would be greatly appreciated.

• Ludisposed: Welcome to Code Review! Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .

Answer 1

$(which foo) ...

First of all, I completely agree with the other answer, this is pointless, inefficient and ugly.

Clearing variables

Instead of var="", you can write simply: var=

Exit code

When a dependency is missing, the script does exit 0. Exit code 0 means success. It would make more sense to exit with non-zero, to indicate failure, for example exit 1.

Usability

Instead of exiting immediately when any dependency is missing, it would be more user-friendly to collect all the missing dependencies, and report them all at once. A user might get irritated to run the script repeatedly when multiple dependencies are missing.

Here-strings

Instead of echo ... | mysql ... it's better to write mysql ... <<< ....

Quoting command line arguments

This is not safe:

$(which mysqldump) -h ${DB_HOST} -u ${DB_USER} -p${DB_PASSWORD} ${database} > "$TMP_DIR/$database.sql" 2> /dev/null

Variables and the result of $(...) should be double-quoted to protect from parameter expansion and globbing:

"$(which mysqldump)" -h "${DB_HOST}" -u "${DB_USER}" -p"${DB_PASSWORD}" "${database}" > "$TMP_DIR/$database.sql" 2> /dev/null

Answer 2

is there a reason for $(which foo) ?

If foo is in your path, it will be found, if not which will give you an error.

Expressions like command will have a return code, so do not write

if [[ $(command -v "$dependency") = "" ]]

just use

if command -v "$dependency" > /dev/null
then ...

• Enijar: Thank for the feedback. The reason I use $(which command) is because I ran into an issue where the script said rm: command not found when running it from a cronjob. I've updated the code to your suggestion.
• Gao: @Enijar Then that probably indicates something is seriously wrong with your setup. Try adding . ~/.bash_profile or . ~/.bashrc at the top of your script and see if it works. Also, you're not supposed to edit your question to reflect changes suggested by the answers. It'll make your post super confusing.