search

aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.67k stars 3.92k forks source link

cloudformation-diff crashes if forced to diff against a non-CDK template #7413

Closed yangtina closed 1 year ago

yangtina commented 4 years ago

@aws-cdk/cloudformation-diff/lib/iam does not handle Fn::If in IAM policy statements in existing CloudFormation stacks.

Reproduction Steps

  1. Create a CloudFormation stack with raw CloudFormation and utilize Fn::If: in IAM policies.

for example:

Parameters:
  AttachLambdaFunctionToVPC: { Type: String, Default: 'false', AllowedValues: ['true', 'false']}
Conditions:
  RunLambdaInVPC:
    Fn::Equals: [ {Ref: AttachLambdaFunctionToVPC}, "true"]
Resources:
  LambdaRole:
    Properties:
      ManagedPolicyArns:
      - arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole
      AssumeRolePolicyDocument:
        Statement:
        - Action: ['sts:AssumeRole']
          Effect: Allow
          Principal:
            Service: [lambda.amazonaws.com]
        Version: '2012-10-17'
      Policies:
      - Fn::If:
        - RunLambdaInVPC
        - PolicyDocument:
            Statement:
            - Action: ['ec2:CreateNetworkInterface', 'ec2:DescribeNetworkInterfaces', 'ec2:DeleteNetworkInterface']
              Effect: Allow
              Resource: '*'
            Version: '2012-10-17'
          PolicyName: lambdaRoleAPIG
        - {Ref: 'AWS::NoValue'}
    Type: AWS::IAM::Role

alternatively, this following valid policy definition will also result in an error:

      Policies:
      - PolicyDocument:
          Statement:
          - Action: ['cloudwatch:*', 'logs:*', 'dynamodb:GetItem', 'dynamodb:PutItem']
            Effect: Allow
            Resource: '*'
          - Fn::If:
            - RunLambdaInVPC
            - Action: ['ec2:CreateNetworkInterface', 'ec2:DescribeNetworkInterfaces', 'ec2:DeleteNetworkInterface']
              Effect: Allow
              Resource: '*'
            - {Ref: 'AWS::NoValue'}
          Version: '2012-10-17'
        PolicyName: lambdaRoleAPIG
  1. Rewrite the IAM role in CDK
  2. Synthesize templates
  3. Run cdk diff or cdk deploy against the CloudFormation stack created in step 1

Error Log

Errors observed when running cdk diff or cdk deploy for the templates listed in the repro step respectively:

TypeError: Cannot read property 'Statement' of undefined
    at /Users/yangtina/.../node_modules/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts:187:106
TypeError: Cannot use 'in' operator to search for 'NotResource' in {"Fn::If":["RunLambdaInVPC",{"Action":["ec2:CreateNetworkInterface","ec2:DescribeNetworkInterfaces","ec2:DeleteNetworkInterface"],"Effect":"Allow","Resource":"*"}]}
    at new Targets (/Users/yangtina/.../node_modules/@aws-cdk/cloudformation-diff/lib/iam/statement.ts:160:20)

Environment

Other

The use case this serves is for migrating a production services originally created with CloudFormation to CDK. To avoid doing endpoint migrations for production services due to essential resources being recreated by CloudFormation, we are writing CDK to target an existing stack previously defined by raw CloudFormation instead of creating a whole new CDK stack.

This is :bug: Bug Report

rix0rrr commented 4 years ago

Other occurrence: it also does not handle things like !Ref in the existing template:

https://github.com/aws/aws-cdk/issues/7500

yangtina commented 4 years ago

We also observed this issue with @aws-cdk/cloudformation-diff/lib/network when using Fn::If in the SecurityGroupEgress property of SecurityGroups.

I do want to appeal that this is higher than a p2 because it's a common case for customers already on CloudFormation. cdk diff is a crucial function for customers wanting to migrate from CloudFormation to CDK without doing an CloudFormation stack / service endpoint migration. This crash masks other potential issues so it's a big hit to development productivity.

Regarding the problems with shorthand function in existing CloudFormation templates, is it related to or a duplicate of https://github.com/aws/aws-cdk/issues/6537?

filletofish commented 4 years ago

Hi,

Our team is also impacted by this issue. The context is the same as @yangtina described above. Our project is in the process of migrating from CloudFormation to CDK and contains a mix of CloudFormation templates and CDK.

Due to this crash we have either to

  1. Rewrite existing Cfn templates where Fn::If is used inside IAM policies

or

  1. Set flag --require-approval to never (in this case cdk diff is not called during deployment). Again, this workaround in turn can mask sensitive changes to the infrastructure 😞

Error stack trace:

