Skip to content

Commit

Permalink
chore(cdk): Follow up on PR #556 (#562)
Browse files Browse the repository at this point in the history
* Use node.js "crypto" module instead of md5
* Remove `construct.name` (and fix all dependencies)
* Remove non-alphanumeric after "Resource" is omitted

BREAKING CHANGE: `construct.name` => `construct.id`
  • Loading branch information
Elad Ben-Israel committed Aug 14, 2018
1 parent bde710f commit 3524bf4
Show file tree
Hide file tree
Showing 15 changed files with 59 additions and 330 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export abstract class CloudFormationAction extends codepipeline.DeployAction {
});

if (props.outputFileName) {
this.artifact = this.addOutputArtifact(props.outputArtifactName || (parent.name + this.name + 'Artifact'));
this.artifact = this.addOutputArtifact(props.outputArtifactName || (parent.id + this.id + 'Artifact'));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudtrail/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export class CloudTrail extends cdk.Construct {
includeGlobalServiceEvents: props.includeGlobalServiceEvents == null ? true : props.includeGlobalServiceEvents,
trailName: props.trailName,
kmsKeyId: props.kmsKey && props.kmsKey.keyArn,
s3BucketName: s3bucket.name,
s3BucketName: s3bucket.bucketName,
s3KeyPrefix: props.s3KeyPrefix,
cloudWatchLogsLogGroupArn: this.cloudWatchLogsGroupArn,
cloudWatchLogsRoleArn: this.cloudWatchLogsRoleArn,
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export abstract class ProjectRef extends cdk.Construct implements events.IEventR
}

return {
id: this.name,
id: this.id,
arn: this.projectArn,
roleArn: this.eventsRole.roleArn,
};
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-codepipeline/lib/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export abstract class Action extends cdk.Construct {
*/
public render(): cloudformation.PipelineResource.ActionDeclarationProperty {
return {
name: this.name,
name: this.id,
inputArtifacts: this.inputArtifacts.map(a => ({ name: a.name })),
actionTypeId: {
category: this.category.toString(),
Expand All @@ -174,8 +174,8 @@ export abstract class Action extends cdk.Construct {
source: [ 'aws.codepipeline' ],
resources: [ this.stage.pipeline.pipelineArn ],
detail: {
stage: [ this.stage.name ],
action: [ this.name ],
stage: [ this.stage.id ],
action: [ this.id ],
},
});
return rule;
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
}

return {
id: this.name,
id: this.id,
arn: this.pipelineArn,
roleArn: this.eventsRole.roleArn,
};
Expand Down Expand Up @@ -224,8 +224,8 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
}

private appendStage(stage: Stage) {
if (this.stages.find(x => x.name === stage.name)) {
throw new Error(`A stage with name '${stage.name}' already exists`);
if (this.stages.find(x => x.id === stage.id)) {
throw new Error(`A stage with name '${stage.id}' already exists`);
}

this.stages.push(stage);
Expand All @@ -235,7 +235,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
return util.flatMap(this.stages, (stage, i) => {
const onlySourceActionsPermitted = i === 0;
return util.flatMap(stage.actions, (action, _) =>
validation.validateSourceAction(onlySourceActionsPermitted, action.category, action.name, stage.name)
validation.validateSourceAction(onlySourceActionsPermitted, action.category, action.id, stage.id)
);
});
}
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-codepipeline/lib/stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class Stage extends cdk.Construct {

public render(): cloudformation.PipelineResource.StageDeclarationProperty {
return {
name: this.name,
name: this.id,
actions: this._actions.map(action => action.render())
};
}
Expand All @@ -59,7 +59,7 @@ export class Stage extends cdk.Construct {
source: [ 'aws.codepipeline' ],
resources: [ this.pipeline.pipelineArn ],
detail: {
stage: [ this.name ],
stage: [ this.id ],
},
});
return rule;
Expand All @@ -84,7 +84,7 @@ export class Stage extends cdk.Construct {

private validateHasActions(): string[] {
if (this._actions.length === 0) {
return [`Stage '${this.name}' must have at least one action`];
return [`Stage '${this.id}' must have at least one action`];
}
return [];
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export abstract class FunctionRef extends cdk.Construct
}

return {
id: this.name,
id: this.id,
arn: this.functionArn,
};
}
Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-sns/lib/topic-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ export abstract class TopicRef extends cdk.Construct implements events.IEventRul
* @param queue The target queue
*/
public subscribeQueue(queue: sqs.QueueRef) {
const subscriptionName = queue.name + 'Subscription';
const subscriptionName = queue.id + 'Subscription';
if (this.tryFindChild(subscriptionName)) {
throw new Error(`A subscription between the topic ${this.name} and the queue ${queue.name} already exists`);
throw new Error(`A subscription between the topic ${this.id} and the queue ${queue.id} already exists`);
}

// we use the queue name as the subscription's. there's no meaning to subscribing
Expand Down Expand Up @@ -114,10 +114,10 @@ export abstract class TopicRef extends cdk.Construct implements events.IEventRul
* @param lambdaFunction The Lambda function to invoke
*/
public subscribeLambda(lambdaFunction: lambda.FunctionRef) {
const subscriptionName = lambdaFunction.name + 'Subscription';
const subscriptionName = lambdaFunction.id + 'Subscription';

if (this.tryFindChild(subscriptionName)) {
throw new Error(`A subscription between the topic ${this.name} and the lambda ${lambdaFunction.name} already exists`);
throw new Error(`A subscription between the topic ${this.id} and the lambda ${lambdaFunction.id} already exists`);
}

const sub = new Subscription(this, subscriptionName, {
Expand All @@ -126,7 +126,7 @@ export abstract class TopicRef extends cdk.Construct implements events.IEventRul
protocol: SubscriptionProtocol.Lambda
});

lambdaFunction.addPermission(this.name, {
lambdaFunction.addPermission(this.id, {
sourceArn: this.topicArn,
principal: new cdk.ServicePrincipal('sns.amazonaws.com'),
});
Expand Down Expand Up @@ -223,7 +223,7 @@ export abstract class TopicRef extends cdk.Construct implements events.IEventRul
}

return {
id: this.name,
id: this.id,
arn: this.topicArn,
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/lib/cloudformation/logical-id.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { makeUniqueId } from '../util/uniqueid';
import { StackElement } from "./stack";
import { StackElement } from './stack';

const PATH_SEP = '/';

Expand Down
9 changes: 8 additions & 1 deletion packages/@aws-cdk/cdk/lib/cloudformation/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,16 @@ export class Stack extends Construct {
*/
public readonly templateOptions: TemplateOptions = {};

/**
* The CloudFormation stack name.
*/
public readonly name: string;

/**
* Creates a new stack.
*
* @param parent Parent of this stack, usually a Program instance.
* @param name The name of the CloudFormation stack. Defaults to "Stack".
* @param props Stack properties.
*/
public constructor(parent?: App, name?: string, props?: StackProps) {
Expand All @@ -98,6 +104,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';
}

/**
Expand Down Expand Up @@ -195,7 +202,7 @@ export class Stack extends Construct {
* CloudFormation stack names can include dashes in addition to the regular identifier
* character classes, and we don't allow one of the magic markers.
*/
protected _validateName(name: string) {
protected _validateId(name: string) {
if (!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}'`);
}
Expand Down
25 changes: 10 additions & 15 deletions packages/@aws-cdk/cdk/lib/core/construct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,9 @@ export class Construct {
public readonly parent?: 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`.
* The local id of the construct.
* This id is unique amongst its siblings.
* To obtain a tree-global unique id for this construct, use `uniqueId`.
*/
public readonly id: string;

Expand All @@ -30,7 +26,7 @@ export class Construct {
public readonly path: string;

/**
* A tree-unique alpha-numeric identifier for this construct.
* A tree-global unique alphanumeric identifier for this construct.
* Includes all components of the tree.
*/
public readonly uniqueId: string;
Expand All @@ -56,7 +52,6 @@ export class Construct {
*/
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
Expand All @@ -75,7 +70,7 @@ export class Construct {

// Validate the name we ended up with
if (this.id !== '') {
this._validateName(this.id);
this._validateId(this.id);
}

const components = this.rootPath().map(c => c.id);
Expand Down Expand Up @@ -294,12 +289,12 @@ export class Construct {
}

/**
* Validate that the name of the construct is a legal identifier.
* Construct names can be any characters besides the path separator.
* Validate that the id of the construct legal.
* Construct IDs can be any characters besides the path separator.
*/
protected _validateName(name: string) {
if (name.indexOf(PATH_SEP) !== -1) {
throw new Error(`Construct names cannot include '${PATH_SEP}': ${name}`);
protected _validateId(id: string) {
if (id.indexOf(PATH_SEP) !== -1) {
throw new Error(`Construct names cannot include '${PATH_SEP}': ${id}`);
}
}

Expand Down
2 changes: 0 additions & 2 deletions packages/@aws-cdk/cdk/lib/util/.gitignore

This file was deleted.

Loading

0 comments on commit 3524bf4

Please sign in to comment.