[OAS] Detect colliding shared schema ids during OpenAPI generation#271815
Open
tobio wants to merge 1 commit into
Open
[OAS] Detect colliding shared schema ids during OpenAPI generation#271815tobio wants to merge 1 commit into
tobio wants to merge 1 commit into
Conversation
Throw a descriptive `OasSchemaCollisionError` from `@kbn/router-to-openapispec` when two distinct shapes are registered under the same shared-schema id during a single generation pass. Catches the silent component-overwrite class of bug behind elastic#271809 (e.g. `Base.extends({...})` inheriting `meta.id` from its base). Companion to elastic#271812 (Path A — short-term Fleet-side fix that removes the existing collisions). Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Pinging @elastic/kibana-core (Team:Core) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes part of #271809.
Companion to #271812 (Path A).
Summary
Adds a runtime guardrail in
@kbn/router-to-openapispecthat failsOAS generation loudly when two distinct schema shapes are registered
under the same shared-schema id during a single generation pass. This
is the silent-overwrite class of bug that caused the Fleet regressions
in #271809.
The OAS converter keeps a
Map<id, schema>of named components so itcan emit
$ref-based reuse. Today that map is last-write-wins: if tworoutes register the same id with different shapes (most often through
Base.extends({...})inheritingmeta.idfrom its base), one shapesilently overwrites the other in the bundled spec, dropping fields. With
this PR the converter throws an
OasSchemaCollisionErrorinstead, witha key-set diff and a concrete fix suggestion pointing back to #271809:
Re-using the same shared schema across multiple routes (the common,
intended case) stays a no-op — equality is deep, but ignores the
transient
META_FIELD_X_OAS_OPTIONALannotation thatMaybeTypeattaches mid-parse and
removeInternalOptionalMarkerstrips on theway out.
Changes
oas_converter/kbn_config_schema/post_process_mutations/schema_collision.ts(new) —
OasSchemaCollisionError,describeSchemaCollision,and a
schemasMatchdeep-equality helper that ignores theinternal optional marker.
oas_converter/kbn_config_schema/post_process_mutations/context.ts—
Context.addSharedSchemanow compares any existing entry againstthe incoming shape via
schemasMatchand dispatches on a newOnCollision = 'throw' | 'warn' | 'ignore'option (default'throw').oas_converter/index.ts—OasConverteraccepts anOasConverterOptions { onCollision }second-arg, threads it throughto the underlying
Context, and applies the same check as a backstopin
#addComponents(coversconvertPathParameters/convertQuery, which currently create their ownContext).kbn_config_schema/lib.ts+type.ts— propagateonCollisionthrough the converter API.
index.ts— re-exportOasSchemaCollisionErrorso callers cancatch it.
context.test.ts— 7 cases (idempotent re-registration, throwon shape mismatch, error-message contents,
'warn'mode,'ignore'mode, and a@kbn/config-schema-driven repro ofthe issue [Fleet] Named OpenAPI components from #270560 silently drop fields (output_id, policy_id, policy_ids, supports_agentless, id, package) #271809 pattern).
generate_oas.test.ts— 2 end-to-end cases that wire tworoutes through
generateOpenApiDocument: one collides, onedoesn't.
Sequencing
This PR should land after #271812.
Without Path A merged first, the existing Fleet collisions (the very
ones #271809 reports) would now trip the strict throw and break the
OAS snapshot CI step. Once Path A is in, this PR locks the door behind
it: any future regression of the same shape gets caught at build time
rather than silently corrupting the bundle.
Test plan
node scripts/jest src/platform/packages/shared/kbn-router-to-openapispec— 252/252 pass (18 suites; +9 new tests covering the guardrail).node scripts/type_check --project src/platform/packages/shared/kbn-router-to-openapispec/tsconfig.json— clean.node scripts/eslinton the diff — clean.Risks
Low. The throw is a build-time check on the OAS bundle pipeline; it
does not affect any runtime route handler. The new
'warn'and'ignore'modes are escape hatches for transitional or test usage —nothing in production sets them. The biggest exposure is the
sequencing dependency on Path A: if this lands first, the next OAS
snapshot run will fail on the existing Fleet collisions until Path A
also merges.
Release Notes
None — internal tooling change. Failures here surface during OAS
generation, not at runtime.
Made with Cursor