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

@@ -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.

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)
 	}
 

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 {

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 {

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)
 	}
 

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"]

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
+}

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

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