Skip to content

New Sink: Webex - #560

Merged
RoiGlinik merged 12 commits into
robusta-dev:masterfrom
Xgrid-Engineering:master
Oct 19, 2022
Merged

New Sink: Webex#560
RoiGlinik merged 12 commits into
robusta-dev:masterfrom
Xgrid-Engineering:master

Conversation

@alikhanxgrid

@alikhanxgrid alikhanxgrid commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

Issue

New Sink: Webex

Description

This PR integrates Webex as a sink to Robusta. Main logic of sending Findings to Webex is in sender.py file. The docs/ folder contain the instructions for configuring Webex with your Robusta deployment.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the docs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes (if applicable)
  • Every function, interface, class has a comment describing what it does and input/output parameters

@aantn

aantn commented Oct 9, 2022

Copy link
Copy Markdown
Collaborator

Hey, thanks for the PR!

We're a little behind schedule due to the holidays. Just want to say thank you and that we haven't forgotten about this.

Comment thread poetry.lock
python-versions = ">=3.5"

[package.extras]
dev = ["cloudpickle", "coverage[toml] (>=5.0.2)", "furo", "hypothesis", "mypy (>=0.900,!=0.940)", "pre-commit", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "sphinx", "sphinx-notfound-page", "zope.interface"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what command did you use to update the dependencies?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

poetry lock --no-update

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok thats good 👍



INVESTIGATE_ICON = "\U0001F50E"
SILENCE_ICON = "\U0001F515"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can switch these with the actual icons , 🔕🔎 just a syntactic sugar

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated. Let me know if its ok or anything needs to be changed.

ATTACHMENT_CONTENT_TYPE = "application/vnd.microsoft.card.adaptive"

CARD_TYPES = {
1: "AdaptiveCard",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think an enum could be more suitable instead of these dicts. you could use string enums as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated. Let me know if its ok or anything needs to be changed.


return message_content, table_blocks, file_blocks, description

def _create_pdf(self, file_blocks: List[FileBlock]):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I tested the sink, and the PDF workaround does not seem to work (I've added an image). To be honest, while the idea is clever, it is also very fragile. Also complicates the code.

I suggest we lose that part and send all the files separately with different messages or stop at the limit. Hopefully, they will add multifile support soon.

I didn't take it lightly but I think it will be a better approach

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will update it and send the files separately.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated. Let me know if its ok or anything needs to be changed.

adaptive_card["body"] = [message_content_container]

#Parsing table blocks for adaptive card
if table_blocks:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The table block has an to_markdown option as well , might work here as well?
I would take that table code to a different function if not just to separate the logic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes I tried to send the table using markdown but the rows and columns were not aligned properly thats why used adaptive cards to send table. I will separate the logic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated. Let me know if its ok or anything needs to be changed.

Thanks for reviewing

* Added webex_sink

* Bug fix with fileblocks, code refactoring, documentation add and minor bug fixes

* Added Enums and replaced emoji code with emoji icons
Comment thread src/robusta/integrations/webex/sender.py Outdated
… function. (#7)

* Added webex_sink

* Bug fix with fileblocks, code refactoring, documentation add and minor bug fixes

* Added Enums and replaced emoji code with emoji icons

* Removed PDF functionality and seperated JSON creation logic from the adaptivecard function logic

* Formatted using Black
# This function sends the files individually to webex
for blocks in files:
file_type = (
FileTypes.PHOTO if is_image(blocks.filename) else FileTypes.DOCUMENT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems this enum is reduandent here, you could just check is_image

@alikhanxgrid alikhanxgrid Oct 19, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated. Let me know if its ok or anything needs to be changed.

file_type = (
FileTypes.PHOTO if is_image(blocks.filename) else FileTypes.DOCUMENT
)
if file_type is FileTypes.DOCUMENT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just want to make sure if you tried to use the same logic in the Slack sink for the files.
there is this function which you can apply on the files to convert to png. ( which will save the image handling you do)
https://github.com/robusta-dev/robusta/blob/master/src/robusta/core/reporting/utils.py#L36

Also using the tempFile for the files you upload, such as here
https://github.com/robusta-dev/robusta/blob/master/src/robusta/integrations/slack/sender.py#L141

It seems to save the extra handle between image/nonimage

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated. Let me know if its ok or anything needs to be changed.
I was using the logic used in telegram sink used in send_file function.
Thanks for linking the files and helping on how to improve the code. ❤️ .

@RoiGlinik RoiGlinik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @alikhanxgrid Great Work.
I tested it looks good,
I forgot some minor changes regarding the files but after that, it's good to go

Thanks for the hard work 👍

* Added webex_sink

* Bug fix with fileblocks, code refactoring, documentation add and minor bug fixes

* Added Enums and replaced emoji code with emoji icons

* Removed PDF functionality and seperated JSON creation logic from the adaptivecard function logic

* Formatted using Black

* Changes in _send_files and _separate_blocks function
* Added webex_sink

* Bug fix with fileblocks, code refactoring, documentation add and minor bug fixes

* Added Enums and replaced emoji code with emoji icons

* Removed PDF functionality and seperated JSON creation logic from the adaptivecard function logic

* Formatted using Black

* Changes in _send_files and _separate_blocks function
@RoiGlinik
RoiGlinik merged commit 8d08efc into robusta-dev:master Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants