From eb617c230604dbe6eb31dff7aa17d79c7d553694 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 14 Aug 2018 09:24:19 +0300 Subject: [PATCH] feat(cdk): construct.uniqueId and relax construct id constraints Adds a property `uniqueId` to Construct which returns an alphanumeric 255-length-limited tree-unique identity for a construct. Relax constraints for construct id (previously known as "name") to only restrict the usage of the path separator. Otherwise, all characters are allowed. This will allow using the construct id for a wider range of purposes, but since logical IDs (and uniqueId now) are alpha-safe, it's okay. Deprecate `construct.name` in favor of `construct.id`. --- packages/@aws-cdk/cdk/lib/app.ts | 4 +- .../cdk/lib/cloudformation/logical-id.ts | 67 +-------------- .../@aws-cdk/cdk/lib/cloudformation/output.ts | 2 +- .../@aws-cdk/cdk/lib/cloudformation/stack.ts | 2 +- packages/@aws-cdk/cdk/lib/core/construct.ts | 68 ++++++++------- packages/@aws-cdk/cdk/lib/core/jsx.ts | 8 +- packages/@aws-cdk/cdk/lib/util/uniqueid.ts | 85 +++++++++++++++++++ .../test/cloudformation/test.logical-id.ts | 11 +++ .../@aws-cdk/cdk/test/core/test.construct.ts | 51 +++++++---- packages/@aws-cdk/cdk/test/core/test.jsx.tsx | 21 ++--- packages/@aws-cdk/cdk/test/test.app.ts | 2 +- packages/@aws-cdk/cdk/test/test.context.ts | 2 +- 12 files changed, 190 insertions(+), 133 deletions(-) create mode 100644 packages/@aws-cdk/cdk/lib/util/uniqueid.ts diff --git a/packages/@aws-cdk/cdk/lib/app.ts b/packages/@aws-cdk/cdk/lib/app.ts index e79938d4478e0..6d157783aef34 100644 --- a/packages/@aws-cdk/cdk/lib/app.ts +++ b/packages/@aws-cdk/cdk/lib/app.ts @@ -47,7 +47,7 @@ export class App extends Root { throw new Error(`The child ${child.toString()} of Program must be a Stack`); } - out[child.name] = child as Stack; + out[child.id] = child as Stack; } return out; } @@ -113,7 +113,7 @@ export class App extends Root { } return { - name: stack.name, + name: stack.id, environment, missing: Object.keys(stack.missingContext).length ? stack.missingContext : undefined, template: stack.toCloudFormation(), diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/logical-id.ts b/packages/@aws-cdk/cdk/lib/cloudformation/logical-id.ts index 4bc66fd13b4f9..5f89093795981 100644 --- a/packages/@aws-cdk/cdk/lib/cloudformation/logical-id.ts +++ b/packages/@aws-cdk/cdk/lib/cloudformation/logical-id.ts @@ -1,11 +1,7 @@ +import { makeUniqueId } from '../util/uniqueid'; import { StackElement } from "./stack"; -// tslint:disable-next-line:no-var-requires -const md5 = require('../util/md5'); - const PATH_SEP = '/'; -const HASH_LEN = 8; -const MAX_HUMAN_LEN = 240; // max ID len is 255 /** * Interface for classes that implementation logical ID assignment strategies @@ -55,42 +51,10 @@ export interface IAddressingScheme { */ export class HashedAddressingScheme implements IAddressingScheme { public allocateAddress(addressComponents: string[]): string { - addressComponents = addressComponents.filter(x => x !== HIDDEN_ID); - - if (addressComponents.length === 0) { - throw new Error('Construct has empty Logical ID'); - } - - // top-level resources will simply use the `name` as-is in order to support - // transparent migration of cloudformation templates to the CDK without the - // need to rename all resources. - if (addressComponents.length === 1) { - return addressComponents[0]; - } - - const hash = pathHash(addressComponents); - const human = removeDupes(addressComponents) - .filter(x => x !== HIDDEN_FROM_HUMAN_ID) - .join('') - .slice(0, MAX_HUMAN_LEN); - - return human + hash; + return makeUniqueId(addressComponents); } } -/** - * Resources with this ID are hidden from humans - * - * They do not appear in the human-readable part of the logical ID, - * but they are included in the hash calculation. - */ -const HIDDEN_FROM_HUMAN_ID = 'Resource'; - -/** - * Resources with this ID are complete hidden from the logical ID calculation. - */ -const HIDDEN_ID = 'Default'; - /** * Class that keeps track of the logical IDs that are assigned to resources * @@ -176,15 +140,6 @@ export class LogicalIDs { } } -/** - * Take a hash of the given path. - * - * The hash is limited in size. - */ -function pathHash(path: string[]): string { - return md5(path.join(PATH_SEP)).slice(0, HASH_LEN).toUpperCase(); -} - const VALID_LOGICALID_REGEX = /^[A-Za-z][A-Za-z0-9]{1,254}$/; /** @@ -195,21 +150,3 @@ function validateLogicalId(logicalId: string) { throw new Error(`Logical ID must adhere to the regular expression: ${VALID_LOGICALID_REGEX.toString()}, got '${logicalId}'`); } } - -/** - * Remove duplicate "terms" from the path list - * - * If a component name is completely the same as the suffix of - * the previous component name, we get rid of it. - */ -function removeDupes(path: string[]): string[] { - const ret = new Array(); - - for (const component of path) { - if (ret.length === 0 || !ret[ret.length - 1].endsWith(component)) { - ret.push(component); - } - } - - return ret; -} diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/output.ts b/packages/@aws-cdk/cdk/lib/cloudformation/output.ts index f4135f9b3e264..c3e8e298c10c3 100644 --- a/packages/@aws-cdk/cdk/lib/cloudformation/output.ts +++ b/packages/@aws-cdk/cdk/lib/cloudformation/output.ts @@ -91,7 +91,7 @@ export class Output extends StackElement { this.export = props.export; } else if (!props.disableExport) { // prefix export name with stack name since exports are global within account + region. - const stackName = Stack.find(this).name; + const stackName = Stack.find(this).id; this.export = stackName ? stackName + ':' : ''; this.export += this.logicalId; } diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts b/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts index 81ee55645eb2a..9afe7265bdec4 100644 --- a/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts +++ b/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts @@ -332,7 +332,7 @@ export abstract class StackElement extends Construct implements IDependable { * Return the path with respect to the stack */ public get stackPath(): string { - return this.ancestors(this.stack).map(c => c.name).join(PATH_SEP); + return this.ancestors(this.stack).map(c => c.id).join(PATH_SEP); } public get dependencyElements(): IDependable[] { diff --git a/packages/@aws-cdk/cdk/lib/core/construct.ts b/packages/@aws-cdk/cdk/lib/core/construct.ts index f07790d4c4c91..3b52c7092b7d1 100644 --- a/packages/@aws-cdk/cdk/lib/core/construct.ts +++ b/packages/@aws-cdk/cdk/lib/core/construct.ts @@ -1,4 +1,5 @@ import cxapi = require('@aws-cdk/cx-api'); +import { makeUniqueId } from '../util/uniqueid'; export const PATH_SEP = '/'; /** @@ -6,18 +7,34 @@ export const PATH_SEP = '/'; * When a construct is created, it is always added as a child */ export class Construct { - private static readonly VALID_NAME_REGEX = /^[A-Za-z][A-Za-z0-9]*$/; - /** * Returns the parent of this node or undefined if this is a root node. */ public readonly parent?: Construct; /** - * The name of this construct + * @deprecated use `id` */ public readonly name: string; + /** + * The subtree-local id of the construct. + * This id is unique within the subtree. To obtain a tree-unique id, use `uniqueId`. + */ + public readonly id: string; + + /** + * The full path of this construct in the tree. + * Components are separated by '/'. + */ + public readonly path: string; + + /** + * A tree-unique alpha-numeric identifier for this construct. + * Includes all components of the tree. + */ + public readonly uniqueId: string; + /** * List of children and their names */ @@ -37,28 +54,33 @@ export class Construct { * @param parent The parent construct * @param props Properties for this construct */ - constructor(parent: Construct, name: string) { - this.name = name; + constructor(parent: Construct, id: string) { + this.id = id; + this.name = id; // legacy this.parent = parent; // We say that parent is required, but some root constructs bypass the type checks and // actually pass in 'undefined'. if (parent != null) { - if (name === '') { + if (id === '') { throw new Error('Only root constructs may have an empty name'); } // Has side effect so must be very last thing in constructor - parent.addChild(this, this.name); + parent.addChild(this, this.id); } else { // This is a root construct. - this.name = name; + this.id = id; } // Validate the name we ended up with - if (this.name !== '') { - this._validateName(this.name); + if (this.id !== '') { + this._validateName(this.id); } + + const components = this.rootPath().map(c => c.id); + this.path = components.join(PATH_SEP); + this.uniqueId = components.length > 0 ? makeUniqueId(components) : ''; } /** @@ -77,7 +99,7 @@ export class Construct { for (let i = 0; i < depth; ++i) { out += ' '; } - const name = this.name || ''; + const name = this.id || ''; out += `${this.typename}${name.length > 0 ? ' [' + name + ']' : ''}\n`; for (const child of this.children) { out += child.toTreeString(depth + 1); @@ -137,7 +159,7 @@ export class Construct { */ public setContext(key: string, value: any) { if (this.children.length > 0) { - const names = this.children.map(c => c.name); + const names = this.children.map(c => c.id); throw new Error('Cannot set context after children have been added: ' + names.join(',')); } this.context[key] = value; @@ -173,15 +195,6 @@ export class Construct { return value; } - /** - * Returns the path of all constructs from root to this construct, in string form. - * - * @returns /-separated path of this Construct. - */ - public get path(): string { - return this.rootPath().map(c => c.name).join(PATH_SEP); - } - /** * An array of metadata objects associated with this construct. * This can be used, for example, to implement support for deprecation notices, source mapping, etc. @@ -281,17 +294,12 @@ export class Construct { } /** - * Validate that the name of the construct is a legal identifier - * - * At the moment, we restrict these to valid CloudFormation identifiers. - * - * Protected so it can be overridden by subclasses. Starts with _ to hide the virtual function from JSII, - * because we don't want this validation to involve asynchrony. This restricts it to only - * be overridable in (Type|Java)Script, but that suffices for now. + * Validate that the name of the construct is a legal identifier. + * Construct names can be any characters besides the path separator. */ protected _validateName(name: string) { - if (!Construct.VALID_NAME_REGEX.test(name)) { - throw new Error(`Name must adhere to the regular expression: ${Construct.VALID_NAME_REGEX.toString()}, got '${name}'`); + if (name.indexOf(PATH_SEP) !== -1) { + throw new Error(`Construct names cannot include '${PATH_SEP}': ${name}`); } } diff --git a/packages/@aws-cdk/cdk/lib/core/jsx.ts b/packages/@aws-cdk/cdk/lib/core/jsx.ts index 9c14f2452acbf..3d22e33cb72fe 100644 --- a/packages/@aws-cdk/cdk/lib/core/jsx.ts +++ b/packages/@aws-cdk/cdk/lib/core/jsx.ts @@ -37,16 +37,16 @@ export namespace jsx { * @returns A Construct object */ export function construct(tree: any, parent?: Construct): Construct { - const name = (tree.props && tree.props.name) || ''; - const root = new tree.type(parent, name, tree.props); // create root + const id = (tree.props && tree.props.id) || ''; + const root = new tree.type(parent, id, tree.props); // create root createChildren(root, tree.children); return root; } function createChildren(parent: Construct, children: any[]) { for (const child of children) { - const name = (child.props && child.props.name) || ''; - const childObj = new child.type(parent, name, child.props); + const id = (child.props && child.props.id) || ''; + const childObj = new child.type(parent, id, child.props); createChildren(childObj, child.children); } } diff --git a/packages/@aws-cdk/cdk/lib/util/uniqueid.ts b/packages/@aws-cdk/cdk/lib/util/uniqueid.ts new file mode 100644 index 0000000000000..6c7e27a87ebf9 --- /dev/null +++ b/packages/@aws-cdk/cdk/lib/util/uniqueid.ts @@ -0,0 +1,85 @@ +// tslint:disable-next-line:no-var-requires +const md5 = require('./md5'); + +/** + * Resources with this ID are hidden from humans + * + * They do not appear in the human-readable part of the logical ID, + * but they are included in the hash calculation. + */ +const HIDDEN_FROM_HUMAN_ID = 'Resource'; + +/** + * Resources with this ID are complete hidden from the logical ID calculation. + */ +const HIDDEN_ID = 'Default'; + +const PATH_SEP = '/'; + +const HASH_LEN = 8; +const MAX_HUMAN_LEN = 240; // max ID len is 255 + +/** + * Given a set of named path components, returns a unique alpha-numeric identifier + * with a maximum length of 255. This is done by calculating a hash on the full path + * and using it as a suffix of a length-limited "human" rendition of the path components. + * + * @param components The path components + */ +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 path'); + } + + // top-level resources will simply use the `name` as-is in order to support + // transparent migration of cloudformation templates to the CDK without the + // need to rename all resources. + if (components.length === 1) { + return components[0]; + } + + const hash = pathHash(components); + const human = removeDupes(components) + .map(removeNonAlpha) + .filter(x => x !== HIDDEN_FROM_HUMAN_ID) + .join('') + .slice(0, MAX_HUMAN_LEN); + + return human + hash; +} + +/** + * Take a hash of the given path. + * + * The hash is limited in size. + */ +function pathHash(path: string[]): string { + return md5(path.join(PATH_SEP)).slice(0, HASH_LEN).toUpperCase(); +} + +/** + * Removes all non-alphanumeric characters in a string. + */ +function removeNonAlpha(s: string) { + return s.replace(/[^A-Za-z0-9]/g, ''); +} + +/** + * Remove duplicate "terms" from the path list + * + * If a component name is completely the same as the suffix of + * the previous component name, we get rid of it. + */ +function removeDupes(path: string[]): string[] { + const ret = new Array(); + + for (const component of path) { + if (ret.length === 0 || !ret[ret.length - 1].endsWith(component)) { + ret.push(component); + } + } + + return ret; +} diff --git a/packages/@aws-cdk/cdk/test/cloudformation/test.logical-id.ts b/packages/@aws-cdk/cdk/test/cloudformation/test.logical-id.ts index 7f8348b71db68..da8bda26f7672 100644 --- a/packages/@aws-cdk/cdk/test/cloudformation/test.logical-id.ts +++ b/packages/@aws-cdk/cdk/test/cloudformation/test.logical-id.ts @@ -131,6 +131,17 @@ const uniqueTests = { // THEN: same ID, same object test.equal(theId1, theId2); + test.done(); + }, + + 'non-alphanumeric characters are removed from the human part of the logical ID'(test: Test) { + const scheme = new HashedAddressingScheme(); + const val1 = scheme.allocateAddress([ 'Foo-bar', 'B00m', 'Hello_World', '&&Horray Horray.' ]); + const val2 = scheme.allocateAddress([ 'Foobar', 'B00m', 'HelloWorld', 'HorrayHorray' ]); + + // same human part, different hash + test.deepEqual(val1, 'FoobarB00mHelloWorldHorrayHorray640E99FB'); + test.deepEqual(val2, 'FoobarB00mHelloWorldHorrayHorray744334FD'); test.done(); } }; diff --git a/packages/@aws-cdk/cdk/test/core/test.construct.ts b/packages/@aws-cdk/cdk/test/core/test.construct.ts index 0036e81520532..70d4c80577d59 100644 --- a/packages/@aws-cdk/cdk/test/core/test.construct.ts +++ b/packages/@aws-cdk/cdk/test/core/test.construct.ts @@ -8,7 +8,7 @@ import { Construct, Root } from '../../lib'; export = { 'the "Root" construct is a special construct which can be used as the root of the tree'(test: Test) { const root = new Root(); - test.equal(root.name, '', 'if not specified, name of a root construct is an empty string'); + test.equal(root.id, '', 'if not specified, name of a root construct is an empty string'); test.ok(!root.parent, 'no parent'); test.equal(root.children.length, 0, 'a construct is created without children'); // no children test.done(); @@ -23,32 +23,47 @@ export = { 'construct.name returns the name of the construct'(test: Test) { const t = createTree(); - test.equal(t.child1.name, 'Child1'); - test.equal(t.child2.name, 'Child2'); - test.equal(t.child1_1.name, 'Child11'); - test.equal(t.child1_2.name, 'Child12'); - test.equal(t.child1_1_1.name, 'Child111'); - test.equal(t.child2_1.name, 'Child21'); + test.equal(t.child1.id, 'Child1'); + test.equal(t.child2.id, 'Child2'); + test.equal(t.child1_1.id, 'Child11'); + test.equal(t.child1_2.id, 'Child12'); + test.equal(t.child1_1_1.id, 'Child111'); + test.equal(t.child2_1.id, 'Child21'); test.done(); }, - 'construct name can only be alpha-numeric at least one character long'(test: Test) { + 'construct id can use any character except the path separator'(test: Test) { const root = new Root(); new Construct(root, 'valid'); new Construct(root, 'ValiD'); new Construct(root, 'Va123lid'); new Construct(root, 'v'); - test.throws(() => new Construct(root, ' invalid' ), Error, 'no spaces before'); - test.throws(() => new Construct(root, 'invalid ' ), Error, 'no spaces after'); - test.throws(() => new Construct(root, '123invalid' ), Error, 'name can\'t begin with a number'); - test.throws(() => new Construct(root, 'in valid' ), Error, 'spaces are not allowed'); - test.throws(() => new Construct(root, 'in_Valid' ), Error, 'underscores are not allowed'); - test.throws(() => new Construct(root, 'in-Valid' ), Error, 'hyphens are not allowed'); + new Construct(root, ' invalid' ); + new Construct(root, 'invalid ' ); + new Construct(root, '123invalid' ); + new Construct(root, 'in valid' ); + new Construct(root, 'in_Valid' ); + new Construct(root, 'in-Valid' ); + new Construct(root, 'in\\Valid' ); + new Construct(root, 'in.Valid' ); + test.throws(() => new Construct(root, 'in/Valid' ), Error, 'backslashes are not allowed'); - test.throws(() => new Construct(root, 'in\\Valid' ), Error, 'slashes are not allowed'); - test.throws(() => new Construct(root, 'in.Valid' ), Error, 'periods are not allowed'); + test.done(); + }, + + 'construct.uniqueId returns a tree-unique alphanumeric id of this construct'(test: Test) { + const root = new Root(); + + const child1 = new Construct(root, 'This is the first child'); + const child2 = new Construct(child1, 'Second level'); + const c1 = new Construct(child2, 'My construct'); + const c2 = new Construct(child1, 'My construct'); + test.deepEqual(c1.path, 'This is the first child/Second level/My construct'); + test.deepEqual(c2.path, 'This is the first child/My construct'); + test.deepEqual(c1.uniqueId, 'ThisisthefirstchildSecondlevelMyconstruct202131E0'); + test.deepEqual(c2.uniqueId, 'ThisisthefirstchildMyconstruct8C288DF9'); test.done(); }, @@ -64,7 +79,7 @@ export = { 'construct.findChild(name) can be used to retrieve a child from a parent'(test: Test) { const root = new Root(); const child = new Construct(root, 'Contruct'); - test.strictEqual(root.tryFindChild(child.name), child, 'findChild(name) can be used to retrieve the child from a parent'); + test.strictEqual(root.tryFindChild(child.id), child, 'findChild(name) can be used to retrieve the child from a parent'); test.ok(!root.tryFindChild('NotFound'), 'findChild(name) returns undefined if the child is not found'); test.done(); }, @@ -72,7 +87,7 @@ export = { 'construct.getChild(name) can be used to retrieve a child from a parent'(test: Test) { const root = new Root(); const child = new Construct(root, 'Contruct'); - test.strictEqual(root.findChild(child.name), child, 'getChild(name) can be used to retrieve the child from a parent'); + test.strictEqual(root.findChild(child.id), child, 'getChild(name) can be used to retrieve the child from a parent'); test.throws(() => { root.findChild('NotFound'); }, '', 'getChild(name) returns undefined if the child is not found'); diff --git a/packages/@aws-cdk/cdk/test/core/test.jsx.tsx b/packages/@aws-cdk/cdk/test/core/test.jsx.tsx index 29b34e3819540..3ba98f8f63092 100644 --- a/packages/@aws-cdk/cdk/test/core/test.jsx.tsx +++ b/packages/@aws-cdk/cdk/test/core/test.jsx.tsx @@ -4,11 +4,11 @@ import { Construct, jsx, Root } from '../../lib'; export = { 'jsx can be used to create "trees" of constructs'(test: Test) { const tree = - - - + + + - + ; const root = jsx.construct(tree); @@ -21,7 +21,7 @@ export = { }, 'jsx.construct(tree) will actually create the object'(test: Test) { - const my = jsx.construct() as MyConstruct; + const my = jsx.construct() as MyConstruct; test.equal(my.calculate(), 'prop1=hey, id=foo'); test.done(); }, @@ -29,9 +29,9 @@ export = { 'jsx.construct(tree, parent) can be used to add a JSX tree into an existing construct tree'(test: Test) { const root = new Root(); - jsx.construct(, root); + jsx.construct(, root); - test.equal(root.findChild('child').name, 'child'); + test.equal(root.findChild('child').id, 'child'); test.done(); } }; @@ -57,14 +57,15 @@ class MyConstruct extends Construct { /** * Constructor will always receive `props` as the 3rd argument with properties passed via JSX. */ - constructor(parent: Construct, name: string, props: MyConstructProps) { - super(parent, name); + constructor(parent: Construct, id: string, props: MyConstructProps) { + super(parent, id); this.str = 'prop1=' + props.prop1; if (props.prop2) { this.str += ', prop2=' + props.prop2.toString(); } - this.str += ', id=' + this.name; + this.str += ', id=' + this.id; + this.foo = 123; } public calculate() { diff --git a/packages/@aws-cdk/cdk/test/test.app.ts b/packages/@aws-cdk/cdk/test/test.app.ts index 65eabdd056b08..2e2f264257cde 100644 --- a/packages/@aws-cdk/cdk/test/test.app.ts +++ b/packages/@aws-cdk/cdk/test/test.app.ts @@ -222,7 +222,7 @@ export = { class Child extends Construct { public validate() { - return [ `Error from ${this.name}` ]; + return [ `Error from ${this.id}` ]; } } diff --git a/packages/@aws-cdk/cdk/test/test.context.ts b/packages/@aws-cdk/cdk/test/test.context.ts index 558f324c06c51..de59851775862 100644 --- a/packages/@aws-cdk/cdk/test/test.context.ts +++ b/packages/@aws-cdk/cdk/test/test.context.ts @@ -62,7 +62,7 @@ export = { test.deepEqual(new AvailabilityZoneProvider(stack).availabilityZones, [ 'dummy1a', 'dummy1b', 'dummy1c' ]); test.deepEqual(new SSMParameterProvider(child).getString('foo'), 'dummy'); - const output = app.synthesizeStack(stack.name); + const output = app.synthesizeStack(stack.id); const azError: MetadataEntry | undefined = output.metadata['/test-stack'].find(x => x.type === cxapi.ERROR_METADATA_KEY); const ssmError: MetadataEntry | undefined = output.metadata['/test-stack/ChildConstruct'].find(x => x.type === cxapi.ERROR_METADATA_KEY);