Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HTTP/OAS] Ability to exclude routes from introspection #192675

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Sep 12, 2024

Summary

Certain routes, like "system" routes should be excludable from introspection. Among these are also "HTTP resource" routes that are intended to server rendered HTML, CSS or JS.

To this end a new registration option was added: excludeFromOAS: boolean that will exclude routes from OAS documents created by Kibana.

@jloleysens
Copy link
Contributor Author

/ci

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #97 / data plugin Session management Dashboard starts a session on filter change

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/router-to-openapispec 5 10 +5
Unknown metric groups

API count

id before after diff
@kbn/core-http-server 495 496 +1
@kbn/router-to-openapispec 5 10 +5
total +6

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

@jloleysens
Copy link
Contributor Author

/ci

let filters: GenerateOpenApiDocumentOptionsFilters;
let query: TypeOf<typeof querySchema>;
try {
query = querySchema.validate(req.query);
Copy link
Contributor Author

@jloleysens jloleysens Oct 9, 2024

Choose a reason for hiding this comment

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

Refactor to using config-schema, just cleanup

@jloleysens jloleysens marked this pull request as ready for review October 9, 2024 11:01
@jloleysens jloleysens requested a review from a team as a code owner October 9, 2024 11:01
@jloleysens jloleysens added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v9.0.0 Feature:OAS Work or issues related to Core-provided mechanisms for generating OAS v8.16.0 backport:version Backport to applied version labels labels Oct 9, 2024
@elasticmachine
Copy link
Contributor

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

@jloleysens jloleysens added the release_note:skip Skip the PR/issue when compiling release notes label Oct 9, 2024
>(
routes: R[],
filters: GenerateOpenApiDocumentOptionsFilters = {}
): R[] => {
if (Object.getOwnPropertyNames(filters).length === 0) return routes;
return routes.filter((route) => {
if (route.options.excludeFromOAS) return false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where we actually exclude routes from OAS docs.

Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

Added a nit you can ignore or address as you see fit. otherwise LGTM

@@ -237,7 +238,7 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
if (excludeVersionedRoutes) {
return this.routes.filter((route) => !route.isVersioned);
}
return [...this.routes];
return this.routes;
Copy link
Member

Choose a reason for hiding this comment

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

probably unnecessary but this might remove a potential mutation guard we accedentaly had

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah man, I somehow missed this. That's a good point, will fix.

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #102 / Fleet Endpoints Integrations inputs_with_standalone_docker_agent generate a valid config for standalone agents

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/router-to-openapispec 5 10 +5
Unknown metric groups

API count

id before after diff
@kbn/core-http-server 530 531 +1
@kbn/router-to-openapispec 5 10 +5
total +6

History

@jloleysens jloleysens merged commit 2327681 into elastic:main Oct 9, 2024
23 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 9, 2024
… (#195652)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[HTTP/OAS] Ability to exclude routes from introspection
(#192675)](#192675)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jean-Louis
Leysens","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-09T16:31:42Z","message":"[HTTP/OAS]
Ability to exclude routes from introspection
(#192675)","sha":"2327681de7306c20bcca69fe77660c0a586c979d","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","release_note:skip","v9.0.0","Feature:OAS","v8.16.0","backport:version"],"title":"[HTTP/OAS]
Ability to exclude routes from
introspection","number":192675,"url":"https://github.com/elastic/kibana/pull/192675","mergeCommit":{"message":"[HTTP/OAS]
Ability to exclude routes from introspection
(#192675)","sha":"2327681de7306c20bcca69fe77660c0a586c979d"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192675","number":192675,"mergeCommit":{"message":"[HTTP/OAS]
Ability to exclude routes from introspection
(#192675)","sha":"2327681de7306c20bcca69fe77660c0a586c979d"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Jean-Louis Leysens <[email protected]>
jloleysens added a commit that referenced this pull request Oct 10, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 10, 2024
## Summary

Small follow up based on
elastic#192675 (comment)

(cherry picked from commit a481da6)
kibanamachine added a commit that referenced this pull request Oct 10, 2024
…195714)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[HTTP] Copy array returned by &#x60;getRoutes&#x60;
(#195647)](#195647)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jean-Louis
Leysens","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-10T08:53:59Z","message":"[HTTP]
Copy array returned by `getRoutes` (#195647)\n\n## Summary\r\n\r\nSmall
follow up based
on\r\nhttps://github.com//pull/192675#discussion_r1793601519","sha":"a481da68e58cb20d6407c9866c1511717addfdb0","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:http","Team:Core","release_note:skip","v9.0.0","v8.16.0","backport:version"],"title":"[HTTP]
Copy array returned by
`getRoutes`","number":195647,"url":"https://github.com/elastic/kibana/pull/195647","mergeCommit":{"message":"[HTTP]
Copy array returned by `getRoutes` (#195647)\n\n## Summary\r\n\r\nSmall
follow up based
on\r\nhttps://github.com//pull/192675#discussion_r1793601519","sha":"a481da68e58cb20d6407c9866c1511717addfdb0"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195647","number":195647,"mergeCommit":{"message":"[HTTP]
Copy array returned by `getRoutes` (#195647)\n\n## Summary\r\n\r\nSmall
follow up based
on\r\nhttps://github.com//pull/192675#discussion_r1793601519","sha":"a481da68e58cb20d6407c9866c1511717addfdb0"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Jean-Louis Leysens <[email protected]>
jloleysens added a commit that referenced this pull request Oct 15, 2024
…#195940)

## Summary

Follow up on #195464

Adds public route registrar option `httpResource` to distinguish API
routes from routes intended to be used for loading resources, [for
ex](https://github.com/elastic/kibana/blob/bd22f1370fc55179ea6f2737176570176f700b6e/x-pack/plugins/security/server/routes/authentication/oidc.ts#L36).

This enables us to avoid returning the version header
`elastic-api-version` for HTTP resource routes.

It's still possible for API authors to use the versioned router for
things that should be HTTP resources, but it's assumed that all routes
registered through HTTP resources services are:

1. Public
2. Not versioned (focus of this PR)
3. Not documented (done in
#192675)

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 15, 2024
…elastic#195940)

## Summary

Follow up on elastic#195464

Adds public route registrar option `httpResource` to distinguish API
routes from routes intended to be used for loading resources, [for
ex](https://github.com/elastic/kibana/blob/bd22f1370fc55179ea6f2737176570176f700b6e/x-pack/plugins/security/server/routes/authentication/oidc.ts#L36).

This enables us to avoid returning the version header
`elastic-api-version` for HTTP resource routes.

It's still possible for API authors to use the versioned router for
things that should be HTTP resources, but it's assumed that all routes
registered through HTTP resources services are:

1. Public
2. Not versioned (focus of this PR)
3. Not documented (done in
elastic#192675)

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 72f3d2d)
kibanamachine added a commit that referenced this pull request Oct 15, 2024
…header (#195940) (#196324)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Http] Make HTTP resource routes respond without the versioned header
(#195940)](#195940)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jean-Louis
Leysens","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-15T14:09:42Z","message":"[Http]
Make HTTP resource routes respond without the versioned header
(#195940)\n\n## Summary\r\n\r\nFollow up on
https://github.com/elastic/kibana/pull/195464\r\n\r\nAdds public route
registrar option `httpResource` to distinguish API\r\nroutes from routes
intended to be used for loading resources,
[for\r\nex](https://github.com/elastic/kibana/blob/bd22f1370fc55179ea6f2737176570176f700b6e/x-pack/plugins/security/server/routes/authentication/oidc.ts#L36).\r\n\r\nThis
enables us to avoid returning the version
header\r\n`elastic-api-version` for HTTP resource routes.\r\n\r\nIt's
still possible for API authors to use the versioned router for\r\nthings
that should be HTTP resources, but it's assumed that all
routes\r\nregistered through HTTP resources services are:\r\n\r\n1.
Public\r\n2. Not versioned (focus of this PR)\r\n3. Not documented (done
in\r\nhttps://github.com//pull/192675)\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"72f3d2d3491f6da4b0c9147e766635e9dbb9cbe8","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:http","Team:Core","release_note:skip","v9.0.0","v8.16.0","backport:version"],"title":"[Http]
Make HTTP resource routes respond without the versioned
header","number":195940,"url":"https://github.com/elastic/kibana/pull/195940","mergeCommit":{"message":"[Http]
Make HTTP resource routes respond without the versioned header
(#195940)\n\n## Summary\r\n\r\nFollow up on
https://github.com/elastic/kibana/pull/195464\r\n\r\nAdds public route
registrar option `httpResource` to distinguish API\r\nroutes from routes
intended to be used for loading resources,
[for\r\nex](https://github.com/elastic/kibana/blob/bd22f1370fc55179ea6f2737176570176f700b6e/x-pack/plugins/security/server/routes/authentication/oidc.ts#L36).\r\n\r\nThis
enables us to avoid returning the version
header\r\n`elastic-api-version` for HTTP resource routes.\r\n\r\nIt's
still possible for API authors to use the versioned router for\r\nthings
that should be HTTP resources, but it's assumed that all
routes\r\nregistered through HTTP resources services are:\r\n\r\n1.
Public\r\n2. Not versioned (focus of this PR)\r\n3. Not documented (done
in\r\nhttps://github.com//pull/192675)\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"72f3d2d3491f6da4b0c9147e766635e9dbb9cbe8"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195940","number":195940,"mergeCommit":{"message":"[Http]
Make HTTP resource routes respond without the versioned header
(#195940)\n\n## Summary\r\n\r\nFollow up on
https://github.com/elastic/kibana/pull/195464\r\n\r\nAdds public route
registrar option `httpResource` to distinguish API\r\nroutes from routes
intended to be used for loading resources,
[for\r\nex](https://github.com/elastic/kibana/blob/bd22f1370fc55179ea6f2737176570176f700b6e/x-pack/plugins/security/server/routes/authentication/oidc.ts#L36).\r\n\r\nThis
enables us to avoid returning the version
header\r\n`elastic-api-version` for HTTP resource routes.\r\n\r\nIt's
still possible for API authors to use the versioned router for\r\nthings
that should be HTTP resources, but it's assumed that all
routes\r\nregistered through HTTP resources services are:\r\n\r\n1.
Public\r\n2. Not versioned (focus of this PR)\r\n3. Not documented (done
in\r\nhttps://github.com//pull/192675)\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"72f3d2d3491f6da4b0c9147e766635e9dbb9cbe8"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Jean-Louis Leysens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:OAS Work or issues related to Core-provided mechanisms for generating OAS release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants