Skip to content

Make CreateRef idempotent for componentized OAS input#2010

Closed
TinaHeiligers wants to merge 3 commits intoelastic:mainfrom
TinaHeiligers:make-createref-idempotent
Closed

Make CreateRef idempotent for componentized OAS input#2010
TinaHeiligers wants to merge 3 commits intoelastic:mainfrom
TinaHeiligers:make-createref-idempotent

Conversation

@TinaHeiligers
Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers commented Mar 24, 2026

Closes #2009

Why

Kibana is progressively improving its OAS output by adding named components to schemas via meta:{id} on kbn-config-schema types. When a Kibana plugin adds meta:{id}, the OAS bundler produces a $ref to a named component instead of an inline schema.

The provider's CreateRef function currently panics when:

  1. The target path doesn't exist (Kibana already extracted it to a component)
  2. The target is already a $ref (Kibana produced a reference instead of inline)

This blocks the provider from consuming improved Kibana OAS output without per-change updates to transform_schema.go.

What

Makes CreateRef idempotent by adding two guards:

  • Path not found: Uses Get instead of MustGet. When the target path doesn't exist, logs a message and returns the $ref value without panicking. This handles cases where Kibana's componentizer has already extracted the schema.
  • Already a $ref: Detects when the target is already a $ref (both Map and map[string]any variants) and returns early without re-extracting.

No behavioral change for existing OAS input — the guards only activate when Kibana produces componentized output.

Motivation

Kibana PR elastic/kibana#258986 is the first change adding meta:{id} to fleet output schemas, producing named components. More Kibana plugins will follow this pattern as part of the ongoing OAS quality initiative.

Test plan

  • Added 5 unit tests for CreateRef covering:
    • Normal inline extraction (existing behavior)
    • Target already a $ref (Map type) — returns without panic
    • Target already a $ref (map[string]any type) — returns without panic
    • Target path not found — returns without panic
    • Duplicate identical component registration — dedup works
  • go build ./... passes
  • Full transform + codegen pipeline validated locally against current kibana.yaml
go test -v -run "TestCreateRef" generated/kbapi/transform_schema.go generated/kbapi/transform_schema_test.go

Note

Make Map.CreateRef idempotent for componentized OAS input

  • CreateRef in transform_schema.go previously panicked when the target path was missing or already a $ref, causing failures on pre-componentized OpenAPI schemas.
  • Adds a safe Get lookup so missing paths log a message and return the $ref without mutating the map.
  • Adds early-return checks for targets already stored as a $ref (both Map and map[string]any types), skipping component registration.
  • Adds tests covering inline extraction, already-a-ref (both types), missing path, and duplicate identical component cases.

Macroscope summarized d4304a8.

TinaHeiligers and others added 2 commits March 24, 2026 12:24
When the target path doesn't exist (because the Kibana componentizer
already extracted it to a $ref), skip gracefully instead of panicking.
Also skip when the target is already a $ref.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 24, 2026 19:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

@TinaHeiligers TinaHeiligers requested a review from tobio March 24, 2026 19:59
@tobio
Copy link
Copy Markdown
Member

tobio commented Mar 25, 2026

This blocks the provider from consuming improved Kibana OAS output without per-change updates to transform_schema.go.

I actually see this as a good thing TBH. It's a clear signal that we can clean something up in transform_schema.go. Without this PR:

  1. Renovate bumps the Kibana schema
  2. The build fails
  3. Renovate opens a PR
  4. We remove the now dead code
  5. Renovate merges the PR

With the alternative (this PR), we still have to eventually clean up transform_schema, but now we have to check if a piece of code is still used or not. So the process becomes:

  1. We have a 'tidy up transform_schema' task on the backlog
  2. Someone picks it up in the future
  3. Investigates what can be removed
  4. Maybe misses something
  5. PR, review, merge.

Whilst it reduces the short term friction, IMO the overall human in the loop cost is higher. We're also assuming an exact 1:1 component renaming, I understand that's the goal, but any time we don't accomplish that we're left with the same blocking change anyway.

If it we me, we'd close this PR and address the changes as they arise, WDYT?

@TinaHeiligers
Copy link
Copy Markdown
Contributor Author

Closing as not needed. The clean-up work will be handled through the renovate workflow

@TinaHeiligers
Copy link
Copy Markdown
Contributor Author

FYI, without the changes in this PR, CreateRef will panic when elastic/kibana#258986 lands

@TinaHeiligers TinaHeiligers reopened this Mar 25, 2026
@TinaHeiligers
Copy link
Copy Markdown
Contributor Author

TinaHeiligers commented Mar 26, 2026

@TinaHeiligers
Copy link
Copy Markdown
Contributor Author

TinaHeiligers commented Mar 26, 2026

@tobio I tested the current Kibana main OAS against the provider's transform pipeline:

Panic in transformFleetPaths when CreateRef tries to extract schemas.output_elasticsearch.properties.shipper and the schema structure changed.

This PR works for CreateRef (8 skips logged for shipper/ssl extraction), but panics at MustGetMap("schemas.output_kafka.properties"). The downstream kafka type fixes, discriminator injection, and id removal in transformFleetPaths would also need guarding.

All three of those transforms are now dead code because elastic/kibana#258986 already fixed them in Kibana (kafka types, discriminator, id removal). So the real fix on the next Renovate bump is: merge this PR for CreateRef safety + remove the dead transformFleetPaths post-extraction code:

That cleanup can happen in the Renovate PR itself since the build failure makes it obvious which code to remove. This PR just ensures CreateRef doesn't panic before we even reach those lines.

@tobio
Copy link
Copy Markdown
Member

tobio commented Mar 27, 2026

So the real fix on the next Renovate bump is: merge this PR for CreateRef safety + remove the dead transformFleetPaths post-extraction code:

TBH I feel like this backs up my hesitation around this PR. It would have had a negligible impact on the work involved in getting #2031 merged.

There were multiple changes required to the transformation here, followed up with wider changes adopting the new client code in the provider.

With this PR merged, we would have saved maybe 30 seconds, but have left this now redundant code block around for someone to investigate in the future. At the moment, I don't see any tangible benefit in going down this path.

@TinaHeiligers TinaHeiligers deleted the make-createref-idempotent branch March 27, 2026 18:14
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.

[Enhancement] Make CreateRef idempotent to support Kibana's evolving OAS output

3 participants