Merge pull request #14181 from aws-amplify/gen2-migrations-execute

fix: custom resource error handling, formatting messages

Author: abhi7cr

packages/amplify-migration-template-gen/src/migration-readme-generator.ts

@@ -31,9 +31,13 @@ s3Bucket.bucketName = YOUR_GEN1_BUCKET_NAME;
       `## REDEPLOY GEN2 APPLICATION
 1.a) Uncomment the following lines in \`amplify/backend.ts\` file
 ${this.categories.includes('storage') ? s3BucketChanges : ''}
-\`\`\`
+${
+  this.categories.includes('auth')
+    ? `\`\`\`
 backend.auth.resources.userPool.node.tryRemoveChild('UserPoolDomain');
-\`\`\`
+\`\`\``
+    : ''
+}
 
 \`\`\`
 Tags.of(backend.stack).add("gen1-migrated-app", "true");

packages/amplify-migration-template-gen/src/template-generator.ts

@@ -39,7 +39,8 @@ const CATEGORIES: CATEGORY[] = ['auth', 'storage'];
 const TEMPLATES_DIR = '.amplify/migration/templates';
 const SEPARATOR = ' to ';
 
-const SOURCE_TO_DESTINATION_STACKS = [`Gen1`, `Gen2`];
+const GEN1 = 'Gen 1';
+const GEN2 = 'Gen 2';
 const AUTH_RESOURCES_TO_REFACTOR = [
   CFN_AUTH_TYPE.UserPool,
   CFN_AUTH_TYPE.UserPoolClient,
@@ -247,6 +248,10 @@ class TemplateGenerator {
     );
   }
 
+  private getStackCategoryName(category: string) {
+    return !this.isCustomResource(category) ? category : 'custom';
+  }
+
   private async processGen1Stack(
     category: string,
     categoryTemplateGenerator: CategoryTemplateGenerator<CFN_CATEGORY_TYPE>,
@@ -257,17 +262,19 @@ class TemplateGenerator {
       const { newTemplate, parameters: gen1StackParameters } = await categoryTemplateGenerator.generateGen1PreProcessTemplate();
 
       assert(gen1StackParameters);
-      updatingGen1CategoryStack = ora(`Updating Gen1 ${category} stack...`).start();
+      updatingGen1CategoryStack = ora(`Updating Gen 1 ${this.getStackCategoryName(category)} stack...`).start();
 
       const gen1StackUpdateStatus = await tryUpdateStack(this.cfnClient, sourceCategoryStackId, gen1StackParameters, newTemplate);
 
       assert(gen1StackUpdateStatus === CFNStackStatus.UPDATE_COMPLETE);
-      updatingGen1CategoryStack.succeed(`Updated Gen1 ${category} stack successfully`);
+      updatingGen1CategoryStack.succeed(`Updated Gen 1 ${this.getStackCategoryName(category)} stack successfully`);
 
       return newTemplate;
     } catch (e) {
       if (this.isNoResourcesError(e)) {
-        updatingGen1CategoryStack?.succeed(`No resources found to move in Gen1 ${category} stack. Skipping update.`);
+        updatingGen1CategoryStack?.succeed(
+          `No resources found to move in Gen 1 ${this.getStackCategoryName(category)} stack. Skipping update.`,
+        );
         return undefined;
       }
       throw e;
@@ -286,12 +293,12 @@ class TemplateGenerator {
     try {
       const { newTemplate, oldTemplate, parameters } = await categoryTemplateGenerator.generateGen2ResourceRemovalTemplate();
 
-      const updatingGen2CategoryStack = ora(`Updating Gen2 ${category} stack...`).start();
+      const updatingGen2CategoryStack = ora(`Updating Gen 2 ${this.getStackCategoryName(category)} stack...`).start();
 
       const gen2StackUpdateStatus = await tryUpdateStack(this.cfnClient, destinationCategoryStackId, parameters ?? [], newTemplate);
 
       assert(gen2StackUpdateStatus === CFNStackStatus.UPDATE_COMPLETE);
-      updatingGen2CategoryStack.succeed(`Updated Gen2 ${category} stack successfully`);
+      updatingGen2CategoryStack.succeed(`Updated Gen 2 ${this.getStackCategoryName(category)} stack successfully`);
 
       return { newTemplate, oldTemplate, parameters };
     } catch (e) {
@@ -361,6 +368,12 @@ class TemplateGenerator {
     );
   }
 
+  private isCustomResource(category: string) {
+    return !Object.values(NON_CUSTOM_RESOURCE_CATEGORY)
+      .map((nonCustomCategory) => nonCustomCategory.valueOf())
+      .includes(category);
+  }
+
   private async generateCategoryTemplates(isRevert = false, customResourceMap?: ResourceMapping[]) {
     this.initializeCategoryGenerators(customResourceMap);
     for (const [category, sourceCategoryStackId, destinationCategoryStackId, categoryTemplateGenerator] of this
@@ -373,7 +386,7 @@ class TemplateGenerator {
       let destinationTemplateForRefactor: CFNTemplate | undefined;
       let logicalIdMappingForRefactor: Map<string, string> | undefined;
 
-      if (customResourceMap && !Object.values(NON_CUSTOM_RESOURCE_CATEGORY).includes(category as NON_CUSTOM_RESOURCE_CATEGORY)) {
+      if (customResourceMap && this.isCustomResource(category)) {
         newSourceTemplate = await this.processGen1Stack(category, categoryTemplateGenerator, sourceCategoryStackId);
         if (!newSourceTemplate) continue;
         const { newTemplate } = await this.processGen2Stack(category, categoryTemplateGenerator, destinationCategoryStackId);
@@ -447,7 +460,9 @@ class TemplateGenerator {
       assert(newSourceTemplate);
       assert(newDestinationTemplate);
 
-      const refactorResources = ora(`Moving ${category} resources from ${this.getSourceToDestinationMessage(isRevert)} stack...`).start();
+      const refactorResources = ora(
+        `Moving ${this.getStackCategoryName(category)} resources from ${this.getSourceToDestinationMessage(isRevert)} stack...`,
+      ).start();
       const { success, failedRefactorMetadata } = await this.refactorResources(
         logicalIdMappingForRefactor,
         sourceCategoryStackId,
@@ -459,17 +474,21 @@ class TemplateGenerator {
       );
       if (!success) {
         refactorResources.fail(
-          `Moving ${category} resources from ${this.getSourceToDestinationMessage(isRevert)} stack failed. Reason: ${
-            failedRefactorMetadata?.reason
-          }. Status: ${failedRefactorMetadata?.status}. RefactorId: ${failedRefactorMetadata?.stackRefactorId}.`,
+          `Moving ${this.getStackCategoryName(category)} resources from ${this.getSourceToDestinationMessage(
+            isRevert,
+          )} stack failed. Reason: ${failedRefactorMetadata?.reason}. Status: ${failedRefactorMetadata?.status}. RefactorId: ${
+            failedRefactorMetadata?.stackRefactorId
+          }.`,
         );
         await pollStackForCompletionState(this.cfnClient, destinationCategoryStackId, 30);
         if (!isRevert && oldDestinationTemplate) {
           await this.rollbackGen2Stack(category, destinationCategoryStackId, destinationStackParameters, oldDestinationTemplate);
         }
         return false;
       } else {
-        refactorResources.succeed(`Moved ${category} resources from ${this.getSourceToDestinationMessage(isRevert)} stack successfully`);
+        refactorResources.succeed(
+          `Moved ${this.getStackCategoryName(category)} resources from ${this.getSourceToDestinationMessage(isRevert)} stack successfully`,
+        );
       }
     }
     if (!isRevert) {
@@ -527,10 +546,10 @@ class TemplateGenerator {
     gen2StackParameters: Parameter[] | undefined,
     oldGen2Template: CFNTemplate,
   ) {
-    const rollingBackGen2Stack = ora(`Rolling back Gen2 ${category} stack...`).start();
+    const rollingBackGen2Stack = ora(`Rolling back Gen 2 ${this.getStackCategoryName(category)} stack...`).start();
     const gen2StackUpdateStatus = await tryUpdateStack(this.cfnClient, gen2CategoryStackId, gen2StackParameters ?? [], oldGen2Template);
-    assert(gen2StackUpdateStatus === CFNStackStatus.UPDATE_COMPLETE, `Gen2 Stack in a failed state: ${gen2StackUpdateStatus}.`);
-    rollingBackGen2Stack.succeed(`Rolled back Gen2 ${category} stack successfully`);
+    assert(gen2StackUpdateStatus === CFNStackStatus.UPDATE_COMPLETE, `Gen 2 Stack is in a failed state: ${gen2StackUpdateStatus}.`);
+    rollingBackGen2Stack.succeed(`Rolled back Gen 2 ${this.getStackCategoryName(category)} stack successfully`);
   }
 
   private async generateRefactorTemplatesForRevert(
@@ -621,7 +640,8 @@ class TemplateGenerator {
   }
 
   private getSourceToDestinationMessage(revert: boolean) {
-    return revert ? [...SOURCE_TO_DESTINATION_STACKS].reverse().join(SEPARATOR) : SOURCE_TO_DESTINATION_STACKS.join(SEPARATOR);
+    const SOURCE_TO_DESTINATION_STACKS = [GEN1, GEN2];
+    return revert ? SOURCE_TO_DESTINATION_STACKS.reverse().join(SEPARATOR) : SOURCE_TO_DESTINATION_STACKS.join(SEPARATOR);
   }
 
   private constructRoleArn(roleName: string) {

packages/amplify-migration/src/command-handler.test.ts

@@ -270,21 +270,7 @@ describe('updateCdkStackFile', () => {
   const mockCustomResourcesPath = 'amplify-gen2/amplify/custom';
   const mockProjectRoot = '/mockRootDir';
 
-  beforeEach(() => {
-    jest.clearAllMocks();
-    jest.mocked(updateCdkStackFile).mockImplementation(actualUpdateCdkStackFile);
-    jest.mocked(getProjectInfo).mockImplementation(actualGetProjectInfoFile);
-    jest.mocked(pathManager.findProjectRoot).mockReturnValue('/mockRootDir');
-  });
-
-  afterEach(() => {
-    // Reset to the mock after each test if needed
-    jest.mocked(updateCdkStackFile).mockReset();
-    jest.mocked(getProjectInfo).mockReset();
-  });
-
-  it('should correctly transform CDK stack file content', async () => {
-    const originalCdkContent = `
+  const originalCdkContentWithError = `
       import * as AmplifyHelpers from '@aws-amplify/cli-extensibility-helper';
       import * as cdk from 'aws-cdk-lib';
       
@@ -307,6 +293,46 @@ describe('updateCdkStackFile', () => {
       }
     `;
 
