Merge pull request #504 from weaveworks/nop-notifier

fix: nil pointer on notifier

charts/flagger/templates/deployment.yaml

@@ -85,7 +85,11 @@ spec:
           {{- end }}
           {{- if .Values.slack.url }}
           - -slack-url={{ .Values.slack.url }}
+          {{- end }}
+          {{- if .Values.slack.user }}
           - -slack-user={{ .Values.slack.user }}
+          {{- end }}
+          {{- if .Values.slack.channel }}
           - -slack-channel={{ .Values.slack.channel }}
           {{- end }}
           {{- if .Values.msteams.url }}

cmd/flagger/main.go

@@ -318,21 +318,19 @@ func initNotifier(logger *zap.SugaredLogger) (client notifier.Interface) {
 	}
 	notifierFactory := notifier.NewFactory(notifierURL, slackUser, slackChannel)
 
-	if notifierURL != "" {
-		var err error
-		client, err = notifierFactory.Notifier(provider)
-		if err != nil {
-			logger.Errorf("Notifier %v", err)
-		} else {
-			logger.Infof("Notifications enabled for %s", notifierURL[0:30])
-		}
+	var err error
+	client, err = notifierFactory.Notifier(provider)
+	if err != nil {
+		logger.Errorf("Notifier %v", err)
+	} else if len(notifierURL) > 30 {
+		logger.Infof("Notifications enabled for %s", notifierURL[0:30])
 	}
 	return
 }
 
 func fromEnv(envVar string, defaultVal string) string {
-	if os.Getenv(envVar) != "" {
-		return os.Getenv(envVar)
+	if v := os.Getenv(envVar); v != "" {
+		return v
 	}
 	return defaultVal
 }

pkg/canary/daemonset_controller.go

@@ -289,7 +289,7 @@ func (c *DaemonSetController) getSelectorLabel(daemonSet *appsv1.DaemonSet) (str
 
 	return "", fmt.Errorf(
 		"daemonset %s.%s spec.selector.matchLabels must contain one of %v'",
-		c.labels, daemonSet.Name, daemonSet.Namespace,
+		daemonSet.Name, daemonSet.Namespace, c.labels,
 	)
 }
 

pkg/canary/deployment_controller.go

@@ -369,7 +369,7 @@ func (c *DeploymentController) getSelectorLabel(deployment *appsv1.Deployment) (
 
 	return "", fmt.Errorf(
 		"deployment %s.%s spec.selector.matchLabels must contain one of %v",
-		c.labels, deployment.Name, deployment.Namespace,
+		deployment.Name, deployment.Namespace, c.labels,
 	)
 }
 

pkg/controller/events.go

@@ -50,10 +50,6 @@ func (c *Controller) sendEventToWebhook(r *flaggerv1.Canary, eventType, template
 }
 
 func (c *Controller) alert(canary *flaggerv1.Canary, message string, metadata bool, severity flaggerv1.AlertSeverity) {
-	if c.notifier == nil && len(canary.GetAnalysis().Alerts) == 0 {
-		return
-	}
-
 	var fields []notifier.Field
 	if metadata {
 		fields = alertMetadata(canary)

pkg/controller/scheduler_daemonset_fixture_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/weaveworks/flagger/pkg/logger"
 	"github.com/weaveworks/flagger/pkg/metrics"
 	"github.com/weaveworks/flagger/pkg/metrics/observers"
+	"github.com/weaveworks/flagger/pkg/notifier"
 	"github.com/weaveworks/flagger/pkg/router"
 )
 
@@ -102,6 +103,7 @@ func newDaemonSetFixture(c *flaggerv1.Canary) daemonSetFixture {
 		observerFactory:  observerFactory,
 		recorder:         metrics.NewRecorder(controllerAgentName, false),
 		routerFactory:    rf,
+		notifier:         &notifier.NopNotifier{},
 	}
 	ctrl.flaggerSynced = alwaysReady
 	ctrl.flaggerInformers.CanaryInformer.Informer().GetIndexer().Add(c)

pkg/controller/scheduler_deployment_fixture_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/weaveworks/flagger/pkg/logger"
 	"github.com/weaveworks/flagger/pkg/metrics"
 	"github.com/weaveworks/flagger/pkg/metrics/observers"
+	"github.com/weaveworks/flagger/pkg/notifier"
 	"github.com/weaveworks/flagger/pkg/router"
 )
 
@@ -104,6 +105,7 @@ func newDeploymentFixture(c *flaggerv1.Canary) fixture {
 		observerFactory:  observerFactory,
 		recorder:         metrics.NewRecorder(controllerAgentName, false),
 		routerFactory:    rf,
+		notifier:         &notifier.NopNotifier{},
 	}
 	ctrl.flaggerSynced = alwaysReady
 	ctrl.flaggerInformers.CanaryInformer.Informer().GetIndexer().Add(c)

pkg/notifier/factory.go

@@ -1,6 +1,8 @@
 package notifier
 
-import "fmt"
+import (
+	"fmt"
+)
 
 type Factory struct {
 	URL      string
@@ -8,25 +10,36 @@ type Factory struct {
 	Channel  string
 }
 
-func NewFactory(URL string, username string, channel string) *Factory {
+func NewFactory(url string, username string, channel string) *Factory {
 	return &Factory{
-		URL:      URL,
+		URL:      url,
 		Channel:  channel,
 		Username: username,
 	}
 }
 
 func (f Factory) Notifier(provider string) (Interface, error) {
-	switch provider {
-	case "slack":
-		return NewSlack(f.URL, f.Username, f.Channel)
-	case "discord":
-		return NewDiscord(f.URL, f.Username, f.Channel)
-	case "rocket":
-		return NewRocket(f.URL, f.Username, f.Channel)
-	case "msteams":
-		return NewMSTeams(f.URL)
+	if f.URL == "" {
+		return &NopNotifier{}, nil
 	}
 
-	return nil, fmt.Errorf("provider %s not supported", provider)
+	var n Interface
+	var err error
+	switch provider {
+	case "slack":
+		n, err = NewSlack(f.URL, f.Username, f.Channel)
+	case "discord":
+		n, err = NewDiscord(f.URL, f.Username, f.Channel)
+	case "rocket":
+		n, err = NewRocket(f.URL, f.Username, f.Channel)
+	case "msteams":
+		n, err = NewMSTeams(f.URL)
+	default:
+		err = fmt.Errorf("provider %s not supported", provider)
+	}
+
+	if err != nil {
+		n = &NopNotifier{}
+	}
+	return n, err
 }

pkg/notifier/nop.go

@@ -0,0 +1,7 @@
+package notifier
+
+type NopNotifier struct{}
+
+func (n *NopNotifier) Post(string, string, string, []Field, string) error {
+	return nil
+}