fix(ec2): render iamInstanceProfile on instances, fix IAM profile association state and checks

crates/fakecloud-aws/src/arn.rs

@@ -59,9 +59,43 @@ impl Arn {
     }
 }
 
+/// An AWS unique id derived from a resource ARN: the 4-char prefix AWS uses
+/// for that resource family (`AIPA` for an instance profile, `AIDA` for a
+/// user, `AROA` for a role) followed by 17 uppercase base32 characters, the
+/// 21-character shape AWS returns.
+///
+/// Deriving it from the ARN rather than minting it randomly lets two services
+/// that both report the same resource's id agree on it without sharing state:
+/// IAM reports the instance profile's `InstanceProfileId`, and EC2 reports the
+/// same value on every instance the profile is attached to.
+///
+/// The trade-off is that the id is a pure function of the ARN, so deleting a
+/// resource and creating it again under the same name returns the same id
+/// where AWS would mint a fresh one. Id inequality is therefore not a reliable
+/// "different resource" signal here.
+pub fn unique_id_for(prefix: &str, arn: &str) -> String {
+    // FNV-1a over the ARN, so the id is stable across restarts and processes
+    // (a random value, or one from a seeded hasher, is not).
+    let mut hash: u64 = 0xcbf2_9ce4_8422_2325;
+    for b in arn.as_bytes() {
+        hash ^= u64::from(*b);
+        hash = hash.wrapping_mul(0x0000_0100_0000_01b3);
+    }
+    // RFC 4648 base32: the uppercase letters plus 2-7, which is the character
+    // set AWS's unique ids use.
+    const ALPHABET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ234567";
+    let mut suffix = String::with_capacity(17);
+    for i in 0..17 {
+        // Stir between characters so every one varies with the whole hash.
+        let shifted = hash.rotate_left(i * 5);
+        suffix.push(ALPHABET[(shifted % ALPHABET.len() as u64) as usize] as char);
+    }
+    format!("{prefix}{suffix}")
+}
+
 /// Map an AWS region name to its partition. Mirrors the AWS SDK's
-/// region-to-partition lookup so synthesized ARNs in cn/gov-cloud
-/// regions emit the correct partition prefix.
+/// region-to-partition lookup so synthesized ARNs in cn/gov-cloud and the
+/// isolated regions emit the correct partition prefix.
 pub fn partition_for(region: &str) -> &'static str {
     if region.starts_with("cn-") {
         "aws-cn"
@@ -71,6 +105,10 @@ pub fn partition_for(region: &str) -> &'static str {
         "aws-iso"
     } else if region.starts_with("us-isob-") {
         "aws-iso-b"
+    } else if region.starts_with("us-isof-") {
+        "aws-iso-f"
+    } else if region.starts_with("eu-isoe-") {
+        "aws-iso-e"
     } else {
         "aws"
     }
@@ -140,6 +178,31 @@ mod tests {
         assert_eq!(arn.to_string(), "arn:aws-cn:sqs:cn-north-1:123:q");
     }
 
+    #[test]
+    fn unique_id_is_stable_and_aws_shaped() {
+        let arn = "arn:aws:iam::123456789012:instance-profile/web";
+        let id = unique_id_for("AIPA", arn);
+        assert_eq!(id.len(), 21, "{id}");
+        assert!(id.starts_with("AIPA"), "{id}");
+        assert!(
+            id[4..]
+                .chars()
+                .all(|c| c.is_ascii_uppercase() || ('2'..='7').contains(&c)),
+            "{id}"
+        );
+        // Same ARN -> same id; a different ARN -> a different one.
+        assert_eq!(id, unique_id_for("AIPA", arn));
+        assert_ne!(
+            id,
+            unique_id_for("AIPA", "arn:aws:iam::123456789012:instance-profile/other")
+        );
+        // The partition is part of the ARN, so it is part of the id.
+        assert_ne!(
+            id,
+            unique_id_for("AIPA", "arn:aws-cn:iam::123456789012:instance-profile/web")
+        );
+    }
+
     #[test]
     fn partition_for_region() {
         assert_eq!(partition_for("us-east-1"), "aws");
@@ -149,5 +212,7 @@ mod tests {
         assert_eq!(partition_for("us-gov-west-1"), "aws-us-gov");
         assert_eq!(partition_for("us-iso-east-1"), "aws-iso");
         assert_eq!(partition_for("us-isob-east-1"), "aws-iso-b");
+        assert_eq!(partition_for("us-isof-south-1"), "aws-iso-f");
+        assert_eq!(partition_for("eu-isoe-west-1"), "aws-iso-e");
     }
 }

crates/fakecloud-cloudformation/src/extras.rs

