-
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
Add SentinelOne connector #159157
Add SentinelOne connector #159157
Conversation
ce54dd2
to
a29c934
Compare
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, added a note about updating CODEOWNERS, and a brief note on schema.nullable()
vs schema.maybe()
usage
@@ -0,0 +1,21 @@ | |||
/* |
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.
Can you add the sentinelone
directories to the CODEOWNERS
file, like we did with the gen_ai connector?
Lines 1143 to 1146 in a42df25
## Explore owner connectors | |
/x-pack/plugins/stack_connectors/public/connector_types/gen_ai @elastic/security-threat-hunting-explore | |
/x-pack/plugins/stack_connectors/server/connector_types/gen_ai @elastic/security-threat-hunting-explore | |
/x-pack/plugins/stack_connectors/common/gen_ai @elastic/security-threat-hunting-explore |
That way, we'll only get pinged for review when the framework code changes ...
inputInstructions: schema.nullable(schema.string()), | ||
signature: schema.string(), | ||
createdByUser: schema.string(), | ||
requiresApproval: schema.maybe(schema.boolean()), |
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 always check connector schema for uses of schema.maybe()
. You can't use it in connector config
or secrets
, but it can be used in params
, and I don't think we have requirements on the "response" of a connector, since we generally throw it away, only not throwing it away for http invocations (where we return it in the response body).
The usage looks ok here, but thought I'd just double-check that's your understanding as well.
Besides that, these few uses of schema.maybe()
look almost out-of-place, with everything else using schema.nullable()
. So just thought I'd note that, in case you intended these to be schema.nullable()
instead.
7481ce1
to
752c441
Compare
buildkite test this |
buildkite test this |
Pinging @elastic/security-defend-workflows (Team:Defend Workflows) |
buildkite test this |
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
## Summary Adds new connector type to support https://www.sentinelone.com/ The scope of this PR was limited to the Connector logic, schemas, and types to make PR more digestible. In the current PR, the connector is NOT registered, so it's not going to be available to the users. In the follow-up PR I'm going to improve the UX of Param's form and then enable the connector <img width="1685" alt="Zrzut ekranu 2023-08-3 o 11 18 54" src="https://github.com/elastic/kibana/assets/5188868/965ef8ef-497f-42a8-983e-38fd0370cba8"> visual changes include a screenshot or gif. <img width="1685" alt="image" src="https://github.com/elastic/kibana/assets/5188868/119d2255-ed9f-4923-886d-eb139223a47d"> <img width="1690" alt="image" src="https://github.com/elastic/kibana/assets/5188868/e2c569d2-b497-4641-a6a6-454494223ffc">
Summary
Adds new connector type to support https://www.sentinelone.com/
The scope of this PR was limited to the Connector logic, schemas, and types to make PR more digestible.
visual changes include a screenshot or gif.In the current PR, the connector is NOT registered, so it's not going to be available to the users.
In the follow-up PR I'm going to improve the UX of Param's form and then enable the connector