fix: component download timeout retry (#1389)

Author: jon-stewart

lwcomponent/component.go

@@ -257,7 +257,7 @@ func (s State) Install(component *Component, version string) error {
 		return err
 	}
 
-	err = downloadFile(path, artifact.URL)
+	err = DownloadFile(path, artifact.URL, 0)
 	if err != nil {
 		return errors.Wrap(err, "unable to download component artifact")
 	}

lwcomponent/component_internal_test.go

@@ -125,15 +125,15 @@ type pathTest struct {
 }
 
 var pathTests = []pathTest{
-	pathTest{
+	{
 		"NotExists",
 		Component{
 			Name: "no-such-component",
 			Type: "STANDALONE",
 		},
 		errors.New("component not found on disk"),
 	},
-	pathTest{
+	{
 		"Exists",
 		mockComponent,
 		nil,
@@ -176,15 +176,15 @@ type isVerifiedTest struct {
 }
 
 var isVerifiedTests = []isVerifiedTest{
-	isVerifiedTest{
+	{
 		Name: "NoSignature",
 		Component: Component{
 			Name: "lacework-mock-component",
 		},
 		Version: "0.1.0",
 		Error:   errors.New("component signature file does not exist"),
 	},
-	isVerifiedTest{
+	{
 		Name: "Mismatch",
 		Component: Component{
 			Name: "lacework-mock-component",
@@ -193,7 +193,7 @@ var isVerifiedTests = []isVerifiedTest{
 		Signature: base64.StdEncoding.EncodeToString([]byte("blah blah blah")),
 		Error:     errors.New("unable to parse signature"),
 	},
-	isVerifiedTest{
+	{
 		Name:      "Verified",
 		Component: mockComponent,
 		Version:   "0.1.0\n",
@@ -231,26 +231,26 @@ type runTest struct {
 }
 
 var runTests = []runTest{
-	runTest{
+	{
 		Name:      "IsNotBinary",
 		Component: Component{Name: "IsNotBinary"},
 		Version:   "0.1.0",
 		Error:     errors.New("unable to run component: component IsNotBinary is not a binary"),
 	},
-	runTest{
+	{
 		Name:      "IsNotVerified",
 		Component: Component{Name: "IsNotVerified", Type: "BINARY"},
 		Version:   "0.1.0",
 		Error:     errors.New("unable to run component: component signature file does not exist"),
 	},
-	runTest{
+	{
 		Name:      "OK",
 		Component: mockComponent,
 		Version:   "0.1.0",
 		Signature: helloWorldSig,
 		Error:     nil,
 	},
-	runTest{
+	{
 		Name:      "Error",
 		Component: mockComponent,
 		Version:   "0.1.0",

lwcomponent/http.go

@@ -22,13 +22,36 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"time"
+)
+
+const (
+	defaultTimeout  = 30 * time.Second
+	defaultMaxRetry = 2
 )
 
 // downloadFile is an internal helper that downloads a file to the provided file path
-func downloadFile(filepath string, url string) error {
-	resp, err := http.Get(url)
+func DownloadFile(filepath string, url string, timeout time.Duration) error {
+	var (
+		resp     *http.Response
+		err      error
+		_timeout time.Duration = timeout
+	)
+
+	if _timeout == 0 {
+		_timeout = defaultTimeout
+	}
+
+	client := &http.Client{Timeout: _timeout}
+
+	resp, err = client.Get(url)
 	if err != nil {
-		return err
+		for retry := 0; retry < defaultMaxRetry && os.IsTimeout(err); retry++ {
+			resp, err = client.Get(url)
+		}
+		if err != nil {
+			return err
+		}
 	}
 	defer resp.Body.Close()
 

lwcomponent/http_test.go

@@ -0,0 +1,56 @@
+package lwcomponent_test
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/lacework/go-sdk/lwcomponent"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDownloadFile(t *testing.T) {
+	var (
+		urlPath string = "/lw-cdk-store/catalog/component-example/1.0.0/component-example-linux-amd64.tar.gz"
+		content string = "CDK component"
+	)
+
+	file, err := os.CreateTemp("", "lwcomponent-downloadFile")
+	assert.Nil(t, err)
+	defer file.Close()
+
+	mux := http.NewServeMux()
+
+	server := httptest.NewServer(mux)
+	defer server.Close()
+
+	mux.HandleFunc(urlPath, func(w http.ResponseWriter, r *http.Request) {
+		if assert.Equal(t, "GET", r.Method, "Get() should be a GET method") {
+			fmt.Fprint(w, content)
+		}
+	})
+
+	t.Run("happy path", func(t *testing.T) {
+		err = lwcomponent.DownloadFile(file.Name(), fmt.Sprintf("%s%s", server.URL, urlPath), 0)
+		assert.Nil(t, err)
+
+		buf, err := os.ReadFile(file.Name())
+		assert.Nil(t, err)
+		assert.Equal(t, content, string(buf))
+	})
+
+	t.Run("timeout error", func(t *testing.T) {
+		err = lwcomponent.DownloadFile(file.Name(), fmt.Sprintf("%s%s", server.URL, urlPath), 1*time.Microsecond)
+		assert.NotNil(t, err)
+		assert.True(t, os.IsTimeout(err))
+	})
+
+	t.Run("non-timeout error", func(t *testing.T) {
+		err = lwcomponent.DownloadFile(file.Name(), "", 0)
+		assert.NotNil(t, err)
+		assert.False(t, os.IsTimeout(err))
+	})
+}

lwcomponent/minisign_internal_test.go

@@ -35,43 +35,43 @@ type verifySignatureTest struct {
 }
 
 var verifySignatureTests = []verifySignatureTest{
-	verifySignatureTest{
+	{
 		Name:  "UnableToParseSignature",
 		Error: errors.New("unable to parse signature"),
 	},
-	verifySignatureTest{
+	{
 		Name: "InvalidTrustedComment",
 		Signature: []byte(`untrusted comment: 
 RWQnyHnTKz1RiRl+vm7dNpp7Jrt+qLANsu9m+TaHSgVyJO9Ldeo5ZPw/GQJu1fJZBuCrsMVW5KbuB6dEPzQvi8ct1zJfMkgVsgs=
 trusted comment: RWQnyHnTKz1RidqMl45Ou5XbJCroV7zohE1jpbigTUGXyQY94AF5uo4v.1.dW50cnVzdGVkIGNvbW1lbnQ6IApSV1RjbVljdjlQMHlNcExyVnA3V3lIdmx2RU02VlhJbFhzUGE0QVZVL1MraHlwUCtQdHBFWEVvck5MS3ZReTJSV0QxTmNKNzNlU1NMYUE2TnpLNnBvczlpb2RhbENMczhOZzg9CnRydXN0ZWQgY29tbWVudDogCm1iMmpJRVZBUTNPZlIvL0pRcjUxVzdQR3FrWDhGbWM1YUFKcCt0Q3JXOHZmWmZ6YnVMM2ZvejdtQmV6MHpQdlBldmQxU0FrUklmSW1EMkdjSkVlUEJ3PT0=
 cJwh7XDctGR8dpc1NEJnPGU5IXNvMNXa6hdMA0BBZvvnsKQHeVVEkMc7zo72iFsHEKRxe+AmXkryNgVi7Gg0DQ==`),
 		Error: errors.New("invalid signature trusted comment"),
 	},
-	verifySignatureTest{
+	{
 		Name: "UnableToParseRootSignature",
 		Signature: []byte(`untrusted comment: 
 RWQnyHnTKz1RiRl+vm7dNpp7Jrt+qLANsu9m+TaHSgVyJO9Ldeo5ZPw/GQJu1fJZBuCrsMVW5KbuB6dEPzQvi8ct1zJfMkgVsgs=
 trusted comment: 1.RWQnyHnTz1RidqMl45Ou5XbJCroV7zohE1jpbigTUGXyQY94AF5uo4v.1.W50cnVzdGVkIGNvbW1lbnQ6IApSV1RjbVljdjlQMHlNcExyVnA3V3lIdmx2RU02VlhJbFhzUGE0QVZVL1MraHlwUCtQdHBFWEVvck5MS3ZReTJSV0QxTmNKNzNlU1NMYUE2TnpLNnBvczlpb2RhbENMczhOZzg9CnRydXN0ZWQgY29tbWVudDogCm1iMmpJRVZBUTNPZlIvL0pRcjUxVzdQR3FrWDhGbWM1YUFKcCt0Q3JXOHZmWmZ6YnVMM2ZvejdtQmV6MHpQdlBldmQxU0FrUklmSW1EMkdjSkVlUEJ3PT0=
 cJwh7XDctGR8dpc1NEJnPGU5IXNvMNXa6hdMA0BBZvvnsKQHeVVEkMc7zo72iFsHEKRxe+AmXkryNgVi7Gg0DQ==`),
 		Error: errors.New("unable to parse root signature from trusted comment: illegal base64 data at input byte 303"),
 	},
-	verifySignatureTest{
+	{
 		Name: "InvalidRootSignatureOverSigningKey",
 		Signature: []byte(`untrusted comment: 
 RWQnyHnTKz1RiRl+vm7dNpp7Jrt+qLANsu9m+TaHSgVyJO9Ldeo5ZPw/GQJu1fJZBuCrsMVW5KbuB6dEPzQvi8ct1zJfMkgVsgs=
 trusted comment: 1.RWQnyHnTz1RidqMl45Ou5XbJCroV7zohE1jpbigTUGXyQY94AF5uo4v.1.dW50cnVzdGVkIGNvbW1lbnQ6IApSV1RjbVljdjlQMHlNcExyVnA3V3lIdmx2RU02VlhJbFhzUGE0QVZVL1MraHlwUCtQdHBFWEVvck5MS3ZReTJSV0QxTmNKNzNlU1NMYUE2TnpLNnBvczlpb2RhbENMczhOZzg9CnRydXN0ZWQgY29tbWVudDogCm1iMmpJRVZBUTNPZlIvL0pRcjUxVzdQR3FrWDhGbWM1YUFKcCt0Q3JXOHZmWmZ6YnVMM2ZvejdtQmV6MHpQdlBldmQxU0FrUklmSW1EMkdjSkVlUEJ3PT0=
 cJwh7XDctGR8dpc1NEJnPGU5IXNvMNXa6hdMA0BBZvvnsKQHeVVEkMc7zo72iFsHEKRxe+AmXkryNgVi7Gg0DQ==`),
 		Error: errors.New("invalid root signature over signing key"),
 	},
-	verifySignatureTest{
+	{
 		Name: "InvalidSignatureOverComponent",
 		Signature: []byte(`untrusted comment: 
 RWQnyHnTKz1RiRl+vm7dNpp7Jrt+qLANsu9m+TaHSgVyJO9Ldeo5ZPw/GQJu1fJZBuCrsMVW5KbuB6dEPzQvi8ct1zJfMkgVsgs=
 trusted comment: 1.RWQnyHnTKz1RidqMl45Ou5XbJCroV7zohE1jpbigTUGXyQY94AF5uo4v.1.dW50cnVzdGVkIGNvbW1lbnQ6IApSV1RjbVljdjlQMHlNcExyVnA3V3lIdmx2RU02VlhJbFhzUGE0QVZVL1MraHlwUCtQdHBFWEVvck5MS3ZReTJSV0QxTmNKNzNlU1NMYUE2TnpLNnBvczlpb2RhbENMczhOZzg9CnRydXN0ZWQgY29tbWVudDogCm1iMmpJRVZBUTNPZlIvL0pRcjUxVzdQR3FrWDhGbWM1YUFKcCt0Q3JXOHZmWmZ6YnVMM2ZvejdtQmV6MHpQdlBldmQxU0FrUklmSW1EMkdjSkVlUEJ3PT0=
 cJwh7XDctGR8dpc1NEJnPGU5IXNvMNXa6hdMA0BBZvvnsKQHeVVEkMc7zo72iFsHEKRxe+AmXkryNgVi7Gg0DQ==`),
 		Error: errors.New("invalid signature over component"),
 	},
-	verifySignatureTest{
+	{
 		Name:      "OK",
 		Message:   helloWorld,
 		Signature: helloWorldSigDecoded,