Skip to content

Commit b196b13

Browse files
authoredJun 17, 2024··
fix(core): overrideLogicalId validation (#29708)
### Issue # (if applicable) Closes #29701 ### Reason for this change Calling `overrideLogicalId` on a `Construct` with an invalid logical ID ([docs](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/resources-section-structure.html#resources-section-structure-logicalid)) would not throw an error at synthesis time. CloudFormation would ### Description of changes * Validate `overrideLogicalId` (must not be empty, must not be over 255 characters, must match `/^[A-Za-z0-9]+$/` * Document exceptions with `@error` JSDoc tags ### Description of how you validated changes I've added unit tests, integration tests should not be necessary ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 25835e4 commit b196b13

File tree

2 files changed

+99
-8
lines changed

2 files changed

+99
-8
lines changed
 

‎packages/aws-cdk-lib/core/lib/cfn-element.ts

+21-2
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,33 @@ export abstract class CfnElement extends Construct {
8181
/**
8282
* Overrides the auto-generated logical ID with a specific ID.
8383
* @param newLogicalId The new logical ID to use for this stack element.
84+
*
85+
* @throws an error if `logicalId` has already been locked
86+
* @throws an error if `newLogicalId` is empty
87+
* @throws an error if `newLogicalId` contains more than 255 characters
88+
* @throws an error if `newLogicalId` contains non-alphanumeric characters
89+
*
90+
* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/resources-section-structure.html#resources-section-structure-logicalid
8491
*/
8592
public overrideLogicalId(newLogicalId: string) {
8693
if (this._logicalIdLocked) {
8794
throw new Error(`The logicalId for resource at path ${Node.of(this).path} has been locked and cannot be overridden\n` +
8895
'Make sure you are calling "overrideLogicalId" before Stack.exportValue');
89-
} else {
90-
this._logicalIdOverride = newLogicalId;
9196
}
97+
98+
if (!Token.isUnresolved(newLogicalId)) {
99+
if (!newLogicalId) {
100+
throw new Error('Cannot set an empty logical ID');
101+
}
102+
if (newLogicalId.length > 255) {
103+
throw new Error(`Invalid logical ID override: '${newLogicalId}'. It must be at most 255 characters long, got ${newLogicalId.length} characters.`);
104+
}
105+
if (!newLogicalId.match(/^[A-Za-z0-9]+$/)) {
106+
throw new Error(`Invalid logical ID override: '${newLogicalId}'. It must only contain alphanumeric characters.`);
107+
}
108+
}
109+
110+
this._logicalIdOverride = newLogicalId;
92111
}
93112

94113
/**

‎packages/aws-cdk-lib/core/test/stack.test.ts

+78-6
Original file line numberDiff line numberDiff line change
@@ -1370,37 +1370,109 @@ describe('stack', () => {
13701370

13711371
// THEN - producers are the same
13721372
expect(() => {
1373-
resourceM.overrideLogicalId('OVERRIDE_LOGICAL_ID');
1373+
resourceM.overrideLogicalId('OVERRIDELOGICALID');
13741374
}).toThrow(/The logicalId for resource at path Producer\/ResourceXXX has been locked and cannot be overridden/);
13751375
});
13761376

1377+
test('throw error if overrideLogicalId contains non-alphanumeric characters', () => {
1378+
// GIVEN: manual
1379+
const appM = new App();
1380+
const producerM = new Stack(appM, 'Producer');
1381+
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });
1382+
1383+
// THEN - producers are the same
1384+
expect(() => {
1385+
resourceM.overrideLogicalId('INVALID_LOGICAL_ID');
1386+
}).toThrow(/must only contain alphanumeric characters/);
1387+
});
1388+
1389+
test('throw error if overrideLogicalId is over 255 characters', () => {
1390+
// GIVEN: manual
1391+
const appM = new App();
1392+
const producerM = new Stack(appM, 'Producer');
1393+
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });
1394+
1395+
// THEN - producers are the same
1396+
expect(() => {
1397+
resourceM.overrideLogicalId(
1398+
// 256 character long string of "aaaa..."
1399+
Array(256).fill('a').join(''),
1400+
);
1401+
}).toThrow(/must be at most 255 characters long, got 256 characters/);
1402+
});
1403+
1404+
test('throw error if overrideLogicalId is an empty string', () => {
1405+
// GIVEN: manual
1406+
const appM = new App();
1407+
const producerM = new Stack(appM, 'Producer');
1408+
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });
1409+
1410+
// THEN - producers are the same
1411+
expect(() => {
1412+
resourceM.overrideLogicalId('');
1413+
}).toThrow('Cannot set an empty logical ID');
1414+
});
1415+
13771416
test('do not throw error if overrideLogicalId is used and logicalId is not locked', () => {
13781417
// GIVEN: manual
13791418
const appM = new App();
13801419
const producerM = new Stack(appM, 'Producer');
13811420
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });
13821421

13831422
// THEN - producers are the same
1384-
resourceM.overrideLogicalId('OVERRIDE_LOGICAL_ID');
1423+
resourceM.overrideLogicalId('OVERRIDELOGICALID');
1424+
producerM.exportValue(resourceM.getAtt('Att'));
1425+
1426+
const template = appM.synth().getStackByName(producerM.stackName).template;
1427+
expect(template).toMatchObject({
1428+
Outputs: {
1429+
ExportsOutputFnGetAttOVERRIDELOGICALIDAtt76AC816F: {
1430+
Export: {
1431+
Name: 'Producer:ExportsOutputFnGetAttOVERRIDELOGICALIDAtt76AC816F',
1432+
},
1433+
Value: {
1434+
'Fn::GetAtt': [
1435+
'OVERRIDELOGICALID',
1436+
'Att',
1437+
],
1438+
},
1439+
},
1440+
},
1441+
Resources: {
1442+
OVERRIDELOGICALID: {
1443+
Type: 'AWS::Resource',
1444+
},
1445+
},
1446+
});
1447+
});
1448+
1449+
test('do not throw if overrideLogicalId is unresolved', () => {
1450+
// GIVEN: manual
1451+
const appM = new App();
1452+
const producerM = new Stack(appM, 'Producer');
1453+
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });
1454+
1455+
// THEN - producers are the same
1456+
resourceM.overrideLogicalId(Lazy.string({ produce: () => 'INVALID_LOGICAL_ID' }));
13851457
producerM.exportValue(resourceM.getAtt('Att'));
13861458

13871459
const template = appM.synth().getStackByName(producerM.stackName).template;
13881460
expect(template).toMatchObject({
13891461
Outputs: {
1390-
ExportsOutputFnGetAttOVERRIDELOGICALIDAtt2DD28019: {
1462+
ExportsOutputFnGetAttINVALIDLOGICALIDAtt6CB9E5B9: {
13911463
Export: {
1392-
Name: 'Producer:ExportsOutputFnGetAttOVERRIDELOGICALIDAtt2DD28019',
1464+
Name: 'Producer:ExportsOutputFnGetAttINVALIDLOGICALIDAtt6CB9E5B9',
13931465
},
13941466
Value: {
13951467
'Fn::GetAtt': [
1396-
'OVERRIDE_LOGICAL_ID',
1468+
'INVALID_LOGICAL_ID',
13971469
'Att',
13981470
],
13991471
},
14001472
},
14011473
},
14021474
Resources: {
1403-
OVERRIDE_LOGICAL_ID: {
1475+
INVALID_LOGICAL_ID: {
14041476
Type: 'AWS::Resource',
14051477
},
14061478
},

0 commit comments

Comments
 (0)
Please sign in to comment.