Skip to content

Commit

Permalink
feat: introduce "node.addr" (deprecates "uniqueId") (#314)
Browse files Browse the repository at this point in the history
The `uniqueId` property has a few issues:

1. It uses MD5 which is not FIPS-compliant (fixes #272)
2. It has a 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. Its 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` to allow refactoring of construct trees which preserve names within the tree.


*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Elad Ben-Israel authored Oct 26, 2020
1 parent 8e63305 commit 754a84d
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 9 deletions.
3 changes: 2 additions & 1 deletion API.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ new Node(host: Construct, scope: IConstruct, id: string)

Name | Type | Description
-----|------|-------------
**addr** | <code>string</code> | Returns an opaque tree-unique address for this construct.
**children** | <code>Array<[IConstruct](#constructs-iconstruct)></code> | All direct children of this construct.
**dependencies** | <code>Array<[Dependency](#constructs-dependency)></code> | Return all dependencies registered on this node or any of its children.
**id** | <code>string</code> | The id of this construct within the current scope.
Expand All @@ -181,7 +182,7 @@ Name | Type | Description
**path** | <code>string</code> | The full, absolute path of this construct in the tree.
**root** | <code>[IConstruct](#constructs-iconstruct)</code> | Returns the root of the construct tree.
**scopes** | <code>Array<[IConstruct](#constructs-iconstruct)></code> | All parent scopes of this construct.
**uniqueId** | <code>string</code> | A tree-global unique alphanumeric identifier for this construct.
**uniqueId**⚠️ | <code>string</code> | A tree-global unique alphanumeric identifier for this construct.
**defaultChild**? | <code>[IConstruct](#constructs-iconstruct)</code> | Returns the child construct that has the id `Default` or `Resource"`.<br/>__*Optional*__
**scope**? | <code>[IConstruct](#constructs-iconstruct)</code> | Returns the scope in which this construct is defined.<br/>__*Optional*__
*static* **PATH_SEP** | <code>string</code> | Separator used to delimit construct path components.
Expand Down
36 changes: 32 additions & 4 deletions src/construct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -56,6 +56,7 @@ export class Node {
private readonly invokedAspects: IAspect[] = [];
private _defaultChild: IConstruct | undefined;
private readonly _validations = new Array<IValidation>();
private _addr?: string; // cache

constructor(private readonly host: Construct, scope: IConstruct, id: string) {
id = id || ''; // if undefined, convert to empty string
Expand Down Expand Up @@ -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`
* 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) : '';
}

/**
Expand Down
28 changes: 25 additions & 3 deletions src/private/uniqueid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,28 @@ 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 sensitive) 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 components called "Default" to enable refactorings
if (c === HIDDEN_ID) { 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.
*
Expand All @@ -29,7 +51,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) {
Expand All @@ -54,7 +76,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)
Expand All @@ -69,7 +91,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();
}
Expand Down
40 changes: 39 additions & 1 deletion test/construct.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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'); // <-- this is a "hidden node"
const c1a = new Construct(group1, 'c1');
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;
const addrB = Node.of(c1b).addr;

expect(addr).toEqual('c86a34031367d11f4bef80afca42b7e7e5c6253b77');
expect(addrA).toEqual(addr);
expect(addrB).toEqual('c8fa72abd28f794f6bacb100b26beb761d004572f5');
expect(addrB).not.toEqual(addr);
});

test('construct.getChildren() returns an array of all children', () => {
const root = new Root();
const child = new Construct(root, 'Child1');
Expand Down

0 comments on commit 754a84d

Please sign in to comment.