packer: fix plugin version sorting and pickup

When Packer orders the plugins by their version, we'd assumed it was
ordered from highest to lowest.

However, this was the case because our implementation of `Less' was
actually returning `>=', which is not what it was supposed to do.

This commit therefore fixes the implementation of `Less' to do what it
is documented to do, and ensures the behaviour is correct through
additional testing.
Changing this requires some changes to the loading process as well
because of the aforementioned assumption with regards to ordering.

hcl2template/plugin.go

@@ -94,7 +94,7 @@ func (cfg *PackerConfig) DetectPluginBinaries() hcl.Diagnostics {
 			continue
 		}
 		log.Printf("[TRACE] Found the following %q installations: %v", pluginRequirement.Identifier, sortedInstalls)
-		install := sortedInstalls[0]
+		install := sortedInstalls[len(sortedInstalls)-1]
 		err = cfg.parser.PluginConfig.DiscoverMultiPlugin(pluginRequirement.Accessor, install.BinaryPath)
 		if err != nil {
 			diags = append(diags, &hcl.Diagnostic{

packer/plugin-getter/plugins.go

@@ -267,7 +267,7 @@ func (l InstallList) Less(i, j int) bool {
 		return lowRawPluginName < hiRawPluginName
 	}
 
-	return semver.Compare(lowPluginPath.Version, hiPluginPath.Version) > 0
+	return semver.Compare(lowPluginPath.Version, hiPluginPath.Version) < 0
 }
 
 // Swap swaps the elements with indexes i and j.

packer/plugin-getter/plugins_test.go

@@ -507,3 +507,123 @@ func zipFile(content map[string]string) io.ReadCloser {
 }
 
 var _ Getter = &mockPluginGetter{}
+
+func Test_LessInstallList(t *testing.T) {
+	tests := []struct {
+		name       string
+		installs   InstallList
+		expectLess bool
+	}{
+		{
+			"v1.2.1 < v1.2.2 => true",
+			InstallList{
+				&Installation{
+					BinaryPath: "host/org/plugin",
+					Version:    "v1.2.1",
+				},
+				&Installation{
+					BinaryPath: "github.com",
+					Version:    "v1.2.2",
+				},
+			},
+			true,
+		},
+		{
+			// Impractical with the changes to the loading model
+			"v1.2.1 = v1.2.1 => false",
+			InstallList{
+				&Installation{
+					BinaryPath: "host/org/plugin",
+					Version:    "v1.2.1",
+				},
+				&Installation{
+					BinaryPath: "github.com",
+					Version:    "v1.2.1",
+				},
+			},
+			false,
+		},
+		{
+			"v1.2.2 < v1.2.1 => false",
+			InstallList{
+				&Installation{
+					BinaryPath: "host/org/plugin",
+					Version:    "v1.2.2",
+				},
+				&Installation{
+					BinaryPath: "github.com",
+					Version:    "v1.2.1",
+				},
+			},
+			false,
+		},
+		{
+			"v1.2.2-dev < v1.2.2 => true",
+			InstallList{
+				&Installation{
+					BinaryPath: "host/org/plugin",
+					Version:    "v1.2.2-dev",
+				},
+				&Installation{
+					BinaryPath: "github.com",
+					Version:    "v1.2.2",
+				},
+			},
+			true,
+		},
+		{
+			"v1.2.2 < v1.2.2-dev => false",
+			InstallList{
+				&Installation{
+					BinaryPath: "host/org/plugin",
+					Version:    "v1.2.2",
+				},
+				&Installation{
+					BinaryPath: "github.com",
+					Version:    "v1.2.2-dev",
+				},
+			},
+			false,
+		},
+		{
+			"v1.2.1 < v1.2.2-dev => true",
+			InstallList{
+				&Installation{
+					BinaryPath: "host/org/plugin",
+					Version:    "v1.2.1",
+				},
+				&Installation{
+					BinaryPath: "github.com",
+					Version:    "v1.2.2-dev",
+				},
+			},
+			true,
+		},
+		{
+			"v1.2.3 < v1.2.2-dev => false",
+			InstallList{
+				&Installation{
+					BinaryPath: "host/org/plugin",
+					Version:    "v1.2.3",
+				},
+				&Installation{
+					BinaryPath: "github.com",
+					Version:    "v1.2.2-dev",
+				},
+			},
+			false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			isLess := tt.installs.Less(0, 1)
+			if isLess != tt.expectLess {
+				t.Errorf("Less mismatch for %s < %s, expected %t, got %t",
+					tt.installs[0].Version,
+					tt.installs[1].Version,
+					tt.expectLess, isLess)
+			}
+		})
+	}
+}

packer/plugin.go

@@ -100,16 +100,6 @@ func (c *PluginConfig) Discover() error {
 		}
 
 		pluginName := matches[1]
-
-		// If the plugin is already registered in the plugin map, we
-		// can ignore the current executable, as they're sorted by
-		// version in descending order, so if it's already in the map,
-		// a more recent version was already discovered.
-		_, ok := pluginMap[pluginName]
-		if ok {
-			continue
-		}
-
 		pluginMap[pluginName] = install.BinaryPath
 	}