Fix issue which saw versions resolved twice

The version of a tool was being looked up twice - once prior to downloading and again post downloading. This was because GetUrl was being reached twice, initially to determine the download url and then again to determine whether the download was an archive. This change introduces a new function which takes the download url and uses that to determine whether its an archive. This removes the additional round trip and the associated output. The new function is also covered by an additional test. Signed-off-by: Richard Gee <[email protected]>
alexellis · May 11, 2024 · 9be14a2 · 9be14a2
1 parent 03f5980
commit 9be14a2
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 4 deletions.
diff --git a/pkg/get/download.go b/pkg/get/download.go
@@ -52,10 +52,7 @@ func Download(tool *Tool, arch, operatingSystem, version string, movePath string
 		fmt.Printf("%s written.\n", outFilePath)
 	}
 
-	if isArchive, err := tool.IsArchive(quiet); isArchive {
-		if err != nil {
-			return "", "", err
-		}
+	if isArchiveStr(downloadURL) {
 
 		outPath, err := decompress(tool, downloadURL, outFilePath, operatingSystem, arch, version, quiet)
 		if err != nil {

diff --git a/pkg/get/get.go b/pkg/get/get.go
@@ -86,6 +86,13 @@ func (tool Tool) IsArchive(quiet bool) (bool, error) {
 		strings.HasSuffix(downloadURL, "tgz"), nil
 }
 
+func isArchiveStr(downloadURL string) bool {
+
+	return strings.HasSuffix(downloadURL, "tar.gz") ||
+		strings.HasSuffix(downloadURL, "zip") ||
+		strings.HasSuffix(downloadURL, "tgz")
+}
+
 // GetDownloadURL fetches the download URL for a release of a tool
 // for a given os, architecture and version
 func GetDownloadURL(tool *Tool, os, arch, version string, quiet bool) (string, error) {

diff --git a/pkg/get/get_test.go b/pkg/get/get_test.go
@@ -155,6 +155,50 @@ sudo install -m 755 /tmp/bin/jq-linux64 /usr/local/bin/jq`,
 	}
 }
 
+func TestIsArchiveStr(t *testing.T) {
+
+	testCases := []struct {
+		description string // Description of the test case
+		downloadURL string // Input download URL
+		expected    bool   // Expected output
+	}{
+		{
+			description: "URL ends with '.tar.gz'",
+			downloadURL: "https://example.com/download.tar.gz",
+			expected:    true,
+		},
+		{
+			description: "URL ends with '.zip'",
+			downloadURL: "https://example.com/download.zip",
+			expected:    true,
+		},
+		{
+			description: "URL ends with '.tgz'",
+			downloadURL: "https://example.com/download.tgz",
+			expected:    true,
+		},
+		{
+			description: "URL does not end with any known archive extension",
+			downloadURL: "https://example.com/download.txt",
+			expected:    false,
+		},
+		{
+			description: "URL ends with '.tgz' but has extra characters",
+			downloadURL: "https://example.com/download.tgz123",
+			expected:    false,
+		},
+	}
+
+	for _, tc := range testCases {
+
+		result := isArchiveStr(tc.downloadURL)
+
+		if result != tc.expected {
+			t.Errorf("%s: For URL %s, expected %v but got %v", tc.description, tc.downloadURL, tc.expected, result)
+		}
+	}
+}
+
 func Test_GetDownloadURLs(t *testing.T) {
 	tools := MakeTools()
 	kubectlVersion := "v1.29.1"