Refact pkg/cwhub (part 4) (#2518)

* generalize function: GetInstalledItems, GetInstalledItemsAsString
* extracted function itemKey, happy path
* review comments / remove redundant; rename file to remove build tags
* remove unused fields in Item struct
* unix build tag

cmd/crowdsec-cli/capi.go

@@ -156,7 +156,7 @@ func NewCapiStatusCmd() *cobra.Command {
 				log.Info("Run 'sudo cscli hub update' to get the hub index")
 				log.Fatalf("Failed to load hub index : %s", err)
 			}
-			scenarios, err := cwhub.GetInstalledScenariosAsString()
+			scenarios, err := cwhub.GetInstalledItemsAsString(cwhub.SCENARIOS)
 			if err != nil {
 				log.Fatalf("failed to get scenarios : %s", err)
 			}

cmd/crowdsec-cli/console.go

@@ -75,7 +75,7 @@ After running this command your will need to validate the enrollment in the weba
 				return err
 			}
 
-			scenarios, err := cwhub.GetInstalledScenariosAsString()
+			scenarios, err := cwhub.GetInstalledItemsAsString(cwhub.SCENARIOS)
 			if err != nil {
 				return fmt.Errorf("failed to get installed scenarios: %s", err)
 			}

cmd/crowdsec-cli/lapi.go

@@ -42,7 +42,7 @@ func runLapiStatus(cmd *cobra.Command, args []string) error {
 		log.Fatal(err)
 	}
 
-	scenarios, err := cwhub.GetInstalledScenariosAsString()
+	scenarios, err := cwhub.GetInstalledItemsAsString(cwhub.SCENARIOS)
 	if err != nil {
 		log.Fatalf("failed to get scenarios : %s", err)
 	}

cmd/crowdsec-cli/support.go

