Skip to content

Commit be4123e

Browse files
authored
Merge pull request #297 from ClickHouse/codex/issue-295-openapi-drift-analyzer
Refit OpenAPI drift checks around shared syn analyzer
2 parents 1ba0b8a + 7b68dc1 commit be4123e

23 files changed

Lines changed: 3650 additions & 2624 deletions

.github/workflows/openapi-drift.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ jobs:
1919
with:
2020
persist-credentials: false
2121

22+
- name: Install Rust toolchain
23+
uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable @ 2026-03-27
24+
25+
- name: Cache Rust build
26+
uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1
27+
2228
- name: Check for drift and create issue
2329
env:
2430
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/test-cloud-api.yml

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ on:
55
pull_request:
66
paths:
77
- "crates/clickhouse-cloud-api/**"
8+
- "crates/clickhouse-openapi-analyzer/**"
9+
- "scripts/check-openapi-drift.py"
10+
- "scripts/tests/**"
11+
- "Cargo.toml"
812
- "Cargo.lock"
913
- ".github/workflows/test-cloud-api.yml"
1014

@@ -24,10 +28,13 @@ jobs:
2428
uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable @ 2026-03-27
2529

2630
- name: Build
27-
run: cargo build -p clickhouse-cloud-api
31+
run: cargo build -p clickhouse-cloud-api -p clickhouse-openapi-analyzer
2832

2933
- name: Clippy
30-
run: cargo clippy -p clickhouse-cloud-api -- -D warnings
34+
run: cargo clippy -p clickhouse-cloud-api -p clickhouse-openapi-analyzer --all-targets -- -D warnings
3135

3236
- name: Test
33-
run: cargo test -p clickhouse-cloud-api
37+
run: cargo test -p clickhouse-cloud-api -p clickhouse-openapi-analyzer
38+
39+
- name: Test drift report rendering
40+
run: python3 -m unittest discover -s scripts/tests -p 'test_*.py'

AGENTS.md

Lines changed: 38 additions & 45 deletions
Large diffs are not rendered by default.

Cargo.lock

Lines changed: 18 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@
22
members = [
33
"crates/clickhousectl",
44
"crates/clickhouse-cloud-api",
5+
"crates/clickhouse-openapi-analyzer",
56
]
67
resolver = "3"

crates/clickhouse-cloud-api/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ chrono = { version = "0.4", features = ["serde"] }
3131
[dev-dependencies]
3232
aws-config = { version = "1.8.16", features = ["credentials-login"] }
3333
aws-credential-types = "1.2.14"
34+
clickhouse-openapi-analyzer = { path = "../clickhouse-openapi-analyzer" }
3435
# Disable default features on aws-sdk-* crates to drop the legacy `rustls` feature,
3536
# which pulls in vulnerable rustls 0.21 / rustls-webpki 0.101 (GHSA / RUSTSEC).
3637
# `default-https-client` gives us the modern rustls-aws-lc-based HTTP client.

