Drillster / drone-email

Drone plugin for sending email notifications
Apache License 2.0
46 stars 43 forks source link

Allow to set hostname sent to the SMTP server with the HELO command #33

Closed ekrgb closed 5 years ago

ekrgb commented 5 years ago

Hi,

I'd like to use drillster/drone-email for notifications in my drone pipelines.

Unfortunately the e-mail server of my organisation does not allow connections if "localhost" is used in the HELO command.

Although, in my opinion, this does not seem to have any big impact any more, as any spam sender would know about it, this restriction is frequently recommended in best practices for preventing spam (e.g. https://www.linuxmagic.com/best_practices/valid_helo_domain.html).

gomail, which is used in drillster/drone-email, does support to set the host name used in the HELO command via the property "LocalName" (see https://godoc.org/gopkg.in/gomail.v2#Dialer).

I would like to ask to extend the configuration mechanism of drillster/drone-email to be able to configure the host name used in the HELO command. Would that be possible?

Best regards

ekrgb commented 5 years ago

I have successfully implemented the suggested change locally. Here is a patch of my approach, if you want to consider it:

diff --git a/defaults.go b/defaults.go
index 0bf7d73..3643145 100644
--- a/defaults.go
+++ b/defaults.go
@@ -7,6 +7,8 @@
    DefaultOnlyRecipients = false
    // DefaultSkipVerify controls wether to skip SSL verification for the SMTP server
    DefaultSkipVerify = false
+  // DefaultClientHostname is the client hostname used in the HELO command sent to the SMTP server
+  DefaultClientHostname = "localhost"
 )

 // DefaultSubject is the default subject template to use for the email
diff --git a/main.go b/main.go
index 00356ab..8371d65 100644
--- a/main.go
+++ b/main.go
@@ -84,6 +84,13 @@
            Usage:  "attachment filename(s)",
            EnvVar: "PLUGIN_ATTACHMENTS",
        },
+    cli.StringFlag{
+           Name:   "clienthostname",
+           Value:  DefaultClientHostname,
+           Usage:  "smtp client hostname",
+           EnvVar: "EMAIL_CLIENTHOSTNAME,PLUGIN_CLIENTHOSTNAME",
+       },
+       

        // Drone environment
        // Repo
@@ -381,6 +388,7 @@
            Body:           c.String("template.body"),
            Attachment:     c.String("attachment"),
            Attachments:    c.StringSlice("attachments"),
+           ClientHostname:   c.String("clienthostname"),
        },
    }
diff --git a/plugin.go b/plugin.go
index 8f43efe..e7bb98c 100644
--- a/plugin.go
+++ b/plugin.go
@@ -91,6 +91,7 @@
        Body           string
        Attachment     string
        Attachments    []string
+       ClientHostname   string
    }

    Plugin struct {
@@ -133,6 +134,7 @@
    if p.Config.SkipVerify {
        dialer.TLSConfig = &tls.Config{InsecureSkipVerify: true}
    }
+  dialer.LocalName = p.Config.ClientHostname

    closer, err := dialer.Dial()
    if err != nil {
mjwwit commented 5 years ago

Fixed in #34

ekrgb commented 5 years ago

@mjwwit thank you very much!