Skip to content

Commit

Permalink
data-source/aws_ami and data-source/aws_ami_ids: Require owners argument
Browse files Browse the repository at this point in the history 
* 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"

make testacc TEST=./aws TESTARGS='-run=TestAcc\(AWSAmiDataSource\|DataSourceAwsAmiIds\)_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAcc\(AWSAmiDataSource\|DataSourceAwsAmiIds\)_ -timeout 120m
=== RUN   TestAccDataSourceAwsAmiIds_basic
--- PASS: TestAccDataSourceAwsAmiIds_basic (11.55s)
=== RUN   TestAccDataSourceAwsAmiIds_sorted
--- PASS: TestAccDataSourceAwsAmiIds_sorted (251.95s)
=== RUN   TestAccAWSAmiDataSource_natInstance
--- PASS: TestAccAWSAmiDataSource_natInstance (11.88s)
=== RUN   TestAccAWSAmiDataSource_windowsInstance
--- PASS: TestAccAWSAmiDataSource_windowsInstance (22.03s)
=== RUN   TestAccAWSAmiDataSource_instanceStore
--- PASS: TestAccAWSAmiDataSource_instanceStore (9.35s)
=== RUN   TestAccAWSAmiDataSource_localNameFilter
--- PASS: TestAccAWSAmiDataSource_localNameFilter (16.45s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	323.842s
  • Loading branch information
@bflad
bflad committed Aug 22, 2018
1 parent ae3096d commit 1b9cd62
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 437 deletions.
58 changes: 13 additions & 45 deletions aws/data_source_aws_ami.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"log"
"regexp"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform/helper/hashcode"
"github.com/hashicorp/terraform/helper/schema"
Expand Down Expand Up @@ -39,9 +38,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": {
Expand Down Expand Up @@ -180,49 +182,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)
Expand All @@ -232,7 +200,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
Expand Down
59 changes: 13 additions & 46 deletions aws/data_source_aws_ami_ids.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"log"
"regexp"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform/helper/hashcode"
"github.com/hashicorp/terraform/helper/schema"
Expand All @@ -32,9 +31,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,
Expand All @@ -48,50 +50,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")

if executableUsersOk == false && filtersOk == false && nameRegexOk == false && ownersOk == false {
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)
Expand All @@ -103,7 +70,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
Expand Down
25 changes: 0 additions & 25 deletions aws/data_source_aws_ami_ids_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,6 @@ func TestAccDataSourceAwsAmiIds_sorted(t *testing.T) {
})
}

func TestAccDataSourceAwsAmiIds_empty(t *testing.T) {
resource.Test(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"]
Expand Down Expand Up @@ -117,12 +101,3 @@ data "aws_ami_ids" "test" {
}
`, uuid)
}

const testAccDataSourceAwsAmiIdsConfig_empty = `
data "aws_ami_ids" "empty" {
filter {
name = "name"
values = []
}
}
`
86 changes: 6 additions & 80 deletions aws/data_source_aws_ami_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,37 +127,6 @@ func TestAccAWSAmiDataSource_instanceStore(t *testing.T) {
})
}

func TestAccAWSAmiDataSource_owners(t *testing.T) {
resource.Test(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.Test(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.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -174,12 +143,7 @@ func TestAccAWSAmiDataSource_localNameFilter(t *testing.T) {
})
}

func testAccCheckAwsAmiDataSourceDestroy(s *terraform.State) error {
return nil
}

func testAccCheckAwsAmiDataSourceID(n string) resource.TestCheckFunc {
// Wait for IAM role
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
Expand All @@ -200,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*"]
Expand All @@ -227,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*"]
Expand All @@ -254,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*"]
Expand All @@ -273,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" {
Expand Down
12 changes: 2 additions & 10 deletions aws/data_source_aws_autoscaling_groups_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,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"
Expand Down Expand Up @@ -161,11 +157,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"
Expand Down
6 changes: 1 addition & 5 deletions aws/data_source_aws_instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading

0 comments on commit 1b9cd62

Please sign in to comment.