Merge branch 'main' of https://github.com/adambarreiro/terraform-prov…

…ider-vcd into add-metadata-org-disk-vapptempl

.changes/v3.6.0/793-improvements.md

@@ -1,2 +1,2 @@
-* `resource/vcd_nsxt_edgegateway` and `resource/vcd_nsxt_edgegateway` support VDC groups via new
+* `resource/vcd_nsxt_edgegateway` and `datasource/vcd_nsxt_edgegateway` support VDC Groups via new
   field `owner_id` replacing `vdc` [GH-793]

.changes/v3.6.0/798-features.md

@@ -0,0 +1 @@
+* **New Data Source:** `vcd_org_group` allows to fetch an Organization Group to use it with other resources [GH-798]

.changes/v3.6.0/798-improvements.md

@@ -0,0 +1,4 @@
+* `vcd_org_user` resource and data source have now `is_external` attribute to support the importing of LDAP users into the Organization [GH-798]
+* `vcd_org_user` resource does not have a default value for `deployed_vm_quota` and `stored_vm_quota`. Local users will have unlimited quota by default, imported from LDAP will have no quota [GH-798]
+* `vcd_org_user` resource and data source have now `group_names` attribute to list group names if the user comes from an LDAP group [GH-798]
+* `vcd_org_group` resource and data source have now `user_names` attribute to list user names if the user was imported from LDAP [GH-798]

.changes/v3.6.0/798-notes.md

@@ -0,0 +1 @@
+* Default values for `deployed_vm_quota` and `stored_vm_quota` for `org_user` have changed from 10 to 0 (unlimited) [GH-798]

.changes/v3.6.0/801-improvements.md

@@ -0,0 +1,6 @@
+* `resource/vcd_network_routed_v2` and `datasource/vcd_network_routed_v2` support VDC Groups by
+  inheriting parent VDC or VDC Group from Edge Gateway  [GH-801]
+* `resource/vcd_network_isolated_v2` and `datasource/vcd_network_isolated_v2` support VDC Groups via
+  new field `owner_id` replacing `vdc` [GH-801]
+* `resource/vcd_nsxt_network_imported` and `datasource/vcd_nsxt_network_imported` support VDC Groups
+  via new field `owner_id` replacing `vdc`  [GH-801]

.changes/v3.6.0/801-notes.md

@@ -0,0 +1,6 @@
+* Internal functions `lockParentEdgeGtw`, `unLockParentEdgeGtw`, `lockEdgeGateway`,
+  `unlockEdgeGateway` were converted to use just their ID for lock key instead of full path
+  `org:vdc:edge_id`. This is done because paths for VDC and VDC Groups can differ, but UUID is
+  unique so it makes it simpler to manage [GH-801]
+* Additional locking mechanisms `lockIfOwnerIsVdcGroup`, `unLockIfOwnerIsVdcGroup`, `lockById`,
+  `unlockById` [GH-801]

TESTING.md

@@ -545,3 +545,37 @@ used in the documentation index.
 
 
 When both the environment variable and the command line option are possible, the environment variable gets evaluated first.
+
+## Troubleshooting code issues
+
+### Functions for dumping state and pause during acceptance testing
+
+These functions match signature of Terraform's own `resource.TestCheckResourceAttr` and can be
+dropped in for troubleshooting problems. 
+
+This function will dump the state at the test run (while executing all field evaluations). It can
+help troubleshooting why some fields fail and find typos, wrong state, etc.
+
+```go
+func stateDumper() resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		spew.Dump(s)
+		return nil
+	}
+}
+```
+
+This function can pause test run in the middle which gives the chance to investigate environment
+(UI, API calls, etc)
+
+```go
+func sleepTester() resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		fmt.Println("sleeping")
+		time.Sleep(4 * time.Minute)
+		fmt.Println("finished sleeping")
+		return nil
+	}
+}
+```
+

go.mod

