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.

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

Remove CDK generated hash suffix on resource names #215

Merged
merged 4 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions integration/examples_nodejs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
})

Expand All @@ -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'")
},
})

Expand Down
2 changes: 1 addition & 1 deletion integration/replace-on-changes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion integration/replace-on-changes/step2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
87 changes: 87 additions & 0 deletions src/cdk-logical-id.ts
Original file line number Diff line number Diff line change
@@ -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, '');
}
31 changes: 31 additions & 0 deletions src/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> };

Expand Down Expand Up @@ -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);
}
}

/**
Expand Down
92 changes: 85 additions & 7 deletions tests/cdk-resource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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',
},
],
Expand Down Expand Up @@ -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');
});
});
Loading