crates/clickhouse-cloud-api/README.md

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ Typed Rust client for the [ClickHouse Cloud API](https://clickhouse.com/docs/en/
1212
| `src/models.rs` | Request/response types matching the OpenAPI spec |
1313
| `src/error.rs` | Error types (`Http`, `Json`, `Api`) |
1414
| `clickhouse_cloud_openapi.json` | Checked-in copy of the spec (used by tests) |
15-
| `tests/spec_coverage_test.rs` | Validates client methods, model types, and field optionality match the spec |
15+
| `tests/spec_coverage_test.rs` | Thin snapshot/live-spec consumer of the shared drift analyzer |
16+
| `../clickhouse-openapi-analyzer` | Canonical Rust/OpenAPI parsing, comparison, report, and exemptions |
1617

1718
### Field optionality
1819

1920
The OpenAPI spec uses two conventions for marking fields required vs optional:
2021

2122
- **Schemas with a `required` array** (newer/beta endpoints) use standard OpenAPI semantics.
2223
- **Schemas without `required`** (GA/legacy endpoints) treat fields whose description starts with `"Optional"` as optional. Everything else is implicitly required.
24+
- **Known partial `required` arrays** use the union of that array and the description heuristic, as configured by the analyzer.
2325

2426
Additional rules:
2527

@@ -41,6 +43,9 @@ python3 scripts/resolve-field-requirements.py
4143
# Regenerate the DEPRECATED_FIELDS constant from the snapshot
4244
python3 scripts/regenerate-deprecated-fields.py
4345

46+
# Regenerate the BETA_OPERATIONS constant from the snapshot
47+
python3 scripts/regenerate-beta-lists.py
48+
4449
# Check for drift between the live spec and the library (dry run)
4550
python3 scripts/check-openapi-drift.py --dry-run
4651
```
@@ -51,9 +56,10 @@ Field optionality is maintained by hand — edit `models.rs` directly when the d
5156

5257
```bash
5358
cargo test -p clickhouse-cloud-api # all tests
54-
cargo test --test spec_coverage_test # spec coverage + field optionality only
55-
cargo test --test client_test # wiremock-based client tests
56-
cargo test --test models_test # serde round-trip tests
59+
cargo test -p clickhouse-openapi-analyzer # analyzer fixtures + executable parity
60+
cargo test -p clickhouse-cloud-api --test spec_coverage_test # shared snapshot report
61+
cargo test -p clickhouse-cloud-api --test client_test # wiremock client tests
62+
cargo test -p clickhouse-cloud-api --test models_test # serde round trips
5763
```
5864

5965
Live-API lifecycle suites are `#[ignore]`d by default (they provision real resources):
@@ -79,17 +85,24 @@ cargo test --test clickpipe_smoke_test -- --ignored --nocapture # creat
7985

8086
All require `CLICKHOUSE_CLOUD_API_KEY`, `CLICKHOUSE_CLOUD_API_SECRET`, `CLICKHOUSE_CLOUD_TEST_ORG_ID`, `CLICKHOUSE_CLOUD_TEST_PROVIDER`, and `CLICKHOUSE_CLOUD_TEST_REGION` in the environment, and are wired into the scheduled `Cloud Integration` GitHub Actions workflow. The ClickPipes E2E suites additionally need AWS credentials and an `eu-west-1` region quota; `clickpipe_smoke_test` reads a pre-provisioned service ID from `CLICKHOUSE_CLOUD_TEST_CLICKPIPE_SERVICE_ID`.
8187

82-
The `spec_coverage_test` suite checks three things against the checked-in spec:
83-
84-
1. Every OpenAPI operation has a matching `pub async fn` in `client.rs`
85-
2. Every OpenAPI schema has a matching `pub struct`/`pub enum` in `models.rs`
86-
3. Every field's `Option<T>` vs `T` matches the spec's required/optional semantics
87-
4. `DEPRECATED_FIELDS` matches the spec's `deprecated: true` fields (request- and response-side), and each one carries the `#[cfg(feature = "deprecated-fields")]` marker in `models.rs`
88-
89-
There are also `#[ignore]`d variants that run the same checks against the live spec.
88+
`spec_coverage_test` sends the checked-in sources and snapshot through the
89+
private `clickhouse-openapi-analyzer` crate. That same analyzer powers the
90+
scheduled live-spec issue, so operation, model, field, optionality, beta,
91+
deprecation, enum, snapshot, and stale-exemption findings share one
92+
implementation. The single ignored test runs the same report against the live
93+
spec.
9094

9195
### Optionality exemptions
9296

93-
Occasionally the spec marks a field as required but the API actually treats it as optional (e.g. sending an empty string triggers a `400 BAD_REQUEST`). In these cases we override the field to `Option<T>` in `models.rs` and add the field to the `OPTIONALITY_EXEMPTIONS` list in `spec_coverage_test.rs`.
94-
95-
The test prints a `NOTE:` line for every exemption that fires, and **fails** if an exemption becomes stale (i.e. the spec was corrected upstream and the override is no longer needed). To add a new exemption, add a `("RustStructName", "specFieldName")` entry to the constant with a comment explaining why.
97+
Occasionally the spec cannot be followed literally because verified API
98+
behavior differs. All such policy lives in
99+
`crates/clickhouse-openapi-analyzer/src/config.rs`, including optionality,
100+
extra-field, deprecated-field, extra-enum-value, non-OpenAPI-method, partial
101+
required-schema, and unsupported-enum configuration.
102+
103+
Add an exemption only for a deliberate runtime behavior and document why the
104+
spec cannot be followed. New unsupported-enum acknowledgements also require a
105+
tracking issue. The analyzer reports stale field/enum exemptions and vanished
106+
unsupported locations so obsolete entries are removed during normal drift
107+
remediation. See the repository `AGENTS.md` for exact key formats and the full
108+
remediation and verification procedure.

crates/clickhouse-cloud-api/src/meta.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
//! python3 scripts/regenerate-beta-lists.py
1515
//! ```
1616
//!
17-
//! The `beta_operations_match_spec` test in `tests/spec_coverage_test.rs` fails
18-
//! if this list drifts from the spec.
17+
//! The shared OpenAPI analyzer reports drift if this list differs from the
18+
//! snapshot or live spec.
1919
2020
/// Snake-case operation IDs (matching [`crate::client::Client`] method names)
2121
/// that the OpenAPI spec marks Beta via `x-badges`.
@@ -95,9 +95,8 @@ pub fn is_beta_operation(name: &str) -> bool {
9595
/// python3 scripts/regenerate-deprecated-fields.py
9696
/// ```
9797
///
98-
/// The `deprecated_fields_match_spec` test in `tests/spec_coverage_test.rs`
99-
/// fails if this list drifts from the spec, and `deprecated_fields_hidden`
100-
/// fails if a field here lacks the `#[cfg(feature = "deprecated-fields")]`
98+
/// The shared OpenAPI analyzer reports drift if this list differs from the
99+
/// spec or if a field here lacks the `#[cfg(feature = "deprecated-fields")]`
101100
/// marker in `models.rs` (or vice versa).
102101
pub const DEPRECATED_FIELDS: &[(&str, &str)] = &[
103102
("ApiKey", "roles"),

0 commit comments

Comments
 (0)