Refact cwhub: minor cleanups and comments (#2574)

* check response status before body; close file
* err check one-liners, lint, comments
* simplify function logic, reduce code
* comments, xxx, whitespace

cmd/crowdsec-cli/item_suggest.go

@@ -5,7 +5,6 @@ import (
 	"strings"
 
 	"github.com/agext/levenshtein"
-	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"slices"
 
@@ -15,38 +14,26 @@ import (
 
 const MaxDistance = 7
 
-func Suggest(itemType string, baseItem string, suggestItem string, score int, ignoreErr bool) {
-	errMsg := ""
-	if score < MaxDistance {
-		errMsg = fmt.Sprintf("can't find '%s' in %s, did you mean %s?", baseItem, itemType, suggestItem)
-	} else {
-		errMsg = fmt.Sprintf("can't find '%s' in %s", baseItem, itemType)
-	}
-	if ignoreErr {
-		log.Error(errMsg)
-	} else {
-		log.Fatalf(errMsg)
+// SuggestNearestMessage returns a message with the most similar item name, if one is found
+func SuggestNearestMessage(hub *cwhub.Hub, itemType string, itemName string) string {
+	score := 100
+	nearest := ""
+
+	for _, item := range hub.GetItemMap(itemType) {
+		d := levenshtein.Distance(itemName, item.Name, nil)
+		if d < score {
+			score = d
+			nearest = item.Name
+		}
 	}
-}
 
-func GetDistance(hub *cwhub.Hub, itemType string, itemName string) (*cwhub.Item, int) {
-	allItems := make([]string, 0)
-	nearestScore := 100
-	nearestItem := &cwhub.Item{}
+	msg := fmt.Sprintf("can't find '%s' in %s", itemName, itemType)
 
-	hubItems := hub.GetItemMap(itemType)
-	for _, item := range hubItems {
-		allItems = append(allItems, item.Name)
+	if score < MaxDistance {
+		msg += fmt.Sprintf(", did you mean '%s'?", nearest)
 	}
 
-	for _, s := range allItems {
-		d := levenshtein.Distance(itemName, s, nil)
-		if d < nearestScore {
-			nearestScore = d
-			nearestItem = hub.GetItem(itemType, s)
-		}
-	}
-	return nearestItem, nearestScore
+	return msg
 }
 
 func compAllItems(itemType string, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
@@ -56,13 +43,15 @@ func compAllItems(itemType string, args []string, toComplete string) ([]string,
 	}
 
 	comp := make([]string, 0)
-	hubItems := hub.GetItemMap(itemType)
-	for _, item := range hubItems {
+
+	for _, item := range hub.GetItemMap(itemType) {
 		if !slices.Contains(args, item.Name) && strings.Contains(item.Name, toComplete) {
 			comp = append(comp, item.Name)
 		}
 	}
+
 	cobra.CompDebugln(fmt.Sprintf("%s: %+v", itemType, comp), true)
+
 	return comp, cobra.ShellCompDirectiveNoFileComp
 }
 

cmd/crowdsec-cli/itemcommands.go

@@ -203,11 +203,13 @@ func itemsInstallRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 		}
 
 		for _, name := range args {
-			t := hub.GetItem(it.name, name)
-			if t == nil {
-				nearestItem, score := GetDistance(hub, it.name, name)
-				Suggest(it.name, name, nearestItem.Name, score, ignoreError)
+			if hub.GetItem(it.name, name) == nil {
+				msg := SuggestNearestMessage(hub, it.name, name)
+				if !ignoreError {
+					return fmt.Errorf(msg)
+				}
 
+				log.Errorf(msg)
 				continue
 			}
 
@@ -218,6 +220,7 @@ func itemsInstallRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 				log.Errorf("Error while installing '%s': %s", name, err)
 			}
 		}
+
 		// XXX: only reload if we installed something
 		log.Infof(ReloadMessage())
 		return nil
@@ -281,6 +284,7 @@ func itemsRemoveRunner(it hubItemType) func(cmd *cobra.Command, args []string) e
 			}
 
 			removed := 0
+
 			for _, item := range items {
 				didRemove, err := hub.RemoveItem(it.name, item.Name, purge, force)
 				if err != nil {
@@ -290,6 +294,7 @@ func itemsRemoveRunner(it hubItemType) func(cmd *cobra.Command, args []string) e
 					removed++
 				}
 			}
+
 			log.Infof("Removed %d %s", removed, it.name)
 			if removed > 0 {
 				log.Infof(ReloadMessage())
@@ -478,7 +483,7 @@ func itemsInspectRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 			if item == nil {
 				return fmt.Errorf("can't find '%s' in %s", name, it.name)
 			}
-			if err = InspectItem(hub, item, noMetrics); err != nil {
+			if err = InspectItem(hub, item, !noMetrics); err != nil {
 				return err
 			}
 		}

cmd/crowdsec-cli/items.go

@@ -19,6 +19,7 @@ func selectItems(hub *cwhub.Hub, itemType string, args []string, installedOnly b
 	itemNames := hub.GetItemNames(itemType)
 
 	notExist := []string{}
+
 	if len(args) > 0 {
 		for _, arg := range args {
 			if !slices.Contains(itemNames, arg) {
@@ -98,6 +99,7 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 		out.Write(x)
 	case "raw":
 		csvwriter := csv.NewWriter(out)
+
 		if showHeader {
 			header := []string{"name", "status", "version", "description"}
 			if showType {
@@ -124,8 +126,7 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 				if showType {
 					row = append(row, itemType)
 				}
-				err := csvwriter.Write(row)
-				if err != nil {
+				if err := csvwriter.Write(row); err != nil {
 					return fmt.Errorf("failed to write raw output: %s", err)
 				}
 			}
@@ -138,34 +139,26 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 	return nil
 }
 
-func InspectItem(hub *cwhub.Hub, item *cwhub.Item, noMetrics bool) error {
-	var (
-		b   []byte
-		err error
-	)
-
+func InspectItem(hub *cwhub.Hub, item *cwhub.Item, showMetrics bool) error {
 	switch csConfig.Cscli.Output {
 	case "human", "raw":
 		enc := yaml.NewEncoder(os.Stdout)
 		enc.SetIndent(2)
-		err = enc.Encode(item)
-		if err != nil {
+		if err := enc.Encode(item); err != nil {
 			return fmt.Errorf("unable to encode item: %s", err)
 		}
 	case "json":
-		b, err = json.MarshalIndent(*item, "", "  ")
+		b, err := json.MarshalIndent(*item, "", "  ")
 		if err != nil {
 			return fmt.Errorf("unable to marshal item: %s", err)
 		}
-		fmt.Printf("%s", string(b))
+		fmt.Print(string(b))
 	}
 
-	if noMetrics || csConfig.Cscli.Output == "json" || csConfig.Cscli.Output == "raw" {
-		return nil
+	if csConfig.Cscli.Output == "human" && showMetrics {
+		fmt.Printf("\nCurrent metrics: \n")
+		ShowMetrics(hub, item)
 	}
 
-	fmt.Printf("\nCurrent metrics: \n")
-	ShowMetrics(hub, item)
-
 	return nil
 }

pkg/cwhub/cwhub.go

@@ -3,9 +3,3 @@
 // This includes retrieving the index, the items to install (parsers, scenarios, data files...)
 // and managing the dependencies and taints.
 package cwhub
-
-import (
-	"errors"
-)
-
-var ErrMissingReference = errors.New("Reference(s) missing in collection")

pkg/cwhub/dataset.go

@@ -32,27 +32,27 @@ func downloadFile(url string, destPath string) error {
 	}
 	defer resp.Body.Close()
 
+	if resp.StatusCode != http.StatusOK {
+		return fmt.Errorf("got HTTP status '%s' from %s", resp.Status, url)
+	}
+
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return err
 	}
 
-	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("download response 'HTTP %d' : %s", resp.StatusCode, string(body))
-	}
-
 	file, err := os.OpenFile(destPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o644)
 	if err != nil {
 		return err
 	}
+	defer file.Close()
 
 	_, err = file.Write(body)
 	if err != nil {
 		return err
 	}
 
-	err = file.Sync()
-	if err != nil {
+	if err = file.Sync(); err != nil {
 		return err
 	}
 
@@ -64,8 +64,7 @@ func GetData(data []*types.DataSource, dataDir string) error {
 		destPath := filepath.Join(dataDir, dataS.DestPath)
 		log.Infof("downloading data '%s' in '%s'", dataS.SourceURL, destPath)
 
-		err := downloadFile(dataS.SourceURL, destPath)
-		if err != nil {
+		if err := downloadFile(dataS.SourceURL, destPath); err != nil {
 			return err
 		}
 	}
@@ -75,15 +74,12 @@ func GetData(data []*types.DataSource, dataDir string) error {
 
 // downloadData downloads the data files for an item
 func downloadData(dataFolder string, force bool, reader io.Reader) error {
-	var err error
-
 	dec := yaml.NewDecoder(reader)
 
 	for {
 		data := &DataSet{}
 
-		err = dec.Decode(data)
-		if err != nil {
+		if err := dec.Decode(data); err != nil {
 			if errors.Is(err, io.EOF) {
 				break
 			}
@@ -94,14 +90,13 @@ func downloadData(dataFolder string, force bool, reader io.Reader) error {
 		download := false
 
 		for _, dataS := range data.Data {
-			if _, err = os.Stat(filepath.Join(dataFolder, dataS.DestPath)); os.IsNotExist(err) {
+			if _, err := os.Stat(filepath.Join(dataFolder, dataS.DestPath)); os.IsNotExist(err) {
 				download = true
 			}
 		}
 
 		if download || force {
-			err = GetData(data.Data, dataFolder)
-			if err != nil {
+			if err := GetData(data.Data, dataFolder); err != nil {
 				return fmt.Errorf("while getting data: %w", err)
 			}
 		}

pkg/cwhub/enable.go

@@ -14,8 +14,6 @@ import (
 // creates symlink between actual config file at hub.HubDir and hub.ConfigDir
 // Handles collections recursively
 func (h *Hub) EnableItem(target *Item) error {
-	var err error
-
 	parentDir := filepath.Clean(h.local.InstallDir + "/" + target.Type + "/" + target.Stage + "/")
 
 	// create directories if needed
@@ -35,7 +33,7 @@ func (h *Hub) EnableItem(target *Item) error {
 		}
 	}
 
-	if _, err = os.Stat(parentDir); os.IsNotExist(err) {
+	if _, err := os.Stat(parentDir); os.IsNotExist(err) {
 		log.Infof("%s doesn't exist, create", parentDir)
 
 		if err = os.MkdirAll(parentDir, os.ModePerm); err != nil {
@@ -51,15 +49,14 @@ func (h *Hub) EnableItem(target *Item) error {
 				return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, target.Name)
 			}
 
-			err = h.EnableItem(&val)
-			if err != nil {
+			if err := h.EnableItem(&val); err != nil {
 				return fmt.Errorf("while installing %s: %w", sub.Name, err)
 			}
 		}
 	}
 
 	// check if file already exists where it should in configdir (eg /etc/crowdsec/collections/)
-	if _, err = os.Lstat(parentDir + "/" + target.FileName); !os.IsNotExist(err) {
+	if _, err := os.Lstat(parentDir + "/" + target.FileName); !os.IsNotExist(err) {
 		log.Infof("%s already exists.", parentDir+"/"+target.FileName)
 		return nil
 	}
@@ -79,7 +76,7 @@ func (h *Hub) EnableItem(target *Item) error {
 		return fmt.Errorf("while creating symlink from %s to %s: %w", srcPath, dstPath, err)
 	}
 
-	log.Infof("Enabled %s : %s", target.Type, target.Name)
+	log.Infof("Enabled %s: %s", target.Type, target.Name)
 	target.Installed = true
 	h.Items[target.Type][target.Name] = *target
 
@@ -103,6 +100,7 @@ func (h *Hub) purgeItem(target Item) (Item, error) {
 
 // DisableItem to disable an item managed by the hub, removes the symlink if purge is true
 func (h *Hub) DisableItem(target *Item, purge bool, force bool) error {
+	// XXX: should return the number of disabled/purged items to inform the upper layer whether to reload or not
 	var err error
 
 	// already disabled, noop unless purge
@@ -145,8 +143,7 @@ func (h *Hub) DisableItem(target *Item, purge bool, force bool) error {
 			}
 
 			if toRemove {
-				err = h.DisableItem(&val, purge, force)
-				if err != nil {
+				if err = h.DisableItem(&val, purge, force); err != nil {
 					return fmt.Errorf("while disabling %s: %w", sub.Name, err)
 				}
 			} else {
@@ -164,7 +161,7 @@ func (h *Hub) DisableItem(target *Item, purge bool, force bool) error {
 	if os.IsNotExist(err) {
 		// we only accept to "delete" non existing items if it's a forced purge
 		if !purge && !force {
-			return fmt.Errorf("can't delete %s : %s doesn't exist", target.Name, syml)
+			return fmt.Errorf("can't delete %s: %s doesn't exist", target.Name, syml)
 		}
 	} else {
 		// if it's managed by hub, it's a symlink to csconfig.GConfig.hub.HubDir / ...
@@ -193,7 +190,7 @@ func (h *Hub) DisableItem(target *Item, purge bool, force bool) error {
 			return fmt.Errorf("while removing symlink: %w", err)
 		}
 
-		log.Infof("Removed symlink [%s] : %s", target.Name, syml)
+		log.Infof("Removed symlink [%s]: %s", target.Name, syml)
 	}
 
 	target.Installed = false

pkg/cwhub/enable_test.go

@@ -72,7 +72,7 @@ func testDisable(hub *Hub, t *testing.T, item Item) {
 	// Local sync and check status
 	warns, err := hub.LocalSync()
 	require.NoError(t, err, "failed to run localSync")
-	require.Empty(t, warns, "unexpected warnings : %+v", warns)
+	require.Empty(t, warns, "unexpected warnings: %+v", warns)
 
 	assert.False(t, hub.Items[item.Type][item.Name].Tainted, "%s should not be tainted anymore", item.Name)
 	assert.False(t, hub.Items[item.Type][item.Name].Installed, "%s should not be installed anymore", item.Name)
@@ -85,7 +85,7 @@ func testDisable(hub *Hub, t *testing.T, item Item) {
 	// Local sync and check status
 	warns, err = hub.LocalSync()
 	require.NoError(t, err, "failed to run localSync")
-	require.Empty(t, warns, "unexpected warnings : %+v", warns)
+	require.Empty(t, warns, "unexpected warnings: %+v", warns)
 
 	assert.False(t, hub.Items[item.Type][item.Name].Installed, "%s should not be installed anymore", item.Name)
 	assert.False(t, hub.Items[item.Type][item.Name].Downloaded, "%s should not be downloaded", item.Name)

pkg/cwhub/errors.go

@@ -0,0 +1,12 @@
+package cwhub
+
+import (
+	"errors"
+)
+
+var (
+	// ErrNilRemoteHub is returned when the remote hub configuration is not provided to the NewHub constructor.
+	// All attempts to download index or items will return this error.
+	ErrMissingReference = errors.New("Reference(s) missing in collection")
+	ErrNilRemoteHub = errors.New("remote hub configuration is not provided. Please report this issue to the developers")
+)

pkg/cwhub/helpers.go

@@ -34,22 +34,24 @@ func (h *Hub) InstallItem(name string, itemType string, force bool, downloadOnly
 		}
 	}
 
-	err := h.DownloadLatest(item, force, true)
-	if err != nil {
+	// XXX: confusing semantic between force and updateOnly?
+	if err := h.DownloadLatest(item, force, true); err != nil {
 		return fmt.Errorf("while downloading %s: %w", item.Name, err)
 	}
 
-	if err = h.AddItem(*item); err != nil {
+	if err := h.AddItem(*item); err != nil {
 		return fmt.Errorf("while adding %s: %w", item.Name, err)
 	}
 
 	if downloadOnly {
+		// XXX: should get the path from DownloadLatest
 		log.Infof("Downloaded %s to %s", item.Name, filepath.Join(h.local.HubDir, item.RemotePath))
 		return nil
 	}
 
-	err = h.EnableItem(item)
-	if err != nil {
+	// XXX: should we stop here if the item is already installed?
+
+	if err := h.EnableItem(item); err != nil {
 		return fmt.Errorf("while enabling %s: %w", item.Name, err)
 	}
 
@@ -153,13 +155,12 @@ func (h *Hub) UpgradeItem(itemType string, name string, force bool) (bool, error
 
 // DownloadLatest will download the latest version of Item to the tdir directory
 func (h *Hub) DownloadLatest(target *Item, overwrite bool, updateOnly bool) error {
-	var err error
-
+	// XXX: should return the path of the downloaded file (taken from DownloadItem)
 	log.Debugf("Downloading %s %s", target.Type, target.Name)
 
 	if target.Type != COLLECTIONS {
 		if !target.Installed && updateOnly && target.Downloaded {
-			log.Debugf("skipping upgrade of %s : not installed", target.Name)
+			log.Debugf("skipping upgrade of %s: not installed", target.Name)
 			return nil
 		}
 
@@ -174,32 +175,31 @@ func (h *Hub) DownloadLatest(target *Item, overwrite bool, updateOnly bool) erro
 		}
 
 		if !val.Installed && updateOnly && val.Downloaded {
-			log.Debugf("skipping upgrade of %s : not installed", target.Name)
+			log.Debugf("skipping upgrade of %s: not installed", target.Name)
 			continue
 		}
 
-		log.Debugf("Download %s sub-item : %s %s (%t -> %t)", target.Name, sub.Type, sub.Name, target.Installed, updateOnly)
-		//recurse as it's a collection
+		log.Debugf("Download %s sub-item: %s %s (%t -> %t)", target.Name, sub.Type, sub.Name, target.Installed, updateOnly)
+
+		// recurse as it's a collection
 		if sub.Type == COLLECTIONS {
 			log.Tracef("collection, recurse")
 
-			err = h.DownloadLatest(&val, overwrite, updateOnly)
-			if err != nil {
+			if err := h.DownloadLatest(&val, overwrite, updateOnly); err != nil {
 				return fmt.Errorf("while downloading %s: %w", val.Name, err)
 			}
 		}
 
 		downloaded := val.Downloaded
 
-		err = h.DownloadItem(&val, overwrite)
-		if err != nil {
+		if err := h.DownloadItem(&val, overwrite); err != nil {
 			return fmt.Errorf("while downloading %s: %w", val.Name, err)
 		}
 
 		// We need to enable an item when it has been added to a collection since latest release of the collection.
 		// We check if val.Downloaded is false because maybe the item has been disabled by the user.
 		if !val.Installed && !downloaded {
-			if err = h.EnableItem(&val); err != nil {
+			if err := h.EnableItem(&val); err != nil {
 				return fmt.Errorf("enabling '%s': %w", val.Name, err)
 			}
 		}
@@ -207,8 +207,7 @@ func (h *Hub) DownloadLatest(target *Item, overwrite bool, updateOnly bool) erro
 		h.Items[sub.Type][sub.Name] = val
 	}
 
-	err = h.DownloadItem(target, overwrite)
-	if err != nil {
+	if err := h.DownloadItem(target, overwrite); err != nil {
 		return fmt.Errorf("failed to download item: %w", err)
 	}
 
@@ -226,13 +225,13 @@ func (h *Hub) DownloadItem(target *Item, overwrite bool) error {
 	// if user didn't --force, don't overwrite local, tainted, up-to-date files
 	if !overwrite {
 		if target.Tainted {
-			log.Debugf("%s : tainted, not updated", target.Name)
+			log.Debugf("%s: tainted, not updated", target.Name)
 			return nil
 		}
 
 		if target.UpToDate {
 			//  We still have to check if data files are present
-			log.Debugf("%s : up-to-date, not updated", target.Name)
+			log.Debugf("%s: up-to-date, not updated", target.Name)
 		}
 	}
 
@@ -245,13 +244,12 @@ func (h *Hub) DownloadItem(target *Item, overwrite bool) error {
 	if err != nil {
 		return fmt.Errorf("while downloading %s: %w", req.URL.String(), err)
 	}
+	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("bad http code %d for %s", resp.StatusCode, req.URL.String())
 	}
 
-	defer resp.Body.Close()
-
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return fmt.Errorf("while reading %s: %w", req.URL.String(), err)
@@ -296,10 +294,10 @@ func (h *Hub) DownloadItem(target *Item, overwrite bool) error {
 
 	// check actual file
 	if _, err = os.Stat(finalPath); !os.IsNotExist(err) {
-		log.Warningf("%s : overwrite", target.Name)
+		log.Warningf("%s: overwrite", target.Name)
 		log.Debugf("target: %s/%s", tdir, target.RemotePath)
 	} else {
-		log.Infof("%s : OK", target.Name)
+		log.Infof("%s: OK", target.Name)
 	}
 
 	f, err := os.OpenFile(tdir+"/"+target.RemotePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)

pkg/cwhub/hub.go

@@ -44,8 +44,7 @@ func NewHub(local *csconfig.LocalHubCfg, remote *RemoteHubCfg, downloadIndex boo
 	}
 
 	if downloadIndex {
-		err := remote.DownloadIndex(local.HubIndexFile)
-		if err != nil {
+		if err := remote.DownloadIndex(local.HubIndexFile); err != nil {
 			return nil, err
 		}
 	}
@@ -73,8 +72,7 @@ func NewHub(local *csconfig.LocalHubCfg, remote *RemoteHubCfg, downloadIndex boo
 		remote: remote,
 	}
 
-	_, err = theHub.LocalSync()
-	if err != nil {
+	if _, err = theHub.LocalSync(); err != nil {
 		return nil, fmt.Errorf("failed to sync hub index: %w", err)
 	}
 

pkg/cwhub/items.go

@@ -76,6 +76,7 @@ func (i *Item) IsLocal() bool {
 // It must not use a pointer receiver
 func (i Item) MarshalJSON() ([]byte, error) {
 	type Alias Item
+
 	return json.Marshal(&struct {
 		Alias
 		Local bool `json:"local"` // XXX: omitempty?
@@ -90,6 +91,7 @@ func (i Item) MarshalJSON() ([]byte, error) {
 // It must not use a pointer receiver
 func (i Item) MarshalYAML() (interface{}, error) {
 	type Alias Item
+
 	return &struct {
 		Alias `yaml:",inline"`
 		Local bool `yaml:"local"`

pkg/cwhub/items_test.go

@@ -15,7 +15,7 @@ func TestItemStatus(t *testing.T) {
 	x := hub.GetItemMap(COLLECTIONS)
 	require.NotEmpty(t, x)
 
-	// Get item : good and bad
+	// Get item: good and bad
 	for k := range x {
 		item := hub.GetItem(COLLECTIONS, k)
 		require.NotNil(t, item)
@@ -52,7 +52,7 @@ func TestGetters(t *testing.T) {
 	x := hub.GetItemMap(COLLECTIONS)
 	require.NotEmpty(t, x)
 
-	// Get item : good and bad
+	// Get item: good and bad
 	for k := range x {
 		empty := hub.GetItem(COLLECTIONS, k+"nope")
 		require.Nil(t, empty)

pkg/cwhub/remote.go

@@ -17,9 +17,6 @@ type RemoteHubCfg struct {
 	IndexPath   string
 }
 
-// ErrNilRemoteHub is returned when the remote hub configuration is not provided to the NewHub constructor. All attempts to download index or items will return this error.
-var ErrNilRemoteHub = fmt.Errorf("remote hub configuration is not provided. Please report this issue to the developers")
-
 func (r *RemoteHubCfg) urlTo(remotePath string) (string, error) {
 	if r == nil {
 		return "", ErrNilRemoteHub
@@ -59,7 +56,7 @@ func (r *RemoteHubCfg) DownloadIndex(localPath string) error {
 			return ErrIndexNotFound
 		}
 
-		return fmt.Errorf("bad http code %d while requesting %s", resp.StatusCode, req.URL.String())
+		return fmt.Errorf("bad http code %d for %s", resp.StatusCode, req.URL.String())
 	}
 
 	body, err := io.ReadAll(resp.Body)

pkg/cwhub/sync.go

@@ -81,7 +81,7 @@ func (h *Hub) getItemInfo(path string) (itemFileInfo, bool, error) {
 		//.../hub/scenarios/crowdsec/ssh_bf.yaml
 		//.../hub/profiles/crowdsec/linux.yaml
 		if len(subs) < 4 {
-			return itemFileInfo{}, false, fmt.Errorf("path is too short : %s (%d)", path, len(subs))
+			return itemFileInfo{}, false, fmt.Errorf("path is too short: %s (%d)", path, len(subs))
 		}
 
 		ret.fname = subs[len(subs)-1]
@@ -115,7 +115,7 @@ func (h *Hub) getItemInfo(path string) (itemFileInfo, bool, error) {
 		ret.ftype = COLLECTIONS
 		ret.stage = ""
 	} else if ret.ftype != PARSERS && ret.ftype != POSTOVERFLOWS {
-		// its a PARSER / POSTOVERFLOW with a stage
+		// it's a PARSER / POSTOVERFLOW with a stage
 		return itemFileInfo{}, inhub, fmt.Errorf("unknown configuration type for file '%s'", path)
 	}
 
@@ -205,7 +205,7 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 	match := false
 
 	for name, item := range h.Items[info.ftype] {
-		log.Tracef("check [%s] vs [%s] : %s", info.fname, item.RemotePath, info.ftype+"/"+info.stage+"/"+info.fname+".yaml")
+		log.Tracef("check [%s] vs [%s]: %s", info.fname, item.RemotePath, info.ftype+"/"+info.stage+"/"+info.fname+".yaml")
 
 		if info.fname != item.FileName {
 			log.Tracef("%s != %s (filename)", info.fname, item.FileName)
@@ -241,10 +241,11 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 
 		sha, err := getSHA256(path)
 		if err != nil {
-			log.Fatalf("Failed to get sha of %s : %v", path, err)
+			log.Fatalf("Failed to get sha of %s: %v", path, err)
 		}
 
 		// let's reverse sort the versions to deal with hash collisions (#154)
+		// XXX: we sure, lexical sorting?
 		versions := make([]string, 0, len(item.Versions))
 		for k := range item.Versions {
 			versions = append(versions, k)
@@ -253,8 +254,7 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 		sort.Sort(sort.Reverse(sort.StringSlice(versions)))
 
 		for _, version := range versions {
-			val := item.Versions[version]
-			if sha != val.Digest {
+			if item.Versions[version].Digest != sha {
 				// log.Infof("matching filenames, wrong hash %s != %s -- %s", sha, val.Digest, spew.Sdump(v))
 				continue
 			}
@@ -287,6 +287,7 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 			log.Tracef("got tainted match for %s: %s", item.Name, path)
 
 			h.skippedTainted++
+
 			// the file and the stage is right, but the hash is wrong, it has been tainted by user
 			if !inhub {
 				item.LocalPath = path
@@ -400,8 +401,7 @@ func (h *Hub) SyncDir(dir string) ([]string, error) {
 			continue
 		}
 
-		err = filepath.WalkDir(cpath, h.itemVisit)
-		if err != nil {
+		if err = filepath.WalkDir(cpath, h.itemVisit); err != nil {
 			return warnings, err
 		}
 	}
@@ -424,7 +424,7 @@ func (h *Hub) SyncDir(dir string) ([]string, error) {
 			warnings = append(warnings, fmt.Sprintf("collection %s is in the future (currently:%s, latest:%s)", item.Name, item.LocalVersion, item.Version))
 		}
 
-		log.Debugf("installed (%s) - status:%d | installed:%s | latest : %s | full : %+v", item.Name, vs, item.LocalVersion, item.Version, item.Versions)
+		log.Debugf("installed (%s) - status: %d | installed: %s | latest: %s | full: %+v", item.Name, vs, item.LocalVersion, item.Version, item.Versions)
 	}
 
 	return warnings, nil
@@ -440,8 +440,7 @@ func (h *Hub) LocalSync() ([]string, error) {
 		return warnings, fmt.Errorf("failed to scan %s: %w", h.local.InstallDir, err)
 	}
 
-	_, err = h.SyncDir(h.local.HubDir)
-	if err != nil {
+	if _, err = h.SyncDir(h.local.HubDir); err != nil {
 		return warnings, fmt.Errorf("failed to scan %s: %w", h.local.HubDir, err)
 	}