Refact pkg/cwhub: constructor, cscli output

* Single constructor: NewHub() to replace InitHub(), InitHubUpdate()
* sort cscli hub list output
* log.Fatal -> fmt.Errorf

cmd/crowdsec-cli/hub.go

@@ -64,9 +64,7 @@ func runHubList(cmd *cobra.Command, args []string) error {
 		log.Info(line)
 	}
 
-	err = ListItems(hub, color.Output, []string{
-		cwhub.COLLECTIONS, cwhub.PARSERS, cwhub.SCENARIOS, cwhub.POSTOVERFLOWS,
-	}, nil, true, false, all)
+	err = ListItems(hub, color.Output, cwhub.ItemTypes, nil, true, false, all)
 	if err != nil {
 		return err
 	}
@@ -94,16 +92,16 @@ func runHubUpdate(cmd *cobra.Command, args []string) error {
 	remote := require.RemoteHub(csConfig)
 
 	// don't use require.Hub because if there is no index file, it would fail
-	hub, err := cwhub.InitHubUpdate(local, remote)
+	hub, err := cwhub.NewHub(local, remote, true)
 	if err != nil {
 		// XXX: this should be done when downloading items, too
 		// but what is the fallback to master actually solving?
 		if !errors.Is(err, cwhub.ErrIndexNotFound) {
-			return fmt.Errorf("failed to get Hub index : %w", err)
+			return fmt.Errorf("failed to get Hub index: %w", err)
 		}
 		log.Warnf("Could not find index file for branch '%s', using 'master'", remote.Branch)
 		remote.Branch = "master"
-		if hub, err = cwhub.InitHubUpdate(local, remote); err != nil {
+		if hub, err = cwhub.NewHub(local, remote, true); err != nil {
 			return fmt.Errorf("failed to get Hub index after retry: %w", err)
 		}
 	}

cmd/crowdsec-cli/items.go

@@ -9,7 +9,6 @@ import (
 	"sort"
 	"strings"
 
-	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v3"
 	"slices"
 
@@ -50,20 +49,22 @@ func selectItems(hub *cwhub.Hub, itemType string, args []string, installedOnly b
 }
 
 func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string, showType bool, showHeader bool, all bool) error {
-	var err error
-
 	items := make(map[string][]string)
 	for _, itemType := range itemTypes {
-		if items[itemType], err = selectItems(hub, itemType, args, !all); err != nil {
+		selected, err := selectItems(hub, itemType, args, !all)
+		if err != nil {
 			return err
 		}
+		sort.Strings(selected)
+		items[itemType] = selected
 	}
 
-	if csConfig.Cscli.Output == "human" {
+	switch csConfig.Cscli.Output {
+	case "human":
 		for _, itemType := range itemTypes {
 			listHubItemTable(hub, out, "\n"+strings.ToUpper(itemType), itemType, items[itemType])
 		}
-	} else if csConfig.Cscli.Output == "json" {
+	case "json":
 		type itemHubStatus struct {
 			Name         string `json:"name"`
 			LocalVersion string `json:"local_version"`
@@ -89,15 +90,13 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 					UTF8Status:   fmt.Sprintf("%v  %s", emo, status),
 				}
 			}
-			h := hubStatus[itemType]
-			sort.Slice(h, func(i, j int) bool { return h[i].Name < h[j].Name })
 		}
 		x, err := json.MarshalIndent(hubStatus, "", " ")
 		if err != nil {
-			log.Fatalf("failed to unmarshal")
+			return fmt.Errorf("failed to unmarshal: %w", err)
 		}
 		out.Write(x)
-	} else if csConfig.Cscli.Output == "raw" {
+	case "raw":
 		csvwriter := csv.NewWriter(out)
 		if showHeader {
 			header := []string{"name", "status", "version", "description"}
@@ -106,7 +105,7 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 			}
 			err := csvwriter.Write(header)
 			if err != nil {
-				log.Fatalf("failed to write header: %s", err)
+				return fmt.Errorf("failed to write header: %s", err)
 			}
 		}
 		for _, itemType := range itemTypes {
@@ -127,12 +126,15 @@ func ListItems(hub *cwhub.Hub, out io.Writer, itemTypes []string, args []string,
 				}
 				err := csvwriter.Write(row)
 				if err != nil {
-					log.Fatalf("failed to write raw output : %s", err)
+					return fmt.Errorf("failed to write raw output: %s", err)
 				}
 			}
 		}
 		csvwriter.Flush()
+	default:
+		return fmt.Errorf("unknown output format '%s'", csConfig.Cscli.Output)
 	}
+
 	return nil
 }
 

