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

New rule for Lambda zip deployment #2682

Merged
merged 9 commits into from
Jul 3, 2023
Merged

New rule for Lambda zip deployment #2682

merged 9 commits into from
Jul 3, 2023

Conversation

tal66
Copy link
Contributor

@tal66 tal66 commented Apr 11, 2023

(closes #2676)

Hi, implemented this new rule. what do you think?

*quick failure test:
cfn-lint test/fixtures/templates/bad/resources/lambda/zipfile_required_properties.yaml

@kddejong
Copy link
Contributor

Thanks @tal66 I'll take a look. I think this is a good start the big thing we have to cover for is when people use an intrinsic function (Fn::If, Ref, GetAtt, FindInMap, etc.).

@kftsehk
Copy link
Contributor

kftsehk commented Apr 11, 2023

@tal66 When implementing

You can check the implementation of #2673, isn't simple to compare these cases including correctly check !Ref AWS::NoValue as undefined

@tal66 @kddejong What do you think of this proposal

Would generally solve the problem of implementing rules enforcing For resource of Type X, if A, assert B is <op> C, which can be less prone to problems of correctly re-implementing checking to !Ref / !GetAtt in each rule.

@tal66
Copy link
Contributor Author

tal66 commented Apr 14, 2023

Do you mean templates such as this (*)?

is there already a way that you use to resolve possible scenarios and attributes?
or, since the Code(**) property is always required here, maybe it's enough just to see if by removing ImageUri (sub)property there's still something left under Code, which means zip file scenario (and then search for Handler and Runtime)

(*)

AWSTemplateFormatVersion: '2010-09-09'
Description: Properties with conditions.

Parameters:
  UseZipDeployment:
    Type: String
    Default: true
    AllowedValues:
      - true
      - false
Conditions:
  IsZipDeployment:
    !Equals [true, !Ref UseZipDeployment]

Resources:
  Function1:
    Type: AWS::Lambda::Function
    Properties:
      Role: arn:aws:iam::123456789012:role/lambda-role
      Code:
        !If
        - IsZipDeployment
        - S3Bucket: my-bucket
          S3Key: my-lambda-function.zip
        - ImageUri: 111122223333.dkr.ecr.us-east-1.amazonaws.com/app:latest
      PackageType: !If [IsZipDeployment, "Zip", "Image"]

(**)

# Code:
  ImageUri: String
  S3Bucket: String
  S3Key: String
  S3ObjectVersion: String
  ZipFile: String

@kftsehk
Copy link
Contributor

kftsehk commented Apr 15, 2023

afaik, Code (or CodeURI in Serverless::Function)
can be

  • Inplace code ZipFile: requires Handler and PackageType: Zip
  • String, e.g. "./my-lambda-function", where the artifacts are automatically uploaded and substituted into S3 url when using sam deploy, requires Handler and PackageType: Zip
  • S3, requires Handler and PackageType: Zip
  • ImageUri, PackageType: Image

For your example, a valid template for both IsZipDeployment is true / false would be

Resources:
  Function1:
    Type: AWS::Lambda::Function
    Properties:
      Role: arn:aws:iam::123456789012:role/lambda-role
      Code:
        !If
        - IsZipDeployment
        - S3Bucket: my-bucket
          S3Key: my-lambda-function.zip
        - ImageUri: 111122223333.dkr.ecr.us-east-1.amazonaws.com/app:latest
      Handler: !If [IsZipDeployment, "my.handler", !Ref AWS::NoValue]
      PackageType: !If [IsZipDeployment, "Zip", "Image"]

@kftsehk
Copy link
Contributor

kftsehk commented Apr 15, 2023

Not sure if everyone agree with this, what do you think?
As

  • cfn-lint is best effort basis
  • there is probably not a good reason one would maintain 2 different deployment of the same lambda function (one zip, one image)

we can opt to implement only simple cases of the following, ignoring the very complicated one

AWSTemplateFormatVersion: "2010-09-09"
Conditions:
  IsZipDeployment: !Equals [true, !Ref 'UseZipDeployment']
Description: Properties with conditions.
Parameters:
  MyImage:
    Type: String
  MyImageVersion:
    Type: String
  UseZipDeployment:
    AllowedValues:
      - true
      - false
    Default: true
    Type: String
Resources:
  Bucket:
    Properties:
      BucketName: my-bucket
    Type: AWS::S3::Bucket
  SimpleLambdaImage:
    Properties:
      Code:
        ImageUri: !Ref 'MyImage'
      PackageType: Image
      Role: arn:aws:iam::123456789012:role/lambda-role
    Type: AWS::Lambda::Function
  SimpleLambdaImageWithVersion:
    Properties:
      Code:
        ImageUri: !Sub
          - '${S}:${V}'
          - S: !Ref 'MyImage'
            V: !Ref 'MyImageVersion'
      PackageType: Image
      Role: arn:aws:iam::123456789012:role/lambda-role
    Type: AWS::Lambda::Function
  SimpleLambdaInplaceSource:
    Properties:
      Code:
        ZipFile: !Sub
          - |
            def handler(event, context):
              return "Hello World ${Region}"
          - Region: !Ref 'AWS::Region'
      Handler: index.handler
      PackageType: Zip
      Role: arn:aws:iam::123456789012:role/lambda-role
    Type: AWS::Lambda::Function
  SimpleLambdaZipLocalSource:
    Properties:
      Code: ./src
      Handler: !Join
        - '.'
        - - index
          - !Ref 'AWS::Region'
      PackageType: Zip
      Role: arn:aws:iam::123456789012:role/lambda-role
    Type: AWS::Lambda::Function
  SimpleLambdaZipS3:
    Properties:
      Code:
        S3Bucket: !Ref 'Bucket'
        S3Key: !Sub
          - 'my-function-${R}'
          - R: !Ref 'AWS::Region'
      Handler: index.handler
      PackageType: Zip
      Role: arn:aws:iam::123456789012:role/lambda-role
    Type: AWS::Lambda::Function
  VeryComplicatedConditionUnlikelyToBeUsed:
    Properties:
      Code: !If
        - IsZipDeployment
        - S3Bucket: !Ref 'Bucket'
          S3Key: !Sub
            - "my-function-${R}"
            - R: !Ref 'AWS::Region'
        - ImageUri: 111122223333.dkr.ecr.us-east-1.amazonaws.com/app:latest
      Handler: !If
        - IsZipDeployment
        - !Sub "my.${AWS::Region}.handler}"
        - !Ref 'AWS::NoValue'
      PackageType: !If
        - IsZipDeployment
        - "Zip"
        - "Image"
      Role: arn:aws:iam::123456789012:role/lambda-role
    Type: AWS::Lambda::Function

@kddejong
Copy link
Contributor

kddejong commented May 4, 2023

Can you run and submit the updates?

black src/ test/
isort --profile black src/cfnlint test/

@kddejong kddejong merged commit b7bc622 into aws-cloudformation:main Jul 3, 2023
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

Successfully merging this pull request may close these issues.

New rule for package command and Lambda
3 participants