fix(cloudformation): apply in-place UpdateStack changes for common resource types (bug-hunt) (#2313)

Author: vieiralucas

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

@@ -140,6 +140,73 @@ impl ResourceProvisioner {
             .with("UserPoolId", pool_id))
     }
 
+    /// Apply a CFN property update to an existing Cognito user pool in place.
+    /// Mirrors the property extraction in `create_cognito_user_pool` for the
+    /// fields that update without replacement (policies, MFA, tier, deletion
+    /// protection, tags, email/SMS config, admin-create config, recovery
+    /// settings, auto-verified attributes) so a stack update reaches the pool
+    /// and `DescribeUserPool` reflects the new config instead of the stale one.
+    /// The pool id/ARN, creation date and signing keys are preserved.
+    pub(super) fn update_cognito_user_pool(
+        &self,
+        existing: &StackResource,
+        resource: &ResourceDefinition,
+    ) -> Result<ProvisionResult, String> {
+        let props = &resource.properties;
+        let pool_id = &existing.physical_id;
+
+        let mut accounts = self.cognito_state.write();
+        let state = accounts.get_or_create(&self.account_id);
+        let pool = state
+            .user_pools
+            .get_mut(pool_id)
+            .ok_or_else(|| format!("User pool {pool_id} not yet provisioned"))?;
+
+        if let Some(pool_name) = props.get("PoolName").and_then(|v| v.as_str()) {
+            pool.name = pool_name.to_string();
+        }
+        pool.policies.password_policy = parse_cognito_password_policy(props.get("Policies"));
+        pool.auto_verified_attributes =
+            parse_cognito_string_array(props.get("AutoVerifiedAttributes"));
+        if let Some(mfa) = props.get("MfaConfiguration").and_then(|v| v.as_str()) {
+            pool.mfa_configuration = mfa.to_string();
+        }
+        if let Some(tier) = props.get("UserPoolTier").and_then(|v| v.as_str()) {
+            pool.user_pool_tier = tier.to_string();
+        }
+        if let Some(dp) = props.get("DeletionProtection").and_then(|v| v.as_str()) {
+            pool.deletion_protection = Some(dp.to_string());
+        }
+        if props.get("UserPoolTags").is_some() {
+            pool.user_pool_tags = parse_cognito_tags(props.get("UserPoolTags"));
+        }
+        if props.get("EmailConfiguration").is_some() {
+            pool.email_configuration =
+                parse_cognito_email_configuration(props.get("EmailConfiguration"));
+        }
+        if props.get("SmsConfiguration").is_some() {
+            pool.sms_configuration = parse_cognito_sms_configuration(props.get("SmsConfiguration"));
+        }
+        if props.get("AdminCreateUserConfig").is_some() {
+            pool.admin_create_user_config =
+                parse_cognito_admin_create_user_config(props.get("AdminCreateUserConfig"));
+        }
+        if props.get("AccountRecoverySetting").is_some() {
+            pool.account_recovery_setting =
+                parse_cognito_account_recovery(props.get("AccountRecoverySetting"));
+        }
+        pool.last_modified_date = Utc::now();
+
+        let arn = pool.arn.clone();
+        let provider_name = format!("cognito-idp.{}.amazonaws.com/{}", self.region, pool_id);
+        let provider_url = format!("https://{provider_name}");
+        Ok(ProvisionResult::new(pool_id.clone())
+            .with("Arn", arn)
+            .with("ProviderName", provider_name)
+            .with("ProviderURL", provider_url)
+            .with("UserPoolId", pool_id.clone()))
+    }
+
     pub(super) fn delete_cognito_user_pool(&self, physical_id: &str) -> Result<(), String> {
         let mut accounts = self.cognito_state.write();
         let state = accounts.get_or_create(&self.account_id);
@@ -278,6 +345,83 @@ impl ResourceProvisioner {
         Ok(result)
     }
 
+    /// Apply a CFN property update to an existing Cognito user pool client in
+    /// place. Mirrors the property extraction in `create_cognito_user_pool_client`
+    /// for the fields that update without replacement (auth flows, token
+    /// validity, callback/logout URLs, OAuth config, read/write attributes,
+    /// etc.) so a stack update reaches the client and `DescribeUserPoolClient`
+    /// reflects the new config. The client id/secret, pool id and creation date
+    /// are preserved (`GenerateSecret` is create-only).
+    pub(super) fn update_cognito_user_pool_client(
+        &self,
+        existing: &StackResource,
+        resource: &ResourceDefinition,
+    ) -> Result<ProvisionResult, String> {
+        let props = &resource.properties;
+        let client_id = &existing.physical_id;
+
+        let mut accounts = self.cognito_state.write();
+        let state = accounts.get_or_create(&self.account_id);
+        let client = state
+            .user_pool_clients
+            .get_mut(client_id)
+            .ok_or_else(|| format!("User pool client {client_id} not yet provisioned"))?;
+
+        if let Some(name) = props.get("ClientName").and_then(|v| v.as_str()) {
+            client.client_name = name.to_string();
+        }
+        client.explicit_auth_flows = parse_cognito_string_array(props.get("ExplicitAuthFlows"));
+        if let Some(v) = props.get("AccessTokenValidity").and_then(|v| v.as_i64()) {
+            client.access_token_validity = Some(v);
+        }
+        if let Some(v) = props.get("IdTokenValidity").and_then(|v| v.as_i64()) {
+            client.id_token_validity = Some(v);
+        }
+        if let Some(v) = props.get("RefreshTokenValidity").and_then(|v| v.as_i64()) {
+            client.refresh_token_validity = Some(v);
+        }
+        client.callback_urls = parse_cognito_string_array(props.get("CallbackURLs"));
+        client.logout_urls = parse_cognito_string_array(props.get("LogoutURLs"));
+        client.supported_identity_providers =
+            parse_cognito_string_array(props.get("SupportedIdentityProviders"));
+        client.allowed_o_auth_flows = parse_cognito_string_array(props.get("AllowedOAuthFlows"));
+        client.allowed_o_auth_scopes = parse_cognito_string_array(props.get("AllowedOAuthScopes"));
+        if let Some(b) = props
+            .get("AllowedOAuthFlowsUserPoolClient")
+            .and_then(|v| v.as_bool())
+        {
+            client.allowed_o_auth_flows_user_pool_client = b;
+        }
+        if let Some(s) = props
+            .get("PreventUserExistenceErrors")
+            .and_then(|v| v.as_str())
+        {
+            client.prevent_user_existence_errors = Some(s.to_string());
+        }
+        client.read_attributes = parse_cognito_string_array(props.get("ReadAttributes"));
+        client.write_attributes = parse_cognito_string_array(props.get("WriteAttributes"));
+        if let Some(b) = props.get("EnableTokenRevocation").and_then(|v| v.as_bool()) {
+            client.enable_token_revocation = b;
+        }
+        if let Some(v) = props.get("AuthSessionValidity").and_then(|v| v.as_i64()) {
+            client.auth_session_validity = Some(v);
+        }
+        if let Some(b) = props
+            .get("EnablePropagateAdditionalUserContextData")
+            .and_then(|v| v.as_bool())
+        {
+            client.enable_propagate_additional_user_context_data = b;
+        }
+        if let Some(v) = props.get("AnalyticsConfiguration") {
+            client.analytics_configuration = if v.is_null() { None } else { Some(v.clone()) };
+        }
+        client.last_modified_date = Utc::now();
+
+        Ok(ProvisionResult::new(client_id.clone())
+            .with("ClientId", client_id.clone())
+            .with("Name", client_id.clone()))
+    }
+
     pub(super) fn delete_cognito_user_pool_client(&self, physical_id: &str) -> Result<(), String> {
         let mut accounts = self.cognito_state.write();
         let state = accounts.get_or_create(&self.account_id);

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

@@ -50,6 +50,48 @@ impl ResourceProvisioner {
         Ok(ProvisionResult::new(arn.clone()).with("Arn", arn))
     }
 
+    /// Apply a CFN property update to an existing CloudWatch Logs log group in
+    /// place. `RetentionInDays` and `KmsKeyId` are updatable-without-replacement
+    /// in real CloudFormation, so a stack update must reach the log group and be
+    /// reflected by `DescribeLogGroups` instead of being silently dropped.
+    /// Omitting `RetentionInDays` clears retention (never expire), matching the
+    /// direct `DeleteRetentionPolicy` semantics CFN applies when the property
+    /// is removed.
+    pub(crate) fn update_log_group(
+        &self,
+        existing: &StackResource,
+        resource: &ResourceDefinition,
+    ) -> Result<ProvisionResult, String> {
+        let props = &resource.properties;
+        let arn = &existing.physical_id;
+
+        let retention_in_days = props
+            .get("RetentionInDays")
+            .and_then(|v| v.as_i64())
+            .map(|v| v as i32);
+        let kms_key_id = props
+            .get("KmsKeyId")
+            .and_then(|v| v.as_str())
+            .map(String::from);
+
+        let mut logs_accounts = self.logs_state.write();
+        let state = logs_accounts.get_or_create(&self.account_id);
+        let group = state
+            .log_groups
+            .values_mut()
+            .find(|g| g.arn == *arn)
+            .ok_or_else(|| format!("Log group {arn} not yet provisioned"))?;
+        group.retention_in_days = retention_in_days;
+        if kms_key_id.is_some() {
+            group.kms_key_id = kms_key_id;
+        }
+        if let Some(class) = props.get("LogGroupClass").and_then(|v| v.as_str()) {
+            group.log_group_class = Some(class.to_string());
+        }
+
+        Ok(ProvisionResult::new(arn.clone()).with("Arn", arn.clone()))
+    }
+
     /// Look up an S3 object's bytes from the in-process S3 state. Used by
     /// the Lambda function provisioner to hydrate `Code.S3Bucket` /
     /// `Code.S3Key` references into real ZIP content. Returns an error

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

@@ -239,6 +239,107 @@ impl ResourceProvisioner {
         Ok(result)
     }
 
+    /// Apply a CFN property update to an existing DynamoDB table in place.
+    /// `BillingMode`, `ProvisionedThroughput`, `GlobalSecondaryIndexes`,
+    /// `OnDemandThroughput`, `DeletionProtectionEnabled`, `TableClass` and
+    /// `SSESpecification` are all update-without-replacement in real
+    /// CloudFormation, so a stack update must reach the table and be reflected
+    /// by `DescribeTable` instead of being silently dropped. Key schema and
+    /// attribute definitions are replacement-only and left untouched here.
+    pub(super) fn update_dynamodb_table(
+        &self,
+        existing: &StackResource,
+        resource: &ResourceDefinition,
+    ) -> Result<ProvisionResult, String> {
+        let props = &resource.properties;
+        let arn = &existing.physical_id;
+
+        let mut __ddb_mas = self.dynamodb_state.write();
+        let state = __ddb_mas.get_or_create(&self.account_id);
+        let table = state
+            .tables
+            .values_mut()
+            .find(|t| &t.arn == arn)
+            .ok_or_else(|| format!("DynamoDB table {arn} not yet provisioned"))?;
+
+        if let Some(billing_mode) = props.get("BillingMode").and_then(|v| v.as_str()) {
+            table.billing_mode = billing_mode.to_string();
+        }
+        // Provisioned throughput only applies under PROVISIONED billing; when
+        // switching to PAY_PER_REQUEST the units go to zero, matching Describe.
+        if table.billing_mode == "PROVISIONED" {
+            if let Some(pt) = props.get("ProvisionedThroughput") {
+                table.provisioned_throughput = ProvisionedThroughput {
+                    read_capacity_units: pt
+                        .get("ReadCapacityUnits")
+                        .and_then(|v| v.as_i64())
+                        .unwrap_or(table.provisioned_throughput.read_capacity_units),
+                    write_capacity_units: pt
+                        .get("WriteCapacityUnits")
+                        .and_then(|v| v.as_i64())
+                        .unwrap_or(table.provisioned_throughput.write_capacity_units),
+                };
+            }
+        } else {
+            table.provisioned_throughput = ProvisionedThroughput {
+                read_capacity_units: 0,
+                write_capacity_units: 0,
+            };
+        }
+        if let Some(gsi) = props.get("GlobalSecondaryIndexes") {
+            table.gsi = fakecloud_dynamodb::parse_gsi(gsi, &table.billing_mode);
+        }
+        if let Some(odt) = props.get("OnDemandThroughput") {
+            table.on_demand_throughput = Some(OnDemandThroughput {
+                max_read_request_units: odt
+                    .get("MaxReadRequestUnits")
+                    .and_then(|v| v.as_i64())
+                    .unwrap_or(-1),
+                max_write_request_units: odt
+                    .get("MaxWriteRequestUnits")
+                    .and_then(|v| v.as_i64())
+                    .unwrap_or(-1),
+            });
+        }
+        if let Some(dp) = props
+            .get("DeletionProtectionEnabled")
+            .and_then(|v| v.as_bool().or_else(|| v.as_str().map(|s| s == "true")))
+        {
+            table.deletion_protection_enabled = dp;
+        }
+        if let Some(class) = props.get("TableClass").and_then(|v| v.as_str()) {
+            table.table_class = class.to_string();
+        }
+        if let Some(sse_spec) = props.get("SSESpecification") {
+            let enabled = sse_spec
+                .get("SSEEnabled")
+                .and_then(|v| v.as_bool().or_else(|| v.as_str().map(|s| s == "true")))
+                .unwrap_or(false);
+            if enabled {
+                table.sse_type = Some(
+                    sse_spec
+                        .get("SSEType")
+                        .and_then(|v| v.as_str())
+                        .unwrap_or("KMS")
+                        .to_string(),
+                );
+                table.sse_kms_key_arn = sse_spec
+                    .get("KMSMasterKeyId")
+                    .and_then(|v| v.as_str())
+                    .map(|s| s.to_string());
+            } else {
+                table.sse_type = None;
+                table.sse_kms_key_arn = None;
+            }
+        }
+
+        let mut result = ProvisionResult::new(arn.clone()).with("Arn", arn.clone());
+        if let Some(stream_arn) = table.stream_arn.clone() {
+            result = result.with("StreamArn", stream_arn);
+        }
+        Ok(result)
+    }
+
     pub(super) fn delete_dynamodb_table(&self, physical_id: &str) -> Result<(), String> {
         let mut __ddb_mas = self.dynamodb_state.write();
         let state = __ddb_mas.get_or_create(&self.account_id);

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

@@ -96,6 +96,67 @@ impl ResourceProvisioner {
         Ok(ProvisionResult::new(arn.clone()).with("Arn", arn))
     }
 
+    /// Apply a CFN property update to an existing EventBridge rule in place.
+    /// Mirrors the property extraction in `create_eventbridge_rule` so a stack
+    /// update re-applies ScheduleExpression / EventPattern / State / Targets /
+    /// Description / RoleArn — all update-without-replacement in real
+    /// CloudFormation — instead of being silently dropped. Without this a
+    /// `cdk deploy` that changes a schedule or its targets reports success but
+    /// the rule keeps firing on the stale schedule into the old targets.
+    pub(super) fn update_eventbridge_rule(
+        &self,
+        existing: &StackResource,
+        resource: &ResourceDefinition,
+    ) -> Result<ProvisionResult, String> {
+        let props = &resource.properties;
+        let arn = &existing.physical_id;
+
+        let mut eb_accounts = self.eventbridge_state.write();
+        let state = eb_accounts.get_or_create(&self.account_id);
+        let rule = state
+            .rules
+            .values_mut()
+            .find(|r| &r.arn == arn)
+            .ok_or_else(|| format!("EventBridge rule {arn} not yet provisioned"))?;
+
+        rule.event_pattern = props.get("EventPattern").map(|v| {
+            if v.is_string() {
+                v.as_str().unwrap_or_default().to_string()
+            } else {
+                serde_json::to_string(v).unwrap_or_default()
+            }
+        });
+        rule.schedule_expression = props
+            .get("ScheduleExpression")
+            .and_then(|v| v.as_str())
+            .map(|s| s.to_string());
+        rule.state = props
+            .get("State")
+            .and_then(|v| v.as_str())
+            .unwrap_or("ENABLED")
+            .to_string();
+        rule.description = props
+            .get("Description")
+            .and_then(|v| v.as_str())
+            .map(|s| s.to_string());
+        rule.role_arn = props
+            .get("RoleArn")
+            .and_then(|v| v.as_str())
+            .map(|s| s.to_string());
+        rule.targets = props
+            .get("Targets")
+            .and_then(|v| v.as_array())
+            .map(|arr| {
+                arr.iter()
+                    .filter(|t| t.get("Arn").and_then(|v| v.as_str()).is_some())
+                    .map(fakecloud_eventbridge::parse_target)
+                    .collect()
+            })
+            .unwrap_or_default();
+
+        Ok(ProvisionResult::new(arn.clone()).with("Arn", arn.clone()))
+    }
+
     pub(super) fn delete_eventbridge_rule(&self, physical_id: &str) -> Result<(), String> {
         let mut eb_accounts = self.eventbridge_state.write();
         let state = eb_accounts.default_mut();