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

[Marketplace Contribution] FireMon Security Manager - Content Pack Update #33266

Merged
merged 10 commits into from Mar 21, 2024
Merged

[Marketplace Contribution] FireMon Security Manager - Content Pack Update #33266

merged 10 commits into from Mar 21, 2024

Conversation

xsoar-bot
Copy link
Contributor

Status

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

Contributor

@davistonehub

Notes

This is the New PR created based on the last unfinished PR#31240.
During the review of PR#31240, there is an update of a Repo causing a huge number of changed needed.
The PR#31240 was close on Jan 10th while I was away.

  • Please assign to original reviewer, Adi Daud, if possible.

Video Link

Short demo video of the Pack usage. Speeds up the review. Optional but recommended. Use a video sharing service such as Google Drive or YouTube.

@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 Mar 8, 2024
@content-bot content-bot changed the base branch from master to contrib/xsoar-contrib_davistonehub-contrib-FireMonSecurityManager-1 March 8, 2024 19:35
@content-bot content-bot requested a review from moishce March 8, 2024 19:35
@content-bot
Copy link
Collaborator

Thank you for your contribution. Your generosity and caring are unrivaled! Rest assured - our content wizard @moishce will very shortly look over your proposed changes.
For your convenience, here is a link to the contributions SLAs document.

@moishce moishce requested review from adi88d and removed request for moishce March 10, 2024 11:53
@moishce moishce assigned adi88d and unassigned moishce Mar 10, 2024
@adi88d adi88d requested review from moishce and removed request for adi88d March 11, 2024 10:56
@adi88d adi88d assigned moishce and unassigned adi88d Mar 11, 2024
@kgal-pan
Copy link
Contributor

Hi @davistonehub, thanks for the contribution. Since this is a Partner-supported Pack we require the FireMon team to review and approve these changes. We're reached out to them to get someone to approve these changes.

@davistonehub
Copy link
Contributor

davistonehub commented Mar 13, 2024 via email

@davistonehub
Copy link
Contributor

davistonehub commented Mar 19, 2024 via email

@kgal-pan
Copy link
Contributor

Hi @davistonehub @moishce - All clear, go ahead with the review.

@davistonehub
Copy link
Contributor

davistonehub commented Mar 19, 2024 via email

moishce
moishce reviewed Mar 20, 2024
View reviewed changes
Copy link
Contributor

@moishce moishce left a comment •
edited by kgal-pan

Choose a reason for hiding this comment

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

Great Job!
Thanks a lot for this contribution,
Sorry for the long waiting time, for the approval of the partner.
I added some review comments and suggestions:

  • Please add the new commands to README file.

  • Please add a unit test for your new command. You can use the following guide - https://xsoar.pan.dev/docs/integrations/unit-testing  as well as examples of unit tests in the HelloWorld integration and other packs from the content repository.

See my other comments below.

Packs/FireMonSecurityManager/ReleaseNotes/1_2_0.md Outdated Show resolved Hide resolved
Packs/FireMonSecurityManager/ReleaseNotes/1_2_0.md Outdated Show resolved Hide resolved
Packs/FireMonSecurityManager/Integrations/FireMonSecurityManager/FireMonSecurityManager.yml Outdated
Comment on lines 118 to 119
- name: page
- name: pageSize
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 them argument description

Copy link
Contributor

Choose a reason for hiding this comment

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

The description of these two parameters has been added.

Packs/FireMonSecurityManager/Integrations/FireMonSecurityManager/FireMonSecurityManager.yml Outdated
Comment on lines 126 to 127
- description: collector id.
name: id
Copy link
Contributor

Choose a reason for hiding this comment

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

It shouldn't be required?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the "id" is needed for the command "firemon-collector-get-status-byid". It retrieve the status of the collector by using its ID.
I have updated the file to change the charactor c to upper case "C".

@kgal-pan kgal-pan mentioned this pull request Mar 20, 2024
Merged
5 tasks
davistonehub and others added 3 commits March 20, 2024 08:41
@davistonehub @moishce
Update Packs/FireMonSecurityManager/ReleaseNotes/1_2_0.md
4bcfa21 
Co-authored-by: Moshe Galitzky <112559840+moishce@users.noreply.github.com>
@davistonehub @moishce
Update Packs/FireMonSecurityManager/ReleaseNotes/1_2_0.md
f80d9f2 
Co-authored-by: Moshe Galitzky <112559840+moishce@users.noreply.github.com>
@davistonehub
Update FireMonSecurityManager.yml
595fdc4
@davistonehub
Copy link
Contributor

davistonehub commented Mar 20, 2024 via email

@moishce moishce added 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 Mar 21, 2024
@content-bot
Copy link
Collaborator

content-bot commented Mar 21, 2024 •
edited

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

moishce
moishce approved these changes Mar 21, 2024
View reviewed changes
Copy link
Contributor

@moishce moishce left a comment

Choose a reason for hiding this comment

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

Ok I will handle it
Thanks again for this contribution.
Great job!

@moishce moishce merged commit efcaeaf into demisto:contrib/xsoar-contrib_davistonehub-contrib-FireMonSecurityManager-1 Mar 21, 2024
23 of 29 checks passed
@content-bot content-bot mentioned this pull request Mar 21, 2024
Merged
3 tasks
@github-actions GitHub Actions
Copy link

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.

dantavori pushed a commit that referenced this pull request Mar 25, 2024
@content-bot @xsoar-bot
@davistonehub @moishce
[Marketplace Contribution] FireMon Security Manager - Content Pack Up…
c0c7dda 
…date (#33266) (#33478)

* "contribution update to pack "FireMon Security Manager""

* Update Packs/FireMonSecurityManager/ReleaseNotes/1_2_0.md



* Update Packs/FireMonSecurityManager/ReleaseNotes/1_2_0.md



* Update FireMonSecurityManager.yml

* changed the argument id to be required

* Update 1_2_0.md

* Update README.md

* Update docker tag

* Update 1_2_0.md

---------

Co-authored-by: xsoar-bot <67315154+xsoar-bot@users.noreply.github.com>
Co-authored-by: davistonehub <111578758+davistonehub@users.noreply.github.com>
Co-authored-by: Moshe Galitzky <112559840+moishce@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davistonehub davistonehub davistonehub left review comments

@moishce moishce moishce approved these changes

Assignees

@kgal-pan kgal-pan

@moishce moishce

Labels
Contribution Thank you! Contributions are always welcome! docs-approved External PR Partner Support Level Indicates that the contribution is for Partner supported pack Partner-Approved ready-for-instance-test In contribution PRs, this label will cause a trigger of a build with a modified pack from the PR.
Projects
None yet
Milestone
No milestone
6 participants
@xsoar-bot @content-bot @kgal-pan @davistonehub @moishce @adi88d