diff --git a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts index de2fab0677c58..bbfb3c88970b7 100644 --- a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts +++ b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts @@ -121,6 +121,31 @@ export = { test.done(); }, + 'extractMetric allows passing in namespaces with "/"'(test: Test) { + // GIVEN + const stack = new Stack(); + const lg = new LogGroup(stack, 'LogGroup'); + + // WHEN + const metric = lg.extractMetric('$.myField', 'MyNamespace/MyService', 'Field'); + + // THEN + expect(stack).to(haveResource('AWS::Logs::MetricFilter', { + FilterPattern: "{ $.myField = \"*\" }", + MetricTransformations: [ + { + MetricName: "Field", + MetricNamespace: "MyNamespace/MyService", + MetricValue: "$.myField" + } + ] + })); + test.equal(metric.namespace, 'MyNamespace/MyService'); + test.equal(metric.metricName, 'Field'); + + test.done(); + }, + 'grant'(test: Test) { // GIVEN const stack = new Stack(); diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts b/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts index 930b6ffb27111..c4305e8cd7b36 100644 --- a/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts +++ b/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts @@ -114,7 +114,7 @@ export class Stack extends Construct { this.env = this.parseEnvironment(props); this.logicalIds = new LogicalIDs(props && props.namingScheme ? props.namingScheme : new HashedAddressingScheme()); - this.name = name || 'Stack'; + this.name = this.id; } /** @@ -236,7 +236,7 @@ export class Stack extends Construct { * character classes, and we don't allow one of the magic markers. */ protected _validateId(name: string) { - if (!Stack.VALID_STACK_NAME_REGEX.test(name)) { + if (name && !Stack.VALID_STACK_NAME_REGEX.test(name)) { throw new Error(`Stack name must match the regular expression: ${Stack.VALID_STACK_NAME_REGEX.toString()}, got '${name}'`); } } diff --git a/packages/@aws-cdk/cdk/lib/core/construct.ts b/packages/@aws-cdk/cdk/lib/core/construct.ts index 66b720aec5d0a..1fff75c333bbe 100644 --- a/packages/@aws-cdk/cdk/lib/core/construct.ts +++ b/packages/@aws-cdk/cdk/lib/core/construct.ts @@ -48,9 +48,13 @@ export class Construct { * Creates a new construct node. * * @param parent The parent construct - * @param props Properties for this construct + * @param id The local logical ID of the construct. Must be unique amongst + * siblings. If the ID includes a path separator (`/`), then it will be + * replaced by double dash `--`. */ constructor(parent: Construct, id: string) { + id = id || ''; // if undefined, convert to empty string + this.id = id; this.parent = parent; @@ -68,10 +72,11 @@ export class Construct { this.id = id; } - // Validate the name we ended up with - if (this.id !== '') { - this._validateId(this.id); - } + // escape any path separators so they don't wreck havoc + this.id = this._escapePathSeparator(this.id); + + // allow derived classes to validate the construct id + this._validateId(this.id); const components = this.rootPath().map(c => c.id); this.path = components.join(PATH_SEP); @@ -105,6 +110,9 @@ export class Construct { /** * Return a descendant by path, or undefined * + * Note that if the original ID of the construct you are looking for contained + * a '/', then it would have been replaced by '--'. + * * @param name Relative name of a direct or indirect child * @returns a child by path or undefined if not found. */ @@ -127,6 +135,9 @@ export class Construct { * * Throws an exception if the descendant is not found. * + * Note that if the original ID of the construct you are looking for contained + * a '/', then it would have been replaced by '--'. + * * @param name Relative name of a direct or indirect child * @returns Child with the given path. */ @@ -292,10 +303,8 @@ export class Construct { * Validate that the id of the construct legal. * Construct IDs can be any characters besides the path separator. */ - protected _validateId(id: string) { - if (id.indexOf(PATH_SEP) !== -1) { - throw new Error(`Construct names cannot include '${PATH_SEP}': ${id}`); - } + protected _validateId(_id: string) { + // can be used by derived classes to customize ID validation. } /** @@ -387,6 +396,14 @@ export class Construct { return false; } + + /** + * If the construct ID contains a path separator, it is replaced by double dash (`--`). + */ + private _escapePathSeparator(id: string) { + if (!id) { return id; } + return id.split(PATH_SEP).join('--'); + } } /** diff --git a/packages/@aws-cdk/cdk/lib/util/uniqueid.ts b/packages/@aws-cdk/cdk/lib/util/uniqueid.ts index 5c3630d39ea95..b9a9e34eb97fe 100644 --- a/packages/@aws-cdk/cdk/lib/util/uniqueid.ts +++ b/packages/@aws-cdk/cdk/lib/util/uniqueid.ts @@ -1,5 +1,6 @@ // tslint:disable-next-line:no-var-requires import crypto = require('crypto'); +import { unresolved } from '../core/tokens'; /** * Resources with this ID are hidden from humans @@ -35,6 +36,11 @@ export function makeUniqueId(components: string[]) { throw new Error('Unable to calculate a unique id for an empty set of components'); } + const unresolvedTokens = components.filter(c => unresolved(c)); + if (unresolvedTokens.length > 0) { + throw new Error(`ID components may not include unresolved tokens: ${unresolvedTokens.join(',')}`); + } + // 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. diff --git a/packages/@aws-cdk/cdk/test/core/test.construct.ts b/packages/@aws-cdk/cdk/test/core/test.construct.ts index 59e9d5c63a38e..22146f69fd077 100644 --- a/packages/@aws-cdk/cdk/test/core/test.construct.ts +++ b/packages/@aws-cdk/cdk/test/core/test.construct.ts @@ -1,6 +1,6 @@ import cxapi = require('@aws-cdk/cx-api'); import { Test } from 'nodeunit'; -import { Construct, Root } from '../../lib'; +import { Construct, Root, Token } from '../../lib'; // tslint:disable:variable-name // tslint:disable:max-line-length @@ -47,8 +47,29 @@ export = { new Construct(root, 'in-Valid' ); new Construct(root, 'in\\Valid' ); new Construct(root, 'in.Valid' ); + test.done(); + }, + + 'if construct id contains path seperators, they will be replaced by double-dash'(test: Test) { + const root = new Root(); + const c = new Construct(root, 'Boom/Boom/Bam'); + test.deepEqual(c.id, 'Boom--Boom--Bam'); + test.done(); + }, + + 'if "undefined" is forcefully used as an "id", it will be treated as an empty string'(test: Test) { + const c = new Construct(undefined as any, undefined as any); + test.deepEqual(c.id, ''); + test.done(); + }, + + "dont allow unresolved tokens to be used in construct IDs"(test: Test) { + // GIVEN + const root = new Root(); + const token = new Token(() => 'lazy'); - test.throws(() => new Construct(root, 'in/Valid' ), Error, 'backslashes are not allowed'); + // WHEN + THEN + test.throws(() => new Construct(root, `MyID: ${token}`), /ID components may not include unresolved tokens: MyID: \${Token/); test.done(); }, diff --git a/packages/@aws-cdk/cdk/test/test.environment.ts b/packages/@aws-cdk/cdk/test/test.environment.ts index fed5b067e89c6..1e47890f207c9 100644 --- a/packages/@aws-cdk/cdk/test/test.environment.ts +++ b/packages/@aws-cdk/cdk/test/test.environment.ts @@ -17,7 +17,7 @@ export = { app.setContext(DEFAULT_ACCOUNT_CONTEXT_KEY, 'my-default-account'); app.setContext(DEFAULT_REGION_CONTEXT_KEY, 'my-default-region'); - const stack = new Stack(app); + const stack = new Stack(app, 'my-stack'); test.equal(stack.env.account, 'my-default-account'); test.equal(stack.env.region, 'my-default-region');