fix: nil pointer on notifier

[mathetake]

resolves #496

---

if slack's url is configured through SLACK_URL environment variable without providing slack.url, current helm chart ignores slack.channel and slack.user, which results in notifier in the controller struct becoming nil.

just in case, i added nopNotifer for warning users about misconfigurations in notifiers.

[codecov-io]

Codecov Report

Merging #504 into master will decrease coverage by 0.09%.
The diff coverage is 5.55%.

@@            Coverage Diff             @@
##           master     #504      +/-   ##
==========================================
- Coverage   52.17%   52.08%   -0.10%     
==========================================
  Files          60       61       +1     
  Lines        4529     4537       +8     
==========================================
  Hits         2363     2363              
- Misses       1770     1778       +8     
  Partials      396      396              

Impacted Files	Coverage Δ
pkg/canary/daemonset_controller.go	59.88% <0.00%> (ø)
pkg/canary/deployment_controller.go	58.62% <0.00%> (ø)
pkg/notifier/factory.go	0.00% <0.00%> (ø)
pkg/notifier/nop.go	0.00% <0.00%> (ø)
pkg/controller/events.go	48.73% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8682cc...38b893f. Read the comment docs.

[mathetake]

https://github.com/weaveworks/flagger/blob/nop-notifier/cmd/flagger/main.go#L329 there's still a possible branch for notifier being nil... 😞

[stefanprodan]

@mathetake you are right, I made a mess out of the global notifier when I added env vars support.

[mathetake]

@stefanprodan thank you for quick response 👍
i will come back to this PR early in the next week and find a better way to refactor

[stefanprodan]

@mathetake I think the NopNotifier shouldn't return an error and just be nop. This way the initNotifier could return a nop instead of nil and this will solve the issue.

[mathetake]

@mathetake I think the NopNotifier shouldn't return an error and just be nop. This way the initNotifier could return a nop instead of nil and this will solve the issue.

got it

[mathetake]

@stefanprodan i am not sure what 30 means. could you tell me what is the purpose for slicing in the logging🤔 ?

[stefanprodan]

I wanted to avoid logging secrets as the Slack/Teams URL will contain the API Key. But this is poorly done since the URL can be shorter for other providers (for example a self-hosted Rocket) and the info will never get logged.

[stefanprodan]

LGTM

Thanks @mathetake 🏅