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

Bug fixes and enchancements in integration #30292

Merged
merged 25 commits into from
Nov 8, 2023
Merged

Bug fixes and enchancements in integration #30292

merged 25 commits into from
Nov 8, 2023

Conversation

cyble-dev
Copy link
Contributor

@cyble-dev cyble-dev commented Oct 18, 2023

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

fixes: link to the issue

Description

A few sentences describing the overall goals of the pull request's commits.

Must have

  • Tests
  • Documentation

@cyble-dev cyble-dev changed the title Cyble enhancements Bug fixes and enchancements in integration Oct 18, 2023
@RosenbergYehuda RosenbergYehuda requested review from RosenbergYehuda and removed request for sapirshuker October 18, 2023 10:15
@RosenbergYehuda
Copy link
Contributor

RosenbergYehuda commented Oct 18, 2023

Hello @cyble-dev, thank you for your resubmission.
As you can see in the GitHub actions, in the "ci/circleci: Run Validations" section, that there are many validation failures.Do you know how to use our demisto-sdk format?
If yes, please try that and see if the errors are gone.Please check after that that everything is in the right place and working.

@RosenbergYehuda
Copy link
Contributor

RosenbergYehuda commented Oct 18, 2023 via email

Comment on lines 440 to 459
"filters": [
[
{
"left": {
"isContext": true,
"value": {
"simple": "eventtype"
}
},
"operator": "isEqualString",
"right": {
"isContext": false,
"value": {
"simple": "compromised_cards"
}
},
"type": "shortText"
}
]
],
Copy link
Contributor Author

@cyble-dev cyble-dev Oct 18, 2023

Choose a reason for hiding this comment

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

Hi @RosenbergYehuda,
As per your suggestion, I have updated all the files and fixed all the issues that were coming in ci/circleci: Run Validations section

However, I am still getting this error

[2023-10-18 15:59:47,043] [ERROR] Packs/CybleEventsV2/Layouts/layoutscontainer-CybleEventsv2-Layout.json: [ST111] - The field "filters" in path 'quickView'-> 'sections'-> 'Compromised Card Details' was not defined in the scheme
[2023-10-18 15:59:47,043] [INFO] To fix the problem, try running `demisto-sdk format -i Packs/CybleEventsV2/Layouts/layoutscontainer-CybleEventsv2-Layout.json`

I was able to fix this too, but I had to remove the filter key from the layout as highlighted:
If I remove this part, then the test is passing.

But I believe this filter is required to check if the coming incident is of this particular type: 'compromised_card'. So removing this filter does not make sense. But keeping it is not letting the test pass.
I tried suggestion given with the errors too, but not working.

I have exported this layout json from XSOAR instance.

Please suggest how we can go about this.

Copy link
Contributor

@RosenbergYehuda RosenbergYehuda Oct 18, 2023

Choose a reason for hiding this comment

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

Good job solving most of validation failures!
@melamedbn , can you please assist here?

@cyble-dev
Copy link
Contributor Author

Hi @melamedbn,
Please take a look at this.

@melamedbn
Copy link
Contributor

Hi @cyble-dev,

Everything looks great, just one fix needed:
Please remove the hardcoded values from the playbook inputs.

Best regards,
Ben

@cyble-dev
Copy link
Contributor Author

Hi @melamedbn,
Can you please mention which values are you referring to?

@melamedbn
Copy link
Contributor

Hi @melamedbn, Can you please mention which values are you referring to?

The dates in the playbook inputs.

]
},
"group": "incident",
"id": "CybleEventsv2-Layout",
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 suffix -Layout

},
"group": "incident",
"id": "CybleEventsv2-Layout",
"name": "CybleEventsv2-Layout",
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 suffix -Layout

