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

Add attachment support to SMTP state and module #47442

Merged
merged 10 commits into from Jun 13, 2018

Conversation

Projects
None yet
3 participants
@jeduardo
Copy link
Contributor

commented May 2, 2018

What does this PR do?

This PR adds support to sending out email messages containing one or more attachments specified through a new parameter called attachments.

The acceptable value for this parameter is a list of files present in the local filesystem that will be added as attachments to the outgoing email sent by the Salt state/module for SMTP.

What issues does this PR fix or reference?

When trying to deploy some automation bits where Salt generates a file periodically (for example, a client certificate) I have faced the issue that I cannot easily send out the generated file to a recipient through email.

The enhancement on this PR makes this particular use case possible.

Previous Behavior

The attachments key is not present and is ignored by Salt.

New Behavior

All files in the minion filesystem specified as a list under the attachments key are added as individual attachments to an outgoing email message.

Tests written?

No

Commits signed with GPG?

No

@jeduardo

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2018

@cachedout hey! You've helped me with my last PR when Jenkins was failing. This time even the reviewer bot is not making an appearance. Any idea about what might be going on here?

@cachedout

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2018

Hi @jeduardo

The bot broke a little while ago. :(

Let's start by having you fix up these lint errors, please:

https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/21534/violations/file/salt/modules/smtp.py/

jeduardo added some commits May 12, 2018

@jeduardo

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2018

@cachedout it took some time specially after the linting task was not accessible for a couple days but linting problems are finally resolved! What else?

jeduardo added some commits May 17, 2018

@jeduardo

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2018

@cachedout hey! Builds seem to be failing now because of issues not related to the changes suggested in this PR. Could you please review it and send forward any other suggestions? I'm really keen to get this functionality added to Salt, either as I'm proposing or in any other way. Thanks!

@@ -76,6 +82,7 @@ def send_msg(recipient,
use_ssl='True',
username=None,
password=None,
attachments=None,

This comment has been minimized.

Copy link
@rallytime

rallytime May 31, 2018

Contributor

Can you move this to the end of the list so we don't cause an API error?

This comment has been minimized.

Copy link
@jeduardo

jeduardo May 31, 2018

Author Contributor

Ah, of course! Moved to the end, thanks for spotting it. :)

subject,
sender=None,
profile=None,
attachments=None,

This comment has been minimized.

Copy link
@rallytime

rallytime May 31, 2018

Contributor

Same comment here. :)

@rallytime rallytime requested a review from cachedout May 31, 2018

@rallytime
Copy link
Contributor

left a comment

LGTM

@jeduardo

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2018

@rallytime thanks for approving it! :)

@rallytime rallytime merged commit 95dfa26 into saltstack:develop Jun 13, 2018

4 of 10 checks passed

jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #5456 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23389 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10428 — ABORTED
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #19512 — ABORTED
Details
codeclimate 2 issues to fix
Details
default Build finished.
Details
WIP ready for review
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #25653 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #17723 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22353 — SUCCESS
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.