Skip to content

[kbn-api-contracts] Detect request-body additionalProperties:false tightening#267546

Merged
Bamieh merged 6 commits into
elastic:mainfrom
Bamieh:oasdiff_detect_additional_props
May 5, 2026
Merged

[kbn-api-contracts] Detect request-body additionalProperties:false tightening#267546
Bamieh merged 6 commits into
elastic:mainfrom
Bamieh:oasdiff_detect_additional_props

Conversation

@Bamieh
Copy link
Copy Markdown
Contributor

@Bamieh Bamieh commented May 4, 2026

Summary

oasdiff breaking does not flag request bodies that gain additionalProperties: false, so clients sending extra keys start getting HTTP 400s without any CI signal. This PR adds a detector that catches the transition.

How it works:

  • Calls oasdiff diff --format json --flatten-allof, which does expose additionalPropertiesAllowed: { from: null|true, to: false }.
  • Walks the structural diff and uses a reverse index of the current OAS to fan component-level tightenings out to each consumer (path, method).
  • Emits synthetic OasdiffEntry items that flow through the existing parse, Terraform-impact, allowlist, and report stages with no other code changes.

All work stays inside packages/kbn-api-contracts/. oas_docs/ is untouched.

The README also restructures the allowlist guidance: granular entries (oasdiffId + source) lead, and the coarse path+method form is flagged as hiding future breaks on the same endpoint.

Closes https://github.com/elastic/kibana-team/issues/3239

Live demo on the real Kibana spec

Edited the Data_views_create_data_view_request_object component in oas_docs/output/kibana.yaml to add additionalProperties: false, then ran node scripts/check_api_contracts.js --distribution stack against origin/main.

Before (clean working tree)

 info Checking stack API contracts...
 info Current spec: oas_docs/output/kibana.yaml
 info Base: origin/main
 info Filtering oasdiff to 20 Terraform provider API paths
 succ No breaking changes detected

After (one-line edit added additionalProperties: false)

 info Checking stack API contracts...
 info Current spec: oas_docs/output/kibana.yaml
 info Base: origin/main
 info Filtering oasdiff to 20 Terraform provider API paths
ERROR
      ╔════════════════════════════════════════════════════════════════════════════╗
      ║                     API CONTRACT BREAKING CHANGES DETECTED                 ║
      ╚════════════════════════════════════════════════════════════════════════════╝

      Found 1 breaking change(s):

      1. Request body schema disallows extra fields (additionalProperties: false). Clients sending unknown keys will now receive 400.
         Path: /api/data_views/data_view
         Method: POST


      ╔════════════════════════════════════════════════════════════════════════════╗
      ║                        TERRAFORM PROVIDER IMPACT                           ║
      ╚════════════════════════════════════════════════════════════════════════════╝

      ⚠️  The following breaking changes affect Terraform Provider APIs:


      • /api/data_views/data_view POST
        Terraform Resource: elasticstack_kibana_data_view
        Reason: Request body schema disallows extra fields (additionalProperties: false). Clients sending unknown keys will now receive 400.
        Owners: @elastic/kibana-data-discovery

      Coordinate with @elastic/terraform-provider before merging.

ERROR Error: Found 1 breaking change(s) affecting Terraform provider APIs

The detection lands exactly where designed: the data_views route, mapped to elasticstack_kibana_data_view, owned by \@\elastic/kibana-data-discovery.

@Bamieh Bamieh added release_note:skip Skip the PR/issue when compiling release notes backport:all-open Backport to all branches that could still receive a release labels May 4, 2026
oasdiff 1.15.1's --flatten-allof stack-overflows on the full Kibana spec
during local manual verification (`oasdiff diff --format json --flatten-allof`
on `oas_docs/output/kibana.yaml` exhausts a 1 GiB goroutine stack). Without
the flag, oasdiff completes and emits allOf branches the same way it does
oneOf and anyOf: `allOf.modified[i].diff.<...>`. The detector already walks
oneOf and anyOf, so adding `'allOf'` to the composition list keeps coverage
identical and removes the production-blocking flag.
@Bamieh Bamieh force-pushed the oasdiff_detect_additional_props branch from 5bbb06f to a4577fd Compare May 4, 2026 15:53
Comment thread packages/kbn-api-contracts/src/diff/detect_additional_properties_tightening.ts Outdated
Bamieh added 2 commits May 4, 2026 20:03
- build_request_body_index.ts → build_request_body_index/ folder:
  - index.ts (entry point + lodash groupBy)
  - extract_request_body_consumers.ts (paths × HTTP methods → schemas)
  - collect_reachable_components.ts ($ref / allOf / oneOf / anyOf / properties / items / additionalProperties walker)
  - types.ts

