2
\$\begingroup\$

I am fairly new to bash script programming so I would love some feedback. I am trying to write a series of migration scripts for our DBs (please don't direct me to other tools; I spent a lot of time evaluating them and am forced to write custom code) and this is script that contains validation logic.

Basic requirements:

  1. Validate file format.
  2. Make sure the script exists in at least 1 descriptor. A descriptor dictates the scripts to execute and in which order. If a release file isn't in a descriptor it's inert so validate it's present
  3. ensure a corresponding rollback exists
  4. ensure a validation script exists
  5. execute the validation script and verify results.

how I intend it (for now) to be used is

source ./validation.sh
validateFormat scripts/release/DBA-1234 || echo "fail"

as an example. Or this, which would apply all:

validate

ideally, I would love to be able to chain them together functionally but when I try the below, it doesn't seem to know my symbols:

xargs: validateFormat: No such file or directory

find scripts/releases -name '*.sql' | xargs validateFormat | validateDescriptor | cat | mysql 

or just

find scripts/releases -name '*.sql' | xargs validate | cat | mysql

release-format (DBA-1234_some_message.sql)

([A-Z]{2,5}[_-][0-9]{1,5})[_-]?(.*)    

validation.sh

#!/usr/bin/env bash

user=root
password=my-secret-pw
port=3306
host=127.0.0.1
release_format=$(cat .utils/release-format)
releases=${scripts}/releases/
validators=${scripts}/validators/
descriptors=${scripts}/descriptors/
rollbacks=${scripts}/rollbacks/


validateDescriptor() {
  [[ -z $1 ]] && return -1 
  local script=$(basename $1)
  grep -rq ${script} ${descriptors};
  return $?
}


ensureRollback() {
  [[ -z $1 ]] && return -1
  ls ${rollbacks}/$(toTicket ${1})* 2>&1 >/dev/null
  return $?
}

ensureValidator() {
  [[ -z $1 ]] && return -1
  ls ${validators}/$(toTicket ${1})* 2>&1 >/dev/null
  return $?
}

validateFormat() {
  [[ -z $1 ]] && return -1
  [[ $(basename -s .sql $1) =~ ${release_format} ]] && return 0 || return -1
}


toTicket() {
  [[ -n $1 ]] && basename -s .sql ${1} | sed -E "s/$release_format/\1/"
}

toValidator() {
  [[ -z $1 ]] && return -1
  local ticket=$(toTicket ${1})
  local validator=$(ls ${validators}/${ticket}* | head -1)
  [[ $? != 0 ]] && return $? || echo ${validator}
}

applyValidationScripts() {
  [[ -z $1 ]] && return -1
  local validator=$(toValidator ${1})

  if [[ ! -e ${validator} ]]; then
    return -1
  fi

  result=$(mysql -sN -h ${host} -u ${user} -p${password} -P ${port} <${validator})
  if [[ $? != 0 ]]; then
    return $?
  fi

  if [[ $(echo ${result} | wc -l | tr -d ' ') != 1 ]]; then
    echo "validation result count was more than 1 row. Invalid results"
    return -1
  else
    return $((result - 1))
  fi
}

validate() {
  for release in $(find ${releases} -type f -exec basename -s .sql {} \;); do
    #  applyValidationScripts ${release}
    validateFormat ${release}  && ensureRollback ${release} && ensureValidator ${release} && validateDescriptor ${release}
    [[ $? != 0 ]] && echo "${release} failed validation"
  done;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ validateFormat doesn't produce any output. I wonder how you can pipe it. \$\endgroup\$ – vnp Nov 22 '17 at 6:20
  • \$\begingroup\$ If I were you, I'd remove the description of how you fail to run under xargs when you fail to export -f the function - that makes it look like your code doesn't work, which would make it not yet ready for review. The code itself is ready; you just need to go to Stack Overflow to learn to use it more effectively. \$\endgroup\$ – Toby Speight Nov 22 '17 at 16:37
  • \$\begingroup\$ @vnp I removed the output from validate format because I couldn't get it to chain. \$\endgroup\$ – Christian Bongiorno Nov 22 '17 at 18:12
  • \$\begingroup\$ @TobySpeight thanks for the insight. I will give this a try \$\endgroup\$ – Christian Bongiorno Nov 22 '17 at 18:13
  • \$\begingroup\$ I ran export -f and it doesn't help. How should I post a code update? \$\endgroup\$ – Christian Bongiorno Nov 22 '17 at 22:41
3
\$\begingroup\$

Prefer portable shell

There's no reason to use Bash [[ where [ will do, unless you already depend on other Bash features (such as arrays). This will of course increase the number of places where you must:

Quote parameter expansions

In expansions like $(basename -s .sql $1), don't forget the quotes:

[[ $(basename -s .sql "$1") =~ ${release_format} ]]
# I removed `&& return 0 || return -1` - see next item

Let exit status flow naturally

The Bash manual says:

When executed, the exit status of a function is the exit status of the last command executed in the body.

This means that adding return $? to the end of every function adds no value, as do constructs such as && return 0 || return -1 - just let success/fail flow through.

Put credentials in a separate file

Don't put user and password in the script (particularly if you want to put it into a version control server). Instead, require each user to provide their credentials in a (non-world-readable) file. If you're particularly diligent, you could fail the script if the permissions on the credentials file are too permissive.

Don't invoke ls just to check existence

# true if the first argument exists
one_exists()
{
    test -e "$1"
}

ensureRollback() {
     test -n "$1" && one_exists "${rollbacks}/$(toTicket ${1})"*
}
\$\endgroup\$
  • \$\begingroup\$ Great suggestions, let me implement those. \$\endgroup\$ – Christian Bongiorno Nov 22 '17 at 18:10
  • \$\begingroup\$ What's the benefit of quoting "$1" ? That expands it. But it currently works too. So, what benefit is it? I just don't know the difference \$\endgroup\$ – Christian Bongiorno Nov 22 '17 at 18:22
  • \$\begingroup\$ re: passwords - these credentials are for a docker container used for test. It's of no consequence and they can be overriden. I intend to have this script sourced into a build which takes commandline args which would include these variables. for the release process I will consider the password file. For build: It just complicates something where username and pass are totally optional \$\endgroup\$ – Christian Bongiorno Nov 22 '17 at 18:27
  • \$\begingroup\$ If parameters are expanded outside of quotes, the result is subject to word splitting and pathname expansion. Quoted strings are preserved as single words by the above; this will reduce the incidence of surprises. (Even if you know you'll never get spaces in the value, there's no harm to using quotes, and it signals to the reader that you're aware of the issue). \$\endgroup\$ – Toby Speight Nov 23 '17 at 7:58

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.