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

Allow suppression of cfn_nag rules in manifest.yaml #119

Open willdady opened 2 years ago

willdady commented 2 years ago

It should be possible to suppress cfn_nag rules on a per-resource basis for stack-set deployments.

I am trying to add the CDK bootstrap to accounts within my organisation. I retrieve the bootstrap template from the AWS CLI via cdk bootstrap --show-template and save this as a file in my repo. This is then linked as a stack_set resource in my manifest.yaml.

Attempting to deploy this as-is causes the deployment to fail as cfn_nag raises issues. It fails against rules F19 and F76.

To get around this issue I have to edit the template to adding rules_to_suppress where appropriate. As this is a third-party template, generated from an official AWS tool no-less, I really don't want to have to edit the template to get around this issue.

adam-daily commented 2 years ago

Hey Will, thanks for reaching out. We're actually considering removing cfn-nag outright from this step, with the thinking that it's not really the appropriate place to check for that. If we go that route, would that work for your use case?

willdady commented 2 years ago

@adam-daily 100% yes.

If not completely removed perhaps even a flag that can be set in the manifest.yaml? The flag could be a global opt-out or even an opt-out per-resource.

I think it's reasonable for CfCT to be less-opinionated on this. Leave it the responsibility of consumers of CfCT to provide valid templates.

greenaussie commented 2 years ago

There should be an early build step so the pipeline fails as fast as possible when there is a problem.

  1. cfn_nag
  2. cfn-lint (overlaps with cfn_nag but not the same)
  3. aws cloudformation validate-template

I guess different orgs might have different requirements, so levering something like pre-commit could allow .pre-commit.yaml to be owned by the owning enterprise.

Precommit isn't only a pre-commit hook, but the same hooks would work locally for the CfCT operator and thus fewer problems would ever reach the pipeline itself.

PeterBengtson commented 1 year ago

Just get rid of it completely. The present solution is over-engineered.

dlahn commented 1 year ago

Any update on removing cfn-nag?

PeterBengtson commented 1 year ago

Yes, please. This issue alone bars the use of this tool.

willdady commented 1 year ago

@adam-daily Can we get an update on this? Our team finds cfn_nag extremely annoying to deal with as it often results in a lot of trial and error when doing deployments which are already slow enough with CfCT. There should be simple way to opt out of using it (or just remove it from the project entirely). Linting should not be a concern of this project.

PeterBengtson commented 1 year ago

yes, please remove cfn_nag entirely. It is of no use whatsoever. Currently, we only do SCP deployments using CfCT, as the StackSet functionality is, for all intents and purposes, completely useless.

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?