Skip to content

[Http] Make HTTP resource routes respond without the versioned header#195940

Merged
jloleysens merged 18 commits intoelastic:mainfrom
jloleysens:http/fix-http-resources-responses
Oct 15, 2024
Merged

[Http] Make HTTP resource routes respond without the versioned header#195940
jloleysens merged 18 commits intoelastic:mainfrom
jloleysens:http/fix-http-resources-responses

Conversation

@jloleysens
Copy link
Copy Markdown
Contributor

@jloleysens jloleysens commented Oct 11, 2024

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.

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 [HTTP/OAS] Ability to exclude routes from introspection #192675)

Checklist

@jloleysens jloleysens added Feature:http Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// release_note:skip Skip the PR/issue when compiling release notes v9.0.0 v8.16.0 backport:version Backport to applied version labels labels Oct 11, 2024
Comment thread src/core/server/integration_tests/http_resources/http_resources_service.test.ts Outdated
@jloleysens
Copy link
Copy Markdown
Contributor Author

/ci


await root.start();
const { header } = await request.get(root, '/render-html').expect(200);
expect(header).not.toMatchObject({ 'elastic-api-version': expect.any(String) });
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Making this test work is the main focus of the PR.

@jloleysens jloleysens self-assigned this Oct 11, 2024
@jloleysens
Copy link
Copy Markdown
Contributor Author

/ci

@jloleysens
Copy link
Copy Markdown
Contributor Author

/ci

@jloleysens jloleysens marked this pull request as ready for review October 15, 2024 08:12
@jloleysens jloleysens requested a review from a team as a code owner October 15, 2024 08:12
@elasticmachine
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

🧡

@jloleysens jloleysens enabled auto-merge (squash) October 15, 2024 12:31
@jloleysens jloleysens merged commit 72f3d2d into elastic:main Oct 15, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

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

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #106 / discover/group2/data_grid1 discover data grid context tests should open the context view with the selected document as anchor

Metrics [docs]

Unknown metric groups

API count

id before after diff
@kbn/core-http-server 531 532 +1

History

cc @jloleysens

kibanamachine added 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 <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 72f3d2d)
@kibanamachine
Copy link
Copy Markdown
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 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":"jeanlouis.leysens@elastic.co"},"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
<42973632+kibanamachine@users.noreply.github.com>","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
<42973632+kibanamachine@users.noreply.github.com>","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
<42973632+kibanamachine@users.noreply.github.com>","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 <jeanlouis.leysens@elastic.co>
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:http release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.16.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants