mag37 / dockcheck

CLI tool to automate docker image updates. Selective, notifications, autoprune, no pre-pulling.
https://mag37.org
GNU General Public License v3.0
1.08k stars 36 forks source link

ssmtp is considered abandoned and should be depreciated. #89

Closed ShanDestromp closed 3 months ago

ShanDestromp commented 3 months ago

Per the subject, sSMTP is abandoned / no longer maintained since March of 2019; as far as I can tell the last update to the Debian upstream is Oct 2022 and only being very basic updates between 2019-2022, basically enough to keep it building on modern systems. Given that, I think either the notif-ssmtp.sh script should be depreciated, or at the very least incorporate a message letting users know ssmtp shouldn't be used. On the plus side, the notif-ssmtp.sh script however works just fine using mSMTP (and likely others that use a similar format) as long as you change line 19:

from: ssmtp $SendMailTo << __EOF

to: msmtp $SendMailTo << __EOF

So basically no-work needs to be done on dockchecks side to keep the same functionality.

I did go ahead and make a simple modification of notify-ssmtp.sh to work with either, checking to see which is installed and using it (looking first for msmtp, then ssmtp if msmtp can't be found, in the event of both it will default to msmtp).

### DISCLAIMER: This is a third party addition to dockcheck - best effort testing.
# Modification of notify-ssmtp.sh to check for msmtp (preferred) or ssmtp (depreciated)
#
# Copy/rename this file to notify.sh to enable the notification snipppet.
# sSMTP has to be installed and configured manually.
# Modify to fit your setup - changing SendMailFrom, SendMailTo, SubjectTag

MSMTP=$(which msmtp)
SSMTP=$(which ssmtp)

if [ -n $MSMPT ]
then 
    MAIL=$MSMTP
elif [ -n $SSMTP ] && [ -z $MAIL ]
then
    MAIL=$SSMTP
else
    echo "No msmtp or ssmtp binary found in PATH: $PATH"
    exit 1
fi

send_notification() {
Updates=("$@")
UpdToString=$( printf "%s\n" "${Updates[@]}" )
FromHost=$(hostname)

# User variables:
SendMailFrom="me@mydomain.tld"
SendMailTo="me@mydomain.tld"
SubjectTag="dockcheck"

printf "\nSending email notification.\n"

$MAIL $SendMailTo << __EOF
From: "$FromHost" <$SendMailFrom>
date:$(date -R)
To: <$SendMailTo>
Subject: [$SubjectTag] Updates available on $FromHost
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

The following containers on $FromHost have updates available:

$UpdToString

__EOF
}
mag37 commented 3 months ago

Thank you kindly for the suggestion and testing. Very thorough with the link to sources too, thanks.

I'll probably remove the notify_ssmtp.sh then create a new one with your changes called just notify_smtp.sh and re-write the info some up top.

If I remember correctly the notify_ssmtp.sh was created from the DSM-version, which still uses ssmtp it seems.

I did the changes in another branch, merged to main and will credit you in the release notes. Thank you!

yoyoma2 commented 3 months ago

If I remember correctly the notify_ssmtp.sh was created from the DSM-version, which still uses ssmtp it seems.

Yes DSM 7.1 is still officially supported and only has ssmtp by default so falling back to ssmtp as you did is a great idea.

yoyoma2 commented 1 month ago

As part of upgrading to v0.4.9 I went ahead and upgraded to the current notify_DSM.sh and it fails since pull request #90 . I added quotes as recommended but avoiding the system variable MAIL was necessary to make it work. On DSM 7.2.x this runs as root and && [ -z $MAIL ] makes everything fail since the root user has MAIL defined to something. Also added the line to support the new urls.list feature.

diff --git a/notify_templates/notify_DSM.sh b/notify_templates/notify_DSM.sh
index 2affe30..77349fc 100644
--- a/notify_templates/notify_DSM.sh
+++ b/notify_templates/notify_DSM.sh
@@ -8,18 +8,17 @@

 MSMTP=$(which msmtp)
 SSMTP=$(which ssmtp)
-
-if [ -n $MSMPT ] ; then
-       MAIL=$MSMTP
-elif [ -n $SSMTP ] && [ -z $MAIL ] ; then
-       MAIL=$SSMTP
+if [ -n "$MSMPT" ] ; then
+       MAILPROG=$MSMTP
+elif [ -n "$SSMTP" ] && [ -z "$MAILPROG" ] ; then
+       MAILPROG=$SSMTP
 else
        echo "No msmtp or ssmtp binary found in PATH: $PATH" ; exit 1
 fi

 send_notification() {
 Updates=("$@")
-UpdToString=$( printf "%s\n" "${Updates[@]}" )
+[ -s "$ScriptWorkDir"/urls.list ] && UpdToString=$( releasenotes ) || UpdToString=$( printf "%s\n" "$
{Updates[@]}" )
 FromHost=$(hostname)
 CfgFile="/usr/syno/etc/synosmtp.conf"

@@ -36,7 +35,7 @@ SenderMail=${SenderMail:-$(grep 'eventmail1' $CfgFile | sed -n 's/.*"\([^"]*\)".

Is the && [ -z $MAIL ] really needed? Will the line to support the new urls.list feature be added. Will whatever fix you choose also go into notify_smtp.sh? This issue should probably be re-opened.

PS. It would be nice if the notification template could be optionally downloaded during automatic upgrades. I would have noticed this much sooner that way. In the DSM case I use the "template" with zero modifications (assuming urls.list line gets added).

mag37 commented 1 month ago

You're correct as always :) good spot and suggestions.

I've changed the variable name, MAIL was bound to have issues. And you're correct, the && [ -z $MAIL ] was completely unnecessary - removed that and quoted the variables in the checks. Did the same to smtp.

It was late last night when I pushed to main - completely forgot to add the new releasenotes-logic to all templates, fixed that now. Will keep the same version number for now - might show up more tuning.


When it comes to updating the templates - I rather have people doing it themselves, checking if their template got changes. Both because it would require something more added to each template to have them comparable, but mainly because I dont want people screwing up their setup by mistake. I'll consider it thou, it has value still.

yoyoma2 commented 1 month ago

I'm now using the latest notify_DSM.sh unchanged and it works perfectly (template was originally for DSM 7.1 but now on DSM 7.2 which still uses ssmtp). The new urls.list feature is terrific!

No credit to me. I didn't see anything wrong when the ssmtp & DSM templates changed. Even when DSM told me something was wrong it took me longer than I'd like to admit to understand it. :confused:

I was thinking if the notify.sh file is a symbolic link that readlink could be used during updates to download notify_xyz.sh to notify_xyz.sh.sample so no risk of breaking the working template.

$ readlink notify.sh
notify_DSM.sh

Cost/benefit not adding up though for a small convenience.

mag37 commented 1 month ago

Still, you spot things and have great suggestions/solutions.

It would be nice to have them update, or at least notify the user of changes - but as it's only templates I think it'll be hard, as people are meant to edit them - so cant just diff file VS file and notify "your notify template have an update". I've just thought (and hope) that if people got issues with their notifications - they'll check back or open a issue.

yoyoma2 commented 1 month ago

Agreed, templates are designed to be user modified so dropping auto-update thoughts.

I'm very happy with dockcheck and the new feature makes step 2 even easier.

  1. dockcheck tells me there is an update
  2. I investigate the changes
  3. I manually update at an appropriate time based on step 2 findings