test and log fixes (#2690)

* cscli inspect: suggest --diff if an item is tainted
* appropriate warning, or error if context configuration file is empty
* fix user/group lookup unit test
* fix: allow hub upgrade --force with local items
* fix pkg/parser lookup for 8.8.8.8
* fix func test
* fix hubtests: machines add --force

cmd/crowdsec-cli/itemcli.go

@@ -568,6 +568,10 @@ func (cli cliItem) whyTainted(hub *cwhub.Hub, item *cwhub.Item, reverse bool) st
 			ret = append(ret, diff)
 		} else if len(sub.State.TaintedBy) > 0 {
 			taintList := strings.Join(sub.State.TaintedBy, ", ")
+			if sub.FQName() == taintList {
+				// hack: avoid message "item is tainted by itself"
+				continue
+			}
 			ret = append(ret, fmt.Sprintf("# %s is tainted by %s", sub.FQName(), taintList))
 		}
 	}

cmd/crowdsec-cli/items.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"gopkg.in/yaml.v3"
@@ -156,7 +157,17 @@ func InspectItem(item *cwhub.Item, showMetrics bool) error {
 		fmt.Print(string(b))
 	}
 
-	if csConfig.Cscli.Output == "human" && showMetrics {
+	if csConfig.Cscli.Output != "human" {
+		return nil
+	}
+
+	if item.State.Tainted {
+		fmt.Println()
+		fmt.Printf(`This item is tainted. Use "%s %s inspect --diff %s" to see why.`, filepath.Base(os.Args[0]), item.Type, item.Name)
+		fmt.Println()
+	}
+
+	if showMetrics {
 		fmt.Printf("\nCurrent metrics: \n")
 		if err := ShowMetrics(item); err != nil {
 			return err

pkg/alertcontext/config.go

@@ -23,7 +23,11 @@ type HubItemWrapper struct {
 }
 
 // mergeContext adds the context from src to dest.
-func mergeContext(dest map[string][]string, src map[string][]string) {
+func mergeContext(dest map[string][]string, src map[string][]string) error {
+	if len(src) == 0 {
+		return fmt.Errorf("no context data to merge")
+	}
+
 	for k, v := range src {
 		if _, ok := dest[k]; !ok {
 			dest[k] = make([]string, 0)
@@ -34,6 +38,8 @@ func mergeContext(dest map[string][]string, src map[string][]string) {
 			}
 		}
 	}
+
+	return nil
 }
 
 // addContextFromItem merges the context from an item into the context to send to the console.
@@ -52,7 +58,11 @@ func addContextFromItem(toSend map[string][]string, item *cwhub.Item) error {
 		return fmt.Errorf("%s: %w", filePath, err)
 	}
 
-	mergeContext(toSend, wrapper.Context)
+	err = mergeContext(toSend, wrapper.Context)
+	if err != nil {
+		// having an empty hub item deserves an error
+		log.Errorf("while merging context from %s: %s. Note that context data should be under the 'context:' key, the top-level is metadata.", filePath, err)
+	}
 
 	return nil
 }
@@ -72,7 +82,10 @@ func addContextFromFile(toSend map[string][]string, filePath string) error {
 		return fmt.Errorf("%s: %w", filePath, err)
 	}
 
-	mergeContext(toSend, newContext)
+	err = mergeContext(toSend, newContext)
+	if err != nil {
+		log.Warningf("while merging context from %s: %s", filePath, err)
+	}
 
 	return nil
 }

pkg/csplugin/broker_test.go

@@ -111,7 +111,7 @@ func (s *PluginSuite) TestBrokerInit() {
 		},
 		{
 			name:        "Invalid user and group",
-			expectedErr: "unknown user toto1234",
+			expectedErr: "toto1234",
 			procCfg: csconfig.PluginCfg{
 				User:  "toto1234",
 				Group: "toto1234",
@@ -119,7 +119,7 @@ func (s *PluginSuite) TestBrokerInit() {
 		},
 		{
 			name:        "Valid user and invalid group",
-			expectedErr: "unknown group toto1234",
+			expectedErr: "toto1234",
 			procCfg: csconfig.PluginCfg{
 				User:  "nobody",
 				Group: "toto1234",

pkg/cwhub/itemupgrade.go

@@ -154,9 +154,17 @@ func (i *Item) FetchLatest() ([]byte, string, error) {
 
 // download downloads the item from the hub and writes it to the hub directory.
 func (i *Item) download(overwrite bool) (string, error) {
-	if i.State.IsLocal() {
-		return "", fmt.Errorf("%s is local, can't download", i.Name)
+	// ensure that target file is within target dir
+	finalPath, err := i.downloadPath()
+	if err != nil {
+		return "", err
 	}
+
+	if i.State.IsLocal() {
+		i.hub.logger.Warningf("%s is local, can't download", i.Name)
+		return finalPath, nil
+	}
+
 	// if user didn't --force, don't overwrite local, tainted, up-to-date files
 	if !overwrite {
 		if i.State.Tainted {
@@ -177,12 +185,6 @@ func (i *Item) download(overwrite bool) (string, error) {
 
 	// all good, install
 
-	// ensure that target file is within target dir
-	finalPath, err := i.downloadPath()
-	if err != nil {
-		return "", err
-	}
-
 	parentDir := filepath.Dir(finalPath)
 
 	if err = os.MkdirAll(parentDir, os.ModePerm); err != nil {

pkg/hubtest/hubtest_item.go

@@ -731,7 +731,7 @@ func (t *HubTestItem) RunWithLogFile() error {
 		return fmt.Errorf("log file '%s' is empty, please fill it with log", logFile)
 	}
 
-	cmdArgs := []string{"-c", t.RuntimeConfigFilePath, "machines", "add", "testMachine", "--auto"}
+	cmdArgs := []string{"-c", t.RuntimeConfigFilePath, "machines", "add", "testMachine", "--force", "--auto"}
 	cscliRegisterCmd := exec.Command(t.CscliPath, cmdArgs...)
 	log.Debugf("%s", cscliRegisterCmd.String())
 

pkg/parser/tests/reverse-dns-enrich/test.yaml

@@ -1,14 +1,14 @@
 #these are the events we input into parser
 lines:
   - Enriched:
-      IpToResolve: 8.8.8.8
+      IpToResolve: 1.1.1.1
   - Enriched:
       IpToResolve: 1.2.3.4
 #these are the results we expect from the parser
 results:
   - Enriched:
-      reverse_dns: dns.google.
-      IpToResolve: 8.8.8.8
+      reverse_dns: one.one.one.one.
+      IpToResolve: 1.1.1.1
     Meta:
       did_dns_succeeded: yes
     Process: true

pkg/parser/tests/whitelist-base/base-grok.yaml

@@ -4,7 +4,7 @@ debug: true
 whitelist:
   reason: "Whitelist tests"
   ip: 
-    - 8.8.8.8
+    - 1.1.1.1
   cidr:
     - "1.2.3.0/24"
   expression:

pkg/parser/tests/whitelist-base/test.yaml

@@ -2,7 +2,7 @@
 lines:
   - Meta:
       test: test1
-      source_ip: 8.8.8.8
+      source_ip: 1.1.1.1
       statics: toto
   - Meta:
       test: test2

test/bats/20_hub_items.bats

@@ -152,9 +152,9 @@ teardown() {
     rune -0 mkdir -p "$CONFIG_DIR/collections"
     rune -0 touch "$CONFIG_DIR/collections/foobar.yaml"
     rune -1 cscli collections install foobar.yaml
-    assert_stderr --partial "failed to download item: foobar.yaml is local, can't download"
+    assert_stderr --partial "foobar.yaml is local, can't download"
     rune -1 cscli collections install foobar.yaml --force
-    assert_stderr --partial "failed to download item: foobar.yaml is local, can't download"
+    assert_stderr --partial "foobar.yaml is local, can't download"
 }
 
 @test "a local item cannot be removed by cscli" {