From c161eb270bfcbf130c38bb76a516151ef5208229 Mon Sep 17 00:00:00 2001 From: mmetc <92726601+mmetc@users.noreply.github.com> Date: Tue, 25 Feb 2025 10:09:29 +0100 Subject: [PATCH] pkg/cwhub: refact Item.State.(Downloaded | Installed) (#3476) --- cmd/crowdsec-cli/clihub/items.go | 2 +- cmd/crowdsec-cli/cliitem/cmdinspect.go | 6 +- cmd/crowdsec-cli/cliitem/cmdremove.go | 2 +- cmd/crowdsec-cli/clisimulation/simulation.go | 2 +- pkg/appsec/appsec.go | 72 ++++++++++++++++++-- pkg/cwhub/fetch.go | 10 ++- pkg/cwhub/hub.go | 22 ++---- pkg/cwhub/item.go | 44 +++++++----- pkg/cwhub/state.go | 32 +++++---- pkg/cwhub/state_test.go | 42 ++++++------ pkg/cwhub/sync.go | 33 ++++----- pkg/hubops/disable.go | 16 ++--- pkg/hubops/download.go | 11 ++- pkg/hubops/enable.go | 14 ++-- pkg/hubops/purge.go | 12 ++-- 15 files changed, 189 insertions(+), 131 deletions(-) diff --git a/cmd/crowdsec-cli/clihub/items.go b/cmd/crowdsec-cli/clihub/items.go index 87cb10b1f93..483691fc5f9 100644 --- a/cmd/crowdsec-cli/clihub/items.go +++ b/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 } diff --git a/cmd/crowdsec-cli/cliitem/cmdinspect.go b/cmd/crowdsec-cli/cliitem/cmdinspect.go index b5ee0816d72..25906c30c7a 100644 --- a/cmd/crowdsec-cli/cliitem/cmdinspect.go +++ b/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, "", " ") diff --git a/cmd/crowdsec-cli/cliitem/cmdremove.go b/cmd/crowdsec-cli/cliitem/cmdremove.go index 42f72f25ca9..506599f3efd 100644 --- a/cmd/crowdsec-cli/cliitem/cmdremove.go +++ b/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) } } diff --git a/cmd/crowdsec-cli/clisimulation/simulation.go b/cmd/crowdsec-cli/clisimulation/simulation.go index 1b46c70c90a..1189f3f4ba3 100644 --- a/cmd/crowdsec-cli/clisimulation/simulation.go +++ b/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) diff --git a/pkg/appsec/appsec.go b/pkg/appsec/appsec.go index 5f01f76d993..feb84af109e 100644 --- a/pkg/appsec/appsec.go +++ b/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 } diff --git a/pkg/cwhub/fetch.go b/pkg/cwhub/fetch.go index e8dacad4a6d..fc5cd9d6230 100644 --- a/pkg/cwhub/fetch.go +++ b/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 } diff --git a/pkg/cwhub/hub.go b/pkg/cwhub/hub.go index b75c173bc9b..5c0caad1473 100644 --- a/pkg/cwhub/hub.go +++ b/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 - pathIndex map[string]*Item - local *csconfig.LocalHubCfg - logger *logrus.Logger - Warnings []string // Warnings encountered during sync + items HubItems // Items read from HubDir and InstallDir + local *csconfig.LocalHubCfg + logger *logrus.Logger + 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, - logger: logger, - pathIndex: make(map[string]*Item, 0), + local: local, + logger: logger, } 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) } } diff --git a/pkg/cwhub/item.go b/pkg/cwhub/item.go index f0b447c6c4e..41087af1ac9 100644 --- a/pkg/cwhub/item.go +++ b/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) { - if i.State.Installed { +func (i *Item) PathForInstall() (string, error) { + 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) { - ret, err := SafePath(i.hub.local.HubDir, i.RemotePath) - if err != nil { - return "", err +func (i *Item) PathForDownload() (string, error) { + path, err := SafePath(i.hub.local.HubDir, i.RemotePath) + + 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, - Downloaded: i.State.Downloaded, + Installed: i.State.IsInstalled(), + 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"` - State ItemState `yaml:",inline"` - Local bool `yaml:"local"` + Alias `yaml:",inline"` + State ItemState `yaml:",inline"` + Installed bool `yaml:"installed"` + Local bool `yaml:"local"` }{ - Alias: Alias(i), - State: i.State, - Local: i.State.IsLocal(), + Alias: Alias(i), + State: i.State, + 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 } diff --git a/pkg/cwhub/state.go b/pkg/cwhub/state.go index 63a433151cd..3e1876712b3 100644 --- a/pkg/cwhub/state.go +++ b/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"` - LocalVersion string `json:"local_version,omitempty" yaml:"local_version,omitempty"` - LocalHash string `json:"local_hash,omitempty" yaml:"local_hash,omitempty"` - Installed bool `json:"installed"` + // Path to the install link or local file -- keep LocalPath for compatibility + LocalPath string `yaml:"local_path,omitempty"` + LocalVersion string `yaml:"local_version,omitempty"` + LocalHash string `yaml:"local_hash,omitempty"` + DownloadPath string local bool - Downloaded bool `json:"downloaded"` - UpToDate bool `json:"up_to_date"` - Tainted bool `json:"tainted"` - TaintedBy []string `json:"tainted_by,omitempty" yaml:"tainted_by,omitempty"` - BelongsToCollections []string `json:"belongs_to_collections,omitempty" yaml:"belongs_to_collections,omitempty"` + UpToDate bool `yaml:"up_to_date"` + Tainted bool `yaml:"tainted"` + TaintedBy []string `yaml:"tainted_by,omitempty"` + BelongsToCollections []string `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 != "" +} diff --git a/pkg/cwhub/state_test.go b/pkg/cwhub/state_test.go index 20741809ae2..78e68140143 100644 --- a/pkg/cwhub/state_test.go +++ b/pkg/cwhub/state_test.go @@ -20,47 +20,47 @@ func TestItemStateText(t *testing.T) { tests := []test{ { ItemState{ - Installed: true, - UpToDate: false, - Tainted: false, - Downloaded: true, + LocalPath: "path/to/install", + UpToDate: false, + Tainted: false, + DownloadPath: "path/to/download", }, "enabled,update-available", emoji.Warning, }, { ItemState{ - Installed: true, - UpToDate: true, - Tainted: false, - Downloaded: true, + LocalPath: "path/to/install", + UpToDate: true, + Tainted: false, + DownloadPath: "path/to/download", }, "enabled", emoji.CheckMark, }, { ItemState{ - Installed: true, - UpToDate: false, - local: true, - Tainted: false, - Downloaded: false, + LocalPath: "path/to/install", + UpToDate: false, + local: true, + Tainted: false, + DownloadPath: "", }, "enabled,local", emoji.House, }, { ItemState{ - Installed: false, - UpToDate: false, - Tainted: false, - Downloaded: true, + LocalPath: "", + UpToDate: false, + Tainted: false, + DownloadPath: "path/to/download", }, "disabled,update-available", emoji.Prohibited, }, { ItemState{ - Installed: true, - UpToDate: false, - Tainted: true, - Downloaded: true, + LocalPath: "path/to/install", + UpToDate: false, + Tainted: true, + DownloadPath: "path/to/download", }, "enabled,tainted", emoji.Warning, diff --git a/pkg/cwhub/sync.go b/pkg/cwhub/sync.go index 5de548a521a..65c81d2c333 100644 --- a/pkg/cwhub/sync.go +++ b/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 { diff --git a/pkg/hubops/disable.go b/pkg/hubops/disable.go index 7340920c249..5ac959319cd 100644 --- a/pkg/hubops/disable.go +++ b/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 err != nil { - return err - } - - if hubpath != src { + if target != i.State.DownloadPath { 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 diff --git a/pkg/hubops/download.go b/pkg/hubops/download.go index 552fddc775c..c4ee4e5b017 100644 --- a/pkg/hubops/download.go +++ b/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) } diff --git a/pkg/hubops/enable.go b/pkg/hubops/enable.go index 40de40c8662..af581883b69 100644 --- a/pkg/hubops/enable.go +++ b/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() - if err != nil { - return err - } + src := i.State.DownloadPath 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 diff --git a/pkg/hubops/purge.go b/pkg/hubops/purge.go index 3b415b27428..f6cdf0fdc3e 100644 --- a/pkg/hubops/purge.go +++ b/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 != nil { - return err - } - - if err := os.Remove(src); err != nil { + if err := os.Remove(i.State.DownloadPath); 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