fix(cscli): correct and test the behavior of "cscli collections delete" (#1824)

pkg/cwhub/helpers.go

@@ -133,6 +133,9 @@ func RemoveMany(csConfig *csconfig.Config, itemType string, name string, all boo
 
 	// remove all
 	for _, v := range GetItemMap(itemType) {
+		if !v.Installed {
+			continue
+		}
 		v, err = DisableItem(csConfig.Hub, v, purge, forceAction)
 		if err != nil {
 			log.Fatalf("unable to disable %s : %v", v.Name, err)

pkg/cwhub/install.go

@@ -5,15 +5,42 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/crowdsecurity/crowdsec/pkg/csconfig"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
+
+	"github.com/crowdsecurity/crowdsec/pkg/csconfig"
 )
 
+func purgeItem(hub *csconfig.Hub, target Item) (Item, error) {
+	var hdir = hub.HubDir
+	hubpath := hdir + "/" + target.RemotePath
+
+	// disable hub file
+	if err := os.Remove(hubpath); err != nil {
+		return target, errors.Wrap(err, "while removing file")
+	}
+
+	target.Downloaded = false
+	log.Infof("Removed source file [%s] : %s", target.Name, hubpath)
+	hubIdx[target.Type][target.Name] = target
+	return target, nil
+}
+
 //DisableItem to disable an item managed by the hub, removes the symlink if purge is true
 func DisableItem(hub *csconfig.Hub, target Item, purge bool, force bool) (Item, error) {
 	var tdir = hub.ConfigDir
 	var hdir = hub.HubDir
+	var err error
+
+	if !target.Installed {
+		if purge {
+			target, err = purgeItem(hub, target)
+			if err != nil {
+				return target, err
+			}
+		}
+		return target, nil
+	}
 
 	syml, err := filepath.Abs(tdir + "/" + target.Type + "/" + target.Stage + "/" + target.FileName)
 	if err != nil {
@@ -91,13 +118,10 @@ func DisableItem(hub *csconfig.Hub, target Item, purge bool, force bool) (Item,
 	target.Installed = false
 
 	if purge {
-		hubpath := hdir + "/" + target.RemotePath
-		//if purge, disable hub file
-		if err = os.Remove(hubpath); err != nil {
-			return target, errors.Wrap(err, "while removing file")
+		target, err = purgeItem(hub, target)
+		if err != nil {
+			return target, err
 		}
-		target.Downloaded = false
-		log.Infof("Removed source file [%s] : %s", target.Name, hubpath)
 	}
 	hubIdx[target.Type][target.Name] = target
 	return target, nil

tests/bats/20_collections.bats

@@ -58,15 +58,55 @@ teardown() {
     refute_line "crowdsecurity/mysql"
 }
 
-@test "cannot remove a collection twice" {
-    run -0 cscli collections install crowdsecurity/mysql -o human
-    run -0 --separate-stderr cscli collections remove crowdsecurity/mysql
-    run -1 --separate-stderr cscli collections remove crowdsecurity/mysql -o json
-    run -0 jq -r '.level' <(stderr)
-    assert_output 'fatal'
-    run -0 jq -r '.msg' <(stderr)
-    assert_output --partial "unable to disable crowdsecurity/mysql"
-    assert_output --partial "doesn't exist"
+@test "must use --force to remove a collection that belongs to another, which becomes tainted" {
+    # we expect no error since we may have multiple collections, some removed and some not
+    run -0 --separate-stderr cscli collections remove crowdsecurity/sshd
+    assert_stderr --partial "crowdsecurity/sshd belongs to other collections"
+    assert_stderr --partial "[crowdsecurity/linux]"
+
+    run -0 --separate-stderr cscli collections remove crowdsecurity/sshd --force
+    assert_stderr --partial "Removed symlink [crowdsecurity/sshd]"
+    run -0 cscli collections inspect crowdsecurity/linux -o json
+    run -0 jq -r '.tainted' <(output)
+    assert_output "true"
+}
+
+@test "can remove a collection" {
+    run -0 cscli collections remove crowdsecurity/linux
+    assert_output --partial "Removed"
+    assert_output --regexp   ".*for the new configuration to be effective."
+    run -0 cscli collections inspect crowdsecurity/linux -o human
+    assert_line 'installed: false'
+}
+
+@test "collections delete is an alias for collections remove" {
+    run -0 cscli collections delete crowdsecurity/linux
+    assert_output --partial "Removed"
+    assert_output --regexp   ".*for the new configuration to be effective."
+}
+
+@test "removing a collection that does not exist is noop" {
+    run -0 cscli collections remove crowdsecurity/apache2
+    refute_output --partial "Removed"
+    assert_output --regexp   ".*for the new configuration to be effective."
+}
+
+@test "can remove a removed collection" {
+    run -0 cscli collections install crowdsecurity/mysql
+    run -0 cscli collections remove crowdsecurity/mysql
+    assert_output --partial "Removed"
+    run -0 cscli collections remove crowdsecurity/mysql
+    refute_output --partial "Removed"
+}
+
+@test "can remove all collections" {
+    run -0 cscli collections remove --all
+    assert_output --partial "Removed symlink [crowdsecurity/sshd]"
+    assert_output --partial "Removed symlink [crowdsecurity/linux]"
+    run -0 cscli hub list -o json
+    assert_json '{collections:[],parsers:[],postoverflows:[],scenarios:[]}'
+    run -0 cscli collections remove --all
+    assert_output --partial 'Disabled 0 items'
 }
 
 # TODO test download-only