Add Microsoft Azure ARM as an IaC Provider #736

gauravgahlot · 2021-05-05T14:04:18Z

The PR adds Microsoft Azure ARM as an IaC Provider for terrascan. Here is a quick run:

➜ terrascan scan -h
...
  -i, --iac-type string           iac type (arm, helm, k8s, kustomize, terraform, tfplan)
      --iac-version string        iac version (arm: v1, helm: v3, k8s: v1, kustomize: v3, terraform: v12, v13, v14, tfplan: v1)
  -p, --policy-path stringArray   policy path directory
...

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

➜ terrascan scan -i arm

Violation Details -
	Description    :	Ensure that logging for Azure KeyVault is 'Enabled'
	File           :	/home/gg/go/src/github.com/infracloudio/mapper-test-files/arm/key-vault/azuredeploy.json
	Line           :	1
	Severity       :	HIGH
	-----------------------------------------------------------------------

Scan Summary -
	File/Folder         :	/home/gg/go/src/github.com/infracloudio/mapper-test-files/arm/key-vault
	IaC Type            :	arm
	Scanned At          :	2021-05-05 14:03:11.701178176 +0000 UTC
	Policies Validated  :	562
	Violated Policies   :	1
	Low                 :	0
	Medium              :	0
	High                :	1

codecov · 2021-05-13T17:37:33Z

Codecov Report

Merging #736 (49499d0) into master (b8fda7c) will increase coverage by 0.67%.
The diff coverage is 83.79%.

@@            Coverage Diff             @@
##           master     #736      +/-   ##
==========================================
+ Coverage   78.41%   79.08%   +0.67%     
==========================================
  Files         168      211      +43     
  Lines        4470     5117     +647     
==========================================
+ Hits         3505     4047     +542     
- Misses        741      826      +85     
- Partials      224      244      +20

Impacted Files	Coverage Δ
...mapper/iac-providers/arm/config/auditing-policy.go	`0.00% <0.00%> (ø)`
.../mapper/iac-providers/arm/config/patch-schedule.go	`0.00% <0.00%> (ø)`
...c-providers/arm/config/postgresql-configuration.go	`0.00% <0.00%> (ø)`
...er/iac-providers/arm/config/redis-firewall-rule.go	`0.00% <0.00%> (ø)`
...pper/iac-providers/arm/config/storage-container.go	`0.00% <0.00%> (ø)`
...c-providers/arm/config/network-watcher-flow-log.go	`61.11% <61.11%> (ø)`
pkg/iac-providers/arm/v1/load-dir.go	`67.27% <67.27%> (ø)`
pkg/iac-providers/arm/v1/load-file.go	`72.05% <72.05%> (ø)`
pkg/mapper/iac-providers/arm/functions/to-lower.go	`80.00% <80.00%> (ø)`
.../mapper/iac-providers/arm/functions/resource-id.go	`83.33% <83.33%> (ø)`
... and 78 more

kanchwala-yusuf · 2021-05-24T14:36:42Z

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

