refactor notifier config validation

cmd/cmd_debug.go

@@ -51,7 +51,6 @@ var debugCacheCmd = &cobra.Command{
 	},
 }
 
-
 var debugNotificationsCmd = &cobra.Command{
 	Use:   "notifications",
 	Short: "Verify that notifications are working",
@@ -66,11 +65,11 @@ to healthchecks.default.notifications.options as defined in the config file`,
 
 		for transport, notifier := range core.Notifiers {
 			notLog := log.WithField("transport", transport)
-			opts := defaultNotifyConf.Notifications.Options
+			opts := defaultNotifyConf.Notifications
-			notLog.Infof("testing notifer %s with options %v", transport, opts)
+			notLog.Infof("testing notifer %s with options %v", transport, opts.Options)
 			n, err := notifier.New(opts)
 			if err != nil {
-				notLog.Warnf("could not initialize %s notifier. configuration might be missing?", transport)
+				notLog.Warnf("could not initialize %s notifier: %s", transport, err)
 				continue
 			}
 

cmd/config.go

@@ -39,24 +39,22 @@ func initConfig() {
 }
 
 func validateConfig() {
-	transport := viper.GetString("defaultHealthchecks.notifications.transport")
+	if viper.GetString("notify") != "" {
-	if viper.GetBool("notify") && transport == "email" {
+		if len(viper.GetStringSlice("healthchecks.default.notifications.options.recipients")) == 0 {
-		if len(viper.GetStringSlice("defaultHealthchecks.notifications.options.recipients")) == 0 {
+			log.Warn("No default recipients set up for notifications!")
-			log.Warn("No recipients set up for transport email")
 		}
 
-		emailRequired := []string{
+		var conf = &core.TransportConfig{}
-			viper.GetString("email.host"),
+		if err := viper.UnmarshalKey("healthchecks.default.notifications", conf); err != nil {
-			viper.GetString("email.port"),
+			log.Error("invalid default notification configuration: ", err)
-			viper.GetString("email.user"),
-			viper.GetString("email.pass"),
-			viper.GetString("email.from"),
-		}
-		for _, conf := range emailRequired {
-			if conf == "" {
-				log.Error("Default transport set as email, but missing email config")
 			os.Exit(1)
 		}
+
+		// creating a notifier validates its configuration
+		_, err := core.GetNotifier(conf)
+		if err != nil {
+			log.Error(err)
+			os.Exit(1)
 		}
 	}
 }
@@ -85,8 +83,7 @@ func getNotifyConf(boxID string) (*core.NotifyConfig, error) {
 	if keyDefined("healthchecks.default.events") {
 		conf.Events = []core.NotifyEvent{}
 	}
-	err := viper.UnmarshalKey("healthchecks.default", conf)
+	if err := viper.UnmarshalKey("healthchecks.default", conf); err != nil {
-	if err != nil {
 		return nil, err
 	}
 
@@ -94,8 +91,7 @@ func getNotifyConf(boxID string) (*core.NotifyConfig, error) {
 	if keyDefined("healthchecks." + boxID + ".events") {
 		conf.Events = []core.NotifyEvent{}
 	}
-	err = viper.UnmarshalKey("healthchecks."+boxID, conf)
+	if err := viper.UnmarshalKey("healthchecks."+boxID, conf); err != nil {
-	if err != nil {
 		return nil, err
 	}
 

core/notifier_email.go

@@ -10,22 +10,31 @@ import (
 	"github.com/spf13/viper"
 )
 
-// box config required for the EmailNotifier
+// box config required for the EmailNotifier (TransportConfig.Options)
 type EmailNotifier struct {
 	Recipients []string
 }
 
-func (n EmailNotifier) New(config interface{}) (AbstractNotifier, error) {
+func (n EmailNotifier) New(config TransportConfig) (AbstractNotifier, error) {
+	// validate transport configuration
+	// :TransportConfSourceHack @FIXME: dont get these values from viper, as the core package
+	// should be agnostic of the source of configuration!
+	requiredConf := []string{"email.user", "email.pass", "email.host", "email.port", "email.from"}
+	for _, key := range requiredConf {
+		if viper.GetString(key) == "" {
+			return nil, fmt.Errorf("Missing configuration key %s", key)
+		}
+	}
+
 	// assign configuration to the notifier after ensuring the correct type.
 	// lesson of this project: golang requires us to fuck around with type
 	// assertions, instead of providing us with proper inheritance.
-
+	asserted, ok := config.Options.(EmailNotifier)
-	asserted, ok := config.(EmailNotifier)
 	if !ok || asserted.Recipients == nil {
 		// config did not contain valid options.
 		// first try fallback: parse result of viper is a map[string]interface{},
 		// which requires a different assertion change
-		asserted2, ok := config.(map[string]interface{})
+		asserted2, ok := config.Options.(map[string]interface{})
 		if ok {
 			asserted3, ok := asserted2["recipients"].([]interface{})
 			if ok {
@@ -79,6 +88,7 @@ func (n EmailNotifier) ComposeNotification(box *Box, checks []CheckResult) Notif
 }
 
 func (n EmailNotifier) Submit(notification Notification) error {
+	// :TransportConfSourceHack
 	auth := smtp.PlainAuth(
 		"",
 		viper.GetString("email.user"),

core/notifier_xmpp.go

@@ -6,26 +6,34 @@ import (
 	"strings"
 	"time"
 
-	"github.com/spf13/viper"
 	xmpp "github.com/mattn/go-xmpp"
+	"github.com/spf13/viper"
 )
 
-// box config required for the XmppNotifier
+// box config required for the XmppNotifier (TransportConfig.Options)
 type XmppNotifier struct {
 	Recipients []string
 }
 
-func (n XmppNotifier) New(config interface{}) (AbstractNotifier, error) {
+func (n XmppNotifier) New(config TransportConfig) (AbstractNotifier, error) {
+	// validate transport configuration
+	// :TransportConfSourceHack
+	requiredConf := []string{"xmpp.user", "xmpp.pass", "xmpp.host", "xmpp.starttls"}
+	for _, key := range requiredConf {
+		if viper.GetString(key) == "" {
+			return nil, fmt.Errorf("Missing configuration key %s", key)
+		}
+	}
+
 	// assign configuration to the notifier after ensuring the correct type.
 	// lesson of this project: golang requires us to fuck around with type
 	// assertions, instead of providing us with proper inheritance.
-
+	asserted, ok := config.Options.(XmppNotifier)
-	asserted, ok := config.(XmppNotifier)
 	if !ok || asserted.Recipients == nil {
 		// config did not contain valid options.
 		// first try fallback: parse result of viper is a map[string]interface{},
 		// which requires a different assertion change
-		asserted2, ok := config.(map[string]interface{})
+		asserted2, ok := config.Options.(map[string]interface{})
 		if ok {
 			asserted3, ok := asserted2["recipients"].([]interface{})
 			if ok {
@@ -79,6 +87,7 @@ func (n XmppNotifier) ComposeNotification(box *Box, checks []CheckResult) Notifi
 }
 
 func (n XmppNotifier) Submit(notification Notification) error {
+	// :TransportConfSourceHack
 	xmppOpts := xmpp.Options{
 		Host:     viper.GetString("xmpp.host"),
 		User:     viper.GetString("xmpp.user"),

core/notifiers.go

@@ -14,7 +14,7 @@ var Notifiers = map[string]AbstractNotifier{
 }
 
 type AbstractNotifier interface {
-	New(config interface{}) (AbstractNotifier, error)
+	New(config TransportConfig) (AbstractNotifier, error)
 	ComposeNotification(box *Box, checks []CheckResult) Notification
 	Submit(notification Notification) error
 }
@@ -27,7 +27,12 @@ type Notification struct {
 //////
 
 func (box Box) GetNotifier() (AbstractNotifier, error) {
-	transport := box.NotifyConf.Notifications.Transport
+	return GetNotifier(&box.NotifyConf.Notifications)
+}
+
+func GetNotifier(config *TransportConfig) (AbstractNotifier, error) {
+	transport := config.Transport
+
 	if transport == "" {
 		return nil, fmt.Errorf("No notification transport provided")
 	}
@@ -37,7 +42,7 @@ func (box Box) GetNotifier() (AbstractNotifier, error) {
 		return nil, fmt.Errorf("%s is not a supported notification transport", transport)
 	}
 
-	return notifier.New(box.NotifyConf.Notifications.Options)
+	return notifier.New(*config)
 }
 
 func (results BoxCheckResults) SendNotifications(notifyTypes []string, useCache bool) error {