From 77fadbaa8cac1e6d1af02208efc230493e440564 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Tue, 9 Jun 2020 11:43:10 -0500 Subject: [PATCH 1/9] Doc: tools: improve cibsecret help Also use cat< "; - echo "--version Display version information, then exit"; - echo ""; - echo "-C: don't read/write the CIB" - echo "" - echo "command: set | delete | stash | unstash | get | check | sync" - echo "" - echo " set " - echo "" - echo " get " - echo "" - echo " check " - echo "" - echo " stash (if not -C)" - echo "" - echo " unstash (if not -C)" - echo "" - echo " delete " - echo "" - echo " sync" - echo "" - echo "stash/unstash: move the parameter from/to the CIB (if you already" - echo "have the parameter set in the CIB)." - echo "" - echo "set/delete: add/remove a parameter from the local file." - echo "" - echo "get: display the parameter from the local file." - echo "" - echo "check: verify MD5 hash of the parameter from the local file and the CIB." - echo "" - echo "sync: copy $LRM_CIBSECRETS to other nodes." - echo "" - echo "Examples:" - echo "" - echo " $PROG set ipmi_node1 passwd SecreT_PASS" - echo "" - echo " $PROG stash ipmi_node1 passwd" - echo "" - echo " $PROG get ipmi_node1 passwd" - echo "" - echo " $PROG check ipmi_node1 passwd" - echo "" - echo " $PROG sync" - - exit $1 + cat <] [] + +Options: + --help Show this message, then exit + --version Display version information, then exit + -C Don't read or write the CIB + +Commands and their parameters: + set + Set the value of a sensitive resource parameter. + + get + Display the locally stored value of a sensitive resource parameter. + + check + Verify that the locally stored value of a sensitive resource parameter + matches its locally stored MD5 hash. + + stash + Make a non-sensitive resource parameter that is already in the CIB + sensitive (move its value to a locally stored and protected file). + This may not be used with -C. + + unstash + Make a sensitive resource parameter that is already in the CIB + non-sensitive (move its value from the locally stored file to the CIB). + This may not be used with -C. + + delete + Remove a sensitive resource parameter value. + + sync + Copy all locally stored secrets to all other nodes. + +This command manages sensitive resource parameter values that should not be +stored directly in Pacemaker's Cluster Information Base (CIB). Such values +are handled by storing a special string directly in the CIB that tells +Pacemaker to look in a separate, protected file for the actual value. + +The secret files are not encrypted, but protected by file system permissions +such that only root can read or modify them. + +Since the secret files are stored locally, they must be synchronized across all +cluster nodes. This command handles the synchronization using (in order of +preference) pssh, pdsh, or ssh, so one of those must be installed. Before +synchronizing, this command will ping the cluster nodes to determine which are +alive, using fping if it is installed, otherwise the ping command. Installing +fping is strongly recommended for better performance. + +Known limitations: + + This command can only be run from full cluster nodes (not Pacemaker Remote + nodes). + + Changes are not atomic, so the cluster may use different values while a + change is in progress. To avoid problems, it is recommended to put the + cluster in maintenance mode when making changes with this command. + + Changes in secret values do not trigger a reload or restart of the affected + resource, since they do not change the CIB. If a response is desired before + the next cluster recheck interval, any CIB change (such as setting a node + attribute) will trigger it. + + If any node is down when changes to secrets are made, or a new node is + later added to the cluster, it may have different values when it joins the + cluster, before "$PROG sync" is run. To avoid this, it is recommended to + run the sync command (from another node) before starting Pacemaker on the + node. + +Examples: + + $PROG set ipmi_node1 passwd SecreT_PASS + + $PROG get ipmi_node1 passwd + + $PROG check ipmi_node1 passwd + + $PROG stash ipmi_node2 passwd + + $PROG sync +EOF + exit $1 } fatal() { echo "ERROR: $*" From 8745b690deb7e2dd7fbedb66cf1114fc5c23a737 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Tue, 9 Jun 2020 11:59:58 -0500 Subject: [PATCH 2/9] Refactor: tools: make cibsecret follow current whitespace style guidelines no substantive changes --- tools/cibsecret.in | 386 ++++++++++++++++++++++++--------------------- 1 file changed, 203 insertions(+), 183 deletions(-) diff --git a/tools/cibsecret.in b/tools/cibsecret.in index eedb131fc76..f35e7317ec6 100644 --- a/tools/cibsecret.in +++ b/tools/cibsecret.in @@ -107,261 +107,281 @@ Examples: EOF exit $1 } + fatal() { - echo "ERROR: $*" - exit 1 + echo "ERROR: $*" + exit 1 } + warn() { - echo "WARNING: $*" + echo "WARNING: $*" } + info() { - echo "INFO: $*" + echo "INFO: $*" } check_env() { - which md5sum >/dev/null 2>&1 || - fatal "please install md5sum to run $PROG" - if which pssh >/dev/null 2>&1; then - rsh=pssh_fun - rcp=pscp_fun - elif which pdsh >/dev/null 2>&1; then - rsh=pdsh_fun - rcp=pdcp_fun - elif which ssh >/dev/null 2>&1; then - rsh=ssh_fun - rcp=scp_fun - else - fatal "please install pssh, pdsh, or ssh to run $PROG" - fi - ps -ef | grep '[p]acemaker-controld' >/dev/null || - fatal "pacemaker not running? $PROG needs pacemaker" + which md5sum >/dev/null 2>&1 || + fatal "please install md5sum to run $PROG" + if which pssh >/dev/null 2>&1; then + rsh=pssh_fun + rcp=pscp_fun + elif which pdsh >/dev/null 2>&1; then + rsh=pdsh_fun + rcp=pdcp_fun + elif which ssh >/dev/null 2>&1; then + rsh=ssh_fun + rcp=scp_fun + else + fatal "please install pssh, pdsh, or ssh to run $PROG" + fi + ps -ef | grep '[p]acemaker-controld' >/dev/null || + fatal "pacemaker not running? $PROG needs pacemaker" } get_other_nodes() { - crm_node -l | awk '{print $2}' | grep -v `uname -n` + crm_node -l | awk '{print $2}' | grep -v `uname -n` } get_live_nodes() { - if [ `id -u` = 0 ] && which fping >/dev/null 2>&1; then - fping -a $@ 2>/dev/null - else - local h - for h; do ping -c 2 -q $h >/dev/null 2>&1 && echo $h; done - fi + if [ `id -u` = 0 ] && which fping >/dev/null 2>&1; then + fping -a $@ 2>/dev/null + else + local h + for h; do ping -c 2 -q $h >/dev/null 2>&1 && echo $h; done + fi } check_down_nodes() { - local n down_nodes - down_nodes=`(for n; do echo $n; done) | sort | uniq -u` - if [ -n "$down_nodes" ]; then - if [ `echo $down_nodes | wc -w` = 1 ]; then - warn "node $down_nodes is down" - warn "you'll need to update it using $PROG sync later" - else - warn "nodes `echo $down_nodes` are down" - warn "you'll need to update them using $PROG sync later" - fi - fi + local n down_nodes + down_nodes=`(for n; do echo $n; done) | sort | uniq -u` + if [ -n "$down_nodes" ]; then + if [ `echo $down_nodes | wc -w` = 1 ]; then + warn "node $down_nodes is down" + warn "you'll need to update it using $PROG sync later" + else + warn "nodes `echo $down_nodes` are down" + warn "you'll need to update them using $PROG sync later" + fi + fi } pssh_fun() { - pssh -qi -H "$nodes" -x "$SSH_OPTS" $* + pssh -qi -H "$nodes" -x "$SSH_OPTS" $* } + pscp_fun() { - pscp -q -H "$nodes" -x "-pr" -x "$SSH_OPTS" $* + pscp -q -H "$nodes" -x "-pr" -x "$SSH_OPTS" $* } + pdsh_fun() { - local pdsh_nodes=`echo $nodes | tr ' ' ','` - export PDSH_SSH_ARGS_APPEND="$SSH_OPTS" - pdsh -w $pdsh_nodes $* + local pdsh_nodes=`echo $nodes | tr ' ' ','` + export PDSH_SSH_ARGS_APPEND="$SSH_OPTS" + pdsh -w $pdsh_nodes $* } + pdcp_fun() { - local pdsh_nodes=`echo $nodes | tr ' ' ','` - export PDSH_SSH_ARGS_APPEND="$SSH_OPTS" - pdcp -pr -w $pdsh_nodes $* + local pdsh_nodes=`echo $nodes | tr ' ' ','` + export PDSH_SSH_ARGS_APPEND="$SSH_OPTS" + pdcp -pr -w $pdsh_nodes $* } + ssh_fun() { - local h - for h in $nodes; do - ssh $SSH_OPTS $h $* || return - done + local h + for h in $nodes; do + ssh $SSH_OPTS $h $* || return + done } + scp_fun() { - local h src="$1" dest=$2 - for h in $nodes; do - scp -pr -q $SSH_OPTS $src $h:$dest || return - done + local h src="$1" dest=$2 + for h in $nodes; do + scp -pr -q $SSH_OPTS $src $h:$dest || return + done } + # TODO: this procedure should be replaced with csync2 # provided that csync2 has already been configured sync_files() { - local crm_nodes=`get_other_nodes` - local nodes=`get_live_nodes $crm_nodes` - check_down_nodes $nodes $crm_nodes - [ "$nodes" = "" ] && { - info "no other nodes live" - return - } - info "syncing $LRM_CIBSECRETS to `echo $nodes` ..." - $rsh rm -rf $LRM_CIBSECRETS && - $rsh mkdir -p `dirname $LRM_CIBSECRETS` && - $rcp $LRM_CIBSECRETS `dirname $LRM_CIBSECRETS` + local crm_nodes=`get_other_nodes` + local nodes=`get_live_nodes $crm_nodes` + check_down_nodes $nodes $crm_nodes + [ "$nodes" = "" ] && { + info "no other nodes live" + return + } + info "syncing $LRM_CIBSECRETS to `echo $nodes` ..." + $rsh rm -rf $LRM_CIBSECRETS && + $rsh mkdir -p `dirname $LRM_CIBSECRETS` && + $rcp $LRM_CIBSECRETS `dirname $LRM_CIBSECRETS` } + sync_one() { - local f=$1 f_all="$1 $1.sign" - local crm_nodes=`get_other_nodes` - local nodes=`get_live_nodes $crm_nodes` - check_down_nodes $nodes $crm_nodes - [ "$nodes" = "" ] && { - info "no other nodes live" - return - } - info "syncing $f to `echo $nodes` ..." - $rsh mkdir -p `dirname $f` && - if [ -f "$f" ]; then - $rcp "$f_all" `dirname $f` - else - $rsh rm -f $f_all - fi + local f=$1 f_all="$1 $1.sign" + local crm_nodes=`get_other_nodes` + local nodes=`get_live_nodes $crm_nodes` + check_down_nodes $nodes $crm_nodes + [ "$nodes" = "" ] && { + info "no other nodes live" + return + } + info "syncing $f to `echo $nodes` ..." + $rsh mkdir -p `dirname $f` && + if [ -f "$f" ]; then + $rcp "$f_all" `dirname $f` + else + $rsh rm -f $f_all + fi } is_secret() { - # assume that the secret is in the CIB if we cannot talk to - # cib - [ "$NO_CRM" ] || - test "$1" = "$MAGIC" + # assume that the secret is in the CIB if we cannot talk to + # cib + [ "$NO_CRM" ] || test "$1" = "$MAGIC" } + check_cib_rsc() { - local rsc=$1 output - output=`$NO_CRM crm_resource -r $rsc -W >/dev/null 2>&1` || - fatal "resource $rsc doesn't exist: $output" + local rsc=$1 output + output=`$NO_CRM crm_resource -r $rsc -W >/dev/null 2>&1` || + fatal "resource $rsc doesn't exist: $output" } + get_cib_param() { - local rsc=$1 param=$2 - check_cib_rsc $rsc - $NO_CRM crm_resource -r $rsc -g $param 2>/dev/null + local rsc=$1 param=$2 + check_cib_rsc $rsc + $NO_CRM crm_resource -r $rsc -g $param 2>/dev/null } + set_cib_param() { - local rsc=$1 param=$2 value=$3 - check_cib_rsc $rsc - $NO_CRM crm_resource -r $rsc -p $param -v "$value" 2>/dev/null + local rsc=$1 param=$2 value=$3 + check_cib_rsc $rsc + $NO_CRM crm_resource -r $rsc -p $param -v "$value" 2>/dev/null } + remove_cib_param() { - local rsc=$1 param=$2 - check_cib_rsc $rsc - $NO_CRM crm_resource -r $rsc -d $param 2>/dev/null + local rsc=$1 param=$2 + check_cib_rsc $rsc + $NO_CRM crm_resource -r $rsc -d $param 2>/dev/null } localfiles() { - local cmd=$1 - local rsc=$2 param=$3 value=$4 - local local_file=$LRM_CIBSECRETS/$rsc/$param - case $cmd in - "get") - cat $local_file 2>/dev/null - true - ;; - "getsum") - cat $local_file.sign 2>/dev/null - true - ;; - "set") - local md5sum - md5sum=`printf $value | md5sum` || - fatal "md5sum failed to produce hash for resource $rsc parameter $param" - md5sum=`echo $md5sum | awk '{print $1}'` - mkdir -p `dirname $local_file` && - echo $value > $local_file && - echo $md5sum > $local_file.sign && - sync_one $local_file - ;; - "remove") - rm -f $local_file - rm -f $local_file.sign - sync_one $local_file - ;; - *) - # not reached, this is local interface - ;; - esac + local cmd=$1 + local rsc=$2 param=$3 value=$4 + local local_file=$LRM_CIBSECRETS/$rsc/$param + case $cmd in + get) + cat $local_file 2>/dev/null + true + ;; + + getsum) + cat $local_file.sign 2>/dev/null + true + ;; + + set) + local md5sum + md5sum=`printf $value | md5sum` || + fatal "md5sum failed to produce hash for resource $rsc parameter $param" + md5sum=`echo $md5sum | awk '{print $1}'` + mkdir -p `dirname $local_file` && + echo $value > $local_file && + echo $md5sum > $local_file.sign && + sync_one $local_file + ;; + + remove) + rm -f $local_file + rm -f $local_file.sign + sync_one $local_file + ;; + + *) + # not reached, this is local interface + ;; + esac } + get_local_param() { - local rsc=$1 param=$2 - localfiles get $rsc $param + local rsc=$1 param=$2 + localfiles get $rsc $param } + set_local_param() { - local rsc=$1 param=$2 value=$3 - localfiles set $rsc $param $value + local rsc=$1 param=$2 value=$3 + localfiles set $rsc $param $value } + remove_local_param() { - local rsc=$1 param=$2 - localfiles remove $rsc $param + local rsc=$1 param=$2 + localfiles remove $rsc $param } cibsecret_set() { - local value=$1 + local value=$1 - if [ -z "$NO_CRM" ]; then - [ "$current" -a "$current" != "$MAGIC" -a "$current" != "$value" ] && - fatal "CIB value <$current> different for $rsc parameter $param; please delete it first" - fi - set_local_param $rsc $param $value && - set_cib_param $rsc $param "$MAGIC" + if [ -z "$NO_CRM" ]; then + [ "$current" -a "$current" != "$MAGIC" -a "$current" != "$value" ] && + fatal "CIB value <$current> different for $rsc parameter $param; please delete it first" + fi + set_local_param $rsc $param $value && + set_cib_param $rsc $param "$MAGIC" } cibsecret_check() { - local md5sum local_md5sum - is_secret "$current" || - fatal "resource $rsc parameter $param not set as secret, nothing to check" - local_md5sum=`localfiles getsum $rsc $param` - [ "$local_md5sum" ] || - fatal "no MD5 hash for resource $rsc parameter $param" - md5sum=`printf "$current_local" | md5sum | awk '{print $1}'` - [ "$md5sum" = "$local_md5sum" ] || - fatal "MD5 hash mismatch for resource $rsc parameter $param" + local md5sum local_md5sum + is_secret "$current" || + fatal "resource $rsc parameter $param not set as secret, nothing to check" + local_md5sum=`localfiles getsum $rsc $param` + [ "$local_md5sum" ] || + fatal "no MD5 hash for resource $rsc parameter $param" + md5sum=`printf "$current_local" | md5sum | awk '{print $1}'` + [ "$md5sum" = "$local_md5sum" ] || + fatal "MD5 hash mismatch for resource $rsc parameter $param" } cibsecret_get() { - cibsecret_check - echo "$current_local" + cibsecret_check + echo "$current_local" } cibsecret_delete() { - remove_local_param $rsc $param && - remove_cib_param $rsc $param + remove_local_param $rsc $param && + remove_cib_param $rsc $param } cibsecret_stash() { - [ "$NO_CRM" ] && - fatal "no access to Pacemaker, stash not supported" - [ "$current" = "" ] && - fatal "nothing to stash for resource $rsc parameter $param" - is_secret "$current" && - fatal "resource $rsc parameter $param already set as secret, nothing to stash" - cibsecret_set "$current" + [ "$NO_CRM" ] && + fatal "no access to Pacemaker, stash not supported" + [ "$current" = "" ] && + fatal "nothing to stash for resource $rsc parameter $param" + is_secret "$current" && + fatal "resource $rsc parameter $param already set as secret, nothing to stash" + cibsecret_set "$current" } cibsecret_unstash() { - [ "$NO_CRM" ] && - fatal "no access to Pacemaker, unstash not supported" - [ "$current_local" = "" ] && - fatal "nothing to unstash for resource $rsc parameter $param" - is_secret "$current" || - warn "resource $rsc parameter $param not set as secret, but we have local value so proceeding anyway" - remove_local_param $rsc $param && - set_cib_param $rsc $param $current_local + [ "$NO_CRM" ] && + fatal "no access to Pacemaker, unstash not supported" + [ "$current_local" = "" ] && + fatal "nothing to unstash for resource $rsc parameter $param" + is_secret "$current" || + warn "resource $rsc parameter $param not set as secret, but we have local value so proceeding anyway" + remove_local_param $rsc $param && + set_cib_param $rsc $param $current_local } cibsecret_sync() { - sync_files + sync_files } MAGIC="lrm://" umask 0077 if [ "$1" = "-C" ]; then - NO_CRM=':' - shift 1 + NO_CRM=':' + shift 1 fi cmd=$1 @@ -370,18 +390,16 @@ param=$3 value=$4 case "$cmd" in - set) [ $# -ne 4 ] && usage 1;; - get) [ $# -ne 3 ] && usage 1;; - check) [ $# -ne 3 ] && usage 1;; - stash) [ $# -ne 3 ] && usage 1;; - unstash) [ $# -ne 3 ] && usage 1;; - delete) [ $# -ne 3 ] && usage 1;; - sync) [ $# -ne 1 ] && usage 1;; - --help) usage 0;; - --version) - crm_attribute --version - exit $?;; - *) usage 1; + set) [ $# -ne 4 ] && usage 1 ;; + get) [ $# -ne 3 ] && usage 1 ;; + check) [ $# -ne 3 ] && usage 1 ;; + stash) [ $# -ne 3 ] && usage 1 ;; + unstash) [ $# -ne 3 ] && usage 1 ;; + delete) [ $# -ne 3 ] && usage 1 ;; + sync) [ $# -ne 1 ] && usage 1 ;; + --help) usage 0 ;; + --version) crm_attribute --version; exit $? ;; + *) usage 1 ;; esac . @OCF_ROOT_DIR@/lib/heartbeat/ocf-shellfuncs @@ -393,3 +411,5 @@ current=`get_cib_param $rsc $param` current_local=`get_local_param $rsc $param` cibsecret_$cmd $value + +# vim: set expandtab tabstop=8 softtabstop=4 shiftwidth=4 textwidth=80: From 26187410b7e1917973dae70d9e199d5bcda46ce2 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Tue, 9 Jun 2020 12:04:05 -0500 Subject: [PATCH 3/9] Refactor: tools: make cibsecret use best practices for shell scripts ... which makes shellcheck happier. Mainly this involves: * quoting variables * not using "local", which isn't portable to e.g. zsh * using $(cmd) instead of `cmd` --- tools/cibsecret.in | 256 +++++++++++++++++++++++---------------------- 1 file changed, 129 insertions(+), 127 deletions(-) diff --git a/tools/cibsecret.in b/tools/cibsecret.in index f35e7317ec6..aa1efb67df0 100644 --- a/tools/cibsecret.in +++ b/tools/cibsecret.in @@ -14,9 +14,9 @@ # Secrets are ASCII files, holding one value per file: # // -LRM_CIBSECRETS=@LRM_CIBSECRETS_DIR@ +LRM_CIBSECRETS="@LRM_CIBSECRETS_DIR@" -PROG=`basename $0` +PROG="$(basename "$0")" SSH_OPTS="-o StrictHostKeyChecking=no" usage() { @@ -105,7 +105,7 @@ Examples: $PROG sync EOF - exit $1 + exit "$1" } fatal() { @@ -126,13 +126,13 @@ check_env() { fatal "please install md5sum to run $PROG" if which pssh >/dev/null 2>&1; then rsh=pssh_fun - rcp=pscp_fun + rcp_to_from=pscp_fun elif which pdsh >/dev/null 2>&1; then rsh=pdsh_fun - rcp=pdcp_fun + rcp_to_from=pdcp_fun elif which ssh >/dev/null 2>&1; then rsh=ssh_fun - rcp=scp_fun + rcp_to_from=scp_fun else fatal "please install pssh, pdsh, or ssh to run $PROG" fi @@ -140,161 +140,162 @@ check_env() { fatal "pacemaker not running? $PROG needs pacemaker" } -get_other_nodes() { - crm_node -l | awk '{print $2}' | grep -v `uname -n` -} - +# This must be called (and return success) before calling $rsh or $rcp_to_from get_live_nodes() { - if [ `id -u` = 0 ] && which fping >/dev/null 2>&1; then - fping -a $@ 2>/dev/null + # Get a list of all cluster nodes + GLN_ALL_NODES="$(crm_node -l | awk '{print $2}' | grep -v "$(uname -n)")" + + # Make a list of those that respond to pings + if [ "$(id -u)" = "0" ] && which fping >/dev/null 2>&1; then + LIVE_NODES=$(fping -a $GLN_ALL_NODES 2>/dev/null) else - local h - for h; do ping -c 2 -q $h >/dev/null 2>&1 && echo $h; done + LIVE_NODES="" + for GLN_NODE in $GLN_ALL_NODES; do \ + ping -c 2 -q "$GLN_NODE" >/dev/null 2>&1 && + LIVE_NODES="$LIVE_NODES $GLN_NODE" + done fi -} -check_down_nodes() { - local n down_nodes - down_nodes=`(for n; do echo $n; done) | sort | uniq -u` - if [ -n "$down_nodes" ]; then - if [ `echo $down_nodes | wc -w` = 1 ]; then - warn "node $down_nodes is down" - warn "you'll need to update it using $PROG sync later" - else - warn "nodes `echo $down_nodes` are down" - warn "you'll need to update them using $PROG sync later" - fi + # Warn the user about any that didn't respond to pings + GLN_DOWN="$( (for GLN_NODE in $LIVE_NODES $GLN_ALL_NODES; do echo "$GLN_NODE"; done) | sort | uniq -u)" + if [ "$(echo "$GLN_DOWN" | wc -w)" = "1" ]; then + warn "node $GLN_DOWN is down" + warn "you'll need to update it using \"$PROG sync\" later" + elif [ -n "$GLN_DOWN" ]; then + warn "nodes $(echo "$GLN_DOWN" | tr '\n' ' ')are down" + warn "you'll need to update them using \"$PROG sync\" later" fi + + if [ "$LIVE_NODES" = "" ]; then + info "no other nodes live" + return 1 + fi + return 0 } pssh_fun() { - pssh -qi -H "$nodes" -x "$SSH_OPTS" $* + pssh -qi -H "$LIVE_NODES" -x "$SSH_OPTS" -- "$@" } pscp_fun() { - pscp -q -H "$nodes" -x "-pr" -x "$SSH_OPTS" $* + PSCP_DEST="$1" + shift + pscp -q -H "$LIVE_NODES" -x "-pr" -x "$SSH_OPTS" -- "$@" "$PSCP_DEST" } pdsh_fun() { - local pdsh_nodes=`echo $nodes | tr ' ' ','` + PDSH_NODES=$(echo "$LIVE_NODES" | tr '[:space:]' ',') export PDSH_SSH_ARGS_APPEND="$SSH_OPTS" - pdsh -w $pdsh_nodes $* + pdsh -w "$PDSH_NODES" -- "$@" } pdcp_fun() { - local pdsh_nodes=`echo $nodes | tr ' ' ','` + PDCP_DEST="$1" + shift + PDCP_NODES=$(echo "$LIVE_NODES" | tr '[:space:]' ',') export PDSH_SSH_ARGS_APPEND="$SSH_OPTS" - pdcp -pr -w $pdsh_nodes $* + pdcp -pr -w "$PDCP_NODES" -- "$@" "$PDCP_DEST" } ssh_fun() { - local h - for h in $nodes; do - ssh $SSH_OPTS $h $* || return + for SSH_NODE in $LIVE_NODES; do + ssh $SSH_OPTS "$SSH_NODE" -- "$@" || return done } scp_fun() { - local h src="$1" dest=$2 - for h in $nodes; do - scp -pr -q $SSH_OPTS $src $h:$dest || return + SCP_DEST="$1" + shift + for SCP_NODE in $LIVE_NODES; do + scp -pqr $SSH_OPTS "$@" "$SCP_NODE:$SCP_DEST" || return done } # TODO: this procedure should be replaced with csync2 # provided that csync2 has already been configured sync_files() { - local crm_nodes=`get_other_nodes` - local nodes=`get_live_nodes $crm_nodes` - check_down_nodes $nodes $crm_nodes - [ "$nodes" = "" ] && { - info "no other nodes live" - return - } - info "syncing $LRM_CIBSECRETS to `echo $nodes` ..." - $rsh rm -rf $LRM_CIBSECRETS && - $rsh mkdir -p `dirname $LRM_CIBSECRETS` && - $rcp $LRM_CIBSECRETS `dirname $LRM_CIBSECRETS` + get_live_nodes || return + info "syncing $LRM_CIBSECRETS to $(echo "$LIVE_NODES" | tr '\n' ' ') ..." + $rsh rm -rf "$LRM_CIBSECRETS" && + $rsh mkdir -p "$(dirname "$LRM_CIBSECRETS")" && + $rcp_to_from "$(dirname "$LRM_CIBSECRETS")" "$LRM_CIBSECRETS" } sync_one() { - local f=$1 f_all="$1 $1.sign" - local crm_nodes=`get_other_nodes` - local nodes=`get_live_nodes $crm_nodes` - check_down_nodes $nodes $crm_nodes - [ "$nodes" = "" ] && { - info "no other nodes live" - return - } - info "syncing $f to `echo $nodes` ..." - $rsh mkdir -p `dirname $f` && - if [ -f "$f" ]; then - $rcp "$f_all" `dirname $f` + SO_FILE="$1" + get_live_nodes || return + info "syncing $SO_FILE to $(echo "$LIVE_NODES" | tr '\n' ' ') ..." + $rsh mkdir -p "$(dirname "$SO_FILE")" && + if [ -f "$SO_FILE" ]; then + $rcp_to_from "$(dirname "$SO_FILE")" "$SO_FILE" "${SO_FILE}.sign" else - $rsh rm -f $f_all + $rsh rm -f "$SO_FILE" "${SO_FILE}.sign" fi } is_secret() { - # assume that the secret is in the CIB if we cannot talk to - # cib + # assume that the secret is in the CIB if we cannot talk to cib [ "$NO_CRM" ] || test "$1" = "$MAGIC" } check_cib_rsc() { - local rsc=$1 output - output=`$NO_CRM crm_resource -r $rsc -W >/dev/null 2>&1` || - fatal "resource $rsc doesn't exist: $output" + CCR_RSC="$1" + CCR_OUT="$($NO_CRM crm_resource -r "$CCR_RSC" -W >/dev/null 2>&1)" || + fatal "resource $CCR_RSC doesn't exist: $CCR_OUT" } get_cib_param() { - local rsc=$1 param=$2 - check_cib_rsc $rsc - $NO_CRM crm_resource -r $rsc -g $param 2>/dev/null + GCP_RSC="$1" + GCP_PARAM="$2" + check_cib_rsc "$GCP_RSC" + $NO_CRM crm_resource -r "$GCP_RSC" -g "$GCP_PARAM" 2>/dev/null } set_cib_param() { - local rsc=$1 param=$2 value=$3 - check_cib_rsc $rsc - $NO_CRM crm_resource -r $rsc -p $param -v "$value" 2>/dev/null + SET_RSC="$1" + SET_PARAM="$2" + SET_VAL="$3" + check_cib_rsc "$SET_RSC" + $NO_CRM crm_resource -r "$SET_RSC" -p "$SET_PARAM" -v "$SET_VAL" 2>/dev/null } remove_cib_param() { - local rsc=$1 param=$2 - check_cib_rsc $rsc - $NO_CRM crm_resource -r $rsc -d $param 2>/dev/null + RM_RSC="$1" + RM_PARAM="$2" + check_cib_rsc "$RM_RSC" + $NO_CRM crm_resource -r "$RM_RSC" -d "$RM_PARAM" 2>/dev/null } localfiles() { - local cmd=$1 - local rsc=$2 param=$3 value=$4 - local local_file=$LRM_CIBSECRETS/$rsc/$param - case $cmd in + LF_CMD="$1" + LF_RSC="$2" + LF_PARAM="$3" + LF_VALUE="$4" + LF_FILE="$LRM_CIBSECRETS/$LF_RSC/$LF_PARAM" + case "$LF_CMD" in get) - cat $local_file 2>/dev/null + cat "$LF_FILE" 2>/dev/null true ;; getsum) - cat $local_file.sign 2>/dev/null + cat "${LF_FILE}.sign" 2>/dev/null true ;; set) - local md5sum - md5sum=`printf $value | md5sum` || - fatal "md5sum failed to produce hash for resource $rsc parameter $param" - md5sum=`echo $md5sum | awk '{print $1}'` - mkdir -p `dirname $local_file` && - echo $value > $local_file && - echo $md5sum > $local_file.sign && - sync_one $local_file + LF_SUM="$(printf %s "$LF_VALUE" | md5sum)" || + fatal "md5sum failed to produce hash for resource $LF_RSC parameter $LF_PARAM" + LF_SUM="$(echo "$LF_SUM" | awk '{print $1}')" + mkdir -p "$(dirname "$LF_FILE")" && + echo "$LF_VALUE" > "$LF_FILE" && + echo "$LF_SUM" > "${LF_FILE}.sign" && + sync_one "$LF_FILE" ;; remove) - rm -f $local_file - rm -f $local_file.sign - sync_one $local_file + rm -f "$LF_FILE" "${LF_FILE}.sign" + sync_one "$LF_FILE" ;; *) @@ -304,40 +305,44 @@ localfiles() { } get_local_param() { - local rsc=$1 param=$2 - localfiles get $rsc $param + GLP_RSC="$1" + GLP_PARAM="$2" + localfiles get "$GLP_RSC" "$GLP_PARAM" } set_local_param() { - local rsc=$1 param=$2 value=$3 - localfiles set $rsc $param $value + SLP_RSC="$1" + SLP_PARAM="$2" + SLP_VALUE="$3" + localfiles set "$SLP_RSC" "$SLP_PARAM" "$SLP_VALUE" } remove_local_param() { - local rsc=$1 param=$2 - localfiles remove $rsc $param + RLP_RSC="$1" + RLP_PARAM="$2" + localfiles remove "$RLP_RSC" "$RLP_PARAM" } cibsecret_set() { - local value=$1 + CS_VALUE="$1" - if [ -z "$NO_CRM" ]; then - [ "$current" -a "$current" != "$MAGIC" -a "$current" != "$value" ] && - fatal "CIB value <$current> different for $rsc parameter $param; please delete it first" - fi - set_local_param $rsc $param $value && - set_cib_param $rsc $param "$MAGIC" + [ -z "$NO_CRM" ] && + [ ! -z "$current" ] && + [ "$current" != "$MAGIC" ] && + [ "$current" != "$CS_VALUE" ] && + fatal "CIB value <$current> different for $rsc parameter $param; please delete it first" + set_local_param "$rsc" "$param" "$CS_VALUE" && + set_cib_param "$rsc" "$param" "$MAGIC" } cibsecret_check() { - local md5sum local_md5sum is_secret "$current" || fatal "resource $rsc parameter $param not set as secret, nothing to check" - local_md5sum=`localfiles getsum $rsc $param` - [ "$local_md5sum" ] || + CSC_LOCAL_SUM="$(localfiles getsum "$rsc" "$param")" + [ "$CSC_LOCAL_SUM" ] || fatal "no MD5 hash for resource $rsc parameter $param" - md5sum=`printf "$current_local" | md5sum | awk '{print $1}'` - [ "$md5sum" = "$local_md5sum" ] || + CSC_CALC_SUM="$(printf "%s" "$current_local" | md5sum | awk '{print $1}')" + [ "$CSC_CALC_SUM" = "$CSC_LOCAL_SUM" ] || fatal "MD5 hash mismatch for resource $rsc parameter $param" } @@ -347,13 +352,11 @@ cibsecret_get() { } cibsecret_delete() { - remove_local_param $rsc $param && - remove_cib_param $rsc $param + remove_local_param "$rsc" "$param" && remove_cib_param "$rsc" "$param" } cibsecret_stash() { - [ "$NO_CRM" ] && - fatal "no access to Pacemaker, stash not supported" + [ "$NO_CRM" ] && fatal "no access to Pacemaker, stash not supported" [ "$current" = "" ] && fatal "nothing to stash for resource $rsc parameter $param" is_secret "$current" && @@ -362,14 +365,13 @@ cibsecret_stash() { } cibsecret_unstash() { - [ "$NO_CRM" ] && - fatal "no access to Pacemaker, unstash not supported" + [ "$NO_CRM" ] && fatal "no access to Pacemaker, unstash not supported" [ "$current_local" = "" ] && fatal "nothing to unstash for resource $rsc parameter $param" is_secret "$current" || warn "resource $rsc parameter $param not set as secret, but we have local value so proceeding anyway" - remove_local_param $rsc $param && - set_cib_param $rsc $param $current_local + remove_local_param "$rsc" "$param" && + set_cib_param "$rsc" "$param" "$current_local" } cibsecret_sync() { @@ -384,10 +386,10 @@ if [ "$1" = "-C" ]; then shift 1 fi -cmd=$1 -rsc=$2 -param=$3 -value=$4 +cmd="$1" +rsc="$2" +param="$3" +value="$4" case "$cmd" in set) [ $# -ne 4 ] && usage 1 ;; @@ -402,14 +404,14 @@ case "$cmd" in *) usage 1 ;; esac -. @OCF_ROOT_DIR@/lib/heartbeat/ocf-shellfuncs +. "@OCF_ROOT_DIR@/lib/heartbeat/ocf-shellfuncs" check_env # we'll need these two often -current=`get_cib_param $rsc $param` -current_local=`get_local_param $rsc $param` +current="$(get_cib_param "$rsc" "$param")" +current_local="$(get_local_param "$rsc" "$param")" -cibsecret_$cmd $value +"cibsecret_$cmd" "$value" # vim: set expandtab tabstop=8 softtabstop=4 shiftwidth=4 textwidth=80: From 388ead80c82162dd052865b281135d5c99d51924 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Wed, 10 Jun 2020 13:39:59 -0500 Subject: [PATCH 4/9] Refactor: tools: simplify cibsecret code Functionize checking usage, eliminate an unnecessary layer of function wrapping, and don't check current parameter values unless needed. --- tools/cibsecret.in | 105 +++++++++++++++++++-------------------------- 1 file changed, 43 insertions(+), 62 deletions(-) diff --git a/tools/cibsecret.in b/tools/cibsecret.in index aa1efb67df0..1c10516bf6b 100644 --- a/tools/cibsecret.in +++ b/tools/cibsecret.in @@ -18,6 +18,7 @@ LRM_CIBSECRETS="@LRM_CIBSECRETS_DIR@" PROG="$(basename "$0")" SSH_OPTS="-o StrictHostKeyChecking=no" +MAGIC="lrm://" usage() { cat < different for $rsc parameter $param; please delete it first" - set_local_param "$rsc" "$param" "$CS_VALUE" && + [ ! -z "$CIBSET_CURRENT" ] && + [ "$CIBSET_CURRENT" != "$MAGIC" ] && + [ "$CIBSET_CURRENT" != "$CS_VALUE" ] && + fatal "CIB value <$CIBSET_CURRENT> different for $rsc parameter $param; please delete it first" + localfiles set "$rsc" "$param" "$CS_VALUE" && set_cib_param "$rsc" "$param" "$MAGIC" } cibsecret_check() { - is_secret "$current" || + is_secret "$(get_cib_param "$rsc" "$param")" || fatal "resource $rsc parameter $param not set as secret, nothing to check" CSC_LOCAL_SUM="$(localfiles getsum "$rsc" "$param")" [ "$CSC_LOCAL_SUM" ] || fatal "no MD5 hash for resource $rsc parameter $param" - CSC_CALC_SUM="$(printf "%s" "$current_local" | md5sum | awk '{print $1}')" + CSC_LOCAL_VALUE="$(localfiles get "$rsc" "$param")" + CSC_CALC_SUM="$(printf "%s" "$CSC_LOCAL_VALUE" | md5sum | awk '{print $1}')" [ "$CSC_CALC_SUM" = "$CSC_LOCAL_SUM" ] || fatal "MD5 hash mismatch for resource $rsc parameter $param" } cibsecret_get() { cibsecret_check - echo "$current_local" + localfiles get "$rsc" "$param" } cibsecret_delete() { - remove_local_param "$rsc" "$param" && remove_cib_param "$rsc" "$param" + localfiles remove "$rsc" "$param" && remove_cib_param "$rsc" "$param" } cibsecret_stash() { [ "$NO_CRM" ] && fatal "no access to Pacemaker, stash not supported" - [ "$current" = "" ] && + CIBSTASH_CURRENT="$(get_cib_param "$rsc" "$param")" + [ "$CIBSTASH_CURRENT" = "" ] && fatal "nothing to stash for resource $rsc parameter $param" - is_secret "$current" && + is_secret "$CIBSTASH_CURRENT" && fatal "resource $rsc parameter $param already set as secret, nothing to stash" - cibsecret_set "$current" + cibsecret_set "$CIBSTASH_CURRENT" } cibsecret_unstash() { [ "$NO_CRM" ] && fatal "no access to Pacemaker, unstash not supported" - [ "$current_local" = "" ] && + UNSTASH_LOCAL_VALUE="$(localfiles get "$rsc" "$param")" + [ "$UNSTASH_LOCAL_VALUE" = "" ] && fatal "nothing to unstash for resource $rsc parameter $param" - is_secret "$current" || + is_secret "$(get_cib_param "$rsc" "$param")" || warn "resource $rsc parameter $param not set as secret, but we have local value so proceeding anyway" - remove_local_param "$rsc" "$param" && - set_cib_param "$rsc" "$param" "$current_local" + localfiles remove "$rsc" "$param" && + set_cib_param "$rsc" "$param" "$UNSTASH_LOCAL_VALUE" } cibsecret_sync() { sync_files } -MAGIC="lrm://" -umask 0077 - +# Grab arguments if [ "$1" = "-C" ]; then NO_CRM=':' - shift 1 + shift fi - cmd="$1" rsc="$2" param="$3" value="$4" -case "$cmd" in - set) [ $# -ne 4 ] && usage 1 ;; - get) [ $# -ne 3 ] && usage 1 ;; - check) [ $# -ne 3 ] && usage 1 ;; - stash) [ $# -ne 3 ] && usage 1 ;; - unstash) [ $# -ne 3 ] && usage 1 ;; - delete) [ $# -ne 3 ] && usage 1 ;; - sync) [ $# -ne 1 ] && usage 1 ;; - --help) usage 0 ;; - --version) crm_attribute --version; exit $? ;; - *) usage 1 ;; -esac - -. "@OCF_ROOT_DIR@/lib/heartbeat/ocf-shellfuncs" - +# Ensure we have everything we need +check_usage "$cmd" $# check_env +umask 0077 -# we'll need these two often -current="$(get_cib_param "$rsc" "$param")" -current_local="$(get_local_param "$rsc" "$param")" +# for dirname() function (@TODO why are we replacing dirname?) +. "@OCF_ROOT_DIR@/lib/heartbeat/ocf-shellfuncs" "cibsecret_$cmd" "$value" From a9914da1d1419472206c9398a876aa168f9dc110 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Wed, 10 Jun 2020 13:45:17 -0500 Subject: [PATCH 5/9] Low: tools: check resource separately from managing parameter in cibsecret Previously, check_cib_rsc() redirected all output to /dev/null, then attempted to use the output as an error message. Don't null it. Also, check_cib_rsc() was called by get_cib_param(), which was called via command subsitution (i.e. `cmd` or $(cmd)), which means it was executed in a subshell. Therefore exiting in the failure condition did not exit the main script. Instead, call check_cib_rsc() before calling functions like get_cib_param(), so the failure message and exit take effect. --- tools/cibsecret.in | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/cibsecret.in b/tools/cibsecret.in index 1c10516bf6b..36e08201fd7 100644 --- a/tools/cibsecret.in +++ b/tools/cibsecret.in @@ -255,15 +255,12 @@ is_secret() { } check_cib_rsc() { - CCR_RSC="$1" - CCR_OUT="$($NO_CRM crm_resource -r "$CCR_RSC" -W >/dev/null 2>&1)" || - fatal "resource $CCR_RSC doesn't exist: $CCR_OUT" + CCR_OUT="$($NO_CRM crm_resource -r "$1" -W 2>&1)" || fatal "$CCR_OUT" } get_cib_param() { GCP_RSC="$1" GCP_PARAM="$2" - check_cib_rsc "$GCP_RSC" $NO_CRM crm_resource -r "$GCP_RSC" -g "$GCP_PARAM" 2>/dev/null } @@ -271,14 +268,12 @@ set_cib_param() { SET_RSC="$1" SET_PARAM="$2" SET_VAL="$3" - check_cib_rsc "$SET_RSC" $NO_CRM crm_resource -r "$SET_RSC" -p "$SET_PARAM" -v "$SET_VAL" 2>/dev/null } remove_cib_param() { RM_RSC="$1" RM_PARAM="$2" - check_cib_rsc "$RM_RSC" $NO_CRM crm_resource -r "$RM_RSC" -d "$RM_PARAM" 2>/dev/null } @@ -319,6 +314,7 @@ localfiles() { cibsecret_set() { CS_VALUE="$1" + check_cib_rsc "$rsc" CIBSET_CURRENT="$(get_cib_param "$rsc" "$param")" [ -z "$NO_CRM" ] && [ ! -z "$CIBSET_CURRENT" ] && @@ -330,6 +326,7 @@ cibsecret_set() { } cibsecret_check() { + check_cib_rsc "$rsc" is_secret "$(get_cib_param "$rsc" "$param")" || fatal "resource $rsc parameter $param not set as secret, nothing to check" CSC_LOCAL_SUM="$(localfiles getsum "$rsc" "$param")" @@ -347,11 +344,13 @@ cibsecret_get() { } cibsecret_delete() { + check_cib_rsc "$rsc" localfiles remove "$rsc" "$param" && remove_cib_param "$rsc" "$param" } cibsecret_stash() { [ "$NO_CRM" ] && fatal "no access to Pacemaker, stash not supported" + check_cib_rsc "$rsc" CIBSTASH_CURRENT="$(get_cib_param "$rsc" "$param")" [ "$CIBSTASH_CURRENT" = "" ] && fatal "nothing to stash for resource $rsc parameter $param" @@ -365,6 +364,7 @@ cibsecret_unstash() { UNSTASH_LOCAL_VALUE="$(localfiles get "$rsc" "$param")" [ "$UNSTASH_LOCAL_VALUE" = "" ] && fatal "nothing to unstash for resource $rsc parameter $param" + check_cib_rsc "$rsc" is_secret "$(get_cib_param "$rsc" "$param")" || warn "resource $rsc parameter $param not set as secret, but we have local value so proceeding anyway" localfiles remove "$rsc" "$param" && From 6100dd8c99f9d232cb2be80ae284cf028e21c28d Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Tue, 9 Jun 2020 12:52:21 -0500 Subject: [PATCH 6/9] Test: cts-cli: delete unused variables ... to make shellcheck happy --- cts/cts-cli.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/cts/cts-cli.in b/cts/cts-cli.in index d6425b608a2..1e585062ffe 100755 --- a/cts/cts-cli.in +++ b/cts/cts-cli.in @@ -66,8 +66,6 @@ CRM_EX_EXISTS=108 CRM_EX_MULTIPLE=109 CRM_EX_EXPIRED=110 CRM_EX_NOT_YET_IN_EFFECT=111 -CRM_EX_INDETERMINATE=112 -CRM_EX_UNSATISFIED=113 function test_assert() { target=$1; shift From a886051f98a59d4a01a81a180e1e890518bb811e Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Tue, 9 Jun 2020 12:56:12 -0500 Subject: [PATCH 7/9] Refactor: libstonithd: don't set variable value that will never be used ... to make static analysis happy --- lib/fencing/st_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fencing/st_client.c b/lib/fencing/st_client.c index 79bcbab2cb2..11543ccdbb1 100644 --- a/lib/fencing/st_client.c +++ b/lib/fencing/st_client.c @@ -2069,7 +2069,7 @@ stonith_api_validate(stonith_t *st, int call_options, const char *rsc_id, if (rc != pcmk_rc_ok) { crm_warn("Could not replace secret parameters for validation of %s: %s", agent, pcmk_rc_str(rc)); - rc = pcmk_rc2legacy(rc); + // rc is standard return value, don't return it in this function } #endif From 8b9dcbf792bf327db71e985ea5206438b35b9887 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Tue, 9 Jun 2020 13:03:06 -0500 Subject: [PATCH 8/9] Low: tools: verify newly created CIB connection is not NULL In practice it couldn't get out of cib_new_variant() with NULLs, but this makes static analysis happy. --- tools/crm_resource.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/crm_resource.c b/tools/crm_resource.c index 52ed9cb232a..6853ad5330e 100644 --- a/tools/crm_resource.c +++ b/tools/crm_resource.c @@ -1101,6 +1101,10 @@ main(int argc, char **argv) // Establish a connection to the CIB cib_conn = cib_new(); + if ((cib_conn == NULL) || (cib_conn->cmds == NULL)) { + CMD_ERR("Could not create CIB connection: %s", pcmk_strerror(rc)); + goto bail; + } rc = cib_conn->cmds->signon(cib_conn, crm_system_name, cib_command); if (rc != pcmk_ok) { CMD_ERR("Could not connect to the CIB: %s", pcmk_strerror(rc)); From ac89c661c552a067ee1894ee42ccf526667c94c7 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Mon, 15 Jun 2020 12:03:22 -0500 Subject: [PATCH 9/9] Fix: cibsecret: don't use pssh -q option unless supported --- tools/cibsecret.in | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/cibsecret.in b/tools/cibsecret.in index 36e08201fd7..9b74ba331a1 100644 --- a/tools/cibsecret.in +++ b/tools/cibsecret.in @@ -143,6 +143,11 @@ check_env() { if which pssh >/dev/null 2>&1; then rsh=pssh_fun rcp_to_from=pscp_fun + + # -q is a SUSE patch not present in upstream pssh + PSSH_QUIET_OPTION="" + pssh -q 2>&1|grep "no such option: -q" > /dev/null || + PSSH_QUIET_OPTION="-q" elif which pdsh >/dev/null 2>&1; then rsh=pdsh_fun rcp_to_from=pdcp_fun @@ -190,13 +195,13 @@ get_live_nodes() { } pssh_fun() { - pssh -qi -H "$LIVE_NODES" -x "$SSH_OPTS" -- "$@" + pssh $PSSH_QUIET_OPTION -i -H "$LIVE_NODES" -x "$SSH_OPTS" -- "$@" } pscp_fun() { PSCP_DEST="$1" shift - pscp -q -H "$LIVE_NODES" -x "-pr" -x "$SSH_OPTS" -- "$@" "$PSCP_DEST" + pscp $PSSH_QUIET_OPTION -H "$LIVE_NODES" -x "-pr" -x "$SSH_OPTS" -- "$@" "$PSCP_DEST" } pdsh_fun() {