diff --git a/config/config.go b/config/config.go index 3ba6da59..d6ad565d 100644 --- a/config/config.go +++ b/config/config.go @@ -15,15 +15,16 @@ package config import ( "encoding/json" - "errors" "fmt" "io/ioutil" + "net" "net/url" "path/filepath" "regexp" "strings" "time" + "github.com/pkg/errors" commoncfg "github.com/prometheus/common/config" "github.com/prometheus/common/model" "gopkg.in/yaml.v2" @@ -264,8 +265,8 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { } } for _, ec := range rcv.EmailConfigs { - if ec.Smarthost == "" { - if c.Global.SMTPSmarthost == "" { + if ec.Smarthost.String() == "" { + if c.Global.SMTPSmarthost.String() == "" { return fmt.Errorf("no global SMTP smarthost set") } ec.Smarthost = c.Global.SMTPSmarthost @@ -487,6 +488,51 @@ func parseURL(s string) (*URL, error) { return &URL{u}, nil } +// HostPort represents a "host:port" network address. +type HostPort struct { + Host string + Port string +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface for HostPort. +func (hp *HostPort) UnmarshalYAML(unmarshal func(interface{}) error) error { + var ( + s string + err error + ) + if err = unmarshal(&s); err != nil { + return err + } + if s == "" { + return nil + } + hp.Host, hp.Port, err = net.SplitHostPort(s) + if err != nil { + return err + } + if hp.Port == "" { + return errors.Errorf("address %q: port cannot be empty", s) + } + return nil +} + +// MarshalYAML implements the yaml.Marshaler interface for HostPort. +func (hp HostPort) MarshalYAML() (interface{}, error) { + return hp.String(), nil +} + +// MarshalJSON implements the json.Marshaler interface for HostPort. +func (hp HostPort) MarshalJSON() ([]byte, error) { + return json.Marshal(hp.String()) +} + +func (hp HostPort) String() string { + if hp.Host == "" && hp.Port == "" { + return "" + } + return fmt.Sprintf("%s:%s", hp.Host, hp.Port) +} + // GlobalConfig defines configuration parameters that are valid globally // unless overwritten. type GlobalConfig struct { @@ -498,7 +544,7 @@ type GlobalConfig struct { SMTPFrom string `yaml:"smtp_from,omitempty" json:"smtp_from,omitempty"` SMTPHello string `yaml:"smtp_hello,omitempty" json:"smtp_hello,omitempty"` - SMTPSmarthost string `yaml:"smtp_smarthost,omitempty" json:"smtp_smarthost,omitempty"` + SMTPSmarthost HostPort `yaml:"smtp_smarthost,omitempty" json:"smtp_smarthost,omitempty"` SMTPAuthUsername string `yaml:"smtp_auth_username,omitempty" json:"smtp_auth_username,omitempty"` SMTPAuthPassword Secret `yaml:"smtp_auth_password,omitempty" json:"smtp_auth_password,omitempty"` SMTPAuthSecret Secret `yaml:"smtp_auth_secret,omitempty" json:"smtp_auth_secret,omitempty"` diff --git a/config/config_test.go b/config/config_test.go index 26a75990..0f4814ba 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -29,7 +29,6 @@ import ( ) func TestLoadEmptyString(t *testing.T) { - var in string _, err := Load(in) @@ -442,7 +441,7 @@ func TestUnmarshalURL(t *testing.T) { } require.Equal(t, "http://example.com/a%20b", u.String(), "URL not properly unmarshalled in JSON.") - err = json.Unmarshal(b, &u) + err = yaml.Unmarshal(b, &u) if err != nil { t.Fatal(err) } @@ -550,7 +549,7 @@ func TestEmptyFieldsAndRegex(t *testing.T) { Global: &GlobalConfig{ HTTPConfig: &commoncfg.HTTPClientConfig{}, ResolveTimeout: model.Duration(5 * time.Minute), - SMTPSmarthost: "localhost:25", + SMTPSmarthost: HostPort{Host: "localhost", Port: "25"}, SMTPFrom: "alertmanager@example.org", HipchatAuthToken: "mysecret", HipchatAPIURL: mustParseURL("https://hipchat.foobar.org/"), @@ -594,7 +593,7 @@ func TestEmptyFieldsAndRegex(t *testing.T) { { To: "team-X+alerts@example.org", From: "alertmanager@example.org", - Smarthost: "localhost:25", + Smarthost: HostPort{Host: "localhost", Port: "25"}, HTML: "{{ template \"email.default.html\" . }}", RequireTLS: &boolFoo, }, @@ -716,3 +715,64 @@ func TestOpsGenieDeprecatedTeamSpecified(t *testing.T) { t.Errorf("Expected: %s\nGot: %s", expectedErr, err.Error()) } } + +func TestUnmarshalHostPort(t *testing.T) { + for _, tc := range []struct { + in string + + exp HostPort + jsonOut string + yamlOut string + err bool + }{ + { + in: `""`, + exp: HostPort{}, + yamlOut: `"" +`, + jsonOut: `""`, + }, + { + in: `"localhost:25"`, + exp: HostPort{Host: "localhost", Port: "25"}, + yamlOut: `localhost:25 +`, + jsonOut: `"localhost:25"`, + }, + { + in: `":25"`, + exp: HostPort{Host: "", Port: "25"}, + yamlOut: `:25 +`, + jsonOut: `":25"`, + }, + { + in: `"localhost"`, + err: true, + }, + { + in: `"localhost:"`, + err: true, + }, + } { + tc := tc + t.Run(tc.in, func(t *testing.T) { + hp := HostPort{} + err := yaml.Unmarshal([]byte(tc.in), &hp) + if tc.err { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.exp, hp) + + b, err := yaml.Marshal(&hp) + require.NoError(t, err) + require.Equal(t, tc.yamlOut, string(b)) + + b, err = json.Marshal(&hp) + require.NoError(t, err) + require.Equal(t, tc.jsonOut, string(b)) + }) + } +} diff --git a/config/notifiers.go b/config/notifiers.go index e97734e0..16416eb1 100644 --- a/config/notifiers.go +++ b/config/notifiers.go @@ -158,7 +158,7 @@ type EmailConfig struct { To string `yaml:"to,omitempty" json:"to,omitempty"` From string `yaml:"from,omitempty" json:"from,omitempty"` Hello string `yaml:"hello,omitempty" json:"hello,omitempty"` - Smarthost string `yaml:"smarthost,omitempty" json:"smarthost,omitempty"` + Smarthost HostPort `yaml:"smarthost,omitempty" json:"smarthost,omitempty"` AuthUsername string `yaml:"auth_username,omitempty" json:"auth_username,omitempty"` AuthPassword Secret `yaml:"auth_password,omitempty" json:"auth_password,omitempty"` AuthSecret Secret `yaml:"auth_secret,omitempty" json:"auth_secret,omitempty"` diff --git a/notify/email/email.go b/notify/email/email.go index 390189b5..73fcf577 100644 --- a/notify/email/email.go +++ b/notify/email/email.go @@ -89,12 +89,7 @@ func (n *Email) auth(mechs string) (smtp.Auth, error) { } identity := n.conf.AuthIdentity - // We need to know the hostname for both auth and TLS. - host, _, err := net.SplitHostPort(n.conf.Smarthost) - if err != nil { - return nil, errors.Wrap(err, "split address") - } - return smtp.PlainAuth(identity, username, password, host), nil + return smtp.PlainAuth(identity, username, password, n.conf.Smarthost.Host), nil case "LOGIN": password := string(n.conf.AuthPassword) if password == "" { @@ -112,28 +107,22 @@ func (n *Email) auth(mechs string) (smtp.Auth, error) { // Notify implements the Notifier interface. func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { - // TODO: move the check to the config package. - // We need to know the hostname for both auth and TLS. - host, port, err := net.SplitHostPort(n.conf.Smarthost) - if err != nil { - return false, errors.Wrap(err, "split address") - } - var ( c *smtp.Client conn net.Conn + err error success = false ) - if port == "465" { + if n.conf.Smarthost.Port == "465" { tlsConfig, err := commoncfg.NewTLSConfig(&n.conf.TLSConfig) if err != nil { return false, errors.Wrap(err, "parse TLS configuration") } if tlsConfig.ServerName == "" { - tlsConfig.ServerName = host + tlsConfig.ServerName = n.conf.Smarthost.Host } - conn, err = tls.Dial("tcp", n.conf.Smarthost, tlsConfig) + conn, err = tls.Dial("tcp", n.conf.Smarthost.String(), tlsConfig) if err != nil { return true, errors.Wrap(err, "establish TLS connection to server") } @@ -142,12 +131,12 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { d = net.Dialer{} err error ) - conn, err = d.DialContext(ctx, "tcp", n.conf.Smarthost) + conn, err = d.DialContext(ctx, "tcp", n.conf.Smarthost.String()) if err != nil { return true, errors.Wrap(err, "establish connection to server") } } - c, err = smtp.NewClient(conn, host) + c, err = smtp.NewClient(conn, n.conf.Smarthost.Host) if err != nil { conn.Close() return true, errors.Wrap(err, "create SMTP client") @@ -177,7 +166,7 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { return false, errors.Wrap(err, "parse TLS configuration") } if tlsConf.ServerName == "" { - tlsConf.ServerName = host + tlsConf.ServerName = n.conf.Smarthost.Host } if err := c.StartTLS(tlsConf); err != nil { diff --git a/notify/email/email_test.go b/notify/email/email_test.go index caea1f9a..595d9912 100644 --- a/notify/email/email_test.go +++ b/notify/email/email_test.go @@ -137,10 +137,10 @@ func (m *mailDev) doEmailRequest(method string, path string) (int, []byte, error // emailTestConfig is the configuration for the tests. type emailTestConfig struct { - Smarthost string `yaml:"smarthost"` - Username string `yaml:"username"` - Password string `yaml:"password"` - Server *mailDev `yaml:"server"` + Smarthost config.HostPort `yaml:"smarthost"` + Username string `yaml:"username"` + Password string `yaml:"password"` + Server *mailDev `yaml:"server"` } func loadEmailTestConfiguration(f string) (emailTestConfig, error) { @@ -223,13 +223,6 @@ func TestEmailNotifyWithErrors(t *testing.T) { errMsg string hasEmail bool }{ - { - title: "invalid address", - updateCfg: func(cfg *config.EmailConfig) { - cfg.Smarthost = "example.com" - }, - errMsg: "split address:", - }, { title: "invalid 'from' template", updateCfg: func(cfg *config.EmailConfig) {