refactor(core): CfnResource.options => cfnOptions (#3030)

In order to avoid future conflicts with L1 properties, we prefix all properties of CfnResource with `cfn`. BREAKING CHANGE: The `CfnResource.options` property was renamed to `CfnResource.cfnOptions` to avoid conflicts with properties introduced by derived classes.
aws · Jun 24, 2019 · e537e4c · e537e4c
1 parent 767687d
commit e537e4c
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 34 deletions.
diff --git a/packages/@aws-cdk/aws-apigateway/lib/deployment.ts b/packages/@aws-cdk/aws-apigateway/lib/deployment.ts
@@ -72,7 +72,7 @@ export class Deployment extends Resource {
     });
 
     if (props.retainDeployments) {
-      this.resource.options.deletionPolicy = CfnDeletionPolicy.RETAIN;
+      this.resource.cfnOptions.deletionPolicy = CfnDeletionPolicy.RETAIN;
     }
 
     this.api = props.api;

diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
@@ -518,8 +518,8 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
    */
   private applyUpdatePolicies(props: AutoScalingGroupProps) {
     if (props.updateType === UpdateType.REPLACING_UPDATE) {
-      this.autoScalingGroup.options.updatePolicy = {
-        ...this.autoScalingGroup.options.updatePolicy,
+      this.autoScalingGroup.cfnOptions.updatePolicy = {
+        ...this.autoScalingGroup.cfnOptions.updatePolicy,
         autoScalingReplacingUpdate: {
           willReplace: true
         }
@@ -531,31 +531,31 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
         // during the update?
         //
         // https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-creationpolicy.html
-        this.autoScalingGroup.options.creationPolicy = {
-          ...this.autoScalingGroup.options.creationPolicy,
+        this.autoScalingGroup.cfnOptions.creationPolicy = {
+          ...this.autoScalingGroup.cfnOptions.creationPolicy,
           autoScalingCreationPolicy: {
             minSuccessfulInstancesPercent: validatePercentage(props.replacingUpdateMinSuccessfulInstancesPercent)
           }
         };
       }
     } else if (props.updateType === UpdateType.ROLLING_UPDATE) {
-      this.autoScalingGroup.options.updatePolicy = {
-        ...this.autoScalingGroup.options.updatePolicy,
+      this.autoScalingGroup.cfnOptions.updatePolicy = {
+        ...this.autoScalingGroup.cfnOptions.updatePolicy,
         autoScalingRollingUpdate: renderRollingUpdateConfig(props.rollingUpdateConfiguration)
       };
     }
 
     // undefined is treated as 'true'
     if (props.ignoreUnmodifiedSizeProperties !== false) {
-      this.autoScalingGroup.options.updatePolicy = {
-        ...this.autoScalingGroup.options.updatePolicy,
+      this.autoScalingGroup.cfnOptions.updatePolicy = {
+        ...this.autoScalingGroup.cfnOptions.updatePolicy,
         autoScalingScheduledAction: { ignoreUnmodifiedGroupSizeProperties: true }
       };
     }
 
     if (props.resourceSignalCount !== undefined || props.resourceSignalTimeout !== undefined) {
-      this.autoScalingGroup.options.creationPolicy = {
-        ...this.autoScalingGroup.options.creationPolicy,
+      this.autoScalingGroup.cfnOptions.creationPolicy = {
+        ...this.autoScalingGroup.cfnOptions.creationPolicy,
         resourceSignal: {
           count: props.resourceSignalCount,
           timeout: props.resourceSignalTimeout && props.resourceSignalTimeout.toISOString(),

diff --git a/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts
@@ -183,7 +183,7 @@ export class LambdaDeploymentGroup extends cdk.Resource implements ILambdaDeploy
       this.addPostHook(props.postHook);
     }
 
-    (props.alias.node.defaultChild as lambda.CfnAlias).options.updatePolicy = {
+    (props.alias.node.defaultChild as lambda.CfnAlias).cfnOptions.updatePolicy = {
       codeDeployLambdaAliasUpdate: {
         applicationName: this.application.applicationName,
         deploymentGroupName: resource.ref,

diff --git a/packages/@aws-cdk/aws-iam/test/test.escape-hatch.ts b/packages/@aws-cdk/aws-iam/test/test.escape-hatch.ts
@@ -61,7 +61,7 @@ export = {
     const stack = new Stack();
     const user = new iam.User(stack, 'user', { userName: 'MyUserName' });
     const cfn = user.node.findChild('Resource') as iam.CfnUser;
-    cfn.options.updatePolicy = { useOnlineResharding: true };
+    cfn.cfnOptions.updatePolicy = { useOnlineResharding: true };
 
     // WHEN
     cfn.addOverride('Properties.Hello.World', 'Bam');

diff --git a/packages/@aws-cdk/aws-s3-assets/lib/asset.ts b/packages/@aws-cdk/aws-s3-assets/lib/asset.ts
@@ -169,9 +169,9 @@ export class Asset extends cdk.Construct implements assets.IAsset {
 
     // tell tools such as SAM CLI that the "Code" property of this resource
     // points to a local path in order to enable local invocation of this function.
-    resource.options.metadata = resource.options.metadata || { };
-    resource.options.metadata[cxapi.ASSET_RESOURCE_METADATA_PATH_KEY] = this.assetPath;
-    resource.options.metadata[cxapi.ASSET_RESOURCE_METADATA_PROPERTY_KEY] = resourceProperty;
+    resource.cfnOptions.metadata = resource.cfnOptions.metadata || { };
+    resource.cfnOptions.metadata[cxapi.ASSET_RESOURCE_METADATA_PATH_KEY] = this.assetPath;
+    resource.cfnOptions.metadata[cxapi.ASSET_RESOURCE_METADATA_PROPERTY_KEY] = resourceProperty;
   }
 
   /**

diff --git a/packages/@aws-cdk/core/lib/cfn-resource.ts b/packages/@aws-cdk/core/lib/cfn-resource.ts
@@ -36,10 +36,18 @@ export class CfnResource extends CfnRefElement {
     return (construct as any).cfnResourceType !== undefined;
   }
 
+  // MAINTAINERS NOTE: this class serves as the base class for the generated L1
+  // ("CFN") resources (such as `s3.CfnBucket`). These resources will have a
+  // property for each CloudFormation property of the resource. This means that
+  // if at some point in the future a property is introduced with a name similar
+  // to one of the properties here, it will be "masked" by the derived class. To
+  // that end, we prefix all properties in this class with `cfnXxx` with the
+  // hope to avoid those conflicts in the future.
+
   /**
    * Options for this resource, such as condition, update policy etc.
    */
-  public readonly options: IResourceOptions = {};
+  public readonly cfnOptions: ICfnResourceOptions = {};
 
   /**
    * AWS resource type.
@@ -84,7 +92,7 @@ export class CfnResource extends CfnRefElement {
     // path in the CloudFormation template, so it will be possible to trace
     // back to the actual construct path.
     if (this.node.tryGetContext(cxapi.PATH_METADATA_ENABLE_CONTEXT)) {
-      this.options.metadata = {
+      this.cfnOptions.metadata = {
         [cxapi.PATH_METADATA_KEY]: this.node.path
       };
     }
@@ -111,9 +119,9 @@ export class CfnResource extends CfnRefElement {
         throw new Error(`Invalid removal policy: ${policy}`);
     }
 
-    this.options.deletionPolicy = deletionPolicy;
+    this.cfnOptions.deletionPolicy = deletionPolicy;
     if (options.applyToUpdateReplacePolicy) {
-      this.options.updateReplacePolicy = deletionPolicy;
+      this.cfnOptions.updateReplacePolicy = deletionPolicy;
     }
   }
 
@@ -215,12 +223,12 @@ export class CfnResource extends CfnRefElement {
             Type: this.cfnResourceType,
             Properties: ignoreEmpty(this.cfnProperties),
             DependsOn: ignoreEmpty(renderDependsOn(this.dependsOn)),
-            CreationPolicy:  capitalizePropertyNames(this, renderCreationPolicy(this.options.creationPolicy)),
-            UpdatePolicy: capitalizePropertyNames(this, this.options.updatePolicy),
-            UpdateReplacePolicy: capitalizePropertyNames(this, this.options.updateReplacePolicy),
-            DeletionPolicy: capitalizePropertyNames(this, this.options.deletionPolicy),
-            Metadata: ignoreEmpty(this.options.metadata),
-            Condition: this.options.condition && this.options.condition.logicalId
+            CreationPolicy:  capitalizePropertyNames(this, renderCreationPolicy(this.cfnOptions.creationPolicy)),
+            UpdatePolicy: capitalizePropertyNames(this, this.cfnOptions.updatePolicy),
+            UpdateReplacePolicy: capitalizePropertyNames(this, this.cfnOptions.updateReplacePolicy),
+            DeletionPolicy: capitalizePropertyNames(this, this.cfnOptions.deletionPolicy),
+            Metadata: ignoreEmpty(this.cfnOptions.metadata),
+            Condition: this.cfnOptions.condition && this.cfnOptions.condition.logicalId
           }, props => {
             props.Properties = this.renderProperties(props.Properties);
             return deepMerge(props, this.rawOverrides);
@@ -294,7 +302,7 @@ export enum TagType {
   NOT_TAGGABLE = 'NotTaggable',
 }
 
-export interface IResourceOptions {
+export interface ICfnResourceOptions {
   /**
    * A condition to associate with this resource. This means that only if the condition evaluates to 'true' when the stack
    * is deployed, the resource will be included. This is provided to allow CDK projects to produce legacy templates, but noramlly

diff --git a/packages/@aws-cdk/core/test/test.resource.ts b/packages/@aws-cdk/core/test/test.resource.ts
@@ -190,7 +190,7 @@ export = {
     const stack = new Stack();
     const r1 = new CfnResource(stack, 'Resource', { type: 'Type' });
     const cond = new CfnCondition(stack, 'MyCondition', { expression: Fn.conditionNot(Fn.conditionEquals('a', 'b')) });
-    r1.options.condition = cond;
+    r1.cfnOptions.condition = cond;
 
     test.deepEqual(toCloudFormation(stack), {
       Resources: { Resource: { Type: 'Type', Condition: 'MyCondition' } },
@@ -204,9 +204,9 @@ export = {
     const stack = new Stack();
     const r1 = new CfnResource(stack, 'Resource', { type: 'Type' });
 
-    r1.options.creationPolicy = { autoScalingCreationPolicy: { minSuccessfulInstancesPercent: 10 } };
+    r1.cfnOptions.creationPolicy = { autoScalingCreationPolicy: { minSuccessfulInstancesPercent: 10 } };
     // tslint:disable-next-line:max-line-length
-    r1.options.updatePolicy = {
+    r1.cfnOptions.updatePolicy = {
       autoScalingScheduledAction: { ignoreUnmodifiedGroupSizeProperties: false },
       autoScalingReplacingUpdate: { willReplace: true },
       codeDeployLambdaAliasUpdate: {
@@ -215,8 +215,8 @@ export = {
         beforeAllowTrafficHook: 'lambda1',
       },
     };
-    r1.options.deletionPolicy = CfnDeletionPolicy.RETAIN;
-    r1.options.updateReplacePolicy = CfnDeletionPolicy.SNAPSHOT;
+    r1.cfnOptions.deletionPolicy = CfnDeletionPolicy.RETAIN;
+    r1.cfnOptions.updateReplacePolicy = CfnDeletionPolicy.SNAPSHOT;
 
     test.deepEqual(toCloudFormation(stack), {
       Resources: {
@@ -245,7 +245,7 @@ export = {
     const stack = new Stack();
     const r1 = new CfnResource(stack, 'Resource', { type: 'Type' });
 
-    r1.options.updatePolicy = { useOnlineResharding: true };
+    r1.cfnOptions.updatePolicy = { useOnlineResharding: true };
 
     test.deepEqual(toCloudFormation(stack), {
       Resources: {
@@ -265,7 +265,7 @@ export = {
     const stack = new Stack();
     const r1 = new CfnResource(stack, 'Resource', { type: 'Type' });
 
-    r1.options.metadata = {
+    r1.cfnOptions.metadata = {
       MyKey: 10,
       MyValue: 99
     };