bash_replace_or_append fixes

shared/macros/10-bash.jinja

@@ -1465,6 +1465,22 @@ cce="{{{ cce_identifiers['cce'] }}}"
 {{%- endmacro -%}}
 
 
+{{#
+    Ensure file ends with newline
+
+    Do not modify file at all if there already is newline. Always follows
+    symlinks.
+
+    :param file:     file to check
+#}}
+{{%- macro bash_ensure_nl_at_eof(file) -%}}
+{{#- Plain sed '$a\' updates stat even if it dones not change the file. -#}}
+if [[ -s "{{{ file }}}" ]] && [[ -n "$(tail -c 1 -- "{{{ file }}}" || true)" ]]; then
+    LC_ALL=C sed -i --follow-symlinks '$a'\\ "{{{ file }}}"
+fi
+{{%- endmacro -%}}
+
+
 {{#
     Macro to replace configuration setting in config file or add the configuration setting if
     it does not exist.
@@ -1491,14 +1507,6 @@ cce="{{{ cce_identifiers['cce'] }}}"
 
 #}}
 {{%- macro bash_replace_or_append(config_file, key, value, format='%s = %s') -%}}
-
-# Test if the config_file is a symbolic link. If so, use --follow-symlinks with sed.
-# Otherwise, regular sed command will do.
-sed_command=('sed' '-i')
-if test -L "{{{ config_file }}}"; then
-    sed_command+=('--follow-symlinks')
-fi
-
 # Strip any search characters in the key arg so that the key can be replaced without
 # adding any search characters to the config file.
 stripped_key=$(sed 's/[\^=\$,;+]*//g' <<< "{{{ key }}}")
@@ -1511,12 +1519,12 @@ printf -v formatted_output "{{{ format }}}" "$stripped_key" "{{{ value }}}"
 # so if we search for 'setting', 'setting2' won't match.
 if LC_ALL=C grep -q -m 1 -i -e "{{{ key }}}\\>" "{{{ config_file }}}"; then
     escaped_formatted_output=$(sed -e 's|/|\\/|g' <<< "$formatted_output")
-    "${sed_command[@]}" "s/{{{ key }}}\\>.*/$escaped_formatted_output/gi" "{{{ config_file }}}"
+    LC_ALL=C sed -i --follow-symlinks "s/{{{ key }}}\\>.*/$escaped_formatted_output/gi" "{{{ config_file }}}"
 else
-    # \n is precaution for case where file ends without trailing newline
-    {{% if cce_identifiers and 'cce' in cce_identifiers -%}}
+    {{{ bash_ensure_nl_at_eof(config_file) | indent }}}
+    {{%- if cce_identifiers and 'cce' in cce_identifiers %}}
     {{{ set_cce_value() }}}
-    printf '\n# Per %s: Set %s in %s\n' "$cce" "$formatted_output" "{{{ config_file }}}" >> "{{{ config_file }}}"
+    printf '# Per %s: Set %s in %s\n' "${cce}" "${formatted_output}" "{{{ config_file }}}" >> "{{{ config_file }}}"
     {{%- endif %}}
     printf '%s\n' "$formatted_output" >> "{{{ config_file }}}"
 fi

tests/unit/bash/bash_os_linux_conditional.bats.jinja

@@ -1,8 +1,46 @@
 #!/bin/bash
 
-@test "bash_os_linux_conditional - test OS release - RHEL" {
-    os_release_path="$(mktemp)"
+set -pu
+
+is_old_bats=0
+
+setup() {
+    if [[ -z "${BATS_TEST_TMPDIR:-}" ]] || [[ ! -d "${BATS_TEST_TMPDIR}" ]]; then
+        BATS_TEST_TMPDIR="$(mktemp -d)"  # 1.4.0
+        # shellcheck disable=SC2034
+        BATS_TEARDOWN_STARTED=  # 1.3.0
+        is_old_bats=1
+    else
+        is_old_bats=0
+    fi
+    pushd "${BATS_TEST_TMPDIR}" || exit 1
+    os_release_path="os-release"
+}
+
+teardown() {
+    if (( is_old_bats )); then
+        if [[ -z "${BATS_TEST_TMPDIR:-}" ]] || [[ ! -d "${BATS_TEST_TMPDIR}" ]]; then
+            >&2 echo "INTERNAL ERROR"
+            exit 3
+        fi
+        local tmppath xpwd
+        tmppath="$(readlink -f -- "${BATS_TEST_TMPDIR}")"
+        if [[ ! "${tmppath}" =~ ^/tmp/ ]] || [[ ! -d "${tmppath}" ]]; then
+            >&2 echo "INTERNAL ERROR"
+            exit 3
+        fi
+        xpwd="$(readlink -f -- .)"
+        if [[ "${tmppath}" != "${xpwd}" ]]; then
+            >&2 echo "INTERNAL ERROR"
+            exit 3
+        fi
+        popd || exit 1
+        rm -rf -- "${tmppath}"
+        BATS_TEST_TMPDIR=""
+    fi
+}
 
+@test "bash_os_linux_conditional - test OS release - RHEL" {
     cat << EOF > "$os_release_path"
 NAME="Red Hat Enterprise Linux"
 VERSION="9.2 (Plow)"
@@ -42,13 +80,9 @@ EOF
     ! ( {{{ bash_os_linux_conditional("fedora", "38", "==", "$os_release_path") }}} )
     ! ( {{{ bash_os_linux_conditional("fedora", "9.2", "==", "$os_release_path") }}} )
     ! ( {{{ bash_os_linux_conditional("fedora", "9.4", "<", "$os_release_path") }}} )
-
-    rm -rf "$os_release_path"
 }
 
 @test "bash_os_linux_conditional - test OS release - Ubuntu" {
-    os_release_path="$(mktemp)"
-
     cat << EOF > "$os_release_path"
 PRETTY_NAME="Ubuntu 22.04.1 LTS"
 NAME="Ubuntu"
@@ -82,6 +116,4 @@ EOF
 
     ! ( {{{ bash_os_linux_conditional("fedora", "38", "==", "$os_release_path") }}} )
     ! ( {{{ bash_os_linux_conditional("fedora", "22.10", "<", "$os_release_path") }}} )
-
-    rm -rf "$os_release_path"
 }

tests/unit/bash/bash_pkg_conditional_rpm.bats.jinja

@@ -1,5 +1,7 @@
 #!/bin/bash
 
+set -pu
+
 # Mock the "rpm" command
 # mocked package: coconut-4.5.17
 # The mock is used to avoid the need to have a specific RPM package in a

tests/unit/bash/execute_tests.sh

@@ -1,10 +1,90 @@
 #!/bin/bash
 
-PYTHON_EXECUTABLE="$1"
-TESTS_ROOT="$2"
-TESTDIR="$3"
-OUTDIR="$4"
+set -epu
 
-mkdir -p "$OUTDIR"
+usage() {
+    printf "Usage: %s [OPTIONS] PYTHON_EXECUTABLE TESTS_ROOT TESTDIR OUTDIR [bats opts]" "${0##/}"
+    printf "       %s [--help]" "${0##/}"
+    printf "\nOPTIONS\n"
+    printf "    --verbose\n"
+    printf "    --quiet\n"
+    printf "    --debug\n"
+    printf "    --parallel | --no-parallel\n"
+    printf "    --\n"
+}
 
-PYTHONPATH="$TESTS_ROOT/.." ${PYTHON_EXECUTABLE} "$TESTS_ROOT/../build-scripts/expand_jinja.py" --outdir "$OUTDIR" $TESTDIR/*.jinja && bats "$OUTDIR"
+OPT_debug=0
+OPT_parallel=0
+if [[ -x /usr/bin/parallel ]]; then
+    OPT_parallel=1
+fi
+
+OPT_verbose=1
+while (( $# )); do
+    case "$1" in
+        --verbose) (( OPT_verbose++, 1 )); shift ;;
+        --quiet) OPT_verbose=0; shift ;;
+        --debug) set -x; OPT_debug=1; shift ;;
+
+        --parallel) OPT_parallel=1; shift ;;
+        --no[_-]parallel) OPT_parallel=0; shift ;;
+
+        --help) usage; exit 2 ;;
+        --) shift; break ;;
+
+        *) break ;;
+    esac
+done
+
+bats_version="$(bats -v)" || :
+case "${bats_version##* }" in
+    # Debian 10 v0.4.0
+    # Usage: bats [-c] [-p | -t]
+    ""|"0."*|"1.0."*|"1.1."*)
+        OPT_parallel=0
+        OPT_verbose=0
+        OPT_debug=0
+        ;;
+    # Ubuntu 22.04
+    # Error: Bad command line option '--print-output-on-failure'
+    "1.2."*|"1.3."*|"1.4."*)
+        OPT_verbose=0
+        OPT_debug=0
+        ;;
+esac
+
+PYTHON_EXECUTABLE="$1"; shift
+TESTS_ROOT="$1"; shift
+TESTDIR="$1"; shift
+OUTDIR="$1"; shift
+
+mkdir -p "${OUTDIR}"
+
+bats_opts=()
+
+if (( OPT_parallel )); then
+    bats_opts+=(--jobs "$(nproc)")  # 1.2.0
+fi
+
+if (( OPT_verbose > 1 )); then
+    bats_opts+=(--verbose-run)  # 1.5.0
+elif (( OPT_verbose == 1 )); then
+    bats_opts+=(--print-output-on-failure)  # 1.5.0
+fi
+
+if (( OPT_debug )); then
+    bats_opts+=(
+        --no-tempdir-cleanup  # 1.4.0
+        --trace  # 1.5.0
+    )
+fi
+
+rc=0
+PYTHONPATH="${TESTS_ROOT}/.." ${PYTHON_EXECUTABLE} \
+"${TESTS_ROOT}/../build-scripts/expand_jinja.py" --outdir "${OUTDIR}" \
+    "${TESTDIR}"/*.jinja || rc=$?
+if (( rc )); then
+    >&2 echo "ERROR: expand_jinja.py could not create bats files"
+    exit "${rc}"
+fi
+bats "${bats_opts[@]}" "$@" "${OUTDIR}"

tests/unit/bash/test_bash_ensure_ini_config.bats.jinja

@@ -1,25 +1,60 @@
 #!/bin/bash
 
+set -pu
 
 function call_bash_ensure_ini_config {
     {{{ bash_ensure_ini_config("$1", "$2", "$3", "$4") | indent(4) }}}
 }
 
+is_old_bats=0
+
+setup() {
+    if [[ -z "${BATS_TEST_TMPDIR:-}" ]] || [[ ! -d "${BATS_TEST_TMPDIR}" ]]; then
+        BATS_TEST_TMPDIR="$(mktemp -d)"  # 1.4.0
+        # shellcheck disable=SC2034
+        BATS_TEARDOWN_STARTED=  # 1.3.0
+        is_old_bats=1
+    else
+        is_old_bats=0
+    fi
+    pushd "${BATS_TEST_TMPDIR}" || exit 1
+    mkdir -p sssd_test
+}
+
+teardown() {
+    if (( is_old_bats )); then
+        if [[ -z "${BATS_TEST_TMPDIR:-}" ]] || [[ ! -d "${BATS_TEST_TMPDIR}" ]]; then
+            >&2 echo "INTERNAL ERROR"
+            exit 3
+        fi
+        local tmppath xpwd
+        tmppath="$(readlink -f -- "${BATS_TEST_TMPDIR}")"
+        if [[ ! "${tmppath}" =~ ^/tmp/ ]] || [[ ! -d "${tmppath}" ]]; then
+            >&2 echo "INTERNAL ERROR"
+            exit 3
+        fi
+        xpwd="$(readlink -f -- .)"
+        if [[ "${tmppath}" != "${xpwd}" ]]; then
+            >&2 echo "INTERNAL ERROR"
+            exit 3
+        fi
+        popd || exit 1
+        rm -rf -- "${tmppath}"
+        BATS_TEST_TMPDIR=""
+    fi
+}
+
 @test "bash_ensure_ini_config - Basic value remediation" {
-    mkdir sssd_test
     printf "[pam]\npam_cert_auth = false\n" > sssd_test/sssd.conf
     expected_output="[pam]\npam_cert_auth = true\n"
 
     call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"
 
     run diff "sssd_test/sssd.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-
-    rm -rf sssd_test
 }
 
 @test "bash_ensure_ini_config - Value remediation in multiple files" {
-    mkdir sssd_test
     printf "[pam]\npam_cert_auth = false\n" > sssd_test/sssd.conf
     printf "[pam]\npam_cert_auth = false\n" > pam_cert_auth.conf
     expected_output="[pam]\npam_cert_auth = true\n"
@@ -31,35 +66,26 @@ function call_bash_ensure_ini_config {
 
     run diff "pam_cert_auth.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-
-    rm pam_cert_auth.conf
-    rm -rf sssd_test
 }
 
 @test "bash_ensure_ini_config - No remediation happened" {
-    mkdir sssd_test
     printf "[pam]\npam_cert_auth = true\n" > sssd_test/sssd.conf
     expected_output="[pam]\npam_cert_auth = true\n"
 
     call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"
 
     run diff "sssd_test/sssd.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-
-    rm -rf sssd_test
 }
 
 @test "bash_ensure_ini_config - Append section with option to empty file" {
-    mkdir sssd_test
     printf "" > sssd_test/sssd.conf
     expected_output="[pam]\npam_cert_auth = true\n"
 
     call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"
 
     run diff "sssd_test/sssd.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-
-    rm -rf sssd_test
 }
 
 @test "bash_ensure_ini_config - Create file with section and option" {
@@ -69,40 +95,30 @@ function call_bash_ensure_ini_config {
 
     run diff "sssd_test/sssd.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-
-    rm -rf sssd_test
 }
 
 @test "bash_ensure_ini_config - Append option to section" {
-    mkdir sssd_test
     printf "[pam]\n" > sssd_test/sssd.conf
     expected_output="[pam]\npam_cert_auth = true\n"
 
     call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"
 
     run diff "sssd_test/sssd.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-
-    rm -rf sssd_test
 }
 
 @test "bash_ensure_ini_config - Append option to section when section is substring of option" {
-    mkdir sssd_test
     printf "[pam]\n" > sssd_test/sssd.conf
     expected_output="[pam]\npam_verbosity = 1\npam_cert_auth = true\n"
 
     call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"
     call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_verbosity" "1"
 
     run diff "sssd_test/sssd.conf" <(printf "$expected_output")
-
-    rm -rf sssd_test
-
     [ "$status" -eq 0 ]
 }
 
 @test "bash_ensure_ini_config - Append option to section in multiple files" {
-    mkdir sssd_test
     printf "[pam]\n" > sssd_test/sssd.conf
     printf "[pam]\n" > pam_cert_auth.conf
     expected_output="[pam]\npam_cert_auth = true\n"
@@ -111,23 +127,17 @@ function call_bash_ensure_ini_config {
 
     run diff "sssd_test/sssd.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-    
+
      run diff "pam_cert_auth.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-
-    rm pam_cert_auth.conf
-    rm -rf sssd_test
 }
 
 @test "bash_ensure_ini_config - Append section with option to non-empty file" {
-    mkdir sssd_test
     printf "[section]\nkey = value\n" > sssd_test/sssd.conf
     expected_output="[section]\nkey = value\n[pam]\npam_cert_auth = true\n"
 
     call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"
 
     run diff "sssd_test/sssd.conf" <(printf "$expected_output")
     [ "$status" -eq 0 ]
-
-    rm -rf sssd_test
 }