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] Set explicit access for public HTTP APIs #192554

Merged
merged 11 commits into from
Sep 23, 2024

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Sep 11, 2024

Summary

We will be enforcing restricted access to internal HTTP APIs from 9.0. This PR is part 1 of audit checking that our public APIs have their access tag set explicitly to ensure they are still available to end users after we start enforcing HTTP API restrictions. APIs reviewed in this PR (docs):

Screenshot 2024-09-11 at 11 25 55

Note to reviewers

This audit is focussed on set access: 'public' where needed. Per the screenshot our public-facing documentation is taken as the source of truth for which APIs should be public. This may differ per offering so please consider whether a given HTTP API should be public on both serverless and stateful offerings.

Risks

  • If we miss an API that should be public, end users will encounter a 400 response when they try to use the HTTP API on 9.0
  • If we set an API's access to "public" it will not have the same restrictions applied to it.

@jloleysens jloleysens added Feature:http Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes v8.16.0 labels Sep 11, 2024
@jloleysens jloleysens self-assigned this Sep 11, 2024
@jloleysens jloleysens requested review from a team as code owners September 11, 2024 09:33
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor Author

@jloleysens jloleysens Sep 11, 2024

Choose a reason for hiding this comment

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

Technically this is public on stateful, because it is documented BUT since this is scheduled to be deleted for 9.0 we could also leave this unchanged for the remainder of 8. Open to just leave these legacy dashboard import/export routes unchanged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't really have a strong opinion as we'd be removing the whole route from main soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to leave the legacy dashboard import/export routes as 'internal'.

Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

We actually have an open issue to make spaces CRUD endpoints public in serverless #192153. But as that will involve updating the serverless API integration tests, we can just open a separate PR for that.

@jloleysens jloleysens changed the title [HTTP] Set explicit access [HTTP] Set explicit access for public HTTP APIs Sep 11, 2024
@jloleysens jloleysens added backport:version Backport to applied version labels v9.0.0 labels Sep 23, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't really have a strong opinion as we'd be removing the whole route from main soon.

@jloleysens jloleysens enabled auto-merge (squash) September 23, 2024 14:40
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

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-saved-objects-server-internal 70 72 +2
Unknown metric groups

API count

id before after diff
@kbn/core-saved-objects-server-internal 71 73 +2

History

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

cc @jloleysens

@jloleysens jloleysens merged commit 3fa5bdf into elastic:main Sep 23, 2024
27 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 23, 2024
## Summary

