From a951ab79b70f2031a4b5b3e2c4fc6c065ca52b79 Mon Sep 17 00:00:00 2001 From: James Price Date: Tue, 8 Sep 2026 16:04:04 +0100 Subject: [PATCH] fix(cloudformation): keep CloudFront CustomErrorResponses through provisioning Two fidelity mismatches on the same block, both of which left a CFN-provisioned SPA distribution with no deep-link fallback. Translating the CFN block ran each rule through `serde_json::from_value` into the wire struct and dropped anything that failed. CloudFormation types `ResponseCode` as an Integer while the CloudFront API carries it as a string, so every rule failed to deserialize and the distribution came out with `Quantity: 0`. Map the fields explicitly instead, coercing the number and accepting the quoted form YAML templates and resolved intrinsics produce. A rule missing the required `ErrorCode` is skipped rather than defaulted to a code that would match nothing. `ErrorCachingMinTTL` then still failed to round-trip: `rename_all = "PascalCase"` renders it `ErrorCachingMinTtl`, so the field was mis-named on the wire and dropped on parse from real SDK requests. Pin the AWS spelling, as `WebACLId` in the same file already does. --- .../src/resource_provisioner/cloudfront.rs | 120 +++++++++++++++++- crates/fakecloud-cloudfront/src/model.rs | 10 +- .../tests/cloudformation_cloudfront.rs | 91 +++++++++++++ 3 files changed, 219 insertions(+), 2 deletions(-) diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/cloudfront.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/cloudfront.rs index 86d0a0799..3ff38f5f4 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/cloudfront.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/cloudfront.rs @@ -87,13 +87,32 @@ impl ResourceProvisioner { } }); // CustomErrorResponses: flat [{ ErrorCode, ... }, ...]. + // + // Mapped field by field rather than through `serde_json::from_value`: + // CloudFormation types `ResponseCode` as an Integer while the CloudFront + // API carries it as a string, so deserializing the CFN shape into the + // wire struct fails and every rule was silently dropped -- a SPA + // distribution came out with `Quantity: 0` and no deep-link fallback. config.custom_error_responses = cfg .get("CustomErrorResponses") .and_then(|v| v.as_array()) .map(|arr| { let custom_error_response: Vec = arr .iter() - .filter_map(|v| serde_json::from_value(v.clone()).ok()) + .filter_map(|v| { + Some(CustomErrorResponse { + // Required by CloudFormation. A rule without it is + // skipped rather than defaulted to a code that + // would match nothing. + error_code: cfn_i64(v.get("ErrorCode"))? as i32, + response_page_path: v + .get("ResponsePagePath") + .and_then(|p| p.as_str()) + .map(String::from), + response_code: cfn_number_as_string(v.get("ResponseCode")), + error_caching_min_ttl: cfn_i64(v.get("ErrorCachingMinTTL")), + }) + }) .collect(); CustomErrorResponses { quantity: custom_error_response.len() as i32, @@ -1224,3 +1243,102 @@ impl ResourceProvisioner { .with("Stage", "DEVELOPMENT")) } } + +/// Read a CloudFormation numeric property. Templates carry these as JSON +/// numbers, but YAML templates and resolved intrinsics quote them, and both are +/// valid CloudFormation. +fn cfn_i64(value: Option<&serde_json::Value>) -> Option { + match value? { + serde_json::Value::Number(n) => n.as_i64(), + serde_json::Value::String(s) => s.parse().ok(), + _ => None, + } +} + +/// Read a CloudFormation numeric property that the AWS API carries as a string +/// (CloudFront's `ResponseCode`). +fn cfn_number_as_string(value: Option<&serde_json::Value>) -> Option { + match value? { + serde_json::Value::Number(n) => Some(n.to_string()), + serde_json::Value::String(s) => Some(s.clone()), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The `CustomErrorResponses` block CDK synthesizes for a SPA distribution. + /// CloudFormation types `ResponseCode` and `ErrorCachingMinTTL` as Integer; + /// the CloudFront API carries `ResponseCode` as a string. + fn cdk_spa_config() -> serde_json::Value { + serde_json::json!({ + "CustomErrorResponses": [ + {"ErrorCode": 403, "ResponseCode": 200, "ResponsePagePath": "/index.html", "ErrorCachingMinTTL": 300}, + {"ErrorCode": 404, "ResponseCode": 200, "ResponsePagePath": "/index.html", "ErrorCachingMinTTL": 300} + ] + }) + } + + #[test] + fn cfn_custom_error_responses_survive_translation() { + let mut config = DistributionConfig::default(); + ResourceProvisioner::apply_cfn_distribution_extras(&mut config, &cdk_spa_config()); + + let rules = config + .custom_error_responses + .expect("CustomErrorResponses translated"); + assert_eq!(rules.quantity, 2); + let items = rules.items.expect("items"); + let codes: Vec = items + .custom_error_response + .iter() + .map(|r| r.error_code) + .collect(); + assert_eq!(codes, vec![403, 404]); + for rule in &items.custom_error_response { + // The integer CFN gives must reach the wire model as a string, + // not be dropped for failing to deserialize into Option. + assert_eq!(rule.response_code.as_deref(), Some("200")); + assert_eq!(rule.response_page_path.as_deref(), Some("/index.html")); + assert_eq!(rule.error_caching_min_ttl, Some(300)); + } + } + + #[test] + fn cfn_custom_error_responses_accept_stringified_numbers() { + // YAML templates and `Fn::Sub` outputs quote numbers; both shapes are + // valid CloudFormation. + let cfg = serde_json::json!({ + "CustomErrorResponses": [ + {"ErrorCode": "404", "ResponseCode": "200", "ResponsePagePath": "/index.html"} + ] + }); + let mut config = DistributionConfig::default(); + ResourceProvisioner::apply_cfn_distribution_extras(&mut config, &cfg); + + let items = config + .custom_error_responses + .expect("translated") + .items + .expect("items"); + let rule = items.custom_error_response.first().expect("one rule"); + assert_eq!(rule.error_code, 404); + assert_eq!(rule.response_code.as_deref(), Some("200")); + } + + #[test] + fn a_rule_without_an_error_code_is_skipped_not_defaulted() { + // ErrorCode is required by CloudFormation; inventing a 0 would silently + // install a rule that matches nothing. + let cfg = serde_json::json!({ + "CustomErrorResponses": [{"ResponsePagePath": "/index.html"}] + }); + let mut config = DistributionConfig::default(); + ResourceProvisioner::apply_cfn_distribution_extras(&mut config, &cfg); + + let rules = config.custom_error_responses.expect("translated"); + assert_eq!(rules.quantity, 0); + } +} diff --git a/crates/fakecloud-cloudfront/src/model.rs b/crates/fakecloud-cloudfront/src/model.rs index 97cd19aa0..2bac4e208 100644 --- a/crates/fakecloud-cloudfront/src/model.rs +++ b/crates/fakecloud-cloudfront/src/model.rs @@ -555,7 +555,15 @@ pub struct CustomErrorResponse { pub response_page_path: Option, #[serde(default, skip_serializing_if = "skip_if_none")] pub response_code: Option, - #[serde(default, skip_serializing_if = "skip_if_none")] + // AWS spells this `ErrorCachingMinTTL` (upper-case TTL). The default + // PascalCase rule would emit `ErrorCachingMinTtl`, which drops the field on + // parse from real SDK requests and mis-names it on the wire. Pin the exact + // name, as `WebACLId` above does. + #[serde( + default, + rename = "ErrorCachingMinTTL", + skip_serializing_if = "skip_if_none" + )] pub error_caching_min_ttl: Option, } diff --git a/crates/fakecloud-e2e/tests/cloudformation_cloudfront.rs b/crates/fakecloud-e2e/tests/cloudformation_cloudfront.rs index b512f6e62..2526256d7 100644 --- a/crates/fakecloud-e2e/tests/cloudformation_cloudfront.rs +++ b/crates/fakecloud-e2e/tests/cloudformation_cloudfront.rs @@ -268,3 +268,94 @@ async fn cfn_provisions_cloudfront_distribution() { let after = cf.get_distribution().id(&dist_id).send().await; assert!(after.is_err(), "distribution should be gone"); } + +/// A SPA distribution as CDK synthesizes one: `CustomErrorResponses` mapping +/// 403/404 to `/index.html` with a 200. CloudFormation types `ResponseCode` as +/// an Integer while the CloudFront API carries it as a string, so translating +/// the CFN block through the wire struct dropped every rule and the +/// distribution came out with none — deep links 404'd instead of serving the +/// app shell. +const SPA_ERROR_TEMPLATE: &str = r#"{ + "Resources": { + "Dist": { + "Type": "AWS::CloudFront::Distribution", + "Properties": { + "DistributionConfig": { + "Comment": "spa error rules", + "Enabled": true, + "DefaultRootObject": "index.html", + "Origins": [ + {"Id": "origin-1", "DomainName": "origin.example.com", + "CustomOriginConfig": {"OriginProtocolPolicy": "http-only", "HTTPPort": 80, "HTTPSPort": 443}} + ], + "DefaultCacheBehavior": { + "TargetOriginId": "origin-1", + "ViewerProtocolPolicy": "allow-all" + }, + "CustomErrorResponses": [ + {"ErrorCode": 403, "ResponseCode": 200, "ResponsePagePath": "/index.html", "ErrorCachingMinTTL": 300}, + {"ErrorCode": 404, "ResponseCode": 200, "ResponsePagePath": "/index.html", "ErrorCachingMinTTL": 300} + ] + } + } + } + }, + "Outputs": { + "DistId": {"Value": {"Ref": "Dist"}} + } +}"#; + +#[tokio::test] +async fn cfn_provisions_spa_custom_error_responses() { + let server = TestServer::start().await; + let cfn = server.cloudformation_client().await; + let cf = aws_sdk_cloudfront::Client::new(&server.aws_config().await); + + cfn.create_stack() + .stack_name("cf-spa-errors") + .template_body(SPA_ERROR_TEMPLATE) + .send() + .await + .expect("create_stack"); + + let described = cfn + .describe_stacks() + .stack_name("cf-spa-errors") + .send() + .await + .expect("describe_stacks"); + let stack = described.stacks().first().unwrap(); + assert_eq!(stack.stack_status().unwrap().as_str(), "CREATE_COMPLETE"); + + let dist_id = stack + .outputs() + .iter() + .find(|o| o.output_key() == Some("DistId")) + .and_then(|o| o.output_value()) + .map(|s| s.to_string()) + .expect("DistId"); + + let got = cf + .get_distribution() + .id(&dist_id) + .send() + .await + .expect("get_distribution"); + let dcfg = got + .distribution() + .and_then(|d| d.distribution_config()) + .expect("config"); + + let rules = dcfg + .custom_error_responses() + .expect("CustomErrorResponses provisioned"); + assert_eq!(rules.quantity(), 2, "both rules must survive translation"); + let mut codes: Vec = rules.items().iter().map(|r| r.error_code()).collect(); + codes.sort_unstable(); + assert_eq!(codes, vec![403, 404]); + for rule in rules.items() { + assert_eq!(rule.response_code(), Some("200")); + assert_eq!(rule.response_page_path(), Some("/index.html")); + assert_eq!(rule.error_caching_min_ttl(), Some(300)); + } +}