From 4c1310dcfe8f49a68d2b49b8fd3c29f7633eb27e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 25 Oct 2020 12:06:40 +0200 Subject: [PATCH 1/2] feat: `node.addr` (deprecates `node.uniqueId`) The `uniqueId` property has a few issues: 1. It uses MD5 which is not FIPS-compliant (fixes #273) 2. They have variable length of up to 255 chars which may not be suitable in certain domains (e.g. k8s label names are limited to 63 characters). 3. The human-readable portion of `uniqueId` is sometimes confusing and in many cases does not offer sufficient value. 4. Their charset might be restrictive or too broad for certain domains. To that end, we introduce `node.addr` as an alternative to `uniqueId`. It returns a 42 character hexadecimal, lowercase and opaque address for a construct which is unique within the tree (app). The address is calculated using SHA-1 which is FIPS-compliant. For example: c83a2846e506bcc5f10682b564084bca2d275709ee Similar to `uniqueId`, `addr` intentionally skips any path components called `default` (case insensitive) to allow refactoring of construct trees which preserve names within the tree. --- API.md | 3 ++- src/construct.ts | 36 ++++++++++++++++++++++++++++++++---- src/private/uniqueid.ts | 30 +++++++++++++++++++++++++++--- test/construct.test.ts | 40 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 100 insertions(+), 9 deletions(-) diff --git a/API.md b/API.md index 5174618f..446a3752 100644 --- a/API.md +++ b/API.md @@ -173,6 +173,7 @@ new Node(host: Construct, scope: IConstruct, id: string) Name | Type | Description -----|------|------------- +**addr** | string | Returns an opaque tree-unique address for this construct. **children** | Array<[IConstruct](#constructs-iconstruct)> | All direct children of this construct. **dependencies** | Array<[Dependency](#constructs-dependency)> | Return all dependencies registered on this node or any of its children. **id** | string | The id of this construct within the current scope. @@ -181,7 +182,7 @@ Name | Type | Description **path** | string | The full, absolute path of this construct in the tree. **root** | [IConstruct](#constructs-iconstruct) | Returns the root of the construct tree. **scopes** | Array<[IConstruct](#constructs-iconstruct)> | All parent scopes of this construct. -**uniqueId** | string | A tree-global unique alphanumeric identifier for this construct. +**uniqueId**⚠️ | string | A tree-global unique alphanumeric identifier for this construct. **defaultChild**? | [IConstruct](#constructs-iconstruct) | Returns the child construct that has the id `Default` or `Resource"`.
__*Optional*__ **scope**? | [IConstruct](#constructs-iconstruct) | Returns the scope in which this construct is defined.
__*Optional*__ *static* **PATH_SEP** | string | Separator used to delimit construct path components. diff --git a/src/construct.ts b/src/construct.ts index ae333bfb..d9783f3b 100644 --- a/src/construct.ts +++ b/src/construct.ts @@ -2,7 +2,7 @@ import { IAspect } from './aspect'; import { ConstructMetadata, MetadataEntry } from './metadata'; import { DependableTrait } from './private/dependency'; import { captureStackTrace } from './private/stack-trace'; -import { makeUniqueId } from './private/uniqueid'; +import { makeLegacyUniqueId, addressOf } from './private/uniqueid'; const CONSTRUCT_NODE_PROPERTY_SYMBOL = Symbol.for('constructs.Construct.node'); @@ -56,6 +56,7 @@ export class Node { private readonly invokedAspects: IAspect[] = []; private _defaultChild: IConstruct | undefined; private readonly _validations = new Array(); + private _addr?: string; // cache constructor(private readonly host: Construct, scope: IConstruct, id: string) { id = id || ''; // if undefined, convert to empty string @@ -89,12 +90,39 @@ export class Node { } /** - * A tree-global unique alphanumeric identifier for this construct. - * Includes all components of the tree. + * Returns an opaque tree-unique address for this construct. + * + * Addresses are 42 characters hexadecimal strings. They begin with "c8" + * followed by 40 lowercase hexadecimal characters (0-9a-f). + * + * Addresses are calculated using a SHA-1 of the components of the construct + * path. + * + * To enable refactorings of construct trees, constructs with the ID `default` + * (case insensitive) will be excluded from the calculation. In those cases + * constructs in the same tree may have the same addreess. + * + * @example c83a2846e506bcc5f10682b564084bca2d275709ee + */ + public get addr(): string { + if (!this._addr) { + this._addr = addressOf(this.scopes.map(c => Node.of(c).id)); + } + + return this._addr; + } + + /** + * A tree-global unique alphanumeric identifier for this construct. Includes + * all components of the tree. + * + * @deprecated please avoid using this property and use `uid` instead. This + * algorithm uses MD5, which is not FIPS-complient and also excludes the + * identity of the root construct from the calculation. */ public get uniqueId(): string { const components = this.scopes.slice(1).map(c => Node.of(c).id); - return components.length > 0 ? makeUniqueId(components) : ''; + return components.length > 0 ? makeLegacyUniqueId(components) : ''; } /** diff --git a/src/private/uniqueid.ts b/src/private/uniqueid.ts index 9d82e419..dce9061d 100644 --- a/src/private/uniqueid.ts +++ b/src/private/uniqueid.ts @@ -20,6 +20,30 @@ const HASH_LEN = 8; const MAX_HUMAN_LEN = 240; // max ID len is 255 const MAX_ID_LEN = 255; +/** + * Calculates the construct uid based on path components. + * + * Components named `Default` (case insensitive) are excluded from uid calculation + * to allow tree refactorings. + * + * @param components path components + */ +export function addressOf(components: string[]) { + const hash = crypto.createHash('sha1'); + for (const c of components) { + // skip "default". + if (c.toLocaleLowerCase() === HIDDEN_ID.toLocaleLowerCase()) { + continue; + } + + hash.update(c); + hash.update('\n'); + } + + // prefix with "c8" so to ensure it starts with non-digit. + return 'c8' + hash.digest('hex'); +} + /** * Calculates a unique ID for a set of textual components. * @@ -29,7 +53,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 makeLegacyUniqueId(components: string[]) { components = components.filter(x => x !== HIDDEN_ID); if (components.length === 0) { @@ -54,7 +78,7 @@ export function makeUniqueId(components: string[]) { } } - const hash = pathHash(components); + const hash = legacyPathHash(components); const human = removeDupes(components) .filter(x => x !== HIDDEN_FROM_HUMAN_ID) .map(removeNonAlphanumeric) @@ -69,7 +93,7 @@ export function makeUniqueId(components: string[]) { * * The hash is limited in size. */ -function pathHash(path: string[]): string { +function legacyPathHash(path: string[]): string { const md5 = crypto.createHash('md5').update(path.join(PATH_SEP)).digest('hex'); return md5.slice(0, HASH_LEN).toUpperCase(); } diff --git a/test/construct.test.ts b/test/construct.test.ts index daf37353..6bbfbfc7 100644 --- a/test/construct.test.ts +++ b/test/construct.test.ts @@ -57,7 +57,7 @@ test('if "undefined" is forcefully used as an "id", it will be treated as an emp expect(Node.of(c).id).toBe(''); }); -test('construct.uniqueId returns a tree-unique alphanumeric id of this construct', () => { +test('construct.uniqueId (deprecated) returns a tree-unique alphanumeric id of this construct', () => { const root = new Root(); const child1 = new Construct(root, 'This is the first child'); @@ -77,6 +77,44 @@ test('cannot calculate uniqueId if the construct path is ["Default"]', () => { expect(() => Node.of(c).uniqueId).toThrow(/Unable to calculate a unique id for an empty set of components/); }); +test('node.addr returns an opaque app-unique address for any construct', () => { + 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'); + + expect(Node.of(c1).path).toBe('This is the first child/Second level/My construct'); + expect(Node.of(c2).path).toBe('This is the first child/My construct'); + expect(Node.of(child1).addr).toBe('c8a0dfcbdc45cb728d75ebe6914d369e565dc3f61c'); + expect(Node.of(child2).addr).toBe('c825c5541e02ebd68e79ea636e370985b6c2de40a9'); + expect(Node.of(c1).addr).toBe('c83a2846e506bcc5f10682b564084bca2d275709ee'); + expect(Node.of(c2).addr).toBe('c8003bcb3e82977712d0d7220b155cb69abd9ad383'); +}); + +test('node.addr excludes "default" from the address calculation', () => { + // GIVEN + const root = new Root(); + const c1 = new Construct(root, 'c1'); + + // WHEN: + const group1 = new Construct(root, 'default'); + const c1a = new Construct(group1, 'c1'); + const group2 = new Construct(root, 'DeFAULt'); + const c1b = new Construct(group2, 'c1'); + + + // THEN: all addresses are the same because they go through "default" + const addr = Node.of(c1).addr; + const addrA = Node.of(c1a).addr; + const addrB = Node.of(c1b).addr; + + expect(addr).toEqual('c86a34031367d11f4bef80afca42b7e7e5c6253b77'); + expect(addrA).toEqual(addr); + expect(addrB).toEqual(addr); +}); + test('construct.getChildren() returns an array of all children', () => { const root = new Root(); const child = new Construct(root, 'Child1'); From d98c34cef1b50fe2e8b1b88d0b4c2e91e0269043 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 25 Oct 2020 12:17:47 +0200 Subject: [PATCH 2/2] change "Default" to case sensitive --- src/construct.ts | 6 +++--- src/private/uniqueid.ts | 8 +++----- test/construct.test.ts | 8 ++++---- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/construct.ts b/src/construct.ts index d9783f3b..4bd3b16c 100644 --- a/src/construct.ts +++ b/src/construct.ts @@ -98,9 +98,9 @@ export class Node { * Addresses are calculated using a SHA-1 of the components of the construct * path. * - * To enable refactorings of construct trees, constructs with the ID `default` - * (case insensitive) will be excluded from the calculation. In those cases - * constructs in the same tree may have the same addreess. + * To enable refactorings of construct trees, constructs with the ID `Default` + * will be excluded from the calculation. In those cases constructs in the + * same tree may have the same addreess. * * @example c83a2846e506bcc5f10682b564084bca2d275709ee */ diff --git a/src/private/uniqueid.ts b/src/private/uniqueid.ts index dce9061d..3cc5767e 100644 --- a/src/private/uniqueid.ts +++ b/src/private/uniqueid.ts @@ -23,7 +23,7 @@ const MAX_ID_LEN = 255; /** * Calculates the construct uid based on path components. * - * Components named `Default` (case insensitive) are excluded from uid calculation + * Components named `Default` (case sensitive) are excluded from uid calculation * to allow tree refactorings. * * @param components path components @@ -31,10 +31,8 @@ const MAX_ID_LEN = 255; export function addressOf(components: string[]) { const hash = crypto.createHash('sha1'); for (const c of components) { - // skip "default". - if (c.toLocaleLowerCase() === HIDDEN_ID.toLocaleLowerCase()) { - continue; - } + // skip components called "Default" to enable refactorings + if (c === HIDDEN_ID) { continue; } hash.update(c); hash.update('\n'); diff --git a/test/construct.test.ts b/test/construct.test.ts index 6bbfbfc7..e0026b67 100644 --- a/test/construct.test.ts +++ b/test/construct.test.ts @@ -99,12 +99,11 @@ test('node.addr excludes "default" from the address calculation', () => { const c1 = new Construct(root, 'c1'); // WHEN: - const group1 = new Construct(root, 'default'); + const group1 = new Construct(root, 'Default'); // <-- this is a "hidden node" const c1a = new Construct(group1, 'c1'); - const group2 = new Construct(root, 'DeFAULt'); + const group2 = new Construct(root, 'DeFAULt'); // <-- not hidden, "Default" is case sensitive const c1b = new Construct(group2, 'c1'); - // THEN: all addresses are the same because they go through "default" const addr = Node.of(c1).addr; const addrA = Node.of(c1a).addr; @@ -112,7 +111,8 @@ test('node.addr excludes "default" from the address calculation', () => { expect(addr).toEqual('c86a34031367d11f4bef80afca42b7e7e5c6253b77'); expect(addrA).toEqual(addr); - expect(addrB).toEqual(addr); + expect(addrB).toEqual('c8fa72abd28f794f6bacb100b26beb761d004572f5'); + expect(addrB).not.toEqual(addr); }); test('construct.getChildren() returns an array of all children', () => {