cfn-lint icon indicating copy to clipboard operation
cfn-lint copied to clipboard

Add support for Fn::Transform

Open atkinsonm opened this issue 5 years ago • 32 comments

cfn-lint version: 0.7.4

Description of issue.

cfn-lint does not properly evaluate Fn::Transform.

Example:

Resources:
  
  'Fn::Transform':
    Name: 'AWS::Include'
    Parameters:
      Location: "s3://mybucket/mySnippet.yaml"

  'Fn::Transform':
    Name: 'AWS::Include'
    Parameters:
      Location: "s3://mybucket/mySecondSnippet.yaml"

Returns:

Illegal cfn - missing Type: id: Fn::Transform

Additionally, multiple Fn::Transforms produce this error:

Duplicate resource found "Fn::Transform" (line 299)

The template used which generated these errors passed aws cloudformation validate-template without error, so it is valid CFN syntax.

atkinsonm avatar Nov 20 '18 21:11 atkinsonm

Agreed. Transforms and Macros are an area we need to work on. @cmmeyer and @melusom what would you like to see here?

One approach would be to read the file and process it into the template just like the import would do. The other approach, sadly, would be to lint what we could. However this would require us to know what rules could break when a transform is used. Like we can't validate Refs or GetAtts to resources cause as in provided example all the resources don't exist. Basically the result would be a stripped down linting. Even harder is if you had the following template. The rules to check may change as Ref and GetAtts to other resources could now be checked.

Resources:
  Bucket:
    Type: AWS::S3::Bucket
    Properties:
      'Fn::Transform':
         Name: 'AWS::Include'
         Parameters:
            Location : "s3://mybucket/mySnippet.yaml"

That being said we have tried to not have much of a requirement on the network. Also we would have to hope the person running the cfn-lint would have read access to the template in the bucket (or have a profile setup for us to access it).

kddejong avatar Nov 21 '18 03:11 kddejong

Agreed @kddejong, it would be impossible to know whether missing Refs or other errors were legitimate without evaluating macros. Though we want to minimize network requirements, I think it is necessary in this case. I can imagine scenarios where the cfn-lint running environment has separate permissions than the one running Create|UpdateStack, but I feel that it is reasonable to require read access to the templates in this case.

Processing macros before running cfn-lint would mitigate the two errors I ran into as well as any false-positive "missing Ref or GetAtt" errors.

atkinsonm avatar Nov 21 '18 12:11 atkinsonm

I would love to see the cloudformation package command extended to handle includes -- that way you could have local file paths in your template that we could evaluate pre-deploy and CFN could handle uploading the included files and deploying. But since it doesn't, I think our option is to lint around unless you can supply us a converged file to evaluate.

I would discourage doing this convergence within cfn-lint for network files, but it starts to outline a larger tool for local composition and deployment -- perhaps with pre-and-post macro evaluation linting phases. :)

cmmeyer avatar Nov 21 '18 16:11 cmmeyer

@cmmeyer brings up an interesting point regarding CI. It may happen that the file used for the Include is not in the same path/repository or was even developed by the same user. We can't assume that mySnippet.yaml is available locally.

If the approach will be not to evaluate network files, we will need to suppress the warnings above and we will also need to accept that users will receive false-positive "missing Ref or GetAtt" errors.

atkinsonm avatar Nov 21 '18 17:11 atkinsonm

Just adding another opinion here. 😄

I think the choice to limit network and IAM role reliance is a good one. Any sort of change that requires actually hitting AWS resources is going to create a lot of issues in this repo with people trying to diagnose IAM issues, and that shouldn't be the focus of this repository.

Since aws cloudformation package supports the Include transform for local files, I think that is a sane feature to support here as well.

If the Include transform matches a local file, I think this project should pull it in before linting.

If the Include transform does not match a local file, I think this project should not try do anything too fancy. If it does anything at all with the included block, treat it similarly to how the errors for aws cloudformation package issues work -- with a single code that can be ignored easily.

spockNinja avatar Dec 20 '18 23:12 spockNinja

@spockninja I can get on board with that approach. It won't fit all use cases but it makes a reasonable effort while avoiding IAM and network dependencies.

