From ab8de1950668d7649a410e78c074ebbd725e2f46 Mon Sep 17 00:00:00 2001
From: mmetc <92726601+mmetc@users.noreply.github.com>
Date: Thu, 9 Nov 2023 15:19:38 +0100
Subject: [PATCH] Refact cwhub: move methods from hub to item (#2585)

* Add back pointer Item.hub
* Hub.enableItem() -> Item.enable()
* Rename variable i -> idx (i is used for item instances)
* Move Hub.purgeItem() -> Item.purge()
* Move Hub.disableItem() -> Item.disable()
* Move Hub.downloadItem() -> Item.download()
* Move Hub.downloadLatest() -> Item.downloadLatest()
* Move Hub.DownloadDataIfNeeded() -> Item.DownloadDataIfNeeded()
* Move Hub.InstallItem() -> Item.Install()
* Move Hub.RemoveItem() -> Item.Remove()
* Move Hub.UpgradeItem() -> Item.Upgrade()
* store hub items as pointers
* No need to re-add items to the hub if we use pointers
* Fix parameter calling order + regression test
---
 cmd/crowdsec-cli/config_restore.go |   7 +-
 cmd/crowdsec-cli/hub.go            |   2 +-
 cmd/crowdsec-cli/itemcommands.go   |  53 ++++----
 pkg/cwhub/enable.go                | 103 ++++++++-------
 pkg/cwhub/enable_test.go           |  18 +--
 pkg/cwhub/helpers.go               | 199 ++++++++++++-----------------
 pkg/cwhub/helpers_test.go          |  29 +++--
 pkg/cwhub/hub.go                   |   2 +-
 pkg/cwhub/items.go                 |  42 ++----
 pkg/cwhub/items_test.go            |   9 +-
 pkg/cwhub/sync.go                  |  24 ++--
 pkg/hubtest/hubtest_item.go        |   6 +-
 pkg/setup/install.go               |  36 ++++--
 test/bats/07_setup.bats            |  25 ++--
 14 files changed, 269 insertions(+), 286 deletions(-)

diff --git a/cmd/crowdsec-cli/config_restore.go b/cmd/crowdsec-cli/config_restore.go
index d33b2e61a..56f628281 100644
--- a/cmd/crowdsec-cli/config_restore.go
+++ b/cmd/crowdsec-cli/config_restore.go
@@ -45,7 +45,12 @@ func restoreHub(dirPath string) error {
 			return fmt.Errorf("error unmarshaling %s : %s", upstreamListFN, err)
 		}
 		for _, toinstall := range upstreamList {
-			err := hub.InstallItem(toinstall, itype, false, false)
+			item := hub.GetItem(itype, toinstall)
+			if item == nil {
+				log.Errorf("Item %s/%s not found in hub", itype, toinstall)
+				continue
+			}
+			err := item.Install(false, false)
 			if err != nil {
 				log.Errorf("Error while installing %s : %s", toinstall, err)
 			}
diff --git a/cmd/crowdsec-cli/hub.go b/cmd/crowdsec-cli/hub.go
index f934bd856..3fc4579cb 100644
--- a/cmd/crowdsec-cli/hub.go
+++ b/cmd/crowdsec-cli/hub.go
@@ -145,7 +145,7 @@ func runHubUpgrade(cmd *cobra.Command, args []string) error {
 		updated := 0
 		log.Infof("Upgrading %s", itemType)
 		for _, item := range items {
-			didUpdate, err := hub.UpgradeItem(itemType, item.Name, force)
+			didUpdate, err := item.Upgrade(force)
 			if err != nil {
 				return err
 			}
diff --git a/cmd/crowdsec-cli/itemcommands.go b/cmd/crowdsec-cli/itemcommands.go
index 5a31dda2e..b6beac1f4 100644
--- a/cmd/crowdsec-cli/itemcommands.go
+++ b/cmd/crowdsec-cli/itemcommands.go
@@ -203,7 +203,8 @@ func itemsInstallRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 		}
 
 		for _, name := range args {
-			if hub.GetItem(it.name, name) == nil {
+			item := hub.GetItem(it.name, name)
+			if item == nil {
 				msg := SuggestNearestMessage(hub, it.name, name)
 				if !ignoreError {
 					return fmt.Errorf(msg)
@@ -213,11 +214,11 @@ func itemsInstallRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 				continue
 			}
 
-			if err := hub.InstallItem(name, it.name, force, downloadOnly); err != nil {
+			if err := item.Install(force, downloadOnly); err != nil {
 				if !ignoreError {
-					return fmt.Errorf("error while installing '%s': %w", name, err)
+					return fmt.Errorf("error while installing '%s': %w", item.Name, err)
 				}
-				log.Errorf("Error while installing '%s': %s", name, err)
+				log.Errorf("Error while installing '%s': %s", item.Name, err)
 			}
 		}
 
@@ -286,7 +287,7 @@ func itemsRemoveRunner(it hubItemType) func(cmd *cobra.Command, args []string) e
 			removed := 0
 
 			for _, item := range items {
-				didRemove, err := hub.RemoveItem(it.name, item.Name, purge, force)
+				didRemove, err := item.Remove(purge, force)
 				if err != nil {
 					return err
 				}
@@ -308,27 +309,25 @@ func itemsRemoveRunner(it hubItemType) func(cmd *cobra.Command, args []string) e
 		}
 
 		removed := 0
-		for _, name := range args {
-			if !force {
-				item := hub.GetItem(it.name, name)
-				if item == nil {
-					// XXX: this should be in GetItem?
-					return fmt.Errorf("can't find '%s' in %s", name, it.name)
-				}
-				if len(item.BelongsToCollections) > 0 {
-					log.Warningf("%s belongs to collections: %s", name, item.BelongsToCollections)
-					log.Warningf("Run 'sudo cscli %s remove %s --force' if you want to force remove this %s", it.name, name, it.singular)
-					continue
-				}
+		for _, itemName := range args {
+			item := hub.GetItem(it.name, itemName)
+			if item == nil {
+				return fmt.Errorf("can't find '%s' in %s", itemName, it.name)
 			}
 
-			didRemove, err := hub.RemoveItem(it.name, name, purge, force)
+			if !force && len(item.BelongsToCollections) > 0 {
+				log.Warningf("%s belongs to collections: %s", item.Name, item.BelongsToCollections)
+				log.Warningf("Run 'sudo cscli %s remove %s --force' if you want to force remove this %s", item.Type, item.Name, it.singular)
+				continue
+			}
+
+			didRemove, err := item.Remove(purge, force)
 			if err != nil {
 				return err
 			}
 
 			if didRemove {
-				log.Infof("Removed %s", name)
+				log.Infof("Removed %s", item.Name)
 				removed++
 			}
 		}
@@ -392,7 +391,7 @@ func itemsUpgradeRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 
 			updated := 0
 			for _, item := range items {
-				didUpdate, err := hub.UpgradeItem(it.name, item.Name, force)
+				didUpdate, err := item.Upgrade(force)
 				if err != nil {
 					return err
 				}
@@ -400,7 +399,9 @@ func itemsUpgradeRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 					updated++
 				}
 			}
+
 			log.Infof("Updated %d %s", updated, it.name)
+
 			if updated > 0 {
 				log.Infof(ReloadMessage())
 			}
@@ -413,13 +414,19 @@ func itemsUpgradeRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 		}
 
 		updated := 0
-		for _, name := range args {
-			didUpdate, err := hub.UpgradeItem(it.name, name, force)
+		for _, itemName := range args {
+			item := hub.GetItem(it.name, itemName)
+			if item == nil {
+				return fmt.Errorf("can't find '%s' in %s", itemName, it.name)
+			}
+
+			didUpdate, err := item.Upgrade(force)
 			if err != nil {
 				return err
 			}
+
 			if didUpdate {
-				log.Infof("Updated %s", name)
+				log.Infof("Updated %s", item.Name)
 				updated++
 			}
 		}
diff --git a/pkg/cwhub/enable.go b/pkg/cwhub/enable.go
index 2f137fa76..e0c20628f 100644
--- a/pkg/cwhub/enable.go
+++ b/pkg/cwhub/enable.go
@@ -10,24 +10,24 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-// enableItem creates a symlink between actual config file at hub.HubDir and hub.ConfigDir
+// enable creates a symlink between actual config file at hub.HubDir and hub.ConfigDir
 // Handles collections recursively
-func (h *Hub) enableItem(target *Item) error {
-	parentDir := filepath.Clean(h.local.InstallDir + "/" + target.Type + "/" + target.Stage + "/")
+func (i *Item) enable() error {
+	parentDir := filepath.Clean(i.hub.local.InstallDir + "/" + i.Type + "/" + i.Stage + "/")
 
 	// create directories if needed
-	if target.Installed {
-		if target.Tainted {
-			return fmt.Errorf("%s is tainted, won't enable unless --force", target.Name)
+	if i.Installed {
+		if i.Tainted {
+			return fmt.Errorf("%s is tainted, won't enable unless --force", i.Name)
 		}
 
-		if target.IsLocal() {
-			return fmt.Errorf("%s is local, won't enable", target.Name)
+		if i.IsLocal() {
+			return fmt.Errorf("%s is local, won't enable", i.Name)
 		}
 
 		// if it's a collection, check sub-items even if the collection file itself is up-to-date
-		if target.UpToDate && !target.HasSubItems() {
-			log.Tracef("%s is installed and up-to-date, skip.", target.Name)
+		if i.UpToDate && !i.HasSubItems() {
+			log.Tracef("%s is installed and up-to-date, skip.", i.Name)
 			return nil
 		}
 	}
@@ -41,30 +41,30 @@ func (h *Hub) enableItem(target *Item) error {
 	}
 
 	// install sub-items if any
-	for _, sub := range target.SubItems() {
-		val, ok := h.Items[sub.Type][sub.Name]
+	for _, sub := range i.SubItems() {
+		val, ok := i.hub.Items[sub.Type][sub.Name]
 		if !ok {
-			return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, target.Name)
+			return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, i.Name)
 		}
 
-		if err := h.enableItem(&val); err != nil {
+		if err := val.enable(); err != nil {
 			return fmt.Errorf("while installing %s: %w", sub.Name, err)
 		}
 	}
 
 	// check if file already exists where it should in configdir (eg /etc/crowdsec/collections/)
-	if _, err := os.Lstat(parentDir + "/" + target.FileName); !os.IsNotExist(err) {
-		log.Infof("%s already exists.", parentDir+"/"+target.FileName)
+	if _, err := os.Lstat(parentDir + "/" + i.FileName); !os.IsNotExist(err) {
+		log.Infof("%s already exists.", parentDir+"/"+i.FileName)
 		return nil
 	}
 
 	// hub.ConfigDir + target.RemotePath
-	srcPath, err := filepath.Abs(h.local.HubDir + "/" + target.RemotePath)
+	srcPath, err := filepath.Abs(i.hub.local.HubDir + "/" + i.RemotePath)
 	if err != nil {
 		return fmt.Errorf("while getting source path: %w", err)
 	}
 
-	dstPath, err := filepath.Abs(parentDir + "/" + target.FileName)
+	dstPath, err := filepath.Abs(parentDir + "/" + i.FileName)
 	if err != nil {
 		return fmt.Errorf("while getting destination path: %w", err)
 	}
@@ -73,37 +73,36 @@ func (h *Hub) enableItem(target *Item) error {
 		return fmt.Errorf("while creating symlink from %s to %s: %w", srcPath, dstPath, err)
 	}
 
-	log.Infof("Enabled %s: %s", target.Type, target.Name)
-	target.Installed = true
-	h.Items[target.Type][target.Name] = *target
+	log.Infof("Enabled %s: %s", i.Type, i.Name)
+	i.Installed = true
 
 	return nil
 }
 
-func (h *Hub) purgeItem(target Item) (Item, error) {
-	itempath := h.local.HubDir + "/" + target.RemotePath
+// purge removes the actual config file that was downloaded
+func (i *Item) purge() error {
+	itempath := i.hub.local.HubDir + "/" + i.RemotePath
 
 	// disable hub file
 	if err := os.Remove(itempath); err != nil {
-		return target, fmt.Errorf("while removing file: %w", err)
+		return fmt.Errorf("while removing file: %w", err)
 	}
 
-	target.Downloaded = false
-	log.Infof("Removed source file [%s]: %s", target.Name, itempath)
-	h.Items[target.Type][target.Name] = target
+	i.Downloaded = false
+	log.Infof("Removed source file [%s]: %s", i.Name, itempath)
 
-	return target, nil
+	return nil
 }
 
-// disableItem to disable an item managed by the hub, removes the symlink if purge is true
-func (h *Hub) disableItem(target *Item, purge bool, force bool) error {
+// disable removes the symlink to the downloaded content, also removes the content if purge is true
+func (i *Item) disable(purge bool, force bool) error {
 	// XXX: should return the number of disabled/purged items to inform the upper layer whether to reload or not
 	var err error
 
 	// already disabled, noop unless purge
-	if !target.Installed {
+	if !i.Installed {
 		if purge {
-			*target, err = h.purgeItem(*target)
+			err = i.purge()
 			if err != nil {
 				return err
 			}
@@ -112,20 +111,20 @@ func (h *Hub) disableItem(target *Item, purge bool, force bool) error {
 		return nil
 	}
 
-	if target.IsLocal() {
-		return fmt.Errorf("%s isn't managed by hub. Please delete manually", target.Name)
+	if i.IsLocal() {
+		return fmt.Errorf("%s isn't managed by hub. Please delete manually", i.Name)
 	}
 
-	if target.Tainted && !force {
-		return fmt.Errorf("%s is tainted, use '--force' to overwrite", target.Name)
+	if i.Tainted && !force {
+		return fmt.Errorf("%s is tainted, use '--force' to overwrite", i.Name)
 	}
 
 	// disable sub-items if any - it's a collection
-	for _, sub := range target.SubItems() {
+	for _, sub := range i.SubItems() {
 		// XXX: we do this already when syncing, do we really need to do consistency checks here and there?
-		val, ok := h.Items[sub.Type][sub.Name]
+		val, ok := i.hub.Items[sub.Type][sub.Name]
 		if !ok {
-			log.Errorf("Referred %s %s in collection %s doesn't exist.", sub.Type, sub.Name, target.Name)
+			log.Errorf("Referred %s %s in collection %s doesn't exist.", sub.Type, sub.Name, i.Name)
 			continue
 		}
 
@@ -133,14 +132,14 @@ func (h *Hub) disableItem(target *Item, purge bool, force bool) error {
 		toRemove := true
 
 		for _, collection := range val.BelongsToCollections {
-			if collection != target.Name {
+			if collection != i.Name {
 				toRemove = false
 				break
 			}
 		}
 
 		if toRemove {
-			if err = h.disableItem(&val, purge, force); err != nil {
+			if err = val.disable(purge, force); err != nil {
 				return fmt.Errorf("while disabling %s: %w", sub.Name, err)
 			}
 		} else {
@@ -148,7 +147,7 @@ func (h *Hub) disableItem(target *Item, purge bool, force bool) error {
 		}
 	}
 
-	syml, err := filepath.Abs(h.local.InstallDir + "/" + target.Type + "/" + target.Stage + "/" + target.FileName)
+	syml, err := filepath.Abs(i.hub.local.InstallDir + "/" + i.Type + "/" + i.Stage + "/" + i.FileName)
 	if err != nil {
 		return err
 	}
@@ -157,13 +156,13 @@ func (h *Hub) disableItem(target *Item, purge bool, force bool) error {
 	if os.IsNotExist(err) {
 		// we only accept to "delete" non existing items if it's a forced purge
 		if !purge && !force {
-			return fmt.Errorf("can't delete %s: %s doesn't exist", target.Name, syml)
+			return fmt.Errorf("can't delete %s: %s doesn't exist", i.Name, syml)
 		}
 	} else {
 		// if it's managed by hub, it's a symlink to csconfig.GConfig.hub.HubDir / ...
 		if stat.Mode()&os.ModeSymlink == 0 {
-			log.Warningf("%s (%s) isn't a symlink, can't disable", target.Name, syml)
-			return fmt.Errorf("%s isn't managed by hub", target.Name)
+			log.Warningf("%s (%s) isn't a symlink, can't disable", i.Name, syml)
+			return fmt.Errorf("%s isn't managed by hub", i.Name)
 		}
 
 		hubpath, err := os.Readlink(syml)
@@ -171,14 +170,14 @@ func (h *Hub) disableItem(target *Item, purge bool, force bool) error {
 			return fmt.Errorf("while reading symlink: %w", err)
 		}
 
-		absPath, err := filepath.Abs(h.local.HubDir + "/" + target.RemotePath)
+		absPath, err := filepath.Abs(i.hub.local.HubDir + "/" + i.RemotePath)
 		if err != nil {
 			return fmt.Errorf("while abs path: %w", err)
 		}
 
 		if hubpath != absPath {
-			log.Warningf("%s (%s) isn't a symlink to %s", target.Name, syml, absPath)
-			return fmt.Errorf("%s isn't managed by hub", target.Name)
+			log.Warningf("%s (%s) isn't a symlink to %s", i.Name, syml, absPath)
+			return fmt.Errorf("%s isn't managed by hub", i.Name)
 		}
 
 		// remove the symlink
@@ -186,19 +185,17 @@ func (h *Hub) disableItem(target *Item, purge bool, force bool) error {
 			return fmt.Errorf("while removing symlink: %w", err)
 		}
 
-		log.Infof("Removed symlink [%s]: %s", target.Name, syml)
+		log.Infof("Removed symlink [%s]: %s", i.Name, syml)
 	}
 
-	target.Installed = false
+	i.Installed = false
 
 	if purge {
-		*target, err = h.purgeItem(*target)
+		err = i.purge()
 		if err != nil {
 			return err
 		}
 	}
 
-	h.Items[target.Type][target.Name] = *target
-
 	return nil
 }
diff --git a/pkg/cwhub/enable_test.go b/pkg/cwhub/enable_test.go
index a476c76d7..771edd8a7 100644
--- a/pkg/cwhub/enable_test.go
+++ b/pkg/cwhub/enable_test.go
@@ -8,9 +8,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func testInstall(hub *Hub, t *testing.T, item Item) {
+func testInstall(hub *Hub, t *testing.T, item *Item) {
 	// Install the parser
-	err := hub.downloadLatest(&item, false, false)
+	err := item.downloadLatest(false, false)
 	require.NoError(t, err, "failed to download %s", item.Name)
 
 	err = hub.localSync()
@@ -20,7 +20,7 @@ func testInstall(hub *Hub, t *testing.T, item Item) {
 	assert.False(t, hub.Items[item.Type][item.Name].Installed, "%s should not be installed", item.Name)
 	assert.False(t, hub.Items[item.Type][item.Name].Tainted, "%s should not be tainted", item.Name)
 
-	err = hub.enableItem(&item)
+	err = item.enable()
 	require.NoError(t, err, "failed to enable %s", item.Name)
 
 	err = hub.localSync()
@@ -29,7 +29,7 @@ func testInstall(hub *Hub, t *testing.T, item Item) {
 	assert.True(t, hub.Items[item.Type][item.Name].Installed, "%s should be installed", item.Name)
 }
 
-func testTaint(hub *Hub, t *testing.T, item Item) {
+func testTaint(hub *Hub, t *testing.T, item *Item) {
 	assert.False(t, hub.Items[item.Type][item.Name].Tainted, "%s should not be tainted", item.Name)
 
 	f, err := os.OpenFile(item.LocalPath, os.O_APPEND|os.O_WRONLY, 0600)
@@ -47,11 +47,11 @@ func testTaint(hub *Hub, t *testing.T, item Item) {
 	assert.True(t, hub.Items[item.Type][item.Name].Tainted, "%s should be tainted", item.Name)
 }
 
-func testUpdate(hub *Hub, t *testing.T, item Item) {
+func testUpdate(hub *Hub, t *testing.T, item *Item) {
 	assert.False(t, hub.Items[item.Type][item.Name].UpToDate, "%s should not be up-to-date", item.Name)
 
 	// Update it + check status
-	err := hub.downloadLatest(&item, true, true)
+	err := item.downloadLatest(true, true)
 	require.NoError(t, err, "failed to update %s", item.Name)
 
 	// Local sync and check status
@@ -62,11 +62,11 @@ func testUpdate(hub *Hub, t *testing.T, item Item) {
 	assert.False(t, hub.Items[item.Type][item.Name].Tainted, "%s should not be tainted anymore", item.Name)
 }
 
-func testDisable(hub *Hub, t *testing.T, item Item) {
+func testDisable(hub *Hub, t *testing.T, item *Item) {
 	assert.True(t, hub.Items[item.Type][item.Name].Installed, "%s should be installed", item.Name)
 
 	// Remove
-	err := hub.disableItem(&item, false, false)
+	err := item.disable(false, false)
 	require.NoError(t, err, "failed to disable %s", item.Name)
 
 	// Local sync and check status
@@ -79,7 +79,7 @@ func testDisable(hub *Hub, t *testing.T, item Item) {
 	assert.True(t, hub.Items[item.Type][item.Name].Downloaded, "%s should still be downloaded", item.Name)
 
 	// Purge
-	err = hub.disableItem(&item, true, false)
+	err = item.disable(true, false)
 	require.NoError(t, err, "failed to purge %s", item.Name)
 
 	// Local sync and check status
diff --git a/pkg/cwhub/helpers.go b/pkg/cwhub/helpers.go
index e8b49b814..774a7386a 100644
--- a/pkg/cwhub/helpers.go
+++ b/pkg/cwhub/helpers.go
@@ -19,15 +19,10 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-// InstallItem installs an item from the hub
-func (h *Hub) InstallItem(name string, itemType string, force bool, downloadOnly bool) error {
-	item := h.GetItem(itemType, name)
-	if item == nil {
-		return fmt.Errorf("unable to retrieve item: %s", name)
-	}
-
-	if downloadOnly && item.Downloaded && item.UpToDate {
-		log.Warningf("%s is already downloaded and up-to-date", item.Name)
+// Install installs the item from the hub, downloading it if needed
+func (i *Item) Install(force bool, downloadOnly bool) error {
+	if downloadOnly && i.Downloaded && i.UpToDate {
+		log.Warningf("%s is already downloaded and up-to-date", i.Name)
 
 		if !force {
 			return nil
@@ -35,90 +30,68 @@ func (h *Hub) InstallItem(name string, itemType string, force bool, downloadOnly
 	}
 
 	// XXX: confusing semantic between force and updateOnly?
-	if err := h.downloadLatest(item, force, true); err != nil {
-		return fmt.Errorf("while downloading %s: %w", item.Name, err)
-	}
-
-	if err := h.AddItem(*item); err != nil {
-		return fmt.Errorf("while adding %s: %w", item.Name, err)
+	if err := i.downloadLatest(force, true); err != nil {
+		return fmt.Errorf("while downloading %s: %w", i.Name, err)
 	}
 
 	if downloadOnly {
 		// XXX: should get the path from downloadLatest
-		log.Infof("Downloaded %s to %s", item.Name, filepath.Join(h.local.HubDir, item.RemotePath))
+		log.Infof("Downloaded %s to %s", i.Name, filepath.Join(i.hub.local.HubDir, i.RemotePath))
 		return nil
 	}
 
 	// XXX: should we stop here if the item is already installed?
 
-	if err := h.enableItem(item); err != nil {
-		return fmt.Errorf("while enabling %s: %w", item.Name, err)
+	if err := i.enable(); err != nil {
+		return fmt.Errorf("while enabling %s: %w", i.Name, err)
 	}
 
-	if err := h.AddItem(*item); err != nil {
-		return fmt.Errorf("while adding %s: %w", item.Name, err)
-	}
-
-	log.Infof("Enabled %s", item.Name)
+	log.Infof("Enabled %s", i.Name)
 
 	return nil
 }
 
-// RemoveItem disables one item, optionally removing the downloaded content
-func (h *Hub) RemoveItem(itemType string, name string, purge bool, forceAction bool) (bool, error) {
+// Remove disables the item, optionally removing the downloaded content
+func (i *Item) Remove(purge bool, forceAction bool) (bool, error) {
 	removed := false
 
-	item := h.GetItem(itemType, name)
-	if item == nil {
-		return false, fmt.Errorf("can't find '%s' in %s", name, itemType)
-	}
-
-	if !item.Downloaded {
-		log.Infof("removing %s: not downloaded -- no removal required", item.Name)
+	if !i.Downloaded {
+		log.Infof("removing %s: not downloaded -- no removal required", i.Name)
 		return false, nil
 	}
 
-	if !item.Installed && !purge {
-		log.Infof("removing %s: already uninstalled", item.Name)
+	if !i.Installed && !purge {
+		log.Infof("removing %s: already uninstalled", i.Name)
 		return false, nil
 	}
 
-	if err := h.disableItem(item, purge, forceAction); err != nil {
-		return false, fmt.Errorf("unable to disable %s: %w", item.Name, err)
+	if err := i.disable(purge, forceAction); err != nil {
+		return false, fmt.Errorf("unable to disable %s: %w", i.Name, err)
 	}
 
-	// XXX: should take the value from disableItem
+	// XXX: should take the value from disable()
 	removed = true
 
-	if err := h.AddItem(*item); err != nil {
-		return false, fmt.Errorf("unable to refresh item state %s: %w", item.Name, err)
-	}
-
 	return removed, nil
 }
 
-// UpgradeItem upgrades an item from the hub
-func (h *Hub) UpgradeItem(itemType string, name string, force bool) (bool, error) {
+// Upgrade downloads and applies the last version from the hub
+func (i *Item) Upgrade(force bool) (bool, error) {
 	updated := false
 
-	item := h.GetItem(itemType, name)
-	if item == nil {
-		return false, fmt.Errorf("can't find '%s' in %s", name, itemType)
+	if !i.Downloaded {
+		return false, fmt.Errorf("can't upgrade %s: not installed", i.Name)
 	}
 
-	if !item.Downloaded {
-		return false, fmt.Errorf("can't upgrade %s: not installed", item.Name)
+	if !i.Installed {
+		return false, fmt.Errorf("can't upgrade %s: downloaded but not installed", i.Name)
 	}
 
-	if !item.Installed {
-		return false, fmt.Errorf("can't upgrade %s: downloaded but not installed", item.Name)
-	}
+	if i.UpToDate {
+		log.Infof("%s: up-to-date", i.Name)
 
-	if item.UpToDate {
-		log.Infof("%s: up-to-date", item.Name)
-
-		if err := h.DownloadDataIfNeeded(*item, force); err != nil {
-			return false, fmt.Errorf("%s: download failed: %w", item.Name, err)
+		if err := i.DownloadDataIfNeeded(force); err != nil {
+			return false, fmt.Errorf("%s: download failed: %w", i.Name, err)
 		}
 
 		if !force {
@@ -127,112 +100,106 @@ func (h *Hub) UpgradeItem(itemType string, name string, force bool) (bool, error
 		}
 	}
 
-	if err := h.downloadLatest(item, force, true); err != nil {
-		return false, fmt.Errorf("%s: download failed: %w", item.Name, err)
+	if err := i.downloadLatest(force, true); err != nil {
+		return false, fmt.Errorf("%s: download failed: %w", i.Name, err)
 	}
 
-	if !item.UpToDate {
-		if item.Tainted {
-			log.Infof("%v %s is tainted, --force to overwrite", emoji.Warning, item.Name)
-		} else if item.IsLocal() {
-			log.Infof("%v %s is local", emoji.Prohibited, item.Name)
+	if !i.UpToDate {
+		if i.Tainted {
+			log.Infof("%v %s is tainted, --force to overwrite", emoji.Warning, i.Name)
+		} else if i.IsLocal() {
+			log.Infof("%v %s is local", emoji.Prohibited, i.Name)
 		}
 	} else {
 		// a check on stdout is used while scripting to know if the hub has been upgraded
 		// and a configuration reload is required
 		// TODO: use a better way to communicate this
-		fmt.Printf("updated %s\n", item.Name)
-		log.Infof("%v %s: updated", emoji.Package, item.Name)
+		fmt.Printf("updated %s\n", i.Name)
+		log.Infof("%v %s: updated", emoji.Package, i.Name)
 		updated = true
 	}
 
-	if err := h.AddItem(*item); err != nil {
-		return false, fmt.Errorf("unable to refresh item state %s: %w", item.Name, err)
-	}
-
 	return updated, nil
 }
 
 // downloadLatest will download the latest version of Item to the tdir directory
-func (h *Hub) downloadLatest(target *Item, overwrite bool, updateOnly bool) error {
-	// XXX: should return the path of the downloaded file (taken from downloadItem)
-	log.Debugf("Downloading %s %s", target.Type, target.Name)
+func (i *Item) downloadLatest(overwrite bool, updateOnly bool) error {
+	// XXX: should return the path of the downloaded file (taken from download())
+	log.Debugf("Downloading %s %s", i.Type, i.Name)
 
-	if !target.HasSubItems() {
-		if !target.Installed && updateOnly && target.Downloaded {
-			log.Debugf("skipping upgrade of %s: not installed", target.Name)
+	if !i.HasSubItems() {
+		if !i.Installed && updateOnly && i.Downloaded {
+			log.Debugf("skipping upgrade of %s: not installed", i.Name)
 			return nil
 		}
 
 		// XXX:
-		return h.downloadItem(target, overwrite)
+		return i.download(overwrite)
 	}
 
 	// collection
-	for _, sub := range target.SubItems() {
-		val, ok := h.Items[sub.Type][sub.Name]
+	for _, sub := range i.SubItems() {
+		val, ok := i.hub.Items[sub.Type][sub.Name]
 		if !ok {
-			return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, target.Name)
+			return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, i.Name)
 		}
 
 		if !val.Installed && updateOnly && val.Downloaded {
-			log.Debugf("skipping upgrade of %s: not installed", target.Name)
+			log.Debugf("skipping upgrade of %s: not installed", i.Name)
 			continue
 		}
 
-		log.Debugf("Download %s sub-item: %s %s (%t -> %t)", target.Name, sub.Type, sub.Name, target.Installed, updateOnly)
+		log.Debugf("Download %s sub-item: %s %s (%t -> %t)", i.Name, sub.Type, sub.Name, i.Installed, updateOnly)
 
 		// recurse as it's a collection
 		if sub.HasSubItems() {
 			log.Tracef("collection, recurse")
 
-			if err := h.downloadLatest(&val, overwrite, updateOnly); err != nil {
+			if err := val.downloadLatest(overwrite, updateOnly); err != nil {
 				return fmt.Errorf("while downloading %s: %w", val.Name, err)
 			}
 		}
 
 		downloaded := val.Downloaded
 
-		if err := h.downloadItem(&val, overwrite); err != nil {
+		if err := val.download(overwrite); err != nil {
 			return fmt.Errorf("while downloading %s: %w", val.Name, err)
 		}
 
 		// We need to enable an item when it has been added to a collection since latest release of the collection.
 		// We check if val.Downloaded is false because maybe the item has been disabled by the user.
 		if !val.Installed && !downloaded {
-			if err := h.enableItem(&val); err != nil {
+			if err := val.enable(); err != nil {
 				return fmt.Errorf("enabling '%s': %w", val.Name, err)
 			}
 		}
-
-		h.Items[sub.Type][sub.Name] = val
 	}
 
-	if err := h.downloadItem(target, overwrite); err != nil {
+	if err := i.download(overwrite); err != nil {
 		return fmt.Errorf("failed to download item: %w", err)
 	}
 
 	return nil
 }
 
-func (h *Hub) downloadItem(target *Item, overwrite bool) error {
-	url, err := h.remote.urlTo(target.RemotePath)
+func (i *Item) download(overwrite bool) error {
+	url, err := i.hub.remote.urlTo(i.RemotePath)
 	if err != nil {
 		return fmt.Errorf("failed to build hub item request: %w", err)
 	}
 
-	tdir := h.local.HubDir
+	tdir := i.hub.local.HubDir
 
 	// if user didn't --force, don't overwrite local, tainted, up-to-date files
 	if !overwrite {
-		if target.Tainted {
-			log.Debugf("%s: tainted, not updated", target.Name)
+		if i.Tainted {
+			log.Debugf("%s: tainted, not updated", i.Name)
 			return nil
 		}
 
-		if target.UpToDate {
+		if i.UpToDate {
 			//  We still have to check if data files are present
-			log.Debugf("%s: up-to-date, not updated", target.Name)
+			log.Debugf("%s: up-to-date, not updated", i.Name)
 		}
 	}
 
@@ -253,30 +220,30 @@ func (h *Hub) downloadItem(target *Item, overwrite bool) error {
 
 	hash := sha256.New()
 	if _, err = hash.Write(body); err != nil {
-		return fmt.Errorf("while hashing %s: %w", target.Name, err)
+		return fmt.Errorf("while hashing %s: %w", i.Name, err)
 	}
 
 	meow := hex.EncodeToString(hash.Sum(nil))
-	if meow != target.Versions[target.Version].Digest {
+	if meow != i.Versions[i.Version].Digest {
 		log.Errorf("Downloaded version doesn't match index, please 'hub update'")
-		log.Debugf("got %s, expected %s", meow, target.Versions[target.Version].Digest)
+		log.Debugf("got %s, expected %s", meow, i.Versions[i.Version].Digest)
 
-		return fmt.Errorf("invalid download hash for %s", target.Name)
+		return fmt.Errorf("invalid download hash for %s", i.Name)
 	}
 
 	//all good, install
 	//check if parent dir exists
-	tmpdirs := strings.Split(tdir+"/"+target.RemotePath, "/")
+	tmpdirs := strings.Split(tdir+"/"+i.RemotePath, "/")
 	parentDir := strings.Join(tmpdirs[:len(tmpdirs)-1], "/")
 
 	// ensure that target file is within target dir
-	finalPath, err := filepath.Abs(tdir + "/" + target.RemotePath)
+	finalPath, err := filepath.Abs(tdir + "/" + i.RemotePath)
 	if err != nil {
-		return fmt.Errorf("filepath.Abs error on %s: %w", tdir+"/"+target.RemotePath, err)
+		return fmt.Errorf("filepath.Abs error on %s: %w", tdir+"/"+i.RemotePath, err)
 	}
 
 	if !strings.HasPrefix(finalPath, tdir) {
-		return fmt.Errorf("path %s escapes %s, abort", target.RemotePath, tdir)
+		return fmt.Errorf("path %s escapes %s, abort", i.RemotePath, tdir)
 	}
 
 	// check dir
@@ -290,13 +257,13 @@ func (h *Hub) downloadItem(target *Item, overwrite bool) error {
 
 	// check actual file
 	if _, err = os.Stat(finalPath); !os.IsNotExist(err) {
-		log.Warningf("%s: overwrite", target.Name)
-		log.Debugf("target: %s/%s", tdir, target.RemotePath)
+		log.Warningf("%s: overwrite", i.Name)
+		log.Debugf("target: %s/%s", tdir, i.RemotePath)
 	} else {
-		log.Infof("%s: OK", target.Name)
+		log.Infof("%s: OK", i.Name)
 	}
 
-	f, err := os.OpenFile(tdir+"/"+target.RemotePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
+	f, err := os.OpenFile(tdir+"/"+i.RemotePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
 	if err != nil {
 		return fmt.Errorf("while opening file: %w", err)
 	}
@@ -308,22 +275,20 @@ func (h *Hub) downloadItem(target *Item, overwrite bool) error {
 		return fmt.Errorf("while writing file: %w", err)
 	}
 
-	target.Downloaded = true
-	target.Tainted = false
-	target.UpToDate = true
+	i.Downloaded = true
+	i.Tainted = false
+	i.UpToDate = true
 
-	if err = downloadData(h.local.InstallDataDir, overwrite, bytes.NewReader(body)); err != nil {
-		return fmt.Errorf("while downloading data for %s: %w", target.FileName, err)
+	if err = downloadData(i.hub.local.InstallDataDir, overwrite, bytes.NewReader(body)); err != nil {
+		return fmt.Errorf("while downloading data for %s: %w", i.FileName, err)
 	}
 
-	h.Items[target.Type][target.Name] = *target
-
 	return nil
 }
 
-// DownloadDataIfNeeded downloads the data files for an item
-func (h *Hub) DownloadDataIfNeeded(target Item, force bool) error {
-	itemFilePath := fmt.Sprintf("%s/%s/%s/%s", h.local.InstallDir, target.Type, target.Stage, target.FileName)
+// DownloadDataIfNeeded downloads the data files for the item
+func (i *Item) DownloadDataIfNeeded(force bool) error {
+	itemFilePath := fmt.Sprintf("%s/%s/%s/%s", i.hub.local.InstallDir, i.Type, i.Stage, i.FileName)
 
 	itemFile, err := os.Open(itemFilePath)
 	if err != nil {
@@ -332,7 +297,7 @@ func (h *Hub) DownloadDataIfNeeded(target Item, force bool) error {
 
 	defer itemFile.Close()
 
-	if err = downloadData(h.local.InstallDataDir, force, itemFile); err != nil {
+	if err = downloadData(i.hub.local.InstallDataDir, force, itemFile); err != nil {
 		return fmt.Errorf("while downloading data for %s: %w", itemFilePath, err)
 	}
 
diff --git a/pkg/cwhub/helpers_test.go b/pkg/cwhub/helpers_test.go
index 15f31cdca..030dacc23 100644
--- a/pkg/cwhub/helpers_test.go
+++ b/pkg/cwhub/helpers_test.go
@@ -17,7 +17,8 @@ func TestUpgradeItemNewScenarioInCollection(t *testing.T) {
 	require.False(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Downloaded)
 	require.False(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Installed)
 
-	require.NoError(t, hub.InstallItem("crowdsecurity/test_collection", COLLECTIONS, false, false))
+	item := hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")
+	require.NoError(t, item.Install(false, false))
 
 	require.True(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Downloaded)
 	require.True(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Installed)
@@ -25,8 +26,7 @@ func TestUpgradeItemNewScenarioInCollection(t *testing.T) {
 	require.False(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Tainted)
 
 	// This is the scenario that gets added in next version of collection
-	require.False(t, hub.Items[SCENARIOS]["crowdsecurity/barfoo_scenario"].Downloaded)
-	require.False(t, hub.Items[SCENARIOS]["crowdsecurity/barfoo_scenario"].Installed)
+	require.Nil(t, hub.Items[SCENARIOS]["crowdsecurity/barfoo_scenario"])
 
 	assertCollectionDepsInstalled(t, "crowdsecurity/test_collection")
 
@@ -49,7 +49,8 @@ func TestUpgradeItemNewScenarioInCollection(t *testing.T) {
 	require.False(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].UpToDate)
 	require.False(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Tainted)
 
-	didUpdate, err := hub.UpgradeItem(COLLECTIONS, "crowdsecurity/test_collection", false)
+	item = hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")
+	didUpdate, err := item.Upgrade(false)
 	require.NoError(t, err)
 	require.True(t, didUpdate)
 	assertCollectionDepsInstalled(t, "crowdsecurity/test_collection")
@@ -68,7 +69,8 @@ func TestUpgradeItemInDisabledScenarioShouldNotBeInstalled(t *testing.T) {
 	require.False(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Installed)
 	require.False(t, hub.Items[SCENARIOS]["crowdsecurity/foobar_scenario"].Installed)
 
-	require.NoError(t, hub.InstallItem("crowdsecurity/test_collection", COLLECTIONS, false, false))
+	item := hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")
+	require.NoError(t, item.Install(false, false))
 
 	require.True(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Downloaded)
 	require.True(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Installed)
@@ -77,7 +79,8 @@ func TestUpgradeItemInDisabledScenarioShouldNotBeInstalled(t *testing.T) {
 	require.True(t, hub.Items[SCENARIOS]["crowdsecurity/foobar_scenario"].Installed)
 	assertCollectionDepsInstalled(t, "crowdsecurity/test_collection")
 
-	didRemove, err := hub.RemoveItem(SCENARIOS, "crowdsecurity/foobar_scenario", false, false)
+	item = hub.GetItem(SCENARIOS, "crowdsecurity/foobar_scenario")
+	didRemove, err := item.Remove(false, false)
 	require.NoError(t, err)
 	require.True(t, didRemove)
 
@@ -98,7 +101,8 @@ func TestUpgradeItemInDisabledScenarioShouldNotBeInstalled(t *testing.T) {
 	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)
+	item = hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")
+	didUpdate, err := item.Upgrade(false)
 	require.NoError(t, err)
 	require.False(t, didUpdate)
 
@@ -125,7 +129,8 @@ func TestUpgradeItemNewScenarioIsInstalledWhenReferencedScenarioIsDisabled(t *te
 	require.False(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Installed)
 	require.False(t, hub.Items[SCENARIOS]["crowdsecurity/foobar_scenario"].Installed)
 
-	require.NoError(t, hub.InstallItem("crowdsecurity/test_collection", COLLECTIONS, false, false))
+	item := hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")
+	require.NoError(t, item.Install(false, false))
 
 	require.True(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Downloaded)
 	require.True(t, hub.Items[COLLECTIONS]["crowdsecurity/test_collection"].Installed)
@@ -134,7 +139,8 @@ func TestUpgradeItemNewScenarioIsInstalledWhenReferencedScenarioIsDisabled(t *te
 	require.True(t, hub.Items[SCENARIOS]["crowdsecurity/foobar_scenario"].Installed)
 	assertCollectionDepsInstalled(t, "crowdsecurity/test_collection")
 
-	didRemove, err := hub.RemoveItem(SCENARIOS, "crowdsecurity/foobar_scenario", false, false)
+	item = hub.GetItem(SCENARIOS, "crowdsecurity/foobar_scenario")
+	didRemove, err := item.Remove(false, false)
 	require.NoError(t, err)
 	require.True(t, didRemove)
 
@@ -164,7 +170,8 @@ func TestUpgradeItemNewScenarioIsInstalledWhenReferencedScenarioIsDisabled(t *te
 	require.False(t, hub.Items[SCENARIOS]["crowdsecurity/foobar_scenario"].Installed)
 	hub = getHubOrFail(t, hub.local, remote)
 
-	didUpdate, err := hub.UpgradeItem(COLLECTIONS, "crowdsecurity/test_collection", false)
+	item = hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")
+	didUpdate, err := item.Upgrade(false)
 	require.NoError(t, err)
 	require.True(t, didUpdate)
 
@@ -180,7 +187,7 @@ func assertCollectionDepsInstalled(t *testing.T, collection string) {
 	require.NoError(t, err)
 
 	c := hub.Items[COLLECTIONS][collection]
-	require.NoError(t, hub.checkSubItems(&c))
+	require.NoError(t, hub.checkSubItems(c))
 }
 
 func pushUpdateToCollectionInHub() {
diff --git a/pkg/cwhub/hub.go b/pkg/cwhub/hub.go
index f2b6c4b88..2d4d7bc85 100644
--- a/pkg/cwhub/hub.go
+++ b/pkg/cwhub/hub.go
@@ -82,6 +82,7 @@ func (h *Hub) parseIndex() error {
 		log.Tracef("%s: %d items", itemType, len(h.Items[itemType]))
 
 		for name, item := range h.Items[itemType] {
+			item.hub = h
 			item.Name = name
 
 			// if the item has no (redundant) author, take it from the json key
@@ -92,7 +93,6 @@ func (h *Hub) parseIndex() error {
 			item.Type = itemType
 			x := strings.Split(item.RemotePath, "/")
 			item.FileName = x[len(x)-1]
-			h.Items[itemType][name] = item
 
 			// if it's a collection, check its sub-items are present
 			// XXX should be done later, maybe report all missing at once?
diff --git a/pkg/cwhub/items.go b/pkg/cwhub/items.go
index 53ccd5414..bbd2864e0 100644
--- a/pkg/cwhub/items.go
+++ b/pkg/cwhub/items.go
@@ -26,7 +26,7 @@ const (
 // The order is important, as it is used to range over sub-items in collections
 var ItemTypes = []string{PARSERS, POSTOVERFLOWS, SCENARIOS, COLLECTIONS}
 
-type HubItems map[string]map[string]Item
+type HubItems map[string]map[string]*Item
 
 // ItemVersion is used to detect the version of a given item
 // by comparing the hash of each version to the local file.
@@ -38,6 +38,9 @@ type ItemVersion struct {
 
 // Item represents an object managed in the hub. It can be a parser, scenario, collection..
 type Item struct {
+	// back pointer to the hub, to retrieve subitems and call install/remove methods
+	hub *Hub
+
 	// descriptive info
 	Type                 string   `json:"type,omitempty"                   yaml:"type,omitempty"`                   // can be any of the ItemTypes
 	Stage                string   `json:"stage,omitempty"                  yaml:"stage,omitempty"`                  // Stage for parser|postoverflow: s00-raw/s01-...
@@ -220,23 +223,13 @@ func (i *Item) validPath(dirName, fileName string) bool {
 }
 
 // GetItemMap returns the map of items for a given type
-func (h *Hub) GetItemMap(itemType string) map[string]Item {
-	m, ok := h.Items[itemType]
-	if !ok {
-		return nil
-	}
-
-	return m
+func (h *Hub) GetItemMap(itemType string) map[string]*Item {
+	return h.Items[itemType]
 }
 
 // GetItem returns the item from hub based on its type and full name (author/name)
 func (h *Hub) GetItem(itemType string, itemName string) *Item {
-	m, ok := h.GetItemMap(itemType)[itemName]
-	if !ok {
-		return nil
-	}
-
-	return &m
+	return h.GetItemMap(itemType)[itemName]
 }
 
 // GetItemNames returns the list of item (full) names for a given type
@@ -256,27 +249,14 @@ func (h *Hub) GetItemNames(itemType string) []string {
 	return names
 }
 
-// AddItem adds an item to the hub index
-func (h *Hub) AddItem(item Item) error {
-	for _, t := range ItemTypes {
-		if t == item.Type {
-			h.Items[t][item.Name] = item
-			return nil
-		}
-	}
-
-	// XXX: can this happen?
-	return fmt.Errorf("ItemType %s is unknown", item.Type)
-}
-
 // GetInstalledItems returns the list of installed items
-func (h *Hub) GetInstalledItems(itemType string) ([]Item, error) {
+func (h *Hub) GetInstalledItems(itemType string) ([]*Item, error) {
 	items, ok := h.Items[itemType]
 	if !ok {
 		return nil, fmt.Errorf("no %s in the hub index", itemType)
 	}
 
-	retItems := make([]Item, 0)
+	retItems := make([]*Item, 0)
 
 	for _, item := range items {
 		if item.Installed {
@@ -296,8 +276,8 @@ func (h *Hub) GetInstalledItemsAsString(itemType string) ([]string, error) {
 
 	retStr := make([]string, len(items))
 
-	for i, it := range items {
-		retStr[i] = it.Name
+	for idx, it := range items {
+		retStr[idx] = it.Name
 	}
 
 	return retStr, nil
diff --git a/pkg/cwhub/items_test.go b/pkg/cwhub/items_test.go
index 430d6a574..ed11d80ae 100644
--- a/pkg/cwhub/items_test.go
+++ b/pkg/cwhub/items_test.go
@@ -4,8 +4,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-
-	"github.com/crowdsecurity/go-cs-lib/cstest"
 )
 
 func TestItemStatus(t *testing.T) {
@@ -62,14 +60,9 @@ func TestGetters(t *testing.T) {
 
 		// Add item and get it
 		item.Name += "nope"
-		err := hub.AddItem(*item)
-		require.NoError(t, err)
+		hub.Items[item.Type][item.Name] = item
 
 		newitem := hub.GetItem(COLLECTIONS, item.Name)
 		require.NotNil(t, newitem)
-
-		item.Type = "ratata"
-		err = hub.AddItem(*item)
-		cstest.RequireErrorContains(t, err, "ItemType ratata is unknown")
 	}
 }
diff --git a/pkg/cwhub/sync.go b/pkg/cwhub/sync.go
index e9ad1b41c..e9855027f 100644
--- a/pkg/cwhub/sync.go
+++ b/pkg/cwhub/sync.go
@@ -129,20 +129,20 @@ func (h *Hub) getItemInfo(path string) (itemFileInfo, bool, error) {
 // sortedVersions returns the input data, sorted in reverse order by semver
 func sortedVersions(raw []string) ([]string, error) {
 	vs := make([]*semver.Version, len(raw))
-	for i, r := range raw {
+	for idx, r := range raw {
 		v, err := semver.NewVersion(r)
 		if err != nil {
 			return nil, fmt.Errorf("%s: %w", r, err)
 		}
 
-		vs[i] = v
+		vs[idx] = v
 	}
 
 	sort.Sort(sort.Reverse(semver.Collection(vs)))
 
 	ret := make([]string, len(vs))
-	for i, v := range vs {
-		ret[i] = v.Original()
+	for idx, v := range vs {
+		ret[idx] = v.Original()
 	}
 
 	return ret, nil
@@ -209,7 +209,8 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 
 		_, fileName := filepath.Split(path)
 
-		h.Items[info.ftype][info.fname] = Item{
+		h.Items[info.ftype][info.fname] = &Item{
+			hub:       h,
 			Name:      info.fname,
 			Stage:     info.stage,
 			Installed: true,
@@ -360,7 +361,11 @@ func (h *Hub) checkSubItems(v *Item) error {
 			continue
 		}
 
-		if err := h.checkSubItems(&subItem); err != nil {
+		if err := h.checkSubItems(subItem); err != nil {
+			if subItem.Tainted {
+				v.Tainted = true
+			}
+
 			return fmt.Errorf("sub collection %s is broken: %w", subItem.Name, err)
 		}
 
@@ -383,8 +388,6 @@ func (h *Hub) checkSubItems(v *Item) error {
 			subItem.BelongsToCollections = append(subItem.BelongsToCollections, v.Name)
 		}
 
-		h.Items[sub.Type][sub.Name] = subItem
-
 		log.Tracef("checking for %s - tainted:%t uptodate:%t", sub.Name, v.Tainted, v.UpToDate)
 	}
 
@@ -412,7 +415,7 @@ func (h *Hub) syncDir(dir string) ([]string, error) {
 		}
 	}
 
-	for name, item := range h.Items[COLLECTIONS] {
+	for _, item := range h.Items[COLLECTIONS] {
 		if !item.Installed {
 			continue
 		}
@@ -420,9 +423,8 @@ func (h *Hub) syncDir(dir string) ([]string, error) {
 		vs := item.versionStatus()
 		switch vs {
 		case VersionUpToDate: // latest
-			if err := h.checkSubItems(&item); err != nil {
+			if err := h.checkSubItems(item); err != nil {
 				warnings = append(warnings, fmt.Sprintf("dependency of %s: %s", item.Name, err))
-				h.Items[COLLECTIONS][name] = item
 			}
 		case VersionUpdateAvailable: // not up-to-date
 			warnings = append(warnings, fmt.Sprintf("update for collection %s available (currently:%s, latest:%s)", item.Name, item.LocalVersion, item.Version))
diff --git a/pkg/hubtest/hubtest_item.go b/pkg/hubtest/hubtest_item.go
index 329e74a5d..82107c6fe 100644
--- a/pkg/hubtest/hubtest_item.go
+++ b/pkg/hubtest/hubtest_item.go
@@ -410,7 +410,7 @@ func (t *HubTestItem) InstallHub() error {
 	ret := hub.GetItemMap(cwhub.PARSERS)
 	for parserName, item := range ret {
 		if item.Installed {
-			if err := hub.DownloadDataIfNeeded(item, true); err != nil {
+			if err := item.DownloadDataIfNeeded(true); err != nil {
 				return fmt.Errorf("unable to download data for parser '%s': %+v", parserName, err)
 			}
 
@@ -422,7 +422,7 @@ func (t *HubTestItem) InstallHub() error {
 	ret = hub.GetItemMap(cwhub.SCENARIOS)
 	for scenarioName, item := range ret {
 		if item.Installed {
-			if err := hub.DownloadDataIfNeeded(item, true); err != nil {
+			if err := item.DownloadDataIfNeeded(true); err != nil {
 				return fmt.Errorf("unable to download data for parser '%s': %+v", scenarioName, err)
 			}
 
@@ -434,7 +434,7 @@ func (t *HubTestItem) InstallHub() error {
 	ret = hub.GetItemMap(cwhub.POSTOVERFLOWS)
 	for postoverflowName, item := range ret {
 		if item.Installed {
-			if err := hub.DownloadDataIfNeeded(item, true); err != nil {
+			if err := item.DownloadDataIfNeeded(true); err != nil {
 				return fmt.Errorf("unable to download data for parser '%s': %+v", postoverflowName, err)
 			}
 
diff --git a/pkg/setup/install.go b/pkg/setup/install.go
index 50d16b523..fc922c5d1 100644
--- a/pkg/setup/install.go
+++ b/pkg/setup/install.go
@@ -62,14 +62,19 @@ func InstallHubItems(hub *cwhub.Hub, input []byte, dryRun bool) error {
 
 		if len(install.Collections) > 0 {
 			for _, collection := range setupItem.Install.Collections {
+				item := hub.GetItem(cwhub.COLLECTIONS, collection)
+				if item == nil {
+					return fmt.Errorf("collection %s not found", collection)
+				}
+
 				if dryRun {
 					fmt.Println("dry-run: would install collection", collection)
 
 					continue
 				}
 
-				if err := hub.InstallItem(collection, cwhub.COLLECTIONS, forceAction, downloadOnly); err != nil {
-					return fmt.Errorf("while installing collection %s: %w", collection, err)
+				if err := item.Install(forceAction, downloadOnly); err != nil {
+					return fmt.Errorf("while installing collection %s: %w", item.Name, err)
 				}
 			}
 		}
@@ -82,8 +87,13 @@ func InstallHubItems(hub *cwhub.Hub, input []byte, dryRun bool) error {
 					continue
 				}
 
-				if err := hub.InstallItem(parser, cwhub.PARSERS, forceAction, downloadOnly); err != nil {
-					return fmt.Errorf("while installing parser %s: %w", parser, err)
+				item := hub.GetItem(cwhub.PARSERS, parser)
+				if item == nil {
+					return fmt.Errorf("parser %s not found", parser)
+				}
+
+				if err := item.Install(forceAction, downloadOnly); err != nil {
+					return fmt.Errorf("while installing parser %s: %w", item.Name, err)
 				}
 			}
 		}
@@ -96,8 +106,13 @@ func InstallHubItems(hub *cwhub.Hub, input []byte, dryRun bool) error {
 					continue
 				}
 
-				if err := hub.InstallItem(scenario, cwhub.SCENARIOS, forceAction, downloadOnly); err != nil {
-					return fmt.Errorf("while installing scenario %s: %w", scenario, err)
+				item := hub.GetItem(cwhub.SCENARIOS, scenario)
+				if item == nil {
+					return fmt.Errorf("scenario %s not found", scenario)
+				}
+
+				if err := item.Install(forceAction, downloadOnly); err != nil {
+					return fmt.Errorf("while installing scenario %s: %w", item.Name, err)
 				}
 			}
 		}
@@ -110,8 +125,13 @@ func InstallHubItems(hub *cwhub.Hub, input []byte, dryRun bool) error {
 					continue
 				}
 
-				if err := hub.InstallItem(postoverflow, cwhub.POSTOVERFLOWS, forceAction, downloadOnly); err != nil {
-					return fmt.Errorf("while installing postoverflow %s: %w", postoverflow, err)
+				item := hub.GetItem(cwhub.POSTOVERFLOWS, postoverflow)
+				if item == nil {
+					return fmt.Errorf("postoverflow %s not found", postoverflow)
+				}
+
+				if err := item.Install(forceAction, downloadOnly); err != nil {
+					return fmt.Errorf("while installing postoverflow %s: %w", item.Name, err)
 				}
 			}
 		}
diff --git a/test/bats/07_setup.bats b/test/bats/07_setup.bats
index 9d6b32d15..1748d8040 100644
--- a/test/bats/07_setup.bats
+++ b/test/bats/07_setup.bats
@@ -519,6 +519,11 @@ update-notifier-motd.timer              enabled enabled
     rune -0 cscli collections list -o json
     rune -0 jq -r '.collections[].name' <(output)
     refute_line "crowdsecurity/apache2"
+
+    # same with dependencies
+    rune -0 cscli collections remove --all
+    rune -0 cscli setup install-hub /dev/stdin --dry-run <<< '{"setup":[{"install":{"collections":["crowdsecurity/linux"]}}]}'
+    assert_output 'dry-run: would install collection crowdsecurity/linux'
 }
 
 @test "cscli setup install-hub (dry run: install multiple collections)" {
@@ -538,15 +543,17 @@ update-notifier-motd.timer              enabled enabled
 }
 
 @test "cscli setup install-hub (dry run: install multiple collections, parsers, scenarios, postoverflows)" {
-    rune -0 cscli setup install-hub /dev/stdin --dry-run <<< '{"setup":[{"install":{"collections":["crowdsecurity/foo","johndoe/bar"],"parsers":["crowdsecurity/fooparser","johndoe/barparser"],"scenarios":["crowdsecurity/fooscenario","johndoe/barscenario"],"postoverflows":["crowdsecurity/foopo","johndoe/barpo"]}}]}'
-    assert_line 'dry-run: would install collection crowdsecurity/foo'
-    assert_line 'dry-run: would install collection johndoe/bar'
-    assert_line 'dry-run: would install parser crowdsecurity/fooparser'
-    assert_line 'dry-run: would install parser johndoe/barparser'
-    assert_line 'dry-run: would install scenario crowdsecurity/fooscenario'
-    assert_line 'dry-run: would install scenario johndoe/barscenario'
-    assert_line 'dry-run: would install postoverflow crowdsecurity/foopo'
-    assert_line 'dry-run: would install postoverflow johndoe/barpo'
+    rune -0 cscli setup install-hub /dev/stdin --dry-run <<< '{"setup":[{"install":{"collections":["crowdsecurity/aws-console","crowdsecurity/caddy"],"parsers":["crowdsecurity/asterisk-logs"],"scenarios":["crowdsecurity/smb-fs"],"postoverflows":["crowdsecurity/cdn-whitelist","crowdsecurity/rdns"]}}]}'
+    assert_line 'dry-run: would install collection crowdsecurity/aws-console'
+    assert_line 'dry-run: would install collection crowdsecurity/caddy'
+    assert_line 'dry-run: would install parser crowdsecurity/asterisk-logs'
+    assert_line 'dry-run: would install scenario crowdsecurity/smb-fs'
+    assert_line 'dry-run: would install postoverflow crowdsecurity/cdn-whitelist'
+    assert_line 'dry-run: would install postoverflow crowdsecurity/rdns'
+
+    rune -1 cscli setup install-hub /dev/stdin --dry-run <<< '{"setup":[{"install":{"collections":["crowdsecurity/foo"]}}]}'
+    assert_stderr --partial 'collection crowdsecurity/foo not found'
+
 }
 
 @test "cscli setup datasources" {