From de7256e671c3cc6106a154a8fd49df426f4ce56a Mon Sep 17 00:00:00 2001 From: Calvin Henderson Date: Sat, 4 Nov 2023 19:28:06 -0400 Subject: [PATCH] feat(alerting): make authentication optional for email provider (#608) * feat(alerting): Made authentication optional for the email alert provider * docs: Added parameter to email alert provider docs * feat(alerting): Updated email alert to set the LocalName attribute based on the From key * Updated email provider to disable authentication when no credentials are provided * Removed `disable-authentication` flag from email provider documentation * Apply suggestions from code review --------- Co-authored-by: TwiN --- README.md | 28 +++++++++++++-------------- alerting/provider/email/email.go | 17 ++++++++++++++-- alerting/provider/email/email_test.go | 7 +++++++ 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index aba5fb99..65de7880 100644 --- a/README.md +++ b/README.md @@ -470,20 +470,20 @@ endpoints: #### Configuring Email alerts -| Parameter | Description | Default | -|:-----------------------------------|:-------------------------------------------------------------------------------------------|:--------------| -| `alerting.email` | Configuration for alerts of type `email` | `{}` | -| `alerting.email.from` | Email used to send the alert | Required `""` | -| `alerting.email.username` | Username of the SMTP server used to send the alert. If empty, uses `alerting.email.from`. | `""` | -| `alerting.email.password` | Password of the SMTP server used to send the alert | Required `""` | -| `alerting.email.host` | Host of the mail server (e.g. `smtp.gmail.com`) | Required `""` | -| `alerting.email.port` | Port the mail server is listening to (e.g. `587`) | Required `0` | -| `alerting.email.to` | Email(s) to send the alerts to | Required `""` | -| `alerting.email.default-alert` | Default alert configuration.
See [Setting a default alert](#setting-a-default-alert) | N/A | -| `alerting.email.client.insecure` | Whether to skip TLS verification | `false` | -| `alerting.email.overrides` | List of overrides that may be prioritized over the default configuration | `[]` | -| `alerting.email.overrides[].group` | Endpoint group for which the configuration will be overridden by this configuration | `""` | -| `alerting.email.overrides[].to` | Email(s) to send the alerts to | `""` | +| Parameter | Description | Default | +|:-----------------------------------|:----------------------------------------------------------------------------------------------|:--------------| +| `alerting.email` | Configuration for alerts of type `email` | `{}` | +| `alerting.email.from` | Email used to send the alert | Required `""` | +| `alerting.email.username` | Username of the SMTP server used to send the alert. If empty, uses `alerting.email.from`. | `""` | +| `alerting.email.password` | Password of the SMTP server used to send the alert. If empty, no authentication is performed. | `""` | +| `alerting.email.host` | Host of the mail server (e.g. `smtp.gmail.com`) | Required `""` | +| `alerting.email.port` | Port the mail server is listening to (e.g. `587`) | Required `0` | +| `alerting.email.to` | Email(s) to send the alerts to | Required `""` | +| `alerting.email.default-alert` | Default alert configuration.
See [Setting a default alert](#setting-a-default-alert) | N/A | +| `alerting.email.client.insecure` | Whether to skip TLS verification | `false` | +| `alerting.email.overrides` | List of overrides that may be prioritized over the default configuration | `[]` | +| `alerting.email.overrides[].group` | Endpoint group for which the configuration will be overridden by this configuration | `""` | +| `alerting.email.overrides[].to` | Email(s) to send the alerts to | `""` | ```yaml alerting: diff --git a/alerting/provider/email/email.go b/alerting/provider/email/email.go index 2729d38e..47940cae 100644 --- a/alerting/provider/email/email.go +++ b/alerting/provider/email/email.go @@ -49,7 +49,7 @@ func (provider *AlertProvider) IsValid() bool { } } - return len(provider.From) > 0 && len(provider.Password) > 0 && len(provider.Host) > 0 && len(provider.To) > 0 && provider.Port > 0 && provider.Port < math.MaxUint16 + return len(provider.From) > 0 && len(provider.Host) > 0 && len(provider.To) > 0 && provider.Port > 0 && provider.Port < math.MaxUint16 } // Send an alert using the provider @@ -66,7 +66,20 @@ func (provider *AlertProvider) Send(endpoint *core.Endpoint, alert *alert.Alert, m.SetHeader("To", strings.Split(provider.getToForGroup(endpoint.Group), ",")...) m.SetHeader("Subject", subject) m.SetBody("text/plain", body) - d := gomail.NewDialer(provider.Host, provider.Port, username, provider.Password) + var d *gomail.Dialer + if len(provider.Password) == 0 { + // Get the domain in the From address + localName := "localhost" + fromParts := strings.Split(provider.From, `@`) + if len(fromParts) == 2 { + localName = fromParts[1] + } + // Create a dialer with no authentication + d = &gomail.Dialer{Host: provider.Host, Port: provider.Port, LocalName: localName} + } else { + // Create an authenticated dialer + d = gomail.NewDialer(provider.Host, provider.Port, username, provider.Password) + } if provider.ClientConfig != nil && provider.ClientConfig.Insecure { d.TLSConfig = &tls.Config{InsecureSkipVerify: true} } diff --git a/alerting/provider/email/email_test.go b/alerting/provider/email/email_test.go index e409ca05..325432a3 100644 --- a/alerting/provider/email/email_test.go +++ b/alerting/provider/email/email_test.go @@ -18,6 +18,13 @@ func TestAlertDefaultProvider_IsValid(t *testing.T) { } } +func TestAlertProvider_IsValidWithNoCredentials(t *testing.T) { + validProvider := AlertProvider{From: "from@example.com", Host: "smtp-relay.gmail.com", Port: 587, To: "to@example.com"} + if !validProvider.IsValid() { + t.Error("provider should've been valid") + } +} + func TestAlertProvider_IsValidWithOverride(t *testing.T) { providerWithInvalidOverrideGroup := AlertProvider{ Overrides: []Override{