Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/aws_ami: Refactor tagging logic to keyvaluetags package #10751

Merged
merged 8 commits into from
Nov 6, 2019
Merged

r/aws_ami: Refactor tagging logic to keyvaluetags package #10751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c0f6d5e
Merge pull request #5 from terraform-providers/master
DrFaust92 Oct 26, 2019
07ddc49
Merge pull request #6 from terraform-providers/master
DrFaust92 Oct 30, 2019
653c81e
Merge pull request #7 from terraform-providers/master
DrFaust92 Nov 2, 2019
e5b7302
Merge pull request #8 from terraform-providers/master
DrFaust92 Nov 4, 2019
a0f310e
refactor ami resource to use keyvaluetags
DrFaust92 Nov 5, 2019
1a5f89a
Update aws/resource_aws_ami_test.go
DrFaust92 Nov 6, 2019
e29db8e
Update aws/resource_aws_ami_test.go
DrFaust92 Nov 6, 2019
a872916
fix parm positioning
DrFaust92 Nov 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions aws/resource_aws_ami.go
View file
Open in desktop
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
Open in desktop
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)
}