fix: remove CloudFormation property renames (#973)

The construct id parameter used to be a property called `name` in the props object, and so properties called `name` were renamed to `xxxName` (where `xxx` is the name of the resource). Since we now pass names as a constructor argument, we can get rid of the properly rename mechanism. BREAKING CHANGE: if you use L1s, you may need to change some `XxxName` properties back into `Name`. These will match the CloudFormation property names. Fixes #852.
aws · Oct 22, 2018 · 3f86603 · 3f86603
1 parent 770f9aa
commit 3f86603
Show file tree

Hide file tree

Showing 17 changed files with 37 additions and 92 deletions.
diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts
@@ -176,7 +176,7 @@ export class RestApi extends RestApiRef implements cdk.IDependable {
     super(parent, id);
 
     const resource = new cloudformation.RestApiResource(this, 'Resource', {
-      restApiName: props.restApiName || id,
+      name: props.restApiName || id,
       description: props.description,
       policy: props.policy,
       failOnWarnings: props.failOnWarnings,

diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts
@@ -522,7 +522,7 @@ export class Project extends ProjectRef {
       encryptionKey: props.encryptionKey && props.encryptionKey.keyArn,
       badgeEnabled: props.badge,
       cache,
-      projectName: props.projectName,
+      name: props.projectName,
     });
 
     this.projectArn = resource.projectArn;

diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
@@ -97,7 +97,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
       stages: new cdk.Token(() => this.renderStages()) as any,
       roleArn: this.role.roleArn,
       restartExecutionOnUpdate: props && props.restartExecutionOnUpdate,
-      pipelineName: props && props.pipelineName,
+      name: props && props.pipelineName,
     });
 
     // this will produce a DependsOn for both the role and the policy resources.

diff --git a/packages/@aws-cdk/aws-events/lib/rule.ts b/packages/@aws-cdk/aws-events/lib/rule.ts
@@ -74,7 +74,7 @@ export class EventRule extends EventRuleRef {
     super(parent, name);
 
     const resource = new cloudformation.RuleResource(this, 'Resource', {
-      ruleName: props.ruleName,
+      name: props.ruleName,
       description: props.description,
       state: props.enabled == null ? 'ENABLED' : (props.enabled ? 'ENABLED' : 'DISABLED'),
       scheduleExpression: new Token(() => this.scheduleExpression),

diff --git a/packages/@aws-cdk/aws-kinesis/lib/stream.ts b/packages/@aws-cdk/aws-kinesis/lib/stream.ts
@@ -297,7 +297,7 @@ export class Stream extends StreamRef {
     const { streamEncryption, encryptionKey } = this.parseEncryption(props);
 
     this.stream = new cloudformation.StreamResource(this, "Resource", {
-      streamName: props.streamName,
+      name: props.streamName,
       retentionPeriodHours,
       shardCount,
       streamEncryption

diff --git a/packages/@aws-cdk/aws-lambda/lib/alias.ts b/packages/@aws-cdk/aws-lambda/lib/alias.ts
@@ -86,7 +86,7 @@ export class Alias extends FunctionRef {
     this.underlyingLambda = props.version.lambda;
 
     const alias = new cloudformation.AliasResource(this, 'Resource', {
-      aliasName: props.aliasName,
+      name: props.aliasName,
       description: props.description,
       functionName: this.underlyingLambda.functionName,
       functionVersion: props.version.functionVersion,

diff --git a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts
@@ -125,9 +125,9 @@ function toVpcProperty(vpc: ec2.VpcNetworkRef): cloudformation.HostedZoneResourc
 }
 
 function determineHostedZoneProps(props: PublicHostedZoneProps) {
-  const hostedZoneName = props.zoneName + '.';
+  const name = props.zoneName + '.';
   const hostedZoneConfig = props.comment ? { comment: props.comment } : undefined;
   const queryLoggingConfig = props.queryLogsLogGroupArn ? { cloudWatchLogsLogGroupArn: props.queryLogsLogGroupArn } : undefined;
 
-  return { hostedZoneName, hostedZoneConfig, queryLoggingConfig };
+  return { name, hostedZoneConfig, queryLoggingConfig };
 }
diff --git a/packages/@aws-cdk/aws-route53/lib/records/txt.ts b/packages/@aws-cdk/aws-route53/lib/records/txt.ts
@@ -23,7 +23,7 @@ export class TXTRecord extends Construct {
 
     new cloudformation.RecordSetResource(this, 'Resource', {
       hostedZoneId: parent.hostedZoneId,
-      recordSetName: determineFullyQualifiedDomainName(props.recordName, parent),
+      name: determineFullyQualifiedDomainName(props.recordName, parent),
       type: 'TXT',
       resourceRecords: [recordValue],
       ttl: ttl.toString()

diff --git a/packages/@aws-cdk/aws-route53/lib/records/zone-delegation.ts b/packages/@aws-cdk/aws-route53/lib/records/zone-delegation.ts
@@ -40,7 +40,7 @@ export class ZoneDelegationRecord extends Construct {
 
     new cloudformation.RecordSetResource(this, 'Resource', {
       hostedZoneId: parent.hostedZoneId,
-      recordSetName: determineFullyQualifiedDomainName(props.delegatedZoneName, parent),
+      name: determineFullyQualifiedDomainName(props.delegatedZoneName, parent),
       type: 'NS',
       ttl: ttl.toString(),
       comment: props.comment,

diff --git a/packages/@aws-cdk/aws-ssm/test/test.ssm-document.ts b/packages/@aws-cdk/aws-ssm/test/test.ssm-document.ts
@@ -0,0 +1,22 @@
+import { expect, haveResource } from '@aws-cdk/assert';
+import cdk = require('@aws-cdk/cdk');
+import { Test } from 'nodeunit';
+import ssm = require('../lib');
+
+export = {
+  'association name is rendered properly in L1 construct'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+
+    // WHEN
+    new ssm.cloudformation.AssociationResource(stack, 'Assoc', {
+      name: 'document',
+    });
+
+    // THEN
+    expect(stack).to(haveResource('AWS::SSM::Association', {
+      Name: 'document',
+    }));
+    test.done();
+  }
+};
diff --git a/packages/@aws-cdk/aws-stepfunctions/lib/activity.ts b/packages/@aws-cdk/aws-stepfunctions/lib/activity.ts
@@ -23,7 +23,7 @@ export class Activity extends cdk.Construct implements IStepFunctionsTaskResourc
         super(parent, id);
 
         const resource = new cloudformation.ActivityResource(this, 'Resource', {
-            activityName: props.activityName || this.generateName()
+            name: props.activityName || this.generateName()
         });
 
         this.activityArn = resource.activityArn;

diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/resource.ts b/packages/@aws-cdk/cdk/lib/cloudformation/resource.ts
@@ -86,12 +86,6 @@ export class Resource extends Referenceable {
 
     this.resourceType = props.type;
     this.properties = props.properties || { };
-
-    // 'name' is a special property included for resource constructs and passed
-    // as 'name', but we don't want it to be serialized into the template.
-    if (this.properties.name) {
-      delete this.properties.name;
-    }
   }
 
   /**

diff --git a/packages/@aws-cdk/cdk/test/cloudformation/test.resource.ts b/packages/@aws-cdk/cdk/test/cloudformation/test.resource.ts
@@ -240,22 +240,6 @@ export = {
     test.done();
   },
 
-  'the "name" property is deleted when synthesizing into a CloudFormation resource'(test: Test) {
-    const stack = new Stack();
-
-    new Resource(stack, 'Bla', {
-      type: 'MyResource',
-      properties: {
-        Prop1: 'value1',
-        name: 'Bla'
-      }
-    });
-
-    test.deepEqual(stack.toCloudFormation(), { Resources:
-      { Bla: { Type: 'MyResource', Properties: { Prop1: 'value1' } } } });
-    test.done();
-  },
-
   'removal policy is a high level abstraction of deletion policy used by l2'(test: Test) {
     const stack = new Stack();
 

diff --git a/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.json b/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.json
diff --git a/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.md b/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.md
diff --git a/packages/@aws-cdk/runtime-values/lib/rtv.ts b/packages/@aws-cdk/runtime-values/lib/rtv.ts
@@ -57,7 +57,7 @@ export class RuntimeValue extends cdk.Construct {
     this.parameterName = `/rtv/${new cdk.AwsStackName()}/${props.package}/${name}`;
 
     new ssm.cloudformation.ParameterResource(this, 'Parameter', {
-      parameterName: this.parameterName,
+      name: this.parameterName,
       type: 'String',
       value: props.value,
     });

diff --git a/tools/cfn2ts/lib/codegen.ts b/tools/cfn2ts/lib/codegen.ts
@@ -144,28 +144,13 @@ export default class CodeGenerator {
   private emitPropsTypeProperties(resourceName: SpecName, propertiesSpec: { [name: string]: schema.Property }): Dictionary<string> {
     const propertyMap: Dictionary<string> = {};
 
-    // Sanity check that our renamed "Name" is not going to conflict with a real property
-    const renamedNameProperty = resourceNameProperty(resourceName);
-    const lowerNames = Object.keys(propertiesSpec).map(s => s.toLowerCase());
-    if (lowerNames.indexOf('name') !== -1 && lowerNames.indexOf(renamedNameProperty.toLowerCase()) !== -1) {
-      // tslint:disable-next-line:max-line-length
-      throw new Error(`Oh gosh, we want to rename ${resourceName.fqn}'s 'Name' property to '${renamedNameProperty}', but that property already exists! We need to find a solution to this problem.`);
-    }
-
     Object.keys(propertiesSpec).sort(propertyComparator).forEach(propName => {
-      const originalName = propName;
       const propSpec = propertiesSpec[propName];
       const additionalDocs = resourceName.relativeName(propName).fqn;
 
-      if (propName.toLocaleLowerCase() === 'name') {
-        propName = renamedNameProperty;
-        // tslint:disable-next-line:no-console
-        console.error(`Renamed property 'Name' of ${resourceName.fqn} to '${renamedNameProperty}'`);
-      }
-
       const resourceCodeName = genspec.CodeName.forResource(resourceName);
       const newName = this.emitProperty(resourceCodeName, propName, propSpec, quoteCode(additionalDocs));
-      propertyMap[originalName] = newName;
+      propertyMap[propName] = newName;
     });
     return propertyMap;
 
@@ -270,10 +255,9 @@ export default class CodeGenerator {
     this.code.line(`super(parent, name, { type: ${resourceName.className}.resourceTypeName${propsType ? ', properties' : ''} });`);
     // verify all required properties
     if (spec.Properties) {
-      for (const pname of Object.keys(spec.Properties)) {
-        const prop = spec.Properties[pname];
+      for (const propName of Object.keys(spec.Properties)) {
+        const prop = spec.Properties[propName];
         if (prop.Required) {
-          const propName = pname.toLocaleLowerCase() === 'name' ? resourceNameProperty(resourceName.specName!) : pname;
           this.code.line(`${CORE}.requireProperty(properties, '${genspec.cloudFormationToScriptName(propName)}', this);`);
         }
       }
@@ -633,20 +617,6 @@ function mapperNames(types: genspec.CodeName[]): string {
   return types.map(type => genspec.cfnMapperName(type).fqn).join(', ');
 }
 
-/**
- * Return the name of the literal "name" property of a resource
- *
- * A number of resources have a "Name" property. Since Constructs already have a "Name" property
- * (which means something different), we must call the original property something else.
- *
- * We name it after the resource, so for a bucket the "Name" property gets renamed to "BucketName".
- *
- * (We can leave the name PascalCased, as it's going to be camelCased later).
- */
-function resourceNameProperty(resourceName: SpecName) {
-  return `${resourceName.resourceName}Name`;
-}
-
 /**
  * Quotes a code name for inclusion in a JSDoc comment, so it will render properly
  * in the Sphinx output.