fix(cli-internal): handle root changeset failures, template format errors, and cascading IAM drift

9pace · 9pace · commit 3dfa0b211c23 · 2026-04-09T19:02:22.000Z
Three fixes discovered during E2E lock + rollback testing:

1. Root changeset fall-through: Root changeset FAILED status references nested
   failures without containing "EarlyValidation". Add isRoot parameter so root
   always falls through to classify nested changesets individually.

2. Template format error as recoverable: IncludeNestedStacks fails on Amplify
   AppSync API stacks where model sub-stacks export names via Fn::Join. CFN
   reports "duplicate Export names". Changes are still populated — treat as
   recoverable like EarlyValidation.

3. Cascading IAM Policy changes: When DeletionPolicy changes on a DynamoDB
   table, CFN flags IAM policies referencing TableName.Arn as Dynamic
   ResourceAttribute re-evaluations. These are harmless and must be filtered
   alongside direct DeletionPolicy drift in lock rollback validation.
diff --git a/packages/amplify-cli/src/__tests__/commands/gen2-migration/lock.test.ts b/packages/amplify-cli/src/__tests__/commands/gen2-migration/lock.test.ts
@@ -535,18 +535,22 @@ describe('AmplifyMigrationLockStep', () => {
 
     it('should fail validation when nested changes contain real drift at leaf level', async () => {
       mockDetectTemplateDrift.mockResolvedValueOnce({
-        changes: [{
-          Action: 'Modify',
-          LogicalResourceId: 'authStack',
-          ResourceType: 'AWS::CloudFormation::Stack',
-          Scope: ['Properties'],
-          nestedChanges: [{
+        changes: [
+          {
             Action: 'Modify',
-            LogicalResourceId: 'UserPool',
-            ResourceType: 'AWS::Cognito::UserPool',
+            LogicalResourceId: 'authStack',
+            ResourceType: 'AWS::CloudFormation::Stack',
             Scope: ['Properties'],
-          }],
-        }],
+            nestedChanges: [
+              {
+                Action: 'Modify',
+                LogicalResourceId: 'UserPool',
+                ResourceType: 'AWS::Cognito::UserPool',
+                Scope: ['Properties'],
+              },
+            ],
+          },
+        ],
         skipped: false,
       });
 
@@ -569,21 +573,157 @@ describe('AmplifyMigrationLockStep', () => {
       expect(valid).toBe(false);
     });
 
+    it('should pass validation when only cascading IAM Policy drift from DeletionPolicy change exists', async () => {
+      mockDetectTemplateDrift.mockResolvedValueOnce({
+        changes: [
+          {
+            Action: 'Modify',
+            LogicalResourceId: 'TodoTable',
+            ResourceType: 'AWS::DynamoDB::Table',
+            Scope: ['DeletionPolicy'],
+            Replacement: 'False',
+          },
+          {
+            Action: 'Modify',
+            LogicalResourceId: 'TodoIAMRoleDefaultPolicy7BBBF45B',
+            ResourceType: 'AWS::IAM::Policy',
+            Scope: ['Properties'],
+            Details: [
+              {
+                ChangeSource: 'ResourceAttribute',
+                Evaluation: 'Dynamic',
+                Target: { Attribute: 'Properties', Name: 'PolicyDocument', RequiresRecreation: 'Never' },
+                CausingEntity: 'TodoTable.Arn',
+              },
+            ],
+          },
+        ],
+        skipped: false,
+      });
+
+      const plan = await lockStep.rollback();
+      const valid = await plan.validate();
+
+      expect(valid).toBe(true);
+    });
+
+    it('should fail validation when IAM Policy has a static/direct change mixed with dynamic', async () => {
+      mockDetectTemplateDrift.mockResolvedValueOnce({
+        changes: [
+          {
+            Action: 'Modify',
+            LogicalResourceId: 'TodoIAMRoleDefaultPolicy7BBBF45B',
+            ResourceType: 'AWS::IAM::Policy',
+            Scope: ['Properties'],
+            Details: [
+              {
+                ChangeSource: 'ResourceAttribute',
+                Evaluation: 'Dynamic',
+                Target: { Attribute: 'Properties', Name: 'PolicyDocument', RequiresRecreation: 'Never' },
+                CausingEntity: 'TodoTable.Arn',
+              },
+              {
+                ChangeSource: 'DirectModification',
+                Evaluation: 'Static',
+                Target: { Attribute: 'Properties', Name: 'PolicyDocument', RequiresRecreation: 'Never' },
+              },
+            ],
+          },
+        ],
+        skipped: false,
+      });
+
+      const plan = await lockStep.rollback();
+      const valid = await plan.validate();
+
+      expect(valid).toBe(false);
+    });
+
+    it('should fail validation when IAM Policy change requires recreation', async () => {
+      mockDetectTemplateDrift.mockResolvedValueOnce({
+        changes: [
+          {
+            Action: 'Modify',
+            LogicalResourceId: 'SomePolicy',
+            ResourceType: 'AWS::IAM::Policy',
+            Scope: ['Properties'],
+            Details: [
+              {
+                ChangeSource: 'ResourceAttribute',
+                Evaluation: 'Dynamic',
+                Target: { Attribute: 'Properties', Name: 'PolicyDocument', RequiresRecreation: 'Conditionally' },
+                CausingEntity: 'TodoTable.Arn',
+              },
+            ],
+          },
+        ],
+        skipped: false,
+      });
+
+      const plan = await lockStep.rollback();
+      const valid = await plan.validate();
+
+      expect(valid).toBe(false);
+    });
+
+    it('should pass validation when nested tree has only DeletionPolicy and cascading IAM drift', async () => {
+      mockDetectTemplateDrift.mockResolvedValueOnce({
+        changes: [
+          {
+            Action: 'Modify',
+            LogicalResourceId: 'apiStack',
+            ResourceType: 'AWS::CloudFormation::Stack',
+            Scope: ['Properties'],
+            nestedChanges: [
+              {
+                Action: 'Modify',
+                LogicalResourceId: 'TodoTable',
+                ResourceType: 'AWS::DynamoDB::Table',
+                Scope: ['DeletionPolicy'],
+              },
+              {
+                Action: 'Modify',
+                LogicalResourceId: 'TodoIAMRoleDefaultPolicy',
+                ResourceType: 'AWS::IAM::Policy',
+                Scope: ['Properties'],
+                Details: [
+                  {
+                    ChangeSource: 'ResourceAttribute',
+                    Evaluation: 'Dynamic',
+                    Target: { Attribute: 'Properties', RequiresRecreation: 'Never' },
+                    CausingEntity: 'TodoTable.Arn',
+                  },
+                ],
+              },
+            ],
+          },
+        ],
+        skipped: false,
+      });
+
+      const plan = await lockStep.rollback();
+      const valid = await plan.validate();
+
+      expect(valid).toBe(true);
+    });
+
     it('should pass validation when CloudFormation::Stack wrapper has no nestedChanges', async () => {
       mockDetectTemplateDrift.mockResolvedValueOnce({
-        changes: [{
-          Action: 'Modify',
-          LogicalResourceId: 'apiStack',
-          ResourceType: 'AWS::CloudFormation::Stack',
-          Scope: ['Properties'],
-        }],
+        changes: [
+          {
+            Action: 'Modify',
+            LogicalResourceId: 'apiStack',
+            ResourceType: 'AWS::CloudFormation::Stack',
+            Scope: ['Properties'],
+          },
+        ],
         skipped: false,
       });
 
       const plan = await lockStep.rollback();
       const valid = await plan.validate();
 
-      expect(valid).toBe(true)
+      expect(valid).toBe(true);
     });
   });
 });
