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

AWS::Serverless::Function ignores resource-based metadata #1294

Closed
tazatwell opened this issue Jan 8, 2020 · 6 comments
Closed

AWS::Serverless::Function ignores resource-based metadata #1294

tazatwell opened this issue Jan 8, 2020 · 6 comments

Comments

@tazatwell
Copy link
Contributor

*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
Copy link
Contributor

kddejong commented Jan 9, 2020

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
Copy link

jlhood commented Jan 9, 2020

@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
Copy link

jlhood commented Jan 9, 2020

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

@kddejong
Copy link
Contributor

kddejong commented Jan 9, 2020

Thanks

@tazatwell
Copy link
Contributor Author

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
Copy link
Contributor

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

@PatMyron PatMyron changed the title W2531 ignores resource-based metadata AWS::Serverless::Function ignores resource-based metadata Jun 30, 2020
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