@@ -167,7 +167,7 @@ func collectAPIStatus(login string, password string, endpoint string, prefix str
 	if err != nil {
 		return []byte(fmt.Sprintf("cannot parse API URL: %s", err))
 	}
-	scenarios, err := cwhub.GetInstalledScenariosAsString()
+	scenarios, err := cwhub.GetInstalledItemsAsString(cwhub.SCENARIOS)
 	if err != nil {
 		return []byte(fmt.Sprintf("could not collect scenarios: %s", err))
 	}

cmd/crowdsec-cli/utils.go

@@ -120,25 +120,12 @@ func compInstalledItems(itemType string, args []string, toComplete string) ([]st
 		return nil, cobra.ShellCompDirectiveDefault
 	}
 
-	var items []string
-	var err error
-	switch itemType {
-	case cwhub.PARSERS:
-		items, err = cwhub.GetInstalledParsersAsString()
-	case cwhub.SCENARIOS:
-		items, err = cwhub.GetInstalledScenariosAsString()
-	case cwhub.PARSERS_OVFLW:
-		items, err = cwhub.GetInstalledPostOverflowsAsString()
-	case cwhub.COLLECTIONS:
-		items, err = cwhub.GetInstalledCollectionsAsString()
-	default:
-		return nil, cobra.ShellCompDirectiveDefault
-	}
-
+	items, err := cwhub.GetInstalledItemsAsString(itemType)
 	if err != nil {
 		cobra.CompDebugln(fmt.Sprintf("list installed %s err: %s", itemType, err), true)
 		return nil, cobra.ShellCompDirectiveDefault
 	}
+
 	comp := make([]string, 0)
 
 	if toComplete != "" {

cmd/crowdsec/output.go

@@ -70,7 +70,7 @@ func runOutput(input chan types.Event, overflow chan types.Event, buckets *leaky
 	var cache []types.RuntimeAlert
 	var cacheMutex sync.Mutex
 
-	scenarios, err := cwhub.GetInstalledScenariosAsString()
+	scenarios, err := cwhub.GetInstalledItemsAsString(cwhub.SCENARIOS)
 	if err != nil {
 		return fmt.Errorf("loading list of installed hub scenarios: %w", err)
 	}
@@ -93,7 +93,7 @@ func runOutput(input chan types.Event, overflow chan types.Event, buckets *leaky
 		URL:            apiURL,
 		PapiURL:        papiURL,
 		VersionPrefix:  "v1",
-		UpdateScenario: cwhub.GetInstalledScenariosAsString,
+		UpdateScenario: func() ([]string, error) {return cwhub.GetInstalledItemsAsString(cwhub.SCENARIOS)},
 	})
 	if err != nil {
 		return fmt.Errorf("new client api: %w", err)

pkg/cwhub/cwhub.go

@@ -15,7 +15,7 @@ import (
 	"golang.org/x/mod/semver"
 )
 
-// managed configuration types
+// managed item types
 const PARSERS = "parsers"
 const PARSERS_OVFLW = "postoverflows"
 const SCENARIOS = "scenarios"
@@ -31,7 +31,7 @@ var HubBranch = "master"
 const HubIndexFile = ".index.json"
 
 type ItemVersion struct {
-	Digest     string `json:"digest,omitempty"`
+	Digest     string `json:"digest,omitempty"`	// meow
 	Deprecated bool   `json:"deprecated,omitempty"`
 }
 
@@ -44,7 +44,7 @@ type ItemHubStatus struct {
 	Status       string `json:"status"`
 }
 
-// Item can be: parsed, scenario, collection
+// Item can be: parser, scenario, collection..
 type Item struct {
 	// descriptive info
 	Type                 string   `json:"type,omitempty"                   yaml:"type,omitempty"`                   // parser|postoverflows|scenario|collection(|enrich)
@@ -54,18 +54,15 @@ type Item struct {
 	Description          string   `json:"description,omitempty"            yaml:"description,omitempty"`            // as seen in .config.json
 	Author               string   `json:"author,omitempty"`                                                         // as seen in .config.json
 	References           []string `json:"references,omitempty"             yaml:"references,omitempty"`             // as seen in .config.json
-	BelongsToCollections []string `json:"belongs_to_collections,omitempty" yaml:"belongs_to_collections,omitempty"` // if it's part of collections, track name here
+	BelongsToCollections []string `json:"belongs_to_collections,omitempty" yaml:"belongs_to_collections,omitempty"` // parent collection if any
 
 	// remote (hub) info
-	RemoteURL  string                 `json:"remoteURL,omitempty" yaml:"remoteURL,omitempty"`   // the full remote uri of file in http
-	RemotePath string                 `json:"path,omitempty"      yaml:"remote_path,omitempty"` // the path relative to git ie. /parsers/stage/author/file.yaml
-	RemoteHash string                 `json:"hash,omitempty"      yaml:"hash,omitempty"`        // the meow
+	RemotePath string                 `json:"path,omitempty"      yaml:"remote_path,omitempty"` // the path relative to (git | hub API) ie. /parsers/stage/author/file.yaml
 	Version    string                 `json:"version,omitempty"`                                // the last version
 	Versions   map[string]ItemVersion `json:"versions,omitempty"  yaml:"-"`                     // the list of existing versions
 
 	// local (deployed) info
 	LocalPath string `json:"local_path,omitempty" yaml:"local_path,omitempty"` // the local path relative to ${CFG_DIR}
-	// LocalHubPath string
 	LocalVersion string `json:"local_version,omitempty"`
 	LocalHash    string `json:"local_hash,omitempty"` // the local meow
 	Installed    bool   `json:"installed,omitempty"`
@@ -74,7 +71,7 @@ type Item struct {
 	Tainted      bool   `json:"tainted,omitempty"` // has it been locally modified
 	Local        bool   `json:"local,omitempty"`   // if it's a non versioned control one
 
-	// if it's a collection, it not a single file
+	// if it's a collection, it's not a single file
 	Parsers       []string `json:"parsers,omitempty"       yaml:"parsers,omitempty"`
 	PostOverflows []string `json:"postoverflows,omitempty" yaml:"postoverflows,omitempty"`
 	Scenarios     []string `json:"scenarios,omitempty"     yaml:"scenarios,omitempty"`
@@ -119,7 +116,6 @@ func (i *Item) toHubStatus() ItemHubStatus {
 var skippedLocal = 0
 var skippedTainted = 0
 
-// To be used when reference(s) (is/are) missing in a collection
 var ReferenceMissingError = errors.New("Reference(s) missing in collection")
 
 // GetVersionStatus: semver requires 'v' prefix
@@ -127,9 +123,7 @@ func GetVersionStatus(v *Item) int {
 	return semver.Compare("v"+v.Version, "v"+v.LocalVersion)
 }
 
-// calculate sha256 of a file
 func getSHA256(filepath string) (string, error) {
-	// Digest of file
 	f, err := os.Open(filepath)
 	if err != nil {
 		return "", fmt.Errorf("unable to open '%s': %w", filepath, err)
@@ -154,44 +148,51 @@ func GetItemMap(itemType string) map[string]Item {
 	return m
 }
 
-// GetItemByPath retrieves the item from hubIdx based on the path. To achieve this it will resolve symlink to find associated hub item.
-func GetItemByPath(itemType string, itemPath string) (*Item, error) {
-	// try to resolve symlink
-	finalName := ""
-
+// Given a FileInfo, extract the map key. Follow a symlink if necessary
+func itemKey(itemPath string) (string, error) {
 	f, err := os.Lstat(itemPath)
 	if err != nil {
-		return nil, fmt.Errorf("while performing lstat on %s: %w", itemPath, err)
+		return "", fmt.Errorf("while performing lstat on %s: %w", itemPath, err)
 	}
 
 	if f.Mode()&os.ModeSymlink == 0 {
-		// it's not a symlink, it should be the filename itsef the key
-		finalName = filepath.Base(itemPath)
-	} else {
-		// resolve the symlink to hub file
-		pathInHub, err := os.Readlink(itemPath)
-		if err != nil {
-			return nil, fmt.Errorf("while reading symlink of %s: %w", itemPath, err)
-		}
-		// extract author from path
-		fname := filepath.Base(pathInHub)
-		author := filepath.Base(filepath.Dir(pathInHub))
-		// trim yaml suffix
-		fname = strings.TrimSuffix(fname, ".yaml")
-		fname = strings.TrimSuffix(fname, ".yml")
-		finalName = fmt.Sprintf("%s/%s", author, fname)
+		// it's not a symlink, so the filename itsef should be the key
+		return filepath.Base(itemPath), nil
 	}
 
-	// it's not a symlink, it should be the filename itsef the key
-	if m := GetItemMap(itemType); m != nil {
-		if v, ok := m[finalName]; ok {
-			return &v, nil
-		}
+	// resolve the symlink to hub file
+	pathInHub, err := os.Readlink(itemPath)
+	if err != nil {
+		return "", fmt.Errorf("while reading symlink of %s: %w", itemPath, err)
+	}
+
+	fname := filepath.Base(pathInHub)
+	author := filepath.Base(filepath.Dir(pathInHub))
+
+	fname = strings.TrimSuffix(fname, ".yaml")
+	fname = strings.TrimSuffix(fname, ".yml")
 
-		return nil, fmt.Errorf("%s not found in %s", finalName, itemType)
+	return fmt.Sprintf("%s/%s", author, fname), nil
+}
+
+// GetItemByPath retrieves the item from hubIdx based on the path. To achieve this it will resolve symlink to find associated hub item.
+func GetItemByPath(itemType string, itemPath string) (*Item, error) {
+	itemKey, err := itemKey(itemPath)
+	if err != nil {
+		return nil, err
+	}
+
+	m := GetItemMap(itemType)
+	if m == nil {
+		return nil, fmt.Errorf("item type %s doesn't exist", itemType)
 	}
 
-	return nil, fmt.Errorf("item type %s doesn't exist", itemType)
+	v, ok := m[itemKey]
+	if !ok {
+		return nil, fmt.Errorf("%s not found in %s", itemKey, itemType)
+	}
+
+	return &v, nil
 }
 
 func GetItem(itemType string, itemName string) *Item {
@@ -251,76 +252,15 @@ func ItemStatus(v Item) (string, bool, bool, bool) {
 	return strret, Ok, Warning, Managed
 }
 
-func GetInstalledScenariosAsString() ([]string, error) {
-	var retStr []string
-
-	items, err := GetInstalledScenarios()
-	if err != nil {
-		return nil, fmt.Errorf("while fetching scenarios: %w", err)
-	}
-
-	for _, it := range items {
-		retStr = append(retStr, it.Name)
-	}
-
-	return retStr, nil
-}
-
-func GetInstalledScenarios() ([]Item, error) {
-	var retItems []Item
-
-	if _, ok := hubIdx[SCENARIOS]; !ok {
-		return nil, fmt.Errorf("no scenarios in hubIdx")
-	}
-
-	for _, item := range hubIdx[SCENARIOS] {
-		if item.Installed {
-			retItems = append(retItems, item)
-		}
-	}
-
-	return retItems, nil
-}
-
-func GetInstalledParsers() ([]Item, error) {
-	var retItems []Item
-
-	if _, ok := hubIdx[PARSERS]; !ok {
-		return nil, fmt.Errorf("no parsers in hubIdx")
-	}
-
-	for _, item := range hubIdx[PARSERS] {
-		if item.Installed {
-			retItems = append(retItems, item)
-		}
-	}
-
-	return retItems, nil
-}
-
-func GetInstalledParsersAsString() ([]string, error) {
-	var retStr []string
-
-	items, err := GetInstalledParsers()
-	if err != nil {
-		return nil, fmt.Errorf("while fetching parsers: %w", err)
-	}
-
-	for _, it := range items {
-		retStr = append(retStr, it.Name)
-	}
-
-	return retStr, nil
-}
-
-func GetInstalledPostOverflows() ([]Item, error) {
+func GetInstalledItems(itemType string) ([]Item, error) {
 	var retItems []Item
 
-	if _, ok := hubIdx[PARSERS_OVFLW]; !ok {
-		return nil, fmt.Errorf("no post overflows in hubIdx")
+	items, ok := hubIdx[itemType]
+	if !ok {
+		return nil, fmt.Errorf("no %s in hubIdx", itemType)
 	}
 
-	for _, item := range hubIdx[PARSERS_OVFLW] {
+	for _, item := range items {
 		if item.Installed {
 			retItems = append(retItems, item)
 		}
@@ -329,27 +269,12 @@ func GetInstalledPostOverflows() ([]Item, error) {
 	return retItems, nil
 }
 
-func GetInstalledPostOverflowsAsString() ([]string, error) {
-	var retStr []string
-
-	items, err := GetInstalledPostOverflows()
-	if err != nil {
-		return nil, fmt.Errorf("while fetching post overflows: %w", err)
-	}
-
-	for _, it := range items {
-		retStr = append(retStr, it.Name)
-	}
-
-	return retStr, nil
-}
-
-func GetInstalledCollectionsAsString() ([]string, error) {
+func GetInstalledItemsAsString(itemType string) ([]string, error) {
 	var retStr []string
 
-	items, err := GetInstalledCollections()
+	items, err := GetInstalledItems(itemType)
 	if err != nil {
-		return nil, fmt.Errorf("while fetching collections: %w", err)
+		return nil, fmt.Errorf("while fetching %s: %w", itemType, err)
 	}
 
 	for _, it := range items {
@@ -359,23 +284,7 @@ func GetInstalledCollectionsAsString() ([]string, error) {
 	return retStr, nil
 }
 
-func GetInstalledCollections() ([]Item, error) {
-	var retItems []Item
-
-	if _, ok := hubIdx[COLLECTIONS]; !ok {
-		return nil, fmt.Errorf("no collection in hubIdx")
-	}
-
-	for _, item := range hubIdx[COLLECTIONS] {
-		if item.Installed {
-			retItems = append(retItems, item)
-		}
-	}
-
-	return retItems, nil
-}
-
-// Returns a list of entries for packages: name, status, local_path, local_version, utf8_status (fancy)
+// Returns a slice of entries for packages: name, status, local_path, local_version, utf8_status (fancy)
 func GetHubStatusForItemType(itemType string, name string, all bool) []ItemHubStatus {
 	if _, ok := hubIdx[itemType]; !ok {
 		log.Errorf("type %s doesn't exist", itemType)

pkg/cwhub/helpers.go

@@ -18,7 +18,7 @@ func chooseHubBranch() (string, error) {
 	if err != nil {
 		log.Warningf("Unable to retrieve latest crowdsec version: %s, defaulting to master", err)
 		//lint:ignore nilerr
-		return "master", nil // ignore
+		return "master", nil
 	}
 
 	csVersion := cwversion.VersionStrip()

pkg/cwhub/install.go

@@ -87,11 +87,11 @@ func DisableItem(hub *csconfig.Hub, target Item, purge bool, force bool) (Item,
 
 	stat, err := os.Lstat(syml)
 	if os.IsNotExist(err) {
-		if !purge && !force { //we only accept to "delete" non existing items if it's a purge
+		if !purge && !force { // we only accept to "delete" non existing items if it's a purge
 			return target, 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 / ...
+		// if it's managed by hub, it's a symlink to csconfig.GConfig.hub.HubDir / ...
 		if stat.Mode()&os.ModeSymlink == 0 {
 			log.Warningf("%s (%s) isn't a symlink, can't disable", target.Name, syml)
 			return target, fmt.Errorf("%s isn't managed by hub", target.Name)
@@ -112,7 +112,7 @@ func DisableItem(hub *csconfig.Hub, target Item, purge bool, force bool) (Item,
 			return target, fmt.Errorf("%s isn't managed by hub", target.Name)
 		}
 
-		//remove the symlink
+		// remove the symlink
 		if err = os.Remove(syml); err != nil {
 			return target, fmt.Errorf("while removing symlink: %w", err)
 		}

pkg/cwhub/loader.go

@@ -484,7 +484,7 @@ func GetHubIdx(hub *csconfig.Hub) error {
 	return nil
 }
 
-// LoadPkgIndex loads a local .index.json file and returns the map of parsers/scenarios/collections associated
+// LoadPkgIndex loads a local .index.json file and returns the map of associated parsers/scenarios/collections
 func LoadPkgIndex(buff []byte) (map[string]map[string]Item, error) {
 	var (
 		RawIndex     map[string]map[string]Item
@@ -497,9 +497,8 @@ func LoadPkgIndex(buff []byte) (map[string]map[string]Item, error) {
 
 	log.Debugf("%d item types in hub index", len(ItemTypes))
 
-	// Iterate over the different types to complete struct
+	// Iterate over the different types to complete the struct
 	for _, itemType := range ItemTypes {
-		// complete struct
 		log.Tracef("%d item", len(RawIndex[itemType]))
 
 		for name, item := range RawIndex[itemType] {

pkg/cwhub/pathseparator.go → pkg/cwhub/pathseparator_unix.go

@@ -1,5 +1,4 @@
-//go:build linux || freebsd || netbsd || openbsd || solaris || !windows
-// +build linux freebsd netbsd openbsd solaris !windows
+//go:build unix
 
 package cwhub