atkinsonm avatar Dec 21 '18 00:12 atkinsonm

Any movement on this? Last post was almost a year ago. I'm not sure if the error I'm seeing is the same as this?

image

iDVB avatar Jul 18 '19 00:07 iDVB

@iDVB I think for your scenarios we can probably work on a fix. The hard part is that Transform can complete change templates. Ideally we would be linting the resulting template from Transforms. That being said I think we should be able to not fail those two rules if we run into Fn::Transform.

kddejong avatar Jul 19 '19 00:07 kddejong

Another example:

Resources:
  ApiName:
    Type: AWS::Serverless::Api
    Properties:
      StageName: foo
      DefinitionBody:
        'Fn::Transform':
          Name: AWS::Include
          Parameters:
            Location: swagger.yaml
  FunctionName:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: ./dist
      Handler: FunctionNameHandler.default
      Runtime: nodejs10.x
      Events:
        FunctionNameEvent:
          Type: Api
          Properties:
            Path: /foo
            Method: post
            RestApiId:
              Ref: ApiName
            RequestModel:
              Model: RequestPayloadModel
              Required: true

The above template produces the error: E0001 Error transforming template: Resource with id [FunctionName] is invalid. Event with id [FunctionNameEvent] is invalid. Unable to set RequestModel [RequestPayloadModel] on API method [post] for path [/foo] because the related API does not define any Models.

cfn-lint is not executing the transform to include the local swagger.yaml file, which contains the RequestPayloadModel that it complains about!

While running sam validate, no error is thrown.

hugoduraes avatar Nov 30 '19 18:11 hugoduraes

@hugoduraes, I sort have the same as you but I solved it differently, so:

Api:
  Type: AWS::Serverless::Api
  Properties:
    DefinitionBody:
      Fn::Transform:
        Name: AWS::Include
        Parameters:
          Location: resources/openapi.yaml

and then in your resources/openapi.yaml file you would use the x-amazon-apigateway-integration:

x-amazon-apigateway-integration:
  type: aws
  uri: 
    Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${FunctionName.Arn}/invocations
  httpMethod: POST
  credentials:
    Fn::Ref: MyRoleArn

This way you will be able to deploy the template as you intended but you need to extend your swagger definition.

@kddejong I noticed that the AWS::Include transformation does not support the shorthand syntax so that is something that you would like to catch if you would validate the imported snippet.

Nr18 avatar Mar 06 '20 07:03 Nr18

Will try it. Thanks @Nr18 👍

hugoduraes avatar Mar 06 '20 10:03 hugoduraes

@Nr18 I've revisited my implementation and I've found that your suggestion is what I have at the moment. One thing you don't show in your comment is the template for your lambda function.

What causes the issue are the RestApiId and RequestModel properties on the lambda function template. Do you have these on your template?

hugoduraes avatar Mar 11 '20 10:03 hugoduraes

@hugoduraes so in your example it should look like this:

  ApiName:
    Type: AWS::Serverless::Api
    Properties:
      StageName: foo
      DefinitionBody:
        'Fn::Transform':
          Name: AWS::Include
          Parameters:
            Location: swagger.yaml
  FunctionName:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: ./dist
      Handler: FunctionNameHandler.default
      Runtime: nodejs10.x

And then in your swagger.yaml file it will look like:

...rest of the file...
x-amazon-apigateway-request-validators:
  full:
    validateRequestBody: true
    validateRequestParameters: true
x-amazon-apigateway-request-validator: full

paths:
  /foo:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/RequestPayloadModel'
      responses:
        "200":
          description: 200 Accepted
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Success'
      x-amazon-apigateway-integration:
        credentials:
          Fn::GetAtt: FunctionNameRole.Arn
        uri:
          Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${FunctionName.Arn}/invocations
        passthroughBehavior: when_no_templates
        httpMethod: POST
        type: aws
        responses:
          default:
            statusCode: "200" # Bad Request
            responseTemplates:
              application/json: |
                {
                  "Message": "Success"
                }
...rest of the file...

So you don't need the Events on your AWS::Serverless::Function resource.

Nr18 avatar Mar 12 '20 08:03 Nr18