- detect_additional_properties_tightening.ts → detect_additional_properties_tightening/ folder:
  - index.ts (orchestrator: collect → expand → collect inline)
  - collect_tightenings_in_schema_diff.ts (root + property + items + composition extractors)
  - collect_component_tightenings.ts
  - expand_component_tightenings.ts (fan-out via reverse index)
  - collect_inline_tightenings.ts (path-side, with skipKey dedup)
  - types.ts (DetectionResult, buildEntry, makeSkipKey, constants)

- New shared is_record.ts wraps lodash isPlainObject as a TS type predicate; replaces the per-file isObject helpers.

- run_oasdiff_structural.ts: extracted assertAbsoluteExistingPath, buildArgs, execOasdiff. matchPath now folded into args via spread; no in-place mutation.

Style: replaced every `if (!isObject(...)) continue` with named flatMap/filter chains. Each new file is short (≤74 lines including license header). Behaviour, tests, and public exports unchanged.
`types.ts` now holds only TS types (DetectionResult, ComponentTightening,
SkipKey). The runtime helpers live with the values they construct:

- `build_entry.ts`: REQUEST_ADDITIONAL_PROPERTIES_TIGHTENED_ID,
  TIGHTENING_TEXT, buildEntry()
- `make_skip_key.ts`: makeSkipKey()

Also moved detect_additional_properties_tightening.test.ts inside the
folder as index.test.ts so the test sits next to the module it covers.
@Bamieh Bamieh marked this pull request as ready for review May 4, 2026 16:32
@Bamieh Bamieh requested a review from a team as a code owner May 4, 2026 16:32
Copy link
Copy Markdown
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Code only review, looks good to me!

@Bamieh Bamieh enabled auto-merge (squash) May 5, 2026 06:48
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

@Bamieh Bamieh merged commit bec2b55 into elastic:main May 5, 2026
24 checks passed
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.19, 9.2, 9.3, 9.4

https://github.com/elastic/kibana/actions/runs/25364888890

@kibanamachine
Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
8.19 Backport failed because of merge conflicts
9.2 Backport failed because of merge conflicts
9.3 Backport failed because of merge conflicts
9.4 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 267546

Questions ?

Please refer to the Backport tool documentation

mbondyra added a commit to mbondyra/kibana that referenced this pull request May 5, 2026
…ilder_new_vis_attachment

* commit '6fd683609eb6dee81f242f8ff6951edbe3bfd66c': (226 commits)
  Remove Model Author group-by option from external inference endpoints (elastic#264761)
  [Streams][Streamlang] Align ES|QL condition transpiler with Painless on null propagation (elastic#264751)
  chore(axios,workflows-eng): remove axios from workflows connector utils (elastic#267512)
  [failed-test-reporter] avoid opening issues for scout env failures (elastic#267649)
  [kbn-api-contracts] Detect request-body additionalProperties:false tightening (elastic#267546)
  [main] Sync bundled packages with Package Storage (elastic#267644)
  Centralize phase colors and descriptions (elastic#266680)
  [Unified Waterfall] Add "Scroll to origin" button  (elastic#266594)
  [APM] Add alert and SLO badges to the service map embeddable (elastic#266360)
  [CI] Speed up telemetry_check by pre-filtering to collector files (elastic#265978)
  [Discover] Address flaky large CSV test (elastic#266642)
  avoid passing unrelated props within integration card icon component conditional render (elastic#266569)
  [Cases][Templates] Extend cases search by template field label (elastic#266414)
  [Background search] Migrate custom SplitButton to EuiSplitButton (elastic#267447)
  [i18n] Report translation coverage during integrate (elastic#264124)
  [api-docs] 2026-05-05 Daily api_docs build (elastic#267639)
  [Scout] Update test config manifests (elastic#267636)
  [content list] Add saved object provider services (elastic#266428)
  [Fleet] Otel UI add health and implement it in OTelComponentDetail (elastic#267292)
  Update dependency msw to v2.13.4 (main) (elastic#266770)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 6, 2026
@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 267546 locally
cc: @Bamieh

3 similar comments
@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 267546 locally
cc: @Bamieh

@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 267546 locally
cc: @Bamieh

@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 267546 locally
cc: @Bamieh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:all-open Backport to all branches that could still receive a release backport missing Added to PRs automatically when the are determined to be missing a backport. release_note:skip Skip the PR/issue when compiling release notes v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants