Various AI inspired code changes (And MOAR TESTS)

build.sh

@@ -20,8 +20,8 @@ usage() {
 
 # Set default values
 TAG="pihole:local"
-DOCKER_BUILD_CMD="docker buildx build src/. --tag ${TAG} --load --no-cache"
 FTL_FLAG=false
+USE_CACHE=false
 CORE_FORK="pi-hole"
 WEB_FORK="pi-hole"
 PADD_FORK="pi-hole"
@@ -56,6 +56,9 @@ check_branch_exists() {
     fi
 }
 
+# Collect extra --build-arg values from flags
+BUILD_ARGS=()
+
 # Parse command line arguments
 while [[ $# -gt 0 ]]; do
     key="$1"
@@ -71,7 +74,7 @@ while [[ $# -gt 0 ]]; do
             usage
         fi
         FTL_FLAG=true
-        DOCKER_BUILD_CMD+=" --build-arg FTL_SOURCE=local"
+        BUILD_ARGS+=(--build-arg "FTL_SOURCE=local")
         shift
         ;;
     -f | --ftlbranch)
@@ -82,58 +85,56 @@ while [[ $# -gt 0 ]]; do
         FTL_FLAG=true
         FTL_BRANCH="$2"
         check_branch_exists "ftl" "$FTL_BRANCH"
-        DOCKER_BUILD_CMD+=" --build-arg FTL_BRANCH=$FTL_BRANCH"
+        BUILD_ARGS+=(--build-arg "FTL_BRANCH=$FTL_BRANCH")
         shift
         shift
         ;;
     -c | --corebranch)
         CORE_BRANCH="$2"
         check_branch_exists "pi-hole" "$CORE_BRANCH" "$CORE_FORK"
-        DOCKER_BUILD_CMD+=" --build-arg CORE_BRANCH=$CORE_BRANCH"
+        BUILD_ARGS+=(--build-arg "CORE_BRANCH=$CORE_BRANCH")
         shift
         shift
         ;;
     -w | --webbranch)
         WEB_BRANCH="$2"
         check_branch_exists "web" "$WEB_BRANCH" "$WEB_FORK"
-        DOCKER_BUILD_CMD+=" --build-arg WEB_BRANCH=$WEB_BRANCH"
+        BUILD_ARGS+=(--build-arg "WEB_BRANCH=$WEB_BRANCH")
         shift
         shift
         ;;
     -p | --paddbranch)
         PADD_BRANCH="$2"
         check_branch_exists "padd" "$PADD_BRANCH"
-        DOCKER_BUILD_CMD+=" --build-arg PADD_BRANCH=$PADD_BRANCH"
+        BUILD_ARGS+=(--build-arg "PADD_BRANCH=$PADD_BRANCH")
         shift
         shift
         ;;
     -cf | --corefork)
         CORE_FORK="$2"
-        DOCKER_BUILD_CMD+=" --build-arg CORE_FORK=$CORE_FORK"
+        BUILD_ARGS+=(--build-arg "CORE_FORK=$CORE_FORK")
         shift
         shift
         ;;
     -wf | --webfork)
         WEB_FORK="$2"
-        DOCKER_BUILD_CMD+=" --build-arg WEB_FORK=$WEB_FORK"
+        BUILD_ARGS+=(--build-arg "WEB_FORK=$WEB_FORK")
         shift
         shift
         ;;
     -pf | --paddfork)
         PADD_FORK="$2"
-        DOCKER_BUILD_CMD+=" --build-arg PADD_FORK=$PADD_FORK"
+        BUILD_ARGS+=(--build-arg "PADD_FORK=$PADD_FORK")
         shift
         shift
         ;;
     -t | --tag)
-        CUSTOM_TAG="$2"
-        DOCKER_BUILD_CMD=${DOCKER_BUILD_CMD/$TAG/$CUSTOM_TAG}
-        TAG=$CUSTOM_TAG
+        TAG="$2"
         shift
         shift
         ;;
     use_cache)
-        DOCKER_BUILD_CMD=${DOCKER_BUILD_CMD/--no-cache/}
+        USE_CACHE=true
         shift
         ;;
     *)