@Nr18 are the request parameters validated on API Gateway if I don't specify the Events section?

hugoduraes avatar Mar 12 '20 09:03 hugoduraes

Yes, the following section in the OpenAPI definition would take care of that:

x-amazon-apigateway-request-validators:
  full:
    validateRequestBody: true
    validateRequestParameters: true
x-amazon-apigateway-request-validator: full

You could also place the x-amazon-apigateway-request-validator property on a specific method if you need to mix validators.

Nr18 avatar Mar 12 '20 10:03 Nr18

I thought those two properties (RestApiId and RequestModel) were also needed for it to validate requests. I've updated my template and open api docs and everything is looking great now. Thanks @Nr18

However, the issue reported here remains.

hugoduraes avatar Mar 20 '20 10:03 hugoduraes

Is there a way to get cfn-lint to ignore this issue?

I'm suffering from this same problem;

Api:
  Type: AWS::Serverless::Api
  Properties:
    DefinitionBody:
      Fn::Transform:
        Name: AWS::Include
        Parameters:
          Location: resources/openapi.yaml

E0001 Error transforming template: Resource with id [FunctionName] is invalid. Event with id [FunctionNameEvent] is invalid. Unable to set RequestModel [RequestPayloadModel] on API method [post] for path [/foo] because the related API does not define any Models.

EDIT: I tried this

api:
    Type: AWS::Serverless::HttpApi
    Metadata:
      cfn-lint:
        config:
          ignore_checks:
          - E0001

Which does not work

tkeeber avatar Apr 17 '20 07:04 tkeeber

@tkeeber you can workaround this issue by not defining the RequestModel on your function resource (commented out the RequestModel bit):

Resources:
  ApiName:
    Type: AWS::Serverless::Api
    Properties:
      StageName: foo
      DefinitionBody:
        'Fn::Transform':
          Name: AWS::Include
          Parameters:
            Location: swagger.yaml
  FunctionName:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: ./dist
      Handler: FunctionNameHandler.default
      Runtime: nodejs10.x
      Events:
        FunctionNameEvent:
          Type: Api
          Properties:
            Path: /foo
            Method: post
            RestApiId:
              Ref: ApiName
            # RequestModel:
            #  Model: RequestPayloadModel
            #  Required: true

hugoduraes avatar Apr 17 '20 08:04 hugoduraes

Thanks @hugoduraes, but I do not currently have that defined

tkeeber avatar Apr 17 '20 09:04 tkeeber

  Bucket:
    Type: AWS::S3::Bucket
  'Fn::Transform':
    Type: 'AWS::Include'
    Properties:

This snipped from AWS documentation didn't work for me, which is to be expected with this issue still open.

samj avatar Jun 17 '20 18:06 samj

 api:
     Type: AWS::Serverless::HttpApi
     Metadata:
       cfn-lint:
         config:
           ignore_checks:
           - E0001

Add this to disable the linter error at resource level:-

Resources:
  WebsiteBucket:
    Type: AWS::S3::Bucket
    Metadata:
      cfn-lint:
        config:
          ignore_checks:
          - E3001
    Fn::Transform:
      Name: AWS::Include
      Parameters:
        Location: 's3://xxxxx'
...
...

iamkhalidbashir avatar Oct 01 '20 11:10 iamkhalidbashir

does it make sense to at least have the option to completely ignore linting when Fn::Transform blocks are encountered?

could we support something like this?

Metadata:
  cfn-lint:
    config:
      ignore_transform: true

victoru avatar Jan 05 '21 16:01 victoru

We've been hitting this for a while, in our case it's error E3002 Invalid Property when using Fn::Transform to import container definitions for an ECS task definition.

qidydl avatar Jan 11 '21 19:01 qidydl

I have a similar issue and I'm not sure if it's due to the same thing, i.e. cfn-lint doesn't handle macros well. Here's my template:

Parameters:
  Environment:
    Type: String
  VpcSecurityGroupId:
    Type: String
  SubnetId001:
    Type: String
  SubnetId002:
    Type: String
  SubnetId003:
    Type: String

Conditions:
  IncludeLambdasInVpc:
    !Equals [!Ref Environment, "test"]

