Create vApp VM Security Tag Tests

.changes/v3.9.0/1006-improvements.md

@@ -1 +1 @@
-* `vcd_vapp_vm` and `vcd_vm` resources support security tag management via new field `security_tags` [GH-1006]
+* `vcd_vapp_vm` and `vcd_vm` resources support security tag management via new field `security_tags` [GH-1006,GH-1046]

scripts/skip-upgrade-tests.txt

@@ -268,3 +268,5 @@ vcd.ResourceSchema-vcd_nsxt_ipsec_vpn_tunnel.tf v3.8.2 "New fields 'authenticati
 vcd.ResourceSchema-vcd_nsxt_alb_virtual_service.tf v3.8.2 "New field 'is_transparent_mode_enabled'"
 vcd.ResourceSchema-vcd_nsxt_alb_pool.tf v3.8.2 "New field 'member_group_id'"
 vcd.ResourceSchema-vcd_nsxt_network_imported.tf v3.8.2 "New field dvpg_id"
+vcd.ResourceSchema-vcd_vapp_vm.tf v3.9.0 "New field security_tags"
+vcd.ResourceSchema-vcd_vm.tf v3.9.0 "New field security_tags"

vcd/resource_vcd_security_tag_test.go

@@ -1,4 +1,4 @@
-//go:build network || nsxt || ALL || functional
+//go:build network || nsxt || vm || ALL || functional
 
 package vcd
 
