[Security Solution] Implement OpenAPI specs merger utility #188110

maximpn · 2024-07-11T14:28:16Z

Addresses: #186356

Summary

This PR adds OpenAPI spec files merger utility (programmatic API). It provides a similar functionality as npx @redocly/cli join does and takes into account discussion results

provides a simple way to produce a single Kibana OpenAPI bundle
extends requestBody and responses MIME types with a version parameters Elastic-Api-Version=<version> to avoid different API endpoint versions conflicts
has flexibility to adjust Kibana needs

The utility is exposed from kbn-openapi-bundler package.

Details

OpenAPI merger is a tool for merging multiple OpenAPI specification files. It's useful to merge already processed specification files to produce a result bundle. OpenAPI bundler uses the merger under the hood to merge bundled OpenAPI specification files. Exposed externally merger is a wrapper of the bundler's merger but extended with an ability to parse JSON files and forced to produce a single result file.

It is able to read OpenAPI spec files defined in JSON and YAML formats. The result file is always written in YAML format where every requestBody and response in responses extended with document's info.version value added as a MIME type parameter Elastic-Api-Version=<version>.

Currently package supports only programmatic API. As the next step you need to create a JavaScript script file like below

require('../../src/setup_node_env');
const { resolve } = require('path');
const { merge } = require('@kbn/openapi-bundler');
const { REPO_ROOT } = require('@kbn/repo-info');

(async () => {
  await merge({
    sourceGlobs: [
      `${REPO_ROOT}/my/path/to/spec1.json`,
      `${REPO_ROOT}/my/path/to/spec2.yml`,
      `${REPO_ROOT}/my/path/to/spec3.yaml`,
    ],
    outputFilePath: `${REPO_ROOT}/oas_docs/bundle.yaml`,
    mergedSpecInfo: {
      title: 'My merged OpenAPI specs',
      version: '1.0.0',
    },
  });
})();

Finally you should be able to run OpenAPI merger via

node ./path/to/the/script.js

or it could be added to package.json and run via yarn.

After running the script it will log different information and write a merged OpenAPI specification to a the provided path.

Caveats

Items below don't look critical at the moment and can be addressed later on.

It doesn't support merging of specs having different OpenAPI versions (Kibana's OpenAPI specs use version 3.0.x but we should keep an eye on that)
It doesn't support top level $ref for
- Path item
- Request body
- Responses

elasticmachine · 2024-07-15T11:26:01Z

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

elasticmachine · 2024-07-15T11:26:03Z

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

elasticmachine · 2024-07-15T11:26:04Z

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

xcrzx

Thanks, @maximpn! The changes look good to me. Leaving just a couple of minor comments

xcrzx · 2024-07-16T10:55:19Z

packages/kbn-openapi-bundler/src/bundler/merge_documents/merge_arrays.ts

+      continue;
+    }
+
+    merged.push(item);


Let's avoid mutating function arguments. That often leads to many kinds of subtle bugs. Instead, a new array can be easily created and returned here.

xcrzx · 2024-07-16T11:01:48Z

packages/kbn-openapi-bundler/src/bundler/merge_documents/merge_arrays.ts

