fix: match EC2 instance type to source AMI architecture in dry run#649
fix: match EC2 instance type to source AMI architecture in dry run#649gnought wants to merge 1 commit intohashicorp:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts the EC2 “dry-run” instance launch used for IAM instance profile propagation validation so it chooses an instance type that matches the source AMI’s architecture, avoiding architecture-mismatch failures during validation.
Changes:
- Derives the EC2
InstanceTypefor the dry-runRunInstancescall fromsource_image.Architecture. - Adds explicit mappings for
arm64,x86_64_mac, andarm64_mac, with fallback tot3.nano. - Uses the derived instance type in the dry-run
RunInstancesInput.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| instanceType := map[ec2types.ArchitectureValues]ec2types.InstanceType{ | ||
| ec2types.ArchitectureValuesArm64: ec2types.InstanceTypeT4gNano, | ||
| ec2types.ArchitectureValuesX8664Mac: ec2types.InstanceTypeMac1Metal, | ||
| ec2types.ArchitectureValuesArm64Mac: ec2types.InstanceTypeMac2Metal, | ||
| }[sourceImage.Architecture] | ||
| if instanceType == "" { | ||
| instanceType = ec2types.InstanceTypeT3Nano | ||
| } |
There was a problem hiding this comment.
The dry-run now selects mac1.metal/mac2.metal for *_mac AMIs, but the RunInstancesInput here does not include any placement/tenancy/host parameters. Since this step treats any non-DryRunOperation error as “instance profile not visible yet”, missing placement settings can cause false failures unrelated to IAM propagation. Consider either (a) skipping the EC2 dry-run IAM visibility check for mac architectures, or (b) threading the builder’s placement/tenancy/host settings into this step and using them in the dry-run call so validation matches the real launch parameters.
| instanceType := map[ec2types.ArchitectureValues]ec2types.InstanceType{ | ||
| ec2types.ArchitectureValuesArm64: ec2types.InstanceTypeT4gNano, | ||
| ec2types.ArchitectureValuesX8664Mac: ec2types.InstanceTypeMac1Metal, | ||
| ec2types.ArchitectureValuesArm64Mac: ec2types.InstanceTypeMac2Metal, | ||
| }[sourceImage.Architecture] | ||
| if instanceType == "" { | ||
| instanceType = ec2types.InstanceTypeT3Nano | ||
| } |
There was a problem hiding this comment.
This PR introduces architecture-to-instance-type selection logic, but there’s no unit test coverage to ensure the mapping and default fallback behave as expected (e.g., arm64 => t4g.nano, unknown/empty => t3.nano). Adding a focused test for this selection would help prevent regressions and make future architecture additions safer.
Description
During EC2 dry-run, packer currently uses a default
t3.nanoinstance type which does not match the architecture of the source AMI. This causes the dry-run fail.The derivation of instance type should be based on the source architecture. If not matched, use
t3.nanoin default.A excerpt of packer log:
Resolved Issues
If your PR resolves any open issue(s), please indicate them like this so they will be closed when your PR is merged:
Rollback Plan
If a change needs to be reverted, we will roll out an update to the code within 7 days.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
No