Skip to content

Commit 4ff8d8c

Browse files
author
Sai Ray
committed
fix: add recursive nested stack validation for stateful resources
1 parent 7c9d56a commit 4ff8d8c

File tree

3 files changed

+210
-14
lines changed

3 files changed

+210
-14
lines changed

‎packages/amplify-cli/src/__tests__/commands/gen2-migration/_validations.test.ts‎

Lines changed: 174 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { AmplifyGen2MigrationValidations } from '../../../commands/gen2-migration/_validations';
22
import { $TSContext } from '@aws-amplify/amplify-cli-core';
3-
import { DescribeChangeSetOutput } from '@aws-sdk/client-cloudformation';
3+
import { CloudFormationClient, DescribeChangeSetOutput } from '@aws-sdk/client-cloudformation';
4+
5+
jest.mock('@aws-sdk/client-cloudformation');
46

57
describe('AmplifyGen2MigrationValidations', () => {
68
let mockContext: $TSContext;
@@ -317,4 +319,175 @@ describe('AmplifyGen2MigrationValidations', () => {
317319
});
318320
});
319321
});
322+
323+
describe('validateStatefulResources - nested stacks', () => {
324+
let mockSend: jest.Mock;
325+
326+
beforeEach(() => {
327+
mockSend = jest.fn();
328+
(CloudFormationClient as jest.Mock).mockImplementation(() => ({
329+
send: mockSend,
330+
}));
331+
});
332+
333+
afterEach(() => {
334+
jest.clearAllMocks();
335+
});
336+
337+
it('should throw when nested stack contains stateful resources', async () => {
338+
mockSend.mockResolvedValueOnce({
339+
StackResources: [
340+
{
341+
ResourceType: 'AWS::DynamoDB::Table',
342+
PhysicalResourceId: 'MyTable',
343+
LogicalResourceId: 'Table',
344+
},
345+
],
346+
});
347+
348+
const changeSet: DescribeChangeSetOutput = {
349+
Changes: [
350+
{
351+
Type: 'Resource',
352+
ResourceChange: {
353+
Action: 'Remove',
354+
ResourceType: 'AWS::CloudFormation::Stack',
355+
LogicalResourceId: 'AuthStack',
356+
PhysicalResourceId: 'auth-stack',
357+
},
358+
},
359+
],
360+
};
361+
362+
await expect(validations.validateStatefulResources(changeSet)).rejects.toMatchObject({
363+
name: 'DestructiveMigrationError',
364+
message: 'Stateful resources scheduled for deletion: AuthStack (AWS::CloudFormation::Stack).',
365+
});
366+
});
367+
368+
it('should pass when nested stack contains only stateless resources', async () => {
369+
mockSend.mockResolvedValueOnce({
370+
StackResources: [
371+
{
372+
ResourceType: 'AWS::Lambda::Function',
373+
PhysicalResourceId: 'MyFunction',
374+
LogicalResourceId: 'Function',
375+
},
376+
],
377+
});
378+
379+
const changeSet: DescribeChangeSetOutput = {
380+
Changes: [
381+
{
382+
Type: 'Resource',
383+
ResourceChange: {
384+
Action: 'Remove',
385+
ResourceType: 'AWS::CloudFormation::Stack',
386+
LogicalResourceId: 'LambdaStack',
387+
PhysicalResourceId: 'lambda-stack',
388+
},
389+
},
390+
],
391+
};
392+
393+
await expect(validations.validateStatefulResources(changeSet)).resolves.not.toThrow();
394+
});
395+
396+
it('should handle multiple levels of nested stacks', async () => {
397+
mockSend.mockResolvedValueOnce({
398+
StackResources: [
399+
{
400+
ResourceType: 'AWS::CloudFormation::Stack',
401+
PhysicalResourceId: 'storage-nested-stack',
402+
LogicalResourceId: 'StorageNestedStack',
403+
},
404+
],
405+
});
406+
407+
mockSend.mockResolvedValueOnce({
408+
StackResources: [
409+
{
410+
ResourceType: 'AWS::S3::Bucket',
411+
PhysicalResourceId: 'my-bucket',
412+
LogicalResourceId: 'Bucket',
413+
},
414+
],
415+
});
416+
417+
const changeSet: DescribeChangeSetOutput = {
418+
Changes: [
419+
{
420+
Type: 'Resource',
421+
ResourceChange: {
422+
Action: 'Remove',
423+
ResourceType: 'AWS::CloudFormation::Stack',
424+
LogicalResourceId: 'StorageStack',
425+
PhysicalResourceId: 'storage-stack',
426+
},
427+
},
428+
],
429+
};
430+
431+
await expect(validations.validateStatefulResources(changeSet)).rejects.toMatchObject({
432+
name: 'DestructiveMigrationError',
433+
});
434+
});
435+
436+
it('should pass when nested stack is missing PhysicalResourceId', async () => {
437+
const changeSet: DescribeChangeSetOutput = {
438+
Changes: [
439+
{
440+
Type: 'Resource',
441+
ResourceChange: {
442+
Action: 'Remove',
443+
ResourceType: 'AWS::CloudFormation::Stack',
444+
LogicalResourceId: 'IncompleteStack',
445+
PhysicalResourceId: undefined,
446+
},
447+
},
448+
],
449+
};
450+
451+
await expect(validations.validateStatefulResources(changeSet)).resolves.not.toThrow();
452+
});
453+
454+
it('should handle mixed direct and nested stateful resources', async () => {
455+
mockSend.mockResolvedValueOnce({
456+
StackResources: [
457+
{
458+
ResourceType: 'AWS::Cognito::UserPool',
459+
PhysicalResourceId: 'user-pool',
460+
LogicalResourceId: 'UserPool',
461+
},
462+
],
463+
});
464+
465+
const changeSet: DescribeChangeSetOutput = {
466+
Changes: [
467+
{
468+
Type: 'Resource',
469+
ResourceChange: {
470+
Action: 'Remove',
471+
ResourceType: 'AWS::DynamoDB::Table',
472+
LogicalResourceId: 'DirectTable',
473+
},
474+
},
475+
{
476+
Type: 'Resource',
477+
ResourceChange: {
478+
Action: 'Remove',
479+
ResourceType: 'AWS::CloudFormation::Stack',
480+
LogicalResourceId: 'AuthStack',
481+
PhysicalResourceId: 'auth-stack',
482+
},
483+
},
484+
],
485+
};
486+
487+
await expect(validations.validateStatefulResources(changeSet)).rejects.toMatchObject({
488+
name: 'DestructiveMigrationError',
489+
message: expect.stringContaining('DirectTable'),
490+
});
491+
});
492+
});
320493
});

‎packages/amplify-cli/src/commands/gen2-migration/_validations.ts‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { AmplifyDriftDetector } from '../drift';
22
import { $TSContext, AmplifyError } from '@aws-amplify/amplify-cli-core';
33
import { printer } from '@aws-amplify/amplify-prompts';
4-
import { DescribeChangeSetOutput } from '@aws-sdk/client-cloudformation';
4+
import { CloudFormationClient, DescribeChangeSetOutput, DescribeStackResourcesCommand } from '@aws-sdk/client-cloudformation';
55
import { STATEFUL_RESOURCES } from './stateful-resources';
66

77
export class AmplifyGen2MigrationValidations {
@@ -31,20 +31,27 @@ export class AmplifyGen2MigrationValidations {
3131
public async validateStatefulResources(changeSet: DescribeChangeSetOutput): Promise<void> {
3232
if (!changeSet.Changes) return;
3333

34-
const statefulRemoves = changeSet.Changes.filter(
35-
(change) =>
36-
change.Type === 'Resource' &&
37-
change.ResourceChange?.Action === 'Remove' &&
38-
change.ResourceChange?.ResourceType &&
39-
STATEFUL_RESOURCES.has(change.ResourceChange.ResourceType),
40-
);
34+
const statefulRemoves: string[] = [];
35+
for (const change of changeSet.Changes) {
36+
if (change.Type === 'Resource' && change.ResourceChange?.Action === 'Remove' && change.ResourceChange?.ResourceType) {
37+
if (change.ResourceChange.ResourceType === 'AWS::CloudFormation::Stack' && change.ResourceChange.PhysicalResourceId) {
38+
const nestedResources = await this.getStatefulResources(change.ResourceChange.PhysicalResourceId);
39+
if (nestedResources.length > 0) {
40+
statefulRemoves.push(
41+
`${change.ResourceChange.LogicalResourceId} (${change.ResourceChange.ResourceType}) containing: ${nestedResources.join(
42+
', ',
43+
)}`,
44+
);
45+
}
46+
} else if (STATEFUL_RESOURCES.has(change.ResourceChange.ResourceType)) {
47+
statefulRemoves.push(`${change.ResourceChange.LogicalResourceId} (${change.ResourceChange.ResourceType})`);
48+
}
49+
}
50+
}
4151

4252
if (statefulRemoves.length > 0) {
43-
const resources = statefulRemoves
44-
.map((c) => `${c.ResourceChange?.LogicalResourceId ?? 'Unknown'} (${c.ResourceChange?.ResourceType})`)
45-
.join(', ');
4653
throw new AmplifyError('DestructiveMigrationError', {
47-
message: `Stateful resources scheduled for deletion: ${resources}.`,
54+
message: `Stateful resources scheduled for deletion: ${statefulRemoves.join(', ')}.`,
4855
resolution: 'Review the migration plan and ensure data is backed up before proceeding.',
4956
});
5057
}
@@ -53,4 +60,20 @@ export class AmplifyGen2MigrationValidations {
5360
public async validateIngressTraffic(): Promise<void> {
5461
printer.warn('Not implemented');
5562
}
63+
64+
private async getStatefulResources(stackName: string): Promise<string[]> {
65+
const statefulResources: string[] = [];
66+
const cfn = new CloudFormationClient({});
67+
const { StackResources } = await cfn.send(new DescribeStackResourcesCommand({ StackName: stackName }));
68+
69+
for (const resource of StackResources ?? []) {
70+
if (resource.ResourceType === 'AWS::CloudFormation::Stack' && resource.PhysicalResourceId) {
71+
const nested = await this.getStatefulResources(resource.PhysicalResourceId);
72+
statefulResources.push(...nested);
73+
} else if (resource.ResourceType && STATEFUL_RESOURCES.has(resource.ResourceType)) {
74+
statefulResources.push(`${resource.LogicalResourceId} (${resource.ResourceType})`);
75+
}
76+
}
77+
return statefulResources;
78+
}
5679
}

‎packages/amplify-cli/src/commands/gen2-migration/stateful-resources.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
export const STATEFUL_RESOURCES = new Set([
77
'AWS::Backup::BackupVault',
8-
'AWS::CloudFormation::Stack',
8+
// 'AWS::CloudFormation::Stack',
99
'AWS::Cognito::UserPool',
1010
'AWS::DocDB::DBCluster',
1111
'AWS::DocDB::DBInstance',

0 commit comments

Comments
 (0)