Cfn-python-lint: AWS::Serverless::Function ignores resource-based metadata

[tazatwell]

*cfn-lint version: 0.25.0

Hi, I have the following cfn template (just a sample, I can't provide my organization's template) named sample-template.yaml:

# Metadata:  # nodeJS8.10 EOL warning
#   cfn-lint:
#     config:
#       ignore_checks:
#       - W2531
AWSTemplateFormatVersion: '2010-09-09'
Transform: 'AWS::Serverless-2016-10-31'
Resources:
  Lambda:
    Metadata:  # nodeJS8.10 EOL warning
      cfn-lint:
        config:
          ignore_checks:
          - W2531
    Type: 'AWS::Serverless::Function'
    Properties:
      Handler: "index.handler"
      Runtime: "nodejs8.10"

When I run cfn-lint sample-template.yaml, I receive the following warning:

~/Documents/workspace/temp|
⇒  cfn-lint sample-template.yaml
W2531 EOL runtime (nodejs8.10) specified. Runtime is EOL since 2019-12-
31 and updating will be disabled at 2020-02-03, please consider to upda
te to nodejs10.x
sample-template.yaml:9:3
~/Documents/workspace/temp|
⇒  

When I un-comment the root-level Metadata, then the warning is gone:

Metadata:  # nodeJS8.10 EOL warning
  cfn-lint:
    config:
      ignore_checks:
      - W2531
AWSTemplateFormatVersion: '2010-09-09'
Transform: 'AWS::Serverless-2016-10-31'
Resources:
  Lambda:
    Metadata:  # nodeJS8.10 EOL warning
      cfn-lint:
        config:
          ignore_checks:
          - W2531
    Type: 'AWS::Serverless::Function'
    Properties:
      Handler: "index.handler"
      Runtime: "nodejs8.10"

~/Documents/workspace/temp|
⇒  cfn-lint sample-template.yaml
~/Documents/workspace/temp|
⇒  

I have no other files in my dir:

~/Documents/workspace/temp|
⇒  tree .
.
└── sample-template.yaml

0 directories, 1 file
~/Documents/workspace/temp|
⇒  

It seems as though for W2531, cfn-lint does not process resource-based metadata, but it _does_ process root-based metadata. I believe this is a bug.

I can provide more info on request.

Thank you!

[kddejong]

I think there are a few correctable things here but I need to think through a few things.

First thing is when we have Transform: "AWS::Serverless-2016-10-31" we run it through aws-sam-translator and we can lint the completed template. When this happens a lot of the template can be modified. Serverless function is translated into a lambda, etc. When this happens we lose a lot of line information because properties are changed, one resource becomes many, etc.

It looks like when those resources are translated the metadata is lost. @jlhood I'm curious if this intentional or not. If it was copied to the newly created resources you wouldn't have an issue (I believe).

We could use the original template to build the directives but the line and path information wouldn't be accurate so I'm not sure this would work correctly.

[jlhood]

@kddejong Thanks for tagging me. You're correct that today, SAM does not pass through Metadata on a AWS::Serverless::Function resource through to the underlying generated AWS::Lambda::Function resource.

One challenge is SAM can expand a AWS::Serverless::Function resource into many CloudFormation resources depending on what properties are used. Should SAM add the Metadata on just the AWS::Lambda::Function resource or all resources created by SAM? Metadata is so generic, I'm not sure there's a right answer here, but my first intuition is to just add it to the AWS::Lambda::Function.

Anyway, I'll open an issue for this in the SAM GitHub repo to discuss further there.

[jlhood]

Looks like there's an existing issue for it. Added a comment there.

[kddejong]

Thanks

[tazatwell]

Hi, thanks for the update.

It makes sense that some metadata may be lost since we're dealing with an AWS::Serverless::Function resource. And I'm also not sure how to go about including metadata since the resource is passed to aws-sam-translator. Resolving this will probably take time and careful consideration.

I made a quick PR to include info in the README about this here: #1297. Let me know if you feel that's appropriate.

Thanks for the help and clarification :) since there's already an existing issue, feel free to close this one.

[kddejong]

I'm going to close this issue. We can reopen later if needed.