fix: scope IAM instance profile propagation check to IAM only

[cchristous]

Description

Fixes a v1.8.1 regression (introduced in #639) in the temporary IAM instance profile flow.

#639 replaced the post-attach time.Sleep with a RunInstances dry-run that waits for the newly-created instance profile to propagate to EC2. However, the dry-run hardcoded InstanceType: t3.nano and passed no networking context, so it could fail for reasons unrelated to IAM propagation:

• non-x86_64 (e.g. arm64) source AMIs → architecture mismatch, since t3.nano is x86_64.
• accounts/regions without a default VPC → VPCIdNotSpecified (with no subnet, EC2 falls back to the default VPC and resolves a security group by name).

ShouldRetry only matched Invalid IAM Instance Profile, so both of these were treated as terminal and the build failed at the propagation check — even though the real launch (which uses the configured instance type and the resolved subnet/security groups) would have succeeded. Separately, roleIsAttached was only set after the dry-run passed, so a failed dry-run left the temporary role attached to the instance profile, producing DeleteConflict on DeleteRole/DeleteInstanceProfile during cleanup.

Change. The propagation check has one job: confirm the instance profile is visible to EC2. This scopes it to exactly that:

• The dry-run now treats any non-Invalid IAM Instance Profile outcome — DryRunOperation or a downstream error such as an architecture or instance-type/AZ mismatch — as "EC2 got past IAM validation, so the profile has propagated." Everything else is deferred to the real launch step, which validates and reports it with accurate context.
• To keep the request well-formed enough to reach the IAM check (rather than being rejected earlier for an invalid type or the default-VPC fallback), it now passes the configured instance type and the resolved subnet. Instance-type resolution moved to a RunConfig.EffectiveInstanceType() helper that falls back to the first spot_instance_types entry when instance_type is unset (spot-only builds), since Prepare guarantees exactly one is set.
• roleIsAttached is set as soon as the role is attached, so Cleanup detaches it even if the propagation check fails.
• On failure, the error message now distinguishes an actual propagation timeout from an unrelated request error, instead of always reporting "timed out waiting for IAM changes to propagate."

This affects the amazon-ebs, amazon-ebsvolume, and amazon-ebssurrogate builders (which share this step). The amazon-instance builder is unaffected — it uses a separate step that never had the dry-run.

Alternatives considered. Making the dry-run fully mirror the real launch (subnet + security groups + exact type) was rejected: security groups add no signal for an IAM-propagation check and introduce a fresh eventual-consistency surface (a just-created temporary security group can transiently return InvalidGroup.NotFound), and pinning a single instance type is stricter than a spot fleet, which launches on any of several types available in the AZ. Scoping the check to IAM alone is simpler and defers every non-IAM error to the real launch rather than treating it as a propagation failure, so it stops failing the builds the old code broke.

Testing. go build ./..., go vet, and the common, builder/ebs, builder/ebsvolume, and builder/ebssurrogate package tests pass. Added unit tests for EffectiveInstanceType() (including the spot fallback) and for the dry-run request construction.

Resolved Issues

Closes #675

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

No changes to access controls, encryption, or logging. The temporary IAM role/instance-profile lifecycle and the policy documents applied are unchanged; the granted permissions are identical. The only security-adjacent effect is a reliability improvement to cleanup: the temporary role is now reliably detached and deleted even when the propagation check fails, avoiding leftover temporary roles/instance profiles.

[hashicorp-cla-app]

All committers have signed the CLA.

[hashicorp-cla-app]

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

_{Have you signed the CLA already but the status is still pending? Recheck it.}

[hariom-hashicorp]

Thanks for the fix, @cchristous! I reviewed and tested it.

• Reproduced the ARM64 issue on v1.8.1
• Verified the changes and unit test coverage
• Confirmed we now use the configured instance type instead of the hardcoded t3.nano
• Confirmed the resolved subnet is passed through, which fixes the no-default-VPC case
• The cleanup issue is also fixed by setting roleIsAttached before the check

One non-blocking question:
The check for a missing subnet_id makes sense as a safeguard. Do you know if there are any cases where subnet_id could actually be missing at this point? My understanding is that StepNetworkInfo should always set it before this step, but I may be missing an edge case.

Other than that, everything looks good.

[cchristous]

Thanks for testing it, @hariom-hashicorp! Your understanding is correct — there's no reachable path today where subnet_id is missing. StepNetworkInfo runs before this step in all three builders (amazon-ebs/ebsvolume/ebssurrogate) and always calls state.Put("subnet_id", …). It can be an empty string (when no subnet_id/subnet_filter is set and a default VPC is used), which the dry-run request handles by simply omitting SubnetId — but the key is always present.

The guard is purely defensive/consistency: the line previously used a bare state.Get("subnet_id").(string), which would panic on a nil interface if the key were ever absent, so I switched it to the same GetOk + clean-error form already used for source_image a few lines above. That way, if this shared step is ever wired into a flow without StepNetworkInfo ahead of it, it fails with an actionable error instead of panicking. Happy to drop it if you'd prefer to keep the step minimal — it's not guarding a currently-reachable case.