From 8080e17b947b0d43b277fe6c035acba5a0fc2810 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Fri, 11 Dec 2020 19:07:51 +0530 Subject: [PATCH 01/17] human readable output for terrascan --- pkg/cli/register.go | 2 +- pkg/cli/root.go | 2 +- pkg/cli/run.go | 11 ++++- pkg/cli/run_test.go | 2 +- pkg/cli/scan.go | 6 ++- pkg/policy/opa/engine.go | 5 +++ pkg/results/types.go | 47 +++++++++++++++++++++ pkg/runtime/executor.go | 10 +++++ pkg/writer/humanReadable.go | 83 +++++++++++++++++++++++++++++++++++++ 9 files changed, 162 insertions(+), 6 deletions(-) create mode 100644 pkg/writer/humanReadable.go diff --git a/pkg/cli/register.go b/pkg/cli/register.go index c1c847a83..81fd7fc3a 100644 --- a/pkg/cli/register.go +++ b/pkg/cli/register.go @@ -56,7 +56,7 @@ func setDefaultCommandIfNonePresent() { func Execute() { rootCmd.PersistentFlags().StringVarP(&LogLevel, "log-level", "l", "info", "log level (debug, info, warn, error, panic, fatal)") rootCmd.PersistentFlags().StringVarP(&LogType, "log-type", "x", "console", "log output type (console, json)") - rootCmd.PersistentFlags().StringVarP(&OutputType, "output", "o", "yaml", "output type (json, yaml, xml)") + rootCmd.PersistentFlags().StringVarP(&OutputType, "output", "o", "human", "output type (human, json, yaml, xml)") rootCmd.PersistentFlags().StringVarP(&ConfigFile, "config-path", "c", "", "config file path") // Function to execute before processing commands diff --git a/pkg/cli/root.go b/pkg/cli/root.go index 814daed43..f7872e995 100644 --- a/pkg/cli/root.go +++ b/pkg/cli/root.go @@ -25,7 +25,7 @@ var ( LogLevel string // LogType Logging output type (console, json) LogType string - // OutputType Violation output type (text, json, yaml, xml) + // OutputType Violation output type (human, json, yaml, xml) OutputType string // ConfigFile Config file path ConfigFile string diff --git a/pkg/cli/run.go b/pkg/cli/run.go index 14c298ee0..d939e5c36 100644 --- a/pkg/cli/run.go +++ b/pkg/cli/run.go @@ -20,8 +20,10 @@ import ( "flag" "os" "path/filepath" + "strings" "github.com/accurics/terrascan/pkg/downloader" + result "github.com/accurics/terrascan/pkg/results" "github.com/accurics/terrascan/pkg/runtime" "github.com/accurics/terrascan/pkg/utils" "github.com/accurics/terrascan/pkg/writer" @@ -31,7 +33,7 @@ import ( // Run executes terrascan in CLI mode func Run(iacType, iacVersion string, cloudType []string, iacFilePath, iacDirPath, configFile string, policyPath []string, - format, remoteType, remoteURL string, configOnly, useColors bool) { + format, remoteType, remoteURL string, configOnly, useColors, verbose bool) { // temp dir to download the remote repo tempDir := filepath.Join(os.TempDir(), utils.GenRandomString(6)) @@ -69,7 +71,12 @@ func Run(iacType, iacVersion string, cloudType []string, if configOnly { writer.Write(format, results.ResourceConfig, outputWriter) } else { - writer.Write(format, results.Violations, outputWriter) + if strings.EqualFold(format, "human") { + defaultScanResult := result.NewDefaultScanResult(iacType, iacFilePath, iacDirPath, executor.GetTotalPolicyCount(), verbose, *results.Violations.ViolationStore) + writer.Write(format, defaultScanResult, outputWriter) + } else { + writer.Write(format, results.Violations, outputWriter) + } } if results.Violations.ViolationStore.Count.TotalCount != 0 && flag.Lookup("test.v") == nil { diff --git a/pkg/cli/run_test.go b/pkg/cli/run_test.go index dd327e30b..e9082ccb7 100644 --- a/pkg/cli/run_test.go +++ b/pkg/cli/run_test.go @@ -60,7 +60,7 @@ func TestRun(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - Run(tt.iacType, tt.iacVersion, tt.cloudType, tt.iacFilePath, tt.iacDirPath, tt.configFile, []string{}, "", "", "", tt.configOnly, false) + Run(tt.iacType, tt.iacVersion, tt.cloudType, tt.iacFilePath, tt.iacDirPath, tt.configFile, []string{}, "", "", "", tt.configOnly, false, false) }) } } diff --git a/pkg/cli/scan.go b/pkg/cli/scan.go index 478ee7431..54568ac6a 100644 --- a/pkg/cli/scan.go +++ b/pkg/cli/scan.go @@ -60,6 +60,9 @@ var ( // UseColors indicates whether to use color output UseColors bool useColors string // used for flag processing + + // Verbose indicates whether to display all fields in default human readlbe output + Verbose bool ) var scanCmd = &cobra.Command{ @@ -100,7 +103,7 @@ Detect compliance and security violations across Infrastructure as Code to mitig func scan(cmd *cobra.Command, args []string) { zap.S().Debug("running terrascan in cli mode") Run(IacType, IacVersion, PolicyType, IacFilePath, IacDirPath, ConfigFile, - PolicyPath, OutputType, RemoteType, RemoteURL, ConfigOnly, UseColors) + PolicyPath, OutputType, RemoteType, RemoteURL, ConfigOnly, UseColors, Verbose) } func init() { @@ -115,5 +118,6 @@ func init() { scanCmd.Flags().BoolVarP(&ConfigOnly, "config-only", "", false, "will output resource config (should only be used for debugging purposes)") // flag passes a string, but we normalize to bool in PreRun scanCmd.Flags().StringVar(&useColors, "use-colors", "auto", "color output (auto, t, f)") + scanCmd.Flags().BoolVarP(&Verbose, "verbose", "v", false, "will show violations with details (applicable for default output)") RegisterCommand(rootCmd, scanCmd) } diff --git a/pkg/policy/opa/engine.go b/pkg/policy/opa/engine.go index 00bc22f7f..3e10db979 100644 --- a/pkg/policy/opa/engine.go +++ b/pkg/policy/opa/engine.go @@ -392,3 +392,8 @@ func (e *Engine) Evaluate(engineInput policy.EngineInput) (policy.EngineOutput, e.stats.runTime = time.Since(start) return e.results, nil } + +// GetRuleCount will return the ruleCount value +func (e Engine) GetRuleCount() int { + return e.stats.ruleCount +} diff --git a/pkg/results/types.go b/pkg/results/types.go index 0942f76f9..1f0083ddf 100644 --- a/pkg/results/types.go +++ b/pkg/results/types.go @@ -16,6 +16,12 @@ package results +import ( + "time" + + "github.com/accurics/terrascan/pkg/utils" +) + // Violation Contains data for each violation type Violation struct { RuleName string `json:"rule_name" yaml:"rule_name" xml:"rule_name,attr"` @@ -46,6 +52,16 @@ type ViolationStore struct { Count ViolationStats `json:"count" yaml:"count" xml:"count"` } +// DefaultScanResult will hold the default scan summary data +type DefaultScanResult struct { + IacType string + ResourcePath string + Timestamp string + TotalPolicies int + ShowViolationDetails bool + ViolationStore +} + // Add adds two ViolationStores func (vs ViolationStore) Add(extra ViolationStore) ViolationStore { // Just concatenate the slices, since order shouldn't be important @@ -59,3 +75,34 @@ func (vs ViolationStore) Add(extra ViolationStore) ViolationStore { return vs } + +// NewDefaultScanResult will initialize DefaultScanResult +func NewDefaultScanResult(iacType, iacFilePath, iacDirPath string, totalPolicyCount int, verbose bool, violationStore ViolationStore) *DefaultScanResult { + sr := new(DefaultScanResult) + + if iacType == "" { + // the default scan type is terraform + sr.IacType = "terraform" + } else { + sr.IacType = iacType + } + + if iacFilePath != "" { + // can skip the error as the file validation is already done + // while executor is initialized + filePath, _ := utils.GetAbsPath(iacFilePath) + sr.ResourcePath = filePath + } else { + // can skip the error as the directory validation is already done + // while executor is initialized + dirPath, _ := utils.GetAbsPath(iacDirPath) + sr.ResourcePath = dirPath + } + sr.ShowViolationDetails = verbose + sr.TotalPolicies = totalPolicyCount + sr.ViolationStore = violationStore + // set current time as scan time + sr.Timestamp = time.Now().UTC().String() + + return sr +} diff --git a/pkg/runtime/executor.go b/pkg/runtime/executor.go index fd3520676..44101ac78 100644 --- a/pkg/runtime/executor.go +++ b/pkg/runtime/executor.go @@ -131,3 +131,13 @@ func (e *Executor) Execute() (results Output, err error) { // successful return results, nil } + +// GetTotalPolicyCount will return the total count of policies in all engines +func (e Executor) GetTotalPolicyCount() int { + policyCount := 0 + for _, engine := range e.policyEngine { + opaEngine := engine.(*opa.Engine) + policyCount += opaEngine.GetRuleCount() + } + return policyCount +} diff --git a/pkg/writer/humanReadable.go b/pkg/writer/humanReadable.go new file mode 100644 index 000000000..2a3d5494f --- /dev/null +++ b/pkg/writer/humanReadable.go @@ -0,0 +1,83 @@ +/* + Copyright (C) 2020 Accurics, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package writer + +import ( + "bytes" + "fmt" + "io" + "os" + "text/template" +) + +const ( + humanReadbleFormat supportedFormat = "human" + + defaultTemplate string = ` +{{if (gt (len .ViolationStore.Violations) 0) }} +Violation Details - + {{- $showDetails := .ShowViolationDetails}} + {{range $index, $element := .ViolationStore.Violations}} + Description{{"\t"}}:{{"\t"}}{{$element.Description}} + File{{"\t\t"}}:{{"\t"}}{{$element.File}} + Line{{"\t\t"}}:{{"\t"}}{{$element.LineNumber}} + Severity{{"\t"}}:{{"\t"}}{{$element.Severity}} + {{if $showDetails -}} + Rule Name{{"\t"}}:{{"\t"}}{{$element.RuleName}} + Rule ID{{"\t\t"}}:{{"\t"}}{{$element.RuleID}} + Resource Name{{"\t"}}:{{"\t"}}{{$element.ResourceName}} + Resource Type{{"\t"}}:{{"\t"}}{{$element.ResourceType}} + Category{{"\t"}}:{{"\t"}}{{$element.Category}} + {{end}} + ----------------------------------------------------------------------- + {{end}} +{{end}} +Scan Summary - + + File/Folder{{"\t\t"}}:{{"\t"}}{{.ResourcePath}} + IaC Type{{"\t\t"}}:{{"\t"}}{{.IacType}} + Scanned At{{"\t\t"}}:{{"\t"}}{{.Timestamp}} + Policies Validated{{"\t"}}:{{"\t"}}{{.TotalPolicies}} + Violated Policies{{"\t"}}:{{"\t"}}{{.ViolationStore.Count.TotalCount}} + Low{{"\t\t\t"}}:{{"\t"}}{{.ViolationStore.Count.LowCount}} + Medium{{"\t\t\t"}}:{{"\t"}}{{.ViolationStore.Count.MediumCount}} + High{{"\t\t\t"}}:{{"\t"}}{{.ViolationStore.Count.HighCount}} +` +) + +func init() { + RegisterWriter(humanReadbleFormat, HumanReadbleWriter) +} + +// HumanReadbleWriter display scan summary in human readable format +func HumanReadbleWriter(data interface{}, writer io.Writer) error { + tmpl, err := template.New("Report").Parse(defaultTemplate) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + buffer := bytes.Buffer{} + tmpl.Execute(&buffer, data) + + _, err = writer.Write(buffer.Bytes()) + if err != nil { + return err + } + writer.Write([]byte{'\n'}) + return nil +} From c33ebed3678526dc983a9d88a14aa1d3f010f0a6 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Fri, 11 Dec 2020 19:20:35 +0530 Subject: [PATCH 02/17] -support color for default output -change color for 'file' --- pkg/termcolor/colorpatterns.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/termcolor/colorpatterns.go b/pkg/termcolor/colorpatterns.go index 60397e62d..63dc52291 100644 --- a/pkg/termcolor/colorpatterns.go +++ b/pkg/termcolor/colorpatterns.go @@ -3,10 +3,11 @@ package termcolor import ( "encoding/json" "fmt" - "go.uber.org/zap" "io/ioutil" "os" "regexp" + + "go.uber.org/zap" ) var ( @@ -53,17 +54,18 @@ type colorPatternSerialized struct { **/ var defaultColorPatterns = map[FieldSpec]FieldStyle{ - {"description", defaultValuePattern}: {"", "Fg#0c0"}, - {"severity", defaultValuePattern}: {"", "?HIGH=Fg#f00?MEDIUM=Fg#c84?LOW=Fg#cc0"}, - {"resource_name", defaultValuePattern}: {"", "Fg#0ff|Bold"}, - {"resource_type", defaultValuePattern}: {"", "Fg#0cc"}, - {"file", defaultValuePattern}: {"", "Fg#fff|Bold"}, - {"low", `\d+`}: {"Fg#cc0", "Fg#cc0"}, - {"medium", `\d+`}: {"Fg#c84", "Fg#c84"}, - {"high", `\d+`}: {"Fg#f00", "Fg#f00"}, - - {"count", ""}: {"Bg#ccc|Fg#000", ""}, - {"rule_name", defaultValuePattern}: {"Bg#ccc|Fg#000", ""}, + {"[dD]escription", defaultValuePattern}: {"", "Fg#0c0"}, + {"[sS]everity", defaultValuePattern}: {"", "?HIGH=Fg#f00?MEDIUM=Fg#c84?LOW=Fg#cc0"}, + {`[rR]esource[_\s][nN]ame`, defaultValuePattern}: {"", "Fg#0ff|Bold"}, + {`[rR]esource[_\s][tT]ype`, defaultValuePattern}: {"", "Fg#0cc"}, + {"[fF]ile", defaultValuePattern}: {"", "Fg#00768B|Bold"}, + {"[lL]ow", `\d+`}: {"Fg#cc0", "Fg#cc0"}, + {"[mM]edium", `\d+`}: {"Fg#c84", "Fg#c84"}, + {"[hH]igh", `\d+`}: {"Fg#f00", "Fg#f00"}, + {"count", ""}: {"Bg#ccc|Fg#000", ""}, + {`[rR]ule[_\s][nN]ame`, defaultValuePattern}: {"Bg#ccc|Fg#000", ""}, + {"File/Folder", defaultValuePattern}: {"", "Fg#00768B|Bold"}, + {"Policies Validated", defaultValuePattern}: {"Bg#ccc|Fg#000", ""}, } func init() { @@ -131,9 +133,9 @@ func GetColorPatterns() map[*regexp.Regexp]FieldStyle { /* rePtn should process a whole line and have 5 subgroups */ if len(ptn.ValuePattern) == 0 { - rePtn = fmt.Sprintf(`^([-\s]*"?)(%s)("?:\s*?)()(.*?)\s*$`, ptn.KeyPattern) + rePtn = fmt.Sprintf(`^([-\s]*"?)(%s)("?\s*:\s*?)()(.*?)\s*$`, ptn.KeyPattern) } else { - rePtn = fmt.Sprintf(`^([-\s]*"?)(%s)("?: "?)(%s)("?,?)\s*$`, ptn.KeyPattern, ptn.ValuePattern) + rePtn = fmt.Sprintf(`^([-\s]*"?)(%s)("?\s*:\s*"?)(%s)("?,?)\s*$`, ptn.KeyPattern, ptn.ValuePattern) } ColorPatterns[regexp.MustCompile("(?m)"+rePtn)] = fmts } From 12ce5c6061696f60e759d981737a496a345e90fd Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Fri, 11 Dec 2020 19:46:52 +0530 Subject: [PATCH 03/17] For human readable output display json output when '--config-only' is used --- pkg/cli/run.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/cli/run.go b/pkg/cli/run.go index d939e5c36..816924ef5 100644 --- a/pkg/cli/run.go +++ b/pkg/cli/run.go @@ -30,6 +30,11 @@ import ( "go.uber.org/zap" ) +const ( + humanOutputFormat = "human" + jsonOutputFormat = "json" +) + // Run executes terrascan in CLI mode func Run(iacType, iacVersion string, cloudType []string, iacFilePath, iacDirPath, configFile string, policyPath []string, @@ -69,9 +74,14 @@ func Run(iacType, iacVersion string, cloudType []string, outputWriter := NewOutputWriter(useColors) if configOnly { + // human readable output doesn't support --config-only flag + // if --config-only flag is set, then display json output + if strings.EqualFold(format, humanOutputFormat) { + format = jsonOutputFormat + } writer.Write(format, results.ResourceConfig, outputWriter) } else { - if strings.EqualFold(format, "human") { + if strings.EqualFold(format, humanOutputFormat) { defaultScanResult := result.NewDefaultScanResult(iacType, iacFilePath, iacDirPath, executor.GetTotalPolicyCount(), verbose, *results.Violations.ViolationStore) writer.Write(format, defaultScanResult, outputWriter) } else { From cb111304d636fe3b0765f8cb3945865fb045e60b Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Sat, 12 Dec 2020 17:10:41 +0530 Subject: [PATCH 04/17] tests for writers --- pkg/writer/humanReadable.go | 8 +-- pkg/writer/humanReadable_test.go | 71 ++++++++++++++++++ pkg/writer/json_test.go | 119 +++++++++++++++++++++++++++++++ pkg/writer/xml_test.go | 70 ++++++++++++++++++ pkg/writer/yaml_test.go | 106 +++++++++++++++++++++++++++ 5 files changed, 370 insertions(+), 4 deletions(-) create mode 100644 pkg/writer/humanReadable_test.go create mode 100644 pkg/writer/json_test.go create mode 100644 pkg/writer/xml_test.go create mode 100644 pkg/writer/yaml_test.go diff --git a/pkg/writer/humanReadable.go b/pkg/writer/humanReadable.go index 2a3d5494f..0b9294718 100644 --- a/pkg/writer/humanReadable.go +++ b/pkg/writer/humanReadable.go @@ -18,10 +18,10 @@ package writer import ( "bytes" - "fmt" "io" - "os" "text/template" + + "go.uber.org/zap" ) const ( @@ -67,8 +67,8 @@ func init() { func HumanReadbleWriter(data interface{}, writer io.Writer) error { tmpl, err := template.New("Report").Parse(defaultTemplate) if err != nil { - fmt.Println(err) - os.Exit(1) + zap.S().Errorf("failed to write human readable output. error: '%v'", err) + return err } buffer := bytes.Buffer{} diff --git a/pkg/writer/humanReadable_test.go b/pkg/writer/humanReadable_test.go new file mode 100644 index 000000000..fdc120095 --- /dev/null +++ b/pkg/writer/humanReadable_test.go @@ -0,0 +1,71 @@ +package writer + +import ( + "bytes" + "testing" + + "github.com/accurics/terrascan/pkg/results" +) + +// TODO: string comparision - expected and output + +func TestHumanReadbleWriter(t *testing.T) { + type funcInput interface{} + tests := []struct { + name string + input funcInput + expectedError bool + }{ + { + name: "Human Readable Writer: Violations", + input: results.DefaultScanResult{ + IacType: "terraform", + ResourcePath: "/test", + Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", + TotalPolicies: 566, + ShowViolationDetails: true, + ViolationStore: results.ViolationStore{ + Violations: []*results.Violation{ + { + RuleName: "s3EnforceUserACL", + Description: "S3 bucket Access is allowed to all AWS Account Users.", + RuleID: "AWS.S3Bucket.DS.High.1043", + Severity: "HIGH", + Category: "S3", + ResourceName: "bucket", + ResourceType: "aws_s3_bucket", + File: "modules/m1/main.tf", + LineNumber: 20, + }, + }, + Count: results.ViolationStats{ + LowCount: 0, + MediumCount: 0, + HighCount: 1, + TotalCount: 1, + }, + }, + }, + }, + { + name: "Human Readable Writer: No Violations", + input: results.DefaultScanResult{ + IacType: "k8s", + ResourcePath: "/test", + Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", + TotalPolicies: 566, + ShowViolationDetails: false, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + writer := &bytes.Buffer{} + if err := HumanReadbleWriter(tt.input, writer); (err != nil) != tt.expectedError { + t.Errorf("HumanReadbleWriter() error = gotErr: %v, wantErr: %v", err, tt.expectedError) + return + } + }) + } +} diff --git a/pkg/writer/json_test.go b/pkg/writer/json_test.go new file mode 100644 index 000000000..904d54134 --- /dev/null +++ b/pkg/writer/json_test.go @@ -0,0 +1,119 @@ +package writer + +import ( + "bytes" + "strings" + "testing" + + "github.com/accurics/terrascan/pkg/iac-providers/output" + "github.com/accurics/terrascan/pkg/policy" + "github.com/accurics/terrascan/pkg/results" +) + +const ( + configOnlyTestOutputJSON = `{ + "aws_s3_bucket": [ + { + "id": "aws_s3_bucket.bucket", + "name": "bucket", + "source": "modules/m1/main.tf", + "line": 20, + "type": "aws_s3_bucket", + "config": { + "bucket": "${module.m3.fullbucketname}", + "policy": "${module.m2.fullbucketpolicy}" + } + } + ] +}` + + scanTestOutputJSON = `{ + "results": { + "violations": [ + { + "rule_name": "s3EnforceUserACL", + "description": "S3 bucket Access is allowed to all AWS Account Users.", + "rule_id": "AWS.S3Bucket.DS.High.1043", + "severity": "HIGH", + "category": "S3", + "resource_name": "bucket", + "resource_type": "aws_s3_bucket", + "file": "modules/m1/main.tf", + "line": 20 + } + ], + "count": { + "low": 0, + "medium": 0, + "high": 1, + "total": 1 + } + } +}` +) + +func TestJSONWriter(t *testing.T) { + type funcInput interface{} + tests := []struct { + name string + input funcInput + expectedOutput string + }{ + { + name: "JSON Writer: ResourceConfig", + input: output.AllResourceConfigs{ + "aws_s3_bucket": []output.ResourceConfig{ + { + ID: "aws_s3_bucket.bucket", + Name: "bucket", + Source: "modules/m1/main.tf", + Line: 20, + Type: "aws_s3_bucket", + Config: map[string]string{ + "bucket": "${module.m3.fullbucketname}", + "policy": "${module.m2.fullbucketpolicy}", + }, + }, + }, + }, + expectedOutput: configOnlyTestOutputJSON, + }, + { + name: "JSON Writer: Violations", + input: policy.EngineOutput{ + ViolationStore: &results.ViolationStore{ + Violations: []*results.Violation{ + { + RuleName: "s3EnforceUserACL", + Description: "S3 bucket Access is allowed to all AWS Account Users.", + RuleID: "AWS.S3Bucket.DS.High.1043", + Severity: "HIGH", + Category: "S3", + ResourceName: "bucket", + ResourceType: "aws_s3_bucket", + File: "modules/m1/main.tf", + LineNumber: 20, + }, + }, + Count: results.ViolationStats{ + LowCount: 0, + MediumCount: 0, + HighCount: 1, + TotalCount: 1, + }, + }, + }, + expectedOutput: scanTestOutputJSON, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + writer := &bytes.Buffer{} + JSONWriter(tt.input, writer) + if gotOutput := writer.String(); !strings.EqualFold(strings.TrimSpace(gotOutput), strings.TrimSpace(tt.expectedOutput)) { + t.Errorf("JSONWriter() = got: %v, want: %v", gotOutput, tt.expectedOutput) + } + }) + } +} diff --git a/pkg/writer/xml_test.go b/pkg/writer/xml_test.go new file mode 100644 index 000000000..d49a9701d --- /dev/null +++ b/pkg/writer/xml_test.go @@ -0,0 +1,70 @@ +package writer + +import ( + "bytes" + "strings" + "testing" + + "github.com/accurics/terrascan/pkg/policy" + "github.com/accurics/terrascan/pkg/results" +) + +const ( + // TODO: --config-only test for XML Writer (xml.Marshal doesn't support maps) + + scanTestOutputXML = ` + + + + + + + ` +) + +func TestXMLWriter(t *testing.T) { + type funcInput interface{} + tests := []struct { + name string + input funcInput + expectedOutput string + }{ + { + name: "XML Writer: Violations", + input: policy.EngineOutput{ + ViolationStore: &results.ViolationStore{ + Violations: []*results.Violation{ + { + RuleName: "s3EnforceUserACL", + Description: "S3 bucket Access is allowed to all AWS Account Users.", + RuleID: "AWS.S3Bucket.DS.High.1043", + Severity: "HIGH", + Category: "S3", + ResourceName: "bucket", + ResourceType: "aws_s3_bucket", + File: "modules/m1/main.tf", + LineNumber: 20, + }, + }, + Count: results.ViolationStats{ + LowCount: 0, + MediumCount: 0, + HighCount: 1, + TotalCount: 1, + }, + }, + }, + expectedOutput: scanTestOutputXML, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + writer := &bytes.Buffer{} + XMLWriter(tt.input, writer) + if gotOutput := writer.String(); !strings.EqualFold(strings.TrimSpace(gotOutput), strings.TrimSpace(tt.expectedOutput)) { + t.Errorf("XMLWriter() = got: %v, want: %v", gotOutput, tt.expectedOutput) + } + }) + } +} diff --git a/pkg/writer/yaml_test.go b/pkg/writer/yaml_test.go new file mode 100644 index 000000000..003022a35 --- /dev/null +++ b/pkg/writer/yaml_test.go @@ -0,0 +1,106 @@ +package writer + +import ( + "bytes" + "strings" + "testing" + + "github.com/accurics/terrascan/pkg/iac-providers/output" + "github.com/accurics/terrascan/pkg/policy" + "github.com/accurics/terrascan/pkg/results" +) + +const ( + configOnlyTestOutputYAML = `aws_s3_bucket: + - id: aws_s3_bucket.bucket + name: bucket + source: modules/m1/main.tf + line: 20 + type: aws_s3_bucket + config: + bucket: ${module.m3.fullbucketname} + policy: ${module.m2.fullbucketpolicy}` + + scanTestOutputYAML = `results: + violations: + - rule_name: s3EnforceUserACL + description: S3 bucket Access is allowed to all AWS Account Users. + rule_id: AWS.S3Bucket.DS.High.1043 + severity: HIGH + category: S3 + resource_name: bucket + resource_type: aws_s3_bucket + file: modules/m1/main.tf + line: 20 + count: + low: 0 + medium: 0 + high: 1 + total: 1` +) + +func TestYAMLWriter(t *testing.T) { + type funcInput interface{} + tests := []struct { + name string + input funcInput + expectedOutput string + }{ + { + name: "YAML Writer: ResourceConfig", + input: output.AllResourceConfigs{ + "aws_s3_bucket": []output.ResourceConfig{ + { + ID: "aws_s3_bucket.bucket", + Name: "bucket", + Source: "modules/m1/main.tf", + Line: 20, + Type: "aws_s3_bucket", + Config: map[string]string{ + "bucket": "${module.m3.fullbucketname}", + "policy": "${module.m2.fullbucketpolicy}", + }, + }, + }, + }, + expectedOutput: configOnlyTestOutputYAML, + }, + { + name: "YAML Writer: Violations", + input: policy.EngineOutput{ + ViolationStore: &results.ViolationStore{ + Violations: []*results.Violation{ + { + RuleName: "s3EnforceUserACL", + Description: "S3 bucket Access is allowed to all AWS Account Users.", + RuleID: "AWS.S3Bucket.DS.High.1043", + Severity: "HIGH", + Category: "S3", + ResourceName: "bucket", + ResourceType: "aws_s3_bucket", + File: "modules/m1/main.tf", + LineNumber: 20, + }, + }, + Count: results.ViolationStats{ + LowCount: 0, + MediumCount: 0, + HighCount: 1, + TotalCount: 1, + }, + }, + }, + expectedOutput: scanTestOutputYAML, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + writer := &bytes.Buffer{} + YAMLWriter(tt.input, writer) + if gotOutput := writer.String(); !strings.EqualFold(strings.TrimSpace(gotOutput), strings.TrimSpace(tt.expectedOutput)) { + t.Errorf("YAMLWriter() = got: %v, want: %v", gotOutput, tt.expectedOutput) + } + }) + } +} From 62236dfe5a60d98f8cd8087e002b4da854d8b327 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Sat, 12 Dec 2020 17:26:18 +0530 Subject: [PATCH 05/17] add cases for human readable --- pkg/cli/run_test.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/cli/run_test.go b/pkg/cli/run_test.go index e9082ccb7..f2d0f28a5 100644 --- a/pkg/cli/run_test.go +++ b/pkg/cli/run_test.go @@ -26,10 +26,12 @@ func TestRun(t *testing.T) { iacType string iacVersion string cloudType []string + format string iacFilePath string iacDirPath string configFile string configOnly bool + verbose bool stdOut string want string wantErr error @@ -56,11 +58,24 @@ func TestRun(t *testing.T) { iacFilePath: "testdata/run-test/config-only.yaml", configOnly: true, }, + { + name: "config-only flag true with human readable format", + cloudType: []string{"terraform"}, + iacFilePath: "testdata/run-test/config-only.tf", + configOnly: true, + format: "human", + }, + { + name: "config-only flag false with human readable format", + cloudType: []string{"k8s"}, + iacFilePath: "testdata/run-test/config-only.yaml", + format: "human", + }, } for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - Run(tt.iacType, tt.iacVersion, tt.cloudType, tt.iacFilePath, tt.iacDirPath, tt.configFile, []string{}, "", "", "", tt.configOnly, false, false) + Run(tt.iacType, tt.iacVersion, tt.cloudType, tt.iacFilePath, tt.iacDirPath, tt.configFile, []string{}, tt.format, "", "", tt.configOnly, false, tt.verbose) }) } } From bf202e0e626d507b6100843297cf6e455a2ee4cd Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Sat, 12 Dec 2020 19:01:18 +0530 Subject: [PATCH 06/17] 1. format summary 2. print empty quotes when resource name is absent --- pkg/writer/humanReadable.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/writer/humanReadable.go b/pkg/writer/humanReadable.go index 0b9294718..02adc0bba 100644 --- a/pkg/writer/humanReadable.go +++ b/pkg/writer/humanReadable.go @@ -32,30 +32,30 @@ const ( Violation Details - {{- $showDetails := .ShowViolationDetails}} {{range $index, $element := .ViolationStore.Violations}} - Description{{"\t"}}:{{"\t"}}{{$element.Description}} - File{{"\t\t"}}:{{"\t"}}{{$element.File}} - Line{{"\t\t"}}:{{"\t"}}{{$element.LineNumber}} - Severity{{"\t"}}:{{"\t"}}{{$element.Severity}} + {{printf "%-15v" "Description"}}:{{"\t"}}{{$element.Description}} + {{printf "%-15v" "File"}}:{{"\t"}}{{$element.File}} + {{printf "%-15v" "Line"}}:{{"\t"}}{{$element.LineNumber}} + {{printf "%-15v" "Severity"}}:{{"\t"}}{{$element.Severity}} {{if $showDetails -}} - Rule Name{{"\t"}}:{{"\t"}}{{$element.RuleName}} - Rule ID{{"\t\t"}}:{{"\t"}}{{$element.RuleID}} - Resource Name{{"\t"}}:{{"\t"}}{{$element.ResourceName}} - Resource Type{{"\t"}}:{{"\t"}}{{$element.ResourceType}} - Category{{"\t"}}:{{"\t"}}{{$element.Category}} + {{printf "%-15v" "Rule Name"}}:{{"\t"}}{{$element.RuleName}} + {{printf "%-15v" "Rule ID"}}:{{"\t"}}{{$element.RuleID}} + {{printf "%-15v" "Resource Name"}}:{{"\t"}}{{if $element.ResourceName}}{{$element.ResourceName}}{{else}}""{{end}} + {{printf "%-15v" "Resource Type"}}:{{"\t"}}{{$element.ResourceType}} + {{printf "%-15v" "Category"}}:{{"\t"}}{{$element.Category}} {{end}} ----------------------------------------------------------------------- {{end}} {{end}} Scan Summary - - File/Folder{{"\t\t"}}:{{"\t"}}{{.ResourcePath}} - IaC Type{{"\t\t"}}:{{"\t"}}{{.IacType}} - Scanned At{{"\t\t"}}:{{"\t"}}{{.Timestamp}} - Policies Validated{{"\t"}}:{{"\t"}}{{.TotalPolicies}} - Violated Policies{{"\t"}}:{{"\t"}}{{.ViolationStore.Count.TotalCount}} - Low{{"\t\t\t"}}:{{"\t"}}{{.ViolationStore.Count.LowCount}} - Medium{{"\t\t\t"}}:{{"\t"}}{{.ViolationStore.Count.MediumCount}} - High{{"\t\t\t"}}:{{"\t"}}{{.ViolationStore.Count.HighCount}} + {{printf "%-20v" "File/Folder"}}:{{"\t"}}{{.ResourcePath}} + {{printf "%-20v" "IaC Type"}}:{{"\t"}}{{.IacType}} + {{printf "%-20v" "Scanned At"}}:{{"\t"}}{{.Timestamp}} + {{printf "%-20v" "Policies Validated"}}:{{"\t"}}{{.TotalPolicies}} + {{printf "%-20v" "Violated Policies"}}:{{"\t"}}{{.ViolationStore.Count.TotalCount}} + {{printf "%-20v" "Low"}}:{{"\t"}}{{.ViolationStore.Count.LowCount}} + {{printf "%-20v" "Medium"}}:{{"\t"}}{{.ViolationStore.Count.MediumCount}} + {{printf "%-20v" "High"}}:{{"\t"}}{{.ViolationStore.Count.HighCount}} ` ) From 9eaf1a2da288fb923a4fa5bb5e97cedfaa1a5845 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 15 Dec 2020 12:45:04 +0530 Subject: [PATCH 07/17] writer test --- pkg/writer/writer_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 pkg/writer/writer_test.go diff --git a/pkg/writer/writer_test.go b/pkg/writer/writer_test.go new file mode 100644 index 000000000..c2fcc06e2 --- /dev/null +++ b/pkg/writer/writer_test.go @@ -0,0 +1,20 @@ +package writer + +import ( + "bytes" + "testing" +) + +func TestWriteWithNonRegisteredWriter(t *testing.T) { + err := Write("test", nil, &bytes.Buffer{}) + if err != errNotSupported { + t.Errorf("Expected error = %v but got %v", errNotSupported, err) + } +} + +func TestWriteWithRegisteredWriter(t *testing.T) { + err := Write("json", nil, &bytes.Buffer{}) + if err != nil { + t.Errorf("Unexpected error for json writer = %v", err) + } +} From dcd02a1d5369d15b69252555d01e6910c5db60b6 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 15 Dec 2020 12:51:02 +0530 Subject: [PATCH 08/17] snake case for human readable --- pkg/writer/{humanReadable.go => human_readable.go} | 0 pkg/writer/{humanReadable_test.go => human_readable_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename pkg/writer/{humanReadable.go => human_readable.go} (100%) rename pkg/writer/{humanReadable_test.go => human_readable_test.go} (100%) diff --git a/pkg/writer/humanReadable.go b/pkg/writer/human_readable.go similarity index 100% rename from pkg/writer/humanReadable.go rename to pkg/writer/human_readable.go diff --git a/pkg/writer/humanReadable_test.go b/pkg/writer/human_readable_test.go similarity index 100% rename from pkg/writer/humanReadable_test.go rename to pkg/writer/human_readable_test.go From cecc7bf245c29cc6574f1c249bba2ddbd7ac2a17 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 15 Dec 2020 17:37:41 +0530 Subject: [PATCH 09/17] add ScanSummary to ViolationStore --- pkg/cli/run.go | 10 +---- pkg/policy/opa/engine.go | 16 ++++---- pkg/policy/types.go | 2 +- pkg/results/types.go | 79 +++++++++--------------------------- pkg/runtime/executor.go | 10 ----- pkg/writer/human_readable.go | 18 ++++---- 6 files changed, 38 insertions(+), 97 deletions(-) diff --git a/pkg/cli/run.go b/pkg/cli/run.go index 816924ef5..6977e04c3 100644 --- a/pkg/cli/run.go +++ b/pkg/cli/run.go @@ -23,7 +23,6 @@ import ( "strings" "github.com/accurics/terrascan/pkg/downloader" - result "github.com/accurics/terrascan/pkg/results" "github.com/accurics/terrascan/pkg/runtime" "github.com/accurics/terrascan/pkg/utils" "github.com/accurics/terrascan/pkg/writer" @@ -81,15 +80,10 @@ func Run(iacType, iacVersion string, cloudType []string, } writer.Write(format, results.ResourceConfig, outputWriter) } else { - if strings.EqualFold(format, humanOutputFormat) { - defaultScanResult := result.NewDefaultScanResult(iacType, iacFilePath, iacDirPath, executor.GetTotalPolicyCount(), verbose, *results.Violations.ViolationStore) - writer.Write(format, defaultScanResult, outputWriter) - } else { - writer.Write(format, results.Violations, outputWriter) - } + writer.Write(format, results.Violations, outputWriter) } - if results.Violations.ViolationStore.Count.TotalCount != 0 && flag.Lookup("test.v") == nil { + if results.Violations.ViolationStore.Summary.ViolatedPolicies != 0 && flag.Lookup("test.v") == nil { os.RemoveAll(tempDir) os.Exit(3) } diff --git a/pkg/policy/opa/engine.go b/pkg/policy/opa/engine.go index 3e10db979..b849e1e4c 100644 --- a/pkg/policy/opa/engine.go +++ b/pkg/policy/opa/engine.go @@ -303,16 +303,16 @@ func (e *Engine) reportViolation(regoData *RegoData, resource *output.ResourceCo severity := regoData.Metadata.Severity if strings.ToLower(severity) == "high" { - e.results.ViolationStore.Count.HighCount++ + e.results.ViolationStore.Summary.HighCount++ } else if strings.ToLower(severity) == "medium" { - e.results.ViolationStore.Count.MediumCount++ + e.results.ViolationStore.Summary.MediumCount++ } else if strings.ToLower(severity) == "low" { - e.results.ViolationStore.Count.LowCount++ + e.results.ViolationStore.Summary.LowCount++ } else { zap.S().Warn("invalid severity found in rule definition", zap.String("rule id", violation.RuleID), zap.String("severity", severity)) } - e.results.ViolationStore.Count.TotalCount++ + e.results.ViolationStore.Summary.ViolatedPolicies++ e.results.ViolationStore.AddResult(&violation) } @@ -390,10 +390,8 @@ func (e *Engine) Evaluate(engineInput policy.EngineInput) (policy.EngineOutput, } e.stats.runTime = time.Since(start) - return e.results, nil -} -// GetRuleCount will return the ruleCount value -func (e Engine) GetRuleCount() int { - return e.stats.ruleCount + // add the rule count of the policy engine to result summary + e.results.ViolationStore.Summary.TotalPolicies += e.stats.ruleCount + return e.results, nil } diff --git a/pkg/policy/types.go b/pkg/policy/types.go index 3de2a1591..3250ea60e 100644 --- a/pkg/policy/types.go +++ b/pkg/policy/types.go @@ -33,6 +33,6 @@ func (me EngineOutput) AsViolationStore() results.ViolationStore { } return results.ViolationStore{ Violations: me.Violations, - Count: me.Count, + Summary: me.Summary, } } diff --git a/pkg/results/types.go b/pkg/results/types.go index 1f0083ddf..f9889ec15 100644 --- a/pkg/results/types.go +++ b/pkg/results/types.go @@ -16,12 +16,6 @@ package results -import ( - "time" - - "github.com/accurics/terrascan/pkg/utils" -) - // Violation Contains data for each violation type Violation struct { RuleName string `json:"rule_name" yaml:"rule_name" xml:"rule_name,attr"` @@ -38,28 +32,23 @@ type Violation struct { LineNumber int `json:"line" yaml:"line" xml:"line,attr"` } -// ViolationStats Contains stats related to the violation data -type ViolationStats struct { - LowCount int `json:"low" yaml:"low" xml:"low,attr"` - MediumCount int `json:"medium" yaml:"medium" xml:"medium,attr"` - HighCount int `json:"high" yaml:"high" xml:"high,attr"` - TotalCount int `json:"total" yaml:"total" xml:"total,attr"` -} - // ViolationStore Storage area for violation data type ViolationStore struct { - Violations []*Violation `json:"violations" yaml:"violations" xml:"violations>violation"` - Count ViolationStats `json:"count" yaml:"count" xml:"count"` + Violations []*Violation `json:"violations" yaml:"violations" xml:"violations>violation"` + Summary ScanSummary `json:"scan_summary" yaml:"scan_summary" xml:"scan_summary"` } -// DefaultScanResult will hold the default scan summary data -type DefaultScanResult struct { - IacType string - ResourcePath string - Timestamp string - TotalPolicies int - ShowViolationDetails bool - ViolationStore +// ScanSummary will hold the default scan summary data +type ScanSummary struct { + IacType string `json:"iac_type" yaml:"iac_type" xml:"iac_type,attr"` + ResourcePath string `json:"file/folder" yaml:"file/folder" xml:"file/folder,attr"` + Timestamp string `json:"timestamp" yaml:"timestamp" xml:"timestamp,attr"` + ShowViolationDetails bool `json:"-" yaml:"-" xml:"-"` + TotalPolicies int `json:"policies_validated" yaml:"policies_validated" xml:"policies_validated,attr"` + ViolatedPolicies int `json:"violated_policies" yaml:"violated_policies" xml:"violated_policies,attr"` + LowCount int `json:"low" yaml:"low" xml:"low,attr"` + MediumCount int `json:"medium" yaml:"medium" xml:"medium,attr"` + HighCount int `json:"high" yaml:"high" xml:"high,attr"` } // Add adds two ViolationStores @@ -67,42 +56,12 @@ func (vs ViolationStore) Add(extra ViolationStore) ViolationStore { // Just concatenate the slices, since order shouldn't be important vs.Violations = append(vs.Violations, extra.Violations...) - // Add the counts - vs.Count.LowCount += extra.Count.LowCount - vs.Count.MediumCount += extra.Count.MediumCount - vs.Count.HighCount += extra.Count.HighCount - vs.Count.TotalCount += extra.Count.TotalCount + // Add the scan summary + vs.Summary.LowCount += extra.Summary.LowCount + vs.Summary.MediumCount += extra.Summary.MediumCount + vs.Summary.HighCount += extra.Summary.HighCount + vs.Summary.ViolatedPolicies += extra.Summary.ViolatedPolicies + vs.Summary.TotalPolicies += extra.Summary.TotalPolicies return vs } - -// NewDefaultScanResult will initialize DefaultScanResult -func NewDefaultScanResult(iacType, iacFilePath, iacDirPath string, totalPolicyCount int, verbose bool, violationStore ViolationStore) *DefaultScanResult { - sr := new(DefaultScanResult) - - if iacType == "" { - // the default scan type is terraform - sr.IacType = "terraform" - } else { - sr.IacType = iacType - } - - if iacFilePath != "" { - // can skip the error as the file validation is already done - // while executor is initialized - filePath, _ := utils.GetAbsPath(iacFilePath) - sr.ResourcePath = filePath - } else { - // can skip the error as the directory validation is already done - // while executor is initialized - dirPath, _ := utils.GetAbsPath(iacDirPath) - sr.ResourcePath = dirPath - } - sr.ShowViolationDetails = verbose - sr.TotalPolicies = totalPolicyCount - sr.ViolationStore = violationStore - // set current time as scan time - sr.Timestamp = time.Now().UTC().String() - - return sr -} diff --git a/pkg/runtime/executor.go b/pkg/runtime/executor.go index 44101ac78..fd3520676 100644 --- a/pkg/runtime/executor.go +++ b/pkg/runtime/executor.go @@ -131,13 +131,3 @@ func (e *Executor) Execute() (results Output, err error) { // successful return results, nil } - -// GetTotalPolicyCount will return the total count of policies in all engines -func (e Executor) GetTotalPolicyCount() int { - policyCount := 0 - for _, engine := range e.policyEngine { - opaEngine := engine.(*opa.Engine) - policyCount += opaEngine.GetRuleCount() - } - return policyCount -} diff --git a/pkg/writer/human_readable.go b/pkg/writer/human_readable.go index 02adc0bba..27fe4ca36 100644 --- a/pkg/writer/human_readable.go +++ b/pkg/writer/human_readable.go @@ -30,7 +30,7 @@ const ( defaultTemplate string = ` {{if (gt (len .ViolationStore.Violations) 0) }} Violation Details - - {{- $showDetails := .ShowViolationDetails}} + {{- $showDetails := .ViolationStore.Summary.ShowViolationDetails}} {{range $index, $element := .ViolationStore.Violations}} {{printf "%-15v" "Description"}}:{{"\t"}}{{$element.Description}} {{printf "%-15v" "File"}}:{{"\t"}}{{$element.File}} @@ -48,14 +48,14 @@ Violation Details - {{end}} Scan Summary - - {{printf "%-20v" "File/Folder"}}:{{"\t"}}{{.ResourcePath}} - {{printf "%-20v" "IaC Type"}}:{{"\t"}}{{.IacType}} - {{printf "%-20v" "Scanned At"}}:{{"\t"}}{{.Timestamp}} - {{printf "%-20v" "Policies Validated"}}:{{"\t"}}{{.TotalPolicies}} - {{printf "%-20v" "Violated Policies"}}:{{"\t"}}{{.ViolationStore.Count.TotalCount}} - {{printf "%-20v" "Low"}}:{{"\t"}}{{.ViolationStore.Count.LowCount}} - {{printf "%-20v" "Medium"}}:{{"\t"}}{{.ViolationStore.Count.MediumCount}} - {{printf "%-20v" "High"}}:{{"\t"}}{{.ViolationStore.Count.HighCount}} + {{printf "%-20v" "File/Folder"}}:{{"\t"}}{{.ViolationStore.Summary.ResourcePath}} + {{printf "%-20v" "IaC Type"}}:{{"\t"}}{{.ViolationStore.Summary.IacType}} + {{printf "%-20v" "Scanned At"}}:{{"\t"}}{{.ViolationStore.Summary.Timestamp}} + {{printf "%-20v" "Policies Validated"}}:{{"\t"}}{{.ViolationStore.Summary.TotalPolicies}} + {{printf "%-20v" "Violated Policies"}}:{{"\t"}}{{.ViolationStore.Summary.ViolatedPolicies}} + {{printf "%-20v" "Low"}}:{{"\t"}}{{.ViolationStore.Summary.LowCount}} + {{printf "%-20v" "Medium"}}:{{"\t"}}{{.ViolationStore.Summary.MediumCount}} + {{printf "%-20v" "High"}}:{{"\t"}}{{.ViolationStore.Summary.HighCount}} ` ) From 5a6d70eb7b857e1cad8217389098cc38560bf5d2 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 15 Dec 2020 18:31:09 +0530 Subject: [PATCH 10/17] display error to user if config only used with human --- pkg/cli/run.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/cli/run.go b/pkg/cli/run.go index 6977e04c3..8c12d8374 100644 --- a/pkg/cli/run.go +++ b/pkg/cli/run.go @@ -31,7 +31,6 @@ import ( const ( humanOutputFormat = "human" - jsonOutputFormat = "json" ) // Run executes terrascan in CLI mode @@ -70,13 +69,18 @@ func Run(iacType, iacVersion string, cloudType []string, return } + // add verbose flag to the scan summary + results.Violations.ViolationStore.Summary.ShowViolationDetails = verbose + outputWriter := NewOutputWriter(useColors) if configOnly { // human readable output doesn't support --config-only flag - // if --config-only flag is set, then display json output + // if --config-only flag is set, then exit with an error + // asking the user to use yaml or json output format if strings.EqualFold(format, humanOutputFormat) { - format = jsonOutputFormat + zap.S().Error("please use yaml or json output format when using --config-only flag") + return } writer.Write(format, results.ResourceConfig, outputWriter) } else { From 9103637f89a61dca3718bd15938a93a48e741edb Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 15 Dec 2020 18:31:55 +0530 Subject: [PATCH 11/17] other scan summary details --- pkg/results/types.go | 29 +++++++++++++++++++++++++++++ pkg/runtime/executor.go | 3 +++ 2 files changed, 32 insertions(+) diff --git a/pkg/results/types.go b/pkg/results/types.go index f9889ec15..047c05b26 100644 --- a/pkg/results/types.go +++ b/pkg/results/types.go @@ -16,6 +16,12 @@ package results +import ( + "time" + + "github.com/accurics/terrascan/pkg/utils" +) + // Violation Contains data for each violation type Violation struct { RuleName string `json:"rule_name" yaml:"rule_name" xml:"rule_name,attr"` @@ -65,3 +71,26 @@ func (vs ViolationStore) Add(extra ViolationStore) ViolationStore { return vs } + +// AddSummary will update the summary with remaining details +func (vs *ViolationStore) AddSummary(iacType, iacFilePath, iacDirPath string) { + if iacType == "" { + // the default scan type is terraform + vs.Summary.IacType = "terraform" + } else { + vs.Summary.IacType = iacType + } + + if iacFilePath != "" { + // can skip the error as the file validation is already done + // while executor is initialized + filePath, _ := utils.GetAbsPath(iacFilePath) + vs.Summary.ResourcePath = filePath + } else { + // can skip the error as the directory validation is already done + // while executor is initialized + dirPath, _ := utils.GetAbsPath(iacDirPath) + vs.Summary.ResourcePath = dirPath + } + vs.Summary.Timestamp = time.Now().UTC().String() +} diff --git a/pkg/runtime/executor.go b/pkg/runtime/executor.go index fd3520676..97f86cd16 100644 --- a/pkg/runtime/executor.go +++ b/pkg/runtime/executor.go @@ -123,6 +123,9 @@ func (e *Executor) Execute() (results Output, err error) { results.Violations = policy.EngineOutputFromViolationStore(&violations) + // add other summary details after policies are evaluated + results.Violations.ViolationStore.AddSummary(e.iacType, e.filePath, e.dirPath) + // send notifications, if configured if err = e.SendNotifications(results); err != nil { return results, err From 117c40cab61e558e96850fb88d560f359025d585 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 15 Dec 2020 18:57:49 +0530 Subject: [PATCH 12/17] update writer tests --- pkg/results/types.go | 2 +- pkg/writer/human_readable_test.go | 43 ++++++++++++++++++------------- pkg/writer/json_test.go | 24 +++++++++++------ pkg/writer/xml_test.go | 16 +++++++----- pkg/writer/yaml_test.go | 24 +++++++++++------ 5 files changed, 68 insertions(+), 41 deletions(-) diff --git a/pkg/results/types.go b/pkg/results/types.go index 047c05b26..29ae336d5 100644 --- a/pkg/results/types.go +++ b/pkg/results/types.go @@ -48,7 +48,7 @@ type ViolationStore struct { type ScanSummary struct { IacType string `json:"iac_type" yaml:"iac_type" xml:"iac_type,attr"` ResourcePath string `json:"file/folder" yaml:"file/folder" xml:"file/folder,attr"` - Timestamp string `json:"timestamp" yaml:"timestamp" xml:"timestamp,attr"` + Timestamp string `json:"scanned_at" yaml:"scanned_at" xml:"scanned_at,attr"` ShowViolationDetails bool `json:"-" yaml:"-" xml:"-"` TotalPolicies int `json:"policies_validated" yaml:"policies_validated" xml:"policies_validated,attr"` ViolatedPolicies int `json:"violated_policies" yaml:"violated_policies" xml:"violated_policies,attr"` diff --git a/pkg/writer/human_readable_test.go b/pkg/writer/human_readable_test.go index fdc120095..cc901f7d4 100644 --- a/pkg/writer/human_readable_test.go +++ b/pkg/writer/human_readable_test.go @@ -4,6 +4,7 @@ import ( "bytes" "testing" + "github.com/accurics/terrascan/pkg/policy" "github.com/accurics/terrascan/pkg/results" ) @@ -18,13 +19,8 @@ func TestHumanReadbleWriter(t *testing.T) { }{ { name: "Human Readable Writer: Violations", - input: results.DefaultScanResult{ - IacType: "terraform", - ResourcePath: "/test", - Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", - TotalPolicies: 566, - ShowViolationDetails: true, - ViolationStore: results.ViolationStore{ + input: policy.EngineOutput{ + ViolationStore: &results.ViolationStore{ Violations: []*results.Violation{ { RuleName: "s3EnforceUserACL", @@ -38,23 +34,34 @@ func TestHumanReadbleWriter(t *testing.T) { LineNumber: 20, }, }, - Count: results.ViolationStats{ - LowCount: 0, - MediumCount: 0, - HighCount: 1, - TotalCount: 1, + Summary: results.ScanSummary{ + ResourcePath: "test", + IacType: "terraform", + Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", + TotalPolicies: 566, + LowCount: 0, + MediumCount: 0, + HighCount: 1, + ViolatedPolicies: 1, }, }, }, }, { name: "Human Readable Writer: No Violations", - input: results.DefaultScanResult{ - IacType: "k8s", - ResourcePath: "/test", - Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", - TotalPolicies: 566, - ShowViolationDetails: false, + input: policy.EngineOutput{ + ViolationStore: &results.ViolationStore{ + Summary: results.ScanSummary{ + ResourcePath: "test", + IacType: "terraform", + Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", + TotalPolicies: 566, + LowCount: 0, + MediumCount: 0, + HighCount: 1, + ViolatedPolicies: 1, + }, + }, }, }, } diff --git a/pkg/writer/json_test.go b/pkg/writer/json_test.go index 904d54134..ba4d0835d 100644 --- a/pkg/writer/json_test.go +++ b/pkg/writer/json_test.go @@ -42,11 +42,15 @@ const ( "line": 20 } ], - "count": { + "scan_summary": { + "iac_type": "terraform", + "file/folder": "test", + "scanned_at": "2020-12-12 11:21:29.902796 +0000 UTC", + "policies_validated": 566, + "violated_policies": 1, "low": 0, "medium": 0, - "high": 1, - "total": 1 + "high": 1 } } }` @@ -95,11 +99,15 @@ func TestJSONWriter(t *testing.T) { LineNumber: 20, }, }, - Count: results.ViolationStats{ - LowCount: 0, - MediumCount: 0, - HighCount: 1, - TotalCount: 1, + Summary: results.ScanSummary{ + ResourcePath: "test", + IacType: "terraform", + Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", + TotalPolicies: 566, + LowCount: 0, + MediumCount: 0, + HighCount: 1, + ViolatedPolicies: 1, }, }, }, diff --git a/pkg/writer/xml_test.go b/pkg/writer/xml_test.go index d49a9701d..786a647ba 100644 --- a/pkg/writer/xml_test.go +++ b/pkg/writer/xml_test.go @@ -17,7 +17,7 @@ const ( - + ` ) @@ -46,11 +46,15 @@ func TestXMLWriter(t *testing.T) { LineNumber: 20, }, }, - Count: results.ViolationStats{ - LowCount: 0, - MediumCount: 0, - HighCount: 1, - TotalCount: 1, + Summary: results.ScanSummary{ + ResourcePath: "test", + IacType: "terraform", + Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", + TotalPolicies: 566, + LowCount: 0, + MediumCount: 0, + HighCount: 1, + ViolatedPolicies: 1, }, }, }, diff --git a/pkg/writer/yaml_test.go b/pkg/writer/yaml_test.go index 003022a35..0d7abd8c8 100644 --- a/pkg/writer/yaml_test.go +++ b/pkg/writer/yaml_test.go @@ -32,11 +32,15 @@ const ( resource_type: aws_s3_bucket file: modules/m1/main.tf line: 20 - count: + scan_summary: + iac_type: terraform + file/folder: test + scanned_at: 2020-12-12 11:21:29.902796 +0000 UTC + policies_validated: 566 + violated_policies: 1 low: 0 medium: 0 - high: 1 - total: 1` + high: 1` ) func TestYAMLWriter(t *testing.T) { @@ -82,11 +86,15 @@ func TestYAMLWriter(t *testing.T) { LineNumber: 20, }, }, - Count: results.ViolationStats{ - LowCount: 0, - MediumCount: 0, - HighCount: 1, - TotalCount: 1, + Summary: results.ScanSummary{ + ResourcePath: "test", + IacType: "terraform", + Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", + TotalPolicies: 566, + LowCount: 0, + MediumCount: 0, + HighCount: 1, + ViolatedPolicies: 1, }, }, }, From 33cd0e70978483520a9b238caebcd08690cec52c Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 15 Dec 2020 19:25:34 +0530 Subject: [PATCH 13/17] 1. fix summary order 2. fix colored writer test --- pkg/results/types.go | 2 +- pkg/termcolor/colorpatterns.go | 23 +++++++++++------------ pkg/termcolor/writer_test.go | 8 ++++---- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/pkg/results/types.go b/pkg/results/types.go index 29ae336d5..5fd4c11cc 100644 --- a/pkg/results/types.go +++ b/pkg/results/types.go @@ -46,8 +46,8 @@ type ViolationStore struct { // ScanSummary will hold the default scan summary data type ScanSummary struct { - IacType string `json:"iac_type" yaml:"iac_type" xml:"iac_type,attr"` ResourcePath string `json:"file/folder" yaml:"file/folder" xml:"file/folder,attr"` + IacType string `json:"iac_type" yaml:"iac_type" xml:"iac_type,attr"` Timestamp string `json:"scanned_at" yaml:"scanned_at" xml:"scanned_at,attr"` ShowViolationDetails bool `json:"-" yaml:"-" xml:"-"` TotalPolicies int `json:"policies_validated" yaml:"policies_validated" xml:"policies_validated,attr"` diff --git a/pkg/termcolor/colorpatterns.go b/pkg/termcolor/colorpatterns.go index 63dc52291..eb7f4616e 100644 --- a/pkg/termcolor/colorpatterns.go +++ b/pkg/termcolor/colorpatterns.go @@ -54,18 +54,17 @@ type colorPatternSerialized struct { **/ var defaultColorPatterns = map[FieldSpec]FieldStyle{ - {"[dD]escription", defaultValuePattern}: {"", "Fg#0c0"}, - {"[sS]everity", defaultValuePattern}: {"", "?HIGH=Fg#f00?MEDIUM=Fg#c84?LOW=Fg#cc0"}, - {`[rR]esource[_\s][nN]ame`, defaultValuePattern}: {"", "Fg#0ff|Bold"}, - {`[rR]esource[_\s][tT]ype`, defaultValuePattern}: {"", "Fg#0cc"}, - {"[fF]ile", defaultValuePattern}: {"", "Fg#00768B|Bold"}, - {"[lL]ow", `\d+`}: {"Fg#cc0", "Fg#cc0"}, - {"[mM]edium", `\d+`}: {"Fg#c84", "Fg#c84"}, - {"[hH]igh", `\d+`}: {"Fg#f00", "Fg#f00"}, - {"count", ""}: {"Bg#ccc|Fg#000", ""}, - {`[rR]ule[_\s][nN]ame`, defaultValuePattern}: {"Bg#ccc|Fg#000", ""}, - {"File/Folder", defaultValuePattern}: {"", "Fg#00768B|Bold"}, - {"Policies Validated", defaultValuePattern}: {"Bg#ccc|Fg#000", ""}, + {"[dD]escription", defaultValuePattern}: {"", "Fg#0c0"}, + {"[sS]everity", defaultValuePattern}: {"", "?HIGH=Fg#f00?MEDIUM=Fg#c84?LOW=Fg#cc0"}, + {`[rR]esource[_\s][nN]ame`, defaultValuePattern}: {"", "Fg#0ff|Bold"}, + {`[rR]esource[_\s][tT]ype`, defaultValuePattern}: {"", "Fg#0cc"}, + {"[fF]ile", defaultValuePattern}: {"", "Fg#00768B|Bold"}, + {"[lL]ow", `\d+`}: {"Fg#cc0", "Fg#cc0"}, + {"[mM]edium", `\d+`}: {"Fg#c84", "Fg#c84"}, + {"[hH]igh", `\d+`}: {"Fg#f00", "Fg#f00"}, + {`[rR]ule[_\s][nN]ame`, defaultValuePattern}: {"Bg#ccc|Fg#000", ""}, + {"[fF]ile/[fF]older", defaultValuePattern}: {"", "Fg#00768B|Bold"}, + {`[pP]olicies[_\s][vV]alidated`, defaultValuePattern}: {"Bg#ccc|Fg#000", ""}, } func init() { diff --git a/pkg/termcolor/writer_test.go b/pkg/termcolor/writer_test.go index 9e41161e4..42159b272 100644 --- a/pkg/termcolor/writer_test.go +++ b/pkg/termcolor/writer_test.go @@ -103,8 +103,8 @@ func TestYAMLFileIsColorized(t *testing.T) { verifyLineWithStringIsColorized("file", yamlData.String(), t) } -func TestYAMLCountIsColorized(t *testing.T) { - verifyLineWithStringIsColorized("count", yamlData.String(), t) +func TestYAMLPoliciesValidatedIsColorized(t *testing.T) { + verifyLineWithStringIsColorized("policies_validated", yamlData.String(), t) } func TestYAMLCountLowIsColorized(t *testing.T) { verifyLineWithStringIsColorized("low", yamlData.String(), t) @@ -162,8 +162,8 @@ func TestJSONFileIsColorized(t *testing.T) { verifyLineWithStringIsColorized("file", jsonData.String(), t) } -func TestJSONCountIsColorized(t *testing.T) { - verifyLineWithStringIsColorized("count", jsonData.String(), t) +func TestJSONPoliciesValidatedIsColorized(t *testing.T) { + verifyLineWithStringIsColorized("policies_validated", jsonData.String(), t) } func TestJSONCountLowIsColorized(t *testing.T) { verifyLineWithStringIsColorized("low", jsonData.String(), t) From 305a8e22be6519af2da94a8af72dd05762ea12a5 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 15 Dec 2020 23:36:04 +0530 Subject: [PATCH 14/17] fix sonar duplication issue --- pkg/writer/human_readable_test.go | 30 +--------- pkg/writer/json_test.go | 54 ++--------------- pkg/writer/xml_test.go | 35 +---------- pkg/writer/yaml_test.go | 97 +++++++++++++++++-------------- 4 files changed, 62 insertions(+), 154 deletions(-) diff --git a/pkg/writer/human_readable_test.go b/pkg/writer/human_readable_test.go index cc901f7d4..7a83beb37 100644 --- a/pkg/writer/human_readable_test.go +++ b/pkg/writer/human_readable_test.go @@ -18,34 +18,8 @@ func TestHumanReadbleWriter(t *testing.T) { expectedError bool }{ { - name: "Human Readable Writer: Violations", - input: policy.EngineOutput{ - ViolationStore: &results.ViolationStore{ - Violations: []*results.Violation{ - { - RuleName: "s3EnforceUserACL", - Description: "S3 bucket Access is allowed to all AWS Account Users.", - RuleID: "AWS.S3Bucket.DS.High.1043", - Severity: "HIGH", - Category: "S3", - ResourceName: "bucket", - ResourceType: "aws_s3_bucket", - File: "modules/m1/main.tf", - LineNumber: 20, - }, - }, - Summary: results.ScanSummary{ - ResourcePath: "test", - IacType: "terraform", - Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", - TotalPolicies: 566, - LowCount: 0, - MediumCount: 0, - HighCount: 1, - ViolatedPolicies: 1, - }, - }, - }, + name: "Human Readable Writer: Violations", + input: violationsInput, }, { name: "Human Readable Writer: No Violations", diff --git a/pkg/writer/json_test.go b/pkg/writer/json_test.go index ba4d0835d..c7a5325c7 100644 --- a/pkg/writer/json_test.go +++ b/pkg/writer/json_test.go @@ -4,10 +4,6 @@ import ( "bytes" "strings" "testing" - - "github.com/accurics/terrascan/pkg/iac-providers/output" - "github.com/accurics/terrascan/pkg/policy" - "github.com/accurics/terrascan/pkg/results" ) const ( @@ -43,8 +39,8 @@ const ( } ], "scan_summary": { - "iac_type": "terraform", "file/folder": "test", + "iac_type": "terraform", "scanned_at": "2020-12-12 11:21:29.902796 +0000 UTC", "policies_validated": 566, "violated_policies": 1, @@ -64,53 +60,13 @@ func TestJSONWriter(t *testing.T) { expectedOutput string }{ { - name: "JSON Writer: ResourceConfig", - input: output.AllResourceConfigs{ - "aws_s3_bucket": []output.ResourceConfig{ - { - ID: "aws_s3_bucket.bucket", - Name: "bucket", - Source: "modules/m1/main.tf", - Line: 20, - Type: "aws_s3_bucket", - Config: map[string]string{ - "bucket": "${module.m3.fullbucketname}", - "policy": "${module.m2.fullbucketpolicy}", - }, - }, - }, - }, + name: "JSON Writer: ResourceConfig", + input: resourceConfigInput, expectedOutput: configOnlyTestOutputJSON, }, { - name: "JSON Writer: Violations", - input: policy.EngineOutput{ - ViolationStore: &results.ViolationStore{ - Violations: []*results.Violation{ - { - RuleName: "s3EnforceUserACL", - Description: "S3 bucket Access is allowed to all AWS Account Users.", - RuleID: "AWS.S3Bucket.DS.High.1043", - Severity: "HIGH", - Category: "S3", - ResourceName: "bucket", - ResourceType: "aws_s3_bucket", - File: "modules/m1/main.tf", - LineNumber: 20, - }, - }, - Summary: results.ScanSummary{ - ResourcePath: "test", - IacType: "terraform", - Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", - TotalPolicies: 566, - LowCount: 0, - MediumCount: 0, - HighCount: 1, - ViolatedPolicies: 1, - }, - }, - }, + name: "JSON Writer: Violations", + input: violationsInput, expectedOutput: scanTestOutputJSON, }, } diff --git a/pkg/writer/xml_test.go b/pkg/writer/xml_test.go index 786a647ba..f7cba8baf 100644 --- a/pkg/writer/xml_test.go +++ b/pkg/writer/xml_test.go @@ -4,9 +4,6 @@ import ( "bytes" "strings" "testing" - - "github.com/accurics/terrascan/pkg/policy" - "github.com/accurics/terrascan/pkg/results" ) const ( @@ -17,7 +14,7 @@ const ( - + ` ) @@ -30,34 +27,8 @@ func TestXMLWriter(t *testing.T) { expectedOutput string }{ { - name: "XML Writer: Violations", - input: policy.EngineOutput{ - ViolationStore: &results.ViolationStore{ - Violations: []*results.Violation{ - { - RuleName: "s3EnforceUserACL", - Description: "S3 bucket Access is allowed to all AWS Account Users.", - RuleID: "AWS.S3Bucket.DS.High.1043", - Severity: "HIGH", - Category: "S3", - ResourceName: "bucket", - ResourceType: "aws_s3_bucket", - File: "modules/m1/main.tf", - LineNumber: 20, - }, - }, - Summary: results.ScanSummary{ - ResourcePath: "test", - IacType: "terraform", - Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", - TotalPolicies: 566, - LowCount: 0, - MediumCount: 0, - HighCount: 1, - ViolatedPolicies: 1, - }, - }, - }, + name: "XML Writer: Violations", + input: violationsInput, expectedOutput: scanTestOutputXML, }, } diff --git a/pkg/writer/yaml_test.go b/pkg/writer/yaml_test.go index 0d7abd8c8..6c7daec7a 100644 --- a/pkg/writer/yaml_test.go +++ b/pkg/writer/yaml_test.go @@ -10,6 +10,53 @@ import ( "github.com/accurics/terrascan/pkg/results" ) +// these variables would be used as test input accross the writer package +var ( + resourceConfigInput = output.AllResourceConfigs{ + "aws_s3_bucket": []output.ResourceConfig{ + { + ID: "aws_s3_bucket.bucket", + Name: "bucket", + Source: "modules/m1/main.tf", + Line: 20, + Type: "aws_s3_bucket", + Config: map[string]string{ + "bucket": "${module.m3.fullbucketname}", + "policy": "${module.m2.fullbucketpolicy}", + }, + }, + }, + } + + violationsInput = policy.EngineOutput{ + ViolationStore: &results.ViolationStore{ + Violations: []*results.Violation{ + { + RuleName: "s3EnforceUserACL", + Description: "S3 bucket Access is allowed to all AWS Account Users.", + RuleID: "AWS.S3Bucket.DS.High.1043", + Severity: "HIGH", + Category: "S3", + ResourceName: "bucket", + ResourceType: "aws_s3_bucket", + File: "modules/m1/main.tf", + LineNumber: 20, + }, + }, + Summary: results.ScanSummary{ + ResourcePath: "test", + IacType: "terraform", + Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", + TotalPolicies: 566, + LowCount: 0, + MediumCount: 0, + HighCount: 1, + ViolatedPolicies: 1, + }, + }, + } +) + const ( configOnlyTestOutputYAML = `aws_s3_bucket: - id: aws_s3_bucket.bucket @@ -33,8 +80,8 @@ const ( file: modules/m1/main.tf line: 20 scan_summary: - iac_type: terraform file/folder: test + iac_type: terraform scanned_at: 2020-12-12 11:21:29.902796 +0000 UTC policies_validated: 566 violated_policies: 1 @@ -51,53 +98,13 @@ func TestYAMLWriter(t *testing.T) { expectedOutput string }{ { - name: "YAML Writer: ResourceConfig", - input: output.AllResourceConfigs{ - "aws_s3_bucket": []output.ResourceConfig{ - { - ID: "aws_s3_bucket.bucket", - Name: "bucket", - Source: "modules/m1/main.tf", - Line: 20, - Type: "aws_s3_bucket", - Config: map[string]string{ - "bucket": "${module.m3.fullbucketname}", - "policy": "${module.m2.fullbucketpolicy}", - }, - }, - }, - }, + name: "YAML Writer: ResourceConfig", + input: resourceConfigInput, expectedOutput: configOnlyTestOutputYAML, }, { - name: "YAML Writer: Violations", - input: policy.EngineOutput{ - ViolationStore: &results.ViolationStore{ - Violations: []*results.Violation{ - { - RuleName: "s3EnforceUserACL", - Description: "S3 bucket Access is allowed to all AWS Account Users.", - RuleID: "AWS.S3Bucket.DS.High.1043", - Severity: "HIGH", - Category: "S3", - ResourceName: "bucket", - ResourceType: "aws_s3_bucket", - File: "modules/m1/main.tf", - LineNumber: 20, - }, - }, - Summary: results.ScanSummary{ - ResourcePath: "test", - IacType: "terraform", - Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", - TotalPolicies: 566, - LowCount: 0, - MediumCount: 0, - HighCount: 1, - ViolatedPolicies: 1, - }, - }, - }, + name: "YAML Writer: Violations", + input: violationsInput, expectedOutput: scanTestOutputYAML, }, } From 49562fe66e707f40b97e3d00fa09ce78838522b5 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Wed, 16 Dec 2020 00:50:26 +0530 Subject: [PATCH 15/17] incorporated the review comments --- pkg/results/types.go | 23 +++-------------------- pkg/runtime/executor.go | 7 ++++++- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/pkg/results/types.go b/pkg/results/types.go index 5fd4c11cc..f7a0ac16e 100644 --- a/pkg/results/types.go +++ b/pkg/results/types.go @@ -18,8 +18,6 @@ package results import ( "time" - - "github.com/accurics/terrascan/pkg/utils" ) // Violation Contains data for each violation @@ -73,24 +71,9 @@ func (vs ViolationStore) Add(extra ViolationStore) ViolationStore { } // AddSummary will update the summary with remaining details -func (vs *ViolationStore) AddSummary(iacType, iacFilePath, iacDirPath string) { - if iacType == "" { - // the default scan type is terraform - vs.Summary.IacType = "terraform" - } else { - vs.Summary.IacType = iacType - } +func (vs *ViolationStore) AddSummary(iacType, iacResourcePath string) { - if iacFilePath != "" { - // can skip the error as the file validation is already done - // while executor is initialized - filePath, _ := utils.GetAbsPath(iacFilePath) - vs.Summary.ResourcePath = filePath - } else { - // can skip the error as the directory validation is already done - // while executor is initialized - dirPath, _ := utils.GetAbsPath(iacDirPath) - vs.Summary.ResourcePath = dirPath - } + vs.Summary.IacType = iacType + vs.Summary.ResourcePath = iacResourcePath vs.Summary.Timestamp = time.Now().UTC().String() } diff --git a/pkg/runtime/executor.go b/pkg/runtime/executor.go index 97f86cd16..6357ff084 100644 --- a/pkg/runtime/executor.go +++ b/pkg/runtime/executor.go @@ -123,8 +123,13 @@ func (e *Executor) Execute() (results Output, err error) { results.Violations = policy.EngineOutputFromViolationStore(&violations) + resourcePath := e.filePath + if resourcePath == "" { + resourcePath = e.dirPath + } + // add other summary details after policies are evaluated - results.Violations.ViolationStore.AddSummary(e.iacType, e.filePath, e.dirPath) + results.Violations.ViolationStore.AddSummary(e.iacType, resourcePath) // send notifications, if configured if err = e.SendNotifications(results); err != nil { From 209fb65a270a80900f54322eceb162515613bf46 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Wed, 16 Dec 2020 15:09:06 +0530 Subject: [PATCH 16/17] run refactor and tests --- pkg/cli/run.go | 51 +++++++++++++------ pkg/cli/run_test.go | 119 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 16 deletions(-) diff --git a/pkg/cli/run.go b/pkg/cli/run.go index 8c12d8374..b0918f068 100644 --- a/pkg/cli/run.go +++ b/pkg/cli/run.go @@ -17,6 +17,7 @@ package cli import ( + "errors" "flag" "os" "path/filepath" @@ -43,16 +44,12 @@ func Run(iacType, iacVersion string, cloudType []string, defer os.RemoveAll(tempDir) // download remote repository - d := downloader.NewDownloader() - path, err := d.DownloadWithType(remoteType, remoteURL, tempDir) - if err == downloader.ErrEmptyURLType { - // url and type empty, proceed with regular scanning - zap.S().Debugf("remote url and type not configured, proceeding with regular scanning") - } else if err != nil { - // some error while downloading remote repository + path, err := downloadRemoteRepository(remoteType, remoteURL, tempDir) + if err != nil { return - } else { - // successfully downloaded remote repository + } + + if path != "" { iacDirPath = path } @@ -69,6 +66,33 @@ func Run(iacType, iacVersion string, cloudType []string, return } + // write results to console + err = writeResults(results, useColors, verbose, configOnly, humanOutputFormat) + if err != nil { + zap.S().Error("failed to write results", zap.Error(err)) + return + } + + if results.Violations.ViolationStore.Summary.ViolatedPolicies != 0 && flag.Lookup("test.v") == nil { + os.RemoveAll(tempDir) + os.Exit(3) + } +} + +func downloadRemoteRepository(remoteType, remoteURL, tempDir string) (string, error) { + d := downloader.NewDownloader() + path, err := d.DownloadWithType(remoteType, remoteURL, tempDir) + if err == downloader.ErrEmptyURLType { + // url and type empty, proceed with regular scanning + zap.S().Debugf("remote url and type not configured, proceeding with regular scanning") + } else if err != nil { + // some error while downloading remote repository + return path, err + } + return path, nil +} + +func writeResults(results runtime.Output, useColors, verbose, configOnly bool, format string) error { // add verbose flag to the scan summary results.Violations.ViolationStore.Summary.ShowViolationDetails = verbose @@ -79,16 +103,11 @@ func Run(iacType, iacVersion string, cloudType []string, // if --config-only flag is set, then exit with an error // asking the user to use yaml or json output format if strings.EqualFold(format, humanOutputFormat) { - zap.S().Error("please use yaml or json output format when using --config-only flag") - return + return errors.New("please use yaml or json output format when using --config-only flag") } writer.Write(format, results.ResourceConfig, outputWriter) } else { writer.Write(format, results.Violations, outputWriter) } - - if results.Violations.ViolationStore.Summary.ViolatedPolicies != 0 && flag.Lookup("test.v") == nil { - os.RemoveAll(tempDir) - os.Exit(3) - } + return nil } diff --git a/pkg/cli/run_test.go b/pkg/cli/run_test.go index f2d0f28a5..589f2d453 100644 --- a/pkg/cli/run_test.go +++ b/pkg/cli/run_test.go @@ -17,7 +17,15 @@ package cli import ( + "os" + "path/filepath" "testing" + + "github.com/accurics/terrascan/pkg/iac-providers/output" + "github.com/accurics/terrascan/pkg/policy" + "github.com/accurics/terrascan/pkg/results" + "github.com/accurics/terrascan/pkg/runtime" + "github.com/accurics/terrascan/pkg/utils" ) func TestRun(t *testing.T) { @@ -79,3 +87,114 @@ func TestRun(t *testing.T) { }) } } + +func TestWriteResults(t *testing.T) { + testInput := runtime.Output{ + ResourceConfig: output.AllResourceConfigs{}, + Violations: policy.EngineOutput{ + ViolationStore: &results.ViolationStore{}, + }, + } + type args struct { + results runtime.Output + useColors bool + verbose bool + configOnly bool + format string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "config only true with human readable output format", + args: args{ + results: testInput, + configOnly: true, + format: "human", + }, + wantErr: true, + }, + { + name: "config only true with non human readable output format", + args: args{ + results: testInput, + configOnly: true, + format: "json", + }, + wantErr: false, + }, + { + name: "config only false", + args: args{ + results: testInput, + configOnly: false, + format: "human", + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := writeResults(tt.args.results, tt.args.useColors, tt.args.verbose, tt.args.configOnly, tt.args.format); (err != nil) != tt.wantErr { + t.Errorf("writeResults() error = gotErr: %v, wantErr: %v", err, tt.wantErr) + } + }) + } +} + +func TestDownloadRemoteRepository(t *testing.T) { + testTempdir := filepath.Join(os.TempDir(), utils.GenRandomString(6)) + + type args struct { + remoteType string + remoteURL string + tempDir string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "blank input paramters", + args: args{ + remoteType: "", + remoteURL: "", + tempDir: "", + }, + }, + { + name: "invalid input parameters", + args: args{ + remoteType: "test", + remoteURL: "test", + tempDir: "test", + }, + wantErr: true, + }, + { + name: "valid inputs paramters", + args: args{ + remoteType: "git", + remoteURL: "github.com/accurics/terrascan", + tempDir: testTempdir, + }, + want: testTempdir, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := downloadRemoteRepository(tt.args.remoteType, tt.args.remoteURL, tt.args.tempDir) + if (err != nil) != tt.wantErr { + t.Errorf("downloadRemoteRepository() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("downloadRemoteRepository() = %v, want %v", got, tt.want) + } + }) + } +} From c79dce5268460ef0d94b7ce4f8a95a218668a409 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Wed, 16 Dec 2020 15:17:13 +0530 Subject: [PATCH 17/17] fix output format --- pkg/cli/run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/run.go b/pkg/cli/run.go index b0918f068..629f26603 100644 --- a/pkg/cli/run.go +++ b/pkg/cli/run.go @@ -67,7 +67,7 @@ func Run(iacType, iacVersion string, cloudType []string, } // write results to console - err = writeResults(results, useColors, verbose, configOnly, humanOutputFormat) + err = writeResults(results, useColors, verbose, configOnly, format) if err != nil { zap.S().Error("failed to write results", zap.Error(err)) return