fix(cli): avoid panic while updating progress (#1597)

afiune · lacework-releng · web-flow · commit 570e578779af · 2024-03-20T14:23:04.000-07:00
* fix(cli): avoid panic while updating progress

Signed-off-by: Lacework &lt;tech-ally@lacework.net&gt;

* style(lwcomponent): do not obsure named returns

Signed-off-by: Lacework &lt;tech-ally@lacework.net&gt;

---------

Signed-off-by: Lacework &lt;tech-ally@lacework.net&gt;
Co-authored-by: Lacework &lt;tech-ally@lacework.net&gt;
diff --git a/Makefile b/Makefile
@@ -61,7 +61,10 @@ help:
 prepare: git-env install-tools go-vendor ## Initialize the go environment
 
 .PHONY: test
-test: prepare ## Run all go-sdk tests
+test: prepare test-only ## Run all go-sdk tests
+
+.PHONY: test-only
+test-only: ## Run all go-sdk tests only (without prepare)
 	$(eval PACKAGES := $(shell go list ./... | grep -v integration))
 	gotestsum -f testname --rerun-fails=3 --packages="$(PACKAGES)" \
 		-- -v -cover -run=$(regex) -coverprofile=$(COVERAGEOUT) $(PACKAGES)
diff --git a/cli/cmd/component.go b/cli/cmd/component.go
@@ -432,6 +432,15 @@ func installComponent(args []string) (err error) {
 		return
 	}
 
+	installedVersion := component.InstalledVersion()
+	if installedVersion != nil {
+		return errors.Errorf(
+			"component %s is already installed. To upgrade run '%s'",
+			color.HiYellowString(componentName),
+			color.HiGreenString("lacework component upgrade %s", componentName),
+		)
+	}
+
 	cli.OutputChecklist(successIcon, fmt.Sprintf("Component %s found\n", component.Name))
 
 	cli.StartProgress(fmt.Sprintf("Staging component %s...", componentName))
@@ -1139,7 +1148,7 @@ func downloadProgress(complete chan int8, path string, sizeB int64) {
 		spinnerSuffix string  = ""
 	)
 
-	if !cli.nonInteractive {
+	if cli.spinner != nil {
 		spinnerSuffix = cli.spinner.Suffix
 	}
 
@@ -1163,24 +1172,18 @@ func downloadProgress(complete chan int8, path string, sizeB int64) {
 				mb := float64(size) / (1 << 20)
 
 				if mb > previous {
-					if !cli.nonInteractive {
-						cli.spinner.Suffix = fmt.Sprintf("%s Downloaded: %.0fmb", spinnerSuffix, mb)
-					} else {
-						cli.OutputHuman("..Downloaded: %.0fmb\n", mb)
-					}
-
+					sizeString := fmt.Sprintf("%.0fmb", mb)
+					cli.Log.Infow("downloading component", "size", sizeString)
+					cli.StartProgress(fmt.Sprintf("%s (%s downloaded)", spinnerSuffix, sizeString))
 					previous = mb
 				}
 			} else {
 				percent := float64(size) / float64(sizeB) * 100
 
 				if percent > previous {
-					if !cli.nonInteractive {
-						cli.spinner.Suffix = fmt.Sprintf("%s Downloaded: %.0f%s", spinnerSuffix, percent, "%")
-					} else {
-						cli.OutputHuman("..Downloaded: %.0f%s\n", percent, "%")
-					}
-
+					percentString := fmt.Sprintf("%.0f%%", percent)
+					cli.Log.Infow("downloading component", "percent", percentString)
+					cli.StartProgress(fmt.Sprintf("%s (%s downloaded)", spinnerSuffix, percentString))
 					previous = percent
 				}
 			}
diff --git a/integration/component_test.go b/integration/component_test.go
@@ -109,6 +109,11 @@ func TestCDKComponentInstall(t *testing.T) {
 	out = run(t, dir, "component-example")
 	assert.Contains(t, out, "component")
 
+	outBytes, errBytes, exitcode := LaceworkCLIWithHome(dir, "component", "install", "component-example")
+	assert.NotContains(t, outBytes.String(), "Installation completed.", "STDOUT should be empty")
+	assert.Contains(t, errBytes.String(), "already installed")
+	assert.Equal(t, 1, exitcode, "EXITCODE is not the expected one")
+
 	cleanup(dir)
 }
 
diff --git a/lwcomponent/catalog.go b/lwcomponent/catalog.go
@@ -73,15 +73,13 @@ func (c *Catalog) GetComponent(name string) (*CDKComponent, error) {
 	return &component, nil
 }
 
-func (c *Catalog) ListComponentVersions(component *CDKComponent) (versions []*semver.Version, err error) {
+func (c *Catalog) ListComponentVersions(component *CDKComponent) ([]*semver.Version, error) {
 	if component.ApiInfo == nil {
-		err = errors.Errorf("component '%s' api info not available", component.Name)
-		return
+		return nil, errors.Errorf("component '%s' api info not available", component.Name)
 	}
 
-	versions = component.ApiInfo.AllVersions
-	if versions != nil {
-		return
+	if component.ApiInfo.AllVersions != nil {
+		return component.ApiInfo.AllVersions, nil
 	}
 
 	return listComponentVersions(c.client, component.ApiInfo.Id)
@@ -180,7 +178,7 @@ func (c *Catalog) Stage(
 	return
 }
 
-func (c *Catalog) Verify(component *CDKComponent) (err error) {
+func (c *Catalog) Verify(component *CDKComponent) error {
 	path := filepath.Join(component.stage.Directory(), component.Name)
 
 	if operatingSystem == "windows" {
@@ -189,12 +187,12 @@ func (c *Catalog) Verify(component *CDKComponent) (err error) {
 
 	data, err := os.ReadFile(path)
 	if err != nil {
-		return
+		return err
 	}
 
 	sig, err := component.stage.Signature()
 	if err != nil {
-		return
+		return err
 	}
 
 	rootPublicKey := minisign.PublicKey{}
@@ -205,29 +203,29 @@ func (c *Catalog) Verify(component *CDKComponent) (err error) {
 	return verifySignature(rootPublicKey, data, sig)
 }
 
-func (c *Catalog) Install(component *CDKComponent) (err error) {
+func (c *Catalog) Install(component *CDKComponent) error {
 	if component.stage == nil {
 		return errors.Errorf("component '%s' not staged", component.Name)
 	}
 
 	componentDir, err := componentDirectory(component.Name)
 	if err != nil {
-		return
+		return err
 	}
 
 	err = os.MkdirAll(componentDir, os.ModePerm)
 	if err != nil {
-		return
+		return err
 	}
 
 	err = component.stage.Commit(componentDir)
 	if err != nil {
-		return
+		return err
 	}
 
 	component.HostInfo, err = NewHostInfo(componentDir, component.Description, component.Type)
 	if err != nil {
-		return
+		return err
 	}
 
 	path := filepath.Join(componentDir, component.Name)
@@ -243,27 +241,25 @@ func (c *Catalog) Install(component *CDKComponent) (err error) {
 		}
 	}
 
-	return
+	return nil
 }
 
 // Delete a CDKComponent
 //
 // Remove the Component install directory and all sub-directory.  This function will not return an
 // error if the Component is not installed.
-func (c *Catalog) Delete(component *CDKComponent) (err error) {
+func (c *Catalog) Delete(component *CDKComponent) error {
 	componentDir, err := componentDirectory(component.Name)
 	if err != nil {
-		return
+		return err
 	}
 
 	_, err = os.Stat(componentDir)
 	if err != nil {
 		return errors.Errorf("component not installed. Try running 'lacework component install %s'", component.Name)
 	}
 
-	os.RemoveAll(componentDir)
-
-	return
+	return os.RemoveAll(componentDir)
 }
 
 func NewCatalog(
@@ -412,7 +408,7 @@ func LoadLocalComponents() (components map[string]CDKComponent, err error) {
 	return
 }
 
-func listComponentVersions(client *api.Client, componentId int32) (versions []*semver.Version, err error) {
+func listComponentVersions(client *api.Client, componentId int32) ([]*semver.Version, error) {
 	response, err := client.V2.Components.ListComponentVersions(componentId, operatingSystem, architecture)
 	if err != nil {
 		return nil, err
@@ -424,7 +420,7 @@ func listComponentVersions(client *api.Client, componentId int32) (versions []*s
 		rawVersions = response.Data[0].Versions
 	}
 
-	versions = make([]*semver.Version, len(rawVersions))
+	versions := make([]*semver.Version, len(rawVersions))
 
 	for idx, v := range rawVersions {
 		ver, err := semver.NewVersion(v)
diff --git a/lwcomponent/cdk_component.go b/lwcomponent/cdk_component.go
@@ -124,34 +124,32 @@ func (c *CDKComponent) EnterDevMode() error {
 	return nil
 }
 
-func (c *CDKComponent) InstalledVersion() (version *semver.Version) {
-	var err error
-
+func (c *CDKComponent) InstalledVersion() *semver.Version {
 	if c.HostInfo != nil {
-		version, err = c.HostInfo.Version()
+		version, err := c.HostInfo.Version()
 		if err == nil {
-			return
+			return version
 		}
 
 		if componentDir, err := c.Dir(); err == nil {
 			if devInfo, err := newDevInfo(componentDir); err == nil {
 				version, err = semver.NewVersion(devInfo.Version)
 				if err == nil {
-					return
+					return version
 				}
 			}
 		}
 	}
 
-	return
+	return nil
 }
 
-func (c *CDKComponent) LatestVersion() (version *semver.Version) {
+func (c *CDKComponent) LatestVersion() *semver.Version {
 	if c.ApiInfo != nil {
-		version = c.ApiInfo.Version
+		return c.ApiInfo.Version
 	}
 
-	return
+	return nil
 }
 
 func (c *CDKComponent) PrintSummary() []string {
diff --git a/lwcomponent/http.go b/lwcomponent/http.go
@@ -19,7 +19,6 @@
 package lwcomponent
 
 import (
-	"fmt"
 	"os"
 	"strconv"
 	"time"
@@ -32,12 +31,12 @@ const (
 	DefaultMaxRetry = 3
 )
 
-var log = lwlogger.New("INFO")
+var log = lwlogger.New("INFO").Sugar()
 
 // Retry 3 times (4 requests total)
 // Resty default RetryWaitTime is 100ms
 // Exponential backoff to a maximum of RetryWaitTime of 2s
-func DownloadFile(path string, url string) (err error) {
+func DownloadFile(path string, url string) error {
 	client := resty.New()
 
 	download_timeout := os.Getenv("CDK_DOWNLOAD_TIMEOUT_MINUTES")
@@ -52,14 +51,31 @@ func DownloadFile(path string, url string) (err error) {
 	client.SetRetryCount(DefaultMaxRetry)
 
 	client.OnError(func(req *resty.Request, err error) {
+		fields := []interface{}{
+			"raw_error", err,
+		}
+
 		if v, ok := err.(*resty.ResponseError); ok {
-			log.Warn(fmt.Sprintf("Failed to download component: %s: %s", v.Response.Body(), v.Err))
+
+			fields = append(fields, "response_body", string(v.Response.Body()))
+
+			if v.Response.Request != nil {
+				trace := v.Response.Request.TraceInfo()
+				fields = append(fields, "trace_info", trace)
+			}
+
+			if v.Err != nil {
+				fields = append(fields, "response_error", v.Err.Error())
+			}
 		}
 
-		log.Warn(fmt.Sprintf("Failed to download component: %s", err.Error()))
+		log.Warnw("Failed to download component", fields...)
 	})
 
-	_, err = client.R().SetOutput(path).Get(url)
+	_, err := client.R().
+		EnableTrace().
+		SetOutput(path).
+		Get(url)
 
-	return
+	return err
 }
diff --git a/lwcomponent/http_test.go b/lwcomponent/http_test.go
@@ -1,4 +1,4 @@
-package lwcomponent_test
+package lwcomponent
 
 import (
 	"fmt"
@@ -7,8 +7,10 @@ import (
 	"os"
 	"testing"
 
-	"github.com/lacework/go-sdk/lwcomponent"
 	"github.com/stretchr/testify/assert"
+
+	capturer "github.com/lacework/go-sdk/internal/capturer"
+	"github.com/lacework/go-sdk/lwlogger"
 )
 
 func TestDownloadFile(t *testing.T) {
@@ -33,7 +35,7 @@ func TestDownloadFile(t *testing.T) {
 	})
 
 	t.Run("happy path", func(t *testing.T) {
-		err = lwcomponent.DownloadFile(file.Name(), fmt.Sprintf("%s%s", server.URL, urlPath))
+		err = DownloadFile(file.Name(), fmt.Sprintf("%s%s", server.URL, urlPath))
 		assert.Nil(t, err)
 
 		buf, err := os.ReadFile(file.Name())
@@ -53,14 +55,33 @@ func TestDownloadFile(t *testing.T) {
 			}
 		})
 
-		err = lwcomponent.DownloadFile(file.Name(), fmt.Sprintf("%s%s", server.URL, "/err"))
+		logsCaptured := capturer.CaptureOutput(func() {
+			log = lwlogger.New("INFO").Sugar()
+			err = DownloadFile(file.Name(), fmt.Sprintf("%s%s", server.URL, "/err"))
+		})
 		assert.NotNil(t, err)
-		assert.Equal(t, lwcomponent.DefaultMaxRetry+1, count)
+		assert.Equal(t, DefaultMaxRetry+1, count)
+
+		assert.Contains(t, logsCaptured, "WARN RESTY Get")
+		assert.Contains(t, logsCaptured, "/err\": EOF")
+		assert.Contains(t, logsCaptured, "Attempt 4") // the fifth attempt will error
+		assert.Contains(t, logsCaptured, "ERROR RESTY Get")
+		assert.Contains(t, logsCaptured, "Failed to download component")
+		assert.Contains(t, logsCaptured, "trace_info")
 	})
 
 	t.Run("url error", func(t *testing.T) {
-		err = lwcomponent.DownloadFile(file.Name(), "")
+		logsCaptured := capturer.CaptureOutput(func() {
+			log = lwlogger.New("INFO").Sugar()
+			err = DownloadFile(file.Name(), "")
+		})
 		assert.NotNil(t, err)
 		assert.False(t, os.IsTimeout(err))
+
+		assert.Contains(t, logsCaptured, "WARN RESTY Get")
+		assert.Contains(t, logsCaptured, "Attempt 4") // the fifth attempt will error
+		assert.Contains(t, logsCaptured, "ERROR RESTY Get")
+		assert.Contains(t, logsCaptured, "Failed to download component")
+		assert.Contains(t, logsCaptured, "trace_info")
 	})
 }
diff --git a/lwcomponent/staging.go b/lwcomponent/staging.go

Original file line number	Diff line number	Diff line change
`@@ -73,15 +73,13 @@ func (c Catalog) GetComponent(name string) (CDKComponent, error) {`
`73`	`73`	`return &component, nil`
`74`	`74`	`}`
`75`	`75`
`76`		`-func (c Catalog) ListComponentVersions(component CDKComponent) (versions []*semver.Version, err error) {`
	`76`	`+func (c Catalog) ListComponentVersions(component CDKComponent) ([]*semver.Version, error) {`
`77`	`77`	`if component.ApiInfo == nil {`
`78`		`- err = errors.Errorf("component '%s' api info not available", component.Name)`
`79`		`- return`
	`78`	`+ return nil, errors.Errorf("component '%s' api info not available", component.Name)`
`80`	`79`	`}`
`81`	`80`
`82`		`- versions = component.ApiInfo.AllVersions`
`83`		`- if versions != nil {`
`84`		`- return`
	`81`	`+ if component.ApiInfo.AllVersions != nil {`
	`82`	`+ return component.ApiInfo.AllVersions, nil`
`85`	`83`	`}`
`86`	`84`
`87`	`85`	`return listComponentVersions(c.client, component.ApiInfo.Id)`
`@@ -180,7 +178,7 @@ func (c *Catalog) Stage(`
`180`	`178`	`return`
`181`	`179`	`}`
`182`	`180`
`183`		`-func (c Catalog) Verify(component CDKComponent) (err error) {`
	`181`	`+func (c Catalog) Verify(component CDKComponent) error {`
`184`	`182`	`path := filepath.Join(component.stage.Directory(), component.Name)`
`185`	`183`
`186`	`184`	`if operatingSystem == "windows" {`
`@@ -189,12 +187,12 @@ func (c Catalog) Verify(component CDKComponent) (err error) {`
`189`	`187`
`190`	`188`	`data, err := os.ReadFile(path)`
`191`	`189`	`if err != nil {`
`192`		`- return`
	`190`	`+ return err`
`193`	`191`	`}`
`194`	`192`
`195`	`193`	`sig, err := component.stage.Signature()`
`196`	`194`	`if err != nil {`
`197`		`- return`
	`195`	`+ return err`
`198`	`196`	`}`
`199`	`197`
`200`	`198`	`rootPublicKey := minisign.PublicKey{}`
`@@ -205,29 +203,29 @@ func (c Catalog) Verify(component CDKComponent) (err error) {`
`205`	`203`	`return verifySignature(rootPublicKey, data, sig)`
`206`	`204`	`}`
`207`	`205`
`208`		`-func (c Catalog) Install(component CDKComponent) (err error) {`
	`206`	`+func (c Catalog) Install(component CDKComponent) error {`
`209`	`207`	`if component.stage == nil {`
`210`	`208`	`return errors.Errorf("component '%s' not staged", component.Name)`
`211`	`209`	`}`
`212`	`210`
`213`	`211`	`componentDir, err := componentDirectory(component.Name)`
`214`	`212`	`if err != nil {`
`215`		`- return`
	`213`	`+ return err`
`216`	`214`	`}`
`217`	`215`
`218`	`216`	`err = os.MkdirAll(componentDir, os.ModePerm)`
`219`	`217`	`if err != nil {`
`220`		`- return`
	`218`	`+ return err`
`221`	`219`	`}`
`222`	`220`
`223`	`221`	`err = component.stage.Commit(componentDir)`
`224`	`222`	`if err != nil {`
`225`		`- return`
	`223`	`+ return err`
`226`	`224`	`}`
`227`	`225`
`228`	`226`	`component.HostInfo, err = NewHostInfo(componentDir, component.Description, component.Type)`
`229`	`227`	`if err != nil {`
`230`		`- return`
	`228`	`+ return err`
`231`	`229`	`}`
`232`	`230`
`233`	`231`	`path := filepath.Join(componentDir, component.Name)`
`@@ -243,27 +241,25 @@ func (c Catalog) Install(component CDKComponent) (err error) {`
`243`	`241`	`}`
`244`	`242`	`}`
`245`	`243`
`246`		`- return`
	`244`	`+ return nil`
`247`	`245`	`}`
`248`	`246`
`249`	`247`	`// Delete a CDKComponent`
`250`	`248`	`//`
`251`	`249`	`// Remove the Component install directory and all sub-directory. This function will not return an`
`252`	`250`	`// error if the Component is not installed.`
`253`		`-func (c Catalog) Delete(component CDKComponent) (err error) {`
	`251`	`+func (c Catalog) Delete(component CDKComponent) error {`
`254`	`252`	`componentDir, err := componentDirectory(component.Name)`
`255`	`253`	`if err != nil {`
`256`		`- return`
	`254`	`+ return err`
`257`	`255`	`}`
`258`	`256`
`259`	`257`	`_, err = os.Stat(componentDir)`
`260`	`258`	`if err != nil {`
`261`	`259`	`return errors.Errorf("component not installed. Try running 'lacework component install %s'", component.Name)`
`262`	`260`	`}`
`263`	`261`
`264`		`- os.RemoveAll(componentDir)`
`265`		`-`
`266`		`- return`
	`262`	`+ return os.RemoveAll(componentDir)`
`267`	`263`	`}`
`268`	`264`
`269`	`265`	`func NewCatalog(`
`@@ -412,7 +408,7 @@ func LoadLocalComponents() (components map[string]CDKComponent, err error) {`
`412`	`408`	`return`
`413`	`409`	`}`
`414`	`410`
`415`		`-func listComponentVersions(client api.Client, componentId int32) (versions []semver.Version, err error) {`
	`411`	`+func listComponentVersions(client api.Client, componentId int32) ([]semver.Version, error) {`
`416`	`412`	`response, err := client.V2.Components.ListComponentVersions(componentId, operatingSystem, architecture)`
`417`	`413`	`if err != nil {`
`418`	`414`	`return nil, err`
`@@ -424,7 +420,7 @@ func listComponentVersions(client api.Client, componentId int32) (versions []s`
`424`	`420`	`rawVersions = response.Data[0].Versions`
`425`	`421`	`}`
`426`	`422`
`427`		`- versions = make([]*semver.Version, len(rawVersions))`
	`423`	`+ versions := make([]*semver.Version, len(rawVersions))`
`428`	`424`
`429`	`425`	`for idx, v := range rawVersions {`
`430`	`426`	`ver, err := semver.NewVersion(v)`
Original file line number	Diff line number	Diff line change
`@@ -124,34 +124,32 @@ func (c *CDKComponent) EnterDevMode() error {`
`124`	`124`	`return nil`
`125`	`125`	`}`
`126`	`126`
`127`		`-func (c CDKComponent) InstalledVersion() (version semver.Version) {`
`128`		`- var err error`
`129`		`-`
	`127`	`+func (c CDKComponent) InstalledVersion() semver.Version {`
`130`	`128`	`if c.HostInfo != nil {`
`131`		`- version, err = c.HostInfo.Version()`
	`129`	`+ version, err := c.HostInfo.Version()`
`132`	`130`	`if err == nil {`
`133`		`- return`
	`131`	`+ return version`
`134`	`132`	`}`
`135`	`133`
`136`	`134`	`if componentDir, err := c.Dir(); err == nil {`
`137`	`135`	`if devInfo, err := newDevInfo(componentDir); err == nil {`
`138`	`136`	`version, err = semver.NewVersion(devInfo.Version)`
`139`	`137`	`if err == nil {`
`140`		`- return`
	`138`	`+ return version`
`141`	`139`	`}`
`142`	`140`	`}`
`143`	`141`	`}`
`144`	`142`	`}`
`145`	`143`
`146`		`- return`
	`144`	`+ return nil`
`147`	`145`	`}`
`148`	`146`
`149`		`-func (c CDKComponent) LatestVersion() (version semver.Version) {`
	`147`	`+func (c CDKComponent) LatestVersion() semver.Version {`
`150`	`148`	`if c.ApiInfo != nil {`
`151`		`- version = c.ApiInfo.Version`
	`149`	`+ return c.ApiInfo.Version`
`152`	`150`	`}`
`153`	`151`
`154`		`- return`
	`152`	`+ return nil`
`155`	`153`	`}`
`156`	`154`
`157`	`155`	`func (c *CDKComponent) PrintSummary() []string {`