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.

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

[Marketplace Contribution] VMRay Analyzer - Content Pack Update #28872

Conversation

xsoar-bot
Copy link
Contributor

Status

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

Contributor

@Drizzt-IT

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.

@Drizzt-IT
Copy link
Contributor

Hello,
I don't know where the last error comes from.
"[2023-08-11 10:25:23,819] [ERROR] Packs/VMRay/ReleaseNotes/1_2_0.md: [RN107] - No release note entry was found for the playbook "File Enrichment - VMRay""

I didn't change anything in aplaybook here. Could you please advice?

Kind regards,
Konrad

@mmhw mmhw mentioned this pull request Aug 13, 2023
3 tasks
@CLAassistant
Copy link

CLAassistant commented Aug 13, 2023

CLA assistant check
All committers have signed the CLA.

@kgal-pan
Copy link
Contributor

@jthom-vmray @tkorten-vmray - Can you please have a look and see if the commands added are something you're interested in adding to the integration?

@kgal-pan
Copy link
Contributor

kgal-pan commented Aug 13, 2023

Hello, I don't know where the last error comes from. "[2023-08-11 10:25:23,819] [ERROR] Packs/VMRay/ReleaseNotes/1_2_0.md: [RN107] - No release note entry was found for the playbook "File Enrichment - VMRay""

I didn't change anything in aplaybook here. Could you please advice?

Looks like the initial commit did include a Playbook.

Please work on increasing code coverage and add unit tests for the new commands. You can see the coverage report in CircleCI artifacts.

@tkorten-vmray
Copy link
Contributor

@kgal-pan Thank you for letting us know.
I will forward it internally.

@Drizzt-IT
Copy link
Contributor

@kgal-pan Thank you for letting us know. I will forward it internally.

Just for you to know I'm implementing this on a customer request.
Kind regards,
Konrad

@Drizzt-IT
Copy link
Contributor

Any update?

@mmhw
Copy link
Contributor

mmhw commented Aug 23, 2023

Hi @Drizzt-IT,
Once @jthom-vmray @tkorten-vmray confirm they are interested in it, we can start reviewing the PR.
Thanks!

@kgal-pan
Copy link
Contributor

kgal-pan commented Sep 6, 2023

@kgal-pan Thank you for letting us know. I will forward it internally.

@tkorten-vmray - Any updates on this?

@tkorten-vmray
Copy link
Contributor

tkorten-vmray commented Sep 7, 2023

Hey @kgal-pan,
sorry for the long response time. We clarified some smaller changes with @Drizzt-IT which are now implemented as part of commit 918d7f8.
With these changes made, we are now interested in adding these commands.
@Drizzt-IT thank you for implementing and @kgal-pan, @mmhw thank you for reviewing them

@RotemAmit RotemAmit requested review from RosenbergYehuda and removed request for mmhw September 7, 2023 09:21
@RotemAmit RotemAmit assigned RosenbergYehuda and unassigned mmhw Sep 7, 2023
Copy link
Contributor

@RosenbergYehuda RosenbergYehuda left a comment

Choose a reason for hiding this comment

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

Hello @Drizzt-IT,

Great job on your work! I noticed that there were some unresolved conflicts, and since they were causing issues with my code review, I took the liberty to resolve them. I hope that's okay with you.

You've done an excellent job overall, and I've added some minor comments. Please review them and make the necessary corrections. Once you're done, please ping me, and I'll take another look.

If you require any assistance or have any questions, don't hesitate to reach out.

@@ -1068,9 +1088,16 @@ script:
- deprecated: true
description: Retrieves a sample using the sample ID. (Deprecated)
name: upload_sample
dockerimage: demisto/python3:3.10.12.63474
arguments: []
- arguments: []
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the empty arguments field, unless this is because a new validation i'm not aware of...

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind, I think this was done automatically, I will take care of it.

