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

(cloudformation-include): Incorrect rendering of Sub functions when using CfnInclude parameters #14047

Closed
daxAKAhackerman opened this issue Apr 8, 2021 · 2 comments · Fixed by #14068
Assignees
Labels
@aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@daxAKAhackerman
Copy link

When importing a CloudFormation template using the CfnInclude class and specifying parameters with values that are the valueAsString property of instances of core CfnParameter, every instances of short sub functions (!Sub) are incorrectly translated. However, when the included template uses the long version of sub functions (Fn::Sub), the synthesized template is valid.

Reproduction Steps

topic.yml

AWSTemplateFormatVersion: "2010-09-09"

Parameters:
  Environment:
    Type: String

Resources:
  SNSTopic:
    Type: AWS::SNS::Topic
    Properties: 
      TopicName: !Sub my-sns-topic-${Environment}

short-sub-stack.ts

import * as cdk from '@aws-cdk/core';
import * as cfninc from '@aws-cdk/cloudformation-include'


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

    const environment = new cdk.CfnParameter(this, 'Environment', {
      type: 'String'
    })

    new cfninc.CfnInclude(this, 'UserPool', {
      templateFile: './lib/cfn-modules/topic.yml',
      parameters: {
        Environment: environment.valueAsString
      }
    })
  }
}
cdk synth --no-version-reporting --no-path-metadata --no-asset-metadata

What did you expect to happen?

The resulting template's sub functions should either be a valid short sub, or a valid long sub as such:

Fn::Sub:
  - my-sns-topic-${Environment}
  - Environment:
      Ref: Environment

What actually happened?

An invalid sub function is produced as such:

Fn::Sub:
  Fn::Join:
    - ""
    - - my-sns-topic-
      - Ref: Environment

Resulting in the following cfn-lint error: E1019: Sub should be a string or array of 2 items

And the following CloudFormation error when deployed: An error occurred (ValidationError) when calling the CreateChangeSet operation: Template error: One or more Fn::Sub intrinsic functions don't specify expected arguments. Specify a string as first argument, and an optional second argument to specify a mapping of values to replace in the string

Environment

  • CDK CLI Version :1.97.0
  • Framework Version:
  • Node.js Version:v14.16.0
  • OS : macOS Big Sur 11.2.3
  • Language (Version):TypeScript (4.2.3)

Other

When using a long sub function, this issue does not happen and the produced cloudformation is valid. The following topic.yml file can be used to reproduce this behaviour:

AWSTemplateFormatVersion: "2010-09-09"

Parameters:
  Environment:
    Type: String

Resources:
  SNSTopic:
    Type: AWS::SNS::Topic
    Properties: 
      TopicName: 
        Fn::Sub: 
          - my-sns-topic-${Environment}
          - Environment: !Ref Environment

This is 🐛 Bug Report

@daxAKAhackerman daxAKAhackerman added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 8, 2021
@github-actions github-actions bot added the @aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package label Apr 8, 2021
@skinny85
Copy link
Contributor

skinny85 commented Apr 8, 2021

Thanks for the awesome bug report @daxAKAhackerman. Confirming I was able to reproduce it. Working on a fix.

@skinny85 skinny85 added effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 8, 2021
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Apr 9, 2021
… in Fn::Sub expressions

When a Token that resolved to an object, like `{ Ref }`,
was passed as the value to be substituted for a CloudFormation Parameter,
and that Parameter was used in an Fn::Sub expression,
it caused incorrect CloudFormation to be generated
(an Fn::Join as an argument to Fn::Sub, which is not allowed).
Check for that case explicitly, and move that substitution into the Fn::Sub map instead.

Fixes aws#14047
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Apr 9, 2021
… in Fn::Sub expressions

When a Token that resolved to an object, like `{ Ref }`,
was passed as the value to be substituted for a CloudFormation Parameter,
and that Parameter was used in an Fn::Sub expression,
it caused incorrect CloudFormation to be generated
(an Fn::Join as an argument to Fn::Sub, which is not allowed).
Check for that case explicitly, and move that substitution into the Fn::Sub map instead.

Fixes aws#14047
@mergify mergify bot closed this as completed in #14068 Apr 13, 2021
mergify bot pushed a commit that referenced this issue Apr 13, 2021
… in Fn::Sub expressions (#14068)

When a Token that resolved to an object, like `{ Ref }`,
was passed as the value to be substituted for a CloudFormation Parameter,
and that Parameter was used in an Fn::Sub expression,
it caused incorrect CloudFormation to be generated
(an Fn::Join as an argument to Fn::Sub, which is not allowed).
Check for that case explicitly, and move that substitution into the Fn::Sub map instead.

Fixes #14047

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
… in Fn::Sub expressions (aws#14068)

When a Token that resolved to an object, like `{ Ref }`,
was passed as the value to be substituted for a CloudFormation Parameter,
and that Parameter was used in an Fn::Sub expression,
it caused incorrect CloudFormation to be generated
(an Fn::Join as an argument to Fn::Sub, which is not allowed).
Check for that case explicitly, and move that substitution into the Fn::Sub map instead.

Fixes aws#14047

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
2 participants