Skip to content

Commit b96c3c8

Browse files
iliapoloiliapolo
authored
fix(cli): gen2-migration lock doesn't locate all api model nested stacks (#14880)
* fix(cli-internal): replace DescribeStackResources with paginated ListStackResources DescribeStackResources silently truncates results at 100 resources. Replace all usages in gen2-migration with paginateListStackResources which handles arbitrarily large stacks. Added listNestedStacks, listStackResources, and findResourcePhysicalId methods to AwsFetcher. Updated StackFacade, lock, retain, kinesis generator, and geo generator to use the new paginated APIs. Changed the StackResource type to StackResourceSummary throughout. --- Prompt: add a new method to AwsFetcher - listNestedStack, implement with a paginator of listStackResources and replace all usages of DescribeStackResourcesCommand with that. * chore: remove 50 limit in mood board test
1 parent 2d7f345 commit b96c3c8

19 files changed

Lines changed: 250 additions & 229 deletions

File tree

‎amplify-migration-apps/mood-board/_snapshot.pre.generate/amplify/backend/api/moodboard/resolvers/Query.listBoards.req.vtl‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
## [Start] Custom List Request - Override to cap results at 50. **
22
#set( $args = $util.defaultIfNull($ctx.stash.transformedArgs, $ctx.args) )
33
#set( $limit = $util.defaultIfNull($args.limit, 50) )
4-
## Cap the limit at 50 to keep the board list manageable
5-
#if( $limit > 50 )
6-
#set( $limit = 50 )
7-
#end
84
#set( $ListRequest = {
95
"version": "2018-05-29",
106
"limit": $limit

‎packages/amplify-cli/src/__tests__/commands/gen2-migration/_framework/clients/cloudformation.ts‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import * as path from 'path';
1515
*
1616
* Mocks five commands:
1717
*
18-
* - `DescribeStackResourcesCommand`: Lists resources in a stack by parsing the
18+
* - `ListStackResourcesCommand`: Lists resources in a stack by parsing the
1919
* `Resources` section of the corresponding local CloudFormation template.
2020
*
2121
* - `DescribeStacksCommand`: Returns stack parameters and outputs for a nested stack.
@@ -47,7 +47,7 @@ export class CloudFormationMock {
4747
this._templateForStack.set(stackName, fs.readFileSync(path.join(refactorInputPath, stackFile), { encoding: 'utf-8' }));
4848
}
4949

