Skip to content

Commit

Permalink
Revert "fix(core): overrideLogicalId validation (#29708)"
Browse files Browse the repository at this point in the history
This reverts commit b196b13.
  • Loading branch information
TheRealAmazonKendra authored Jun 27, 2024
1 parent 0247013 commit 0acb590
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 99 deletions.
23 changes: 2 additions & 21 deletions packages/aws-cdk-lib/core/lib/cfn-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,33 +81,14 @@ export abstract class CfnElement extends Construct {
/**
* Overrides the auto-generated logical ID with a specific ID.
* @param newLogicalId The new logical ID to use for this stack element.
*
* @throws an error if `logicalId` has already been locked
* @throws an error if `newLogicalId` is empty
* @throws an error if `newLogicalId` contains more than 255 characters
* @throws an error if `newLogicalId` contains non-alphanumeric characters
*
* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/resources-section-structure.html#resources-section-structure-logicalid
*/
public overrideLogicalId(newLogicalId: string) {
if (this._logicalIdLocked) {
throw new Error(`The logicalId for resource at path ${Node.of(this).path} has been locked and cannot be overridden\n` +
'Make sure you are calling "overrideLogicalId" before Stack.exportValue');
} else {
this._logicalIdOverride = newLogicalId;
}

if (!Token.isUnresolved(newLogicalId)) {
if (!newLogicalId) {
throw new Error('Cannot set an empty logical ID');
}
if (newLogicalId.length > 255) {
throw new Error(`Invalid logical ID override: '${newLogicalId}'. It must be at most 255 characters long, got ${newLogicalId.length} characters.`);
}
if (!newLogicalId.match(/^[A-Za-z0-9]+$/)) {
throw new Error(`Invalid logical ID override: '${newLogicalId}'. It must only contain alphanumeric characters.`);
}
}

this._logicalIdOverride = newLogicalId;
}

/**
Expand Down
84 changes: 6 additions & 78 deletions packages/aws-cdk-lib/core/test/stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1370,109 +1370,37 @@ describe('stack', () => {

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

test('throw error if overrideLogicalId contains non-alphanumeric characters', () => {
// GIVEN: manual
const appM = new App();
const producerM = new Stack(appM, 'Producer');
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });

// THEN - producers are the same
expect(() => {
resourceM.overrideLogicalId('INVALID_LOGICAL_ID');
}).toThrow(/must only contain alphanumeric characters/);
});

test('throw error if overrideLogicalId is over 255 characters', () => {
// GIVEN: manual
const appM = new App();
const producerM = new Stack(appM, 'Producer');
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });

// THEN - producers are the same
expect(() => {
resourceM.overrideLogicalId(
// 256 character long string of "aaaa..."
Array(256).fill('a').join(''),
);
}).toThrow(/must be at most 255 characters long, got 256 characters/);
});

test('throw error if overrideLogicalId is an empty string', () => {
// GIVEN: manual
const appM = new App();
const producerM = new Stack(appM, 'Producer');
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });

// THEN - producers are the same
expect(() => {
resourceM.overrideLogicalId('');
}).toThrow('Cannot set an empty logical ID');
});

test('do not throw error if overrideLogicalId is used and logicalId is not locked', () => {
// GIVEN: manual
const appM = new App();
const producerM = new Stack(appM, 'Producer');
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });

// THEN - producers are the same
resourceM.overrideLogicalId('OVERRIDELOGICALID');
producerM.exportValue(resourceM.getAtt('Att'));

const template = appM.synth().getStackByName(producerM.stackName).template;
expect(template).toMatchObject({
Outputs: {
ExportsOutputFnGetAttOVERRIDELOGICALIDAtt76AC816F: {
Export: {
Name: 'Producer:ExportsOutputFnGetAttOVERRIDELOGICALIDAtt76AC816F',
},
Value: {
'Fn::GetAtt': [
'OVERRIDELOGICALID',
'Att',
],
},
},
},
Resources: {
OVERRIDELOGICALID: {
Type: 'AWS::Resource',
},
},
});
});

test('do not throw if overrideLogicalId is unresolved', () => {
// GIVEN: manual
const appM = new App();
const producerM = new Stack(appM, 'Producer');
const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });

// THEN - producers are the same
resourceM.overrideLogicalId(Lazy.string({ produce: () => 'INVALID_LOGICAL_ID' }));
resourceM.overrideLogicalId('OVERRIDE_LOGICAL_ID');
producerM.exportValue(resourceM.getAtt('Att'));

const template = appM.synth().getStackByName(producerM.stackName).template;
expect(template).toMatchObject({
Outputs: {
ExportsOutputFnGetAttINVALIDLOGICALIDAtt6CB9E5B9: {
ExportsOutputFnGetAttOVERRIDELOGICALIDAtt2DD28019: {
Export: {
Name: 'Producer:ExportsOutputFnGetAttINVALIDLOGICALIDAtt6CB9E5B9',
Name: 'Producer:ExportsOutputFnGetAttOVERRIDELOGICALIDAtt2DD28019',
},
Value: {
'Fn::GetAtt': [
'INVALID_LOGICAL_ID',
'OVERRIDE_LOGICAL_ID',
'Att',
],
},
},
},
Resources: {
INVALID_LOGICAL_ID: {
OVERRIDE_LOGICAL_ID: {
Type: 'AWS::Resource',
},
},
Expand Down

0 comments on commit 0acb590

Please sign in to comment.