From 38b893f9c72e17d7679db087e85629f5944b07a3 Mon Sep 17 00:00:00 2001
From: mathetake <cz.rk.t0415y.g@gmail.com>
Date: Sun, 15 Mar 2020 17:28:28 +0900
Subject: [PATCH] fix: nil pointer on notifier

---
 charts/flagger/templates/deployment.yaml |  4 ++++
 cmd/flagger/main.go                      |  4 ++--
 pkg/canary/daemonset_controller.go       |  2 +-
 pkg/canary/deployment_controller.go      |  2 +-
 pkg/controller/events.go                 |  4 ----
 pkg/notifier/factory.go                  | 22 ++++++++++++++--------
 pkg/notifier/nop.go                      | 11 +++++++++++
 7 files changed, 33 insertions(+), 16 deletions(-)
 create mode 100644 pkg/notifier/nop.go

diff --git a/charts/flagger/templates/deployment.yaml b/charts/flagger/templates/deployment.yaml
index f5c264a4d..5d6c5278e 100644
--- a/charts/flagger/templates/deployment.yaml
+++ b/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 }}
diff --git a/cmd/flagger/main.go b/cmd/flagger/main.go
index ec9ef41b6..2a2c6b0cf 100644
--- a/cmd/flagger/main.go
+++ b/cmd/flagger/main.go
@@ -331,8 +331,8 @@ func initNotifier(logger *zap.SugaredLogger) (client notifier.Interface) {
 }
 
 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
 }
diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go
index 4beb1b1db..0642f6b83 100644
--- a/pkg/canary/daemonset_controller.go
+++ b/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,
 	)
 }
 
diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go
index 41ff1b570..3a89526c9 100644
--- a/pkg/canary/deployment_controller.go
+++ b/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,
 	)
 }
 
diff --git a/pkg/controller/events.go b/pkg/controller/events.go
index 6f9740c2f..cc7bd22b4 100644
--- a/pkg/controller/events.go
+++ b/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)
diff --git a/pkg/notifier/factory.go b/pkg/notifier/factory.go
index 8bcab4df2..af555efc4 100644
--- a/pkg/notifier/factory.go
+++ b/pkg/notifier/factory.go
@@ -8,25 +8,31 @@ 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) {
+	var n Interface
+	var err error
 	switch provider {
 	case "slack":
-		return NewSlack(f.URL, f.Username, f.Channel)
+		n, err = NewSlack(f.URL, f.Username, f.Channel)
 	case "discord":
-		return NewDiscord(f.URL, f.Username, f.Channel)
+		n, err = NewDiscord(f.URL, f.Username, f.Channel)
 	case "rocket":
-		return NewRocket(f.URL, f.Username, f.Channel)
+		n, err = NewRocket(f.URL, f.Username, f.Channel)
 	case "msteams":
-		return NewMSTeams(f.URL)
+		n, err = NewMSTeams(f.URL)
+	default:
+		err = fmt.Errorf("provider %s not supported", provider)
 	}
-
-	return nil, fmt.Errorf("provider %s not supported", provider)
+	if err != nil {
+		n = &nopNotifier{}
+	}
+	return n, err
 }
diff --git a/pkg/notifier/nop.go b/pkg/notifier/nop.go
new file mode 100644
index 000000000..61c64dad0
--- /dev/null
+++ b/pkg/notifier/nop.go
@@ -0,0 +1,11 @@
+package notifier
+
+import "fmt"
+
+type nopNotifier struct{}
+
+var errNopNotifier = fmt.Errorf("notifier not configured properly")
+
+func (n *nopNotifier) Post(string, string, string, []Field, string) error {
+	return errNopNotifier
+}