From 082c1dde71dad5564960bb0059c1715186c62da4 Mon Sep 17 00:00:00 2001 From: mmetc <92726601+mmetc@users.noreply.github.com> Date: Fri, 13 Dec 2024 15:44:21 +0100 Subject: [PATCH] lint/deep-exit: avoid log.Fatal (#3360) * deep-exit: bubble up error from item_metrics.go * deep-exit: bubble up error from password.go --- .golangci.yml | 10 ---- cmd/crowdsec-cli/clicapi/capi.go | 7 ++- cmd/crowdsec-cli/clihub/item_metrics.go | 67 ++++++++++++++++++------- cmd/crowdsec-cli/clilapi/register.go | 7 ++- cmd/crowdsec-cli/climachine/add.go | 5 +- cmd/crowdsec-cli/dashboard.go | 6 ++- cmd/crowdsec-cli/idgen/machineid.go | 6 ++- cmd/crowdsec-cli/idgen/password.go | 9 ++-- pkg/leakybucket/overflows.go | 2 + 9 files changed, 80 insertions(+), 39 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a1b215ae8fd..097cc86d20c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -420,16 +420,6 @@ issues: path: "cmd/crowdsec-cli/main.go" text: "deep-exit: .*" - - linters: - - revive - path: "cmd/crowdsec-cli/clihub/item_metrics.go" - text: "deep-exit: .*" - - - linters: - - revive - path: "cmd/crowdsec-cli/idgen/password.go" - text: "deep-exit: .*" - - linters: - revive path: "pkg/leakybucket/overflows.go" diff --git a/cmd/crowdsec-cli/clicapi/capi.go b/cmd/crowdsec-cli/clicapi/capi.go index 61d59836fdd..2cce01c7d3e 100644 --- a/cmd/crowdsec-cli/clicapi/capi.go +++ b/cmd/crowdsec-cli/clicapi/capi.go @@ -66,7 +66,12 @@ func (cli *cliCapi) register(ctx context.Context, capiUserPrefix string, outputF return fmt.Errorf("unable to generate machine id: %w", err) } - password := strfmt.Password(idgen.GeneratePassword(idgen.PasswordLength)) + pstr, err := idgen.GeneratePassword(idgen.PasswordLength) + if err != nil { + return err + } + + password := strfmt.Password(pstr) apiurl, err := url.Parse(types.CAPIBaseURL) if err != nil { diff --git a/cmd/crowdsec-cli/clihub/item_metrics.go b/cmd/crowdsec-cli/clihub/item_metrics.go index f4af8f635db..ac9c18640fa 100644 --- a/cmd/crowdsec-cli/clihub/item_metrics.go +++ b/cmd/crowdsec-cli/clihub/item_metrics.go @@ -1,6 +1,7 @@ package clihub import ( + "fmt" "net/http" "strconv" "strings" @@ -19,10 +20,16 @@ import ( func showMetrics(prometheusURL string, hubItem *cwhub.Item, wantColor string) error { switch hubItem.Type { case cwhub.PARSERS: - metrics := getParserMetric(prometheusURL, hubItem.Name) + metrics, err := getParserMetric(prometheusURL, hubItem.Name) + if err != nil { + return err + } parserMetricsTable(color.Output, wantColor, hubItem.Name, metrics) case cwhub.SCENARIOS: - metrics := getScenarioMetric(prometheusURL, hubItem.Name) + metrics, err := getScenarioMetric(prometheusURL, hubItem.Name) + if err != nil { + return err + } scenarioMetricsTable(color.Output, wantColor, hubItem.Name, metrics) case cwhub.COLLECTIONS: for _, sub := range hubItem.SubItems() { @@ -31,7 +38,10 @@ func showMetrics(prometheusURL string, hubItem *cwhub.Item, wantColor string) er } } case cwhub.APPSEC_RULES: - metrics := getAppsecRuleMetric(prometheusURL, hubItem.Name) + metrics, err := getAppsecRuleMetric(prometheusURL, hubItem.Name) + if err != nil { + return err + } appsecMetricsTable(color.Output, wantColor, hubItem.Name, metrics) default: // no metrics for this item type } @@ -40,11 +50,15 @@ func showMetrics(prometheusURL string, hubItem *cwhub.Item, wantColor string) er } // getParserMetric is a complete rip from prom2json -func getParserMetric(url string, itemName string) map[string]map[string]int { +func getParserMetric(url string, itemName string) (map[string]map[string]int, error) { stats := make(map[string]map[string]int) - result := getPrometheusMetric(url) - for idx, fam := range result { + results, err := getPrometheusMetric(url) + if err != nil { + return nil, err + } + + for idx, fam := range results { if !strings.HasPrefix(fam.Name, "cs_") { continue } @@ -128,10 +142,10 @@ func getParserMetric(url string, itemName string) map[string]map[string]int { } } - return stats + return stats, nil } -func getScenarioMetric(url string, itemName string) map[string]int { +func getScenarioMetric(url string, itemName string) (map[string]int, error) { stats := make(map[string]int) stats["instantiation"] = 0 @@ -140,8 +154,12 @@ func getScenarioMetric(url string, itemName string) map[string]int { stats["pour"] = 0 stats["underflow"] = 0 - result := getPrometheusMetric(url) - for idx, fam := range result { + results, err := getPrometheusMetric(url) + if err != nil { + return nil, err + } + + for idx, fam := range results { if !strings.HasPrefix(fam.Name, "cs_") { continue } @@ -192,16 +210,20 @@ func getScenarioMetric(url string, itemName string) map[string]int { } } - return stats + return stats, nil } -func getAppsecRuleMetric(url string, itemName string) map[string]int { +func getAppsecRuleMetric(url string, itemName string) (map[string]int, error) { stats := make(map[string]int) stats["inband_hits"] = 0 stats["outband_hits"] = 0 - results := getPrometheusMetric(url) + results, err := getPrometheusMetric(url) + if err != nil { + return nil, err + } + for idx, fam := range results { if !strings.HasPrefix(fam.Name, "cs_") { continue @@ -257,10 +279,10 @@ func getAppsecRuleMetric(url string, itemName string) map[string]int { } } - return stats + return stats, nil } -func getPrometheusMetric(url string) []*prom2json.Family { +func getPrometheusMetric(url string) ([]*prom2json.Family, error) { mfChan := make(chan *dto.MetricFamily, 1024) // Start with the DefaultTransport for sane defaults. @@ -271,12 +293,15 @@ func getPrometheusMetric(url string) []*prom2json.Family { // Timeout early if the server doesn't even return the headers. transport.ResponseHeaderTimeout = time.Minute + var fetchErr error + go func() { defer trace.CatchPanic("crowdsec/GetPrometheusMetric") - err := prom2json.FetchMetricFamilies(url, mfChan, transport) - if err != nil { - log.Fatalf("failed to fetch prometheus metrics : %v", err) + // mfChan is closed by prom2json.FetchMetricFamilies in all cases. + if err := prom2json.FetchMetricFamilies(url, mfChan, transport); err != nil { + fetchErr = fmt.Errorf("failed to fetch prometheus metrics: %w", err) + return } }() @@ -285,7 +310,11 @@ func getPrometheusMetric(url string) []*prom2json.Family { result = append(result, prom2json.NewFamily(mf)) } + if fetchErr != nil { + return nil, fetchErr + } + log.Debugf("Finished reading prometheus output, %d entries", len(result)) - return result + return result, nil } diff --git a/cmd/crowdsec-cli/clilapi/register.go b/cmd/crowdsec-cli/clilapi/register.go index 4c9b0f39903..e8eb7ddc543 100644 --- a/cmd/crowdsec-cli/clilapi/register.go +++ b/cmd/crowdsec-cli/clilapi/register.go @@ -28,7 +28,12 @@ func (cli *cliLapi) register(ctx context.Context, apiURL string, outputFile stri } } - password := strfmt.Password(idgen.GeneratePassword(idgen.PasswordLength)) + pstr, err := idgen.GeneratePassword(idgen.PasswordLength) + if err != nil { + return err + } + + password := strfmt.Password(pstr) apiurl, err := prepareAPIURL(cfg.API.Client, apiURL) if err != nil { diff --git a/cmd/crowdsec-cli/climachine/add.go b/cmd/crowdsec-cli/climachine/add.go index afddb4e4b65..4f28119dde6 100644 --- a/cmd/crowdsec-cli/climachine/add.go +++ b/cmd/crowdsec-cli/climachine/add.go @@ -65,7 +65,10 @@ func (cli *cliMachines) add(ctx context.Context, args []string, machinePassword return errors.New("please specify a password with --password or use --auto") } - machinePassword = idgen.GeneratePassword(idgen.PasswordLength) + machinePassword, err = idgen.GeneratePassword(idgen.PasswordLength) + if err != nil { + return err + } } else if machinePassword == "" && interactive { qs := &survey.Password{ Message: "Please provide a password for the machine:", diff --git a/cmd/crowdsec-cli/dashboard.go b/cmd/crowdsec-cli/dashboard.go index 7ddac093dcd..a653fcb3a47 100644 --- a/cmd/crowdsec-cli/dashboard.go +++ b/cmd/crowdsec-cli/dashboard.go @@ -144,7 +144,11 @@ cscli dashboard setup -l 0.0.0.0 -p 443 --password if metabasePassword == "" { isValid := passwordIsValid(metabasePassword) for !isValid { - metabasePassword = idgen.GeneratePassword(16) + var err error + metabasePassword, err = idgen.GeneratePassword(16) + if err != nil { + return err + } isValid = passwordIsValid(metabasePassword) } } diff --git a/cmd/crowdsec-cli/idgen/machineid.go b/cmd/crowdsec-cli/idgen/machineid.go index 4bd356b3abc..434f79128e9 100644 --- a/cmd/crowdsec-cli/idgen/machineid.go +++ b/cmd/crowdsec-cli/idgen/machineid.go @@ -42,7 +42,11 @@ func GenerateMachineID(prefix string) (string, error) { } prefix = strings.ReplaceAll(prefix, "-", "")[:32] - suffix := GeneratePassword(16) + + suffix, err := GeneratePassword(16) + if err != nil { + return "", err + } return prefix + suffix, nil } diff --git a/cmd/crowdsec-cli/idgen/password.go b/cmd/crowdsec-cli/idgen/password.go index e0faa4daacc..9f1925288ce 100644 --- a/cmd/crowdsec-cli/idgen/password.go +++ b/cmd/crowdsec-cli/idgen/password.go @@ -2,14 +2,13 @@ package idgen import ( saferand "crypto/rand" + "fmt" "math/big" - - log "github.com/sirupsen/logrus" ) const PasswordLength = 64 -func GeneratePassword(length int) string { +func GeneratePassword(length int) (string, error) { upper := "ABCDEFGHIJKLMNOPQRSTUVWXY" lower := "abcdefghijklmnopqrstuvwxyz" digits := "0123456789" @@ -22,11 +21,11 @@ func GeneratePassword(length int) string { for i := range length { rInt, err := saferand.Int(saferand.Reader, big.NewInt(int64(charsetLength))) if err != nil { - log.Fatalf("failed getting data from prng for password generation : %s", err) + return "", fmt.Errorf("prng failed to generate unique id or password: %w", err) } buf[i] = charset[rInt.Int64()] } - return string(buf) + return string(buf), nil } diff --git a/pkg/leakybucket/overflows.go b/pkg/leakybucket/overflows.go index 126bcd05685..62ba3bc9a81 100644 --- a/pkg/leakybucket/overflows.go +++ b/pkg/leakybucket/overflows.go @@ -149,6 +149,7 @@ func eventSources(evt types.Event, leaky *Leaky) (map[string]models.Source, erro leaky.logger.Tracef("Valid range from %s : %s", src.IP, src.Range) } } + if leaky.scopeType.Scope == types.Ip { src.Value = &src.IP } else if leaky.scopeType.Scope == types.Range { @@ -364,6 +365,7 @@ func NewAlert(leaky *Leaky, queue *types.Queue) (types.RuntimeAlert, error) { if err := newApiAlert.Validate(strfmt.Default); err != nil { log.Errorf("Generated alerts isn't valid") log.Errorf("->%s", spew.Sdump(newApiAlert)) + // XXX: deep-exit - note other errors returned from this function are not fatal log.Fatalf("error : %s", err) }