Add AWS CFT as an IaC Provider #815

mahendrabagul · 2021-05-26T07:33:00Z

The PR adds AWS CFT as an IaC Provider for terrascan. Here is a quick run:

➜ terrascan scan -h
...
 -i, --iac-type string           iac type (cft, helm, k8s, kustomize, terraform, tfplan)
     --iac-version string        iac version (cft: v1, helm: v3, k8s: v1, kustomize: v3, terraform: v12, v13, v14, tfplan: v1)
...

This allows terrascan users to validate their CF templates against the existing policies for AWS. For example:

➜ terrascan scan -i cft

Violation Details -
	Description    :	Ensure Neptune Cluster is Encrypted
	File           :	/home/mahendrabagul/DevEnv/Accurics/mapper-test-files/cft/neptune-cluster/deploy.json
	Line           :	1
	Severity       :	MEDIUM
	-----------------------------------------------------------------------
Scan Summary -
	File/Folder         :	/home/mahendrabagul/DevEnv/Accurics/mapper-test-files/cft/neptune-cluster
	IaC Type            :	cft
	Scanned At          :	2021-05-26 06:42:45.454097592 +0000 UTC
	Policies Validated  :	607
	Violated Policies   :	1
	Low                 :	0
	Medium              :	1
	High                :	0

mahendrabagul · 2021-05-26T07:34:47Z

@amirbenv Kindly have a look at this PR.

patilpankaj212

Hi @mahendrabagul, @sigmabaryon,
Please add headers missing in go files for config mappers.

codecov · 2021-05-28T07:40:49Z

Codecov Report

Merging #815 (2874061) into master (971845a) will increase coverage by 3.27%.
The diff coverage is 91.15%.

@@            Coverage Diff             @@
##           master     #815      +/-   ##
==========================================
+ Coverage   74.95%   78.22%   +3.27%     
==========================================
  Files         113      162      +49     
  Lines        3466     4354     +888     
==========================================
+ Hits         2598     3406     +808     
- Misses        676      734      +58     
- Partials      192      214      +22

Impacted Files	Coverage Δ
.../mapper/iac-providers/cft/config/iam-access-key.go	`0.00% <0.00%> (ø)`
pkg/mapper/iac-providers/cft/config/cloudtrail.go	`50.00% <50.00%> (ø)`
pkg/iac-providers/cft/v1/load-file.go	`56.14% <56.14%> (ø)`
...ers/cft/config/kinesis-firehose-delivery-stream.go	`71.42% <71.42%> (ø)`
.../mapper/iac-providers/cft/config/kinesis-stream.go	`75.00% <75.00%> (ø)`
pkg/mapper/iac-providers/cft/config/mq-broker.go	`75.00% <75.00%> (ø)`
pkg/mapper/mapper.go	`75.00% <75.00%> (ø)`
.../mapper/iac-providers/cft/config/ecr-repository.go	`78.94% <78.94%> (ø)`
...r/iac-providers/cft/config/cloudformation-stack.go	`81.81% <81.81%> (ø)`
...pper/iac-providers/cft/config/api-gateway-stage.go	`84.37% <84.37%> (ø)`
... and 91 more

patilpankaj212 · 2021-05-29T10:30:55Z

pkg/iac-providers/cft/v1/load-file.go

+	case YAMLExtension:
+		fallthrough
+	case YAMLExtension2:
+		iacDocuments, err = utils.LoadYAML(absFilePath)


utils.LoadYAML is not very useful here, because the number of iac documents is always going to be 1.
Also, using this function is adding extra code as it removes the intrinsic tags.

I agree with @patilpankaj212. It is not necessary to use utils.LoadYAML(). In this particular case. The file data is being parsed multiple number of times which redundant and unnecessary.

We can implement our own loader and optimize.

patilpankaj212 · 2021-05-29T10:32:13Z

pkg/iac-providers/cft/v1/load-file.go

+		// intrinsic tags for cloudformation templates
+		// (!Ref, !Fn::<> etc are removed and resolved to a string
+		// which disables parameter resolution by goformation)
+		if fileExt != JSONExtension {


we should be able to avoid this step by using a different yaml loader.

pkg/iac-providers/cft/v1/load-file.go

patilpankaj212 · 2021-05-29T10:35:28Z

pkg/iac-providers/cft/v1/load-file_test.go

+		typeOnly bool
+		want     output.AllResourceConfigs
+		wantErr  error
+	}{}


please add unit tests.

pkg/mapper/mapper_test.go

pkg/iac-providers/cft/v1/load-file.go

patilpankaj212 · 2021-05-31T10:27:34Z

pkg/mapper/iac-providers/cft/mapper.go

