Skip to content

Commit d4f8c2f

Browse files
sai-raysai-ray
authored
fix(gen2-migration): retain GraphQL model tables during decommission (#14662)
* chore: mid work * chore: update logging * chore: update validations to exclude ddb tables with deletionPolicy as retain * chore: update tests * chore: add changeset validation before stack update * chore: add capabilities for changeset creation * fix(cli-internal): tighten changeset validation and add debug log --------- Co-authored-by: Sai Ray <saisujit@amazon.com>
1 parent 83351ca commit d4f8c2f

File tree

4 files changed

+414
-6
lines changed

4 files changed

+414
-6
lines changed

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,14 @@ describe('AmplifyGen2MigrationValidations', () => {
357357
],
358358
NextToken: undefined,
359359
});
360+
// GetTemplate response — DeletionPolicy is Delete
361+
mockCfnSend.mockResolvedValueOnce({
362+
TemplateBody: JSON.stringify({
363+
Resources: {
364+
Table: { Type: 'AWS::DynamoDB::Table', DeletionPolicy: 'Delete' },
365+
},
366+
}),
367+
});
360368

361369
const changeSet: DescribeChangeSetOutput = {
362370
Changes: [
@@ -378,6 +386,82 @@ describe('AmplifyGen2MigrationValidations', () => {
378386
});
379387
});
380388

