(cloudformation-include): detect cycles in the input template

[acdoussan]

Cloudformation include gets into a recursive loop and stack overflows with a valid lambda behind API gateway configuration

Reproduction Steps

import the following cloudformation, run cdk ls, get stack overflow. I've stripped down this cloudformation as best as I can, so it may not be strictly valid, but its valid enough to show off the issue.

In this example, the APIGatewayExecutionRole depends on the LambdaFunction.Arn, then the LambdaFunction depends on the LambdaAPIDefinition for its events, and finally the LambdaAPIDefinition depends on the APIGatewayExecutionRole for the credentials.

"AWSTemplateFormatVersion": "2010-09-09"
"Resources":
  "APIGatewayExecutionRole":
    "Properties":
      "AssumeRolePolicyDocument":
        "Statement":
        - "Action":
          - "sts:AssumeRole"
          "Effect": "Allow"
          "Principal":
            "Service":
            - "apigateway.amazonaws.com"
        "Version": "2012-10-17"
      "Policies":
      - "PolicyDocument":
          "Statement":
          - "Action":
            - "lambda:Invoke*"
            "Effect": "Allow"
            "Resource":
              "Fn::Join":
              - ""
              - - "Fn::GetAtt": "LambdaFunction.Arn"
                - "*"
          "Version": "2012-10-17"
        "PolicyName": "apigInvokeLambda"
    "Type": "AWS::IAM::Role"
  "LambdaAPIDefinition":
    "DependsOn": "APIGatewayAccount"
    "Properties":
      "DefinitionBody":
        "basePath": "/"
        "info":
          "description": "api"
          "title": "api"
          "version": "2017-07-25"
        "paths":
          "/api":
            "options":
              "consumes":
              - "application/json"
              "produces":
              - "application/json"
              "responses":
                "200":
                  "description": "Default response for CORS method"
                  "headers":
                    "Access-Control-Allow-Headers":
                      "type": "string"
                    "Access-Control-Allow-Methods":
                      "type": "string"
                    "Access-Control-Allow-Origin":
                      "type": "string"
              "x-amazon-apigateway-integration":
                "requestTemplates":
                  "application/json": "{\"statusCode\":200}"
                "responses":
                  "default":
                    "responseParameters":
                      "method.response.header.Access-Control-Allow-Headers": "'Content-Type,X-Amz-Date,Authorization,X-Api-Key,X-Amz-Security-Token,X-Amz-Content-Sha256,X-Amz-User-Agent'"
                      "method.response.header.Access-Control-Allow-Methods": "'POST'"
                      "method.response.header.Access-Control-Allow-Origin": "'*'"
                    "statusCode": "200"
                "type": "mock"
            "post":
              "consumes":
              - "application/json"
              "operationId": "api"
              "parameters":
              - "description": "api"
                "in": "body"
                "name": "api"
                "required": !!bool "true"
              "produces":
              - "application/json"
              "responses":
                "200":
                  "description": "api"
                "400":
                  "description": "api 400"
              "security":
              - "sigv4": []
              "x-amazon-apigateway-integration":
                "credentials":
                  "Fn::GetAtt":
                  - "APIGatewayExecutionRole"
                  - "Arn"
                "httpMethod": "POST"
                "passthroughBehavior": "NEVER"
                "responses":
                  "default":
                    "statusCode": "200"
                "type": "aws_proxy"
                "uri":
                  "Fn::Sub":
                  - "arn:${AWS::Partition}:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${LambdaArn}${LambdaAlias}/invocations"
                  - "LambdaAlias": ":live"
                    "LambdaArn":
                      "Fn::GetAtt":
                      - "LambdaFunction"
                      - "Arn"
  "LambdaFunction":
    "Properties":
      "AutoPublishAlias": "apilambda"
      "Events":
        "APIG":
          "Properties":
            "Method": "ANY"
            "Path": "/"
            "RestApiId":
              "Ref": "LambdaAPIDefinition"
          "Type": "Api"
      "Handler": "foo.bar::handleRequest"
      "MemorySize": !!int "2048"
      "Runtime": "java8"
      "Timeout": !!int "30"
    "Type": "AWS::Serverless::Function"

import { App, Stack } from 'monocdk';
import * as cfnInc from 'monocdk/cloudformation-include';

const app = new App();
const stack = new Stack(app, 'stack');

new cfnInc.CfnInclude(stack, 'example', {
  templateFile: './example.yaml',
});

What did you expect to happen?

CDK imports the cfn successfully.

What actually happened?

Stack overflow error. At a minimum this should throw an error for a circular reference, however, cloudformation itself can handle this, so I believe CDK should be able to handle this as well.