@@ -773,10 +773,10 @@ impl CloudFormationService {
                     .or_insert_with(|| stack_name.clone());
                 full_params
                     .entry("AWS::Partition".to_string())
-                    .or_insert_with(|| "aws".to_string());
+                    .or_insert_with(|| template::partition_for_region(&req.region).to_string());
                 full_params
                     .entry("AWS::URLSuffix".to_string())
-                    .or_insert_with(|| "amazonaws.com".to_string());
+                    .or_insert_with(|| template::url_suffix_for_region(&req.region).to_string());
                 if let Some((sid, _)) = &stack_lookup {
                     full_params
                         .entry("AWS::StackId".to_string())
@@ -1328,10 +1328,10 @@ impl CloudFormationService {
                     .or_insert_with(|| stack_name.clone());
                 cs_params
                     .entry("AWS::Partition".to_string())
-                    .or_insert_with(|| "aws".to_string());
+                    .or_insert_with(|| template::partition_for_region(&req.region).to_string());
                 cs_params
                     .entry("AWS::URLSuffix".to_string())
-                    .or_insert_with(|| "amazonaws.com".to_string());
+                    .or_insert_with(|| template::url_suffix_for_region(&req.region).to_string());
 
                 // An empty body (a CREATE change set with no resources, or a
                 // probe with a placeholder template) parses to an empty

crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs

@@ -514,25 +514,19 @@ impl ResourceProvisioner {
                 }
             });
 
-        // IamInstanceProfile can be a string (profile name / ARN) or an object
-        // with `Arn` / `Name` (the CFN property shape).
-        let iam_profile = props.get("IamInstanceProfile");
-        let (iam_instance_profile_arn, iam_instance_profile_name) = match iam_profile {
-            Some(serde_json::Value::String(s)) => {
-                // A bare string is the profile name (or an ARN); classify by
-                // prefix so both round-trip.
-                if s.starts_with("arn:") {
-                    (Some(s.clone()), None)
-                } else {
-                    (None, Some(s.clone()))
-                }
-            }
-            Some(serde_json::Value::Object(o)) => (
-                o.get("Arn").and_then(|v| v.as_str()).map(String::from),
-                o.get("Name").and_then(|v| v.as_str()).map(String::from),
-            ),
-            _ => (None, None),
-        };
+        let (iam_instance_profile_arn, iam_instance_profile_name) = cfn_iam_instance_profile(props);
+        // Validate here, the way Associate/Replace do, so a template that
+        // resolves IamInstanceProfile to a role ARN (a common Fn::GetAtt
+        // mistake) fails the create instead of storing a value the next
+        // UpdateStack cannot re-submit.
+        validate_cfn_iam_instance_profile(&iam_instance_profile_arn, &iam_instance_profile_name)?;
+        // A `Ref` to an AWS::IAM::InstanceProfile is the profile name; resolve
+        // it to the ARN IAM stored so a non-default Path survives.
+        let iam_instance_profile_arn = iam_instance_profile_arn.or_else(|| {
+            iam_instance_profile_name
+                .as_deref()
+                .and_then(|n| self.resolve_instance_profile_arn(n))
+        });
 
         let spec = fakecloud_ec2::cfn_provision::CfnInstanceSpec {
             image_id: prop_str(props, "ImageId").map(String::from),
@@ -673,11 +667,95 @@ impl ResourceProvisioner {
             }
         }
 
+        self.sync_ec2_instance_profile(props, &instance_id)?;
+
         // The identity attributes (private ip, AZ, public ip) do not change on
         // an in-place modify; carry the ones captured at create time forward.
         Ok(ProvisionResult::new(instance_id).merge_attributes(existing.attributes.clone()))
     }
 
