From d721ff6d66e2377f76f9b61f1133769386311eb6 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Wed, 26 Apr 2017 08:11:21 +1000 Subject: [PATCH] provider/aws: Sort AMI and snapshot IDs (#13866) As a follow up to #13844, this pull request sorts the AMIs and snapshots returned from the aws_ami_ids and aws_ebs_snapshot_ids data sources, respectively. --- builtin/providers/aws/data_source_aws_ami.go | 16 +--- .../providers/aws/data_source_aws_ami_ids.go | 5 +- .../aws/data_source_aws_ami_ids_test.go | 80 +++++++++++++++++-- .../aws/data_source_aws_ebs_snapshot.go | 15 +--- .../aws/data_source_aws_ebs_snapshot_ids.go | 5 +- .../data_source_aws_ebs_snapshot_ids_test.go | 74 ++++++++++++++++- builtin/providers/aws/sort.go | 53 ++++++++++++ .../providers/aws/d/ami_ids.html.markdown | 3 +- .../aws/d/ebs_snapshot_ids.html.markdown | 3 +- 9 files changed, 211 insertions(+), 43 deletions(-) create mode 100644 builtin/providers/aws/sort.go diff --git a/builtin/providers/aws/data_source_aws_ami.go b/builtin/providers/aws/data_source_aws_ami.go index 877069a22..05513c32e 100644 --- a/builtin/providers/aws/data_source_aws_ami.go +++ b/builtin/providers/aws/data_source_aws_ami.go @@ -5,8 +5,6 @@ import ( "fmt" "log" "regexp" - "sort" - "time" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform/helper/hashcode" @@ -249,21 +247,9 @@ func dataSourceAwsAmiRead(d *schema.ResourceData, meta interface{}) error { return amiDescriptionAttributes(d, image) } -type imageSort []*ec2.Image - -func (a imageSort) Len() int { return len(a) } -func (a imageSort) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a imageSort) Less(i, j int) bool { - itime, _ := time.Parse(time.RFC3339, *a[i].CreationDate) - jtime, _ := time.Parse(time.RFC3339, *a[j].CreationDate) - return itime.Unix() < jtime.Unix() -} - // Returns the most recent AMI out of a slice of images. func mostRecentAmi(images []*ec2.Image) *ec2.Image { - sortedImages := images - sort.Sort(imageSort(sortedImages)) - return sortedImages[len(sortedImages)-1] + return sortImages(images)[0] } // populate the numerous fields that the image description returns. diff --git a/builtin/providers/aws/data_source_aws_ami_ids.go b/builtin/providers/aws/data_source_aws_ami_ids.go index bbf4438d5..20df34ac3 100644 --- a/builtin/providers/aws/data_source_aws_ami_ids.go +++ b/builtin/providers/aws/data_source_aws_ami_ids.go @@ -36,10 +36,9 @@ func dataSourceAwsAmiIds() *schema.Resource { }, "tags": dataSourceTagsSchema(), "ids": &schema.Schema{ - Type: schema.TypeSet, + Type: schema.TypeList, Computed: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, }, }, } @@ -101,7 +100,7 @@ func dataSourceAwsAmiIdsRead(d *schema.ResourceData, meta interface{}) error { filteredImages = resp.Images[:] } - for _, image := range filteredImages { + for _, image := range sortImages(filteredImages) { imageIds = append(imageIds, *image.ImageId) } diff --git a/builtin/providers/aws/data_source_aws_ami_ids_test.go b/builtin/providers/aws/data_source_aws_ami_ids_test.go index e2a7ac2d8..52582eaba 100644 --- a/builtin/providers/aws/data_source_aws_ami_ids_test.go +++ b/builtin/providers/aws/data_source_aws_ami_ids_test.go @@ -1,9 +1,11 @@ package aws import ( + "fmt" "testing" "github.com/hashicorp/terraform/helper/resource" + "github.com/satori/uuid" ) func TestAccDataSourceAwsAmiIds_basic(t *testing.T) { @@ -21,6 +23,37 @@ func TestAccDataSourceAwsAmiIds_basic(t *testing.T) { }) } +func TestAccDataSourceAwsAmiIds_sorted(t *testing.T) { + uuid := uuid.NewV4().String() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccDataSourceAwsAmiIdsConfig_sorted1(uuid), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("aws_ami_from_instance.a", "id"), + resource.TestCheckResourceAttrSet("aws_ami_from_instance.b", "id"), + ), + }, + { + Config: testAccDataSourceAwsAmiIdsConfig_sorted2(uuid), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsEbsSnapshotDataSourceID("data.aws_ami_ids.test"), + resource.TestCheckResourceAttr("data.aws_ami_ids.test", "ids.#", "2"), + resource.TestCheckResourceAttrPair( + "data.aws_ami_ids.test", "ids.0", + "aws_ami_from_instance.b", "id"), + resource.TestCheckResourceAttrPair( + "data.aws_ami_ids.test", "ids.1", + "aws_ami_from_instance.a", "id"), + ), + }, + }, + }) +} + func TestAccDataSourceAwsAmiIds_empty(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -39,15 +72,52 @@ func TestAccDataSourceAwsAmiIds_empty(t *testing.T) { const testAccDataSourceAwsAmiIdsConfig_basic = ` data "aws_ami_ids" "ubuntu" { - owners = ["099720109477"] + owners = ["099720109477"] - filter { - name = "name" - values = ["ubuntu/images/ubuntu-*-*-amd64-server-*"] - } + filter { + name = "name" + values = ["ubuntu/images/ubuntu-*-*-amd64-server-*"] + } } ` +func testAccDataSourceAwsAmiIdsConfig_sorted1(uuid string) string { + return fmt.Sprintf(` +resource "aws_instance" "test" { + ami = "ami-efd0428f" + instance_type = "m3.medium" + + count = 2 +} + +resource "aws_ami_from_instance" "a" { + name = "tf-test-%s-a" + source_instance_id = "${aws_instance.test.*.id[0]}" + snapshot_without_reboot = true +} + +resource "aws_ami_from_instance" "b" { + name = "tf-test-%s-b" + source_instance_id = "${aws_instance.test.*.id[1]}" + snapshot_without_reboot = true + + // We want to ensure that 'aws_ami_from_instance.a.creation_date' is less + // than 'aws_ami_from_instance.b.creation_date' so that we can ensure that + // the images are being sorted correctly. + depends_on = ["aws_ami_from_instance.a"] +} +`, uuid, uuid) +} + +func testAccDataSourceAwsAmiIdsConfig_sorted2(uuid string) string { + return testAccDataSourceAwsAmiIdsConfig_sorted1(uuid) + fmt.Sprintf(` +data "aws_ami_ids" "test" { + owners = ["self"] + name_regex = "^tf-test-%s-" +} +`, uuid) +} + const testAccDataSourceAwsAmiIdsConfig_empty = ` data "aws_ami_ids" "empty" { filter { diff --git a/builtin/providers/aws/data_source_aws_ebs_snapshot.go b/builtin/providers/aws/data_source_aws_ebs_snapshot.go index b16f61f22..a36334723 100644 --- a/builtin/providers/aws/data_source_aws_ebs_snapshot.go +++ b/builtin/providers/aws/data_source_aws_ebs_snapshot.go @@ -3,7 +3,6 @@ package aws import ( "fmt" "log" - "sort" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform/helper/schema" @@ -138,20 +137,8 @@ func dataSourceAwsEbsSnapshotRead(d *schema.ResourceData, meta interface{}) erro return snapshotDescriptionAttributes(d, snapshot) } -type snapshotSort []*ec2.Snapshot - -func (a snapshotSort) Len() int { return len(a) } -func (a snapshotSort) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a snapshotSort) Less(i, j int) bool { - itime := *a[i].StartTime - jtime := *a[j].StartTime - return itime.Unix() < jtime.Unix() -} - func mostRecentSnapshot(snapshots []*ec2.Snapshot) *ec2.Snapshot { - sortedSnapshots := snapshots - sort.Sort(snapshotSort(sortedSnapshots)) - return sortedSnapshots[len(sortedSnapshots)-1] + return sortSnapshots(snapshots)[0] } func snapshotDescriptionAttributes(d *schema.ResourceData, snapshot *ec2.Snapshot) error { diff --git a/builtin/providers/aws/data_source_aws_ebs_snapshot_ids.go b/builtin/providers/aws/data_source_aws_ebs_snapshot_ids.go index 57dc20e9c..bd4f2ad8b 100644 --- a/builtin/providers/aws/data_source_aws_ebs_snapshot_ids.go +++ b/builtin/providers/aws/data_source_aws_ebs_snapshot_ids.go @@ -28,10 +28,9 @@ func dataSourceAwsEbsSnapshotIds() *schema.Resource { }, "tags": dataSourceTagsSchema(), "ids": &schema.Schema{ - Type: schema.TypeSet, + Type: schema.TypeList, Computed: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, }, }, } @@ -67,7 +66,7 @@ func dataSourceAwsEbsSnapshotIdsRead(d *schema.ResourceData, meta interface{}) e snapshotIds := make([]string, 0) - for _, snapshot := range resp.Snapshots { + for _, snapshot := range sortSnapshots(resp.Snapshots) { snapshotIds = append(snapshotIds, *snapshot.SnapshotId) } diff --git a/builtin/providers/aws/data_source_aws_ebs_snapshot_ids_test.go b/builtin/providers/aws/data_source_aws_ebs_snapshot_ids_test.go index 869152ac4..0c5f3ec4d 100644 --- a/builtin/providers/aws/data_source_aws_ebs_snapshot_ids_test.go +++ b/builtin/providers/aws/data_source_aws_ebs_snapshot_ids_test.go @@ -1,9 +1,11 @@ package aws import ( + "fmt" "testing" "github.com/hashicorp/terraform/helper/resource" + "github.com/satori/uuid" ) func TestAccDataSourceAwsEbsSnapshotIds_basic(t *testing.T) { @@ -21,6 +23,37 @@ func TestAccDataSourceAwsEbsSnapshotIds_basic(t *testing.T) { }) } +func TestAccDataSourceAwsEbsSnapshotIds_sorted(t *testing.T) { + uuid := uuid.NewV4().String() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccDataSourceAwsEbsSnapshotIdsConfig_sorted1(uuid), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("aws_ebs_snapshot.a", "id"), + resource.TestCheckResourceAttrSet("aws_ebs_snapshot.b", "id"), + ), + }, + { + Config: testAccDataSourceAwsEbsSnapshotIdsConfig_sorted2(uuid), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsEbsSnapshotDataSourceID("data.aws_ebs_snapshot_ids.test"), + resource.TestCheckResourceAttr("data.aws_ebs_snapshot_ids.test", "ids.#", "2"), + resource.TestCheckResourceAttrPair( + "data.aws_ebs_snapshot_ids.test", "ids.0", + "aws_ebs_snapshot.b", "id"), + resource.TestCheckResourceAttrPair( + "data.aws_ebs_snapshot_ids.test", "ids.1", + "aws_ebs_snapshot.a", "id"), + ), + }, + }, + }) +} + func TestAccDataSourceAwsEbsSnapshotIds_empty(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -40,7 +73,7 @@ func TestAccDataSourceAwsEbsSnapshotIds_empty(t *testing.T) { const testAccDataSourceAwsEbsSnapshotIdsConfig_basic = ` resource "aws_ebs_volume" "test" { availability_zone = "us-west-2a" - size = 40 + size = 1 } resource "aws_ebs_snapshot" "test" { @@ -52,6 +85,45 @@ data "aws_ebs_snapshot_ids" "test" { } ` +func testAccDataSourceAwsEbsSnapshotIdsConfig_sorted1(uuid string) string { + return fmt.Sprintf(` +resource "aws_ebs_volume" "test" { + availability_zone = "us-west-2a" + size = 1 + + count = 2 +} + +resource "aws_ebs_snapshot" "a" { + volume_id = "${aws_ebs_volume.test.*.id[0]}" + description = "tf-test-%s" +} + +resource "aws_ebs_snapshot" "b" { + volume_id = "${aws_ebs_volume.test.*.id[1]}" + description = "tf-test-%s" + + // We want to ensure that 'aws_ebs_snapshot.a.creation_date' is less than + // 'aws_ebs_snapshot.b.creation_date'/ so that we can ensure that the + // snapshots are being sorted correctly. + depends_on = ["aws_ebs_snapshot.a"] +} +`, uuid, uuid) +} + +func testAccDataSourceAwsEbsSnapshotIdsConfig_sorted2(uuid string) string { + return testAccDataSourceAwsEbsSnapshotIdsConfig_sorted1(uuid) + fmt.Sprintf(` +data "aws_ebs_snapshot_ids" "test" { + owners = ["self"] + + filter { + name = "description" + values = ["tf-test-%s"] + } +} +`, uuid) +} + const testAccDataSourceAwsEbsSnapshotIdsConfig_empty = ` data "aws_ebs_snapshot_ids" "empty" { owners = ["000000000000"] diff --git a/builtin/providers/aws/sort.go b/builtin/providers/aws/sort.go new file mode 100644 index 000000000..0d90458eb --- /dev/null +++ b/builtin/providers/aws/sort.go @@ -0,0 +1,53 @@ +package aws + +import ( + "sort" + "time" + + "github.com/aws/aws-sdk-go/service/ec2" +) + +type imageSort []*ec2.Image +type snapshotSort []*ec2.Snapshot + +func (a imageSort) Len() int { + return len(a) +} + +func (a imageSort) Swap(i, j int) { + a[i], a[j] = a[j], a[i] +} + +func (a imageSort) Less(i, j int) bool { + itime, _ := time.Parse(time.RFC3339, *a[i].CreationDate) + jtime, _ := time.Parse(time.RFC3339, *a[j].CreationDate) + return itime.Unix() < jtime.Unix() +} + +// Sort images by creation date, in descending order. +func sortImages(images []*ec2.Image) []*ec2.Image { + sortedImages := images + sort.Sort(sort.Reverse(imageSort(sortedImages))) + return sortedImages +} + +func (a snapshotSort) Len() int { + return len(a) +} + +func (a snapshotSort) Swap(i, j int) { + a[i], a[j] = a[j], a[i] +} + +func (a snapshotSort) Less(i, j int) bool { + itime := *a[i].StartTime + jtime := *a[j].StartTime + return itime.Unix() < jtime.Unix() +} + +// Sort snapshots by creation date, in descending order. +func sortSnapshots(snapshots []*ec2.Snapshot) []*ec2.Snapshot { + sortedSnapshots := snapshots + sort.Sort(sort.Reverse(snapshotSort(sortedSnapshots))) + return sortedSnapshots +} diff --git a/website/source/docs/providers/aws/d/ami_ids.html.markdown b/website/source/docs/providers/aws/d/ami_ids.html.markdown index 526977bdc..8bad15bee 100644 --- a/website/source/docs/providers/aws/d/ami_ids.html.markdown +++ b/website/source/docs/providers/aws/d/ami_ids.html.markdown @@ -46,6 +46,7 @@ options to narrow down the list AWS returns. ## Attributes Reference -`ids` is set to the list of AMI IDs. +`ids` is set to the list of AMI IDs, sorted by creation time in descending +order. [1]: http://docs.aws.amazon.com/cli/latest/reference/ec2/describe-images.html diff --git a/website/source/docs/providers/aws/d/ebs_snapshot_ids.html.markdown b/website/source/docs/providers/aws/d/ebs_snapshot_ids.html.markdown index 6d4ef617d..dfe472b5f 100644 --- a/website/source/docs/providers/aws/d/ebs_snapshot_ids.html.markdown +++ b/website/source/docs/providers/aws/d/ebs_snapshot_ids.html.markdown @@ -43,6 +43,7 @@ several valid keys, for a full reference, check out ## Attributes Reference -`ids` is set to the list of EBS snapshot IDs. +`ids` is set to the list of EBS snapshot IDs, sorted by creation time in +descending order. [1]: http://docs.aws.amazon.com/cli/latest/reference/ec2/describe-snapshots.html