Cannot use 'in' operator to search for 'NotResource' in {"Fn::If":["TestCondition",{"Action":["ec2:CreateNetworkInterface","ec2:DescribeNetworkInterfaces","ec2:DeleteNetworkInterface"],"Effect":"Allow","Resource":"*"}]}
TypeError: Cannot use 'in' operator to search for 'NotResource' in {"Fn::If":["TestCondition",{"Action":["ec2:CreateNetworkInterface","ec2:DescribeNetworkInterfaces","ec2:DeleteNetworkInterface"],"Effect":"Allow","Resource":"*"}]}
    at new Targets (/usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/iam/statement.ts:160:20)
    at new Statement (/usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/iam/statement.ts:38:22)
    at /usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/iam/statement.ts:99:28
    at Array.map (<anonymous>)
    at Object.parseStatements (/usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/iam/statement.ts:99:12)
    at /usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts:187:51
    at Object.flatMap (/usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/util.ts:46:17)
    at IamChanges.readIdentityPolicies (/usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts:186:12)
    at IamChanges.readPropertyChange (/usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts:144:40)
    at new IamChanges (/usr/local/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts:36:12)

Best regards, Filipp

phillipCouto commented 4 years ago

To work around this I actually used the Designer in AWS Management console to convert my YAML template into a JSON one and updated the stack. Even though there was only some metadata change the update took. Then I could run cdk diff pointing to my non-cdn stack and actually migrated cleanly.

I strongly recommend creating a changeset when you convert it just to make sure there are no substantial changes due to the conversion from YAML to JSON not accounting for everything correctly.

gabrielibagon commented 3 years ago

Has there been any updates to this issue? I'm trying to migrate AWS API Gateway Developer Portal to CDK so that it can be deployed with the rest of our stack.

I tried 2 approaches: 1) creating a CloudFormation template from the SAM template (using sam-translate) 2) deploying the dev portal, then downloading the JSON/YAML template from the cloudformation designer console (mentioned by @phillipCouto in https://github.com/aws/aws-cdk/issues/7413#issuecomment-650505381).

In both cases, I was unable to deploy after importing the json/yaml into CDK using cfn_inc.CfnInclude. The error returned is:

Cannot use 'in' operator to search for 'NotResource' in {"Fn::If":["ConditionsEnableFeedbackSubmissionDE4E49A7",{"Action":["dynamodb:Scan","dynamodb:PutItem","dynamodb:UpdateItem","dynamodb:DeleteItem"],"Resource":"arn:aws:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${FeedbackTableE4CCF159}","Effect":"Allow"}]}

ds17f commented 3 years ago

I'm seeing the same problem when using CfnInclude on a template for VPC where the SecurityGroups have Fn::If in them. When I run a cdk diff the output prints rules.map is not a function. When I run cdk diff -v I get

rules.map is not a function
TypeError: rules.map is not a function
    at SecurityGroupChanges.readInlineRules (/Users/ds17f/.nvm/versions/node/v16.2.0/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/network/security-group-changes.ts:106:4)
    at new SecurityGroupChanges (/Users/ds17f/.nvm/versions/node/v16.2.0/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/network/security-group-changes.ts:25:35)
    at new TemplateDiff (/Users/ds17f/.nvm/versions/node/v16.2.0/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/diff/types.ts:57:33)
    at calculateTemplateDiff (/Users/ds17f/.nvm/versions/node/v16.2.0/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/diff-template.ts:110:10)
    at Object.diffTemplate (/Users/ds17f/.nvm/versions/node/v16.2.0/lib/node_modules/aws-cdk/node_modules/@aws-cdk/cloudformation-diff/lib/diff-template.ts:43:19)
    at Object.printStackDiff (/Users/ds17f/.nvm/versions/node/v16.2.0/lib/node_modules/aws-cdk/lib/diff.ts:24:24)
    at CdkToolkit.diff (/Users/ds17f/.nvm/versions/node/v16.2.0/lib/node_modules/aws-cdk/lib/cdk-toolkit.ts:103:18)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at initCommandLine (/Users/ds17f/.nvm/versions/node/v16.2.0/lib/node_modules/aws-cdk/bin/cdk.ts:209:9)

I modified the code that appears to be causing an error to print out the content in rules and return an empty array. This allow the template to run and shows me the data that's causing an error. It looks like (formatted for human readability by me): 

{
  "Fn::If": [
    "IsIPv6Enabled",
    [
      {
        "Description": "Allow all traffic TO all IPv4 addresses",
        "IpProtocol": "-1",
        "CidrIp": "0.0.0.0/0"
      },
      {
        "Description": "Allow all traffic TO all IPv6 addresses",
        "IpProtocol": "-1",
        "CidrIpv6": "::/0"
      }
    ],
    [
      {
        "Description": "Allow all traffic TO all IPv4 addresses",
        "IpProtocol": "-1",
        "CidrIp": "0.0.0.0/0"
      }
    ]
  ]
}

It appears that the data in rules should be an array but is not, and since that's the case it produces an exception because map() isn't a method on a raw object.

skinny85 commented 3 years ago

This is similar to https://github.com/aws/aws-cdk/issues/12887. We should definitely fix it.

awsiv commented 2 years ago

Same issue with Security groups 😞

ozeebee commented 1 year ago

To work around this, I created a nested stack (that contains the included CFN template). This way, the diff does not crash. But this is really an ugly hack ... Hope this will be fixed some day.

github-actions[bot] commented 1 year ago

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

github-actions[bot] commented 1 year ago

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