Domain Tools Iris - First Release

[ChuckWoodraska]

Status

Ready

Required version of Demisto

4.1.0

Does it break backward compatibility?

• No

Must have

• Tests
• Documentation (with link to it) Not sure where this goes in the repo.
• Code Review

Additional changes

Add List named tags for the IrisTags playbook

Technical writer review

Mention and link to the files that require a technical writer review.

• YAML file
• CHANGELOG

[Itay4]

Looks good 👍
A few general comments:

1. Please generate integration documentation
2. Please add tests for all content entities (playbooks and scripts) and not only for the integration.
3. Please outputs types to all commands outputs.
4. Add Python 3 docker image.
5. File names should be same the directory name, so if the directory name is DomainTools_Iris, so should be the Python, YAML, etc.
6. Please use ' instead of " in the code.
7. Please change the command prefix to something like domaintools- or domaintoolsiris-
8. Please don't access dictionary keys directly, use .get() instead.
9. Please don't access list indices without checking length first.
10. Please add Google style docstrings.
11. Split each function to two, command and request. For more details please have a look here
12. No need for the format functions, please move the parsing logic code to be inside the command functions instead.
13. Please add DBot Score outputs.
14. Make sure all entities id and name are the same, and not GUIDs
15. Did not review playbooks and scripts yet

[ChuckWoodraska]

I believe all the suggested changes for the integration are done other than what I have left unresolved. Still working on updating playbooks and scripts due to changes in the integration.

[ChuckWoodraska]

Also can you point to examples of testing automation scripts and playbooks? I'm not seeing much in the docs on the best way to do this.

[Itay4]

Also can you point to examples of testing automation scripts and playbooks? I'm not seeing much in the docs on the best way to do this.

Example for automation script test: ParseEmailFiles

Example for playbook test: Phishing playbook

[Itay4]

Looks good, some general comments:

1. Please use Python 3 docker image.
2. Some of the docstrings missing details about the arguments, please improve these.
3. Some of the context outputs have Unknown type, please change to the relevant type, if possible.
4. Please add the DBotScore object to the context outputs in the YAML.
5. Please update the command names in the test playbook.

[ChuckWoodraska]

Is the repo for docker images an entirely different repo? Or where does that image/dockerfile have to be?

[Itay4]

Is the repo for docker images an entirely different repo? Or where does that image/dockerfile have to be?

you can see the python 3 docker image here
in order to use it in your integration, you need to specify in the yml dockerimage: demisto/python3:3.7.4.2718

[idovandijk]

Thank you for providing this contribution. I could find use to some of your playbooks, but I have some comments and there are things that need to be fixed to meet our standards and enter Demisto as out-of-the-box playbooks. I commented on the first lane of each relevant playbook, as commenting in exact areas in the YML can confuse. I added screenshots and task numbers for your convenience. There are also some general points for the playbooks:

• Please provide meaningful descriptions for all playbooks and all tasks. If you write them for your integration commands, then upon creating the tasks, the description will be used automatically and save you the time of re-writing it.

• Please set the "Auto extract indicators" setting of each task according to what fits that task best. You can read more about this setting in our documentation. Generally, this determines whether indicators should be extracted and have their reputation calculated, from the data that you print to the war-room in that specific task (its "results", not "outputs"). Sometimes data printed to the war-room does not need this functionality and it unnecessarily extends playbook run-time and provides excessive context. A rule of thumb is to set it to "none" unless that task prints to the war-room some interesting indicators on which we want to run automatically !ip, !domain and so on, in the background.
  

• Please change the task names to go hand in hand with our conventions. For example, condition tasks should be questions.
  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.

• Please replace all "DT" (things like ${something}) to our normal selectors, and add the "Unique" operator where possible and applicable. This prevents mistakes and data duplication. Do this for inputs of tasks and for inputs of the playbooks.
  

DT should only be used in cases where complex logic unsupported by our filters and transformers is needed.

• For all playbooks that work on input data, always check for the existence of that data before working with it. Never assume inputs are present, or the tasks will fail whenever an input is empty. The example playbooks I provided above demonstrate that as well (where we check if a file exists, or if an IP exists).

Thanks again for your efforts.

[Itay4]

@ChuckWoodraska please pull from the base branch

[CLAassistant]

All committers have signed the CLA.

[Itay4]

@ChuckWoodraska looks good, just note 1 comment i've unresolved
pretty much done on my side, waiting for @idovandijk approval and @kirbles19 review

[idovandijk]

Playbook review complete. Final review by @kirbles19 now

[ChuckWoodraska]

Is there an ETA for the final review being done?

[Itay4]

Is there an ETA for the final review being done?

mid next week

[richardbluestone]

All docs have been reviewed.