Merge branch 'master' into madhulog/invoke-stepfunctions

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts

@@ -52,6 +52,7 @@ export interface IncludedNestedStack {
  */
 export class CfnInclude extends core.CfnElement {
   private readonly conditions: { [conditionName: string]: core.CfnCondition } = {};
+  private readonly conditionsScope: core.Construct;
   private readonly resources: { [logicalId: string]: core.CfnResource } = {};
   private readonly parameters: { [logicalId: string]: core.CfnParameter } = {};
   private readonly outputs: { [logicalId: string]: core.CfnOutput } = {};
@@ -75,8 +76,9 @@ export class CfnInclude extends core.CfnElement {
     }
 
     // instantiate the conditions
+    this.conditionsScope = new core.Construct(this, '$Conditions');
     for (const conditionName of Object.keys(this.template.Conditions || {})) {
-      this.createCondition(conditionName);
+      this.getOrCreateCondition(conditionName);
     }
 
     this.nestedStacksToInclude = props.nestedStacks || {};
@@ -262,32 +264,44 @@ export class CfnInclude extends core.CfnElement {
           return self.getCondition(outputAttributes.Condition);
         }
 
-        throw new Error(`Output with name '${logicalId}' refers to a Condition with name\
- '${outputAttributes.Condition}' which was not found in this template`);
+        throw new Error(`Output with name '${logicalId}' refers to a Condition with name ` +
+          `'${outputAttributes.Condition}' which was not found in this template`);
       })(),
     });
 
     cfnOutput.overrideLogicalId(logicalId);
     this.outputs[logicalId] = cfnOutput;
   }
 