+    /// The ARN of a profile this stack's IAM state knows by name. `Ref` on an
+    /// `AWS::IAM::InstanceProfile` resolves to the profile *name*, and only IAM
+    /// knows the Path that name's ARN carries, so resolving here is what keeps
+    /// a pathed profile's ARN (and the id derived from it) the same on the
+    /// instance as in `GetInstanceProfile`. `None` for a profile IAM does not
+    /// hold, which stays a name-addressed association as before.
+    fn resolve_instance_profile_arn(&self, name: &str) -> Option<String> {
+        let accounts = self.iam_state.read();
+        let state = accounts.get(&self.account_id)?;
+        state
+            .instance_profiles
+            .get(name)
+            .map(|profile| profile.arn.clone())
+    }
+
+    /// Bring the instance's IAM instance-profile association in line with the
+    /// template. AWS updates `IamInstanceProfile` in place ("some interruption",
+    /// no replacement), so this replaces an existing association, associates
+    /// when the instance has none, and disassociates when the template dropped
+    /// the property.
+    fn sync_ec2_instance_profile(&self, props: &Value, instance_id: &str) -> Result<(), String> {
+        let (arn, name) = cfn_iam_instance_profile(props);
+        validate_cfn_iam_instance_profile(&arn, &name)?;
+        // Prefer an ARN: the template's own, else the one IAM stored for that
+        // name (which carries the Path). A name IAM does not hold stays a
+        // name-addressed association, as before.
+        // Whether the template addressed the profile by name. Kept because the
+        // "unchanged" test below has to compare on the name in that case: the
+        // stored association may hold a path-less ARN synthesized before IAM
+        // had the profile, which names the same profile the template does.
+        let by_name = arn.is_none() && name.is_some();
+        let wanted = arn
+            .or_else(|| {
+                name.as_deref()
+                    .and_then(|n| self.resolve_instance_profile_arn(n))
+            })
+            .map(|a| ("IamInstanceProfile.Arn", a))
+            .or_else(|| name.map(|n| ("IamInstanceProfile.Name", n)));
+
+        let mut lookup = HashMap::new();
+        lookup.insert("Filter.1.Name".to_string(), "instance-id".to_string());
+        lookup.insert("Filter.1.Value.1".to_string(), instance_id.to_string());
+        let existing = self.ec2_dispatch("DescribeIamInstanceProfileAssociations", lookup)?;
+        let existing_id = xml_elem(&existing, "associationId");
+        let existing_arn = xml_elem(&existing, "arn");
+
+        match (wanted, existing_id) {
+            (None, None) => Ok(()),
+            (None, Some(id)) => {
+                let mut params = HashMap::new();
+                params.insert("AssociationId".to_string(), id);
+                self.ec2_dispatch("DisassociateIamInstanceProfile", params)?;
+                Ok(())
+            }
+            (Some((key, value)), Some(id)) => {
+                // An in-place update runs for any changed property, so only
+                // replace when the profile itself changed. Replacing anyway
+                // would retire the association id on an unrelated edit (an
+                // InstanceType bump, a new tag), which AWS leaves alone.
+                let profile_name = |arn: &str| arn.rsplit('/').next().unwrap_or(arn).to_string();
+                let unchanged = existing_arn.as_deref().is_some_and(|arn| {
+                    arn == value || (by_name && profile_name(arn) == profile_name(&value))
+                });
+                if unchanged {
+                    return Ok(());
+                }
+                let mut params = HashMap::new();
+                params.insert("AssociationId".to_string(), id);
+                params.insert(key.to_string(), value);
+                self.ec2_dispatch("ReplaceIamInstanceProfileAssociation", params)?;
+                Ok(())
+            }
+            (Some((key, value)), None) => {
+                let mut params = HashMap::new();
+                params.insert("InstanceId".to_string(), instance_id.to_string());
+                params.insert(key.to_string(), value);
+                self.ec2_dispatch("AssociateIamInstanceProfile", params)?;
+                Ok(())
+            }
+        }
+    }
+
     /// Delete an EC2 resource by its physical id, routing through the real
     /// handler so dependent default resources are cleaned up correctly.
     pub(super) fn delete_ec2_resource(
@@ -714,3 +792,44 @@ impl ResourceProvisioner {
         }
     }
 }
+
+/// `IamInstanceProfile` as CloudFormation writes it: either a bare string
+/// (profile name, or an ARN) or an object with `Arn` / `Name`. Returns
+/// `(arn, name)`.
+fn cfn_iam_instance_profile(props: &Value) -> (Option<String>, Option<String>) {
+    match props.get("IamInstanceProfile") {
+        Some(Value::String(s)) => {
+            // A bare string is the profile name (or an ARN); classify by prefix
+            // so both round-trip.
+            if s.starts_with("arn:") {
+                (Some(s.clone()), None)
+            } else {
+                (None, Some(s.clone()))
+            }
+        }
+        Some(Value::Object(o)) => (
+            o.get("Arn").and_then(|v| v.as_str()).map(String::from),
+            o.get("Name").and_then(|v| v.as_str()).map(String::from),
+        ),
+        _ => (None, None),
+    }
+}
+
+/// Reject an `IamInstanceProfile` the EC2 handlers would reject, so a bad
+/// template value fails the stack operation rather than being stored.
+fn validate_cfn_iam_instance_profile(
+    arn: &Option<String>,
+    name: &Option<String>,
+) -> Result<(), String> {
+    if let Some(arn) = arn {
+        if !fakecloud_ec2::service_helpers::is_instance_profile_arn(arn) {
+            return Err(format!("The IAM instance profile ARN '{arn}' is malformed"));
+        }
+    }
+    if let Some(name) = name {
+        if !fakecloud_ec2::service_helpers::is_instance_profile_name(name) {
+            return Err(format!("Invalid IAM Instance Profile name: {name}"));
+        }
+    }
+    Ok(())
+}