diff --git a/.github/workflows/pr_test_latest.yaml b/.github/workflows/pr_test_latest.yaml new file mode 100644 index 0000000..d125197 --- /dev/null +++ b/.github/workflows/pr_test_latest.yaml @@ -0,0 +1,153 @@ +name: PR Test Latest Images + +# This workflow runs on pull requests and tests the latest images +# from the internal repository to ensure they still work with any changes. + +on: + pull_request: + branches: + - main + +permissions: + contents: read + packages: read + +env: + IMAGE_REGISTRY: ghcr.io/pgedge + PACKAGE_REPOSITORY: pgedge-postgres-internal + +jobs: + # Get latest tags and generate test matrix + setup: + # Skip for forked PRs since they won't have access to internal packages + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + outputs: + matrix: ${{ steps.generate.outputs.matrix }} + tags: ${{ steps.get-tags.outputs.tags }} + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.x' + + - name: Get latest tags + id: get-tags + run: | + set -e + TAGS=$(make latest-tags) + + # Validate that tags were retrieved successfully + if [ -z "$TAGS" ]; then + echo "Error: Failed to retrieve latest tags. make latest-tags returned empty output." + exit 1 + fi + + echo "tags=$TAGS" >> $GITHUB_OUTPUT + echo "Latest tags: $TAGS" + + - name: Generate test matrix + id: generate + run: | + # Parse tags from make output + TAGS="${{ steps.get-tags.outputs.tags }}" + IFS=',' read -ra TAG_ARRAY <<< "$TAGS" + + # Test on both architectures + ARCHS=("x86" "arm") + + # Build matrix JSON + matrix_items="" + for tag in "${TAG_ARRAY[@]}"; do + tag=$(echo "$tag" | xargs) # trim whitespace + + # Determine flavor from tag + if [[ "$tag" == *"-minimal"* ]]; then + flavor="minimal" + elif [[ "$tag" == *"-standard"* ]]; then + flavor="standard" + else + # Default to standard if not specified + flavor="standard" + fi + + for arch in "${ARCHS[@]}"; do + arch=$(echo "$arch" | xargs) # trim whitespace + + # Map user-friendly arch names to runner names + if [[ "$arch" == "arm" ]] || [[ "$arch" == "arm64" ]]; then + runner="ubuntu-24.04-arm" + arch_display="arm" + elif [[ "$arch" == "x86" ]] || [[ "$arch" == "amd64" ]]; then + runner="ubuntu-latest" + arch_display="x86" + else + echo "Error: Unknown architecture '$arch'. Use 'x86' or 'arm'" + exit 1 + fi + + if [[ -n "$matrix_items" ]]; then + matrix_items+="," + fi + matrix_items+="{\"tag\":\"$tag\",\"arch\":\"$arch_display\",\"flavor\":\"$flavor\",\"runner\":\"$runner\"}" + done + done + + echo "matrix={\"include\":[$matrix_items]}" >> $GITHUB_OUTPUT + echo "Generated matrix: {\"include\":[$matrix_items]}" + + test: + # Skip for forked PRs since they won't have access to internal packages + if: github.event.pull_request.head.repo.full_name == github.repository + needs: setup + runs-on: ${{ matrix.runner }} + strategy: + fail-fast: false + matrix: ${{ fromJson(needs.setup.outputs.matrix) }} + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.23' + cache-dependency-path: tests/go.sum + + - name: Pull image + run: | + IMAGE="${{ env.IMAGE_REGISTRY }}/${{ env.PACKAGE_REPOSITORY }}:${{ matrix.tag }}" + echo "Pulling image: $IMAGE" + docker pull "$IMAGE" + + - name: Run tests + run: | + IMAGE="${{ env.IMAGE_REGISTRY }}/${{ env.PACKAGE_REPOSITORY }}:${{ matrix.tag }}" + make test-image IMAGE="$IMAGE" FLAVOR="${{ matrix.flavor }}" + + results: + # Skip for forked PRs since they won't have access to internal packages + # Also use always() to ensure results are shown even if test job fails + if: github.event.pull_request.head.repo.full_name == github.repository && always() + needs: [setup, test] + runs-on: ubuntu-latest + steps: + - name: Output + run: | + echo "## Test Results" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "| Input | Value |" >> $GITHUB_STEP_SUMMARY + echo "|-------|-------|" >> $GITHUB_STEP_SUMMARY + echo "| Package Repository | ${{ env.PACKAGE_REPOSITORY }} |" >> $GITHUB_STEP_SUMMARY + echo "| Tags | ${{ needs.setup.outputs.tags }} |" >> $GITHUB_STEP_SUMMARY + echo "| Architectures | x86,arm |" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + + if [[ "${{ needs.test.result }}" == "success" ]]; then + echo "✅ **All tests passed!**" >> $GITHUB_STEP_SUMMARY + else + echo "❌ **Some tests failed.** Check the job logs for details." >> $GITHUB_STEP_SUMMARY + fi diff --git a/.github/workflows/test_images.yaml b/.github/workflows/test_images.yaml index 0d3db5b..d3385fc 100644 --- a/.github/workflows/test_images.yaml +++ b/.github/workflows/test_images.yaml @@ -16,9 +16,9 @@ on: type: string required: true architectures: - description: "Comma-separated list of architectures to test (amd64,arm64)" + description: "Comma-separated list of architectures to test (x86,arm)" type: string - default: "amd64,arm64" + default: "x86,arm" required: false permissions: @@ -62,17 +62,24 @@ jobs: for arch in "${ARCHS[@]}"; do arch=$(echo "$arch" | xargs) # trim whitespace - # Determine runner based on architecture - if [[ "$arch" == "arm64" ]]; then - runner="ubuntu-24.04-arm64" - else + # Map user-friendly arch names to runner names + # Accept "arm" or "arm64" -> use ubuntu-24.04-arm + # Accept "x86" or "amd64" -> use ubuntu-latest + if [[ "$arch" == "arm" ]] || [[ "$arch" == "arm64" ]]; then + runner="ubuntu-24.04-arm" + arch_display="arm" + elif [[ "$arch" == "x86" ]] || [[ "$arch" == "amd64" ]]; then runner="ubuntu-latest" + arch_display="x86" + else + echo "Error: Unknown architecture '$arch'. Use 'x86' or 'arm'" + exit 1 fi if [[ -n "$matrix_items" ]]; then matrix_items+="," fi - matrix_items+="{\"tag\":\"$tag\",\"arch\":\"$arch\",\"flavor\":\"$flavor\",\"runner\":\"$runner\"}" + matrix_items+="{\"tag\":\"$tag\",\"arch\":\"$arch_display\",\"flavor\":\"$flavor\",\"runner\":\"$runner\"}" done done @@ -92,16 +99,9 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: '1.24.11' + go-version: '1.23' cache-dependency-path: tests/go.sum - - name: Login to GitHub Container Registry - env: - GH_USER: ${{ github.actor }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - echo "${GH_TOKEN}" | docker login ghcr.io -u "${GH_USER}" --password-stdin - - name: Pull image run: | IMAGE="${{ env.IMAGE_REGISTRY }}/${{ inputs.package_repository }}:${{ matrix.tag }}" @@ -113,15 +113,13 @@ jobs: IMAGE="${{ env.IMAGE_REGISTRY }}/${{ inputs.package_repository }}:${{ matrix.tag }}" make test-image IMAGE="$IMAGE" FLAVOR="${{ matrix.flavor }}" - summary: + results: needs: [setup, test] runs-on: ubuntu-latest if: always() steps: - - name: Test Summary + - name: Output run: | - echo "## Test Results Summary" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY echo "| Input | Value |" >> $GITHUB_STEP_SUMMARY echo "|-------|-------|" >> $GITHUB_STEP_SUMMARY echo "| Package Repository | ${{ inputs.package_repository }} |" >> $GITHUB_STEP_SUMMARY diff --git a/Makefile b/Makefile index 504bc6d..5e4066a 100644 --- a/Makefile +++ b/Makefile @@ -68,3 +68,6 @@ ifndef FLAVOR endif cd tests && go run main.go -image $(IMAGE) -flavor $(FLAVOR) +.PHONY: latest-tags +latest-tags: + @PGEDGE_LIST_LATEST_TAGS=1 ./scripts/build_pgedge_images.py diff --git a/README.md b/README.md index 84d21ab..f4443d0 100644 --- a/README.md +++ b/README.md @@ -112,3 +112,60 @@ volumes: - Mutable tags also exist for: - The latest image for a given Postgres major.minor + spock major version, `pg-spock-` , e.g. `17.6-spock5-standard` - The latest image for a given Postgres major + spock major version, `pg-spock-`, e.g. `17-spock5-standard` + +## Testing + +This repository includes a comprehensive test suite to validate Postgres images. The tests verify: +- Default entrypoint functionality +- Patroni entrypoint (standard images only) +- PostgreSQL connectivity and version checks +- Extension availability and functionality (Spock, LOLOR, Snowflake, pgvector, PostGIS, pgaudit) +- pgBackRest installation (standard images only) + +### Running Tests Locally + +To run tests locally, you'll need: +- Go 1.24.11 or later +- Docker installed and running +- Access to pull the image you want to test + +Run the test suite using the Makefile: + +```bash +make test-image IMAGE= FLAVOR= +``` + +Example: + +```bash +make test-image IMAGE=ghcr.io/pgedge/pgedge-postgres:17-spock5-standard FLAVOR=standard +``` + +Or run directly with Go: + +```bash +cd tests && go run main.go -image -flavor +``` + +### Local Testing Limitations + +**Architecture Limitations:** When running tests locally, you can only test images that match your local machine's architecture. For example: +- On an x86_64/amd64 machine, you can only test amd64 images +- On an ARM64 machine, you can only test arm64 images + +To test images for multiple architectures, use the GitHub Actions workflow which runs tests on architecture-specific runners: +- `ubuntu-latest` runner (amd64/x86_64 architecture) +- `ubuntu-24.04-arm` runner (arm64 architecture) + +### CI/CD Testing + +The GitHub Actions workflow (`.github/workflows/test_images.yaml`) can be triggered manually to test images across multiple architectures. The workflow uses specific runner labels to target CPU architectures: +- **amd64/x86_64**: Uses `ubuntu-latest` runner +- **arm64**: Uses `ubuntu-24.04-arm` runner + +The workflow accepts: +- **Package Repository**: The container registry repository name +- **Tags**: Comma-separated list of image tags to test +- **Architectures**: Comma-separated list of architectures (`x86,arm` or `amd64,arm64`) + +The workflow will automatically test each tag on each specified architecture by mapping the architecture names to the appropriate runner labels. diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 90c7074..2089557 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -114,7 +114,41 @@ docker_init_database_dir() { fi # --pwfile refuses to handle a properly-empty file (hence the "\n"): https://github.com/docker-library/postgres/issues/1025 - eval 'initdb --username="$POSTGRES_USER" --pwfile=<(printf "%s\n" "$POSTGRES_PASSWORD") '"$POSTGRES_INITDB_ARGS"' "$@"' + # Create a temporary password file to avoid command injection via POSTGRES_INITDB_ARGS + local pwfile + pwfile="$(mktemp)" + # Ensure cleanup on exit (including errors) to prevent password file from being left on disk + trap 'rm -f "$pwfile"' EXIT + printf '%s\n' "$POSTGRES_PASSWORD" > "$pwfile" + + # Build initdb command arguments safely using an array + local initdb_args=( + --username="$POSTGRES_USER" + --pwfile="$pwfile" + ) + + # Safely parse POSTGRES_INITDB_ARGS if it exists + # Use read -a to split the string into an array without shell interpretation + # This prevents command injection while preserving argument structure + if [ -n "${POSTGRES_INITDB_ARGS:-}" ]; then + # Read the arguments into an array, splitting on whitespace + # Note: This does not handle quoted arguments with spaces. + # For complex cases, pass arguments as function parameters instead. + local args_array + IFS=' ' read -r -a args_array <<< "$POSTGRES_INITDB_ARGS" + initdb_args+=("${args_array[@]}") + fi + + # Add any function arguments (these are already safely parsed by the shell) + initdb_args+=("$@") + + # Execute initdb with the safely constructed arguments + initdb "${initdb_args[@]}" + + # Clean up temporary password file (trap will also handle this on exit, but explicit cleanup is good) + rm -f "$pwfile" + # Remove the trap since we've cleaned up manually + trap - EXIT # unset/cleanup "nss_wrapper" bits if [[ "${LD_PRELOAD:-}" == */libnss_wrapper.so ]]; then diff --git a/scripts/build_pgedge_images.py b/scripts/build_pgedge_images.py index 0e30445..24d188d 100755 --- a/scripts/build_pgedge_images.py +++ b/scripts/build_pgedge_images.py @@ -17,6 +17,7 @@ class Config: only_postgres_version: str only_spock_version: str only_arch: str + list_latest_tags: bool @staticmethod def from_env() -> "Config": @@ -29,6 +30,7 @@ def from_env() -> "Config": only_postgres_version=os.getenv("PGEDGE_IMAGE_ONLY_POSTGRES_VERSION", ""), only_spock_version=os.getenv("PGEDGE_IMAGE_ONLY_SPOCK_VERSION", ""), only_arch=os.getenv("PGEDGE_IMAGE_ONLY_ARCH", ""), + list_latest_tags=(os.getenv("PGEDGE_LIST_LATEST_TAGS", "0") == "1"), ) @@ -324,6 +326,13 @@ def main(): config = Config.from_env() + # If list_latest_tags is enabled, output tags and exit + if config.list_latest_tags: + tags = get_latest_tags() + # Output as comma-separated list + print(",".join(tags)) + return + if config.dry_run: logging.info("dry run enabled. build and publish actions will be skipped.") @@ -397,5 +406,19 @@ def main(): logging.info(f"{tag} is already up-to-date") +def get_latest_tags() -> list[str]: + """ + Returns a list of the latest immutable tags (with epoch and pg minor version) + for all images that are marked as latest for their Postgres major version. + Returns tags in the format: {postgres_version}-spock{spock_version}-{flavor}-{epoch} + """ + latest_tags = [] + for image in all_images: + if image.is_latest_for_pg_major: + # Get the immutable build tag with epoch and full postgres version + latest_tags.append(str(image.build_tag)) + return latest_tags + + if __name__ == "__main__": main() diff --git a/tests/go.mod b/tests/go.mod index 5ddcb13..e8d1d7e 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -1,8 +1,6 @@ module github.com/pgedge/postgres-images/tests -go 1.24.0 - -toolchain go1.24.11 +go 1.24.11 require github.com/docker/docker v28.0.0+incompatible diff --git a/tests/main.go b/tests/main.go index 0aa91bb..90ecb76 100644 --- a/tests/main.go +++ b/tests/main.go @@ -15,6 +15,13 @@ import ( "github.com/docker/docker/pkg/stdcopy" ) +const ( + // postgresStabilizationPeriod is the duration to wait after PostgreSQL + // passes readiness checks to allow background workers, extensions, and + // internal caches to fully initialize before running tests. + postgresStabilizationPeriod = 2 * time.Second +) + // Test represents a single test case type Test struct { Name string @@ -40,6 +47,27 @@ type DefaultEntrypointRunner struct { } func main() { + image, flavor := parseFlags() + + printHeader(image, flavor) + + cli, ctx := setupDockerClient() + defaultRunner := &DefaultEntrypointRunner{ + cli: cli, + ctx: ctx, + image: image, + } + + errorCount := runEntrypointTests(defaultRunner, flavor) + errorCount += runExtensionTests(cli, ctx, image, flavor) + + printSummary(errorCount, flavor) + if errorCount > 0 { + os.Exit(1) + } +} + +func parseFlags() (string, string) { image := flag.String("image", "", "Docker image to test (required)") flavor := flag.String("flavor", "", "Image flavor: minimal or standard (required)") flag.Parse() @@ -57,35 +85,32 @@ func main() { log.Fatalf("Invalid flavor '%s'. Must be 'minimal' or 'standard'", *flavor) } - fmt.Printf("╔══════════════════════════════════════════════════════════════════╗\n") - fmt.Printf("║ pgEdge Postgres Image Test Suite ║\n") - fmt.Printf("╠══════════════════════════════════════════════════════════════════╣\n") - fmt.Printf("║ Image: %-56s║\n", truncateString(*image, 56)) - fmt.Printf("║ Flavor: %-56s║\n", *flavor) - fmt.Printf("╚══════════════════════════════════════════════════════════════════╝\n") + return *image, *flavor +} + +func printHeader(image, flavor string) { + fmt.Println("pgEdge Postgres Image Test Suite") + fmt.Println() + fmt.Printf(" Image: %s\n", truncateString(image, 80)) + fmt.Printf(" Flavor: %s\n", flavor) fmt.Println() +} +func setupDockerClient() (*client.Client, context.Context) { ctx := context.Background() - cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) if err != nil { log.Fatalf("Error creating Docker client: %v", err) } + return cli, ctx +} +func runEntrypointTests(runner *DefaultEntrypointRunner, flavor string) int { errorCount := 0 // Phase 1: Test default entrypoint - fmt.Println("═══════════════════════════════════════════════════════════════════") - fmt.Println(" Phase 1: Default Entrypoint Test") - fmt.Println("═══════════════════════════════════════════════════════════════════") - fmt.Println() - - defaultRunner := &DefaultEntrypointRunner{ - cli: cli, - ctx: ctx, - image: *image, - } - if err := defaultRunner.TestDefaultEntrypoint(); err != nil { + printPhaseHeader("Phase 1: Default Entrypoint Test") + if err := runner.TestDefaultEntrypoint(); err != nil { errorCount++ fmt.Printf(" Default entrypoint test ❌\n") log.Printf(" Error: %v", err) @@ -95,13 +120,9 @@ func main() { fmt.Println() // Phase 2: Test Patroni entrypoint (standard only) - if *flavor == "standard" { - fmt.Println("═══════════════════════════════════════════════════════════════════") - fmt.Println(" Phase 2: Patroni Entrypoint Test") - fmt.Println("═══════════════════════════════════════════════════════════════════") - fmt.Println() - - if err := defaultRunner.TestPatroniEntrypoint(); err != nil { + if flavor == "standard" { + printPhaseHeader("Phase 2: Patroni Entrypoint Test") + if err := runner.TestPatroniEntrypoint(); err != nil { errorCount++ fmt.Printf(" Patroni entrypoint test ❌\n") log.Printf(" Error: %v", err) @@ -111,57 +132,59 @@ func main() { fmt.Println() } - // Phase 3: Extension tests (requires custom postgres config) - fmt.Println("═══════════════════════════════════════════════════════════════════") - fmt.Println(" Phase 3: Extension Tests") - fmt.Println("═══════════════════════════════════════════════════════════════════") - fmt.Println() + return errorCount +} + +func runExtensionTests(cli *client.Client, ctx context.Context, image, flavor string) int { + printPhaseHeader("Phase 3: Extension Tests") runner := &TestRunner{ cli: cli, ctx: ctx, - image: *image, - flavor: *flavor, + image: image, + flavor: flavor, } if err := runner.Start(); err != nil { - log.Fatalf("Failed to start container: %v", err) + log.Printf("Failed to start container: %v", err) + // Start() handles its own cleanup on error via defer, but call cleanupContainer + // as a safety net in case the container was created but not started + runner.cleanupContainer() + return 1 } defer runner.Cleanup() tests := buildTestSuite() - errorCount += runner.RunTests(tests) + return runner.RunTests(tests) +} +func printPhaseHeader(title string) { + fmt.Printf("%s\n", title) fmt.Println() - fmt.Printf("╔══════════════════════════════════════════════════════════════════╗\n") - fmt.Printf("║ Test Summary ║\n") - fmt.Printf("╠══════════════════════════════════════════════════════════════════╣\n") +} - // Count extension tests +func printSummary(errorCount int, flavor string) { + tests := buildTestSuite() extensionTests := 0 for _, t := range tests { - if !t.StandardOnly || *flavor == "standard" { + if !t.StandardOnly || flavor == "standard" { extensionTests++ } } - // Total tests: default entrypoint (1) + patroni entrypoint (1 if standard) + extension tests testsRun := 1 + extensionTests // default entrypoint + extensions - if *flavor == "standard" { + if flavor == "standard" { testsRun++ // patroni entrypoint } - fmt.Printf("║ Tests Executed: %-48d║\n", testsRun) - fmt.Printf("║ Errors: %-48d║\n", errorCount) + fmt.Println() + fmt.Println("Test Summary") + fmt.Printf(" Tests Executed: %d\n", testsRun) + fmt.Printf(" Errors: %d\n", errorCount) if errorCount == 0 { - fmt.Printf("║ Status: %-48s║\n", "✅ ALL TESTS PASSED") + fmt.Printf(" Status: ✅ ALL TESTS PASSED\n") } else { - fmt.Printf("║ Status: %-48s║\n", "❌ SOME TESTS FAILED") - } - fmt.Printf("╚══════════════════════════════════════════════════════════════════╝\n") - - if errorCount > 0 { - os.Exit(1) + fmt.Printf(" Status: ❌ SOME TESTS FAILED\n") } } @@ -201,37 +224,36 @@ func (r *DefaultEntrypointRunner) TestDefaultEntrypoint() error { // Wait for PostgreSQL to be ready fmt.Println(" Waiting for PostgreSQL to be ready...") - deadline := time.Now().Add(60 * time.Second) - for time.Now().Before(deadline) { - execID, err := r.cli.ContainerExecCreate(r.ctx, containerID, container.ExecOptions{ - Cmd: []string{"pg_isready", "-U", "postgres"}, - AttachStdout: true, - AttachStderr: true, - }) - if err == nil { - execResp, err := r.cli.ContainerExecAttach(r.ctx, execID.ID, container.ExecAttachOptions{}) - if err == nil { - execResp.Close() - inspectResp, _ := r.cli.ContainerExecInspect(r.ctx, execID.ID) - if inspectResp.ExitCode == 0 { - fmt.Println(" PostgreSQL started successfully with default entrypoint!") - return nil - } - } - } - time.Sleep(1 * time.Second) - } - - return fmt.Errorf("timeout waiting for PostgreSQL to be ready with default entrypoint") + return r.waitForContainerCommand( + containerID, + []string{"pg_isready", "-U", "postgres"}, + 60*time.Second, + 1*time.Second, + "PostgreSQL started successfully with default entrypoint!", + "timeout waiting for PostgreSQL to be ready with default entrypoint", + ) } // TestPatroniEntrypoint tests that Patroni can start and initialize func (r *DefaultEntrypointRunner) TestPatroniEntrypoint() error { fmt.Println(" Starting container with Patroni entrypoint...") - // Create a minimal patroni config for testing - patroniConfig := ` -scope: pgedge-test + patroniConfig := createPatroniTestConfig() + containerID, err := r.startPatroniContainer(patroniConfig) + if err != nil { + return err + } + defer r.cleanupContainer(containerID) + + if err := r.cli.ContainerStart(r.ctx, containerID, container.StartOptions{}); err != nil { + return fmt.Errorf("error starting container: %w", err) + } + + return r.waitForPatroniAPI(containerID) +} + +func createPatroniTestConfig() string { + return `scope: pgedge-test name: node1 restapi: @@ -260,6 +282,16 @@ postgresql: username: replicator password: testpassword ` +} + +func (r *DefaultEntrypointRunner) startPatroniContainer(patroniConfig string) (string, error) { + // Use a here-document to safely write the config file without shell interpretation + // This prevents command injection if patroniConfig contains special characters + // The heredoc approach avoids needing to escape quotes or other shell metacharacters + cmd := fmt.Sprintf(`cat > /tmp/patroni.yml <<'PATRONI_EOF' +%s +PATRONI_EOF +patroni /tmp/patroni.yml`, patroniConfig) resp, err := r.cli.ContainerCreate(r.ctx, &container.Config{ Image: r.image, @@ -268,47 +300,73 @@ postgresql: "PATRONI_NAME=node1", }, Cmd: []string{ - "sh", "-c", - fmt.Sprintf("echo '%s' > /tmp/patroni.yml && patroni /tmp/patroni.yml", patroniConfig), + "sh", "-c", cmd, }, }, &container.HostConfig{}, nil, nil, "") if err != nil { - return fmt.Errorf("error creating container: %w", err) + return "", fmt.Errorf("error creating container: %w", err) } - containerID := resp.ID - defer func() { - r.cli.ContainerStop(r.ctx, containerID, container.StopOptions{}) - r.cli.ContainerRemove(r.ctx, containerID, container.RemoveOptions{}) - }() + return resp.ID, nil +} - if err := r.cli.ContainerStart(r.ctx, containerID, container.StartOptions{}); err != nil { - return fmt.Errorf("error starting container: %w", err) - } +func (r *DefaultEntrypointRunner) cleanupContainer(containerID string) { + r.cli.ContainerStop(r.ctx, containerID, container.StopOptions{}) + r.cli.ContainerRemove(r.ctx, containerID, container.RemoveOptions{}) +} - // Wait for Patroni REST API to respond +func (r *DefaultEntrypointRunner) waitForPatroniAPI(containerID string) error { fmt.Println(" Waiting for Patroni to initialize...") - deadline := time.Now().Add(90 * time.Second) + return r.waitForContainerCommand( + containerID, + []string{"curl", "-sf", "http://127.0.0.1:8008/health"}, + 90*time.Second, + 2*time.Second, + "Patroni started and responding on REST API!", + "timeout waiting for Patroni to initialize", + ) +} + +// waitForContainerCommand executes a command in a container repeatedly until it succeeds or times out +func (r *DefaultEntrypointRunner) waitForContainerCommand( + containerID string, + cmd []string, + timeout time.Duration, + interval time.Duration, + successMsg string, + timeoutMsg string, +) error { + deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { execID, err := r.cli.ContainerExecCreate(r.ctx, containerID, container.ExecOptions{ - Cmd: []string{"curl", "-sf", "http://127.0.0.1:8008/health"}, + Cmd: cmd, AttachStdout: true, AttachStderr: true, }) - if err == nil { - execResp, err := r.cli.ContainerExecAttach(r.ctx, execID.ID, container.ExecAttachOptions{}) - if err == nil { - execResp.Close() - inspectResp, _ := r.cli.ContainerExecInspect(r.ctx, execID.ID) - if inspectResp.ExitCode == 0 { - fmt.Println(" Patroni started and responding on REST API!") - return nil - } - } + if err != nil { + time.Sleep(interval) + continue + } + + execResp, err := r.cli.ContainerExecAttach(r.ctx, execID.ID, container.ExecAttachOptions{}) + if err != nil { + time.Sleep(interval) + continue + } + execResp.Close() + + inspectResp, err := r.cli.ContainerExecInspect(r.ctx, execID.ID) + if err != nil { + time.Sleep(interval) + continue } - time.Sleep(2 * time.Second) + if inspectResp.ExitCode == 0 { + fmt.Printf(" %s\n", successMsg) + return nil + } + time.Sleep(interval) } - return fmt.Errorf("timeout waiting for Patroni to initialize") + return fmt.Errorf(timeoutMsg) } func (r *TestRunner) Start() error { @@ -323,6 +381,7 @@ func (r *TestRunner) Start() error { } // Build postgres command with required configuration + // Note: We pass these as postgres arguments, which the entrypoint will handle cmd := []string{ "postgres", "-c", fmt.Sprintf("shared_preload_libraries=%s", sharedLibs), @@ -331,7 +390,6 @@ func (r *TestRunner) Start() error { "-c", "max_replication_slots=10", "-c", "max_wal_senders=10", "-c", "snowflake.node=1", - "-c", "lolor.node=1", } resp, err := r.cli.ContainerCreate(r.ctx, &container.Config{ @@ -342,7 +400,6 @@ func (r *TestRunner) Start() error { "POSTGRES_DB=testdb", }, Cmd: cmd, - Tty: true, }, &container.HostConfig{}, nil, nil, "") if err != nil { return fmt.Errorf("error creating container: %w", err) @@ -350,6 +407,15 @@ func (r *TestRunner) Start() error { r.containerID = resp.ID fmt.Printf("Container created: %s\n", r.containerID[:12]) + // Track if we've successfully started to avoid double cleanup + started := false + defer func() { + // If Start() fails after container creation, clean up the container + if !started && r.containerID != "" { + r.cleanupContainer() + } + }() + if err := r.cli.ContainerStart(r.ctx, r.containerID, container.StartOptions{}); err != nil { return fmt.Errorf("error starting container: %w", err) } @@ -363,25 +429,57 @@ func (r *TestRunner) Start() error { fmt.Println("PostgreSQL is ready!") fmt.Println() + // Mark as successfully started so defer won't clean up + started = true return nil } func (r *TestRunner) waitForPostgres(timeout time.Duration) error { deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { + // First check if pg_isready succeeds exitCode, _, err := r.exec("pg_isready -U postgres") if err == nil && exitCode == 0 { - return nil + // Then verify we can actually connect and query + exitCode, _, err := r.exec("psql -U postgres -d testdb -t -A -c 'SELECT 1'") + if err == nil && exitCode == 0 { + // Give PostgreSQL a short grace period even after a successful readiness check. + // Although pg_isready and a trivial SELECT can succeed, background workers, + // extensions, and internal caches may still be initializing. This delay helps + // ensure a stable database state and reduces test flakiness in subsequent + // operations that depend on a fully-initialized instance. + time.Sleep(postgresStabilizationPeriod) + return nil + } } time.Sleep(1 * time.Second) } return fmt.Errorf("timeout waiting for PostgreSQL to be ready") } +// cleanupContainer removes a container, attempting to stop it first if it's running +func (r *TestRunner) cleanupContainer() { + if r.containerID == "" { + return + } + + // Try to stop the container first (ignore errors if it's not running) + _ = r.cli.ContainerStop(r.ctx, r.containerID, container.StopOptions{}) + + // Remove the container + if err := r.cli.ContainerRemove(r.ctx, r.containerID, container.RemoveOptions{}); err != nil { + log.Printf("Error removing container: %v", err) + } +} + func (r *TestRunner) Cleanup() { fmt.Println() fmt.Println("Cleaning up...") + if r.containerID == "" { + return + } + if err := r.cli.ContainerStop(r.ctx, r.containerID, container.StopOptions{}); err != nil { log.Printf("Error stopping container: %v", err) } else { @@ -395,9 +493,61 @@ func (r *TestRunner) Cleanup() { } } +// parseCommand safely parses a command string into command and arguments +// This prevents command injection by avoiding shell interpretation +func parseCommand(cmd string) []string { + // Simple parser that splits on spaces while respecting single and double quotes + // This is safe for the hardcoded commands we use in tests + var args []string + var current strings.Builder + inSingleQuote := false + inDoubleQuote := false + + for _, char := range cmd { + switch { + case char == '\'' && !inDoubleQuote: + inSingleQuote = !inSingleQuote + case char == '"' && !inSingleQuote: + inDoubleQuote = !inDoubleQuote + case char == ' ' && !inSingleQuote && !inDoubleQuote: + if current.Len() > 0 { + args = append(args, current.String()) + current.Reset() + } + default: + current.WriteRune(char) + } + } + + // Add any remaining content after the loop + if current.Len() > 0 { + args = append(args, current.String()) + } + + // If no arguments were parsed, return the original command as a single argument + if len(args) == 0 { + return []string{cmd} + } + + return args +} + func (r *TestRunner) exec(cmd string) (int, string, error) { + // Check if container is still running + inspect, err := r.cli.ContainerInspect(r.ctx, r.containerID) + if err != nil { + return -1, "", fmt.Errorf("error inspecting container: %w", err) + } + if !inspect.State.Running { + return -1, "", fmt.Errorf("container is not running (status: %s)", inspect.State.Status) + } + + // Parse command string safely to avoid command injection + // This prevents shell interpretation of the command string + cmdArgs := parseCommand(cmd) + execID, err := r.cli.ContainerExecCreate(r.ctx, r.containerID, container.ExecOptions{ - Cmd: []string{"sh", "-c", cmd}, + Cmd: cmdArgs, AttachStdout: true, AttachStderr: true, }) @@ -459,8 +609,15 @@ func (r *TestRunner) RunTests(tests []Test) int { } func buildTestSuite() []Test { - tests := []Test{ - // Basic PostgreSQL functionality + tests := []Test{} + tests = append(tests, getPostgreSQLTests()...) + tests = append(tests, getCommonExtensionTests()...) + tests = append(tests, getStandardOnlyTests()...) + return tests +} + +func getPostgreSQLTests() []Test { + return []Test{ { Name: "PostgreSQL accepts connections", Cmd: "psql -U postgres -d testdb -t -A -c 'SELECT 1'", @@ -487,17 +644,15 @@ func buildTestSuite() []Test { return nil }, }, + } +} - // Spock extension (minimal + standard) +func getCommonExtensionTests() []Test { + return []Test{ { - Name: "Spock extension can be created", - Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS spock; SELECT 1;\"", - ExpectedOutput: func(exitCode int, output string) error { - if exitCode != 0 { - return fmt.Errorf("unexpected exit code: %d", exitCode) - } - return nil - }, + Name: "Spock extension can be created", + Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS spock; SELECT 1;\"", + ExpectedOutput: expectSuccess, }, { Name: "Spock subscription table accessible", @@ -512,17 +667,10 @@ func buildTestSuite() []Test { return nil }, }, - - // LOLOR extension (minimal + standard) { - Name: "LOLOR extension can be created", - Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS lolor; SELECT 1;\"", - ExpectedOutput: func(exitCode int, output string) error { - if exitCode != 0 { - return fmt.Errorf("unexpected exit code: %d", exitCode) - } - return nil - }, + Name: "LOLOR extension can be created", + Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS lolor; SELECT 1;\"", + ExpectedOutput: expectSuccess, }, { Name: "LOLOR lo_create works", @@ -537,17 +685,10 @@ func buildTestSuite() []Test { return nil }, }, - - // Snowflake extension (minimal + standard) { - Name: "Snowflake extension can be created", - Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS snowflake; SELECT 1;\"", - ExpectedOutput: func(exitCode int, output string) error { - if exitCode != 0 { - return fmt.Errorf("unexpected exit code: %d", exitCode) - } - return nil - }, + Name: "Snowflake extension can be created", + Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS snowflake; SELECT 1;\"", + ExpectedOutput: expectSuccess, }, { Name: "Snowflake ID generation works", @@ -562,18 +703,16 @@ func buildTestSuite() []Test { return nil }, }, + } +} - // pgvector extension (standard only) +func getStandardOnlyTests() []Test { + return []Test{ { - Name: "pgvector extension can be created", - StandardOnly: true, - Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS vector; SELECT 1;\"", - ExpectedOutput: func(exitCode int, output string) error { - if exitCode != 0 { - return fmt.Errorf("unexpected exit code: %d", exitCode) - } - return nil - }, + Name: "pgvector extension can be created", + StandardOnly: true, + Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS vector; SELECT 1;\"", + ExpectedOutput: expectSuccess, }, { Name: "pgvector distance calculation works", @@ -583,25 +722,17 @@ func buildTestSuite() []Test { if exitCode != 0 { return fmt.Errorf("unexpected exit code: %d", exitCode) } - // Expected: 5.196152422706632 if !strings.HasPrefix(strings.TrimSpace(output), "5.196") { return fmt.Errorf("unexpected output: %s", output) } return nil }, }, - - // PostGIS extension (standard only) { - Name: "PostGIS extension can be created", - StandardOnly: true, - Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS postgis; SELECT 1;\"", - ExpectedOutput: func(exitCode int, output string) error { - if exitCode != 0 { - return fmt.Errorf("unexpected exit code: %d", exitCode) - } - return nil - }, + Name: "PostGIS extension can be created", + StandardOnly: true, + Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS postgis; SELECT 1;\"", + ExpectedOutput: expectSuccess, }, { Name: "PostGIS ST_Distance works", @@ -617,21 +748,12 @@ func buildTestSuite() []Test { return nil }, }, - - // pgaudit extension (standard only) { - Name: "pgaudit extension can be created", - StandardOnly: true, - Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS pgaudit; SELECT 1;\"", - ExpectedOutput: func(exitCode int, output string) error { - if exitCode != 0 { - return fmt.Errorf("unexpected exit code: %d", exitCode) - } - return nil - }, + Name: "pgaudit extension can be created", + StandardOnly: true, + Cmd: "psql -U postgres -d testdb -t -A -c \"CREATE EXTENSION IF NOT EXISTS pgaudit; SELECT 1;\"", + ExpectedOutput: expectSuccess, }, - - // pgBackRest (standard only) { Name: "pgBackRest is installed", StandardOnly: true, @@ -647,6 +769,11 @@ func buildTestSuite() []Test { }, }, } +} - return tests +func expectSuccess(exitCode int, output string) error { + if exitCode != 0 { + return fmt.Errorf("unexpected exit code: %d", exitCode) + } + return nil }