-  private createCondition(conditionName: string): void {
-    // ToDo condition expressions can refer to other conditions -
-    // will be important when implementing preserveLogicalIds=false
-    const expression = new cfn_parse.CfnParser({
+  private getOrCreateCondition(conditionName: string): core.CfnCondition {
+    if (conditionName in this.conditions) {
+      return this.conditions[conditionName];
+    }
+
+    const self = this;
+    const cfnParser = new cfn_parse.CfnParser({
       finder: {
         findResource() { throw new Error('Using GetAtt in Condition definitions is not allowed'); },
-        findRefTarget() { throw new Error('Using Ref expressions in Condition definitions is not allowed'); },
-        // ToDo handle one Condition referencing another using the { Condition: "ConditionName" } syntax
-        findCondition() { return undefined; },
+        findRefTarget(elementName: string): core.CfnElement | undefined {
+          // only Parameters can be referenced in the 'Conditions' section
+          return self.parameters[elementName];
+        },
+        findCondition(cName: string): core.CfnCondition | undefined {
+          return cName in (self.template.Conditions || {})
+            ? self.getOrCreateCondition(cName)
+            : undefined;
+        },
       },
-    }).parseValue(this.template.Conditions[conditionName]);
-    const cfnCondition = new core.CfnCondition(this, conditionName, {
-      expression,
+      context: cfn_parse.CfnParsingContext.CONDITIONS,
     });
+    const cfnCondition = new core.CfnCondition(this.conditionsScope, conditionName, {
+      expression: cfnParser.parseValue(this.template.Conditions[conditionName]),
+    });
+
     // ToDo handle renaming of the logical IDs of the conditions
     cfnCondition.overrideLogicalId(conditionName);
     this.conditions[conditionName] = cfnCondition;
+    return cfnCondition;
   }
 
   private getOrCreateResource(logicalId: string): core.CfnResource {

packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts

@@ -47,12 +47,30 @@ describe('CDK Include', () => {
     }).toThrow(/Resource 'Bucket2' depends on 'Bucket1' that doesn't exist/);
   });
 
+  test("throws a validation exception for a template referencing a Condition in the Conditions section that doesn't exist", () => {
+    expect(() => {
+      includeTestTemplate(stack, 'non-existent-condition-in-conditions.json');
+    }).toThrow(/Referenced Condition with name 'AlwaysFalse' was not found in the template/);
+  });
+
+  test('throws a validation exception for a template using Fn::GetAtt in the Conditions section', () => {
+    expect(() => {
+      includeTestTemplate(stack, 'getatt-in-conditions.json');
+    }).toThrow(/Using GetAtt in Condition definitions is not allowed/);
+  });
+
   test("throws a validation exception for a template referencing a Condition resource attribute that doesn't exist", () => {
     expect(() => {
       includeTestTemplate(stack, 'non-existent-condition.json');
     }).toThrow(/Resource 'Bucket' uses Condition 'AlwaysFalseCond' that doesn't exist/);
   });
 
+  test("throws a validation exception for a template referencing a Condition in an If expression that doesn't exist", () => {
+    expect(() => {
+      includeTestTemplate(stack, 'non-existent-condition-in-if.json');
+    }).toThrow(/Condition 'AlwaysFalse' used in an Fn::If expression does not exist in the template/);
+  });
+
   test("throws an exception when encountering a CFN function it doesn't support", () => {
     expect(() => {
       includeTestTemplate(stack, 'only-codecommit-repo-using-cfn-functions.json');

packages/@aws-cdk/cloudformation-include/test/test-templates/condition-same-name-as-resource.json

@@ -0,0 +1,28 @@
+{
+  "Parameters": {
+    "Param": {
+      "Type": "String"
+    }
+  },
+  "Conditions": {
+    "AlwaysTrue": {
+      "Fn::Not": [{ "Condition": "AlwaysFalse" }]
+    },
+    "AlwaysFalse": {
+      "Fn::Equals": [{ "Ref":  "Param" }, 2]
+    }
+  },
+  "Resources": {
+    "AlwaysTrue": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": {
+          "Fn::If": ["AlwaysFalse",
+            { "Ref":  "Param" },
+            { "Ref":  "AWS::NoValue" }
+          ]
+        }
+      }
+    }
+  }
+}

packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/getatt-in-conditions.json

@@ -0,0 +1,15 @@
+{
+  "Conditions": {
+    "MyCond": {
+      "Fn::Equals": [
+        { "Fn::GetAtt": ["Bucket", "Arn"] },
+        "my-arn"
+      ]
+    }
+  },
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket"
+    }
+  }
+}

packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/non-existent-condition-in-conditions.json

@@ -0,0 +1,7 @@
+{
+  "Conditions": {
+    "AlwaysTrue": {
+      "Fn::Not": [{ "Condition": "AlwaysFalse" }]
+    }
+  }
+}

packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/non-existent-condition-in-if.json

@@ -0,0 +1,15 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": {
+          "Fn::If": ["AlwaysFalse",
+            "ThenName",
+            "ElseName"
+          ]
+        }
+      }
+    }
+  }
+}

packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts

@@ -251,6 +251,41 @@ describe('CDK Include', () => {
     );
   });
 
