Refact cwhub (#2596)

* unused param
* (slightly) simpler ListItems() -> listItems()
* listItems(): always showHeader, deduce showType
ref. https://github.com/crowdsecurity/crowdsec/issues/1068
* simplify Item.disable()
also, .tainted and .installed do not need a default since they are always in the json output now
* Drop unused parameters

cmd/crowdsec-cli/hub.go

@@ -54,7 +54,16 @@ func runHubList(cmd *cobra.Command, args []string) error {
 		log.Info(line)
 	}
 
-	err = ListItems(hub, color.Output, cwhub.ItemTypes, nil, true, false, all)
+	items := make(map[string][]*cwhub.Item)
+
+	for _, itemType := range cwhub.ItemTypes {
+		items[itemType], err = selectItems(hub, itemType, nil, !all)
+		if err != nil {
+			return err
+		}
+	}
+
+	err = listItems(color.Output, cwhub.ItemTypes, items)
 	if err != nil {
 		return err
 	}

cmd/crowdsec-cli/itemcommands.go

@@ -10,6 +10,7 @@ import (
 	"github.com/crowdsecurity/go-cs-lib/coalesce"
 
 	"github.com/crowdsecurity/crowdsec/cmd/crowdsec-cli/require"
+	"github.com/crowdsecurity/crowdsec/pkg/cwhub"
 )
 
 type cmdHelp struct {
@@ -508,7 +509,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(item, !noMetrics); err != nil {
 				return err
 			}
 		}
@@ -556,7 +557,14 @@ func itemsListRunner(it hubItemType) func(cmd *cobra.Command, args []string) err
 			return err
 		}
 