@@ -143,9 +144,14 @@ while [[ $# -gt 0 ]]; do
     esac
 done
 
+# Build the command as an array to avoid eval and shell injection
+DOCKER_BUILD_CMD=(docker buildx build src/. --tag "${TAG}" --load)
+[ "$USE_CACHE" = false ] && DOCKER_BUILD_CMD+=(--no-cache)
+DOCKER_BUILD_CMD+=("${BUILD_ARGS[@]}")
+
 # Execute the docker build command
-echo "Executing command: $DOCKER_BUILD_CMD"
-eval "${DOCKER_BUILD_CMD}"
+echo "Executing command: ${DOCKER_BUILD_CMD[*]}"
+"${DOCKER_BUILD_CMD[@]}"
 
 # Check exit code of previous command
 if [ $? -ne 0 ]; then

src/bash_functions.sh

@@ -85,7 +85,9 @@ start_cron() {
     sed -i "s/59 1 /$((1 + RANDOM % 58)) $((3 + RANDOM % 2))/" /crontab.txt
     # Randomize update checker time
     sed -i "s/59 17/$((1 + RANDOM % 58)) $((12 + RANDOM % 8))/" /crontab.txt
-    /usr/bin/crontab /crontab.txt
+    if ! /usr/bin/crontab /crontab.txt; then
+        echo "  [!] Failed to install crontab - scheduled tasks (gravity, update checker) will not run"
+    fi
 
     /usr/sbin/crond
     echo ""

test/helpers.sh

@@ -0,0 +1,27 @@
+#!/usr/bin/env bash
+# Shared container helper functions for BATS test files
+
+start_container() {
+    local platform_args=()
+    [ -n "${CIPLATFORM:-}" ] && platform_args=(--platform "${CIPLATFORM}")
+    docker run -d -t "${platform_args[@]}" -e TZ="Europe/London" "$@" pihole:test
+}
+
+wait_for_log() {
+    local container="$1"
+    local pattern="$2"
+    local timeout=60
+    local elapsed=0
+    printf "Waiting for '%s' in %.30s... " "${pattern}" "${container}"
+    until docker logs "${container}" 2>&1 | grep -q "${pattern}"; do
+        sleep 1
+        elapsed=$(( elapsed + 1 ))
+        if (( elapsed >= timeout )); then
+            echo "TIMEOUT"
+            echo "--- Container logs ---"
+            docker logs "${container}"
+            return 1
+        fi
+    done
+    echo "ready (${elapsed}s)"
+}

test/run.sh

@@ -34,68 +34,20 @@ if [ -z "${BATS:-}" ]; then
     BATS=libs/bats/bin/bats
 fi
 
-# ---- Start containers -------------------------------------------------------
-
-# Cleanup all test containers on exit (success or failure)
-CONTAINERS=()
-cleanup() {
-    if [ ${#CONTAINERS[@]} -gt 0 ]; then
-        docker rm -f "${CONTAINERS[@]}" > /dev/null 2>&1 || true
-    fi
-}
-trap cleanup EXIT
-
-start_container() {
-    local id
-    id=$(docker run -d -t "${PLATFORM_ARGS[@]}" -e TZ="Europe/London" "$@" pihole:test)
-    CONTAINERS+=("$id")
-    echo "$id"
-}
-
-CONTAINER_DEFAULT=$(start_container)
-CONTAINER_CUSTOM=$(start_container \
-    -e PIHOLE_UID=456 \
-    -e PIHOLE_GID=456 \
-    -e FTLCONF_webserver_api_password=1234567890 \
-    -e FTLCONF_webserver_port=8080 \
-    -e FTLCONF_dns_upstreams="8.8.8.8;1.1.1.1")
-
-export CONTAINER_DEFAULT CONTAINER_CUSTOM CIPLATFORM
-
-# ---- Wait for containers to be ready ----------------------------------------
-
-wait_for_ftl() {
-    local container="$1"
-    local timeout=60
-    local elapsed=0
-    printf "Waiting for FTL in %.12s... " "${container}"
-    until docker logs "${container}" 2>&1 | grep -q "########## FTL started"; do
-        sleep 1
-        elapsed=$(( elapsed + 1 ))
-        if (( elapsed >= timeout )); then
-            echo "TIMEOUT"
-            echo "--- Container logs ---"
-            docker logs "${container}"
-            return 1
-        fi
-    done
-    echo "ready (${elapsed}s)"
-}
-
-for container in "$CONTAINER_DEFAULT" "$CONTAINER_CUSTOM"; do
-    wait_for_ftl "$container"
-done
-
 # ---- Run BATS ---------------------------------------------------------------
 
 echo "Running tests with BATS"
 
+export CIPLATFORM
+
 TEST_FILES=(
-    test_suite.bats
+    test_default.bats
+    test_env_vars.bats
+    test_secrets.bats
 )
 
 # Configure BATS output and parallelization
-BATS_FLAGS=("--print-output-on-failure");
+BATS_FLAGS=();
 
 # Use pretty output when stdout is a terminal; TAP format for CI
 if [[ -t 1 ]]; then

test/test_default.bats

@@ -0,0 +1,120 @@
+#!/usr/bin/env bats
+
+load 'libs/bats-support/load'
+load 'libs/bats-assert/load'
+load 'helpers.sh'
+
+setup_file() {
+    CONTAINER=$(start_container)
+    wait_for_log "$CONTAINER" "########## FTL started"
+    export CONTAINER
+    # Force tests in this file to run sequentially since the shutdown test will destroy the container that other tests depend on
+    export BATS_NO_PARALLELIZE_WITHIN_FILE=true
+}
+
+teardown_file() {
+    docker rm -f "$CONTAINER" > /dev/null 2>&1 || true
+}
+
+# ---- FTL binary -------------------------------------------------------------
+
+@test "FTL reports version" {
+    run docker exec "$CONTAINER" pihole-FTL -vv
+    assert_success
+    assert_output --partial "Version:"
+}
+
+@test "FTL reports correct architecture" {
+    [ -n "${CIPLATFORM:-}" ] || skip "CIPLATFORM not set, running locally"
+    run docker exec "$CONTAINER" pihole-FTL -vv
+    assert_success
+    assert_output --partial "Architecture:"
+    assert_output --partial "$CIPLATFORM"
+}
+
+# ---- Container configuration ------------------------------------------------
+
+@test "Cron file is valid" {
+    run docker exec "$CONTAINER" /usr/bin/crontab /crontab.txt
+    assert_success
+}
+
+# ---- Web password setup -----------------------------------------------------
+
+@test "Random password is assigned on fresh start" {
+    run docker logs "$CONTAINER"
+    assert_success
+    assert_output --partial "assigning random password:"
+}
+
+# ---- Container services -----------------------------------------------------
+
+@test "crond is running" {
+    run docker exec "$CONTAINER" pgrep crond
+    assert_success
+}
+
+@test "Logrotate config is installed" {
+    run docker exec "$CONTAINER" test -f /etc/pihole/logrotate
+    assert_success
+}
+
+# ---- Default configuration --------------------------------------------------
+
+@test "Default DNS upstreams are applied when none are configured" {
+    run docker exec "$CONTAINER" pihole-FTL --config -q dns.upstreams
+    assert_success
+    assert_output --partial "8.8.8.8"
+    assert_output --partial "8.8.4.4"
+}
+
+# ---- Web interface ----------------------------------------------------------
+
+@test "Web interface is accessible" {
+    run docker exec "$CONTAINER" curl -sf /dev/null http://localhost/admin/
+    assert_success
+}
+
+# ---- Docker image -----------------------------------------------------------
+
+@test "/pihole.docker.tag is present" {
+    run docker exec "$CONTAINER" test -f /pihole.docker.tag
+    assert_success
+}
+
+@test "macvendor.db is present" {
+    run docker exec "$CONTAINER" test -f /macvendor.db
+    assert_success
+}
+
+@test "macvendor.db path is configured in FTL" {
+    run docker exec "$CONTAINER" pihole-FTL --config -q files.macvendor
+    assert_success
+    assert_output "/macvendor.db"
+}
+
+# ---- Runtime ----------------------------------------------------------------
+
+@test "FTL is running as the pihole user" {
+    run docker exec "$CONTAINER" pgrep -u pihole pihole-FTL
+    assert_success
+}
+
+@test "Capabilities are applied to pihole-FTL" {
+    run docker exec "$CONTAINER" getcap /usr/bin/pihole-FTL
+    assert_success
+    assert_output --partial "cap_net_raw"
+}
+
+# ---- FTL shutdown (DO THIS LAST!)---------------------------------------------
+
+@test "FTL starts up and shuts down cleanly" {
+    # Stop gracefully (SIGTERM), then capture logs before teardown_file removes it
+    run docker stop "$CONTAINER"
+    local logs
+    logs=$(docker logs "$CONTAINER" 2>&1)
+
+    run echo "$logs"
+    assert_output --partial "INFO: ########## FTL terminated after"
+    assert_output --partial "(code 0)"
+}