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

Errant E1029 for AWS::Serverless::StateMachine DefinitionSubstitutions, and cannot ignore #1602

Closed
tomislacker opened this issue Jun 23, 2020 · 3 comments

Comments

@tomislacker
Copy link
Contributor

tomislacker commented Jun 23, 2020

cfn-lint version: (cfn-lint --version)

$ cfn-lint --version
cfn-lint 0.33.1

Description of Issue

When using the AWS::Serverless::StateMachine.DefinitionSubstitutions property, cfn-lint believes that a substitution is required within the Definitions property structure despite the intent for the SAM transform to handle this.

Additionally, a resource-level ignore does not avoid the issue but a template-level ignore does which I believe is related to aws/serverless-application-model#450

The raw output is:

$ cfn-lint thing.yml
E1029 Found an embedded parameter "${MyFunctionArn}" outside of an "Fn::Sub" at Resources/StateMachine/Properties/DefinitionString/Fn::Join/1/5
thing.yml:44:3

Example Template

---
AWSTemplateFormatVersion: 2010-09-09
Transform: AWS::Serverless-2016-10-31

Globals:
  Function:
    Runtime: python3.6
    MemorySize: 128
    Timeout: 30
    Tracing: Active

Resources:
  MyFunctionLogGroup:
    Type: AWS::Logs::LogGroup
    Properties:
      LogGroupName: !Sub /aws/lambda/${MyFunction}
      RetentionInDays: 1

  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: ../src/
      Handler: index.handler

  StateMachineRole:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Version: 2012-10-17
        Statement:
        - Effect: Allow
          Principal:
            Service: !Sub states.${AWS::Region}.amazonaws.com
          Action: sts:AssumeRole
      Policies:
      - PolicyName: lambda
        PolicyDocument:
          Statement:
          - Effect: Allow
            Action: lambda:InvokeFunction
            Resource:
              - !GetAtt MyFunction.Arn

  StateMachine:
    Type: AWS::Serverless::StateMachine
    Metadata:
      cfn-lint:
        config:
          ignore_checks:
            # Found an embedded parameter "${MyFunctionArn}" outside of an "Fn::Sub"
            - E1029
    Properties:
      DefinitionSubstitutions:
        MyFunctionArn: !GetAtt MyFunction.Arn
      Definition:
        StartAt: MyFunction
        States:
          MyFunction:
            End: True
            Resource: '${MyFunctionArn}'
            Type: Task
      Role: !GetAtt StateMachineRole.Arn

# vim:ft=yaml.cloudformation sw=2 ts=2

Expected Behavior

One-or-multiple of:

  1. The E1029 error can be ignored at the resource level
  2. The check appropriately realizes that the '${MyFunctionArn}' is in the DefinitionSubstitutions structure
@PatMyron
Copy link
Contributor

  • The E1029 error can be ignored at the resource level

This one is blocked on the Serverless transform passing along Metadata

The Linter actually runs the Serverless transform before evaluating this rule:

$ cfn-lint --info template.yaml # to see the transformed template

so this is actually a problem with AWS::StepFunctions::StateMachine after the transformation is complete

might need to exclude that property from being evaluated in:

https://github.com/aws-cloudformation/cfn-python-lint/blob/5006b4f31b4381ba2f1db53fc21c2461204a7b76/src/cfnlint/rules/functions/SubNeeded.py#L13

@dontirun
Copy link
Contributor

Similarly if we define a resource without a Definition Substitution String and Attempt to use an Intrinsic Function such as !Sub or !GetAtt a similar error will occur as SAM creates Definition Substitutions for those resources.

Example

  rTransferStateMachine:
    Type: AWS::Serverless::StateMachine
    Metadata:
      cfn-lint:
        config:
          ignore_checks:
            # Found an embedded parameter "${definition_substitution_1}" outside of an "Fn::Sub" at Resources/rTransferStateMachine/Properties/DefinitionString/Fn::Join/1/5"
            - E1029
    Properties:
      Definition:
        StartAt: TransferArtifact
        States:
          TransferArtifact:
            Type: Task
            Resource: !GetAtt rTransferFunction.Arn
            ResultPath: $.result
            End: true
      Role: !GetAtt rTransferRole.Arn

Transformed to

    "rTransferStateMachine": {
      "Properties": {
        "DefinitionString": {
          "Fn::Join": [
            "\n",
            [
              "{",
              "    \"StartAt\": \"TransferArtifact\",",
              "    \"States\": {",
              "        \"TransferArtifact\": {",
              "            \"End\": true,",
              "            \"Resource\": \"${definition_substitution_1}\",",
              "            \"ResultPath\": \"$.result\",",
              "            \"Type\": \"Task\"",
              "        }",
              "    }",
              "}"
            ]
          ]
        },
        "DefinitionSubstitutions": {
          "definition_substitution_1": {
            "Fn::GetAtt": [
              "rTransferFunction",
              "Arn"
            ]
          }
        },
        "RoleArn": {
          "Fn::GetAtt": [
            "rTransferRole",
            "Arn"
          ]
        },
        "Tags": [
          {
            "Key": "stateMachine:createdBy",
            "Value": "SAM"
          }
        ]
      },
      "Type": "AWS::StepFunctions::StateMachine"
    }

Instead of adding a blanket exception on the DefinitionString Field, an exception can be added for anything inside a ${} within a DefinitionString field that matches a top level key within DefinitionSubstitutions

@dontirun
Copy link
Contributor

This issue should be resolved with the the recent pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants