Skip to content

Commit

Permalink
feat: physical names in the entire Construct Library (#2894)
Browse files Browse the repository at this point in the history
This PR changes the xyzName attribute of all Resources in the Construct Library to be of type PhysicalName,
and adds an AWS linter rule that checks ensures conformance.

The name of the property can be any ending substring of the base name of the resource with the "Name" suffix -
for example, if my resource is AWS::MyService::AwesomeFoobar, the name can be foobarName or awesomeFoobarName.

BREAKING CHANGE: all <resource>Name attributes had their type changed from string to cdk.PhysicalName.
  • Loading branch information
skinny85 committed Jun 19, 2019
1 parent ea10f0d commit d9d3a99
Show file tree
Hide file tree
Showing 135 changed files with 1,087 additions and 511 deletions.
10 changes: 6 additions & 4 deletions packages/@aws-cdk/aws-apigateway/lib/api-key.ts
@@ -1,4 +1,4 @@
import { Construct, IResource as IResourceBase, Resource } from '@aws-cdk/cdk';
import { Construct, IResource as IResourceBase, PhysicalName, 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 name?: string;
readonly apiKeyName?: PhysicalName;
}

/**
Expand All @@ -83,14 +83,16 @@ export class ApiKey extends Resource implements IApiKey {
public readonly keyId: string;

constructor(scope: Construct, id: string, props: ApiKeyProps = { }) {
super(scope, id);
super(scope, id, {
physicalName: props.apiKeyName,
});

const resource = new CfnApiKey(this, 'Resource', {
customerId: props.customerId,
description: props.description,
enabled: props.enabled || true,
generateDistinctId: props.generateDistinctId,
name: props.name,
name: this.physicalName.value,
stageKeys: this.renderStageKeys(props.resources)
});

Expand Down
10 changes: 6 additions & 4 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
@@ -1,5 +1,5 @@
import iam = require('@aws-cdk/aws-iam');
import { CfnOutput, Construct, IResource as IResourceBase, Resource, Stack } from '@aws-cdk/cdk';
import { CfnOutput, Construct, IResource as IResourceBase, PhysicalName, 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?: string;
readonly restApiName?: PhysicalName;

/**
* Custom header parameters for the request.
Expand Down Expand Up @@ -198,10 +198,12 @@ export class RestApi extends Resource implements IRestApi {
private _latestDeployment: Deployment | undefined;

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

const resource = new CfnRestApi(this, 'Resource', {
name: props.restApiName || id,
name: this.physicalName.value!,
description: props.description,
policy: props.policy,
failOnWarnings: props.failOnWarnings,
Expand Down
11 changes: 7 additions & 4 deletions packages/@aws-cdk/aws-apigateway/lib/vpc-link.ts
@@ -1,5 +1,5 @@
import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2');
import { Construct, Lazy, Resource } from '@aws-cdk/cdk';
import { Construct, Lazy, PhysicalName, 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?: string;
readonly vpcLinkName?: PhysicalName;

/**
* The description of the VPC link.
Expand Down Expand Up @@ -41,10 +41,13 @@ export class VpcLink extends Resource {
private readonly targets = new Array<elbv2.INetworkLoadBalancer>();

constructor(scope: Construct, id: string, props: VpcLinkProps = {}) {
super(scope, id);
super(scope, id, {
physicalName: props.vpcLinkName ||
PhysicalName.of(Lazy.stringValue({ produce: () => this.node.uniqueId })),
});

const cfnResource = new CfnVpcLink(this, 'Resource', {
name: props.vpcLinkName || this.node.uniqueId,
name: this.physicalName.value!,
description: props.description,
targetArns: Lazy.listValue({ produce: () => this.renderTargets() })
});
Expand Down
11 changes: 9 additions & 2 deletions packages/@aws-cdk/aws-apigateway/package.json
Expand Up @@ -93,8 +93,15 @@
"exclude": [
"from-method:@aws-cdk/aws-apigateway.Resource",
"from-method:@aws-cdk/aws-apigateway.ApiKey",
"ref-via-interface:@aws-cdk/aws-apigateway.ApiKeyProps.resources"
"ref-via-interface:@aws-cdk/aws-apigateway.ApiKeyProps.resources",
"props-physical-name:@aws-cdk/aws-apigateway.DeploymentProps",
"props-physical-name:@aws-cdk/aws-apigateway.MethodProps",
"props-physical-name:@aws-cdk/aws-apigateway.ProxyResourceProps",
"props-physical-name:@aws-cdk/aws-apigateway.ResourceProps",
"props-physical-name:@aws-cdk/aws-apigateway.StageProps",
"props-physical-name:@aws-cdk/aws-apigateway.UsagePlanProps",
"props-physical-name:@aws-cdk/aws-apigateway.LambdaRestApiProps"
]
},
"stability": "experimental"
}
}
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts
Expand Up @@ -81,7 +81,7 @@ export = {
runtime: lambda.Runtime.Nodejs810,
});
const alias = new lambda.Alias(stack, 'alias', {
aliasName: 'my-alias',
aliasName: cdk.PhysicalName.of('my-alias'),
version: new lambda.Version(stack, 'version', {
lambda: handler
})
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-apigateway/test/test.restapi.ts
Expand Up @@ -141,7 +141,7 @@ export = {
const api = new apigateway.RestApi(stack, 'restapi', {
deploy: false,
cloudWatchRole: false,
restApiName: 'my-rest-api'
restApiName: cdk.PhysicalName.of('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: 'my-rest-api'
restApiName: cdk.PhysicalName.of('my-rest-api'),
});

// WHEN
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-apigateway/test/test.vpc-link.ts
Expand Up @@ -16,7 +16,7 @@ export = {

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

Expand Down Expand Up @@ -71,4 +71,4 @@ export = {
test.throws(() => app.synth(), /No targets added to vpc link/);
test.done();
}
};
};
7 changes: 6 additions & 1 deletion packages/@aws-cdk/aws-applicationautoscaling/package.json
Expand Up @@ -85,5 +85,10 @@
"engines": {
"node": ">= 8.10.0"
},
"awslint": {
"exclude": [
"props-physical-name:@aws-cdk/aws-applicationautoscaling.ScalableTargetProps"
]
},
"stability": "experimental"
}
}
10 changes: 6 additions & 4 deletions packages/@aws-cdk/aws-autoscaling/lib/lifecycle-hook.ts
@@ -1,5 +1,5 @@
import iam = require('@aws-cdk/aws-iam');
import { Construct, IResource, Resource } from '@aws-cdk/cdk';
import { Construct, IResource, PhysicalName, 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?: string;
readonly lifecycleHookName?: PhysicalName;

/**
* The action the Auto Scaling group takes when the lifecycle hook timeout elapses or if an unexpected failure occurs.
Expand Down Expand Up @@ -92,7 +92,9 @@ export class LifecycleHook extends Resource implements ILifecycleHook {
public readonly lifecycleHookName: string;

constructor(scope: Construct, id: string, props: LifecycleHookProps) {
super(scope, id);
super(scope, id, {
physicalName: props.lifecycleHookName,
});

this.role = props.role || new iam.Role(this, 'Role', {
assumedBy: new iam.ServicePrincipal('autoscaling.amazonaws.com')
Expand All @@ -104,7 +106,7 @@ export class LifecycleHook extends Resource implements ILifecycleHook {
autoScalingGroupName: props.autoScalingGroup.autoScalingGroupName,
defaultResult: props.defaultResult,
heartbeatTimeout: props.heartbeatTimeoutSec,
lifecycleHookName: props.lifecycleHookName,
lifecycleHookName: this.physicalName.value,
lifecycleTransition: props.lifecycleTransition,
notificationMetadata: props.notificationMetadata,
notificationTargetArn: targetProps.notificationTargetArn,
Expand Down
6 changes: 4 additions & 2 deletions packages/@aws-cdk/aws-autoscaling/package.json
Expand Up @@ -97,8 +97,10 @@
"exclude": [
"import-props-interface:@aws-cdk/aws-autoscaling.AutoScalingGroupImportProps",
"resource-interface-extends-construct:@aws-cdk/aws-autoscaling.IAutoScalingGroup",
"export:@aws-cdk/aws-autoscaling.IAutoScalingGroup"
"export:@aws-cdk/aws-autoscaling.IAutoScalingGroup",
"props-physical-name:@aws-cdk/aws-autoscaling.AutoScalingGroupProps",
"props-physical-name:@aws-cdk/aws-autoscaling.ScheduledActionProps"
]
},
"stability": "experimental"
}
}
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-certificatemanager/package.json
Expand Up @@ -86,5 +86,11 @@
"engines": {
"node": ">= 8.10.0"
},
"awslint": {
"exclude": [
"props-physical-name:@aws-cdk/aws-certificatemanager.CertificateProps",
"props-physical-name:@aws-cdk/aws-certificatemanager.DnsValidatedCertificateProps"
]
},
"stability": "experimental"
}
}
5 changes: 3 additions & 2 deletions packages/@aws-cdk/aws-cloudformation/package.json
Expand Up @@ -101,8 +101,9 @@
"exclude": [
"construct-ctor:@aws-cdk/aws-cloudformation.PipelineCloudFormationAction.<initializer>",
"construct-ctor:@aws-cdk/aws-cloudformation.PipelineCloudFormationDeployAction.<initializer>",
"construct-ctor-props-optional:@aws-cdk/aws-cloudformation.AwsCustomResource"
"construct-ctor-props-optional:@aws-cdk/aws-cloudformation.AwsCustomResource",
"props-physical-name:@aws-cdk/aws-cloudformation.CustomResourceProps"
]
},
"stability": "experimental"
}
}
21 changes: 16 additions & 5 deletions packages/@aws-cdk/aws-cloudtrail/lib/index.ts
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, Resource, Stack } from '@aws-cdk/cdk';
import { Construct, PhysicalName, Resource, ResourceIdentifiers, 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?: string;
readonly trailName?: PhysicalName;

/** An Amazon S3 object key prefix that precedes the name of all log files.
*
Expand Down Expand Up @@ -125,7 +125,9 @@ export class Trail extends Resource {
private eventSelectors: EventSelector[] = [];

constructor(scope: Construct, id: string, props: TrailProps = {}) {
super(scope, id);
super(scope, id, {
physicalName: props.trailName,
});

const s3bucket = new s3.Bucket(this, 'S3', {encryption: s3.BucketEncryption.Unencrypted});
const cloudTrailPrincipal = new iam.ServicePrincipal("cloudtrail.amazonaws.com");
Expand Down Expand Up @@ -173,7 +175,7 @@ export class Trail extends Resource {
enableLogFileValidation: props.enableFileValidation == null ? true : props.enableFileValidation,
isMultiRegionTrail: props.isMultiRegionTrail == null ? true : props.isMultiRegionTrail,
includeGlobalServiceEvents: props.includeGlobalServiceEvents == null ? true : props.includeGlobalServiceEvents,
trailName: props.trailName,
trailName: this.physicalName.value,
kmsKeyId: props.kmsKey && props.kmsKey.keyArn,
s3BucketName: s3bucket.bucketName,
s3KeyPrefix: props.s3KeyPrefix,
Expand All @@ -183,7 +185,16 @@ export class Trail extends Resource {
eventSelectors: this.eventSelectors
});

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

const s3BucketPolicy = s3bucket.node.findChild("Policy").node.findChild("Resource") as s3.CfnBucketPolicy;
Expand Down
25 changes: 19 additions & 6 deletions packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
@@ -1,4 +1,4 @@
import { Construct, IResource, Lazy, Resource, Stack } from '@aws-cdk/cdk';
import { Construct, IResource, Lazy, Resource, ResourceIdentifiers, Stack } from '@aws-cdk/cdk';
import { IAlarmAction } from './alarm-action';
import { CfnAlarm } from './cloudwatch.generated';
import { HorizontalAnnotation } from './graph';
Expand Down Expand Up @@ -115,7 +115,9 @@ export class Alarm extends Resource implements IAlarm {
private readonly annotation: HorizontalAnnotation;

constructor(scope: Construct, id: string, props: AlarmProps) {
super(scope, id);
super(scope, id, {
physicalName: props.alarmName,
});

const comparisonOperator = props.comparisonOperator || ComparisonOperator.GreaterThanOrEqualToThreshold;

Expand All @@ -124,7 +126,7 @@ export class Alarm extends Resource implements IAlarm {
const alarm = new CfnAlarm(this, 'Resource', {
// Meta
alarmDescription: props.alarmDescription,
alarmName: props.alarmName,
alarmName: this.physicalName.value,

// Evaluation
comparisonOperator,
Expand All @@ -149,8 +151,19 @@ export class Alarm extends Resource implements IAlarm {
})
});

this.alarmArn = alarm.attrArn;
this.alarmName = alarm.refAsString;
const resourceIdentifiers = new ResourceIdentifiers(this, {
arn: alarm.attrArn,
name: alarm.refAsString,
arnComponents: {
service: 'cloudwatch',
resource: 'alarm',
resourceName: this.physicalName.value,
sep: ':',
},
});
this.alarmArn = resourceIdentifiers.arn;
this.alarmName = resourceIdentifiers.name;

this.metric = props.metric;
this.annotation = {
// tslint:disable-next-line:max-line-length
Expand Down Expand Up @@ -239,4 +252,4 @@ function dropUndef<T extends object>(x: T): T {
}
}
return ret;
}
}
10 changes: 6 additions & 4 deletions packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts
@@ -1,4 +1,4 @@
import { Construct, Lazy, Resource, Stack } from "@aws-cdk/cdk";
import { Construct, Lazy, PhysicalName, 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?: string;
readonly dashboardName?: PhysicalName;

/**
* The start of the time range to use for each widget on the dashboard.
Expand Down Expand Up @@ -63,10 +63,12 @@ export class Dashboard extends Resource {
private readonly rows: IWidget[] = [];

constructor(scope: Construct, id: string, props: DashboardProps = {}) {
super(scope, id);
super(scope, id, {
physicalName: props.dashboardName,
});

new CfnDashboard(this, 'Resource', {
dashboardName: props.dashboardName,
dashboardName: this.physicalName.value,
dashboardBody: Lazy.stringValue({ produce: () => {
const column = new Column(...this.rows);
column.position(0, 0);
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
Expand Up @@ -260,7 +260,7 @@ export interface CreateAlarmOptions {
*
* @default Automatically generated name
*/
readonly alarmName?: string;
readonly alarmName?: cdk.PhysicalName;

/**
* Description for the alarm
Expand Down

0 comments on commit d9d3a99

Please sign in to comment.