diff --git a/packages/amplify-cli/src/commands/drift-detection/detect-template-drift.ts b/packages/amplify-cli/src/commands/drift-detection/detect-template-drift.ts
@@ -48,8 +48,15 @@ function extractChangeSetNameFromArn(changeSetArn: string): string {
   return resource.split('/')[1];
 }
 
-function isEarlyValidationFailure(reason?: string): boolean {
-  return !!reason?.includes('EarlyValidation');
+function isRecoverableFailure(reason?: string): boolean {
+  if (!reason) return false;
+  // EarlyValidation failures (e.g., ResourceExistenceCheck) — Changes are populated
+  if (reason.includes('EarlyValidation')) return true;
+  // Template format errors (e.g., duplicate Export names from Fn::Join in nested stacks) —
+  // Changes are partially populated. CFN validates exports after building the change list,
+  // so some changes are available even though the changeset ultimately failed.
+  if (reason.includes('Template format error')) return true;
+  return false;
 }
 
 const CHANGESET_PREFIX = 'amplify-drift-detection-';
@@ -220,7 +227,7 @@ export async function detectTemplateDrift(
     }
 
     // Changeset is kept for user inspection via console URL — cleaned up on next run
-    const result = await analyzeChangeSet(cfn, changeSet, print);
+    const result = await analyzeChangeSet(cfn, changeSet, print, true);
     result.changeSetId = changeSet.ChangeSetId;
     return result;
   } catch (error: any) {
@@ -236,6 +243,7 @@ async function analyzeChangeSet(
   cfn: CloudFormationClient,
   changeSet: DescribeChangeSetCommandOutput,
   print: SpinningLogger,
+  isRoot = false,
 ): Promise<TemplateDriftResults> {
   const result: TemplateDriftResults = {
     changes: [],
@@ -254,10 +262,16 @@ async function analyzeChangeSet(
     }
 
     // EarlyValidation failures still have Changes populated — fall through to process them
-    if (isEarlyValidationFailure(changeSet.StatusReason)) {
-      print.warn(`Nested changeset FAILED (EarlyValidation): ${changeSet.StatusReason}`);
+    if (isRecoverableFailure(changeSet.StatusReason)) {
+      print.warn(`Nested changeset FAILED (recoverable): ${changeSet.StatusReason}`);
+    } else if (isRoot) {
+      // Root changeset FAILED because a nested changeset failed. The root's StatusReason
+      // references the first failing nested changeset (e.g., "Nested change set <ARN> was
+      // not successfully created: Currently in FAILED.") but does NOT contain "EarlyValidation".
+      // Classification must happen per-nested-stack, so fall through to process Changes.
+      print.debug(`Root changeset FAILED due to nested failure — falling through to analyze nested changesets`);
     } else {
-      // Unknown failure — treat as genuine error, skip this stack
+      // Unknown failure on a nested stack — treat as genuine error, skip this stack
       print.warn(`ChangeSet failed with unexpected reason: ${changeSet.StatusReason || 'No reason provided'}`);
       return {
         changes: [],
diff --git a/packages/amplify-cli/src/commands/gen2-migration/lock.ts b/packages/amplify-cli/src/commands/gen2-migration/lock.ts
@@ -52,13 +52,32 @@ const ALLOW_ALL_POLICY = {
 /**
  * Identifies changeset changes that are expected DeletionPolicy drift from the lock step.
  *
- * The lock step adds `DeletionPolicy: Retain` to stateful resources. These show up as
- * Modify changes in changesets with Scope limited to `['DeletionPolicy']`. For lock rollback
- * to determine whether the environment is safe to revert, these expected changes must be
- * filtered out so only real drift blocks the rollback.
+ * The lock step adds `DeletionPolicy: Retain` to stateful resources. These show up as:
+ * 1. Direct DeletionPolicy changes — Modify with Scope exactly `['DeletionPolicy']`
+ * 2. Cascading IAM Policy changes — CFN flags IAM policies that reference the modified
+ *    table's attributes (e.g., `TodoTable.Arn` in PolicyDocument) as Dynamic re-evaluations.
+ *    These have `ChangeSource: ResourceAttribute`, `Evaluation: Dynamic`, and
+ *    `RequiresRecreation: Never` — they are harmless re-evaluations, not actual changes.
+ *
+ * For lock rollback to determine whether the environment is safe to revert, these expected
+ * changes must be filtered out so only real drift blocks the rollback.
  */
-const isExpectedLockDrift = (change: ResourceChangeWithNested): boolean =>
-  change.Action === 'Modify' && change.Scope?.length === 1 && change.Scope[0] === 'DeletionPolicy';
+const isExpectedLockDrift = (change: ResourceChangeWithNested): boolean => {
+  if (change.Action !== 'Modify') return false;
+
+  // Direct DeletionPolicy change on a resource
+  if (change.Scope?.length === 1 && change.Scope[0] === 'DeletionPolicy') return true;
+
+  // Cascading IAM Policy change caused by DeletionPolicy modification on a referenced resource.
+  // All Details must be Dynamic ResourceAttribute re-evaluations (no static/direct changes).
+  if (change.ResourceType === 'AWS::IAM::Policy' && change.Details?.length) {
+    return change.Details.every(
+      (d) => d.ChangeSource === 'ResourceAttribute' && d.Evaluation === 'Dynamic' && d.Target?.RequiresRecreation === 'Never',
+    );
+  }
+
+  return false;
+};
 
 /**
  * Recursively walks the change tree to find any leaf resource changes that are
@@ -270,11 +289,7 @@ export class AmplifyMigrationLockStep extends AmplifyMigrationStep {
    */
   private async validateLockRollbackDrift(): Promise<ValidationResult> {
     try {
-      const driftResults = await detectTemplateDrift(
-        this.gen1App.rootStackName,
-        this.logger,
-        this.gen1App.clients.cloudFormation,
-      );
+      const driftResults = await detectTemplateDrift(this.gen1App.rootStackName, this.logger, this.gen1App.clients.cloudFormation);
 
       if (driftResults.skipped) {
         return { valid: false, report: `Template drift detection was skipped: ${driftResults.skipReason}` };
