Skip to content

Stop depending on Postgres get echoing credentials#307

Open
sdairs wants to merge 2 commits into
mainfrom
issue-306-postgres-credential-echo
Open

Stop depending on Postgres get echoing credentials#307
sdairs wants to merge 2 commits into
mainfrom
issue-306-postgres-credential-echo

Conversation

@sdairs

@sdairs sdairs commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

The Managed Postgres API stops returning the superuser password and connectionString outside the create and password-reset responses on July 31, 2026. This PR removes our dependence on the get endpoint echoing credentials, ahead of the 0.4.0 release — covering steps 1 and 2 of #306. Step 3 (model/snapshot updates) waits for the updated upstream spec and follows the standard drift-remediation flow.

Changes

  • postgres_cdc_test.rs: capture username/password from the create response (the only surface guaranteed to keep returning them), parse port/database from the create response's connection string (default 5432/postgres), and connect to Postgres from discrete parts (hostname from the polled get + saved credentials) instead of the get-echoed connection string. Dropped the get-response connection-string assertion; added create-response credential assertions.
  • integration_postgres_test.rs: assert username/password/connectionString are non-empty on the create response, and drop the connection-string assertion on the polled get (which inverts on July 31).
  • CLI postgres get: no longer prints the echoed connection string (it was already guarded by an is-empty check, so it would have silently disappeared; hostname and username remain in the rendered output). Credentials are available from postgres create and postgres reset-password.

Both test changes are safe against the old and new API behavior.

Testing

  • cargo test --workspace — all pass (live-cloud integration tests remain #[ignore]d but compile).
  • cargo clippy --workspace --all-targets -- -D warnings — clean.
  • cargo fmt --all --check — clean.

Part of #306

🤖 Generated with Claude Code

The Managed Postgres API stops returning the superuser password and
connectionString outside the create and password-reset responses on
July 31, 2026. Source credentials from the create response instead:

- postgres_cdc_test: capture username/password and parse port/database
  from the create response's connection string, connect to Postgres
  from discrete parts (get's hostname + saved credentials), and drop
  the get-response connection-string assertion.
- integration_postgres_test: assert credentials on the create response
  and drop the connection-string assertion on the polled get.
- CLI `postgres get`: stop printing the echoed connection string.

Both test changes are safe against the old and new API behavior. Model
changes to PostgresService land later with the standard drift
remediation once the upstream spec is published.

Part of #306

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 8fdf24e. Configure here.

Comment thread crates/clickhouse-cloud-api/tests/clickpipes/postgres_cdc_test.rs
Addresses Bugbot review: pg_port/pg_database silently fell back to
defaults if the create response's connection string was empty.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sdairs
sdairs had a problem deploying to cloud-integration July 22, 2026 18:10 — with GitHub Actions Failure
@sdairs

sdairs commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the Bugbot finding in 414367f: the CDC test now asserts the create response's connection string is non-empty before parsing port/database from it, so a missing value fails at provision time instead of silently falling back to 5432/postgres.

The Cloud integration tests failure is not caused by this PR: ClickPipe creation returned Api { status: 500, message: "Internal error" }. The scheduled runs on main failed at the exact same step with the identical error on July 17 (29553927900) and July 20 (29716642475), while July 21–22 runs passed — it's an intermittent upstream API failure. Notably, the direct Postgres connection in the same test run succeeded using the credentials captured from the create response, which is exactly the code path this PR changed.

@sdairs

sdairs commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

The re-run on 414367f reproduced the same failure: ClickPipe create → Api { status: 500, message: "Internal error" } (~3 min after the request), with every prior step passing — including the direct Postgres connection built from the create-response credentials. Four occurrences now (nightlies July 17 & 20, both PR runs today), all identical, while the July 21–22 morning runs passed. This looks like an upstream ClickPipes API issue rather than anything in this PR; worth flagging to the ClickPipes team if it persists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Managed Postgres API stops returning credentials outside create/password-reset on July 31, 2026 — fix before 0.4.0

1 participant