refactor: physical names in the entire Construct Library (#2878)

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 class with the `Name` suffix - for example, if my resource is `AwesomeFoobar`, the name can be `foobarName` or `awesomeFoobarName` (that's because we often have `Specialized1AwesomeFoobar` and `Specialized2AwesomeFoobar` in our library, that share a base prop interface). There were a few interesting cases in the library which I didn't change, as I wasn't sure of the semantics of the resources, or they would require class name changes. Would appreciate some guidance there. These were: * @aws-kms: AliasProps.name * @aws-ssm: ParameterOptions.name * @aws-applicationautoscaling: ScalableTargetProps.resourceId * @aws-route53: CommonHostedZoneProps.zoneName * @aws-route53: RecordSetOptions.recordName * @aws-apigateway: StageProps.stageName * @aws-servicediscovery: BaseNamespaceProps.name * @aws-config: CustomRuleProps.ruleName * @aws-config: ManagedRuleProps.ruleName * @aws-config: AccessKeysRotatedProps.ruleName * @aws-config: CloudFormationStackNotificationCheckProps.ruleName * @aws-config: CloudFormationStackDriftDetectionCheckProps.ruleName * @aws-rds: DatabaseClusterProps.databaseName / clusterIdentifier * @aws-rds: DatabaseInstanceSourceProps.databaseName BREAKING CHANGE: all resourceName attributes have their type changed from string to cdk.PhysicalName.
aws · Jun 17, 2019 · f0d8127 · f0d8127
1 parent b8a1c8e
commit f0d8127
Show file tree

Hide file tree

Showing 130 changed files with 1,019 additions and 486 deletions.
diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/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';
@@ -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.
@@ -204,10 +204,12 @@ export class RestApi extends Resource implements IRestApi {
   private readonly methods = new Array<Method>();
 
   constructor(scope: Construct, id: string, props: RestApiProps = { }) {
-    super(scope, id);
+    super(scope, id, {
+      physicalName: props.restApiName,
+    });
 
     const resource = new CfnRestApi(this, 'Resource', {
-      name: props.restApiName || id,
+      name: this.physicalName.value || id,
       description: props.description,
       policy: props.policy,
       failOnWarnings: props.failOnWarnings,

diff --git a/packages/@aws-cdk/aws-apigateway/lib/vpc-link.ts b/packages/@aws-cdk/aws-apigateway/lib/vpc-link.ts
@@ -1,5 +1,5 @@
 import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2');
-import { Construct, Resource } from '@aws-cdk/cdk';
+import { Construct, PhysicalName, Resource } from '@aws-cdk/cdk';
 import { CfnVpcLink } from './apigateway.generated';
 
 /**
@@ -10,7 +10,7 @@ export interface VpcLinkProps {
    * The name used to label and identify the VPC link.
    * @default automatically generated name
    */
-  readonly name?: string;
+  readonly vpcLinkName?: PhysicalName;
 
   /**
    * The description of the VPC link.
@@ -37,14 +37,16 @@ export class VpcLink extends Resource {
   public readonly vpcLinkId: string;
 
   constructor(scope: Construct, id: string, props: VpcLinkProps) {
-    super(scope, id);
+    super(scope, id, {
+      physicalName: props.vpcLinkName,
+    });
 
     const cfnResource = new CfnVpcLink(this, 'Resource', {
-      name: props.name || this.node.uniqueId,
+      name: this.physicalName.value || this.node.uniqueId,
       description: props.description,
       targetArns: props.targets.map(nlb => nlb.loadBalancerArn)
     });
 
     this.vpcLinkId = cfnResource.vpcLinkId;
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-apigateway/package.json b/packages/@aws-cdk/aws-apigateway/package.json
@@ -91,8 +91,15 @@
   },
   "awslint": {
     "exclude": [
-      "from-method:@aws-cdk/aws-apigateway.Resource"
+      "from-method:@aws-cdk/aws-apigateway.Resource",
+      "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"
-}
+}
diff --git a/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts b/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts
@@ -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
       })

diff --git a/packages/@aws-cdk/aws-apigateway/test/test.restapi.ts b/packages/@aws-cdk/aws-apigateway/test/test.restapi.ts
@@ -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');
@@ -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

diff --git a/packages/@aws-cdk/aws-apigateway/test/test.vpc-link.ts b/packages/@aws-cdk/aws-apigateway/test/test.vpc-link.ts
@@ -16,7 +16,7 @@ export = {
 
     // WHEN
     new apigateway.VpcLink(stack, 'VpcLink', {
-      name: 'MyLink',
+      vpcLinkName: cdk.PhysicalName.of('MyLink'),
       targets: [nlb]
     });
 
@@ -28,4 +28,4 @@ export = {
 
     test.done();
   },
-};
+};
diff --git a/packages/@aws-cdk/aws-applicationautoscaling/package.json b/packages/@aws-cdk/aws-applicationautoscaling/package.json
@@ -85,5 +85,10 @@
   "engines": {
     "node": ">= 8.10.0"
   },
+  "awslint": {
+    "exclude": [
+      "props-physical-name:@aws-cdk/aws-applicationautoscaling.ScalableTargetProps"
+    ]
+  },
   "stability": "experimental"
-}
+}
diff --git a/packages/@aws-cdk/aws-autoscaling/lib/lifecycle-hook.ts b/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';
@@ -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.
@@ -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')
@@ -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,

diff --git a/packages/@aws-cdk/aws-autoscaling/package.json b/packages/@aws-cdk/aws-autoscaling/package.json
@@ -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"
-}
+}
diff --git a/packages/@aws-cdk/aws-certificatemanager/package.json b/packages/@aws-cdk/aws-certificatemanager/package.json
@@ -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"
-}
+}
diff --git a/packages/@aws-cdk/aws-cloudformation/package.json b/packages/@aws-cdk/aws-cloudformation/package.json
@@ -100,8 +100,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"
-}
+}
diff --git a/packages/@aws-cdk/aws-cloudtrail/lib/index.ts b/packages/@aws-cdk/aws-cloudtrail/lib/index.ts
@@ -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:
@@ -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.
    *
@@ -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 = "cloudtrail.amazonaws.com";
@@ -168,7 +170,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,
@@ -178,7 +180,16 @@ export class Trail extends Resource {
       eventSelectors: this.eventSelectors
     });
 
-    this.trailArn = trail.trailArn;
+    const resourceIdentifiers = new ResourceIdentifiers(this, {
+      arn: trail.trailArn,
+      name: trail.trailName,
+      arnComponents: {
+        service: 'cloudtrail',
+        resource: 'trail',
+        resourceName: this.physicalName.value,
+      },
+    });
+    this.trailArn = resourceIdentifiers.arn;
     this.trailSnsTopicArn = trail.trailSnsTopicArn;
 
     const s3BucketPolicy = s3bucket.node.findChild("Policy").node.findChild("Resource") as s3.CfnBucketPolicy;

diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/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';
@@ -114,14 +114,16 @@ 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;
 
     const alarm = new CfnAlarm(this, 'Resource', {
       // Meta
       alarmDescription: props.alarmDescription,
-      alarmName: props.alarmName,
+      alarmName: this.physicalName.value,
 
       // Evaluation
       comparisonOperator,
@@ -141,8 +143,19 @@ export class Alarm extends Resource implements IAlarm {
       ...metricJson(props.metric)
     });
 
-    this.alarmArn = alarm.alarmArn;
-    this.alarmName = alarm.alarmName;
+    const resourceIdentifiers = new ResourceIdentifiers(this, {
+      arn: alarm.alarmArn,
+      name: alarm.alarmName,
+      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

diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts b/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";
@@ -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.
@@ -54,10 +54,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 && props.dashboardName,
+    });
 
     new CfnDashboard(this, 'Resource', {
-      dashboardName: (props && props.dashboardName) || undefined,
+      dashboardName: this.physicalName.value,
       dashboardBody: Lazy.stringValue({ produce: () => {
         const column = new Column(...this.rows);
         column.position(0, 0);

diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
@@ -327,7 +327,7 @@ export interface MetricAlarmProps {
    *
    * @default Automatically generated name
    */
-  readonly alarmName?: string;
+  readonly alarmName?: cdk.PhysicalName;
 
   /**
    * Description for the alarm

diff --git a/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.ts b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.ts
@@ -27,7 +27,7 @@ const alarm = metric.newAlarm(stack, 'Alarm', {
 });
 
 const dashboard = new cloudwatch.Dashboard(stack, 'Dash', {
-  dashboardName: 'MyCustomDashboardName',
+  dashboardName: cdk.PhysicalName.of('MyCustomDashboardName'),
   start: '-9H',
   end: '2018-12-17T06:00:00.000Z',
   periodOverride: PeriodOverride.Inherit

diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts
@@ -1,5 +1,5 @@
 import { expect, haveResource, isSuperObject } from '@aws-cdk/assert';
-import { App, Stack } from '@aws-cdk/cdk';
+import { App, PhysicalName, Stack } from '@aws-cdk/cdk';
 import { Test } from 'nodeunit';
 import { Dashboard, GraphWidget, PeriodOverride, TextWidget } from '../lib';
 
@@ -127,7 +127,7 @@ export = {
 
     // WHEN
     new Dashboard(stack, 'MyDashboard', {
-      dashboardName: 'MyCustomDashboardName'
+      dashboardName: PhysicalName.of('MyCustomDashboardName'),
     });
 
     // THEN

diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts
@@ -745,7 +745,7 @@ export class Project extends ProjectBase {
   public addToRoleInlinePolicy(statement: iam.PolicyStatement) {
     if (this.role) {
       const policy = new iam.Policy(this, 'PolicyDocument', {
-        policyName: 'CodeBuildEC2Policy',
+        policyName: PhysicalName.of('CodeBuildEC2Policy'),
         statements: [statement]
       });
       this.role.attachInlinePolicy(policy);