[api-contracts] Check the whole stable and tech_preview API surface for breaking changes

[TinaHeiligers]

Fix #269134.

Summary

The API contracts check used to only look at APIs the Terraform provider consumes, so it missed breaking changes in every other stable or Technical Preview API.

This PR extends the check to the whole Kibana OpenAPI surface. Each breaking change is classified by its stability tier (read from x-state in the base spec). Stable and Technical Preview breaks fail the check; experimental breaks are reported for visibility but never gate, since experimental APIs are allowed to break.

Terraform-provider tracking is removed entirely. The check no longer keys off Terraform-consumed APIs, so the src/terraform/* matching, terraform_provider_apis.yaml, and the owner @mentions in the PR comment are all gone.

The hard gate and the tech_preview approval signal are follow-up in #261072.

Detaileds

• Diffs the whole public OAS surface against main (unchanged).
• Classifies each break by x-state tier from the base spec. Parsing is tolerant of the casing and spacing variance in hand-written specs, and a round-trip test asserts the parser still decodes what getXState in @kbn/router-to-openapispec produces, so generator drift fails loudly.
• Missing or unrecognized x-state defaults to stable, the most conservative tier. An unknown state can only resolve up to stable, never into experimental, so a break is never under-classified.
• The allowlist is the per-change escape hatch, tier-agnostic: an entry clears a change whether it's stable or tech_preview.
• Soft gate: a stable/tech_preview break fails the check (non-zero exit) but Buildkite soft_fail keeps merge unblocked. The flip to a hard gate is removing one line.
• The notifier posts (or updates) the PR comment whenever there's anything to report, regardless of exit code, so experimental-only BCs still surface. No team @mentions.

Implementation flow

flowchart LR
    A[diff whole OAS surface] --> B[apply allowlist]
    B --> C{classify by tier}
    C -->|stable / tech_preview| D[report + non-zero exit]
    C -->|experimental| E[report as informational, exit 0]
    D --> F{soft_fail on?}
    F -->|yes| G[warn: comment, merge unblocked]
    F -->|no| H[hard gate: merge blocked]
    E --> I[comment posts, no gate]

Loading

• soft_fail is the only warn-vs-block control, so the rollout flips one line rather than shipping new code.
• The allowlist is tier-agnostic, so the hard-gate future has its escape hatch in place already.

How to test this

The behavior to verify is what the checker gates on, reports as informational, and suppresses. Run it against main after editing the generated spec to simulate each case, then revert the spec edit.

node scripts/check_api_contracts.js --distribution stack --reportPath /tmp/impact.json

1. Gate on a stable break: remove a path or a required response property from a stable endpoint in oas_docs/output/kibana.yaml (e.g. /api/alerting/rule/{id}/snooze_schedule), then run the command. Expect a non-zero exit, Detected 1 breaking change(s) in stable/tech_preview APIs: 1 stable, 0 tech_preview, and a report entry with "tier": "stable".

2. Gate on a Technical Preview break: repeat on an endpoint whose x-state is Technical Preview (e.g. /api/features). Expect a non-zero exit, 0 stable, 1 tech_preview, and "tier": "tech_preview".

3. Report an experimental break without gating: repeat on an endpoint whose x-state is Experimental (e.g. /api/agent_builder/a2a/{agentId}). Expect exit 0, 1 experimental-tier breaking change(s) reported (informational, not blocking), No breaking changes detected in stable or tech_preview APIs, and a report entry with "tier": "experimental" still present so the notifier can surface it.

4. Suppress via the allowlist: add an entry for the stable break from step 1 to packages/kbn-api-contracts/allowlist.json (use "method": "all" for a path-level change) and re-run. Expect exit 0 and All breaking changes are allowlisted.

5. Confirm the CI posture: the check exits non-zero on a stable/tech_preview break, the Buildkite step is soft_fail, merge isn't blocked, and the notifier posts (or updates) the PR comment regardless of exit code.

Checklist

• Unit tests were updated or added to match the change in functionality
• The package README was updated to reflect the new behavior

Identify risks

This is scoped to the contract-checker tooling and its CI wiring. It changes what CI reports, not any runtime API behavior.

Risk	Severity	Likelihood	Mitigation
The whole-surface check surfaces more breaking-change warnings than the old Terraform-scoped check	Low	Expected	soft_fail keeps merge unblocked; the allowlist suppresses approved BCs; experimental BCs are informational, not gating
Most operations (~70%) don't declare x-state, so their BCs classify as stable	Low	Expected	The conservative default over-gates rather than under-gates, so nothing slips through; the check nudges owners to declare x-state for a precise tier
An already-merged approved BC re-flags on later PRs	None	Not observed	The check diffs against main's current OAS, so merged changes are in the baseline and don't re-trigger
A stable or tech_preview BC merges while the check is soft	Low	Possible	Always-on PR comment plus a visible step failure; the hard-gate flip removes soft_fail when ready

[updated after review feedback]

[infra-vault-gh-plugin-prod]

Pinging @elastic/kibana-core (Team:Core)

[jloleysens]

Nice work @TinaHeiligers ! Not fully done reviewing, left a few comments I'd like to align on first. Specifically the use of @ mentions and when PR comments are surfaced (currently it seems we are gating the comment based on the tier?)

[jloleysens]

I understand the goal is to not let this slip by unnoticed, but I think surfacing a comment in the PR already achieves that. I'm concerned including an @ mention is going to be too noisey, especially if we work is in progress (PR authors may have unintentionally made a change that we are notifying owning teams of).

Pointing PR authors to the team to talk to is a cool idea though!

[TinaHeiligers]

Um, I had a look into using CODEOWNERS for all endpoints and it's a bigger lift than I think is worth adding in this PR (details below for future enhandcements)

Why adding who to talk to is a project on it's own

Details

@kbn/code-owners gives us getOwningTeamsForPath(filePath), resoling teams from a repo file path. But a breaking change only gives us an API endpoint (e.g. /api/fleet/agents, method POST) and a schema JSON-pointer in source (e.g. /components/schemas/...). The checker's only input is the bundled oas_docs/output/kibana*.yaml, and the spec carries nothing that bridges endpoint → source file:

• operations only have x-metaTags: product_name: Kibana, x-state, x-codeSamples — no team, no plugin, no path;

• source is a JSON pointer into the schema tree, not a file;

• there isn't an API-path→plugin/owner mapping anywhere in oas_docs/.
  So to resolve CODEOWNERS for the whole surface involves having to create an endpoint to file bridge, and every option is a real project:

• Generation-time enrichment (cleanest): stamp an x-owner/source hint onto each operation in router-to-openapispec so the checker can read it. Cross-cutting — touches the OAS pipeline and every producer, and handwritten specs still wouldn't have it (same class of gap we've been designing around).

• Route-registry introspection: enumerate registered routes → registering plugin dir → CODEOWNERS. Requires booting/inspecting Kibana; not available to a script that just diffs two YAML files.

• URL-prefix → plugin heuristic: brittle and incomplete; CODEOWNERS is keyed by file path, not URL, and lots of endpoints don't map cleanly.

The Terraform path avoids all this because it uses a hand-maintained terraform_provider_apis.yaml mapping endpoint → resource → owners, which I admit is outdated already. Replicating that by hand for the whole surface would be a maintenance sink and not worth it.

[jloleysens]

May not be following quite right: but I think it would still be nice to surface that a breaking change is being introduced due to the impact even for experimental APIs. That at least makes sure that authors are informed that what they are about to ship is gonna break clients.

[TinaHeiligers]

My concern with including experimental APIs is that it's going to be too noisy and folks will ignore the comments, knowing that the checks also applying to APIs that are allowed to introduce breaking changes.

That being said, the original issue specifically scopes the work as

detect breaking changes across the whole public API surface, enforce policy by API stability tier (stable / GA, tech_preview, experimental)

so yes, I'll add it.

[jloleysens]

Bc these utilities are coupled in this way, I wonder if we should import that utility from router-to-openapispec and assert that this utility can "decode" it's output from a test.

Note: not all spec for Kibana APIs comes from router-to-openapispec... Hand written APIs may define x-state differently. Worth consider for this classification to work.

[TinaHeiligers]

Hand written APIs may define x-state differently

The only stability signal in the OAS is x-state and the handwritten variants are case/spacing deviations of the same nomenclature (Technical preview, Generally available; Added in X, added in X, ''/absent.)

[TinaHeiligers]

Good call on both points.

I've added a round-trip test that asserts parseXState decodes its output for every stability tier (stack + serverless env), so if the generator's wording ever changes the test fails loudly.

On hand-written specs, I deliberately didn't couple classification to getXState. parseXState's tolerant to case etc with a conservative stable default, and I've added the data-driven test over the what we have as x-state values in oas_docs/output/*.yaml.

The tests cover different things — the round-trip guards the generated contract, the data-driven test guards hand-written reality.

Also LMK if the comment/essay for parseXState's too verbose. I lean towards over-documenting rather than under.

[TinaHeiligers]

example reports after the changes: https://gist.github.com/TinaHeiligers/0b118410bb6929163b77496419e4e5ac

[TinaHeiligers]

address review comments

[TinaHeiligers]

naming's hard, open to changing CaughtTier to DetectedTier or something else.

[TinaHeiligers]

My concern with including experimental APIs is that it's going to be too noisy and folks will ignore the comments, knowing that the checks also applying to APIs that are allowed to introduce breaking changes.

That being said, the original issue specifically scopes the work as

detect breaking changes across the whole public API surface, enforce policy by API stability tier (stable / GA, tech_preview, experimental)

so yes, I'll add it.

[TinaHeiligers]

Choosing stable as the fall back tier so that it mirrors the platform contract:
/src/core/packages/http/server/src/router/route.ts

availability?: {
  /** @default stable */
  stability?: 'experimental' | 'stable' | 'tech_preview';

most operations don't carry x-state (~ 70% stack, ~69% serverless) so catching BC's in unknowns carries the benefit of strongly encouraging API owners to declare one.

What I'm concerned about is that ~70% don't declare x-state. It's ok while we run the contracts check as a soft fail but might catch contributors off guard.

[TinaHeiligers]

gating tier and reporting tier are intentionally separate. Reporting tier is what comments surface in the notification, the gating tier determines the check's exit code.

[kibanamachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: d8ba1f9
• Build duration: 98 mins

Failed CI Steps

• FTR Configs #127

Metrics [docs]

✅ unchanged

History

• 💔 Build #475555 failed dd2d02d
• 💛 Build #475464 was flaky 64b386a
• 💚 Build #475004 succeeded b42952c
• 💚 Build #474989 succeeded 0b1b414