ARO-26543: add ShellSpec and shfmt validation for first-wave bash scripts

.editorconfig

@@ -1,6 +1,13 @@
-# top-most EditorConfig file
 root = true
 
+[*.sh]
+indent_style = space
+indent_size = 4
+shell_variant = bash
+binary_next_line = true
+switch_case_indent = true
+space_redirects = true
+
 # Unix-style newlines with a newline ending every file
 [*]
 end_of_line = lf

.github/workflows/ci-bash.yml

@@ -0,0 +1,32 @@
+name: ci-bash
+
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+  pull_request:
+
+permissions:
+  contents: read
+
+jobs:
+  bash-unit-tests:
+    name: bash-unit-tests
+    runs-on: ubuntu-latest
+    steps:
+      - name: Harden Runner
+        uses: step-security/harden-runner@fe104658747b27e96e4f7e80cd0a94068e53901d # v2.16.1
+        with:
+          egress-policy: audit
+      - name: Checkout
+        uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+      - name: Install shfmt
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y shfmt
+      - name: Run bash validation
+        env:
+          BASH_TEST_JUNIT: "0"
+        run: make validate-bash

.shellspec

@@ -0,0 +1,4 @@
+--default-path test/bash/spec
+--helperdir test/bash/spec
+--shell bash
+--execdir @project

Makefile

@@ -7,6 +7,12 @@ E2E_FLAGS ?= -test.v --ginkgo.vv --ginkgo.timeout 180m --ginkgo.flake-attempts=2
 E2E_LABEL ?= !smoke&&!regressiontest
 GO_FLAGS ?= -tags=containers_image_openpgp,exclude_graphdriver_btrfs,exclude_graphdriver_devicemapper
 OC ?= oc
