From 3d44f91da1b58948f426d0e63e2349c7edd17da6 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 7 Nov 2018 22:49:13 -0500 Subject: [PATCH 1/2] resource/aws_ebs_volume: Set tags on creation ``` --- PASS: TestAccAWSEBSVolume_basic (23.19s) --- PASS: TestAccAWSEBSVolume_withTags (23.28s) --- PASS: TestAccAWSEBSVolume_NoIops (23.31s) --- PASS: TestAccAWSEBSVolume_updateIops (43.79s) --- PASS: TestAccAWSEBSVolume_updateType (43.95s) --- PASS: TestAccAWSEBSVolume_updateSize (44.21s) --- PASS: TestAccAWSEBSVolume_kmsKey (52.88s) --- PASS: TestAccAWSEBSVolume_updateAttachedEbsVolume (167.64s) ``` --- aws/resource_aws_ebs_volume.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/aws/resource_aws_ebs_volume.go b/aws/resource_aws_ebs_volume.go index 1d37720b608a..45dc65a89ba8 100644 --- a/aws/resource_aws_ebs_volume.go +++ b/aws/resource_aws_ebs_volume.go @@ -91,6 +91,14 @@ func resourceAwsEbsVolumeCreate(d *schema.ResourceData, meta interface{}) error if value, ok := d.GetOk("snapshot_id"); ok { request.SnapshotId = aws.String(value.(string)) } + if value, ok := d.GetOk("tags"); ok { + request.TagSpecifications = []*ec2.TagSpecification{ + { + ResourceType: aws.String(ec2.ResourceTypeVolume), + Tags: tagsFromMap(value.(map[string]interface{})), + }, + } + } // IOPs are only valid, and required for, storage type io1. The current minimu // is 100. Instead of a hard validation we we only apply the IOPs to the @@ -138,12 +146,6 @@ func resourceAwsEbsVolumeCreate(d *schema.ResourceData, meta interface{}) error d.SetId(*result.VolumeId) - if _, ok := d.GetOk("tags"); ok { - if err := setTags(conn, d); err != nil { - return fmt.Errorf("Error setting tags for EBS Volume: %s", err) - } - } - return resourceAwsEbsVolumeRead(d, meta) } From d0ea6e0e7133735258f7d3bd881c846fd625b544 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 7 Nov 2018 22:49:55 -0500 Subject: [PATCH 2/2] tests/resource/aws_ebs_volume: Consolidate import test, enhance basic test, and refactor for current practices ``` --- PASS: TestAccAWSEBSVolume_basic (23.19s) --- PASS: TestAccAWSEBSVolume_withTags (23.28s) --- PASS: TestAccAWSEBSVolume_NoIops (23.31s) --- PASS: TestAccAWSEBSVolume_updateIops (43.79s) --- PASS: TestAccAWSEBSVolume_updateType (43.95s) --- PASS: TestAccAWSEBSVolume_updateSize (44.21s) --- PASS: TestAccAWSEBSVolume_kmsKey (52.88s) --- PASS: TestAccAWSEBSVolume_updateAttachedEbsVolume (167.64s) ``` --- aws/resource_aws_ebs_volume_test.go | 167 +++++++++++++++++----------- 1 file changed, 102 insertions(+), 65 deletions(-) diff --git a/aws/resource_aws_ebs_volume_test.go b/aws/resource_aws_ebs_volume_test.go index db343a82de5c..2d484625ce2e 100644 --- a/aws/resource_aws_ebs_volume_test.go +++ b/aws/resource_aws_ebs_volume_test.go @@ -13,17 +13,28 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestAccAWSEBSVolume_importBasic(t *testing.T) { +func TestAccAWSEBSVolume_basic(t *testing.T) { + var v ec2.Volume resourceName := "aws_ebs_volume.test" resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: resourceName, + Providers: testAccProviders, Steps: []resource.TestStep{ { Config: testAccAwsEbsVolumeConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckVolumeExists(resourceName, &v), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`volume/vol-.+`)), + resource.TestCheckResourceAttr(resourceName, "encrypted", "false"), + resource.TestCheckNoResourceAttr(resourceName, "iops"), + resource.TestCheckNoResourceAttr(resourceName, "kms_key_id"), + resource.TestCheckResourceAttr(resourceName, "size", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "type", "gp2"), + ), }, - { ResourceName: resourceName, ImportState: true, @@ -33,43 +44,32 @@ func TestAccAWSEBSVolume_importBasic(t *testing.T) { }) } -func TestAccAWSEBSVolume_basic(t *testing.T) { - var v ec2.Volume - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_ebs_volume.test", - Providers: testAccProviders, - Steps: []resource.TestStep{ - { - Config: testAccAwsEbsVolumeConfig, - Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttrSet("aws_ebs_volume.test", "arn"), - ), - }, - }, - }) -} - func TestAccAWSEBSVolume_updateAttachedEbsVolume(t *testing.T) { var v ec2.Volume + resourceName := "aws_ebs_volume.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_ebs_volume.test", + IDRefreshName: resourceName, Providers: testAccProviders, Steps: []resource.TestStep{ { Config: testAccAwsEbsAttachedVolumeConfig, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "size", "10"), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "size", "10"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, { Config: testAccAwsEbsAttachedVolumeConfigUpdateSize, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "size", "20"), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "size", "20"), ), }, }, @@ -78,23 +78,30 @@ func TestAccAWSEBSVolume_updateAttachedEbsVolume(t *testing.T) { func TestAccAWSEBSVolume_updateSize(t *testing.T) { var v ec2.Volume + resourceName := "aws_ebs_volume.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_ebs_volume.test", + IDRefreshName: resourceName, Providers: testAccProviders, Steps: []resource.TestStep{ { Config: testAccAwsEbsVolumeConfig, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "size", "1"), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "size", "1"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, { Config: testAccAwsEbsVolumeConfigUpdateSize, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "size", "10"), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "size", "10"), ), }, }, @@ -103,23 +110,30 @@ func TestAccAWSEBSVolume_updateSize(t *testing.T) { func TestAccAWSEBSVolume_updateType(t *testing.T) { var v ec2.Volume + resourceName := "aws_ebs_volume.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_ebs_volume.test", + IDRefreshName: resourceName, Providers: testAccProviders, Steps: []resource.TestStep{ { Config: testAccAwsEbsVolumeConfig, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "type", "gp2"), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "type", "gp2"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, { Config: testAccAwsEbsVolumeConfigUpdateType, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "type", "sc1"), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "type", "sc1"), ), }, }, @@ -128,23 +142,30 @@ func TestAccAWSEBSVolume_updateType(t *testing.T) { func TestAccAWSEBSVolume_updateIops(t *testing.T) { var v ec2.Volume + resourceName := "aws_ebs_volume.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_ebs_volume.test", + IDRefreshName: resourceName, Providers: testAccProviders, Steps: []resource.TestStep{ { Config: testAccAwsEbsVolumeConfigWithIops, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "iops", "100"), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "iops", "100"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, { Config: testAccAwsEbsVolumeConfigWithIopsUpdated, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "iops", "200"), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "iops", "200"), ), }, }, @@ -155,27 +176,35 @@ func TestAccAWSEBSVolume_kmsKey(t *testing.T) { var v ec2.Volume ri := acctest.RandInt() config := fmt.Sprintf(testAccAwsEbsVolumeConfigWithKmsKey, ri) - keyRegex := regexp.MustCompile("^arn:aws[\\w-]*:([a-zA-Z0-9\\-])+:([a-z-]+-\\d{1})?:(\\d{12})?:(.*)$") + kmsKeyResourceName := "aws_kms_key.test" + resourceName := "aws_ebs_volume.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_ebs_volume.test", + IDRefreshName: resourceName, Providers: testAccProviders, Steps: []resource.TestStep{ { Config: config, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.test", &v), - resource.TestCheckResourceAttr("aws_ebs_volume.test", "encrypted", "true"), - resource.TestMatchResourceAttr("aws_ebs_volume.test", "kms_key_id", keyRegex), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "encrypted", "true"), + resource.TestCheckResourceAttrPair(resourceName, "kms_key_id", kmsKeyResourceName, "arn"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } func TestAccAWSEBSVolume_NoIops(t *testing.T) { var v ec2.Volume + resourceName := "aws_ebs_volume.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -183,26 +212,41 @@ func TestAccAWSEBSVolume_NoIops(t *testing.T) { { Config: testAccAwsEbsVolumeConfigWithNoIops, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.iops_test", &v), + testAccCheckVolumeExists(resourceName, &v), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"iops"}, + }, }, }) } func TestAccAWSEBSVolume_withTags(t *testing.T) { var v ec2.Volume + resourceName := "aws_ebs_volume.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_ebs_volume.tags_test", + IDRefreshName: resourceName, Providers: testAccProviders, Steps: []resource.TestStep{ { Config: testAccAwsEbsVolumeConfigWithTags, Check: resource.ComposeTestCheckFunc( - testAccCheckVolumeExists("aws_ebs_volume.tags_test", &v), + testAccCheckVolumeExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.Name", "TerraformTest"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } @@ -242,9 +286,6 @@ resource "aws_ebs_volume" "test" { availability_zone = "${data.aws_availability_zones.available.names[0]}" type = "gp2" size = 1 - tags { - Name = "tf-acc-test-ebs-volume-test" - } } ` @@ -276,9 +317,7 @@ data "aws_ami" "debian_jessie_latest" { } resource "aws_instance" "test" { - ami = "${data.aws_ami.debian_jessie_latest.id}" - associate_public_ip_address = true - count = 1 + ami = "${data.aws_ami.debian_jessie_latest.id}" instance_type = "t2.medium" root_block_device { @@ -337,9 +376,7 @@ data "aws_ami" "debian_jessie_latest" { } resource "aws_instance" "test" { - ami = "${data.aws_ami.debian_jessie_latest.id}" - associate_public_ip_address = true - count = 1 + ami = "${data.aws_ami.debian_jessie_latest.id}" instance_type = "t2.medium" root_block_device { @@ -423,7 +460,7 @@ resource "aws_ebs_volume" "test" { ` const testAccAwsEbsVolumeConfigWithKmsKey = ` -resource "aws_kms_key" "foo" { +resource "aws_kms_key" "test" { description = "Terraform acc test %d" policy = <