Skip to content

Validate cloud postgres list --filter keys and values - #626

Open
sdairs wants to merge 1 commit into
feat/615-postgres-delete-forcefrom
fix/603-postgres-filter-validation
Open

Validate cloud postgres list --filter keys and values#626
sdairs wants to merge 1 commit into
feat/615-postgres-delete-forcefrom
fix/603-postgres-filter-validation

Conversation

@sdairs

@sdairs sdairs commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Fixes #603.

Stacked PR: base is feat/615-postgres-delete-force, not main.

What was broken

cloud postgres list --filter matched keys with a catch-all _ => true, so any key outside {state, region, name, provider} — including isPrimary, which the listing shows as the Primary column — silently returned the whole unfiltered list with exit 0. A filter with an empty value (state=) or no = at all was ignored the same way. The state and provider predicates also compared format!("{:?}", ..) Debug forms against the wire value, so an Unknown(..) state could never match.

What changed

  • --filter is parsed at clap time by a value_parser into a closed PostgresListFilter enum. An unknown key, a missing =, an empty/whitespace-only value, or a non-boolean isPrimary is now a usage error (exit 2) whose message lists the valid keys — and no API request is issued.
  • isPrimary is a supported key (true/false, plus the yes/no the Primary column renders).
  • state and provider are compared against the serde wire value via Display, case-insensitively, so multi-word states and Unknown(..) values match correctly. region/name match exactly; keys themselves are case-insensitive.
  • Per CLAUDE.md, an absent (API-omitted) response field matches no filter value, and all filters must hold for an item to be kept.
  • README documents the supported keys, the matching rules and the exit-2 behaviour.

cloud service list --filter is not affected: it forwards server-side resource-tag filters (tag:env=production) to the API as query parameters and shares none of this code, so client-side key validation there would wrongly reject valid server-side syntax.

Tests

  • Clap parse tests next to the command definition: the accepted forms (including case-insensitive keys and yes/no), and one test per rejection asserting ErrorKind::ValueValidation and exit_code() == 2.
  • Inline apply_filter tests: wire-value comparison for state/provider (including Restoring_backup and Unknown(..)), absent fields matching nothing, multi-filter conjunction, and the empty-filter passthrough.
  • tests/cli_request_shape_test.rs: subprocess + wiremock tests pinning exit 2 with an expect(0) mock for invalid filters (no API call, no stdout), and the client-side filtering results for isPrimary, state=restoring_backup, region and a non-matching name.

Verified with cargo fmt --all, cargo clippy --workspace --all-targets -- -D warnings, and cargo test -p clickhousectl (all green). The clickhouse-cloud-api crate is untouched.

🤖 Generated with Claude Code

@sdairs
sdairs force-pushed the fix/603-postgres-filter-validation branch from 1825be7 to c99ecc8 Compare August 28, 2026 14:46
@sdairs sdairs linked an issue Aug 28, 2026 that may be closed by this pull request
@sdairs
sdairs force-pushed the fix/603-postgres-filter-validation branch from c99ecc8 to 30c98b8 Compare August 28, 2026 19:42
@sdairs
sdairs force-pushed the fix/603-postgres-filter-validation branch from 30c98b8 to cf970f3 Compare September 1, 2026 15:05
`cloud postgres list --filter` fell through a catch-all `_ => true`, so any
key outside {state, region, name, provider} — including `isPrimary`, which is
shown as the `Primary` column — silently returned the whole unfiltered list
with exit 0. An empty value and a filter with no `=` were ignored the same way,
and the `state`/`provider` predicates compared `format!("{:?}", ..)` Debug
forms against the wire value, so an `Unknown(..)` state matched nothing.

`--filter` now parses at clap time into a closed `PostgresListFilter` enum via
a `value_parser`, so an unknown key, a missing `=`, an empty value or a
non-boolean `isPrimary` is a usage error (exit 2) that lists the valid keys and
issues no API request. `isPrimary` joins the supported keys, `state` and
`provider` compare against the serde wire value (`Display`), and an absent
response field still matches no filter value.

`cloud service list --filter` is unaffected: it forwards server-side
resource-tag filters (`tag:env=production`) to the API as query parameters and
shares none of this code.

Tests: clap parse coverage for the accepted forms and each rejection (asserting
ErrorKind::ValueValidation and exit code 2), inline `apply_filter` tests for
wire-value comparison, absent fields and multi-filter conjunction, and wiremock
subprocess tests that pin exit 2 with no API call for invalid filters and the
client-side filtering results for valid ones.

Fixes #603

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

@iskakaushik iskakaushik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

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.

cloud postgres list --filter silently ignores unknown/empty keys

2 participants