test(pg-compat): SP-2 — polyglot PG test foundation (dbdeployer-PG infra, Toxiproxy, differential engine, routing oracle, CI) - #5903
Conversation
…ved-ports claim, final command block)
Fill in infra-dbdeployer-pgsql17-repl/bin/docker-proxy-post.bash (was a
no-op placeholder) and add its conf/proxysql/infra-config.sql: all three
PG backends load into the writer hostgroup addressed through Toxiproxy
(toxiproxy.${INFRA_ID}:6001/6002/6003), pgsql_replication_hostgroups
drives automatic rw-split via check_type='read_only', and the monitor
(monitor/monitor role, 1000ms read_only_interval) demotes replicas by
polling pg_is_in_recovery() through the proxy.
Uses ${INFRA_ID} rather than ${INFRA} for hostnames/comment-tagging:
${INFRA} is only reliably exported when this script runs via
docker-compose-init.bash, not via ensure-infras.bash's already-running
reconfigure path, which would otherwise template unresolvable
"toxiproxy." hostnames. docker-compose.yml already provisions the
toxiproxy.${INFRA_ID}/dbdeployer1.${INFRA_ID} aliases for this reason.
Verified end-to-end on infra sdd-sp2: applied via
test/infra/control/ensure-infras.bash, monitor demotes both replicas
within a few seconds (runtime_pgsql_servers: 1 writer + 2 readers,
pgsql_server_read_only_log clean), and frontend routing on port 6133
is correct (SELECT pg_is_in_recovery() -> reader, writes -> writer).
…TOP) Add -v ON_ERROR_STOP=1 to the psql invocation that applies infra-config.sql to the ProxySQL admin (PG protocol, port 6132). Without it psql prints SQL-level errors (bad token, constraint violation, duplicate rule_id) but continues and exits 0, so set -e never fires and the script's fail-non-zero contract was silently defeated -- connection failures were caught, SQL failures were not. Matches the precedent in this infra's docker/entrypoint.sh and infra-pgsql17-repl's init-replication.sh. Verified on sdd-sp2: a bogus statement piped with ON_ERROR_STOP=1 now aborts with exit 3 at the first error (without the flag the same input kept executing and exited 0), and a full ensure-infras.bash re-apply still exits 0 leaving runtime_pgsql_servers in the expected 1-writer/2-reader state.
… self-check (Task 6 review)
Adds the driver-agnostic behavior set (connect, transactions, prepared, session_isolation) behind a small adapter interface, plus the psycopg3 reference adapter, so SP-3 can add Java/Go/Node adapters against the same behaviors with no changes to the behavior modules. Two adaptations to the original brief, folded in from SP-1 lessons: - session_isolation probes TimeZone instead of application_name, since ProxySQL hardcodes application_name in ignore_vars (lib/PgSQL_Variables.cpp) and never forwards it to the backend. - transactions wraps every verification read in an explicit begin()/commit() so it pins to the writer hostgroup, avoiding a replica read-after-write race under the RW-split (^SELECT -> reader). Verified empirically via the routing oracle: the verify-read lands 2/0/0 on primary/replica1/replica2. Also fixes a real psycopg3 API mismatch in the brief's prepared.py (SQL must use %s placeholders, not raw $1/$2, for cursor.execute(sql, params)).
…le, non-gating)
Wires test/pg-compat/ into CI: schedule (nightly) + pg-compat-labeled PRs +
manual dispatch, building ProxySQL inline (CI-3p-* model) rather than
chaining off CI-trigger/CI-builds. Non-gating during the discovery phase
(spec sec 2.1): the run step uses `|| true` and publishes the junitxml as
an artifact regardless of outcome.
Follows the two-branch caller/reusable split: CI-pg-compat.yml (caller,
v3.0) + gh-actions-reusable/ci-pg-compat.yml (reusable, staged here for
merge to GH-Actions first, per doc/GH-Actions/README.md's merge-order
requirement -- both files call this out at the top).
Also fixes a real bug in run-pg-compat.bash traced while wiring this up:
the pytest container runs with --rm and had no volume mount, so any
--junitxml report written inside it was destroyed on exit before ever
reaching the host. run-pg-compat.bash now bind-mounts a host directory
(default ${WORKSPACE}/pg-compat-reports, override via
PGCOMPAT_REPORT_DIR) to /pg-compat-reports in the container so reports
survive; verified end-to-end with a local dry run (throwaway ci-dryrun
infra) that the file lands on the host at the exact path the CI artifact
step reads, including the root-owned-file chown needed before upload.
…consistency hardening (final review) - diff.py: snapshot/restore pgsql-use_native_backend_protocol around the target loop in _run() (shared by run_case/run_case_sql) so a native-mode toggle never leaks into later cases once PR #5882 lands the variable; a pure no-op today since the variable is absent. - conftest.py: pin client_encoding=UTF8 on the proxy DSN, matching targets.py and drivers/python/adapter.py. - behaviors/{connect,prepared,session_isolation}.py: wrap bodies in try/finally so connections close even on assert failure; make PsycopgAdapter.close() idempotent since session_isolation.py's finally may close an already-closed connection. - behaviors/transactions.py: fix stale comment pointing at a nonexistent harness/oracle.py; oracle_w lives in tests/test_routing_oracle.py.
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
📝 WalkthroughWalkthroughThis PR adds a pg-compat discovery-phase test suite for ProxySQL PostgreSQL wire-protocol compatibility. It includes CI workflows, PostgreSQL 17 replication infrastructure with Toxiproxy, a Python/pytest harness, differential and routing tests, and documentation. ChangesPG-Compat Suite
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant CI as CI-pg-compat workflow
participant Infra as Compose infrastructure
participant Proxy as ProxySQL
participant PG as PostgreSQL replicas
participant Suite as pg-compat pytest suite
CI->>Infra: provision replication environment
Infra->>PG: deploy and configure primary/replicas
Infra->>Proxy: configure Toxiproxy backends and rw-split
CI->>Suite: run discovery-phase tests
Suite->>Proxy: execute proxied SQL
Suite->>PG: execute direct baseline SQL
Suite-->>CI: upload JUnit report
CI->>Infra: destroy infrastructure
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
.github/workflows/gh-actions-reusable/ci-pg-compat.yml (1)
43-43: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider narrowing
write-allto least-privilege permissions.Same as the caller workflow — only
contents: read(checkout) andactions: write(upload-artifact) are needed here. Since reusable-workflow permissions are the intersection of caller and callee, both sides should be narrowed together.🔐 Suggested permission narrowing
- permissions: write-all + permissions: + contents: read + actions: write🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/gh-actions-reusable/ci-pg-compat.yml at line 43, The reusable workflow currently uses overly broad permissions in the ci-pg-compat workflow. Update the permissions block to least privilege by narrowing it to the specific symbols used here: contents access for checkout and actions access for upload-artifact, and keep it aligned with the caller workflow since reusable-workflow permissions are intersected with the caller. Ensure the workflow-level permissions declaration reflects only the minimum required access instead of write-all.Source: Linters/SAST tools
.github/workflows/CI-pg-compat.yml (1)
43-43: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider narrowing
write-allto least-privilege permissions.The comment explains the rationale, but
write-allgrants every GitHub token scope. The only permissions actually needed arecontents: read(for checkout) andactions: write(for upload-artifact). Narrowing both caller and callee to these specific scopes reduces the blast radius if the token is exfiltrated.🔐 Suggested permission narrowing
- permissions: write-all + permissions: + contents: read + actions: write🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/CI-pg-compat.yml at line 43, The workflow currently uses broad `write-all` permissions, so narrow the permissions in the CI-pg-compat workflow to least privilege. Update the workflow’s permissions block to grant only the scopes needed by the jobs, specifically `contents: read` for checkout and `actions: write` for artifact upload, and make sure any called workflow or reusable job configuration matches these same limited scopes.Source: Linters/SAST tools
test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bash (1)
70-76: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAvoid
evalfor variable expansion in path resolution.
eval "ACTUAL_PATH=${RAW_PATH}"re-parsesRAW_PATHas shell code. WhileRAW_PATHoriginates fromdocker-compose.ymlin a controlled CI context (not attacker-controlled),evalis an unsafe pattern. Useenvsubstor direct variable substitution instead.🔒 Safer alternative using envsubst
- eval "ACTUAL_PATH=${RAW_PATH}" + ACTUAL_PATH="$(echo "${RAW_PATH}" | envsubst)"This requires
INFRA_LOGS_PATH,COMPOSE_PROJECT, etc. to be in the environment (which they are by this point in the script). Ifenvsubstis unavailable, a simpler approach isACTUAL_PATH="${RAW_PATH//\$\{INFRA_LOGS_PATH\}/${INFRA_LOGS_PATH}}"butenvsubsthandles all variables cleanly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bash` around lines 70 - 76, The path resolution loop in docker-compose-init.bash currently uses eval to expand RAW_PATH into ACTUAL_PATH, which should be removed. Update the expansion logic in the MOUNTED_PATHS loop to use a safe substitution approach such as envsubst or direct shell variable replacement, while preserving the existing handling in the RAW_PATH and ACTUAL_PATH flow. Make sure the replacement still resolves placeholders like INFRA_LOGS_PATH and COMPOSE_PROJECT without re-parsing shell code.test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-destroy.bash (1)
1-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider mirroring the COMPOSE_CMD fallback from the init script.
This script hardcodes
docker compose(line 11), whiledocker-compose-init.bash(lines 37-44) detects and falls back todocker-composev1. If the CI environment only has the v1 binary, teardown will fail. For consistency, reuse the same detection logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-destroy.bash` around lines 1 - 11, The teardown script hardcodes the Compose CLI in the destroy flow, which can fail in environments that only provide the legacy binary. Update docker-compose-destroy.bash to mirror the COMPOSE_CMD detection logic used by docker-compose-init.bash, then use that resolved command in the destroy step instead of calling docker compose directly. Keep the change centered around the existing compose invocation and the script’s setup section so the init and destroy scripts stay consistent.test/pg-compat/Dockerfile (1)
1-7: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider adding a non-root
USERdirective.Both SonarCloud and Trivy flag running as root. For a disposable test container this is low-risk, but adding
USERis a trivial best-practice hardening step.🔒️ Suggested non-root user
FROM python:3.11-slim RUN apt-get update && apt-get install -y --no-install-recommends libpq5 curl && rm -rf /var/lib/apt/lists/* +RUN useradd -m -u 1000 pgcompat WORKDIR /pg-compat COPY requirements.txt . RUN pip install --no-cache-dir -r requirements.txt COPY . . +USER pgcompat ENTRYPOINT ["pytest", "-q"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/pg-compat/Dockerfile` around lines 1 - 7, The Dockerfile for the pg-compat test container currently runs as root; add a non-root USER directive near the end of the Dockerfile so the container executes pytest as an unprivileged user. Use the existing build steps in the Dockerfile and place the USER change after dependencies and source copy are complete, keeping the ENTRYPOINT on pytest unchanged.Source: Linters/SAST tools
test/pg-compat/harness/proxysql.py (1)
52-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
close()method toAdminfor fixture cleanup.The session-scoped
adminfixture inconftest.pycreatesAdmin()but never closes the underlying psycopg connection. Adding aclose()method and switching the fixture to ayieldpattern ensures clean teardown.♻️ Proposed fix
def restore(self, saved): for name, value in saved.items(): self.set_var(name, value) self.load_vars() + + def close(self): + if not self.conn.closed: + self.conn.close()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/pg-compat/harness/proxysql.py` around lines 52 - 106, The Admin helper currently leaves its psycopg connection open, so add a close() method on Admin that closes self.conn and update the session-scoped admin fixture to use a yield-based teardown that calls it. Keep the fix localized to the Admin class and the fixture in conftest.py so the connection is always cleaned up after tests.test/pg-compat/conftest.py (2)
15-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDSN construction is duplicated across 4 files.
The same
host=... port=... user=... password=... dbname=... sslmode=disable client_encoding=UTF8pattern appears inadapter.py,targets.py(_dsn),conftest.py(_proxy_dsn), andoracle.py(_dsn). A shared helper would keep these in sync if the DSN contract changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/pg-compat/conftest.py` around lines 15 - 32, The DSN string assembly in _proxy_dsn is duplicated across multiple helpers and should be centralized. Extract the common host/port/user/password/dbname/sslmode/client_encoding construction into a shared helper, then have _proxy_dsn, the _dsn helper in targets.py, the adapter.py DSN code, and oracle.py’s _dsn use that single source so future DSN contract changes stay in sync.
10-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a
yieldfixture to close theAdminconnection on teardown.The session-scoped
adminfixture createsAdmin()but never closes its underlying psycopg connection. OnceAdmin.close()is added (seeharness/proxysql.py), switch to a yield fixture for clean teardown.♻️ Proposed fix
`@pytest.fixture`(scope="session") def admin(): - return Admin() + a = Admin() + yield a + a.close()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/pg-compat/conftest.py` around lines 10 - 12, The session-scoped admin fixture currently returns Admin() without cleanup, so update the admin fixture to use a yield-based teardown that closes the Admin connection after tests finish. Locate the admin fixture in conftest.py and, once Admin.close() is available on the Admin class in harness/proxysql.py, yield the Admin instance from the fixture and ensure close() is called after the yield so the underlying psycopg connection is released cleanly.test/infra/infra-dbdeployer-pgsql17-repl/bin/toxiproxy-bootstrap.sh (1)
50-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace
/tmp/toxi_create_respwith/dev/null.The response body captured by
-o /tmp/toxi_create_respis never read — it's just a sink while-w '%{http_code}'captures the status code. Using/dev/nullavoids the predictable temp file (flagged by static analysis as CWE-377) and is functionally equivalent since the file lives inside an ephemeral--rmcontainer anyway.♻️ Proposed fix
local http_code - http_code=$(curl_in_net -sS -o /tmp/toxi_create_resp -w '%{http_code}' \ + http_code=$(curl_in_net -sS -o /dev/null -w '%{http_code}' \ -XPOST "http://${TOXI_ADMIN}/proxies" -d "${body}" 2>/dev/null || echo "000")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/infra/infra-dbdeployer-pgsql17-repl/bin/toxiproxy-bootstrap.sh` around lines 50 - 51, The curl invocation in toxiproxy-bootstrap.sh is writing an unused response body to a predictable temp file; update the curl_in_net call in the proxy creation path to discard the body via /dev/null instead of /tmp/toxi_create_resp while keeping the -w '%{http_code}' capture intact.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/gh-actions-reusable/ci-pg-compat.yml:
- Line 46: The checkout step in the reusable CI workflow should disable
credential persistence to avoid leaving the GITHUB_TOKEN in the workspace.
Update the actions/checkout@v4 step to include persist-credentials set to false,
so the token is not written to .git/config and cannot be accessed by later
artifact or container steps.
In `@test/infra/infra-dbdeployer-pgsql17-repl/bin/docker-proxy-post.bash`:
- Around line 24-28: The ProxySQL admin readiness loop in docker-proxy-post.bash
has no timeout, so it can hang forever if the service never comes up. Update the
wait logic around the docker exec mysql check to track elapsed time and exit
with a clear failure after a bounded limit, matching the timeout behavior used
in docker-pgsql-post.bash and toxiproxy-bootstrap.sh. Keep the existing
readiness probe but add a timeout guard and an error message so the script fails
fast instead of looping indefinitely.
- Around line 31-32: Update the SQL template loading in docker-proxy-post.bash
so it uses the SCRIPT_DIR-relative path instead of a CWD-relative
./conf/proxysql/infra-config.sql reference, since the script may be invoked
directly by ensure-infras.bash. Also replace the eval-based expansion in
SQL_CONTENT with envsubst using an explicit allowlist of expected variables, and
keep the existing logic around SQL_TEMPLATE/SQL_CONTENT so only intended
placeholders are expanded.
In `@test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bash`:
- Around line 3-9: The INFRA_ID fallback in docker-compose-init.bash is deriving
the name from the parent directory instead of the current infra directory.
Update the INFRA_ID assignment logic to use the current working directory name
directly in the fallback path, then keep the existing sed cleanup so the infra-
prefix is stripped correctly. Use the INFRA_ID initialization block and its
final safety fallback as the places to adjust.
In `@test/pg-compat/behaviors/transactions.py`:
- Around line 50-56: The cleanup in the `finally` block of `transactions.py`
should first reset any failed transaction state before dropping the test table.
Add a `rollback()` on the same connection object used by `exec_simple` and
`begin()`/`commit()` before the `DROP TABLE IF EXISTS` call, so the connection
returns to autocommit mode and the cleanup can succeed even after an exception.
Keep the existing `TABLE` cleanup and `a.close()` flow, but ensure the rollback
happens in the `finally` path before `exec_simple`.
In `@test/pg-compat/requirements.txt`:
- Around line 1-4: The dependency pin for pytest is still on the vulnerable 8.x
line, so update the test/pg-compat requirements entry to a patched 9.x release
that includes the CVE-2025-71176 fix. Adjust the pytest version specifier in the
requirements file while keeping the existing pins for psycopg, asyncpg, and
tomli unchanged.
---
Nitpick comments:
In @.github/workflows/CI-pg-compat.yml:
- Line 43: The workflow currently uses broad `write-all` permissions, so narrow
the permissions in the CI-pg-compat workflow to least privilege. Update the
workflow’s permissions block to grant only the scopes needed by the jobs,
specifically `contents: read` for checkout and `actions: write` for artifact
upload, and make sure any called workflow or reusable job configuration matches
these same limited scopes.
In @.github/workflows/gh-actions-reusable/ci-pg-compat.yml:
- Line 43: The reusable workflow currently uses overly broad permissions in the
ci-pg-compat workflow. Update the permissions block to least privilege by
narrowing it to the specific symbols used here: contents access for checkout and
actions access for upload-artifact, and keep it aligned with the caller workflow
since reusable-workflow permissions are intersected with the caller. Ensure the
workflow-level permissions declaration reflects only the minimum required access
instead of write-all.
In `@test/infra/infra-dbdeployer-pgsql17-repl/bin/toxiproxy-bootstrap.sh`:
- Around line 50-51: The curl invocation in toxiproxy-bootstrap.sh is writing an
unused response body to a predictable temp file; update the curl_in_net call in
the proxy creation path to discard the body via /dev/null instead of
/tmp/toxi_create_resp while keeping the -w '%{http_code}' capture intact.
In `@test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-destroy.bash`:
- Around line 1-11: The teardown script hardcodes the Compose CLI in the destroy
flow, which can fail in environments that only provide the legacy binary. Update
docker-compose-destroy.bash to mirror the COMPOSE_CMD detection logic used by
docker-compose-init.bash, then use that resolved command in the destroy step
instead of calling docker compose directly. Keep the change centered around the
existing compose invocation and the script’s setup section so the init and
destroy scripts stay consistent.
In `@test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bash`:
- Around line 70-76: The path resolution loop in docker-compose-init.bash
currently uses eval to expand RAW_PATH into ACTUAL_PATH, which should be
removed. Update the expansion logic in the MOUNTED_PATHS loop to use a safe
substitution approach such as envsubst or direct shell variable replacement,
while preserving the existing handling in the RAW_PATH and ACTUAL_PATH flow.
Make sure the replacement still resolves placeholders like INFRA_LOGS_PATH and
COMPOSE_PROJECT without re-parsing shell code.
In `@test/pg-compat/conftest.py`:
- Around line 15-32: The DSN string assembly in _proxy_dsn is duplicated across
multiple helpers and should be centralized. Extract the common
host/port/user/password/dbname/sslmode/client_encoding construction into a
shared helper, then have _proxy_dsn, the _dsn helper in targets.py, the
adapter.py DSN code, and oracle.py’s _dsn use that single source so future DSN
contract changes stay in sync.
- Around line 10-12: The session-scoped admin fixture currently returns Admin()
without cleanup, so update the admin fixture to use a yield-based teardown that
closes the Admin connection after tests finish. Locate the admin fixture in
conftest.py and, once Admin.close() is available on the Admin class in
harness/proxysql.py, yield the Admin instance from the fixture and ensure
close() is called after the yield so the underlying psycopg connection is
released cleanly.
In `@test/pg-compat/Dockerfile`:
- Around line 1-7: The Dockerfile for the pg-compat test container currently
runs as root; add a non-root USER directive near the end of the Dockerfile so
the container executes pytest as an unprivileged user. Use the existing build
steps in the Dockerfile and place the USER change after dependencies and source
copy are complete, keeping the ENTRYPOINT on pytest unchanged.
In `@test/pg-compat/harness/proxysql.py`:
- Around line 52-106: The Admin helper currently leaves its psycopg connection
open, so add a close() method on Admin that closes self.conn and update the
session-scoped admin fixture to use a yield-based teardown that calls it. Keep
the fix localized to the Admin class and the fixture in conftest.py so the
connection is always cleaned up after tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8eacc031-1898-4861-b22e-60a40267132e
📒 Files selected for processing (45)
.github/workflows/CI-pg-compat.yml.github/workflows/gh-actions-reusable/ci-pg-compat.yml.gitignoretest/infra/infra-dbdeployer-pgsql17-repl/.envtest/infra/infra-dbdeployer-pgsql17-repl/bin/docker-pgsql-post.bashtest/infra/infra-dbdeployer-pgsql17-repl/bin/docker-proxy-post.bashtest/infra/infra-dbdeployer-pgsql17-repl/bin/toxiproxy-bootstrap.shtest/infra/infra-dbdeployer-pgsql17-repl/conf/proxysql/infra-config.sqltest/infra/infra-dbdeployer-pgsql17-repl/docker-compose-destroy.bashtest/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bashtest/infra/infra-dbdeployer-pgsql17-repl/docker-compose.ymltest/infra/infra-dbdeployer-pgsql17-repl/docker/Dockerfiletest/infra/infra-dbdeployer-pgsql17-repl/docker/build.shtest/infra/infra-dbdeployer-pgsql17-repl/docker/entrypoint.shtest/pg-compat/Dockerfiletest/pg-compat/README.mdtest/pg-compat/SPIKE-dbdeployer-pg.mdtest/pg-compat/behaviors/__init__.pytest/pg-compat/behaviors/connect.pytest/pg-compat/behaviors/prepared.pytest/pg-compat/behaviors/session_isolation.pytest/pg-compat/behaviors/transactions.pytest/pg-compat/cases/001_scalars.sqltest/pg-compat/cases/002_bytea_json_array.sqltest/pg-compat/conftest.pytest/pg-compat/drivers/__init__.pytest/pg-compat/drivers/python/__init__.pytest/pg-compat/drivers/python/adapter.pytest/pg-compat/harness/__init__.pytest/pg-compat/harness/diff.pytest/pg-compat/harness/oracle.pytest/pg-compat/harness/proxysql.pytest/pg-compat/harness/targets.pytest/pg-compat/harness/xfail.pytest/pg-compat/pytest.initest/pg-compat/requirements.txttest/pg-compat/run-pg-compat.bashtest/pg-compat/tests/test_behaviors.pytest/pg-compat/tests/test_differential.pytest/pg-compat/tests/test_differential_selfcheck.pytest/pg-compat/tests/test_routing_oracle.pytest/pg-compat/tests/test_smoke.pytest/pg-compat/xfail.tomltest/tap/groups/pg-compat/env.shtest/tap/groups/pg-compat/infras.lst
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-03-26T16:38:58.553Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5548
File: lib/mysql_connection.cpp:1837-1843
Timestamp: 2026-03-26T16:38:58.553Z
Learning: In `lib/mysql_connection.cpp`, when reviewing `SHOW WARNINGS` handling, treat the digest source as an intentional design choice: both `update_warning_count_from_connection()` and the `add_eof()` call under `ASYNC_USE_RESULT_CONT` detect warnings using `myds->sess->CurrentQuery.QueryParserArgs.digest_text` (comment-stripped digest text). This is expected to fail/behave differently when `mysql-query_digests_keep_comment=1` (digest_text includes comments) or when `mysql-query_digests=0` (digest_text unavailable). Do not require a change unless the regression test coverage is expanded (noting `reg_test_5306-show_warnings_with_comment-t` explicitly excludes these configurations as an accepted limitation).
Applied to files:
.gitignore
📚 Learning: 2026-04-11T13:17:55.508Z
Learnt from: renecannao
Repo: sysown/proxysql PR: 5607
File: doc/GH-Actions/README.md:13-18
Timestamp: 2026-04-11T13:17:55.508Z
Learning: When using GitHub-flavored Markdown headings, be aware that an em-dash surrounded by spaces (written as ` — `) affects the generated anchor/slug: GitHub replaces spaces with hyphens and removes non-alphanumeric punctuation, which can produce double hyphens (e.g., `## Foo — bar` → anchor `#foo--bar`, not `#foo-bar`). If you reference these anchors (e.g., internal links), ensure the expected slug matches this behavior.
Applied to files:
test/pg-compat/README.mdtest/pg-compat/SPIKE-dbdeployer-pg.md
📚 Learning: 2026-04-11T13:17:55.509Z
Learnt from: renecannao
Repo: sysown/proxysql PR: 5607
File: doc/GH-Actions/README.md:13-18
Timestamp: 2026-04-11T13:17:55.509Z
Learning: When reviewing GitHub-flavored Markdown links/anchors, remember that heading-to-anchor slug generation treats spaces as hyphens and removes punctuation. If a heading contains an em-dash surrounded by spaces (e.g. ` — `), the slugs can legitimately include a double hyphen where the two surrounding space-runs become `-` on either side of the removed em-dash (e.g. `...vocabulary--read...`). Do not flag double-hyphens in anchor links for em-dash-containing headings as errors; they reflect GitHub’s correct slug behavior.
Applied to files:
test/pg-compat/README.mdtest/pg-compat/SPIKE-dbdeployer-pg.md
🪛 ast-grep (0.44.1)
test/pg-compat/harness/xfail.py
[warning] 24-24: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(_XFAIL_TOML, "rb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
test/infra/infra-dbdeployer-pgsql17-repl/bin/toxiproxy-bootstrap.sh
[warning] 49-49: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/toxi_create_resp
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
test/infra/infra-dbdeployer-pgsql17-repl/bin/docker-proxy-post.bash
[error] 31-31: eval is invoked on a variable, parameter expansion, or command-substitution result, which re-parses the value as shell code. If any part of that value is attacker-controlled (arguments, environment, file contents, network output), it allows arbitrary command execution. Do not eval dynamic data: invoke the command directly with proper quoting (e.g. "$cmd" "$arg"), use arrays for argument lists (cmd=(prog --flag "$value"); "${cmd[@]}"), or restrict input to a validated allowlist before running it.
Context: eval "echo "${SQL_TEMPLATE}""
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(eval-on-variable-bash)
test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bash
[error] 74-74: eval is invoked on a variable, parameter expansion, or command-substitution result, which re-parses the value as shell code. If any part of that value is attacker-controlled (arguments, environment, file contents, network output), it allows arbitrary command execution. Do not eval dynamic data: invoke the command directly with proper quoting (e.g. "$cmd" "$arg"), use arrays for argument lists (cmd=(prog --flag "$value"); "${cmd[@]}"), or restrict input to a validated allowlist before running it.
Context: eval "ACTUAL_PATH=${RAW_PATH}"
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(eval-on-variable-bash)
[warning] 133-133: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/dbdeployer_ready
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
test/infra/infra-dbdeployer-pgsql17-repl/docker/entrypoint.sh
[warning] 186-186: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/dbdeployer_ready
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
test/pg-compat/harness/diff.py
[warning] 63-63: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(case_file)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 dotenv-linter (4.0.0)
test/infra/infra-dbdeployer-pgsql17-repl/.env
[warning] 6-6: [UnorderedKey] The RHG key should go before the WHG key
(UnorderedKey)
🪛 GitHub Check: SonarCloud Code Analysis
test/pg-compat/Dockerfile
[warning] 1-1: The "python" image runs with "root" as the default user. Make sure it is safe here.
[failure] 6-6: Copying recursively might inadvertently add sensitive data to the container. Make sure it is safe here.
test/pg-compat/behaviors/prepared.py
[warning] 28-28: Rename this parameter "Adapter" to match the regular expression ^[a-z][a-z0-9]*$.
test/pg-compat/behaviors/connect.py
[warning] 8-8: Rename this parameter "Adapter" to match the regular expression ^[a-z][a-z0-9]*$.
test/pg-compat/tests/test_differential_selfcheck.py
[warning] 62-62: Replace the unused local variable "detail" with "_".
.github/workflows/CI-pg-compat.yml
[warning] 43-43: Replace "write-all" with specific permissions (e.g., "contents: write").
[warning] 45-45: Only pass required secrets to this workflow.
[failure] 44-44: Use full commit SHA hash for this dependency.
test/pg-compat/behaviors/session_isolation.py
[warning] 37-37: Rename this parameter "Adapter" to match the regular expression ^[a-z][a-z0-9]*$.
test/pg-compat/run-pg-compat.bash
[failure] 36-36: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
test/pg-compat/behaviors/transactions.py
[warning] 27-27: Rename this parameter "Adapter" to match the regular expression ^[a-z][a-z0-9]*$.
test/pg-compat/harness/targets.py
[warning] 74-74: Remove this commented out code.
🪛 LanguageTool
test/pg-compat/README.md
[uncategorized] ~54-~54: The official name of this software platform is spelled with a capital “H”.
Context: ...ite is wired into CI as CI-pg-compat (.github/workflows/CI-pg-compat.yml caller on `...
(GITHUB)
test/pg-compat/SPIKE-dbdeployer-pg.md
[locale-violation] ~191-~191: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...he highlighted flags were proven no-ops afterwards: ```bash dbdeployer deploy replication...
(AFTERWARDS_US)
🪛 markdownlint-cli2 (0.22.1)
test/pg-compat/SPIKE-dbdeployer-pg.md
[warning] 38-38: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 117-117: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 206-206: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 261-261: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 290-290: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 345-345: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 OpenGrep (1.23.0)
test/pg-compat/tests/test_routing_oracle.py
[ERROR] 45-45: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 46-46: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 58-58: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 Ruff (0.15.20)
test/pg-compat/tests/test_differential_selfcheck.py
[error] 54-57: Possible SQL injection vector through string-based query construction
(S608)
[warning] 62-62: Unpacked variable detail is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[error] 69-69: Possible SQL injection vector through string-based query construction
(S608)
test/pg-compat/tests/test_routing_oracle.py
[error] 46-46: Possible SQL injection vector through string-based query construction
(S608)
test/pg-compat/behaviors/transactions.py
[error] 34-34: Possible SQL injection vector through string-based query construction
(S608)
[error] 38-38: Possible SQL injection vector through string-based query construction
(S608)
[error] 43-43: Possible SQL injection vector through string-based query construction
(S608)
[error] 47-47: Possible SQL injection vector through string-based query construction
(S608)
test/pg-compat/harness/proxysql.py
[error] 98-99: Possible SQL injection vector through string-based query construction
(S608)
test/pg-compat/harness/targets.py
[error] 86-87: Possible SQL injection vector through string-based query construction
(S608)
🪛 Shellcheck (0.11.0)
test/infra/infra-dbdeployer-pgsql17-repl/bin/docker-proxy-post.bash
[info] 12-12: Not following: ./../.env was not specified as input (see shellcheck -x).
(SC1091)
test/pg-compat/run-pg-compat.bash
[info] 11-11: Not following: ./test/tap/groups/pg-compat/env.sh was not specified as input (see shellcheck -x).
(SC1091)
test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-destroy.bash
[warning] 4-4: Quote this to prevent word splitting.
(SC2046)
[info] 4-4: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 6-6: Not following: .env was not specified as input (see shellcheck -x).
(SC1091)
test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bash
[warning] 4-4: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 4-4: Quote this to prevent word splitting.
(SC2046)
[warning] 4-4: Quote this to prevent word splitting.
(SC2046)
[warning] 24-24: Quote this to prevent word splitting.
(SC2046)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[warning] 24-24: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 27-27: Quote this to prevent word splitting.
(SC2046)
[info] 27-27: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 33-33: Not following: .env was not specified as input (see shellcheck -x).
(SC1091)
[warning] 48-48: Declare and assign separately to avoid masking return values.
(SC2155)
test/infra/infra-dbdeployer-pgsql17-repl/docker/entrypoint.sh
[info] 18-18: Use find instead of ls to better handle non-alphanumeric filenames.
(SC2012)
[info] 34-34: Use find instead of ls to better handle non-alphanumeric filenames.
(SC2012)
test/infra/infra-dbdeployer-pgsql17-repl/bin/docker-pgsql-post.bash
[info] 10-10: Not following: .env was not specified as input (see shellcheck -x).
(SC1091)
🪛 Trivy (0.69.3)
test/pg-compat/Dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
test/infra/infra-dbdeployer-pgsql17-repl/docker/Dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[error] 48-60: 'apt-get' missing '--no-install-recommends'
'--no-install-recommends' flag is missed: 'mkdir -p /root/pgdebs && apt-get update && apt-get install -y -qq --download-only --reinstall -o Dir::Cache::archives=/root/pgdebs postgresql-${PG_MAJOR}=${PG_DEB_VERSION} postgresql-client-${PG_MAJOR}=${PG_DEB_VERSION} postgresql-common postgresql-client-common && DEP_DEBS=$(ls /root/pgdebs/.deb | grep -vE "/(postgresql-${PG_MAJOR}|postgresql-client-${PG_MAJOR})_[0-9]") && apt-get install -y -qq ${DEP_DEBS} && ldconfig && rm -rf /var/lib/apt/lists/'
Rule: DS-0029
(IaC/Dockerfile)
🪛 zizmor (1.26.1)
.github/workflows/gh-actions-reusable/ci-pg-compat.yml
[warning] 45-55: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 43-43: overly broad permissions (excessive-permissions): uses write-all permissions
(excessive-permissions)
.github/workflows/CI-pg-compat.yml
[error] 43-43: overly broad permissions (excessive-permissions): uses write-all permissions
(excessive-permissions)
[warning] 44-44: secrets unconditionally inherited by called workflow (secrets-inherit): this reusable workflow
(secrets-inherit)
🔇 Additional comments (42)
test/infra/infra-dbdeployer-pgsql17-repl/docker/Dockerfile (1)
1-88: LGTM!The Dockerfile is well-documented and follows a sound approach for baking a dbdeployer/PostgreSQL 17 image. The port derivation scheme (15000 + major×100 + minor = 16710) is consistent with the
.envcontract (ports 16710/16711/16712), and thePG_VERSIONARG expansion in theENV PATHdirective correctly bakes the concrete versioned path at build time.The Trivy findings (DS-0002 root user, DS-0029 missing
--no-install-recommendson the dependency install) are acceptable here: the entrypoint requires root forsu - pguseruser-switching, and the image is test-only infrastructure. Thegrepfilter on line 57 correctly excludes the mainpostgresql-17/postgresql-client-17packages while retaining dependency.debsfor system-wide installation.test/infra/infra-dbdeployer-pgsql17-repl/docker/build.sh (1)
1-8: LGTM!test/infra/infra-dbdeployer-pgsql17-repl/docker/entrypoint.sh (1)
1-200: LGTM!The entrypoint is well-structured with clear section boundaries, correct timeout/retry logic, and proper error propagation via
set -eandON_ERROR_STOP=1. Key cross-file contracts are satisfied:
- Port derivation (lines 42–46):
BASE_PORT##*_correctly extracts the port frompostgresql_repl_<port>, producing 16710/16711/16712 — matchingenv.shand.env.- pg_hba.conf layering (lines 74–78): The default initdb
host all all 127.0.0.1/32 trustrule takes precedence over the appended0.0.0.0/0 md5for loopback, so allpsql -h 127.0.0.1commands in the script work without a password both before and after the postgres password is set. Non-loopback connections (from other containers) correctly require md5.- ROOT_PASSWORD contract (line 146): The password set here matches the sha256-derived value that
oracle.pycomputes fromINFRA_ID, ensuring downstream oracle connections succeed.- pg_stat_statements provisioning (lines 160–182): Extension created on primary, verified on replicas with a 30s wait — satisfies the routing oracle's requirement for
pg_stat_statementson all nodes.The static analysis findings are false positives in this context:
/tmp/dbdeployer_ready(CWE-377) is a fixed-path readiness signal consumed bydocker-compose-init.bashwith no local attacker in a single-purpose container, and thelsusage (SC2012) on lines 18/34 targets version/sandbox directory names that don't contain special characters..github/workflows/CI-pg-compat.yml (1)
44-44: 🔒 Security & PrivacyVerify pinning the reusable workflow to a commit SHA.
Using
@GH-Actions(branch ref) means any push to that branch immediately changes what this workflow executes. While the two-branch architecture is intentional and documented, pinning to a full commit SHA is the GitHub security best practice for reusable workflows to prevent supply-chain attacks via branch compromise.Source: Linters/SAST tools
.github/workflows/gh-actions-reusable/ci-pg-compat.yml (1)
87-91: LGTM!.gitignore (1)
231-234: LGTM!test/tap/groups/pg-compat/infras.lst (1)
1-1: LGTM!test/tap/groups/pg-compat/env.sh (1)
1-26: LGTM!test/infra/infra-dbdeployer-pgsql17-repl/.env (1)
1-13: LGTM!test/infra/infra-dbdeployer-pgsql17-repl/docker-compose.yml (2)
10-17: LGTM!
32-35: 🩺 Stability & AvailabilityEnsure
${INFRA_ID}_backendexists beforecompose up. Thebackendnetwork isexternal: true, sodocker-compose-init.bashneeds a prior step that creates it; otherwisecompose upwill fail.test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bash (2)
46-51: LGTM!
129-161: LGTM!test/pg-compat/drivers/python/adapter.py (1)
31-70: LGTM!test/pg-compat/harness/targets.py (1)
1-109: LGTM!test/pg-compat/harness/oracle.py (1)
1-91: LGTM!test/pg-compat/harness/xfail.py (1)
1-42: LGTM!test/pg-compat/conftest.py (1)
47-58: LGTM!test/pg-compat/harness/diff.py (7)
63-69: Static analysis path-traversal flag is a false positive.The
open(case_file)on line 64 was flagged by ast-grep for CWE-22 (path traversal). This is not a real risk:case_fileoriginates fromglob.glob(os.path.join(CASES_DIR, "*.sql"))in the test layer (test_differential_selfcheck.py:75), not from user input. No action needed.
37-44: LGTM!
47-60: LGTM!
72-89: LGTM!
92-118: LGTM!
121-143: LGTM!
146-170: LGTM!test/pg-compat/pytest.ini (1)
1-7: LGTM!test/pg-compat/xfail.toml (1)
1-60: LGTM!test/pg-compat/behaviors/connect.py (1)
8-13: LGTM!test/pg-compat/behaviors/prepared.py (1)
28-35: LGTM!test/pg-compat/run-pg-compat.bash (1)
1-59: LGTM!test/pg-compat/behaviors/session_isolation.py (1)
1-57: LGTM!test/pg-compat/tests/test_smoke.py (1)
1-29: LGTM!test/pg-compat/README.md (1)
1-158: LGTM!test/pg-compat/SPIKE-dbdeployer-pg.md (1)
1-399: LGTM!test/pg-compat/cases/001_scalars.sql (1)
1-2: LGTM!test/pg-compat/cases/002_bytea_json_array.sql (1)
1-2: LGTM!test/pg-compat/tests/test_behaviors.py (1)
1-21: LGTM!test/pg-compat/tests/test_differential.py (1)
1-49: LGTM!test/pg-compat/tests/test_differential_selfcheck.py (1)
1-109: LGTM!test/pg-compat/tests/test_routing_oracle.py (1)
1-59: LGTM!test/infra/infra-dbdeployer-pgsql17-repl/bin/docker-pgsql-post.bash (1)
1-63: LGTM!test/infra/infra-dbdeployer-pgsql17-repl/conf/proxysql/infra-config.sql (1)
59-61: 🎯 Functional CorrectnessEnsure
WHG < RHGfor query-rule ordering
pgsql_query_rulesruns in ascendingrule_idorder, so${WHG}01must stay below${RHG}01to keepSELECT ... FOR UPDATEfrom matching the broader^SELECTrule first.
| permissions: write-all | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add persist-credentials: false to the checkout step.
By default, actions/checkout@v4 persists the GITHUB_TOKEN in .git/config inside the workspace. Since this workflow uploads artifacts and runs containerized commands that could access the workspace, disabling credential persistence eliminates the risk of token exfiltration.
🔐 Suggested fix
- name: Checkout
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: actions/checkout@v4 | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/gh-actions-reusable/ci-pg-compat.yml at line 46, The
checkout step in the reusable CI workflow should disable credential persistence
to avoid leaving the GITHUB_TOKEN in the workspace. Update the
actions/checkout@v4 step to include persist-credentials set to false, so the
token is not written to .git/config and cannot be accessed by later artifact or
container steps.
Source: Linters/SAST tools
| # Wait for ProxySQL admin (MySQL protocol, port 6032) to be reachable. | ||
| while ! docker exec "${PROXY_CONTAINER}" mysql -uadmin -padmin -h127.0.0.1 -P6032 -e 'SELECT 1' >/dev/null 2>&1; do | ||
| echo -n '.' | ||
| sleep 1 | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout to the ProxySQL admin wait loop.
Unlike docker-pgsql-post.bash and toxiproxy-bootstrap.sh (both 60s), this loop has no timeout. If ProxySQL fails to start, the script hangs indefinitely, wasting CI time and producing confusing output.
🔒 Proposed fix
# Wait for ProxySQL admin (MySQL protocol, port 6032) to be reachable.
+MAX_WAIT=60; COUNT=0
while ! docker exec "${PROXY_CONTAINER}" mysql -uadmin -padmin -h127.0.0.1 -P6032 -e 'SELECT 1' >/dev/null 2>&1; do
+ if [ "${COUNT}" -ge "${MAX_WAIT}" ]; then
+ echo " TIMEOUT"
+ exit 1
+ fi
echo -n '.'
sleep 1
+ COUNT=$((COUNT + 1))
done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Wait for ProxySQL admin (MySQL protocol, port 6032) to be reachable. | |
| while ! docker exec "${PROXY_CONTAINER}" mysql -uadmin -padmin -h127.0.0.1 -P6032 -e 'SELECT 1' >/dev/null 2>&1; do | |
| echo -n '.' | |
| sleep 1 | |
| done | |
| # Wait for ProxySQL admin (MySQL protocol, port 6032) to be reachable. | |
| MAX_WAIT=60; COUNT=0 | |
| while ! docker exec "${PROXY_CONTAINER}" mysql -uadmin -padmin -h127.0.0.1 -P6032 -e 'SELECT 1' >/dev/null 2>&1; do | |
| if [ "${COUNT}" -ge "${MAX_WAIT}" ]; then | |
| echo " TIMEOUT" | |
| exit 1 | |
| fi | |
| echo -n '.' | |
| sleep 1 | |
| COUNT=$((COUNT + 1)) | |
| done |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/infra/infra-dbdeployer-pgsql17-repl/bin/docker-proxy-post.bash` around
lines 24 - 28, The ProxySQL admin readiness loop in docker-proxy-post.bash has
no timeout, so it can hang forever if the service never comes up. Update the
wait logic around the docker exec mysql check to track elapsed time and exit
with a clear failure after a bounded limit, matching the timeout behavior used
in docker-pgsql-post.bash and toxiproxy-bootstrap.sh. Keep the existing
readiness probe but add a timeout guard and an error message so the script fails
fast instead of looping indefinitely.
| SQL_TEMPLATE=$(cat ./conf/proxysql/infra-config.sql) | ||
| SQL_CONTENT=$(eval "echo \"${SQL_TEMPLATE}\"") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use SCRIPT_DIR-relative path and replace eval with envsubst.
Two issues on these lines:
-
Relative path inconsistency: Line 12 sources
.envvia${SCRIPT_DIR}/../.env(absolute), but line 31 reads the SQL template via./conf/proxysql/infra-config.sql(relative to CWD). The comment at lines 15–17 notesensure-infras.bashinvokes this script directly — if it doesn'tcdto the infra directory first,catfails. -
evalon file contents (CWE-78):eval "echo \"${SQL_TEMPLATE}\""re-parses the entire SQL template as shell code. The template is trusted and has a caution comment, but any stray$, backtick, or$(...)in a future edit would be silently executed.envsubstwith an explicit variable allowlist expands only the intended placeholders and leaves everything else literal.
♻️ Proposed fix for both issues
-# Pre-process the SQL template.
-SQL_TEMPLATE=$(cat ./conf/proxysql/infra-config.sql)
-SQL_CONTENT=$(eval "echo \"${SQL_TEMPLATE}\"")
+# Pre-process the SQL template: expand only the intended placeholders.
+SQL_CONTENT=$(INFRA_ID="${INFRA_ID}" WHG="${WHG}" RHG="${RHG}" ROOT_PASSWORD="${ROOT_PASSWORD}" \
+ envsubst '${INFRA_ID} ${WHG} ${RHG} ${ROOT_PASSWORD}' \
+ < "${SCRIPT_DIR}/../conf/proxysql/infra-config.sql")If envsubst (from gettext-utils) is unavailable in some environments, a sed-based fallback or a pre-check (command -v envsubst) can be added.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SQL_TEMPLATE=$(cat ./conf/proxysql/infra-config.sql) | |
| SQL_CONTENT=$(eval "echo \"${SQL_TEMPLATE}\"") | |
| SQL_CONTENT=$(INFRA_ID="${INFRA_ID}" WHG="${WHG}" RHG="${RHG}" ROOT_PASSWORD="${ROOT_PASSWORD}" \ | |
| envsubst '${INFRA_ID} ${WHG} ${RHG} ${ROOT_PASSWORD}' \ | |
| < "${SCRIPT_DIR}/../conf/proxysql/infra-config.sql") |
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 31-31: eval is invoked on a variable, parameter expansion, or command-substitution result, which re-parses the value as shell code. If any part of that value is attacker-controlled (arguments, environment, file contents, network output), it allows arbitrary command execution. Do not eval dynamic data: invoke the command directly with proper quoting (e.g. "$cmd" "$arg"), use arrays for argument lists (cmd=(prog --flag "$value"); "${cmd[@]}"), or restrict input to a validated allowlist before running it.
Context: eval "echo "${SQL_TEMPLATE}""
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(eval-on-variable-bash)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/infra/infra-dbdeployer-pgsql17-repl/bin/docker-proxy-post.bash` around
lines 31 - 32, Update the SQL template loading in docker-proxy-post.bash so it
uses the SCRIPT_DIR-relative path instead of a CWD-relative
./conf/proxysql/infra-config.sql reference, since the script may be invoked
directly by ensure-infras.bash. Also replace the eval-based expansion in
SQL_CONTENT with envsubst using an explicit allowlist of expected variables, and
keep the existing logic around SQL_TEMPLATE/SQL_CONTENT so only intended
placeholders are expanded.
Source: Linters/SAST tools
| if [ -z "${INFRA_ID}" ]; then | ||
| export INFRA_ID=$(basename $(dirname $(pwd)) | sed 's/infra-//; s/docker-//') | ||
| fi | ||
| # Final safety: if INFRA_ID is still empty or ".", use a default | ||
| if [ -z "${INFRA_ID}" ] || [ "${INFRA_ID}" = "." ]; then | ||
| export INFRA_ID="dev-$USER" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
INFRA_ID fallback derivation uses dirname incorrectly.
basename $(dirname $(pwd)) returns the name of the parent directory, not the current directory. When the script is invoked from within the infra directory (e.g., .../test/infra/infra-dbdeployer-pgsql17-repl/), this yields infra (the parent dir name), not infra-dbdeployer-pgsql17-repl. After the sed substitutions, INFRA_ID becomes "infra", which is wrong and would produce an incorrect ROOT_PASSWORD and COMPOSE_PROJECT.
If the intent is to derive INFRA_ID from the infra directory name (stripping the infra- prefix), use basename on the current directory directly.
🐛 Proposed fix for INFRA_ID fallback
if [ -z "${INFRA_ID}" ]; then
- export INFRA_ID=$(basename $(dirname $(pwd)) | sed 's/infra-//; s/docker-//')
+ INFRA_ID="$(basename "$(pwd)")"
+ INFRA_ID="${INFRA_ID#infra-}"
+ INFRA_ID="${INFRA_ID#docker-}"
+ export INFRA_ID
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -z "${INFRA_ID}" ]; then | |
| export INFRA_ID=$(basename $(dirname $(pwd)) | sed 's/infra-//; s/docker-//') | |
| fi | |
| # Final safety: if INFRA_ID is still empty or ".", use a default | |
| if [ -z "${INFRA_ID}" ] || [ "${INFRA_ID}" = "." ]; then | |
| export INFRA_ID="dev-$USER" | |
| fi | |
| if [ -z "${INFRA_ID}" ]; then | |
| INFRA_ID="$(basename "$(pwd)")" | |
| INFRA_ID="${INFRA_ID#infra-}" | |
| INFRA_ID="${INFRA_ID#docker-}" | |
| export INFRA_ID | |
| fi | |
| # Final safety: if INFRA_ID is still empty or ".", use a default | |
| if [ -z "${INFRA_ID}" ] || [ "${INFRA_ID}" = "." ]; then | |
| export INFRA_ID="dev-$USER" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 4-4: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 4-4: Quote this to prevent word splitting.
(SC2046)
[warning] 4-4: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/infra/infra-dbdeployer-pgsql17-repl/docker-compose-init.bash` around
lines 3 - 9, The INFRA_ID fallback in docker-compose-init.bash is deriving the
name from the parent directory instead of the current infra directory. Update
the INFRA_ID assignment logic to use the current working directory name directly
in the fallback path, then keep the existing sed cleanup so the infra- prefix is
stripped correctly. Use the INFRA_ID initialization block and its final safety
fallback as the places to adjust.
| finally: | ||
| # Leave no state behind, whether or not the assertions above passed, | ||
| # and use a table name distinct from other behaviors/tests | ||
| # (tests/test_routing_oracle.py's own probe table is "oracle_w") so | ||
| # runs never collide. | ||
| a.exec_simple(f"DROP TABLE IF EXISTS {TABLE}") | ||
| a.close() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Rollback any open transaction before cleanup in the finally block.
If exec_simple raises during a begin()/commit() block (e.g., a broken connection or query error), the connection is left in a non-autocommit transaction. The DROP TABLE IF EXISTS in the finally block will then fail with PostgreSQL's "current transaction is aborted, commands ignored until end of transaction block" error, leaving the table behind. Adding a rollback() before the cleanup DROP resets the transaction state and restores autocommit mode so the DROP succeeds.
🛡️ Proposed fix
finally:
+ # Rollback any open transaction so the cleanup DROP succeeds even
+ # if an exec_simple above failed mid-transaction (PG aborts the
+ # transaction and rejects subsequent commands until END).
+ try:
+ a.rollback()
+ except Exception:
+ pass
# Leave no state behind, whether or not the assertions above passed,
# and use a table name distinct from other behaviors/tests
# (tests/test_routing_oracle.py's own probe table is "oracle_w") so
# runs never collide.
a.exec_simple(f"DROP TABLE IF EXISTS {TABLE}")
a.close()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| finally: | |
| # Leave no state behind, whether or not the assertions above passed, | |
| # and use a table name distinct from other behaviors/tests | |
| # (tests/test_routing_oracle.py's own probe table is "oracle_w") so | |
| # runs never collide. | |
| a.exec_simple(f"DROP TABLE IF EXISTS {TABLE}") | |
| a.close() | |
| finally: | |
| # Rollback any open transaction so the cleanup DROP succeeds even | |
| # if an exec_simple above failed mid-transaction (PG aborts the | |
| # transaction and rejects subsequent commands until END). | |
| try: | |
| a.rollback() | |
| except Exception: | |
| pass | |
| # Leave no state behind, whether or not the assertions above passed, | |
| # and use a table name distinct from other behaviors/tests | |
| # (tests/test_routing_oracle.py's own probe table is "oracle_w") so | |
| # runs never collide. | |
| a.exec_simple(f"DROP TABLE IF EXISTS {TABLE}") | |
| a.close() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/pg-compat/behaviors/transactions.py` around lines 50 - 56, The cleanup
in the `finally` block of `transactions.py` should first reset any failed
transaction state before dropping the test table. Add a `rollback()` on the same
connection object used by `exec_simple` and `begin()`/`commit()` before the
`DROP TABLE IF EXISTS` call, so the connection returns to autocommit mode and
the cleanup can succeed even after an exception. Keep the existing `TABLE`
cleanup and `a.close()` flow, but ensure the rollback happens in the `finally`
path before `exec_simple`.
| psycopg[binary]==3.2.* | ||
| asyncpg==0.30.* | ||
| pytest==8.* | ||
| tomli==2.* |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest versions and security advisories for pinned dependencies
for pkg in psycopg asyncpg pytest tomli; do
echo "=== $pkg ==="
curl -s "https://pypi.org/pypi/$pkg/json" | jq '.info.version'
done
# Check GitHub security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: PIP) {
nodes {
advisory { summary severity publishedAt }
package { name }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}' | jq '.data.securityVulnerabilities.nodes[] | select(.package.name | test("psycopg|asyncpg|pytest|tomli"; "i"))Repository: sysown/proxysql
Length of output: 1982
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== repo files ==='
git ls-files 'test/pg-compat/requirements.txt' 'Dockerfile' 'harness/xfail.py' || true
echo
echo '=== file excerpts ==='
for f in test/pg-compat/requirements.txt Dockerfile harness/xfail.py; do
if [ -f "$f" ]; then
echo "--- $f ---"
nl -ba "$f" | sed -n '1,220p'
fi
done
echo
echo '=== PyPI latest versions ==='
for pkg in psycopg asyncpg pytest tomli; do
printf '%s: ' "$pkg"
curl -fsSL "https://pypi.org/pypi/$pkg/json" | python3 - <<'PY'
import json, sys
data = json.load(sys.stdin)
print(data["info"]["version"])
PY
doneRepository: sysown/proxysql
Length of output: 1953
🌐 Web query:
PyPI latest versions and known security advisories for psycopg, asyncpg, pytest, and tomli as of July 2026; also Python 3.11 tomllib standard library docs
💡 Result:
As of July 8, 2026, the status of the requested Python packages and the standard library tomllib module is as follows: Psycopg Latest Version: The package psycopg (Psycopg 3) is at version 3.3.4 [1][2]. The older psycopg2 is at version 2.9.12 [3]. Security Advisories: There are no known active security advisories for psycopg 3.3.4 [1][2]. For psycopg2, it is recommended to use the standard psycopg2 package rather than psycopg2-binary to ensure it links against system-provided libpq libraries, which receive timely security updates [4]. Asyncpg Latest Version: 0.31.0 [5][6]. Security Advisories: There are no known active vulnerabilities for current versions of asyncpg [5]. Past vulnerabilities, such as PYSEC-2020-24, were resolved in version 0.21.0 [7]. Pytest Latest Version: 9.1.1 (released June 19, 2026) [8][9]. Security Advisories: A security vulnerability, CVE-2025-71176, involved insecure temporary directory handling in versions through 9.0.2 [10]. This was patched in version 9.0.3, released April 7, 2026 [9][11]. Tomli Latest Version: 2.4.1 (released March 25, 2026) [12][13]. Security Advisories: No active security advisories were identified for recent versions of tomli [14]. Python 3.11 tomllib The tomllib module was added to the Python standard library in version 3.11 via PEP 680 to provide read-only support for parsing TOML files [15][16]. It is a read-only interface and does not support writing TOML [15]. It provides functions such as tomllib.load for file-like objects and tomllib.loads for strings, raising a TOMLDecodeError for invalid documents [15][17]. Official documentation for this module can be found in the Python standard library documentation [15].
Citations:
- 1: https://deps.dev/pypi/psycopg
- 2: https://secure.software/pypi/packages/psycopg
- 3: https://pypi.org/project/psycopg2/
- 4: https://cve.optibot.re/blog/postgresql-database-dependency-security-2026
- 5: https://secure.software/pypi/packages/asyncpg
- 6: https://data.safetycli.com/packages/pypi/asyncpg/changelog
- 7: https://github.com/pypa/advisory-database/blob/main/vulns/asyncpg/PYSEC-2020-24.yaml
- 8: https://pypi.org/project/pytest/
- 9: https://github.com/pytest-dev/pytest/releases
- 10: https://nvd.nist.gov/vuln/detail/CVE-2025-71176
- 11: https://pytest.org/en/stable/changelog.html
- 12: https://pypi.org/project/tomli/2.4.1/
- 13: https://pypi.org/project/tomli/1.2.0/
- 14: https://github.com/hukkin/tomli?tab=readme-ov-file
- 15: https://docs.python.org/3/library/tomllib.html
- 16: https://github.com/python/peps/blob/main/peps/pep-0680.rst
- 17: https://github.com/python/cpython/blob/3.11/Doc/library/tomllib.rst
Bump pytest off the vulnerable 8.x line
pytest==8.* misses the 9.0.3 fix for CVE-2025-71176; move this pin to a patched 9.x release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/pg-compat/requirements.txt` around lines 1 - 4, The dependency pin for
pytest is still on the vulnerable 8.x line, so update the test/pg-compat
requirements entry to a patched 9.x release that includes the CVE-2025-71176
fix. Adjust the pytest version specifier in the requirements file while keeping
the existing pins for psycopg, asyncpg, and tomli unchanged.
…suite Reusable half of CI-pg-compat (caller lands on v3.0 via PR #5903's stack). Inline PROXYSQL31 debug build, pg-compat infra via ensure-infras, non-gating pytest run (discovery phase, spec 2.1), junit artifact, always() teardown. MERGE ORDER: this must merge to GH-Actions BEFORE the caller merges to v3.0.
…-polyglot-foundation
…-polyglot-foundation # Conflicts: # .gitignore
…-polyglot-foundation
|
|
Superseded by #6020, which combines this PR with the rest of the SP-1..SP-3 stack on top of current These three PRs are a stacked chain (5894 → Leaving this open for now so the review threads here stay reachable; it should be closed once #6020 merges. |
|
superseded by #6020 |
…est stack SP-1..SP-3) into v3.0 # Conflicts: # test/tap/tests/Makefile
…cess timeout, pytest CVE Addresses CodeRabbit findings on sysown#5903 and sysown#5910. diff.py: only-targets/skip-targets are documented as glob patterns, but were matched with exact set membership, so the documented "only-targets: proxy_native_*" matched no target and silently skipped the entire case -- a vacuous pass. Match with fnmatchcase; a name with no metacharacter still compares exactly. Pinned by a new infra-free unit test. transactions.py: if a statement raised between begin() and commit(), the finally block ran DROP TABLE on a session left in PostgreSQL's aborted- transaction state, so the drop failed, the table leaked, and the cleanup error masked the original failure. Roll back (best-effort) first, and close the connection even if the drop fails. _subproc.py: an uncaught subprocess.TimeoutExpired turned a hung driver into a pytest ERROR during teardown and discarded the partial output that identifies where it hung. Report it as a failure with the captured streams, decoding defensively since TimeoutExpired yields bytes even under text=True. The timeout is now overridable via PGCOMPAT_BEHAVIOR_TIMEOUT. requirements.txt: GHSA-6w46-j5rx-g56g (insecure tmpdir handling) affects every pytest before 9.0.3, so the "8.*" pin could not pick up the fix. The suite uses only stable APIs and the 3.11 base image satisfies pytest 9's Python >=3.10 requirement.
…sted creds Addresses CodeRabbit and Gemini findings on sysown#5903 and sysown#5910. ci-pg-compat.yml: the job builds and tests but never pushes, yet checkout persisted its write-all token into .git/config where every later step -- including the third-party driver images this suite builds and runs -- could read it. Set persist-credentials: false. docker-proxy-post.bash: the ProxySQL admin wait loop had no timeout. docker-compose-init.bash re-execs itself under `timeout`, but ensure-infras.bash's reconfigure path calls this script directly, where the loop would hang the run instead of failing it; it now gives up after PROXY_WAIT_SECONDS (default 120) and dumps container logs. The SQL template was expanded with eval-echo, running the whole file through the shell, and was read via a cwd-relative path; use envsubst with an explicit variable list and resolve the template relative to SCRIPT_DIR. Verified to produce byte-identical output to the previous eval for the current template. Dockerfile: verify the downloaded pgjdbc jar against a pinned SHA-256, so a substituted artifact fails the build. The recorded digest is that of the jar whose SHA-1 matches Maven Central's published digest (Central does not publish .sha256 for this artifact). go/java/node behaviors: the scaffold comments claimed only `connect` was implemented and the other three exited 2 as "not implemented", contradicting code where all four behaviors are complete. The Go errNotImplemented sentinel and the Node NotImplementedError class were never raised, so their dispatch branches were dead; removed along with the now-unused errors import.
Addresses CodeRabbit findings filed against the plan documents on sysown#5894 and sysown#5903. Each of these described behaviour the shipped code either implements differently or had to correct. SP-1 plan: the sketched timestamptz row used AT TIME ZONE 'UTC' (OID 1114, i.e. timestamp WITHOUT time zone) and the binary assertion checked only the OID. Both are corrected to match the implemented test, which uses a real timestamptz and compares the DataRow payload byte-for-byte. SP-2 plan: the env contract named a single PGCOMPAT_BACKEND_PORT, which cannot address the dbdeployer layout where all three nodes share one host and differ only by port. The implementation deliberately publishes a _HOST/_PORT pair per node (harness/targets.py records this explicitly); the plan and its sample snippets now say the same. The diff.py sketch also gains the fnmatch-based target filtering that the documented "only-targets: proxy_native_*" example requires.
1 participant
Summary
SP-2 of the phased PostgreSQL protocol testing effort (spec + plans in #5894): the polyglot test-harness foundation — new backend infra, fault-injection layer, and a containerized pytest suite implementing the differential-transparency and routing-oracle techniques adapted from pgdog/pgcat/PgBouncer.
What's in it
Infra (
test/infra/infra-dbdeployer-pgsql17-repl/) — the repo's first dbdeployer-PostgreSQL infra:dbdeployer unpack --provider=postgresql), single container, primary + 2 streaming replicas (ports 16710–12),pg_stat_statementspreloaded on all nodes. A validated spike doc (test/pg-compat/SPIKE-dbdeployer-pg.md) records the exact commands and the PG-provider quirks (e.g.--base-port/-csilently ignored → config injected post-deploy).pgsql_replication_hostgroupspath: the monitor demotes replicas viapg_is_in_recovery()through Toxiproxy (verified live: demotion in ~6–8s, reads → replicas, writes → primary).Harness (
test/pg-compat/, containerized pytest):proxy_native_*pair (backend-protocol axis, spec §2.2) auto-skips until PgSQL native backend protocol: replace libpq on the data path (connect/auth/TLS, simple query, COPY, extended query via stmt pipeline, Describe cache) #5882 merges — lights up with zero code changes. Includes divergence + file-pipeline self-checks (the engine provably detects injected divergence and provably executes real statements).pg_stat_statementsper backend as ground truth for where queries landed (reads: primary=0, replicas sum exact; writes: primary-only), with a write-pin self-check.xfail.toml): discovery-phase machinery (spec §2.1) — exact-nodeid xfail with xpass reporting, unlisted failures never swallowed (all three semantics proven), plus a[[finding]]section for verified divergences.CI-pg-compat.ymlcaller (nightly cron +pg-compatPR label + dispatch) with a staged reusable — non-gating (|| trueon the test step only; build/infra failures still go red), junit artifact always uploaded.[[finding]]catalogue)ProxySQL imposes
client_encoding=UTF8on backend connections instead of inheriting the server default: direct sessions to a SQL_ASCII database default to SQL_ASCII; via-ProxySQL sessions report UTF8. The harness pins UTF8 on all targets (documented apples-to-apples); the session-default asymmetry is recorded intest/pg-compat/xfail.tomlas a candidate issue. (This adds to #5899 / #5900 from SP-1.)The caller references
ci-pg-compat.yml@GH-Actions. Perdoc/GH-Actions/README.md, the staged reusable (.github/workflows/gh-actions-reusable/ci-pg-compat.yml) must land on theGH-Actionsbranch first, then this PR's caller onv3.0— documented in both file headers.Testing
Full suite green on isolated infra (
ensure-infras.bash+run-pg-compat.bash): 16 passed + 2 skipped (the native-axis skips), run repeatedly with zero state drift (config snapshot/restore verified, including the dormant native-var restore proven by forced-branch simulation). Infra verified end-to-end: replication f/t/t,pg_stat_statementson all nodes, idempotent re-provisioning, clean teardown.Tracked follow-ups (non-blocking, from reviews)
ensure-infras.bash's fast path (restart policy alone insufficient).|| true, tighten the reusable'spermissions: write-allto least-privilege.ON_ERROR_STOP+ infra-tagged rule DELETEs for the older reference PG infra.Summary by CodeRabbit