From 6ef90f7ed70a9d85b9ee4dfc854f2df5508551d7 Mon Sep 17 00:00:00 2001 From: Tilen Faganel Date: Fri, 24 Aug 2018 19:58:53 +0100 Subject: [PATCH] Migrated tags to sets instead of lists and moved the definitions and related functions to a separate global space to be used by all resources that can be tagged --- digitalocean/resource_digitalocean_droplet.go | 17 +-- .../resource_digitalocean_firewall.go | 21 +--- .../resource_digitalocean_loadbalancer.go | 7 +- digitalocean/resource_digitalocean_tag.go | 13 +-- .../resource_digitalocean_tag_test.go | 67 ----------- digitalocean/set.go | 12 ++ digitalocean/suppress.go | 11 ++ digitalocean/suppress_test.go | 51 +++++++++ digitalocean/tags.go | 58 +++++++++- digitalocean/tags_test.go | 106 ++++++++++++++++-- 10 files changed, 243 insertions(+), 120 deletions(-) create mode 100644 digitalocean/set.go create mode 100644 digitalocean/suppress.go create mode 100644 digitalocean/suppress_test.go diff --git a/digitalocean/resource_digitalocean_droplet.go b/digitalocean/resource_digitalocean_droplet.go index 0996ba49..52741fa4 100644 --- a/digitalocean/resource_digitalocean_droplet.go +++ b/digitalocean/resource_digitalocean_droplet.go @@ -135,12 +135,6 @@ func resourceDigitalOceanDroplet() *schema.Resource { Elem: &schema.Schema{Type: schema.TypeString}, }, - "tags": { - Type: schema.TypeList, - Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, - }, - "user_data": { Type: schema.TypeString, Optional: true, @@ -157,6 +151,8 @@ func resourceDigitalOceanDroplet() *schema.Resource { Optional: true, ForceNew: true, }, + + "tags": tagsSchema(), }, } } @@ -172,6 +168,7 @@ func resourceDigitalOceanDropletCreate(d *schema.ResourceData, meta interface{}) Name: d.Get("name").(string), Region: d.Get("region").(string), Size: d.Get("size").(string), + Tags: expandTags(d.Get("tags").(*schema.Set).List()), } if attr, ok := d.GetOk("backups"); ok { @@ -249,12 +246,6 @@ func resourceDigitalOceanDropletCreate(d *schema.ResourceData, meta interface{}) "Error waiting for droplet (%s) to become ready: %s", d.Id(), err) } - // droplet needs to be active in order to set tags - err = setTags(client, d) - if err != nil { - return fmt.Errorf("Error setting tags: %s", err) - } - return resourceDigitalOceanDropletRead(d, meta) } @@ -331,7 +322,7 @@ func resourceDigitalOceanDropletRead(d *schema.ResourceData, meta interface{}) e "host": findIPv4AddrByType(droplet, "public"), }) - d.Set("tags", droplet.Tags) + d.Set("tags", flattenTags(droplet.Tags)) return nil } diff --git a/digitalocean/resource_digitalocean_firewall.go b/digitalocean/resource_digitalocean_firewall.go index af5327ca..7ce81615 100644 --- a/digitalocean/resource_digitalocean_firewall.go +++ b/digitalocean/resource_digitalocean_firewall.go @@ -67,12 +67,6 @@ func resourceDigitalOceanFirewall() *schema.Resource { Optional: true, }, - "tags": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - }, - "inbound_rule": { Type: schema.TypeList, Optional: true, @@ -158,6 +152,8 @@ func resourceDigitalOceanFirewall() *schema.Resource { }, }, }, + + "tags": tagsSchema(), }, } } @@ -206,7 +202,7 @@ func resourceDigitalOceanFirewallRead(d *schema.ResourceData, meta interface{}) d.Set("pending_changes", firewallPendingChanges(d, firewall)) d.Set("name", firewall.Name) d.Set("droplet_ids", firewall.DropletIDs) - d.Set("tags", firewall.Tags) + d.Set("tags", flattenTags(firewall.Tags)) if err := d.Set("inbound_rule", flattenFirewallInboundRules(d, firewall.InboundRules)); err != nil { return fmt.Errorf("[DEBUG] Error setting Firewall inbound_rule error: %#v", err) @@ -296,20 +292,15 @@ func firewallRequest(d *schema.ResourceData, client *godo.Client) (*godo.Firewal opts.DropletIDs = droplets } - if v, ok := d.GetOk("tags"); ok { - var tags []string - for _, tag := range v.([]interface{}) { - tags = append(tags, tag.(string)) - } - opts.Tags = tags - } - // Get inbound_rules opts.InboundRules = expandFirewallInboundRules(d) // Get outbound_rules opts.OutboundRules = expandFirewallOutboundRules(d) + // Get tags + opts.Tags = expandTags(d.Get("tags").(*schema.Set).List()) + return opts, nil } diff --git a/digitalocean/resource_digitalocean_loadbalancer.go b/digitalocean/resource_digitalocean_loadbalancer.go index 2ca7ea0b..d1f365c6 100644 --- a/digitalocean/resource_digitalocean_loadbalancer.go +++ b/digitalocean/resource_digitalocean_loadbalancer.go @@ -145,11 +145,14 @@ func resourceDigitalOceanLoadbalancer() *schema.Resource { Type: schema.TypeList, Elem: &schema.Schema{Type: schema.TypeString}, Optional: true, + Computed: true, }, "droplet_tag": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: CaseSensitive, + ValidateFunc: validateTag, }, "redirect_http_to_https": { diff --git a/digitalocean/resource_digitalocean_tag.go b/digitalocean/resource_digitalocean_tag.go index 99018362..2c522d96 100644 --- a/digitalocean/resource_digitalocean_tag.go +++ b/digitalocean/resource_digitalocean_tag.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "log" - "regexp" "github.com/digitalocean/godo" "github.com/hashicorp/terraform/helper/schema" @@ -24,22 +23,12 @@ func resourceDigitalOceanTag() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validateTagName, + ValidateFunc: validateTag, }, }, } } -var tagNameRe = regexp.MustCompile("^[a-z0-9:\\-_]{1,255}$") - -func validateTagName(value interface{}, key string) ([]string, []error) { - if !tagNameRe.MatchString(value.(string)) { - return nil, []error{fmt.Errorf("tags may contain letters, numbers, colons, dashes, and underscores; there is a limit of 255 characters per tag")} - } - - return nil, nil -} - func resourceDigitalOceanTagCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*godo.Client) diff --git a/digitalocean/resource_digitalocean_tag_test.go b/digitalocean/resource_digitalocean_tag_test.go index f46e2ea7..bf2ee7eb 100644 --- a/digitalocean/resource_digitalocean_tag_test.go +++ b/digitalocean/resource_digitalocean_tag_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/digitalocean/godo" - "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" ) @@ -32,72 +31,6 @@ func TestAccDigitalOceanTag_Basic(t *testing.T) { }) } -func TestAccDigitalOceanTag_NameValidation(t *testing.T) { - cases := []struct { - Input string - ExpectError bool - }{ - { - Input: "", - ExpectError: true, - }, - { - Input: "foo", - ExpectError: false, - }, - { - Input: "foo-bar", - ExpectError: false, - }, - { - Input: "foo:bar", - ExpectError: false, - }, - { - Input: "foo_bar", - ExpectError: false, - }, - { - Input: "foo-001", - ExpectError: false, - }, - { - Input: "foo/bar", - ExpectError: true, - }, - { - Input: "foo\bar", - ExpectError: true, - }, - { - Input: "foo.bar", - ExpectError: true, - }, - { - Input: "foo*", - ExpectError: true, - }, - { - Input: acctest.RandString(256), - ExpectError: true, - }, - } - - for _, tc := range cases { - _, errors := validateTagName(tc.Input, tc.Input) - - hasError := len(errors) > 0 - - if tc.ExpectError && !hasError { - t.Fatalf("Expected the DigitalOcean Tag Name to trigger a validation error for '%s'", tc.Input) - } - - if hasError && !tc.ExpectError { - t.Fatalf("Unexpected error validating the DigitalOcean Tag Name '%s': %s", tc.Input, errors[0]) - } - } -} - func testAccCheckDigitalOceanTagDestroy(s *terraform.State) error { client := testAccProvider.Meta().(*godo.Client) diff --git a/digitalocean/set.go b/digitalocean/set.go new file mode 100644 index 00000000..fc3d6da7 --- /dev/null +++ b/digitalocean/set.go @@ -0,0 +1,12 @@ +package digitalocean + +import ( + "strings" + + "github.com/hashicorp/terraform/helper/hashcode" +) + +// Helper function for sets of strings that are case insensitive +func HashStringIgnoreCase(v interface{}) int { + return hashcode.String(strings.ToLower(v.(string))) +} diff --git a/digitalocean/suppress.go b/digitalocean/suppress.go new file mode 100644 index 00000000..adbb2644 --- /dev/null +++ b/digitalocean/suppress.go @@ -0,0 +1,11 @@ +package digitalocean + +import ( + "strings" + + "github.com/hashicorp/terraform/helper/schema" +) + +func CaseSensitive(_, old, new string, _ *schema.ResourceData) bool { + return strings.ToLower(old) == strings.ToLower(new) +} diff --git a/digitalocean/suppress_test.go b/digitalocean/suppress_test.go new file mode 100644 index 00000000..59f69cbb --- /dev/null +++ b/digitalocean/suppress_test.go @@ -0,0 +1,51 @@ +package digitalocean + +import "testing" + +func TestCaseSensitive(t *testing.T) { + cases := []struct { + Name string + Left string + Right string + Suppress bool + }{ + { + Name: "empty", + Left: "", + Right: "", + Suppress: true, + }, + { + Name: "empty and text", + Left: "text", + Right: "", + Suppress: false, + }, + { + Name: "different text", + Left: "text", + Right: "different text", + Suppress: false, + }, + { + Name: "same text", + Left: "text", + Right: "text", + Suppress: true, + }, + { + Name: "same text different case", + Left: "text", + Right: "TeXT", + Suppress: true, + }, + } + + for _, tc := range cases { + t.Run(tc.Name, func(t *testing.T) { + if CaseSensitive("test", tc.Left, tc.Right, nil) != tc.Suppress { + t.Fatalf("Expected CaseSensitive to return %t for '%q' == '%q'", tc.Suppress, tc.Left, tc.Right) + } + }) + } +} diff --git a/digitalocean/tags.go b/digitalocean/tags.go index e3a990e7..4690fd33 100644 --- a/digitalocean/tags.go +++ b/digitalocean/tags.go @@ -2,12 +2,36 @@ package digitalocean import ( "context" + "fmt" "log" + "regexp" "github.com/digitalocean/godo" "github.com/hashicorp/terraform/helper/schema" ) +var tagNameRe = regexp.MustCompile("^[a-zA-Z0-9:\\-_]{1,255}$") + +func tagsSchema() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validateTag, + }, + Set: HashStringIgnoreCase, + } +} + +func validateTag(value interface{}, key string) ([]string, []error) { + if !tagNameRe.MatchString(value.(string)) { + return nil, []error{fmt.Errorf("tags may contain lowercase letters, numbers, colons, dashes, and underscores; there is a limit of 255 characters per tag")} + } + + return nil, nil +} + // setTags is a helper to set the tags for a resource. It expects the // tags field to be named "tags" func setTags(conn *godo.Client, d *schema.ResourceData) error { @@ -31,7 +55,15 @@ func setTags(conn *godo.Client, d *schema.ResourceData) error { log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) for _, tag := range create { - _, err := conn.Tags.TagResources(context.Background(), tag, &godo.TagResourcesRequest{ + + createdTag, _, err := conn.Tags.Create(context.Background(), &godo.TagCreateRequest{ + Name: tag, + }) + if err != nil { + return err + } + + _, err = conn.Tags.TagResources(context.Background(), createdTag.Name, &godo.TagResourcesRequest{ Resources: []godo.Resource{ { ID: d.Id(), @@ -51,7 +83,7 @@ func setTags(conn *godo.Client, d *schema.ResourceData) error { // properly asserted map[string]string func tagsFromSchema(raw interface{}) map[string]string { result := make(map[string]string) - for _, t := range raw.([]interface{}) { + for _, t := range raw.(*schema.Set).List() { result[t.(string)] = t.(string) } @@ -71,3 +103,25 @@ func diffTags(oldTags, newTags map[string]string) (map[string]string, map[string return oldTags, newTags } + +func expandTags(tags []interface{}) []string { + expandedTags := make([]string, len(tags)) + for i, v := range tags { + expandedTags[i] = v.(string) + } + + return expandedTags +} + +func flattenTags(tags []string) *schema.Set { + if tags == nil { + return nil + } + + flattenedTags := schema.NewSet(HashStringIgnoreCase, []interface{}{}) + for _, v := range tags { + flattenedTags.Add(v) + } + + return flattenedTags +} diff --git a/digitalocean/tags_test.go b/digitalocean/tags_test.go index 02686ed2..3cc8ac9f 100644 --- a/digitalocean/tags_test.go +++ b/digitalocean/tags_test.go @@ -3,21 +3,24 @@ package digitalocean import ( "reflect" "testing" + + "github.com/hashicorp/terraform/helper/acctest" + "github.com/hashicorp/terraform/helper/schema" ) func TestDiffTags(t *testing.T) { cases := []struct { - Old, New []interface{} + Old, New *schema.Set Create, Remove map[string]string }{ // Basic add/remove { - Old: []interface{}{ + Old: schema.NewSet(HashStringIgnoreCase, []interface{}{ "foo", - }, - New: []interface{}{ + }), + New: schema.NewSet(HashStringIgnoreCase, []interface{}{ "bar", - }, + }), Create: map[string]string{ "bar": "bar", }, @@ -28,12 +31,12 @@ func TestDiffTags(t *testing.T) { // Noop { - Old: []interface{}{ + Old: schema.NewSet(HashStringIgnoreCase, []interface{}{ "foo", - }, - New: []interface{}{ + }), + New: schema.NewSet(HashStringIgnoreCase, []interface{}{ "foo", - }, + }), Create: map[string]string{}, Remove: map[string]string{}, }, @@ -49,3 +52,88 @@ func TestDiffTags(t *testing.T) { } } } + +func TestAccDigitalOceanTag_NameValidation(t *testing.T) { + cases := []struct { + Input string + ExpectError bool + }{ + { + Input: "", + ExpectError: true, + }, + { + Input: "foo", + ExpectError: false, + }, + { + Input: "foo-bar", + ExpectError: false, + }, + { + Input: "foo:bar", + ExpectError: false, + }, + { + Input: "foo_bar", + ExpectError: false, + }, + { + Input: "foo-001", + ExpectError: false, + }, + { + Input: "foo/bar", + ExpectError: true, + }, + { + Input: "foo\bar", + ExpectError: true, + }, + { + Input: "foo.bar", + ExpectError: true, + }, + { + Input: "foo*", + ExpectError: true, + }, + { + Input: acctest.RandString(256), + ExpectError: true, + }, + } + + for _, tc := range cases { + _, errors := validateTag(tc.Input, tc.Input) + + hasError := len(errors) > 0 + + if tc.ExpectError && !hasError { + t.Fatalf("Expected the DigitalOcean Tag Name to trigger a validation error for '%s'", tc.Input) + } + + if hasError && !tc.ExpectError { + t.Fatalf("Unexpected error validating the DigitalOcean Tag Name '%s': %s", tc.Input, errors[0]) + } + } +} + +func TestExpandTags(t *testing.T) { + tags := []interface{}{"foo", "bar"} + + expandedTags := expandTags(tags) + + if len(tags) != len(expandedTags) { + t.Fatalf("incorrect expected length of expanded tags") + } +} +func TestFlattenTags(t *testing.T) { + tags := []string{"foo", "bar"} + + flattenedTags := flattenTags(tags) + + if len(tags) != flattenedTags.Len() { + t.Fatalf("incorrect expected length of flattened tags") + } +}