389+
it('should pass when nested DynamoDB table has DeletionPolicy Retain', async () => {
390+
mockCfnSend.mockResolvedValueOnce({
391+
StackResourceSummaries: [
392+
{
393+
ResourceType: 'AWS::DynamoDB::Table',
394+
PhysicalResourceId: 'MyTable',
395+
LogicalResourceId: 'Table',
396+
},
397+
],
398+
NextToken: undefined,
399+
});
400+
// GetTemplate response — DeletionPolicy is Retain
401+
mockCfnSend.mockResolvedValueOnce({
402+
TemplateBody: JSON.stringify({
403+
Resources: {
404+
Table: { Type: 'AWS::DynamoDB::Table', DeletionPolicy: 'Retain' },
405+
},
406+
}),
407+
});
408+
409+
const changeSet: DescribeChangeSetOutput = {
410+
Changes: [
411+
{
412+
Type: 'Resource',
413+
ResourceChange: {
414+
Action: 'Remove',
415+
ResourceType: 'AWS::CloudFormation::Stack',
416+
LogicalResourceId: 'ApiStack',
417+
PhysicalResourceId: 'api-stack',
418+
},
419+
},
420+
],
421+
};
422+
423+
await expect(validations.validateStatefulResources(changeSet)).resolves.not.toThrow();
424+
});
425+
426+
it('should throw when nested DynamoDB table has no DeletionPolicy', async () => {
427+
mockCfnSend.mockResolvedValueOnce({
428+
StackResourceSummaries: [
429+
{
430+
ResourceType: 'AWS::DynamoDB::Table',
431+
PhysicalResourceId: 'MyTable',
432+
LogicalResourceId: 'Table',
433+
},
434+
],
435+
NextToken: undefined,
436+
});
437+
// GetTemplate response — no DeletionPolicy set
438+
mockCfnSend.mockResolvedValueOnce({
439+
TemplateBody: JSON.stringify({
440+
Resources: {
441+
Table: { Type: 'AWS::DynamoDB::Table' },
442+
},
443+
}),
444+
});
445+
446+
const changeSet: DescribeChangeSetOutput = {
447+
Changes: [
448+
{
449+
Type: 'Resource',
450+
ResourceChange: {
451+
Action: 'Remove',
452+
ResourceType: 'AWS::CloudFormation::Stack',
453+
LogicalResourceId: 'ApiStack',
454+
PhysicalResourceId: 'api-stack',
455+
},
456+
},
457+
],
458+
};
459+
460+
await expect(validations.validateStatefulResources(changeSet)).rejects.toMatchObject({
461+
name: 'DestructiveMigrationError',
462+
});
463+
});
464+
381465
it('should pass when nested stack contains only stateless resources', async () => {
382466
mockCfnSend.mockResolvedValueOnce({
383467
StackResourceSummaries: [

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

Lines changed: 155 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { AmplifyMigrationLockStep } from '../../../commands/gen2-migration/lock';
22
import { $TSContext } from '@aws-amplify/amplify-cli-core';
3-
import { SetStackPolicyCommand } from '@aws-sdk/client-cloudformation';
3+
import { CreateChangeSetCommand, DeleteChangeSetCommand, SetStackPolicyCommand } from '@aws-sdk/client-cloudformation';
44
import { UpdateAppCommand } from '@aws-sdk/client-amplify';
55
import { SpinningLogger } from '../../../commands/gen2-migration/_infra/spinning-logger';
66
import { Gen1App } from '../../../commands/gen2-migration/generate/_infra/gen1-app';
@@ -14,6 +14,11 @@ jest.mock('@aws-sdk/client-appsync', () => ({
1414
},
1515
})),
1616
}));
17+
jest.mock('@aws-sdk/client-cloudformation', () => ({
18+
...jest.requireActual('@aws-sdk/client-cloudformation'),
19+
waitUntilChangeSetCreateComplete: jest.fn().mockResolvedValue({}),
20+
waitUntilStackUpdateComplete: jest.fn().mockResolvedValue({}),
21+
}));
1722
jest.mock('@aws-sdk/client-dynamodb', () => ({
1823
...jest.requireActual('@aws-sdk/client-dynamodb'),
1924
paginateListTables: jest.fn().mockImplementation(() => ({
@@ -80,7 +85,10 @@ describe('AmplifyMigrationLockStep', () => {
8085

8186
describe('forward stack policy merge', () => {
8287
it('should append lock statement to empty stack policy', async () => {
83-
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: undefined }).mockResolvedValueOnce({});
88+
mockCfnSend
89+
.mockResolvedValueOnce({ StackResources: [] }) // DescribeStackResources for DeletionPolicy operation
90+
.mockResolvedValueOnce({ StackPolicyBody: undefined })
91+
.mockResolvedValueOnce({});
8492
mockAmplifySend.mockResolvedValueOnce({ app: { environmentVariables: {} } }).mockResolvedValueOnce({});
8593

8694
const plan = await lockStep.forward();
@@ -100,7 +108,10 @@ describe('AmplifyMigrationLockStep', () => {
100108
const existingPolicy = {
101109
Statement: [{ Effect: 'Deny', Action: 'Update:Replace', Principal: '*', Resource: 'LogicalResourceId/MyDB' }],
102110
};
103-
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: JSON.stringify(existingPolicy) }).mockResolvedValueOnce({});
111+
mockCfnSend
112+
.mockResolvedValueOnce({ StackResources: [] }) // DescribeStackResources for DeletionPolicy operation
113+
.mockResolvedValueOnce({ StackPolicyBody: JSON.stringify(existingPolicy) })
114+
.mockResolvedValueOnce({});
104115
mockAmplifySend.mockResolvedValueOnce({ app: { environmentVariables: {} } }).mockResolvedValueOnce({});
105116

106117
const plan = await lockStep.forward();
@@ -123,7 +134,9 @@ describe('AmplifyMigrationLockStep', () => {
123134
const alreadyLockedPolicy = {
124135
Statement: [{ Effect: 'Deny', Action: 'Update:*', Principal: '*', Resource: '*' }],
125136
};
126-
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: JSON.stringify(alreadyLockedPolicy) });
137+
mockCfnSend
138+
.mockResolvedValueOnce({ StackResources: [] }) // DescribeStackResources for DeletionPolicy operation
139+
.mockResolvedValueOnce({ StackPolicyBody: JSON.stringify(alreadyLockedPolicy) });
127140
mockAmplifySend.mockResolvedValueOnce({ app: { environmentVariables: {} } }).mockResolvedValueOnce({});
128141

129142
const plan = await lockStep.forward();
@@ -136,7 +149,10 @@ describe('AmplifyMigrationLockStep', () => {
136149

137150
describe('forward env var merge', () => {
138151
it('should merge new env var with existing env vars', async () => {
139-
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: undefined }).mockResolvedValueOnce({});
152+
mockCfnSend
153+
.mockResolvedValueOnce({ StackResources: [] }) // DescribeStackResources for DeletionPolicy operation
154+
.mockResolvedValueOnce({ StackPolicyBody: undefined })
155+
.mockResolvedValueOnce({});
140156
mockAmplifySend.mockResolvedValueOnce({ app: { environmentVariables: { EXISTING: 'value' } } }).mockResolvedValueOnce({});
141157

142158
const plan = await lockStep.forward();
@@ -249,4 +265,138 @@ describe('AmplifyMigrationLockStep', () => {
249265
});
250266
});
251267
});
268+
269+
describe('forward DeletionPolicy changeset validation', () => {
270+
const modelTemplate = {
271+
Resources: {
272+
TodoTable: { Type: 'AWS::DynamoDB::Table', Properties: {} },
273+
},
274+
};
275+
276+
function setupApiStackMocks() {
277+
// DescribeStackResources — root stack has one API nested stack
278+
mockCfnSend.mockResolvedValueOnce({
279+
StackResources: [
280+
{
281+
ResourceType: 'AWS::CloudFormation::Stack',
282+
LogicalResourceId: 'apitestapi',
283+
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/api-stack/abc',
284+
},
285+
],
286+
});
287+
// ListStackResources — API stack has one model nested stack
288+
mockCfnSend.mockResolvedValueOnce({
289+
StackResourceSummaries: [
290+
{
291+
ResourceType: 'AWS::CloudFormation::Stack',
292+
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/model-stack/def',
293+
},
294+
],
295+
});
296+
// GetTemplate — model stack template with DynamoDB table (no Retain)
297+
mockCfnSend.mockResolvedValueOnce({
298+
TemplateBody: JSON.stringify(modelTemplate),
299+
});
300+
// DescribeStacks — model stack parameters
301+
mockCfnSend.mockResolvedValueOnce({
302+
Stacks: [{ Parameters: [{ ParameterKey: 'env', ParameterValue: 'testEnv' }] }],
303+
});
304+
// CreateChangeSet
305+
mockCfnSend.mockResolvedValueOnce({});
306+
}
307+
308+
it('should validate and proceed when only DynamoDB and IAM Policy Modify changes', async () => {
309+
setupApiStackMocks();
310+
// DescribeChangeSet — Modify on DynamoDB table + IAM policy (expected side effect)
311+
mockCfnSend.mockResolvedValueOnce({
312+
Changes: [
313+
{ ResourceChange: { Action: 'Modify', ResourceType: 'AWS::DynamoDB::Table', LogicalResourceId: 'TodoTable' } },
314+
{ ResourceChange: { Action: 'Modify', ResourceType: 'AWS::IAM::Policy', LogicalResourceId: 'TodoIAMRoleDefaultPolicy' } },
315+
],
316+
});
317+
// DeleteChangeSet (cleanup)
318+
mockCfnSend.mockResolvedValueOnce({});
319+
// UpdateStack
320+
mockCfnSend.mockResolvedValueOnce({});
321+
// GetStackPolicy + SetStackPolicy for lock
322+
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: undefined });
323+
mockCfnSend.mockResolvedValueOnce({});
324+
// Amplify env var
325+
mockAmplifySend.mockResolvedValueOnce({ app: { environmentVariables: {} } }).mockResolvedValueOnce({});
326+
327+
const plan = await lockStep.forward();
328+
await plan.execute();
329+
330+
const createCalls = mockCfnSend.mock.calls.filter(([cmd]: [unknown]) => cmd instanceof CreateChangeSetCommand);
331+
expect(createCalls).toHaveLength(1);
332+
const deleteCalls = mockCfnSend.mock.calls.filter(([cmd]: [unknown]) => cmd instanceof DeleteChangeSetCommand);
333+
expect(deleteCalls).toHaveLength(1);
334+
});
335+
336+
it('should abort when changeset contains Add action', async () => {
337+
setupApiStackMocks();
338+
// DescribeChangeSet — unexpected Lambda change
339+
mockCfnSend.mockResolvedValueOnce({
340+
Changes: [{ ResourceChange: { Action: 'Add', ResourceType: 'AWS::Lambda::Function', LogicalResourceId: 'NewFunction' } }],
341+
});
342+
// DeleteChangeSet (cleanup in validation)
343+
mockCfnSend.mockResolvedValueOnce({});
344+
// GetStackPolicy + SetStackPolicy for lock (still runs after error is caught by runner)
345+
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: undefined });
346+
mockCfnSend.mockResolvedValueOnce({});
347+
// Amplify env var
348+
mockAmplifySend.mockResolvedValueOnce({ app: { environmentVariables: {} } }).mockResolvedValueOnce({});
349+
350+
const plan = await lockStep.forward();
351+
await expect(plan.execute()).rejects.toMatchObject({
352+
name: 'MigrationError',
353+
message: expect.stringContaining('unexpected changes'),
354+
});
355+
});
356+
357+
it('should abort when changeset contains Remove action on DynamoDB', async () => {
358+
setupApiStackMocks();
359+
// DescribeChangeSet — Remove on DynamoDB table
360+
mockCfnSend.mockResolvedValueOnce({
361+
Changes: [{ ResourceChange: { Action: 'Remove', ResourceType: 'AWS::DynamoDB::Table', LogicalResourceId: 'TodoTable' } }],
362+
});
363+
// DeleteChangeSet (cleanup in validation)
364+
mockCfnSend.mockResolvedValueOnce({});
365+
// GetStackPolicy + SetStackPolicy
366+
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: undefined });
367+
mockCfnSend.mockResolvedValueOnce({});
368+
// Amplify env var
369+
mockAmplifySend.mockResolvedValueOnce({ app: { environmentVariables: {} } }).mockResolvedValueOnce({});
370+
371+
const plan = await lockStep.forward();
372+
await expect(plan.execute()).rejects.toMatchObject({
373+
name: 'MigrationError',
374+
message: expect.stringContaining('unexpected changes'),
375+
});
376+
});
377+
378+
it('should abort when changeset contains Modify on unexpected resource type', async () => {
379+
setupApiStackMocks();
380+
// DescribeChangeSet — Modify on AppSync resolver (not in allowed set)
381+
mockCfnSend.mockResolvedValueOnce({
382+
Changes: [
383+
{ ResourceChange: { Action: 'Modify', ResourceType: 'AWS::DynamoDB::Table', LogicalResourceId: 'TodoTable' } },
384+
{ ResourceChange: { Action: 'Modify', ResourceType: 'AWS::AppSync::Resolver', LogicalResourceId: 'GetTodoResolver' } },
385+
],
386+
});
387+
// DeleteChangeSet (cleanup in validation)
388+
mockCfnSend.mockResolvedValueOnce({});
389+
// GetStackPolicy + SetStackPolicy
390+
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: undefined });
391+
mockCfnSend.mockResolvedValueOnce({});
392+
// Amplify env var
393+
mockAmplifySend.mockResolvedValueOnce({ app: { environmentVariables: {} } }).mockResolvedValueOnce({});
394+
395+
const plan = await lockStep.forward();
396+
await expect(plan.execute()).rejects.toMatchObject({
397+
name: 'MigrationError',
398+
message: expect.stringContaining('unexpected changes'),
399+
});
400+
});
401+
});
252402
});

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
DescribeStacksCommand,
66
ListStackResourcesCommand,
77
GetStackPolicyCommand,
8+
GetTemplateCommand,
89
} from '@aws-sdk/client-cloudformation';
910
import { STATEFUL_RESOURCES } from './stateful-resources';
1011
import CLITable from 'cli-table3';
@@ -204,6 +205,13 @@ export class AmplifyGen2MigrationValidations {
204205
logicalId: resource.LogicalResourceId,
205206
});
206207
} else if (resource.ResourceType && STATEFUL_RESOURCES.has(resource.ResourceType)) {
208+
if (resource.ResourceType === 'AWS::DynamoDB::Table') {
209+
const templateResponse = await this.gen1App.clients.cloudFormation.send(new GetTemplateCommand({ StackName: stackName }));
210+
const template = JSON.parse(templateResponse.TemplateBody);
211+
if (template.Resources[resource.LogicalResourceId].DeletionPolicy === 'Retain') {
212+
continue;
213+
}
214+
}
207215
const category = parentCategory || extractCategory(resource.LogicalResourceId || '');
208216
const physicalId = resource.PhysicalResourceId || 'N/A';
209217
this.logger.info(`Scanning '${category}' category: found stateful resource "${physicalId}"`);

0 commit comments

Comments
 (0)