Skip to content

Commit

Permalink
flag to allow _ and - in logical ids
Browse files Browse the repository at this point in the history
  • Loading branch information
jsteinich committed Aug 24, 2020
1 parent 5835521 commit 3944df5
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 22 deletions.
8 changes: 7 additions & 1 deletion packages/cdktf/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
*/
export const EXCLUDE_STACK_ID_FROM_LOGICAL_IDS = 'excludeStackIdFromLogicalIds';

/**
* When set, '_' and '-' are allowed in logical ids.
*/
export const ALLOW_SEP_CHARS_IN_LOGICAL_IDS = 'allowSepCharsInLogicalIds';

/**
* This map includes context keys and values for feature flags that enable
* capabilities "from the future", which we could not introduce as the default
Expand All @@ -26,5 +31,6 @@ export const EXCLUDE_STACK_ID_FROM_LOGICAL_IDS = 'excludeStackIdFromLogicalIds';
* Tests must cover the default (disabled) case and the future (enabled) case.
*/
export const FUTURE_FLAGS = {
[EXCLUDE_STACK_ID_FROM_LOGICAL_IDS]: 'true'
[EXCLUDE_STACK_ID_FROM_LOGICAL_IDS]: 'true',
[ALLOW_SEP_CHARS_IN_LOGICAL_IDS]: 'true'
};
19 changes: 16 additions & 3 deletions packages/cdktf/lib/private/unique.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const MAX_ID_LEN = 255;
* @param components The path components
* @returns a unique alpha-numeric identifier with a maximum length of 255
*/
export function makeUniqueId(components: string[]) {
export function makeUniqueId(components: string[], allowSepChars: boolean) {
components = components.filter(x => x !== HIDDEN_ID);

if (components.length === 0) {
Expand All @@ -46,7 +46,7 @@ export function makeUniqueId(components: string[]) {
// logical ID). sadly, changing it in the 1.x version line is impossible
// because it will be a breaking change. we should consider for v2.0.
// https://github.com/aws/aws-cdk/issues/6421
const candidate = removeNonAlphanumeric(components[0]);
const candidate = removeDisallowedCharacters(components[0], allowSepChars);

// if our candidate is short enough, use it as is. otherwise, fall back to
// the normal mode.
Expand All @@ -58,7 +58,7 @@ export function makeUniqueId(components: string[]) {
const hash = pathHash(components);
const human = removeDupes(components)
.filter(x => x !== HIDDEN_FROM_HUMAN_ID)
.map(removeNonAlphanumeric)
.map(s => removeDisallowedCharacters(s, allowSepChars))
.join(UNIQUE_SEP)
.slice(0, MAX_HUMAN_LEN);

Expand All @@ -75,13 +75,26 @@ function pathHash(path: string[]): string {
return md5.slice(0, HASH_LEN).toUpperCase();
}

function removeDisallowedCharacters(s: string, allowSepChars: boolean) {
if (allowSepChars) {
return removeNonAlphanumericSep(s);
}
else {
return removeNonAlphanumeric(s);
}
}

/**
* Removes all non-alphanumeric characters in a string.
*/
function removeNonAlphanumeric(s: string) {
return s.replace(/[^A-Za-z0-9]/g, '');
}

function removeNonAlphanumericSep(s: string) {
return s.replace(/[^A-Za-z0-9_-]/g, '');
}

/**
* Remove duplicate "terms" from the path list
*
Expand Down
4 changes: 2 additions & 2 deletions packages/cdktf/lib/terraform-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as path from 'path';
import { TerraformElement } from './terraform-element';
import { deepMerge } from './util';
import { TerraformProvider } from './terraform-provider';
import { EXCLUDE_STACK_ID_FROM_LOGICAL_IDS } from './features';
import { EXCLUDE_STACK_ID_FROM_LOGICAL_IDS, ALLOW_SEP_CHARS_IN_LOGICAL_IDS } from './features';
import { makeUniqueId } from './private/unique';

const STACK_SYMBOL = Symbol.for('ckdtf/TerraformStack');
Expand Down Expand Up @@ -98,7 +98,7 @@ export class TerraformStack extends Construct {
}

const components = node.scopes.slice(stackIndex + 1).map(c => Node.of(c).id);
return components.length > 0 ? makeUniqueId(components) : '';
return components.length > 0 ? makeUniqueId(components, node.tryGetContext(ALLOW_SEP_CHARS_IN_LOGICAL_IDS)) : '';
}

public allProviders(): TerraformProvider[] {
Expand Down
4 changes: 2 additions & 2 deletions packages/cdktf/test/__snapshots__/data-source.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ exports[`with complex computed list 1`] = `
},
\\"resource\\": {
\\"test_resource\\": {
\\"testresource\\": {
\\"test-resource\\": {
\\"name\\": \\"\${data.test_data_source.test.complex_computed_list.0.id}\\",
\\"//\\": {
\\"metadata\\": {
\\"path\\": \\"test-data-source/test-resource\\",
\\"uniqueId\\": \\"testresource\\"
\\"uniqueId\\": \\"test-resource\\"
}
}
}
Expand Down
22 changes: 11 additions & 11 deletions packages/cdktf/test/__snapshots__/output.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ exports[`boolean output 1`] = `
}
},
\\"output\\": {
\\"testoutput\\": {
\\"test-output\\": {
\\"value\\": true
}
}
Expand All @@ -26,22 +26,22 @@ exports[`dependent output 1`] = `
},
\\"resource\\": {
\\"test_resource\\": {
\\"weirdlongrunningresource\\": {
\\"weird-long-running-resource\\": {
\\"name\\": \\"foo\\",
\\"//\\": {
\\"metadata\\": {
\\"path\\": \\"test/weird-long-running-resource\\",
\\"uniqueId\\": \\"weirdlongrunningresource\\"
\\"uniqueId\\": \\"weird-long-running-resource\\"
}
}
}
}
},
\\"output\\": {
\\"testoutput\\": {
\\"test-output\\": {
\\"value\\": 1,
\\"depends_on\\": [
\\"\${test_resource.weirdlongrunningresource}\\"
\\"\${test_resource.weird-long-running-resource}\\"
]
}
}
Expand All @@ -57,7 +57,7 @@ exports[`description output 1`] = `
}
},
\\"output\\": {
\\"testoutput\\": {
\\"test-output\\": {
\\"value\\": 1,
\\"description\\": \\"test-description\\"
}
Expand All @@ -74,7 +74,7 @@ exports[`list output 1`] = `
}
},
\\"output\\": {
\\"testoutput\\": {
\\"test-output\\": {
\\"value\\": [
\\"foo\\",
\\"bar\\"
Expand All @@ -93,7 +93,7 @@ exports[`map output 1`] = `
}
},
\\"output\\": {
\\"testoutput\\": {
\\"test-output\\": {
\\"value\\": {
\\"foo\\": \\"bar\\"
}
Expand All @@ -111,7 +111,7 @@ exports[`number output 1`] = `
}
},
\\"output\\": {
\\"testoutput\\": {
\\"test-output\\": {
\\"value\\": 1
}
}
Expand All @@ -127,7 +127,7 @@ exports[`sensitive output 1`] = `
}
},
\\"output\\": {
\\"testoutput\\": {
\\"test-output\\": {
\\"value\\": 1,
\\"sensitive\\": true
}
Expand All @@ -144,7 +144,7 @@ exports[`string output 1`] = `
}
},
\\"output\\": {
\\"testoutput\\": {
\\"test-output\\": {
\\"value\\": \\"1\\"
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/cdktf/test/__snapshots__/remote-state.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -431,12 +431,12 @@ exports[`s3 reference 1`] = `
},
\\"resource\\": {
\\"test_resource\\": {
\\"testresource\\": {
\\"test_resource\\": {
\\"name\\": \\"\${data.terraform_remote_state.remote.outputs.name}\\",
\\"//\\": {
\\"metadata\\": {
\\"path\\": \\"test/test_resource\\",
\\"uniqueId\\": \\"testresource\\"
\\"uniqueId\\": \\"test_resource\\"
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/cdktf/test/__snapshots__/stack.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ exports[`stack synthesis merges all elements into a single output 1`] = `
}
},
\\"output\\": {
\\"eksversion\\": {
\\"eks_version\\": {
\\"value\\": \\"7.0.1\\"
}
},
Expand Down

0 comments on commit 3944df5

Please sign in to comment.