Fix fleet output OAS regressions: SSL type explosion and Kafka union wrappers#260842
Conversation
| "path": "/api/fleet/outputs", | ||
| "method": "get", | ||
| "reason": "Extract shared OutputSslSchema/OutputShipperSchema with meta:{id} to fix type explosion, replace schema.conditional() Kafka fields with schema.maybe() to fix union wrapper regression. Coordinated with provider team — these changes eliminate ~450 lines of adapter code.", | ||
| "approvedBy": "@elastic/terraform-provider" |
There was a problem hiding this comment.
@tobio these changes are needed to fix the regression we introduced.
They should clean up the OAS a bit
|
Pinging @elastic/fleet (Team:Fleet) |
ApprovabilityVerdict: Needs human review This PR introduces runtime behavior changes in fleet output service (clearing Kafka auth fields based on auth_type) in addition to OAS schema refactoring. The author does not own any of the modified files, and an open review comment asks whether the changes were tested with the provider code generator. The designated code owners (@elastic/fleet, @elastic/kibana-core) should review. No code changes detected at You can customize Macroscope's approvability policy. Learn more. |
tobio
left a comment
There was a problem hiding this comment.
Changes look simpler, curious what the enum:null translates to though.
| - certificate | ||
| - strict | ||
| type: string | ||
| anyOf: |
There was a problem hiding this comment.
Have we tested this structure with the provider code generator?
There was a problem hiding this comment.
Tested it — oapi-codegen breaks on the anyOf: [enum: [null], $ref: ...] pattern. Fixed the bundler in 085fd80 to emit nullable: true with an allOf wrapper instead, which is the canonical OAS 3.0.x representation and what oapi-codegen expects.
TinaHeiligers
left a comment
There was a problem hiding this comment.
The enum: [null] inside anyOf is the OAS 3.0 idiom for nullable — it's equivalent to nullable: true but works correctly with anyOf/oneOf compositions. In the generated TypeScript types it produces Type | null (e.g., ssl?: OutputSsl | null). For Go codegen it typically translates to a pointer type, which is the standard nullable representation.
There was a problem hiding this comment.
@tobio Correction on my earlier reply — I called this "the OAS 3.0 idiom for nullable" which isn't quite right. The canonical OAS 3.0.x pattern is nullable: true, what the schema was before.
anyOf: [enum: [null], $ref: ...] is technically valid but more aligned with OAS 3.1 semantics.
The pattern's not hand-authored, it's generated by the OAS bundler where the output's reflecting the fleet source schema schema.oneOf([schema.literal(null), ObjectSchema]).
Internally, the bundler serializes schema.literal(null) as enum: [null] rather than collapsing it to nullable: true.
For codegen consumers like the TF provider, most generators handle both patterns correctly since anyOf: [enum: [null], $ref] is structurally unambiguous. But it's worth flagging that we've moved away from the canonical 3.0.3 idiom in these specific schemas. If this causes issues downstream, the fix would be in the bundler's serialization logic rather than in the source schemas.
TinaHeiligers
left a comment
There was a problem hiding this comment.
Good call asking about the codegen, @tobio. I ran oapi-codegen against the PR's bundled spec and it fails — the anyOf: [enum: [null], $ref: ...] pattern on shipper and ssl fields causes an unmarshal error.
The old spec used nullable: true (canonical OAS 3.0.3) which oapi-codegen handles fine. The issue's in Kibana's OAS bundler — replaceNullableOutputWithNullEnum in kbn-router-to-openapispec converts the internal nullable placeholder to enum: [null] instead of collapsing it to nullable: true. This affects shipper and ssl across all fleet output types, plus a searchAfter field.
Putting this back to draft while I work on a bundler fix.
…ef and multi-branch anyOf
… output nullable fix
TinaHeiligers
left a comment
There was a problem hiding this comment.
Adds a fix to the bundler. processNullableOutput in enum.ts now handles $ref targets by wrapping them in allOf with nullable: true. Also replaced replaceNullableOutputWithNullEnum — it now strips the null placeholder branch and sets nullable: true on the parent instead of emitting enum: [null]. OAS snapshots regenerated, API contracts check passes, and oapi-codegen generates fleet types successfully. Ready for re-review.
| schema.anyOf = remaining; | ||
| schema.nullable = true; | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
@elastic/kibana-core Two changes here, both fixing the same root issue: the bundler was emitting enum: [null] to represent nullable, which isn't canonical OAS 3.0 and breaks oapi-codegen (the Go code generator the Terraform provider uses).
processNullableOutput previously bailed when the non-null branch was a $ref, falling through to replaceNullableOutputWithNullEnum which produced enum: [null]. Now it wraps the $ref in allOf so nullable: true can sit as a sibling — OAS 3.0 doesn't allow siblings directly on a $ref.
replaceNullableOutputWithNullEnum → replaceNullableOutputWithNullable: the old function swapped the nullable placeholder for enum: [null] in multi-branch anyOf unions (3+ branches). The replacement strips the placeholder branch entirely and sets nullable: true on the parent schema. enum: [null] is an OAS 3.1 pattern where null is a first-class type — it doesn't belong in a 3.0.3 spec.
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
@TinaHeiligers Today I merged #259308 that touches some of the same files. If my changes don't look good enough feel free to address them with this PR instead. |
…regressions # Conflicts: # oas_docs/output/kibana.serverless.yaml # oas_docs/output/kibana.yaml # x-pack/platform/plugins/shared/fleet/server/types/models/output.ts
|
@criamico your changes are fine from an OAS quality perspective. The only thing it didn't include was to use an extracted |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
There are no new commits on the base branch. |
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]
History
|
* commit '11ed3645c5ededae2a6e29f2a79b31f52208b441': (157 commits) remove sync register uiAction methods (elastic#254590) [performance] Apply minimal auth to the search route (elastic#257497) [ES|QL] Reports correctly the controls server side errors (elastic#263020) [SecuritySolution][Navigation] Enable classic nav updates (elastic#262358) [Inference] Use pretty name and logo on feature settings page (elastic#262531) [Security Solution] fix AT-AB cypress test (elastic#262991) [SigEvents] Seed sigevents env script (elastic#261172) Adjust conditions for validating no refetch for expanded row (elastic#262978) [Agent Builder] update copy for the announcement modal (elastic#263034) [Search] Hide index management links for users without privileges (elastic#262627) Simplify OAS schema for GET `/api/spaces/space` query params (elastic#260831) Fix fleet output OAS regressions: SSL type explosion and Kafka union wrappers (elastic#260842) [Dashboards in chat] fix agent confusing the axes in a horizontal chat (elastic#263064) [One Workflow] Add alert state checkbox UI for workflow connector (elastic#259770) [One Workflow] Deprecate legacy Cases step types in workflow authoring (elastic#262070) skip failing test suite (elastic#248090) fix flaky test: MonitorDetails filter apply button not enabled (elastic#260788) fix: propagate AbortSignal to executeAsReasoningAgent for task cancellation (elastic#262811) [Security Solution][Alert KPI] Fix white space bug in alert KPIs (elastic#260803) [Streams] Move helpers and format_size_unit to utils folder (elastic#262550) ... # Conflicts: # x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/canvas_integration/dashboard_canvas_content.test.tsx # x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/canvas_integration/dashboard_canvas_content.tsx # x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/canvas_integration/use_register_canvas_action_buttons.ts # x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/index.test.tsx # x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/index.tsx
Summary
Follow up to #258986 to fix duplicated SSL/shipper struct definitions and
anyOfcatchall union wrappers for Kafka conditional fields.This PR:
meta:{id}$refcomponentschema.conditional()Kafka fields (compression_level,connection_type,username,password) withschema.maybe()output.ts, matching the existing pattern forcompression_levelclearingThe net result is 27 fewer generated Go types and ~1,500 fewer lines in the bundled OAS. On the provider side, this eliminates the ~450 lines of adapter code added in PR#2031.
Relates to #228077
Breaking changes
The contract checker flags 352 changes across 3 fleet output endpoints. None of these change runtime API behavior — they're all OAS representation improvements that only affect code generators consuming the spec.
object→$ref: oasdiff can't resolve through$ref, so it reports type changes and "removed" sub-properties. The actual data shapes are identical.schema.conditional()→schema.maybe()is strictly more permissive. Service layer now enforces the same cross-field constraints.schema.conditional()producedanyOf: [array, boolean, number, object, string]catchalls. Now they're the correct specific types (number,string, etc.).How to test this
Run the output model and service tests:
All 7 model tests and 99 service tests should pass.
Run the contract checker for both distributions:
Both should report all breaking changes allowlisted.
Verify OAS snapshot changes by inspecting the diff on
oas_docs/output/kibana.yaml:output_sslandoutput_shippereach appear once as component definitions, referenced via$reffrom all output variantscompression_levelistype: number,connection_typeistype: stringwith enum,usernameandpasswordarenullable: true, type: string— noanyOfcatchallsChecklist
release_note:breakinglabel should be applied in these situations.release_note:*label is applied per the guidelinesIdentify risks
$refchanges OAS structureCo-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com