[HTTP/OAS] Commit OAS snapshot by jloleysens · Pull Request #183338 · elastic/kibana

jloleysens · 2024-05-13T20:29:49Z

Close #181992

Summary

First iteration of a CLI to capture an OAS snapshot.

How to test

Run node ./scripts/capture_oas_snapshot.js --update --include-path /api/status and see result in oas_docs/bundle.json.

If you have the bump CLI installed you can preview the hosted output with bump preview ./oas_docs/bundle.json

Notes

Added ability to filter by version, access (public/internal) and excluding paths explicitly to the OAS generation lib
Follows the same general pattern as our other "capture" CLIs like packages/kbn-check-mappings-update-cli
Result includes only /api/status for now, waiting for other paths to add missing parts

…spec per run...

jloleysens · 2024-05-13T20:32:47Z

src/core/server/integration_tests/http/oas.test.ts

    expect(result.status).toBe(200);
    expect(result.body).toMatchObject(includes);
-    expect(result.body).not.toMatchObject(excludes);
+    excludes.forEach((exclude) => {


So... this test was not working as expected 🙈

…aram path names

jloleysens · 2024-05-21T12:51:45Z

packages/core/http/core-http-server-internal/src/http_service.ts

                  title: 'Kibana HTTP APIs',
                  version: '0.0.0', // TODO get a better version here
-                  pathStartsWith,
+                  filters: { pathStartsWith, excludePathsMatching, access, version },


Added a few more filtering options. Most notably: excludePathsMatching, access and version

jloleysens · 2024-05-21T12:53:12Z

packages/kbn-capture-oas-snapshot-cli/src/kibana.ts

+ * (dodges issues with open handles), and so that we can harvest logs and feed them into
+ * the logger when debugging.
+ */
+export function kibana(log: SomeDevLog) {


Mostly copy-pasta from packages/kbn-check-mappings-update-cli/src/compatibility/extract_mappings_from_plugins.ts

Oh yeah, that whole forking the process logic again 🙈.

Note blocker in any way, but shall we try to factorize the process forking / worker code at some point (do we know how many time that code got duplicated)?

Just this time again to my knowledge, I guess we could factorize some parts of it. Do you think they could live in something like @kbn/dev-utils?

packages/kbn-capture-oas-snapshot-cli/src/run_capture_oas_snapshot_cli.ts

jloleysens · 2024-05-21T12:54:34Z

packages/kbn-router-to-openapispec/src/generate_oas.test.util.ts

 const getRouterDefaults = () => ({
  isVersioned: false,
-  path: '/foo/{id}',
+  path: '/foo/{id}/{path*}',


Test special * syntax

jloleysens · 2024-05-21T12:55:39Z

oas_docs/bundle.json

❯ jq ".paths | length" ./oas_docs/bundle.json 141

Might want to consider starting smaller...

If size becomes an issue, we could always consider storing it in a different repo as long as we could also figure out the cadence for publishing it

I just meant it's a lot of paths that need to be audited/quality controlled 😅 . Pretty sure there is a lot of room to grow here. For a start we can just include the paths that are ready for docs.

lcawl · 2024-05-21T16:04:13Z

When I run linters on the bundled.json file, I get some errors that I think are noteworthy. For example:

Missing description in responses

[79] bundle.json:36327:11 at #/paths/~~1api~~1uptime~1settings/put/responses/200
The field description must be present on this level.

This relates to the required field described in https://spec.openapis.org/oas/v3.0.3#response-object

Nullable fields

[77] bundle.json:35842:39 at #/paths/~~1api~~1telemetry~~1v2~~1config/get/responses/200/content/application~1json; Elastic-Api-Version=2023-10-31/schema/properties/optIn/anyOf/1/nullable
The type field must be defined when the nullable field is used.

I think is just related to the placement of the nullable:true qualifiers.

Undefined parameters

[72] bundle.json:34683:9 at > #/paths/~~1api~~1osquery~~1saved_queries~~1{id}/delete/parameters
The operation does not define the path parameter {id} expected by path /api/osquery/saved_queries/{id}.

These seem to have missing definitions.

jloleysens · 2024-05-21T17:37:05Z

/ci

…fix'

jloleysens · 2024-05-24T10:29:47Z

@elasticmachine merge upstream

pgayvallet

LGTM, a few remarks and questions

packages/kbn-router-to-openapispec/src/process_router.ts

pgayvallet · 2024-05-29T09:43:53Z

packages/kbn-router-to-openapispec/src/process_versioned_router.ts

-    const schemas = handler ? extractValidationSchemaFromVersionedHandler(handler) : undefined;
+    let version: undefined | string;
+    let handler: undefined | VersionedRouterRoute['handlers'][0];
+    let versions: string[] = route.handlers.map(({ options: { version: v } }) => v).sort();


Not sure if relevant, but for internal versions, I think just using sort() may be too naive? E.g [1, 2, ...., 10].sort() => 1, 10, 2, 3...

And it seems we're then using that sorting for slicing L47

const versionIdx = versions.indexOf(filters.version); if (versionIdx === -1) return { paths }; versions = versions.slice(0, versionIdx + 1);

(but are we even generating for internal APIs?)

Yea, you found a bug in the logic, will fix!

See 8861024 and 407b3de

pgayvallet · 2024-05-29T09:53:33Z

.github/CODEOWNERS

 packages/kbn-calculate-auto @elastic/obs-ux-management-team
 packages/kbn-calculate-width-from-char-count @elastic/kibana-visualizations
 x-pack/plugins/canvas @elastic/kibana-presentation
+packages/kbn-capture-oas-snapshot-cli @elastic/kibana-core


Hum, this is strange, the files under that new packages aren't considered as owned by Core for the PR review 🤔. I had to remove the code owner filtering.... IIRC it used to work.

pgayvallet · 2024-05-29T09:55:47Z

packages/kbn-capture-oas-snapshot-cli/src/kibana.ts

+ * (dodges issues with open handles), and so that we can harvest logs and feed them into
+ * the logger when debugging.
+ */
+export function kibana(log: SomeDevLog) {


Oh yeah, that whole forking the process logic again 🙈.

Note blocker in any way, but shall we try to factorize the process forking / worker code at some point (do we know how many time that code got duplicated)?

…fix'

jloleysens · 2024-05-30T09:42:06Z

packages/core/http/core-http-router-server-internal/src/versioned_router/handler_resolvers.ts

+ * @example Given 'internal' versions ["1", "10", "2"] it will return ["1", "2", "10]
+ * @example Given 'public' versions ["2023-01-01", "2002-10-10", "2005-01-01"] it will return ["2002-10-10", "2005-01-01", "2023-01-01"]
+ */
+export const sort = (versions: string[], access: 'public' | 'internal') => {


Sort of related to this PR, @pgayvallet found a bug in this logic that was not treating internal versions correctly. Updated and tried to preserve existing APIs as far as possible.

jloleysens · 2024-05-30T09:42:40Z

packages/kbn-dev-utils/src/worker/index.ts

+ * Provide a TS file as the src of a NodeJS Worker with some built-in handling
+ * of std streams and debugging.
+ */
+export function startTSWorker<Message>({ log, src, cwd = REPO_ROOT }: StartTSWorkerArgs) {


New utility that does all the RX piping and starting of a node js worker for us.

kibana-ci · 2024-05-30T13:02:19Z

💚 Build Succeeded

Buildkite Build
Commit: badd079

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/core-http-router-server-internal`	48	54	+6
`@kbn/dev-utils`	32	33	+1
total			+7

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-http-router-server-internal`	48	54	+6
`@kbn/dev-utils`	36	38	+2
total			+8

History

💔 Build #213019 failed 407b3de
💚 Build #212762 succeeded b9b5f9e
💚 Build #212534 succeeded a284322
💚 Build #212350 succeeded 93527f0
💔 Build #211967 failed 66a2d0d

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

It's not an actual dependency of downstream steps, but was added more to provide as a cost cutting gate. However, there are times it delays downstream steps without any added value. Here's the provenance for `check_oas_snapshot` in the pull-request pipeline. ### 1. OAS snapshot capture first added to CI (inside "Checks") - [PR #183338](#183338) / commit [`975eeed255d0`](975eeed255d0) (May 30, 2024) — "[HTTP/OAS] Commit OAS snapshot" - Added `.buildkite/scripts/steps/capture_oas_snapshot.sh` and wired it into `.buildkite/scripts/steps/checks.sh` (so it ran as part of the Checks step, not a standalone pipeline step yet). ### 2. Standalone "Check OAS Snapshot" step added to PR pipeline - [PR #196534](#196534) / commit [`2fbc843e0d69`](2fbc843e0d69) (Oct 17, 2024) — "[ci] Extract OAS check + add retry" - Motivation (from the PR summary): the capture step was flaky and was breaking Checks; they extracted it into its own heavy step (starts ES+Kibana) and added retries. - This is where it gets added as its own step in `.buildkite/pipelines/pull_request/base.yml`, running `.buildkite/scripts/steps/checks/capture_oas_snapshot.sh`. ### 3. `depends_on: check_oas_snapshot` added across PR test suite pipelines - [PR #198452](#198452) / commit [`cbb211abe0cd`](cbb211abe0cd) (Nov 4, 2024) — "[ci] Run checks before tests" - This commit added `key: check_oas_snapshot` in `.buildkite/pipelines/pull_request/base.yml` and updated many PR suite pipeline YAMLs to depend on it (and on `checks`) before running. - Example change: `.buildkite/pipelines/pull_request/response_ops.yml` adds `- check_oas_snapshot` under `depends_on` in that commit. Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

It's not an actual dependency of downstream steps, but was added more to provide as a cost cutting gate. However, there are times it delays downstream steps without any added value. Here's the provenance for `check_oas_snapshot` in the pull-request pipeline. ### 1. OAS snapshot capture first added to CI (inside "Checks") - [PR elastic#183338](elastic#183338) / commit [`975eeed255d0`](elastic@975eeed255d0) (May 30, 2024) — "[HTTP/OAS] Commit OAS snapshot" - Added `.buildkite/scripts/steps/capture_oas_snapshot.sh` and wired it into `.buildkite/scripts/steps/checks.sh` (so it ran as part of the Checks step, not a standalone pipeline step yet). ### 2. Standalone "Check OAS Snapshot" step added to PR pipeline - [PR elastic#196534](elastic#196534) / commit [`2fbc843e0d69`](elastic@2fbc843e0d69) (Oct 17, 2024) — "[ci] Extract OAS check + add retry" - Motivation (from the PR summary): the capture step was flaky and was breaking Checks; they extracted it into its own heavy step (starts ES+Kibana) and added retries. - This is where it gets added as its own step in `.buildkite/pipelines/pull_request/base.yml`, running `.buildkite/scripts/steps/checks/capture_oas_snapshot.sh`. ### 3. `depends_on: check_oas_snapshot` added across PR test suite pipelines - [PR elastic#198452](elastic#198452) / commit [`cbb211abe0cd`](elastic@cbb211abe0cd) (Nov 4, 2024) — "[ci] Run checks before tests" - This commit added `key: check_oas_snapshot` in `.buildkite/pipelines/pull_request/base.yml` and updated many PR suite pipeline YAMLs to depend on it (and on `checks`) before running. - Example change: `.buildkite/pipelines/pull_request/response_ops.yml` adds `- check_oas_snapshot` under `depends_on` in that commit. Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

jloleysens added 6 commits May 13, 2024 14:42

added access filter

4cccfe3

updated generate openapi to generate only a single version of openapi…

f4d3ba1

…spec per run...

remove elastic-api-version string

ec05633

added test for new filter paramters

d85b619

ok, preserve behaviour of injecting version into content type...

c2a82ae

fix jest integration tests 🤦

6019074

jloleysens commented May 13, 2024

View reviewed changes

jloleysens added 12 commits May 14, 2024 12:20

added exclude paths capability

e39dd2a

added new option to API and updated tests

91c4a42

some further updates to support more parameters and handle stars in p…

cb81116

…aram path names

fix unit tests

456076a

wrap query strings in arrays

0e31b92

added unit test

be7066e

added capture snapshot CLI

465fc5c

big new OAS file

17ccc55

add PR step

f470ae8

add step script

9ffdb46

white space

b15e3df

remove comment

113a0cf

jloleysens commented May 21, 2024

View reviewed changes

jloleysens and others added 4 commits May 21, 2024 18:38

Merge branch 'main' into oas/write-bundled-file-to-disk

a5b6148

fix oas route

ada87a9

added cmd flags

6a02507

Merge branch 'main' into oas/write-bundled-file-to-disk

a2a7d89

jloleysens and others added 2 commits May 21, 2024 19:40

comment

6506925

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

a361593

…fix'

lcawl mentioned this pull request May 21, 2024

[HTTP] Make alerting APIs public #183945

Merged

bootstrap on CI

6458d98

jloleysens added the v8.15.0 label May 24, 2024

jloleysens and others added 2 commits May 24, 2024 10:27

remove individial CI step and include OAS snapshot in "checks" step

3053cfa

Merge branch 'main' into oas/write-bundled-file-to-disk

5449bd0

Merge branch 'main' into oas/write-bundled-file-to-disk

66a2d0d

jbudz approved these changes May 24, 2024

View reviewed changes

jloleysens added 2 commits May 27, 2024 12:35

Merge branch 'main' into oas/write-bundled-file-to-disk

93527f0

Merge branch 'main' into oas/write-bundled-file-to-disk

a284322

lcawl mentioned this pull request May 28, 2024

[HTTP/OAS] Add descriptions for role management APIs #184265

Merged

Merge branch 'main' into oas/write-bundled-file-to-disk

b9b5f9e

pgayvallet approved these changes May 30, 2024

View reviewed changes

jloleysens and others added 7 commits May 30, 2024 10:50

factorize ts worker logic

c218a09

move log

06c08f6

used and tests startTSWorker util in check mappings update too

2c996ac

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

fa02188

…fix'

remove ambiguous comment

675b863

fix sorting logic

8861024

update usages and also expose "sort" method for use in OAS logic

407b3de

jloleysens commented May 30, 2024

View reviewed changes

eslint

badd079

jloleysens enabled auto-merge (squash) May 30, 2024 11:32

jloleysens merged commit 975eeed into elastic:main May 30, 2024

kibanamachine added the backport:skip This PR does not require backporting label May 30, 2024

jloleysens deleted the oas/write-bundled-file-to-disk branch May 30, 2024 13:37

maximpn mentioned this pull request Jun 3, 2024

[Security Solution] Collect the first feedback on documentation bundling approaches #183019

Closed

4 tasks

tylersmalley mentioned this pull request Feb 14, 2026

[CI] Remove check_oas_snapshot as dependency #253156

Merged

Conversation

jloleysens commented May 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jloleysens May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcawl commented May 21, 2024

Uh oh!

jloleysens commented May 21, 2024

Uh oh!

jloleysens commented May 24, 2024

Uh oh!

pgayvallet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented May 30, 2024

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

API count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jloleysens commented May 13, 2024 •

edited

Loading

jloleysens May 21, 2024 •

edited

Loading

pgayvallet left a comment •

edited

Loading