From d966246c9aa778e941de6a7b6e987e669024e480 Mon Sep 17 00:00:00 2001 From: Kostiantyn Lysenko Date: Sat, 12 Sep 2026 23:07:57 +0700 Subject: [PATCH 1/8] fix(ec2): render iamInstanceProfile on instances, fix IAM profile association state and checks DescribeInstances never emitted . AssociateIamInstanceProfile, RunInstances --iam-instance-profile and the CloudFormation instance provisioner all recorded an association, but the instance render ignored it, so `describe-instances --query ...IamInstanceProfile.Arn` stayed null forever. RunInstances additionally rendered the instance before recording the association, so the launch response omitted it too. Render the profile from the association map at describe time (same pattern as security-group names and block-device mappings) and pass it into the instance render; build the association before rendering on RunInstances. Rendering ignores the association state so records persisted by earlier versions still show. Adjacent fixes in the same operation family, matching the AWS docs: - Associate and Replace answer `associating` on the wire and store `associated` (Associate used to store `associating` and never advance it; Replace answered `associated`). Same shape Disassociate already used for `disassociating`. - Replace retires the old association and mints a new id, as the AWS doc sample shows. - Associate rejects an unknown instance (InvalidInstanceID.NotFound) and a second profile on an instance that already has one (IncorrectState). Replace and Disassociate reject an unknown association id (InvalidAssociationID.NotFound). All three used to answer success with a fabricated record. - Associate and Replace require IamInstanceProfile (MissingParameter). Both used to store an association with an empty ARN. One constructor builds the association record for all four writers and one helper renders the fragment for both instance and association responses. The conformance smoke tests for Associate, Replace and Disassociate now launch an instance (and associate a profile) first instead of using dummy ids. --- crates/fakecloud-conformance/tests/ec2.rs | 40 +- .../tests/ec2_instance_control_plane.rs | 63 +++ crates/fakecloud-ec2/src/lib.rs | 52 +++ crates/fakecloud-ec2/src/service/instance.rs | 256 ++++++++--- crates/fakecloud-ec2/src/service/rest/mod.rs | 427 +++++++++++++----- crates/fakecloud-ec2/src/service_helpers.rs | 16 + 6 files changed, 654 insertions(+), 200 deletions(-) diff --git a/crates/fakecloud-conformance/tests/ec2.rs b/crates/fakecloud-conformance/tests/ec2.rs index 29ce0b9a1..55101ad5c 100644 --- a/crates/fakecloud-conformance/tests/ec2.rs +++ b/crates/fakecloud-conformance/tests/ec2.rs @@ -10676,8 +10676,15 @@ async fn ec2_associate_enclave_certificate_iam_role() { async fn ec2_associate_iam_instance_profile() { let s = TestServer::start().await; let c = s.ec2_client().await; + // The instance must exist (InvalidInstanceID.NotFound otherwise). + let id = run_one(&c).await; c.associate_iam_instance_profile() - .instance_id("x") + .instance_id(id) + .iam_instance_profile( + aws_sdk_ec2::types::IamInstanceProfileSpecification::builder() + .name("web-profile") + .build(), + ) .send() .await .unwrap(); @@ -11898,13 +11905,33 @@ async fn ec2_disassociate_enclave_certificate_iam_role() { .unwrap(); } +async fn make_iam_profile_association(c: &aws_sdk_ec2::Client) -> String { + let id = run_one(c).await; + c.associate_iam_instance_profile() + .instance_id(id) + .iam_instance_profile( + aws_sdk_ec2::types::IamInstanceProfileSpecification::builder() + .name("web-profile") + .build(), + ) + .send() + .await + .unwrap() + .iam_instance_profile_association() + .and_then(|a| a.association_id()) + .unwrap() + .to_string() +} + #[test_action("ec2", "DisassociateIamInstanceProfile", checksum = "865568f3")] #[tokio::test] async fn ec2_disassociate_iam_instance_profile() { let s = TestServer::start().await; let c = s.ec2_client().await; + // The association must exist (InvalidAssociationID.NotFound otherwise). + let assoc_id = make_iam_profile_association(&c).await; c.disassociate_iam_instance_profile() - .association_id("x") + .association_id(assoc_id) .send() .await .unwrap(); @@ -12610,8 +12637,15 @@ async fn ec2_purchase_scheduled_instances() { async fn ec2_replace_iam_instance_profile_association() { let s = TestServer::start().await; let c = s.ec2_client().await; + // The association must exist (InvalidAssociationID.NotFound otherwise). + let assoc_id = make_iam_profile_association(&c).await; c.replace_iam_instance_profile_association() - .association_id("x") + .association_id(assoc_id) + .iam_instance_profile( + aws_sdk_ec2::types::IamInstanceProfileSpecification::builder() + .name("admin-profile") + .build(), + ) .send() .await .unwrap(); diff --git a/crates/fakecloud-e2e/tests/ec2_instance_control_plane.rs b/crates/fakecloud-e2e/tests/ec2_instance_control_plane.rs index 945c99734..eef96577d 100644 --- a/crates/fakecloud-e2e/tests/ec2_instance_control_plane.rs +++ b/crates/fakecloud-e2e/tests/ec2_instance_control_plane.rs @@ -706,3 +706,66 @@ async fn describe_instances_reports_ami_architecture() { .flat_map(|r| r.instances()) .any(|i| i.instance_id() == Some(id.as_str()))); } + +#[tokio::test] +async fn describe_instances_reports_iam_instance_profile_round_trip() { + // Attach a profile, read it back on the instance, detach, read again. + // Covers the SDK-visible shape: DescribeInstances rendered no + // at all, so `IamInstanceProfile.Arn` stayed null. + let s = TestServer::start().await; + let c = s.ec2_client().await; + let id = run(&c, 1, 1).await.remove(0); + + let assoc = c + .associate_iam_instance_profile() + .instance_id(&id) + .iam_instance_profile( + aws_sdk_ec2::types::IamInstanceProfileSpecification::builder() + .name("web-profile") + .build(), + ) + .send() + .await + .unwrap(); + let assoc_id = assoc + .iam_instance_profile_association() + .and_then(|a| a.association_id()) + .expect("association id") + .to_string(); + + let desc = c + .describe_instances() + .instance_ids(&id) + .send() + .await + .unwrap(); + let inst = &desc.reservations()[0].instances()[0]; + let profile = inst.iam_instance_profile().expect("profile on instance"); + assert_eq!( + profile.arn(), + Some("arn:aws:iam::123456789012:instance-profile/web-profile"), + "{profile:?}" + ); + assert!( + profile.id().is_some_and(|i| i.starts_with("AIPA")), + "{profile:?}" + ); + + c.disassociate_iam_instance_profile() + .association_id(&assoc_id) + .send() + .await + .unwrap(); + let desc = c + .describe_instances() + .instance_ids(&id) + .send() + .await + .unwrap(); + assert!( + desc.reservations()[0].instances()[0] + .iam_instance_profile() + .is_none(), + "profile must be gone after disassociate" + ); +} diff --git a/crates/fakecloud-ec2/src/lib.rs b/crates/fakecloud-ec2/src/lib.rs index 6a71c38c4..8752ef1f2 100644 --- a/crates/fakecloud-ec2/src/lib.rs +++ b/crates/fakecloud-ec2/src/lib.rs @@ -32,6 +32,58 @@ pub(crate) mod test_support { } } + /// Insert a minimal running instance into the default test account so + /// handlers that look the instance up have something to find. + pub(crate) fn seed_instance(svc: &crate::service::Ec2Service, id: &str) { + let mut accounts = svc.state.write(); + let state = accounts.get_or_create("000000000000"); + let inst = crate::state::Instance { + instance_id: id.into(), + image_id: "ami-1".into(), + instance_type: "t3.micro".into(), + state_code: 16, + state_name: "running".into(), + private_ip: "10.0.0.5".into(), + public_ip: None, + subnet_id: Some("subnet-1".into()), + vpc_id: Some("vpc-1".into()), + key_name: None, + security_group_ids: vec![], + reservation_id: "r-1".into(), + ami_launch_index: 0, + monitoring: false, + az: "us-east-1a".into(), + launch_time: "2024-01-01T00:00:00.000Z".into(), + container_id: None, + disable_api_termination: false, + disable_api_stop: false, + source_dest_check: true, + ebs_optimized: false, + instance_initiated_shutdown_behavior: "stop".into(), + user_data: None, + metadata_options: Default::default(), + cpu_options: None, + bandwidth_weighting: None, + maintenance_options: Default::default(), + placement_tenancy: None, + placement_affinity: None, + placement_group_name: None, + private_dns_hostname_type: None, + enable_resource_name_dns_a_record: false, + enable_resource_name_dns_aaaa_record: false, + }; + state.instances.insert(id.to_string(), inst); + } + + /// The `` of an IAM instance-profile association response. + pub(crate) fn assoc_id_of(xml: &str) -> String { + xml.split("") + .nth(1) + .and_then(|s| s.split("").next()) + .expect("associationId in response") + .to_string() + } + /// Build a minimal query-protocol [`AwsRequest`] for handler unit tests. pub(crate) fn ec2_request(action: &str, query: &[(&str, &str)]) -> AwsRequest { AwsRequest { diff --git a/crates/fakecloud-ec2/src/service/instance.rs b/crates/fakecloud-ec2/src/service/instance.rs index 2c20f9943..64129b7a8 100644 --- a/crates/fakecloud-ec2/src/service/instance.rs +++ b/crates/fakecloud-ec2/src/service/instance.rs @@ -12,7 +12,7 @@ use crate::service_helpers::{ gen_id, indexed_list, instance_limit_exceeded, invalid_parameter_value, missing_parameter, parse_filters, require, require_struct, validate_enum, Filter, }; -use crate::state::{Ec2State, Instance, Tag}; +use crate::state::{Ec2State, IamInstanceProfileAssociation, Instance, Tag}; const LAUNCH_TIME: &str = "2024-01-01T00:00:00.000Z"; @@ -78,6 +78,11 @@ fn platform_for(state: &Ec2State, image_id: &str) -> String { super::image::platform_details_label(raw) } +/// Render one `` of `instancesSet`. `iam_assoc` is the instance's +/// IAM instance-profile association, looked up by the caller from +/// `state.iam_instance_profile_associations` (the instance record itself does +/// not carry the profile, mirroring how `sg_names` and block-device mappings +/// are resolved at describe time). fn instance_xml( i: &Instance, tags: &[Tag], @@ -85,7 +90,14 @@ fn instance_xml( sg_names: &HashMap, architecture: &str, platform_details: &str, + iam_assoc: Option<&IamInstanceProfileAssociation>, ) -> String { + // Rendered whenever an association exists, regardless of its `state`, so + // records persisted by older versions (which never left `associating`) + // still reflect on the instance. + let iam_profile = iam_assoc + .map(super::rest::iam_instance_profile_xml) + .unwrap_or_default(); let groups: Vec = i .security_group_ids .iter() @@ -168,7 +180,7 @@ fn instance_xml( .map(|a| ec2_elem("affinity", a)) .unwrap_or_default(); format!( - "{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}", + "{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}", ec2_elem("instanceId", &i.instance_id), ec2_elem("imageId", &i.image_id), state_xml("instanceState", i.state_code, &i.state_name), @@ -219,6 +231,7 @@ fn instance_xml( ec2_list("groupSet", &groups), ec2_elem("ownerId", owner) ), + iam_profile, metadata_options, cpu_options, super::tags::tag_set_xml(tags), @@ -352,17 +365,7 @@ pub(crate) async fn run_instances( .map(|v| v == "true") .unwrap_or(false); let metadata_options = parse_metadata_options(&req.query_params); - // `IamInstanceProfile.Arn` / `.Name` (either half is accepted; synthesize - // the ARN from the name when only the name is given). - let iam_profile_arn = req - .query_params - .get("IamInstanceProfile.Arn") - .cloned() - .or_else(|| { - req.query_params - .get("IamInstanceProfile.Name") - .map(|n| format!("arn:aws:iam::{}:instance-profile/{n}", req.account_id)) - }); + let iam_profile_arn = super::rest::iam_profile_arn(req); let az = format!( "{}a", if req.region.is_empty() { @@ -552,6 +555,14 @@ pub(crate) async fn run_instances( let tags = state.tags_for(id).to_vec(); let architecture = arch_for(state, &inst.image_id); let platform_details = platform_for(state, &inst.image_id); + // Build the IAM instance-profile association (when the request + // supplies `IamInstanceProfile`) before rendering, so the launch + // response carries as on AWS. The record + // matches a direct AssociateIamInstanceProfile so + // DescribeIamInstanceProfileAssociations reflects it too. + let iam_assoc = iam_profile_arn.clone().map(|profile_arn| { + super::rest::new_iam_profile_association(id.clone(), profile_arn) + }); rendered.push(instance_xml( &inst, &tags, @@ -559,21 +570,10 @@ pub(crate) async fn run_instances( &sg_names, &architecture, &platform_details, + iam_assoc.as_ref(), )); state.instances.insert(id.clone(), inst); - - // Record an IAM instance-profile association when the request - // supplies `IamInstanceProfile`, matching a direct - // AssociateIamInstanceProfile so - // DescribeIamInstanceProfileAssociations reflects it. - if let Some(profile_arn) = iam_profile_arn.clone() { - let assoc = crate::state::IamInstanceProfileAssociation { - association_id: gen_id("iip-assoc"), - instance_id: id.clone(), - iam_instance_profile_arn: profile_arn, - iam_instance_profile_id: gen_id("AIPA"), - state: "associated".to_string(), - }; + if let Some(assoc) = iam_assoc { state .iam_instance_profile_associations .insert(assoc.association_id.clone(), assoc); @@ -927,13 +927,7 @@ pub(crate) fn cfn_create_instance( let name = spec.iam_instance_profile_name.clone().unwrap_or_default(); format!("arn:aws:iam::{account_id}:instance-profile/{name}") }); - let assoc = crate::state::IamInstanceProfileAssociation { - association_id: gen_id("iip-assoc"), - instance_id: id.clone(), - iam_instance_profile_arn: arn, - iam_instance_profile_id: gen_id("AIPA"), - state: "associated".to_string(), - }; + let assoc = super::rest::new_iam_profile_association(id.clone(), arn); state .iam_instance_profile_associations .insert(assoc.association_id.clone(), assoc); @@ -1438,6 +1432,16 @@ pub(crate) fn describe_instances( } } let no_bdm: Vec<&crate::state::VolumeAttachment> = Vec::new(); + // Same idea for the IAM instance profile: it lives in the association map, + // not on the instance, so resolve it once per request for the render. + // Associate rejects a second profile per instance, so at most one record + // matches; a snapshot from before that check could hold two, in which case + // the last in map order wins. + let profile_by_instance: HashMap<&str, &IamInstanceProfileAssociation> = state + .iam_instance_profile_associations + .values() + .map(|a| (a.instance_id.as_str(), a)) + .collect(); // Flatten matching instances into a stable order (by reservation, then id), // then paginate over the flat instance list — AWS counts instances, not @@ -1481,6 +1485,7 @@ pub(crate) fn describe_instances( &sg_names, &arch_for(state, &i.image_id), &platform_for(state, &i.image_id), + profile_by_instance.get(i.instance_id.as_str()).copied(), )); } let reservations: Vec = order @@ -2468,6 +2473,7 @@ mod tests { #[cfg(test)] mod modify_tests { use super::*; + use crate::test_support::{assoc_id_of, seed_instance}; fn req(action: &str, query: &[(&str, &str)]) -> AwsRequest { AwsRequest { @@ -2577,47 +2583,6 @@ mod modify_tests { assert!(!xml.contains("b "), "raw < must not appear: {xml}"); } - fn seed_instance(svc: &Ec2Service, id: &str) { - let mut accounts = svc.state.write(); - let state = accounts.get_or_create("000000000000"); - let inst = Instance { - instance_id: id.into(), - image_id: "ami-1".into(), - instance_type: "t3.micro".into(), - state_code: 16, - state_name: "running".into(), - private_ip: "10.0.0.5".into(), - public_ip: None, - subnet_id: Some("subnet-1".into()), - vpc_id: Some("vpc-1".into()), - key_name: None, - security_group_ids: vec![], - reservation_id: "r-1".into(), - ami_launch_index: 0, - monitoring: false, - az: "us-east-1a".into(), - launch_time: "2024-01-01T00:00:00.000Z".into(), - container_id: None, - disable_api_termination: false, - disable_api_stop: false, - source_dest_check: true, - ebs_optimized: false, - instance_initiated_shutdown_behavior: "stop".into(), - user_data: None, - metadata_options: Default::default(), - cpu_options: None, - bandwidth_weighting: None, - maintenance_options: Default::default(), - placement_tenancy: None, - placement_affinity: None, - placement_group_name: None, - private_dns_hostname_type: None, - enable_resource_name_dns_a_record: false, - enable_resource_name_dns_aaaa_record: false, - }; - state.instances.insert(id.to_string(), inst); - } - // Docker-free coverage of the metadata threading the StartInstances // stopped-then-restart fallback relies on: after a restart the runtime // registry has no handle for a `stopped` instance, so the boot task must @@ -2714,6 +2679,15 @@ mod modify_tests { }; let attrs = cfn_create_instance(&svc, "000000000000", "us-east-1", &spec); + // ...and DescribeInstances renders the profile on the instance. + let desc = body(describe_instances(&svc, &req("DescribeInstances", &[])).unwrap()); + assert!( + desc.contains( + "arn:aws:iam::000000000000:instance-profile/my-profileAIPA" + ), + "got: {desc}" + ); + let accounts = svc.state.read(); let state = accounts.get("000000000000").unwrap(); let inst = state.instances.get(&attrs.instance_id).unwrap(); @@ -2889,6 +2863,140 @@ mod modify_tests { assert!(out.contains("i-1"), "got: {out}"); } + #[test] + fn describe_instances_renders_iam_instance_profile_regardless_of_association_state() { + // Snapshots written before Associate advanced the state hold records + // stuck at `associating`; they must still show on the instance. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-old"); + { + let mut accounts = svc.state.write(); + let state = accounts.get_or_create("000000000000"); + let mut assoc = super::super::rest::new_iam_profile_association( + "i-old".into(), + "arn:aws:iam::000000000000:instance-profile/legacy".into(), + ); + assoc.state = "associating".into(); + state + .iam_instance_profile_associations + .insert(assoc.association_id.clone(), assoc); + } + let out = body(describe_instances(&svc, &req("DescribeInstances", &[])).unwrap()); + assert!( + out.contains("arn:aws:iam::000000000000:instance-profile/legacy"), + "got: {out}" + ); + } + + #[test] + fn describe_instances_reports_iam_instance_profile_after_associate() { + // AssociateIamInstanceProfile recorded an association but the instance + // render never emitted , so `describe-instances + // --query ...IamInstanceProfile.Arn` stayed null forever. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-prof"); + let assoc = body( + super::super::rest::associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-prof"), + ("IamInstanceProfile.Name", "web-profile"), + ], + ), + ) + .unwrap(), + ); + let out = body(describe_instances(&svc, &req("DescribeInstances", &[])).unwrap()); + assert!( + out.contains( + "arn:aws:iam::000000000000:instance-profile/web-profileAIPA" + ), + "got: {out}" + ); + + // Disassociating drops the element again. + let assoc_id = assoc_id_of(&assoc); + super::super::rest::disassociate_iam_instance_profile( + &svc, + &req( + "DisassociateIamInstanceProfile", + &[("AssociationId", &assoc_id)], + ), + ) + .unwrap(); + let out = body(describe_instances(&svc, &req("DescribeInstances", &[])).unwrap()); + assert!(!out.contains(""), "got: {out}"); + } + + #[test] + fn describe_instances_reports_replaced_iam_instance_profile() { + let svc = Ec2Service::new(); + seed_instance(&svc, "i-prof"); + let assoc = body( + super::super::rest::associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-prof"), + ("IamInstanceProfile.Name", "web-profile"), + ], + ), + ) + .unwrap(), + ); + let assoc_id = assoc_id_of(&assoc); + super::super::rest::replace_iam_instance_profile_association( + &svc, + &req( + "ReplaceIamInstanceProfileAssociation", + &[ + ("AssociationId", &assoc_id), + ("IamInstanceProfile.Name", "admin-profile"), + ], + ), + ) + .unwrap(); + let out = body(describe_instances(&svc, &req("DescribeInstances", &[])).unwrap()); + assert!( + out.contains("arn:aws:iam::000000000000:instance-profile/admin-profile"), + "got: {out}" + ); + assert!(!out.contains("instance-profile/web-profile"), "got: {out}"); + } + + #[tokio::test] + async fn run_instances_reports_iam_instance_profile_in_response() { + // The launch response rendered the instance before the association was + // recorded, so `run-instances --iam-instance-profile` never echoed the + // profile even though DescribeInstances (now) does. + let svc = Ec2Service::new(); + let out = body( + run_instances( + &svc, + &req( + "RunInstances", + &[ + ("ImageId", "ami-12345678"), + ("MinCount", "1"), + ("MaxCount", "1"), + ("IamInstanceProfile.Name", "web-profile"), + ], + ), + ) + .await + .unwrap(), + ); + assert!( + out.contains( + "arn:aws:iam::000000000000:instance-profile/web-profileAIPA" + ), + "got: {out}" + ); + } + #[test] fn describe_instances_group_and_attachment_filters() { let svc = Ec2Service::new(); diff --git a/crates/fakecloud-ec2/src/service/rest/mod.rs b/crates/fakecloud-ec2/src/service/rest/mod.rs index 7a3fd0191..b52de58a1 100644 --- a/crates/fakecloud-ec2/src/service/rest/mod.rs +++ b/crates/fakecloud-ec2/src/service/rest/mod.rs @@ -16,8 +16,9 @@ pub(crate) use fakecloud_core::service::{AwsRequest, AwsResponse, AwsServiceErro pub(crate) use crate::service::Ec2Service; pub(crate) use crate::service_helpers::{ - gen_id, indexed_list, require, require_struct, validate_enum, validate_int_range, - validate_length, validate_max_results, + association_not_found, gen_id, incorrect_state, indexed_list, instance_not_found, + missing_parameter, require, require_struct, validate_enum, validate_int_range, validate_length, + validate_max_results, }; pub(crate) use crate::state::{ AccountVpcEncryptionControl, ByoipCidr, CapacityManagerDataExport, Ec2State, @@ -105,11 +106,12 @@ pub(crate) fn associate_enclave_certificate_iam_role( )) } -/// Resolve the request's `IamInstanceProfile.Arn`/`.Name` into (arn, id). AWS -/// takes either; we synthesize the missing half so the association round-trips. -fn iam_profile_ref(req: &AwsRequest) -> (String, String) { - let arn = req - .query_params +/// Resolve the request's `IamInstanceProfile.Arn`/`.Name` into an ARN. AWS +/// takes either; we synthesize the ARN from the name so the association +/// round-trips. `None` when the request carries neither (optional on +/// RunInstances, required on Associate/Replace). +pub(crate) fn iam_profile_arn(req: &AwsRequest) -> Option { + req.query_params .get("IamInstanceProfile.Arn") .cloned() .or_else(|| { @@ -117,19 +119,56 @@ fn iam_profile_ref(req: &AwsRequest) -> (String, String) { .get("IamInstanceProfile.Name") .map(|n| format!("arn:aws:iam::{}:instance-profile/{n}", req.account_id)) }) - .unwrap_or_default(); - let id = gen_id("AIPA"); - (arn, id) } -fn iam_profile_assoc_xml(a: &IamInstanceProfileAssociation) -> String { +/// A fresh, `associated` IAM instance-profile association with newly minted +/// association and profile ids. Shared by RunInstances, the CloudFormation +/// instance provisioner, and the Associate/Replace handlers so all four write +/// the same record. +pub(crate) fn new_iam_profile_association( + instance_id: String, + profile_arn: String, +) -> IamInstanceProfileAssociation { + IamInstanceProfileAssociation { + association_id: gen_id("iip-assoc"), + instance_id, + iam_instance_profile_arn: profile_arn, + iam_instance_profile_id: gen_id("AIPA"), + state: "associated".to_string(), + } +} + +/// `` as rendered on both an instance and an +/// IamInstanceProfileAssociation. +pub(crate) fn iam_instance_profile_xml(a: &IamInstanceProfileAssociation) -> String { format!( - "{}{}{}{}{}", + "{}{}", + ec2_elem("arn", &a.iam_instance_profile_arn), + ec2_elem("id", &a.iam_instance_profile_id) + ) +} + +/// One association item. `state` is passed separately because the mutating +/// handlers answer with a transitional state (see below) while Describe +/// renders the stored one. +fn iam_profile_assoc_xml(a: &IamInstanceProfileAssociation, state: &str) -> String { + format!( + "{}{}{}{}", ec2_elem("associationId", &a.association_id), ec2_elem("instanceId", &a.instance_id), - ec2_elem("arn", &a.iam_instance_profile_arn), - ec2_elem("id", &a.iam_instance_profile_id), - a.state, + iam_instance_profile_xml(a), + state, + ) +} + +/// The `` body of an Associate / Replace / +/// Disassociate response. AWS answers those with the transitional state +/// (`associating` / `disassociating`) while the stored record already reads +/// `associated` (or is gone), so `state` here is wire-only. +fn iam_profile_assoc_response(a: &IamInstanceProfileAssociation, state: &str) -> String { + format!( + "{}", + iam_profile_assoc_xml(a, state) ) } @@ -138,28 +177,36 @@ pub(crate) fn associate_iam_instance_profile( req: &AwsRequest, ) -> Result { let instance_id = require(&req.query_params, "InstanceId")?; - let (arn, id) = iam_profile_ref(req); - let assoc = IamInstanceProfileAssociation { - association_id: gen_id("iip-assoc"), - instance_id, - iam_instance_profile_arn: arn, - iam_instance_profile_id: id, - state: "associating".to_string(), - }; - { + let arn = iam_profile_arn(req).ok_or_else(|| missing_parameter("IamInstanceProfile"))?; + let assoc = { let mut accounts = svc.state.write(); - accounts - .get_or_create(&req.account_id) + let state = accounts.get_or_create(&req.account_id); + if !state.instances.contains_key(&instance_id) { + return Err(instance_not_found(&instance_id)); + } + // AWS (AssociateIamInstanceProfile docs): "You cannot associate more + // than one IAM instance profile with an instance". The error code is + // IncorrectState (EC2 error-code list); the message text is ours. + // ReplaceIamInstanceProfileAssociation is the way to swap it. + if state + .iam_instance_profile_associations + .values() + .any(|a| a.instance_id == instance_id) + { + return Err(incorrect_state(format!( + "There is an existing association for instance {instance_id}" + ))); + } + let assoc = new_iam_profile_association(instance_id, arn); + state .iam_instance_profile_associations .insert(assoc.association_id.clone(), assoc.clone()); - } + assoc + }; Ok(Ec2Service::respond( "AssociateIamInstanceProfile", &req.request_id, - &format!( - "{}", - iam_profile_assoc_xml(&assoc) - ), + &iam_profile_assoc_response(&assoc, "associating"), )) } @@ -705,7 +752,7 @@ pub(crate) fn describe_iam_instance_profile_associations( .iam_instance_profile_associations .values() .filter(|a| wanted.is_empty() || wanted.contains(&a.association_id)) - .map(iam_profile_assoc_xml) + .map(|a| iam_profile_assoc_xml(a, &a.state)) .collect(); Ok(Ec2Service::respond( "DescribeIamInstanceProfileAssociations", @@ -1186,24 +1233,11 @@ pub(crate) fn disassociate_iam_instance_profile( .iam_instance_profile_associations .remove(&assoc_id) } - .map(|mut a| { - a.state = "disassociating".to_string(); - a - }) - .unwrap_or(IamInstanceProfileAssociation { - association_id: assoc_id.clone(), - instance_id: String::new(), - iam_instance_profile_arn: String::new(), - iam_instance_profile_id: String::new(), - state: "disassociating".to_string(), - }); + .ok_or_else(|| association_not_found(&assoc_id))?; Ok(Ec2Service::respond( "DisassociateIamInstanceProfile", &req.request_id, - &format!( - "{}", - iam_profile_assoc_xml(&assoc) - ), + &iam_profile_assoc_response(&assoc, "disassociating"), )) } @@ -1786,38 +1820,26 @@ pub(crate) fn replace_iam_instance_profile_association( req: &AwsRequest, ) -> Result { let assoc_id = require(&req.query_params, "AssociationId")?; - let (arn, id) = iam_profile_ref(req); + let arn = iam_profile_arn(req).ok_or_else(|| missing_parameter("IamInstanceProfile"))?; let assoc = { let mut accounts = svc.state.write(); let state = accounts.get_or_create(&req.account_id); - // AWS keeps the same association id but swaps the profile; a new id is - // minted (below) when the association is unknown to us. - if let Some(a) = state.iam_instance_profile_associations.get_mut(&assoc_id) { - a.iam_instance_profile_arn = arn; - a.iam_instance_profile_id = id; - a.state = "associated".to_string(); - a.clone() - } else { - let a = IamInstanceProfileAssociation { - association_id: assoc_id.clone(), - instance_id: String::new(), - iam_instance_profile_arn: arn, - iam_instance_profile_id: id, - state: "associated".to_string(), - }; - state - .iam_instance_profile_associations - .insert(assoc_id.clone(), a.clone()); - a - } + // AWS retires the old association and mints a new one for the same + // instance (its doc sample answers with a fresh association id). + let old = state + .iam_instance_profile_associations + .remove(&assoc_id) + .ok_or_else(|| association_not_found(&assoc_id))?; + let a = new_iam_profile_association(old.instance_id, arn); + state + .iam_instance_profile_associations + .insert(a.association_id.clone(), a.clone()); + a }; Ok(Ec2Service::respond( "ReplaceIamInstanceProfileAssociation", &req.request_id, - &format!( - "{}", - iam_profile_assoc_xml(&assoc) - ), + &iam_profile_assoc_response(&assoc, "associating"), )) } @@ -1891,6 +1913,7 @@ pub(crate) fn describe_capacity_reservation_cancellation_quotes( #[cfg(test)] mod tests { use super::*; + use crate::test_support::{assoc_id_of, err_of, seed_instance}; fn req(action: &str, query: &[(&str, &str)]) -> AwsRequest { AwsRequest { @@ -2478,54 +2501,10 @@ mod tests { assert!(attr.contains("111122223333"), "{attr}"); } - fn seed_instance(id: &str) -> crate::state::Instance { - crate::state::Instance { - instance_id: id.into(), - image_id: "ami-1".into(), - instance_type: "t3.micro".into(), - state_code: 16, - state_name: "running".into(), - private_ip: "10.0.0.1".into(), - public_ip: None, - subnet_id: Some("subnet-1".into()), - vpc_id: Some("vpc-1".into()), - key_name: None, - security_group_ids: vec![], - reservation_id: "r-1".into(), - ami_launch_index: 0, - monitoring: false, - az: "us-east-1a".into(), - launch_time: "2024-01-01T00:00:00.000Z".into(), - container_id: None, - disable_api_termination: false, - disable_api_stop: false, - source_dest_check: true, - ebs_optimized: false, - instance_initiated_shutdown_behavior: "stop".into(), - user_data: None, - metadata_options: Default::default(), - cpu_options: None, - bandwidth_weighting: None, - maintenance_options: Default::default(), - placement_tenancy: None, - placement_affinity: None, - placement_group_name: None, - private_dns_hostname_type: None, - enable_resource_name_dns_a_record: false, - enable_resource_name_dns_aaaa_record: false, - } - } - #[test] fn private_dns_name_options_persist_on_instance() { let svc = Ec2Service::new(); - { - let mut accounts = svc.state.write(); - let state = accounts.get_or_create("000000000000"); - state - .instances - .insert("i-1".to_string(), seed_instance("i-1")); - } + seed_instance(&svc, "i-1"); modify_private_dns_name_options( &svc, &req( @@ -2582,6 +2561,7 @@ mod tests { #[test] fn iam_instance_profile_association_round_trips() { let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); let out = body( associate_iam_instance_profile( &svc, @@ -2596,12 +2576,7 @@ mod tests { .unwrap(), ); // The response carries a real association id + resolved profile arn. - let assoc_id = out - .split("") - .nth(1) - .and_then(|s| s.split("").next()) - .unwrap() - .to_string(); + let assoc_id = assoc_id_of(&out); assert!(assoc_id.starts_with("iip-assoc-"), "{out}"); assert!(out.contains(":instance-profile/web-role"), "{out}"); @@ -2637,6 +2612,212 @@ mod tests { assert!(!described.contains(&assoc_id), "{described}"); } + fn associate_profile(svc: &Ec2Service, instance_id: &str, name: &str) -> String { + body( + associate_iam_instance_profile( + svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", instance_id), + ("IamInstanceProfile.Name", name), + ], + ), + ) + .unwrap(), + ) + } + + fn describe_associations(svc: &Ec2Service) -> String { + body( + describe_iam_instance_profile_associations( + svc, + &req("DescribeIamInstanceProfileAssociations", &[]), + ) + .unwrap(), + ) + } + + #[test] + fn associate_iam_instance_profile_answers_associating_then_reads_associated() { + // AWS answers `associating` and the association reads back as + // `associated`. We used to store `associating` and never advance it. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + let out = associate_profile(&svc, "i-123", "web-role"); + assert!(out.contains("associating"), "{out}"); + + let described = describe_associations(&svc); + assert!( + described.contains("associated"), + "{described}" + ); + assert!( + !described.contains("associating"), + "{described}" + ); + } + + #[test] + fn replace_iam_instance_profile_association_mints_new_association() { + // AWS answers `associating` with a *new* association id; the old one + // is gone from the describe and the new one reads back `associated`. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + let old_id = assoc_id_of(&associate_profile(&svc, "i-123", "web-role")); + + let replaced = body( + replace_iam_instance_profile_association( + &svc, + &req( + "ReplaceIamInstanceProfileAssociation", + &[ + ("AssociationId", &old_id), + ("IamInstanceProfile.Name", "admin-role"), + ], + ), + ) + .unwrap(), + ); + let new_id = assoc_id_of(&replaced); + assert_ne!(new_id, old_id, "{replaced}"); + assert!(new_id.starts_with("iip-assoc-"), "{replaced}"); + assert!( + replaced.contains("associating"), + "{replaced}" + ); + assert!( + replaced.contains("i-123"), + "{replaced}" + ); + assert!( + replaced.contains(":instance-profile/admin-role"), + "{replaced}" + ); + + let described = describe_associations(&svc); + assert!(!described.contains(&old_id), "{described}"); + assert!(described.contains(&new_id), "{described}"); + assert!( + described.contains("associated"), + "{described}" + ); + } + + #[test] + fn associate_iam_instance_profile_rejects_unknown_instance() { + // AWS: InvalidInstanceID.NotFound. We used to record an association + // pointing at nothing. + let svc = Ec2Service::new(); + let err = err_of(associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-missing"), + ("IamInstanceProfile.Name", "web-role"), + ], + ), + )); + assert_eq!(err.code(), "InvalidInstanceID.NotFound"); + let described = describe_associations(&svc); + assert!(!described.contains("iip-assoc-"), "{described}"); + } + + #[test] + fn associate_iam_instance_profile_rejects_second_profile() { + // AWS: "You cannot associate more than one IAM instance profile with + // an instance" -> IncorrectState. Use Replace instead. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + associate_profile(&svc, "i-123", "web-role"); + let err = err_of(associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-123"), + ("IamInstanceProfile.Name", "admin-role"), + ], + ), + )); + assert_eq!(err.code(), "IncorrectState"); + let described = describe_associations(&svc); + assert!( + described.contains(":instance-profile/web-role"), + "{described}" + ); + assert!(!described.contains("admin-role"), "{described}"); + } + + #[test] + fn replace_iam_instance_profile_association_rejects_unknown_id() { + // AWS: InvalidAssociationID.NotFound. We used to mint a record under + // the requested id and answer success. + let svc = Ec2Service::new(); + let err = err_of(replace_iam_instance_profile_association( + &svc, + &req( + "ReplaceIamInstanceProfileAssociation", + &[ + ("AssociationId", "iip-assoc-missing"), + ("IamInstanceProfile.Name", "web-role"), + ], + ), + )); + assert_eq!(err.code(), "InvalidAssociationID.NotFound"); + let described = describe_associations(&svc); + assert!(!described.contains("iip-assoc-"), "{described}"); + } + + #[test] + fn disassociate_iam_instance_profile_rejects_unknown_id() { + // AWS: InvalidAssociationID.NotFound. We used to answer a fabricated + // `disassociating` record. + let svc = Ec2Service::new(); + let err = err_of(disassociate_iam_instance_profile( + &svc, + &req( + "DisassociateIamInstanceProfile", + &[("AssociationId", "iip-assoc-missing")], + ), + )); + assert_eq!(err.code(), "InvalidAssociationID.NotFound"); + } + + #[test] + fn associate_iam_instance_profile_requires_profile() { + // `IamInstanceProfile` is a required member. We used to store an + // association with an empty ARN. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + let err = err_of(associate_iam_instance_profile( + &svc, + &req("AssociateIamInstanceProfile", &[("InstanceId", "i-123")]), + )); + assert_eq!(err.code(), "MissingParameter"); + let described = describe_associations(&svc); + assert!(!described.contains("iip-assoc-"), "{described}"); + } + + #[test] + fn replace_iam_instance_profile_association_requires_profile() { + // Same required member on Replace; the old association must survive. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + let old_id = assoc_id_of(&associate_profile(&svc, "i-123", "web-role")); + let err = err_of(replace_iam_instance_profile_association( + &svc, + &req( + "ReplaceIamInstanceProfileAssociation", + &[("AssociationId", &old_id)], + ), + )); + assert_eq!(err.code(), "MissingParameter"); + let described = describe_associations(&svc); + assert!(described.contains(&old_id), "{described}"); + } + #[test] fn public_ipv4_pool_round_trips() { let svc = Ec2Service::new(); diff --git a/crates/fakecloud-ec2/src/service_helpers.rs b/crates/fakecloud-ec2/src/service_helpers.rs index 2298e8dbb..b0759db02 100644 --- a/crates/fakecloud-ec2/src/service_helpers.rs +++ b/crates/fakecloud-ec2/src/service_helpers.rs @@ -84,6 +84,22 @@ pub fn incorrect_instance_state(id: &str, current: &str) -> AwsServiceError { ) } +/// `IncorrectState` (HTTP 400) — the resource is in the wrong state for the +/// request (e.g. associating a second IAM instance profile with an instance). +pub fn incorrect_state(message: impl Into) -> AwsServiceError { + AwsServiceError::aws_error(StatusCode::BAD_REQUEST, "IncorrectState", message.into()) +} + +/// `InvalidAssociationID.NotFound` (HTTP 400) — the requested association id +/// does not exist. +pub fn association_not_found(id: &str) -> AwsServiceError { + AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidAssociationID.NotFound", + format!("The association ID '{id}' does not exist"), + ) +} + /// Match an EC2 filter value against a candidate, honoring the `*` (any run) /// and `?` (any single char) wildcards AWS supports in filter values. A value /// with no wildcard is an exact match. From 8f8741040430518482b50e556c0a6a88f23672ee Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 20 Sep 2026 16:32:12 -0300 Subject: [PATCH 2/8] fix(ec2): finish the IAM instance-profile association contract Follow-ups on top of the iamInstanceProfile render, found reviewing it. Conformance: EC2 declares no Smithy errors, so the probe treats any 4xx on a Success variant as undeclared unless the code is in the service's shared-error list. Add InvalidAssociationID.NotFound, IncorrectState and InvalidIamInstanceProfileArn.Malformed there, so the new AWS-correct errors read as handler responses instead of silently shedding ec2 variants (the per-service flake margin would have hidden the loss). DescribeIamInstanceProfileAssociations honored only AssociationId.N and dropped Filter.N. The terraform AWS provider reads an instance's profile with Filter Name=instance-id and takes item [0], so with two instances in an account it got the other instance's association and called Replace on it. With Replace now retargeting the stored instance, that silently moved instance A's profile onto B. Honor instance-id and state, sort the set by association id so repeated describes agree, and return InvalidAssociationID.NotFound for an explicitly named unknown id rather than an empty set. TerminateInstances left the association behind, so a terminated instance kept reporting , kept showing in the association describe, and could never be associated again (the new one-profile check saw the stale record). Drop the account's associations for the instances a Terminate affects, as AWS does. Validate the profile reference instead of fabricating one: an empty Arn=/Name= (what the SDK builder sends for .name("")) now reads as absent rather than storing an empty ARN, a name outside [\w+=,.@-]{1,128} is InvalidParameterValue, and an ARN that is not an instance-profile ARN is InvalidIamInstanceProfileArn.Malformed. Associate also rejects a shutting-down or terminated instance with IncorrectInstanceState; AWS associates with a running or stopped instance only. The rendered profile id used gen_id("AIPA"), which is EC2's resource-id shape (AIPA-); AWS unique ids are a 4-char prefix plus 17 uppercase alphanumerics with no separator, the shape fakecloud's own IAM service mints. Add aws_unique_id and use it, now that this PR makes the value visible on every DescribeInstances. DescribeInstances gains the iam-instance-profile.arn and .id filters, reading the same record the instance renders. CloudFormation applied IamInstanceProfile on create but never on update, so an UpdateStack that changed it reported UPDATE_COMPLETE with the old profile still attached; AWS updates it in place. The update arm now replaces, associates or disassociates to match the template. That path also needed the actions it already issued -- ModifyInstanceAttribute and CreateTags -- added to the EC2 provisioner dispatch, which they were missing, so an in-place instance update failed the whole stack update with action_not_implemented. Tests: handler-level coverage for each error and for the filter, sort and terminate behavior; a DescribeInstances profile-filter test; and an e2e that creates a stack with an instance profile, updates the stack to a different profile, and asserts the instance id is unchanged and describe-instances reports the new profile. --- .../src/resource_provisioner/ec2.rs | 90 ++++-- .../src/probe/response.rs | 10 + ...loudformation_rds_port_and_ec2_instance.rs | 87 ++++++ crates/fakecloud-ec2/src/service/instance.rs | 134 +++++++- crates/fakecloud-ec2/src/service/mod.rs | 16 + crates/fakecloud-ec2/src/service/rest/mod.rs | 288 ++++++++++++++++-- crates/fakecloud-ec2/src/service_helpers.rs | 43 +++ 7 files changed, 631 insertions(+), 37 deletions(-) diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs index 0a2c23a36..b93174585 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs @@ -514,25 +514,7 @@ 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); let spec = fakecloud_ec2::cfn_provision::CfnInstanceSpec { image_id: prop_str(props, "ImageId").map(String::from), @@ -673,11 +655,59 @@ 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())) } + /// 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); + let wanted = arn + .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 = existing + .split("") + .nth(1) + .and_then(|s| s.split("").next()) + .map(str::to_string); + + 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)) => { + 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 +744,25 @@ 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, Option) { + 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), + } +} diff --git a/crates/fakecloud-conformance/src/probe/response.rs b/crates/fakecloud-conformance/src/probe/response.rs index 251217bed..149868107 100644 --- a/crates/fakecloud-conformance/src/probe/response.rs +++ b/crates/fakecloud-conformance/src/probe/response.rs @@ -235,6 +235,16 @@ pub(super) fn service_common_errors(service_name: &str) -> &'static [&'static st // IPAM internet-registry associations: the probe addresses an // association by a synthetic id, which AWS answers with this code. "InvalidIpamInternetRegistryAssociationId.NotFound", + // IAM instance-profile associations: Replace/Disassociate address + // an association by the probe's synthetic id, which AWS answers + // with this code. `IncorrectState` is the code AWS returns when the + // resource is in the wrong state for the request (associating a + // second IAM instance profile with an instance, for one). + "InvalidAssociationID.NotFound", + "IncorrectState", + // A synthetic `IamInstanceProfile.Arn` is not a well-formed + // instance-profile ARN, which AWS rejects with this code. + "InvalidIamInstanceProfileArn.Malformed", "InvalidID", ], // EKS under-declares two client errors that the real API returns for diff --git a/crates/fakecloud-e2e/tests/cloudformation_rds_port_and_ec2_instance.rs b/crates/fakecloud-e2e/tests/cloudformation_rds_port_and_ec2_instance.rs index 6fecc3102..ef4e0d1b7 100644 --- a/crates/fakecloud-e2e/tests/cloudformation_rds_port_and_ec2_instance.rs +++ b/crates/fakecloud-e2e/tests/cloudformation_rds_port_and_ec2_instance.rs @@ -121,3 +121,90 @@ async fn cfn_rds_engine_port_and_real_ec2_instance() { "CFN-created instance {instance_ref} must be described" ); } + +const PROFILE_TEMPLATE: &str = r#"{ + "AWSTemplateFormatVersion": "2010-09-09", + "Resources": { + "Instance": { + "Type": "AWS::EC2::Instance", + "Properties": { + "ImageId": "ami-0123456789abcdef0", + "InstanceType": "t3.micro", + "IamInstanceProfile": "PROFILE" + } + } + }, + "Outputs": { + "InstanceRef": {"Value": {"Ref": "Instance"}} + } +}"#; + +async fn instance_profile_arn(ec2: &aws_sdk_ec2::Client, id: &str) -> Option { + ec2.describe_instances() + .instance_ids(id) + .send() + .await + .expect("describe_instances") + .reservations() + .iter() + .flat_map(|r| r.instances()) + .find(|i| i.instance_id() == Some(id)) + .and_then(|i| i.iam_instance_profile()) + .and_then(|p| p.arn()) + .map(str::to_string) +} + +#[tokio::test] +async fn cfn_update_swaps_the_ec2_instance_profile_in_place() { + // An UpdateStack that changes IamInstanceProfile reported UPDATE_COMPLETE + // while the instance kept the old profile, because the in-place update arm + // never touched the association. + let server = TestServer::start().await; + let cfn = server.cloudformation_client().await; + let ec2 = server.ec2_client().await; + + cfn.create_stack() + .stack_name("profile-update-stack") + .template_body(PROFILE_TEMPLATE.replace("PROFILE", "web-profile")) + .capabilities(Capability::CapabilityIam) + .send() + .await + .expect("create_stack"); + + let described = cfn + .describe_stacks() + .stack_name("profile-update-stack") + .send() + .await + .expect("describe_stacks"); + let stack = described.stacks().first().expect("stack present"); + assert_eq!(stack.stack_status().unwrap().as_str(), "CREATE_COMPLETE"); + let id = output(stack, "InstanceRef").to_string(); + assert_eq!( + instance_profile_arn(&ec2, &id).await.as_deref(), + Some("arn:aws:iam::123456789012:instance-profile/web-profile") + ); + + cfn.update_stack() + .stack_name("profile-update-stack") + .template_body(PROFILE_TEMPLATE.replace("PROFILE", "admin-profile")) + .capabilities(Capability::CapabilityIam) + .send() + .await + .expect("update_stack"); + + let described = cfn + .describe_stacks() + .stack_name("profile-update-stack") + .send() + .await + .expect("describe_stacks"); + let stack = described.stacks().first().expect("stack present"); + assert_eq!(stack.stack_status().unwrap().as_str(), "UPDATE_COMPLETE"); + // The instance is updated in place, not replaced. + assert_eq!(output(stack, "InstanceRef"), id); + assert_eq!( + instance_profile_arn(&ec2, &id).await.as_deref(), + Some("arn:aws:iam::123456789012:instance-profile/admin-profile") + ); +} diff --git a/crates/fakecloud-ec2/src/service/instance.rs b/crates/fakecloud-ec2/src/service/instance.rs index 64129b7a8..692bd639a 100644 --- a/crates/fakecloud-ec2/src/service/instance.rs +++ b/crates/fakecloud-ec2/src/service/instance.rs @@ -365,7 +365,7 @@ pub(crate) async fn run_instances( .map(|v| v == "true") .unwrap_or(false); let metadata_options = parse_metadata_options(&req.query_params); - let iam_profile_arn = super::rest::iam_profile_arn(req); + let iam_profile_arn = super::rest::iam_profile_arn(req)?; let az = format!( "{}a", if req.region.is_empty() { @@ -1172,6 +1172,16 @@ async fn change_state( state_xml("previousState", prev_code, &prev_name), )); } + // AWS detaches the IAM instance profile when an instance terminates: + // the record leaves DescribeIamInstanceProfileAssociations and the + // instance stops reporting . Without this the + // stale record also makes a later AssociateIamInstanceProfile on that + // id fail forever with IncorrectState. + if new_code == 48 { + state + .iam_instance_profile_associations + .retain(|_, a| !affected.contains(&a.instance_id)); + } } // Drive the backing container's lifecycle in the background so the response @@ -1458,6 +1468,7 @@ pub(crate) fn describe_instances( &arch_for(state, &i.image_id), &sg_names, bdm_by_instance.get(&i.instance_id).unwrap_or(&no_bdm), + profile_by_instance.get(i.instance_id.as_str()).copied(), ) }) .collect(); @@ -1523,6 +1534,7 @@ fn inst_match( architecture: &str, sg_names: &HashMap, block_devices: &[&crate::state::VolumeAttachment], + iam_assoc: Option<&IamInstanceProfileAssociation>, ) -> bool { use crate::service_helpers::filter_value_matches; filters.iter().all(|f| { @@ -1586,6 +1598,16 @@ fn inst_match( .iter() .map(|a| a.delete_on_termination.to_string()) .collect(), + // The IAM instance profile lives in the association map, so these + // read from the same record the instance renders. + "iam-instance-profile.arn" => iam_assoc + .map(|a| a.iam_instance_profile_arn.clone()) + .into_iter() + .collect(), + "iam-instance-profile.id" => iam_assoc + .map(|a| a.iam_instance_profile_id.clone()) + .into_iter() + .collect(), "tag-key" => tags.iter().map(|t| t.key.clone()).collect(), "tag-value" => tags.iter().map(|t| t.value.clone()).collect(), name => { @@ -1649,6 +1671,8 @@ pub(crate) fn describe_instance_status( &arch_for(state, &i.image_id), &sg_names, bdm_by_instance.get(&i.instance_id).unwrap_or(&no_bdm), + // DescribeInstanceStatus takes no iam-instance-profile filter. + None, ) }) .collect(); @@ -2997,6 +3021,114 @@ mod modify_tests { ); } + #[tokio::test] + async fn terminate_instances_drops_the_iam_instance_profile_association() { + // AWS detaches the profile on terminate. A stale record kept rendering + // on a terminated instance and made a later + // Associate on that id fail forever with IncorrectState. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-gone"); + seed_instance(&svc, "i-stays"); + for id in ["i-gone", "i-stays"] { + super::super::rest::associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", id), + ("IamInstanceProfile.Name", "web-profile"), + ], + ), + ) + .unwrap(); + } + + terminate_instances( + &svc, + &req("TerminateInstances", &[("InstanceId.1", "i-gone")]), + ) + .await + .unwrap(); + + let described = body( + super::super::rest::describe_iam_instance_profile_associations( + &svc, + &req("DescribeIamInstanceProfileAssociations", &[]), + ) + .unwrap(), + ); + assert!( + !described.contains("i-gone"), + "{described}" + ); + assert!( + described.contains("i-stays"), + "{described}" + ); + + let out = body( + describe_instances( + &svc, + &req("DescribeInstances", &[("InstanceId.1", "i-gone")]), + ) + .unwrap(), + ); + assert!(!out.contains(""), "{out}"); + } + + #[test] + fn describe_instances_filters_on_iam_instance_profile() { + let svc = Ec2Service::new(); + seed_instance(&svc, "i-a"); + seed_instance(&svc, "i-b"); + super::super::rest::associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-a"), + ("IamInstanceProfile.Name", "web-profile"), + ], + ), + ) + .unwrap(); + + let out = body( + describe_instances( + &svc, + &req( + "DescribeInstances", + &[ + ("Filter.1.Name", "iam-instance-profile.arn"), + ( + "Filter.1.Value.1", + "arn:aws:iam::000000000000:instance-profile/web-profile", + ), + ], + ), + ) + .unwrap(), + ); + assert!(out.contains("i-a"), "{out}"); + assert!(!out.contains("i-b"), "{out}"); + + // An instance with no profile matches neither filter name. + let out = body( + describe_instances( + &svc, + &req( + "DescribeInstances", + &[ + ("Filter.1.Name", "iam-instance-profile.id"), + ("Filter.1.Value.1", "AIPANOTHERE000000000"), + ], + ), + ) + .unwrap(), + ); + assert!(!out.contains(""), "{out}"); + } + #[test] fn describe_instances_group_and_attachment_filters() { let svc = Ec2Service::new(); diff --git a/crates/fakecloud-ec2/src/service/mod.rs b/crates/fakecloud-ec2/src/service/mod.rs index 4a2b45e56..ad7c94768 100644 --- a/crates/fakecloud-ec2/src/service/mod.rs +++ b/crates/fakecloud-ec2/src/service/mod.rs @@ -974,6 +974,22 @@ impl Ec2Service { "ModifySubnetAttribute" => subnet::modify_subnet_attribute(self, request), "AuthorizeSecurityGroupIngress" => sg::authorize_security_group_ingress(self, request), "AuthorizeSecurityGroupEgress" => sg::authorize_security_group_egress(self, request), + // What an in-place UpdateStack on an AWS::EC2::Instance re-applies: + // mutable attributes, the template's tags, and the IAM instance + // profile. Without these arms the provisioner's update path fails + // the whole stack update with `action_not_implemented`. + "ModifyInstanceAttribute" => instance::modify_instance_attribute(self, request), + "CreateTags" => tags::create_tags(self, request), + "AssociateIamInstanceProfile" => rest::associate_iam_instance_profile(self, request), + "ReplaceIamInstanceProfileAssociation" => { + rest::replace_iam_instance_profile_association(self, request) + } + "DisassociateIamInstanceProfile" => { + rest::disassociate_iam_instance_profile(self, request) + } + "DescribeIamInstanceProfileAssociations" => { + rest::describe_iam_instance_profile_associations(self, request) + } other => Err(AwsServiceError::action_not_implemented("ec2", other)), } } diff --git a/crates/fakecloud-ec2/src/service/rest/mod.rs b/crates/fakecloud-ec2/src/service/rest/mod.rs index b52de58a1..b6f340ad8 100644 --- a/crates/fakecloud-ec2/src/service/rest/mod.rs +++ b/crates/fakecloud-ec2/src/service/rest/mod.rs @@ -16,9 +16,11 @@ pub(crate) use fakecloud_core::service::{AwsRequest, AwsResponse, AwsServiceErro pub(crate) use crate::service::Ec2Service; pub(crate) use crate::service_helpers::{ - association_not_found, gen_id, incorrect_state, indexed_list, instance_not_found, - missing_parameter, require, require_struct, validate_enum, validate_int_range, validate_length, - validate_max_results, + association_not_found, aws_unique_id, filter_value_matches, gen_id, incorrect_instance_state, + incorrect_state, indexed_list, instance_not_found, invalid_parameter_value, + is_instance_profile_arn, is_instance_profile_name, malformed_instance_profile_arn, + missing_parameter, parse_filters, require, require_struct, validate_enum, validate_int_range, + validate_length, validate_max_results, Filter, }; pub(crate) use crate::state::{ AccountVpcEncryptionControl, ByoipCidr, CapacityManagerDataExport, Ec2State, @@ -110,15 +112,37 @@ pub(crate) fn associate_enclave_certificate_iam_role( /// takes either; we synthesize the ARN from the name so the association /// round-trips. `None` when the request carries neither (optional on /// RunInstances, required on Associate/Replace). -pub(crate) fn iam_profile_arn(req: &AwsRequest) -> Option { - req.query_params +pub(crate) fn iam_profile_arn(req: &AwsRequest) -> Result, AwsServiceError> { + // An empty `Arn=` / `Name=` is the same as the member being absent: the + // SDKs serialize `IamInstanceProfileSpecification::builder().name("")` as + // a present-but-empty parameter, and storing that would leave an + // association whose ARN renders as `` on every instance. + if let Some(arn) = req + .query_params .get("IamInstanceProfile.Arn") - .cloned() - .or_else(|| { - req.query_params - .get("IamInstanceProfile.Name") - .map(|n| format!("arn:aws:iam::{}:instance-profile/{n}", req.account_id)) - }) + .filter(|v| !v.is_empty()) + { + if !is_instance_profile_arn(arn) { + return Err(malformed_instance_profile_arn(arn)); + } + return Ok(Some(arn.clone())); + } + if let Some(name) = req + .query_params + .get("IamInstanceProfile.Name") + .filter(|v| !v.is_empty()) + { + if !is_instance_profile_name(name) { + return Err(invalid_parameter_value(format!( + "Invalid IAM Instance Profile name: {name}" + ))); + } + return Ok(Some(format!( + "arn:aws:iam::{}:instance-profile/{name}", + req.account_id + ))); + } + Ok(None) } /// A fresh, `associated` IAM instance-profile association with newly minted @@ -133,7 +157,7 @@ pub(crate) fn new_iam_profile_association( association_id: gen_id("iip-assoc"), instance_id, iam_instance_profile_arn: profile_arn, - iam_instance_profile_id: gen_id("AIPA"), + iam_instance_profile_id: aws_unique_id("AIPA"), state: "associated".to_string(), } } @@ -177,12 +201,19 @@ pub(crate) fn associate_iam_instance_profile( req: &AwsRequest, ) -> Result { let instance_id = require(&req.query_params, "InstanceId")?; - let arn = iam_profile_arn(req).ok_or_else(|| missing_parameter("IamInstanceProfile"))?; + let arn = iam_profile_arn(req)?.ok_or_else(|| missing_parameter("IamInstanceProfile"))?; let assoc = { let mut accounts = svc.state.write(); let state = accounts.get_or_create(&req.account_id); - if !state.instances.contains_key(&instance_id) { - return Err(instance_not_found(&instance_id)); + let inst = state + .instances + .get(&instance_id) + .ok_or_else(|| instance_not_found(&instance_id))?; + // AWS: "Associates an IAM instance profile with a running or stopped + // instance." A shutting-down (32) or terminated (48) instance is + // rejected with IncorrectInstanceState. + if inst.state_code == 32 || inst.state_code == 48 { + return Err(incorrect_instance_state(&instance_id, &inst.state_name)); } // AWS (AssociateIamInstanceProfile docs): "You cannot associate more // than one IAM instance profile with an instance". The error code is @@ -739,19 +770,52 @@ pub(crate) fn describe_host_reservations( )) } +/// Match one association against the `Filter.N` names AWS documents for +/// DescribeIamInstanceProfileAssociations (`instance-id`, `state`). An unknown +/// filter name matches nothing, as elsewhere in this crate. +fn iam_assoc_match(a: &IamInstanceProfileAssociation, filters: &[Filter]) -> bool { + filters.iter().all(|f| { + let candidates: Vec<&str> = match f.name.as_str() { + "instance-id" => vec![a.instance_id.as_str()], + "state" => vec![a.state.as_str()], + _ => return false, + }; + f.values + .iter() + .any(|v| candidates.iter().any(|c| filter_value_matches(v, c))) + }) +} + pub(crate) fn describe_iam_instance_profile_associations( svc: &Ec2Service, req: &AwsRequest, ) -> Result { validate_max_results(&req.query_params, 5, 1000)?; let wanted = indexed_list(&req.query_params, "AssociationId"); + let filters = parse_filters(&req.query_params); let accounts = svc.state.read(); let empty = Ec2State::new(&req.account_id, &req.region); let state = accounts.get(&req.account_id).unwrap_or(&empty); - let items: Vec = state + // An explicitly-named association that does not exist is a hard error on + // AWS, not an empty set — the same contract Replace/Disassociate now hold. + for id in &wanted { + if !state.iam_instance_profile_associations.contains_key(id) { + return Err(association_not_found(id)); + } + } + // The terraform AWS provider reads an instance's profile by filtering on + // `instance-id` and taking the first item, so honoring `Filter.N` is what + // keeps it from picking up another instance's association. + let mut matching: Vec<&IamInstanceProfileAssociation> = state .iam_instance_profile_associations .values() .filter(|a| wanted.is_empty() || wanted.contains(&a.association_id)) + .filter(|a| iam_assoc_match(a, &filters)) + .collect(); + // The backing map is unordered; sort so repeated describes agree. + matching.sort_by(|a, b| a.association_id.cmp(&b.association_id)); + let items: Vec = matching + .into_iter() .map(|a| iam_profile_assoc_xml(a, &a.state)) .collect(); Ok(Ec2Service::respond( @@ -1820,7 +1884,7 @@ pub(crate) fn replace_iam_instance_profile_association( req: &AwsRequest, ) -> Result { let assoc_id = require(&req.query_params, "AssociationId")?; - let arn = iam_profile_arn(req).ok_or_else(|| missing_parameter("IamInstanceProfile"))?; + let arn = iam_profile_arn(req)?.ok_or_else(|| missing_parameter("IamInstanceProfile"))?; let assoc = { let mut accounts = svc.state.write(); let state = accounts.get_or_create(&req.account_id); @@ -2800,6 +2864,196 @@ mod tests { assert!(!described.contains("iip-assoc-"), "{described}"); } + #[test] + fn describe_iam_instance_profile_associations_filters_by_instance() { + // The terraform provider reads an instance's profile by filtering on + // `instance-id`; ignoring Filter.N handed it another instance's + // association, which Replace would then retarget. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-a"); + seed_instance(&svc, "i-b"); + associate_profile(&svc, "i-a", "a-role"); + associate_profile(&svc, "i-b", "b-role"); + + let out = body( + describe_iam_instance_profile_associations( + &svc, + &req( + "DescribeIamInstanceProfileAssociations", + &[ + ("Filter.1.Name", "instance-id"), + ("Filter.1.Value.1", "i-b"), + ], + ), + ) + .unwrap(), + ); + assert!(out.contains("i-b"), "{out}"); + assert!(!out.contains("i-a"), "{out}"); + assert!(!out.contains("a-role"), "{out}"); + + // `state` is the other documented filter. + let out = body( + describe_iam_instance_profile_associations( + &svc, + &req( + "DescribeIamInstanceProfileAssociations", + &[ + ("Filter.1.Name", "state"), + ("Filter.1.Value.1", "disassociated"), + ], + ), + ) + .unwrap(), + ); + assert!(!out.contains("iip-assoc-"), "{out}"); + } + + #[test] + fn describe_iam_instance_profile_associations_rejects_unknown_id() { + // Same fabricated-success class the mutating handlers dropped: an + // explicitly-named association that does not exist is an error, not an + // empty set. + let svc = Ec2Service::new(); + let err = err_of(describe_iam_instance_profile_associations( + &svc, + &req( + "DescribeIamInstanceProfileAssociations", + &[("AssociationId.1", "iip-assoc-missing")], + ), + )); + assert_eq!(err.code(), "InvalidAssociationID.NotFound"); + } + + #[test] + fn associate_iam_instance_profile_rejects_empty_profile() { + // `IamInstanceProfileSpecification::builder().name("")` serializes as a + // present-but-empty parameter; it must read as absent, not store an + // association whose ARN renders as an empty on the instance. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + for param in ["IamInstanceProfile.Name", "IamInstanceProfile.Arn"] { + let err = err_of(associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[("InstanceId", "i-123"), (param, "")], + ), + )); + assert_eq!(err.code(), "MissingParameter", "{param}"); + } + let described = describe_associations(&svc); + assert!(!described.contains("iip-assoc-"), "{described}"); + } + + #[test] + fn associate_iam_instance_profile_validates_the_profile_reference() { + // A name outside `[\w+=,.@-]{1,128}` and an ARN that is not an + // instance-profile ARN are both rejected instead of being fabricated + // into a stored association. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + let err = err_of(associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-123"), + ("IamInstanceProfile.Name", "not a/valid name"), + ], + ), + )); + assert_eq!(err.code(), "InvalidParameterValue"); + let err = err_of(associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-123"), + ( + "IamInstanceProfile.Arn", + "arn:aws:iam::000000000000:role/web", + ), + ], + ), + )); + assert_eq!(err.code(), "InvalidIamInstanceProfileArn.Malformed"); + let described = describe_associations(&svc); + assert!(!described.contains("iip-assoc-"), "{described}"); + + // A well-formed ARN is taken verbatim. + let out = body( + associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-123"), + ( + "IamInstanceProfile.Arn", + "arn:aws:iam::000000000000:instance-profile/team/web", + ), + ], + ), + ) + .unwrap(), + ); + assert!( + out.contains("arn:aws:iam::000000000000:instance-profile/team/web"), + "{out}" + ); + } + + #[test] + fn associate_iam_instance_profile_rejects_a_terminated_instance() { + // AWS associates a profile with a running or stopped instance only. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-dead"); + { + let mut accounts = svc.state.write(); + let state = accounts.get_or_create("000000000000"); + let inst = state.instances.get_mut("i-dead").unwrap(); + inst.state_code = 48; + inst.state_name = "terminated".into(); + } + let err = err_of(associate_iam_instance_profile( + &svc, + &req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-dead"), + ("IamInstanceProfile.Name", "web-role"), + ], + ), + )); + assert_eq!(err.code(), "IncorrectInstanceState"); + } + + #[test] + fn iam_instance_profile_id_uses_the_aws_unique_id_shape() { + // AWS unique ids are a 4-char prefix plus 17 uppercase alphanumerics + // with no separator, the same shape fakecloud's IAM service mints for + // InstanceProfileId. EC2's `gen_id` resource shape (`AIPA-`) + // is wrong, and this PR makes the value user-visible on every + // DescribeInstances. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + let out = associate_profile(&svc, "i-123", "web-role"); + let id = out + .split("") + .nth(1) + .and_then(|s| s.split("").next()) + .expect("profile id"); + assert_eq!(id.len(), 21, "{id}"); + assert!(id.starts_with("AIPA"), "{id}"); + assert!( + id[4..] + .chars() + .all(|c| c.is_ascii_uppercase() || c.is_ascii_digit()), + "{id}" + ); + } + #[test] fn replace_iam_instance_profile_association_requires_profile() { // Same required member on Replace; the old association must survive. diff --git a/crates/fakecloud-ec2/src/service_helpers.rs b/crates/fakecloud-ec2/src/service_helpers.rs index b0759db02..6d5dc00bd 100644 --- a/crates/fakecloud-ec2/src/service_helpers.rs +++ b/crates/fakecloud-ec2/src/service_helpers.rs @@ -26,6 +26,15 @@ pub fn gen_id(prefix: &str) -> String { format!("{prefix}-{}", &hex[..17]) } +/// Generate an AWS unique id: a 4-char prefix (`AIPA`, `AIDA`, `AROA`, ...) +/// followed by 17 uppercase alphanumerics, with no separator — the shape IAM +/// mints instance-profile, user and role ids in. `gen_id`'s `-` form is the EC2 *resource* id shape and is wrong for this family. +pub fn aws_unique_id(prefix: &str) -> String { + let hex = uuid::Uuid::new_v4().simple().to_string().to_uppercase(); + format!("{prefix}{}", &hex[..17]) +} + /// `InvalidParameterValue` — the catch-all 400 for bad EC2 input. pub fn invalid_parameter_value(message: impl Into) -> AwsServiceError { AwsServiceError::aws_error( @@ -90,6 +99,40 @@ pub fn incorrect_state(message: impl Into) -> AwsServiceError { AwsServiceError::aws_error(StatusCode::BAD_REQUEST, "IncorrectState", message.into()) } +/// `InvalidIamInstanceProfileArn.Malformed` (HTTP 400) — the supplied IAM +/// instance-profile ARN is not a well-formed instance-profile ARN. +pub fn malformed_instance_profile_arn(arn: &str) -> AwsServiceError { + AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidIamInstanceProfileArn.Malformed", + format!("The IAM instance profile ARN '{arn}' is malformed"), + ) +} + +/// Whether `arn` is a well-formed IAM instance-profile ARN: +/// `arn::iam:::instance-profile/`. +pub fn is_instance_profile_arn(arn: &str) -> bool { + let parts: Vec<&str> = arn.splitn(6, ':').collect(); + parts.len() == 6 + && parts[0] == "arn" + && !parts[1].is_empty() + && parts[2] == "iam" + && parts[3].is_empty() + && parts[5] + .strip_prefix("instance-profile/") + .is_some_and(|rest| !rest.is_empty() && !rest.ends_with('/')) +} + +/// Whether `name` is a legal IAM instance-profile name: `[\w+=,.@-]{1,128}`, +/// the pattern the IAM API documents for the `InstanceProfileName` member. +pub fn is_instance_profile_name(name: &str) -> bool { + !name.is_empty() + && name.len() <= 128 + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || "_+=,.@-".contains(c)) +} + /// `InvalidAssociationID.NotFound` (HTTP 400) — the requested association id /// does not exist. pub fn association_not_found(id: &str) -> AwsServiceError { From 874b915116774204426ae9f397406aac4248f096 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 20 Sep 2026 16:47:42 -0300 Subject: [PATCH 3/8] fix(ec2): tie the profile id to the profile, not the association Second review pass on the association contract. The rendered InstanceProfileId was minted per association, so two instances sharing one profile reported two different ids and the new iam-instance-profile.id filter could never select both; re-pointing at the same profile changed it again. AWS reports the profile's own id. EC2 cannot read IAM's store (no fakecloud-iam dependency), so derive the id from the profile ARN instead: stable across restarts, shared by every association with that profile, still AIPA + 17 uppercase alphanumerics. A name-based association synthesized arn:aws: regardless of region while IAM mints the ARN in the region's partition, so in cn-* / us-gov-* / us-iso* the two never compared equal. Use partition_for(region) in both the handler and the CloudFormation create path. The CloudFormation update arm replaced the association on every in-place update, because it never compared the template's profile to the attached one. An UpdateStack that only bumped InstanceType or added a tag retired the association id and the profile id under callers holding them; AWS leaves the association alone when the profile is unchanged. Compare first, and pull the id out with the file's existing xml_elem rather than a hand-rolled split. The CloudFormation create path built the association straight from the template with none of the validation Associate and Replace apply, so a template resolving IamInstanceProfile to a role ARN (the usual Fn::GetAtt mistake) created fine and then made every later UpdateStack fail when the stored value went back through the validating handler. Validate on create and update alike. Also drop the redundant sort in the association describe: the backing map is a BTreeMap keyed by association id, so its iteration order is already stable. Tests: one profile shared by two instances reports one id; a cn-north-1 association renders an aws-cn ARN; an unrelated stack update keeps the association id; a role ARN in the template fails the create. --- .../src/resource_provisioner/ec2.rs | 44 +++++++++++-- ...loudformation_rds_port_and_ec2_instance.rs | 65 +++++++++++++++++++ crates/fakecloud-ec2/src/service/instance.rs | 5 +- crates/fakecloud-ec2/src/service/rest/mod.rs | 65 ++++++++++++++++--- crates/fakecloud-ec2/src/service_helpers.rs | 33 +++++++--- 5 files changed, 187 insertions(+), 25 deletions(-) diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs index b93174585..ae548fc3f 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs @@ -515,6 +515,11 @@ impl ResourceProvisioner { }); 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)?; let spec = fakecloud_ec2::cfn_provision::CfnInstanceSpec { image_id: prop_str(props, "ImageId").map(String::from), @@ -669,6 +674,7 @@ impl ResourceProvisioner { /// 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)?; let wanted = arn .map(|a| ("IamInstanceProfile.Arn", a)) .or_else(|| name.map(|n| ("IamInstanceProfile.Name", n))); @@ -677,11 +683,8 @@ impl ResourceProvisioner { 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 = existing - .split("") - .nth(1) - .and_then(|s| s.split("").next()) - .map(str::to_string); + let existing_id = xml_elem(&existing, "associationId"); + let existing_arn = xml_elem(&existing, "arn"); match (wanted, existing_id) { (None, None) => Ok(()), @@ -692,6 +695,18 @@ impl ResourceProvisioner { 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 unchanged = existing_arn.as_deref().is_some_and(|arn| { + arn == value + || (key == "IamInstanceProfile.Name" + && arn.rsplit('/').next() == Some(value.as_str())) + }); + if unchanged { + return Ok(()); + } let mut params = HashMap::new(); params.insert("AssociationId".to_string(), id); params.insert(key.to_string(), value); @@ -766,3 +781,22 @@ fn cfn_iam_instance_profile(props: &Value) -> (Option, Option) { _ => (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, + name: &Option, +) -> 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(()) +} diff --git a/crates/fakecloud-e2e/tests/cloudformation_rds_port_and_ec2_instance.rs b/crates/fakecloud-e2e/tests/cloudformation_rds_port_and_ec2_instance.rs index ef4e0d1b7..ac6314624 100644 --- a/crates/fakecloud-e2e/tests/cloudformation_rds_port_and_ec2_instance.rs +++ b/crates/fakecloud-e2e/tests/cloudformation_rds_port_and_ec2_instance.rs @@ -139,6 +139,23 @@ const PROFILE_TEMPLATE: &str = r#"{ } }"#; +async fn instance_association_id(ec2: &aws_sdk_ec2::Client, id: &str) -> Option { + ec2.describe_iam_instance_profile_associations() + .filters( + aws_sdk_ec2::types::Filter::builder() + .name("instance-id") + .values(id) + .build(), + ) + .send() + .await + .expect("describe_iam_instance_profile_associations") + .iam_instance_profile_associations() + .first() + .and_then(|a| a.association_id()) + .map(str::to_string) +} + async fn instance_profile_arn(ec2: &aws_sdk_ec2::Client, id: &str) -> Option { ec2.describe_instances() .instance_ids(id) @@ -185,6 +202,28 @@ async fn cfn_update_swaps_the_ec2_instance_profile_in_place() { Some("arn:aws:iam::123456789012:instance-profile/web-profile") ); + let before = instance_association_id(&ec2, &id) + .await + .expect("association"); + + // An update that leaves IamInstanceProfile alone must not retire the + // association: AWS only touches it when the profile itself changes. + cfn.update_stack() + .stack_name("profile-update-stack") + .template_body( + PROFILE_TEMPLATE + .replace("PROFILE", "web-profile") + .replace("t3.micro", "t3.small"), + ) + .capabilities(Capability::CapabilityIam) + .send() + .await + .expect("update_stack"); + assert_eq!( + instance_association_id(&ec2, &id).await.as_deref(), + Some(&*before) + ); + cfn.update_stack() .stack_name("profile-update-stack") .template_body(PROFILE_TEMPLATE.replace("PROFILE", "admin-profile")) @@ -208,3 +247,29 @@ async fn cfn_update_swaps_the_ec2_instance_profile_in_place() { Some("arn:aws:iam::123456789012:instance-profile/admin-profile") ); } + +#[tokio::test] +async fn cfn_rejects_an_instance_profile_that_is_not_an_instance_profile() { + // `{"Fn::GetAtt": ["Role", "Arn"]}` resolves to a role ARN, a common + // template mistake. Storing it made the stack un-updatable later, because + // the value could not be re-submitted through the validating handler. + let server = TestServer::start().await; + let cfn = server.cloudformation_client().await; + + cfn.create_stack() + .stack_name("bad-profile-stack") + .template_body(PROFILE_TEMPLATE.replace("PROFILE", "arn:aws:iam::123456789012:role/web")) + .capabilities(Capability::CapabilityIam) + .send() + .await + .expect("create_stack"); + + let described = cfn + .describe_stacks() + .stack_name("bad-profile-stack") + .send() + .await + .expect("describe_stacks"); + let stack = described.stacks().first().expect("stack present"); + assert_eq!(stack.stack_status().unwrap().as_str(), "CREATE_FAILED"); +} diff --git a/crates/fakecloud-ec2/src/service/instance.rs b/crates/fakecloud-ec2/src/service/instance.rs index 692bd639a..0a846b712 100644 --- a/crates/fakecloud-ec2/src/service/instance.rs +++ b/crates/fakecloud-ec2/src/service/instance.rs @@ -925,7 +925,10 @@ pub(crate) fn cfn_create_instance( if spec.iam_instance_profile_arn.is_some() || spec.iam_instance_profile_name.is_some() { let arn = spec.iam_instance_profile_arn.clone().unwrap_or_else(|| { let name = spec.iam_instance_profile_name.clone().unwrap_or_default(); - format!("arn:aws:iam::{account_id}:instance-profile/{name}") + format!( + "arn:{}:iam::{account_id}:instance-profile/{name}", + fakecloud_aws::arn::partition_for(region) + ) }); let assoc = super::rest::new_iam_profile_association(id.clone(), arn); state diff --git a/crates/fakecloud-ec2/src/service/rest/mod.rs b/crates/fakecloud-ec2/src/service/rest/mod.rs index b6f340ad8..7bbe3e9ea 100644 --- a/crates/fakecloud-ec2/src/service/rest/mod.rs +++ b/crates/fakecloud-ec2/src/service/rest/mod.rs @@ -16,8 +16,8 @@ pub(crate) use fakecloud_core::service::{AwsRequest, AwsResponse, AwsServiceErro pub(crate) use crate::service::Ec2Service; pub(crate) use crate::service_helpers::{ - association_not_found, aws_unique_id, filter_value_matches, gen_id, incorrect_instance_state, - incorrect_state, indexed_list, instance_not_found, invalid_parameter_value, + association_not_found, filter_value_matches, gen_id, incorrect_instance_state, incorrect_state, + indexed_list, instance_not_found, instance_profile_id_for, invalid_parameter_value, is_instance_profile_arn, is_instance_profile_name, malformed_instance_profile_arn, missing_parameter, parse_filters, require, require_struct, validate_enum, validate_int_range, validate_length, validate_max_results, Filter, @@ -137,8 +137,12 @@ pub(crate) fn iam_profile_arn(req: &AwsRequest) -> Result, AwsSer "Invalid IAM Instance Profile name: {name}" ))); } + // IAM mints the profile ARN in the region's partition, so a name-based + // association has to synthesize the same one or the ARNs never compare + // equal in cn-* / us-gov-* / us-iso*. return Ok(Some(format!( - "arn:aws:iam::{}:instance-profile/{name}", + "arn:{}:iam::{}:instance-profile/{name}", + fakecloud_aws::arn::partition_for(&req.region), req.account_id ))); } @@ -156,8 +160,8 @@ pub(crate) fn new_iam_profile_association( IamInstanceProfileAssociation { association_id: gen_id("iip-assoc"), instance_id, + iam_instance_profile_id: instance_profile_id_for(&profile_arn), iam_instance_profile_arn: profile_arn, - iam_instance_profile_id: aws_unique_id("AIPA"), state: "associated".to_string(), } } @@ -806,16 +810,13 @@ pub(crate) fn describe_iam_instance_profile_associations( // The terraform AWS provider reads an instance's profile by filtering on // `instance-id` and taking the first item, so honoring `Filter.N` is what // keeps it from picking up another instance's association. - let mut matching: Vec<&IamInstanceProfileAssociation> = state + // The backing map is a BTreeMap keyed by association id, so iterating it + // already yields a stable order. + let items: Vec = state .iam_instance_profile_associations .values() .filter(|a| wanted.is_empty() || wanted.contains(&a.association_id)) .filter(|a| iam_assoc_match(a, &filters)) - .collect(); - // The backing map is unordered; sort so repeated describes agree. - matching.sort_by(|a, b| a.association_id.cmp(&b.association_id)); - let items: Vec = matching - .into_iter() .map(|a| iam_profile_assoc_xml(a, &a.state)) .collect(); Ok(Ec2Service::respond( @@ -3004,6 +3005,50 @@ mod tests { ); } + #[test] + fn instance_profile_id_is_shared_by_every_association_with_that_profile() { + // AWS reports the *profile's* InstanceProfileId, so two instances on + // one profile report the same id and re-pointing at the same profile + // keeps it. A per-association random id made the new + // `iam-instance-profile.id` filter unable to select both. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-a"); + seed_instance(&svc, "i-b"); + let id_of = |xml: &str| { + xml.split("") + .nth(1) + .and_then(|s| s.split("").next()) + .expect("profile id") + .to_string() + }; + seed_instance(&svc, "i-c"); + let a = id_of(&associate_profile(&svc, "i-a", "web-role")); + let b = id_of(&associate_profile(&svc, "i-b", "web-role")); + assert_eq!(a, b); + assert_ne!(a, id_of(&associate_profile(&svc, "i-c", "other-role"))); + } + + #[test] + fn name_based_profile_arn_uses_the_regions_partition() { + // IAM mints the profile ARN in the region's partition; synthesizing + // `arn:aws:` in cn-* / us-gov-* would never compare equal to it. + let svc = Ec2Service::new(); + seed_instance(&svc, "i-123"); + let mut r = req( + "AssociateIamInstanceProfile", + &[ + ("InstanceId", "i-123"), + ("IamInstanceProfile.Name", "web-role"), + ], + ); + r.region = "cn-north-1".to_string(); + let out = body(associate_iam_instance_profile(&svc, &r).unwrap()); + assert!( + out.contains("arn:aws-cn:iam::000000000000:instance-profile/web-role"), + "{out}" + ); + } + #[test] fn associate_iam_instance_profile_rejects_a_terminated_instance() { // AWS associates a profile with a running or stopped instance only. diff --git a/crates/fakecloud-ec2/src/service_helpers.rs b/crates/fakecloud-ec2/src/service_helpers.rs index 6d5dc00bd..28ec79d2f 100644 --- a/crates/fakecloud-ec2/src/service_helpers.rs +++ b/crates/fakecloud-ec2/src/service_helpers.rs @@ -26,15 +26,6 @@ pub fn gen_id(prefix: &str) -> String { format!("{prefix}-{}", &hex[..17]) } -/// Generate an AWS unique id: a 4-char prefix (`AIPA`, `AIDA`, `AROA`, ...) -/// followed by 17 uppercase alphanumerics, with no separator — the shape IAM -/// mints instance-profile, user and role ids in. `gen_id`'s `-` form is the EC2 *resource* id shape and is wrong for this family. -pub fn aws_unique_id(prefix: &str) -> String { - let hex = uuid::Uuid::new_v4().simple().to_string().to_uppercase(); - format!("{prefix}{}", &hex[..17]) -} - /// `InvalidParameterValue` — the catch-all 400 for bad EC2 input. pub fn invalid_parameter_value(message: impl Into) -> AwsServiceError { AwsServiceError::aws_error( @@ -99,6 +90,30 @@ pub fn incorrect_state(message: impl Into) -> AwsServiceError { AwsServiceError::aws_error(StatusCode::BAD_REQUEST, "IncorrectState", message.into()) } +/// The `InstanceProfileId` reported for an instance profile, derived from its +/// ARN so every association with the same profile reports the same id (AWS +/// reports the profile's own id, which two instances on one profile share). +/// EC2 cannot read IAM's store — `fakecloud-ec2` does not depend on +/// `fakecloud-iam` — so the id is a stable function of the ARN rather than the +/// value IAM minted; the shape is AWS's (`AIPA` + 17 uppercase alphanumerics). +pub fn instance_profile_id_for(arn: &str) -> String { + // FNV-1a over the ARN, rendered in base36 and padded, so the id is stable + // across restarts and processes (a random or hash-map-seeded value 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); + } + const ALPHABET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"; + let mut suffix = String::with_capacity(17); + for i in 0..17 { + // Stir between characters so all 17 vary with the whole hash. + let shifted = hash.rotate_left((i * 5) as u32); + suffix.push(ALPHABET[(shifted % ALPHABET.len() as u64) as usize] as char); + } + format!("AIPA{suffix}") +} + /// `InvalidIamInstanceProfileArn.Malformed` (HTTP 400) — the supplied IAM /// instance-profile ARN is not a well-formed instance-profile ARN. pub fn malformed_instance_profile_arn(arn: &str) -> AwsServiceError { From cdb291a94ec612169f2086cb315dbf44dcc2155e Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 20 Sep 2026 16:53:09 -0300 Subject: [PATCH 4/8] test(rds): gate the k8s postgres test on a real query, not pg_isready `postgres_pod_exec_readiness_query_and_dump` failed with psql: error: connection to server on socket "/var/run/postgresql/.s.PGSQL.5432" failed: No such file or directory right after its pg_isready loop reported ready. The postgres image runs a temporary server on its own socket while initdb runs, then stops it and starts the real one, so pg_isready can answer yes against the temporary server and the socket is gone by the next exec. Wait for `psql -tAc 'SELECT 1'` to succeed instead, which is the only signal that outlives the restart, and reuse that result as the query assertion. --- crates/fakecloud-rds/tests/k8s_integration.rs | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/crates/fakecloud-rds/tests/k8s_integration.rs b/crates/fakecloud-rds/tests/k8s_integration.rs index 068a216c8..5bf481a8d 100644 --- a/crates/fakecloud-rds/tests/k8s_integration.rs +++ b/crates/fakecloud-rds/tests/k8s_integration.rs @@ -138,32 +138,38 @@ async fn postgres_pod_exec_readiness_query_and_dump() { .await .expect("pg pod Running"); - // Readiness via exec pg_isready (API-server path, no pod-IP routing). - let mut ready = false; + // Readiness via exec (API-server path, no pod-IP routing). `pg_isready` + // alone is not enough: the postgres image runs a temporary server on its + // own socket while initdb runs, then stops it and starts the real one, so + // pg_isready can answer yes and the very next command still fails with + // `connection to server on socket ... failed: No such file or directory`. + // Require the query itself to succeed, the only signal that outlives that + // restart. + let mut query = None; for _ in 0..60 { if let Ok(out) = c .exec(name, Some("db"), &["pg_isready", "-U", "postgres"]) .await { if out.success() { - ready = true; - break; + if let Ok(q) = c + .exec( + name, + Some("db"), + &["psql", "-U", "postgres", "-tAc", "SELECT 1"], + ) + .await + { + if q.success() { + query = Some(q); + break; + } + } } } tokio::time::sleep(Duration::from_millis(1000)).await; } - assert!(ready, "postgres did not become ready via pg_isready"); - - // A query through exec psql. - let q = c - .exec( - name, - Some("db"), - &["psql", "-U", "postgres", "-tAc", "SELECT 1"], - ) - .await - .expect("psql query"); - assert!(q.success(), "psql failed: {}", q.stderr); + let q = query.expect("postgres did not become ready (pg_isready + psql SELECT 1)"); assert!(q.stdout_str().contains('1')); // The dump path RDS uses: pg_dump via exec produces a non-empty dump. From 51f5f38e3847c368c9cfd8723f008d6a1d1882e8 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 20 Sep 2026 17:11:55 -0300 Subject: [PATCH 5/8] fix(iam): derive InstanceProfileId so EC2 and IAM report one id The previous commit derived EC2's rendered IamInstanceProfile.Id from the profile ARN, which made it stable and shared across instances, but IAM still minted its own InstanceProfileId at random. So GetInstanceProfile and DescribeInstances reported two different ids for the same profile, and a describe-instances filter on the id IAM handed out matched nothing. Move the derivation into fakecloud-aws as arn::unique_id_for(prefix, arn) -- the AWS unique-id shape, a 4-char family prefix plus 17 uppercase base32 characters -- and have both services call it. IAM derives from the ARN it just built, EC2 from the ARN on the association, so the two agree without EC2 reading IAM's store. Also report the last real error when the RDS k8s postgres readiness loop times out, rather than a fixed string, so a renamed container or an auth failure is not indistinguishable from slow startup. Tests: an e2e that creates the profile through IAM, attaches it through EC2, and asserts DescribeInstances reports IAM's own InstanceProfileId and that the iam-instance-profile.id filter selects the instance; plus unit coverage of the id's shape, stability and partition sensitivity. --- crates/fakecloud-aws/src/arn.rs | 54 +++++++++++++++ .../tests/ec2_instance_control_plane.rs | 67 +++++++++++++++++++ crates/fakecloud-ec2/src/service_helpers.rs | 27 ++------ .../src/iam_service/instance_profiles.rs | 24 ++++--- crates/fakecloud-rds/tests/k8s_integration.rs | 18 +++-- 5 files changed, 154 insertions(+), 36 deletions(-) diff --git a/crates/fakecloud-aws/src/arn.rs b/crates/fakecloud-aws/src/arn.rs index c4423814d..c6b76eed2 100644 --- a/crates/fakecloud-aws/src/arn.rs +++ b/crates/fakecloud-aws/src/arn.rs @@ -62,6 +62,35 @@ impl Arn { /// 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. +/// 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. +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}") +} + pub fn partition_for(region: &str) -> &'static str { if region.starts_with("cn-") { "aws-cn" @@ -140,6 +169,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"); diff --git a/crates/fakecloud-e2e/tests/ec2_instance_control_plane.rs b/crates/fakecloud-e2e/tests/ec2_instance_control_plane.rs index eef96577d..2618488a9 100644 --- a/crates/fakecloud-e2e/tests/ec2_instance_control_plane.rs +++ b/crates/fakecloud-e2e/tests/ec2_instance_control_plane.rs @@ -769,3 +769,70 @@ async fn describe_instances_reports_iam_instance_profile_round_trip() { "profile must be gone after disassociate" ); } + +#[tokio::test] +async fn instance_reports_the_iam_profile_id_that_iam_assigned() { + // EC2 renders IamInstanceProfile.Id on the instance and filters on it, and + // IAM reports InstanceProfileId for the same profile. They have to be the + // same value: EC2 cannot read IAM's store, so both derive it from the + // profile ARN rather than minting one each. + let s = TestServer::start().await; + let ec2 = s.ec2_client().await; + let iam = s.iam_client().await; + + let created = iam + .create_instance_profile() + .instance_profile_name("shared-profile") + .send() + .await + .unwrap(); + let profile = created.instance_profile().expect("instance profile"); + let iam_id = profile.instance_profile_id().to_string(); + let iam_arn = profile.arn().to_string(); + + let id = run(&ec2, 1, 1).await.remove(0); + ec2.associate_iam_instance_profile() + .instance_id(&id) + .iam_instance_profile( + aws_sdk_ec2::types::IamInstanceProfileSpecification::builder() + .arn(&iam_arn) + .build(), + ) + .send() + .await + .unwrap(); + + let desc = ec2 + .describe_instances() + .instance_ids(&id) + .send() + .await + .unwrap(); + let rendered = desc.reservations()[0].instances()[0] + .iam_instance_profile() + .expect("profile on instance"); + assert_eq!(rendered.arn(), Some(iam_arn.as_str())); + assert_eq!( + rendered.id(), + Some(iam_id.as_str()), + "EC2 and IAM must report one id for one profile" + ); + + // And the filter over that id selects the instance. + let filtered = ec2 + .describe_instances() + .filters( + aws_sdk_ec2::types::Filter::builder() + .name("iam-instance-profile.id") + .values(&iam_id) + .build(), + ) + .send() + .await + .unwrap(); + assert!(filtered + .reservations() + .iter() + .flat_map(|r| r.instances()) + .any(|i| i.instance_id() == Some(id.as_str()))); +} diff --git a/crates/fakecloud-ec2/src/service_helpers.rs b/crates/fakecloud-ec2/src/service_helpers.rs index 28ec79d2f..41002a029 100644 --- a/crates/fakecloud-ec2/src/service_helpers.rs +++ b/crates/fakecloud-ec2/src/service_helpers.rs @@ -90,28 +90,13 @@ pub fn incorrect_state(message: impl Into) -> AwsServiceError { AwsServiceError::aws_error(StatusCode::BAD_REQUEST, "IncorrectState", message.into()) } -/// The `InstanceProfileId` reported for an instance profile, derived from its -/// ARN so every association with the same profile reports the same id (AWS -/// reports the profile's own id, which two instances on one profile share). -/// EC2 cannot read IAM's store — `fakecloud-ec2` does not depend on -/// `fakecloud-iam` — so the id is a stable function of the ARN rather than the -/// value IAM minted; the shape is AWS's (`AIPA` + 17 uppercase alphanumerics). +/// The `InstanceProfileId` reported for an instance profile. AWS reports the +/// profile's own id, so every association with the same profile reports the +/// same value; EC2 cannot read IAM's store (`fakecloud-ec2` does not depend on +/// `fakecloud-iam`), so both services derive it from the profile ARN through +/// the same shared helper instead of minting it independently. pub fn instance_profile_id_for(arn: &str) -> String { - // FNV-1a over the ARN, rendered in base36 and padded, so the id is stable - // across restarts and processes (a random or hash-map-seeded value 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); - } - const ALPHABET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"; - let mut suffix = String::with_capacity(17); - for i in 0..17 { - // Stir between characters so all 17 vary with the whole hash. - let shifted = hash.rotate_left((i * 5) as u32); - suffix.push(ALPHABET[(shifted % ALPHABET.len() as u64) as usize] as char); - } - format!("AIPA{suffix}") + fakecloud_aws::arn::unique_id_for("AIPA", arn) } /// `InvalidIamInstanceProfileArn.Malformed` (HTTP 400) — the supplied IAM diff --git a/crates/fakecloud-iam/src/iam_service/instance_profiles.rs b/crates/fakecloud-iam/src/iam_service/instance_profiles.rs index 0c6db8ac0..7edd6c979 100644 --- a/crates/fakecloud-iam/src/iam_service/instance_profiles.rs +++ b/crates/fakecloud-iam/src/iam_service/instance_profiles.rs @@ -7,8 +7,8 @@ use fakecloud_core::validation::*; use crate::state::IamInstanceProfile; use super::{ - empty_response, generate_id, paginated_tags_response, parse_tag_keys, parse_tags, - partition_for_region, tags_xml, url_encode, validate_tags, validate_untag_keys, IamService, + empty_response, paginated_tags_response, parse_tag_keys, parse_tags, partition_for_region, + tags_xml, url_encode, validate_tags, validate_untag_keys, IamService, }; use fakecloud_core::query::required_param; @@ -39,15 +39,19 @@ impl IamService { } let partition = partition_for_region(&req.region); + let arn = format!( + "arn:{}:iam::{}:instance-profile{}{}", + partition, + state.account_id, + if path == "/" { "/" } else { &path }, + name + ); let ip = IamInstanceProfile { - instance_profile_id: format!("AIPA{}", generate_id()), - arn: format!( - "arn:{}:iam::{}:instance-profile{}{}", - partition, - state.account_id, - if path == "/" { "/" } else { &path }, - name - ), + // Derived from the ARN, not minted randomly, so EC2 reports the + // same InstanceProfileId on instances this profile is attached to + // (it resolves the profile by ARN and cannot read this store). + instance_profile_id: fakecloud_aws::arn::unique_id_for("AIPA", &arn), + arn, instance_profile_name: name.clone(), path, created_at: Utc::now(), diff --git a/crates/fakecloud-rds/tests/k8s_integration.rs b/crates/fakecloud-rds/tests/k8s_integration.rs index 5bf481a8d..98fff1aca 100644 --- a/crates/fakecloud-rds/tests/k8s_integration.rs +++ b/crates/fakecloud-rds/tests/k8s_integration.rs @@ -146,13 +146,16 @@ async fn postgres_pod_exec_readiness_query_and_dump() { // Require the query itself to succeed, the only signal that outlives that // restart. let mut query = None; + // Keep the last failure so a real regression (renamed container, bad auth, + // missing binary) reports its own error instead of a bare timeout. + let mut last_err = "no attempt completed".to_string(); for _ in 0..60 { - if let Ok(out) = c + match c .exec(name, Some("db"), &["pg_isready", "-U", "postgres"]) .await { - if out.success() { - if let Ok(q) = c + Ok(out) if out.success() => { + match c .exec( name, Some("db"), @@ -160,16 +163,21 @@ async fn postgres_pod_exec_readiness_query_and_dump() { ) .await { - if q.success() { + Ok(q) if q.success() => { query = Some(q); break; } + Ok(q) => last_err = format!("psql failed: {}", q.stderr), + Err(e) => last_err = format!("psql exec failed: {e}"), } } + Ok(out) => last_err = format!("pg_isready not ready: {}", out.stderr), + Err(e) => last_err = format!("pg_isready exec failed: {e}"), } tokio::time::sleep(Duration::from_millis(1000)).await; } - let q = query.expect("postgres did not become ready (pg_isready + psql SELECT 1)"); + let q = query + .unwrap_or_else(|| panic!("postgres did not become ready in 60s; last error: {last_err}")); assert!(q.stdout_str().contains('1')); // The dump path RDS uses: pg_dump via exec produces a non-empty dump. From 0e8ca74127f6070412cc6de5e53b05dd095cdc39 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 20 Sep 2026 18:38:15 -0300 Subject: [PATCH 6/8] fix(iam): one InstanceProfileId across IAM, CloudFormation, EC2 and IMDS The last commit made IAM and EC2 agree. Three gaps remained. The CloudFormation AWS::IAM::InstanceProfile provisioner is a third constructor and still minted a random id, 20 characters rather than the 21 AWS returns, and built the ARN with a hardcoded arn:aws:. A CFN-created profile attached to an instance therefore reproduced exactly the bug this PR fixes: one id from GetInstanceProfile, a different one from DescribeInstances. It now derives from the ARN like the others, in the request's partition. IMDS iam/info returned a fourth value, the 22-character constant AIPAFAKECLOUDINSTPROF0, matching neither AWS's shape nor the other three. It builds the profile ARN already, so derive from that. partition_for was missing us-isof-* and eu-isoe-*, which IAM's and STS's private copies both had, so in those regions EC2's synthesized ARN (arn:aws:) and IAM's (arn:aws-iso-f:) disagreed and the derived ids came out different. Add them, and have the two copies delegate, so one lookup serves the workspace. Also: restore partition_for's doc comment, which the previous commit accidentally left attached to unique_id_for; note on the helper that a derived id is a pure function of the ARN, so a delete-then-recreate returns the same id where AWS mints a fresh one; and note on the EC2 side that a name-addressed profile on a non-default path is the one case the two ARNs cannot be made to agree, since EC2 cannot resolve the path. Tests: a CFN stack profile whose id matches what DescribeInstances renders for an instance it is attached to, the iam/info id derived from its own ARN, and partition coverage for the two added regions. --- crates/fakecloud-aws/src/arn.rs | 17 ++++- .../src/resource_provisioner/iam.rs | 15 +++-- .../fakecloud-e2e/tests/cloudformation_iam.rs | 66 +++++++++++++++++++ crates/fakecloud-ec2/src/service/rest/mod.rs | 5 +- crates/fakecloud-ec2/src/service_helpers.rs | 4 +- .../fakecloud-iam/src/iam_service/helpers.rs | 18 +---- crates/fakecloud-iam/src/sts_service/mod.rs | 18 +---- crates/fakecloud-server/src/imds.rs | 36 ++++++++-- 8 files changed, 133 insertions(+), 46 deletions(-) diff --git a/crates/fakecloud-aws/src/arn.rs b/crates/fakecloud-aws/src/arn.rs index c6b76eed2..0057c09eb 100644 --- a/crates/fakecloud-aws/src/arn.rs +++ b/crates/fakecloud-aws/src/arn.rs @@ -59,9 +59,6 @@ impl Arn { } } -/// 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. /// 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 @@ -71,6 +68,11 @@ impl Arn { /// 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). @@ -91,6 +93,9 @@ pub fn unique_id_for(prefix: &str, arn: &str) -> String { 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 and the +/// isolated regions emit the correct partition prefix. pub fn partition_for(region: &str) -> &'static str { if region.starts_with("cn-") { "aws-cn" @@ -100,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" } @@ -203,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"); } } diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/iam.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/iam.rs index 621c7fe24..b3b05dd0d 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/iam.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/iam.rs @@ -951,13 +951,16 @@ impl ResourceProvisioner { } } let arn = format!( - "arn:aws:iam::{}:instance-profile{}{}", - state.account_id, path, name - ); - let id = format!( - "AIPA{}", - &Uuid::new_v4().to_string().replace('-', "").to_uppercase()[..16] + "arn:{}:iam::{}:instance-profile{}{}", + fakecloud_aws::arn::partition_for(&self.region), + state.account_id, + path, + name ); + // Derived from the ARN, like the IAM service's own CreateInstanceProfile + // and the id EC2 renders on instances, so all three agree on one + // InstanceProfileId for one profile. + let id = fakecloud_aws::arn::unique_id_for("AIPA", &arn); state.instance_profiles.insert( name.clone(), IamInstanceProfile { diff --git a/crates/fakecloud-e2e/tests/cloudformation_iam.rs b/crates/fakecloud-e2e/tests/cloudformation_iam.rs index b8f861a9f..dab4cea57 100644 --- a/crates/fakecloud-e2e/tests/cloudformation_iam.rs +++ b/crates/fakecloud-e2e/tests/cloudformation_iam.rs @@ -152,3 +152,69 @@ async fn cfn_provisions_iam_user_group_policy_etc() { let after_user = iam.get_user().user_name("cfn-alice").send().await; assert!(after_user.is_err(), "user should be gone after delete"); } + +#[tokio::test] +async fn cfn_instance_profile_id_matches_the_one_ec2_renders() { + // The CloudFormation provisioner is a third constructor of an instance + // profile, alongside IAM's CreateInstanceProfile and EC2's association + // render. It used to mint a random 20-character id, so a CFN-created + // profile attached to an instance reported one id through GetInstanceProfile + // and a different one through DescribeInstances. + let server = TestServer::start().await; + let cfn = server.cloudformation_client().await; + let iam = server.iam_client().await; + let ec2 = server.ec2_client().await; + + cfn.create_stack() + .stack_name("profile-id-stack") + .template_body(TEMPLATE) + .capabilities(Capability::CapabilityNamedIam) + .on_failure(OnFailure::Rollback) + .send() + .await + .expect("create_stack"); + + let profile = iam + .get_instance_profile() + .instance_profile_name("cfn-ec2-profile") + .send() + .await + .expect("get_instance_profile"); + let profile = profile.instance_profile().expect("instance profile"); + let iam_id = profile.instance_profile_id().to_string(); + assert_eq!(iam_id.len(), 21, "AWS ids are 21 characters: {iam_id}"); + + let instance = ec2 + .run_instances() + .image_id("ami-0123456789abcdef0") + .min_count(1) + .max_count(1) + .send() + .await + .expect("run_instances"); + let instance_id = instance.instances()[0] + .instance_id() + .expect("instance id") + .to_string(); + ec2.associate_iam_instance_profile() + .instance_id(&instance_id) + .iam_instance_profile( + aws_sdk_ec2::types::IamInstanceProfileSpecification::builder() + .arn(profile.arn()) + .build(), + ) + .send() + .await + .expect("associate_iam_instance_profile"); + + let desc = ec2 + .describe_instances() + .instance_ids(&instance_id) + .send() + .await + .expect("describe_instances"); + let rendered = desc.reservations()[0].instances()[0] + .iam_instance_profile() + .expect("profile on instance"); + assert_eq!(rendered.id(), Some(iam_id.as_str())); +} diff --git a/crates/fakecloud-ec2/src/service/rest/mod.rs b/crates/fakecloud-ec2/src/service/rest/mod.rs index 7bbe3e9ea..8637d7a6b 100644 --- a/crates/fakecloud-ec2/src/service/rest/mod.rs +++ b/crates/fakecloud-ec2/src/service/rest/mod.rs @@ -139,7 +139,10 @@ pub(crate) fn iam_profile_arn(req: &AwsRequest) -> Result, AwsSer } // IAM mints the profile ARN in the region's partition, so a name-based // association has to synthesize the same one or the ARNs never compare - // equal in cn-* / us-gov-* / us-iso*. + // equal outside the default partition. The one case this cannot match + // is a profile created under a non-default path: its IAM ARN carries + // that path, and EC2 has no way to resolve a bare name to it (this + // crate does not depend on fakecloud-iam). Pass the ARN for those. return Ok(Some(format!( "arn:{}:iam::{}:instance-profile/{name}", fakecloud_aws::arn::partition_for(&req.region), diff --git a/crates/fakecloud-ec2/src/service_helpers.rs b/crates/fakecloud-ec2/src/service_helpers.rs index 41002a029..c8ada6f00 100644 --- a/crates/fakecloud-ec2/src/service_helpers.rs +++ b/crates/fakecloud-ec2/src/service_helpers.rs @@ -94,7 +94,9 @@ pub fn incorrect_state(message: impl Into) -> AwsServiceError { /// profile's own id, so every association with the same profile reports the /// same value; EC2 cannot read IAM's store (`fakecloud-ec2` does not depend on /// `fakecloud-iam`), so both services derive it from the profile ARN through -/// the same shared helper instead of minting it independently. +/// the same shared helper instead of minting it independently. That holds +/// whenever the two agree on the ARN: every profile addressed by ARN, and +/// every name-addressed profile on the default path. pub fn instance_profile_id_for(arn: &str) -> String { fakecloud_aws::arn::unique_id_for("AIPA", arn) } diff --git a/crates/fakecloud-iam/src/iam_service/helpers.rs b/crates/fakecloud-iam/src/iam_service/helpers.rs index f56e51055..2e13ded94 100644 --- a/crates/fakecloud-iam/src/iam_service/helpers.rs +++ b/crates/fakecloud-iam/src/iam_service/helpers.rs @@ -2,21 +2,9 @@ use super::*; /// Get the AWS partition from a region string. pub(crate) fn partition_for_region(region: &str) -> &str { - if region.starts_with("cn-") { - "aws-cn" - } else if region.starts_with("us-gov-") { - "aws-us-gov" - } else if region.starts_with("us-iso-") { - "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" - } + // One lookup for the whole workspace: an ARN synthesized here has to match + // one synthesized by another service for the same resource. + fakecloud_aws::arn::partition_for(region) } /// Actions on the IAM service that mutate state. Kept in sync with the diff --git a/crates/fakecloud-iam/src/sts_service/mod.rs b/crates/fakecloud-iam/src/sts_service/mod.rs index 8e4873541..f54ef0542 100644 --- a/crates/fakecloud-iam/src/sts_service/mod.rs +++ b/crates/fakecloud-iam/src/sts_service/mod.rs @@ -333,21 +333,9 @@ pub(super) fn sts_issuer_url(region: &str) -> String { /// Get the AWS partition from a region string. fn partition_for_region(region: &str) -> &str { - if region.starts_with("cn-") { - "aws-cn" - } else if region.starts_with("us-gov-") { - "aws-us-gov" - } else if region.starts_with("us-iso-") { - "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" - } + // One lookup for the whole workspace: an ARN synthesized here has to match + // one synthesized by another service for the same resource. + fakecloud_aws::arn::partition_for(region) } /// Collect session policies from the STS request parameters. diff --git a/crates/fakecloud-server/src/imds.rs b/crates/fakecloud-server/src/imds.rs index 02b762748..773d2eb95 100644 --- a/crates/fakecloud-server/src/imds.rs +++ b/crates/fakecloud-server/src/imds.rs @@ -240,14 +240,20 @@ fn security_credentials(ctx: &ImdsContext, role: &str) -> Response { /// `iam/info` -- the instance profile association. fn iam_info(ctx: &ImdsContext) -> Response { let creds = ctx.credentials(); + let profile_arn = format!( + "arn:{}:iam::{}:instance-profile/{}", + ctx.partition(), + ctx.account_id, + ctx.role_name() + ); Json(serde_json::json!({ "Code": "Success", "LastUpdated": creds.issued_at_iso8601(), - "InstanceProfileArn": format!( - "arn:{}:iam::{}:instance-profile/{}", - ctx.partition(), ctx.account_id, ctx.role_name() - ), - "InstanceProfileId": "AIPAFAKECLOUDINSTPROF0", + "InstanceProfileArn": profile_arn, + // Derived from the ARN, so IMDS, IAM and DescribeInstances all report + // one InstanceProfileId for one profile (and its 21-character AWS + // shape, which the old constant was not). + "InstanceProfileId": fakecloud_aws::arn::unique_id_for("AIPA", &profile_arn), })) .into_response() } @@ -306,6 +312,26 @@ mod tests { assert_eq!(ctx("eu-west-2", "x").availability_zone(), "eu-west-2a"); } + #[test] + fn iam_info_profile_id_is_derived_from_the_profile_arn() { + // It was a 22-character constant, so it matched neither AWS's shape nor + // the id IAM and DescribeInstances report for the same profile. + let c = ctx("us-east-1", "arn:aws:iam::123456789012:role/app-role"); + let arn = "arn:aws:iam::123456789012:instance-profile/app-role"; + let id = fakecloud_aws::arn::unique_id_for("AIPA", arn); + assert_eq!(id.len(), 21, "{id}"); + + let body = iam_info(&c).into_body(); + let bytes = tokio::runtime::Builder::new_current_thread() + .build() + .unwrap() + .block_on(axum::body::to_bytes(body, usize::MAX)) + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(json["InstanceProfileArn"], arn); + assert_eq!(json["InstanceProfileId"], id); + } + #[test] fn partition_follows_role_arn() { assert_eq!( From 5b1855b1fc18487da2af86e673b16c75bc6ac551 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 20 Sep 2026 20:05:50 -0300 Subject: [PATCH 7/8] fix(cloudformation): one partition and one profile ARN across the stack Widening arn::partition_for in the last commit exposed three places that had their own answer, and the id invariant still had a hole. IAM's policy validator rejected any partition outside a hardcoded five, so in us-isof-* / eu-isoe-* a policy naming an ARN fakecloud itself had just minted failed with MalformedPolicyDocument. Add the two partitions. CloudFormation's AWS::Partition resolver knew only cn- and us-gov-, and the IAM provisioner hardcoded arn:aws: for roles, policies, users, groups and OIDC providers while the instance profile had just become partition-aware. In an isolated region that left one stack holding ARNs in two partitions, and an Fn::Sub over AWS::Partition resolving to a profile that does not exist. Route both through arn::partition_for, and accept any partition where the Roles[] parser strips a role ARN. `Ref` on an AWS::IAM::InstanceProfile resolves to the profile NAME, and only IAM knows the Path that name's ARN carries, so a profile created with Path /app/ was stored at instance-profile/app/p while the instance rendered instance-profile/p -- two ARNs and two ids for one profile, which is the divergence this PR set out to remove, reachable from a template with no ARN to pass instead. The provisioner now resolves the name through IAM on both the create and the update path, falling back to a name-addressed association when IAM holds no such profile. IMDS iam/info synthesized its profile ARN from the credentials role name, so it agreed with IAM only for a profile named after the role on the default path. Prefer the ARN of a profile IAM actually holds for that role. Tests: a stack whose pathed profile has one ARN and one id on the instance and in GetInstanceProfile; iam/info preferring the stored profile's ARN. --- .../src/resource_provisioner/ec2.rs | 29 ++++++ .../src/resource_provisioner/iam.rs | 33 ++++--- .../src/template/mod.rs | 18 ++-- .../fakecloud-e2e/tests/cloudformation_iam.rs | 89 +++++++++++++++++++ crates/fakecloud-iam/src/policy_validation.rs | 12 ++- crates/fakecloud-server/src/imds.rs | 75 ++++++++++++++-- 6 files changed, 227 insertions(+), 29 deletions(-) diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs index ae548fc3f..ffa64dc8e 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs @@ -520,6 +520,13 @@ impl ResourceProvisioner { // 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), @@ -667,6 +674,21 @@ impl ResourceProvisioner { 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 { + 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 @@ -675,7 +697,14 @@ impl ResourceProvisioner { 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. 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))); diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/iam.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/iam.rs index b3b05dd0d..776a11bfd 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/iam.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/iam.rs @@ -65,7 +65,8 @@ impl ResourceProvisioner { &Uuid::new_v4().to_string().replace('-', "").to_uppercase()[..16] ); let arn = format!( - "arn:aws:iam::{}:role{}{}", + "arn:{}:iam::{}:role{}{}", + fakecloud_aws::arn::partition_for(&self.region), state.account_id, if path == "/" { "/" } else { path }, role_name @@ -256,7 +257,8 @@ impl ResourceProvisioner { &Uuid::new_v4().to_string().replace('-', "").to_uppercase()[..16] ); let arn = format!( - "arn:aws:iam::{}:policy{}{}", + "arn:{}:iam::{}:policy{}{}", + fakecloud_aws::arn::partition_for(&self.region), state.account_id, if path == "/" { "/" } else { path }, policy_name @@ -377,8 +379,11 @@ impl ResourceProvisioner { return Err(format!("User {user_name} already exists")); } let arn = format!( - "arn:aws:iam::{}:user{}{}", - state.account_id, path, user_name + "arn:{}:iam::{}:user{}{}", + fakecloud_aws::arn::partition_for(&self.region), + state.account_id, + path, + user_name ); let user_id = format!( "AIDA{}", @@ -556,8 +561,11 @@ impl ResourceProvisioner { return Err(format!("Group {group_name} already exists")); } let arn = format!( - "arn:aws:iam::{}:group{}{}", - state.account_id, path, group_name + "arn:{}:iam::{}:group{}{}", + fakecloud_aws::arn::partition_for(&self.region), + state.account_id, + path, + group_name ); let group_id = format!( "AGPA{}", @@ -700,7 +708,8 @@ impl ResourceProvisioner { let mut accounts = self.iam_state.write(); let state = accounts.get_or_create(&self.account_id); let arn = format!( - "arn:aws:iam::{}:policy{}{}", + "arn:{}:iam::{}:policy{}{}", + fakecloud_aws::arn::partition_for(&self.region), state.account_id, if path == "/" { "/" } else { path.as_str() }, policy_name @@ -921,8 +930,8 @@ impl ResourceProvisioner { arr.iter() .filter_map(|r| r.as_str()) .map(|s| { - if let Some(rest) = s.strip_prefix("arn:aws:iam::") { - rest.split(":role/") + if s.starts_with("arn:") { + s.split(":role/") .nth(1) .map(|name| name.to_string()) .unwrap_or_else(|| s.to_string()) @@ -1018,8 +1027,10 @@ impl ResourceProvisioner { .trim_start_matches("http://") .to_string(); let arn = format!( - "arn:aws:iam::{}:oidc-provider/{}", - self.account_id, url_path + "arn:{}:iam::{}:oidc-provider/{}", + fakecloud_aws::arn::partition_for(&self.region), + self.account_id, + url_path ); let provider = OidcProvider { arn: arn.clone(), diff --git a/crates/fakecloud-cloudformation/src/template/mod.rs b/crates/fakecloud-cloudformation/src/template/mod.rs index d64134865..578bdcd4e 100644 --- a/crates/fakecloud-cloudformation/src/template/mod.rs +++ b/crates/fakecloud-cloudformation/src/template/mod.rs @@ -59,18 +59,14 @@ const PSEUDO_REFS: &[&str] = &[ type Mappings = BTreeMap>>; -/// Map an AWS region to its IAM/ARN partition. China regions land on -/// `aws-cn`, GovCloud on `aws-us-gov`, everything else on `aws`. Used -/// by both `AWS::Partition` resolution and the URL-suffix derivation -/// below so partition decisions stay consistent. +/// Map an AWS region to its IAM/ARN partition, for `AWS::Partition` and the +/// URL-suffix derivation below. One lookup for the whole workspace, so a +/// template resolving `AWS::Partition` gets the partition the services +/// actually mint their ARNs in (this used to know only cn- and us-gov-, so an +/// `Fn::Sub` over `AWS::Partition` in an isolated region built an ARN that +/// matched nothing). pub(crate) fn partition_for_region(region: &str) -> &'static str { - if region.starts_with("cn-") { - "aws-cn" - } else if region.starts_with("us-gov-") { - "aws-us-gov" - } else { - "aws" - } + fakecloud_aws::arn::partition_for(region) } /// Map an AWS region to its DNS URL suffix. China regions use diff --git a/crates/fakecloud-e2e/tests/cloudformation_iam.rs b/crates/fakecloud-e2e/tests/cloudformation_iam.rs index dab4cea57..6af6c937b 100644 --- a/crates/fakecloud-e2e/tests/cloudformation_iam.rs +++ b/crates/fakecloud-e2e/tests/cloudformation_iam.rs @@ -218,3 +218,92 @@ async fn cfn_instance_profile_id_matches_the_one_ec2_renders() { .expect("profile on instance"); assert_eq!(rendered.id(), Some(iam_id.as_str())); } + +const PATHED_PROFILE_TEMPLATE: &str = r#"{ + "AWSTemplateFormatVersion": "2010-09-09", + "Resources": { + "ServerProfile": { + "Type": "AWS::IAM::InstanceProfile", + "Properties": { + "InstanceProfileName": "pathed-profile", + "Path": "/app/" + } + }, + "Server": { + "Type": "AWS::EC2::Instance", + "Properties": { + "ImageId": "ami-0123456789abcdef0", + "InstanceType": "t3.micro", + "IamInstanceProfile": {"Ref": "ServerProfile"} + } + } + }, + "Outputs": { + "InstanceRef": {"Value": {"Ref": "Server"}} + } +}"#; + +#[tokio::test] +async fn cfn_instance_gets_the_pathed_profile_arn_iam_stored() { + // `Ref` on an AWS::IAM::InstanceProfile resolves to the profile NAME, and + // EC2 alone cannot know the Path that name's ARN carries, so it used to + // build `instance-profile/pathed-profile` while IAM held + // `instance-profile/app/pathed-profile` -- two ARNs and two ids for one + // profile. The provisioner resolves the name through IAM instead. + let server = TestServer::start().await; + let cfn = server.cloudformation_client().await; + let iam = server.iam_client().await; + let ec2 = server.ec2_client().await; + + cfn.create_stack() + .stack_name("pathed-profile-stack") + .template_body(PATHED_PROFILE_TEMPLATE) + .capabilities(Capability::CapabilityNamedIam) + .on_failure(OnFailure::Rollback) + .send() + .await + .expect("create_stack"); + + let described = cfn + .describe_stacks() + .stack_name("pathed-profile-stack") + .send() + .await + .expect("describe_stacks"); + let stack = described.stacks().first().expect("stack present"); + assert_eq!(stack.stack_status().unwrap().as_str(), "CREATE_COMPLETE"); + let instance_id = stack + .outputs() + .iter() + .find(|o| o.output_key() == Some("InstanceRef")) + .and_then(|o| o.output_value()) + .expect("InstanceRef") + .to_string(); + + let profile = iam + .get_instance_profile() + .instance_profile_name("pathed-profile") + .send() + .await + .expect("get_instance_profile"); + let profile = profile.instance_profile().expect("instance profile"); + assert!( + profile + .arn() + .contains("instance-profile/app/pathed-profile"), + "{}", + profile.arn() + ); + + let desc = ec2 + .describe_instances() + .instance_ids(&instance_id) + .send() + .await + .expect("describe_instances"); + let rendered = desc.reservations()[0].instances()[0] + .iam_instance_profile() + .expect("profile on instance"); + assert_eq!(rendered.arn(), Some(profile.arn())); + assert_eq!(rendered.id(), Some(profile.instance_profile_id())); +} diff --git a/crates/fakecloud-iam/src/policy_validation.rs b/crates/fakecloud-iam/src/policy_validation.rs index af5a95925..8307537ce 100644 --- a/crates/fakecloud-iam/src/policy_validation.rs +++ b/crates/fakecloud-iam/src/policy_validation.rs @@ -32,7 +32,17 @@ const CONDITION_OPERATORS: &[&str] = &[ ]; /// Valid AWS partitions for resource ARN validation. -const VALID_PARTITIONS: &[&str] = &["aws", "aws-cn", "aws-us-gov", "aws-iso", "aws-iso-b"]; +const VALID_PARTITIONS: &[&str] = &[ + "aws", + "aws-cn", + "aws-us-gov", + "aws-iso", + "aws-iso-b", + // The isolated partitions fakecloud itself mints ARNs in; rejecting them + // here made a policy fail on an ARN the emulator had just handed out. + "aws-iso-e", + "aws-iso-f", +]; /// Valid IAM resource path prefixes. const IAM_RESOURCE_PREFIXES: &[&str] = &[ diff --git a/crates/fakecloud-server/src/imds.rs b/crates/fakecloud-server/src/imds.rs index 773d2eb95..f806bdd14 100644 --- a/crates/fakecloud-server/src/imds.rs +++ b/crates/fakecloud-server/src/imds.rs @@ -81,6 +81,24 @@ impl ImdsContext { /// Partition derived from the role ARN, so the instance-profile ARN matches /// the partition of the credentials' assumed-role principal. + /// The ARN of an instance profile in IAM that carries this instance's role, + /// preferring one named after the role. `None` when IAM holds none. + fn instance_profile_arn_for_role(&self) -> Option { + let role = self.role_name(); + let accounts = self.iam.read(); + let state = accounts.get(&self.account_id)?; + let carrying: Vec<_> = state + .instance_profiles + .values() + .filter(|p| p.roles.iter().any(|r| r == role)) + .collect(); + carrying + .iter() + .find(|p| p.instance_profile_name == role) + .or_else(|| carrying.first()) + .map(|p| p.arn.clone()) + } + fn partition(&self) -> &str { partition_of(&self.role_arn) } @@ -240,12 +258,18 @@ fn security_credentials(ctx: &ImdsContext, role: &str) -> Response { /// `iam/info` -- the instance profile association. fn iam_info(ctx: &ImdsContext) -> Response { let creds = ctx.credentials(); - let profile_arn = format!( - "arn:{}:iam::{}:instance-profile/{}", - ctx.partition(), - ctx.account_id, - ctx.role_name() - ); + // Prefer the ARN of a real instance profile carrying this role, so the id + // below matches what GetInstanceProfile and DescribeInstances report for + // it (the profile's Path is part of its ARN, and only IAM knows it). Fall + // back to a profile named after the role when IAM holds none. + let profile_arn = ctx.instance_profile_arn_for_role().unwrap_or_else(|| { + format!( + "arn:{}:iam::{}:instance-profile/{}", + ctx.partition(), + ctx.account_id, + ctx.role_name() + ) + }); Json(serde_json::json!({ "Code": "Success", "LastUpdated": creds.issued_at_iso8601(), @@ -332,6 +356,45 @@ mod tests { assert_eq!(json["InstanceProfileId"], id); } + #[test] + fn iam_info_prefers_a_real_instance_profile_carrying_the_role() { + // The synthesized ARN assumes a profile named after the role on the + // default path. When IAM holds the real profile, its ARN (Path and + // all) is what IAM and DescribeInstances report, so use that one. + let c = ctx("us-east-1", "arn:aws:iam::123456789012:role/app-role"); + let real_arn = { + let mut accounts = c.iam.write(); + let state = accounts.get_or_create("123456789012"); + let arn = "arn:aws:iam::123456789012:instance-profile/svc/app-profile".to_string(); + state.instance_profiles.insert( + "app-profile".to_string(), + fakecloud_iam::IamInstanceProfile { + instance_profile_name: "app-profile".to_string(), + instance_profile_id: fakecloud_aws::arn::unique_id_for("AIPA", &arn), + arn: arn.clone(), + path: "/svc/".to_string(), + created_at: chrono::Utc::now(), + roles: vec!["app-role".to_string()], + tags: Vec::new(), + }, + ); + arn + }; + + let body = iam_info(&c).into_body(); + let bytes = tokio::runtime::Builder::new_current_thread() + .build() + .unwrap() + .block_on(axum::body::to_bytes(body, usize::MAX)) + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(json["InstanceProfileArn"], real_arn); + assert_eq!( + json["InstanceProfileId"], + fakecloud_aws::arn::unique_id_for("AIPA", &real_arn) + ); + } + #[test] fn partition_follows_role_arn() { assert_eq!( From 0c04c6049e14c187f0666c611ae505f5a6cff506 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 20 Sep 2026 21:25:40 -0300 Subject: [PATCH 8/8] fix(cloudformation): resolve AWS::Partition and AWS::URLSuffix per region Widening the partition lookup reached CreateStack but not the change-set path, and left the two pseudo-parameters disagreeing. CreateChangeSet and ExecuteChangeSet both seeded AWS::Partition with a literal "aws" and AWS::URLSuffix with "amazonaws.com". So a template resolving AWS::Partition previewed one value and created another, and a stack provisioned through ExecuteChangeSet in a non-default partition minted ARNs in the wrong one -- the divergence this branch just removed from the IAM provisioner. Both now resolve from the request's region. AWS::URLSuffix knew only cn-, so an isolated region got the newly correct AWS::Partition next to amazonaws.com, and an Fn::Sub building an endpoint from the pair produced a host that matches nothing. Derive it from the same partition lookup: c2s.ic.gov, sc2s.sgov.gov, cloud.adc-e.uk and csp.hci.ic.gov for the four isolated partitions. The don't-churn-the-association guard compared on the profile name only when the request was name-addressed on the wire, which stopped being the case once a name is resolved to IAM's ARN. A template naming a profile IAM did not yet hold got a path-less ARN stored, and the next unrelated UpdateStack then saw IAM's pathed ARN as a change and retired the association id. Compare on the profile name whenever the template addressed the profile by name. IMDS picked any profile carrying the role when several do, which AWS allows and IMDS cannot disambiguate without the EC2 association. Answer only when the choice is unambiguous -- a profile named after the role, or the single one carrying it -- and otherwise keep the synthesized ARN. Also move partition()'s doc comment back onto partition(), where the last commit's insertion had displaced it. Tests: AWS::Partition and AWS::URLSuffix agree across all seven partitions. --- crates/fakecloud-cloudformation/src/extras.rs | 8 ++--- .../src/resource_provisioner/ec2.rs | 10 ++++-- .../src/template/mod.rs | 36 +++++++++++++++---- crates/fakecloud-server/src/imds.rs | 14 +++++--- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/crates/fakecloud-cloudformation/src/extras.rs b/crates/fakecloud-cloudformation/src/extras.rs index 2a37030a6..528d8f48e 100644 --- a/crates/fakecloud-cloudformation/src/extras.rs +++ b/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 diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs index ffa64dc8e..9e745c7a6 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/ec2.rs @@ -700,6 +700,11 @@ impl ResourceProvisioner { // 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() @@ -728,10 +733,9 @@ impl ResourceProvisioner { // 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 - || (key == "IamInstanceProfile.Name" - && arn.rsplit('/').next() == Some(value.as_str())) + arn == value || (by_name && profile_name(arn) == profile_name(&value)) }); if unchanged { return Ok(()); diff --git a/crates/fakecloud-cloudformation/src/template/mod.rs b/crates/fakecloud-cloudformation/src/template/mod.rs index 578bdcd4e..42e8c8d48 100644 --- a/crates/fakecloud-cloudformation/src/template/mod.rs +++ b/crates/fakecloud-cloudformation/src/template/mod.rs @@ -70,13 +70,18 @@ pub(crate) fn partition_for_region(region: &str) -> &'static str { } /// Map an AWS region to its DNS URL suffix. China regions use -/// `amazonaws.com.cn`; every other partition (commercial + GovCloud) -/// uses `amazonaws.com`, matching the real CFN `AWS::URLSuffix`. +/// `amazonaws.com.cn`, each isolated partition its own suffix, and commercial +/// plus GovCloud `amazonaws.com`, matching the real CFN `AWS::URLSuffix`. +/// Derived from the same partition lookup as `AWS::Partition`, so the two +/// pseudo-parameters never disagree about which partition a region is in. pub(crate) fn url_suffix_for_region(region: &str) -> &'static str { - if region.starts_with("cn-") { - "amazonaws.com.cn" - } else { - "amazonaws.com" + match partition_for_region(region) { + "aws-cn" => "amazonaws.com.cn", + "aws-iso" => "c2s.ic.gov", + "aws-iso-b" => "sc2s.sgov.gov", + "aws-iso-e" => "cloud.adc-e.uk", + "aws-iso-f" => "csp.hci.ic.gov", + _ => "amazonaws.com", } } @@ -84,6 +89,25 @@ pub(crate) fn url_suffix_for_region(region: &str) -> &'static str { mod tests { use super::*; + #[test] + fn partition_and_url_suffix_agree_on_the_partition() { + // AWS::Partition knew only cn-/us-gov-, so an isolated region got + // `aws` next to a suffix that matched nothing. Both now come from one + // lookup. + for (region, partition, suffix) in [ + ("us-east-1", "aws", "amazonaws.com"), + ("cn-north-1", "aws-cn", "amazonaws.com.cn"), + ("us-gov-west-1", "aws-us-gov", "amazonaws.com"), + ("us-iso-east-1", "aws-iso", "c2s.ic.gov"), + ("us-isob-east-1", "aws-iso-b", "sc2s.sgov.gov"), + ("us-isof-south-1", "aws-iso-f", "csp.hci.ic.gov"), + ("eu-isoe-west-1", "aws-iso-e", "cloud.adc-e.uk"), + ] { + assert_eq!(partition_for_region(region), partition, "{region}"); + assert_eq!(url_suffix_for_region(region), suffix, "{region}"); + } + } + #[test] fn parse_json_template() { let template = r#"{ diff --git a/crates/fakecloud-server/src/imds.rs b/crates/fakecloud-server/src/imds.rs index f806bdd14..933661b1a 100644 --- a/crates/fakecloud-server/src/imds.rs +++ b/crates/fakecloud-server/src/imds.rs @@ -79,10 +79,12 @@ impl ImdsContext { format!("{}a", self.region) } - /// Partition derived from the role ARN, so the instance-profile ARN matches - /// the partition of the credentials' assumed-role principal. - /// The ARN of an instance profile in IAM that carries this instance's role, - /// preferring one named after the role. `None` when IAM holds none. + /// The ARN of the instance profile in IAM that carries this instance's + /// role. AWS lets many profiles carry one role, and IMDS has no view of + /// the EC2 association, so this answers only when the choice is + /// unambiguous: a profile named after the role, or the single profile + /// carrying it. `None` otherwise, leaving the caller its synthesized ARN + /// rather than naming an arbitrary profile. fn instance_profile_arn_for_role(&self) -> Option { let role = self.role_name(); let accounts = self.iam.read(); @@ -95,10 +97,12 @@ impl ImdsContext { carrying .iter() .find(|p| p.instance_profile_name == role) - .or_else(|| carrying.first()) + .or(carrying.first().filter(|_| carrying.len() == 1)) .map(|p| p.arn.clone()) } + /// Partition derived from the role ARN, so the instance-profile ARN matches + /// the partition of the credentials' assumed-role principal. fn partition(&self) -> &str { partition_of(&self.role_arn) }