fix: nil pointer on notifier

fluxcd · Mar 15, 2020 · 8b49b4a · 8b49b4a
1 parent b8682cc
commit 8b49b4a
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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/controller/scheduler_daemonset_fixture_test.go b/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)

diff --git a/pkg/controller/scheduler_deployment_fixture_test.go b/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)

diff --git 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
@@ -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
+}