@@ -173,6 +173,276 @@ resource "vcd_security_tag" "{{.SecurityTag2}}" {
 }
 `
 
+// TestAccVcdVappVmWithSecurityTags tests security_tags field of a vcd_vapp_vm
+// There is still a vcd_security_tag resource which is responsible for managing
+// the tag in VCD as security_tags field only creates and doesn't remove the
+// tag itself if it is removed from a VM to not break other VMs.
+func TestAccVcdVappVmWithSecurityTags(t *testing.T) {
+	tag1 := strings.ToLower(t.Name() + "-tag1") // security tags are always lowercase in serverside
+	tag2 := strings.ToLower(t.Name() + "-tag2")
+	vAppName := t.Name() + "-vapp"
+	vmName := t.Name() + "-vm"
+
+	var params = StringMap{
+		"Org":          testConfig.VCD.Org,
+		"Vdc":          testConfig.Nsxt.Vdc,
+		"vappName":     vAppName,
+		"vmName":       vmName,
+		"computerName": t.Name() + "-vm",
+		"securityTag1": tag1,
+		"securityTag2": tag2,
+		"FuncName":     t.Name(),
+	}
+	testParamsNotEmpty(t, params)
+
+	configText1 := templateFill(testAccVappVmWithSecurityTagsStep1, params)
+
+	params["FuncName"] = t.Name() + "step2"
+	configText2 := templateFill(testAccVappVmUpdateSecurityTagsStep2, params)
+
+	params["FuncName"] = t.Name() + "step3"
+	configText3 := templateFill(testAccVappVmUpdateSecurityTagsStep3, params)
+
+	params["FuncName"] = t.Name() + "step4DS"
+	configText4DS := templateFill(testAccVappVmUpdateSecurityTagsStep4DS, params)
+
+	params["FuncName"] = t.Name() + "step5"
+	configText5 := templateFill(testAccVappVmUpdateSecurityTagsStep5, params)
+
+	params["FuncName"] = t.Name() + "step6"
+	configText6 := templateFill(testAccVappVmUpdateSecurityTagsStep6, params)
+
+	debugPrintf("#[DEBUG] CONFIGURATION step 1: %s\n", configText1)
+	debugPrintf("#[DEBUG] CONFIGURATION step 2: %s\n", configText2)
+	debugPrintf("#[DEBUG] CONFIGURATION step 3: %s\n", configText3)
+	debugPrintf("#[DEBUG] CONFIGURATION step 4: %s\n", configText4DS)
+	debugPrintf("#[DEBUG] CONFIGURATION step 5: %s\n", configText5)
+	debugPrintf("#[DEBUG] CONFIGURATION step 6: %s\n", configText6)
+
+	if vcdShortTest {
+		t.Skip(acceptanceTestsSkipped)
+		return
+	}
+
+	resourceName := "vcd_vapp_vm." + vmName
+	resource.Test(t, resource.TestCase{
+		ProviderFactories: testAccProviders,
+		// We don't use CheckDestroy to assert that tags are destroyed because VCD takes some time
+		// to clean up orphaned tags
+		Steps: []resource.TestStep{
+			{ // VM with tag
+				Config: configText1,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckSecurityTagCreated(tag1),
+					testAccCheckSecurityTagOnVMCreated(tag1, vAppName, vmName),
+					resource.TestCheckResourceAttr(resourceName, "security_tags.#", "1"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "security_tags.*", tag1),
+				),
+			},
+			{ // VM tag changed
+				Config: configText2,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "security_tags.#", "1"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "security_tags.*", tag2),
+				),
+			},
+			{ // Standalone VM with 1 tag and vApp VM with 2 security tags
+				Config: configText3,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "security_tags.#", "2"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "security_tags.*", tag1),
+					resource.TestCheckTypeSetElemAttr(resourceName, "security_tags.*", tag2),
+					// Standalone VM
+					resource.TestCheckResourceAttr("vcd_vm.standalone", "security_tags.#", "1"),
+					resource.TestCheckTypeSetElemAttr("vcd_vm.standalone", "security_tags.*", tag2),
+				),
+			},
+			{ // Test that standalone and vApp VM data sources report tags correctly
+				Config: configText4DS,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "security_tags.#", "2"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "security_tags.*", tag1),
+					resource.TestCheckTypeSetElemAttr(resourceName, "security_tags.*", tag2),
+					resource.TestCheckResourceAttr("data.vcd_vapp_vm.with-tags", "security_tags.#", "2"),
+					resource.TestCheckTypeSetElemAttr("data.vcd_vapp_vm.with-tags", "security_tags.*", tag1),
+					resource.TestCheckTypeSetElemAttr("data.vcd_vapp_vm.with-tags", "security_tags.*", tag2),
+					resourceFieldsEqual("data.vcd_vapp_vm.with-tags", resourceName, []string{"%"}),
+
+					// Standalone VM
+					resource.TestCheckResourceAttr("data.vcd_vm.with-tags", "security_tags.#", "1"),
+					resource.TestCheckTypeSetElemAttr("data.vcd_vm.with-tags", "security_tags.*", tag2),
+					resourceFieldsEqual("data.vcd_vm.with-tags", "vcd_vm.standalone", []string{"%"}),
+				),
+			},
+			{ // stop managing `security_tags` - tags from previous setting should still be available
+				Config: configText5,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "security_tags.#", "2"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "security_tags.*", tag1),
+					resource.TestCheckTypeSetElemAttr(resourceName, "security_tags.*", tag2),
+					testAccCheckSecurityTagOnVMCreated(tag1, vAppName, vmName),
+					testAccCheckSecurityTagOnVMCreated(tag2, vAppName, vmName),
+				),
+			},
+			{ // Removing security tags using `security_tags = []` notation
+				Config: configText6,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "security_tags.#", "0"),
+				),
+			},
+		},
+	})
+	postTestChecks(t)
+}
+
+const testAccVappVmWithSecurityTagsStep1 = `
+# skip-binary-test: Too similar to step 3
+resource "vcd_vapp" "{{.vappName}}" {
+  name = "{{.vappName}}"
+  org  = "{{.Org}}"
+  vdc  = "{{.Vdc}}"
+}
+
+resource "vcd_vapp_vm" "{{.vmName}}" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+  vapp_name        = vcd_vapp.{{.vappName}}.name
+  name             = "{{.vmName}}"
+  computer_name    = "{{.computerName}}"
+  memory           = 1024
+  cpus             = 2
+  cpu_cores        = 1
+  os_type          = "sles10_64Guest"
+  hardware_version = "vmx-14"
+  security_tags    = ["{{.securityTag1}}"]
+}
+`
+
+const testAccVappVmUpdateSecurityTagsStep2 = `
+# skip-binary-test: Too similar to step 3
+resource "vcd_vapp" "{{.vappName}}" {
+  name = "{{.vappName}}"
+  org  = "{{.Org}}"
+  vdc  = "{{.Vdc}}"
+}
+
+resource "vcd_vapp_vm" "{{.vmName}}" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  vapp_name        = vcd_vapp.{{.vappName}}.name
+  name             = "{{.vmName}}"
+  computer_name    = "{{.computerName}}"
+  memory           = 1024
+  cpus             = 2
+  cpu_cores        = 1
+  os_type          = "sles10_64Guest"
+  hardware_version = "vmx-14"
+  security_tags    = ["{{.securityTag2}}"]
+}
+`
+
+const testAccVappVmUpdateSecurityTagsStep3 = `
+resource "vcd_vapp" "{{.vappName}}" {
+  name = "{{.vappName}}"
+  org  = "{{.Org}}"
+  vdc  = "{{.Vdc}}"
+}
+
+resource "vcd_vapp_vm" "{{.vmName}}" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  vapp_name        = vcd_vapp.{{.vappName}}.name
+  name             = "{{.vmName}}"
+  computer_name    = "{{.computerName}}"
+  memory           = 1024
+  cpus             = 2
+  cpu_cores        = 1
+  os_type          = "sles10_64Guest"
+  hardware_version = "vmx-14"
+  security_tags    = ["{{.securityTag1}}","{{.securityTag2}}"]
+}
+
+
+resource "vcd_vm" "standalone" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  name             = "{{.vmName}}-standalone"
+  computer_name    = "{{.computerName}}"
+  memory           = 1024
+  cpus             = 2
+  cpu_cores        = 1
+  os_type          = "sles10_64Guest"
+  hardware_version = "vmx-14"
+  security_tags    = ["{{.securityTag2}}"]
+}
+`
+const testAccVappVmUpdateSecurityTagsStep4DS = testAccVappVmUpdateSecurityTagsStep3 + `
+# skip-binary-test: Data Source test
+data "vcd_vapp_vm" "with-tags" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  vapp_name = vcd_vapp.{{.vappName}}.name
+  name      = "{{.vmName}}"
+}
+
+data "vcd_vm" "with-tags" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  name = "{{.vmName}}-standalone"
+}
+`
+
+const testAccVappVmUpdateSecurityTagsStep5 = `
+# skip-binary-test: only useful for update
+resource "vcd_vapp" "{{.vappName}}" {
+  name = "{{.vappName}}"
+  org  = "{{.Org}}"
+  vdc  = "{{.Vdc}}"
+}
+
+resource "vcd_vapp_vm" "{{.vmName}}" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  vapp_name        = vcd_vapp.{{.vappName}}.name
+  name             = "{{.vmName}}"
+  computer_name    = "{{.computerName}}"
+  memory           = 1024
+  cpus             = 2
+  cpu_cores        = 1
+  os_type          = "sles10_64Guest"
+  hardware_version = "vmx-14"
+}
+`
+
+const testAccVappVmUpdateSecurityTagsStep6 = `
+resource "vcd_vapp" "{{.vappName}}" {
+  name = "{{.vappName}}"
+  org  = "{{.Org}}"
+  vdc  = "{{.Vdc}}"
+}
+
+resource "vcd_vapp_vm" "{{.vmName}}" {
+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  vapp_name        = vcd_vapp.{{.vappName}}.name
+  name             = "{{.vmName}}"
+  computer_name    = "{{.computerName}}"
+  memory           = 1024
+  cpus             = 2
+  cpu_cores        = 1
+  os_type          = "sles10_64Guest"
+  hardware_version = "vmx-14"
+  security_tags    = []
+}
+`
+
 func testAccCheckSecurityTagDestroy(securityTags ...string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		conn := testAccProvider.Meta().(*VCDClient)

vcd/resource_vcd_vapp_vm.go

@@ -801,9 +801,11 @@ func genericResourceVmCreate(d *schema.ResourceData, meta interface{}, vmType ty
 	// Handle VM Security Tags settings
 	// Such schema fields are processed:
 	// * security_tags
-	err = createOrUpdateVmSecurityTags(d, vm)
-	if err != nil {
-		return diag.Errorf("[VM create] error creating security tags for VM %s : %s", vm.VM.Name, err)
+	if _, isSet := d.GetOk("security_tags"); isSet {
+		err = createOrUpdateVmSecurityTags(d, vm)
+		if err != nil {
+			return diag.Errorf("[VM create] error creating security tags for VM %s : %s", vm.VM.Name, err)
+		}
 	}
 
 	////////////////////////////////////////////////////////////////////////////////////////////////

vcd/resource_vcd_vapp_vm_tools.go

@@ -1221,7 +1221,7 @@ func createOrUpdateVmSecurityTags(d *schema.ResourceData, vm *govcd.VM) error {
 	var err error
 	entitySecurityTags := &types.EntitySecurityTags{}
 
-	entitySecurityTagsFromSchema, _ := d.GetOk("security_tags")
+	entitySecurityTagsFromSchema := d.Get("security_tags")
 	entitySecurityTagsSlice := convertSchemaSetToSliceOfStrings(entitySecurityTagsFromSchema.(*schema.Set))
 	entitySecurityTags.Tags = entitySecurityTagsSlice
 	log.Printf("[DEBUG] Setting security_tags %s", entitySecurityTags)

website/docs/guides/container_service_extension_4_0.html.markdown

@@ -248,7 +248,7 @@ In order to do so, the [configuration][step2] asks for the following variables t
 - `cpi_version`: The version for CPI. It should be "1.2.0" for CSE v4.0.
 - `csi_version`: The version for CSI. It should be "1.3.0" for CSE v4.0.
 - `github_personal_access_token`: Create this one [here](https://github.com/settings/tokens),
-  this will avoid installation errors caused by GitHub rate limiting, as the TKGm tenant creation process requires downloading
+  this will avoid installation errors caused by GitHub rate limiting, as the TKGm cluster creation process requires downloading
   some Kubernetes components from GitHub.
   The token should have the `public_repo` scope for classic tokens and `Public Repositories` for fine-grained tokens.
 - `cse_admin_user`: This should reference the CSE Administrator [User][user] that was created in Step 1.
@@ -409,4 +409,4 @@ Once all clusters are removed in the background by CSE Server, you may destroy t
 [user]: /providers/vmware/vcd/latest/docs/resources/org_user
 [catalog_vapp_template]: /providers/vmware/vcd/latest/docs/resources/catalog_vapp_template
 [vdc]: /providers/vmware/vcd/latest/docs/resources/org_vdc
-[vm]: /providers/vmware/vcd/latest/docs/resources/vapp_vm
+[vm]: /providers/vmware/vcd/latest/docs/resources/vapp_vm

website/docs/r/security_tag.html.markdown

@@ -14,6 +14,9 @@ used to assign security tag to VMs.
 
 Supported in provider *v3.7+* and requires VCD 10.3.0+
 
+~> **Note:** Only one of `vcd_security_tag` resource or [`security_tags` attribute from `vcd_vapp_vm`](/providers/vmware/vcd/latest/docs/resources/vapp_vm)
+should be used. Using both would cause a behavioral conflict.
+
 -> **Note:** This resource requires either system or org administrator privileges.
 
 ## Example Usage

website/docs/r/vapp_vm.html.markdown

@@ -503,7 +503,13 @@ example for usage details.
   using `vcd_org_vdc.vm_placement_policy_ids` (and optionally `vcd_org_vdc.default_compute_policy_id` to make it default).
   In this case, if the placement policy is not set, it will pick the VDC default on creation. It must be set explicitly
   if one wants to update it to another policy (the VM requires at least one Compute Policy), and needs to be set to `""` to be removed.
-* `security_tags` - (Optional; *v3.9+*) Set of security tags to be managed by the `vcd_vapp_vm` resource. Removing `security_tags` will not remove the security tags themselves. They remain unchanged and just stop being managed by `security_tags` in the `vcd_vapp_vm` resource. This is to keep backwards compatibility with existing security tags that were created by the `vcd_security_tags` resource. Setting `security_tags` to `[]` will remove the security tags.
+* `security_tags` - (Optional; *v3.9+*) Set of security tags to be managed by the `vcd_vapp_vm` resource.
+  To remove `security_tags` you must set `security_tags = []` and do not remove the attribute. Removing the attribute will cause the tags to remain unchanged and just stop being managed by this resource.
+  This is to be consistent with existing security tags that were created by the `vcd_security_tags` resource.
+
+~> **Note:** Only one of `security_tags` attribute or [`vcd_security_tag`](/providers/vmware/vcd/latest/docs/resources/security_tag) resource
+  should be used. Using both would cause a behavioral conflict.
+
 * `catalog_name` - (Deprecated; *v2.9+*) Use a [`vcd_catalog`](/providers/vmware/vcd/latest/docs/data-sources/catalog) data source along with `vapp_template_id` or `boot_image_id` instead. The catalog name in which to find the given vApp Template or media for `boot_image`.
 * `template_name` - (Deprecated; *v2.9+*) Use `vapp_template_id` instead. The name of the vApp Template to use
 * `boot_image` - (Deprecated; *v2.9+*) Use `boot_image_id` instead. Media name to mount as boot image. Image is mounted only during VM creation. On update if value is changed to empty it will eject the mounted media. If you want to mount an image later, please use [vcd_inserted_media](/providers/vmware/vcd/latest/docs/resources/inserted_media).