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.

Sign up for GitHub

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

Jump to bottom

[ResponseOps][Rules] Add 4xx and 5xx responses to OAS #188514

Closed
2 tasks
cnasikas opened this issue Jul 17, 2024 · 4 comments · Fixed by #192616
Closed
2 tasks

[ResponseOps][Rules] Add 4xx and 5xx responses to OAS #188514

cnasikas opened this issue Jul 17, 2024 · 4 comments · Fixed by #192616
Assignees
@Zacqary
Labels
Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@cnasikas
Copy link
Member

cnasikas commented Jul 17, 2024 •
edited
Loading

There is an ongoing effort (#187356) to support OAS to the Rules public APIs. This PR #188445 added the 200 response. We should expand our OAS to include any supported 4xx or 5xx responses.

DoD
Preview Give feedback
@cnasikas cnasikas added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework labels Jul 17, 2024
@elasticmachine
Copy link
Contributor

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

@js-jankisalvi js-jankisalvi mentioned this issue Jul 23, 2024
Merged
@Zacqary Zacqary self-assigned this Sep 11, 2024
@Zacqary
Copy link
Contributor

Zacqary commented Sep 11, 2024

The only specifically handled errors I was able to find for the public rule APIs are a specific case of 403 Forbidden when the license is invalid. Opening a PR to add these, not sure if there's anything I'm missing? I don't see any obvious 5xx responses to document.

@Zacqary Zacqary mentioned this issue Sep 11, 2024
Merged
2 tasks
@cnasikas
Copy link
Member Author

cnasikas commented Sep 13, 2024 •
edited
Loading

Hey @Zacqary! 500 errors can happen on all routes if some part of the code throws an Error. Also, the SO client can throw 404 errors if the SO is not found, for example, in the case of the Get Roule API.

@cnasikas
Copy link
Member Author

cnasikas commented Sep 17, 2024 •
edited
Loading

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.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Sep 19, 2024
@Zacqary
[ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule ap…
8a4a3bd 
…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 <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 18afcae)
kibanamachine added a commit that referenced this issue Sep 19, 2024
@kibanamachine @Zacqary
[8.x] [ResponseOps][Rules] Add OAS schema for handled 4xx errors on r…
e2edfd5 
…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)

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

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

<!--BACKPORT [{"author":{"name":"Zacqary Adam
Xeper","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-19T16:52:17Z","message":"[ResponseOps][Rules]
Add OAS schema for handled 4xx errors on rule apis (#192616)\n\n##
Summary\r\n\r\nCloses #188514 \r\n\r\nAdds OAS schemas for the `403
Forbidden` errors that public rule apis\r\ncan return if a license is
invalid, `400 Bad Request` for unregistered\r\nrule types, and `404 Not
Found` for missing saved objects.\r\n\r\n### Checklist\r\n\r\n- [x] Any
text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n\r\n###
Testing\r\n\r\n1. Start ES\r\n2. Add `server.oas.enabled: true` to
`kibana.dev.yml`\r\n3. Start Kibana `yarn start --no-base-path`\r\n4.
`curl -s
-uelastic:changeme\r\nhttp://localhost:5601/api/oas\\?pathStartsWith\\=/api/alerting/rule/
| jq`\r\n(If you have `jq` installed, otherwise pipe to `pbcopy` and
paste the\r\nresult into a JSON prettifier)\r\n5. Search the output for
the word `Forbidden` to ensure this schema has\r\nbeen added to
`create`, `update`, `enable`, `disable`, `mute`, `unmute`,\r\nand
`update_rule_api_key`\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"18afcae609c9dd142ef158f6f19dd392bc9d6327","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","v9.0.0","Feature:Alerting/RulesFramework","v8.16.0","backport:version"],"title":"[ResponseOps][Rules]
Add OAS schema for handled 4xx errors on rule
apis","number":192616,"url":"https://github.com/elastic/kibana/pull/192616","mergeCommit":{"message":"[ResponseOps][Rules]
Add OAS schema for handled 4xx errors on rule apis (#192616)\n\n##
Summary\r\n\r\nCloses #188514 \r\n\r\nAdds OAS schemas for the `403
Forbidden` errors that public rule apis\r\ncan return if a license is
invalid, `400 Bad Request` for unregistered\r\nrule types, and `404 Not
Found` for missing saved objects.\r\n\r\n### Checklist\r\n\r\n- [x] Any
text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n\r\n###
Testing\r\n\r\n1. Start ES\r\n2. Add `server.oas.enabled: true` to
`kibana.dev.yml`\r\n3. Start Kibana `yarn start --no-base-path`\r\n4.
`curl -s
-uelastic:changeme\r\nhttp://localhost:5601/api/oas\\?pathStartsWith\\=/api/alerting/rule/
| jq`\r\n(If you have `jq` installed, otherwise pipe to `pbcopy` and
paste the\r\nresult into a JSON prettifier)\r\n5. Search the output for
the word `Forbidden` to ensure this schema has\r\nbeen added to
`create`, `update`, `enable`, `disable`, `mute`, `unmute`,\r\nand
`update_rule_api_key`\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"18afcae609c9dd142ef158f6f19dd392bc9d6327"}},"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/192616","number":192616,"mergeCommit":{"message":"[ResponseOps][Rules]
Add OAS schema for handled 4xx errors on rule apis (#192616)\n\n##
Summary\r\n\r\nCloses #188514 \r\n\r\nAdds OAS schemas for the `403
Forbidden` errors that public rule apis\r\ncan return if a license is
invalid, `400 Bad Request` for unregistered\r\nrule types, and `404 Not
Found` for missing saved objects.\r\n\r\n### Checklist\r\n\r\n- [x] Any
text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n\r\n###
Testing\r\n\r\n1. Start ES\r\n2. Add `server.oas.enabled: true` to
`kibana.dev.yml`\r\n3. Start Kibana `yarn start --no-base-path`\r\n4.
`curl -s
-uelastic:changeme\r\nhttp://localhost:5601/api/oas\\?pathStartsWith\\=/api/alerting/rule/
| jq`\r\n(If you have `jq` installed, otherwise pipe to `pbcopy` and
paste the\r\nresult into a JSON prettifier)\r\n5. Search the output for
the word `Forbidden` to ensure this schema has\r\nbeen added to
`create`, `update`, `enable`, `disable`, `mute`, `unmute`,\r\nand
`update_rule_api_key`\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"18afcae609c9dd142ef158f6f19dd392bc9d6327"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Zacqary Adam Xeper <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Zacqary Zacqary

Labels
Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ResponseOps][Rules] Add OAS schema for handled 4xx errors on rule apis Zacqary/kibana
3 participants
@Zacqary @cnasikas @elasticmachine