From f43528957ed89d15595f43e24d63066707c5831c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 17:35:14 -0700 Subject: [PATCH 1/6] helper/schema: support top-level TypeMap --- helper/schema/resource_data.go | 19 ++++++++++++++++--- helper/schema/schema_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index 69ac090b37..fbef8c88b7 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -372,9 +372,22 @@ func (d *ResourceData) getMap( if d.config != nil && source == getSourceConfig { // For config, we always set the result to exactly what was requested - if m, ok := d.config.Get(k); ok { - result = m.(map[string]interface{}) - resultSet = true + if mraw, ok := d.config.Get(k); ok { + switch m := mraw.(type) { + case []interface{}: + for _, innerRaw := range m { + for k, v := range innerRaw.(map[string]interface{}) { + result[k] = v + } + } + + resultSet = true + case map[string]interface{}: + result = m + resultSet = true + default: + panic(fmt.Sprintf("unknown type: %#v", mraw)) + } } else { result = nil } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 876393a9b8..531891271d 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -835,6 +835,35 @@ func TestSchemaMap_Diff(t *testing.T) { * Maps */ + { + Schema: map[string]*Schema{ + "config_vars": &Schema{ + Type: TypeMap, + }, + }, + + State: nil, + + Config: map[string]interface{}{ + "config_vars": []interface{}{ + map[string]interface{}{ + "bar": "baz", + }, + }, + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "config_vars.bar": &terraform.ResourceAttrDiff{ + Old: "", + New: "baz", + }, + }, + }, + + Err: false, + }, + { Schema: map[string]*Schema{ "config_vars": &Schema{ From 6eafac8a34fef297d49c9f42080e2175bb32a6e0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 17:54:00 -0700 Subject: [PATCH 2/6] providers/aws: aws_vpc supports tags --- builtin/providers/aws/resource_aws_vpc.go | 14 ++- .../providers/aws/resource_aws_vpc_test.go | 32 +++++++ builtin/providers/aws/tags.go | 89 +++++++++++++++++++ builtin/providers/aws/tags_test.go | 26 ++++++ helper/schema/resource_data.go | 8 ++ 5 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 builtin/providers/aws/tags.go create mode 100644 builtin/providers/aws/tags_test.go diff --git a/builtin/providers/aws/resource_aws_vpc.go b/builtin/providers/aws/resource_aws_vpc.go index e089957529..cde7b90651 100644 --- a/builtin/providers/aws/resource_aws_vpc.go +++ b/builtin/providers/aws/resource_aws_vpc.go @@ -5,8 +5,8 @@ import ( "log" "time" - "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/helper/schema" "github.com/mitchellh/goamz/ec2" ) @@ -35,6 +35,8 @@ func resourceAwsVpc() *schema.Resource { Optional: true, Computed: true, }, + + "tags": tagsSchema(), }, } } @@ -120,10 +122,15 @@ func resourceAwsVpcUpdate(d *schema.ResourceData, meta interface{}) error { d.SetPartial("enable_dns_support") } + if err := setTags(ec2conn, d); err != nil { + return err + } else { + d.SetPartial("tags") + } + return nil } - func resourceAwsVpcDelete(d *schema.ResourceData, meta interface{}) error { p := meta.(*ResourceProvider) ec2conn := p.ec2conn @@ -158,6 +165,9 @@ func resourceAwsVpcRead(d *schema.ResourceData, meta interface{}) error { vpc := vpcRaw.(*ec2.VPC) d.Set("cidr_block", vpc.CidrBlock) + // Tags + d.Set("tags", tagsToMap(vpc.Tags)) + // Attributes resp, err := ec2conn.VpcAttribute(d.Id(), "enableDnsSupport") if err != nil { diff --git a/builtin/providers/aws/resource_aws_vpc_test.go b/builtin/providers/aws/resource_aws_vpc_test.go index f19686adaa..83528640bd 100644 --- a/builtin/providers/aws/resource_aws_vpc_test.go +++ b/builtin/providers/aws/resource_aws_vpc_test.go @@ -30,6 +30,28 @@ func TestAccVpc_basic(t *testing.T) { }) } +func TestAccVpc_tags(t *testing.T) { + var vpc ec2.VPC + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckVpcDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccVpcConfigTags, + Check: resource.ComposeTestCheckFunc( + testAccCheckVpcExists("aws_vpc.foo", &vpc), + testAccCheckVpcCidr(&vpc, "10.1.0.0/16"), + resource.TestCheckResourceAttr( + "aws_vpc.foo", "cidr_block", "10.1.0.0/16"), + testAccCheckTags(&vpc.Tags, "foo", "bar"), + ), + }, + }, + }) +} + func TestAccVpcUpdate(t *testing.T) { var vpc ec2.VPC @@ -138,3 +160,13 @@ resource "aws_vpc" "foo" { enable_dns_hostnames = true } ` + +const testAccVpcConfigTags = ` +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" + + tags { + foo = "bar" + } +} +` diff --git a/builtin/providers/aws/tags.go b/builtin/providers/aws/tags.go new file mode 100644 index 0000000000..c0dd4e04d2 --- /dev/null +++ b/builtin/providers/aws/tags.go @@ -0,0 +1,89 @@ +package aws + +import ( + "log" + + "github.com/hashicorp/terraform/helper/schema" + "github.com/mitchellh/goamz/ec2" +) + +// tagsSchema returns the schema to use for tags. +func tagsSchema() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeMap, + Optional: true, + Computed: true, + } +} + +// setTags is a helper to set the tags for a resource. It expects the +// tags field to be named "tags" +func setTags(conn *ec2.EC2, d *schema.ResourceData) 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)) + + // Set tags + if len(remove) > 0 { + log.Printf("[DEBUG] Removing tags: %#v", remove) + if _, err := conn.DeleteTags([]string{d.Id()}, remove); err != nil { + return err + } + } + if len(create) > 0 { + log.Printf("[DEBUG] Creating tags: %#v", create) + if _, err := conn.CreateTags([]string{d.Id()}, create); 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 diffTags(oldTags, newTags []ec2.Tag) ([]ec2.Tag, []ec2.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 []ec2.Tag + for _, t := range oldTags { + if _, ok := create[t.Key]; !ok { + // Delete it! + remove = append(remove, t) + } + } + + return tagsFromMap(create), remove +} + +// tagsFromMap returns the tags for the given map of data. +func tagsFromMap(m map[string]interface{}) []ec2.Tag { + result := make([]ec2.Tag, 0, len(m)) + for k, v := range m { + result = append(result, ec2.Tag{ + Key: k, + Value: v.(string), + }) + } + + return result +} + +// tagsToMap turns the list of tags into a map. +func tagsToMap(ts []ec2.Tag) map[string]string { + result := make(map[string]string) + for _, t := range ts { + result[t.Key] = t.Value + } + + return result +} diff --git a/builtin/providers/aws/tags_test.go b/builtin/providers/aws/tags_test.go new file mode 100644 index 0000000000..55f842dbea --- /dev/null +++ b/builtin/providers/aws/tags_test.go @@ -0,0 +1,26 @@ +package aws + +import ( + "fmt" + + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" + "github.com/mitchellh/goamz/ec2" +) + +// testAccCheckTags can be used to check the tags on a resource. +func testAccCheckTags( + ts *[]ec2.Tag, key string, value string) resource.TestCheckFunc { + return func(s *terraform.State) error { + m := tagsToMap(*ts) + v, ok := m[key] + if !ok { + return fmt.Errorf("Missing tag: %s", key) + } + if v != value { + return fmt.Errorf("%s: bad value: %s", key, v) + } + + return nil + } +} diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index fbef8c88b7..ee2e36339b 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -381,6 +381,14 @@ func (d *ResourceData) getMap( } } + resultSet = true + case []map[string]interface{}: + for _, innerRaw := range m { + for k, v := range innerRaw { + result[k] = v + } + } + resultSet = true case map[string]interface{}: result = m From 00bdef30934cb86fc2a25b7f51960a1ae57f8aa3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 18:21:21 -0700 Subject: [PATCH 3/6] providers/aws: test tag removal --- .../providers/aws/resource_aws_vpc_test.go | 19 +++++++++++++++++++ builtin/providers/aws/tags_test.go | 8 +++++++- helper/schema/resource_data.go | 1 + helper/schema/schema.go | 4 ++++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_vpc_test.go b/builtin/providers/aws/resource_aws_vpc_test.go index 83528640bd..86c85655ea 100644 --- a/builtin/providers/aws/resource_aws_vpc_test.go +++ b/builtin/providers/aws/resource_aws_vpc_test.go @@ -48,6 +48,15 @@ func TestAccVpc_tags(t *testing.T) { testAccCheckTags(&vpc.Tags, "foo", "bar"), ), }, + + resource.TestStep{ + Config: testAccVpcConfigTagsUpdate, + Check: resource.ComposeTestCheckFunc( + testAccCheckVpcExists("aws_vpc.foo", &vpc), + testAccCheckTags(&vpc.Tags, "foo", ""), + testAccCheckTags(&vpc.Tags, "bar", "baz"), + ), + }, }, }) } @@ -170,3 +179,13 @@ resource "aws_vpc" "foo" { } } ` + +const testAccVpcConfigTagsUpdate = ` +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" + + tags { + bar = "baz" + } +} +` diff --git a/builtin/providers/aws/tags_test.go b/builtin/providers/aws/tags_test.go index 55f842dbea..d249a04b1e 100644 --- a/builtin/providers/aws/tags_test.go +++ b/builtin/providers/aws/tags_test.go @@ -14,9 +14,15 @@ func testAccCheckTags( return func(s *terraform.State) error { m := tagsToMap(*ts) v, ok := m[key] - if !ok { + 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) } diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index ee2e36339b..4f7a4b9931 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -373,6 +373,7 @@ func (d *ResourceData) getMap( if d.config != nil && source == getSourceConfig { // For config, we always set the result to exactly what was requested if mraw, ok := d.config.Get(k); ok { + result = make(map[string]interface{}) switch m := mraw.(type) { case []interface{}: for _, innerRaw := range m { diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 8a813fbb37..20baf17f64 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -143,6 +143,10 @@ func (s *Schema) finalizeDiff( return d } + if d.NewRemoved { + return d + } + if s.Computed { if d.Old != "" && d.New == "" { // This is a computed value with an old value set already, From 753f6c6f8e4b4123f160e72ab57bacd2b5b287fe Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 18:25:31 -0700 Subject: [PATCH 4/6] helper/schema: fix failing tests --- helper/schema/schema.go | 3 +++ helper/schema/schema_test.go | 37 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 20baf17f64..7d373bd0dc 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -620,6 +620,9 @@ func (m schemaMap) diffString( if o != nil && n == nil { removed = true } + if removed && schema.Computed { + return nil + } diff.Attributes[k] = schema.finalizeDiff(&terraform.ResourceAttrDiff{ Old: os, diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 531891271d..8190ff2a6c 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -864,6 +864,43 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + { + Schema: map[string]*Schema{ + "config_vars": &Schema{ + Type: TypeMap, + }, + }, + + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "config_vars.foo": "bar", + }, + }, + + Config: map[string]interface{}{ + "config_vars": []interface{}{ + map[string]interface{}{ + "bar": "baz", + }, + }, + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "config_vars.foo": &terraform.ResourceAttrDiff{ + Old: "bar", + NewRemoved: true, + }, + "config_vars.bar": &terraform.ResourceAttrDiff{ + Old: "", + New: "baz", + }, + }, + }, + + Err: false, + }, + { Schema: map[string]*Schema{ "config_vars": &Schema{ From 4a8021ff8e0a71301a9f7b26e7745bef1e50d2a1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 8 Oct 2014 23:46:36 -0700 Subject: [PATCH 5/6] website: update docs for tags on VPC --- .../source/docs/providers/aws/r/vpc.html.markdown | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/website/source/docs/providers/aws/r/vpc.html.markdown b/website/source/docs/providers/aws/r/vpc.html.markdown index 8104c2d654..5955c401b7 100644 --- a/website/source/docs/providers/aws/r/vpc.html.markdown +++ b/website/source/docs/providers/aws/r/vpc.html.markdown @@ -10,12 +10,26 @@ Provides an VPC resource. ## Example Usage +Basic usage: + ``` resource "aws_vpc" "main" { cidr_block = "10.0.0.0/16" } ``` +Basic usage with tags: + +``` +resource "aws_vpc" "main" { + cidr_block = "10.0.0.0/16" + + tags { + Name = "main" + } +} +``` + ## Argument Reference The following arguments are supported: @@ -23,6 +37,7 @@ The following arguments are supported: * `cidr_block` - (Required) The CIDR block for the VPC. * `enable_dns_support` - (Optional) A boolean flag to enable/disable DNS support in the VPC. Defaults true. * `enable_dns_hostnames` - (Optional) A boolean flag to enable/disable DNS hostnames in the VPC. Defaults false. +* `tags` - (Optional) A mapping of tags to assign to the resource. ## Attributes Reference From 6591e6313807c7cfb5fbe7f32391b50f0f215549 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Oct 2014 10:47:33 -0700 Subject: [PATCH 6/6] providers/aws: should delete tags that are changing in-place --- builtin/providers/aws/tags.go | 3 +- builtin/providers/aws/tags_test.go | 53 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/builtin/providers/aws/tags.go b/builtin/providers/aws/tags.go index c0dd4e04d2..a188eaf9d3 100644 --- a/builtin/providers/aws/tags.go +++ b/builtin/providers/aws/tags.go @@ -56,7 +56,8 @@ func diffTags(oldTags, newTags []ec2.Tag) ([]ec2.Tag, []ec2.Tag) { // Build the list of what to remove var remove []ec2.Tag for _, t := range oldTags { - if _, ok := create[t.Key]; !ok { + old, ok := create[t.Key] + if !ok || old != t.Value { // Delete it! remove = append(remove, t) } diff --git a/builtin/providers/aws/tags_test.go b/builtin/providers/aws/tags_test.go index d249a04b1e..eb30f346e1 100644 --- a/builtin/providers/aws/tags_test.go +++ b/builtin/providers/aws/tags_test.go @@ -2,12 +2,65 @@ package aws import ( "fmt" + "reflect" + "testing" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/goamz/ec2" ) +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(tagsFromMap(tc.Old), tagsFromMap(tc.New)) + cm := tagsToMap(c) + rm := tagsToMap(r) + if !reflect.DeepEqual(cm, tc.Create) { + t.Fatalf("%i: bad create: %#v", i, cm) + } + if !reflect.DeepEqual(rm, tc.Remove) { + t.Fatalf("%i: bad remove: %#v", i, rm) + } + } +} + // testAccCheckTags can be used to check the tags on a resource. func testAccCheckTags( ts *[]ec2.Tag, key string, value string) resource.TestCheckFunc {