Krdytkyu

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.

$(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

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 ...