Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

revert: fix(core): overrideLogicalId validation #30695

Merged
merged 1 commit into from
Jun 27, 2024
Merged

revert: fix(core): overrideLogicalId validation #30695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 2 additions & 21 deletions packages/aws-cdk-lib/core/lib/cfn-element.ts
View file
Open in desktop
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
View file
Open in desktop
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
Loading