Include govcd with catalog regression fix for GetCatalogByName and GetCatalogById

.changes/v3.3.1/691-bug-fixes.md

@@ -0,0 +1 @@
+* (Issue #689) `resource/vcd_vm` and `resource/vcd_vapp_vm` cannot find templates in shared catalogs from other Orgs [GH-691]

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 3.3.1 (5 July, 2021)
+
+## BUG FIXES
+* (Issue #689) `resource/vcd_vm` and `resource/vcd_vapp_vm` cannot find templates in shared catalogs from other Orgs 
+  ([#691](https://github.com/vmware/terraform-provider-vcd/pull/691))
+
 ## 3.3.0 (June 30, 2021)
 
 ## FEATURES

PREVIOUS_VERSION

@@ -1 +1 @@
-v3.2.0
+v3.3.0

VERSION

@@ -1 +1 @@
-v3.3.0
+v3.3.1

go.mod

@@ -7,5 +7,5 @@ require (
 	github.com/hashicorp/go-version v1.2.1
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.4.4
 	github.com/kr/pretty v0.2.1
-	github.com/vmware/go-vcloud-director/v2 v2.12.0
+	github.com/vmware/go-vcloud-director/v2 v2.12.1
 )

go.sum

@@ -288,8 +288,8 @@ github.com/ulikunitz/xz v0.5.8/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oW
 github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
 github.com/vmihailenco/msgpack v4.0.4+incompatible h1:dSLoQfGFAo3F6OoNhwUmLwVgaUXK79GlxNBwueZn0xI=
 github.com/vmihailenco/msgpack v4.0.4+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
-github.com/vmware/go-vcloud-director/v2 v2.12.0 h1:macKuZuPNrj7MRNcrzFTnV4nbEUIKSMWatqnEvUfeoc=
-github.com/vmware/go-vcloud-director/v2 v2.12.0/go.mod h1:poaOwg7CoXO4m9Pv4TVhMpNF1wQQwKzxpdGYTfjzajs=
+github.com/vmware/go-vcloud-director/v2 v2.12.1 h1:PO+dVQyZlECX87oVWgLb4/S2v5hiqKREVGsAwTCJHbQ=
+github.com/vmware/go-vcloud-director/v2 v2.12.1/go.mod h1:poaOwg7CoXO4m9Pv4TVhMpNF1wQQwKzxpdGYTfjzajs=
 github.com/xanzy/ssh-agent v0.2.1 h1:TCbipTQL2JiiCprBWx9frJ2eJlCYT00NmctrHxVAr70=
 github.com/xanzy/ssh-agent v0.2.1/go.mod h1:mLlQY/MoOhWBj+gOGMQkOeiEvkx+8pJSI+0Bx9h2kr4=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

vcd/provider_test.go

@@ -3,6 +3,9 @@
 package vcd
 
 import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
 	"strings"
 	"testing"
 
@@ -80,6 +83,46 @@ func createTemporaryVCDConnection() *VCDClient {
 	return conn
 }
 
+// createSystemTemporaryVCDConnection is like createTemporaryVCDConnection but it will ignore all conditional
+// configurations like `VCD_TEST_ORG_USER=1` and will still return a System client instead of user one. This allows to
+// perform System actions (entities which require System rights - Org, Vdc, etc...)
+func createSystemTemporaryVCDConnection() *VCDClient {
+	var configStruct TestConfig
+	configFileName := getConfigFileName()
+
+	// Looks if the configuration file exists before attempting to read it
+	if configFileName == "" {
+		panic(fmt.Errorf("configuration file %s not found", configFileName))
+	}
+	jsonFile, err := ioutil.ReadFile(configFileName)
+	if err != nil {
+		panic(fmt.Errorf("could not read config file %s: %v", configFileName, err))
+	}
+	err = json.Unmarshal(jsonFile, &configStruct)
+	if err != nil {
+		panic(fmt.Errorf("could not unmarshal json file: %v", err))
+	}
+
+	config := Config{
+		User:            configStruct.Provider.User,
+		Password:        configStruct.Provider.Password,
+		Token:           configStruct.Provider.Token,
+		UseSamlAdfs:     configStruct.Provider.UseSamlAdfs,
+		CustomAdfsRptId: configStruct.Provider.CustomAdfsRptId,
+		SysOrg:          configStruct.Provider.SysOrg,
+		Org:             configStruct.VCD.Org,
+		Vdc:             configStruct.VCD.Vdc,
+		Href:            configStruct.Provider.Url,
+		InsecureFlag:    configStruct.Provider.AllowInsecure,
+		MaxRetryTimeout: configStruct.Provider.MaxRetryTimeout,
+	}
+	conn, err := config.Client()
+	if err != nil {
+		panic("unable to initialize VCD connection :" + err.Error())
+	}
+	return conn
+}
+
 // minIfLess returns:
 // `min` if `value` is less than min
 // `value` if `value` > `min`

vcd/resource_vcd_catalog_test.go

@@ -6,6 +6,10 @@ import (
 	"fmt"
 	"regexp"
 	"testing"
+	"time"
+
+	"github.com/vmware/go-vcloud-director/v2/govcd"
+	"github.com/vmware/go-vcloud-director/v2/types/v56"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
@@ -226,3 +230,272 @@ resource "vcd_catalog" "test-catalog" {
   delete_recursive  = "true"
 }
 `
+
+// TestAccVcdCatalogSharedAccess is a test to cover bugfix when Organization Administrator is not able to lookup shared
+// catalog from another Org
+// Because of limited Terraform acceptance test functionality it uses go-vcloud-director SDK to pre-configure
+// environment explicitly using System Org (even if it the test is run as Org user). The following objects are created
+// using SDK (their cleanup is deferred):
+// * Org
+// * Vdc inside newly created Org
+// * Catalog inside newly created Org. This catalog is shared with Org defined in testConfig.VCD.Org variable
+// * Uploads A minimal vApp template to save on upload / VM spawn time
+//
+// After these objects are pre-created using SDK, terraform definition is used to spawn a VM by using template in a
+// catalog from another Org. This test works in both System and Org admin roles but the bug (which was introduced in SDK
+// v2.12.0 and terraform-provider-vcd v3.3.0) occurred only for Organization Administrator user.
+//
+// Original issue -  https://github.com/vmware/terraform-provider-vcd/issues/689
+func TestAccVcdCatalogSharedAccess(t *testing.T) {
+	preTestChecks(t)
+	// This test manipulates VCD during runtime using SDK and is not possible to run as binary or upgrade test
+	if vcdShortTest {
+		t.Skip(acceptanceTestsSkipped)
+		return
+	}
+
+	// initiate System client ignoring value of `VCD_TEST_ORG_USER` flag and create pre-requisite objects
+	systemClient := createSystemTemporaryVCDConnection()
+	catalog, vdc, oldOrg, newOrg, err := spawnTestOrgVdcSharedCatalog(systemClient, t.Name())
+	if err != nil {
+		testOrgVdcSharedCatalogCleanUp(catalog, vdc, oldOrg, newOrg, t)
+		t.Fatalf("%s", err)
+	}
+	// call cleanup ath the end of the test with any of the entities that have been created up to that point
+	defer func() { testOrgVdcSharedCatalogCleanUp(catalog, vdc, oldOrg, newOrg, t) }()
+
+	var params = StringMap{
+		"Org":               testConfig.VCD.Org,
+		"Vdc":               testConfig.VCD.Vdc,
+		"TestName":          t.Name(),
+		"SharedCatalog":     t.Name(),
+		"SharedCatalogItem": "vapp-template",
+		"Tags":              "catalog",
+	}
+
+	configText1 := templateFill(testAccCheckVcdCatalogShared, params)
+	debugPrintf("#[DEBUG] CONFIGURATION: %s", configText1)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: testAccProviders,
+		CheckDestroy: resource.ComposeTestCheckFunc(
+			testAccCheckVcdVAppVmDestroy(t.Name()),
+			testAccCheckVcdStandaloneVmDestroy("test-standalone-vm", "", ""),
+		),
+
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: configText1,
+				Check: resource.ComposeTestCheckFunc(
+					// There is no need to check for much resources - the main point is to have the VMs created
+					// without failures for catalog lookups or similar problems
+					resource.TestCheckResourceAttrSet("vcd_vm.test-vm", "id"),
+					resource.TestCheckResourceAttrSet("vcd_vapp.singleVM", "id"),
+					resource.TestCheckResourceAttrSet("vcd_vapp_vm.test-vm", "id"),
+				),
+			},
+		},
+	})
+
+	postTestChecks(t)
+}
+
+const testAccCheckVcdCatalogShared = `
+resource "vcd_vm" "test-vm" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  name          = "test-standalone-vm"
+  catalog_name  = "{{.SharedCatalog}}"
+  template_name = "{{.SharedCatalogItem}}"
+  power_on      = false
+}
+
+resource "vcd_vapp" "singleVM" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  name = "{{.TestName}}"
+}
+
+resource "vcd_vapp_vm" "test-vm" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  vapp_name     = vcd_vapp.singleVM.name
+  name          = "test-vapp-vm"
+  catalog_name  = "{{.SharedCatalog}}"
+  template_name = "{{.SharedCatalogItem}}"
+  power_on      = false
+
+  depends_on = [vcd_vapp.singleVM]
+}
+`
+
+// spawnTestOrgVdcSharedCatalog spawns an Org to be used in tests
+func spawnTestOrgVdcSharedCatalog(client *VCDClient, name string) (govcd.AdminCatalog, *govcd.Vdc, *govcd.AdminOrg, *govcd.AdminOrg, error) {
+	fmt.Println("# Setting up prerequisites using SDK (non Terraform definitions)")
+	fmt.Printf("# Using user 'System' (%t) to prepare environment\n", client.Client.IsSysAdmin)
+
+	existingOrg, err := client.GetAdminOrgByName(testConfig.VCD.Org)
+	if err != nil {
+		return govcd.AdminCatalog{}, nil, nil, nil, fmt.Errorf("error getting existing Org '%s': %s", testConfig.VCD.Org, err)
+	}
+	task, err := govcd.CreateOrg(client.VCDClient, name, name, name, existingOrg.AdminOrg.OrgSettings, true)
+	if err != nil {
+		return govcd.AdminCatalog{}, nil, existingOrg, nil, fmt.Errorf("error creating Org '%s': %s", name, err)
+	}
+	err = task.WaitTaskCompletion()
+	if err != nil {
+		return govcd.AdminCatalog{}, nil, existingOrg, nil, fmt.Errorf("task failed for Org '%s' creation: %s", name, err)
+	}
+	newAdminOrg, err := client.GetAdminOrgByName(name)
+	if err != nil {
+		return govcd.AdminCatalog{}, nil, existingOrg, nil, fmt.Errorf("error getting new Org '%s': %s", name, err)
+	}
+	fmt.Printf("# Created new Org '%s'\n", newAdminOrg.AdminOrg.Name)
+
+	existingVdc, err := existingOrg.GetAdminVDCByName(testConfig.VCD.Vdc, false)
+	if err != nil {
+		return govcd.AdminCatalog{}, nil, existingOrg, newAdminOrg, fmt.Errorf("error retrieving existing VDC '%s': %s", testConfig.VCD.Vdc, err)
+
+	}
+	vdcConfiguration := &types.VdcConfiguration{
+		Name:            name + "-VDC",
+		Xmlns:           types.XMLNamespaceVCloud,
+		AllocationModel: "Flex",
+		ComputeCapacity: []*types.ComputeCapacity{
+			&types.ComputeCapacity{
+				CPU: &types.CapacityWithUsage{
+					Units:     "MHz",
+					Allocated: 1024,
+					Limit:     1024,
+				},
+				Memory: &types.CapacityWithUsage{
+					Allocated: 1024,
+					Limit:     1024,
+					Units:     "MB",
+				},
+			},
+		},
+		VdcStorageProfile: []*types.VdcStorageProfileConfiguration{&types.VdcStorageProfileConfiguration{
+			Enabled: true,
+			Units:   "MB",
+			Limit:   1024,
+			Default: true,
+			ProviderVdcStorageProfile: &types.Reference{
+				HREF: getVdcProviderVdcStorageProfileHref(client),
+			},
+		},
+		},
+		NetworkPoolReference: &types.Reference{
+			HREF: existingVdc.AdminVdc.NetworkPoolReference.HREF,
+		},
+		ProviderVdcReference: &types.Reference{
+			HREF: existingVdc.AdminVdc.ProviderVdcReference.HREF,
+		},
+		IsEnabled:             true,
+		IsThinProvision:       true,
+		UsesFastProvisioning:  true,
+		IsElastic:             takeBoolPointer(true),
+		IncludeMemoryOverhead: takeBoolPointer(true),
+	}
+
+	vdc, err := newAdminOrg.CreateOrgVdc(vdcConfiguration)
+	if err != nil {
+		return govcd.AdminCatalog{}, nil, existingOrg, newAdminOrg, err
+	}
+	fmt.Printf("# Created new Vdc '%s' inside Org '%s'\n", vdc.Vdc.Name, newAdminOrg.AdminOrg.Name)
+
+	catalog, err := newAdminOrg.CreateCatalog(name, name)
+	if err != nil {
+		return govcd.AdminCatalog{}, vdc, existingOrg, newAdminOrg, err
+	}
+	fmt.Printf("# Created new Catalog '%s' inside Org '%s'\n", catalog.AdminCatalog.Name, newAdminOrg.AdminOrg.Name)
+
+	// Share new Catalog in newOrgName1 with default test Org vcd.Org
+	readOnly := "ReadOnly"
+	accessControl := &types.ControlAccessParams{
+		IsSharedToEveryone:  false,
+		EveryoneAccessLevel: &readOnly,
+		AccessSettings: &types.AccessSettingList{
+			AccessSetting: []*types.AccessSetting{&types.AccessSetting{
+				Subject: &types.LocalSubject{
+					HREF: existingOrg.AdminOrg.HREF,
+					Name: existingOrg.AdminOrg.Name,
+					Type: types.MimeOrg,
+				},
+				AccessLevel: "ReadOnly",
+			}},
+		},
+	}
+	err = catalog.SetAccessControl(accessControl, false)
+	if err != nil {
+		return catalog, vdc, existingOrg, newAdminOrg, err
+	}
+	fmt.Printf("# Shared new Catalog '%s' with existing Org '%s'\n",
+		catalog.AdminCatalog.Name, existingOrg.AdminOrg.Name)
+
+	uploadTask, err := catalog.UploadOvf(testConfig.Ova.OvaPath, "vapp-template", "upload from test", 1024)
+	if err != nil {
+		return catalog, vdc, existingOrg, newAdminOrg, fmt.Errorf("error uploading template: %s", err)
+	}
+
+	err = uploadTask.WaitTaskCompletion()
+	if err != nil {
+		return catalog, vdc, existingOrg, newAdminOrg, fmt.Errorf("error uploading template: %s", err)
+	}
+	fmt.Printf("# Uploaded vApp template '%s' to shared new Catalog '%s' in new Org '%s' with existing Org '%s'\n",
+		"vapp-template", catalog.AdminCatalog.Name, newAdminOrg.AdminOrg.Name, existingOrg.AdminOrg.Name)
+
+	return catalog, vdc, existingOrg, newAdminOrg, nil
+}
+
+func testOrgVdcSharedCatalogCleanUp(catalog govcd.AdminCatalog, vdc *govcd.Vdc, existingOrg, newAdminOrg *govcd.AdminOrg, t *testing.T) {
+	fmt.Println("# Cleaning up")
+	var err error
+	if catalog != (govcd.AdminCatalog{}) {
+		err = catalog.Delete(true, true)
+		if err != nil {
+			t.Errorf("error cleaning up catalog: %s", err)
+		}
+		// The catalog.Delete ignores the task returned and does not wait for the operation to complete. This code
+		// was made for a particular bugfix therefore other parts of code were not altered/fixed.
+		for i := 0; i < 30; i++ {
+			_, err := existingOrg.GetAdminCatalogById(catalog.AdminCatalog.ID, true)
+			if govcd.ContainsNotFound(err) {
+				break
+			} else {
+				time.Sleep(time.Second)
+			}
+		}
+	}
+
+	if vdc != nil {
+		err = vdc.DeleteWait(true, true)
+		if err != nil {
+			t.Errorf("error cleaning up VDC: %s", err)
+		}
+	}
+
+	if newAdminOrg != nil {
+		err = newAdminOrg.Refresh()
+		if err != nil {
+			t.Errorf("error refreshing Org: %s", err)
+		}
+		err = newAdminOrg.Delete(true, true)
+		if err != nil {
+			t.Errorf("error cleaning up Org: %s", err)
+		}
+	}
+}
+
+func getVdcProviderVdcStorageProfileHref(client *VCDClient) string {
+	results, _ := client.QueryWithNotEncodedParams(nil, map[string]string{
+		"type":   "providerVdcStorageProfile",
+		"filter": fmt.Sprintf("name==%s", testConfig.VCD.ProviderVdc.StorageProfile2),
+	})
+	providerVdcStorageProfileHref := results.Results.ProviderVdcStorageProfileRecord[0].HREF
+	return providerVdcStorageProfileHref
+}

vcd/resource_vcd_vapp_vm_checks_test.go

@@ -1,4 +1,4 @@
-// +build vapp vm ALL functional
+// +build vapp vm catalog ALL functional
 
 package vcd