+// Map transforms the provider specific template to terrascan native format.
+func (m cftMapper) Map(doc *utils.IacDocument, params ...map[string]interface{}) (output.AllResourceConfigs, error) {
+	allRC := make(map[string][]output.ResourceConfig)
+	t, err := extractTemplate(doc)


any unmarshalling errors by the goformation library are also logged to console. eg:
ERROR: json: cannot unmarshal string into Go struct field SecurityGroup_Ingress.Properties.SecurityGroupIngress.FromPort of type int
we should see how these logs can be suppressed.

patilpankaj212 · 2021-06-01T11:25:21Z

CFT supports modules https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/modules.html
Is it possible to support modules?

pkg/mapper/iac-providers/cft/config/security-group.go

pkg/mapper/iac-providers/cft/config/secretsmanager-secret.go

pkg/mapper/iac-providers/cft/config/redshift-cluster.go

pkg/mapper/iac-providers/cft/config/rds-cluster.go

patilpankaj212 · 2021-06-01T12:09:04Z

pkg/mapper/iac-providers/cft/config/iam-role.go

+
+	// aws_iam_role_policy as a SubResource
+	// multiple Policies can be defined for a resource in cft
+	if r.Policies != nil {


is it required to create a separate resource for policies when the policy could be part of iam role properties?

It is required because Policy is just a property in CloudFormation but a resource in terraform. There are many other resources where this is the case.

Thanks @sigmabaryon. In that case should it also be part of the respective cft resource properties? What is your opinion on this @kanchwala-yusuf ?

Co-authored-by: Mahendra Bagul <bagulm123@gmail.com>

pkg/iac-providers/cft/v1/load-dir.go

patilpankaj212 · 2021-06-02T10:54:37Z

pkg/iac-providers/cft/v1/load-dir_test.go

+	for i := 1; i < len(dirList); i++ {
+		dir := dirList[i]
+		t.Run(dir, func(t *testing.T) {
+			_, gotErr := cftv1.LoadIacDir(dir, false)


we should be verifying the output of LoadDir to an expected output.

patilpankaj212 · 2021-06-02T10:55:24Z

pkg/iac-providers/cft/v1/load-dir_test.go

+		dir := dirList[i]
+		t.Run(dir, func(t *testing.T) {
+			_, gotErr := cftv1.LoadIacDir(dir, false)
+			_, ok := gotErr.(*multierror.Error)


this assertion doesn't seem to be correct, we are just expecting the type of error to be multi-error. we should be asserting on expected errors (if any)

patilpankaj212 · 2021-06-02T10:55:54Z

pkg/iac-providers/cft/v1/load-dir_test.go

+	}
+}
+
+func TestCFTMapper(t *testing.T) {


I think the right place for mapper tests is in mapper package.

sonarcloud · 2021-06-02T13:35:30Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
0.0% Duplication

sigmabaryon force-pushed the iac-provider-cft branch 2 times, most recently from 0c29f46 to 166c63b Compare May 27, 2021 14:49

patilpankaj212 reviewed May 28, 2021

View reviewed changes

patilpankaj212 suggested changes May 29, 2021

View reviewed changes

patilpankaj212 reviewed May 31, 2021

View reviewed changes

pkg/iac-providers/cft/v1/load-file.go Show resolved Hide resolved

patilpankaj212 reviewed May 31, 2021

View reviewed changes

patilpankaj212 reviewed Jun 1, 2021

View reviewed changes

patilpankaj212 assigned patilpankaj212 and kanchwala-yusuf Jun 1, 2021

patilpankaj212 added this to In progress in Main Board via automation Jun 1, 2021

patilpankaj212 moved this from In progress to Review in progress in Main Board Jun 1, 2021

mahendrabagul and others added 3 commits June 2, 2021 15:50

added cft structure

77ff645

fixed elasticache-cluster rego policy

33666f8

added cft mapper

acff9b2

Co-authored-by: Mahendra Bagul <bagulm123@gmail.com>

mahendrabagul force-pushed the iac-provider-cft branch from 26b76cc to 8cebb68 Compare June 2, 2021 10:22

patilpankaj212 suggested changes Jun 2, 2021

View reviewed changes

mahendrabagul force-pushed the iac-provider-cft branch from 0898808 to 6292a6d Compare June 2, 2021 11:59

patilpankaj212 and others added 6 commits June 2, 2021 17:37

fix failing tests

215ff25

added copyright headers

8bec6ee

added kai monkey tests for CFT

eaf1e76

Add support for skipping violations

486b18f

Refactor and add unit tests

efef559

read .txt and .template cft files

74321df

mahendrabagul force-pushed the iac-provider-cft branch 2 times, most recently from 5efeac3 to 84f2681 Compare June 2, 2021 13:14

added relative source path for cft

2874061

mahendrabagul force-pushed the iac-provider-cft branch from 84f2681 to 2874061 Compare June 2, 2021 13:34

kanchwala-yusuf approved these changes Jun 2, 2021

View reviewed changes

patilpankaj212 approved these changes Jun 2, 2021

View reviewed changes

patilpankaj212 merged commit 95aba12 into tenable:master Jun 2, 2021

Main Board automation moved this from Review in progress to Done Jun 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AWS CFT as an IaC Provider #815

Add AWS CFT as an IaC Provider #815

mahendrabagul commented May 26, 2021

mahendrabagul commented May 26, 2021

patilpankaj212 left a comment

codecov bot commented May 28, 2021 •

edited

Loading

patilpankaj212 May 29, 2021

kanchwala-yusuf May 31, 2021

patilpankaj212 May 29, 2021

patilpankaj212 May 29, 2021

patilpankaj212 May 31, 2021

patilpankaj212 commented Jun 1, 2021

patilpankaj212 Jun 1, 2021

sigmabaryon Jun 1, 2021

patilpankaj212 Jun 1, 2021 •

edited

Loading

patilpankaj212 Jun 2, 2021

patilpankaj212 Jun 2, 2021

patilpankaj212 Jun 2, 2021

sonarcloud bot commented Jun 2, 2021

Add AWS CFT as an IaC Provider #815

Add AWS CFT as an IaC Provider #815

Conversation

mahendrabagul commented May 26, 2021

mahendrabagul commented May 26, 2021

patilpankaj212 left a comment

Choose a reason for hiding this comment

codecov bot commented May 28, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patilpankaj212 commented Jun 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patilpankaj212 Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jun 2, 2021

codecov bot commented May 28, 2021 •

edited

Loading

patilpankaj212 Jun 1, 2021 •

edited

Loading