+	for fileDir, files := range fileMap {
+		for i := range files {
+			// continue if file is a *.parameters.json or metadata.json
+			if isParametersFile(*files[i]) || isMetadataFile(*files[i]) {


Would it make sense to have a nil pointer check before accessing *files[i]?

This should be okay as long as range is generating it

kanchwala-yusuf · 2021-05-24T16:01:19Z

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

+
+func (a *ARMV1) hasValidParametersFile(i int, fileDir string, files []*string) bool {
+	f := strings.TrimSuffix(*files[i], filepath.Ext(*files[i]))
+	for n := range files {


Can we get rid of this loop here. If we know the file name f+".parameters.json", why loop over all the files?

This has been fixed

kanchwala-yusuf · 2021-05-24T16:31:26Z

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

+
+const errFileLoad = "error while loading iac files"
+
+func (a *ARMV1) hasValidParametersFile(i int, fileDir string, files []*string) bool {


Is there not be a need to pass i. Can we try passing the filename directly?

kanchwala-yusuf · 2021-05-24T17:24:22Z

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

+	return false
+}
+
+func extractParameterValues(params map[string]interface{}) (map[string]interface{}, error) {


Can we associate this function with ARMV1 struct? Seems specific to ARM feature

kanchwala-yusuf · 2021-05-25T10:54:05Z

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

+			continue
+		}
+
+		if strings.EqualFold(*files[n], f+".parameters.json") {


Can we convert ".parameters.json" into a const, similar to JSONExtension?

kanchwala-yusuf · 2021-05-25T10:54:42Z

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

+}
+
+func (*ARMV1) getFileType(file string) string {
+	if strings.HasSuffix(file, JSONExtension) {


Wondering if filepath.Ext() would be a better way of checking file extensions?

kanchwala-yusuf · 2021-05-25T11:00:06Z

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

+		zap.S().Debug("failed to load file", zap.String("file", absFilePath))
+		return allResourcesConfig, err
+	}
+	doc := iacDocuments[0]


Do you think, it would be better to check the len(iacDocuments) > 0? There is a possibility of a panic if iacDocuments is empty?

kanchwala-yusuf · 2021-05-25T14:28:18Z

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

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


Can we please add some unit test cases?

kanchwala-yusuf · 2021-05-25T14:30:46Z

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

+			wantErr: fmt.Errorf("no directories found for path ./testdata/testfile"),
+		},
+		{
+			name:    "load invalid config dir",


Wondering, should the wantErr=nil? From the test case name it seems that we are testing invalid config dir

patilpankaj212 · 2021-06-02T05:49:10Z

pkg/mapper/mapper_test.go

+			if err != nil {
+				t.Error(err)
+			}
+			_, err = m.Map(doc, params)


apart from asserting on the error, we should also verify that the output we get from Map method is expected. It may happen that the test file gets modified or Map is modified which may result in a different output.

patilpankaj212 · 2021-06-02T05:50:28Z

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

+	}{
+		{
+			name:    "empty config",
+			dirPath: "./testdata/testfile",


we should use filepath.Join() here

patilpankaj212 · 2021-06-02T05:50:41Z

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

+			name:    "empty config",
+			dirPath: "./testdata/testfile",
+			armv1:   ARMV1{},
+			wantErr: fmt.Errorf("no directories found for path ./testdata/testfile"),


we should use filepath.Join() here

patilpankaj212 · 2021-06-02T05:50:57Z

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

+		},
+		{
+			name:    "key-vault",
+			dirPath: "./testdata/key-vault",


we should use filepath.Join() here

patilpankaj212 · 2021-06-02T05:51:18Z

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

+			name:    "invalid dirPath",
+			dirPath: "not-there",
+			armv1:   ARMV1{},
+			wantErr: &os.PathError{Err: syscall.ENOENT, Op: "lstat", Path: "not-there"},


this error check would fail on windows platform.

patilpankaj212 · 2021-06-02T06:01:32Z

pkg/mapper/iac-providers/arm/types/template_test.go

+	}
+
+	var tmp Template
+	err = json.Unmarshal(data, &tmp)


if possible, we should also verify the unmarshalled fields.

Will do the changed similar to cft, in upcoming PR, because ti will also need changes in KaiMonkey

patilpankaj212 · 2021-06-02T06:13:09Z

pkg/iac-providers/arm/v1/types.go

+
+// ARMV1 struct implements the IacProvider interface
+type ARMV1 struct {
+	templateParameters map[string]interface{}


Associate a multierror with the struct. Its job would be collecting all the file loading related errors. Please refer to other iac provider implementations.

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

…ests

sonarcloud · 2021-06-29T06:37:50Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
0.0% Duplication

* Add Microsoft Azure ARM as an IaC Provider (#736) * Add support for linked templates

gauravgahlot force-pushed the iac-provider-arm branch from f9cdb1d to c78d031 Compare May 5, 2021 14:11

amirbenv requested review from kanchwala-yusuf and jlk May 5, 2021 15:12

gauravgahlot force-pushed the iac-provider-arm branch 5 times, most recently from 7594d9f to b01edba Compare May 11, 2021 07:42

sigmabaryon force-pushed the iac-provider-arm branch 2 times, most recently from b909941 to 4f6adf2 Compare May 12, 2021 09:50

gauravgahlot force-pushed the iac-provider-arm branch 5 times, most recently from c099d11 to af92d33 Compare May 13, 2021 17:29

gauravgahlot force-pushed the iac-provider-arm branch 3 times, most recently from bda2654 to fd339ad Compare May 15, 2021 12:02

amirbenv assigned kanchwala-yusuf and patilpankaj212 May 19, 2021

kanchwala-yusuf suggested changes May 28, 2021

View reviewed changes

gauravgahlot force-pushed the iac-provider-arm branch from fd339ad to a37976e Compare June 1, 2021 12:10

patilpankaj212 suggested changes Jun 2, 2021

View reviewed changes

patilpankaj212 reviewed Jun 2, 2021

View reviewed changes

gauravgahlot force-pushed the iac-provider-arm branch from 11a3873 to d8a6831 Compare June 2, 2021 06:40

sigmabaryon force-pushed the iac-provider-arm branch 3 times, most recently from b83d49c to 5800881 Compare June 28, 2021 16:22

gauravgahlot and others added 11 commits June 28, 2021 21:55

register arm as an IaC provider

00cf9ff

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

implement IacProvider interface

c12c509

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

arm mapper for terrascan

1fb07bb

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

mapper helper functions

a102e33

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

config for each mapped resource

068e8c7

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

mapper test data

6182b57

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

refactored the mapper API

9349ecc

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

skip resources

7290bc5

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

Add SkipResources, Change api

8749a3b

Add support for linked templates, resource de-duplication, add unit t…

a35e7d0

…ests

Change fields according to golint

e70490e

sigmabaryon force-pushed the iac-provider-arm branch 5 times, most recently from c35a4b1 to 3f4287f Compare June 29, 2021 06:36

Add safe type conversions and parameter resolution

49499d0

sigmabaryon force-pushed the iac-provider-arm branch from 3f4287f to 49499d0 Compare June 29, 2021 06:36

patilpankaj212 approved these changes Jun 30, 2021

View reviewed changes

kanchwala-yusuf approved these changes Jun 30, 2021

View reviewed changes

kanchwala-yusuf merged commit 9e962b9 into tenable:master Jun 30, 2021

gauravgahlot deleted the iac-provider-arm branch June 30, 2021 11:37

sigmabaryon pushed a commit to infracloudio/terrascan that referenced this pull request Jun 30, 2021

Add Microsoft Azure ARM as an IaC Provider (tenable#736)

aabd049

sigmabaryon pushed a commit to infracloudio/terrascan that referenced this pull request Jun 30, 2021

Add Microsoft Azure ARM as an IaC Provider (tenable#736)

fa862b1

kanchwala-yusuf pushed a commit that referenced this pull request Jul 22, 2021

Add support for arm linked templates (#903)

35afbe6

* Add Microsoft Azure ARM as an IaC Provider (#736) * Add support for linked templates

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Microsoft Azure ARM as an IaC Provider #736

Add Microsoft Azure ARM as an IaC Provider #736

gauravgahlot commented May 5, 2021 •

edited

Loading

codecov bot commented May 13, 2021 •

edited

Loading

kanchwala-yusuf May 24, 2021

sigmabaryon Jun 28, 2021

kanchwala-yusuf May 24, 2021

sigmabaryon Jun 30, 2021

kanchwala-yusuf May 24, 2021

sigmabaryon Jun 28, 2021

kanchwala-yusuf May 24, 2021

sigmabaryon Jun 28, 2021

kanchwala-yusuf May 25, 2021

sigmabaryon Jun 28, 2021

kanchwala-yusuf May 25, 2021

sigmabaryon Jun 28, 2021

kanchwala-yusuf May 25, 2021

sigmabaryon Jun 28, 2021

kanchwala-yusuf May 25, 2021

sigmabaryon Jun 28, 2021

kanchwala-yusuf May 25, 2021

patilpankaj212 Jun 2, 2021

sigmabaryon Jun 28, 2021

patilpankaj212 Jun 2, 2021

sigmabaryon Jun 28, 2021

patilpankaj212 Jun 2, 2021

sigmabaryon Jun 28, 2021

patilpankaj212 Jun 2, 2021

sigmabaryon Jun 28, 2021

patilpankaj212 Jun 2, 2021

sigmabaryon Jun 28, 2021

patilpankaj212 Jun 2, 2021

sigmabaryon Jun 28, 2021

patilpankaj212 Jun 2, 2021

sigmabaryon Jun 28, 2021

sonarcloud bot commented Jun 29, 2021


		const errFileLoad = "error while loading iac files"

		func (a ARMV1) hasValidParametersFile(i int, fileDir string, files []string) bool {

Add Microsoft Azure ARM as an IaC Provider #736

Add Microsoft Azure ARM as an IaC Provider #736

Conversation

gauravgahlot commented May 5, 2021 • edited Loading

codecov bot commented May 13, 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

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

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

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

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

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

Choose a reason for hiding this comment

sonarcloud bot commented Jun 29, 2021

gauravgahlot commented May 5, 2021 •

edited

Loading

codecov bot commented May 13, 2021 •

edited

Loading