Skip to content

Commit b887ae4

Browse files
authored
simplify resource state logical id during import/clone (#108)
1 parent 1f2cda5 commit b887ae4

File tree

3 files changed

+106
-29
lines changed

3 files changed

+106
-29
lines changed

src/resourceState/ResourceStateImporter.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -189,21 +189,18 @@ export class ResourceStateImporter {
189189
}
190190

191191
private generateLogicalId(resourceType: string, identifier: string, existingLogicalIds?: Set<string>): string {
192-
// get Bucket from AWS::S3::Bucket or if malformed just the resourceType sanitized
193-
const resourceTypeParts = resourceType.split('::');
194-
const resourceTypeThirdPartOrFull = resourceTypeParts.pop() ?? resourceType.replaceAll(/[^a-zA-Z]/g, '');
195-
if (!existingLogicalIds?.has(resourceTypeThirdPartOrFull)) {
196-
return resourceTypeThirdPartOrFull;
192+
const parts = resourceType.split('::');
193+
const baseName = parts.length >= 3 ? parts[1] + parts[2] : parts[parts.length - 1];
194+
195+
if (!existingLogicalIds?.has(baseName)) {
196+
return baseName;
197197
}
198-
const cleanIdentifier = identifier.replaceAll(/[^a-zA-Z]/g, '');
199-
const typeAndId = resourceTypeThirdPartOrFull + cleanIdentifier.slice(0, 10);
200-
if (!existingLogicalIds?.has(typeAndId)) {
201-
return typeAndId;
198+
199+
let count = 1;
200+
while (existingLogicalIds.has(`${baseName}${count}`)) {
201+
count++;
202202
}
203-
return crypto
204-
.randomUUID()
205-
.replaceAll(/[^a-zA-Z]/g, '')
206-
.slice(0, 10);
203+
return `${baseName}${count}`;
207204
}
208205

209206
private getResourceSection(syntaxTree: SyntaxTree): ResourcesSection | undefined {

tst/unit/resourceState/ResourceStateImporter.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,4 +320,98 @@ describe('ResourceStateImporter', () => {
320320
expect(result.warning).toBeUndefined();
321321
});
322322
});
323+
324+
describe('Logical ID uniqueness', () => {
325+
it('should generate unique logical IDs with numeric suffixes', async () => {
326+
const uri = 'test://test-unique-ids.template';
327+
const initialContent = `{
328+
"AWSTemplateFormatVersion": "2010-09-09",
329+
"Resources": {
330+
"IAMRole": {
331+
"Type": "AWS::IAM::Role",
332+
"Properties": {
333+
"RoleName": "ExistingRole"
334+
}
335+
},
336+
"IAMRole1": {
337+
"Type": "AWS::IAM::Role",
338+
"Properties": {
339+
"RoleName": "ExistingRole1"
340+
}
341+
}
342+
}
343+
}`;
344+
345+
createAndRegisterDocument(uri, initialContent, DocumentType.JSON);
346+
347+
const mockResource = createMockResourceState('AWS::IAM::Role');
348+
const resourceSelections: ResourceSelection[] = [
349+
{
350+
resourceType: 'AWS::IAM::Role',
351+
resourceIdentifiers: [mockResource.identifier],
352+
},
353+
];
354+
355+
mockResourceStateManager.getResource.mockResolvedValue(mockResource);
356+
357+
const params: ResourceStateParams = {
358+
resourceSelections,
359+
textDocument: { uri } as any,
360+
range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } },
361+
context: { diagnostics: [], only: [], triggerKind: 1 },
362+
purpose: ResourceStatePurpose.IMPORT,
363+
};
364+
365+
const result = await importer.importResourceState(params);
366+
367+
expect(result.edit).toBeDefined();
368+
expect(result.edit!.changes![uri]).toBeDefined();
369+
370+
const textEdit = result.edit!.changes![uri][0];
371+
expect(textEdit.newText).toContain('"IAMRole2"');
372+
expect(textEdit.newText).not.toContain('"IAMRole"');
373+
expect(textEdit.newText).not.toContain('"IAMRole1"');
374+
});
375+
376+
it('should generate multiple unique logical IDs in same import', async () => {
377+
const uri = 'test://test-multiple-unique-ids.template';
378+
const initialContent = `{
379+
"AWSTemplateFormatVersion": "2010-09-09",
380+
"Resources": {}
381+
}`;
382+
383+
createAndRegisterDocument(uri, initialContent, DocumentType.JSON);
384+
385+
const mockResource1 = createMockResourceState('AWS::IAM::Role');
386+
const mockResource2 = createMockResourceState('AWS::IAM::Role');
387+
const mockResource3 = createMockResourceState('AWS::IAM::Role');
388+
389+
const resourceSelections: ResourceSelection[] = [
390+
{
391+
resourceType: 'AWS::IAM::Role',
392+
resourceIdentifiers: [mockResource1.identifier, mockResource2.identifier, mockResource3.identifier],
393+
},
394+
];
395+
396+
mockResourceStateManager.getResource.mockResolvedValue(mockResource1);
397+
398+
const params: ResourceStateParams = {
399+
resourceSelections,
400+
textDocument: { uri } as any,
401+
range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } },
402+
context: { diagnostics: [], only: [], triggerKind: 1 },
403+
purpose: ResourceStatePurpose.IMPORT,
404+
};
405+
406+
const result = await importer.importResourceState(params);
407+
408+
expect(result.edit).toBeDefined();
409+
expect(result.edit!.changes![uri]).toBeDefined();
410+
411+
const textEdit = result.edit!.changes![uri][0];
412+
expect(textEdit.newText).toContain('"IAMRole"');
413+
expect(textEdit.newText).toContain('"IAMRole1"');
414+
expect(textEdit.newText).toContain('"IAMRole2"');
415+
});
416+
});
323417
});

tst/unit/resourceState/StateImportExpectation.ts

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -58,22 +58,8 @@ Resources:
5858
];
5959

6060
function getResourceLogicalName(resourceType: string): string {
61-
const typeMap: Record<string, string> = {
62-
'AWS::S3::Bucket': 'Bucket',
63-
'AWS::EC2::Instance': 'Instance',
64-
'AWS::IAM::Role': 'Role',
65-
'AWS::Lambda::Function': 'Function',
66-
'AWS::EC2::VPC': 'VPC',
67-
'AWS::EC2::Subnet': 'Subnet',
68-
'AWS::EC2::SecurityGroup': 'SecurityGroup',
69-
'AWS::EC2::LaunchTemplate': 'LaunchTemplate',
70-
'AWS::AutoScaling::AutoScalingGroup': 'AutoScalingGroup',
71-
'AWS::RDS::DBInstance': 'DBInstance',
72-
'AWS::CloudWatch::Alarm': 'Alarm',
73-
'AWS::SNS::Topic': 'Topic',
74-
'AWS::SSM::Parameter': 'Parameter',
75-
};
76-
return typeMap[resourceType] || 'Resource';
61+
const parts = resourceType.split('::');
62+
return parts.length >= 3 ? parts[1] + parts[2] : parts[parts.length - 1];
7763
}
7864

7965
function formatPropertiesForJson(properties: any, indent: number = 6): string {

0 commit comments

Comments
 (0)