Merge pull request #6684 from snyk/fix/CLI-1402-use-gaf-networking-for-license-downloads

fix: Use GAF network stack for license downloads

Author: PeterSchafer

cliv2/scripts/prepare_licenses.go

@@ -9,15 +9,36 @@ import (
 	"path/filepath"
 	"strings"
 	"time"
+
+	"github.com/snyk/go-application-framework/pkg/configuration"
+	"github.com/snyk/go-application-framework/pkg/networking"
+	"github.com/snyk/go-application-framework/pkg/networking/middleware"
 )
 
 // licensesEmbeddedDir is the cliv2-relative tree where go-licenses and manual downloads write.
 var licensesEmbeddedDir = filepath.Join(".", "internal", "embedded", "_data", "licenses")
 
+const (
+	maxDownloadAttempts = 5
+	perAttemptTimeout   = 30 * time.Second
+)
+
 func log(msg string) {
 	fmt.Fprintln(os.Stderr, msg)
 }
 
+func newHTTPClient() *http.Client {
+	cfg := configuration.NewWithOpts()
+	cfg.Set(middleware.ConfigurationKeyRequestAttempts, maxDownloadAttempts)
+
+	na := networking.NewNetworkAccess(cfg)
+	na.AddHeaderField("User-Agent", "Snyk-CLI-Build/1.0")
+
+	client := na.GetUnauthorizedHttpClient()
+	client.Timeout = time.Duration(maxDownloadAttempts) * perAttemptTimeout
+	return client
+}
+
 func main() {
 	log("Preparing 3rd party licenses...")
 
@@ -50,6 +71,8 @@ func main() {
 		os.Exit(1)
 	}
 
+	client := newHTTPClient()
+
 	log("Downloading manual licenses...")
 	manualLicenses := []struct{ url, pkg string }{
 		{"https://raw.githubusercontent.com/davecgh/go-spew/master/LICENSE", "github.com/davecgh/go-spew"},
@@ -58,7 +81,7 @@ func main() {
 		{"https://go.dev/LICENSE?m=text", "go.dev"},
 	}
 	for _, lic := range manualLicenses {
-		if err := manualLicenseDownload(lic.url, lic.pkg); err != nil {
+		if err := manualLicenseDownload(client, lic.url, lic.pkg); err != nil {
 			log(fmt.Sprintf("Error downloading license: %v", err))
 			os.Exit(1)
 		}
@@ -72,7 +95,7 @@ func main() {
 	log("Done preparing 3rd party licenses.")
 }
 
-func manualLicenseDownload(url, packageName string) error {
+func manualLicenseDownload(client *http.Client, url, packageName string) error {
 	folderPath := filepath.Join(licensesEmbeddedDir, packageName)
 	licenseFile := filepath.Join(folderPath, "LICENSE")
 
@@ -86,14 +109,7 @@ func manualLicenseDownload(url, packageName string) error {
 		return fmt.Errorf("creating directory for %s: %w", packageName, err)
 	}
 
-	req, err := http.NewRequest(http.MethodGet, url, nil)
-	if err != nil {
-		return fmt.Errorf("creating request for %s: %w", packageName, err)
-	}
-	req.Header.Set("User-Agent", "Snyk-CLI-Build/1.0")
-
-	client := &http.Client{Timeout: 30 * time.Second}
-	resp, err := client.Do(req)
+	resp, err := client.Get(url)
 	if err != nil {
 		return fmt.Errorf("downloading license for %s: %w", packageName, err)
 	}

cliv2/scripts/prepare_licenses_test.go

@@ -6,6 +6,9 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestIsEmbeddedLicenseFileName(t *testing.T) {
@@ -44,14 +47,10 @@ func TestIsEmbeddedLicenseFileName(t *testing.T) {
 	}
 
 	for _, name := range keep {
-		if !isEmbeddedLicenseFileName(name) {
-			t.Errorf("expected %q to be kept, but it would be removed", name)
-		}
+		assert.True(t, isEmbeddedLicenseFileName(name), "expected %q to be kept, but it would be removed", name)
 	}
 	for _, name := range remove {
-		if isEmbeddedLicenseFileName(name) {
-			t.Errorf("expected %q to be removed, but it would be kept", name)
-		}
+		assert.False(t, isEmbeddedLicenseFileName(name), "expected %q to be removed, but it would be kept", name)
 	}
 }
 
@@ -62,9 +61,7 @@ func TestCleanupNonLicenseFiles(t *testing.T) {
 	t.Cleanup(func() { licensesEmbeddedDir = origDir })
 
 	pkgDir := filepath.Join(tmpDir, "example.com", "foo")
-	if err := os.MkdirAll(pkgDir, 0o755); err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, os.MkdirAll(pkgDir, 0o755))
 
 	files := map[string]bool{
 		"LICENSE":     true,
@@ -75,21 +72,15 @@ func TestCleanupNonLicenseFiles(t *testing.T) {
 		"main.go":     false,
 	}
 	for name := range files {
-		if err := os.WriteFile(filepath.Join(pkgDir, name), []byte("test"), 0o644); err != nil {
-			t.Fatal(err)
-		}
+		require.NoError(t, os.WriteFile(filepath.Join(pkgDir, name), []byte("test"), 0o644))
 	}
 
-	if err := cleanupNonLicenseFiles(); err != nil {
-		t.Fatalf("cleanupNonLicenseFiles() error: %v", err)
-	}
+	require.NoError(t, cleanupNonLicenseFiles())
 
 	for name, shouldExist := range files {
 		_, err := os.Stat(filepath.Join(pkgDir, name))
 		exists := err == nil
-		if exists != shouldExist {
-			t.Errorf("file %q: exists=%v, want exists=%v", name, exists, shouldExist)
-		}
+		assert.Equal(t, shouldExist, exists, "file %q", name)
 	}
 }
 
@@ -107,22 +98,15 @@ func TestManualLicenseDownload(t *testing.T) {
 	t.Cleanup(func() { licensesEmbeddedDir = origDir })
 
 	pkg := "example.com/testpkg"
-	if err := manualLicenseDownload(server.URL+"/LICENSE", pkg); err != nil {
-		t.Fatalf("manualLicenseDownload() error: %v", err)
-	}
+	client := &http.Client{}
+	require.NoError(t, manualLicenseDownload(client, server.URL+"/LICENSE", pkg))
 
 	content, err := os.ReadFile(filepath.Join(tmpDir, pkg, "LICENSE"))
-	if err != nil {
-		t.Fatalf("reading downloaded license: %v", err)
-	}
-	if string(content) != "MIT License\n" {
-		t.Errorf("license content = %q, want %q", content, "MIT License\n")
-	}
+	require.NoError(t, err)
+	assert.Equal(t, "MIT License\n", string(content))
 
 	// Second call should skip (file already exists).
-	if err := manualLicenseDownload(server.URL+"/LICENSE", pkg); err != nil {
-		t.Fatalf("second manualLicenseDownload() error: %v", err)
-	}
+	require.NoError(t, manualLicenseDownload(client, server.URL+"/LICENSE", pkg))
 }
 
 func TestManualLicenseDownloadHTTPError(t *testing.T) {
@@ -136,8 +120,55 @@ func TestManualLicenseDownloadHTTPError(t *testing.T) {
 	licensesEmbeddedDir = tmpDir
 	t.Cleanup(func() { licensesEmbeddedDir = origDir })
 
-	err := manualLicenseDownload(server.URL+"/LICENSE", "example.com/missing")
-	if err == nil {
-		t.Fatal("expected error for 404 response, got nil")
-	}
+	client := &http.Client{}
+	err := manualLicenseDownload(client, server.URL+"/LICENSE", "example.com/missing")
+	assert.Error(t, err, "expected error for 404 response, got nil")
+}
+
+func TestNewHTTPClient_SetsUserAgent(t *testing.T) {
+	var gotUA string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotUA = r.Header.Get("User-Agent")
+		_, err := w.Write([]byte("OK"))
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	}))
+	defer server.Close()
+
+	client := newHTTPClient()
+	resp, err := client.Get(server.URL)
+	require.NoError(t, err)
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	assert.NoError(t, resp.Body.Close())
+
+	assert.Equal(t, "Snyk-CLI-Build/1.0", gotUA)
+}
+
+func TestNewHTTPClient_RetriesOn429(t *testing.T) {
+	var calls int
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		calls++
+		n := calls
+		if n <= 2 {
+			w.Header().Set("Retry-After", "0")
+			w.WriteHeader(http.StatusTooManyRequests)
+			return
+		}
+		_, err := w.Write([]byte("OK"))
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	}))
+	defer server.Close()
+
+	client := newHTTPClient()
+	resp, err := client.Get(server.URL)
+	require.NoError(t, err)
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	assert.NoError(t, resp.Body.Close())
+
+	assert.GreaterOrEqual(t, calls, 3, "expected at least 3 server calls (2 x 429 + 1 x 200)")
 }