Refact cwhub (#2578)

* Fix suggest functional tests
* comments
* non-empty SubItems() implies collections type
* use "slices" from stdlib
* No need to repeat author field in the index -- take it from the item key

cmd/crowdsec-cli/config_restore.go

@@ -22,6 +22,7 @@ type OldAPICfg struct {
 }
 
 // it's a rip of the cli version, but in silent-mode
+// XXX: redundant, should call InstallItem
 func silentInstallItem(hub *cwhub.Hub, name, obtype string) (string, error) {
 	var item = hub.GetItem(obtype, name)
 	if item == nil {

cmd/crowdsec-cli/require/require.go

@@ -23,6 +23,7 @@ func CAPI(c *csconfig.Config) error {
 	if c.API.Server.OnlineClient == nil {
 		return fmt.Errorf("no configuration for Central API (CAPI) in '%s'", *c.FilePath)
 	}
+
 	return nil
 }
 
@@ -30,6 +31,7 @@ func PAPI(c *csconfig.Config) error {
 	if c.API.Server.OnlineClient.Credentials.PapiURL == "" {
 		return fmt.Errorf("no PAPI URL in configuration")
 	}
+
 	return nil
 }
 
@@ -45,6 +47,7 @@ func DB(c *csconfig.Config) error {
 	if err := c.LoadDBConfig(); err != nil {
 		return fmt.Errorf("this command requires direct database access (must be run on the local API machine): %w", err)
 	}
+
 	return nil
 }
 

pkg/cwhub/enable.go

@@ -1,7 +1,6 @@
 package cwhub
 
-// Enable/disable items already installed (no downloading here)
-// This file is not named install.go to avoid confusion with the functions in helpers.go
+// Enable/disable items already downloaded
 
 import (
 	"fmt"
@@ -11,8 +10,9 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-// creates symlink between actual config file at hub.HubDir and hub.ConfigDir
+// EnableItem creates a symlink between actual config file at hub.HubDir and hub.ConfigDir
 // Handles collections recursively
+// XXX: called from config_restore otherwise no need to export
 func (h *Hub) EnableItem(target *Item) error {
 	parentDir := filepath.Clean(h.local.InstallDir + "/" + target.Type + "/" + target.Stage + "/")
 
@@ -42,16 +42,14 @@ func (h *Hub) EnableItem(target *Item) error {
 	}
 
 	// install sub-items if it's a collection
-	if target.Type == COLLECTIONS {
-		for _, sub := range target.SubItems() {
-			val, ok := h.Items[sub.Type][sub.Name]
-			if !ok {
-				return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, target.Name)
-			}
+	for _, sub := range target.SubItems() {
+		val, ok := h.Items[sub.Type][sub.Name]
+		if !ok {
+			return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, target.Name)
+		}
 
-			if err := h.EnableItem(&val); err != nil {
-				return fmt.Errorf("while installing %s: %w", sub.Name, err)
-			}
+		if err := h.EnableItem(&val); err != nil {
+			return fmt.Errorf("while installing %s: %w", sub.Name, err)
 		}
 	}
 
@@ -123,32 +121,30 @@ func (h *Hub) DisableItem(target *Item, purge bool, force bool) error {
 		return fmt.Errorf("%s is tainted, use '--force' to overwrite", target.Name)
 	}
 
-	// for a COLLECTIONS, disable sub-items
-	if target.Type == COLLECTIONS {
-		for _, sub := range target.SubItems() {
-			val, ok := h.Items[sub.Type][sub.Name]
-			if !ok {
-				log.Errorf("Referred %s %s in collection %s doesn't exist.", sub.Type, sub.Name, target.Name)
-				continue
-			}
+	// disable sub-items if any - it's a collection
+	for _, sub := range target.SubItems() {
+		val, ok := h.Items[sub.Type][sub.Name]
+		if !ok {
+			log.Errorf("Referred %s %s in collection %s doesn't exist.", sub.Type, sub.Name, target.Name)
+			continue
+		}
 
-			// check if the item doesn't belong to another collection before removing it
-			toRemove := true
+		// check if the item doesn't belong to another collection before removing it
+		toRemove := true
 
-			for _, collection := range val.BelongsToCollections {
-				if collection != target.Name {
-					toRemove = false
-					break
-				}
+		for _, collection := range val.BelongsToCollections {
+			if collection != target.Name {
+				toRemove = false
+				break
 			}
+		}
 
-			if toRemove {
-				if err = h.DisableItem(&val, purge, force); err != nil {
-					return fmt.Errorf("while disabling %s: %w", sub.Name, err)
-				}
-			} else {
-				log.Infof("%s was not removed because it belongs to another collection", val.Name)
+		if toRemove {
+			if err = h.DisableItem(&val, purge, force); err != nil {
+				return fmt.Errorf("while disabling %s: %w", sub.Name, err)
 			}
+		} else {
+			log.Infof("%s was not removed because it belongs to another collection", val.Name)
 		}
 	}
 

pkg/cwhub/hub.go

@@ -98,15 +98,17 @@ func ParseIndex(buff []byte) (HubItems, error) {
 
 		for name, item := range RawIndex[itemType] {
 			item.Name = name
+
+			// if the item has no (redundant) author, take it from the json key
+			if item.Author == "" && strings.Contains(name, "/") {
+				item.Author = strings.Split(name, "/")[0]
+			}
+
 			item.Type = itemType
 			x := strings.Split(item.RemotePath, "/")
 			item.FileName = x[len(x)-1]
 			RawIndex[itemType][name] = item
 
-			if itemType != COLLECTIONS {
-				continue
-			}
-
 			// if it's a collection, check its sub-items are present
 			// XXX should be done later
 			for _, sub := range item.SubItems() {
@@ -130,11 +132,6 @@ func (h *Hub) ItemStats() []string {
 	loaded := ""
 
 	for _, itemType := range ItemTypes {
-		// ensure the order is always the same
-		if h.Items[itemType] == nil {
-			continue
-		}
-
 		if len(h.Items[itemType]) == 0 {
 			continue
 		}

pkg/cwhub/items.go

@@ -16,7 +16,7 @@ const (
 	SCENARIOS     = "scenarios"
 )
 
-// XXX: The order is important, as it is used to range over sub-items in collections
+// The order is important, as it is used to range over sub-items in collections
 var ItemTypes = []string{PARSERS, POSTOVERFLOWS, SCENARIOS, COLLECTIONS}
 
 type HubItems map[string]map[string]Item

pkg/cwhub/sync.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	log "github.com/sirupsen/logrus"
+	"slices"
 )
 
 func isYAMLFileName(path string) bool {
@@ -255,7 +256,6 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 
 		for _, version := range versions {
 			if item.Versions[version].Digest != sha {
-				// log.Infof("matching filenames, wrong hash %s != %s -- %s", sha, val.Digest, spew.Sdump(v))
 				continue
 			}
 
@@ -365,15 +365,7 @@ func (h *Hub) CollectDepsCheck(v *Item) error {
 			return fmt.Errorf("outdated %s %s", sub.Type, sub.Name)
 		}
 
-		skip := false
-
-		for idx := range subItem.BelongsToCollections {
-			if subItem.BelongsToCollections[idx] == v.Name {
-				skip = true
-			}
-		}
-
-		if !skip {
+		if !slices.Contains(subItem.BelongsToCollections, v.Name) {
 			subItem.BelongsToCollections = append(subItem.BelongsToCollections, v.Name)
 		}
 

test/bats/20_hub_collections.bats

@@ -132,7 +132,7 @@ teardown() {
 
     # autocorrect
     rune -1 cscli collections install crowdsecurity/ssshd
-    assert_stderr --partial "can't find 'crowdsecurity/ssshd' in collections, did you mean crowdsecurity/sshd?"
+    assert_stderr --partial "can't find 'crowdsecurity/ssshd' in collections, did you mean 'crowdsecurity/sshd'?"
 
     # install multiple
     rune -0 cscli collections install crowdsecurity/sshd crowdsecurity/smb

test/bats/20_hub_parsers.bats

@@ -133,7 +133,7 @@ teardown() {
 
     # autocorrect
     rune -1 cscli parsers install crowdsecurity/sshd-logz
-    assert_stderr --partial "can't find 'crowdsecurity/sshd-logz' in parsers, did you mean crowdsecurity/sshd-logs?"
+    assert_stderr --partial "can't find 'crowdsecurity/sshd-logz' in parsers, did you mean 'crowdsecurity/sshd-logs'?"
 
     # install multiple
     rune -0 cscli parsers install crowdsecurity/pgsql-logs crowdsecurity/postfix-logs

test/bats/20_hub_postoverflows.bats

@@ -132,7 +132,7 @@ teardown() {
 
     # autocorrect
     rune -1 cscli postoverflows install crowdsecurity/rdnf
-    assert_stderr --partial "can't find 'crowdsecurity/rdnf' in postoverflows, did you mean crowdsecurity/rdns?"
+    assert_stderr --partial "can't find 'crowdsecurity/rdnf' in postoverflows, did you mean 'crowdsecurity/rdns'?"
 
     # install multiple
     rune -0 cscli postoverflows install crowdsecurity/rdns crowdsecurity/cdn-whitelist

test/bats/20_hub_scenarios.bats

@@ -133,7 +133,7 @@ teardown() {
 
     # autocorrect
     rune -1 cscli scenarios install crowdsecurity/ssh-tf
-    assert_stderr --partial "can't find 'crowdsecurity/ssh-tf' in scenarios, did you mean crowdsecurity/ssh-bf?"
+    assert_stderr --partial "can't find 'crowdsecurity/ssh-tf' in scenarios, did you mean 'crowdsecurity/ssh-bf'?"
 
     # install multiple
     rune -0 cscli scenarios install crowdsecurity/ssh-bf crowdsecurity/telnet-bf