We will be enforcing restricted access to internal HTTP APIs [from
9.0](elastic#186781). This PR is part 1
of audit checking that our public APIs have their access tag set
explicitly to ensure they are still available to end users after we
start enforcing HTTP API restrictions. APIs reviewed in this PR
([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)):

<img width="260" alt="Screenshot 2024-09-11 at 11 25 55"
src="https://github.com/user-attachments/assets/499b1f1f-8e01-4463-9410-4500e438cd23">

## Note to reviewers

This audit is focussed on set `access: 'public'` where needed. Per the
screenshot our public-facing documentation is taken as the source of
truth for which APIs should be public. This may differ per offering so
please consider whether a given HTTP API should be public on both
serverless and stateful offerings.

## Risks

* If we miss an API that should be public, end users will encounter a
`400` response when they try to use the HTTP API on 9.0
* If we set an API's access to "public" it will not have the same
restrictions applied to it.

(cherry picked from commit 3fa5bdf)
@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 Sep 23, 2024
…92554) (#193735)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[HTTP] Set explicit access for &#x60;public&#x60; HTTP APIs
(#192554)](#192554)

<!--- 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-09-23T14:53:31Z","message":"[HTTP]
Set explicit access for `public` HTTP APIs (#192554)\n\n##
Summary\r\n\r\nWe will be enforcing restricted access to internal HTTP
APIs [from\r\n9.0](#186781).
This PR is part 1\r\nof audit checking that our public APIs have their
access tag set\r\nexplicitly to ensure they are still available to end
users after we\r\nstart enforcing HTTP API restrictions. APIs reviewed
in this
PR\r\n([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)):\r\n\r\n<img
width=\"260\" alt=\"Screenshot 2024-09-11 at 11 25
55\"\r\nsrc=\"https://github.com/user-attachments/assets/499b1f1f-8e01-4463-9410-4500e438cd23\">\r\n\r\n##
Note to reviewers\r\n\r\nThis audit is focussed on set `access:
'public'` where needed. Per the\r\nscreenshot our public-facing
documentation is taken as the source of\r\ntruth for which APIs should
be public. This may differ per offering so\r\nplease consider whether a
given HTTP API should be public on both\r\nserverless and stateful
offerings.\r\n\r\n## Risks\r\n\r\n* If we miss an API that should be
public, end users will encounter a\r\n`400` response when they try to
use the HTTP API on 9.0\r\n* If we set an API's access to \"public\" it
will not have the same\r\nrestrictions applied to
it.","sha":"3fa5bdf8732101812a656ec954e2a8d779838938","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]
Set explicit access for `public` HTTP
APIs","number":192554,"url":"https://github.com/elastic/kibana/pull/192554","mergeCommit":{"message":"[HTTP]
Set explicit access for `public` HTTP APIs (#192554)\n\n##
Summary\r\n\r\nWe will be enforcing restricted access to internal HTTP
APIs [from\r\n9.0](#186781).
This PR is part 1\r\nof audit checking that our public APIs have their
access tag set\r\nexplicitly to ensure they are still available to end
users after we\r\nstart enforcing HTTP API restrictions. APIs reviewed
in this
PR\r\n([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)):\r\n\r\n<img
width=\"260\" alt=\"Screenshot 2024-09-11 at 11 25
55\"\r\nsrc=\"https://github.com/user-attachments/assets/499b1f1f-8e01-4463-9410-4500e438cd23\">\r\n\r\n##
Note to reviewers\r\n\r\nThis audit is focussed on set `access:
'public'` where needed. Per the\r\nscreenshot our public-facing
documentation is taken as the source of\r\ntruth for which APIs should
be public. This may differ per offering so\r\nplease consider whether a
given HTTP API should be public on both\r\nserverless and stateful
offerings.\r\n\r\n## Risks\r\n\r\n* If we miss an API that should be
public, end users will encounter a\r\n`400` response when they try to
use the HTTP API on 9.0\r\n* If we set an API's access to \"public\" it
will not have the same\r\nrestrictions applied to
it.","sha":"3fa5bdf8732101812a656ec954e2a8d779838938"}},"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/192554","number":192554,"mergeCommit":{"message":"[HTTP]
Set explicit access for `public` HTTP APIs (#192554)\n\n##
Summary\r\n\r\nWe will be enforcing restricted access to internal HTTP
APIs [from\r\n9.0](#186781).
This PR is part 1\r\nof audit checking that our public APIs have their
access tag set\r\nexplicitly to ensure they are still available to end
users after we\r\nstart enforcing HTTP API restrictions. APIs reviewed
in this
PR\r\n([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)):\r\n\r\n<img
width=\"260\" alt=\"Screenshot 2024-09-11 at 11 25
55\"\r\nsrc=\"https://github.com/user-attachments/assets/499b1f1f-8e01-4463-9410-4500e438cd23\">\r\n\r\n##
Note to reviewers\r\n\r\nThis audit is focussed on set `access:
'public'` where needed. Per the\r\nscreenshot our public-facing
documentation is taken as the source of\r\ntruth for which APIs should
be public. This may differ per offering so\r\nplease consider whether a
given HTTP API should be public on both\r\nserverless and stateful
offerings.\r\n\r\n## Risks\r\n\r\n* If we miss an API that should be
public, end users will encounter a\r\n`400` response when they try to
use the HTTP API on 9.0\r\n* If we set an API's access to \"public\" it
will not have the same\r\nrestrictions applied to
it.","sha":"3fa5bdf8732101812a656ec954e2a8d779838938"}},{"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 jloleysens deleted the http/explicit-access-part-i branch September 23, 2024 20:54
jloleysens added a commit that referenced this pull request Sep 25, 2024
## Summary

Part 2 of this [PR](#192554). See
more info there.

<img width="229" alt="Screenshot 2024-09-11 at 14 50 11"
src="https://github.com/user-attachments/assets/585f1ce8-2987-4355-a4b1-e13705646b48">

---------

Co-authored-by: shahzad31 <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 25, 2024
## Summary

Part 2 of this [PR](elastic#192554). See
more info there.

<img width="229" alt="Screenshot 2024-09-11 at 14 50 11"
src="https://github.com/user-attachments/assets/585f1ce8-2987-4355-a4b1-e13705646b48">

---------

Co-authored-by: shahzad31 <[email protected]>
(cherry picked from commit 244ff7b)
kibanamachine added a commit that referenced this pull request Sep 25, 2024
…192579) (#193938)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[HTTP] Set explicit access for &#x60;public&#x60; HTTP APIs 2
(#192579)](#192579)

<!--- 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-09-25T07:25:12Z","message":"[HTTP]
Set explicit access for `public` HTTP APIs 2 (#192579)\n\n##
Summary\r\n\r\nPart 2 of this
[PR](#192554). See\r\nmore info
there.\r\n\r\n<img width=\"229\" alt=\"Screenshot 2024-09-11 at 14 50
11\"\r\nsrc=\"https://github.com/user-attachments/assets/585f1ce8-2987-4355-a4b1-e13705646b48\">\r\n\r\n---------\r\n\r\nCo-authored-by:
shahzad31
<[email protected]>","sha":"244ff7b2c37f66518dbb0d7121a8d02b1405fe93","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","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","backport:version"],"title":"[HTTP]
Set explicit access for `public` HTTP APIs
2","number":192579,"url":"https://github.com/elastic/kibana/pull/192579","mergeCommit":{"message":"[HTTP]
Set explicit access for `public` HTTP APIs 2 (#192579)\n\n##
Summary\r\n\r\nPart 2 of this
[PR](#192554). See\r\nmore info
there.\r\n\r\n<img width=\"229\" alt=\"Screenshot 2024-09-11 at 14 50
11\"\r\nsrc=\"https://github.com/user-attachments/assets/585f1ce8-2987-4355-a4b1-e13705646b48\">\r\n\r\n---------\r\n\r\nCo-authored-by:
shahzad31
<[email protected]>","sha":"244ff7b2c37f66518dbb0d7121a8d02b1405fe93"}},"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/192579","number":192579,"mergeCommit":{"message":"[HTTP]
Set explicit access for `public` HTTP APIs 2 (#192579)\n\n##
Summary\r\n\r\nPart 2 of this
[PR](#192554). See\r\nmore info
there.\r\n\r\n<img width=\"229\" alt=\"Screenshot 2024-09-11 at 14 50
11\"\r\nsrc=\"https://github.com/user-attachments/assets/585f1ce8-2987-4355-a4b1-e13705646b48\">\r\n\r\n---------\r\n\r\nCo-authored-by:
shahzad31
<[email protected]>","sha":"244ff7b2c37f66518dbb0d7121a8d02b1405fe93"}},{"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:http 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.

7 participants