@@ -7,5 +7,5 @@ require (
 	github.com/hashicorp/go-version v1.3.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.10.0
 	github.com/kr/pretty v0.2.1
-	github.com/vmware/go-vcloud-director/v2 v2.15.0-alpha.6
+	github.com/vmware/go-vcloud-director/v2 v2.15.0-alpha.7
 )

go.sum

@@ -319,8 +319,8 @@ github.com/vmihailenco/msgpack v4.0.4+incompatible h1:dSLoQfGFAo3F6OoNhwUmLwVgaU
 github.com/vmihailenco/msgpack v4.0.4+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
 github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4=
 github.com/vmihailenco/tagparser v0.1.1/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI=
-github.com/vmware/go-vcloud-director/v2 v2.15.0-alpha.6 h1:dlhIfKpEmfSteiQw28LUko4Lrx/lnO6oaAzahwC5e1I=
-github.com/vmware/go-vcloud-director/v2 v2.15.0-alpha.6/go.mod h1:2BS1yw61VN34WI0/nUYoInFvBc3Zcuf84d4ESiAAl68=
+github.com/vmware/go-vcloud-director/v2 v2.15.0-alpha.7 h1:GBDCWYojAr8oZ186irUIa3TgImzpuxpYwyZp8NGGJKs=
+github.com/vmware/go-vcloud-director/v2 v2.15.0-alpha.7/go.mod h1:2BS1yw61VN34WI0/nUYoInFvBc3Zcuf84d4ESiAAl68=
 github.com/xanzy/ssh-agent v0.3.0 h1:wUMzuKtKilRgBAD1sUb8gOwwRr2FGoBVumcjoOACClI=
 github.com/xanzy/ssh-agent v0.3.0/go.mod h1:3s9xbODqPuuhK9JV1R321M/FlMZSBvE5aY6eAcqrDh0=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

scripts/runtest.sh

@@ -81,12 +81,12 @@ function unit_test {
     if [ -n "$VERBOSE" ]
     then
         echo "go test -race -i ${TEST} || exit 1"
-        echo "go test -race -tags unit -v -timeout 3m"
+        echo "go test -race -tags unit -v -timeout 5m"
     fi
     if [ -z "$DRY_RUN" ]
     then
         go test -race -i ${TEST} || exit 1
-        go test -race -tags unit -v -timeout 3m
+        go test -race -tags unit -v -timeout 5m
     fi
 }
 