Environment

• **CDK CLI Version :**1.122.0
• **Framework Version:**1.119.0
• Node.js Version: v12.21.0
• OS : NAME="Amazon Linux" VERSION="2"
• Language (Version): "typescript": "^3.6.4"

Other

The main issue seems to be

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

Line 587 in 8ad33b8

private getOrCreateResource(logicalId: string): core.CfnResource {

does not recognize circular dependencies, in that it is trying to process something that it is already trying to process. Not totally sure how cloudformation manages to resolve this, but it should probably be mirrored here.

---

This is 🐛 Bug Report

[skinny85]

Hey @acdoussan,

thanks for opening the issue.

Even though you didn't include it in your snippet, I assume this template uses the Serverless transform, correct?

Would you mind downloading the processed template from CloudFormation (the one without AWS::Serverless::* resources), and let me know if it includes successfully?

Thanks,
Adam

[acdoussan]

Hey @skinny85

You're right, here is the transform field:

"Transform": "AWS::Serverless-2016-10-31"

Downloaded both the processed and unprocessed template from the AWS console, both still gave a stack overflow. Unprocessed was in yaml and still had AWS::Serverless::* resources, processed was in json and did not have AWS::Serverless::* resources.

[skinny85]

Would you mind uploading both templates, the one with the transform and the one without the transform, to this issue, so I can take a look?

Feel free to anonymize it if needed, of course.

[acdoussan]

Provided full cfn templates via private channels

[skinny85]

Thanks for providing the templates @acdoussan. Here are the results of my investigation.

For the unprocessed template you provided (the one with the AWS::Serverless-2016-10-31 transform), there is indeed a cycle in the resources. It looks as follows (simplified):

  "APIGatewayExecutionRole":
    "Properties":
      "AssumeRolePolicyDocument":
        "Statement":
        - "Action":
          - "sts:AssumeRole"
          "Effect": "Allow"
          "Principal":
            "Service":
            - "apigateway.amazonaws.com"
        "Version": "2012-10-17"
      "Policies":
      - "PolicyDocument":
          "Statement":
          - "Action":
            - "lambda:Invoke*"
            "Effect": "Allow"
            "Resource":
              "Fn::Join":
              - ""
              - - "Fn::GetAtt": "LambdaFunction.Arn"
                - "*"
          "Version": "2012-10-17"
        "PolicyName": "apigInvokeLambda"
    "Type": "AWS::IAM::Role"

  "LambdaFunction":
    "Properties":
      "AutoPublishAlias":
        "Ref": "LambdaAliasName"
      "CodeUri":
        "Bucket":
          "Fn::If":
          - "UseBatsKey"
          - !Ref "PipelinesControlledRegionBucket"
          - "Fn::ImportValue":
              "Ref": "DeploymentBucketImportName"
        "Key": "TEBALambda/development:TEBALambda-1.0:AWSLambda-1.0:TEBALambda-1.0/24e89002-eb0c-42c5-b58c-fe88b19fd96e/3aa9c6c8-11de-4d1c-9eb1-54e6d840fdf1-lambda.zip"
      "Events":
        "APIG":
          "Properties":
            "Method": "ANY"
            "Path": "/"
            "RestApiId":
              "Ref": "LambdaAPIDefinition"
          "Type": "Api"
      "Handler": "com.amazon.tebalambda.TEBALambdaEntryPoint::handleRequest"
      "MemorySize": !!int "2048"
      "Role":
        "Fn::GetAtt":
        - "LambdaRole"
        - "Arn"
      "Runtime": "java8"
    "Type": "AWS::Serverless::Function"

  "LambdaAPIDefinition":
    "DependsOn": "APIGatewayAccount"
    "Properties":
      "DefinitionBody":
        "paths":
          "/blue-anvil-data-cleanup/update-default-rate-card-sim-upload":
            "post":
              "x-amazon-apigateway-integration":
                "credentials":
                  "Fn::GetAtt":
                  - "APIGatewayExecutionRole"
                  - "Arn"
                "httpMethod": "POST"
                "passthroughBehavior": "NEVER"
                "responses":
                  "default":
                    "statusCode": "200"
                "type": "aws_proxy"
                "uri":
                  "Fn::Sub":
                  - "arn:${AWS::Partition}:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${LambdaArn}${LambdaAlias}/invocations"
                  - "LambdaAlias": ":live"
                    "LambdaArn":
                      "Fn::GetAtt":
                      - "LambdaFunction"
                      - "Arn"
    "Type": "AWS::Serverless::Api"

As you can see, APIGatewayExecutionRole depends on LambdaFunction, which then depends on LambdaAPIDefinition, which then depends on both LambdaFunction, and APIGatewayExecutionRole. That's the reason for the stack overflow.

As for the processed file (the one without any transforms), I was actually able to include it successfully, without any errors. Can you please try to include it in your CDK app, and let me know if it works?

I'm keeping this issue open to provide a better error message when a cycle in the input template has been found.

Thanks,
Adam

[acdoussan]

You're right, looks like the JSON includes, I likely forgot to rebuild the typescript after pointing to the JSON before rerunning the cdk command.

Is it the correct interpretation that you're recommending we use the downloaded JSON to import our stack into CDK? We are trying to migrate to cdk from our existing cloudformation, and then slowly eliminate the cloudformation as we can.

I'm not too sure that importing the JSON from AWS is a great solution, we have some things that may need to be updated at some point that are currently built with jinja, and there are also things that are injected here by dependencies that we would have to handle somehow as well. Not to mention trying to maintain a giant file is also problematic (what is provided is after a build stage, our existing implementation is separated into multiple files).

Looks like we could maybe hack in the translation (appears there is an implementation here) but this sort of feels like something the library should handle. Is integrating that / handling serverless transform something you would consider?

[skinny85]

Is it the correct interpretation that you're recommending we use the downloaded JSON to import our stack into CDK? We are trying to migrate to cdk from our existing cloudformation, and then slowly eliminate the cloudformation as we can.

Yes, I'm recommending you use the processed JSON file with CfnInclude. Now, when you do that, cdk diff will show a ton of changes. However, I'm 99% sure it's actually safe to perform cdk deploy, and that will not cause any changes to your app, and change your template to no longer use the Serverless transform (however, make sure to try it first on a development environment, to confirm I'm right, before doing this in production!).

