aws-solutions / aws-control-tower-customizations

The Customizations for AWS Control Tower solution combines AWS Control Tower and other highly-available, trusted AWS services to help customers more quickly set up a secure, multi-account AWS environment using AWS best practices.
https://docs.aws.amazon.com/controltower/latest/userguide/cfct-overview.html
Apache License 2.0
355 stars 205 forks source link

cfn_nag v0.7.2 fails on templates for lambdas that are packaged as containers #115

Open lots2learn opened 2 years ago

lots2learn commented 2 years ago

We have a lambda packaged as a container image we want to deploy using CfCT. The Cloudformation template itself is valid and a current version of cfn_nag (v0.8.9) finds no failures. In the CfCT pipeline the validation phase fails because cfn_nag v0.7.2 that is used shows a Failure when Handler and Runtime are not defined.

The error message we get using cfn_nag v0.7.2 is:

Basic CloudFormation syntax error:[#<Kwalify::ValidationError: [/Resources/LambdaFunction/Properties] key 'Handler:' is required.>, #<Kwalify::ValidationError: [/Resources/LambdaFunction/Properties] key 'Runtime:' is required.>]

The newest versions of cfn_nag do allow for the fact that lambdas can be packaged as containers.

I kindly request to update the cfn_nag package used version in CfCT.

Best regards, Marcel

balltrev commented 2 years ago

Thanks for bringing this up @lots2learn, I've gone ahead and created a backlog item with the team to address this

lots2learn commented 2 years ago

Thanks @balltrev. You get my +1 of removing cfn_nag from the pipeline validation stage. Additional to using cfn_nag before committing, we've written a script that does all kinds of other validations before we commit and kick off the pipeline. This includes checking if all resource files exist, target OUs, Accounts and regions are valid and if all parameters that are mentioned in the manifest actually exist in the resource templates. So lots of overlap of what is done in the pipeline as well, but when there are issues we catch them sooner.

bfbenf commented 8 months ago

This issue is also impacting myself from creating Container lambdas due to the cfn-nag version being out of date. Any updates on this?

dicknetherlands commented 2 months ago

cfn_nag appears to be abandoned by its authors. There are multiple issues with it that prevent CfCT from deploying stacks using common CloudFormation constructs (Fn::ForEach, etc.) that show no sign of being resolved. Could it be removed entirely from CfCT?

sftim commented 1 week ago

If it's removed, what should replace it? “Nothing” is a viable option I think.