harden even more

[paudley]

This pull request adds a smoke test for the Docker image in the CI workflow and improves the PostgreSQL configuration rendering script to better handle initial and subsequent renders. The main changes focus on increasing reliability and correctness during container initialization and configuration updates.

CI Workflow Improvements:

• Added a smoke test step to the CI workflow (.github/workflows/ci.yml) that starts a PostgreSQL Docker container, checks its readiness, and runs a simple query to verify the image works as expected. The container is always cleaned up after the test.

PostgreSQL Initialization Script Enhancements:

• Introduced a first_render flag in postgres/initdb/00-render-config.sh to distinguish between initial configuration and refresh scenarios, improving the logic for rendering configs.
• Updated the config rendering logic to reload PostgreSQL on initial render and restart it only when refreshing configuration, reducing unnecessary restarts and improving reliability.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull Request Overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The wait_for_database function body is incorrectly indented with tabs. The function body should have consistent indentation aligned with typical shell function conventions. Remove the extra indentation from lines 23-39.

Suggested change

	attempts=${PGHERO_DB_WAIT_ATTEMPTS:-90}
	delay=2
	attempt=1
	while [ "${attempt}" -le "${attempts}" ]; do
	if DATABASE_URL="${DATABASE_URL}" bundle exec ruby -e "require 'pg'; conn = PG.connect(ENV['DATABASE_URL']); conn.exec('SELECT 1'); conn.close" >/dev/null 2>&1; then
	return 0
	fi
	echo "[pghero] waiting for PostgreSQL at ${POSTGRES_HOST}:${POSTGRES_PORT} (attempt ${attempt})" >&2
	sleep "${delay}"
	if [ $((attempt % 10)) -eq 0 ] && [ "${delay}" -lt 10 ]; then
	delay=$((delay + 1))
	fi
	attempt=$((attempt + 1))
	done
	echo "[pghero] timed out waiting for PostgreSQL" >&2
	exit 1
	}
	attempts=${PGHERO_DB_WAIT_ATTEMPTS:-90}
	delay=2
	attempt=1
	while [ "${attempt}" -le "${attempts}" ]; do
	if DATABASE_URL="${DATABASE_URL}" bundle exec ruby -e "require 'pg'; conn = PG.connect(ENV['DATABASE_URL']); conn.exec('SELECT 1'); conn.close" >/dev/null 2>&1; then
	return 0
	fi
	echo "[pghero] waiting for PostgreSQL at ${POSTGRES_HOST}:${POSTGRES_PORT} (attempt ${attempt})" >&2
	sleep "${delay}"
	if [ $((attempt % 10)) -eq 0 ] && [ "${delay}" -lt 10 ]; then
	delay=$((delay + 1))
	fi
	attempt=$((attempt + 1))
	done
	echo "[pghero] timed out waiting for PostgreSQL" >&2
	exit 1
	}

[Copilot]

Pull Request Overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The trap cleanup EXIT will execute when this step finishes, immediately tearing down the stack after it's brought up. This means the next step "Run marker tests" will try to interact with a stack that has already been brought down, causing tests to fail.

Remove the cleanup trap and the set -euo pipefail line from this preflight step, or remove the preflight step entirely if the test suite handles stack bring-up itself.

Suggested change

	set -euo pipefail
	cleanup() { ./scripts/manage.sh down >/dev/null 2>&1 || true; }
	trap cleanup EXIT

[Copilot]

Exposing credentials in plaintext environment variables is a security risk. While this is a test environment, it's better practice to avoid plaintext passwords in CI configuration files.

Consider using GitHub secrets or at minimum, use the PGPASSFILE mechanism instead of PGPASSWORD.

[Copilot]

The unquoted variable in the for loop can cause word splitting issues if container names contain spaces or special characters. While container names typically don't contain spaces, it's a best practice to properly quote variables in shell scripts.

Change line 82 to use a while read loop for safer iteration:

docker ps -a --format '{{.Names}}' 2>/dev/null | grep -E 'core_data|postgres' | while IFS= read -r name; do
	collect_container_artifacts "${name}"
done

Suggested change

	containers=$(docker ps -a --format '{{.Names}}' 2>/dev/null | grep -E 'core_data|postgres' || true)
	for name in ${containers}; do
	docker ps -a --format '{{.Names}}' 2>/dev/null | grep -E 'core_data|postgres' | while IFS= read -r name; do

[Copilot]

The trap cleanup EXIT will execute when this step finishes, immediately tearing down the stack after it's brought up. This means the next step "Run core_data smoke workflow" will try to interact with a stack that has already been brought down, causing the test to fail.

Remove the cleanup trap and the set -euo pipefail line from this preflight step, or remove the preflight step entirely if the test suite handles stack bring-up itself.

Suggested change

	set -euo pipefail
	cleanup() { ./scripts/manage.sh down >/dev/null 2>&1 || true; }
	trap cleanup EXIT