Skip to content

Commit

Permalink
refactor(core): use "string" for physical name props (#3011)
Browse files Browse the repository at this point in the history
We realized there is no need to use a strong type (`PhysicalName`) for resource physical names. Eventually users want to be able to either use the default, pass an explicit string for the name or request that the framework generate a name for them, conditionally (or, in the future, unconditionally) to whether the resource is being referenced across environments.

To that end, all physical name props have been converted back to `string` and a special token marker `PhysicalName.GENERATE_IF_NEEDED` was introduced which can be used to indicate that physical name will be generated if needed for cross environment references.

This change also modifies the protected API provided the `Resource` class:
- `physicalName` returns the value that should be passed to the CFN resource
- `getResourceNameAttribute(nameAttr)` returns a token that should be exposed as the resource name (e.g. `bucketName`).
- `getResourceArnAttribute(arnAttr, components)` returns a token that should bexposed as the resource's ARN (e.g. `bucketArn`).

All of these are "environment-sensitive" and will return "the right" value, depending on the resource's poster and configuration.
  • Loading branch information
Elad Ben-Israel authored Jun 23, 2019
1 parent 3b5b76d commit a942e4c
Show file tree
Hide file tree
Showing 101 changed files with 539 additions and 842 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-apigateway/lib/api-key.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, IResource as IResourceBase, PhysicalName, Resource } from '@aws-cdk/cdk';
import { Construct, IResource as IResourceBase, Resource } from '@aws-cdk/cdk';
import { CfnApiKey } from './apigateway.generated';
import { ResourceOptions } from "./resource";
import { RestApi } from './restapi';
Expand Down Expand Up @@ -70,7 +70,7 @@ export interface ApiKeyProps extends ResourceOptions {
* @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-apikey.html#cfn-apigateway-apikey-name
* @default automically generated name
*/
readonly apiKeyName?: PhysicalName;
readonly apiKeyName?: string;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import iam = require('@aws-cdk/aws-iam');
import { CfnOutput, Construct, IResource as IResourceBase, PhysicalName, Resource, Stack } from '@aws-cdk/cdk';
import { CfnOutput, Construct, IResource as IResourceBase, Resource, Stack } from '@aws-cdk/cdk';
import { ApiKey, IApiKey } from './api-key';
import { CfnAccount, CfnRestApi } from './apigateway.generated';
import { Deployment } from './deployment';
Expand Down Expand Up @@ -64,7 +64,7 @@ export interface RestApiProps extends ResourceOptions {
*
* @default - ID of the RestApi construct.
*/
readonly restApiName?: PhysicalName;
readonly restApiName?: string;

/**
* Custom header parameters for the request.
Expand Down Expand Up @@ -199,7 +199,7 @@ export class RestApi extends Resource implements IRestApi {

constructor(scope: Construct, id: string, props: RestApiProps = { }) {
super(scope, id, {
physicalName: props.restApiName || PhysicalName.of(id),
physicalName: props.restApiName || id,
});

const resource = new CfnRestApi(this, 'Resource', {
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-apigateway/lib/vpc-link.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2');
import { Construct, Lazy, PhysicalName, Resource } from '@aws-cdk/cdk';
import { Construct, Lazy, Resource } from '@aws-cdk/cdk';
import { CfnVpcLink } from './apigateway.generated';

/**
Expand All @@ -10,7 +10,7 @@ export interface VpcLinkProps {
* The name used to label and identify the VPC link.
* @default - automatically generated name
*/
readonly vpcLinkName?: PhysicalName;
readonly vpcLinkName?: string;

/**
* The description of the VPC link.
Expand Down Expand Up @@ -43,7 +43,7 @@ export class VpcLink extends Resource {
constructor(scope: Construct, id: string, props: VpcLinkProps = {}) {
super(scope, id, {
physicalName: props.vpcLinkName ||
PhysicalName.of(Lazy.stringValue({ produce: () => this.node.uniqueId })),
Lazy.stringValue({ produce: () => this.node.uniqueId }),
});

const cfnResource = new CfnVpcLink(this, 'Resource', {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-apigateway/test/test.restapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export = {
const api = new apigateway.RestApi(stack, 'restapi', {
deploy: false,
cloudWatchRole: false,
restApiName: cdk.PhysicalName.of('my-rest-api'),
restApiName: 'my-rest-api',
});

api.root.addMethod('GET');
Expand Down Expand Up @@ -176,7 +176,7 @@ export = {
const api = new apigateway.RestApi(stack, 'restapi', {
deploy: false,
cloudWatchRole: false,
restApiName: cdk.PhysicalName.of('my-rest-api'),
restApiName: 'my-rest-api',
});

// WHEN
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/test/test.vpc-link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export = {

// WHEN
new apigateway.VpcLink(stack, 'VpcLink', {
vpcLinkName: cdk.PhysicalName.of('MyLink'),
vpcLinkName: 'MyLink',
targets: [nlb]
});

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-autoscaling/lib/lifecycle-hook.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import iam = require('@aws-cdk/aws-iam');
import { Construct, Duration, IResource, PhysicalName, Resource } from '@aws-cdk/cdk';
import { Construct, Duration, IResource, Resource } from '@aws-cdk/cdk';
import { IAutoScalingGroup } from './auto-scaling-group';
import { CfnLifecycleHook } from './autoscaling.generated';
import { ILifecycleHookTarget } from './lifecycle-hook-target';
Expand All @@ -13,7 +13,7 @@ export interface BasicLifecycleHookProps {
*
* @default - Automatically generated name.
*/
readonly lifecycleHookName?: PhysicalName;
readonly lifecycleHookName?: string;

/**
* The action the Auto Scaling group takes when the lifecycle hook timeout elapses or if an unexpected failure occurs.
Expand Down
17 changes: 6 additions & 11 deletions packages/@aws-cdk/aws-cloudtrail/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import iam = require('@aws-cdk/aws-iam');
import kms = require('@aws-cdk/aws-kms');
import logs = require('@aws-cdk/aws-logs');
import s3 = require('@aws-cdk/aws-s3');
import { Construct, PhysicalName, Resource, Stack } from '@aws-cdk/cdk';
import { Construct, Resource, Stack } from '@aws-cdk/cdk';
import { CfnTrail } from './cloudtrail.generated';

// AWS::CloudTrail CloudFormation Resources:
Expand Down Expand Up @@ -86,7 +86,7 @@ export interface TrailProps {
*
* @default - AWS CloudFormation generated name.
*/
readonly trailName?: PhysicalName;
readonly trailName?: string;

/** An Amazon S3 object key prefix that precedes the name of all log files.
*
Expand Down Expand Up @@ -185,16 +185,11 @@ export class Trail extends Resource {
eventSelectors: this.eventSelectors
});

const resourceIdentifiers = this.getCrossEnvironmentAttributes({
arn: trail.attrArn,
name: trail.trailName || '',
arnComponents: {
service: 'cloudtrail',
resource: 'trail',
resourceName: this.physicalName,
},
this.trailArn = this.getResourceArnAttribute(trail.attrArn, {
service: 'cloudtrail',
resource: 'trail',
resourceName: this.physicalName,
});
this.trailArn = resourceIdentifiers.arn;
this.trailSnsTopicArn = trail.attrSnsTopicArn;

const s3BucketPolicy = s3bucket.node.findChild("Policy").node.findChild("Resource") as s3.CfnBucketPolicy;
Expand Down
17 changes: 6 additions & 11 deletions packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,18 +151,13 @@ export class Alarm extends Resource implements IAlarm {
})
});

const resourceIdentifiers = this.getCrossEnvironmentAttributes({
arn: alarm.attrArn,
name: alarm.ref,
arnComponents: {
service: 'cloudwatch',
resource: 'alarm',
resourceName: this.physicalName,
sep: ':',
},
this.alarmArn = this.getResourceArnAttribute(alarm.attrArn, {
service: 'cloudwatch',
resource: 'alarm',
resourceName: this.physicalName,
sep: ':',
});
this.alarmArn = resourceIdentifiers.arn;
this.alarmName = resourceIdentifiers.name;
this.alarmName = this.getResourceNameAttribute(alarm.ref);

this.metric = props.metric;
this.annotation = {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, Lazy, PhysicalName, Resource, Stack } from "@aws-cdk/cdk";
import { Construct, Lazy, Resource, Stack } from "@aws-cdk/cdk";
import { CfnDashboard } from './cloudwatch.generated';
import { Column, Row } from "./layout";
import { IWidget } from "./widget";
Expand All @@ -14,7 +14,7 @@ export interface DashboardProps {
*
* @default Automatically generated name
*/
readonly dashboardName?: PhysicalName;
readonly dashboardName?: string;

/**
* The start of the time range to use for each widget on the dashboard.
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ export interface CreateAlarmOptions {
*
* @default Automatically generated name
*/
readonly alarmName?: cdk.PhysicalName;
readonly alarmName?: string;

/**
* Description for the alarm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const alarm = metric.createAlarm(stack, 'Alarm', {
});

const dashboard = new cloudwatch.Dashboard(stack, 'Dash', {
dashboardName: cdk.PhysicalName.of('MyCustomDashboardName'),
dashboardName: 'MyCustomDashboardName',
start: '-9H',
end: '2018-12-17T06:00:00.000Z',
periodOverride: PeriodOverride.INHERIT
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect, haveResource, isSuperObject } from '@aws-cdk/assert';
import { App, PhysicalName, Stack } from '@aws-cdk/cdk';
import { App, Stack } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { Dashboard, GraphWidget, PeriodOverride, TextWidget } from '../lib';

Expand Down Expand Up @@ -127,7 +127,7 @@ export = {

// WHEN
new Dashboard(stack, 'MyDashboard', {
dashboardName: PhysicalName.of('MyCustomDashboardName'),
dashboardName: 'MyCustomDashboardName',
});

// THEN
Expand Down
21 changes: 8 additions & 13 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ export interface CommonProjectProps {
*
* @default - Name is automatically generated.
*/
readonly projectName?: PhysicalName;
readonly projectName?: string;

/**
* VPC network to place codebuild network interfaces
Expand Down Expand Up @@ -633,7 +633,7 @@ export class Project extends ProjectBase {
});

this.role = props.role || new iam.Role(this, 'Role', {
roleName: PhysicalName.auto({ crossEnvironment: true }),
roleName: PhysicalName.GENERATE_IF_NEEDED,
assumedBy: new iam.ServicePrincipal('codebuild.amazonaws.com')
});
this.grantPrincipal = this.role;
Expand Down Expand Up @@ -701,17 +701,12 @@ export class Project extends ProjectBase {

this.addVpcRequiredPermissions(props, resource);

const resourceIdentifiers = this.getCrossEnvironmentAttributes({
arn: resource.attrArn,
name: resource.ref,
arnComponents: {
service: 'codebuild',
resource: 'project',
resourceName: this.physicalName,
},
this.projectArn = this.getResourceArnAttribute(resource.attrArn, {
service: 'codebuild',
resource: 'project',
resourceName: this.physicalName,
});
this.projectArn = resourceIdentifiers.arn;
this.projectName = resourceIdentifiers.name;
this.projectName = this.getResourceNameAttribute(resource.ref);

this.addToRolePolicy(this.createLoggingPermission());

Expand Down Expand Up @@ -883,7 +878,7 @@ export class Project extends ProjectBase {
}));

const policy = new iam.Policy(this, 'PolicyDocument', {
policyName: PhysicalName.of('CodeBuildEC2Policy'),
policyName: 'CodeBuildEC2Policy',
statements: [
new iam.PolicyStatement({
resources: ['*'],
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codebuild/test/integ.project-vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const vpc = new ec2.Vpc(stack, 'MyVPC', {
const securityGroup = new ec2.SecurityGroup(stack, 'SecurityGroup1', {
allowAllOutbound: true,
description: 'Example',
securityGroupName: cdk.PhysicalName.of('Bob'),
securityGroupName: 'Bob',
vpc,
});
new codebuild.Project(stack, 'MyProject', {
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-codebuild/test/test.codebuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ export = {
const bucket = new s3.Bucket(stack, 'MyBucket');
const vpc = new ec2.Vpc(stack, 'MyVPC');
const securityGroup = new ec2.SecurityGroup(stack, 'SecurityGroup1', {
securityGroupName: cdk.PhysicalName.of('Bob'),
securityGroupName: 'Bob',
vpc,
allowAllOutbound: true,
description: 'Example',
Expand Down Expand Up @@ -677,7 +677,7 @@ export = {
const bucket = new s3.Bucket(stack, 'MyBucket');
const vpc = new ec2.Vpc(stack, 'MyVPC');
const securityGroup = new ec2.SecurityGroup(stack, 'SecurityGroup1', {
securityGroupName: cdk.PhysicalName.of('Bob'),
securityGroupName: 'Bob',
vpc,
allowAllOutbound: true,
description: 'Example',
Expand All @@ -699,7 +699,7 @@ export = {
const bucket = new s3.Bucket(stack, 'MyBucket');
const vpc = new ec2.Vpc(stack, 'MyVPC');
const securityGroup = new ec2.SecurityGroup(stack, 'SecurityGroup1', {
securityGroupName: cdk.PhysicalName.of('Bob'),
securityGroupName: 'Bob',
vpc,
allowAllOutbound: true,
description: 'Example',
Expand Down
18 changes: 6 additions & 12 deletions packages/@aws-cdk/aws-codecommit/lib/repository.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import events = require('@aws-cdk/aws-events');
import { Construct, IConstruct, IResource, PhysicalName, Resource, Stack } from '@aws-cdk/cdk';
import { Construct, IConstruct, IResource, Resource, Stack } from '@aws-cdk/cdk';
import { CfnRepository } from './codecommit.generated';

export interface IRepository extends IResource {
Expand Down Expand Up @@ -279,7 +279,7 @@ export class Repository extends RepositoryBase {

constructor(scope: Construct, id: string, props: RepositoryProps) {
super(scope, id, {
physicalName: PhysicalName.of(props.repositoryName),
physicalName: props.repositoryName,
});

this.repository = new CfnRepository(this, 'Resource', {
Expand All @@ -288,17 +288,11 @@ export class Repository extends RepositoryBase {
triggers: this.triggers
});

const resourceIdentifiers = this.getCrossEnvironmentAttributes({
arn: this.repository.attrArn,
name: this.repository.attrName,
arnComponents: {
service: 'codecommit',
resource: props.repositoryName,
},
this.repositoryName = this.getResourceNameAttribute(this.repository.attrName);
this.repositoryArn = this.getResourceArnAttribute(this.repository.attrArn, {
service: 'codecommit',
resource: this.physicalName,
});

this.repositoryArn = resourceIdentifiers.arn;
this.repositoryName = resourceIdentifiers.name;
}

public get repositoryCloneUrlHttp() {
Expand Down
21 changes: 8 additions & 13 deletions packages/@aws-cdk/aws-codedeploy/lib/lambda/application.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, IResource, PhysicalName, Resource } from '@aws-cdk/cdk';
import { Construct, IResource, Resource } from '@aws-cdk/cdk';
import { CfnApplication } from "../codedeploy.generated";
import { arnForApplication } from "../utils";

Expand Down Expand Up @@ -29,7 +29,7 @@ export interface LambdaApplicationProps {
*
* @default an auto-generated name will be used
*/
readonly applicationName?: PhysicalName;
readonly applicationName?: string;
}

/**
Expand Down Expand Up @@ -69,17 +69,12 @@ export class LambdaApplication extends Resource implements ILambdaApplication {
computePlatform: 'Lambda'
});

const resourceIdentifiers = this.getCrossEnvironmentAttributes({
arn: arnForApplication(resource.ref),
name: resource.ref,
arnComponents: {
service: 'codedeploy',
resource: 'application',
resourceName: this.physicalName,
sep: ':',
},
this.applicationName = this.getResourceNameAttribute(resource.ref);
this.applicationArn = this.getResourceArnAttribute(arnForApplication(resource.ref), {
service: 'codedeploy',
resource: 'application',
resourceName: this.physicalName,
sep: ':',
});
this.applicationName = resourceIdentifiers.name;
this.applicationArn = resourceIdentifiers.arn;
}
}
Loading

0 comments on commit a942e4c

Please sign in to comment.