[ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis #192616

Zacqary · 2024-09-11T17:37:34Z

Summary

Adds OAS schemas for the 403 Forbidden errors that public rule apis can return if a license is invalid, 400 Bad Request for unregistered rule types, and 404 Not Found for missing saved objects.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials

Testing

Start ES
Add server.oas.enabled: true to kibana.dev.yml
Start Kibana yarn start --no-base-path
curl -s -uelastic:changeme http://localhost:5601/api/oas\?pathStartsWith\=/api/alerting/rule/ | jq (If you have jq installed, otherwise pipe to pbcopy and paste the result into a JSON prettifier)
Search the output for the word Forbidden to ensure this schema has been added to create, update, enable, disable, mute, unmute, and update_rule_api_key

elasticmachine · 2024-09-11T17:37:36Z

Pinging @elastic/response-ops (Team:ResponseOps)

obltmachine · 2024-09-11T17:37:50Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --update'

adcoelho · 2024-09-12T09:32:20Z

x-pack/plugins/alerting/common/routes/rule/common/schemas/v1.ts

+
+import { schema } from '@kbn/config-schema';
+
+export const forbiddenErrorSchema = schema.object({


Not saying to do it but wdyt about having this schema in x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts instead?

Or maybe we can move the request and response folders(in x-pack/plugins/alerting/common/routes/rule/) here. We prob should consolidate these, they feel similar.

What do you think of x-pack/plugins/alerting/common/routes/rule/errors/schema/v1.ts?

ah, just noticed I wrote it wrong.

I meant to say

Not saying to do it but wdyt about having this schema in x-pack/plugins/alerting/common/routes/rule/ instead?

yeah an errors folder sounds good!

adcoelho · 2024-09-12T09:41:18Z

No other cases of 4xx or 5xx responses appear to be handled, so this is all I've added to OAS.

What about 400s? I see a bunch of instances of Boom.badRequest. This for example.

adcoelho

Some routes appear to be missing:

x-pack/plugins/alerting/server/routes/rule/apis/get/get_rule_route.ts
x-pack/plugins/alerting/server/routes/rule/apis/find/find_rules_route.ts
x-pack/plugins/alerting/server/routes/rule/apis/delete/delete_rule_route.ts

Also, I think we should also document 404s ~~and 500s~~.

~~500s because errors in the code are always possible~~ 😅
404s in case of trying to access missing SOs

Zacqary · 2024-09-13T19:13:04Z

500s because errors in the code are always possible 😅

I can add this, but 500 Internal Server Error means 500 Internal Server Error. That's just HTTP standard. Do we really want to repeat that documentation?

Zacqary · 2024-09-13T19:36:23Z

x-pack/plugins/alerting/server/routes/rule/apis/find/find_rules_route.ts

This is internal

into 188514-rules-4xx-oas

…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --update'

into 188514-rules-4xx-oas

Zacqary · 2024-09-17T14:48:23Z

@cnasikas from the issue:

We discussed it offline with the Core team and the ResponseOps team and we decided to not include the 5xx status codes in the OAS. The reasons are:

Unless it means something specific in the context of the API it is not worth creating a specific entry for. It is pretty generic and could be something the core defines for all routes.

APIs should not return any 5xx codes. They should not be part of the API itself.

The main reason for documenting the 5xx errors in the OAS was for parts of the code that do if (whatever) { throw new Error("my error") } where the route catches it and defaults it to 500. Still, it is better to provide better errors than to document the 500.

Zacqary · 2024-09-17T14:49:19Z

@elasticmachine merge upstream

lcawl · 2024-09-17T18:44:56Z

The API docs output LGTM, thanks!

cnasikas

LGTM! Thank you for your patience with the review.

cnasikas · 2024-09-19T11:51:06Z

@elasticmachine merge upstream

kibana-ci · 2024-09-19T16:52:02Z

💚 Build Succeeded

Buildkite Build
Commit: daec30b

Metrics [docs]

✅ unchanged

History

💛 Build #235580 was flaky 3d8474a
💔 Build #235393 failed 6a4a3df
💔 Build #235389 failed e68a37c
💚 Build #235336 succeeded 0c92742
💛 Build #234900 was flaky 3c6121c
💛 Build #234498 was flaky 5528f14

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

…is (elastic#192616) ## Summary Closes elastic#188514 Adds OAS schemas for the `403 Forbidden` errors that public rule apis can return if a license is invalid, `400 Bad Request` for unregistered rule types, and `404 Not Found` for missing saved objects. ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials ### Testing 1. Start ES 2. Add `server.oas.enabled: true` to `kibana.dev.yml` 3. Start Kibana `yarn start --no-base-path` 4. `curl -s -uelastic:changeme http://localhost:5601/api/oas\?pathStartsWith\=/api/alerting/rule/ | jq` (If you have `jq` installed, otherwise pipe to `pbcopy` and paste the result into a JSON prettifier) 5. Search the output for the word `Forbidden` to ensure this schema has been added to `create`, `update`, `enable`, `disable`, `mute`, `unmute`, and `update_rule_api_key` --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 18afcae)

kibanamachine · 2024-09-19T16:56:56Z

💚 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

…ule apis (#192616) (#193454) # Backport This will backport the following commits from `main` to `8.x`: - [[ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis (#192616)](#192616)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Zacqary Adam Xeper <Zacqary@users.noreply.github.com>

Zacqary added 2 commits September 11, 2024 12:29

Add OAS schema for handled 403 errors on rule apis

273277a

Revert testing code

df6d074

Zacqary added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework v8.16.0 labels Sep 11, 2024

Zacqary requested a review from a team as a code owner September 11, 2024 17:37

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

5f74261

…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --update'

cnasikas self-requested a review September 12, 2024 09:22

adcoelho reviewed Sep 12, 2024

View reviewed changes

adcoelho requested changes Sep 12, 2024

View reviewed changes

Zacqary and others added 4 commits September 13, 2024 14:36

Document 400 and 404 errors

4c2785d

Merge remote-tracking branch 'upstream/main' into 188514-rules-4xx-oas

68b5a50

Merge branch '188514-rules-4xx-oas' of https://github.com/Zacqary/kibana

4c313c1

into 188514-rules-4xx-oas

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

cd4f607

…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --update'

Zacqary changed the title ~~[ResponseOps][Rules] Add OAS schema for handled 403 errors on rule apis~~ [ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis Sep 13, 2024

Zacqary added 3 commits September 16, 2024 10:23

Merge remote-tracking branch 'upstream/main' into 188514-rules-4xx-oas

4fa70e0

Merge branch '188514-rules-4xx-oas' of https://github.com/Zacqary/kibana

dfc8f6f

into 188514-rules-4xx-oas

Merge remote-tracking branch 'upstream/main' into 188514-rules-4xx-oas

5528f14

Zacqary requested a review from adcoelho September 17, 2024 14:48

Merge branch 'main' into 188514-rules-4xx-oas

3c6121c

Zacqary enabled auto-merge (squash) September 17, 2024 19:00

cnasikas approved these changes Sep 19, 2024

View reviewed changes

Merge branch 'main' into 188514-rules-4xx-oas

a5a1717

adcoelho approved these changes Sep 19, 2024

View reviewed changes

adcoelho added the v9.0.0 label Sep 19, 2024

[CI] Auto-commit changed files from 'yarn openapi:bundle'

3d8474a

elastic-vault-github-plugin-prod bot requested a review from a team as a code owner September 19, 2024 12:43

Zacqary added the backport:skip This commit does not require backporting label Sep 19, 2024

Zacqary enabled auto-merge (squash) September 19, 2024 15:08

Zacqary disabled auto-merge September 19, 2024 15:09

Zacqary added backport:version Backport to applied version labels and removed backport:skip This commit does not require backporting labels Sep 19, 2024

Zacqary enabled auto-merge (squash) September 19, 2024 15:09

Zacqary requested a review from a team as a code owner September 19, 2024 15:11

Zacqary requested a review from nikitaindik September 19, 2024 15:11

Zacqary requested a review from a team as a code owner September 19, 2024 15:13

Zacqary requested a review from abhishekbhatia1710 September 19, 2024 15:13

Merge remote-tracking branch 'upstream/main' into 188514-rules-4xx-oas

daec30b

Zacqary force-pushed the 188514-rules-4xx-oas branch from 29c72f7 to daec30b Compare September 19, 2024 15:16

Zacqary removed request for a team, nikitaindik and abhishekbhatia1710 September 19, 2024 15:16

Zacqary merged commit 18afcae into elastic:main Sep 19, 2024
37 checks passed

kibanamachine mentioned this pull request Sep 19, 2024

[8.x] [ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis (#192616) #193454

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis #192616

[ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis #192616

Zacqary commented Sep 11, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Sep 11, 2024

obltmachine commented Sep 11, 2024

adcoelho Sep 12, 2024

cnasikas Sep 12, 2024

adcoelho Sep 12, 2024

adcoelho commented Sep 12, 2024

adcoelho left a comment •

edited

Loading

Zacqary commented Sep 13, 2024

Zacqary commented Sep 13, 2024

Zacqary commented Sep 17, 2024 •

edited

Loading

Zacqary commented Sep 17, 2024

lcawl commented Sep 17, 2024

cnasikas left a comment

cnasikas commented Sep 19, 2024

kibana-ci commented Sep 19, 2024

kibanamachine commented Sep 19, 2024


		import { schema } from '@kbn/config-schema';

		export const forbiddenErrorSchema = schema.object({

[ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis #192616

[ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis #192616

Conversation

Zacqary commented Sep 11, 2024 • edited by kibanamachine Loading

Summary

Checklist

Testing

elasticmachine commented Sep 11, 2024

obltmachine commented Sep 11, 2024

🤖 GitHub comments

adcoelho Sep 12, 2024

Choose a reason for hiding this comment

cnasikas Sep 12, 2024

Choose a reason for hiding this comment

adcoelho Sep 12, 2024

Choose a reason for hiding this comment

adcoelho commented Sep 12, 2024

adcoelho left a comment • edited Loading

Choose a reason for hiding this comment

Zacqary commented Sep 13, 2024

Zacqary commented Sep 13, 2024

Zacqary commented Sep 17, 2024 • edited Loading

Zacqary commented Sep 17, 2024

lcawl commented Sep 17, 2024

cnasikas left a comment

Choose a reason for hiding this comment

cnasikas commented Sep 19, 2024

kibana-ci commented Sep 19, 2024

💚 Build Succeeded

Metrics [docs]

History

kibanamachine commented Sep 19, 2024

💚 All backports created successfully

Questions ?

Zacqary commented Sep 11, 2024 •

edited by kibanamachine

Loading

adcoelho left a comment •

edited

Loading

Zacqary commented Sep 17, 2024 •

edited

Loading