Bash script to backup from S3 to GC
Looks mostly pretty good.



When exiting with error, it's a good practice to specify a non-zero exit code, for example exit 1.



This looks odd:


if env | grep -q ^S3_BUCKET=
then
    echo "Checking if $S3_BUCKET exists"
    if aws s3 ls "s3://$S3_BUCKET" 2&gt;&amp;1 | grep -q 'AllAccessDisabled'



It's odd to check environment variables using env | grep.
And in this example, it looks error-prone too,
because S3_BUCKET might be defined empty, with no values.

I don't know aws, but I have a strong feeling that in case of S3_BUCKET= (empty) you would rather raise an error than run execute aws s3 ls "s3://".

I suggest to replace with a simple variable check:

if test "$S3_BUCKET"; then




In this code:


tar -zcvf $datestamp.tar.gz $datestamp
file=$datestamp.tar.gz



It would be better to set file first, and then use it in the tar



I like to avoid nested if statements when possible. For example here:


if env | grep -q ^S3_BUCKET=
then
    echo "Checking if $S3_BUCKET exists"
    if aws s3 ls "s3://$S3_BUCKET" 2&gt;&amp;1 | grep -q 'AllAccessDisabled'   
    then
        echo "bucket $S3_BUCKET doesn't exist please check again."
        exit
    fi
else
    echo 'S3_BUCKET is not set please set it'
    exit
fi



I would flatten by inverting the outer if:

if test ! "$S3_BUCKET"
then
    echo 'S3_BUCKET is not set please set it'
    exit 1
fi

echo "Checking if $S3_BUCKET exists"
if aws s3 ls "s3://$S3_BUCKET" 2&gt;&amp;1 | grep -q 'AllAccessDisabled'   
then
    echo "bucket $S3_BUCKET doesn't exist please check again."
    exit 1
fi