50-
this.mockDescribeStackResources();
50+
this.mockListStackResources();
5151
this.mockDescribeStacks();
5252
this.mockGetTemplate();
5353
this.mockCreateStackRefactor();
@@ -68,42 +68,39 @@ export class CloudFormationMock {
6868

6969
/**
7070
* Pre-registers a physical resource ID → stack name mapping.
71-
* Used when the new Gen1App code path bypasses DescribeStackResources.
71+
* Used when the new Gen1App code path bypasses ListStackResources.
7272
*/
7373
public registerResource(physicalId: string, stackName: string): void {
7474
this._stackNameForResource.set(physicalId, stackName);
7575
}
7676

77-
private mockDescribeStackResources() {
77+
private mockListStackResources() {
7878
this.mock
79-
.on(cloudformation.DescribeStackResourcesCommand)
80-
.callsFake(async (input: cloudformation.DescribeStackResourcesInput): Promise<cloudformation.DescribeStackResourcesOutput> => {
79+
.on(cloudformation.ListStackResourcesCommand)
80+
.callsFake(async (input: cloudformation.ListStackResourcesInput): Promise<cloudformation.ListStackResourcesOutput> => {
8181
const templatePath = this.app.templatePathForStack(input.StackName!);
8282
const template: any = JSONUtilities.readJson<any>(templatePath);
83-
const stackResources: cloudformation.StackResource[] = [];
83+
const stackResourceSummaries: cloudformation.StackResourceSummary[] = [];
8484
for (const logicalId of Object.keys(template.Resources)) {
85-
if (input.LogicalResourceId && logicalId !== input.LogicalResourceId) {
86-
continue;
87-
}
8885
const resource = template.Resources[logicalId];
8986
const physicalId =
9087
resource.Type === 'AWS::CloudFormation::Stack'
9188
? this.app.nestedStackName(input.StackName!, logicalId)
9289
: this.app.physicalId(input.StackName!, logicalId) ?? `${input.StackName}/${logicalId}`;
93-
stackResources.push({
90+
stackResourceSummaries.push({
9491
LogicalResourceId: logicalId,
9592
PhysicalResourceId: physicalId,
9693
ResourceType: resource.Type,
97-
Timestamp: undefined,
98-
ResourceStatus: undefined,
94+
LastUpdatedTimestamp: new Date(),
95+
ResourceStatus: cloudformation.ResourceStatus.CREATE_COMPLETE,
9996
});
10097

10198
// remember which stack has the resource because we are going to get
10299
// asked later on.
103100
this._stackNameForResource.set(physicalId, input.StackName!);
104101
}
105102

106-
return { StackResources: stackResources };
103+
return { StackResourceSummaries: stackResourceSummaries };
107104
});
108105
}
109106

‎packages/amplify-cli/src/__tests__/commands/gen2-migration/generate/amplify/analytics/kinesis.generator.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ describe('AnalyticsKinesisGenerator', () => {
4040
},
4141
});
4242
jest.spyOn(gen1App, 'json').mockReturnValue({ Parameters: {}, Resources: {}, Conditions: {} });
43+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
4344
(gen1App.clients as any).cloudFormation = { send: jest.fn() };
4445

4546
const generator = new AnalyticsKinesisGenerator(
@@ -73,6 +74,7 @@ describe('AnalyticsKinesisGenerator', () => {
7374
},
7475
});
7576
jest.spyOn(gen1App, 'json').mockReturnValue({ Parameters: {}, Resources: {}, Conditions: {} });
77+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
7678
(gen1App.clients as any).cloudFormation = {
7779
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
7880
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {
@@ -155,6 +157,7 @@ describe('AnalyticsKinesisGenerator', () => {
155157
},
156158
});
157159
jest.spyOn(gen1App, 'json').mockReturnValue({ Parameters: {}, Resources: {}, Conditions: {} });
160+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
158161
(gen1App.clients as any).cloudFormation = {
159162
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
160163
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {
@@ -237,6 +240,7 @@ describe('AnalyticsKinesisGenerator', () => {
237240
},
238241
});
239242
jest.spyOn(gen1App, 'json').mockReturnValue({ Parameters: {}, Resources: {}, Conditions: {} });
243+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
240244
(gen1App.clients as any).cloudFormation = {
241245
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
242246
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {
@@ -294,6 +298,7 @@ describe('AnalyticsKinesisGenerator', () => {
294298
},
295299
Conditions: {},
296300
});
301+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
297302
(gen1App.clients as any).cloudFormation = {
298303
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
299304
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {

‎packages/amplify-cli/src/__tests__/commands/gen2-migration/generate/amplify/geo/geo.generator.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ describe('GeoGenerator', () => {
6767
},
6868
Conditions: {},
6969
});
70+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
7071
(gen1App.clients as any).cloudFormation = {
7172
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
7273
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {
@@ -154,6 +155,7 @@ describe('GeoGenerator', () => {
154155
},
155156
Conditions: {},
156157
});
158+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
157159
(gen1App.clients as any).cloudFormation = {
158160
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
159161
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {
@@ -263,6 +265,7 @@ describe('GeoGenerator', () => {
263265
},
264266
Conditions: {},
265267
});
268+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
266269
(gen1App.clients as any).cloudFormation = {
267270
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
268271
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {
@@ -366,6 +369,7 @@ describe('GeoGenerator', () => {
366369
},
367370
Conditions: {},
368371
});
372+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
369373
(gen1App.clients as any).cloudFormation = {
370374
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
371375
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {
@@ -453,6 +457,7 @@ describe('GeoGenerator', () => {
453457
},
454458
});
455459
jest.spyOn(gen1App, 'json').mockReturnValue({ Parameters: {}, Resources: {}, Conditions: {} });
460+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
456461
(gen1App.clients as any).cloudFormation = {
457462
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
458463
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {
@@ -534,6 +539,7 @@ describe('GeoGenerator', () => {
534539
},
535540
Conditions: {},
536541
});
542+
jest.spyOn(gen1App.aws, 'findResourcePhysicalId').mockResolvedValue('nested-stack-id');
537543
(gen1App.clients as any).cloudFormation = {
538544
send: jest.fn().mockImplementation((cmd: { constructor: { name: string } }) => {
539545
if (cmd.constructor.name === 'DescribeStackResourcesCommand') {

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

Lines changed: 35 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,32 @@ describe('AmplifyMigrationLockStep', () => {
7070
appSync: { send: jest.fn() },
7171
dynamoDB: { send: jest.fn() },
7272
},
73+
aws: {
74+
listNestedStacks: jest.fn().mockImplementation((stackName: string) => {
75+
if (stackName === 'test-root-stack') {
76+
return [
77+
{
78+
LogicalResourceId: 'apitestApp',
79+
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/api-stack/abc',
80+
ResourceType: 'AWS::CloudFormation::Stack',
81+
},
82+
];
83+
}
84+
// api nested stack → model table stacks
85+
return [
86+
{
87+
LogicalResourceId: 'Table1',
88+
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/model-stack-1/def',
89+
ResourceType: 'AWS::CloudFormation::Stack',
90+
},
91+
{
92+
LogicalResourceId: 'Table2',
93+
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/model-stack-2/ghi',
94+
ResourceType: 'AWS::CloudFormation::Stack',
95+
},
96+
];
97+
}),
98+
},
7399
} as unknown as Gen1App,
74100
{} as $TSContext,
75101
{
@@ -87,29 +113,6 @@ describe('AmplifyMigrationLockStep', () => {
87113

88114
/** Mocks the forward() planning phase only (nested stack discovery + changeset creation for 2 model tables). */
89115
function setupForwardPlanningMocks() {
90-
mockCfnSend.mockResolvedValueOnce({
91-
StackResources: [
92-
{
93-
ResourceType: 'AWS::CloudFormation::Stack',
94-
LogicalResourceId: 'apitestApp',
95-
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/api-stack/abc',
96-
},
97-
],
98-
});
99-
mockCfnSend.mockResolvedValueOnce({
100-
StackResources: [
101-
{
102-
ResourceType: 'AWS::CloudFormation::Stack',
103-
LogicalResourceId: 'Table1',
104-
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/model-stack-1/def',
105-
},
106-
{
107-
ResourceType: 'AWS::CloudFormation::Stack',
108-
LogicalResourceId: 'Table2',
109-
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/model-stack-2/ghi',
110-
},
111-
],
112-
});
113116
for (let i = 1; i <= 2; i++) {
114117
mockCfnSend.mockResolvedValueOnce({ TemplateBody: JSON.stringify(modelTemplate) });
115118
mockCfnSend.mockResolvedValueOnce({ Stacks: [{ Parameters: [{ ParameterKey: 'env', ParameterValue: 'testEnv' }] }] });
@@ -197,13 +200,7 @@ describe('AmplifyMigrationLockStep', () => {
197200
});
198201

199202
describe('rollback stack policy removal', () => {
200-
/** Mocks the listNestedStack call that rollback now performs. */
201-
function setupRollbackNestedStackMock() {
202-
mockCfnSend.mockResolvedValueOnce({ StackResources: [] });
203-
}
204-
205203
it('should remove lock statement and preserve customer statements', async () => {
206-
setupRollbackNestedStackMock();
207204
const policy = {
208205
Statement: [
209206
{ Effect: 'Deny', Action: 'Update:Replace', Principal: '*', Resource: 'LogicalResourceId/MyDB' },
@@ -227,7 +224,6 @@ describe('AmplifyMigrationLockStep', () => {
227224
});
228225

229226
it('should set allow-all when lock statement was the only one', async () => {
230-
setupRollbackNestedStackMock();
231227
const policy = { Statement: [{ Effect: 'Deny', Action: 'Update:*', Principal: '*', Resource: '*' }] };
232228
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: JSON.stringify(policy) }).mockResolvedValueOnce({});
233229
mockAmplifySend
@@ -244,7 +240,6 @@ describe('AmplifyMigrationLockStep', () => {
244240
});
245241

246242
it('should skip SetStackPolicy when no existing policy (lock not found)', async () => {
247-
setupRollbackNestedStackMock();
248243
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: undefined });
249244
mockAmplifySend
250245
.mockResolvedValueOnce({ app: { environmentVariables: { GEN2_MIGRATION_ENVIRONMENT_NAME: 'testEnv' } } })
@@ -255,7 +250,6 @@ describe('AmplifyMigrationLockStep', () => {
255250
});
256251

257252
it('should skip SetStackPolicy when lock statement is not found', async () => {
258-
setupRollbackNestedStackMock();
259253
const policy = { Statement: [{ Effect: 'Deny', Action: 'Update:Replace', Principal: '*', Resource: 'LogicalResourceId/MyDB' }] };
260254
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: JSON.stringify(policy) });
261255
mockAmplifySend
@@ -269,7 +263,6 @@ describe('AmplifyMigrationLockStep', () => {
269263

270264
describe('rollback env var removal', () => {
271265
it('should remove GEN2_MIGRATION_ENVIRONMENT_NAME and preserve other env vars', async () => {
272-
mockCfnSend.mockResolvedValueOnce({ StackResources: [] });
273266
mockCfnSend.mockResolvedValueOnce({ StackPolicyBody: undefined });
274267
mockAmplifySend
275268
.mockResolvedValueOnce({ app: { environmentVariables: { GEN2_MIGRATION_ENVIRONMENT_NAME: 'testEnv', OTHER: 'keep' } } })
@@ -308,6 +301,15 @@ describe('AmplifyMigrationLockStep', () => {
308301
appSync: { send: jest.fn() },
309302
dynamoDB: { send: jest.fn() },
310303
},
304+
aws: {
305+
listNestedStacks: jest.fn().mockResolvedValue([
306+
{
307+
LogicalResourceId: 'storagemyTable',
308+
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/storage-stack/abc',
309+
ResourceType: 'AWS::CloudFormation::Stack',
310+
},
311+
]),
312+
},
311313
} as unknown as Gen1App,
312314
{} as $TSContext,
313315
{
@@ -318,16 +320,6 @@ describe('AmplifyMigrationLockStep', () => {
318320
});
319321

320322
it('should pass validation when all local resources exist in deployed template', async () => {
321-
// listNestedStack
322-
mockCfnSend.mockResolvedValueOnce({
323-
StackResources: [
324-
{
325-
ResourceType: 'AWS::CloudFormation::Stack',
326-
LogicalResourceId: 'storagemyTable',
327-
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/storage-stack/abc',
328-
},
329-
],
330-
});
331323
// fetchTemplate for the nested stack
332324
mockCfnSend.mockResolvedValueOnce({
333325
TemplateBody: JSON.stringify({
@@ -345,16 +337,6 @@ describe('AmplifyMigrationLockStep', () => {
345337
});
346338

347339
it('should fail validation when a resource is missing from the deployed template', async () => {
348-
// listNestedStack
349-
mockCfnSend.mockResolvedValueOnce({
350-
StackResources: [
351-
{
352-
ResourceType: 'AWS::CloudFormation::Stack',
353-
LogicalResourceId: 'storagemyTable',
354-
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/storage-stack/abc',
355-
},
356-
],
357-
});
358340
// fetchTemplate - missing TablePolicy
359341
mockCfnSend.mockResolvedValueOnce({
360342
TemplateBody: JSON.stringify({

0 commit comments

Comments
 (0)