Skip to content

Commit

Permalink
fix: nil pointer on notifier
Browse files Browse the repository at this point in the history
  • Loading branch information
mathetake committed Mar 15, 2020
1 parent b8682cc commit f0bd307
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 24 deletions.
4 changes: 4 additions & 0 deletions charts/flagger/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
18 changes: 8 additions & 10 deletions cmd/flagger/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/canary/daemonset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/canary/deployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}

Expand Down
4 changes: 0 additions & 4 deletions pkg/controller/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/scheduler_daemonset_fixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/scheduler_deployment_fixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand Down
29 changes: 21 additions & 8 deletions pkg/notifier/factory.go
Original file line number Diff line number Diff line change
@@ -1,32 +1,45 @@
package notifier

import "fmt"
import (
"fmt"
)

type Factory struct {
URL string
Username string
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) {
if f.URL == "" {
return &NopNotifier{}, nil
}

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
}
7 changes: 7 additions & 0 deletions pkg/notifier/nop.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package notifier

type NopNotifier struct{}

func (n *NopNotifier) Post(string, string, string, []Field, string) error {
return nil
}

0 comments on commit f0bd307

Please sign in to comment.