+  test('correctly change references to Conditions when renaming them', () => {
+    const cfnTemplate = includeTestTemplate(stack, 'condition-same-name-as-resource.json');
+    const alwaysFalse = cfnTemplate.getCondition('AlwaysFalse');
+    alwaysFalse.overrideLogicalId('TotallyFalse');
+
+    expect(stack).toMatchTemplate({
+      "Parameters": {
+        "Param": {
+          "Type": "String",
+        },
+      },
+      "Conditions": {
+        "AlwaysTrue": {
+          "Fn::Not": [{ "Condition": "TotallyFalse" }],
+        },
+        "TotallyFalse": {
+          "Fn::Equals": [{ "Ref": "Param" }, 2],
+        },
+      },
+      "Resources": {
+        "AlwaysTrue": {
+          "Type": "AWS::S3::Bucket",
+          "Properties": {
+            "BucketName": {
+              "Fn::If": ["TotallyFalse",
+                { "Ref": "Param" },
+                { "Ref": "AWS::NoValue" },
+              ],
+            },
+          },
+        },
+      },
+    });
+  });
+
   test('correctly parses templates with parameters', () => {
     const cfnTemplate = includeTestTemplate(stack, 'bucket-with-parameters.json');
     const param = cfnTemplate.getParameter('BucketName');

packages/@aws-cdk/core/lib/cfn-parse.ts

@@ -118,6 +118,20 @@ export class FromCloudFormation {
   }
 }
 
+/**
+ * The context in which the parsing is taking place.
+ *
+ * Some fragments of CloudFormation templates behave differently than others
+ * (for example, the 'Conditions' sections treats { "Condition": "NameOfCond" }
+ * differently than the 'Resources' section).
+ * This enum can be used to change the created {@link CfnParser} behavior,
+ * based on the template context.
+ */
+export enum CfnParsingContext {
+  /** We're currently parsing the 'Conditions' section. */
+  CONDITIONS,
+}
+
 /**
  * The options for {@link FromCloudFormation.parseValue}.
  */
@@ -126,6 +140,13 @@ export interface ParseCfnOptions {
    * The finder interface used to resolve references in the template.
    */
   readonly finder: ICfnFinder;
+
+  /**
+   * The context we're parsing the template in.
+   *
+   * @default - the default context (no special behavior)
+   */
+  readonly context?: CfnParsingContext;
 }
 
 /**
@@ -274,7 +295,7 @@ export class CfnParser {
   }
 
   private parseIfCfnIntrinsic(object: any): any {
-    const key = looksLikeCfnIntrinsic(object);
+    const key = this.looksLikeCfnIntrinsic(object);
     switch (key) {
       case undefined:
         return undefined;
@@ -340,12 +361,14 @@ export class CfnParser {
         return Fn.base64(value);
       }
       case 'Fn::If': {
-        // Fn::If takes a 3-element list as its argument
-        // ToDo the first argument is the name of the condition,
-        // so we will need to retrieve the actual object from the template
-        // when we handle preserveLogicalIds=false
+        // Fn::If takes a 3-element list as its argument,
+        // where the first element is the name of a Condition
         const value = this.parseValue(object[key]);
-        return Fn.conditionIf(value[0], value[1], value[2]);
+        const condition = this.options.finder.findCondition(value[0]);
+        if (!condition) {
+          throw new Error(`Condition '${value[0]}' used in an Fn::If expression does not exist in the template`);
+        }
+        return Fn.conditionIf(condition.logicalId, value[1], value[2]);
       }
       case 'Fn::Equals': {
         const value = this.parseValue(object[key]);
@@ -363,21 +386,33 @@ export class CfnParser {
         const value = this.parseValue(object[key]);
         return Fn.conditionOr(...value);
       }
+      case 'Condition': {
+        // a reference to a Condition from another Condition
+        const condition = this.options.finder.findCondition(object[key]);
+        if (!condition) {
+          throw new Error(`Referenced Condition with name '${object[key]}' was not found in the template`);
+        }
+        return { Condition: condition.logicalId };
+      }
       default:
         throw new Error(`Unsupported CloudFormation function '${key}'`);
     }
   }
-}
 
-function looksLikeCfnIntrinsic(object: object): string | undefined {
-  const objectKeys = Object.keys(object);
-  // a CFN intrinsic is always an object with a single key
-  if (objectKeys.length !== 1) {
-    return undefined;
-  }
+  private looksLikeCfnIntrinsic(object: object): string | undefined {
+    const objectKeys = Object.keys(object);
+    // a CFN intrinsic is always an object with a single key
+    if (objectKeys.length !== 1) {
+      return undefined;
+    }
 
-  const key = objectKeys[0];
-  return key === 'Ref' || key.startsWith('Fn::') ? key : undefined;
+    const key = objectKeys[0];
+    return key === 'Ref' || key.startsWith('Fn::') ||
+        // special intrinsic only available in the 'Conditions' section
+        (this.options.context === CfnParsingContext.CONDITIONS && key === 'Condition')
+      ? key
+      : undefined;
+  }
 }
 
 function specialCaseRefs(value: any): any {