Repository: pulumi/pulumi-cdk (ts)

Issue:
Replace CDK random suffix with Pulumi random suffix
## Hello!
<!-- Please leave this section as-is, it's designed to help others in the community know how to interact with our GitHub issues. -->

- Vote on this issue by adding a 👍 reaction
- If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

## Issue details

<!-- Enhancement requests are most helpful when they describe the problem you're having as well as articulating the potential solution you'd like to see built. -->

CDK automatically adds a random suffix to the resource logicalId to make the name that is eventually generated by CFN unique. We use the LogicalId as the Pulumi resource Id and then autonaming will add it's on random suffix so we end up with two random suffixes which does not look great. For example, we might end up with a role name like `testapimyapiCloudWatchRoleA84BAE8E-120b18f`

The `cdk.Stack` class has a protected property [allocateLogicalId](https://github.com/aws/aws-cdk/blob/ccab485b87a7090ddf0773508d7b8ee84ff654b0/packages/aws-cdk-lib/core/lib/stack.ts?plain=1#L1357) which is responsible for creating this LogicalId. We should override this method to having our own naming logic.

### Affected area/feature

<!-- If you know the specific area where this feature request would go (e.g. Automation API, the Pulumi Service, the Terraform bridge, etc.), feel free to put that area here.  -->


Pull request description:
Remove CDK generated hash suffix on resource names
CDK automatically adds a hash suffix to the resource logicalId (which we
then use as the Pulumi Resource name). When pulumi autonaming takes
place we then append an additional random suffix so we end up with names
like `testapimyapiCloudWatchRoleA84BAE8E-120b18f`. Apart from this
looking bad it also takes up 16 characters for each name and increases
the likelihood of running into naming limits.

This PR overrides the `allocateLogicalId` method on the `cdk.Stack`.
Most of the code is simply forked from the CDK implementation with the
removal of the piece that generates the hash.

You can view the CDK version here
https://github.com/aws/aws-cdk/blob/ccab485b87a7090ddf0773508d7b8ee84ff654b0/packages/aws-cdk-lib/core/lib/stack.ts?plain=1#L1357

closes #185

Patch:
diff --git a/integration/replace-on-changes/index.ts b/integration/replace-on-changes/index.ts
index b3d44f8..1a1213b 100644
--- a/integration/replace-on-changes/index.ts
+++ b/integration/replace-on-changes/index.ts
@@ -9,7 +9,7 @@ class ReplaceOnChangesStack extends pulumicdk.Stack {
             default: true,
         }).id;
         const azs = aws.getAvailabilityZonesOutput({}).names;
