chore: fix cdk migrate not deleting templates and lack of deduplicati…

…on (#228)

* fix bugs

* only create cfnTemplateGenerator if we need it

* fix integ tests

---------

Co-authored-by: Hogan Bobertz <bobertzh@amazon.com>

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts

@@ -600,20 +600,19 @@ integTest('cdk migrate generates migrate.json', withCDKMigrateFixture('typescrip
 }));
 
 integTest('cdk migrate --from-scan with AND/OR filters correctly filters resources', withDefaultFixture(async (fixture) => {
-  const migrateStackJr = 'migrate-stack-jr';
+  const stackName = `cdk-migrate-integ-${fixture.randomString}`;
 
   await fixture.cdkDeploy('migrate-stack', {
     modEnv: { SAMPLE_RESOURCES: '1' },
   });
   await fixture.cdk(
-    ['migrate', '--stack-name', migrateStackJr, '--from-scan', 'new', '--filter', 'type=AWS::SNS::Topic,tag-key=tag1', 'type=AWS::SQS::Queue,tag-key=tag3'],
+    ['migrate', '--stack-name', stackName, '--from-scan', 'new', '--filter', 'type=AWS::SNS::Topic,tag-key=tag1', 'type=AWS::SQS::Queue,tag-key=tag3'],
     { modEnv: { MIGRATE_INTEG_TEST: '1' }, neverRequireApproval: true, verbose: true, captureStderr: false },
   );
 
   try {
-
     const response = await fixture.aws.cloudFormation('describeGeneratedTemplate', {
-      GeneratedTemplateName: migrateStackJr,
+      GeneratedTemplateName: stackName,
     });
     const resourceNames = [];
     for (const resource of response.Resources || []) {
@@ -624,9 +623,11 @@ integTest('cdk migrate --from-scan with AND/OR filters correctly filters resourc
     fixture.log(`Resources: ${resourceNames}`);
     expect(resourceNames.some(ele => ele && ele.includes('migratetopic1'))).toBeTruthy();
     expect(resourceNames.some(ele => ele && ele.includes('migratequeue1'))).toBeTruthy();
-    expect(response.Resources?.length).toEqual(2);
   } finally {
     await fixture.cdkDestroy('migrate-stack');
+    await fixture.aws.cloudFormation('deleteGeneratedTemplate', {
+      GeneratedTemplateName: stackName,
+    });
   }
 }));
 
@@ -663,6 +664,9 @@ integTest('cdk migrate --from-scan for resources with Write Only Properties gene
     }
   } finally {
     await fixture.cdkDestroy('migrate-stack');
+    await fixture.aws.cloudFormation('deleteGeneratedTemplate', {
+      GeneratedTemplateName: stackName,
+    });
   }
 }));
 

packages/aws-cdk/lib/cdk-toolkit.ts

@@ -735,10 +735,12 @@ export class CdkToolkit {
   public async migrate(options: MigrateOptions): Promise<void> {
     warning('This is an experimental feature and development on it is still in progress. We make no guarantees about the outcome or stability of the functionality.');
     const language = options.language?.toLowerCase() ?? 'typescript';
+    const environment = setEnvironment(options.account, options.region);
+    let generateTemplateOutput: GenerateTemplateOutput | undefined;
+    let cfn: CfnTemplateGeneratorProvider | undefined;
+    let templateToDelete: string | undefined;
 
     try {
-      const environment = setEnvironment(options.account, options.region);
-      let generateTemplateOutput: GenerateTemplateOutput | undefined;
       // if neither fromPath nor fromStack is provided, generate a template using cloudformation
       const scanType = parseSourceOptions(options.fromPath, options.fromStack, options.stackName).source;
       if (scanType == TemplateSourceOptions.SCAN) {
@@ -749,6 +751,7 @@ export class CdkToolkit {
           sdkProvider: this.props.sdkProvider,
           environment: environment,
         });
+        templateToDelete = generateTemplateOutput.templateId;
       } else if (scanType == TemplateSourceOptions.PATH) {
         const templateBody = readFromPath(options.fromPath!);
 
@@ -757,7 +760,7 @@ export class CdkToolkit {
         if (templateId) {
           // if we have a template id, we can call describe generated template to get the resource identifiers
           // resource metadata, and template source to generate the template
-          const cfn = new CfnTemplateGeneratorProvider(await buildCfnClient(this.props.sdkProvider, environment));
+          cfn = new CfnTemplateGeneratorProvider(await buildCfnClient(this.props.sdkProvider, environment));
           const generatedTemplateSummary = await cfn.describeGeneratedTemplate(templateId);
           generateTemplateOutput = buildGenertedTemplateOutput(generatedTemplateSummary, templateBody, generatedTemplateSummary.GeneratedTemplateId!);
         } else {
@@ -796,6 +799,15 @@ export class CdkToolkit {
     } catch (e) {
       error(' ❌  Migrate failed for `%s`: %s', options.stackName, (e as Error).message);
       throw e;
+    } finally {
+      if (templateToDelete) {
+        if (!cfn) {
+          cfn = new CfnTemplateGeneratorProvider(await buildCfnClient(this.props.sdkProvider, environment));
+        }
+        if (!process.env.MIGRATE_INTEG_TEST) {
+          await cfn.deleteGeneratedTemplate(templateToDelete);
+        }
+      }
     }
   }
 

packages/aws-cdk/lib/commands/migrate.ts

@@ -539,9 +539,11 @@ export function buildGenertedTemplateOutput(generatedTemplateSummary: CloudForma
       ResourceIdentifier: r.ResourceIdentifier!,
     })),
   };
+  const templateId = generatedTemplateSummary.GeneratedTemplateId!;
   return {
     migrateJson: migrateJson,
     resources: resources,
+    templateId: templateId,
   };
 }
 
@@ -590,6 +592,27 @@ export function appendWarningsToReadme(filepath: string, resources: CloudFormati
   fs.writeFileSync(filepath, lines.join('\n'));
 }
 
+/**
+ * takes a list of resources and returns a list of unique resources based on the resource type and logical resource id.
+ *
+ * @param resources A list of resources to deduplicate
+ * @returns A list of unique resources
+ */
+function deduplicateResources(resources: CloudFormation.ResourceDetails) {
+  let uniqueResources: {[key: string]: CloudFormation.ResourceDetail} = {};
+
+  for (const resource of resources) {
+    const key = Object.keys(resource.ResourceIdentifier!)[0];
+
+    // Creating our unique identifier using the resource type, the key, and the value of the resource identifier
+    // The resource identifier is a combination of a key value pair defined by a resource's schema, and the resource type of the resource.
+    const uniqueIdentifer = `${resource.ResourceType}:${key}:${resource.ResourceIdentifier![key]}`;
+    uniqueResources[uniqueIdentifer] = resource;
+  }
+
+  return Object.values(uniqueResources);
+};
+
 /**
  * Class for making CloudFormation template generator calls
  */
@@ -647,23 +670,7 @@ export class CfnTemplateGeneratorProvider {
       }
     }
 
-    let uniqueResources: {[key: string]: CloudFormation.ScannedResource} = {};
-
-    // de-duplicating the list of related resources based on the resource identifier value
-    // The resource identifier key is not known at compile time so we need to get it using Object.keys. There will only
-    // ever be a single key so [0] retrives the first and only key
-    for (const resource of relatedResourceList) {
-      if (!resource.ResourceIdentifier) continue;
-      const key = Object.keys(resource.ResourceIdentifier!)[0];
-
-      // Creating our unique identifier using the resource type, the key, and the value of the resource identifier
-      // The resource identifier is a combination of a key value pair defined by a resource's schema, and the resource type of the resource.
-      const uniqueIdentifer = `${resource.ResourceType}:${key}:${resource.ResourceIdentifier![key]}`;
-      uniqueResources[uniqueIdentifer] = resource;
-    }
-
-    // convert the dictionary back into a list
-    relatedResourceList = Object.values(uniqueResources);
+    relatedResourceList = deduplicateResources(relatedResourceList);
 
     // prune the managedbystack flag off of them again.
     return process.env.MIGRATE_INTEG_TEST ? resourceIdentifiers(relatedResourceList) : resourceIdentifiers(excludeManaged(relatedResourceList)) ;
@@ -750,6 +757,7 @@ export class CfnTemplateGeneratorProvider {
     if (resourceList.length === 0) {
       throw new Error(`No resources found with filters ${filters.join(' ')}. Please try again with different filters.`);
     }
+    resourceList = deduplicateResources(resourceList);
 
     return process.env.MIGRATE_INTEG_TEST ? resourceIdentifiers(resourceList) : resourceIdentifiers(excludeManaged(resourceList));
   }
@@ -822,6 +830,18 @@ export class CfnTemplateGeneratorProvider {
     }
     return createTemplateOutput;
   }
+
+  /**
+   * Deletes a generated template from the template generator.
+   *
+   * @param templateArn The arn of the template to delete
+   * @returns A promise that resolves when the template has been deleted
+   */
+  async deleteGeneratedTemplate(templateArn: string): Promise<void> {
+    await this.cfn.deleteGeneratedTemplate({
+      GeneratedTemplateName: templateArn,
+    }).promise();
+  }
 }
 
 /**
@@ -870,6 +890,7 @@ export interface GenerateTemplateOptions {
 export interface GenerateTemplateOutput {
   migrateJson: MigrateJsonFormat;
   resources?: CloudFormation.ResourceDetails;
+  templateId?: string;
 }
 
 /**