Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple instances of CfnInclude cause TypeError #8151

Closed
scarybot opened this issue May 22, 2020 · 3 comments · Fixed by #8251
Closed

Multiple instances of CfnInclude cause TypeError #8151

scarybot opened this issue May 22, 2020 · 3 comments · Fixed by #8251
Assignees
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. p1

Comments

@scarybot
Copy link

scarybot commented May 22, 2020

Upon using cdk.CfnInclude multiple times, a TypeError is raised. The content or ordering of the templates doesn't appear to make a difference. Both the templates in the below example will synth correctly when used individually.

Reproduction Steps

import * as cdk from '@aws-cdk/core';
import * as fs from 'fs';
import * as YAML from 'yaml';

export class CdkStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    new cdk.CfnInclude(this, "Artifacts", {
      template: YAML.parse(
        fs.readFileSync("../test1.yml").toString()
      ),
    });

    new cdk.CfnInclude(this, "Buckets", {
      template: YAML.parse(
        fs.readFileSync("../test2.yml").toString()
      ),
    });
}

Error Log

/Users/jd/git/admin/build/cdk/node_modules/@aws-cdk/core/lib/stack.ts:988
        if (id in dest) {
              ^
TypeError: Cannot use 'in' operator to search for '0' in 2010-09-09
    at merge (/Users/jd/git/admin/build/cdk/node_modules/@aws-cdk/core/lib/stack.ts:988:15)
    at CdkStack._toCloudFormation (/Users/jd/git/admin/build/cdk/node_modules/@aws-cdk/core/lib/stack.ts:846:7)
    at CdkStack.synthesize (/Users/jd/git/admin/build/cdk/node_modules/@aws-cdk/core/lib/stack.ts:762:38)
    at CdkStack.onSynthesize (/Users/jd/git/admin/build/cdk/node_modules/@aws-cdk/core/lib/construct-compat.ts:123:10)
    at Node.synthesize (/Users/jd/git/admin/build/cdk/node_modules/constructs/lib/construct.ts:407:28)
    at Function.synth (/Users/jd/git/admin/build/cdk/node_modules/@aws-cdk/core/lib/construct-compat.ts:231:22)
    at App.synth (/Users/jd/git/admin/build/cdk/node_modules/@aws-cdk/core/lib/app.ts:142:36)
    at process.<anonymous> (/Users/jd/git/admin/build/cdk/node_modules/@aws-cdk/core/lib/app.ts:121:45)
    at Object.onceWrapper (events.js:428:26)
    at process.emit (events.js:321:20)
Subprocess exited with error 1

Environment

  • CLI Version : 1.41.0 (build 9e071d2)
  • Framework Version: uncertain
  • OS : MacOS 10.15.1
  • Language : English

Other

Contents of the test templates:

test1.yml:

---
AWSTemplateFormatVersion: "2010-09-09"
Description: Test 1

Resources:
  DummyResource:
    Type: AWS::CloudFormation::WaitConditionHandle

test2.yml:

---
AWSTemplateFormatVersion: "2010-09-09"
Description: Test 2

Resources:
  DummyResource:
    Type: AWS::CloudFormation::WaitConditionHandle

This is 🐛 Bug Report

@scarybot scarybot added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 22, 2020
@skinny85 skinny85 self-assigned this May 26, 2020
@skinny85
Copy link
Contributor

Thanks for opening the issue @scarybot . Confirming this is a bug on our side. Will work on a fix.

@skinny85 skinny85 added @aws-cdk/core Related to core CDK functionality p1 and removed needs-triage This issue or PR still needs to be triaged. labels May 28, 2020
@skinny85
Copy link
Contributor

BTW, you probably reduced the example to the minimum, but to be clear: your templates wouldn't work together anyway, because you use the same logical ID of DummyResource in both (our bug is somewhere else, but just to be clear 🤪).

skinny85 added a commit to skinny85/aws-cdk that referenced this issue May 28, 2020
…incorrectly

In the merge logic in Stack when rendering the template,
it was mistakenly assumed that all CFN sections are objects.
However, there are some sections, like Description and AWSTemplateFormatVersion,
that are in fact strings.
Add special logic for those cases in the merge functionality
(multiple provided CFN versions are checked for being identical,
and mutliple descriptions are merged together, with a newline in between).

Fixes aws#8151
@scarybot
Copy link
Author

BTW, you probably reduced the example to the minimum, but to be clear: your templates wouldn't work together anyway, because you use the same logical ID of DummyResource in both (our bug is somewhere else, but just to be clear 🤪).

Thanks, yes, I was just trying to be illustrative :D

skinny85 added a commit to skinny85/aws-cdk that referenced this issue Jun 2, 2020
…incorrectly

In the merge logic in Stack when rendering the template,
it was mistakenly assumed that all CFN sections are objects.
However, there are some sections, like Description and AWSTemplateFormatVersion,
that are in fact strings.
Add special logic for those cases in the merge functionality
(multiple provided CFN versions are checked for being identical,
and mutliple descriptions are merged together, with a newline in between).

Fixes aws#8151
skinny85 added a commit that referenced this issue Jun 2, 2020
…incorrectly (#8251)

In the merge logic in Stack when rendering the template,
it was mistakenly assumed that all CFN sections are objects.
However, there are some sections, like Description and AWSTemplateFormatVersion,
that are in fact strings.
Add special logic for those cases in the merge functionality
(multiple provided CFN versions are checked for being identical,
and mutliple descriptions are merged together, with a newline in between).

Fixes #8151
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants