Issue 29792596: #3298 - SSL monitoring script for icinga (Closed)
Description#3298 - SSL monitoring script for icinga
Patch Set 1 #Patch Set 2 : #3298 - SSL monitoring script for icinga #
Total comments: 8
Patch Set 3 : #3298 - SSL monitoring script for icinga #
Total comments: 2
Patch Set 4 : #3298 - SSL monitoring script for icinga #
Total comments: 2
Patch Set 5 : #3298 - SSL monitoring script for Icinga #
Total comments: 4
Patch Set 6 : #3298 - SSL monitoring script for Icinga #Patch Set 7 : #3298 - SSL monitoring script for icinga #
Total comments: 1
Patch Set 8 : #3298 - SSL monitoring script for icinga #MessagesTotal messages: 15
https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh File check_ssl_cert.sh (right): https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh#n... check_ssl_cert.sh:8: PROGNAME=`basename $0` What if $0 contains white-space characters? https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh#n... check_ssl_cert.sh:9: PROGPATH=`echo $0 | sed -e 's,[\\/][^\\/][^\\/]*$,,'` Isn't there a regular POSIX or Linux command for that? It should not require a regular expression in the script just to get the directory name. https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh#n... check_ssl_cert.sh:11: #. $PROGPATH/utils.sh Please avoid "commented-out" lines that do no serve a purpose, e.g. in documentation. https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh#n... check_ssl_cert.sh:17: WARNING=$8 What if any of $2, $4, $6, $8 contain white-space characters? https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh#n... check_ssl_cert.sh:19: DAY=`echo | openssl s_client -connect "$HOST":$PORT 2>/dev/null | openssl x509 -noout -enddate 2>/dev/null | awk '{print $2}'` There should be a better way to send non-input to the `openssl` command other than pipe-lining an `echo`. You could use input redirection from /dev/null, for example. See https://www.tutorialspoint.com/unix/unix-io-redirections.htm for an introduction. https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh#n... check_ssl_cert.sh:21: if [ ! $DAY ] Since you `-connect` to `$HOST:$PORT` multiple times during the script, why do you have error handling logic only for the first attempt? https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh#n... check_ssl_cert.sh:82: exit $STATE_CRITICAL Where are the $STATE_{CRITICAL,WARNING,OK,UNKNOWN} variables defined? https://codereview.adblockplus.org/29792596/diff/29793562/check_ssl_cert.sh#n... check_ssl_cert.sh:83: elif [ $DIFF -le $WARNING ] && [ $DIFF -gt $CRITICAL ] You already know at this point that $DIFF is greater than $CRITICAL because you already determined above that $DIFF is not less than or equal $CRITICAL.
https://codereview.adblockplus.org/29792596/diff/29799617/check_ssl_cert.sh File check_ssl_cert.sh (right): https://codereview.adblockplus.org/29792596/diff/29799617/check_ssl_cert.sh#n... check_ssl_cert.sh:18: OUTPUT=`openssl s_client -connect "$HOST":"$PORT" </dev/null 2>/dev/null | openssl x509 -noout -enddate 2>/dev/null` Please place this in a function() and invoke it to assign the output, e.g. OUTPUT=`function_name "$HOST" "$PORT"`. https://codereview.adblockplus.org/29792596/diff/29799617/check_ssl_cert.sh#n... check_ssl_cert.sh:74: EXPIRY_DATE_IN_SEC=`date -d "$YEAR""$MONTH""$DAY" +%s` You can use "$YEAR$MONTH$DAY" or, even better, "${YEAR}${MONTH}${DAY}" here -- there's no need for the additional quotes.
https://codereview.adblockplus.org/29792596/diff/29800555/check_ssl_cert.sh File check_ssl_cert.sh (right): https://codereview.adblockplus.org/29792596/diff/29800555/check_ssl_cert.sh#n... check_ssl_cert.sh:7: echo `openssl s_client -connect "$1":"$2" </dev/null 2>/dev/null | openssl x509 -noout -enddate 2>/dev/null` The surrounding `echo ...` is not necessary. The output will be echo'd anyway, and your basically echoing what's been echo'd in the sub-command / backtick enclosed part. https://codereview.adblockplus.org/29792596/diff/29800555/check_ssl_cert.sh#n... check_ssl_cert.sh:10: PATH="/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin" Why do you set a custom $PATH here and not use the environment's existing value?
https://codereview.adblockplus.org/29792596/diff/29805562/check_ssl_cert.sh File check_ssl_cert.sh (right): https://codereview.adblockplus.org/29792596/diff/29805562/check_ssl_cert.sh#n... check_ssl_cert.sh:4: # Usage: <PluginDir>/check_ssl_cert -H <HOSTNAME> -P <PORT> -c <CRITICAL> -w <WARNING> Please use $PLUGIN_DIR and similar variables, even in examples. https://codereview.adblockplus.org/29792596/diff/29805562/check_ssl_cert.sh#n... check_ssl_cert.sh:7: openssl s_client -connect "$1":"$2" </dev/null 2>/dev/null | openssl x509 -noout -enddate 2>/dev/null Please use a backslash to not exceed 80 characters, i.e. openssl s_client -connect "$1:$2" </dev/null 2>/dev/null \ | openssl x509 -noout -enddate 2>/dev/null https://codereview.adblockplus.org/29792596/diff/29805562/check_ssl_cert.sh#n... check_ssl_cert.sh:21: if [ ! "$OUTPUT" ] Use `-z` to test for empty values. https://codereview.adblockplus.org/29792596/diff/29805562/check_ssl_cert.sh#n... check_ssl_cert.sh:78: DIFF=`expr "$DIFF" / 86400` As discussed: Please use brackets (`\(...\)`) to avoid assigning to DIFF{,ERENCE} twice.
LGTM.
https://codereview.adblockplus.org/29792596/diff/29822568/check_ssl_cert.sh File check_ssl_cert.sh (right): https://codereview.adblockplus.org/29792596/diff/29822568/check_ssl_cert.sh#n... check_ssl_cert.sh:37: fi That should work. But you could also use awk's printf function instead: In line 30 do this: DAY=`echo "$OUTPUT" | awk '{printf "%02d", $2}'`
LGTM 
|
