Fix the tanzu plugin download-bundle when same plugin is part of mu…

…ltiple plugin-groups (vmware-tanzu#630)

* Fix download-bundle when the same plugin is part of multiple plugin-groups

pkg/airgapped/plugin_bundle_download.go

@@ -170,6 +170,11 @@ func (o *DownloadPluginBundleOptions) getSelectedPluginInfo() ([]*plugininventor
 			selectedPluginEntries = append(selectedPluginEntries, pluginEntries...)
 		}
 	}
+
+	// Remove duplicate PluginInventoryEntries and PluginGroups from the selected list
+	selectedPluginEntries = plugininventory.RemoveDuplicatePluginInventoryEntries(selectedPluginEntries)
+	selectedPluginGroups = plugininventory.RemoveDuplicatePluginGroups(selectedPluginGroups)
+
 	return selectedPluginEntries, selectedPluginGroups, nil
 }
 

pkg/airgapped/plugin_bundle_test.go

@@ -109,6 +109,25 @@ var _ = Describe("Unit tests for download and upload bundle", func() {
 			},
 		},
 	}
+	pluginGroupEntry2 := &plugininventory.PluginGroup{
+		Vendor:             "fakevendor",
+		Publisher:          "fakepublisher",
+		Name:               "default2",
+		Description:        "Desc for plugin",
+		Hidden:             false,
+		RecommendedVersion: "v1.0.0",
+		Versions: map[string][]*plugininventory.PluginGroupPluginEntry{
+			"v1.0.0": {
+				&plugininventory.PluginGroupPluginEntry{
+					PluginIdentifier: plugininventory.PluginIdentifier{
+						Name:    "bar",
+						Target:  "kubernetes",
+						Version: "v0.0.1",
+					},
+				},
+			},
+		},
+	}
 
 	// plugin entry bar to be added in the inventory database
 	essentialPluginEntryTelemetry := &plugininventory.PluginInventoryEntry{
@@ -223,6 +242,8 @@ imagesToCopy:
 		Expect(err).ToNot(HaveOccurred())
 		err = db.InsertPluginGroup(pluginGroupEntry, true)
 		Expect(err).ToNot(HaveOccurred())
+		err = db.InsertPluginGroup(pluginGroupEntry2, true)
+		Expect(err).ToNot(HaveOccurred())
 
 		err = db.InsertPlugin(essentialPluginEntryTelemetry)
 		Expect(err).ToNot(HaveOccurred())
@@ -361,7 +382,8 @@ imagesToCopy:
 			fakeImageOperations.DownloadImageAndSaveFilesToDirCalls(downloadInventoryImageAndSaveFilesToDirStub)
 			fakeImageOperations.CopyImageToTarCalls(copyImageToTarStub)
 
-			dpbo.Groups = []string{"fakevendor-fakepublisher/default:v1.0.0"}
+			// Provide 2 plugingroups with overlapping plugins and verify DownloadPluginBundle does not fail
+			dpbo.Groups = []string{"fakevendor-fakepublisher/default:v1.0.0", "fakevendor-fakepublisher/default2:v1.0.0"}
 			err := dpbo.DownloadPluginBundle()
 			Expect(err).NotTo(HaveOccurred())
 
@@ -382,7 +404,7 @@ imagesToCopy:
 			err = yaml.Unmarshal(bytes, &manifest)
 			Expect(err).NotTo(HaveOccurred())
 
-			// Iterate through all the images in the manifest and verify the all image archive
+			// Iterate through all the images in the manifest and verify that all image archive
 			// files mentioned in the manifest exists in the bundle
 			for _, pi := range manifest.ImagesToCopy {
 				exists := utils.PathExists(filepath.Join(tempDir, PluginBundleDirName, pi.SourceTarFilePath))

pkg/plugininventory/plugin_inventory.go

@@ -214,3 +214,33 @@ func (p PluginGroupSorter) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
 func (p PluginGroupSorter) Less(i, j int) bool {
 	return PluginGroupToID(p[i]) < PluginGroupToID(p[j])
 }
+
+// RemoveDuplicatePluginInventoryEntries removes the duplicate PluginInventoryEntries
+// based on Name, Target and RecommendedVersion fields.
+func RemoveDuplicatePluginInventoryEntries(entries []*PluginInventoryEntry) []*PluginInventoryEntry {
+	encountered := make(map[string]bool)
+	result := []*PluginInventoryEntry{}
+	for _, entry := range entries {
+		id := fmt.Sprintf("%s-%s-%s", entry.Name, entry.Target, entry.RecommendedVersion)
+		if !encountered[id] {
+			encountered[id] = true
+			result = append(result, entry)
+		}
+	}
+	return result
+}
+
+// RemoveDuplicatePluginGroups removes the duplicate pluginGroups from the list
+// based on Vendor, Publisher, Name and RecommendedVersion fields.
+func RemoveDuplicatePluginGroups(groups []*PluginGroup) []*PluginGroup {
+	encountered := make(map[string]bool)
+	result := []*PluginGroup{}
+	for _, gp := range groups {
+		id := fmt.Sprintf("%s-%s/%s:%s", gp.Vendor, gp.Publisher, gp.Name, gp.RecommendedVersion)
+		if !encountered[id] {
+			encountered[id] = true
+			result = append(result, gp)
+		}
+	}
+	return result
+}

pkg/plugininventory/plugin_inventory_test.go

@@ -0,0 +1,87 @@
+// Copyright 2023 VMware, Inc. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package plugininventory
+
+import (
+	"reflect"
+	"testing"
+
+	configtypes "github.com/vmware-tanzu/tanzu-plugin-runtime/config/types"
+)
+
+func TestRemoveDuplicatePluginInventoryEntries(t *testing.T) {
+	entry1 := &PluginInventoryEntry{Name: "Plugin1", Target: configtypes.TargetK8s, RecommendedVersion: "1.0"}
+	entry2 := &PluginInventoryEntry{Name: "Plugin2", Target: configtypes.TargetK8s, RecommendedVersion: "2.0"}
+	entry3 := &PluginInventoryEntry{Name: "Plugin1", Target: configtypes.TargetK8s, RecommendedVersion: "1.0"} // Duplicate
+	entry4 := &PluginInventoryEntry{Name: "Plugin3", Target: configtypes.TargetK8s, RecommendedVersion: "3.0"}
+
+	tests := []struct {
+		name   string
+		input  []*PluginInventoryEntry
+		output []*PluginInventoryEntry
+	}{
+		{
+			name:   "NoDuplicates",
+			input:  []*PluginInventoryEntry{entry1, entry2, entry4},
+			output: []*PluginInventoryEntry{entry1, entry2, entry4},
+		},
+		{
+			name:   "WithDuplicates",
+			input:  []*PluginInventoryEntry{entry1, entry2, entry3, entry4},
+			output: []*PluginInventoryEntry{entry1, entry2, entry4},
+		},
+		{
+			name:   "EmptyInput",
+			input:  []*PluginInventoryEntry{},
+			output: []*PluginInventoryEntry{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := RemoveDuplicatePluginInventoryEntries(tt.input)
+			if !reflect.DeepEqual(got, tt.output) {
+				t.Errorf("TestCase: %v, RemoveDuplicatePluginInventoryEntries() = %v, want %v", tt.name, got, tt.output)
+			}
+		})
+	}
+}
+
+func TestRemoveDuplicatePluginGroups(t *testing.T) {
+	entry1 := &PluginGroup{Vendor: "vmware", Publisher: "tkg", Name: "Plugin1", RecommendedVersion: "v1.0"}
+	entry2 := &PluginGroup{Vendor: "vmware", Publisher: "tkg", Name: "Plugin2", RecommendedVersion: "v2.0"}
+	entry3 := &PluginGroup{Vendor: "vmware", Publisher: "tkg", Name: "Plugin1", RecommendedVersion: "v1.0"} // Duplicate
+	entry4 := &PluginGroup{Vendor: "vmware", Publisher: "tkg", Name: "Plugin1", RecommendedVersion: "v1.1"}
+
+	tests := []struct {
+		name   string
+		input  []*PluginGroup
+		output []*PluginGroup
+	}{
+		{
+			name:   "NoDuplicates",
+			input:  []*PluginGroup{entry1, entry2, entry4},
+			output: []*PluginGroup{entry1, entry2, entry4},
+		},
+		{
+			name:   "WithDuplicates",
+			input:  []*PluginGroup{entry1, entry2, entry3, entry4},
+			output: []*PluginGroup{entry1, entry2, entry4},
+		},
+		{
+			name:   "EmptyInput",
+			input:  []*PluginGroup{},
+			output: []*PluginGroup{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := RemoveDuplicatePluginGroups(tt.input)
+			if !reflect.DeepEqual(got, tt.output) {
+				t.Errorf("TestCase: %v, RemoveDuplicatePluginGroups() = %v, want %v", tt.name, got, tt.output)
+			}
+		})
+	}
+}

test/e2e/airgapped/airgapped_test.go

@@ -189,10 +189,12 @@ var _ = framework.CLICoreDescribe("[Tests:E2E][Feature:Airgapped-Plugin-Download
 
 	})
 
-	Context("Download plugin bundle, Upload plugin bundle and plugin lifecycle tests with plugin group 'vmware-tmc/tmc-user:v0.0.1'", func() {
-		// Test case: download plugin bundle for plugin-group vmware-tmc/tmc-user:v0.0.1
+	Context("Download plugin bundle, Upload plugin bundle and plugin lifecycle tests with plugin group 'vmware-tmc/tmc-user:v0.0.1' provided 2 times", func() {
+		// Test case: download plugin bundle for plugin-groups vmware-tmc/tmc-user:v0.0.1 and vmware-tmc/tmc-user:v0.0.1
+		// Note: we are passing same plugin group multiple times to make sure we test the conflicts in the plugin groups
+		// as well as plugins itself are handled properly while downloading and uploading bundle
 		It("download plugin bundle for plugin-group vmware-tmc/tmc-user:v0.0.1", func() {
-			err := tf.PluginCmd.DownloadPluginBundle(e2eTestLocalCentralRepoImage, []string{"vmware-tmc/tmc-user:v0.0.1"}, filepath.Join(tempDir, "plugin_bundle_vmware-tmc-v0.0.1.tar.gz"))
+			err := tf.PluginCmd.DownloadPluginBundle(e2eTestLocalCentralRepoImage, []string{"vmware-tmc/tmc-user:v0.0.1", "vmware-tmc/tmc-user:v0.0.1"}, filepath.Join(tempDir, "plugin_bundle_vmware-tmc-v0.0.1.tar.gz"))
 			Expect(err).To(BeNil(), "should not get any error while downloading plugin bundle with specific group")
 		})