cmd/crowdsec-cli/require/require.go

@@ -86,7 +86,7 @@ func Hub(c *csconfig.Config, remote *cwhub.RemoteHubCfg) (*cwhub.Hub, error) {
 		return nil, fmt.Errorf("you must configure cli before interacting with hub")
 	}
 
-	hub, err := cwhub.InitHub(local, remote)
+	hub, err := cwhub.NewHub(local, remote, false)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read Hub index: '%w'. Run 'sudo cscli hub update' to download the index again", err)
 	}

cmd/crowdsec/crowdsec.go

@@ -23,7 +23,7 @@ import (
 func initCrowdsec(cConfig *csconfig.Config) (*parser.Parsers, error) {
 	var err error
 
-	hub, err := cwhub.InitHub(cConfig.Hub, nil)
+	hub, err := cwhub.NewHub(cConfig.Hub, nil, false)
 	if err != nil {
 		return nil, fmt.Errorf("while loading hub index: %w", err)
 	}

pkg/cwhub/cwhub_test.go

@@ -63,18 +63,13 @@ func testHub(t *testing.T, update bool) *Hub {
 	var hub *Hub
 
 	remote := &RemoteHubCfg{
-		Branch: "master",
+		Branch:      "master",
 		URLTemplate: mockURLTemplate,
-		IndexPath: ".index.json",
+		IndexPath:   ".index.json",
 	}
 
-	if update {
-		hub, err = InitHubUpdate(local, remote)
-		require.NoError(t, err)
-	} else {
-		hub, err = InitHub(local, remote)
-		require.NoError(t, err)
-	}
+	hub, err = NewHub(local, remote, update)
+	require.NoError(t, err)
 
 	return hub
 }

pkg/cwhub/helpers_test.go

@@ -36,10 +36,10 @@ func TestUpgradeItemNewScenarioInCollection(t *testing.T) {
 	remote := &RemoteHubCfg{
 		URLTemplate: mockURLTemplate,
 		Branch:      "master",
-		IndexPath: ".index.json",
+		IndexPath:   ".index.json",
 	}
 
-	hub, err := InitHubUpdate(hub.local, remote)
+	hub, err := NewHub(hub.local, remote, true)
 	require.NoError(t, err, "failed to download index: %s", err)
 
 	hub = getHubOrFail(t, hub.local, remote)
@@ -84,7 +84,7 @@ func TestUpgradeItemInDisabledScenarioShouldNotBeInstalled(t *testing.T) {
 	remote := &RemoteHubCfg{
 		URLTemplate: mockURLTemplate,
 		Branch:      "master",
-		IndexPath: ".index.json",
+		IndexPath:   ".index.json",
 	}
 
 	hub = getHubOrFail(t, hub.local, remote)
@@ -95,7 +95,7 @@ func TestUpgradeItemInDisabledScenarioShouldNotBeInstalled(t *testing.T) {
 	require.True(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Installed)
 	require.True(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].UpToDate)
 
-	hub, err = InitHubUpdate(hub.local, remote)
+	hub, err = NewHub(hub.local, remote, true)
 	require.NoError(t, err, "failed to download index: %s", err)
 
 	didUpdate, err := hub.UpgradeItem(COLLECTIONS, "crowdsecurity/test_collection", false)
@@ -108,7 +108,7 @@ func TestUpgradeItemInDisabledScenarioShouldNotBeInstalled(t *testing.T) {
 
 // getHubOrFail refreshes the hub state (load index, sync) and returns the singleton, or fails the test
 func getHubOrFail(t *testing.T, local *csconfig.LocalHubCfg, remote *RemoteHubCfg) *Hub {
-	hub, err := InitHub(local, remote)
+	hub, err := NewHub(local, remote, false)
 	require.NoError(t, err, "failed to load hub index")
 
 	return hub
@@ -141,7 +141,7 @@ func TestUpgradeItemNewScenarioIsInstalledWhenReferencedScenarioIsDisabled(t *te
 	remote := &RemoteHubCfg{
 		URLTemplate: mockURLTemplate,
 		Branch:      "master",
-		IndexPath: ".index.json",
+		IndexPath:   ".index.json",
 	}
 
 	hub = getHubOrFail(t, hub.local, remote)
@@ -158,7 +158,7 @@ func TestUpgradeItemNewScenarioIsInstalledWhenReferencedScenarioIsDisabled(t *te
 	// we just removed. Nor should it install the newly added scenario
 	pushUpdateToCollectionInHub()
 
-	hub, err = InitHubUpdate(hub.local, remote)
+	hub, err = NewHub(hub.local, remote, true)
 	require.NoError(t, err, "failed to download index: %s", err)
 
 	require.False(t, hub.Items[SCENARIOS]["crowdsecurity/foobar_scenario"].Installed)

pkg/cwhub/hub.go

@@ -1,12 +1,9 @@
 package cwhub
 
 import (
-	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
-	"net/http"
 	"os"
 	"strings"
 
@@ -15,15 +12,6 @@ import (
 	"github.com/crowdsecurity/crowdsec/pkg/csconfig"
 )
 
-// const HubIndexFile = ".index.json"
-
-// RemoteHubCfg contains where to find the remote hub, which branch etc.
-type RemoteHubCfg struct {
-	Branch      string
-	URLTemplate string
-	IndexPath   string
-}
-
 type Hub struct {
 	Items          HubItems
 	local          *csconfig.LocalHubCfg
@@ -48,12 +36,20 @@ func GetHub() (*Hub, error) {
 	return theHub, nil
 }
 
-// InitHub initializes the Hub, syncs the local state and returns the singleton for immediate use
-func InitHub(local *csconfig.LocalHubCfg, remote *RemoteHubCfg) (*Hub, error) {
+// NewHub returns a new Hub instance with local and (optionally) remote configuration, and syncs the local state
+// It also downloads the index if downloadIndex is true
+func NewHub(local *csconfig.LocalHubCfg, remote *RemoteHubCfg, downloadIndex bool) (*Hub, error) {
 	if local == nil {
 		return nil, fmt.Errorf("no hub configuration found")
 	}
 
+	if downloadIndex {
+		err := remote.DownloadIndex(local.HubIndexFile)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	log.Debugf("loading hub idx %s", local.HubIndexFile)
 
 	bidx, err := os.ReadFile(local.HubIndexFile)
@@ -64,7 +60,7 @@ func InitHub(local *csconfig.LocalHubCfg, remote *RemoteHubCfg) (*Hub, error) {
 	ret, err := ParseIndex(bidx)
 	if err != nil {
 		if !errors.Is(err, ErrMissingReference) {
-			return nil, fmt.Errorf("unable to load existing index: %w", err)
+			return nil, fmt.Errorf("failed to load index: %w", err)
 		}
 
 		// XXX: why the error check if we bail out anyway?
@@ -79,110 +75,12 @@ func InitHub(local *csconfig.LocalHubCfg, remote *RemoteHubCfg) (*Hub, error) {
 
 	_, err = theHub.LocalSync()
 	if err != nil {
-		return nil, fmt.Errorf("failed to sync Hub index with local deployment : %w", err)
-	}
-
-	return theHub, nil
-}
-
-// InitHubUpdate is like InitHub but downloads and updates the index instead of reading from the disk
-// It is used to inizialize the hub when there is no index file yet
-func InitHubUpdate(local *csconfig.LocalHubCfg, remote *RemoteHubCfg) (*Hub, error) {
-	if local == nil {
-		return nil, fmt.Errorf("no configuration found for hub")
-	}
-
-	bidx, err := remote.DownloadIndex(local.HubIndexFile)
-	if err != nil {
-		return nil, fmt.Errorf("failed to download index: %w", err)
-	}
-
-	ret, err := ParseIndex(bidx)
-	if err != nil {
-		if !errors.Is(err, ErrMissingReference) {
-			return nil, fmt.Errorf("failed to read index: %w", err)
-		}
-	}
-
-	theHub = &Hub{
-		Items:  ret,
-		local:  local,
-		remote: remote,
-	}
-
-	if _, err := theHub.LocalSync(); err != nil {
-		return nil, fmt.Errorf("failed to sync: %w", err)
+		return nil, fmt.Errorf("failed to sync hub index: %w", err)
 	}
 
 	return theHub, nil
 }
 
-func (r RemoteHubCfg) urlTo(remotePath string) (string, error) {
-	if fmt.Sprintf(r.URLTemplate, "%s", "%s") != r.URLTemplate {
-		return "", fmt.Errorf("invalid URL template '%s'", r.URLTemplate)
-	}
-
-	return fmt.Sprintf(r.URLTemplate, r.Branch, remotePath), nil
-}
-
-// DownloadIndex downloads the latest version of the index and returns the content
-func (r RemoteHubCfg) DownloadIndex(localPath string) ([]byte, error) {
-	url, err := r.urlTo(r.IndexPath)
-	if err != nil {
-		return nil, fmt.Errorf("failed to build hub index request: %w", err)
-	}
-
-	req, err := http.NewRequest(http.MethodGet, url, nil)
-	if err != nil {
-		return nil, fmt.Errorf("failed to build request for hub index: %w", err)
-	}
-
-	resp, err := http.DefaultClient.Do(req)
-	if err != nil {
-		return nil, fmt.Errorf("failed http request for hub index: %w", err)
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		if resp.StatusCode == http.StatusNotFound {
-			return nil, ErrIndexNotFound
-		}
-
-		return nil, fmt.Errorf("bad http code %d while requesting %s", resp.StatusCode, req.URL.String())
-	}
-
-	body, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read request answer for hub index: %w", err)
-	}
-
-	oldContent, err := os.ReadFile(localPath)
-	if err != nil {
-		if !os.IsNotExist(err) {
-			log.Warningf("failed to read hub index: %s", err)
-		}
-	} else if bytes.Equal(body, oldContent) {
-		log.Info("hub index is up to date")
-		return body, nil
-	}
-
-	file, err := os.OpenFile(localPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
-
-	if err != nil {
-		return nil, fmt.Errorf("while opening hub index file: %w", err)
-	}
-	defer file.Close()
-
-	wsize, err := file.Write(body)
-	if err != nil {
-		return nil, fmt.Errorf("while writing hub index file: %w", err)
-	}
-
-	log.Infof("Wrote index to %s, %d bytes", localPath, wsize)
-
-	return body, nil
-}
-
 // ParseIndex takes the content of an index file and returns the map of associated parsers/scenarios/collections
 func ParseIndex(buff []byte) (HubItems, error) {
 	var (

pkg/cwhub/hub_test.go

@@ -13,13 +13,13 @@ import (
 func TestInitHubUpdate(t *testing.T) {
 	hub := envSetup(t)
 
-	remote := &RemoteHubCfg {
+	remote := &RemoteHubCfg{
 		URLTemplate: mockURLTemplate,
-		Branch: "master",
-		IndexPath: ".index.json",
+		Branch:      "master",
+		IndexPath:   ".index.json",
 	}
 
-	_, err := InitHubUpdate(hub.local, remote)
+	_, err := NewHub(hub.local, remote, true)
 	require.NoError(t, err)
 
 	_, err = GetHub()
@@ -39,43 +39,37 @@ func TestDownloadIndex(t *testing.T) {
 
 	hub := envSetup(t)
 
-	hub.remote = &RemoteHubCfg {
+	hub.remote = &RemoteHubCfg{
 		URLTemplate: "x",
-		Branch: "",
-		IndexPath: "",
+		Branch:      "",
+		IndexPath:   "",
 	}
 
-	ret, err := hub.remote.DownloadIndex(tmpIndex.Name())
+	err = hub.remote.DownloadIndex(tmpIndex.Name())
 	cstest.RequireErrorContains(t, err, "failed to build hub index request: invalid URL template 'x'")
 
-	fmt.Printf("->%+v", ret)
-
 	// bad domain
 	fmt.Println("Test 'bad domain'")
 
-	hub.remote = &RemoteHubCfg {
+	hub.remote = &RemoteHubCfg{
 		URLTemplate: "https://baddomain/%s/%s",
-		Branch: "master",
-		IndexPath: ".index.json",
+		Branch:      "master",
+		IndexPath:   ".index.json",
 	}
 
-	ret, err = hub.remote.DownloadIndex(tmpIndex.Name())
-// XXX: this is not failing
-//	cstest.RequireErrorContains(t, err, "failed http request for hub index: Get")
-
-	fmt.Printf("->%+v", ret)
+	err = hub.remote.DownloadIndex(tmpIndex.Name())
+	// XXX: this is not failing
+	//	cstest.RequireErrorContains(t, err, "failed http request for hub index: Get")
 
 	// bad target path
 	fmt.Println("Test 'bad target path'")
 
-	hub.remote = &RemoteHubCfg {
+	hub.remote = &RemoteHubCfg{
 		URLTemplate: mockURLTemplate,
-		Branch: "master",
-		IndexPath: ".index.json",
+		Branch:      "master",
+		IndexPath:   ".index.json",
 	}
 
-	ret, err = hub.remote.DownloadIndex("/does/not/exist/index.json")
+	err = hub.remote.DownloadIndex("/does/not/exist/index.json")
 	cstest.RequireErrorContains(t, err, "while opening hub index file: open /does/not/exist/index.json:")
-
-	fmt.Printf("->%+v", ret)
 }

pkg/cwhub/remote.go

@@ -0,0 +1,95 @@
+package cwhub
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"net/http"
+	"os"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// RemoteHubCfg contains where to find the remote hub, which branch etc.
+type RemoteHubCfg struct {
+	Branch      string
+	URLTemplate string
+	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
+	}
+
+	if fmt.Sprintf(r.URLTemplate, "%s", "%s") != r.URLTemplate {
+		return "", fmt.Errorf("invalid URL template '%s'", r.URLTemplate)
+	}
+
+	return fmt.Sprintf(r.URLTemplate, r.Branch, remotePath), nil
+}
+
+// DownloadIndex downloads the latest version of the index
+func (r *RemoteHubCfg) DownloadIndex(localPath string) error {
+	if r == nil {
+		return ErrNilRemoteHub
+	}
+
+	url, err := r.urlTo(r.IndexPath)
+	if err != nil {
+		return fmt.Errorf("failed to build hub index request: %w", err)
+	}
+
+	req, err := http.NewRequest(http.MethodGet, url, nil)
+	if err != nil {
+		return fmt.Errorf("failed to build request for hub index: %w", err)
+	}
+
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return fmt.Errorf("failed http request for hub index: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		if resp.StatusCode == http.StatusNotFound {
+			return ErrIndexNotFound
+		}
+
+		return fmt.Errorf("bad http code %d while requesting %s", resp.StatusCode, req.URL.String())
+	}
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return fmt.Errorf("failed to read request answer for hub index: %w", err)
+	}
+
+	oldContent, err := os.ReadFile(localPath)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			log.Warningf("failed to read hub index: %s", err)
+		}
+	} else if bytes.Equal(body, oldContent) {
+		log.Info("hub index is up to date")
+		return nil
+	}
+
+	file, err := os.OpenFile(localPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
+
+	if err != nil {
+		return fmt.Errorf("while opening hub index file: %w", err)
+	}
+	defer file.Close()
+
+	wsize, err := file.Write(body)
+	if err != nil {
+		return fmt.Errorf("while writing hub index file: %w", err)
+	}
+
+	log.Infof("Wrote index to %s, %d bytes", localPath, wsize)
+
+	return nil
+}

pkg/hubtest/hubtest_item.go

@@ -391,7 +391,7 @@ func (t *HubTestItem) InstallHub() error {
 	}
 
 	// load installed hub
-	hub, err := cwhub.InitHub(t.RuntimeHubConfig, nil)
+	hub, err := cwhub.NewHub(t.RuntimeHubConfig, nil, false)
 	if err != nil {
 		log.Fatal(err)
 	}

pkg/leakybucket/buckets_test.go

@@ -43,9 +43,9 @@ func TestBucket(t *testing.T) {
 		HubIndexFile: filepath.Join(testdata, "hub", "index.json"),
 	}
 
-	_, err := cwhub.InitHub(hubCfg, nil)
+	_, err := cwhub.NewHub(hubCfg, nil, false)
 	if err != nil {
-		t.Fatalf("failed to init hub : %s", err)
+		t.Fatalf("failed to init hub: %s", err)
 	}
 
 	err = exprhelpers.Init(nil)

test/bats/20_hub.bats

@@ -33,7 +33,7 @@ teardown() {
 @test "cscli hub list" {
     # no items
     rune -0 cscli hub list
-    assert_output --regexp ".*COLLECTIONS.*PARSERS.*SCENARIOS.*POSTOVERFLOWS.*"
+    assert_output --regexp ".*PARSERS.*POSTOVERFLOWS.*SCENARIOS.*COLLECTIONS.*"
     rune -0 cscli hub list -o json
     assert_json '{parsers:[],scenarios:[],collections:[],postoverflows:[]}'
     rune -0 cscli hub list -o raw
@@ -43,7 +43,7 @@ teardown() {
     rune -0 cscli parsers install crowdsecurity/whitelists
     rune -0 cscli scenarios install crowdsecurity/telnet-bf
     rune -0 cscli hub list
-    assert_output --regexp ".*COLLECTIONS.*PARSERS.*crowdsecurity/whitelists.*SCENARIOS.*crowdsecurity/telnet-bf.*POSTOVERFLOWS.*"
+    assert_output --regexp ".*PARSERS.*crowdsecurity/whitelists.*POSTOVERFLOWS.*SCENARIOS.*crowdsecurity/telnet-bf.*COLLECTIONS.*"
     rune -0 cscli hub list -o json
     rune -0 jq -e '(.parsers | length == 1) and (.scenarios | length == 1)' <(output)
     rune -0 cscli hub list -o raw
@@ -53,7 +53,7 @@ teardown() {
 
     # all items
     rune -0 cscli hub list -a
-    assert_output --regexp ".*COLLECTIONS.*crowdsecurity/linux.*PARSERS.*crowdsecurity/whitelists.*SCENARIOS.*crowdsecurity/telnet-bf.*POSTOVERFLOWS.*"
+    assert_output --regexp ".*PARSERS.*crowdsecurity/whitelists.*POSTOVERFLOWS.*SCENARIOS.*crowdsecurity/telnet-bf.*COLLECTIONS.*crowdsecurity/linux.*"
     rune -0 cscli hub list -a -o json
     rune -0 jq -e '(.parsers | length > 1) and (.scenarios | length > 1)' <(output)
     rune -0 cscli hub list -a -o raw