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

Resources created through serverless transforms use hardcoded partitions #1522

Closed
dontirun opened this issue May 9, 2020 · 11 comments
Closed

Comments

@dontirun
Copy link
Contributor

dontirun commented May 9, 2020

*cfn-lint version: v0.30.1

Resources (such as Lambda Roles and Permissions) created by resources using AWS::Serverless use hardcoded Partitions

Example Resource

  rUploadRepo:
    Type: AWS::Serverless::Function
    Properties:
      Description: >-
        Used in CloudFormation to zip up and upload the repository to the
        Specified S3 Bucket
      Handler: uploader.lambda_handler
      MemorySize: 128
      Policies:
        - Version: '2012-10-17'
          Statement:
            - Effect: Allow
              Action:
                - s3:PutObject
              Resource:
                - !Sub 'arn:${AWS::Partition}:s3:::${pDeploymentAssetsBucket}/*'
            - Effect: Allow
              Action:
                - logs:CreateLogGroup
                - logs:CreateLogStream
                - logs:PutLogEvents
              Resource: "*"
      Runtime: python3.8
      CodeUri: CodeCommit/Lambda/Code/Uploader
      Layers:
        - !Ref rCodeCommitCustomResourceLayer
      Timeout: 30
      Tags:
        Name: !Sub '${pProjectName}-repo-upload-lambda'

Hardcoded partition in resulting managed policy arn

Properties": {
        "AssumeRolePolicyDocument": {
            "Statement": [
                {
                    "Action": [
                        "sts:AssumeRole"
                    ],
                    "Effect": "Allow",
                    "Principal": {
                        "Service": [
                            "lambda.amazonaws.com"
                        ]
                    }
                }
            ],
            "Version": "2012-10-17"
        },
        "ManagedPolicyArns": [
            "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
        ],
        "Policies": [
            {
                "PolicyDocument": {
                    "Statement": [
                        {
                            "Action": [
                                "s3:PutObject"
                            ],
                            "Effect": "Allow",
                            "Resource": [
                                {
                                    "Fn::Sub": "arn:${AWS::Partition}:s3:::${pDeploymentAssetsBucket}/*"
                                }
                            ]
                        },
                        {
                            "Action": [
                                "logs:CreateLogGroup",
                                "logs:CreateLogStream",
                                "logs:PutLogEvents"
                            ],
                            "Effect": "Allow",
                            "Resource": "*"
                        }
                    ],
                    "Version": "2012-10-17"
                },
                "PolicyName": "rUploadRepoRolePolicy0"
            }
        ],
        "Tags": [
            {
                "Key": "lambda:createdBy",
                "Value": "SAM"
            },
            {
                "Key": "Name",
                "Value": {
                    "Fn::Sub": "${pProjectName}-repo-upload-lambda"
                }
            }
        ]
    },
    "Type": "AWS::IAM::Role"
}


{
    "Properties": {
        "Action": "lambda:InvokeFunction",
        "FunctionName": {
            "Ref": "rCfnNagNotficationLambda"
        },
        "Principal": "logs.amazonaws.com",
        "SourceArn": {
            "Fn::Sub": [
                "arn:aws:logs:${AWS::Region}:${AWS::AccountId}:log-group:${__LogGroupName__}:*",
                {
                    "__LogGroupName__": {
                        "Fn::GetAtt": [
                            "rDevelopPipeline",
                            "Outputs",
                            "oCfnNagLogGroupName"
                        ]
                    }
                }
            ]
        }
    },
    "Type": "AWS::Lambda::Permission"
}
  • Feature request:
    • Replace that Partition that is generated with ${AWS::Partition} , this way custom checks can be made on whether or not templates will work cross Partition
@PatMyron
Copy link
Contributor

PatMyron commented May 9, 2020

actually been meaning to write a similar rule


think the Linter's just running the AWS::Serverless transform itself though

do you not also experience these hardcoded partitions being generated outside of the Linter?