+
+export function mergeArrays(source: unknown[], merged: unknown[]): void {
+  for (const item of source) {
+    const existing = merged.find((x) => deepEqual(x, item));


Nit: merged.find is a bit more semantically suitable in this case, as we don't need to access the found element, but only need to check if there's an element in the array that satisfies our predicate.

Did you mean merged.some()?

I changed it to merged.some().

xcrzx · 2024-07-16T11:42:29Z

packages/kbn-openapi-bundler/src/bundler/merge_documents/merge_shared_components.ts

+  try {
+    return extractByJsonPointer(resolvedDocument.document, sourcePointer);
+  } catch {
+    return;
  }


Should we at least log the errors here? Silently ignoring them will make the code hard to debug.

Not all sourcePointers may exist. For example path item's requestBodies field is optional. I added debug logging here to have traces for possible debugging.

xcrzx · 2024-07-16T11:50:27Z

packages/kbn-openapi-bundler/src/bundler/merge_documents/merge_shared_components.ts

+      const componentToAdd = object[name];
+      const existingComponent = merged[name];
+
+      if (existingComponent && !deepEqual(componentToAdd, existingComponent)) {


What is the case for checking if there are two components defined in different schemas but they are the same? Shouldn't we just throw an error in that case?

It turned out that sometimes it's convenient to have duplicated schema. For example ExceptionListType is defined in Detections API OpenAPI specs and in Exceptions API OpenAPI specs. Referencing a file locating quite far could lead to long relative paths. It can be mitigated by using packages for OpenAPI specs which was discussed in #184859.

I added logging a warning when duplicated components are encountered.

elasticmachine · 2024-07-16T21:36:59Z

💚 Build Succeeded

Buildkite Build
Commit: 79778bc

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/openapi-bundler`	6	12	+6

Unknown metric groups

API count

id	before	after	diff
`@kbn/openapi-bundler`	6	12	+6

History

💔 Build #221825 failed 274f9376b44f165d3384e708f1f83938593f69df
💛 Build #221433 was flaky 4db41b9e39267289c49f20f65c56c086a3e50005
💔 Build #221398 failed 57cd5941e6a570c820bb47eb37705093059289e3
💔 Build #221135 failed 8854c7d2d420e1716ec8ad01a311206d16b16dbf

cc @maximpn

…unctionality (#188812) **Resolves:** #188817 ## Summary This PR adds automatic shared components conflict resolution functionality for OpenAPI merger. It boils down to a similar result as `npx @redocly/cli join --prefix-components-with-info-prop title` produces by prefixing shared components with document's title in each source. OpenAPI bundler intentionally won't solve conflicts automatically since it's focused on bundling domain APIs where conflicts are usually indicators of upstream problems. ## Details While working with various OpenAPI specs it may happen that different specs use exactly the same name for some shared components but different definitions. It must be avoided inside one API domain but it's a usual situation when merging OpenAPI specs of different API domains. For example domains may define a shared `Id` or `404Response` schemas where `Id` is a string in one domain and a number in another. OpenAPI merger implemented in #188110 and OpenAPI bundler implemented in #171526 do not solve shared components related conflicts automatically. It works perfectly for a single API domain forcing engineers choosing shared schema names carefully. This PR adds automatic shared components conflict resolution for OpenAPI merger. It prefixes shared component names with a normalized document's title. OpenAPI bundler intentionally won't solve conflicts automatically since it's focused on bundling domain APIs where conflicts are usually indicators of upstream problems. ## Example Consider two following OpenAPI specs each defining local `MySchema` **spec1.schema.yaml** ```yaml openapi: 3.0.3 info: title: My endpoint version: '2023-10-31' paths: /api/some_api: get: operationId: MyEndpointGet responses: '200': content: application/json: schema: $ref: '#/components/schemas/MySchema' components: schemas: MySchema: type: string enum: - value1 ``` **spec2.schema.yaml** ```yaml openapi: 3.0.3 info: title: My another endpoint version: '2023-10-31' paths: /api/another_api: get: operationId: MyAnotherEndpointGet responses: '200': content: application/json: schema: $ref: '#/components/schemas/MySchema' components: schemas: MySchema: type: number ``` and a script to merge them ```js require('../../src/setup_node_env'); const { resolve } = require('path'); const { merge } = require('@kbn/openapi-bundler'); const { REPO_ROOT } = require('@kbn/repo-info'); (async () => { await merge({ sourceGlobs: [ `${REPO_ROOT}/oas_docs/spec1.schema.yaml`, `${REPO_ROOT}/oas_docs/spec2.schema.yaml`, ], outputFilePath: resolve(`${REPO_ROOT}/oas_docs/merged.yaml`), options: { mergedSpecInfo: { title: 'Merge result', version: 'my version', }, }, }); })(); ``` will be merged successfully to **merged.yaml** ```yaml openapi: 3.0.3 info: title: Merge result version: 'my version' paths: /api/another_api: get: operationId: MyAnotherEndpointGet responses: '200': content: application/json; Elastic-Api-Version=2023-10-31: schema: $ref: '#/components/schemas/My_another_endpoint_MySchema' /api/some_api: get: operationId: MyEndpointGet responses: '200': content: application/json; Elastic-Api-Version=2023-10-31: schema: $ref: '#/components/schemas/My_endpoint_MySchema' components: schemas: My_another_endpoint_MySchema: type: number My_endpoint_MySchema: enum: - value1 type: string ```

**Addresses:** #186356 **Relates to:** #184428 ## Summary This PR adds a merging JS script based on the utility implemented in #186356. Resulted OpenAPI bundle as committed in `oas_docs/output/kibana.serverless.bundled.yaml`. ## Details #188110 implements and exposes `merge` utility design to merge source OpenAPI specs without processing. It's has only a programmatic API. To merge OpenAPI specs it's required to add a JS script like below ```js const { merge } = require('@kbn/openapi-bundler'); (async () => { await merge({ sourceGlobs: [/* a list of source globs goes here */], outputFilePath: 'path/to/the/output/file.yaml', }); })(); ``` The JS script added in this PR includes source OpenAPI specs presented in `oas_docs/makefile` plus Security Solution OpenAPI specs based on #184428. **To run** the script use the following command from Kibana root folder ```bash node ./oas_docs/scripts/merge_serverless_oas.js ``` ## Known linting issues with Security Solution OpenAPI specs Running Spectral OpenAPI linter on the result bundle shows a number of errors caused by `no-$ref-siblings` rule. This caused by the current code generator implementation which requires `default` property to be set next to `$ref` though it's not correct for OpenAPI `3.0.3` while it's allowed in `3.1`. It seems that Bump.sh handles such cases properly though by properly showing a default value. We need to analyze the problem and decide if/when we should fix it. The rest of warnings look fixable and will be addressed in the next stage after setting up linter rules. ## Next steps Since `@kbn/openapi-bundler` package is tailored specifically for Kibana it should replace Redocly currently used to merge OpenAPI specs. It also means `oas_docs/makefile` should be superseded by JS script(s) using `merge` utility form `@kbn/openapi-bundler` package. `@kbn/openapi-bundler` SHOULD NOT replace OpenAPI linters since it doesn't perform thorough linting. It's good if we continue adopting `spectral-cli` for linting purposes.

…c#189262) **Addresses:** elastic#186356 **Relates to:** elastic#184428 ## Summary This PR adds a merging JS script based on the utility implemented in elastic#186356. Resulted OpenAPI bundle as committed in `oas_docs/output/kibana.serverless.bundled.yaml`. ## Details elastic#188110 implements and exposes `merge` utility design to merge source OpenAPI specs without processing. It's has only a programmatic API. To merge OpenAPI specs it's required to add a JS script like below ```js const { merge } = require('@kbn/openapi-bundler'); (async () => { await merge({ sourceGlobs: [/* a list of source globs goes here */], outputFilePath: 'path/to/the/output/file.yaml', }); })(); ``` The JS script added in this PR includes source OpenAPI specs presented in `oas_docs/makefile` plus Security Solution OpenAPI specs based on elastic#184428. **To run** the script use the following command from Kibana root folder ```bash node ./oas_docs/scripts/merge_serverless_oas.js ``` ## Known linting issues with Security Solution OpenAPI specs Running Spectral OpenAPI linter on the result bundle shows a number of errors caused by `no-$ref-siblings` rule. This caused by the current code generator implementation which requires `default` property to be set next to `$ref` though it's not correct for OpenAPI `3.0.3` while it's allowed in `3.1`. It seems that Bump.sh handles such cases properly though by properly showing a default value. We need to analyze the problem and decide if/when we should fix it. The rest of warnings look fixable and will be addressed in the next stage after setting up linter rules. ## Next steps Since `@kbn/openapi-bundler` package is tailored specifically for Kibana it should replace Redocly currently used to merge OpenAPI specs. It also means `oas_docs/makefile` should be superseded by JS script(s) using `merge` utility form `@kbn/openapi-bundler` package. `@kbn/openapi-bundler` SHOULD NOT replace OpenAPI linters since it doesn't perform thorough linting. It's good if we continue adopting `spectral-cli` for linting purposes.

…88110) **Addresses:** elastic#186356 ## Summary This PR adds OpenAPI spec files merger utility (programmatic API). It provides a similar functionality as `npx @redocly/cli join` does and takes into account [discussion results](elastic#183019 (comment)) - provides a simple way to produce a single Kibana OpenAPI bundle - extends `requestBody` and `responses` MIME types with a version parameters `Elastic-Api-Version=<version>` to avoid different API endpoint versions conflicts - has flexibility to adjust Kibana needs The utility is exposed from `kbn-openapi-bundler` package. ## Details **OpenAPI merger** is a tool for merging multiple OpenAPI specification files. It's useful to merge already processed specification files to produce a result bundle. **OpenAPI bundler** uses the merger under the hood to merge bundled OpenAPI specification files. Exposed externally merger is a wrapper of the bundler's merger but extended with an ability to parse JSON files and forced to produce a single result file. It is able to read OpenAPI spec files defined in JSON and YAML formats. The result file is always written in YAML format where every `requestBody` and response in `responses` extended with document's `info.version` value added as a MIME type parameter `Elastic-Api-Version=<version>`. Currently package supports only programmatic API. As the next step you need to create a JavaScript script file like below ```ts require('../../src/setup_node_env'); const { resolve } = require('path'); const { merge } = require('@kbn/openapi-bundler'); const { REPO_ROOT } = require('@kbn/repo-info'); (async () => { await merge({ sourceGlobs: [ `${REPO_ROOT}/my/path/to/spec1.json`, `${REPO_ROOT}/my/path/to/spec2.yml`, `${REPO_ROOT}/my/path/to/spec3.yaml`, ], outputFilePath: `${REPO_ROOT}/oas_docs/bundle.yaml`, mergedSpecInfo: { title: 'My merged OpenAPI specs', version: '1.0.0', }, }); })(); ``` Finally you should be able to run OpenAPI merger via ```bash node ./path/to/the/script.js ``` or it could be added to `package.json` and run via `yarn`. After running the script it will log different information and write a merged OpenAPI specification to a the provided path. ## Caveats Items below don't look critical at the moment and can be addressed later on. - It doesn't support merging of specs having different OpenAPI versions (Kibana's OpenAPI specs use version `3.0.x` but we should keep an eye on that) - It doesn't support top level `$ref` for - Path item - Request body - Responses

…unctionality (elastic#188812) **Resolves:** elastic#188817 This PR adds automatic shared components conflict resolution functionality for OpenAPI merger. It boils down to a similar result as `npx @redocly/cli join --prefix-components-with-info-prop title` produces by prefixing shared components with document's title in each source. OpenAPI bundler intentionally won't solve conflicts automatically since it's focused on bundling domain APIs where conflicts are usually indicators of upstream problems. While working with various OpenAPI specs it may happen that different specs use exactly the same name for some shared components but different definitions. It must be avoided inside one API domain but it's a usual situation when merging OpenAPI specs of different API domains. For example domains may define a shared `Id` or `404Response` schemas where `Id` is a string in one domain and a number in another. OpenAPI merger implemented in elastic#188110 and OpenAPI bundler implemented in elastic#171526 do not solve shared components related conflicts automatically. It works perfectly for a single API domain forcing engineers choosing shared schema names carefully. This PR adds automatic shared components conflict resolution for OpenAPI merger. It prefixes shared component names with a normalized document's title. OpenAPI bundler intentionally won't solve conflicts automatically since it's focused on bundling domain APIs where conflicts are usually indicators of upstream problems. Consider two following OpenAPI specs each defining local `MySchema` **spec1.schema.yaml** ```yaml openapi: 3.0.3 info: title: My endpoint version: '2023-10-31' paths: /api/some_api: get: operationId: MyEndpointGet responses: '200': content: application/json: schema: $ref: '#/components/schemas/MySchema' components: schemas: MySchema: type: string enum: - value1 ``` **spec2.schema.yaml** ```yaml openapi: 3.0.3 info: title: My another endpoint version: '2023-10-31' paths: /api/another_api: get: operationId: MyAnotherEndpointGet responses: '200': content: application/json: schema: $ref: '#/components/schemas/MySchema' components: schemas: MySchema: type: number ``` and a script to merge them ```js require('../../src/setup_node_env'); const { resolve } = require('path'); const { merge } = require('@kbn/openapi-bundler'); const { REPO_ROOT } = require('@kbn/repo-info'); (async () => { await merge({ sourceGlobs: [ `${REPO_ROOT}/oas_docs/spec1.schema.yaml`, `${REPO_ROOT}/oas_docs/spec2.schema.yaml`, ], outputFilePath: resolve(`${REPO_ROOT}/oas_docs/merged.yaml`), options: { mergedSpecInfo: { title: 'Merge result', version: 'my version', }, }, }); })(); ``` will be merged successfully to **merged.yaml** ```yaml openapi: 3.0.3 info: title: Merge result version: 'my version' paths: /api/another_api: get: operationId: MyAnotherEndpointGet responses: '200': content: application/json; Elastic-Api-Version=2023-10-31: schema: $ref: '#/components/schemas/My_another_endpoint_MySchema' /api/some_api: get: operationId: MyEndpointGet responses: '200': content: application/json; Elastic-Api-Version=2023-10-31: schema: $ref: '#/components/schemas/My_endpoint_MySchema' components: schemas: My_another_endpoint_MySchema: type: number My_endpoint_MySchema: enum: - value1 type: string ```

…c#189262) **Addresses:** elastic#186356 **Relates to:** elastic#184428 This PR adds a merging JS script based on the utility implemented in elastic#186356. Resulted OpenAPI bundle as committed in `oas_docs/output/kibana.serverless.bundled.yaml`. elastic#188110 implements and exposes `merge` utility design to merge source OpenAPI specs without processing. It's has only a programmatic API. To merge OpenAPI specs it's required to add a JS script like below ```js const { merge } = require('@kbn/openapi-bundler'); (async () => { await merge({ sourceGlobs: [/* a list of source globs goes here */], outputFilePath: 'path/to/the/output/file.yaml', }); })(); ``` The JS script added in this PR includes source OpenAPI specs presented in `oas_docs/makefile` plus Security Solution OpenAPI specs based on elastic#184428. **To run** the script use the following command from Kibana root folder ```bash node ./oas_docs/scripts/merge_serverless_oas.js ``` Running Spectral OpenAPI linter on the result bundle shows a number of errors caused by `no-$ref-siblings` rule. This caused by the current code generator implementation which requires `default` property to be set next to `$ref` though it's not correct for OpenAPI `3.0.3` while it's allowed in `3.1`. It seems that Bump.sh handles such cases properly though by properly showing a default value. We need to analyze the problem and decide if/when we should fix it. The rest of warnings look fixable and will be addressed in the next stage after setting up linter rules. Since `@kbn/openapi-bundler` package is tailored specifically for Kibana it should replace Redocly currently used to merge OpenAPI specs. It also means `oas_docs/makefile` should be superseded by JS script(s) using `merge` utility form `@kbn/openapi-bundler` package. `@kbn/openapi-bundler` SHOULD NOT replace OpenAPI linters since it doesn't perform thorough linting. It's good if we continue adopting `spectral-cli` for linting purposes.

…189262) (#190541) # Backport This will backport the bundler aspects of the following commits from `main` to `8.15`: - [[HTTP/OAS] Merge OpenAPI specs by using `kbn-openapi-bundler` (#189262)](#189262) - #189621 - #189482 - #189348 - #189472 - #188110 - #188812  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  --------- Co-authored-by: Maxim Palenov <[email protected]>

maximpn self-assigned this Jul 11, 2024

maximpn force-pushed the oas-merger branch 3 times, most recently from 7c39e7d to c6df87d Compare July 12, 2024 11:29

maximpn changed the title ~~[Security Solution] Add OpenAPI specs merger utility~~ [Security Solution] Implement OpenAPI specs merger utility Jul 12, 2024

maximpn force-pushed the oas-merger branch 2 times, most recently from 8854c7d to 57cd594 Compare July 15, 2024 10:59

maximpn requested a review from xcrzx July 15, 2024 11:22

maximpn marked this pull request as ready for review July 15, 2024 11:25

maximpn requested a review from a team as a code owner July 15, 2024 11:25

xcrzx approved these changes Jul 16, 2024

View reviewed changes

maximpn force-pushed the oas-merger branch from 4db41b9 to 274f937 Compare July 16, 2024 16:03

maximpn added 8 commits July 16, 2024 21:53

oas merger

9437e4f

oas merger

9723d09

update README

60900cd

add merger tests

d7f5f75

fix a misprint

1fbeada

avoid array mutation

241e75f

improve logging

6c54341

revert back extractObjectToMerge changes

79778bc

maximpn force-pushed the oas-merger branch from 274f937 to 79778bc Compare July 16, 2024 20:07

maximpn enabled auto-merge (squash) July 16, 2024 21:07

maximpn merged commit f02a41c into elastic:main Jul 16, 2024
19 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Jul 16, 2024

maximpn deleted the oas-merger branch July 17, 2024 06:46

maximpn mentioned this pull request Jul 26, 2024

[HTTP/OAS] Merge OpenAPI specs by using kbn-openapi-bundler #189262

Merged

lcawl mentioned this pull request Aug 14, 2024

[8.15] [HTTP/OAS] Merge OpenAPI specs by using kbn-openapi-bundler (#189262) #190541

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Implement OpenAPI specs merger utility #188110

[Security Solution] Implement OpenAPI specs merger utility #188110

maximpn commented Jul 11, 2024 •

edited

Loading

elasticmachine commented Jul 15, 2024

elasticmachine commented Jul 15, 2024

elasticmachine commented Jul 15, 2024

xcrzx left a comment

xcrzx Jul 16, 2024

xcrzx Jul 16, 2024

maximpn Jul 16, 2024

xcrzx Jul 16, 2024

maximpn Jul 16, 2024 •

edited

Loading

xcrzx Jul 16, 2024

maximpn Jul 16, 2024

elasticmachine commented Jul 16, 2024

API count

[Security Solution] Implement OpenAPI specs merger utility #188110

[Security Solution] Implement OpenAPI specs merger utility #188110

Conversation

maximpn commented Jul 11, 2024 • edited Loading

Summary

Details

Caveats

elasticmachine commented Jul 15, 2024

elasticmachine commented Jul 15, 2024

elasticmachine commented Jul 15, 2024

xcrzx left a comment

Choose a reason for hiding this comment

xcrzx Jul 16, 2024

Choose a reason for hiding this comment

xcrzx Jul 16, 2024

Choose a reason for hiding this comment

maximpn Jul 16, 2024

Choose a reason for hiding this comment

xcrzx Jul 16, 2024

Choose a reason for hiding this comment

maximpn Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

xcrzx Jul 16, 2024

Choose a reason for hiding this comment

maximpn Jul 16, 2024

Choose a reason for hiding this comment

elasticmachine commented Jul 16, 2024

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

API count

History

maximpn commented Jul 11, 2024 •

edited

Loading

maximpn Jul 16, 2024 •

edited

Loading