From 6d0edbe63066b4d0b7c189afb8e6849458455f65 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Sun, 5 Aug 2018 15:38:25 +0200 Subject: [PATCH] Fix a bunch of unhandled errors (#1501) ...as discovered by "gosec" (many other ones reported, but not all make a lot of sense to fix). Signed-off-by: Julius Volz --- api/api.go | 4 ++-- cli/check_config.go | 5 ++++- cli/format/format_extended.go | 6 ++---- cli/format/format_simple.go | 6 ++---- cli/silence_query.go | 4 +++- cli/silence_update.go | 4 +++- client/client.go | 39 +++++++++++++++++++++++++++-------- cmd/alertmanager/main.go | 4 +++- examples/webhook/echo.go | 4 ++-- inhibit/inhibit.go | 4 +++- notify/impl.go | 5 ++++- notify/notify.go | 6 +++--- test/acceptance.go | 12 ++++++++--- 13 files changed, 70 insertions(+), 33 deletions(-) diff --git a/api/api.go b/api/api.go index ec66c7f4..c4c5d9c0 100644 --- a/api/api.go +++ b/api/api.go @@ -59,8 +59,8 @@ func init() { numReceivedAlerts.WithLabelValues("firing") numReceivedAlerts.WithLabelValues("resolved") - prometheus.Register(numReceivedAlerts) - prometheus.Register(numInvalidAlerts) + prometheus.MustRegister(numReceivedAlerts) + prometheus.MustRegister(numInvalidAlerts) } var corsHeaders = map[string]string{ diff --git a/cli/check_config.go b/cli/check_config.go index efaf7ebb..758bfcd5 100644 --- a/cli/check_config.go +++ b/cli/check_config.go @@ -49,7 +49,10 @@ func (c *checkConfigCmd) checkConfig(ctx *kingpin.ParseContext) error { func CheckConfig(args []string) error { if len(args) == 0 { - stat, _ := os.Stdin.Stat() + stat, err := os.Stdin.Stat() + if err != nil { + kingpin.Fatalf("Failed to stat standard input: %v", err) + } if (stat.Mode() & os.ModeCharDevice) != 0 { kingpin.Fatalf("Failed to read from standard input") } diff --git a/cli/format/format_extended.go b/cli/format/format_extended.go index 84e276eb..924c83f1 100644 --- a/cli/format/format_extended.go +++ b/cli/format/format_extended.go @@ -54,8 +54,7 @@ func (formatter *ExtendedFormatter) FormatSilences(silences []types.Silence) err silence.Comment, ) } - w.Flush() - return nil + return w.Flush() } func (formatter *ExtendedFormatter) FormatAlerts(alerts []*client.ExtendedAlert) error { @@ -73,8 +72,7 @@ func (formatter *ExtendedFormatter) FormatAlerts(alerts []*client.ExtendedAlert) alert.GeneratorURL, ) } - w.Flush() - return nil + return w.Flush() } func (formatter *ExtendedFormatter) FormatConfig(status *client.ServerStatus) error { diff --git a/cli/format/format_simple.go b/cli/format/format_simple.go index cb688e31..daec02b5 100644 --- a/cli/format/format_simple.go +++ b/cli/format/format_simple.go @@ -52,8 +52,7 @@ func (formatter *SimpleFormatter) FormatSilences(silences []types.Silence) error silence.Comment, ) } - w.Flush() - return nil + return w.Flush() } func (formatter *SimpleFormatter) FormatAlerts(alerts []*client.ExtendedAlert) error { @@ -69,8 +68,7 @@ func (formatter *SimpleFormatter) FormatAlerts(alerts []*client.ExtendedAlert) e alert.Annotations["summary"], ) } - w.Flush() - return nil + return w.Flush() } func (formatter *SimpleFormatter) FormatConfig(status *client.ServerStatus) error { diff --git a/cli/silence_query.go b/cli/silence_query.go index 248ca001..34d72a74 100644 --- a/cli/silence_query.go +++ b/cli/silence_query.go @@ -148,7 +148,9 @@ func (c *silenceQueryCmd) query(ctx context.Context, _ *kingpin.ParseContext) er if !found { return errors.New("unknown output formatter") } - formatter.FormatSilences(displaySilences) + if err := formatter.FormatSilences(displaySilences); err != nil { + return fmt.Errorf("error formatting silences: %v", err) + } } return nil } diff --git a/cli/silence_update.go b/cli/silence_update.go index dc9c421e..3dce5757 100644 --- a/cli/silence_update.go +++ b/cli/silence_update.go @@ -118,7 +118,9 @@ func (c *silenceUpdateCmd) update(ctx context.Context, _ *kingpin.ParseContext) if !found { return fmt.Errorf("unknown output formatter") } - formatter.FormatSilences(updatedSilences) + if err := formatter.FormatSilences(updatedSilences); err != nil { + return fmt.Errorf("error formatting silences: %v", err) + } } return nil } diff --git a/client/client.go b/client/client.go index bc8cbee6..976fe62b 100644 --- a/client/client.go +++ b/client/client.go @@ -138,7 +138,10 @@ type httpStatusAPI struct { func (h *httpStatusAPI) Get(ctx context.Context) (*ServerStatus, error) { u := h.client.URL(epStatus, nil) - req, _ := http.NewRequest(http.MethodGet, u.String(), nil) + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + if err != nil { + return nil, fmt.Errorf("error creating request: %v", err) + } _, body, err := h.client.Do(ctx, req) if err != nil { @@ -207,7 +210,10 @@ func (h *httpAlertAPI) List(ctx context.Context, filter, receiver string, silenc params.Add("receiver", receiver) u.RawQuery = params.Encode() - req, _ := http.NewRequest(http.MethodGet, u.String(), nil) + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + if err != nil { + return nil, fmt.Errorf("error creating request: %v", err) + } _, body, err := h.client.Do(ctx, req) if err != nil { @@ -228,9 +234,12 @@ func (h *httpAlertAPI) Push(ctx context.Context, alerts ...Alert) error { return err } - req, _ := http.NewRequest(http.MethodPost, u.String(), &buf) + req, err := http.NewRequest(http.MethodPost, u.String(), &buf) + if err != nil { + return fmt.Errorf("error creating request: %v", err) + } - _, _, err := h.client.Do(ctx, req) + _, _, err = h.client.Do(ctx, req) return err } @@ -260,7 +269,10 @@ func (h *httpSilenceAPI) Get(ctx context.Context, id string) (*types.Silence, er "id": id, }) - req, _ := http.NewRequest(http.MethodGet, u.String(), nil) + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + if err != nil { + return nil, fmt.Errorf("error creating request: %v", err) + } _, body, err := h.client.Do(ctx, req) if err != nil { @@ -278,9 +290,12 @@ func (h *httpSilenceAPI) Expire(ctx context.Context, id string) error { "id": id, }) - req, _ := http.NewRequest(http.MethodDelete, u.String(), nil) + req, err := http.NewRequest(http.MethodDelete, u.String(), nil) + if err != nil { + return fmt.Errorf("error creating request: %v", err) + } - _, _, err := h.client.Do(ctx, req) + _, _, err = h.client.Do(ctx, req) return err } @@ -292,7 +307,10 @@ func (h *httpSilenceAPI) Set(ctx context.Context, sil types.Silence) (string, er return "", err } - req, _ := http.NewRequest(http.MethodPost, u.String(), &buf) + req, err := http.NewRequest(http.MethodPost, u.String(), &buf) + if err != nil { + return "", fmt.Errorf("error creating request: %v", err) + } _, body, err := h.client.Do(ctx, req) if err != nil { @@ -315,7 +333,10 @@ func (h *httpSilenceAPI) List(ctx context.Context, filter string) ([]*types.Sile } u.RawQuery = params.Encode() - req, _ := http.NewRequest(http.MethodGet, u.String(), nil) + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + if err != nil { + return nil, fmt.Errorf("error creating request: %v", err) + } _, body, err := h.client.Do(ctx, req) if err != nil { diff --git a/cmd/alertmanager/main.go b/cmd/alertmanager/main.go index 1115ccaf..4ee29e43 100644 --- a/cmd/alertmanager/main.go +++ b/cmd/alertmanager/main.go @@ -267,7 +267,9 @@ func main() { ctx, cancel := context.WithTimeout(context.Background(), *settleTimeout) defer func() { cancel() - peer.Leave(10 * time.Second) + if err := peer.Leave(10 * time.Second); err != nil { + level.Warn(logger).Log("msg", "unable to leave gossip mesh", "err", err) + } }() go peer.Settle(ctx, *gossipInterval*10) } diff --git a/examples/webhook/echo.go b/examples/webhook/echo.go index 56dcd0ed..7b71b521 100644 --- a/examples/webhook/echo.go +++ b/examples/webhook/echo.go @@ -22,7 +22,7 @@ import ( ) func main() { - http.ListenAndServe(":5001", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + log.Fatal(http.ListenAndServe(":5001", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { b, err := ioutil.ReadAll(r.Body) if err != nil { panic(err) @@ -33,5 +33,5 @@ func main() { panic(err) } log.Println(buf.String()) - })) + }))) } diff --git a/inhibit/inhibit.go b/inhibit/inhibit.go index 80575ec7..be6de784 100644 --- a/inhibit/inhibit.go +++ b/inhibit/inhibit.go @@ -116,7 +116,9 @@ func (ih *Inhibitor) Run() { runCancel() }) - g.Run() + if err := g.Run(); err != nil { + level.Warn(ih.logger).Log("msg", "error running inhibitor", "err", err) + } } // Stop the Inhibitor's background processing. diff --git a/notify/impl.go b/notify/impl.go index b3420ac5..bb7dca84 100644 --- a/notify/impl.go +++ b/notify/impl.go @@ -989,7 +989,10 @@ func (n *Wechat) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { } defer resp.Body.Close() - body, _ := ioutil.ReadAll(resp.Body) + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return true, err + } level.Debug(n.logger).Log("msg", "response: "+string(body), "incident", key) if resp.StatusCode != 200 { diff --git a/notify/notify.go b/notify/notify.go index 75b791e6..431b316d 100644 --- a/notify/notify.go +++ b/notify/notify.go @@ -86,9 +86,9 @@ func init() { notificationLatencySeconds.WithLabelValues("webhook") notificationLatencySeconds.WithLabelValues("victorops") - prometheus.Register(numNotifications) - prometheus.Register(numFailedNotifications) - prometheus.Register(notificationLatencySeconds) + prometheus.MustRegister(numNotifications) + prometheus.MustRegister(numFailedNotifications) + prometheus.MustRegister(notificationLatencySeconds) } type notifierConfig interface { diff --git a/test/acceptance.go b/test/acceptance.go index a0c0ecd2..8c24e358 100644 --- a/test/acceptance.go +++ b/test/acceptance.go @@ -301,16 +301,22 @@ func (am *Alertmanager) Start() { // Terminate kills the underlying Alertmanager process and remove intermediate // data. func (am *Alertmanager) Terminate() { - syscall.Kill(am.cmd.Process.Pid, syscall.SIGTERM) + if err := syscall.Kill(am.cmd.Process.Pid, syscall.SIGTERM); err != nil { + am.t.Fatalf("Error sending SIGTERM to Alertmanager process: %v", err) + } } // Reload sends the reloading signal to the Alertmanager process. func (am *Alertmanager) Reload() { - syscall.Kill(am.cmd.Process.Pid, syscall.SIGHUP) + if err := syscall.Kill(am.cmd.Process.Pid, syscall.SIGHUP); err != nil { + am.t.Fatalf("Error sending SIGHUP to Alertmanager process: %v", err) + } } func (am *Alertmanager) cleanup() { - os.RemoveAll(am.confFile.Name()) + if err := os.RemoveAll(am.confFile.Name()); err != nil { + am.t.Errorf("Error removing test config file %q: %v", am.confFile.Name(), err) + } } // Push declares alerts that are to be pushed to the Alertmanager