[Security Solution] Adjust OpenAPI specs bundler for doc requirements

[maximpn]

Addresses: https://github.com/elastic/security-team/issues/7981
Relates to: #171526

Summary

This PR adjusts OpenAPI Bundler (kbn-openapi-bundler package) based on Docs Engineering team requirements. Main requirements include producing one valid OpenAPI spec bundle file. After adjustments OpenAPI Bundler one valid OpenAPI spec bundle file per API version e.g. 2023-10-31-my-oas.bundled.schema.yaml.

Details

This PR further improves #171526 to satisfy Docs Engineering team requirements and includes the following adjustments and improvements

• Produce one valid OpenAPI spec file per API version
• Make outputFilePath independent from rootDir
outputFilePath can be an absolute or relative to node.js working directory path. Additionally it can contain {version} placeholder to adjust where API version is placed in the result bundle file name. API prepends the result bundle file name if {version} is omitted.
• Reduce folded allOf items
  Inlined schemas lead to allOf folder into each other as well as having multiple object schemas which could be merged. Docs Engineering team stated potential problems related to folded allOf items.
• Prevent bundling incompatible source OpenAPI schemas like 3.0.3 and 3.1.0
• Make sure to bundle path item's summary, description and properties
• Make sure to bundle all shared components (not only schemas)

Besides the changes above PR contains minor bug fixing and improvements.

Note: It intentionally doesn't include bundling configuration changes like integration it into PR's build pipeline and focuses only on functional changes. Bundling configuration will be addressed separately to avoid spreading reviewers focus.

[elasticmachine]

Pinging @elastic/security-detections-response (Team:Detections and Resp)

[elasticmachine]

Pinging @elastic/security-solution (Team: SecuritySolution)

[elasticmachine]

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

[xcrzx]

Thank you, @maximpn, for this awesome improvement to the bundler, it's really huge 🎉 I tested the PR locally, and the bundled Security Solution APIs look correct. I was able to easily export all our APIs at once to Postman, and this is 🔥🔥🔥 It's a big step forward for our tooling.

I left several comments regarding tests that produce invalid OpenAPI, but that doesn't seem to affect our API bundle, though. Maybe it's just the tests themselves that need to be fixed. Also, I've made a couple of comments regarding the readability and maintainability of the bundler code, so please take a look, and I'll be happy to discuss this in more detail if needed.

[xcrzx]

Why is this method called leave? It is somewhat confusing because it's used to modify a node. Let's rename it to something more straightforward, like modify.

[maximpn]

There are two phased at which OpenAPI document is traversed.

• At the first phase diving from root to leaves happens (like event's capture phase in DOM). It means we enter a subtree. Processing isn't happening here so it's convenient to analyze not yet processed nodes. For example understand which nodes should be inlined.
• At the second phase bubbling from leaves to root happens (like event's bubble phase in DOM). It means we leave s subtree. It's possible to modify a node during this phase while it's not necessary.

This naming highlights the flexibility. enter and leave methods could be used to collect metrics or analyze the document. Current implementation doesn't have such functionality but it could be added without any effort.

I extended a comment for DocumentNodeProcessor in packages/kbn-openapi-bundler/src/bundler/types.ts. While we don't expose DocumentNodeProcessor API I think package README shouldn't be updated.

[xcrzx]

Let's remove the commented code and the recursive_spec files

[maximpn]

I managed to uncomment the test by allowing to write documents with circular references by modifying writeYamlDocument.

[xcrzx]

Should we also check that other x- properties are getting removed?

[maximpn]

I've added extra tests.

[xcrzx]

Getting to the last test in this file, I started understanding that this snapshot-like testing requires quite a lot of concentration and effort to read through and understand what is being covered. For each test, you need to read the description here, then navigate to the test folder, locate the source files and the expected YAML, then open them side by side, and compare their content to see if the input matches the expected output. This process is pretty error-prone, and as we've seen above, snapshots just captured invalid output, leaving it up to humans to verify its correctness.

I wonder what options we have to improve this experience. Maybe we could inline some of the test cases that are simple enough so the input and output are easily visible and reduce the boilerplate in the test files? What else could we do?

[maximpn]

After syncing with @xcrzx via Zoom we discussed improvements to maintainability. The following has been decided and addressed

• rename node processor's methods enter -> onNodeEnter, leave -> onNodeLeave and ref -> onRefNodeLeave
• reorganize functional tests to make them much more focused and avoid snapshot like testing as much as possible

[machadoum]

Entity Analytics changes LGTM!

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 843b844

Metrics [docs]

✅ unchanged

History

• 💛 Build #208564 was flaky ee0c313
• 💔 Build #208495 failed 9c70c5ebc34e6cc8db2a4e8435a8fbe3eef31a46
• 💚 Build #208259 succeeded 49e35c6401fc24918f0986f29e231568f7c060df
• 💚 Build #208073 succeeded 8d7cb29afd8c21084e40d506e3790330394f58eb
• 💚 Build #207155 succeeded 6aedaf1a6e0bfff121087aadc11edca9b0e11645
• 💔 Build #207132 failed 09333fd0047f745b5cddee4994b0f8f0cc6d53eb

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @maximpn

[xcrzx]

Reviewed and tested locally, everything works as expected 👍
And thank you for updating the tests, @maximpn, they are much easier to read and follow the logic now 🙌