Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow up on PR #556 #562

Merged
merged 1 commit into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -188,7 +188,7 @@ export abstract class FunctionRef extends cdk.Construct implements events.IEvent
}

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 @@ -76,9 +76,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 @@ -110,10 +110,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 @@ -122,7 +122,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 @@ -217,7 +217,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