+SHFMT ?= shfmt
+
+# Keep Go-templated dnsmasq hooks out of the raw shfmt list; the surrounding
+# gotmpl wrappers are not valid shfmt input.
+BASH_FMT_FILES := hack/unit-test-bash.sh $(wildcard test/bash/spec/*.sh test/bash/spec/support/*.sh)
+BASH_FMT_FILES_ABS := $(abspath $(BASH_FMT_FILES))
 
 # When Go is not installed on the host (e.g., CI Docker-only jobs), provide
 # safe fallback values. Without this guard, every $(shell go ...) call —
@@ -386,6 +392,24 @@ unit-test-go: $(GOTESTSUM)
 unit-test-go-coverpkg: $(GOTESTSUM)
 	$(GOTESTSUM) --format pkgname --junitfile report.xml -- -coverpkg=./... -coverprofile=cover_coverpkg.out ./...
 
+.PHONY: unit-test-bash
+unit-test-bash:
+	bash ./hack/unit-test-bash.sh
+
+.PHONY: fmt-bash
+fmt-bash: ## Format bash framework files using shfmt and .editorconfig
+	$(SHFMT) -w $(BASH_FMT_FILES_ABS)
+
+.PHONY: validate-fmt-bash
+validate-fmt-bash: ## Verify bash framework files are formatted
+	if ! $(SHFMT) -d $(BASH_FMT_FILES_ABS); then \
+		echo "You need to run 'make fmt-bash' to update bash formatting and commit the changes"; \
+		exit 1; \
+	fi
+
+.PHONY: validate-bash
+validate-bash: validate-fmt-bash unit-test-bash ## Validate bash formatting and unit tests
+
 .PHONY: fmt
 fmt: $(GOLANGCI_LINT) ## Format Go source files using golangci-lint formatters (gci, gofumpt)
 	$(GOLANGCI_LINT) fmt

docs/testing.md

@@ -24,6 +24,14 @@ To run python client and `az aro` CLI tests:
 make test-python
 ```
 
+To run bash unit tests for the critical VMSS bootstrap, recovery, and dnsmasq shell assets:
+
+```bash
+make unit-test-bash
+```
+
+The bash suite uses a pinned `ShellSpec` container image via `hack/unit-test-bash.sh`, so the same command works on Linux and macOS as long as Docker or Podman is available. On macOS, the runner defaults to `linux/amd64` for compatibility with the upstream ShellSpec image.
+
 To run Go linting tasks (requires [golanglint-ci](https://golangci-lint.run/usage/install/) to be installed):
 
 ```bash

hack/unit-test-bash.sh

@@ -0,0 +1,179 @@
+#!/bin/bash
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+readonly repo_root
+readonly shellspec_image="${BASH_TEST_IMAGE:-docker.io/shellspec/shellspec-debian:0.28.1}"
+readonly default_report_base="${TMPDIR:-/tmp}/aro-bash-test-report"
+
+# Reject report paths that could make host-side cleanup delete the wrong tree.
+validate_report_dir() {
+    local report_dir="$1"
+    local tmpdir="${TMPDIR:-/tmp}"
+    local path_segment
+
+    tmpdir="${tmpdir%/}"
+
+    if [[ -z "${report_dir}" || "${report_dir}" != /* || -z "${report_dir//\//}" ]]; then
+        echo "error: refusing unsafe report directory '${report_dir}'" >&2
+        return 1
+    fi
+
+    # Only allow report directories under the temp dir with the expected prefix.
+    case "${report_dir}" in
+        "${tmpdir}"/aro-bash-test-report*)
+            ;;
+        *)
+            echo "error: refusing unsafe report directory '${report_dir}'" >&2
+            return 1
+            ;;
+    esac
+
+    IFS='/' read -r -a path_segments <<< "${report_dir#/}"
+    for path_segment in "${path_segments[@]}"; do
+        if [[ "${path_segment}" == "." || "${path_segment}" == ".." ]]; then
+            echo "error: refusing unsafe report directory '${report_dir}'" >&2
+            return 1
+        fi
+    done
+}
+
+# Recreate the host report directory that ShellSpec writes into from the container.
+prepare_report_dir() {
+    local report_dir
+    report_dir="${BASH_TEST_REPORT_DIR:-${default_report_base}}"
+    validate_report_dir "${report_dir}" || return 1
+    rm -rf -- "${report_dir}"
+    mkdir -p -- "${report_dir}"
+    printf '%s\n' "${report_dir}"
+}
+
+# Treat an engine as usable only when its info command succeeds.
+container_engine_usable() {
+    "$1" info > /dev/null 2>&1
+}
+
+# Detect podman-docker wrappers so we can apply Podman-specific run flags.
+docker_is_podman() {
+    if ! command -v docker > /dev/null 2>&1; then
+        return 1
+    fi
+
+    local docker_path podman_path docker_version docker_info
+
+    # Same inode: `docker` is a symlink/hardlink to the podman binary.
+    docker_path="$(type -P docker || true)"
+    podman_path="$(type -P podman || true)"
+    if [[ -n "${docker_path}" && -n "${podman_path}" && "${docker_path}" -ef "${podman_path}" ]]; then
+        return 0
+    fi
+
+    # Authoritative signals that do not depend on the podman-docker wrapper's
+    # "Emulate Docker CLI using podman" stderr message, which is suppressed when
+    # /etc/containers/nodocker exists.
+    docker_version="$(docker version --format '{{.Client.Name}}' 2> /dev/null || true)"
+    if [[ "${docker_version}" == *[Pp]odman* ]]; then
+        return 0
+    fi
+
+    docker_version="$(docker --version 2> /dev/null || true)"
+    if [[ "${docker_version}" == *[Pp]odman* ]]; then
+        return 0
+    fi
+
+    # Fallback for older podman-docker wrappers without /etc/containers/nodocker.
+    docker_info="$(docker info 2>&1 || true)"
+    [[ "${docker_info}" == *"Emulate Docker CLI using podman"* ]]
+}
+
+# Prefer docker when available, unless it is really Podman underneath.
+detect_container_engine() {
+    if command -v docker > /dev/null 2>&1 && container_engine_usable docker; then
+        if docker_is_podman; then
+            echo podman
+        else
+            echo docker
+        fi
+        return 0
+    fi
+
+    if command -v podman > /dev/null 2>&1 && container_engine_usable podman; then
+        echo podman
+        return 0
+    fi
+
+    echo "error: a working docker or podman daemon is required to run bash tests" >&2
+    return 1
+}
+
+# Keep this destructive suite serial unless the caller explicitly opts in.
+detect_jobs() {
+    if [[ -n "${BASH_TEST_JOBS:-}" ]]; then
+        echo "${BASH_TEST_JOBS}"
+        return 0
+    fi
+    echo 1
+}
+
+# Force linux/amd64 only on macOS hosts that default to Apple Silicon containers.
+detect_platform() {
+    if [[ -n "${BASH_TEST_PLATFORM:-}" ]]; then
+        echo "${BASH_TEST_PLATFORM}"
+        return 0
+    fi
+
+    if [[ "$(uname -s)" == "Darwin" ]]; then
+        echo "linux/amd64"
+    fi
+}
+
+# Assemble engine-specific mounts and run ShellSpec inside the container.
+main() {
+    local engine jobs mount_arg report_dir report_mount
+    engine="$(detect_container_engine)"
+    jobs="$(detect_jobs)"
+    report_dir="$(prepare_report_dir)" || return 1
+
+    local -a engine_args=()
+    if [[ "${engine}" == "podman" ]]; then
+        mount_arg="${repo_root}:/work:Z"
+        report_mount="${report_dir}:/work/.bash-test-report:Z"
+        engine_args+=("--userns=keep-id:uid=0,gid=0")
+    else
+        mount_arg="${repo_root}:/work"
+        report_mount="${report_dir}:/work/.bash-test-report"
+    fi
+
+    local -a platform_args=()
+    local platform
+    platform="$(detect_platform)"
+    if [[ -n "${platform}" ]]; then
+        platform_args=(--platform "${platform}")
+    fi
+
+    local junit_enabled="${BASH_TEST_JUNIT:-${CI:-0}}"
+    local -a report_args=()
+    if [[ "${junit_enabled}" == "true" || "${junit_enabled}" == "1" ]]; then
+        report_args=(--output junit --reportdir /work/.bash-test-report)
+        echo "ShellSpec JUnit report directory: ${report_dir}"
+    fi
+
+    echo "Running ShellSpec with ${engine} using image ${shellspec_image}"
+
+    "${engine}" run --rm \
+        "${platform_args[@]}" \
+        "${engine_args[@]}" \
+        -v "${mount_arg}" \
+        -v "${report_mount}" \
+        -w /work \
+        "${shellspec_image}" \
+        --jobs "${jobs}" \
+        --format documentation \
+        "${report_args[@]}" \
+        "$@"
+}
+
+main "$@"

pkg/operator/controllers/dnsmasq/scripts/99-dnsmasq-restart.gotmpl

@@ -1,5 +1,5 @@
 {{ define "99-dnsmasq-restart" }}
-#!/bin/sh
+#!/bin/bash
 # This is a NetworkManager dispatcher script to restart dnsmasq
 # in the event of a network interface change (e. g. host servicing event https://learn.microsoft.com/en-us/azure/developer/intro/hosting-apps-on-azure)
 # this will restart dnsmasq, reapplying our /etc/resolv.conf file and overwriting any modifications made by NetworkManager

test/bash/spec/backupandfixetcd_spec.sh

@@ -0,0 +1,84 @@
+Describe 'backupandfixetcd.sh'
+Include ./test/bash/spec/support/helpers.sh
+script_path="${REPO_ROOT}/pkg/frontend/scripts/backupandfixetcd.sh"
+
+run_backup_mode() {
+    ensure_test_workspace
+    mkdir -p /etc/kubernetes/manifests /var/lib/etcd
+    printf 'manifest-data' > /etc/kubernetes/manifests/etcd-pod.yaml
+    printf 'etcd-data' > /var/lib/etcd/member
+
+    BACKUP=1 FIX_PEERS='' bash "${script_path}"
+
+    backup_dir="$(echo /var/lib/etcd-backup-*)"
+    printf '%s\n' "${backup_dir}"
+    printf '%s\n' "$(cat "${backup_dir}/etcd-pod.yaml")"
+    printf '%s\n' "$(cat "${backup_dir}/etcd/member")"
+    if backup_paths_moved; then
+        printf '%s\n' 'moved=yes'
+    else
+        printf '%s\n' 'moved=no'
+    fi
+}
+
+backup_paths_moved() {
+    [ ! -e /var/lib/etcd ] && [ ! -e /etc/kubernetes/manifests/etcd-pod.yaml ]
+}
+
+run_fix_peers_mode() {
+    ensure_test_workspace
+
+    append_mock_command oc '
+      printf "oc %s\n" "$*" >> "${CALL_LOG}"
+      if [[ "$*" == *"member list"* ]]; then
+        printf "%s\n" "{\"members\":[{\"name\":\"master-0\",\"ID\":\"member-123\"}]}"
+      fi
+    '
+    append_mock_command jq 'printf "%s\n" "member-123"'
+    cp "${MOCK_BIN}/oc" /host/usr/bin/oc
+    cp "${MOCK_BIN}/jq" /host/usr/bin/jq
+
+    FIX_PEERS=1 \
+        BACKUP='' \
+        PEER_PODS="etcd-0 etcd-1" \
+        DEGRADED_NODE="master-0" \
+        bash "${script_path}"
+
+    cat "${CALL_LOG}"
+}
+
+verify_fix_peers_host_mocks_are_cleaned() {
+    ensure_test_workspace
+
+    append_mock_command oc ':'
+    append_mock_command jq ':'
+    cp "${MOCK_BIN}/oc" /host/usr/bin/oc
+    cp "${MOCK_BIN}/jq" /host/usr/bin/jq
+
+    reset_absolute_test_state
+
+    [ ! -e /host/usr/bin/oc ] && [ ! -e /host/usr/bin/jq ]
+}
+
+It 'backs up etcd manifests and data into a timestamped directory'
+When call run_backup_mode
+The status should be success
+The output should include '/var/lib/etcd-backup-'
+The output should include 'manifest-data'
+The output should include 'etcd-data'
+The output should include 'moved=yes'
+End
+
+It 'removes degraded peer members when FIX_PEERS is set'
+When call run_fix_peers_mode
+The status should be success
+The output should include 'Starting peer etcd member removal'
+The output should include 'oc rsh -n openshift-etcd -c etcdctl pod/etcd-0 etcdctl member remove member-123'
+The output should include 'oc rsh -n openshift-etcd -c etcdctl pod/etcd-1 etcdctl member remove member-123'
+End
+
+It 'cleans fix-peers host mocks during helper reset'
+When run verify_fix_peers_host_mocks_are_cleaned
+The status should be success
+End
+End