From 450fa51500a9cd981087bed1b35cebf2486b5636 Mon Sep 17 00:00:00 2001 From: Georg Pfuetzenreuter Date: Jan 05 2024 23:13:23 +0000 Subject: Combined improvement of shell scripts - quote arrays/variables to avoid globbing/splitting - exit on failed cd calls - group file directs for consecutive command outputs - define default exit values and set exit value explicitly for tests which succeed - comment vim modeline instructions - use uniform quoting style - use uniform indentation This makes ShellCheck pass with no errors/warnings. Signed-off-by: Georg Pfuetzenreuter --- diff --git a/bin/test_empty_files.sh b/bin/test_empty_files.sh index ee7b888..fb3a83f 100755 --- a/bin/test_empty_files.sh +++ b/bin/test_empty_files.sh @@ -4,8 +4,8 @@ EMPTY=$(find . -type f -empty) if [[ -n $EMPTY ]]; then echo "The following files are empty:" - for file in ${EMPTY[@]}; do - echo ${file} + for file in "${EMPTY[@]}"; do + echo "${file}" done exit 1 fi diff --git a/bin/test_extension.sh b/bin/test_extension.sh index e35ba47..ebab29a 100755 --- a/bin/test_extension.sh +++ b/bin/test_extension.sh @@ -4,10 +4,10 @@ NON_SLS_PILLAR=$(find pillar -type f | grep -Ev "(/(macros|map)\.jinja|(FORMULAS NON_SLS_SALT=$(find salt -type f | grep -Ev "/(files|templates)/|(pillar\.example|\.sls|/README\.md)$|/(_grains|_modules|_runners|_states)/\w+\.py$") NON_SLS=( ${NON_SLS_PILLAR} ${NON_SLS_SALT} ) -if [[ -n $NON_SLS ]]; then +if [[ -n "${NON_SLS[0]}" ]]; then echo "The following files are missing the .sls extension:" - for file in ${NON_SLS[@]}; do - echo ${file} + for file in "${NON_SLS[@]}"; do + echo "${file}" done exit 1 fi diff --git a/bin/test_haproxy.sh b/bin/test_haproxy.sh index 4c086f7..88b21e2 100755 --- a/bin/test_haproxy.sh +++ b/bin/test_haproxy.sh @@ -5,7 +5,7 @@ set -Cu # systemctl refuses to work in a container, but is needed by service.running. Replace it with /usr/bin/true to avoid useless error messages and breakage. -( cd /usr/bin/ ; ln -sf true systemctl ) +( cd /usr/bin/ || exit 1 ; ln -sf true systemctl ) loglevel='info' logbase_salt='log_salt' @@ -40,7 +40,7 @@ gen_ssl () { cat test/fixtures/domain.{crt,key} > "$out" fi fi - done <<< "$(grep -hoPr '/etc/(ssl/services/(.*.pem|)|haproxy/.*.crt)' pillar/cluster/$1/)" + done <<< "$(grep -hoPr '/etc/(ssl/services/(.*.pem|)|haproxy/.*.crt)' "pillar/cluster/$1/")" } check_haproxy () { diff --git a/bin/test_highstate.sh b/bin/test_highstate.sh index 32f62e5..0e876fd 100755 --- a/bin/test_highstate.sh +++ b/bin/test_highstate.sh @@ -9,7 +9,7 @@ role="$1" [[ $(whoami) == 'root' ]] || { echo 'Please run this script as root'; exit 1; } # sysctl: cannot stat /proc/sys/net/core/netdev_max_backlog (and some other /proc files): No such file or directory -( cd /sbin/ ; ln -sf /usr/bin/true sysctl ) +( cd /sbin/ || exit 1 ; ln -sf /usr/bin/true sysctl ) source bin/get_colors.sh @@ -27,7 +27,7 @@ printf 'roles:\n- %s' "$role" >> "$IDFILE" if [ -x "test/setup/role/$role" ] then echo "Preparing test environment for role $role ..." >> "$out" - test/setup/role/$role + "test/setup/role/$role" fi salt --out=raw --out-file=/dev/null "$HOSTNAME" saltutil.refresh_pillar @@ -53,4 +53,4 @@ journalctl --no-pager > system/journal.txt echo 'Output and logs can be found in the job artifacts!' exit $rolestatus -vim:expandtab +# vim:expandtab diff --git a/bin/test_infra_data.sh b/bin/test_infra_data.sh index c755fc2..c13c2db 100755 --- a/bin/test_infra_data.sh +++ b/bin/test_infra_data.sh @@ -42,10 +42,10 @@ echo echo "test_infra_data --> Results: YAML -> $RESULT_YAML, SCHEMA -> $RESULT_SCHEMA, SORT -> $RESULT_SORT" -if [ "$RESULT_YAML" == 0 -a "$RESULT_SCHEMA" == 0 -a "$RESULT_SORT" == 0 ] +if [ "$RESULT_YAML" = 0 ] && [ "$RESULT_SCHEMA" = 0 ] && [ "$RESULT_SORT" = 0 ] then exit 0 -elif [ "$RESULT_SORT" == 1 ] # 123 in case of xargs +elif [ "$RESULT_SORT" = 1 ] # 123 in case of xargs then echo 'Please run bin/sort_yaml.py against the YAML files you modified and amend your commit.' fi diff --git a/bin/test_nginx.sh b/bin/test_nginx.sh index 7048bf9..041312f 100755 --- a/bin/test_nginx.sh +++ b/bin/test_nginx.sh @@ -1,4 +1,5 @@ #!/bin/bash +STATUS=0 # Validate the salt-generated nginx configs @@ -7,7 +8,7 @@ rpm -qa --qf '%{name}\n' | sort > /tmp/packages-before [[ $(whoami) == 'root' ]] || { echo 'Please run this script as root'; exit 1; } # using a container without systemd, but systemd is needed by service.running. replace it with /usr/bin/true to avoid useless error messages. -( cd /usr/bin/ ; ln -sf true systemctl ) +( cd /usr/bin/ || exit 1 ; ln -sf true systemctl ) source bin/get_colors.sh @@ -19,23 +20,23 @@ create_fake_certs() { # - the key is encrypted and the CI worker can't decrypt it # - the nginx validation command tries to match the pair - PRIVATE_KEYS=( $(grep ssl_certificate_key pillar/role/$role.sls | cut -d':' -f2) ) - for key in ${PRIVATE_KEYS[@]}; do + PRIVATE_KEYS=( $(grep ssl_certificate_key "pillar/role/$role.sls" | cut -d':' -f2) ) + for key in "${PRIVATE_KEYS[@]}"; do if [[ ${key##*.} != 'key' ]]; then echo "pillar/role/$role.sls \"ssl_certificate_key: $key\" should have extension .key" STATUS=1 else - cp test/fixtures/domain.key $key + cp test/fixtures/domain.key "$key" fi done - PUBLIC_CERTS=( $(grep "ssl_certificate:" pillar/role/$role.sls | cut -d':' -f2) ) - for cert in ${PUBLIC_CERTS[@]}; do + PUBLIC_CERTS=( $(grep "ssl_certificate:" "pillar/role/$role.sls" | cut -d':' -f2) ) + for cert in "${PUBLIC_CERTS[@]}"; do if [[ ${cert##*.} != 'crt' ]]; then echo "pillar/role/$role.sls \"ssl_certificate: $cert\" should have extension .crt" STATUS=1 else - cp test/fixtures/domain.crt $cert + cp test/fixtures/domain.crt "$cert" fi done } @@ -65,16 +66,16 @@ out="$role.txt" echo "START OF $role" > "$out" echo_INFO "Testing role: $role" -printf "roles:\n- $role" >> "$IDFILE" +printf 'roles:\n- %s' "$role" >> "$IDFILE" # Reset the grains-retrieved IPs to 127.0.0.1, as `nginx -t` actually tries # to bind to any configured listen IP -sed -i -e "s/{{ ip4_.* }}/127.0.0.1/g" pillar/role/$role.sls +sed -i -e "s/{{ ip4_.* }}/127.0.0.1/g" "pillar/role/$role.sls" if grep -q profile "$sls_role" then #for profile in "$(grep -h '\- profile' $sls_role | yq -o t)" // to-do: add yq to container - for profile in $(grep -h '\- profile' $sls_role | sed 's/^\s\+ -//' | tr '\n' ' ') + for profile in $(grep -h '\- profile' "$sls_role" | sed 's/^\s\+ -//' | tr '\n' ' ') do if [ ! "$profile" == 'profile.web.server.nginx' ] then @@ -101,7 +102,7 @@ fi echo 'Applying nginx ...' >> "$out" salt-call --local state.apply nginx >> "$out" || rolestatus=1 create_fake_certs -touch_includes $role +touch_includes "$role" printf '\nTesting configuration ...\n' >> "$out" mispipe 'nginx -tq' "tee -a $out" || rolestatus=1 @@ -133,6 +134,6 @@ rpm -qa --qf '%{name}\n' | sort > /tmp/packages-after diff -U0 /tmp/packages-before /tmp/packages-after || echo '=== The packages listed above were installed by one of the roles. Consider to add them to the container image to speed up this test.' -exit $STATUS +exit "$STATUS" -vim:expandtab +# vim:expandtab diff --git a/bin/test_show_highstate.sh b/bin/test_show_highstate.sh index 76743be..9f1ad34 100755 --- a/bin/test_show_highstate.sh +++ b/bin/test_show_highstate.sh @@ -13,9 +13,8 @@ if [[ $(whoami) != 'root' ]]; then fi fi -RUN_TEST="salt-call --local --retcode-passthrough state.show_highstate" -## in case of problems feel free to temporally enable line 18 and commend out line 16. -#RUN_TEST="salt-call --local --retcode-passthrough --log-level=debug state.show_highstate" +RUN_TEST='salt-call --local --retcode-passthrough state.show_highstate' +STATUS=0 write_grains() { $SUDO sed -i -e "s/\(country:\).*/\1 $1/" -e "s/\(domain:\).*/\1 $3/" -e "s/\(virtual:\).*/\1 $2/" /etc/salt/grains @@ -24,7 +23,7 @@ write_grains() { show_highstate() { local outfile="$domain.txt" - write_grains $country $virtual $domain + write_grains "$country" "$virtual" "$domain" $RUN_TEST > "$outfile" 2>&1 _STATUS=$? # We ignore exit code 2 as it means that an empty file is produced @@ -32,26 +31,27 @@ show_highstate() { if [[ $_STATUS -eq 0 ]] || [[ $_STATUS -eq 2 ]]; then echo_PASSED else - cat /etc/salt/grains >> "$outfile" - salt-call --local grains.get id >> "$outfile" - salt-call --local grains.get domain >> "$outfile" - salt-call --local grains.get virtual >> "$outfile" - echo 'Dumping the last 100 log lines - the full output can be found in the CI artifacts' - tail -n100 "$outfile" - echo_FAILED - STATUS=1 + { + cat /etc/salt/grains + salt-call --local grains.get id + salt-call --local grains.get domain + salt-call --local grains.get virtual + } >> "$outfile" + echo 'Dumping the last 100 log lines - the full output can be found in the CI artifacts' + tail -n100 "$outfile" + echo_FAILED + STATUS=1 fi echo } ALL_LOCATIONS=( $(bin/get_valid_custom_grains.py) ) -for country in ${ALL_LOCATIONS[@]}; do - default_domain=$(bin/get_valid_custom_grains.py --default-domain $country) +for country in "${ALL_LOCATIONS[@]}"; do virtual='kvm' - DOMAINS=( $(bin/get_valid_custom_grains.py -d $country) ) - for domain in ${DOMAINS[@]}; do + DOMAINS=( $(bin/get_valid_custom_grains.py -d "$country") ) + for domain in "${DOMAINS[@]}"; do show_highstate done done -exit $STATUS +exit "$STATUS" diff --git a/bin/test_validate.sh b/bin/test_validate.sh index a626ca9..d86ebff 100755 --- a/bin/test_validate.sh +++ b/bin/test_validate.sh @@ -1,4 +1,5 @@ #!/bin/bash +STATUS=0 # Run various code validation/syntax checks @@ -13,9 +14,9 @@ TESTS=( infra_data.sh ) -for _test in ${TESTS[@]}; do +for _test in "${TESTS[@]}"; do echo_INFO "## Running test_${_test}" - if bin/test_${_test}; then + if "bin/test_${_test}"; then echo_PASSED else echo_FAILED @@ -24,4 +25,4 @@ for _test in ${TESTS[@]}; do echo done -exit $STATUS +exit "$STATUS"