SMTP sink #1128
SMTP sink #1128
Conversation
|
|
RobertSzefler
force-pushed
the
feature/smtp-sink
branch
from
8325817
to
2f7eb17
Compare
|
|
||
| class MailTransformer(Transformer): | ||
| def __init__(self, *args, **kwargs): | ||
| super(MailTransformer).__init__(*args, **kwargs) |
There was a problem hiding this comment.
| super(MailTransformer).__init__(*args, **kwargs) | |
| super().__init__(*args, **kwargs) |
tests/test_mail_sink.py
Outdated
| with patch("robusta.integrations.mail.sender.apprise") as mock_apprise: | ||
| sink.write_finding(finding, platform_enabled=True) | ||
| mock_apprise.Apprise.return_value.add.assert_called_once_with("mailtos://user:password@example.com?from=a@x&to=b@y") | ||
| expected_body = ( |
There was a problem hiding this comment.
Maybe just to use from unittest.mock import ANY?
I don't think checking for the exact body is helpfull, but it will 100% create some troubles when, for example, removing one space from the body
playbooks/pyproject.toml
Outdated
| @@ -17,7 +17,8 @@ include_trailing_comma = true | |||
| python = "^3.7.1" | |||
| CairoSVG = "^2.5.2" | |||
| Flask = "^2.0.2" | |||
| prometheus-api-client = "^0.4.2" | |||
| #prometheus-api-client = "^0.4.2" | |||
There was a problem hiding this comment.
| #prometheus-api-client = "^0.4.2" |
RobertSzefler
force-pushed
the
feature/smtp-sink
branch
2 times, most recently
from
eb51510
to
2b3e6f2
Compare
docs/configuration/sinks/mail.rst
Outdated
| sinksConfig: | ||
| - mail_sink: | ||
| name: mail_sink | ||
| mailto: mailtos://user:password@server&from=a@x&to=b@y,c@z |
There was a problem hiding this comment.
should the value of mailto be under quotes? ("", since it has :)
There was a problem hiding this comment.
It looks like having a : in a value is ok as long as it's not followed by a space (the YAML spec is weird), and indeed it works ok with PyYAML, but to be extra sure I added quotes and a warning in the docs.
| self.file_blocks = [] | ||
|
|
||
| def block_to_html(self, block: BaseBlock) -> str: | ||
| # TODO should we additionally support ScanReportBlock here? |
There was a problem hiding this comment.
Yes, I think we should
Any reason not to?
We can convert it to PDF and send it as a file attachment (like we do on Slack)
I think users will use it
There was a problem hiding this comment.
In fact it looks like the Slack sink is the only sink that handles ScanReportBlocks, so I was not sure if it's important at all. Will add.
There was a problem hiding this comment.
Added support for ScanReportBlocks via PDF files, just as in the Slack sink. Additionally added a check & a warning in robusta.core.sinks.transformer
| ) | ||
| blocks.append(self.__create_finding_header(finding, status)) | ||
| else: | ||
| # TODO is this correct? Is it possible for the finding to have no title at all? |
There was a problem hiding this comment.
I think it's not possible, and title is mandatory
There was a problem hiding this comment.
removed the "else" part
| f.write(file_block.contents) | ||
| attachments.add(f.name) | ||
| ap_obj.add(self.mailto) | ||
| logging.info(f"MailSender: sending title={finding.title}, body={html_body}") |
There was a problem hiding this comment.
This might be too spammy, maybe change it to debug?
RobertSzefler
force-pushed
the
feature/smtp-sink
branch
3 times, most recently
from
e013478
to
aa0fec6
Compare
RobertSzefler
force-pushed
the
feature/smtp-sink
branch
from
47d31a3
to
f2c6056
Compare
HTML layouts seem to be OK :)