revert ActionPlan info/warning to StandardLogger (#3536)

mmetc · web-flow · commit f735457ca4bc · 2025-03-24T15:13:24.000+01:00
diff --git a/pkg/hubops/disable.go b/pkg/hubops/disable.go
@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"os"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/crowdsecurity/crowdsec/pkg/cwhub"
 )
 
@@ -53,7 +55,7 @@ func (c *DisableCommand) Prepare(plan *ActionPlan) (bool, error) {
 	i := c.Item
 
 	if i.State.IsLocal() {
-		plan.Warning(i.FQName() + " is a local item, please delete manually")
+		log.Warnf("%s is a local item, please delete manually", i.FQName())
 		return false, nil
 	}
 
diff --git a/pkg/hubops/download.go b/pkg/hubops/download.go
@@ -10,7 +10,7 @@ import (
 	"time"
 
 	"github.com/fatih/color"
-	"github.com/sirupsen/logrus"
+	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v3"
 
 	"github.com/crowdsecurity/go-cs-lib/downloader"
@@ -38,13 +38,13 @@ func (c *DownloadCommand) Prepare(plan *ActionPlan) (bool, error) {
 	i := c.Item
 
 	if i.State.IsLocal() {
-		plan.Info(i.FQName() + " - not downloading local item")
+		log.Infof("%s - not downloading local item", i.FQName())
 		return false, nil
 	}
 
 	// XXX: if it's tainted do we upgrade the dependencies anyway?
 	if i.State.Tainted && !c.Force {
-		plan.Warning(i.FQName() + " is tainted, use '--force' to overwrite")
+		log.Warnf("%s is tainted, use '--force' to overwrite", i.FQName())
 		return false, nil
 	}
 
@@ -132,7 +132,7 @@ func downloadDataSet(ctx context.Context, dataFolder string, force bool, reader
 				BeforeRequest(func(req *http.Request) {
 					fmt.Printf("downloading %s\n", req.URL)
 				}).
-				WithLogger(logrus.WithField("url", dataS.SourceURL))
+				WithLogger(log.WithField("url", dataS.SourceURL))
 
 			if !force {
 				d = d.WithLastModified().
diff --git a/pkg/hubops/plan.go b/pkg/hubops/plan.go
@@ -10,7 +10,6 @@ import (
 	"strings"
 
 	"github.com/AlecAivazis/survey/v2"
-	"github.com/fatih/color"
 
 	"github.com/crowdsecurity/go-cs-lib/slicetools"
 
@@ -96,14 +95,6 @@ func (p *ActionPlan) AddCommand(c Command) error {
 	return nil
 }
 
-func (p *ActionPlan) Info(msg string) {
-	fmt.Println(msg)
-}
-
-func (p *ActionPlan) Warning(msg string) {
-	fmt.Printf("%s %s\n", color.YellowString("WARN"), msg)
-}
-
 // Description returns a string representation of the action plan.
 // If verbose is false, the operations are grouped by item type and operation type.
 // If verbose is true, they are listed as they appear in the command slice.
diff --git a/test/bats/20_hub.bats b/test/bats/20_hub.bats
@@ -172,8 +172,9 @@ teardown() {
     mkdir -p "$CONFIG_DIR/collections"
     touch "$CONFIG_DIR/collections/foo.yaml"
     rune -0 cscli hub upgrade
+    assert_stderr --partial 'collections:foo.yaml - not downloading local item'
+
     assert_output - <<-EOT
-	collections:foo.yaml - not downloading local item
 	Action plan:
 	🔄 check & update data files
 	EOT
diff --git a/test/bats/20_hub_items.bats b/test/bats/20_hub_items.bats
@@ -155,29 +155,29 @@ teardown() {
     rune -0 mkdir -p "$CONFIG_DIR/scenarios"
     rune -0 touch "$CONFIG_DIR/scenarios/foobar.yaml"
     rune -0 cscli scenarios remove foobar.yaml
+    assert_stderr --partial 'scenarios:foobar.yaml is a local item, please delete manually'
     assert_output - <<-EOT
-	WARN scenarios:foobar.yaml is a local item, please delete manually
 	Nothing to do.
 	EOT
     rune -0 cscli scenarios remove foobar.yaml --purge
+    assert_stderr --partial 'scenarios:foobar.yaml is a local item, please delete manually'
     assert_output - <<-EOT
-	WARN scenarios:foobar.yaml is a local item, please delete manually
 	Nothing to do.
 	EOT
     rune -0 cscli scenarios remove foobar.yaml --force
+    assert_stderr --partial 'scenarios:foobar.yaml is a local item, please delete manually'
     assert_output - <<-EOT
-	WARN scenarios:foobar.yaml is a local item, please delete manually
 	Nothing to do.
 	EOT
 
     rune -0 cscli scenarios install crowdsecurity/ssh-bf
 
     rune -0 cscli scenarios remove --all
-    assert_line "WARN scenarios:foobar.yaml is a local item, please delete manually"
     assert_line "disabling scenarios:crowdsecurity/ssh-bf"
+    assert_stderr --partial "scenarios:foobar.yaml is a local item, please delete manually"
 
     rune -0 cscli scenarios remove --all --purge
-    assert_line "WARN scenarios:foobar.yaml is a local item, please delete manually"
+    assert_stderr --partial "scenarios:foobar.yaml is a local item, please delete manually"
     assert_line "purging scenarios:crowdsecurity/ssh-bf"
 }
 
diff --git a/test/bats/cscli-hubtype-install.bats b/test/bats/cscli-hubtype-install.bats
@@ -166,18 +166,16 @@ get_latest_version() {
 
     rune -0 cscli parsers install crowdsecurity/whitelists --dry-run
     assert_output - --stderr <<-EOT
-	WARN parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite
 	Nothing to do.
 	EOT
-    refute_stderr
+    assert_stderr --partial "parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite"
 
     # XXX should this fail with status 1 instead?
     rune -0 cscli parsers install crowdsecurity/whitelists
     assert_output - <<-EOT
-	WARN parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite
 	Nothing to do.
 	EOT
-    refute_stderr
+    assert_stderr --partial "parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite"
 
     rune -0 cscli parsers install crowdsecurity/whitelists --force
     latest_whitelists=$(get_latest_version parsers crowdsecurity/whitelists)
@@ -230,17 +228,14 @@ get_latest_version() {
     # and maybe re-evaluate the --ignore flag
     rune -0 cscli parsers install crowdsecurity/whitelists --ignore
     assert_output - <<-EOT
-	WARN parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite
 	Nothing to do.
 	EOT
-    refute_stderr
+    assert_stderr --partial "parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite"
 
     # error on one item, should still install the others
     rune -0 cscli parsers install crowdsecurity/whitelists crowdsecurity/pgsql-logs --ignore
-    refute_stderr
     latest_pgsql=$(get_latest_version parsers crowdsecurity/pgsql-logs)
     assert_output - <<-EOT
-	WARN parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite
 	Action plan:
 	📥 download
 	 parsers: crowdsecurity/pgsql-logs ($latest_pgsql)
@@ -252,6 +247,7 @@ get_latest_version() {
 
 	$RELOAD_MESSAGE
 	EOT
+    assert_stderr --partial "parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite"
     rune -0 cscli parsers inspect crowdsecurity/pgsql-logs --no-metrics -o json
     rune -0 jq -e '.installed==true' <(output)
 }
@@ -268,14 +264,14 @@ get_latest_version() {
 
     # attempt to install from hub
     rune -0 cscli parsers install crowdsecurity/sshd-logs
-    assert_line 'parsers:crowdsecurity/sshd-logs - not downloading local item'
+    assert_stderr --partial 'parsers:crowdsecurity/sshd-logs - not downloading local item'
     rune -0 cscli parsers list -o json
     rune -0 jq -c '.parsers[] | [.name,.status]' <(output)
     assert_json '["crowdsecurity/sshd-logs","enabled,local"]'
 
     # attempt to install from a collection
     rune -0 cscli collections install crowdsecurity/sshd
-    assert_line 'parsers:crowdsecurity/sshd-logs - not downloading local item'
+    assert_stderr --partial 'parsers:crowdsecurity/sshd-logs - not downloading local item'
 
     # verify it installed the rest of the collection
     assert_line 'enabling contexts:crowdsecurity/bf_base'
@@ -293,11 +289,11 @@ get_latest_version() {
 
     # attempt to install from hub
     rune -0 cscli parsers install crowdsecurity/sshd-logs
-    assert_line 'parsers:crowdsecurity/sshd-logs - not downloading local item'
+    assert_stderr --partial 'parsers:crowdsecurity/sshd-logs - not downloading local item'
 
     # attempt to install from a collection
     rune -0 cscli collections install crowdsecurity/sshd
-    assert_line 'parsers:crowdsecurity/sshd-logs - not downloading local item'
+    assert_stderr --partial 'parsers:crowdsecurity/sshd-logs - not downloading local item'
 
     # verify it installed the rest of the collection
     assert_line 'enabling contexts:crowdsecurity/bf_base'
diff --git a/test/bats/cscli-hubtype-upgrade.bats b/test/bats/cscli-hubtype-upgrade.bats
@@ -165,17 +165,15 @@ get_latest_version() {
 
     rune -0 cscli parsers upgrade crowdsecurity/whitelists --dry-run
     assert_output - <<-EOT
-	WARN parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite
 	Nothing to do.
 	EOT
-    refute_stderr
+    assert_stderr --partial "parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite"
 
     rune -0 cscli parsers upgrade crowdsecurity/whitelists
     assert_output - <<-EOT
-	WARN parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite
 	Nothing to do.
 	EOT
-    refute_stderr
+    assert_stderr --partial "parsers:crowdsecurity/whitelists is tainted, use '--force' to overwrite"
 
     latest_whitelists=$(get_latest_version parsers crowdsecurity/whitelists)
 
diff --git a/test/lib/bats-assert b/test/lib/bats-assert
@@ -1 +1 @@
-Subproject commit 44913ffe6020d1561c4c4d1e26cda8e07a1f374f
+Subproject commit b93143a1bfbde41d9b7343aab0d36f3ef6549e6b
diff --git a/test/lib/setup_file.sh b/test/lib/setup_file.sh
@@ -200,50 +200,6 @@ is_stdin_empty() {
 }
 export -f is_stdin_empty
 
-assert_stderr() {
-    # it is never useful to call this without arguments
-    if [[ "$#" -eq 0 ]]; then
-        # maybe the caller forgot to use '-' with an heredoc
-        if ! is_stdin_empty; then
-            fail "${FUNCNAME[0]}: called with stdin and no arguments (heredoc?)"
-        fi
-        fail "${FUNCNAME[0]}: called with no arguments"
-    fi
-
-    local oldout="${output}"
-    run -0 echo "${stderr}"
-    assert_output "$@"
-    output="${oldout}"
-}
-export -f assert_stderr
-
-# like refute_output, but for stderr
-refute_stderr() {
-    # calling this without arguments is ok, as long as stdin in empty
-    if ! is_stdin_empty; then
-        fail "${FUNCNAME[0]}: called with stdin (heredoc?)"
-    fi
-
-    local oldout="${output}"
-    run -0 echo "${stderr}"
-    refute_output "$@"
-    output="${oldout}"
-}
-export -f refute_stderr
-
-# like assert_output, but for stderr
-assert_stderr_line() {
-    if [[ "$#" -eq 0 ]]; then
-        fail "${FUNCNAME[0]}: called with no arguments"
-    fi
-
-    local oldout="${output}"
-    run -0 echo "${stderr}"
-    assert_line "$@"
-    output="${oldout}"
-}
-export -f assert_stderr_line
-
 # remove all installed items and data
 hub_purge_all() {
     local CONFIG_DIR

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,8 @@ import (`
`5`	`5`	`"fmt"`
`6`	`6`	`"os"`
`7`	`7`
	`8`	`+ log "github.com/sirupsen/logrus"`
	`9`	`+`
`8`	`10`	`"github.com/crowdsecurity/crowdsec/pkg/cwhub"`
`9`	`11`	`)`
`10`	`12`
`@@ -53,7 +55,7 @@ func (c DisableCommand) Prepare(plan ActionPlan) (bool, error) {`
`53`	`55`	`i := c.Item`
`54`	`56`
`55`	`57`	`if i.State.IsLocal() {`
`56`		`- plan.Warning(i.FQName() + " is a local item, please delete manually")`
	`58`	`+ log.Warnf("%s is a local item, please delete manually", i.FQName())`
`57`	`59`	`return false, nil`
`58`	`60`	`}`
`59`	`61`