Skip to content

OpenAPI drift detection misses fields removed from the spec but still present in models.rs #237

@sdairs

Description

@sdairs

Problem

Our OpenAPI drift detection is one-directional for fields (spec → code). It catches fields added to the spec that are missing from models.rs, and optionality flips (TOption<T>) on fields present in both. It does not catch the reverse: a field that has been removed from the live spec but still lingers as a struct field in models.rs.

This is exactly how #235 / #236 slipped through — storageSize was dropped from the Postgres request schemas (PostgresServicePostRequest, PostgresServicePatchRequest, BasePostgresService, PostgresServiceListItem) upstream, the vendored snapshot was even refreshed to match, but models.rs kept the field and every check stayed green. A model that is a superset of the spec passes all current field checks.

Root cause

Every field check iterates over spec properties only:

  • scripts/check-openapi-drift.py
    • check_missing_fields (~L415): for prop_name in props: if prop_name not in fields → missing
    • check_field_optionality (~L331): same loop; a field in the model but not the spec hits if prop_name not in fields: continue (silently skipped)
  • crates/clickhouse-cloud-api/tests/spec_coverage_test.rs
    • struct_fields_cover_every_spec_property and field_optionality_matches_spec — same spec → struct direction

Telling asymmetry

For operations the check is already bidirectional: assert_client_operation_coverage (spec_coverage_test.rs:80) reports both missing and extras ("Extra client methods (not in spec)"). The same "extras" concept was simply never built for fields.

Proposed fix

Add an "extra struct fields" check mirroring the existing "extra client methods" one:

  1. In check-openapi-drift.py: a reverse pass that, for each struct mapped to a spec schema, flags model fields with no corresponding spec property. Surface it in the drift report (new "Extra struct fields (not in spec)" section + summary row).
  2. In spec_coverage_test.rs: a struct_fields_have_no_extras_vs_spec test asserting no model field is absent from its spec schema.

Wrinkle to handle: allowlist

Some struct fields legitimately have no spec property, e.g.:

  • Response-only / computed fields we add intentionally.
  • Structs in models.rs that don't map 1:1 to a spec schema (the checks already continue when the schema isn't found — keep that).

Introduce an allowlist constant analogous to OPTIONALITY_EXEMPTIONS (e.g. EXTRA_FIELD_EXEMPTIONS of ("RustStructName", "fieldName")) so intentional additions don't false-positive. The drift script should parse the same list (it already does this for exemptions via parse_optionality_exemptions) so the report and the test stay in sync.

Acceptance criteria

  • Removing a field from a spec schema while leaving it in models.rs fails spec_coverage_test.rs and shows up in the drift report.
  • Intentional code-only fields are covered by a documented allowlist that fails when it goes stale.
  • Reusing Remove storageSize from Postgres create/update requests #236 as a regression reference: had this existed, the stale storageSize fields would have been flagged.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions