Skip to content

fix: custom resource error handling, formatting messages #14181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abhi7cr merged 1 commit into from
Apr 18, 2025
Merged

fix: custom resource error handling, formatting messages #14181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/amplify-migration-template-gen/src/migration-readme-generator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 : ''}
\`\`\`
${
Copy link
Copy Markdown
Contributor Author

@abhi7cr abhi7cr Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conditionally render the auth related section of README

this.categories.includes('auth')
? `\`\`\`
backend.auth.resources.userPool.node.tryRemoveChild('UserPoolDomain');
\`\`\`
\`\`\``
: ''
}

\`\`\`
Tags.of(backend.stack).add("gen1-migrated-app", "true");
Expand Down
52 changes: 36 additions & 16 deletions packages/amplify-migration-template-gen/src/template-generator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) {
Expand Down
64 changes: 46 additions & 18 deletions packages/amplify-migration/src/command-handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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',
Expand Down Expand Up @@ -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
});
});
42 changes: 34 additions & 8 deletions packages/amplify-migration/src/command-handlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand All @@ -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}`);
}
}

Expand All @@ -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`,
Expand All @@ -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",
Expand All @@ -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');
Expand Down Expand Up @@ -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 });
Expand All @@ -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() {
Expand Down Expand Up @@ -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'));
Expand Down
Loading