-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Connectors] Check connector's responses #115797
Conversation
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / general / X-Pack Alerting API Integration Tests.x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/jira·ts.alerting api integration security and spaces enabled Actions Jira Jira - Executor Execution should handle creating an incident without commentsStandard Out
Stack Trace
Kibana Pipeline / general / X-Pack Alerting API Integration Tests.x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/jira·ts.alerting api integration security and spaces enabled Actions Jira Jira - Executor Execution should handle creating an incident without commentsStandard Out
Stack Trace
Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/security_solution/network_dns·ts.apis SecuritySolution Endpoints Network DNS With packetbeat Make sure that we get Dns data and sorting by uniqueDomains descendingStandard Out
Stack Trace
and 52 more failures, only showing the first 3. Metrics [docs]
To update your PR or re-run it, just comment with: cc @cnasikas |
Pinging @elastic/security-threat-hunting-cases (Team:Threat Hunting:Cases) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some comments on the unique-ness of the messages being thrown. I'm a little sensitive to this, as many of the existing connectors like email/webhook don't have very good messages on failures :-). I made the comments on the Jira code, but I think it's probably applicable to the other connectors modified here.
x-pack/plugins/actions/server/builtin_action_types/jira/service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.ts
Outdated
Show resolved
Hide resolved
c89d041
to
4b4a344
Compare
Thank you @pmuellr! I totally agree with your feedback. We need to be careful with our messages as it will help a lot to potential SDHs. Fixed 🙂 |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the changes Christos!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job on this, looks really comprehensive. A few nits and one question in the comments below 👍
x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.test.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/actions/server/builtin_action_types/resilient/service.test.ts
Show resolved
Hide resolved
x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @cnasikas |
Co-authored-by: Kibana Machine <[email protected]>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Christos Nasikas <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Summary
This PR adds validation to the responses of the connectors. Specifically, it checks a) if the response is a JSON and b) if certain fields are defined in the response.
Checklist
Delete any items that are not applicable to this PR.
For maintainers