plugin-getter: don't rely only on version for list

The plugins installed command list installed plugins, and prints out all their paths and versions. The results are listed in a list of unique versions, in ascending order. When listing plugins outside of a specific identifier however, because of the insertion in the list works, we only consider the version number, and ignore all subsequent insertions with the same version. This causes a problem when we have multiple plugins installed with the same version, as only the first one that is discovered (typically the one with lexicographical precedence) gets inserted, and the others are ignored. To support such a use case, we change the insertion routine to not only support versions, but also paths when finding a spot to insert the installation into.
hashicorp · Dec 4, 2023 · f88cd99 · f88cd99
1 parent 24430e2
commit f88cd99
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 18 deletions.
diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go
@@ -177,7 +177,7 @@ func (pr Requirement) ListInstallations(opts ListInstallationsOptions) (InstallL
 				continue
 			}
 
-			res.InsertSortedUniq(&Installation{
+			res = append(res, &Installation{
 				BinaryPath: path,
 				Version:    pluginVersionStr,
 			})
@@ -206,22 +206,6 @@ func (l InstallList) String() string {
 	return v.String()
 }
 
-// InsertSortedUniq inserts the installation in the right spot in the list by
-// comparing the version lexicographically.
-// A Duplicate version will replace any already present version.
-func (l *InstallList) InsertSortedUniq(install *Installation) {
-	pos := sort.Search(len(*l), func(i int) bool { return (*l)[i].Version >= install.Version })
-	if len(*l) > pos && (*l)[pos].Version == install.Version {
-		// already detected, let's ignore any new foundings, this way any plugin
-		// close to cwd or the packer exec takes precedence; this will be better
-		// for plugin development/tests.
-		return
-	}
-	(*l) = append((*l), nil)
-	copy((*l)[pos+1:], (*l)[pos:])
-	(*l)[pos] = install
-}
-
 // Installation describes a plugin installation
 type Installation struct {
 	// path to where binary is installed, if installed.

diff --git a/packer/plugin-getter/plugins_test.go b/packer/plugin-getter/plugins_test.go
@@ -27,6 +27,8 @@ var (
 
 	pluginFolderTwo = filepath.Join("testdata", "plugins_2")
 
+	pluginFolderThree = filepath.Join("testdata", "plugins_3")
+
 	pluginFolderWrongChecksums = filepath.Join("testdata", "wrong_checksums")
 )
 
@@ -106,8 +108,8 @@ func TestPlugin_ListInstallations(t *testing.T) {
 					BinaryPath: filepath.Join(pluginFolderOne, "github.com", "hashicorp", "amazon", "packer-plugin-amazon_v1.2.5_x5.0_windows_amd64.exe"),
 				},
 				{
-					BinaryPath: filepath.Join(pluginFolderOne, "github.com", "hashicorp", "google", "packer-plugin-google_v4.5.6_x5.0_windows_amd64.exe"),
 					Version:    "v4.5.6",
+					BinaryPath: filepath.Join(pluginFolderOne, "github.com", "hashicorp", "google", "packer-plugin-google_v4.5.6_x5.0_windows_amd64.exe"),
 				},
 				{
 					Version:    "v4.5.7",
@@ -117,10 +119,22 @@ func TestPlugin_ListInstallations(t *testing.T) {
 					Version:    "v4.5.8",
 					BinaryPath: filepath.Join(pluginFolderOne, "github.com", "hashicorp", "google", "packer-plugin-google_v4.5.8_x5.0_windows_amd64.exe"),
 				},
+				{
+					Version:    "v4.5.6",
+					BinaryPath: filepath.Join(pluginFolderTwo, "github.com", "hashicorp", "google", "packer-plugin-google_v4.5.6_x5.0_windows_amd64.exe"),
+				},
 				{
 					Version:    "v4.5.9",
 					BinaryPath: filepath.Join(pluginFolderTwo, "github.com", "hashicorp", "google", "packer-plugin-google_v4.5.9_x5.0_windows_amd64.exe"),
 				},
+				{
+					Version:    "v4.5.6",
+					BinaryPath: filepath.Join(pluginFolderTwo, "github.com", "hashicorp copy", "google", "packer-plugin-google_v4.5.6_x5.0_windows_amd64.exe"),
+				},
+				{
+					Version:    "v4.5.9",
+					BinaryPath: filepath.Join(pluginFolderTwo, "github.com", "hashicorp copy", "google", "packer-plugin-google_v4.5.9_x5.0_windows_amd64.exe"),
+				},
 			},
 		},
 
@@ -188,6 +202,10 @@ func TestPlugin_ListInstallations(t *testing.T) {
 					Version:    "v1.2.3",
 					BinaryPath: filepath.Join(pluginFolderOne, "github.com", "hashicorp", "amazon", "packer-plugin-amazon_v1.2.3_x5.0_darwin_amd64"),
 				},
+				{
+					Version:    "v1.2.3",
+					BinaryPath: filepath.Join(pluginFolderOne, "github.com", "hashicorp", "amazon", "packer-plugin-amazon_v1.2.3_x5.1_darwin_amd64"),
+				},
 				{
 					Version:    "v1.2.4",
 					BinaryPath: filepath.Join(pluginFolderOne, "github.com", "hashicorp", "amazon", "packer-plugin-amazon_v1.2.4_x5.0_darwin_amd64"),
@@ -276,12 +294,48 @@ func TestPlugin_ListInstallations(t *testing.T) {
 					Version:    "v4.5.8",
 					BinaryPath: filepath.Join(pluginFolderOne, "github.com", "hashicorp", "google", "packer-plugin-google_v4.5.8_x5.0_windows_amd64.exe"),
 				},
+				{
+					Version:    "v4.5.6",
+					BinaryPath: filepath.Join(pluginFolderTwo, "github.com", "hashicorp", "google", "packer-plugin-google_v4.5.6_x5.0_windows_amd64.exe"),
+				},
 				{
 					Version:    "v4.5.9",
 					BinaryPath: filepath.Join(pluginFolderTwo, "github.com", "hashicorp", "google", "packer-plugin-google_v4.5.9_x5.0_windows_amd64.exe"),
 				},
 			},
 		},
+		{
+			"test nil identifier - multiple plugins with same version",
+			fields{
+				Identifier: "",
+			},
+			ListInstallationsOptions{
+				[]string{
+					pluginFolderThree,
+				},
+				BinaryInstallationOptions{
+					APIVersionMajor: "5", APIVersionMinor: "0",
+					OS: "linux", ARCH: "amd64",
+					Checksummers: []Checksummer{
+						{
+							Type: "sha256",
+							Hash: sha256.New(),
+						},
+					},
+				},
+			},
+			false,
+			[]*Installation{
+				{
+					Version:    "v1.2.5",
+					BinaryPath: filepath.Join(pluginFolderThree, "github.com", "hashicorp", "alazon", "packer-plugin-alazon_v1.2.5_x5.0_linux_amd64"),
+				},
+				{
+					Version:    "v1.2.5",
+					BinaryPath: filepath.Join(pluginFolderThree, "github.com", "hashicorp", "amazon", "packer-plugin-amazon_v1.2.5_x5.0_linux_amd64"),
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/...stdata/plugins_3/github.com/hashicorp/alazon/packer-plugin-alazon_v1.2.5_x5.0_linux_amd64 b/...stdata/plugins_3/github.com/hashicorp/alazon/packer-plugin-alazon_v1.2.5_x5.0_linux_amd64
@@ -0,0 +1 @@
+5
diff --git a/...gins_3/github.com/hashicorp/alazon/packer-plugin-alazon_v1.2.5_x5.0_linux_amd64_SHA256SUM b/...gins_3/github.com/hashicorp/alazon/packer-plugin-alazon_v1.2.5_x5.0_linux_amd64_SHA256SUM
@@ -0,0 +1 @@
+ef2d127de37b942baad06145e54b0c619a1f22327b2ebbcfbec78f5564afe39d
diff --git a/...stdata/plugins_3/github.com/hashicorp/amazon/packer-plugin-amazon_v1.2.5_x5.0_linux_amd64 b/...stdata/plugins_3/github.com/hashicorp/amazon/packer-plugin-amazon_v1.2.5_x5.0_linux_amd64
@@ -0,0 +1 @@
+5
diff --git a/...gins_3/github.com/hashicorp/amazon/packer-plugin-amazon_v1.2.5_x5.0_linux_amd64_SHA256SUM b/...gins_3/github.com/hashicorp/amazon/packer-plugin-amazon_v1.2.5_x5.0_linux_amd64_SHA256SUM
@@ -0,0 +1 @@
+ef2d127de37b942baad06145e54b0c619a1f22327b2ebbcfbec78f5564afe39d
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		ef2d127de37b942baad06145e54b0c619a1f22327b2ebbcfbec78f5564afe39d