tags: Ensure complete removal of tags is working

vsphere/helper_test.go

@@ -243,6 +243,27 @@ func testObjectHasTags(s *terraform.State, client *tags.RestClient, obj object.R
 	return nil
 }
 
+// testObjectHasNoTags checks to make sure that an object has no tags attached
+// to it. The parameters are the same as testObjectHasTags, but no tag resource
+// needs to be supplied.
+func testObjectHasNoTags(s *terraform.State, client *tags.RestClient, obj object.Reference) error {
+	objID := obj.Reference().Value
+	objType, err := tagTypeForObject(obj)
+	if err != nil {
+		return err
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), defaultAPITimeout)
+	defer cancel()
+	actualIDs, err := client.ListAttachedTags(ctx, objID, objType)
+	if err != nil {
+		return err
+	}
+	if len(actualIDs) > 0 {
+		return fmt.Errorf("object %q still has tags (%#v)", obj.Reference().Value, actualIDs)
+	}
+	return nil
+}
+
 // testGetDatastore gets the datastore at the supplied full address. This
 // function works for multiple datastore resources (example:
 // vsphere_nas_datastore and vsphere_vmfs_datastore), hence the need for the

vsphere/resource_vsphere_folder_test.go

@@ -476,6 +476,24 @@ func testAccResourceVSphereFolderCheckTags(tagResName string) resource.TestCheck
 	}
 }
 
+// testAccResourceVSphereFolderCheckNoTags is a check to ensure that a folder
+// has no tags on it. This is used by the vsphere_tag tests specifically to
+// test to make sure that complete tag removal is explicitly working without
+// having to rely on the simple empty diff test after the final step.
+func testAccResourceVSphereFolderCheckNoTags() resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		folder, err := testGetFolder(s, "folder")
+		if err != nil {
+			return err
+		}
+		tagsClient, err := testAccProvider.Meta().(*VSphereClient).TagsClient()
+		if err != nil {
+			return err
+		}
+		return testObjectHasNoTags(s, tagsClient, folder)
+	}
+}
+
 // testAccResourceVSphereFolderCreateOOB creates an out-of-band folder that is
 // not tracked by TF. This is used in deletion checks to make sure we don't
 // perform unsafe recursive deletions.

vsphere/resource_vsphere_tag_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 
@@ -88,6 +89,31 @@ func TestAccResourceVSphereTag(t *testing.T) {
 				},
 			},
 		},
+		{
+			"detach all tags",
+			resource.TestCase{
+				PreCheck: func() {
+					testAccPreCheck(tp)
+				},
+				Providers:    testAccProviders,
+				CheckDestroy: testAccResourceVSphereTagExists(false),
+				Steps: []resource.TestStep{
+					{
+						Config: testAccResourceVSphereTagConfigOnFolderAttached(),
+						Check: resource.ComposeTestCheckFunc(
+							testAccResourceVSphereTagExists(true),
+						),
+					},
+					{
+						Config: testAccResourceVSphereTagConfigOnFolderNotAttached(),
+						Check: resource.ComposeTestCheckFunc(
+							testAccResourceVSphereTagExists(true),
+							testAccResourceVSphereFolderCheckNoTags(),
+						),
+					},
+				},
+			},
+		},
 		{
 			"import",
 			resource.TestCase{
@@ -259,3 +285,67 @@ resource "vsphere_tag" "terraform-test-tag" {
   category_id = "${vsphere_tag_category.terraform-test-category.id}"
 }
 `
+
+func testAccResourceVSphereTagConfigOnFolderAttached() string {
+	return fmt.Sprintf(`
+data "vsphere_datacenter" "dc" {
+  name = "%s"
+}
+
+resource "vsphere_tag_category" "terraform-test-category" {
+  name        = "terraform-test-category"
+  cardinality = "SINGLE"
+
+  associable_types = [
+    "Folder",
+  ]
+}
+
+resource "vsphere_tag" "terraform-test-tag" {
+  name        = "terraform-test-tag"
+  description = "Managed by Terraform"
+  category_id = "${vsphere_tag_category.terraform-test-category.id}"
+}
+
+resource "vsphere_folder" "folder" {
+  path          = "terraform-test-folder"
+  type          = "vm"
+  datacenter_id = "${data.vsphere_datacenter.dc.id}"
+
+  tags = ["${vsphere_tag.terraform-test-tag.id}"]
+}
+`,
+		os.Getenv("VSPHERE_DATACENTER"),
+	)
+}
+
+func testAccResourceVSphereTagConfigOnFolderNotAttached() string {
+	return fmt.Sprintf(`
+data "vsphere_datacenter" "dc" {
+  name = "%s"
+}
+
+resource "vsphere_tag_category" "terraform-test-category" {
+  name        = "terraform-test-category"
+  cardinality = "SINGLE"
+
+  associable_types = [
+    "Folder",
+  ]
+}
+
+resource "vsphere_tag" "terraform-test-tag" {
+  name        = "terraform-test-tag"
+  description = "Managed by Terraform"
+  category_id = "${vsphere_tag_category.terraform-test-category.id}"
+}
+
+resource "vsphere_folder" "folder" {
+  path          = "terraform-test-folder"
+  type          = "vm"
+  datacenter_id = "${data.vsphere_datacenter.dc.id}"
+}
+`,
+		os.Getenv("VSPHERE_DATACENTER"),
+	)
+}

vsphere/tags_helper.go

@@ -323,7 +323,8 @@ func (p *tagDiffProcessor) processDetachOperations() error {
 // make sure it's worth proceeding with most of the operation. The returned
 // client should be checked for nil before passing it to processTagDiff.
 func tagsClientIfDefined(d *schema.ResourceData, meta interface{}) (*tags.RestClient, error) {
-	if _, ok := d.GetOk(vSphereTagAttributeKey); ok {
+	old, new := d.GetChange(vSphereTagAttributeKey)
+	if len(old.(*schema.Set).List()) > 0 || len(new.(*schema.Set).List()) > 0 {
 		client, err := meta.(*VSphereClient).TagsClient()
 		if err != nil {
 			return nil, err