Comment on lines 124 to 130
simple: "2023-05-30T00:00:00+00:00"
required: true
description: ""
playbookInputQuery: null
playbookInputQuery:
- key: end_date
value:
simple: "2023-05-31T00:00:00+00:00"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @melamedbn,
Are you talking about these dates? If yes, then I am not sure what to put here?
Does this section have same inputs as in line 53 and 61 for start and end dates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @melamedbn,
Can we please escalate this? Let me know the expected edit in above lines 124 & 130.

@melamedbn
Copy link
Contributor

Hi @cyble-dev,

There are still things that should be addressed as discussed in our meeting.

  1. Remove the hardcoded values from the playbook inputs.
  2. Add description to the inputs.
  3. Add another input for the 'limit' argument so the user will be able to choose the limit.

Best regards,
Ben

@cyble-dev
Copy link
Contributor Author

Hi @melamedbn,
Can we get on a quick call right now?

@cyble-dev
Copy link
Contributor Author

Hi @melamedbn,
I have made the changes you suggested. Please proceed.

@cyble-dev
Copy link
Contributor Author

Hi @edik24, @melamedbn & @RosenbergYehuda,
I have removed the playbook’s changes and updated the whole pack. Please proceed.

Please let me know if you have any query.

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 image as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall I keep the empty doc_files folder or remove it as well?

@melamedbn
Copy link
Contributor

@cyble-dev,

Just saw that the current image isn't good. so let's keep the new image and delete the old one.

@cyble-dev
Copy link
Contributor Author

cyble-dev commented Nov 8, 2023

@cyble-dev,

Just saw that the current image isn't good. so let's keep the new image and delete the old one.

oh, okay @melamedbn
So no modification is required, right?
We are moving forward as it is now.

@RosenbergYehuda RosenbergYehuda merged commit 00e6480 into demisto:contrib/cyble-dev_cyble-enhancements Nov 8, 2023
18 of 19 checks passed
RosenbergYehuda added a commit that referenced this pull request Nov 8, 2023
* Bug fixes and enchancements in integration (#30292)

* New PR with all the changes

* Updated RN

* Fixed issues in files' format

* Fixed fieldnames

* Fixed field's name in layout file

* Fixed field's name in layout file

* Fixed field's name in layout file

* Removed -Layout suffix

* Updated layout file

* Formatted files

* Fixed playbook

* Updated RNs

* Made changes in object files, uploading updated ones

* Formatted files and bugs, test failure fixes

* Updated RN, tests, playbook changes and formatting

* Updated RN

---------

Co-authored-by: Yehuda Rosenberg <90599084+RosenbergYehuda@users.noreply.github.com>

* RN

* mistake

* RN

---------

Co-authored-by: cyble-dev <101622497+cyble-dev@users.noreply.github.com>
Co-authored-by: Yehuda Rosenberg <90599084+RosenbergYehuda@users.noreply.github.com>
Co-authored-by: Yehuda <yrosenberg@paloaltonetworks.com>
@cyble-dev cyble-dev deleted the cyble-enhancements branch November 9, 2023 10:25
sapirshuker pushed a commit that referenced this pull request Dec 21, 2023
* Bug fixes and enchancements in integration (#30292)

* New PR with all the changes

* Updated RN

* Fixed issues in files' format

* Fixed fieldnames

* Fixed field's name in layout file

* Fixed field's name in layout file

* Fixed field's name in layout file

* Removed -Layout suffix

* Updated layout file

* Formatted files

* Fixed playbook

* Updated RNs

* Made changes in object files, uploading updated ones

* Formatted files and bugs, test failure fixes

* Updated RN, tests, playbook changes and formatting

* Updated RN

---------

Co-authored-by: Yehuda Rosenberg <90599084+RosenbergYehuda@users.noreply.github.com>

* RN

* mistake

* RN

---------

Co-authored-by: cyble-dev <101622497+cyble-dev@users.noreply.github.com>
Co-authored-by: Yehuda Rosenberg <90599084+RosenbergYehuda@users.noreply.github.com>
Co-authored-by: Yehuda <yrosenberg@paloaltonetworks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 Security Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants