Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update Pack: CTM360 #37743

Merged
merged 10 commits into from
Jan 7, 2025
Merged

Update Pack: CTM360 #37743

merged 10 commits into from
Jan 7, 2025

Conversation

edx-sayed-salem
Copy link
Contributor

Contributing to Cortex XSOAR Content

Make sure to register your contribution by filling the contribution registration form

The Pull Request will be reviewed only after the contribution registration form is filled.

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

None

Description

Update the integration docker image.
Add another integration module.

Must have

  • Tests
  • Documentation

@CLAassistant
Copy link

CLAassistant commented Dec 18, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@content-bot content-bot added Contribution Thank you! Contributions are always welcome! External PR Partner Support Level Indicates that the contribution is for Partner supported pack labels Dec 18, 2024
@content-bot content-bot changed the base branch from master to contrib/CTM360-Integrations_ctm360-hackerview-1 December 18, 2024 09:00
@content-bot
Copy link
Collaborator

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.
For your convenience, here is a link to the contributions SLAs document.

@content-bot
Copy link
Collaborator

Hi @edx-sayed-salem, thanks for contributing to the XSOAR marketplace. To receive credit for your generous contribution please follow this link.

@idovandijk idovandijk requested review from karinafishman and removed request for idovandijk December 18, 2024 09:50
@content-bot content-bot added Partner-Approved Contribution Form Filled Whether contribution form filled or not. Partner labels Dec 18, 2024
@karinafishman
Copy link
Contributor

Here are some comments regarding the PR:

Dashboard

  • Please notice that the dashboard is not on full screen, there is a space on the right side. You can see the difference with the default dashboards.
    image
    Release notes
  • Make sure you don't have unnecessary data like: </~XSIAM>.
    IncidentFields
  • Using "unsearchable": false can negatively affect the performance. Please, change it to True.
    Playbook
  • Please fix the general visualization of the playbook. The labels and the linking lines are not clear.
    image
  • Task numbers: 25,26,27,28 are not linked to anything. Please link them to the end of the playbook or the relevant next task.
  • Assign Analyst section header is not needed. Usually, we will use section headers that combine multiple tasks under the section header. In this case, the section header and the task after are the same.
  • You have 2 same section headers: End of Playbook. Please use the same one. (In task 22)
    General comments
  • Please fix all the errors in the build.
  • Check all the comments I added to the PR.

karinafishman
karinafishman requested changes Dec 24, 2024
View reviewed changes
Packs/CTM360-CyberBlindspot/ReleaseNotes/2_0_0.md Outdated
##### New: CTM360 HackerView Potential Impact

- New: Incident field for potential impact of incident on an asset.<~XSIAM> (Available from Cortex XSIAM 2.0).</~XSIAM>
<~XSOAR> (Available from Cortex XSOAR 6.10.0).</~XSOAR>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure you don't have <~XSOAR> in your Release notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change made

Packs/CTM360-CyberBlindspot/ReleaseNotes/2_0_0.md

- New: Incident field for potential types of attacks that may follow such incident.<~XSIAM> (Available from Cortex XSIAM 2.0).</~XSIAM>
<~XSOAR> (Available from Cortex XSOAR 6.10.0).</~XSOAR>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, make sure you don't have: </~XSOAR> or </~XSIAM>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change made

Packs/CTM360-CyberBlindspot/IncidentFields/incidentfields-Asset_Type.json Outdated
],
"associatedToAll": false,
"unmapped": false,
"unsearchable": false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "true" in all IncidentFields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change made

Packs/CTM360-CyberBlindspot/Playbooks/playbook-HackerView_Incident_Management.yml
iscontext: true
right:
value:
simple: In,Both
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add ignore cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change made

Packs/CTM360-CyberBlindspot/Playbooks/playbook-HackerView_Incident_Management.yml Outdated
task:
id: 63cb6508-eefb-4ff1-b157-de4f070e9dca
version: -1
name: End of Playbook
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated header, you can use the same one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used one end section

Packs/CTM360-CyberBlindspot/Playbooks/playbook-HackerView_Incident_Management.yml Outdated
task:
id: 52edca00-4e6d-4739-86da-2a5401f3fd2d
version: -1
name: Is Incident Closed?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the incident closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction made

Packs/CTM360-CyberBlindspot/Playbooks/playbook-HackerView_Incident_Management.yml
iscontext: true
right:
value:
simple: inactive
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add ignore cases

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change made

Packs/CTM360-CyberBlindspot/Playbooks/playbook-HackerView_Incident_Management.yml Outdated
task:
id: bbecc79d-d8ea-4513-8571-c102e9bdf3ea
version: -1
name: Start Investgation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be *Investigation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected

@edx-sayed-salem
Copy link
Contributor Author

I'm not sure I have control over why the CI pre-commit fails, can anyone take a look?

@karinafishman
Copy link
Contributor

@edx-sayed-salem Did you executed "demisto-sdk pre-commit"?

MLainer1
MLainer1 requested changes Dec 25, 2024
View reviewed changes
Copy link
Contributor

@MLainer1 MLainer1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thank you for your contribution.
I'm publishing the first half of the review, the second part will be published later.
Let me know if you need anything.

@edx-sayed-salem
Copy link
Contributor Author

@edx-sayed-salem Did you executed "demisto-sdk pre-commit"?

Yes, it looks like something related to pylint and python 3.12 issue (the docker image version is 3.12)

MLainer1
MLainer1 requested changes Dec 31, 2024
View reviewed changes
Copy link
Contributor

@MLainer1 MLainer1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!
Let's schedule a demo to check your changes and your new pintegration/
I'll reach out over on DFIR to schedule it.

