awsproviderlint: New Check: Prefer aws_ami data source instead of hardcoded Amazon Machine Image identifiers

[breathingdust]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

To ensure test configurations are region and partition agnostic, any hardcoded AMI identifiers (ami-[a-f0-9]{8,17}) should be replaced with the aws_ami data source.

This check was added in #14031 and is AWSAT002. We'll keep this issue open while fixes are made that are identified by the check.

Flagged Code

In test configuration:

ami-123456

Passing Code

In test configuration:

data "aws_ami" "example" {
  # ... configuration ...
}

To Do

• data_source_aws_instance_test.go (Remove hardcoded AMI IDs from data_source_aws_instance_test #14145)
• data_source_aws_launch_configuration_test.go (Remove hardcoded AMI IDs from launch_config data source #14147)
• resource_aws_autoscaling_attachment_test.go (Removed hardcoded AMI IDs from AutoscalingAttachment #14148)
• resource_aws_autoscaling_group_test.go (r/autoscaling group: Remove hardcoded AZs and ami IDs #14222)
• resource_aws_autoscaling_lifecycle_hook_test.go (Remove hardcoded AMI and AZ from autoscaling lifecycle hook tests #14321)
• resource_aws_autoscaling_notification_test.go (Remove hardcoded AMIs and AZs  #14323)
• resource_aws_autoscaling_schedule_test.go (Remove hardcoded AMIs and AZs from autoscaling schedule #14325)
• resource_aws_eip_association_test.go (Remove hardcoded AMI from eip association #14232)
• resource_aws_elb_attachment_test.go (Removed hardcoded AMIs and AZs #14235)
• resource_aws_instance_test.go (r/aws_instance: Remove hardcoded AMI IDs from acceptance tests #14159, original fix) (tests/provider: Fix, Enable AWSAT002 #15143, second fix)
• resource_aws_launch_template_test.go (Ignore hardcoded AMI because not actually used #14233)
• resource_aws_network_interface_attachment_test.go (Remove hardcoded AMI and AZ #14234)
• resource_aws_network_interface_test.go (Remove hardcoded AMIs and AZs in network interface test #14330)
• resource_aws_route_test.go (r/aws_route_test: Remove hardcoded AMI ID, AZ, instance type #14174)
• resource_aws_spot_fleet_request_test.go (Remove hardcoded AMI IDs from spot fleet request #14083)
• resource_aws_spot_instance_request_test.go (Remove hardcoded AMIs in spot instance request tests #14082)
• validators_test.go (tests/provider: Lint AWSAT002 ignore (validators_test) #15142)

[ewbankkit]

There are some configuration generator functions available for common AMIs:

https://github.com/terraform-providers/terraform-provider-aws/blob/dcb00966d9ec8383883ad62750cb2455fedd96f9/aws/resource_aws_instance_test.go#L4379-L4440

[bflad]

@YakDriver this looks really close to complete, do you think we could get this turned on this week?

$ awsproviderlint -AWSAT002 ./aws
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_instance_test.go:3836:46: AWSAT002: AMI IDs should not be hardcoded
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_instance_test.go:4308:53: AWSAT002: AMI IDs should not be hardcoded
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/validators_test.go:1182:14: AWSAT002: AMI IDs should not be hardcoded

[YakDriver]

@bflad validators_test is ready for review #15142. 2 of the 3 instance tests are easy but the other is an AMI in us-west-2 with RootDeviceName: /dev/sda1; but actual root: /dev/sda (ami-ef5b69df). I'll need more investigation into how/why such an AMI exists.

[bflad]

@YakDriver I feel like there may have been one very special instance test that was setup to cover a very specific regression due to the block setup of a specific AMI, which might be that one -- if I'm remembering that correctly (or git blame can verify), then that one test can be setup to run just in us-west-2 and ignored in the linter since trying to find other similar AMIs is likely more work than benefit. 👍

[YakDriver]

@bflad The exciting #15143 PR fixes:

1. ignore the specific regression (TestAccAWSInstance_rootBlockDeviceMismatch),
2. reworked the other test for either partition (TestAccAWSInstance_multipleRegions),
3. removed testAccPartitionPreCheck() since this is the only place it was used (deadcode lint), and
4. enabled AWSAT002

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!