-		if err = ListItems(hub, color.Output, []string{it.name}, args, false, true, all); err != nil {
+		items := make(map[string][]*cwhub.Item)
+
+		items[it.name], err = selectItems(hub, it.name, args, !all)
+		if err != nil {
+			return err
+		}
+
+		if err = listItems(color.Output, []string{it.name}, items); err != nil {
 			return err
 		}
 

cmd/crowdsec-cli/items.go

@@ -53,23 +53,11 @@ func selectItems(hub *cwhub.Hub, itemType string, args []string, installedOnly b
 	return items, nil
 }
 
-// XXX: too complex, should be two functions (itemtypes array and args are not used together)
-func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string, showType bool, showHeader bool, all bool) error {
-	items := make(map[string][]*cwhub.Item)
-
-	for _, itemType := range itemTypes {
-		selected, err := selectItems(hub, itemType, args, !all)
-		if err != nil {
-			return err
-		}
-
-		items[itemType] = selected
-	}
-
+func listItems(out io.Writer, itemTypes []string, items map[string][]*cwhub.Item) error {
 	switch csConfig.Cscli.Output {
 	case "human":
 		for _, itemType := range itemTypes {
-			listHubItemTable(hub, out, "\n"+strings.ToUpper(itemType), itemType, items[itemType])
+			listHubItemTable(out, "\n"+strings.ToUpper(itemType), items[itemType])
 		}
 	case "json":
 		type itemHubStatus struct {
@@ -98,23 +86,23 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 				}
 			}
 		}
+
 		x, err := json.MarshalIndent(hubStatus, "", " ")
 		if err != nil {
 			return fmt.Errorf("failed to unmarshal: %w", err)
 		}
+
 		out.Write(x)
 	case "raw":
 		csvwriter := csv.NewWriter(out)
 
-		if showHeader {
-			header := []string{"name", "status", "version", "description"}
-			if showType {
-				header = append(header, "type")
-			}
-			err := csvwriter.Write(header)
-			if err != nil {
-				return fmt.Errorf("failed to write header: %s", err)
-			}
+		header := []string{"name", "status", "version", "description"}
+		if len(itemTypes) > 1 {
+			header = append(header, "type")
+		}
+
+		if err := csvwriter.Write(header); err != nil {
+			return fmt.Errorf("failed to write header: %s", err)
 		}
 
 		for _, itemType := range itemTypes {
@@ -126,7 +114,7 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 					item.LocalVersion,
 					item.Description,
 				}
-				if showType {
+				if len(itemTypes) > 1 {
 					row = append(row, itemType)
 				}
 				if err := csvwriter.Write(row); err != nil {
@@ -142,7 +130,7 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 	return nil
 }
 
-func InspectItem(hub *cwhub.Hub, item *cwhub.Item, showMetrics bool) error {
+func InspectItem(item *cwhub.Item, showMetrics bool) error {
 	switch csConfig.Cscli.Output {
 	case "human", "raw":
 		enc := yaml.NewEncoder(os.Stdout)

cmd/crowdsec-cli/support.go

@@ -129,9 +129,18 @@ func collectOSInfo() ([]byte, error) {
 }
 
 func collectHubItems(hub *cwhub.Hub, itemType string) []byte {
+	var err error
+
 	out := bytes.NewBuffer(nil)
 	log.Infof("Collecting %s list", itemType)
-	if err := ListItems(hub, out, []string{itemType}, []string{}, false, true, false); err != nil {
+
+	items := make(map[string][]*cwhub.Item)
+
+	if items[itemType], err = selectItems(hub, itemType, nil, true); err != nil {
+		log.Warnf("could not collect %s list: %s", itemType, err)
+	}
+
+	if err := listItems(out, []string{itemType}, items); err != nil {
 		log.Warnf("could not collect %s list: %s", itemType, err)
 	}
 	return out.Bytes()

cmd/crowdsec-cli/utils_table.go

@@ -11,7 +11,7 @@ import (
 	"github.com/crowdsecurity/crowdsec/pkg/cwhub"
 )
 
-func listHubItemTable(hub *cwhub.Hub, out io.Writer, title string, itemType string, items []*cwhub.Item) {
+func listHubItemTable(out io.Writer, title string, items []*cwhub.Item) {
 	t := newLightTable(out)
 	t.SetHeaders("Name", fmt.Sprintf("%v Status", emoji.Package), "Version", "Local Path")
 	t.SetHeaderAlignment(table.AlignLeft, table.AlignLeft, table.AlignLeft, table.AlignLeft)

pkg/cwhub/enable.go

@@ -110,24 +110,14 @@ func (i *Item) disable(purge bool, force bool) error {
 		return fmt.Errorf("%s is tainted, use '--force' to overwrite", i.Name)
 	}
 
-	// disable sub-items if any - it's a collection
 	for _, sub := range i.SubItems() {
-		// check if the item doesn't belong to another collection before removing it
-		removeSub := true
-
-		for _, collection := range sub.BelongsToCollections {
-			if collection != i.Name {
-				removeSub = false
-				break
-			}
+		if len(sub.BelongsToCollections) > 1 {
+			log.Infof("%s was not removed because it belongs to another collection", sub.Name)
+			continue
 		}
 
-		if removeSub {
-			if err := sub.disable(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", sub.Name)
+		if err := sub.disable(purge, force); err != nil {
+			return fmt.Errorf("while disabling %s: %w", sub.Name, err)
 		}
 	}
 

test/bats/20_hub.bats

@@ -38,7 +38,7 @@ teardown() {
     rune -0 cscli hub list -o json
     assert_json '{parsers:[],scenarios:[],collections:[],postoverflows:[]}'
     rune -0 cscli hub list -o raw
-    refute_output
+    assert_output 'name,status,version,description,type'
 
     # some items
     rune -0 cscli parsers install crowdsecurity/whitelists

test/bats/20_hub_collections_dep.bats

@@ -62,7 +62,7 @@ teardown() {
 
     # and now smb is tainted!
     rune -0 cscli collections inspect crowdsecurity/smb -o json
-    rune -0 jq -e '.tainted//false==true' <(output)
+    rune -0 jq -e '.tainted==true' <(output)
     rune -0 cscli collections remove crowdsecurity/smb --force
 
     # empty
@@ -74,12 +74,18 @@ teardown() {
 
     # taint on sshd means smb is tainted as well
     rune -0 cscli collections inspect crowdsecurity/smb -o json
-    jq -e '.tainted//false==false' <(output)
+    rune -0 jq -e '.tainted==false' <(output)
     echo "dirty" >"$CONFIG_DIR/collections/sshd.yaml"
     rune -0 cscli collections inspect crowdsecurity/smb -o json
-    jq -e '.tainted//false==true' <(output)
+    rune -0 jq -e '.tainted==true' <(output)
 
     # now we can't remove smb without --force
     rune -1 cscli collections remove crowdsecurity/smb
     assert_stderr --partial "unable to disable crowdsecurity/smb: crowdsecurity/smb is tainted, use '--force' to overwrite"
+
+    rune -0 cscli collections install crowdsecurity/wireguard baudneo/gotify
+    rune -0 cscli collections remove crowdsecurity/wireguard
+    assert_stderr --partial "crowdsecurity/syslog-logs was not removed because it belongs to another collection"
+    rune -0 cscli collections inspect crowdsecurity/wireguard -o json
+    rune -0 jq -e '.installed==false' <(output)
 }