Add tests for Network Tags for provider/cloudstack and applied tips from PR#4742

builtin/providers/cloudstack/resource_cloudstack_network.go

@@ -176,7 +176,10 @@ func resourceCloudStackNetworkCreate(d *schema.ResourceData, meta interface{}) e
 
 	d.SetId(r.Id)
 
-	setTags(cs, d, "network")
+	err = setTags(cs, d, "network")
+	if err != nil {
+		return fmt.Errorf("Error setting tags")
+	}
 
 	return resourceCloudStackNetworkRead(d, meta)
 }
@@ -202,7 +205,7 @@ func resourceCloudStackNetworkRead(d *schema.ResourceData, meta interface{}) err
 	d.Set("cidr", n.Cidr)
 	d.Set("gateway", n.Gateway)
 
-	// Read the tags and sort them on a map
+	// Read the tags and store them in a map
 	tags := make(map[string]string)
 	for item := range n.Tags {
 		tags[n.Tags[item].Key] = n.Tags[item].Value
@@ -258,9 +261,10 @@ func resourceCloudStackNetworkUpdate(d *schema.ResourceData, meta interface{}) e
 			"Error updating network %s: %s", name, err)
 	}
 
-	// Check if tags have changed
-	if d.HasChange("tags") {
-		setTags(cs, d, "network")
+	// Update tags if they have changed
+	err = setTags(cs, d, "network")
+	if err != nil {
+		return fmt.Errorf("Error updating tags")
 	}
 
 	return resourceCloudStackNetworkRead(d, meta)

builtin/providers/cloudstack/resource_cloudstack_network_test.go

@@ -23,6 +23,7 @@ func TestAccCloudStackNetwork_basic(t *testing.T) {
 					testAccCheckCloudStackNetworkExists(
 						"cloudstack_network.foo", &network),
 					testAccCheckCloudStackNetworkBasicAttributes(&network),
+					testAccCheckNetworkTags(&network, "terraform-tag", "true"),
 				),
 			},
 		},
@@ -93,17 +94,28 @@ func testAccCheckCloudStackNetworkBasicAttributes(
 		}
 
 		if network.Cidr != CLOUDSTACK_NETWORK_2_CIDR {
-			return fmt.Errorf("Bad service offering: %s", network.Cidr)
+			return fmt.Errorf("Bad CIDR: %s", network.Cidr)
 		}
 
 		if network.Networkofferingname != CLOUDSTACK_NETWORK_2_OFFERING {
-			return fmt.Errorf("Bad template: %s", network.Networkofferingname)
+			return fmt.Errorf("Bad network offering: %s", network.Networkofferingname)
 		}
 
 		return nil
 	}
 }
 
+func testAccCheckNetworkTags(
+	n *cloudstack.Network, key string, value string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		tags := make(map[string]string)
+		for item := range n.Tags {
+			tags[n.Tags[item].Key] = n.Tags[item].Value
+		}
+		return testAccCheckTags(tags, key, value)
+	}
+}
+
 func testAccCheckCloudStackNetworkVPCAttributes(
 	network *cloudstack.Network) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
