diff --git a/aws/data_source_aws_security_group.go b/aws/data_source_aws_security_group.go index 7fddc60fe1b2..88a16f4b7465 100644 --- a/aws/data_source_aws_security_group.go +++ b/aws/data_source_aws_security_group.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func dataSourceAwsSecurityGroup() *schema.Resource { @@ -63,7 +64,7 @@ func dataSourceAwsSecurityGroupRead(d *schema.ResourceData, meta interface{}) er }, ) req.Filters = append(req.Filters, buildEC2TagFilterList( - tagsFromMap(d.Get("tags").(map[string]interface{})), + keyvaluetags.New(d.Get("tags").(map[string]interface{})).Ec2Tags(), )...) req.Filters = append(req.Filters, buildEC2CustomFilterList( d.Get("filter").(*schema.Set), @@ -91,7 +92,11 @@ func dataSourceAwsSecurityGroupRead(d *schema.ResourceData, meta interface{}) er d.Set("name", sg.GroupName) d.Set("description", sg.Description) d.Set("vpc_id", sg.VpcId) - d.Set("tags", tagsToMap(sg.Tags)) + + if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(sg.Tags).IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + arn := arn.ARN{ Partition: meta.(*AWSClient).partition, Service: "ec2", diff --git a/aws/data_source_aws_security_groups.go b/aws/data_source_aws_security_groups.go index 665b44a3b4ff..f7636e3eb6f0 100644 --- a/aws/data_source_aws_security_groups.go +++ b/aws/data_source_aws_security_groups.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func dataSourceAwsSecurityGroups() *schema.Resource { @@ -49,7 +50,7 @@ func dataSourceAwsSecurityGroupsRead(d *schema.ResourceData, meta interface{}) e } if tagsOk { req.Filters = append(req.Filters, buildEC2TagFilterList( - tagsFromMap(tags.(map[string]interface{})), + keyvaluetags.New(tags.(map[string]interface{})).Ec2Tags(), )...) } diff --git a/aws/resource_aws_default_security_group.go b/aws/resource_aws_default_security_group.go index b9e07b979208..c07640206ec0 100644 --- a/aws/resource_aws_default_security_group.go +++ b/aws/resource_aws_default_security_group.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsDefaultSecurityGroup() *schema.Resource { @@ -90,8 +91,10 @@ func resourceAwsDefaultSecurityGroupCreate(d *schema.ResourceData, meta interfac log.Printf("[INFO] Default Security Group ID: %s", d.Id()) - if err := setTags(conn, d); err != nil { - return err + if v := d.Get("tags").(map[string]interface{}); len(v) > 0 { + if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil { + return fmt.Errorf("error adding EC2 Default Security Group (%s) tags: %s", d.Id(), err) + } } if err := revokeDefaultSecurityGroupRules(meta, g); err != nil { diff --git a/aws/resource_aws_security_group.go b/aws/resource_aws_security_group.go index fac3db4540a3..17ebc66565f9 100644 --- a/aws/resource_aws_security_group.go +++ b/aws/resource_aws_security_group.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" "github.com/terraform-providers/terraform-provider-aws/aws/internal/naming" ) @@ -268,8 +269,10 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er d.Id(), err) } - if err := setTags(conn, d); err != nil { - return err + if v := d.Get("tags").(map[string]interface{}); len(v) > 0 { + if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil { + return fmt.Errorf("error adding EC2 Security Group (%s) tags: %s", d.Id(), err) + } } // AWS defaults all Security Groups to have an ALLOW ALL egress rule. Here we @@ -389,7 +392,10 @@ func resourceAwsSecurityGroupRead(d *schema.ResourceData, meta interface{}) erro log.Printf("[WARN] Error setting Egress rule set for (%s): %s", d.Id(), err) } - d.Set("tags", tagsToMap(sg.Tags)) + if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(sg.Tags).IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + return nil } @@ -427,11 +433,12 @@ func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) er } } - if !d.IsNewResource() { - if err := setTags(conn, d); err != nil { - return err + if d.HasChange("tags") && !d.IsNewResource() { + o, n := d.GetChange("tags") + + if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil { + return fmt.Errorf("error updating EC2 Security Group (%s) tags: %s", d.Id(), err) } - d.SetPartial("tags") } return resourceAwsSecurityGroupRead(d, meta) diff --git a/aws/tags.go b/aws/tags.go index db1f5438c4a0..0da481b7da68 100644 --- a/aws/tags.go +++ b/aws/tags.go @@ -3,13 +3,9 @@ package aws import ( "log" "regexp" - "strings" - "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" - "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) @@ -39,110 +35,6 @@ func tagsSchemaForceNew() *schema.Schema { } } -// 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 { - err := resource.Retry(5*time.Minute, func() *resource.RetryError { - log.Printf("[DEBUG] Removing tags: %#v from %s", remove, d.Id()) - _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ - Resources: []*string{aws.String(d.Id())}, - Tags: remove, - }) - if err != nil { - ec2err, ok := err.(awserr.Error) - if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) // retry - } - return resource.NonRetryableError(err) - } - return nil - }) - if err != nil { - // Retry without time bounds for EC2 throttling - if isResourceTimeoutError(err) { - log.Printf("[DEBUG] Removing tags: %#v from %s", remove, d.Id()) - _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ - Resources: []*string{aws.String(d.Id())}, - Tags: remove, - }) - if err != nil { - return err - } - } else { - return err - } - } - } - if len(create) > 0 { - err := resource.Retry(5*time.Minute, func() *resource.RetryError { - log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) - _, err := conn.CreateTags(&ec2.CreateTagsInput{ - Resources: []*string{aws.String(d.Id())}, - Tags: create, - }) - if err != nil { - ec2err, ok := err.(awserr.Error) - if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) // retry - } - return resource.NonRetryableError(err) - } - return nil - }) - if err != nil { - // Retry without time bounds for EC2 throttling - if isResourceTimeoutError(err) { - log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) - _, err := conn.CreateTags(&ec2.CreateTagsInput{ - Resources: []*string{aws.String(d.Id())}, - Tags: create, - }) - if err != nil { - return err - } - } else { - 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[aws.StringValue(t.Key)] = aws.StringValue(t.Value) - } - - // Build the list of what to remove - var remove []*ec2.Tag - for _, t := range oldTags { - old, ok := create[aws.StringValue(t.Key)] - if !ok || old != aws.StringValue(t.Value) { - remove = append(remove, t) - } else if ok { - // already present so remove from new - delete(create, aws.StringValue(t.Key)) - } - } - - 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)) diff --git a/aws/tags_test.go b/aws/tags_test.go index e48be9792732..b77da637ee98 100644 --- a/aws/tags_test.go +++ b/aws/tags_test.go @@ -2,7 +2,6 @@ package aws import ( "fmt" - "reflect" "testing" "github.com/aws/aws-sdk-go/aws" @@ -11,89 +10,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/terraform" ) -func TestDiffTags(t *testing.T) { - cases := []struct { - Old, New map[string]interface{} - Create, Remove map[string]string - }{ - // Add - { - Old: map[string]interface{}{ - "foo": "bar", - }, - New: map[string]interface{}{ - "foo": "bar", - "bar": "baz", - }, - Create: map[string]string{ - "bar": "baz", - }, - Remove: map[string]string{}, - }, - - // 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", - }, - }, - - // Overlap - { - Old: map[string]interface{}{ - "foo": "bar", - "hello": "world", - }, - New: map[string]interface{}{ - "foo": "baz", - "hello": "world", - }, - Create: map[string]string{ - "foo": "baz", - }, - Remove: map[string]string{ - "foo": "bar", - }, - }, - - // Remove - { - Old: map[string]interface{}{ - "foo": "bar", - "bar": "baz", - }, - New: map[string]interface{}{ - "foo": "bar", - }, - Create: map[string]string{}, - Remove: map[string]string{ - "bar": "baz", - }, - }, - } - - 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("%d: bad create: %#v", i, cm) - } - if !reflect.DeepEqual(rm, tc.Remove) { - t.Fatalf("%d: bad remove: %#v", i, rm) - } - } -} - func TestIgnoringTags(t *testing.T) { var ignoredTags []*ec2.Tag ignoredTags = append(ignoredTags, &ec2.Tag{