I'd also check which version of aws-sam-translator you have installed and see if upgrading helps:

pip3 show aws-sam-translator
pip3 install aws-sam-translator --upgrade

@dontirun
Copy link
Contributor Author

dontirun commented May 9, 2020

I upgraded

I have generally worked with CloudFormation templates with the SAM Transform directly instead of using the SAM CLI.

actually been meaning to write a similar rule

think the Linter should just be running the AWS::Serverless transform itself though

do you not also experience these hardcoded partitions being generated outside of the Linter?

I'd also check which version of aws-sam-translator you have installed and see if upgrading helps:

pip3 show aws-sam-translator
pip3 install aws-sam-translator --upgrade

I upgraded my version of the aws-sam-translator, but unfortunately that did not solve the issue.

I have generally worked with CloudFormation templates with SAM resources specified directly in those templates packaging and deploying them using the CloudFormation package and deploy CLI
commands. The resulting templates from package still have the SAM resources in them, so I assume CloudFormation does the major changes behind the scenes where those resources are generated.

I do know that the SAM spec does work across partitions, so perhaps CFn does the correct resource selection behind the scenes.

Is there a way to run linting before the linter runs the transform? Specifically for checks for harcoded partitions (or perhaps ARN checks in general)?

@PatMyron
Copy link
Contributor

PatMyron commented May 9, 2020

Console's template tab has a View processed template toggle or via the CLI:

aws cloudformation get-template --template-stage Processed --stack-name $NAME


If processed templates contain generated hardcoded partitions, the AWS::Serverless transform itself should fix that

@dontirun
Copy link
Contributor Author

I checked the processed template and you are correct, this needs to be fixed in the transform itself.

I raised an issue

Is it possible to create a rule that Warns normally, but Errors if the --regions flag is used?

@jfuss
Copy link
Contributor

jfuss commented May 12, 2020

@PatMyron Why does cfn-lint run on the transformed template? Shouldn't this only be run on the source template? I ask because SAM is partition aware but we (in some cases) explicitly set the partition. Which will create false positives. Not sure I understand why cfn-lint does this validation on the transformed template? Does cfn-lint do this for all transforms?

@PatMyron
Copy link
Contributor

PatMyron commented May 12, 2020

Does cfn-lint do this for all transforms?

currently just the AWS::Serverless Transform

the Linter doesn't require networking or credentials to run, which other transforms require

@jfuss
Copy link
Contributor

jfuss commented May 12, 2020

Hmm.. So SAM is the only one that is validated based on the transformed template. How do we address that? I really don't think you guys should be validating on what we do under the hood for customers, that will lead to confusion on how a customer can fix some 'issue' with the generated template.

@PatMyron
Copy link
Contributor

PatMyron commented May 12, 2020

This has come up before

Running the AWS::Serverless Transform has some value, but definitely agree that customers shouldn't feel blocked on generated code they can't control

@kddejong
Copy link
Contributor

My ideal solution @jfuss would be to have a SAM CloudFormation spec file we could use to validate the templates. We need to transform for SAM since the transformed template is the only thing that would make sense to the spec files that exist.
Doing this for other Transforms has come up before but there isn’t as easy of a way to do it at scale as there is with SAM.

I agree it does cause some confusion. The other option would be to disable linting on SAM templates (or just lint them as is... which could cause a lot more false positives).

@jlhood
Copy link

jlhood commented May 13, 2020

+1 to @kddejong's preference for SAM to provide a spec cfn-lint can use rather than running the SAM translator library on the template before processing with cfn-lint. There's a SAM issue for this: aws/serverless-application-model#1133

@jfuss
Copy link
Contributor

jfuss commented May 13, 2020

Yup. I know this has come up in (many) discussions and I think we had something small done in this space but I don't think it ever got completed.

This will help many tools out (cfn lint, cdk, sam cli, etc), so there is value to trying to get this prioritized from our side.

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

5 participants