@MLainer1 MLainer1 added the pending-demo Demo pending label Dec 31, 2024
@edx-sayed-salem @MLainer1
Address requested changes 4
899d703 
Unnecessary decorator

Co-authored-by: MLainer1 <[email protected]>
@MLainer1 MLainer1 added post-demo ready-for-instance-test In contribution PRs, this label will cause a trigger of a build with a modified pack from the PR. and removed pending-demo Demo pending labels Jan 2, 2025
@content-bot
Copy link
Collaborator

For the Reviewer: Trigger build request has been accepted for this contribution PR.

@content-bot
Copy link
Collaborator

For the Reviewer: Successfully created a pipeline in GitLab with url: https://gitlab.xdr.pan.local/xdr/cortex-content/content/-/pipelines/1913531

@content-bot content-bot removed the ready-for-instance-test In contribution PRs, this label will cause a trigger of a build with a modified pack from the PR. label Jan 2, 2025
@MLainer1 MLainer1 added the Security Approved If a contribution has been approved for merge by the security team, then this will allow a merge label Jan 2, 2025
@MLainer1
Copy link
Contributor

MLainer1 commented Jan 5, 2025

Hi @edx-sayed-salem, looks like there are some pre-commit errors. can you have a look at them?

@edx-sayed-salem
Copy link
Contributor Author

Hi @edx-sayed-salem, looks like there are some pre-commit errors. can you have a look at them?

Hi, yes it seems the same error from before about python 12 and ruff/pylint. There's another about too many requests when pulling from docker hub.

image

image

image

I think too many requests will solve itself, but ruff/pylint fix was said to be in the works.

@MLainer1
Copy link
Contributor

MLainer1 commented Jan 6, 2025

Hi @edx-sayed-salem, looks like there are some pre-commit errors. can you have a look at them?

Hi, yes it seems the same error from before about python 12 and ruff/pylint. There's another about too many requests when pulling from docker hub.

image

image

image

I think too many requests will solve itself, but ruff/pylint fix was said to be in the works.

Can you create another commit (can be empty) just to run the checks again? the fix should be deployed already

@edx-sayed-salem
Copy link
Contributor Author

ruff/pylint error remains

image

@MLainer1
Copy link
Contributor

MLainer1 commented Jan 6, 2025

Hi @edx-sayed-salem

ruff/pylint error remains

image

After discussing with my team, we've decided it's best to hold off on updating the Docker image to Python 3.12. Instead, we should revert to the version prior to the latest one (demisto/python3:3.12.7.117934).

@edx-sayed-salem
Copy link
Contributor Author

Hi @edx-sayed-salem

ruff/pylint error remains
image

After discussing with my team, we've decided it's best to hold off on updating the Docker image to Python 3.12. Instead, we should revert to the version prior to the latest one (demisto/python3:3.12.7.117934).

Do I need to do anything?

@MLainer1
Copy link
Contributor

MLainer1 commented Jan 6, 2025

Hi @edx-sayed-salem

ruff/pylint error remains
image

After discussing with my team, we've decided it's best to hold off on updating the Docker image to Python 3.12. Instead, we should revert to the version prior to the latest one (demisto/python3:3.12.7.117934).

Do I need to do anything?

Yes. reverse the dockerimage update you did in this PR

@MLainer1
Copy link
Contributor

MLainer1 commented Jan 7, 2025

Looks good! I'll merge the PR

@MLainer1 MLainer1 merged commit 20981d2 into demisto:contrib/CTM360-Integrations_ctm360-hackerview-1 Jan 7, 2025
14 checks passed
@content-bot content-bot mentioned this pull request Jan 7, 2025
Merged
5 tasks
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 7, 2025

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.

MLainer1 added a commit that referenced this pull request Jan 7, 2025
@content-bot @edx-sayed-salem @MLainer1
Update Pack: CTM360 (#37743) (#37967)
d13a204 
* Modified pack-ignore

* squash pack update commits

* Ignore `unsearchable`

* Correct ID and remove unnecessary fields

* Address requested changes

* Address requested changes 2

* Address requested changes 3

* Address requested changes 4

Unnecessary decorator



* Empty-Commit

* Downgrade docker image to latest 3.11.x

---------

Co-authored-by: S. AlQasim D. <[email protected]>
Co-authored-by: MLainer1 <[email protected]>
sdaniel6 pushed a commit that referenced this pull request Jan 27, 2025
@content-bot @edx-sayed-salem
@MLainer1 @sdaniel6
Update Pack: CTM360 (#37743) (#37967)
ff2c0af 
* Modified pack-ignore

* squash pack update commits

* Ignore `unsearchable`

* Correct ID and remove unnecessary fields

* Address requested changes

* Address requested changes 2

* Address requested changes 3

* Address requested changes 4

Unnecessary decorator



* Empty-Commit

* Downgrade docker image to latest 3.11.x

---------

Co-authored-by: S. AlQasim D. <[email protected]>
Co-authored-by: MLainer1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MLainer1 MLainer1 MLainer1 approved these changes

@karinafishman karinafishman karinafishman approved these changes

Assignees

@idovandijk idovandijk

@edik24 edik24

@MLainer1 MLainer1

Labels
Contribution Form Filled Whether contribution form filled or not. Contribution Thank you! Contributions are always welcome! docs-approved External PR Partner Support Level Indicates that the contribution is for Partner supported pack Partner Partner-Approved post-demo Security Approved If a contribution has been approved for merge by the security team, then this will allow a merge Security Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@edx-sayed-salem @CLAassistant @content-bot @karinafishman @MLainer1 @idovandijk @edik24