@@ -100,12 +100,12 @@ function short_test {
     if [ -n "$VERBOSE" ]
     then
         echo "go test -race  -i ${TEST} || exit 1"
-        echo "VCD_SHORT_TEST=1 go test -race -tags "functional $MORE_TAGS" -v -timeout 3m"
+        echo "VCD_SHORT_TEST=1 go test -race -tags "functional $MORE_TAGS" -v -timeout 5m"
     fi
     if [ -z "$DRY_RUN" ]
     then
         go test -race -i ${TEST} || exit 1
-        VCD_SHORT_TEST=1 go test -race -tags "functional $MORE_TAGS" -v -timeout 3m
+        VCD_SHORT_TEST=1 go test -race -tags "functional $MORE_TAGS" -v -timeout 5m
         check_exit_code
     fi
     if [ -n "$VCD_TEST_ORG_USER" ]

vcd/config.go

@@ -155,28 +155,26 @@ func (cli *VCDClient) unLockVapp(d *schema.ResourceData) {
 	vcdMutexKV.kvUnlock(key)
 }
 
-// locks an edge gateway resource
-// Differs from lockParentEdgeGtw in the resource name. When EGW is the parent,
-// it's named "edge_gateway". When it's the main resource, it's found at "name"
+// lockEdgeGateway locks an edge gateway resource
+// id field is used as key
 func (cli *VCDClient) lockEdgeGateway(d *schema.ResourceData) {
-	edgeGatewayName := d.Get("name").(string)
-	if edgeGatewayName == "" {
-		panic("edge gateway name not found")
+	edgeGatewayId := d.Id()
+	if edgeGatewayId == "" {
+		panic("edge gateway ID not found")
 	}
-	key := fmt.Sprintf("org:%s|vdc:%s|edge:%s", cli.getOrgName(d), cli.getVdcName(d), edgeGatewayName)
-	vcdMutexKV.kvLock(key)
+
+	vcdMutexKV.kvLock(edgeGatewayId)
 }
 
-// unlocks an edge gateway resource
-// Differs from unlockParentEdgeGtw in the resource name. When EGW is the parent,
-// it's named "edge_gateway". When it's the main resource, it's found at "name"
+// unlockEdgeGateway unlocks an Edge Gateway resource
+// id field is used as key
 func (cli *VCDClient) unlockEdgeGateway(d *schema.ResourceData) {
-	edgeGatewayName := d.Get("name").(string)
-	if edgeGatewayName == "" {
-		panic("edge gateway name not found")
+	edgeGatewayId := d.Id()
+	if edgeGatewayId == "" {
+		panic("edge gateway ID not found")
 	}
-	key := fmt.Sprintf("org:%s|vdc:%s|edge:%s", cli.getOrgName(d), cli.getVdcName(d), edgeGatewayName)
-	vcdMutexKV.kvUnlock(key)
+
+	vcdMutexKV.kvUnlock(edgeGatewayId)
 }
 
 // lockParentVappWithName locks using provided vappName.
@@ -245,10 +243,36 @@ func (cli *VCDClient) unLockParentVm(d *schema.ResourceData) {
 	vcdMutexKV.kvUnlock(key)
 }
 
+// lockById locks on supplied ID field
+func (cli *VCDClient) lockById(id string) {
+	vcdMutexKV.kvLock(id)
+}
+
+// unlockById unlocks on supplied ID field
+func (cli *VCDClient) unlockById(id string) {
+	vcdMutexKV.kvUnlock(id)
+}
+
+// lockIfOwnerIsVdcGroup locks VDC Group based on `owner_id` field (if it is a VDC Group)
+func (cli *VCDClient) lockIfOwnerIsVdcGroup(d *schema.ResourceData) {
+	vdcGroupId := d.Get("owner_id")
+	vdcGroupIdValue := vdcGroupId.(string)
+	if govcd.OwnerIsVdcGroup(vdcGroupIdValue) {
+		vcdMutexKV.kvLock(vdcGroupIdValue)
+	}
+}
+
+// unLockIfOwnerIsVdcGroup unlocks VDC Group based on `owner_id` field (if it is a VDC Group)
+func (cli *VCDClient) unLockIfOwnerIsVdcGroup(d *schema.ResourceData) {
+	vdcGroupId := d.Get("owner_id")
+	vdcGroupIdValue := vdcGroupId.(string)
+	if govcd.OwnerIsVdcGroup(vdcGroupIdValue) {
+		vcdMutexKV.kvUnlock(vdcGroupIdValue)
+	}
+}
+
 // function lockParentEdgeGtw locks using edge_gateway or edge_gateway_id name existing in resource parameters.
-// If edge_gateway_id is present it is being looked up and stored as name in the lock so that resources that use ID
-// and resources that use name can acquire the same lock.
-// Parent means the resource belongs to the edge gateway being locked
+// Edge Gateway is used as a lock key. If only `name` is present in resource - it will find the Edge Gateway itself
 func (cli *VCDClient) lockParentEdgeGtw(d *schema.ResourceData) {
 	var edgeGtwIdValue string
 	var edgeGtwNameValue string
@@ -282,8 +306,7 @@ func (cli *VCDClient) lockParentEdgeGtw(d *schema.ResourceData) {
 		panic("edge gateway ID not found")
 	}
 
-	key := fmt.Sprintf("org:%s|vdc:%s|edge:%s", cli.getOrgName(d), cli.getVdcName(d), edgeGtwIdValue)
-	vcdMutexKV.kvLock(key)
+	vcdMutexKV.kvLock(edgeGtwIdValue)
 }
 
 func (cli *VCDClient) unLockParentEdgeGtw(d *schema.ResourceData) {
@@ -319,8 +342,7 @@ func (cli *VCDClient) unLockParentEdgeGtw(d *schema.ResourceData) {
 		panic("edge gateway ID not found")
 	}
 
-	key := fmt.Sprintf("org:%s|vdc:%s|edge:%s", cli.getOrgName(d), cli.getVdcName(d), edgeGtwIdValue)
-	vcdMutexKV.kvUnlock(key)
+	vcdMutexKV.kvUnlock(edgeGtwIdValue)
 }
 
 func (cli *VCDClient) getOrgName(d *schema.ResourceData) string {
@@ -558,6 +580,7 @@ func (c *Config) Client() (*VCDClient, error) {
 		c.Token + "#" +
 		c.ApiToken + "#" +
 		c.SysOrg + "#" +
+		c.Vdc + "#" +
 		c.Href
 	checksum := fmt.Sprintf("%x", sha256.Sum256([]byte(rawData)))
 

vcd/datasource_vcd_network_isolated_v2.go

@@ -22,18 +22,27 @@ func datasourceVcdNetworkIsolatedV2() *schema.Resource {
 					"level. Useful when connected as sysadmin working across different organizations",
 			},
 			"vdc": {
-				Type:        schema.TypeString,
-				Optional:    true,
-				ForceNew:    true,
-				Description: "The name of VDC to use, optional if defined at provider level",
+				Type:          schema.TypeString,
+				Optional:      true,
+				Computed:      true,
+				Description:   "The name of VDC to use, optional if defined at provider level",
+				Deprecated:    "This field is deprecated in favor of 'owner_id' which supports both - VDC and VDC Group IDs",
+				ConflictsWith: []string{"owner_id"},
+			},
+			"owner_id": {
+				Type:          schema.TypeString,
+				Optional:      true,
+				Computed:      true,
+				Description:   "ID of VDC or VDC Group",
+				ConflictsWith: []string{"vdc"},
 			},
-			"name": &schema.Schema{
+			"name": {
 				Type:         schema.TypeString,
 				Optional:     true,
 				ExactlyOneOf: []string{"name", "filter"},
 				Description:  "A unique name for this network (optional if 'filter' is used)",
 			},
-			"filter": &schema.Schema{
+			"filter": {
 				Type:         schema.TypeList,
 				MaxItems:     1,
 				MinItems:     1,
@@ -47,42 +56,42 @@ func datasourceVcdNetworkIsolatedV2() *schema.Resource {
 					},
 				},
 			},
-			"description": &schema.Schema{
+			"description": {
 				Type:        schema.TypeString,
 				Computed:    true,
 				Description: "Network description",
 			},
-			"is_shared": &schema.Schema{
+			"is_shared": {
 				Type:        schema.TypeBool,
 				Computed:    true,
 				Description: "NSX-V only - share this network with other VDCs in this organization",
 			},
-			"gateway": &schema.Schema{
+			"gateway": {
 				Type:        schema.TypeString,
 				Computed:    true,
 				Description: "Gateway IP address",
 			},
-			"prefix_length": &schema.Schema{
+			"prefix_length": {
 				Type:        schema.TypeInt,
 				Computed:    true,
 				Description: "Network prefix",
 			},
-			"dns1": &schema.Schema{
+			"dns1": {
 				Type:        schema.TypeString,
 				Computed:    true,
 				Description: "DNS server 1",
 			},
-			"dns2": &schema.Schema{
+			"dns2": {
 				Type:        schema.TypeString,
 				Computed:    true,
 				Description: "DNS server 1",
 			},
-			"dns_suffix": &schema.Schema{
+			"dns_suffix": {
 				Type:        schema.TypeString,
 				Computed:    true,
 				Description: "DNS suffix",
 			},
-			"static_ip_pool": &schema.Schema{
+			"static_ip_pool": {
 				Type:        schema.TypeSet,
 				Computed:    true,
 				Description: "IP ranges used for static pool allocation in the network",
@@ -95,38 +104,66 @@ func datasourceVcdNetworkIsolatedV2() *schema.Resource {
 func datasourceVcdNetworkIsolatedV2Read(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	vcdClient := meta.(*VCDClient)
 
-	_, vdc, err := vcdClient.GetOrgAndVdcFromResource(d)
+	org, err := vcdClient.GetOrgFromResource(d)
 	if err != nil {
-		return diag.Errorf("[isolated network read v2] error retrieving VDC: %s", err)
+		return diag.Errorf("error retrieving Org: %s", err)
 	}
 
+	inheritedVdcField := vcdClient.Vdc
+	vdcField := d.Get("vdc").(string)
+	ownerIdField := d.Get("owner_id").(string)
+
 	if !nameOrFilterIsSet(d) {
 		return diag.Errorf(noNameOrFilterError, "vcd_network_isolated_v2")
 	}
 
-	name := d.Get("name").(string)
+	networkName := d.Get("name").(string)
 
 	// Try to search by filter if it exists
 	var network *govcd.OpenApiOrgVdcNetwork
 	filter, hasFilter := d.GetOk("filter")
-	if hasFilter && name == "" {
+	switch {
+	// User supplied `filter`, search in the `vdc` (in data source or inherited)
+	case hasFilter && networkName == "" && (vdcField != "" || inheritedVdcField != ""):
+		_, vdc, err := vcdClient.GetOrgAndVdcFromResource(d)
+		if err != nil {
+			return diag.Errorf("error getting VDC: %s", err)
+		}
 		network, err = getOpenApiOrgVdcNetworkByFilter(vdc, filter, "isolated")
 		if err != nil {
 			return diag.FromErr(err)
 		}
+	// TODO - XML Query based API does not support VDC Group networks (does not return them)
+	// User supplied `filter` and `edge_gateway_id` (search scope can be detected - VDC or VDC Group)
+	// case hasFilter && edgeGatewayId != "":
+	// 	network, err = getOpenApiOrgVdcNetworkByFilter(vdc, filter, "isolated")
+	// 	if err != nil {
+	// 		return diag.FromErr(err)
+	// 	}
+	// User supplied `name` and also `owner_id`
+	case ownerIdField != "" && networkName != "":
+		network, err = org.GetOpenApiOrgVdcNetworkByNameAndOwnerId(networkName, ownerIdField)
+		if err != nil {
+			return diag.Errorf("[isolated network read v2] error getting Org VDC network: %s", err)
+		}
+	// Users supplied only `name` (VDC reference will be used from resource or inherited from provider)
+	case networkName != "":
+		_, vdc, err := vcdClient.GetOrgAndVdcFromResource(d)
+		if err != nil {
+			return diag.Errorf("error getting VDC: %s", err)
+		}
 
-	}
-
-	if name != "" {
 		network, err = vdc.GetOpenApiOrgVdcNetworkByName(d.Get("name").(string))
 		if err != nil {
 			return diag.Errorf("[isolated network read v2] error getting Org VDC network: %s", err)
 		}
+	default:
+		return diag.Errorf("error - not all parameters specified for network lookup")
 	}
 
-	// Fix coverity warning
-	if network == nil {
-		return diag.Errorf("[isolated network read v2] error defining network")
+	if !network.IsIsolated() {
+		return diag.Errorf("[isolated network read v2] Org network with name '%s' found, but is not of type Isolated (ISOLATED) (type is '%s')",
+			network.OpenApiOrgVdcNetwork.Name, network.GetType())
 	}
 
 	err = setOpenApiOrgVdcIsolatedNetworkData(d, network.OpenApiOrgVdcNetwork)