fix(core): overrideLogicalId validation

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

@@ -81,14 +81,33 @@ 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;
   }
 
   /**

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

@@ -1370,37 +1370,109 @@ describe('stack', () => {
 
     // THEN - producers are the same
     expect(() => {
-      resourceM.overrideLogicalId('OVERRIDE_LOGICAL_ID');
+      resourceM.overrideLogicalId('OVERRIDELOGICALID');
     }).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('OVERRIDE_LOGICAL_ID');
+    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' }));
     producerM.exportValue(resourceM.getAtt('Att'));
 
     const template = appM.synth().getStackByName(producerM.stackName).template;
     expect(template).toMatchObject({
       Outputs: {
-        ExportsOutputFnGetAttOVERRIDELOGICALIDAtt2DD28019: {
+        ExportsOutputFnGetAttINVALIDLOGICALIDAtt6CB9E5B9: {
           Export: {
-            Name: 'Producer:ExportsOutputFnGetAttOVERRIDELOGICALIDAtt2DD28019',
+            Name: 'Producer:ExportsOutputFnGetAttINVALIDLOGICALIDAtt6CB9E5B9',
           },
           Value: {
             'Fn::GetAtt': [
-              'OVERRIDE_LOGICAL_ID',
+              'INVALID_LOGICAL_ID',
               'Att',
             ],
           },
         },
       },
       Resources: {
-        OVERRIDE_LOGICAL_ID: {
+        INVALID_LOGICAL_ID: {
           Type: 'AWS::Resource',
         },
       },