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

[Actions] Do not return system actions from GET APIs #161845

Merged
merged 12 commits into from
Jul 20, 2023

Conversation

cnasikas
Copy link
Member

Summary

This PR removes the ability to get system actions from GET APIs. Specifically:

  • The Get action API throws a 404 error when requesting a system action
  • The Bulk Get action API throws a 404 error when requesting a system action
  • The Get All API filters out system actions
  • The Get List Types API filters out system actions

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cnasikas cnasikas added release_note:skip Skip the PR/issue when compiling release notes Feature:Actions Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Actions/Framework Issues related to the Actions Framework v8.10.0 labels Jul 13, 2023
@cnasikas cnasikas self-assigned this Jul 13, 2023
@cnasikas cnasikas requested a review from a team as a code owner July 13, 2023 10:22
@elasticmachine
Copy link
Contributor

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

@pmuellr
Copy link
Member

pmuellr commented Jul 13, 2023

I'm curious what this might break :-)

Basically, in the framework, as it's running, using the API client itself to do things; I guess mainly alerting.

At first I was thinking this might be blocking at the HTTP level, which seems probably fine (but who knows!), but blocking at the client API seems a lot more likely to break things ...

@cnasikas
Copy link
Member Author

cnasikas commented Jul 13, 2023

You are right @pmuellr. Also, we may need to access system actions and the actions client is the only way to do it (although we can expose dedicate functions for system actions). About the APIs, until we know what we want to do with them, it is better to not return system actions as removing them afterwards will be a breaking change.

@cnasikas
Copy link
Member Author

@mikecote @pmuellr I applied the changes we discussed offline. The API will not return the system actions but the actions client will accept an option to return the system actions from the getAll and listTypes functions (defaulted to not return them). When I start working on the UI, I will create an internal API to return system actions. Could you please take another look?

@cnasikas
Copy link
Member Author

buildkite test this

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@cnasikas cnasikas enabled auto-merge (squash) July 20, 2023 09:17
@cnasikas cnasikas disabled auto-merge July 20, 2023 09:18
@cnasikas cnasikas mentioned this pull request Jul 20, 2023
15 tasks
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @cnasikas

@cnasikas cnasikas merged commit c66f72a into elastic:main Jul 20, 2023
@cnasikas cnasikas deleted the system_actions_get_apis branch July 20, 2023 16:09
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 20, 2023
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
## Summary

This PR removes the ability to get system actions from `GET` APIs.
Specifically:

- The Get action API throws a 404 error when requesting a system action
- The Bulk Get action API throws a 404 error when requesting a system
action
- The Get All API filters out system actions
- The Get List Types API filters out system actions

### Checklist

Delete any items that are not applicable to this PR.

- [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

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Actions/Framework Issues related to the Actions Framework Feature:Actions release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.10.0
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

6 participants