Update ExtraHop Integration #4481
Conversation
- full featured integration includes new and improved commands
- playbook framework for ExtraHop Detection incident type - updated test playbook - enhanced Endpoint Enrichment playbook to include ExtraHop
|
Hi and welcome to the Demisto Content project! Thank you and congrats on your first pull request, we will review it soon! Until then you can check out our documentation for more details. We would be thrilled to see you get involved in our Slack DFIR community for discussions. Hope you have a great time here :) |
|
|
- unquoted = in playbook yml - detaileddescription in integration yml
content-bot
changed the base branch from
master
to
Dan-at-Extrahop_master_base
|
Thank you for your contribution. Your generosity and caring are unrivaled! Rest assured - our content wizard @yaakovi will very shortly look over your proposed changes. |
- add 'secrets' to whitelist - move script to folder - add tests to playbooks and scripts - add descriptions to all playbook tasks - update integration argument display names
- lint issue - add script key to yml - only failure should be known/expected backwards compatibility breakage
- updated references in playbooks and script - need to deprecate old ExtraHop integration
|
@michalgold please go over the included playbooks. |
|
A lengthy period of time has transpired since the PR was reviewed. @Dan-at-Extrahop Please address the reviewer's comments and push your committed changes. |
|
@idovandijk please review the playbooks here |
There was a problem hiding this comment.
General Points
- Well done, the playbooks look great!
- After configuring your integration, I used the "Test" button which completed successfully. However, when running your test playbook, the very first task resulted in an error:
so currently the "test" command is misleading. Please fix it to properly test whether the integration works and can be used with the entered credentials.
- Please edit all your different playbooks to use commands that aren't tied to a specific integration. What I mean, is that by doing this:
you are telling it to run extrahop-device-search only using "ExtraHop v2" integration. This is unnecessary, and is usually used when one wants to run a general command like !url using only a certain integration that supports it (out of many others). Moreover, if you release a new version of ExtraHop, these tasks won't work anymore, because they're tied to ExtraHop v2. It should look like this:
-
Please change the task names to meet our conventions. You can refer to these playbooks, which meet our standards and are good examples, to see how we name different tasks:
IP Enrichment - Internal - Generic v2,Extract Indicators From File - Generic v2. Note capitalization of letters and the meaningfulness and conciseness of the names. Also, task names shouldn't be the same as the name of the command behind them, they should add a little more insight into what's going to happen, in a few words. -
I can't seem to see alerts, been having this loading screen for over a day:
I will be adding additional playbook-specific points in each playbook file.
Please note that I offered in one of my points to create a call regarding a certain aspect of the playbook. We would like to understand better from you how this playbook should be used, to tweak this to better fit the usage in Demisto. Ping me if you think you'd be able to make a call with @yaakovi and me :)
|
@idovandijk Thank you for the review.
I'm unable to see the playbook-specific points that you added, please advise where to see your comments. Yes, I'll email you now to setup a time for a call, thank you. |
|
The CircleCI build failed again. @Dan-at-Extrahop take a look at the build details here - and try and fix the issues so that we can merge your proposed changes as soon as possible. Failed Build Steps
|
|
@yaakovi I'm going through the integration yml file and I can't review it properly as I don't understand the context output descriptions. For example, "ExtraHop.Record.Source.to" is defined as "to". ExtraHop.Record.Source.till is defined as "till". There are 100's of these. I need more information to understand. |
|
The CircleCI build failed again. @Dan-at-Extrahop take a look at the build details here - and try and fix the issues so that we can merge your proposed changes as soon as possible. Failed Build Steps
|
There was a problem hiding this comment.
@Dan-at-Extrahop please help @richardbluestone better understand the integration output descriptions so we can merge the PR in time for the release.
|
@richardbluestone Thanks for the review, I'm happy to answer any questions to assist. These ExtraHop.Record.Source outputs refer specifically to only our extrahop-query-records command, which is a minor part of the integration used to pull transactional records out of ExtraHop. The descriptions present are the current level of detail available in our product today, since these fields are used generally to cover all 60+ record types. We do not believe this will be an issue for our joint customers, since as mentioned this is the level of detail available for records in ExtraHop and it's understandable in the context of a particular record. Beyond these context outputs, all of the commands and arguments have full descriptions, and the rest of the context outputs have detailed descriptions available. I appreciate your review to get this approved in time for the upcoming release. |
|
@idovandijk all playbook changes have been pushed. @yaakovi the two outstanding build issues are:
Lastly just a reminder note for whatever needs to be done to also deprecate the existing ExtraHop integration when this new ExtraHop Reveal(x) integration is released. This should be all of the changes required for us to merge in time for 11/15 release, with the exception of the review from @richardbluestone. |
|
The CircleCI build failed again. @Dan-at-Extrahop take a look at the build details here - and try and fix the issues so that we can merge your proposed changes as soon as possible. Failed Build Steps
|
|
@yaron-libman Need your review for descriptions of playbooks and tasks, and task names :) |
|
The CircleCI build failed again. @Dan-at-Extrahop take a look at the build details here - and try and fix the issues so that we can merge your proposed changes as soon as possible. Failed Build Steps
|
Status
Ready
Related Issues
Description
New version of the ExtraHop integration (full featured). Complete content package with new powerful commands, real-time incident creation via REST, associated playbooks, enhanced system playbook, end-to-end ticket tracking through a playbook and a field trigger script. This is an ExtraHop supported integration that has been in the works as a partnership since last year, most recently we've been working with:
Screenshots
Related PRs
Required version of Demisto
4.5
Does it break backward compatibility?
Deprecate old ExtraHop integration?
Must have
Dependencies
Additional changes
Technical writer review
Mention and link to the files that require a technical writer review.