Resources:
  MyLambdaFunction:
    Type: AWS::Serverless::Function
    Properties:
      Handler: index.handler
      Runtime: python3.6
      InlineCode: |
        def handler(event, context):
          print("Hello, world!")
      VpcConfig:
        Fn::If: [
          "IncludeLambdasInVpc",
            {
              "SecurityGroupIds": [ {"Ref": "VpcSecurityGroupId"} ],
              "SubnetIds": [ {"Ref": "SubnetId001", "Ref": "SubnetId002", "Ref": "SubnetId003"} ]
            },
            {"Ref": "AWS::NoValue"}
        ]

Running cfn-lint against this template results in the following:

E0000 Duplicate resource found "Ref" (line 29)
/Users/dave.kennedy/Desktop/template.yaml:29:53

dave-kennedy avatar Jan 13 '21 22:01 dave-kennedy

That's because you do have duplicates.

- "SubnetIds": [ {"Ref": "SubnetId001", "Ref": "SubnetId002", "Ref": "SubnetId003"} ]
+ "SubnetIds": [ { "Ref": "SubnetId001" }, { "Ref": "SubnetId002" }, { "Ref": "SubnetId003" } ]

phene avatar Jan 13 '21 22:01 phene

Just want to keep kicking this along. I had my sceptre/service catalog/cloudformation templates all working dandy, with AWS::Include inside my mappings section. But when I issued a PR against our CICD system, it failed cfn-lint there. That means that, in our environment, I cannot use external mappings files and will need to hard-code the mappings section inside my templates. This is obviously both a sign of the level of respect we have for this product (we let you fail us before CICD testing starts) and also a gap that needs to be addressed in some way.

Part of the problem is that the errors generated (E7001 and E7002 are two which we see, for example) are not errors we want to ignore in CICD. If cfn-python-lint was even just modified to return warnings when it sees that a mapping is involved we could at least ignore that warning in our CICD pipeline.

Is there any foreseeable workaround for this?

sql-sith avatar Mar 11 '21 01:03 sql-sith

Hey, ran into this today, similar use-case as @qidydl 's . As noted above in @iamkhalidbashir 's workaround, I'm disabling E0001 in my POC to get past linting.

AledLewis avatar May 05 '21 17:05 AledLewis

Would be great to see resolution on this issue. Fundamentally, synthesizing the transform is an intractable problem, ignore it, perhaps with a message if possible so cfn-lint doesn't have to be disabled.

darrenweiner avatar Feb 13 '22 02:02 darrenweiner

Would be great to see this fixed. Currently getting a "W2001 Parameter not used" error for parameters I am referencing in my OAS.yaml. Snippet from template.yaml;

Parameters:
    IPWhitelist:
...

Resources:
    ApiGateway:
        Type: AWS::Serverless::Api
        Properties:
            OpenApiVersion: 3.0.0
            DefinitionBody:
                'Fn::Transform':
                    Name: 'AWS::Include'
                    Parameters:
                        Location: 'oas3.yaml'

And snippet from oas3.yaml;

x-amazon-apigateway-policy:
  Version: '2012-10-17'
  Statement:
  - Action: execute-api:Invoke
    Resource: "execute-api:/*"
    Effect: Allow
    Principal: '*'
  - Action: execute-api:Invoke
    Resource: "execute-api:/*"
    Effect: Deny
    Condition:
      NotIpAddress:
        aws:SourceIp:
        - Ref: IPWhitelist
    Principal: '*'

cfn-lint is exiting with W2001 Parameter IPWhitelist not used.

I could define the IPWhitelist as Auth.ResourcePolicy.IpRangeWhitelist on the APIGateway resource in template.yaml as opposed to in oas3.yaml, but then that causes sam validate --template template.yaml to fail due to https://github.com/aws/aws-sam-cli/issues/803

evin-murphy avatar Oct 03 '23 23:10 evin-murphy

@evin-murphy this is a different issue. For SAM we run the template through the SAM translator and validate the output. The underlying issue with Fn::Transform is we need to have the code to execute against. Thankfully for SAM they make it available publicly and we can leverage it easily.

kddejong avatar Oct 04 '23 19:10 kddejong