-        new ec2.SecurityGroup(this, 'sg', {
+        new ec2.SecurityGroup(this, 'security-group', {
             description: 'Some Description',
             vpc: ec2.Vpc.fromVpcAttributes(this, 'vpc', {
                 vpcId: pulumicdk.asString(vpc),
diff --git a/integration/replace-on-changes/step2/index.ts b/integration/replace-on-changes/step2/index.ts
index 9070bb4..079b6ca 100644
--- a/integration/replace-on-changes/step2/index.ts
+++ b/integration/replace-on-changes/step2/index.ts
@@ -9,7 +9,7 @@ class ReplaceOnChangesStack extends pulumicdk.Stack {
             default: true,
         }).id;
         const azs = aws.getAvailabilityZonesOutput({}).names;
-        new ec2.SecurityGroup(this, 'sg', {
+        new ec2.SecurityGroup(this, 'security-group', {
             // description is a createOnlyProperty which means this would fail
             // if `replaceOnChanges` was not working
             description: 'Some New Description',
diff --git a/src/cdk-logical-id.ts b/src/cdk-logical-id.ts
new file mode 100644
index 0000000..b65f509
--- /dev/null
+++ b/src/cdk-logical-id.ts
@@ -0,0 +1,87 @@
+// Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// NOTE:
+// Most of this code was copied from https://github.com/aws/aws-cdk/blob/ccab485b87a7090ddf0773508d7b8ee84ff654b0/packages/aws-cdk-lib/core/lib/private/uniqueid.ts
+// with the modification of removing logic related to adding the path hash to the logicalId
+import * as cdk from 'aws-cdk-lib/core';
+/**
+ * Resources with this ID are hidden from humans
+ *
+ * They do not appear in the human-readable part of the logical ID
+ */
+const HIDDEN_FROM_HUMAN_ID = 'Resource';
+
+/**
+ * Resources with this ID are complete hidden from the logical ID calculation.
+ */
+const HIDDEN_ID = 'Default';
+const MAX_HUMAN_LEN = 240; // this is the value in CDK and seems like a good default to keep
+
+/**
+ * Calculates a unique ID for a set of textual components.
+ *
+ * This is forked from the internal cdk implementation with the removal of the hash suffix.
+ * We remove the hash from the CDK logical ID calculation because Pulumi already handles
+ * adding a unique random suffix and we do not want to end up with a double hash.
+ * @see https://github.com/aws/aws-cdk/blob/ccab485b87a7090ddf0773508d7b8ee84ff654b0/packages/aws-cdk-lib/core/lib/private/uniqueid.ts?plain=1#L32
+ *
+ * @param components The path components
+ * @returns a unique alpha-numeric identifier with a maximum length of 255
+ */
+export function makeUniqueId(components: string[]) {
+    components = components.filter((x) => x !== HIDDEN_ID);
+
+    if (components.length === 0) {
+        throw new Error('Unable to calculate a unique id for an empty set of components');
+    }
+
+    const unresolvedTokens = components.filter((c) => cdk.Token.isUnresolved(c));
+    if (unresolvedTokens.length > 0) {
+        throw new Error(`ID components may not include unresolved tokens: ${unresolvedTokens.join(',')}`);
+    }
+
+    const human = removeDupes(components)
+        .filter((x) => x !== HIDDEN_FROM_HUMAN_ID)
+        .map(removeNonAlphanumeric)
+        .join('')
+        .slice(0, MAX_HUMAN_LEN);
+
+    return human;
+}
+
+/**
+ * Remove duplicate "terms" from the path list
+ *
+ * If the previous path component name ends with this component name, skip the
+ * current component.
+ */
+function removeDupes(path: string[]): string[] {
+    const ret = new Array<string>();
+
+    for (const component of path) {
+        if (ret.length === 0 || !ret[ret.length - 1].endsWith(component)) {
+            ret.push(component);
+        }
+    }
+
+    return ret;
+}
+
+/**
+ * Removes all non-alphanumeric characters in a string.
+ */
+function removeNonAlphanumeric(s: string) {
+    return s.replace(/[^A-Za-z0-9]/g, '');
+}
diff --git a/src/stack.ts b/src/stack.ts
index e7f6fbc..c6533dd 100644
--- a/src/stack.ts
+++ b/src/stack.ts
@@ -18,6 +18,7 @@ import { AppConverter, StackConverter } from './converters/app-converter';
 import { PulumiSynthesizer, PulumiSynthesizerBase } from './synthesizer';
 import { AwsCdkCli, ICloudAssemblyDirectoryProducer } from '@aws-cdk/cli-lib-alpha';
 import { CdkConstruct } from './interop';
+import { makeUniqueId } from './cdk-logical-id';
 
 export type AppOutputs = { [outputId: string]: pulumi.Output<any> };
 
@@ -277,6 +278,36 @@ export class Stack extends cdk.Stack {
         // provider or would require unintuitive options in order to produce the expected result).
         return pulumi.output(this.converter.then((converter) => converter.asOutputValue(v)));
     }
+
+    /**
+     * Returns the naming scheme used to allocate logical IDs. This overrides the default method
+     *
+     * If the "human" part of the ID exceeds 240 characters, we simply trim it so
+     * the total ID doesn't exceed CloudFormation's 255 character limit.
+     *
+     * When Pulumi auto names the resource it will add an 8 character random identifier to the end
+     *
+     * Special cases:
+     *
+     * - For aesthetic reasons, if the last components of the path are the same
+     *   (i.e. `L1/L2/Pipeline/Pipeline`), they will be de-duplicated to make the
+     *   resulting human portion of the ID more pleasing: `L1L2Pipeline`
+     *   instead of `L1L2PipelinePipeline`
+     * - If a component is named "Default" it will be omitted from the path. This
+     *   allows refactoring higher level abstractions around constructs without affecting
+     *   the IDs of already deployed resources.
+     * - If a component is named "Resource" it will be omitted from the user-visible
+     *   path. This reduces visual noise in the human readable
+     *   part of the identifier.
+     *
+     * @param cfnElement The element for which the logical ID is allocated.
+     */
+    protected allocateLogicalId(cfnElement: cdk.CfnElement): string {
+        const scopes = cfnElement.node.scopes;
+        const stackIndex = scopes.indexOf(cfnElement.stack);
+        const pathComponents = scopes.slice(stackIndex + 1).map((x) => x.node.id);
+        return makeUniqueId(pathComponents);
+    }
 }
 
 /**


Test patch:
diff --git a/integration/examples_nodejs_test.go b/integration/examples_nodejs_test.go
index e143613..464c23e 100644
--- a/integration/examples_nodejs_test.go
+++ b/integration/examples_nodejs_test.go
@@ -103,7 +103,7 @@ func TestMisc(t *testing.T) {
 			Dir: filepath.Join(getCwd(t), "misc-services"),
 			ExtraRuntimeValidation: func(t *testing.T, stack integration.RuntimeValidationStackInfo) {
 				repoName := stack.Outputs["repoName"].(string)
-				assert.Containsf(t, repoName, "testrepob5dda46f", "Expected repoName to contain 'testrepob5dda46f'; got %s", repoName)
+				assert.Containsf(t, repoName, "testrepo", "Expected repoName to contain 'testrepo'; got %s", repoName)
 			},
 		})
 
@@ -116,7 +116,7 @@ func TestCloudFront(t *testing.T) {
 			Dir: filepath.Join(getCwd(t), "cloudfront"),
 			ExtraRuntimeValidation: func(t *testing.T, stack integration.RuntimeValidationStackInfo) {
 				bucketName := stack.Outputs["bucketName"].(string)
-				assert.Containsf(t, bucketName, "bucket83908e77", "Bucket name should contain 'bucket'")
+				assert.Containsf(t, bucketName, "bucket", "Bucket name should contain 'bucket'")
 			},
 		})
 
diff --git a/tests/cdk-resource.test.ts b/tests/cdk-resource.test.ts
index 2712c32..19d00cf 100644
--- a/tests/cdk-resource.test.ts
+++ b/tests/cdk-resource.test.ts
@@ -10,13 +10,12 @@ import { setMocks, testApp } from './mocks';
 import { MockResourceArgs } from '@pulumi/pulumi/runtime';
 import { Construct } from 'constructs';
 
-let resources: MockResourceArgs[] = [];
-beforeAll(() => {
-    resources = [];
-    setMocks(resources);
-});
-
 describe('CDK Construct tests', () => {
+    let resources: MockResourceArgs[] = [];
+    beforeAll(() => {
+        resources = [];
+        setMocks(resources);
+    });
     // DynamoDB table was previously mapped to the `aws` provider
     // otherwise this level of testing wouldn't be necessary.
     // We also don't need to do this type of testing for _every_ resource
@@ -135,7 +134,7 @@ describe('CDK Construct tests', () => {
                         Action: 'events:PutEvents',
                         Effect: 'Allow',
                         Principal: { AWS: 'arn:aws:iam::12345678912:root' },
-                        Resource: 'testbus9BA9ECFC_arn',
+                        Resource: 'testbus_arn',
                         Sid: 'cdk-testsid',
                     },
                 ],
@@ -197,3 +196,82 @@ describe('CDK Construct tests', () => {
         });
     });
 });
+
+describe('logicalId tests', () => {
+    let resources: MockResourceArgs[] = [];
+    beforeEach(() => {
+        resources = [];
+        setMocks(resources);
+    });
+    test('logicalId is generated without hash for resources', async () => {
+        await testApp((scope: Construct) => {
+            new dynamodb.Table(scope, 'Table', {
+                partitionKey: { name: 'id', type: dynamodb.AttributeType.STRING },
+            });
+        });
+        const table = resources.find((res) => res.type === 'aws-native:dynamodb:Table');
+        expect(table).toBeDefined();
+        expect(table!.name).toEqual('Table');
+    });
+
+    test('logicalId with nested constructs', async () => {
+        await testApp((scope: Construct) => {
+            const construct = new Construct(scope, 'MyConstruct');
+            new dynamodb.Table(construct, 'Table', {
+                partitionKey: { name: 'id', type: dynamodb.AttributeType.STRING },
+            });
+        });
+        const table = resources.find((res) => res.type === 'aws-native:dynamodb:Table');
+        expect(table).toBeDefined();
+        expect(table!.name).toEqual('MyConstructTable');
+    });
+
+    test('logicalId with nested constructs dedupped', async () => {
+        await testApp((scope: Construct) => {
+            const construct = new Construct(scope, 'MyConstruct');
+            const construct2 = new Construct(construct, 'MyConstruct');
+            new dynamodb.Table(construct2, 'Table', {
+                partitionKey: { name: 'id', type: dynamodb.AttributeType.STRING },
+            });
+        });
+        const table = resources.find((res) => res.type === 'aws-native:dynamodb:Table');
+        expect(table).toBeDefined();
+        expect(table!.name).toEqual('MyConstructTable');
+    });
+
+    test('logicalId with Resource', async () => {
+        await testApp((scope: Construct) => {
+            const construct = new Construct(scope, 'Resource');
+            new dynamodb.Table(construct, 'Table', {
+                partitionKey: { name: 'id', type: dynamodb.AttributeType.STRING },
+            });
+        });
+        const table = resources.find((res) => res.type === 'aws-native:dynamodb:Table');
+        expect(table).toBeDefined();
+        expect(table!.name).toEqual('Table');
+    });
+
+    test('logicalId with Default', async () => {
+        await testApp((scope: Construct) => {
+            const construct = new Construct(scope, 'Default');
+            new dynamodb.Table(construct, 'Table', {
+                partitionKey: { name: 'id', type: dynamodb.AttributeType.STRING },
+            });
+        });
+        const table = resources.find((res) => res.type === 'aws-native:dynamodb:Table');
+        expect(table).toBeDefined();
+        expect(table!.name).toEqual('Table');
+    });
+
+    test('logicalId with non-alphanumeric', async () => {
+        await testApp((scope: Construct) => {
+            const construct = new Construct(scope, 'MyConstruct-123');
+            new dynamodb.Table(construct, 'Table', {
+                partitionKey: { name: 'id', type: dynamodb.AttributeType.STRING },
+            });
+        });
+        const table = resources.find((res) => res.type === 'aws-native:dynamodb:Table');
+        expect(table).toBeDefined();
+        expect(table!.name).toEqual('MyConstruct123Table');
+    });
+});