Skip to content

Fix OpenAPI drift: 36 gaps between live spec and library (#298)#299

Merged
sdairs merged 4 commits into
mainfrom
fix/openapi-drift-298
Jul 17, 2026
Merged

Fix OpenAPI drift: 36 gaps between live spec and library (#298)#299
sdairs merged 4 commits into
mainfrom
fix/openapi-drift-298

Conversation

@sdairs

@sdairs sdairs commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Remediates all 36 actionable OpenAPI drift findings from #298. Library-only change; no new CLI surface.

API library (clickhouse-cloud-api)

  • New client method: click_pipe_schema_discovery (POST /v1/organizations/{orgId}/services/{serviceId}/clickpipes/schemaDiscovery, Beta)
  • 4 new model types: ClickPipeSchemaDiscoveryField, ClickPipeSchemaDiscoveryRequest, ClickPipeSchemaDiscoveryResponse, ClickPipeSchemaDiscoverySource
  • New shared enum: AutoscalingMode (Vertical/Horizontal) used by 7 structs
  • 10 new struct fields:
    • autoscalingMode on Service, ServicePostRequest, ServiceReplicaScalingPatchRequest, ServiceScalingPatchResponse, ScalingScheduleBaseConfig, ScalingScheduleEntry, ScalingScheduleEntryRequest
    • serverId on ClickPipeMySQLSource, ClickPipeMutateMySQLSource, ClickPipePatchMySQLSource
  • 5 removed extra fields: replicaMemoryGb from 4 structs, numReplicas + replicaMemoryGb from ScalingScheduleEntry
  • 9 new enum values: ca-central-1 added to 8 region enums; stopped added to PgStateProperty
  • Regenerated BETA_OPERATIONS (42→43, adds click_pipe_schema_discovery)
  • Replaced vendored OpenAPI snapshot with live spec (63→64 ops, 306→310 schemas)

Analyzer (clickhouse-openapi-analyzer)

  • Added optionality exemption ("ServicePostRequest", "autoscalingMode") — server defaults to vertical when omitted
  • Removed stale optionality exemption ("ServicePostRequest", "replicaMemoryGb") — field no longer exists

CLI (clickhousectl)

Compile fixes only (no new commands/flags):

  • Added autoscaling_mode/server_id to struct constructions in commands.rs
  • Removed replica_memory_gb from struct constructions in commands.rs

Tests

  • 10 new model tests: AutoscalingMode round-trip + catch-all, PgStateProperty::Stopped, ServiceRegion/ByocConfigRegionid ca-central-1, schema discovery response/request/field serde, MySQL server_id
  • 1 new wiremock client test: click_pipe_schema_discovery_kafka
  • Updated existing tests for removed/renamed fields

Closes #298.

Verification

  • cargo test -p clickhouse-cloud-api -p clickhouse-openapi-analyzer — all pass
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — clean
  • python3 -m unittest discover -s scripts/tests -p 'test_*.py' — 3/3 pass
  • python3 scripts/check-openapi-drift.py --dry-runActionable drift: 0

Remediate all 36 actionable drift findings from issue #298.

API library changes:
- Add click_pipe_schema_discovery client method (POST /clickpipes/schemaDiscovery)
- Add 4 new model types: ClickPipeSchemaDiscoveryField, Request, Response, Source
- Add shared AutoscalingMode enum (vertical/horizontal) used by 7 structs
- Add autoscalingMode field to Service, ServicePostRequest,
  ServiceReplicaScalingPatchRequest, ServiceScalingPatchResponse,
  ScalingScheduleBaseConfig, ScalingScheduleEntry, ScalingScheduleEntryRequest
- Add serverId field to ClickPipeMySQLSource, ClickPipeMutateMySQLSource,
  ClickPipePatchMySQLSource
- Remove extra fields: replicaMemoryGb from ServicePostRequest,
  ServiceReplicaScalingPatchRequest, ScalingScheduleEntryRequest;
  numReplicas + replicaMemoryGb from ScalingScheduleEntry
- Add ca-central-1 to 8 region enums
- Add stopped to PgStateProperty enum
- Regenerate BETA_OPERATIONS (adds click_pipe_schema_discovery)
- Replace vendored OpenAPI snapshot with live spec

Analyzer config changes:
- Add optionality exemption for ServicePostRequest.autoscalingMode
- Remove stale optionality exemption for ServicePostRequest.replicaMemoryGb

CLI compile fixes (no new surface):
- Add autoscaling_mode/server_id to struct constructions in commands.rs
- Remove replica_memory_gb from struct constructions in commands.rs

Tests:
- Add focused tests for new models, AutoscalingMode, ca-central-1, stopped,
  server_id, and click_pipe_schema_discovery wiremock
- Update existing tests for removed/renamed fields
@sdairs
sdairs requested review from iskakaushik and rndD as code owners July 16, 2026 18:18
@sdairs
sdairs temporarily deployed to cloud-integration July 16, 2026 18:18 — with GitHub Actions Inactive

@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 8b144b8. Configure here.

Comment thread crates/clickhousectl/src/cloud/commands.rs
sdairs and others added 3 commits July 16, 2026 22:00
…291)

Replace eight hand-maintained KNOWN_* constant lists in the CLI with
pub const VALUES on the library enums they validate against. The
analyzer now verifies that any enum declaring a VALUES const has it
exactly equal (as a set) to its non-catch-all wire values, producing
FindingKind::EnumValuesMismatch on mismatch.

Library (models.rs):
- Add pub const VALUES to 9 enums: PgHaType, PgProvider, PgVersion,
  ServicePatchRequestReleasechannel, ServicePostRequestCompliancetype,
  ServicePostRequestProfile, ServicePostRequestProvider,
  ServicePostRequestRegion, ServicePostRequestReleasechannel.
- VALUES on ServicePostRequestRegion includes europe-west2 and
  ca-central-1, fixing existing drift where the CLI rejected valid
  regions.

Analyzer (clickhouse-openapi-analyzer):
- rust_inventory.rs: parse Item::Impl blocks in models.rs to extract
  VALUES const string literals into EnumInfo.values_const.
- report.rs: add FindingKind::EnumValuesMismatch, bump
  REPORT_SCHEMA_VERSION from 1 to 2.
- compare.rs: compare_enum_values_consts checks every enum with a
  VALUES const against its wire values (set equality); emits
  EnumValuesMismatch with details[missing] and details[extra].

Python issue rendering:
- check-openapi-drift.py: schema version check -> 2, summary table
  row and simple_sections entry for enum_values_mismatch.
- test_check_openapi_drift.py: synthetic finding, assertion, and
  schema version rejection test updated.

CLI (clickhousectl):
- commands.rs: delete KNOWN_PROVIDERS, KNOWN_REGIONS,
  KNOWN_RELEASE_CHANNELS, KNOWN_COMPLIANCE_TYPES, KNOWN_PROFILES;
  use Enum::VALUES in parse_serde_enum calls.
- postgres.rs: delete KNOWN_PG_PROVIDERS, KNOWN_PG_VERSIONS,
  KNOWN_PG_HA_TYPES; use Enum::VALUES in parse_serde_enum and
  PossibleValuesParser calls.

Closes #291.
An impl block that lexically precedes its enum declaration was silently
skipped, disabling the VALUES drift guard for that enum. Trait impls
with an associated VALUES const were also misattributed. Address both
review findings and replace the vacuous non-enum-impl test assertion.

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

Derive CLI value lists from library enums with analyzer drift guard
@sdairs
sdairs temporarily deployed to cloud-integration July 17, 2026 13:48 — with GitHub Actions Inactive
Comment on lines +447 to +450
let Some(values_const) = &info.values_const else {
continue;
};
let rust_values = &info.values;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium src/compare.rs:447

compare_enum_values_consts reports every enum value as missing when VALUES is initialized from a non-literal source. Because string_array returns an empty BTreeSet for initializers like pub const VALUES: &[&str] = SHARED_VALUES, lines 451–452 treat that empty parse result as the constant's real value, so rust_values.difference(values_const) flags every variant as missing and the analyzer fails with false drift even though the constant is correct at runtime. Consider skipping the comparison when values_const is empty (or distinguishing a genuinely empty constant from an unparseable one).

Suggested change
let Some(values_const) = &info.values_const else {
continue;
};
let rust_values = &info.values;
let Some(values_const) = &info.values_const else {
continue;
};
if values_const.is_empty() {
continue;
}
let rust_values = &info.values;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @crates/clickhouse-openapi-analyzer/src/compare.rs around lines 447-450:

`compare_enum_values_consts` reports every enum value as missing when `VALUES` is initialized from a non-literal source. Because `string_array` returns an empty `BTreeSet` for initializers like `pub const VALUES: &[&str] = SHARED_VALUES`, lines 451–452 treat that empty parse result as the constant's real value, so `rust_values.difference(values_const)` flags every variant as missing and the analyzer fails with false drift even though the constant is correct at runtime. Consider skipping the comparison when `values_const` is empty (or distinguishing a genuinely empty constant from an unparseable one).

@sdairs
sdairs merged commit cf085ab into main Jul 17, 2026
5 checks passed
@sdairs
sdairs deleted the fix/openapi-drift-298 branch July 17, 2026 14:28
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.

OpenAPI drift: 36 gaps between live spec and library

2 participants