-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
update: octoxlabs integration #37676
update: octoxlabs integration #37676
Conversation
Thank you for your contribution. Your generosity and caring are unrivaled! Make sure to register your contribution by filling the Contribution Registration form, so our content wizard @MLainer1 will know the proposed changes are ready to be reviewed. |
Hi @ogulcanhero, thanks for contributing to the XSOAR marketplace. To receive credit for your generous contribution please follow this link. |
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.
Thank you for your contribution.
A few notes:
- Make sure to add a human readable output for every command.
- Add tests for the new commands.
Let me know if you need any help/
description: 'Specific Scroll Id' | ||
type: String | ||
- contextPath: OctoxLabs.ScrolledApplications.results | ||
description: 'List<Dict> Application information.' |
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.
description: 'List<Dict> Application information.' | |
description: 'List Application information.' |
@@ -43,6 +50,10 @@ def run_command( | |||
"octoxlabs-get-user-by-username": get_user_by_username, | |||
"octoxlabs-get-groups": get_groups, | |||
"octoxlabs-get-permissions": get_permissions, | |||
"octoxlabs-search-scroll-devices": search_scroll_devices, | |||
"octoxlabs-search-scroll-users-inventory": search_scroll_users, |
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.
This command is missing from the .yml file
description: 'Specific Scroll Id' | ||
type: String | ||
- contextPath: OctoxLabs.ScrolledDevices.results | ||
description: 'List<Dict> Device information.' |
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.
description: 'List<Dict> Device information.' | |
description: 'List Device information.' |
- default: | ||
description: 'Page.' | ||
name: page | ||
- default: |
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.
In the code it looks like there is a default size: 50. It is better to add this in the description
description: 'Fields.' | ||
isArray: true | ||
name: fields | ||
- default: |
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.
In the code it looks like there is a default size: 1. It is better to add this in the description
description: 'Fields.' | ||
isArray: true | ||
name: fields | ||
- default: |
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.
In the code it looks like there is a default size: 1. It is better to add this in the description
- default: | ||
description: 'Page.' | ||
name: page | ||
- default: |
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.
In the code it looks like there is a default size: 50. It is better to add this in the description
@@ -28,7 +30,12 @@ def run_command( | |||
"octoxlabs-get-discoveries": get_discoveries, | |||
"octoxlabs-get-last-discovery": get_last_discovery, | |||
"octoxlabs-search-devices": search_devices, | |||
"octoxlabs-search-users-inventory": search_users_inventory, |
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.
This command is missing from the .yml file
# from Packs.Base.Scripts.CommonServerPython.CommonServerPython import CommandResults | ||
|
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.
Is it necessary?
- name: https_proxy | ||
display: HTTPS Proxy | ||
required: false | ||
type: 0 | ||
additionalinfo: Your HTTPS Proxy URL | ||
- name: no_verify | ||
required: false | ||
type: 8 | ||
display: No Verify | ||
additionalinfo: Don't Verify SSL | ||
|
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.
Those 2 new parameters are not documented in the release notes
Hi @MLainer1, thank you for your review. I'm having a problem with the demisto-sdk lint command. When I run the code, I get the error lint() got an unexpected keyword argument 'console_log_threshold'. Is this a known bug? |
Hi @ogulcanhero, it is a known issue, and a fix will be out in the next few days. |
Hi @MLainer1, I have 2 questions;
Thank You! |
Hi @ogulcanhero, are you running the |
Hi @MLainer1 , |
@ogulcanhero It looks like the pre-commit fails to add a dot at the end of some sentences. I think that adding that manually shouldn't take more than a few minutes. |
Hi @MLainer1, |
Hi @ogulcanhero, let's schedule a demo to review your changes and make sure everything works as expected. I'll reach out through DFIR to schedule it. |
Hi @MLainer1 , |
@ogulcanhero Yes, as the code works and my suggestions are for the visibility of the commands (human readable and better error management) |
For the Reviewer: Trigger build request has been accepted for this contribution PR. |
For the Reviewer: Successfully created a pipeline in GitLab with url: https://gitlab.xdr.pan.local/xdr/cortex-content/content/-/pipelines/1893944 |
@ogulcanhero, is the PR finished on your side? |
Yes @MLainer1 , Thank you! |
375fcf7
into
demisto:contrib/ogulcanhero_octoxlabs
Thank you for your contribution. Your external PR has been merged and the changes are now included in an internal PR for further review. The internal PR will be merged to the master branch within 3 business days. |
* initial: scroll * add: new commands * add: tests and fix docs and typo func names * fix: version number * fix: pre-commit problems * add: new tests for coverage * fix: yml file descriptions * fix: yml file descriptions-2 --------- Co-authored-by: ogulcanhero <[email protected]> Co-authored-by: ahmet kotan <[email protected]>
Status
Description
Octox Labs Cyber Security Asset Management platform XSoar Integration update.
Minimum version of Cortex XSOAR
Must have