data-source/aws_ami and data-source/aws_ami_ids: Require owners argument

* data-source/aws_ami: Switch owners argument from Optional to Required * data-source/aws_ami_ids: Switch owners argument from Optional to Required * tests: Update aws_ami data sources to use owners instead of filter > name = "owner-alias" Output from acceptance testing: ``` --- PASS: TestAccAWSAmiDataSource_instanceStore (8.89s) --- PASS: TestAccDataSourceAwsAmiIds_basic (9.72s) --- PASS: TestAccAWSAmiDataSource_natInstance (9.72s) --- PASS: TestAccAWSAmiDataSource_localNameFilter (12.65s) --- PASS: TestAccDataSourceAwsAmiIds_sorted (14.02s) --- PASS: TestAccAWSAmiDataSource_windowsInstance (16.91s) ```
hashicorp · Feb 22, 2019 · 493e23f · 493e23f
1 parent 98f11ae
commit 493e23f
Show file tree

Hide file tree

Showing 16 changed files with 108 additions and 455 deletions.
diff --git a/aws/data_source_aws_ami.go b/aws/data_source_aws_ami.go
@@ -41,9 +41,12 @@ func dataSourceAwsAmi() *schema.Resource {
 			},
 			"owners": {
 				Type:     schema.TypeList,
-				Optional: true,
-				ForceNew: true,
-				Elem:     &schema.Schema{Type: schema.TypeString},
+				Required: true,
+				MinItems: 1,
+				Elem: &schema.Schema{
+					Type:         schema.TypeString,
+					ValidateFunc: validation.NoZeroValues,
+				},
 			},
 			// Computed values.
 			"architecture": {
@@ -182,49 +185,15 @@ func dataSourceAwsAmi() *schema.Resource {
 func dataSourceAwsAmiRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).ec2conn
 
-	executableUsers, executableUsersOk := d.GetOk("executable_users")
-	filters, filtersOk := d.GetOk("filter")
-	nameRegex, nameRegexOk := d.GetOk("name_regex")
-	owners, ownersOk := d.GetOk("owners")
-
-	if !executableUsersOk && !filtersOk && !nameRegexOk && !ownersOk {
-		return fmt.Errorf("One of executable_users, filters, name_regex, or owners must be assigned")
-	}
-
-	params := &ec2.DescribeImagesInput{}
-	if executableUsersOk {
-		params.ExecutableUsers = expandStringList(executableUsers.([]interface{}))
-	}
-	if filtersOk {
-		params.Filters = buildAwsDataSourceFilters(filters.(*schema.Set))
+	params := &ec2.DescribeImagesInput{
+		Owners: expandStringList(d.Get("owners").([]interface{})),
 	}
-	if ownersOk {
-		o := expandStringList(owners.([]interface{}))
 
-		if len(o) > 0 {
-			params.Owners = o
-		}
+	if v, ok := d.GetOk("executable_users"); ok {
+		params.ExecutableUsers = expandStringList(v.([]interface{}))
 	}
-
-	// Deprecated: pre-2.0.0 warning logging
-	if !ownersOk {
-		log.Print("[WARN] The \"owners\" argument will become required in the next major version.")
-		log.Print("[WARN] Documentation can be found at: https://www.terraform.io/docs/providers/aws/d/ami.html#owners")
-
-		missingOwnerFilter := true
-
-		if filtersOk {
-			for _, filter := range params.Filters {
-				if aws.StringValue(filter.Name) == "owner-alias" || aws.StringValue(filter.Name) == "owner-id" {
-					missingOwnerFilter = false
-					break
-				}
-			}
-		}
-
-		if missingOwnerFilter {
-			log.Print("[WARN] Potential security issue: missing \"owners\" filtering for AMI. Check AMI to ensure it came from trusted source.")
-		}
+	if v, ok := d.GetOk("filter"); ok {
+		params.Filters = buildAwsDataSourceFilters(v.(*schema.Set))
 	}
 
 	log.Printf("[DEBUG] Reading AMI: %s", params)
@@ -234,7 +203,7 @@ func dataSourceAwsAmiRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	var filteredImages []*ec2.Image
-	if nameRegexOk {
+	if nameRegex, ok := d.GetOk("name_regex"); ok {
 		r := regexp.MustCompile(nameRegex.(string))
 		for _, image := range resp.Images {
 			// Check for a very rare case where the response would include no

diff --git a/aws/data_source_aws_ami_ids.go b/aws/data_source_aws_ami_ids.go
@@ -34,9 +34,12 @@ func dataSourceAwsAmiIds() *schema.Resource {
 			},
 			"owners": {
 				Type:     schema.TypeList,
-				Optional: true,
-				ForceNew: true,
-				Elem:     &schema.Schema{Type: schema.TypeString},
+				Required: true,
+				MinItems: 1,
+				Elem: &schema.Schema{
+					Type:         schema.TypeString,
+					ValidateFunc: validation.NoZeroValues,
+				},
 			},
 			"ids": {
 				Type:     schema.TypeList,
@@ -55,51 +58,15 @@ func dataSourceAwsAmiIds() *schema.Resource {
 func dataSourceAwsAmiIdsRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).ec2conn
 
-	executableUsers, executableUsersOk := d.GetOk("executable_users")
-	filters, filtersOk := d.GetOk("filter")
-	nameRegex, nameRegexOk := d.GetOk("name_regex")
-	owners, ownersOk := d.GetOk("owners")
-	sortAscending := d.Get("sort_ascending").(bool)
-
-	if !executableUsersOk && !filtersOk && !nameRegexOk && !ownersOk {
-		return fmt.Errorf("One of executable_users, filters, name_regex, or owners must be assigned")
-	}
-
-	params := &ec2.DescribeImagesInput{}
-
-	if executableUsersOk {
-		params.ExecutableUsers = expandStringList(executableUsers.([]interface{}))
-	}
-	if filtersOk {
-		params.Filters = buildAwsDataSourceFilters(filters.(*schema.Set))
+	params := &ec2.DescribeImagesInput{
+		Owners: expandStringList(d.Get("owners").([]interface{})),
 	}
-	if ownersOk {
-		o := expandStringList(owners.([]interface{}))
 
-		if len(o) > 0 {
-			params.Owners = o
-		}
+	if v, ok := d.GetOk("executable_users"); ok {
+		params.ExecutableUsers = expandStringList(v.([]interface{}))
 	}
-
-	// Deprecated: pre-2.0.0 warning logging
-	if !ownersOk {
-		log.Print("[WARN] The \"owners\" argument will become required in the next major version.")
-		log.Print("[WARN] Documentation can be found at: https://www.terraform.io/docs/providers/aws/d/ami.html#owners")
-
-		missingOwnerFilter := true
-
-		if filtersOk {
-			for _, filter := range params.Filters {
-				if aws.StringValue(filter.Name) == "owner-alias" || aws.StringValue(filter.Name) == "owner-id" {
-					missingOwnerFilter = false
-					break
-				}
-			}
-		}
-
-		if missingOwnerFilter {
-			log.Print("[WARN] Potential security issue: missing \"owners\" filtering for AMI. Check AMI to ensure it came from trusted source.")
-		}
+	if v, ok := d.GetOk("filter"); ok {
+		params.Filters = buildAwsDataSourceFilters(v.(*schema.Set))
 	}
 
 	log.Printf("[DEBUG] Reading AMI IDs: %s", params)
@@ -111,7 +78,7 @@ func dataSourceAwsAmiIdsRead(d *schema.ResourceData, meta interface{}) error {
 	var filteredImages []*ec2.Image
 	imageIds := make([]string, 0)
 
-	if nameRegexOk {
+	if nameRegex, ok := d.GetOk("name_regex"); ok {
 		r := regexp.MustCompile(nameRegex.(string))
 		for _, image := range resp.Images {
 			// Check for a very rare case where the response would include no
@@ -134,7 +101,7 @@ func dataSourceAwsAmiIdsRead(d *schema.ResourceData, meta interface{}) error {
 	sort.Slice(filteredImages, func(i, j int) bool {
 		itime, _ := time.Parse(time.RFC3339, aws.StringValue(filteredImages[i].CreationDate))
 		jtime, _ := time.Parse(time.RFC3339, aws.StringValue(filteredImages[j].CreationDate))
-		if sortAscending {
+		if d.Get("sort_ascending").(bool) {
 			return itime.Unix() < jtime.Unix()
 		}
 		return itime.Unix() > jtime.Unix()

diff --git a/aws/data_source_aws_ami_ids_test.go b/aws/data_source_aws_ami_ids_test.go
@@ -58,22 +58,6 @@ func TestAccDataSourceAwsAmiIds_sorted(t *testing.T) {
 	})
 }
 
-func TestAccDataSourceAwsAmiIds_empty(t *testing.T) {
-	resource.ParallelTest(t, resource.TestCase{
-		PreCheck:  func() { testAccPreCheck(t) },
-		Providers: testAccProviders,
-		Steps: []resource.TestStep{
-			{
-				Config: testAccDataSourceAwsAmiIdsConfig_empty,
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAwsAmiDataSourceID("data.aws_ami_ids.empty"),
-					resource.TestCheckResourceAttr("data.aws_ami_ids.empty", "ids.#", "0"),
-				),
-			},
-		},
-	})
-}
-
 const testAccDataSourceAwsAmiIdsConfig_basic = `
 data "aws_ami_ids" "ubuntu" {
     owners = ["099720109477"]
@@ -113,12 +97,3 @@ data "aws_ami_ids" "test" {
 }
 `, sort_ascending)
 }
-
-const testAccDataSourceAwsAmiIdsConfig_empty = `
-data "aws_ami_ids" "empty" {
-  filter {
-    name   = "name"
-    values = []
-  }
-}
-`
diff --git a/aws/data_source_aws_ami_test.go b/aws/data_source_aws_ami_test.go
@@ -127,37 +127,6 @@ func TestAccAWSAmiDataSource_instanceStore(t *testing.T) {
 	})
 }
 
-func TestAccAWSAmiDataSource_owners(t *testing.T) {
-	resource.ParallelTest(t, resource.TestCase{
-		PreCheck:  func() { testAccPreCheck(t) },
-		Providers: testAccProviders,
-		Steps: []resource.TestStep{
-			{
-				Config: testAccCheckAwsAmiDataSourceOwnersConfig,
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAwsAmiDataSourceID("data.aws_ami.amazon_ami"),
-				),
-			},
-		},
-	})
-}
-
-// Acceptance test for: https://github.com/hashicorp/terraform/issues/10758
-func TestAccAWSAmiDataSource_ownersEmpty(t *testing.T) {
-	resource.ParallelTest(t, resource.TestCase{
-		PreCheck:  func() { testAccPreCheck(t) },
-		Providers: testAccProviders,
-		Steps: []resource.TestStep{
-			{
-				Config: testAccCheckAwsAmiDataSourceEmptyOwnersConfig,
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAwsAmiDataSourceID("data.aws_ami.amazon_ami"),
-				),
-			},
-		},
-	})
-}
-
 func TestAccAWSAmiDataSource_localNameFilter(t *testing.T) {
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:  func() { testAccPreCheck(t) },
@@ -175,7 +144,6 @@ func TestAccAWSAmiDataSource_localNameFilter(t *testing.T) {
 }
 
 func testAccCheckAwsAmiDataSourceID(n string) resource.TestCheckFunc {
-	// Wait for IAM role
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
 		if !ok {
@@ -196,10 +164,8 @@ func testAccCheckAwsAmiDataSourceID(n string) resource.TestCheckFunc {
 const testAccCheckAwsAmiDataSourceConfig = `
 data "aws_ami" "nat_ami" {
   most_recent = true
-  filter {
-    name = "owner-alias"
-    values = ["amazon"]
-  }
+  owners      = ["amazon"]
+
   filter {
     name = "name"
     values = ["amzn-ami-vpc-nat*"]
@@ -223,10 +189,8 @@ data "aws_ami" "nat_ami" {
 const testAccCheckAwsAmiDataSourceWindowsConfig = `
 data "aws_ami" "windows_ami" {
   most_recent = true
-  filter {
-    name = "owner-alias"
-    values = ["amazon"]
-  }
+  owners      = ["amazon"]
+
   filter {
     name = "name"
     values = ["Windows_Server-2012-R2*"]
@@ -250,10 +214,8 @@ data "aws_ami" "windows_ami" {
 const testAccCheckAwsAmiDataSourceInstanceStoreConfig = `
 data "aws_ami" "instance_store_ami" {
   most_recent = true
-  filter {
-    name = "owner-id"
-    values = ["099720109477"]
-  }
+  owners      = ["099720109477"]
+
   filter {
     name = "name"
     values = ["ubuntu/images/hvm-instance/ubuntu-trusty-14.04-amd64-server*"]
@@ -269,38 +231,6 @@ data "aws_ami" "instance_store_ami" {
 }
 `
 
-// Testing owner parameter
-const testAccCheckAwsAmiDataSourceOwnersConfig = `
-data "aws_ami" "amazon_ami" {
-  most_recent = true
-  owners = ["amazon"]
-}
-`
-
-const testAccCheckAwsAmiDataSourceEmptyOwnersConfig = `
-data "aws_ami" "amazon_ami" {
-  most_recent = true
-  owners = [""]
-
-  # we need to test the owners = [""] for regressions but we want to filter the results
-  # beyond all public AWS AMIs :)
-  filter {
-    name = "owner-alias"
-    values = ["amazon"]
-  }
-
-  filter {
-    name = "name"
-    values = ["amzn-ami-minimal-hvm-*"]
-  }
-
-  filter {
-    name = "root-device-type"
-    values = ["ebs"]
-  }
-}
-`
-
 // Testing name_regex parameter
 const testAccCheckAwsAmiDataSourceNameRegexConfig = `
 data "aws_ami" "name_regex_filtered_ami" {

diff --git a/aws/data_source_aws_autoscaling_groups_test.go b/aws/data_source_aws_autoscaling_groups_test.go
@@ -84,11 +84,7 @@ func testAccCheckAwsAutoscalingGroupsConfig(rInt1, rInt2, rInt3 int) string {
 	return fmt.Sprintf(`
 data "aws_ami" "test_ami" {
   most_recent = true
-
-  filter {
-    name   = "owner-alias"
-    values = ["amazon"]
-  }
+  owners      = ["amazon"]
 
   filter {
     name   = "name"
@@ -162,11 +158,7 @@ func testAccCheckAwsAutoscalingGroupsConfigWithDataSource(rInt1, rInt2, rInt3 in
 	return fmt.Sprintf(`
 data "aws_ami" "test_ami" {
   most_recent = true
-
-  filter {
-    name   = "owner-alias"
-    values = ["amazon"]
-  }
+  owners      = ["amazon"]
 
   filter {
     name   = "name"

diff --git a/aws/data_source_aws_instance_test.go b/aws/data_source_aws_instance_test.go
@@ -648,11 +648,7 @@ func testAccInstanceDataSourceConfig_getPasswordData(val bool, rInt int) string
 	# Find latest Microsoft Windows Server 2016 Core image (Amazon deletes old ones)
 	data "aws_ami" "win2016core" {
 		most_recent = true
-
-		filter {
-			name = "owner-alias"
-			values = ["amazon"]
-		}
+		owners      = ["amazon"]
 
 		filter {
 			name = "name"