@@ -43,6 +43,8 @@ After you successfully execute a command, a DBot message appears in the War Room
- [vmray-get-iocs](#vmray-get-iocs): Get IOCs for a sample
- [vmray-get-job-by-id](#vmray-get-job-by-id): Get information for a job
- [vmray-get-summary](#vmray-get-summary): Download Summary JSON v2 for an analysis
- [vmray-get-license-usage-verdicts](#vmray-get-license-usage-verdicts): Get the used quota of verdicts
Copy link
Contributor

Choose a reason for hiding this comment

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

When you press the link to the command, you don't see the details of the command as you should.
Please add to the readme all the details of the command, as the rest of them.
You can use the SDK generate-docs command to do it automatic.

Copy link
Contributor

Choose a reason for hiding this comment

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

unfortunatly the SDK is not available to me, so I need to create everything by hand.

Copy link
Contributor

Choose a reason for hiding this comment

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

@RosenbergYehuda I've done the documentation as best as I could.

Kind regards,
Konrad

@RosenbergYehuda RosenbergYehuda added the pending-contributor The PR is pending the response of its creator label Sep 11, 2023
@RosenbergYehuda RosenbergYehuda removed the request for review from ostolero September 13, 2023 14:00
Copy link
Contributor

@RosenbergYehuda RosenbergYehuda left a comment

Choose a reason for hiding this comment

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

Hey @Drizzt-IT,

This PR has my approval!

Let's move on to setting up a live demo so we can see your changes in action.

Let me know what time you are available and your time zone, and I will arrange a meeting.
You can also record a demo of all the changes you made if you prefer.

Waiting to hear from you

Yehuda

@RosenbergYehuda RosenbergYehuda added the pending-demo Demo pending label Sep 13, 2023
@Drizzt-IT
Copy link
Contributor

Hey @Drizzt-IT,

This PR has my approval!

Let's move on to setting up a live demo so we can see your changes in action.

Let me know what time you are available and your time zone, and I will arrange a meeting. You can also record a demo of all the changes you made if you prefer.

Waiting to hear from you

Yehuda

Pinged you via Slack

@RosenbergYehuda
Copy link
Contributor

Hello @tkorten-vmray,

I have approved this pull request, and now we are waiting for a demonstration before it can be merged. As this is a partner pack, we require a representative from our partner to participate in the demo. Could you please provide your available time slots? You can contact me via Slack or email at yrosenberg@paloaltonetworks.com.

@RosenbergYehuda
Copy link
Contributor

Hi @Drizzt-IT,
I sent you an invitation for a demo next week. Please let me know if the time is not good for you.

Have a great weekend!

@RosenbergYehuda
Copy link
Contributor

RosenbergYehuda commented Sep 28, 2023

Hello @Drizzt-IT,

I wanted to let you know that I will be on vacation next week due to the holidays here in Israel. I will return to work on October 8th.

Please accept my apologies for any delays this may cause.
Looking forward to catching up when I'm back in the office.

Regards,Yehuda

@RosenbergYehuda
Copy link
Contributor

My mistake, I typed January instead of October, I am on vacation until the 8th of October.

@RosenbergYehuda
Copy link
Contributor

Vmray.Demo.of.comands.mp4

@RosenbergYehuda
Copy link
Contributor

Hi @tkorten-vmray,

Since we've had difficulty scheduling a live demo, I asked the contributor to record a video walkthrough of their changes. They have uploaded the video.

I've reviewed it and approved the changes. When you have a moment, please watch the video and let me know if you also approve. This contribution has been waiting for a while, so I'd appreciate if you could take a look as soon as possible.

Let me know if you have any other questions!

@tkorten-vmray
Copy link
Contributor

Hi @RosenbergYehuda ,
sorry for the late response. We reviewed it internally and it looks good to us.

@RosenbergYehuda RosenbergYehuda merged commit 7ca5cc4 into demisto:contrib/xsoar-contrib_Drizzt-IT-contrib-VMRay Oct 18, 2023
9 of 11 checks passed
RosenbergYehuda added a commit that referenced this pull request Oct 19, 2023
* [Marketplace Contribution] VMRay Analyzer - Content Pack Update (#28872)

* "contribution update to pack "VMRay Analyzer""

* Fixed several issues / added docs

* Update VMRay.yml

Added  Descriptions to command

* Changed as requested

* fix conflicts

* conflict with docker image

* Added documentation for commands

* fix the readme

* fix yml

* format

* fix validation

---------

Co-authored-by: Konrad <15833518+Drizzt-IT@users.noreply.github.com>
Co-authored-by: Menachem Weinfeld <90556466+mmhw@users.noreply.github.com>
Co-authored-by: Yehuda <yrosenberg@paloaltonetworks.com>

* add a period

* fix RN

* Apply suggestions from Shirley

Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>

* flake 8

* docker

* add outputs

* adding pragma no cover' since the new functions are straightforward and don't need test

* moving the no cover to the new functions instead main

* flake 8

---------

Co-authored-by: xsoar-bot <67315154+xsoar-bot@users.noreply.github.com>
Co-authored-by: Konrad <15833518+Drizzt-IT@users.noreply.github.com>
Co-authored-by: Menachem Weinfeld <90556466+mmhw@users.noreply.github.com>
Co-authored-by: Yehuda <yrosenberg@paloaltonetworks.com>
Co-authored-by: Yehuda Rosenberg <90599084+RosenbergYehuda@users.noreply.github.com>
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
sapirshuker pushed a commit that referenced this pull request Dec 21, 2023
* [Marketplace Contribution] VMRay Analyzer - Content Pack Update (#28872)

* "contribution update to pack "VMRay Analyzer""

* Fixed several issues / added docs

* Update VMRay.yml

Added  Descriptions to command

* Changed as requested

* fix conflicts

* conflict with docker image

* Added documentation for commands

* fix the readme

* fix yml

* format

* fix validation

---------

Co-authored-by: Konrad <15833518+Drizzt-IT@users.noreply.github.com>
Co-authored-by: Menachem Weinfeld <90556466+mmhw@users.noreply.github.com>
Co-authored-by: Yehuda <yrosenberg@paloaltonetworks.com>

* add a period

* fix RN

* Apply suggestions from Shirley

Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>

* flake 8

* docker

* add outputs

* adding pragma no cover' since the new functions are straightforward and don't need test

* moving the no cover to the new functions instead main

* flake 8

---------

Co-authored-by: xsoar-bot <67315154+xsoar-bot@users.noreply.github.com>
Co-authored-by: Konrad <15833518+Drizzt-IT@users.noreply.github.com>
Co-authored-by: Menachem Weinfeld <90556466+mmhw@users.noreply.github.com>
Co-authored-by: Yehuda <yrosenberg@paloaltonetworks.com>
Co-authored-by: Yehuda Rosenberg <90599084+RosenbergYehuda@users.noreply.github.com>
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 pending-contributor The PR is pending the response of its creator pending-demo Demo pending Security Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants