Skip to content

Commit

Permalink
r/aws_ami: Refactor tagging logic to keyvaluetags package (#10751)
Browse files Browse the repository at this point in the history 
Output from acceptance testing:

```
--- PASS: TestAccAWSAMI_snapshotSize (47.60s)
--- PASS: TestAccAWSAMI_basic (48.16s)
--- PASS: TestAccAWSAMI_tags (59.53s)
```
  • Loading branch information
@DrFaust92 @bflad
DrFaust92 authored and bflad committed Nov 6, 2019
1 parent f39af7f commit 786fce7
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 28 deletions.
19 changes: 14 additions & 5 deletions aws/resource_aws_ami.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/helper/hashcode"
"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"
)

const (
Expand Down Expand Up @@ -278,12 +279,18 @@ func resourceAwsAmiCreate(d *schema.ResourceData, meta interface{}) error {
id := *res.ImageId
d.SetId(id)

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(client, id, nil, v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}

_, err = resourceAwsAmiWaitForAvailable(d.Timeout(schema.TimeoutCreate), id, client)
if err != nil {
return err
}

return resourceAwsAmiUpdate(d, meta)
return resourceAwsAmiRead(d, meta)
}

func resourceAwsAmiRead(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -404,10 +411,12 @@ func resourceAwsAmiUpdate(d *schema.ResourceData, meta interface{}) error {

d.Partial(true)

if err := setTags(client, d); err != nil {
return err
} else {
d.SetPartial("tags")
if d.HasChange("tags") {
o, n := d.GetChange("tags")

if err := keyvaluetags.Ec2UpdateTags(client, d.Id(), o, n); err != nil {
return fmt.Errorf("error updating AMI (%s) tags: %s", d.Id(), err)
}
}

if d.Get("description").(string) != "" {
Expand Down
105 changes: 82 additions & 23 deletions aws/resource_aws_ami_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestAccAWSAMI_basic(t *testing.T) {
CheckDestroy: testAccCheckAmiDestroy,
Steps: []resource.TestStep{
{
Config: testAccAmiConfig_basic(rName),
Config: testAccAmiConfig_basic(rName, 8),
Check: resource.ComposeTestCheckFunc(
testAccCheckAmiExists(resourceName, &ami),
resource.TestCheckResourceAttr(resourceName, "ena_support", "true"),
Expand All @@ -46,6 +46,53 @@ func TestAccAWSAMI_basic(t *testing.T) {
})
}

func TestAccAWSAMI_tags(t *testing.T) {
var ami ec2.Image
resourceName := "aws_ami.test"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAmiDestroy,
Steps: []resource.TestStep{
{
Config: testAccAmiConfigTags1(rName, "key1", "value1", 8),
Check: resource.ComposeTestCheckFunc(
testAccCheckAmiExists(resourceName, &ami),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"manage_ebs_snapshots",
},
},
{
Config: testAccAmiConfigTags2(rName, "key1", "value1updated", "key2", "value2", 8),
Check: resource.ComposeTestCheckFunc(
testAccCheckAmiExists(resourceName, &ami),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
),
},
{
Config: testAccAmiConfigTags1(rName, "key2", "value2", 8),
Check: resource.ComposeTestCheckFunc(
testAccCheckAmiExists(resourceName, &ami),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
),
},
},
})
}

func TestAccAWSAMI_snapshotSize(t *testing.T) {
var ami ec2.Image
var bd ec2.BlockDeviceMapping
Expand All @@ -66,7 +113,7 @@ func TestAccAWSAMI_snapshotSize(t *testing.T) {
CheckDestroy: testAccCheckAmiDestroy,
Steps: []resource.TestStep{
{
Config: testAccAmiConfig_snapshotSize(rName),
Config: testAccAmiConfig_basic(rName, 20),
Check: resource.ComposeTestCheckFunc(
testAccCheckAmiExists(resourceName, &ami),
testAccCheckAmiBlockDevice(&ami, &bd, "/dev/sda1"),
Expand Down Expand Up @@ -218,30 +265,35 @@ func testAccCheckAmiEbsBlockDevice(bd *ec2.BlockDeviceMapping, ed *ec2.EbsBlockD
}
}

func testAccAmiConfig_basic(rName string) string {
func testAccAmiConfig_base(rName string, size int) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {}
resource "aws_ebs_volume" "foo" {
availability_zone = "${data.aws_availability_zones.available.names[0]}"
size = 8
size = %d
tags = {
Name = "testAccAmiConfig_basic"
Name = "%[2]s"
}
}
resource "aws_ebs_snapshot" "foo" {
volume_id = "${aws_ebs_volume.foo.id}"
tags = {
Name = "testAccAmiConfig_basic"
Name = "%[2]s"
}
}
`, size, rName)
}

func testAccAmiConfig_basic(rName string, size int) string {
return testAccAmiConfig_base(rName, size) + fmt.Sprintf(`
resource "aws_ami" "test" {
ena_support = true
name = %q
name = %[1]q
root_device_name = "/dev/sda1"
virtualization_type = "hvm"
Expand All @@ -253,36 +305,43 @@ resource "aws_ami" "test" {
`, rName)
}

func testAccAmiConfig_snapshotSize(rName string) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {}
resource "aws_ebs_volume" "foo" {
availability_zone = "${data.aws_availability_zones.available.names[0]}"
size = 20
func testAccAmiConfigTags1(rName, tagKey1, tagValue1 string, size int) string {
return testAccAmiConfig_base(rName, size) + fmt.Sprintf(`
resource "aws_ami" "test" {
ena_support = true
name = %[1]q
root_device_name = "/dev/sda1"
virtualization_type = "hvm"
tags = {
Name = "testAccAmiConfig_snapshotSize"
ebs_block_device {
device_name = "/dev/sda1"
snapshot_id = "${aws_ebs_snapshot.foo.id}"
}
}
resource "aws_ebs_snapshot" "foo" {
volume_id = "${aws_ebs_volume.foo.id}"
tags = {
Name = "TestAccAWSAMI_snapshotSize"
%[2]q = %[3]q
}
}
`, rName, tagKey1, tagValue1)
}

func testAccAmiConfigTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string, size int) string {
return testAccAmiConfig_base(rName, size) + fmt.Sprintf(`
resource "aws_ami" "test" {
name = %q
ena_support = true
name = %[1]q
root_device_name = "/dev/sda1"
virtualization_type = "hvm"
ebs_block_device {
device_name = "/dev/sda1"
snapshot_id = "${aws_ebs_snapshot.foo.id}"
}
tags = {
%[2]q = %[3]q
%[4]q = %[5]q
}
}
`, rName)
`, rName, tagKey1, tagValue1, tagKey2, tagValue2)
}

0 comments on commit 786fce7

Please sign in to comment.