cscli: print errors in plain text with -o json (#2973)

cmd/crowdsec-cli/main.go

@@ -302,6 +302,8 @@ func main() {
 	}
 
 	if err := cmd.Execute(); err != nil {
-		log.Fatal(err)
+		red := color.New(color.FgRed).SprintFunc()
+		fmt.Fprintln(os.Stderr, red("Error:"), err)
+		os.Exit(1)
 	}
 }

test/bats/01_cscli.bats

@@ -33,9 +33,9 @@ teardown() {
 
     # no "usage" output after every error
     rune -1 cscli blahblah
-    # error is displayed as log entry, not with print
-    assert_stderr --partial 'level=fatal msg="unknown command \"blahblah\" for \"cscli\""'
-    refute_stderr --partial 'unknown command "blahblah" for "cscli"'
+    # error is displayed with print, not as a log entry
+    assert_stderr --partial 'unknown command "blahblah" for "cscli"'
+    refute_stderr --partial 'level=fatal'
 }
 
 @test "cscli version" {
@@ -294,7 +294,7 @@ teardown() {
     # it is possible to enable subcommands with feature flags defined in feature.yaml
 
     rune -1 cscli setup
-    assert_stderr --partial 'unknown command \"setup\" for \"cscli\"'
+    assert_stderr --partial 'unknown command "setup" for "cscli"'
     CONFIG_DIR=$(dirname "$CONFIG_YAML")
     echo ' - cscli_setup' >> "$CONFIG_DIR"/feature.yaml
     rune -0 cscli setup

test/bats/01_cscli_lapi.bats

@@ -113,9 +113,8 @@ teardown() {
     LOCAL_API_CREDENTIALS=$(config_get '.api.client.credentials_path')
     config_set "$LOCAL_API_CREDENTIALS" '.url="http://127.0.0.1:-80"'
 
-    rune -1 cscli lapi status -o json
-    rune -0 jq -r '.msg' <(stderr)
-    assert_output 'failed to authenticate to Local API (LAPI): parse "http://127.0.0.1:-80/": invalid port ":-80" after host'
+    rune -1 cscli lapi status
+    assert_stderr 'Error: failed to authenticate to Local API (LAPI): parse "http://127.0.0.1:-80/": invalid port ":-80" after host'
 }
 
 @test "cscli - bad LAPI password" {
@@ -123,9 +122,8 @@ teardown() {
     LOCAL_API_CREDENTIALS=$(config_get '.api.client.credentials_path')
     config_set "$LOCAL_API_CREDENTIALS" '.password="meh"'
 
-    rune -1 cscli lapi status -o json
-    rune -0 jq -r '.msg' <(stderr)
-    assert_output 'failed to authenticate to Local API (LAPI): API error: incorrect Username or Password'
+    rune -1 cscli lapi status
+    assert_stderr 'Error: failed to authenticate to Local API (LAPI): API error: incorrect Username or Password'
 }
 
 @test "cscli lapi register / machines validate" {
@@ -189,8 +187,10 @@ teardown() {
 
     rune -1 cscli lapi register --machine malicious --token 123456789012345678901234badtoken
     assert_stderr --partial "401 Unauthorized: API error: invalid token for auto registration"
-    rune -1 cscli machines inspect malicious -o json
-    assert_stderr --partial "unable to read machine data 'malicious': user 'malicious': user doesn't exist"
+    rune -1 cscli machines inspect malicious
+    # XXX: we may want to remove this warning
+    assert_stderr --partial 'QueryMachineByID : ent: machine not found'
+    assert_stderr --partial "Error: unable to read machine data 'malicious': user 'malicious': user doesn't exist"
 
     rune -0 cscli lapi register --machine newmachine --token 12345678901234567890123456789012
     assert_stderr --partial "Successfully registered to Local API"

test/bats/10_bouncers.bats

@@ -117,12 +117,9 @@ teardown() {
 
 @test "we can't add the same bouncer twice" {
     rune -0 cscli bouncers add ciTestBouncer
-    rune -1 cscli bouncers add ciTestBouncer -o json
+    rune -1 cscli bouncers add ciTestBouncer
 
-    # XXX temporary hack to filter out unwanted log lines that may appear before
-    # log configuration (= not json)
-    rune -0 jq -c '[.level,.msg]' <(stderr | grep "^{")
-    assert_output '["fatal","unable to create bouncer: bouncer ciTestBouncer already exists"]'
+    assert_stderr 'Error: unable to create bouncer: bouncer ciTestBouncer already exists'
 
     rune -0 cscli bouncers list -o json
     rune -0 jq '. | length' <(output)

test/bats/20_hub_items.bats

@@ -80,10 +80,9 @@ teardown() {
     echo "$new_hub" >"$INDEX_PATH"
 
     rune -0 cscli collections install crowdsecurity/sshd
-    rune -1 cscli collections inspect crowdsecurity/sshd --no-metrics -o json
+    rune -1 cscli collections inspect crowdsecurity/sshd --no-metrics
     # XXX: we are on the verbose side here...
-    rune -0 jq -r ".msg" <(stderr)
-    assert_output --regexp "failed to read Hub index: failed to sync hub items: failed to scan .*: while syncing collections sshd.yaml: 1.2.3.4: Invalid Semantic Version. Run 'sudo cscli hub update' to download the index again"
+    assert_stderr --regexp "Error: failed to read Hub index: failed to sync hub items: failed to scan .*: while syncing collections sshd.yaml: 1.2.3.4: Invalid Semantic Version. Run 'sudo cscli hub update' to download the index again"
 }
 
 @test "removing or purging an item already removed by hand" {

test/bats/30_machines.bats

@@ -30,9 +30,8 @@ teardown() {
 }
 
 @test "don't overwrite local credentials by default" {
-    rune -1 cscli machines add local -a -o json
-    rune -0 jq -r '.msg' <(stderr)
-    assert_output --partial 'already exists: please remove it, use "--force" or specify a different file with "-f"'
+    rune -1 cscli machines add local -a
+    assert_stderr --partial 'already exists: please remove it, use "--force" or specify a different file with "-f"'
     rune -0 cscli machines add local -a --force
     assert_stderr --partial "Machine 'local' successfully added to the local API."
 }

test/bats/90_decisions.bats

@@ -31,11 +31,7 @@ teardown() {
 
 @test "'decisions add' requires parameters" {
     rune -1 cscli decisions add
-    assert_stderr --partial "missing arguments, a value is required (--ip, --range or --scope and --value)"
-
-    rune -1 cscli decisions add -o json
-    rune -0 jq -c '[ .level, .msg]' <(stderr | grep "^{")
-    assert_output '["fatal","missing arguments, a value is required (--ip, --range or --scope and --value)"]'
+    assert_stderr "Error: missing arguments, a value is required (--ip, --range or --scope and --value)"
 }
 
 @test "cscli decisions list, with and without --machine" {
@@ -61,16 +57,13 @@ teardown() {
 
 @test "cscli decisions list, incorrect parameters" {
     rune -1 cscli decisions list --until toto
-    assert_stderr --partial 'unable to retrieve decisions: performing request: API error: while parsing duration: time: invalid duration \"toto\"'
-    rune -1 cscli decisions list --until toto -o json
-    rune -0 jq -c '[.level, .msg]' <(stderr | grep "^{")
-    assert_output '["fatal","unable to retrieve decisions: performing request: API error: while parsing duration: time: invalid duration \"toto\""]'
+    assert_stderr 'Error: unable to retrieve decisions: performing request: API error: while parsing duration: time: invalid duration "toto"'
 }
 
 @test "cscli decisions import" {
     # required input
     rune -1 cscli decisions import
-    assert_stderr --partial 'required flag(s) \"input\" not set"'
+    assert_stderr 'Error: required flag(s) "input" not set'
 
     # unsupported format
     rune -1 cscli decisions import -i - <<<'value\n5.6.7.8' --format xml