pkg/cwhub: refact Item.State.(Downloaded | Installed) (#3476)

cmd/crowdsec-cli/clihub/items.go

@@ -43,7 +43,7 @@ func SelectItems(hub *cwhub.Hub, itemType string, args []string, installedOnly b
 
 	for _, itemName := range itemNames {
 		item := hub.GetItem(itemType, itemName)
-		if installedOnly && !item.State.Installed {
+		if installedOnly && !item.State.IsInstalled() {
 			continue
 		}
 

cmd/crowdsec-cli/cliitem/cmdinspect.go

@@ -71,7 +71,7 @@ func (cli cliItem) inspect(ctx context.Context, args []string, url string, diff
 
 // return the diff between the installed version and the latest version
 func (cli cliItem) itemDiff(ctx context.Context, item *cwhub.Item, contentProvider cwhub.ContentProvider, reverse bool) (string, error) {
-	if !item.State.Installed {
+	if !item.State.IsInstalled() {
 		return "", fmt.Errorf("'%s' is not installed", item.FQName())
 	}
 
@@ -113,7 +113,7 @@ func (cli cliItem) itemDiff(ctx context.Context, item *cwhub.Item, contentProvid
 }
 
 func (cli cliItem) whyTainted(ctx context.Context, hub *cwhub.Hub, contentProvider cwhub.ContentProvider, item *cwhub.Item, reverse bool) string {
-	if !item.State.Installed {
+	if !item.State.IsInstalled() {
 		return fmt.Sprintf("# %s is not installed", item.FQName())
 	}
 
@@ -203,7 +203,7 @@ func inspectItem(hub *cwhub.Hub, item *cwhub.Item, wantMetrics bool, output stri
 		enc.SetIndent(2)
 
 		if err := enc.Encode(item); err != nil {
-			return fmt.Errorf("unable to encode item: %w", err)
+			return fmt.Errorf("unable to serialize item: %w", err)
 		}
 	case "json":
 		b, err := json.MarshalIndent(*item, "", "  ")

cmd/crowdsec-cli/cliitem/cmdremove.go

@@ -77,7 +77,7 @@ func installedParentNames(item *cwhub.Item) []string {
 	ret := make([]string, 0)
 
 	for _, parent := range item.Ancestors() {
-		if parent.State.Installed {
+		if parent.State.IsInstalled() {
 			ret = append(ret, parent.Name)
 		}
 	}

cmd/crowdsec-cli/clisimulation/simulation.go

@@ -83,7 +83,7 @@ func (cli *cliSimulation) newEnableCmd() *cobra.Command {
 						log.Errorf("'%s' doesn't exist or is not a scenario", scenario)
 						continue
 					}
-					if !item.State.Installed {
+					if !item.State.IsInstalled() {
 						log.Warningf("'%s' isn't enabled", scenario)
 					}
 					isExcluded := slices.Contains(cli.cfg().Cscli.SimulationConfig.Exclusions, scenario)

pkg/appsec/appsec.go

@@ -40,6 +40,7 @@ const (
 
 func (h *Hook) Build(hookStage int) error {
 	ctx := map[string]interface{}{}
+
 	switch hookStage {
 	case hookOnLoad:
 		ctx = GetOnLoadEnv(&AppsecRuntimeConfig{})
@@ -50,21 +51,26 @@ func (h *Hook) Build(hookStage int) error {
 	case hookOnMatch:
 		ctx = GetOnMatchEnv(&AppsecRuntimeConfig{}, &ParsedRequest{}, types.Event{})
 	}
+
 	opts := exprhelpers.GetExprOptions(ctx)
 	if h.Filter != "" {
 		program, err := expr.Compile(h.Filter, opts...) // FIXME: opts
 		if err != nil {
 			return fmt.Errorf("unable to compile filter %s : %w", h.Filter, err)
 		}
+
 		h.FilterExpr = program
 	}
+
 	for _, apply := range h.Apply {
 		program, err := expr.Compile(apply, opts...)
 		if err != nil {
 			return fmt.Errorf("unable to compile apply %s : %w", apply, err)
 		}
+
 		h.ApplyExpr = append(h.ApplyExpr, program)
 	}
+
 	return nil
 }
 
@@ -165,45 +171,51 @@ func (wc *AppsecConfig) LoadByPath(file string) error {
 
 	yamlFile, err := os.ReadFile(file)
 	if err != nil {
-		return fmt.Errorf("unable to read file %s : %s", file, err)
+		return fmt.Errorf("unable to read file %s : %w", file, err)
 	}
 
-	//as  LoadByPath can be called several time, we append rules/hooks, but override other options
+	// as  LoadByPath can be called several time, we append rules/hooks, but override other options
 	var tmp AppsecConfig
 
 	err = yaml.UnmarshalStrict(yamlFile, &tmp)
 	if err != nil {
-		return fmt.Errorf("unable to parse yaml file %s : %s", file, err)
+		return fmt.Errorf("unable to parse yaml file %s : %w", file, err)
 	}
 
 	if wc.Name == "" && tmp.Name != "" {
 		wc.Name = tmp.Name
 	}
 
-	//We can append rules/hooks
+	// We can append rules/hooks
 	if tmp.OutOfBandRules != nil {
 		wc.OutOfBandRules = append(wc.OutOfBandRules, tmp.OutOfBandRules...)
 	}
+
 	if tmp.InBandRules != nil {
 		wc.InBandRules = append(wc.InBandRules, tmp.InBandRules...)
 	}
+
 	if tmp.OnLoad != nil {
 		wc.OnLoad = append(wc.OnLoad, tmp.OnLoad...)
 	}
+
 	if tmp.PreEval != nil {
 		wc.PreEval = append(wc.PreEval, tmp.PreEval...)
 	}
+
 	if tmp.PostEval != nil {
 		wc.PostEval = append(wc.PostEval, tmp.PostEval...)
 	}
+
 	if tmp.OnMatch != nil {
 		wc.OnMatch = append(wc.OnMatch, tmp.OnMatch...)
 	}
+
 	if tmp.VariablesTracking != nil {
 		wc.VariablesTracking = append(wc.VariablesTracking, tmp.VariablesTracking...)
 	}
 
-	//override other options
+	// override other options
 	wc.LogLevel = tmp.LogLevel
 
 	wc.DefaultRemediation = tmp.DefaultRemediation
@@ -216,12 +228,15 @@ func (wc *AppsecConfig) LoadByPath(file string) error {
 	if tmp.InbandOptions.DisableBodyInspection {
 		wc.InbandOptions.DisableBodyInspection = true
 	}
+
 	if tmp.InbandOptions.RequestBodyInMemoryLimit != nil {
 		wc.InbandOptions.RequestBodyInMemoryLimit = tmp.InbandOptions.RequestBodyInMemoryLimit
 	}
+
 	if tmp.OutOfBandOptions.DisableBodyInspection {
 		wc.OutOfBandOptions.DisableBodyInspection = true
 	}
+
 	if tmp.OutOfBandOptions.RequestBodyInMemoryLimit != nil {
 		wc.OutOfBandOptions.RequestBodyInMemoryLimit = tmp.OutOfBandOptions.RequestBodyInMemoryLimit
 	}
@@ -232,12 +247,14 @@ func (wc *AppsecConfig) LoadByPath(file string) error {
 func (wc *AppsecConfig) Load(configName string) error {
 	item := hub.GetItem(cwhub.APPSEC_CONFIGS, configName)
 
-	if item != nil && item.State.Installed {
+	if item != nil && item.State.IsInstalled() {
 		wc.Logger.Infof("loading %s", item.State.LocalPath)
+
 		err := wc.LoadByPath(item.State.LocalPath)
 		if err != nil {
 			return fmt.Errorf("unable to load appsec-config %s : %s", item.State.LocalPath, err)
 		}
+
 		return nil
 	}
 
@@ -254,6 +271,7 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) {
 	if wc.BouncerBlockedHTTPCode == 0 {
 		wc.BouncerBlockedHTTPCode = http.StatusForbidden
 	}
+
 	if wc.BouncerPassedHTTPCode == 0 {
 		wc.BouncerPassedHTTPCode = http.StatusOK
 	}
@@ -261,12 +279,15 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) {
 	if wc.UserBlockedHTTPCode == 0 {
 		wc.UserBlockedHTTPCode = http.StatusForbidden
 	}
+
 	if wc.UserPassedHTTPCode == 0 {
 		wc.UserPassedHTTPCode = http.StatusOK
 	}
+
 	if wc.DefaultPassAction == "" {
 		wc.DefaultPassAction = AllowRemediation
 	}
+
 	if wc.DefaultRemediation == "" {
 		wc.DefaultRemediation = BanRemediation
 	}
@@ -287,20 +308,25 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) {
 	// load rules
 	for _, rule := range wc.OutOfBandRules {
 		wc.Logger.Infof("loading outofband rule %s", rule)
+
 		collections, err := LoadCollection(rule, wc.Logger.WithField("component", "appsec_collection_loader"))
 		if err != nil {
 			return nil, fmt.Errorf("unable to load outofband rule %s : %s", rule, err)
 		}
+
 		ret.OutOfBandRules = append(ret.OutOfBandRules, collections...)
 	}
 
 	wc.Logger.Infof("Loaded %d outofband rules", len(ret.OutOfBandRules))
+
 	for _, rule := range wc.InBandRules {
 		wc.Logger.Infof("loading inband rule %s", rule)
+
 		collections, err := LoadCollection(rule, wc.Logger.WithField("component", "appsec_collection_loader"))
 		if err != nil {
 			return nil, fmt.Errorf("unable to load inband rule %s : %s", rule, err)
 		}
+
 		ret.InBandRules = append(ret.InBandRules, collections...)
 	}
 
@@ -311,10 +337,12 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) {
 		if hook.OnSuccess != "" && hook.OnSuccess != "continue" && hook.OnSuccess != "break" {
 			return nil, fmt.Errorf("invalid 'on_success' for on_load hook : %s", hook.OnSuccess)
 		}
+
 		err := hook.Build(hookOnLoad)
 		if err != nil {
 			return nil, fmt.Errorf("unable to build on_load hook : %s", err)
 		}
+
 		ret.CompiledOnLoad = append(ret.CompiledOnLoad, hook)
 	}
 
@@ -322,10 +350,12 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) {
 		if hook.OnSuccess != "" && hook.OnSuccess != "continue" && hook.OnSuccess != "break" {
 			return nil, fmt.Errorf("invalid 'on_success' for pre_eval hook : %s", hook.OnSuccess)
 		}
+
 		err := hook.Build(hookPreEval)
 		if err != nil {
 			return nil, fmt.Errorf("unable to build pre_eval hook : %s", err)
 		}
+
 		ret.CompiledPreEval = append(ret.CompiledPreEval, hook)
 	}
 
@@ -333,10 +363,12 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) {
 		if hook.OnSuccess != "" && hook.OnSuccess != "continue" && hook.OnSuccess != "break" {
 			return nil, fmt.Errorf("invalid 'on_success' for post_eval hook : %s", hook.OnSuccess)
 		}
+
 		err := hook.Build(hookPostEval)
 		if err != nil {
 			return nil, fmt.Errorf("unable to build post_eval hook : %s", err)
 		}
+
 		ret.CompiledPostEval = append(ret.CompiledPostEval, hook)
 	}
 
@@ -344,10 +376,12 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) {
 		if hook.OnSuccess != "" && hook.OnSuccess != "continue" && hook.OnSuccess != "break" {
 			return nil, fmt.Errorf("invalid 'on_success' for on_match hook : %s", hook.OnSuccess)
 		}
+
 		err := hook.Build(hookOnMatch)
 		if err != nil {
 			return nil, fmt.Errorf("unable to build on_match hook : %s", err)
 		}
+
 		ret.CompiledOnMatch = append(ret.CompiledOnMatch, hook)
 	}
 
@@ -357,19 +391,23 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) {
 		if err != nil {
 			return nil, fmt.Errorf("cannot compile variable regexp %s: %w", variable, err)
 		}
+
 		ret.CompiledVariablesTracking = append(ret.CompiledVariablesTracking, compiledVariableRule)
 	}
+
 	return ret, nil
 }
 
 func (w *AppsecRuntimeConfig) ProcessOnLoadRules() error {
 	has_match := false
+
 	for _, rule := range w.CompiledOnLoad {
 		if rule.FilterExpr != nil {
 			output, err := exprhelpers.Run(rule.FilterExpr, GetOnLoadEnv(w), w.Logger, w.Logger.Level >= log.DebugLevel)
 			if err != nil {
 				return fmt.Errorf("unable to run appsec on_load filter %s : %w", rule.Filter, err)
 			}
+
 			switch t := output.(type) {
 			case bool:
 				if !t {
@@ -380,14 +418,17 @@ func (w *AppsecRuntimeConfig) ProcessOnLoadRules() error {
 				w.Logger.Errorf("Filter must return a boolean, can't filter")
 				continue
 			}
+
 			has_match = true
 		}
+
 		for _, applyExpr := range rule.ApplyExpr {
 			o, err := exprhelpers.Run(applyExpr, GetOnLoadEnv(w), w.Logger, w.Logger.Level >= log.DebugLevel)
 			if err != nil {
 				w.Logger.Errorf("unable to apply appsec on_load expr: %s", err)
 				continue
 			}
+
 			switch t := o.(type) {
 			case error:
 				w.Logger.Errorf("unable to apply appsec on_load expr: %s", t)
@@ -395,21 +436,25 @@ func (w *AppsecRuntimeConfig) ProcessOnLoadRules() error {
 			default:
 			}
 		}
+
 		if has_match && rule.OnSuccess == "break" {
 			break
 		}
 	}
+
 	return nil
 }
 
 func (w *AppsecRuntimeConfig) ProcessOnMatchRules(request *ParsedRequest, evt types.Event) error {
 	has_match := false
+
 	for _, rule := range w.CompiledOnMatch {
 		if rule.FilterExpr != nil {
 			output, err := exprhelpers.Run(rule.FilterExpr, GetOnMatchEnv(w, request, evt), w.Logger, w.Logger.Level >= log.DebugLevel)
 			if err != nil {
 				return fmt.Errorf("unable to run appsec on_match filter %s : %w", rule.Filter, err)
 			}
+
 			switch t := output.(type) {
 			case bool:
 				if !t {
@@ -420,14 +465,17 @@ func (w *AppsecRuntimeConfig) ProcessOnMatchRules(request *ParsedRequest, evt ty
 				w.Logger.Errorf("Filter must return a boolean, can't filter")
 				continue
 			}
+
 			has_match = true
 		}
+
 		for _, applyExpr := range rule.ApplyExpr {
 			o, err := exprhelpers.Run(applyExpr, GetOnMatchEnv(w, request, evt), w.Logger, w.Logger.Level >= log.DebugLevel)
 			if err != nil {
 				w.Logger.Errorf("unable to apply appsec on_match expr: %s", err)
 				continue
 			}
+
 			switch t := o.(type) {
 			case error:
 				w.Logger.Errorf("unable to apply appsec on_match expr: %s", t)
@@ -435,21 +483,25 @@ func (w *AppsecRuntimeConfig) ProcessOnMatchRules(request *ParsedRequest, evt ty
 			default:
 			}
 		}
+
 		if has_match && rule.OnSuccess == "break" {
 			break
 		}
 	}
+
 	return nil
 }
 
 func (w *AppsecRuntimeConfig) ProcessPreEvalRules(request *ParsedRequest) error {
 	has_match := false
+
 	for _, rule := range w.CompiledPreEval {
 		if rule.FilterExpr != nil {
 			output, err := exprhelpers.Run(rule.FilterExpr, GetPreEvalEnv(w, request), w.Logger, w.Logger.Level >= log.DebugLevel)
 			if err != nil {
 				return fmt.Errorf("unable to run appsec pre_eval filter %s : %w", rule.Filter, err)
 			}
+
 			switch t := output.(type) {
 			case bool:
 				if !t {
@@ -460,6 +512,7 @@ func (w *AppsecRuntimeConfig) ProcessPreEvalRules(request *ParsedRequest) error
 				w.Logger.Errorf("Filter must return a boolean, can't filter")
 				continue
 			}
+
 			has_match = true
 		}
 		// here means there is no filter or the filter matched
@@ -469,6 +522,7 @@ func (w *AppsecRuntimeConfig) ProcessPreEvalRules(request *ParsedRequest) error
 				w.Logger.Errorf("unable to apply appsec pre_eval expr: %s", err)
 				continue
 			}
+
 			switch t := o.(type) {
 			case error:
 				w.Logger.Errorf("unable to apply appsec pre_eval expr: %s", t)
@@ -476,6 +530,7 @@ func (w *AppsecRuntimeConfig) ProcessPreEvalRules(request *ParsedRequest) error
 			default:
 			}
 		}
+
 		if has_match && rule.OnSuccess == "break" {
 			break
 		}
@@ -486,12 +541,14 @@ func (w *AppsecRuntimeConfig) ProcessPreEvalRules(request *ParsedRequest) error
 
 func (w *AppsecRuntimeConfig) ProcessPostEvalRules(request *ParsedRequest) error {
 	has_match := false
+
 	for _, rule := range w.CompiledPostEval {
 		if rule.FilterExpr != nil {
 			output, err := exprhelpers.Run(rule.FilterExpr, GetPostEvalEnv(w, request), w.Logger, w.Logger.Level >= log.DebugLevel)
 			if err != nil {
 				return fmt.Errorf("unable to run appsec post_eval filter %s : %w", rule.Filter, err)
 			}
+
 			switch t := output.(type) {
 			case bool:
 				if !t {
@@ -502,6 +559,7 @@ func (w *AppsecRuntimeConfig) ProcessPostEvalRules(request *ParsedRequest) error
 				w.Logger.Errorf("Filter must return a boolean, can't filter")
 				continue
 			}
+
 			has_match = true
 		}
 		// here means there is no filter or the filter matched
@@ -519,6 +577,7 @@ func (w *AppsecRuntimeConfig) ProcessPostEvalRules(request *ParsedRequest) error
 			default:
 			}
 		}
+
 		if has_match && rule.OnSuccess == "break" {
 			break
 		}
@@ -562,6 +621,7 @@ func (w *AppsecRuntimeConfig) RemoveOutbandRuleByName(name string) error {
 func (w *AppsecRuntimeConfig) CancelEvent() error {
 	w.Logger.Debugf("canceling event")
 	w.Response.SendEvent = false
+
 	return nil
 }
 

pkg/cwhub/fetch.go

@@ -63,8 +63,16 @@ func (i *Item) FetchContentTo(ctx context.Context, contentProvider ContentProvid
 			return false, "", err
 		}
 
+		i.State.DownloadPath = destPath
+
 		return true, fmt.Sprintf("(embedded in %s)", i.hub.local.HubIndexFile), nil
 	}
 
-	return contentProvider.FetchContent(ctx, i.RemotePath, destPath, wantHash, i.hub.logger)
+	downloaded, _, err := contentProvider.FetchContent(ctx, i.RemotePath, destPath, wantHash, i.hub.logger)
+
+	if err == nil && downloaded {
+		i.State.DownloadPath = destPath
+	}
+
+	return downloaded, destPath, err
 }

pkg/cwhub/hub.go

@@ -19,11 +19,10 @@ import (
 
 // Hub is the main structure for the package.
 type Hub struct {
-	items     HubItems // Items read from HubDir and InstallDir
+	items    HubItems // Items read from HubDir and InstallDir
-	pathIndex map[string]*Item
+	local    *csconfig.LocalHubCfg
-	local     *csconfig.LocalHubCfg
+	logger   *logrus.Logger
-	logger    *logrus.Logger
+	Warnings []string // Warnings encountered during sync
-	Warnings  []string // Warnings encountered during sync
 }
 
 // GetDataDir returns the data directory, where data sets are installed.
@@ -45,9 +44,8 @@ func NewHub(local *csconfig.LocalHubCfg, logger *logrus.Logger) (*Hub, error) {
 	}
 
 	hub := &Hub{
-		local:     local,
+		local:  local,
-		logger:    logger,
+		logger: logger,
-		pathIndex: make(map[string]*Item, 0),
 	}
 
 	return hub, nil
@@ -169,7 +167,6 @@ func (h *Hub) addItem(item *Item) {
 	}
 
 	h.items[item.Type][item.Name] = item
-	h.pathIndex[item.State.LocalPath] = item
 }
 
 // GetItemMap returns the map of items for a given type.
@@ -182,11 +179,6 @@ func (h *Hub) GetItem(itemType string, itemName string) *Item {
 	return h.GetItemMap(itemType)[itemName]
 }
 
-// GetItemByPath returns an item from hub based on its (absolute) local path.
-func (h *Hub) GetItemByPath(itemPath string) *Item {
-	return h.pathIndex[itemPath]
-}
-
 // GetItemFQ returns an item from hub based on its type and name (type:author/name).
 func (h *Hub) GetItemFQ(itemFQName string) (*Item, error) {
 	// type and name are separated by a colon
@@ -240,7 +232,7 @@ func (h *Hub) GetInstalledByType(itemType string, sorted bool) []*Item {
 	ret := make([]*Item, 0)
 
 	for _, item := range h.GetItemsByType(itemType, sorted) {
-		if item.State.Installed {
+		if item.State.IsInstalled() {
 			ret = append(ret, item)
 		}
 	}

pkg/cwhub/item.go

@@ -113,10 +113,11 @@ type Item struct {
 	Dependencies
 }
 
-// InstallPath returns the path to use for the install symlink.
+// PathForInstall returns the path to use for the install symlink
+// (eg. /etc/crowdsec/collections/xyz.yaml).
 // Returns an error if an item is already installed or if the path goes outside of the install dir.
-func (i *Item) InstallPath() (string, error) {
+func (i *Item) PathForInstall() (string, error) {
-	if i.State.Installed {
+	if i.State.IsInstalled() {
 		return "", fmt.Errorf("%s is already installed at %s", i.FQName(), i.State.LocalPath)
 	}
 
@@ -128,16 +129,21 @@ func (i *Item) InstallPath() (string, error) {
 	return SafePath(i.hub.local.InstallDir, filepath.Join(p, i.FileName))
 }
 
-// DownloadPath returns the location of the actual config file in the hub
+// PathForDownload returns the path to use to store the item's file from the hub
 // (eg. /etc/crowdsec/hub/collections/author/xyz.yaml).
 // Raises an error if the path goes outside of the hub dir.
-func (i *Item) DownloadPath() (string, error) {
+func (i *Item) PathForDownload() (string, error) {
-	ret, err := SafePath(i.hub.local.HubDir, i.RemotePath)
+	path, err := SafePath(i.hub.local.HubDir, i.RemotePath)
-	if err != nil {
+
-		return "", err
+	if i.State.IsDownloaded() && path != i.State.DownloadPath {
+		// A hub item with the same name is at a different location.
+		// This should not happen.
+		// user is downloading with --force so we are allowed to overwrite but
+		// should we remove the old location from here? Error, warning, more tests?
+		return "", fmt.Errorf("%s is already downloaded at %s", i.FQName(), i.State.DownloadPath)
 	}
 
-	return ret, nil
+	return path, err
 }
 
 // HasSubItems returns true if items of this type can have sub-items. Currently only collections.
@@ -167,8 +173,8 @@ func (i Item) MarshalJSON() ([]byte, error) {
 		LocalPath:            i.State.LocalPath,
 		LocalVersion:         i.State.LocalVersion,
 		LocalHash:            i.State.LocalHash,
-		Installed:            i.State.Installed,
+		Installed:            i.State.IsInstalled(),
-		Downloaded:           i.State.Downloaded,
+		Downloaded:           i.State.IsDownloaded(),
 		UpToDate:             i.State.UpToDate,
 		Tainted:              i.State.Tainted,
 		BelongsToCollections: i.State.BelongsToCollections,
@@ -182,13 +188,15 @@ func (i Item) MarshalYAML() (interface{}, error) {
 	type Alias Item
 
 	return &struct {
-		Alias `yaml:",inline"`
+		Alias     `yaml:",inline"`
-		State ItemState `yaml:",inline"`
+		State     ItemState `yaml:",inline"`
-		Local bool      `yaml:"local"`
+		Installed bool      `yaml:"installed"`
+		Local     bool      `yaml:"local"`
 	}{
-		Alias: Alias(i),
+		Alias:     Alias(i),
-		State: i.State,
+		State:     i.State,
-		Local: i.State.IsLocal(),
+		Installed: i.State.IsInstalled(),
+		Local:     i.State.IsLocal(),
 	}, nil
 }
 
@@ -275,7 +283,7 @@ func (i *Item) SafeToRemoveDeps() ([]*Item, error) {
 		// if the sub depends on a collection that is not a direct or indirect dependency
 		// of the current item, it is not removed
 		for _, subParent := range sub.Ancestors() {
-			if !subParent.State.Installed {
+			if !subParent.State.IsInstalled() {
 				continue
 			}
 

pkg/cwhub/state.go

@@ -7,16 +7,16 @@ import (
 // ItemState is used to keep the local state (i.e. at runtime) of an item.
 // This data is not stored in the index, but is displayed with "cscli ... inspect".
 type ItemState struct {
-	LocalPath            string `json:"local_path,omitempty" yaml:"local_path,omitempty"`
+	// Path to the install link or local file -- keep LocalPath for compatibility
-	LocalVersion         string `json:"local_version,omitempty" yaml:"local_version,omitempty"`
+	LocalPath            string `yaml:"local_path,omitempty"`
-	LocalHash            string `json:"local_hash,omitempty" yaml:"local_hash,omitempty"`
+	LocalVersion         string `yaml:"local_version,omitempty"`
-	Installed            bool   `json:"installed"`
+	LocalHash            string `yaml:"local_hash,omitempty"`
+	DownloadPath         string
 	local                bool
-	Downloaded           bool     `json:"downloaded"`
+	UpToDate             bool     `yaml:"up_to_date"`
-	UpToDate             bool     `json:"up_to_date"`
+	Tainted              bool     `yaml:"tainted"`
-	Tainted              bool     `json:"tainted"`
+	TaintedBy            []string `yaml:"tainted_by,omitempty"`
-	TaintedBy            []string `json:"tainted_by,omitempty" yaml:"tainted_by,omitempty"`
+	BelongsToCollections []string `yaml:"belongs_to_collections,omitempty"`
-	BelongsToCollections []string `json:"belongs_to_collections,omitempty" yaml:"belongs_to_collections,omitempty"`
 }
 
 // IsLocal returns true if the item has been create by a user (not downloaded from the hub).
@@ -28,7 +28,7 @@ func (s *ItemState) IsLocal() bool {
 func (s *ItemState) Text() string {
 	ret := "disabled"
 
-	if s.Installed {
+	if s.IsInstalled() {
 		ret = "enabled"
 	}
 
@@ -50,13 +50,21 @@ func (s *ItemState) Emoji() string {
 	switch {
 	case s.IsLocal():
 		return emoji.House
-	case !s.Installed:
+	case !s.IsInstalled():
 		return emoji.Prohibited
 	case s.Tainted || (!s.UpToDate && !s.IsLocal()):
 		return emoji.Warning
-	case s.Installed:
+	case s.IsInstalled():
 		return emoji.CheckMark
 	default:
 		return emoji.QuestionMark
 	}
 }
+
+func (s *ItemState) IsDownloaded() bool {
+	return s.DownloadPath != ""
+}
+
+func (s *ItemState) IsInstalled() bool {
+	return s.LocalPath != ""
+}

pkg/cwhub/state_test.go

@@ -20,47 +20,47 @@ func TestItemStateText(t *testing.T) {
 	tests := []test{
 		{
 			ItemState{
-				Installed:  true,
+				LocalPath:    "path/to/install",
-				UpToDate:   false,
+				UpToDate:     false,
-				Tainted:    false,
+				Tainted:      false,
-				Downloaded: true,
+				DownloadPath: "path/to/download",
 			},
 			"enabled,update-available",
 			emoji.Warning,
 		}, {
 			ItemState{
-				Installed:  true,
+				LocalPath:    "path/to/install",
-				UpToDate:   true,
+				UpToDate:     true,
-				Tainted:    false,
+				Tainted:      false,
-				Downloaded: true,
+				DownloadPath: "path/to/download",
 			},
 			"enabled",
 			emoji.CheckMark,
 		}, {
 			ItemState{
-				Installed:  true,
+				LocalPath:    "path/to/install",
-				UpToDate:   false,
+				UpToDate:     false,
-				local:      true,
+				local:        true,
-				Tainted:    false,
+				Tainted:      false,
-				Downloaded: false,
+				DownloadPath: "",
 			},
 			"enabled,local",
 			emoji.House,
 		}, {
 			ItemState{
-				Installed:  false,
+				LocalPath:    "",
-				UpToDate:   false,
+				UpToDate:     false,
-				Tainted:    false,
+				Tainted:      false,
-				Downloaded: true,
+				DownloadPath: "path/to/download",
 			},
 			"disabled,update-available",
 			emoji.Prohibited,
 		}, {
 			ItemState{
-				Installed:  true,
+				LocalPath:    "path/to/install",
-				UpToDate:   false,
+				UpToDate:     false,
-				Tainted:    true,
+				Tainted:      true,
-				Downloaded: true,
+				DownloadPath: "path/to/download",
 			},
 			"enabled,tainted",
 			emoji.Warning,

pkg/cwhub/sync.go

@@ -130,7 +130,6 @@ func newInstallItemSpec(path string, subs []string) (*itemSpec, error) {
 	// .../config/postoverflow/stage/file.yaml
 	// .../config/scenarios/scenar.yaml
 	// .../config/collections/linux.yaml //file is empty
-
 	if len(subs) < 2 {
 		return nil, fmt.Errorf("path is too short: %s (%d)", path, len(subs))
 	}
@@ -239,7 +238,6 @@ func newLocalItem(h *Hub, path string, spec *itemSpec) (*Item, error) {
 		State: ItemState{
 			LocalPath: path,
 			local:     true,
-			Installed: true,
 			UpToDate:  true,
 		},
 	}
@@ -331,14 +329,14 @@ func updateNonLocalItem(h *Hub, path string, spec *itemSpec, symlinkTarget strin
 				continue
 			}
 
-			src, err := item.DownloadPath()
+			src, err := item.PathForDownload()
 			if err != nil {
 				return nil, err
 			}
 
 			if spec.path == src {
 				h.logger.Tracef("marking %s as downloaded", item.Name)
-				item.State.Downloaded = true
+				item.State.DownloadPath = src
 			}
 		} else if !hasPathSuffix(symlinkTarget, item.RemotePath) {
 			// wrong file
@@ -389,7 +387,7 @@ func (h *Hub) addItemFromSpec(spec *itemSpec) error {
 		// see if there's another installed item of the same name
 		theOtherItem := h.GetItem(spec.ftype, item.Name)
 		if theOtherItem != nil {
-			if theOtherItem.State.Installed {
+			if theOtherItem.State.IsInstalled() {
 				h.logger.Warnf("multiple %s named %s: ignoring %s", spec.ftype, item.Name, theOtherItem.State.LocalPath)
 			}
 		}
@@ -398,12 +396,10 @@ func (h *Hub) addItemFromSpec(spec *itemSpec) error {
 		if err != nil {
 			return err
 		}
-
-		item.State.LocalPath = spec.path
 	}
 
 	if item == nil {
-		h.logger.Infof("Ignoring file %s of type %s", spec.path, spec.ftype)
+		h.logger.Warningf("Ignoring file %s of type %s", spec.path, spec.ftype)
 		return nil
 	}
 
@@ -426,12 +422,12 @@ func (i *Item) checkSubItemVersions() []string {
 	}
 
 	// ensure all the sub-items are installed, or tag the parent as tainted
-	i.hub.logger.Tracef("checking submembers of %s installed:%t", i.Name, i.State.Installed)
+	i.hub.logger.Tracef("checking submembers of %s installed:%t", i.Name, i.State.IsInstalled())
 
 	for sub := range i.CurrentDependencies().SubItems(i.hub) {
-		i.hub.logger.Tracef("check %s installed:%t", sub.Name, sub.State.Installed)
+		i.hub.logger.Tracef("check %s installed:%t", sub.Name, sub.State.IsInstalled())
 
-		if !i.State.Installed {
+		if !i.State.IsInstalled() {
 			continue
 		}
 
@@ -453,7 +449,7 @@ func (i *Item) checkSubItemVersions() []string {
 			continue
 		}
 
-		if !sub.State.Installed && i.State.Installed {
+		if !sub.State.IsInstalled() && i.State.IsInstalled() {
 			i.addTaint(sub)
 			warn = append(warn, fmt.Sprintf("%s is tainted by missing %s", i.Name, sub.FQName()))
 
@@ -588,7 +584,7 @@ func (h *Hub) localSync() error {
 			sub.State.BelongsToCollections = insertInOrderNoCase(sub.State.BelongsToCollections, item.Name)
 		}
 
-		if !item.State.Installed {
+		if !item.State.IsInstalled() {
 			continue
 		}
 
@@ -619,6 +615,10 @@ func (h *Hub) localSync() error {
 func (i *Item) setVersionState(path string, inhub bool) error {
 	var err error
 
+	if !inhub {
+		i.State.LocalPath = path
+	}
+
 	i.State.LocalHash, err = downloader.SHA256(path)
 	if err != nil {
 		return fmt.Errorf("failed to get sha256 of %s: %w", path, err)
@@ -647,10 +647,6 @@ func (i *Item) setVersionState(path string, inhub bool) error {
 	if i.State.LocalVersion == "?" {
 		i.hub.logger.Tracef("got tainted match for %s: %s", i.Name, path)
 
-		if !inhub {
-			i.State.Installed = true
-		}
-
 		i.State.UpToDate = false
 		i.addTaint(i)
 
@@ -659,13 +655,10 @@ func (i *Item) setVersionState(path string, inhub bool) error {
 
 	// we got an exact match, update struct
 
-	i.State.Downloaded = true
-
 	if !inhub {
 		i.hub.logger.Tracef("found exact match for %s, version is %s, latest is %s", i.Name, i.State.LocalVersion, i.Version)
 		i.State.Tainted = false
 		// if we're walking the hub, present file doesn't means installed file
-		i.State.Installed = true
 	}
 
 	if i.State.LocalVersion == i.Version {

pkg/hubops/disable.go

@@ -20,17 +20,12 @@ func RemoveInstallLink(i *cwhub.Item) error {
 		return fmt.Errorf("%s isn't managed by hub", i.Name)
 	}
 
-	hubpath, err := os.Readlink(i.State.LocalPath)
+	target, err := os.Readlink(i.State.LocalPath)
 	if err != nil {
 		return fmt.Errorf("while reading symlink: %w", err)
 	}
 
-	src, err := i.DownloadPath()
+	if target != i.State.DownloadPath {
-	if err != nil {
-		return err
-	}
-
-	if hubpath != src {
 		return fmt.Errorf("%s isn't managed by hub", i.Name)
 	}
 
@@ -38,6 +33,8 @@ func RemoveInstallLink(i *cwhub.Item) error {
 		return fmt.Errorf("while removing symlink: %w", err)
 	}
 
+	i.State.LocalPath = ""
+
 	return nil
 }
 
@@ -64,7 +61,7 @@ func (c *DisableCommand) Prepare(plan *ActionPlan) (bool, error) {
 		return false, fmt.Errorf("%s is tainted, use '--force' to remove", i.Name)
 	}
 
-	if !i.State.Installed {
+	if !i.State.IsInstalled() {
 		return false, nil
 	}
 
@@ -74,7 +71,7 @@ func (c *DisableCommand) Prepare(plan *ActionPlan) (bool, error) {
 	}
 
 	for _, sub := range subsToRemove {
-		if !sub.State.Installed {
+		if !sub.State.IsInstalled() {
 			continue
 		}
 
@@ -97,7 +94,6 @@ func (c *DisableCommand) Run(ctx context.Context, plan *ActionPlan) error {
 
 	plan.ReloadNeeded = true
 
-	i.State.Installed = false
 	i.State.Tainted = false
 
 	return nil

pkg/hubops/download.go

@@ -52,7 +52,7 @@ func (c *DownloadCommand) Prepare(plan *ActionPlan) (bool, error) {
 
 	var disableKeys []*cwhub.Item
 
-	if i.State.Installed {
+	if i.State.IsInstalled() {
 		for sub := range i.CurrentDependencies().SubItems(plan.hub) {
 			disableKeys = append(disableKeys, sub)
 			toDisable[sub] = struct{}{}
@@ -64,7 +64,7 @@ func (c *DownloadCommand) Prepare(plan *ActionPlan) (bool, error) {
 			return false, err
 		}
 
-		if i.State.Installed {
+		if i.State.IsInstalled() {
 			// ensure the _new_ dependencies are installed too
 			if err := plan.AddCommand(NewEnableCommand(sub, c.Force)); err != nil {
 				return false, err
@@ -84,7 +84,7 @@ func (c *DownloadCommand) Prepare(plan *ActionPlan) (bool, error) {
 		}
 	}
 
-	if i.State.Downloaded && i.State.UpToDate {
+	if i.State.IsDownloaded() && i.State.UpToDate {
 		return false, nil
 	}
 
@@ -157,7 +157,7 @@ func (c *DownloadCommand) Run(ctx context.Context, plan *ActionPlan) error {
 	fmt.Printf("downloading %s\n", colorizeItemName(i.FQName()))
 
 	// ensure that target file is within target dir
-	finalPath, err := i.DownloadPath()
+	finalPath, err := i.PathForDownload()
 	if err != nil {
 		return err
 	}
@@ -171,7 +171,6 @@ func (c *DownloadCommand) Run(ctx context.Context, plan *ActionPlan) error {
 		plan.ReloadNeeded = true
 	}
 
-	i.State.Downloaded = true
 	i.State.Tainted = false
 	i.State.UpToDate = true
 
@@ -208,7 +207,7 @@ func (c *DownloadCommand) Detail() string {
 
 	version := color.YellowString(i.Version)
 
-	if i.State.Downloaded {
+	if i.State.IsDownloaded() {
 		version = c.Item.State.LocalVersion + " -> " + color.YellowString(i.Version)
 	}
 

pkg/hubops/enable.go

@@ -40,7 +40,7 @@ func (c *EnableCommand) Prepare(plan *ActionPlan) (bool, error) {
 		}
 	}
 
-	if i.State.Installed {
+	if i.State.IsInstalled() {
 		return false, nil
 	}
 
@@ -49,7 +49,7 @@ func (c *EnableCommand) Prepare(plan *ActionPlan) (bool, error) {
 
 // CreateInstallLink creates a symlink between the actual config file at hub.HubDir and hub.ConfigDir.
 func CreateInstallLink(i *cwhub.Item) error {
-	dest, err := i.InstallPath()
+	dest, err := i.PathForInstall()
 	if err != nil {
 		return err
 	}
@@ -66,15 +66,14 @@ func CreateInstallLink(i *cwhub.Item) error {
 		return fmt.Errorf("failed to stat %s: %w", dest, err)
 	}
 
-	src, err := i.DownloadPath()
+	src := i.State.DownloadPath
-	if err != nil {
-		return err
-	}
 
 	if err = os.Symlink(src, dest); err != nil {
 		return fmt.Errorf("while creating symlink from %s to %s: %w", src, dest, err)
 	}
 
+	i.State.LocalPath = dest
+
 	return nil
 }
 
@@ -83,7 +82,7 @@ func (c *EnableCommand) Run(ctx context.Context, plan *ActionPlan) error {
 
 	fmt.Println("enabling " + colorizeItemName(i.FQName()))
 
-	if !i.State.Downloaded {
+	if !i.State.IsDownloaded() {
 		// XXX: this a warning?
 		return fmt.Errorf("can't enable %s: not downloaded", i.FQName())
 	}
@@ -94,7 +93,6 @@ func (c *EnableCommand) Run(ctx context.Context, plan *ActionPlan) error {
 
 	plan.ReloadNeeded = true
 
-	i.State.Installed = true
 	i.State.Tainted = false
 
 	return nil

pkg/hubops/purge.go

@@ -43,7 +43,7 @@ func (c *PurgeCommand) Prepare(plan *ActionPlan) (bool, error) {
 		}
 	}
 
-	if !i.State.Downloaded {
+	if !i.State.IsDownloaded() {
 		return false, nil
 	}
 
@@ -55,20 +55,16 @@ func (c *PurgeCommand) Run(ctx context.Context, plan *ActionPlan) error {
 
 	fmt.Println("purging " + colorizeItemName(i.FQName()))
 
-	src, err := i.DownloadPath()
+	if err := os.Remove(i.State.DownloadPath); err != nil {
-	if err != nil {
-		return err
-	}
-
-	if err := os.Remove(src); err != nil {
 		if os.IsNotExist(err) {
+			i.State.DownloadPath = ""
 			return nil
 		}
 
 		return fmt.Errorf("while removing file: %w", err)
 	}
 
-	i.State.Downloaded = false
+	i.State.DownloadPath = ""
 	i.State.Tainted = false
 	i.State.UpToDate = false