Skip to content

Commit

Permalink
Merge pull request #5576 from terraform-providers/s-aws_ami-require-o…
Browse files Browse the repository at this point in the history 
…wners

data-source/aws_ami and data-source/aws_ami_ids: Require owners argument
  • Loading branch information
@bflad
bflad authored Feb 22, 2019
2 parents 98f11ae + 493e23f commit 39d6664
Show file tree
Hide file tree
Showing 16 changed files with 108 additions and 455 deletions.
57 changes: 13 additions & 44 deletions aws/data_source_aws_ami.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
61 changes: 14 additions & 47 deletions aws/data_source_aws_ami_ids.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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()
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 @@ -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"]
Expand Down Expand Up @@ -113,12 +97,3 @@ data "aws_ami_ids" "test" {
}
`, sort_ascending)
}

const testAccDataSourceAwsAmiIdsConfig_empty = `
data "aws_ami_ids" "empty" {
filter {
name = "name"
values = []
}
}
`
82 changes: 6 additions & 76 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.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) },
Expand All @@ -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 {
Expand All @@ -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*"]
Expand All @@ -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*"]
Expand All @@ -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*"]
Expand All @@ -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" {
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 @@ -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"
Expand Down Expand Up @@ -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"
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 39d6664

Please sign in to comment.