@@ -155,6 +167,9 @@ resource "cloudstack_network" "foo" {
   cidr = "%s"
   network_offering = "%s"
   zone = "%s"
+  tags = {
+	terraform-tag = "true"
+  }
 }`,
 	CLOUDSTACK_NETWORK_2_CIDR,
 	CLOUDSTACK_NETWORK_2_OFFERING,

builtin/providers/cloudstack/tags.go

@@ -19,18 +19,17 @@ func tagsSchema() *schema.Schema {
 // setTags is a helper to set the tags for a resource. It expects the
 // tags field to be named "tags"
 func setTags(cs *cloudstack.CloudStackClient, d *schema.ResourceData, resourcetype string) error {
-
 	if d.HasChange("tags") {
 		oraw, nraw := d.GetChange("tags")
 		o := oraw.(map[string]interface{})
 		n := nraw.(map[string]interface{})
-		create, remove := diffTags(tagsFromMap(o), tagsFromMap(n))
-		log.Printf("[DEBUG] tags to remove: %s", remove)
-		log.Printf("[DEBUG] tags to create: %s", create)
+		create, remove := diffTags(tagsFromSchema(o), tagsFromSchema(n))
+		log.Printf("[DEBUG] tags to remove: %v", remove)
+		log.Printf("[DEBUG] tags to create: %v", create)
 
 		// Set tags
 		if len(remove) > 0 {
-			log.Printf("[DEBUG] Removing tags: %s from %s", remove, d.Id())
+			log.Printf("[DEBUG] Removing tags: %v from %s", remove, d.Id())
 			p := cs.Resourcetags.NewDeleteTagsParams([]string{d.Id()}, resourcetype)
 			p.SetTags(remove)
 			_, err := cs.Resourcetags.DeleteTags(p)
@@ -39,7 +38,7 @@ func setTags(cs *cloudstack.CloudStackClient, d *schema.ResourceData, resourcety
 			}
 		}
 		if len(create) > 0 {
-			log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id())
+			log.Printf("[DEBUG] Creating tags: %v for %s", create, d.Id())
 			p := cs.Resourcetags.NewCreateTagsParams([]string{d.Id()}, resourcetype, create)
 			_, err := cs.Resourcetags.CreateTags(p)
 			if err != nil {
@@ -71,8 +70,10 @@ func diffTags(oldTags, newTags map[string]string) (map[string]string, map[string
 	return newTags, remove
 }
 
-// tagsFromMap returns the tags for the given tags schema.
-func tagsFromMap(m map[string]interface{}) map[string]string {
+// tagsFromSchema returns the tags for the given tags schema.
+// It's needed to properly unpack all string:interface types
+// to a proper string:string map
+func tagsFromSchema(m map[string]interface{}) map[string]string {
 	result := make(map[string]string, len(m))
 	for k, v := range m {
 		result[k] = v.(string)

builtin/providers/cloudstack/tags_test.go

@@ -0,0 +1,76 @@
+package cloudstack
+
+import (
+	"fmt"
+	"reflect"
+	"testing"
+)
+
+func TestDiffTags(t *testing.T) {
+	cases := []struct {
+		Old, New       map[string]interface{}
+		Create, Remove map[string]string
+	}{
+		// Basic add/remove
+		{
+			Old: map[string]interface{}{
+				"foo": "bar",
+			},
+			New: map[string]interface{}{
+				"bar": "baz",
+			},
+			Create: map[string]string{
+				"bar": "baz",
+			},
+			Remove: map[string]string{
+				"foo": "bar",
+			},
+		},
+
+		// Modify
+		{
+			Old: map[string]interface{}{
+				"foo": "bar",
+			},
+			New: map[string]interface{}{
+				"foo": "baz",
+			},
+			Create: map[string]string{
+				"foo": "baz",
+			},
+			Remove: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}
+
+	for i, tc := range cases {
+		c, r := diffTags(tagsFromSchema(tc.Old), tagsFromSchema(tc.New))
+		if !reflect.DeepEqual(c, tc.Create) {
+			t.Fatalf("%d: bad create: %#v", i, c)
+		}
+		if !reflect.DeepEqual(r, tc.Remove) {
+			t.Fatalf("%d: bad remove: %#v", i, r)
+		}
+	}
+}
+
+// testAccCheckTags can be used to check the tags on a resource.
+func testAccCheckTags(
+	tags map[string]string, key string, value string) error {
+	v, ok := tags[key]
+	if value != "" && !ok {
+		return fmt.Errorf("Missing tag: %s", key)
+	} else if value == "" && ok {
+		return fmt.Errorf("Extra tag: %s", key)
+	}
+	if value == "" {
+		return nil
+	}
+
+	if v != value {
+		return fmt.Errorf("%s: bad value: %s", key, v)
+	}
+
+	return nil
+}