+  const originalCdkContentWithoutError = `
+      import * as AmplifyHelpers from '@aws-amplify/cli-extensibility-helper';
+      import * as cdk from 'aws-cdk-lib';
+      
+      export class cdkStack extends cdk.Stack {
+        constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
+          super(scope, id, props);
+          
+          const projectInfo = AmplifyHelpers.getProjectInfo();
+          /* AmplifyHelpers.addResourceDependency(this, 
+            {
+              category: "custom",
+              resourceName: "customResource1",
+            },
+            {
+              category: "auth",
+              resourceName: "authResource1",
+            }
+          ); */
+        }
+      }
+    `;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    jest.mocked(updateCdkStackFile).mockImplementation(actualUpdateCdkStackFile);
+    jest.mocked(getProjectInfo).mockImplementation(actualGetProjectInfoFile);
+    jest.mocked(pathManager.findProjectRoot).mockReturnValue('/mockRootDir');
+  });
+
+  afterEach(() => {
+    // Reset to the mock after each test if needed
+    jest.mocked(updateCdkStackFile).mockReset();
+    jest.mocked(getProjectInfo).mockReset();
+  });
+
+  test.each([
+    [originalCdkContentWithError, true],
+    [originalCdkContentWithoutError, false],
+  ])('should correctly transform CDK stack file content', async (originalCdkContent, shouldThrowError) => {
     const mockProjectConfig = JSON.stringify({
       projectName: 'testProject',
       version: '3.1',
@@ -344,9 +370,11 @@ describe('updateCdkStackFile', () => {
 
     // Verify specific transformations
     expect(transformedContent).not.toContain('import { AmplifyHelpers }'); // Import removed
-    expect(transformedContent).toContain(
-      "throw new Error('Follow https://docs.amplify.aws/react/start/migrate-to-gen2/ to update the resource dependency')",
-    ); // Error added
+    if (shouldThrowError) {
+      expect(transformedContent).toContain(
+        "throw new Error('Follow https://docs.amplify.aws/react/start/migrate-to-gen2/ to update the resource dependency')",
+      );
+    } // Error added
     expect(transformedContent).toContain("const projectInfo = {envName: `${AMPLIFY_GEN_1_ENV_NAME}`, projectName: 'testProject'}"); // Project info replaced
   });
 });

packages/amplify-migration/src/command-handlers.ts

@@ -50,7 +50,8 @@ const MIGRATION_DIR = '.amplify/migration';
 const GEN1_COMMAND = 'amplifyPush --simple';
 const GEN2_COMMAND = 'npx ampx pipeline-deploy --branch $AWS_BRANCH --app-id $AWS_APP_ID';
 const GEN2_COMMAND_GENERATION_MESSAGE_SUFFIX = 'your Gen 2 backend code';
-const GEN1_REMOVE_CONFIGURATION_MESSAGE_SUFFIX = 'your Gen1 configuration files';
+const GEN1_REMOVE_CONFIGURATION_MESSAGE_SUFFIX = 'your Gen 1 configuration files';
+const GEN1_CUSTOM_RESOURCES_SUFFIX = 'your Gen 1 custom resources';
 export const GEN1_CONFIGURATION_FILES = ['aws-exports.js', 'amplifyconfiguration.json', 'awsconfiguration.json'];
 const CUSTOM_DIR = 'custom';
 const TYPES_DIR = 'types';
@@ -321,6 +322,7 @@ const getCustomResourceMap = async (): Promise<Map<string, string>> => {
 export async function updateCustomResources() {
   const customResources = getCustomResources();
   if (customResources.length > 0) {
+    const movingGen1CustomResources = ora(`Moving ${GEN1_CUSTOM_RESOURCES_SUFFIX}`).start();
     const rootDir = pathManager.findProjectRoot();
     assert(rootDir);
     const amplifyGen1BackendDir = path.join(rootDir, AMPLIFY_DIR, BACKEND_DIR);
@@ -344,6 +346,7 @@ export async function updateCustomResources() {
     await fs.cp(sourceTypesPath, destinationTypesPath, { recursive: true });
 
     await updateCdkStackFile(customResources, destinationCustomResourcePath, rootDir);
+    movingGen1CustomResources.succeed(`Moved ${GEN1_CUSTOM_RESOURCES_SUFFIX}`);
   }
 }
 
@@ -359,7 +362,7 @@ export async function updateCdkStackFile(customResources: string[], destinationC
       let cdkStackContent = await fs.readFile(cdkStackFilePath, { encoding: 'utf-8' });
 
       // Check for existence of AmplifyHelpers.addResourceDependency and throw an error if found
-      if (cdkStackContent.includes('AmplifyHelpers.addResourceDependency')) {
+      if (hasUncommentedDependency(cdkStackContent, 'AmplifyHelpers.addResourceDependency')) {
         cdkStackContent = cdkStackContent.replace(
           /export class/,
           `throw new Error('Follow https://docs.amplify.aws/react/start/migrate-to-gen2/ to update the resource dependency');\n\nexport class`,
@@ -375,7 +378,7 @@ export async function updateCdkStackFile(customResources: string[], destinationC
 
       // Replace the cdk.CfnParameter definition to include the default property
       cdkStackContent = cdkStackContent.replace(
-        /new cdk\.CfnParameter\(this, "env", {[\s\S]*?}\);/,
+        /new cdk\.CfnParameter\(this, ['"]env['"], {[\s\S]*?}\);/,
         `new cdk.CfnParameter(this, "env", {
                 type: "String",
                 description: "Current Amplify CLI env name",
@@ -402,6 +405,29 @@ export async function updateCdkStackFile(customResources: string[], destinationC
   }
 }
 
+const hasUncommentedDependency = (fileContent: string, matchString: string) => {
+  // Split the content into lines
+  const lines = fileContent.split('\n');
+
+  // Check each line
+  for (const line of lines) {
+    const trimmedLine = line.trim();
+
+    // Check if the line contains the dependency and is not commented
+    if (
+      trimmedLine.includes(matchString) &&
+      !trimmedLine.startsWith('//') &&
+      !trimmedLine.startsWith('/*') &&
+      !trimmedLine.includes('*/') &&
+      !trimmedLine.match(/^\s*\*/)
+    ) {
+      return true;
+    }
+  }
+
+  return false;
+};
+
 export async function getProjectInfo(rootDir: string) {
   const configDir = path.join(rootDir, AMPLIFY_DIR, '.config');
   const projectConfigFilePath = path.join(configDir, 'project-config.json');
@@ -477,7 +503,7 @@ export async function prepare() {
 
   await updateCustomResources();
 
-  const movingGen1BackendFiles = ora(`Moving your Gen1 backend files to ${format.highlight(MIGRATION_DIR)}`).start();
+  const movingGen1BackendFiles = ora(`Moving your Gen 1 backend files to ${format.highlight(MIGRATION_DIR)}`).start();
   // Move gen1 amplify to .amplify/migrations and move gen2 amplify from amplify-gen2 to amplify dir to convert current app to gen2.
   const cwd = process.cwd();
   await fs.rm(MIGRATION_DIR, { force: true, recursive: true });
@@ -486,7 +512,7 @@ export async function prepare() {
   await fs.rename(`${TEMP_GEN_2_OUTPUT_DIR}/amplify`, `${cwd}/amplify`);
   await fs.rename(`${TEMP_GEN_2_OUTPUT_DIR}/package.json`, `${cwd}/package.json`);
   await fs.rm(TEMP_GEN_2_OUTPUT_DIR, { recursive: true });
-  movingGen1BackendFiles.succeed(`Moved your Gen1 backend files to ${format.highlight(MIGRATION_DIR)}`);
+  movingGen1BackendFiles.succeed(`Moved your Gen 1 backend files to ${format.highlight(MIGRATION_DIR)}`);
 }
 
 export async function removeGen1ConfigurationFiles() {
@@ -536,12 +562,12 @@ export async function revertGen2Migration(fromStack: string, toStack: string) {
   const success = await templateGenerator.revert();
   const usageData = await getUsageDataMetric(envName);
   if (success) {
-    printer.print(format.success(`Moved resources back to Gen1 stack successfully.`));
-    const movingGen1BackendFiles = ora(`Moving your Gen1 backend files to ${format.highlight(AMPLIFY_DIR)}`).start();
+    printer.print(format.success(`Moved resources back to Gen 1 stack successfully.`));
+    const movingGen1BackendFiles = ora(`Moving your Gen 1 backend files to ${format.highlight(AMPLIFY_DIR)}`).start();
     // Move gen1 amplify from .amplify/migration/amplify to amplify
     await fs.rm(AMPLIFY_DIR, { force: true, recursive: true });
     await fs.rename(`${MIGRATION_DIR}/amplify`, AMPLIFY_DIR);
-    movingGen1BackendFiles.succeed(`Moved your Gen1 backend files to ${format.highlight(AMPLIFY_DIR)}`);
+    movingGen1BackendFiles.succeed(`Moved your Gen 1 backend files to ${format.highlight(AMPLIFY_DIR)}`);
     await usageData.emitSuccess();
   } else {
     await usageData.emitError(new Error('Failed to run revert command'));