I'm not too sure that importing the JSON from AWS is a great solution, we have some things that may need to be updated at some point that are currently built with jinja, and there are also things that are injected here by dependencies that we would have to handle somehow as well. Not to mention trying to maintain a giant file is also problematic (what is provided is after a build stage, our existing implementation is separated into multiple files).

After you do that migration, you can use CDK to edit your resources, as shown in this video. You will never have to touch that huge JSON file again - everything will be done through CDK code.

Hope that helps!

[acdoussan]

Gotcha, took a look, not the ideal solution I think because you still have to dig through the file to make changes, but otherwise does look pretty nice. Only slightly confusing thing is that example states to use the unprocessed template, I assume this example just didn't run into whatever is going wrong here.

One final question, we use SAM toolkit deploys to allow each developer to have their own stack to test with before deploying to a beta environment, since we are stripping out the SAM stuff from the template I imagine that won't be possible anymore. Is there a something similar we can do from the CDK side?

I suppose we could create multiple instances of the same stack, just wondering if there is a best practice / recommended way to do this.

[skinny85]

Only slightly confusing thing is that example states to use the unprocessed template, I assume this example just didn't run into whatever is going wrong here.

Yes, because that example did not contain a cycle in the SAM template, so using an unprocessed template is better, because of the thing I mentioned in this comment:

Is it the correct interpretation that you're recommending we use the downloaded JSON to import our stack into CDK? We are trying to migrate to cdk from our existing cloudformation, and then slowly eliminate the cloudformation as we can.

Yes, I'm recommending you use the processed JSON file with CfnInclude. Now, when you do that, cdk diff will show a ton of changes.

---

Gotcha, took a look, not the ideal solution I think because you still have to dig through the file to make changes, but otherwise does look pretty nice.

The only time you should need to dig through the template file is to get the logical ID of the resources to retrieve them in your CDK code - no other digging should be necessary 🙂.

---

One final question, we use SAM toolkit deploys to allow each developer to have their own stack to test with before deploying to a beta environment, since we are stripping out the SAM stuff from the template I imagine that won't be possible anymore. Is there a something similar we can do from the CDK side?

Yes, absolutely! You actually have many options in this area:

1. SAM now works with CDK natively 🙂.
2. CDK includes a --hotswap feature that shortens development iteration length, which supports Lambda functions.

---

Thanks,
Adam

[github-actions]

⚠️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.

[AlexanderADietrich]

The fix that closed this issue doesn't solve the problem in full, as stated initially "Not totally sure how cloudformation manages to resolve this, but it should probably be mirrored here." Currently, all the fix does is enable throwing a more descriptive error, not including an unprocessed template using transforms that results in circular references. I believe this issue should be re-opened so that a PR can accurately be prioritized if submitted to resolve this lack of parity.

[skinny85]

@AlexanderADietrich this issue was about detecting cycles, and that was fixed.

Handling cycles is a separate issue 🙂.