From 7b9d36e6aab2aaa3eac49bf95d61758f7e7a96c7 Mon Sep 17 00:00:00 2001 From: Pablo RUTH Date: Sun, 13 Aug 2017 19:50:11 +0200 Subject: [PATCH 1/4] add support for tags on Directory Service --- ...esource_aws_directory_service_directory.go | 13 ++ aws/tagsDS.go | 115 ++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 aws/tagsDS.go diff --git a/aws/resource_aws_directory_service_directory.go b/aws/resource_aws_directory_service_directory.go index b6e8ab64a76..4b484a59692 100644 --- a/aws/resource_aws_directory_service_directory.go +++ b/aws/resource_aws_directory_service_directory.go @@ -61,6 +61,7 @@ func resourceAwsDirectoryServiceDirectory() *schema.Resource { Computed: true, ForceNew: true, }, + "tags": tagsSchema(), "vpc_settings": &schema.Schema{ Type: schema.TypeList, Optional: true, @@ -391,6 +392,10 @@ func resourceAwsDirectoryServiceDirectoryUpdate(d *schema.ResourceData, meta int } } + if err := setTagsDS(dsconn, d, d.Id()); err != nil { + return err + } + return resourceAwsDirectoryServiceDirectoryRead(d, meta) } @@ -438,6 +443,14 @@ func resourceAwsDirectoryServiceDirectoryRead(d *schema.ResourceData, meta inter d.Set("connect_settings", flattenDSConnectSettings(dir.DnsIpAddrs, dir.ConnectSettings)) d.Set("enable_sso", *dir.SsoEnabled) + tagList, err := dsconn.ListTagsForResource(&directoryservice.ListTagsForResourceInput{ + ResourceId: aws.String(d.Id()), + }) + if err != nil { + return fmt.Errorf("Failed to get Directory service tags (id: %s): %s", d.Id(), err) + } + d.Set("tags", tagsToMapDS(tagList.Tags)) + return nil } diff --git a/aws/tagsDS.go b/aws/tagsDS.go new file mode 100644 index 00000000000..49f4d06cfff --- /dev/null +++ b/aws/tagsDS.go @@ -0,0 +1,115 @@ +package aws + +import ( + "log" + "regexp" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/directoryservice" + "github.com/hashicorp/terraform/helper/schema" +) + +// setTags is a helper to set the tags for a resource. It expects the +// tags field to be named "tags" +func setTagsDS(conn *directoryservice.DirectoryService, d *schema.ResourceData, resourceId string) error { + if d.HasChange("tags") { + oraw, nraw := d.GetChange("tags") + o := oraw.(map[string]interface{}) + n := nraw.(map[string]interface{}) + create, remove := diffTagsDS(tagsFromMapDS(o), tagsFromMapDS(n)) + + // Set tags + if len(remove) > 0 { + log.Printf("[DEBUG] Removing tags: %s", remove) + k := make([]*string, len(remove), len(remove)) + for i, t := range remove { + k[i] = t.Key + } + + _, err := conn.RemoveTagsFromResource(&directoryservice.RemoveTagsFromResourceInput{ + ResourceId: aws.String(resourceId), + TagKeys: k, + }) + if err != nil { + return err + } + } + if len(create) > 0 { + log.Printf("[DEBUG] Creating tags: %s", create) + _, err := conn.AddTagsToResource(&directoryservice.AddTagsToResourceInput{ + ResourceId: aws.String(resourceId), + Tags: create, + }) + if err != nil { + return err + } + } + } + + return nil +} + +// diffTags takes our tags locally and the ones remotely and returns +// the set of tags that must be created, and the set of tags that must +// be destroyed. +func diffTagsDS(oldTags, newTags []*directoryservice.Tag) ([]*directoryservice.Tag, []*directoryservice.Tag) { + // First, we're creating everything we have + create := make(map[string]interface{}) + for _, t := range newTags { + create[*t.Key] = *t.Value + } + + // Build the list of what to remove + var remove []*directoryservice.Tag + for _, t := range oldTags { + old, ok := create[*t.Key] + if !ok || old != *t.Value { + // Delete it! + remove = append(remove, t) + } + } + + return tagsFromMapDS(create), remove +} + +// tagsFromMap returns the tags for the given map of data. +func tagsFromMapDS(m map[string]interface{}) []*directoryservice.Tag { + result := make([]*directoryservice.Tag, 0, len(m)) + for k, v := range m { + t := &directoryservice.Tag{ + Key: aws.String(k), + Value: aws.String(v.(string)), + } + if !tagIgnoredDS(t) { + result = append(result, t) + } + } + + return result +} + +// tagsToMap turns the list of tags into a map. +func tagsToMapDS(ts []*directoryservice.Tag) map[string]string { + result := make(map[string]string) + for _, t := range ts { + if !tagIgnoredDS(t) { + result[*t.Key] = *t.Value + } + } + + return result +} + +// compare a tag against a list of strings and checks if it should +// be ignored or not +func tagIgnoredDS(t *directoryservice.Tag) bool { + filter := []string{"^aws:"} + for _, v := range filter { + log.Printf("[DEBUG] Matching %v with %v\n", v, *t.Key) + if r, _ := regexp.MatchString(v, *t.Key); r == true { + log.Printf("[DEBUG] Found AWS specific tag %s (val: %s), ignoring.\n", *t.Key, *t.Value) + return true + } + } + return false +} From 0f53eba19c56a563d0e941a73d0b1c4cb5e2dbfc Mon Sep 17 00:00:00 2001 From: Pablo RUTH Date: Sun, 13 Aug 2017 20:16:16 +0200 Subject: [PATCH 2/4] update doc for directory service tags argument --- website/docs/r/directory_service_directory.html.markdown | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/website/docs/r/directory_service_directory.html.markdown b/website/docs/r/directory_service_directory.html.markdown index 16bc9145ca3..183ddba7943 100644 --- a/website/docs/r/directory_service_directory.html.markdown +++ b/website/docs/r/directory_service_directory.html.markdown @@ -25,6 +25,10 @@ resource "aws_directory_service_directory" "bar" { vpc_id = "${aws_vpc.main.id}" subnet_ids = ["${aws_subnet.foo.id}", "${aws_subnet.bar.id}"] } + + tags { + Project = "foo" + } } resource "aws_vpc" "main" { @@ -58,6 +62,7 @@ The following arguments are supported: * `short_name` - (Optional) The short name of the directory, such as `CORP`. * `enable_sso` - (Optional) Whether to enable single-sign on for the directory. Requires `alias`. Defaults to `false`. * `type` (Optional) - The directory type (`SimpleAD` or `MicrosoftAD` are accepted values). Defaults to `SimpleAD`. +* `tags` - (Optional) A mapping of tags to assign to the resource. **vpc\_settings** supports the following: From fc5fcb92873f75f3ab41a99aa72c889eabfc59d2 Mon Sep 17 00:00:00 2001 From: Pablo RUTH Date: Wed, 16 Aug 2017 18:33:54 +0200 Subject: [PATCH 3/4] add acceptance test for tags on directory service --- ...ce_aws_directory_service_directory_test.go | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/aws/resource_aws_directory_service_directory_test.go b/aws/resource_aws_directory_service_directory_test.go index 56505d05d0f..0ce120eeb3e 100644 --- a/aws/resource_aws_directory_service_directory_test.go +++ b/aws/resource_aws_directory_service_directory_test.go @@ -29,6 +29,23 @@ func TestAccAWSDirectoryServiceDirectory_basic(t *testing.T) { }) } +func TestAccAWSDirectoryServiceDirectory_tags(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckDirectoryServiceDirectoryDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccDirectoryServiceDirectoryTagsConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckServiceDirectoryExists("aws_directory_service_directory.bar"), + resource.TestCheckResourceAttr("aws_directory_service_directory.bar", "tags.%", "2"), + ), + }, + }, + }) +} + func TestAccAWSDirectoryServiceDirectory_microsoft(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -250,6 +267,42 @@ resource "aws_subnet" "bar" { } ` +const testAccDirectoryServiceDirectoryTagsConfig = ` +resource "aws_directory_service_directory" "bar" { + name = "corp.notexample.com" + password = "SuperSecretPassw0rd" + size = "Small" + + vpc_settings { + vpc_id = "${aws_vpc.main.id}" + subnet_ids = ["${aws_subnet.foo.id}", "${aws_subnet.bar.id}"] + } + + tags { + foo = "bar" + project = "test" + } +} + +resource "aws_vpc" "main" { + cidr_block = "10.0.0.0/16" + tags { + Name = "testAccDirectoryServiceDirectoryConfig" + } +} + +resource "aws_subnet" "foo" { + vpc_id = "${aws_vpc.main.id}" + availability_zone = "us-west-2a" + cidr_block = "10.0.1.0/24" +} +resource "aws_subnet" "bar" { + vpc_id = "${aws_vpc.main.id}" + availability_zone = "us-west-2b" + cidr_block = "10.0.2.0/24" +} +` + const testAccDirectoryServiceDirectoryConfig_connector = ` resource "aws_directory_service_directory" "bar" { name = "corp.notexample.com" From bb0aa5109211396706c0d5599ededff91e3a4db6 Mon Sep 17 00:00:00 2001 From: Pablo RUTH Date: Wed, 23 Aug 2017 15:31:22 +0200 Subject: [PATCH 4/4] Add tests for the diff function --- ...ce_aws_directory_service_directory_test.go | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/aws/resource_aws_directory_service_directory_test.go b/aws/resource_aws_directory_service_directory_test.go index 0ce120eeb3e..8cda8ca5ad5 100644 --- a/aws/resource_aws_directory_service_directory_test.go +++ b/aws/resource_aws_directory_service_directory_test.go @@ -2,6 +2,7 @@ package aws import ( "fmt" + "reflect" "testing" "github.com/aws/aws-sdk-go/aws" @@ -13,6 +14,57 @@ import ( "github.com/hashicorp/terraform/terraform" ) +func TestDiffTagsDirectoryService(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 := diffTagsDS(tagsFromMapDS(tc.Old), tagsFromMapDS(tc.New)) + cm := tagsToMapDS(c) + rm := tagsToMapDS(r) + if !reflect.DeepEqual(cm, tc.Create) { + t.Fatalf("%d: bad create: %#v", i, cm) + } + if !reflect.DeepEqual(rm, tc.Remove) { + t.Fatalf("%d: bad remove: %#v", i, rm) + } + } +} + func TestAccAWSDirectoryServiceDirectory_basic(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) },