diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ea22f5d..270ab14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -117,6 +117,31 @@ jobs: name: exitstatus-${{ github.job }} path: exitstatus/ + linux-unit-tests: + name: Linux Unit Tests + runs-on: ubuntu-latest + needs: collect-changed-files + if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || needs.collect-changed-files.outputs.run-tests == 'true' + steps: + - uses: actions/checkout@v6 + - name: Run Unit Tests + run: | + bash tests/linux/test_version_resolution.sh + bash tests/linux/test_minion_conf_parsing.sh + + - name: Set Exit Status + if: always() + run: | + mkdir exitstatus + echo "${{ job.status }}" > exitstatus/${{ github.job }} + + - name: Upload Exit Status + if: always() + uses: actions/upload-artifact@v4 + with: + name: exitstatus-${{ github.job }} + path: exitstatus/ + windows-2022: @@ -158,6 +183,7 @@ jobs: runs-on: ubuntu-latest needs: - lint + - linux-unit-tests - generate-actions-workflow - windows-2022 - rockylinux-9 diff --git a/.github/workflows/templates/ci.yml b/.github/workflows/templates/ci.yml index d1dd364..4cf7af3 100644 --- a/.github/workflows/templates/ci.yml +++ b/.github/workflows/templates/ci.yml @@ -116,3 +116,28 @@ jobs: with: name: exitstatus-${{ github.job }} path: exitstatus/ + + linux-unit-tests: + name: Linux Unit Tests + runs-on: ubuntu-latest + needs: collect-changed-files + if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || needs.collect-changed-files.outputs.run-tests == 'true' + steps: + - uses: actions/checkout@v6 + - name: Run Unit Tests + run: | + bash tests/linux/test_version_resolution.sh + bash tests/linux/test_minion_conf_parsing.sh + + - name: Set Exit Status + if: always() + run: | + mkdir exitstatus + echo "${{ job.status }}" > exitstatus/${{ github.job }} + + - name: Upload Exit Status + if: always() + uses: actions/upload-artifact@v4 + with: + name: exitstatus-${{ github.job }} + path: exitstatus/ diff --git a/.github/workflows/templates/generate.py b/.github/workflows/templates/generate.py index 4d20657..c5400c4 100755 --- a/.github/workflows/templates/generate.py +++ b/.github/workflows/templates/generate.py @@ -147,7 +147,7 @@ def print_upgrade_steps(): def generate_test_jobs(): test_jobs = "" - needs = ["lint", "generate-actions-workflow"] + needs = ["lint", "linux-unit-tests", "generate-actions-workflow"] test_jobs += "\n" for distro in WINDOWS: diff --git a/.github/workflows/test-linux.yml b/.github/workflows/test-linux.yml index 8146dcc..a8d9550 100644 --- a/.github/workflows/test-linux.yml +++ b/.github/workflows/test-linux.yml @@ -38,7 +38,6 @@ jobs: timeout-minutes: ${{ inputs.timeout }} strategy: - max-parallel: 2 fail-fast: false matrix: instance: ${{ fromJSON(inputs.instances) }} diff --git a/linux/svtminion.sh b/linux/svtminion.sh index 85950a5..fee0d97 100755 --- a/linux/svtminion.sh +++ b/linux/svtminion.sh @@ -692,6 +692,12 @@ _fetch_vmtools_salt_minion_conf_tools_conf() { fi elif [[ ${salt_config_flag} -eq 1 ]]; then # read config ahead of section check, better logic flow + if [[ "${line_value}" != *=* ]]; then + _warning_log "$0:${FUNCNAME[0]} ignoring invalid "\ + "config line '${line}' (expected key=value) "\ + "from ${vmtools_conf_file}" + continue + fi cfg_key=$(echo "${line}" | cut -d '=' -f 1) cfg_value=$(echo "${line}" | cut -d '=' -f 2) _update_minion_conf_ary "${cfg_key}" "${cfg_value}" || { @@ -741,6 +747,12 @@ _fetch_vmtools_salt_minion_conf_guestvars() { for idx in ${gvar_args} do + if [[ "${idx}" != *=* ]]; then + _warning_log "$0:${FUNCNAME[0]} ignoring invalid config token "\ + "'${idx}' (expected key=value) from guest variables "\ + "location ${guestvars_salt_args}" + continue + fi cfg_key=$(echo "${idx}" | cut -d '=' -f 1) cfg_value=$(echo "${idx}" | cut -d '=' -f 2) _update_minion_conf_ary "${cfg_key}" "${cfg_value}" || { @@ -782,6 +794,12 @@ _fetch_vmtools_salt_minion_conf_cli_args() { if [[ "${idx}" = --* ]]; then break fi + if [[ "${idx}" != *=* ]]; then + _warning_log "$0:${FUNCNAME[0]} ignoring invalid config "\ + "token '${idx}' (expected key=value) from command "\ + "line arguments" + continue + fi cfg_key=$(echo "${idx}" | cut -d '=' -f 1) cfg_value=$(echo "${idx}" | cut -d '=' -f 2) _update_minion_conf_ary "${cfg_key}" "${cfg_value}" || { diff --git a/tests/linux/test_minion_conf_parsing.sh b/tests/linux/test_minion_conf_parsing.sh new file mode 100644 index 0000000..aec3482 --- /dev/null +++ b/tests/linux/test_minion_conf_parsing.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# +# Lightweight, fixture-free regression test for linux/svtminion.sh's minion +# config-parsing logic (_update_minion_conf_ary / +# _fetch_vmtools_salt_minion_conf_guestvars / +# _fetch_vmtools_salt_minion_conf_tools_conf). +# +# Reproduces https://github.com/saltstack/salt-vmtools/issues/70: when the +# guestVar `vmware.components.salt_minion.args` (or a `tools.conf` +# `[salt_minion]` section) is populated with a raw CLI-argument string +# instead of the documented space-delimited `key=value` pairs, every +# whitespace-separated token -- switches, versions, URLs alike -- was being +# written to the minion config as a bogus self-mapped `token: token` entry. +# +# Like test_version_resolution.sh, this does not perform a real install and +# is not currently wired into CI (.github/workflows/test-linux.yml) -- it's +# meant to be run manually. +# +# Run directly: bash tests/linux/test_minion_conf_parsing.sh + +set -o nounset +set -o errexit +set -o pipefail + +_test_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +_repo_root="$(cd "${_test_dir}/../.." && pwd)" +_script="${_repo_root}/linux/svtminion.sh" + +# Stub the logging functions the functions under test depend on so they can +# run standalone without the rest of svtminion.sh's global state. +_error_log() { echo "ERROR: $*" 1>&2; } +_warning_log() { echo "WARNING: $*" 1>&2; } +_info_log() { :; } +_debug_log() { :; } + +# guestvars_salt_args is normally a readonly global set up earlier in the +# real script; the function under test only reads it, so stub it here. +guestvars_salt_args="guestinfo.vmware.components.salt_minion.args" + +# Extract the functions under test verbatim from the real script, so this +# test always exercises the current implementation rather than a +# hand-copied duplicate that could drift out of sync. +_extracted="$(mktemp)" +_fixture_dir="$(mktemp -d)" +trap 'rm -f "${_extracted}"; rm -rf "${_fixture_dir}"' EXIT + +sed -n '/^_update_minion_conf_ary() {/,/^}/p' "${_script}" > "${_extracted}" +sed -n '/^_fetch_vmtools_salt_minion_conf_guestvars() {/,/^}/p' "${_script}" >> "${_extracted}" + +# shellcheck disable=SC1090 +source "${_extracted}" + +_failed=0 + +# Mock `vmtoolsd --cmd "info-get ${guestvars_salt_args}"` the same way the +# real function invokes it. +vmtoolsd() { + if [[ "$1" = "--cmd" && "$2" = "info-get ${guestvars_salt_args}" ]]; then + echo "${_MOCK_GVAR_ARGS}" + return 0 + fi + return 1 +} + +_reset_conf_ary() { + m_cfg_keys=() + m_cfg_values=() +} + +# --- Case 1: CLI-style switches leaking into the guestVar must not produce +# --- any bogus config entries (the reported bug). +_reset_conf_ary +_MOCK_GVAR_ARGS="--minionversion 3007.1 --source http://example.com/artifactory/saltproject-generic/onedir --loglevel debug" +_fetch_vmtools_salt_minion_conf_guestvars + +if [[ ${#m_cfg_keys[@]} -ne 0 ]]; then + echo "FAILED: CLI-style guestVar args produced ${#m_cfg_keys[@]} bogus config" \ + "entries (expected 0): ${m_cfg_keys[*]}" + _failed=1 +else + echo "OK: CLI-style guestVar args produced no bogus config entries" +fi + +# --- Case 2: legitimate key=value guestVar args must still work. +_reset_conf_ary +_MOCK_GVAR_ARGS="master=gv_master id=gv_minion" +_fetch_vmtools_salt_minion_conf_guestvars + +_found_master=0 +_found_id=0 +for ((_i=0; _i<${#m_cfg_keys[@]}; _i++)); do + if [[ "${m_cfg_keys[${_i}]}" = "master" && "${m_cfg_values[${_i}]}" = "gv_master" ]]; then + _found_master=1 + fi + if [[ "${m_cfg_keys[${_i}]}" = "id" && "${m_cfg_values[${_i}]}" = "gv_minion" ]]; then + _found_id=1 + fi +done + +if [[ ${_found_master} -ne 1 || ${_found_id} -ne 1 ]]; then + echo "FAILED: valid key=value guestVar args were not parsed correctly:" \ + "keys='${m_cfg_keys[*]:-}' values='${m_cfg_values[*]:-}'" + _failed=1 +else + echo "OK: valid key=value guestVar args parsed correctly" +fi + +if [[ "${_failed}" -ne 0 ]]; then + echo "test_minion_conf_parsing.sh: FAILED" + exit 1 +fi + +echo "test_minion_conf_parsing.sh: All tests passed" +exit 0