From 26120dc6641330824b65ac9cb6590b2276c84f0d Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 22 Dec 2020 21:40:39 +0530 Subject: [PATCH 1/7] initial changes for rule id based scan and skip --- pkg/cli/run.go | 11 +++++--- pkg/cli/scan.go | 2 ++ pkg/http-server/file-scan.go | 4 +-- pkg/http-server/remote-repo.go | 2 +- pkg/policy/interface.go | 3 ++- pkg/policy/opa/engine.go | 48 +++++++++++++++++++++++++++++++--- pkg/runtime/executor.go | 19 ++++++++++++-- pkg/runtime/executor_test.go | 2 +- 8 files changed, 78 insertions(+), 13 deletions(-) diff --git a/pkg/cli/run.go b/pkg/cli/run.go index 9bac751b2..0264d6b8f 100644 --- a/pkg/cli/run.go +++ b/pkg/cli/run.go @@ -75,6 +75,12 @@ type ScanOptions struct { UseColors bool useColors string // used for flag processing + // ScanRules is the array of rules to scan + scanRules []string + + // SkipRules is the array of rules to skip while scanning + skipRules []string + // Verbose indicates whether to display all fields in default human readlbe output Verbose bool } @@ -101,8 +107,7 @@ func (s *ScanOptions) Scan() error { //Init initalises and validates ScanOptions func (s *ScanOptions) Init() error { s.initColor() - err := s.validate() - if err != nil { + if err := s.validate(); err != nil { zap.S().Error("failed to start scan", zap.Error(err)) return err } @@ -162,7 +167,7 @@ func (s *ScanOptions) Run() error { // create a new runtime executor for processing IaC executor, err := runtime.NewExecutor(s.iacType, s.iacVersion, s.policyType, - s.iacFilePath, s.iacDirPath, s.configFile, s.policyPath) + s.iacFilePath, s.iacDirPath, s.configFile, s.policyPath, s.scanRules, s.skipRules) if err != nil { return err } diff --git a/pkg/cli/scan.go b/pkg/cli/scan.go index 4ff3d0586..23aa3c482 100644 --- a/pkg/cli/scan.go +++ b/pkg/cli/scan.go @@ -59,5 +59,7 @@ func init() { // flag passes a string, but we normalize to bool in PreRun scanCmd.Flags().StringVar(&scanOptions.useColors, "use-colors", "auto", "color output (auto, t, f)") scanCmd.Flags().BoolVarP(&scanOptions.Verbose, "verbose", "v", false, "will show violations with details (applicable for default output)") + scanCmd.Flags().StringSliceVarP(&scanOptions.scanRules, "scan-rules", "", []string{}, "one or more rules to scan (example: --scan-rules=\"ruleID1,ruleID2\")") + scanCmd.Flags().StringSliceVarP(&scanOptions.skipRules, "skip-rules", "", []string{}, "one or more rules to skip while scanning (example: --skip-rules=\"ruleID1,ruleID2\")") RegisterCommand(rootCmd, scanCmd) } diff --git a/pkg/http-server/file-scan.go b/pkg/http-server/file-scan.go index 60fc02c57..b3a8d01f3 100644 --- a/pkg/http-server/file-scan.go +++ b/pkg/http-server/file-scan.go @@ -86,10 +86,10 @@ func (g *APIHandler) scanFile(w http.ResponseWriter, r *http.Request) { var executor *runtime.Executor if g.test { executor, err = runtime.NewExecutor(iacType, iacVersion, cloudType, - tempFile.Name(), "", "", []string{"./testdata/testpolicies"}) + tempFile.Name(), "", "", []string{"./testdata/testpolicies"}, []string{}, []string{}) } else { executor, err = runtime.NewExecutor(iacType, iacVersion, cloudType, - tempFile.Name(), "", "", []string{}) + tempFile.Name(), "", "", []string{}, []string{}, []string{}) } if err != nil { zap.S().Error(err) diff --git a/pkg/http-server/remote-repo.go b/pkg/http-server/remote-repo.go index efe57eca1..072ddc271 100644 --- a/pkg/http-server/remote-repo.go +++ b/pkg/http-server/remote-repo.go @@ -112,7 +112,7 @@ func (s *scanRemoteRepoReq) ScanRemoteRepo(iacType, iacVersion string, cloudType // create a new runtime executor for scanning the remote repo executor, err := runtime.NewExecutor(iacType, iacVersion, cloudType, - "", iacDirPath, "", policyPath) + "", iacDirPath, "", policyPath, []string{}, []string{}) if err != nil { zap.S().Error(err) return output, err diff --git a/pkg/policy/interface.go b/pkg/policy/interface.go index 30162b724..7296f8b9c 100644 --- a/pkg/policy/interface.go +++ b/pkg/policy/interface.go @@ -18,7 +18,8 @@ package policy // Engine Policy Engine interface type Engine interface { - Init(string) error + // to initialize engine with policy path, scan and skip rules + Init(string, []string, []string) error Configure() error Evaluate(EngineInput) (EngineOutput, error) GetResults() EngineOutput diff --git a/pkg/policy/opa/engine.go b/pkg/policy/opa/engine.go index b849e1e4c..6d380554c 100644 --- a/pkg/policy/opa/engine.go +++ b/pkg/policy/opa/engine.go @@ -46,13 +46,13 @@ var ( ) // NewEngine returns a new OPA policy engine -func NewEngine(policyPath string) (*Engine, error) { +func NewEngine(policyPath string, scanRules, skipRules []string) (*Engine, error) { // opa engine struct engine := &Engine{} // initialize the engine - if err := engine.Init(policyPath); err != nil { + if err := engine.Init(policyPath, scanRules, skipRules); err != nil { zap.S().Error("failed to initialize OPA policy engine", zap.Error(err)) return engine, errInitFailed } @@ -249,7 +249,7 @@ func (e *Engine) CompileRegoFiles() error { // Init initializes the Opa engine // Handles loading all rules, filtering, compiling, and preparing for evaluation -func (e *Engine) Init(policyPath string) error { +func (e *Engine) Init(policyPath string, scanRules, skipRules []string) error { e.context = context.Background() if err := e.LoadRegoFiles(policyPath); err != nil { @@ -257,6 +257,12 @@ func (e *Engine) Init(policyPath string) error { return err } + // before compiling the rego files, filter the rules based on scan and skip rules supplied + filterRules(e, policyPath, scanRules, skipRules) + + // update the rule count + e.stats.ruleCount = len(e.regoDataMap) + err := e.CompileRegoFiles() if err != nil { zap.S().Error("error compiling rego files", zap.String("policy path", policyPath), zap.Error(err)) @@ -395,3 +401,39 @@ func (e *Engine) Evaluate(engineInput policy.EngineInput) (policy.EngineOutput, e.results.ViolationStore.Summary.TotalPolicies += e.stats.ruleCount return e.results, nil } + +func filterRules(e *Engine, policyPath string, scanRules, skipRules []string) { + // before compilation, apply scan rules and skip rules + if len(scanRules) > 0 { + // temporary map to store data from original rego data map + tempMap := make(map[string]*RegoData) + for _, ruleID := range scanRules { + regoData, ok := e.regoDataMap[ruleID] + if ok { + zap.S().Infof("scan rules updated. rule id: %+v found in policy path: %s", ruleID, policyPath) + tempMap[ruleID] = regoData + } else { + zap.S().Warnf("rule id: %+v not found in policy path: %s", ruleID, policyPath) + } + } + if len(tempMap) == 0 { + zap.S().Warnf("rule ID's: %+v not found in policy path: %s", scanRules, policyPath) + } + + // the regoDataMap should only contain regoData for supplied scan rules + e.regoDataMap = tempMap + } + + if len(skipRules) > 0 { + // remove rules to be skipped from the rego data map + for _, ruleID := range skipRules { + _, ok := e.regoDataMap[ruleID] + if ok { + zap.S().Infof("skip rules updated. rule id: %+v found in policy path: %s", ruleID, policyPath) + delete(e.regoDataMap, ruleID) + } else { + zap.S().Warnf("rule id: %+v not found in policy path: %s", ruleID, policyPath) + } + } + } +} diff --git a/pkg/runtime/executor.go b/pkg/runtime/executor.go index 6357ff084..92c95a5ea 100644 --- a/pkg/runtime/executor.go +++ b/pkg/runtime/executor.go @@ -34,13 +34,15 @@ type Executor struct { iacType string iacVersion string configFile string + scanRules []string + skipRules []string iacProvider iacProvider.IacProvider policyEngine []policy.Engine notifiers []notifications.Notifier } // NewExecutor creates a runtime object -func NewExecutor(iacType, iacVersion string, cloudType []string, filePath, dirPath, configFile string, policyPath []string) (e *Executor, err error) { +func NewExecutor(iacType, iacVersion string, cloudType []string, filePath, dirPath, configFile string, policyPath, scanRules, skipRules []string) (e *Executor, err error) { e = &Executor{ filePath: filePath, dirPath: dirPath, @@ -49,6 +51,8 @@ func NewExecutor(iacType, iacVersion string, cloudType []string, filePath, dirPa iacType: iacType, iacVersion: iacVersion, configFile: configFile, + scanRules: scanRules, + skipRules: skipRules, } // initialize executor @@ -68,6 +72,11 @@ func (e *Executor) Init() error { return err } + // read config file and update scan and skip rules + if err := e.initScanAndSkipRules(); err != nil { + return err + } + // create new IacProvider e.iacProvider, err = iacProvider.NewIacProvider(e.iacType, e.iacVersion) if err != nil { @@ -85,7 +94,7 @@ func (e *Executor) Init() error { // create a new policy engine based on IaC type zap.S().Debugf("using policy path %v", e.policyPath) for _, policyPath := range e.policyPath { - engine, err := opa.NewEngine(policyPath) + engine, err := opa.NewEngine(policyPath, e.scanRules, e.skipRules) if err != nil { zap.S().Errorf("failed to create policy engine. error: '%s'", err) return err @@ -139,3 +148,9 @@ func (e *Executor) Execute() (results Output, err error) { // successful return results, nil } + +// yet to implement based on file type +func (e *Executor) initScanAndSkipRules() error { + + return nil +} diff --git a/pkg/runtime/executor_test.go b/pkg/runtime/executor_test.go index 303732242..df6f44d83 100644 --- a/pkg/runtime/executor_test.go +++ b/pkg/runtime/executor_test.go @@ -55,7 +55,7 @@ type MockPolicyEngine struct { err error } -func (m MockPolicyEngine) Init(input string) error { +func (m MockPolicyEngine) Init(input string, scanRules, skipRules []string) error { return m.err } From 2ca27b9345ef6b9ffae596b51cb3dfcb493bcc63 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Wed, 23 Dec 2020 14:20:19 +0530 Subject: [PATCH 2/7] 1. toml config file based rule skipping 2. tests around rule scanning and skipping --- config/terrascan.toml | 15 ++ pkg/cli/run_test.go | 32 ++++ pkg/notifications/notifiers.go | 8 +- pkg/notifications/notifiers_test.go | 4 +- pkg/policy/opa/engine.go | 10 +- pkg/policy/opa/engine_test.go | 97 ++++++++++ pkg/runtime/executor.go | 79 +++++++- pkg/runtime/executor_test.go | 173 +++++++++++++++++- pkg/runtime/testdata/empty.toml | 0 .../testdata/invalid-scan-skip-rules.toml | 10 + pkg/runtime/testdata/invalid-skip-rules.toml | 9 + pkg/runtime/testdata/scan-skip-rules.toml | 11 ++ pkg/runtime/testdata/webhook.toml | 2 + 13 files changed, 436 insertions(+), 14 deletions(-) create mode 100644 pkg/runtime/testdata/empty.toml create mode 100644 pkg/runtime/testdata/invalid-scan-skip-rules.toml create mode 100644 pkg/runtime/testdata/invalid-skip-rules.toml create mode 100644 pkg/runtime/testdata/scan-skip-rules.toml diff --git a/config/terrascan.toml b/config/terrascan.toml index d1af36565..045087c05 100644 --- a/config/terrascan.toml +++ b/config/terrascan.toml @@ -4,3 +4,18 @@ [notifications] [notifications.webhook] url = "https://httpbin.org/post" + +# scan and skip rules configuration +[rules] + # scan rules (list of rules to scan) + # adding rules here will override rules in the policy path + scan-rules = [ + "AWS.S3Bucket.DS.High.1043", + "AWS.S3Bucket.IAM.High.0370" + ] + + # skip rules (list of rules to skip) + skip-rules = [ + "AWS.S3Bucket.DS.High.1043", + "AWS.S3Bucket.IAM.High.0370", + ] \ No newline at end of file diff --git a/pkg/cli/run_test.go b/pkg/cli/run_test.go index 7b3574286..fc06ab12d 100644 --- a/pkg/cli/run_test.go +++ b/pkg/cli/run_test.go @@ -17,6 +17,7 @@ package cli import ( + "fmt" "os" "path/filepath" "testing" @@ -133,10 +134,41 @@ func TestRun(t *testing.T) { }, wantErr: true, }, + { + name: "incorrect config file", + scanOptions: &ScanOptions{ + policyType: []string{"all"}, + iacDirPath: testTerraformFilePath, + outputType: "json", + configFile: "invalidFile", + }, + wantErr: true, + }, + { + name: "run with skip rules", + scanOptions: &ScanOptions{ + policyType: []string{"all"}, + iacDirPath: testDirPath, + outputType: "json", + skipRules: []string{"AWS.ECR.DataSecurity.High.0579", "AWS.SecurityGroup.NetworkPortsSecurity.Low.0561"}, + }, + }, + { + name: "run with scan rules", + scanOptions: &ScanOptions{ + policyType: []string{"all"}, + iacDirPath: testDirPath, + outputType: "yaml", + scanRules: []string{"AWS.ECR.DataSecurity.High.0579", "AWS.SecurityGroup.NetworkPortsSecurity.Low.0561"}, + }, + }, } for _, tt := range table { t.Run(tt.name, func(t *testing.T) { + if tt.name == "run with scan rules" { + fmt.Println() + } err := tt.scanOptions.Run() if (err != nil) != tt.wantErr { t.Errorf("ScanOptions.Run() error = %v, wantErr %v", err, tt.wantErr) diff --git a/pkg/notifications/notifiers.go b/pkg/notifications/notifiers.go index 76d83d279..f6227d245 100644 --- a/pkg/notifications/notifiers.go +++ b/pkg/notifications/notifiers.go @@ -32,7 +32,9 @@ const ( var ( errNotifierNotSupported = fmt.Errorf("notifier not supported") - errTomlKeyNotPresent = fmt.Errorf("key not present in toml config") + + // ErrTomlKeyNotPresent will be returned when config file does not have notificationsConfigKey + ErrTomlKeyNotPresent = fmt.Errorf("key not present in toml config") ) // NewNotifier returns a new notifier @@ -63,7 +65,7 @@ func NewNotifiers(configFile string) ([]Notifier, error) { keyConfig := config.Get(notificationsConfigKey) if keyConfig == nil { zap.S().Infof("key '%s' not present in toml config", notificationsConfigKey) - return notifiers, errTomlKeyNotPresent + return notifiers, ErrTomlKeyNotPresent } // get all the notifier types configured in TOML config @@ -84,7 +86,7 @@ func NewNotifiers(configFile string) ([]Notifier, error) { nTypeConfig := keyTomlConfig.Get(nType) if nTypeConfig.(*toml.Tree).String() == "" { zap.S().Errorf("notifier '%v' config not present", nType) - allErrs = utils.WrapError(errTomlKeyNotPresent, allErrs) + allErrs = utils.WrapError(ErrTomlKeyNotPresent, allErrs) continue } diff --git a/pkg/notifications/notifiers_test.go b/pkg/notifications/notifiers_test.go index bc43f8279..ffaca96a2 100644 --- a/pkg/notifications/notifiers_test.go +++ b/pkg/notifications/notifiers_test.go @@ -62,7 +62,7 @@ func TestNewNotifiers(t *testing.T) { { name: "key not present", configFile: "testdata/nokey.toml", - wantErr: errTomlKeyNotPresent, + wantErr: ErrTomlKeyNotPresent, }, { name: "invalid notifier", @@ -72,7 +72,7 @@ func TestNewNotifiers(t *testing.T) { { name: "empty notifier config", configFile: "testdata/empty-notifier-config.toml", - wantErr: errTomlKeyNotPresent, + wantErr: ErrTomlKeyNotPresent, }, { name: "invalid notifier config", diff --git a/pkg/policy/opa/engine.go b/pkg/policy/opa/engine.go index 6d380554c..7aaeff397 100644 --- a/pkg/policy/opa/engine.go +++ b/pkg/policy/opa/engine.go @@ -410,14 +410,14 @@ func filterRules(e *Engine, policyPath string, scanRules, skipRules []string) { for _, ruleID := range scanRules { regoData, ok := e.regoDataMap[ruleID] if ok { - zap.S().Infof("scan rules updated. rule id: %+v found in policy path: %s", ruleID, policyPath) + zap.S().Infof("scan rule added. rule id: %+v found in policy path: %s", ruleID, policyPath) tempMap[ruleID] = regoData } else { - zap.S().Warnf("rule id: %+v not found in policy path: %s", ruleID, policyPath) + zap.S().Warnf("scan rule id: %+v not found in policy path: %s", ruleID, policyPath) } } if len(tempMap) == 0 { - zap.S().Warnf("rule ID's: %+v not found in policy path: %s", scanRules, policyPath) + zap.S().Warnf("scan rule id's: %+v not found in policy path: %s", scanRules, policyPath) } // the regoDataMap should only contain regoData for supplied scan rules @@ -429,10 +429,10 @@ func filterRules(e *Engine, policyPath string, scanRules, skipRules []string) { for _, ruleID := range skipRules { _, ok := e.regoDataMap[ruleID] if ok { - zap.S().Infof("skip rules updated. rule id: %+v found in policy path: %s", ruleID, policyPath) + zap.S().Infof("skip rule added. rule id: %+v found in policy path: %s", ruleID, policyPath) delete(e.regoDataMap, ruleID) } else { - zap.S().Warnf("rule id: %+v not found in policy path: %s", ruleID, policyPath) + zap.S().Warnf("skip rule id: %+v not found in policy path: %s", ruleID, policyPath) } } } diff --git a/pkg/policy/opa/engine_test.go b/pkg/policy/opa/engine_test.go index f0447de73..d7ceb6585 100644 --- a/pkg/policy/opa/engine_test.go +++ b/pkg/policy/opa/engine_test.go @@ -1 +1,98 @@ package opa + +import ( + "fmt" + "testing" +) + +func TestFilterRules(t *testing.T) { + testPolicyPath := "test" + + type args struct { + e *Engine + policyPath string + scanRules []string + skipRules []string + } + tests := []struct { + name string + args args + assert bool + regoMapSize int + }{ + { + name: "no scan and skip rules", + args: args{ + e: &Engine{}, + }, + }, + { + name: "scan rules test", + args: args{ + e: &Engine{ + regoDataMap: getTestRegoDataMap(10), + }, + policyPath: testPolicyPath, + scanRules: []string{"Rule.0", "Rule.1", "Rule.2", "Rule.3", "Rule.11"}, + }, + assert: true, + regoMapSize: 4, + }, + { + name: "scan rules not found in path", + args: args{ + e: &Engine{ + regoDataMap: getTestRegoDataMap(10), + }, + policyPath: testPolicyPath, + scanRules: []string{"Rule.11", "Rule.12", "Rule.13"}, + }, + assert: true, + regoMapSize: 0, + }, + { + name: "skip rules test", + args: args{ + e: &Engine{ + regoDataMap: getTestRegoDataMap(6), + }, + policyPath: testPolicyPath, + skipRules: []string{"Rule.1"}, + }, + assert: true, + regoMapSize: 5, + }, + { + name: "skip rules not found in policy path", + args: args{ + e: &Engine{ + regoDataMap: getTestRegoDataMap(20), + }, + policyPath: testPolicyPath, + skipRules: []string{"Rule.21", "Rule.22"}, + }, + assert: true, + regoMapSize: 20, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filterRules(tt.args.e, tt.args.policyPath, tt.args.scanRules, tt.args.skipRules) + if tt.assert { + if len(tt.args.e.regoDataMap) != tt.regoMapSize { + t.Errorf("filterRules(): expected regoDataMap size = %d, got = %d", tt.regoMapSize, len(tt.args.e.regoDataMap)) + } + } + }) + } +} + +// helper func to generate test rego data map of given size +func getTestRegoDataMap(size int) map[string]*RegoData { + testRegoDataMap := make(map[string]*RegoData) + for i := 0; i < size; i++ { + ruleID := fmt.Sprintf("Rule.%d", i) + testRegoDataMap[ruleID] = &RegoData{} + } + return testRegoDataMap +} diff --git a/pkg/runtime/executor.go b/pkg/runtime/executor.go index 92c95a5ea..7e84d53a8 100644 --- a/pkg/runtime/executor.go +++ b/pkg/runtime/executor.go @@ -17,12 +17,27 @@ package runtime import ( + "fmt" + "go.uber.org/zap" + "github.com/accurics/terrascan/pkg/config" iacProvider "github.com/accurics/terrascan/pkg/iac-providers" "github.com/accurics/terrascan/pkg/notifications" "github.com/accurics/terrascan/pkg/policy" opa "github.com/accurics/terrascan/pkg/policy/opa" + "github.com/pelletier/go-toml" +) + +const ( + rulesKey = "rules" + scanRulesKey = "scan-rules" + skipRulesKey = "skip-rules" +) + +var ( + errRuleNotString = fmt.Errorf("each scan and skip rule must be string") + errIncorrectValueForRules = fmt.Errorf("'scan-rules' and 'skip-rules' must be an array") ) // Executor object @@ -74,7 +89,9 @@ func (e *Executor) Init() error { // read config file and update scan and skip rules if err := e.initScanAndSkipRules(); err != nil { - return err + if !(err == errIncorrectValueForRules || err == errRuleNotString) { + return err + } } // create new IacProvider @@ -88,7 +105,10 @@ func (e *Executor) Init() error { e.notifiers, err = notifications.NewNotifiers(e.configFile) if err != nil { zap.S().Errorf("failed to create notifier(s). error: '%s'", err) - return err + // do not return an error if a key is not present in the config file + if err != notifications.ErrTomlKeyNotPresent { + return err + } } // create a new policy engine based on IaC type @@ -149,8 +169,61 @@ func (e *Executor) Execute() (results Output, err error) { return results, nil } -// yet to implement based on file type +// read the config file and update scan and skip rules func (e *Executor) initScanAndSkipRules() error { + if e.configFile != "" { + configData, err := config.LoadConfig(e.configFile) + if err != nil { + zap.S().Error("error loading config file", zap.Error(err)) + return err + } + if configData.Has("rules") { + // read scan rules in the toml tree + data := (configData.Get("rules")).(*toml.Tree) + scanRules, err := getRulesInTomlTree(data, e.configFile, "scan-rules") + if err != nil { + zap.S().Error("error reading config file", zap.Error(err)) + return err + } + if len(scanRules) > 0 { + e.scanRules = append(e.scanRules, scanRules...) + } else { + zap.S().Debugf("key 'scan-rules' not found in the config file: %s", e.configFile) + } + + // read skip rules in the toml tree + skipRules, err := getRulesInTomlTree(data, e.configFile, "skip-rules") + if err != nil { + zap.S().Error("error reading config file", zap.Error(err)) + return err + } + if len(skipRules) > 0 { + e.skipRules = append(e.skipRules, skipRules...) + } else { + zap.S().Debugf("key 'skip-rules' not found in the config file: %s", e.configFile) + } + } + } return nil } + +func getRulesInTomlTree(tree *toml.Tree, configFile, key string) ([]string, error) { + ruleSlice := make([]string, 0) + if tree.Has(key) { + rules, ok := (tree.Get(key)).([]interface{}) + if !ok { + zap.S().Errorf("key '%s' must be an array in the config file: %s", key, configFile) + return nil, errIncorrectValueForRules + } + for _, rule := range rules { + r, ok := rule.(string) + if !ok { + zap.S().Errorf("rules must be of type string for key: %s", key) + return nil, errRuleNotString + } + ruleSlice = append(ruleSlice, r) + } + } + return ruleSlice, nil +} diff --git a/pkg/runtime/executor_test.go b/pkg/runtime/executor_test.go index df6f44d83..d3e613894 100644 --- a/pkg/runtime/executor_test.go +++ b/pkg/runtime/executor_test.go @@ -28,6 +28,7 @@ import ( "github.com/accurics/terrascan/pkg/notifications" "github.com/accurics/terrascan/pkg/notifications/webhook" "github.com/accurics/terrascan/pkg/policy" + "github.com/pelletier/go-toml" ) var ( @@ -228,7 +229,7 @@ func TestInit(t *testing.T) { configFile: "./testdata/does-not-exist", }, wantErr: config.ErrNotPresent, - wantIacProvider: &tfv12.TfV12{}, + wantIacProvider: nil, }, { name: "invalid policy path", @@ -264,3 +265,173 @@ func TestInit(t *testing.T) { }) } } + +func TestGetRulesInTomlTree(t *testing.T) { + // test data + emptyTomlTree, err := config.LoadConfig("testdata/empty.toml") + if err != nil { + t.Fatalf("error while loading toml file %v", err) + } + + configFileData, err := config.LoadConfig("testdata/scan-skip-rules.toml") + if err != nil { + t.Fatalf("error while loading toml file %v", err) + } + validRulesFormat := (configFileData.Get("rules")).(*toml.Tree) + + configFileData, err = config.LoadConfig("testdata/invalid-scan-skip-rules.toml") + if err != nil { + t.Fatalf("error while loading toml file %v", err) + } + invalidRulesFormat := (configFileData.Get("rules")).(*toml.Tree) + + type args struct { + tree *toml.Tree + configFile string + key string + } + tests := []struct { + name string + args args + want []string + wantErr bool + }{ + { + name: "empty toml file", + args: args{ + tree: emptyTomlTree, + configFile: "", + key: scanRulesKey, + }, + want: []string{}, + }, + { + name: "get scan rules - valid data", + args: args{ + tree: validRulesFormat, + configFile: "", + key: scanRulesKey, + }, + want: []string{"AWS.S3Bucket.DS.High.1043", "accurics.kubernetes.IAM.107"}, + }, + { + name: "get skip rules - valid data", + args: args{ + tree: validRulesFormat, + configFile: "", + key: skipRulesKey, + }, + want: []string{"AWS.S3Bucket.IAM.High.0370", "accurics.kubernetes.IAM.5", + "accurics.kubernetes.OPS.461", "accurics.kubernetes.IAM.109"}, + }, + { + name: "get scan rules - invalid data", + args: args{ + tree: invalidRulesFormat, + configFile: "", + key: scanRulesKey, + }, + wantErr: true, + }, + { + name: "get skip rules - invalid data", + args: args{ + tree: invalidRulesFormat, + configFile: "", + key: skipRulesKey, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getRulesInTomlTree(tt.args.tree, tt.args.configFile, tt.args.key) + if (err != nil) != tt.wantErr { + t.Errorf("getRulesInTomlTree() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("getRulesInTomlTree() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestExecutor_initScanAndSkipRules(t *testing.T) { + assertionTestName := "valid config file with scan and skip rules" + + type fields struct { + configFile string + scanRules []string + skipRules []string + } + tests := []struct { + name string + fields fields + wantErr bool + }{ + { + name: "no config file", + fields: fields{}, + }, + { + name: "config file doesn't exist", + fields: fields{ + configFile: "testdata/test.toml", + }, + wantErr: true, + }, + { + name: "empty config file", + fields: fields{ + configFile: "testdata/empty.toml", + }, + }, + { + name: "config file with empty rules", + fields: fields{ + configFile: "testdata/webhook.toml", + }, + }, + { + name: assertionTestName, + fields: fields{ + configFile: "testdata/scan-skip-rules.toml", + scanRules: []string{"testRuleA", "testRuleB"}, + skipRules: []string{"testRuleC"}, + }, + }, + { + name: "valid config file with invalid scan rules", + fields: fields{ + configFile: "testdata/invalid-scan-skip-rules.toml", + }, + wantErr: true, + }, + { + name: "valid config file with invalid skip rules", + fields: fields{ + configFile: "testdata/invalid-skip-rules.toml", + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := &Executor{ + configFile: tt.fields.configFile, + scanRules: tt.fields.scanRules, + skipRules: tt.fields.skipRules, + } + if err := e.initScanAndSkipRules(); (err != nil) != tt.wantErr { + t.Errorf("Executor.initScanAndSkipRules() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.name == assertionTestName { + if len(e.scanRules) != 4 && len(e.skipRules) != 5 { + t.Errorf("Expected scanRules: %d and skipRules: %d, got scanRules: %d and skipRules: %d", 4, 5, len(e.scanRules), len(e.skipRules)) + } + } + }) + } +} diff --git a/pkg/runtime/testdata/empty.toml b/pkg/runtime/testdata/empty.toml new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/runtime/testdata/invalid-scan-skip-rules.toml b/pkg/runtime/testdata/invalid-scan-skip-rules.toml new file mode 100644 index 000000000..22c92d9fc --- /dev/null +++ b/pkg/runtime/testdata/invalid-scan-skip-rules.toml @@ -0,0 +1,10 @@ +# notifications configuration +[notifications] + [notifications.invalid] + url = "https://httpbin.org/post" +[rules] + scan-rules = "AWS.S3Bucket.DS.High.1043" + skip-rules = [ + 185, + 2008 + ] \ No newline at end of file diff --git a/pkg/runtime/testdata/invalid-skip-rules.toml b/pkg/runtime/testdata/invalid-skip-rules.toml new file mode 100644 index 000000000..2f82fda75 --- /dev/null +++ b/pkg/runtime/testdata/invalid-skip-rules.toml @@ -0,0 +1,9 @@ +# notifications configuration +[notifications] + [notifications.invalid] + url = "https://httpbin.org/post" +[rules] + skip-rules = [ + 185, + 2008 + ] \ No newline at end of file diff --git a/pkg/runtime/testdata/scan-skip-rules.toml b/pkg/runtime/testdata/scan-skip-rules.toml new file mode 100644 index 000000000..201b89af9 --- /dev/null +++ b/pkg/runtime/testdata/scan-skip-rules.toml @@ -0,0 +1,11 @@ +[rules] + scan-rules = [ + "AWS.S3Bucket.DS.High.1043", + "accurics.kubernetes.IAM.107" + ] + skip-rules = [ + "AWS.S3Bucket.IAM.High.0370", + "accurics.kubernetes.IAM.5", + "accurics.kubernetes.OPS.461", + "accurics.kubernetes.IAM.109" + ] \ No newline at end of file diff --git a/pkg/runtime/testdata/webhook.toml b/pkg/runtime/testdata/webhook.toml index d1af36565..11c29a2bb 100644 --- a/pkg/runtime/testdata/webhook.toml +++ b/pkg/runtime/testdata/webhook.toml @@ -4,3 +4,5 @@ [notifications] [notifications.webhook] url = "https://httpbin.org/post" +# empty rules +[rules] \ No newline at end of file From b6db0aed58ffb8776251f4a601fc7b47934a9796 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Wed, 23 Dec 2020 14:29:21 +0530 Subject: [PATCH 3/7] add test in run_test for scan and skip rules using config file --- pkg/cli/run_test.go | 18 ++++++++++++------ pkg/cli/testdata/configFile.toml | 7 +++++++ 2 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 pkg/cli/testdata/configFile.toml diff --git a/pkg/cli/run_test.go b/pkg/cli/run_test.go index fc06ab12d..6cee107ee 100644 --- a/pkg/cli/run_test.go +++ b/pkg/cli/run_test.go @@ -17,7 +17,6 @@ package cli import ( - "fmt" "os" "path/filepath" "testing" @@ -51,6 +50,7 @@ func TestRun(t *testing.T) { testDirPath := "testdata/run-test" kustomizeTestDirPath := testDirPath + "/kustomize-test" testTerraformFilePath := testDirPath + "/config-only.tf" + ruleSlice := []string{"AWS.ECR.DataSecurity.High.0579", "AWS.SecurityGroup.NetworkPortsSecurity.Low.0561"} table := []struct { name string @@ -150,7 +150,7 @@ func TestRun(t *testing.T) { policyType: []string{"all"}, iacDirPath: testDirPath, outputType: "json", - skipRules: []string{"AWS.ECR.DataSecurity.High.0579", "AWS.SecurityGroup.NetworkPortsSecurity.Low.0561"}, + skipRules: ruleSlice, }, }, { @@ -159,16 +159,22 @@ func TestRun(t *testing.T) { policyType: []string{"all"}, iacDirPath: testDirPath, outputType: "yaml", - scanRules: []string{"AWS.ECR.DataSecurity.High.0579", "AWS.SecurityGroup.NetworkPortsSecurity.Low.0561"}, + scanRules: ruleSlice, + }, + }, + { + name: "config file with rules", + scanOptions: &ScanOptions{ + policyType: []string{"all"}, + iacDirPath: testDirPath, + outputType: "yaml", + configFile: "testdata/configFile.toml", }, }, } for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - if tt.name == "run with scan rules" { - fmt.Println() - } err := tt.scanOptions.Run() if (err != nil) != tt.wantErr { t.Errorf("ScanOptions.Run() error = %v, wantErr %v", err, tt.wantErr) diff --git a/pkg/cli/testdata/configFile.toml b/pkg/cli/testdata/configFile.toml new file mode 100644 index 000000000..68e736a93 --- /dev/null +++ b/pkg/cli/testdata/configFile.toml @@ -0,0 +1,7 @@ +[rules] + scan-rules = [ + "AWS.ECR.DataSecurity.High.0579" + ] + skip-rules = [ + "AWS.SecurityGroup.NetworkPortsSecurity.Low.0561" + ] \ No newline at end of file From e0c2e07cef67f3fcfc7190a8f6ac0a6cbe1c629e Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Wed, 23 Dec 2020 15:06:29 +0530 Subject: [PATCH 4/7] 1. remove _ from test func 2. define variable for error string in test func --- pkg/runtime/executor_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/runtime/executor_test.go b/pkg/runtime/executor_test.go index d3e613894..97043be1a 100644 --- a/pkg/runtime/executor_test.go +++ b/pkg/runtime/executor_test.go @@ -268,22 +268,23 @@ func TestInit(t *testing.T) { func TestGetRulesInTomlTree(t *testing.T) { // test data + fileLoadErr := "error while loading toml file" emptyTomlTree, err := config.LoadConfig("testdata/empty.toml") if err != nil { - t.Fatalf("error while loading toml file %v", err) + t.Fatalf(fileLoadErr, err) } configFileData, err := config.LoadConfig("testdata/scan-skip-rules.toml") if err != nil { - t.Fatalf("error while loading toml file %v", err) + t.Fatalf(fileLoadErr, err) } - validRulesFormat := (configFileData.Get("rules")).(*toml.Tree) + validRulesFormat := (configFileData.Get(rulesKey)).(*toml.Tree) configFileData, err = config.LoadConfig("testdata/invalid-scan-skip-rules.toml") if err != nil { - t.Fatalf("error while loading toml file %v", err) + t.Fatalf(fileLoadErr, err) } - invalidRulesFormat := (configFileData.Get("rules")).(*toml.Tree) + invalidRulesFormat := (configFileData.Get(rulesKey)).(*toml.Tree) type args struct { tree *toml.Tree @@ -358,7 +359,7 @@ func TestGetRulesInTomlTree(t *testing.T) { } } -func TestExecutor_initScanAndSkipRules(t *testing.T) { +func TestExecutorInitScanAndSkipRules(t *testing.T) { assertionTestName := "valid config file with scan and skip rules" type fields struct { From 451e72cd38fdc454dbf3224428e6277f9ec91dfc Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Wed, 23 Dec 2020 19:24:50 +0530 Subject: [PATCH 5/7] fix code smells --- pkg/policy/opa/engine.go | 64 +++++++++++++++++++++++----------------- pkg/runtime/executor.go | 42 +++++++++++++++----------- 2 files changed, 61 insertions(+), 45 deletions(-) diff --git a/pkg/policy/opa/engine.go b/pkg/policy/opa/engine.go index 7aaeff397..fd4e108c0 100644 --- a/pkg/policy/opa/engine.go +++ b/pkg/policy/opa/engine.go @@ -403,37 +403,47 @@ func (e *Engine) Evaluate(engineInput policy.EngineInput) (policy.EngineOutput, } func filterRules(e *Engine, policyPath string, scanRules, skipRules []string) { - // before compilation, apply scan rules and skip rules + // apply scan rules if len(scanRules) > 0 { - // temporary map to store data from original rego data map - tempMap := make(map[string]*RegoData) - for _, ruleID := range scanRules { - regoData, ok := e.regoDataMap[ruleID] - if ok { - zap.S().Infof("scan rule added. rule id: %+v found in policy path: %s", ruleID, policyPath) - tempMap[ruleID] = regoData - } else { - zap.S().Warnf("scan rule id: %+v not found in policy path: %s", ruleID, policyPath) - } - } - if len(tempMap) == 0 { - zap.S().Warnf("scan rule id's: %+v not found in policy path: %s", scanRules, policyPath) - } - - // the regoDataMap should only contain regoData for supplied scan rules - e.regoDataMap = tempMap + filterScanRules(e, policyPath, scanRules) } + // apply skip rules if len(skipRules) > 0 { - // remove rules to be skipped from the rego data map - for _, ruleID := range skipRules { - _, ok := e.regoDataMap[ruleID] - if ok { - zap.S().Infof("skip rule added. rule id: %+v found in policy path: %s", ruleID, policyPath) - delete(e.regoDataMap, ruleID) - } else { - zap.S().Warnf("skip rule id: %+v not found in policy path: %s", ruleID, policyPath) - } + filterSkipRules(e, policyPath, skipRules) + } +} + +func filterScanRules(e *Engine, policyPath string, scanRules []string) { + + // temporary map to store data from original rego data map + tempMap := make(map[string]*RegoData) + for _, ruleID := range scanRules { + regoData, ok := e.regoDataMap[ruleID] + if ok { + zap.S().Infof("scan rule added. rule id: %+v found in policy path: %s", ruleID, policyPath) + tempMap[ruleID] = regoData + } else { + zap.S().Warnf("scan rule id: %+v not found in policy path: %s", ruleID, policyPath) + } + } + if len(tempMap) == 0 { + zap.S().Warnf("scan rule id's: %+v not found in policy path: %s", scanRules, policyPath) + } + + // the regoDataMap should only contain regoData for supplied scan rules + e.regoDataMap = tempMap +} + +func filterSkipRules(e *Engine, policyPath string, skipRules []string) { + // remove rules to be skipped from the rego data map + for _, ruleID := range skipRules { + _, ok := e.regoDataMap[ruleID] + if ok { + zap.S().Infof("skip rule added. rule id: %+v found in policy path: %s", ruleID, policyPath) + delete(e.regoDataMap, ruleID) + } else { + zap.S().Warnf("skip rule id: %+v not found in policy path: %s", ruleID, policyPath) } } } diff --git a/pkg/runtime/executor.go b/pkg/runtime/executor.go index 7e84d53a8..a50f67b3c 100644 --- a/pkg/runtime/executor.go +++ b/pkg/runtime/executor.go @@ -178,36 +178,42 @@ func (e *Executor) initScanAndSkipRules() error { return err } - if configData.Has("rules") { + if configData.Has(rulesKey) { + + data := (configData.Get(rulesKey)).(*toml.Tree) + // read scan rules in the toml tree - data := (configData.Get("rules")).(*toml.Tree) - scanRules, err := getRulesInTomlTree(data, e.configFile, "scan-rules") - if err != nil { - zap.S().Error("error reading config file", zap.Error(err)) + if err := initRules(e, data, scanRulesKey); err != nil { return err } - if len(scanRules) > 0 { - e.scanRules = append(e.scanRules, scanRules...) - } else { - zap.S().Debugf("key 'scan-rules' not found in the config file: %s", e.configFile) - } // read skip rules in the toml tree - skipRules, err := getRulesInTomlTree(data, e.configFile, "skip-rules") - if err != nil { - zap.S().Error("error reading config file", zap.Error(err)) + if err := initRules(e, data, skipRulesKey); err != nil { return err } - if len(skipRules) > 0 { - e.skipRules = append(e.skipRules, skipRules...) - } else { - zap.S().Debugf("key 'skip-rules' not found in the config file: %s", e.configFile) - } } } return nil } +func initRules(e *Executor, tree *toml.Tree, key string) error { + rules, err := getRulesInTomlTree(tree, e.configFile, key) + if err != nil { + zap.S().Error("error reading config file", zap.Error(err)) + return err + } + if len(rules) > 0 { + if key == scanRulesKey { + e.scanRules = append(e.scanRules, rules...) + } else { + e.skipRules = append(e.skipRules, rules...) + } + } else { + zap.S().Debugf("key '%s' not found in the config file: %s", key, e.configFile) + } + return nil +} + func getRulesInTomlTree(tree *toml.Tree, configFile, key string) ([]string, error) { ruleSlice := make([]string, 0) if tree.Has(key) { From 02121802539a8038a4ae0a54d2d7fe6fdb11b2f2 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Mon, 28 Dec 2020 17:48:31 +0530 Subject: [PATCH 6/7] 1. support skip and scan rules in the server mode 2. refactor the existing config reader code 3. update unit tests 4. incorporate PR review comments --- pkg/config/config-reader.go | 84 +++++++++ pkg/config/config-reader_test.go | 136 ++++++++++++++ pkg/config/configfile.go | 60 ------ pkg/config/global.go | 84 +++------ pkg/config/global_test.go | 87 ++++++--- pkg/config/testdata/invalid.toml | 1 + .../testdata/terrascan-config-all-fields.toml | 23 +++ pkg/config/types.go | 34 +++- pkg/http-server/file-scan.go | 19 +- pkg/http-server/file-scan_test.go | 83 ++++++--- pkg/http-server/remote-repo.go | 10 +- pkg/http-server/remote-repo_test.go | 46 +++-- pkg/notifications/notifiers.go | 45 +++-- pkg/notifications/notifiers_test.go | 14 +- .../testdata/invalid-notifier-config.toml | 1 + .../testdata/invalid-notifier-type.toml | 1 + .../testdata/valid-notifier-config.toml | 10 + pkg/notifications/webhook/webhook.go | 23 ++- pkg/notifications/webhook/webhook_test.go | 86 +++++++++ pkg/policy/interface.go | 3 +- pkg/policy/opa/engine.go | 25 +-- pkg/policy/opa/engine_test.go | 45 +++-- pkg/runtime/executor.go | 98 ++-------- pkg/runtime/executor_test.go | 175 +----------------- pkg/runtime/rules.go | 30 +++ pkg/runtime/rules_test.go | 87 +++++++++ pkg/runtime/testdata/invalid-notifier.toml | 1 + pkg/runtime/testdata/webhook.toml | 2 + 28 files changed, 794 insertions(+), 519 deletions(-) create mode 100644 pkg/config/config-reader.go create mode 100644 pkg/config/config-reader_test.go delete mode 100644 pkg/config/configfile.go create mode 100644 pkg/config/testdata/invalid.toml create mode 100644 pkg/config/testdata/terrascan-config-all-fields.toml create mode 100644 pkg/notifications/testdata/valid-notifier-config.toml create mode 100644 pkg/notifications/webhook/webhook_test.go create mode 100644 pkg/runtime/rules.go create mode 100644 pkg/runtime/rules_test.go diff --git a/pkg/config/config-reader.go b/pkg/config/config-reader.go new file mode 100644 index 000000000..f29ea2bfa --- /dev/null +++ b/pkg/config/config-reader.go @@ -0,0 +1,84 @@ +/* + 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 config + +import ( + "fmt" + "io/ioutil" + "os" + + "github.com/pelletier/go-toml" + "go.uber.org/zap" +) + +var ( + // ErrTomlLoadConfig indicates error: Failed to load toml config + errTomlLoadConfig = fmt.Errorf("failed to load toml config") + // ErrNotPresent indicates error: Config file not present + ErrNotPresent = fmt.Errorf("config file not present") +) + +// TerrascanConfigReader holds the terrascan config file name +type TerrascanConfigReader struct { + config TerrascanConfig +} + +// NewTerrascanConfigReader initialises and returns a config reader +func NewTerrascanConfigReader(fileName string) (*TerrascanConfigReader, error) { + config := TerrascanConfig{} + configReader := new(TerrascanConfigReader) + configReader.config = config + + // empty file name check should be done by the caller, this is a safe check + if fileName == "" { + zap.S().Debug("no config file specified") + return configReader, nil + } + + // return error if file doesn't exist + _, err := os.Stat(fileName) + if err != nil { + zap.S().Debugf("config file: %s, doesn't exist", fileName) + return configReader, ErrNotPresent + } + + data, err := ioutil.ReadFile(fileName) + if err != nil { + zap.S().Debugf("error loading config file", zap.Error(err)) + return configReader, errTomlLoadConfig + } + + if err = toml.Unmarshal(data, &configReader.config); err != nil { + return configReader, err + } + return configReader, nil +} + +// GetPolicyConfig will return the policy config from the terrascan config file +func (r TerrascanConfigReader) GetPolicyConfig() Policy { + return r.config.Policy +} + +// GetNotifications will return the notifiers specified in the terrascan config file +func (r TerrascanConfigReader) GetNotifications() map[string]Notifier { + return r.config.Notifications +} + +// GetRules will return the rules specified in the terrascan config file +func (r TerrascanConfigReader) GetRules() Rules { + return r.config.Rules +} diff --git a/pkg/config/config-reader_test.go b/pkg/config/config-reader_test.go new file mode 100644 index 000000000..dcbe448c8 --- /dev/null +++ b/pkg/config/config-reader_test.go @@ -0,0 +1,136 @@ +/* + 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 config + +import ( + "reflect" + "testing" +) + +func TestNewTerrascanConfigReader(t *testing.T) { + testNotifier := Notifier{ + NotifierType: "webhook", + NotifierConfig: map[string]interface{}{ + "url": "testurl1", + }, + } + testPolicy := Policy{ + BasePath: "custom-path", + RepoPath: "rego-subdir", + RepoURL: "https://repository/url", + Branch: "branch-name", + } + testRules := Rules{ + ScanRules: []string{"rule.1", "rule.2", "rule.3", "rule.4", "rule.5"}, + SkipRules: []string{"rule.1"}, + } + + type args struct { + fileName string + } + tests := []struct { + name string + args args + want *TerrascanConfigReader + wantErr bool + assertGetters bool + Policy + notifications map[string]Notifier + Rules + }{ + { + name: "empty config file", + args: args{ + fileName: "", + }, + want: &TerrascanConfigReader{}, + }, + { + name: "non existent config file", + args: args{ + fileName: "test", + }, + wantErr: true, + want: &TerrascanConfigReader{}, + }, + { + name: "invalid toml config file", + args: args{ + fileName: "testdata/invalid.toml", + }, + wantErr: true, + want: &TerrascanConfigReader{}, + }, + { + name: "valid toml config file with partial fields", + args: args{ + fileName: "testdata/terrascan-config.toml", + }, + want: &TerrascanConfigReader{ + config: TerrascanConfig{ + Policy: testPolicy, + }, + }, + }, + { + name: "valid toml config file with all fields", + args: args{ + fileName: "testdata/terrascan-config-all-fields.toml", + }, + want: &TerrascanConfigReader{ + config: TerrascanConfig{ + Policy: testPolicy, + Notifications: map[string]Notifier{ + "webhook1": testNotifier, + }, + Rules: testRules, + }, + }, + assertGetters: true, + notifications: map[string]Notifier{ + "webhook1": testNotifier, + }, + Policy: testPolicy, + Rules: testRules, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NewTerrascanConfigReader(tt.args.fileName) + if (err != nil) != tt.wantErr { + t.Errorf("NewTerrascanConfigReader() got error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("NewTerrascanConfigReader() = got %v, want %v", got, tt.want) + } + if tt.assertGetters { + if !reflect.DeepEqual(got.GetPolicyConfig(), tt.Policy) { + t.Errorf("NewTerrascanConfigReader() = got config: %v, want config: %v", got.GetPolicyConfig(), tt.Policy) + } + + if !reflect.DeepEqual(got.GetNotifications(), tt.notifications) { + t.Errorf("NewTerrascanConfigReader() = got notifications: %v, want notifications: %v", got.GetNotifications(), tt.notifications) + } + + if !reflect.DeepEqual(got.GetRules(), tt.Rules) { + t.Errorf("NewTerrascanConfigReader() = got rules: %v, want rules: %v", got.GetRules(), tt.Rules) + } + } + }) + } +} diff --git a/pkg/config/configfile.go b/pkg/config/configfile.go deleted file mode 100644 index 3b81144be..000000000 --- a/pkg/config/configfile.go +++ /dev/null @@ -1,60 +0,0 @@ -/* - 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 config - -import ( - "fmt" - "os" - - "github.com/pelletier/go-toml" - "go.uber.org/zap" -) - -var ( - // ErrTomlLoadConfig indicates error: Failed to load toml config - ErrTomlLoadConfig = fmt.Errorf("failed to load toml config") - // ErrNotPresent indicates error: Config file not present - ErrNotPresent = fmt.Errorf("config file not present") -) - -// LoadConfig loads a configuration from specified path and -// returns a *toml.Tree with the contents of the config file -func LoadConfig(configFile string) (*toml.Tree, error) { - - // empty config file path - if configFile == "" { - zap.S().Debug("no config file specified") - return nil, nil - } - - // check if file exists - _, err := os.Stat(configFile) - if err != nil { - zap.S().Errorf("Can't find '%s'", configFile) - return nil, ErrNotPresent - } - - // parse toml config file - config, err := toml.LoadFile(configFile) - if err != nil { - zap.S().Errorf("Error loading '%s': %v", configFile, err) - return nil, ErrTomlLoadConfig - } - - // return config Tree - return config, nil -} diff --git a/pkg/config/global.go b/pkg/config/global.go index cff3e1d2d..0073631d8 100644 --- a/pkg/config/global.go +++ b/pkg/config/global.go @@ -17,10 +17,9 @@ package config import ( - "fmt" - "github.com/pelletier/go-toml" - "go.uber.org/zap" "os" + + "go.uber.org/zap" ) const ( @@ -31,87 +30,54 @@ const ( ) var ( - policyRepoPath = os.Getenv("HOME") + "/.terrascan" - policyBasePath = policyRepoPath + "/pkg/policies/opa/rego" - errTomlKeyNotPresent = fmt.Errorf("%s key not present in toml config", policyConfigKey) + policyRepoPath = os.Getenv("HOME") + "/.terrascan" + policyBasePath = policyRepoPath + "/pkg/policies/opa/rego" ) func init() { // If the user specifies a config file in TERRASCAN_CONFIG, // overwrite the defaults with the values from that file. // Retain the defaults for members not specified in the file. - LoadGlobalConfig(os.Getenv(configEnvvarName)) + if err := LoadGlobalConfig(os.Getenv(configEnvvarName)); err != nil { + zap.S().Error("error while loading global config", zap.Error(err)) + } } // LoadGlobalConfig loads policy configuration from specified configFile // into var Global.Policy. Members of Global.Policy that are not specified // in configFile will get default values -func LoadGlobalConfig(configFile string) { +func LoadGlobalConfig(configFile string) error { // Start with the defaults - Global.Policy = PolicyConfig{ + Global.Policy = Policy{ BasePath: policyBasePath, RepoPath: policyRepoPath, RepoURL: policyRepoURL, Branch: policyBranch, } - if len(configFile) > 0 { - p, err := loadConfigFile(configFile) - if err != nil { - zap.S().Error(err) - return - } - if len(p.Policy.BasePath) > 0 { - Global.Policy.BasePath = p.Policy.BasePath - } - if len(p.Policy.RepoPath) > 0 { - Global.Policy.RepoPath = p.Policy.RepoPath - } - if len(p.Policy.RepoURL) > 0 { - Global.Policy.RepoURL = p.Policy.RepoURL - } - if len(p.Policy.Branch) > 0 { - Global.Policy.Branch = p.Policy.Branch - } + if configFile == "" { + zap.S().Debug("global config env variable is not specified") + return nil } -} - -func loadConfigFile(configFile string) (GlobalConfig, error) { - p := GlobalConfig{} - config, err := LoadConfig(configFile) + configReader, err := NewTerrascanConfigReader(configFile) if err != nil { - return p, ErrNotPresent + return err } - keyConfig := config.Get(policyConfigKey) - if keyConfig == nil { - return p, errTomlKeyNotPresent + if len(configReader.GetPolicyConfig().BasePath) > 0 { + Global.Policy.BasePath = configReader.GetPolicyConfig().BasePath } - - keyTomlConfig := keyConfig.(*toml.Tree) - - // We want to treat missing keys as empty strings - str := func(x interface{}) string { - if x == nil { - return "" - } - return x.(string) + if len(configReader.GetPolicyConfig().RepoPath) > 0 { + Global.Policy.RepoPath = configReader.GetPolicyConfig().RepoPath } - - // path = path where repo will be checked out - p.Policy.BasePath = str(keyTomlConfig.Get("path")) - - // repo_url = git url to policy repository - p.Policy.RepoURL = str(keyTomlConfig.Get("repo_url")) - - // rego_subdir = subdir of where rego files are located - p.Policy.RepoPath = str(keyTomlConfig.Get("rego_subdir")) - - // branch = git branch where policies are stored - p.Policy.Branch = str(keyTomlConfig.Get("branch")) - - return p, nil + if len(configReader.GetPolicyConfig().RepoURL) > 0 { + Global.Policy.RepoURL = configReader.GetPolicyConfig().RepoURL + } + if len(configReader.GetPolicyConfig().Branch) > 0 { + Global.Policy.Branch = configReader.GetPolicyConfig().Branch + } + return nil } // GetPolicyBasePath returns policy base path as set in global config diff --git a/pkg/config/global_test.go b/pkg/config/global_test.go index b0aaeb981..28b1cf107 100644 --- a/pkg/config/global_test.go +++ b/pkg/config/global_test.go @@ -20,32 +20,75 @@ import ( "testing" ) -func testConfigEnv(configPath string, t *testing.T) { - // Load the test data to compare against - config, err := loadConfigFile(configPath) - if err != nil { - t.Error(err) - } - - LoadGlobalConfig(configPath) +func TestLoadGlobalConfig(t *testing.T) { + testConfigFile := "./testdata/terrascan-config.toml" - if Global.Policy.BasePath != config.Policy.BasePath { - t.Errorf("BasePath not overridden! %v != %v", Global.Policy.BasePath, config.Policy.BasePath) + type args struct { + configFile string } - - if Global.Policy.RepoPath != config.Policy.RepoPath { - t.Errorf("RepoPath not overridden! %v != %v", Global.Policy.RepoPath, config.Policy.RepoPath) + tests := []struct { + name string + args args + wantErr bool + policyBasePath string + policyRepoPath string + repoURL string + branchName string + }{ + { + // no error expected + name: "global config file not specified", + args: args{ + configFile: "", + }, + policyBasePath: policyBasePath, + policyRepoPath: policyRepoPath, + repoURL: policyRepoURL, + branchName: policyBranch, + }, + { + name: "global config file specified but doesn't exist", + args: args{ + configFile: "test.toml", + }, + wantErr: true, + policyBasePath: policyBasePath, + policyRepoPath: policyRepoPath, + repoURL: policyRepoURL, + branchName: policyBranch, + }, + { + name: "valid global config file specified", + args: args{ + configFile: testConfigFile, + }, + policyBasePath: "custom-path", + policyRepoPath: "rego-subdir", + repoURL: "https://repository/url", + branchName: "branch-name", + }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := LoadGlobalConfig(tt.args.configFile); (err != nil) != tt.wantErr { + t.Errorf("LoadGlobalConfig() error = %v, wantErr %v", err, tt.wantErr) + } - if Global.Policy.RepoURL != config.Policy.RepoURL { - t.Errorf("RepoURL not overridden! %v != %v", Global.Policy.RepoURL, config.Policy.RepoURL) - } + if GetPolicyBasePath() != tt.policyBasePath { + t.Errorf("LoadGlobalConfig() error = got BasePath: %v, want BasePath: %v", tt.policyBasePath, Global.Policy.BasePath) + } - if Global.Policy.Branch != config.Policy.Branch { - t.Errorf("Branch not overridden! %v != %v", Global.Policy.Branch, config.Policy.Branch) - } -} + if GetPolicyRepoPath() != tt.policyRepoPath { + t.Errorf("LoadGlobalConfig() error = got RepoPath: %v, want RepoPath: %v", tt.policyRepoPath, Global.Policy.RepoPath) + } -func TestConfigFileLoadsCorrectly(t *testing.T) { - testConfigEnv("./testdata/terrascan-config.toml", t) + if GetPolicyRepoURL() != tt.repoURL { + t.Errorf("LoadGlobalConfig() error = got RepoURL: %v, want RepoURL: %v", tt.repoURL, Global.Policy.RepoURL) + } + + if GetPolicyBranch() != tt.branchName { + t.Errorf("LoadGlobalConfig() error = got BranchName: %v, want BranchName: %v", tt.branchName, Global.Policy.Branch) + } + }) + } } diff --git a/pkg/config/testdata/invalid.toml b/pkg/config/testdata/invalid.toml new file mode 100644 index 000000000..59cc4a153 --- /dev/null +++ b/pkg/config/testdata/invalid.toml @@ -0,0 +1 @@ +invalid toml file \ No newline at end of file diff --git a/pkg/config/testdata/terrascan-config-all-fields.toml b/pkg/config/testdata/terrascan-config-all-fields.toml new file mode 100644 index 000000000..97f894820 --- /dev/null +++ b/pkg/config/testdata/terrascan-config-all-fields.toml @@ -0,0 +1,23 @@ +[policy] +path = "custom-path" +rego_subdir = "rego-subdir" +repo_url = "https://repository/url" +branch = "branch-name" + +[notifications] +[notifications.webhook1] +type = 'webhook' +[notifications.webhook1.config] +url = 'testurl1' + +[rules] +scan-rules = [ + "rule.1", + "rule.2", + "rule.3", + "rule.4", + "rule.5", +] +skip-rules = [ + "rule.1" +] \ No newline at end of file diff --git a/pkg/config/types.go b/pkg/config/types.go index 3e40ff1ac..2939f786b 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -17,20 +17,34 @@ package config // Global initalizes GlobalConfig struct -var Global *GlobalConfig = &GlobalConfig{} +var Global *TerrascanConfig = &TerrascanConfig{} -// GlobalConfig struct defines global variables/configurations across terrascan -type GlobalConfig struct { - Policy PolicyConfig +// TerrascanConfig struct defines global variables/configurations across terrascan +type TerrascanConfig struct { + Policy `toml:"policy,omitempty"` + Notifications map[string]Notifier `toml:"notifications,omitempty"` + Rules `toml:"rules,omitempty"` } -// PolicyConfig struct define policy specific configurations -type PolicyConfig struct { +// Policy struct defines policy specific configurations +type Policy struct { // policy local path - BasePath string - RepoPath string + BasePath string `toml:"path,omitempty"` + RepoPath string `toml:"rego_subdir,omitempty"` // policy git url and branch - RepoURL string - Branch string + RepoURL string `toml:"repo_url,omitempty"` + Branch string `toml:"branch,omitempty"` +} + +// Notifier represent a single notification in the terrascan config file +type Notifier struct { + NotifierType string `toml:"type"` + NotifierConfig interface{} `toml:"config"` +} + +// Rules represents scan and skip rules in the terrascan config file +type Rules struct { + ScanRules []string `toml:"scan-rules,omitempty"` + SkipRules []string `toml:"skip-rules,omitempty"` } diff --git a/pkg/http-server/file-scan.go b/pkg/http-server/file-scan.go index b3a8d01f3..f51a5899d 100644 --- a/pkg/http-server/file-scan.go +++ b/pkg/http-server/file-scan.go @@ -38,6 +38,8 @@ func (g *APIHandler) scanFile(w http.ResponseWriter, r *http.Request) { iacType = params["iac"] iacVersion = params["iacVersion"] cloudType = strings.Split(params["cloud"], ",") + scanRules = []string{} + skipRules = []string{} ) // parse multipart form, 10 << 20 specifies maximum upload of 10 MB files @@ -82,14 +84,27 @@ func (g *APIHandler) scanFile(w http.ResponseWriter, r *http.Request) { // write this byte array to our temporary file tempFile.Write(fileBytes) + // read scan and skip rules from the form data + // scan and skip rules are comma separated rule id's in the request body + scanRulesValue := r.FormValue("scan_rules") + skipRulesValue := r.FormValue("skip_rules") + + if scanRulesValue != "" { + scanRules = strings.Split(scanRulesValue, ",") + } + + if skipRulesValue != "" { + skipRules = strings.Split(skipRulesValue, ",") + } + // create a new runtime executor for scanning the uploaded file var executor *runtime.Executor if g.test { executor, err = runtime.NewExecutor(iacType, iacVersion, cloudType, - tempFile.Name(), "", "", []string{"./testdata/testpolicies"}, []string{}, []string{}) + tempFile.Name(), "", "", []string{"./testdata/testpolicies"}, scanRules, skipRules) } else { executor, err = runtime.NewExecutor(iacType, iacVersion, cloudType, - tempFile.Name(), "", "", []string{}, []string{}, []string{}) + tempFile.Name(), "", "", []string{}, scanRules, skipRules) } if err != nil { zap.S().Error(err) diff --git a/pkg/http-server/file-scan_test.go b/pkg/http-server/file-scan_test.go index 6928e2b72..023a1e914 100644 --- a/pkg/http-server/file-scan_test.go +++ b/pkg/http-server/file-scan_test.go @@ -9,12 +9,18 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/gorilla/mux" ) func TestUpload(t *testing.T) { + testFilePath := "./testdata/testconfig.tf" + testIacType := "terraform" + testIacVersion := "v12" + testCloudType := "aws" + testParamName := "file" table := []struct { name string @@ -23,65 +29,79 @@ func TestUpload(t *testing.T) { iacType string iacVersion string cloudType string + scanRules []string + skipRules []string wantStatus int }{ { name: "valid file scan", - path: "./testdata/testconfig.tf", - param: "file", - iacType: "terraform", - iacVersion: "v12", - cloudType: "aws", + path: testFilePath, + param: testParamName, + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, wantStatus: http.StatusOK, }, { name: "valid file scan default iac type", - path: "./testdata/testconfig.tf", - param: "file", - cloudType: "aws", + path: testFilePath, + param: testParamName, + cloudType: testCloudType, wantStatus: http.StatusOK, }, { name: "valid file scan default iac version", - path: "./testdata/testconfig.tf", - param: "file", - iacType: "terraform", - cloudType: "aws", + path: testFilePath, + param: testParamName, + iacType: testIacType, + cloudType: testCloudType, wantStatus: http.StatusOK, }, { name: "invalid iacType", - path: "./testdata/testconfig.tf", - param: "file", + path: testFilePath, + param: testParamName, iacType: "notthere", - iacVersion: "v12", - cloudType: "aws", + iacVersion: testIacVersion, + cloudType: testCloudType, wantStatus: http.StatusBadRequest, }, { name: "invalid file param", - path: "./testdata/testconfig.tf", + path: testFilePath, param: "someparam", wantStatus: http.StatusInternalServerError, }, { name: "invalid file config", path: "./testdata/invalid.tf", - param: "file", - iacType: "terraform", - iacVersion: "v12", - cloudType: "aws", + param: testParamName, + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, wantStatus: http.StatusInternalServerError, }, { name: "empty file config", path: "./testdata/empty.tf", - param: "file", - iacType: "terraform", - iacVersion: "v12", - cloudType: "aws", + param: testParamName, + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, wantStatus: http.StatusOK, }, + { + name: "valid file scan with scan and skip rules", + path: testFilePath, + param: testParamName, + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, + wantStatus: http.StatusOK, + scanRules: []string{"AWS.CloudFront.EncryptionandKeyManagement.High.0407", "AWS.CloudFront.EncryptionandKeyManagement.High.0408", + "AWS.CloudFront.Logging.Medium.0567", "AWS.CloudFront.Network Security.Low.0568"}, + skipRules: []string{"AWS.CloudFront.Network Security.Low.0568"}, + }, } for _, tt := range table { @@ -104,6 +124,19 @@ func TestUpload(t *testing.T) { t.Error(err) } io.Copy(part, file) + + if len(tt.scanRules) > 0 { + if err = writer.WriteField("scan_rules", strings.Join(tt.scanRules, ",")); err != nil { + writer.Close() + t.Error(err) + } + } + if len(tt.skipRules) > 0 { + if err = writer.WriteField("skip_rules", strings.Join(tt.scanRules, ",")); err != nil { + writer.Close() + t.Error(err) + } + } writer.Close() // http request of the type "/v1/{iacType}/{iacVersion}/{cloudType}/file/scan" diff --git a/pkg/http-server/remote-repo.go b/pkg/http-server/remote-repo.go index 072ddc271..3c40721ab 100644 --- a/pkg/http-server/remote-repo.go +++ b/pkg/http-server/remote-repo.go @@ -33,9 +33,11 @@ import ( // scanRemoteRepoReq contains request body for remote repository scanning type scanRemoteRepoReq struct { - RemoteType string `json:"remote_type"` - RemoteURL string `json:"remote_url"` - ConfigOnly bool `json:"config_only"` + RemoteType string `json:"remote_type"` + RemoteURL string `json:"remote_url"` + ConfigOnly bool `json:"config_only"` + ScanRules []string `json:"scan_rules"` + SkipRules []string `json:"skip_rules"` d downloader.Downloader } @@ -112,7 +114,7 @@ func (s *scanRemoteRepoReq) ScanRemoteRepo(iacType, iacVersion string, cloudType // create a new runtime executor for scanning the remote repo executor, err := runtime.NewExecutor(iacType, iacVersion, cloudType, - "", iacDirPath, "", policyPath, []string{}, []string{}) + "", iacDirPath, "", policyPath, s.ScanRules, s.SkipRules) if err != nil { zap.S().Error(err) return output, err diff --git a/pkg/http-server/remote-repo_test.go b/pkg/http-server/remote-repo_test.go index 70d0bd470..c930db667 100644 --- a/pkg/http-server/remote-repo_test.go +++ b/pkg/http-server/remote-repo_test.go @@ -81,6 +81,10 @@ func TestScanRemoteRepo(t *testing.T) { } func TestScanRemoteRepoHandler(t *testing.T) { + validRepo := "/~https://github.com/kanchwala-yusuf/Damn-Vulnerable-Terraform-Project.git" + testIacType := "terraform" + testIacVersion := "v12" + testCloudType := "aws" table := []struct { name string @@ -89,44 +93,58 @@ func TestScanRemoteRepoHandler(t *testing.T) { cloudType string remoteURL string remoteType string + scanRules []string + skipRules []string wantStatus int }{ { name: "empty url and type", - iacType: "terraform", - iacVersion: "v12", - cloudType: "aws", + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, remoteURL: "", remoteType: "", wantStatus: http.StatusBadRequest, }, { name: "empty type", - iacType: "terraform", - iacVersion: "v12", - cloudType: "aws", + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, remoteURL: someURL, remoteType: "", wantStatus: http.StatusBadRequest, }, { name: "invalid url and type", - iacType: "terraform", - iacVersion: "v12", - cloudType: "aws", + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, remoteURL: someURL, remoteType: someType, wantStatus: http.StatusBadRequest, }, { name: "valid url and type", - iacType: "terraform", - iacVersion: "v12", - cloudType: "aws", - remoteURL: "/~https://github.com/kanchwala-yusuf/Damn-Vulnerable-Terraform-Project.git", + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, + remoteURL: validRepo, remoteType: "git", wantStatus: http.StatusOK, }, + { + name: "valid url and type with scan and skip rules", + iacType: testIacType, + iacVersion: testIacVersion, + cloudType: testCloudType, + remoteURL: validRepo, + remoteType: "git", + scanRules: []string{"AWS.CloudFront.EncryptionandKeyManagement.High.0407", "AWS.CloudFront.EncryptionandKeyManagement.High.0408", + "AWS.CloudFront.Logging.Medium.0567", "AWS.CloudFront.Network Security.Low.0568"}, + skipRules: []string{"AWS.CloudFront.Network Security.Low.0568"}, + wantStatus: http.StatusOK, + }, } for _, tt := range table { @@ -141,6 +159,8 @@ func TestScanRemoteRepoHandler(t *testing.T) { s := scanRemoteRepoReq{ RemoteURL: tt.remoteURL, RemoteType: tt.remoteType, + ScanRules: tt.scanRules, + SkipRules: tt.skipRules, } reqBody, _ := json.Marshal(s) diff --git a/pkg/notifications/notifiers.go b/pkg/notifications/notifiers.go index f6227d245..0e6cefcb5 100644 --- a/pkg/notifications/notifiers.go +++ b/pkg/notifications/notifiers.go @@ -22,7 +22,6 @@ import ( "github.com/accurics/terrascan/pkg/config" "github.com/accurics/terrascan/pkg/utils" - "github.com/pelletier/go-toml" "go.uber.org/zap" ) @@ -31,8 +30,8 @@ const ( ) var ( - errNotifierNotSupported = fmt.Errorf("notifier not supported") - + errNotifierNotSupported = fmt.Errorf("notifier not supported") + errNotifierTypeNotPresent = fmt.Errorf("notifier type not present in toml config") // ErrTomlKeyNotPresent will be returned when config file does not have notificationsConfigKey ErrTomlKeyNotPresent = fmt.Errorf("key not present in toml config") ) @@ -56,49 +55,47 @@ func NewNotifiers(configFile string) ([]Notifier, error) { var notifiers []Notifier - config, err := config.LoadConfig(configFile) - if err != nil || config == nil { + if configFile == "" { + zap.S().Debug("no config file specified") + return notifiers, nil + } + + configReader, err := config.NewTerrascanConfigReader(configFile) + if err != nil || configReader == nil { return notifiers, err } // get config for 'notifications' - keyConfig := config.Get(notificationsConfigKey) - if keyConfig == nil { - zap.S().Infof("key '%s' not present in toml config", notificationsConfigKey) + notifications := configReader.GetNotifications() + if notifications == nil { + zap.S().Debug("key '%s' not present in toml config", notificationsConfigKey) return notifiers, ErrTomlKeyNotPresent } - // get all the notifier types configured in TOML config - keyTomlConfig := keyConfig.(*toml.Tree) - notifierTypes := keyTomlConfig.Keys() - // create notifiers var allErrs error - for _, nType := range notifierTypes { - - if !IsNotifierSupported(nType) { - zap.S().Errorf("notifier type '%s' not supported", nType) - allErrs = utils.WrapError(errNotifierNotSupported, allErrs) + for _, notifier := range notifications { + if notifier.NotifierType == "" { + zap.S().Error(errNotifierTypeNotPresent) + allErrs = utils.WrapError(errNotifierTypeNotPresent, allErrs) continue } - // check if toml config present for notifier type - nTypeConfig := keyTomlConfig.Get(nType) - if nTypeConfig.(*toml.Tree).String() == "" { - zap.S().Errorf("notifier '%v' config not present", nType) - allErrs = utils.WrapError(ErrTomlKeyNotPresent, allErrs) + if !IsNotifierSupported(notifier.NotifierType) { + zap.S().Errorf("notifier type '%s' not supported", notifier.NotifierType) + allErrs = utils.WrapError(errNotifierNotSupported, allErrs) continue } // create a new notifier - n, err := NewNotifier(nType) + n, err := NewNotifier(notifier.NotifierType) if err != nil { allErrs = utils.WrapError(err, allErrs) continue } // populate data - err = n.Init(nTypeConfig) + err = n.Init(notifier.NotifierConfig) if err != nil { allErrs = utils.WrapError(err, allErrs) continue diff --git a/pkg/notifications/notifiers_test.go b/pkg/notifications/notifiers_test.go index ffaca96a2..fae0ea779 100644 --- a/pkg/notifications/notifiers_test.go +++ b/pkg/notifications/notifiers_test.go @@ -1,6 +1,7 @@ package notifications import ( + "fmt" "reflect" "testing" @@ -57,7 +58,7 @@ func TestNewNotifiers(t *testing.T) { { name: "invalid toml", configFile: "testdata/invalid.toml", - wantErr: config.ErrTomlLoadConfig, + wantErr: fmt.Errorf("(1, 3): was expecting token =, but got \"am\" instead"), }, { name: "key not present", @@ -72,13 +73,22 @@ func TestNewNotifiers(t *testing.T) { { name: "empty notifier config", configFile: "testdata/empty-notifier-config.toml", - wantErr: ErrTomlKeyNotPresent, + wantErr: errNotifierTypeNotPresent, }, { name: "invalid notifier config", configFile: "testdata/invalid-notifier-config.toml", + wantErr: webhook.ErrNilConfigData, + }, + { + name: "valid multiple notifier config", + configFile: "testdata/valid-notifier-config.toml", wantErr: nil, }, + { + name: "no config file specified", + wantErr: nil, + }, } for _, tt := range table { diff --git a/pkg/notifications/testdata/invalid-notifier-config.toml b/pkg/notifications/testdata/invalid-notifier-config.toml index 4b2914425..f38b8b4c2 100644 --- a/pkg/notifications/testdata/invalid-notifier-config.toml +++ b/pkg/notifications/testdata/invalid-notifier-config.toml @@ -1,4 +1,5 @@ [notifications] [notifications.webhook] + type = "webhook" key1 = "val1" key2 = "val2" diff --git a/pkg/notifications/testdata/invalid-notifier-type.toml b/pkg/notifications/testdata/invalid-notifier-type.toml index 74c795e09..bc9a0c9d1 100644 --- a/pkg/notifications/testdata/invalid-notifier-type.toml +++ b/pkg/notifications/testdata/invalid-notifier-type.toml @@ -3,4 +3,5 @@ # notifications configuration [notifications] [notifications.invalid] + type = "invalid" url = "https://httpbin.org/post" diff --git a/pkg/notifications/testdata/valid-notifier-config.toml b/pkg/notifications/testdata/valid-notifier-config.toml new file mode 100644 index 000000000..f1a0e6b32 --- /dev/null +++ b/pkg/notifications/testdata/valid-notifier-config.toml @@ -0,0 +1,10 @@ +[notifications] +[notifications.webhook1] +type = 'webhook' +[notifications.webhook1.config] +url = 'https://httpbin.org/post' + +[notifications.webhook2] +type = 'webhook' +[notifications.webhook2.config] +url = 'https://httpbin.org/post' \ No newline at end of file diff --git a/pkg/notifications/webhook/webhook.go b/pkg/notifications/webhook/webhook.go index fa59598fc..157bc4707 100644 --- a/pkg/notifications/webhook/webhook.go +++ b/pkg/notifications/webhook/webhook.go @@ -22,25 +22,40 @@ import ( "net/http" httputils "github.com/accurics/terrascan/pkg/utils/http" - "github.com/pelletier/go-toml" "go.uber.org/zap" ) var ( errInitFailed = fmt.Errorf("failed to initialize webhook notifier") + // ErrNilConfigData will be returned when config is nil + ErrNilConfigData = fmt.Errorf("config data is nil") ) // Init initalizes the webhook notifier, reads config file and configures the // necessary parameters for webhook notifications to work func (w *Webhook) Init(config interface{}) error { + // return error if config data is not present + if config == nil { + return ErrNilConfigData + } // config to *toml.Tree - tomlConfig := config.(*toml.Tree) + webhookConfig, ok := config.(map[string]interface{}) + if !ok { + zap.S().Errorf("error type casting webhook config data") + return errInitFailed + } // initalize Webhook struct with url and token - err := tomlConfig.Unmarshal(w) + + jsonData, err := json.Marshal(webhookConfig) if err != nil { - zap.S().Error(errInitFailed.Error()) + zap.S().Error("error while marshalling webhook config data", zap.Error(err)) + return errInitFailed + } + + if err = json.Unmarshal(jsonData, w); err != nil { + zap.S().Error("error while un-marshalling webhook config data", zap.Error(err)) return errInitFailed } diff --git a/pkg/notifications/webhook/webhook_test.go b/pkg/notifications/webhook/webhook_test.go new file mode 100644 index 000000000..23eaf5d70 --- /dev/null +++ b/pkg/notifications/webhook/webhook_test.go @@ -0,0 +1,86 @@ +/* + 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 webhook + +import "testing" + +func TestWebhookInit(t *testing.T) { + testURL := "testURL" + testToken := "testToken" + + type args struct { + config interface{} + } + tests := []struct { + name string + args args + wantErr bool + assert bool + url string + token string + }{ + { + name: "nil config", + args: args{ + config: nil, + }, + wantErr: true, + }, + { + name: "valid webhook config data", + args: args{ + config: map[string]interface{}{ + "url": testURL, + "token": testToken, + }, + }, + assert: true, + url: testURL, + token: testToken, + }, + { + name: "invalid webhook config data", + args: args{ + config: struct { + url string + token string + }{ + url: testURL, + token: testToken, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &Webhook{} + if err := w.Init(tt.args.config); (err != nil) != tt.wantErr { + t.Errorf("Webhook.Init() got error: %v, wantErr: %v", err, tt.wantErr) + } + if tt.assert { + if w.URL != tt.url { + t.Errorf("Webhook.Init() got url: %v, want url: %v", w.URL, tt.url) + } + + if w.Token != tt.token { + t.Errorf("Webhook.Init() got token: %v, want token: %v", w.Token, tt.token) + } + } + }) + } +} diff --git a/pkg/policy/interface.go b/pkg/policy/interface.go index 7296f8b9c..5755cafec 100644 --- a/pkg/policy/interface.go +++ b/pkg/policy/interface.go @@ -18,8 +18,9 @@ package policy // Engine Policy Engine interface type Engine interface { - // to initialize engine with policy path, scan and skip rules + //Init method to initialize engine with policy path, scan and skip rules Init(string, []string, []string) error + FilterRules(string, []string, []string) Configure() error Evaluate(EngineInput) (EngineOutput, error) GetResults() EngineOutput diff --git a/pkg/policy/opa/engine.go b/pkg/policy/opa/engine.go index fd4e108c0..8126faffe 100644 --- a/pkg/policy/opa/engine.go +++ b/pkg/policy/opa/engine.go @@ -46,17 +46,11 @@ var ( ) // NewEngine returns a new OPA policy engine -func NewEngine(policyPath string, scanRules, skipRules []string) (*Engine, error) { +func NewEngine() (*Engine, error) { // opa engine struct engine := &Engine{} - // initialize the engine - if err := engine.Init(policyPath, scanRules, skipRules); err != nil { - zap.S().Error("failed to initialize OPA policy engine", zap.Error(err)) - return engine, errInitFailed - } - // successful return engine, nil } @@ -254,11 +248,11 @@ func (e *Engine) Init(policyPath string, scanRules, skipRules []string) error { if err := e.LoadRegoFiles(policyPath); err != nil { zap.S().Error("error loading rego files", zap.String("policy path", policyPath), zap.Error(err)) - return err + return errInitFailed } // before compiling the rego files, filter the rules based on scan and skip rules supplied - filterRules(e, policyPath, scanRules, skipRules) + e.FilterRules(policyPath, scanRules, skipRules) // update the rule count e.stats.ruleCount = len(e.regoDataMap) @@ -266,7 +260,7 @@ func (e *Engine) Init(policyPath string, scanRules, skipRules []string) error { err := e.CompileRegoFiles() if err != nil { zap.S().Error("error compiling rego files", zap.String("policy path", policyPath), zap.Error(err)) - return err + return errInitFailed } // initialize ViolationStore @@ -402,19 +396,20 @@ func (e *Engine) Evaluate(engineInput policy.EngineInput) (policy.EngineOutput, return e.results, nil } -func filterRules(e *Engine, policyPath string, scanRules, skipRules []string) { +// FilterRules will apply the scan and skip rules +func (e *Engine) FilterRules(policyPath string, scanRules, skipRules []string) { // apply scan rules if len(scanRules) > 0 { - filterScanRules(e, policyPath, scanRules) + e.filterScanRules(policyPath, scanRules) } // apply skip rules if len(skipRules) > 0 { - filterSkipRules(e, policyPath, skipRules) + e.filterSkipRules(policyPath, skipRules) } } -func filterScanRules(e *Engine, policyPath string, scanRules []string) { +func (e *Engine) filterScanRules(policyPath string, scanRules []string) { // temporary map to store data from original rego data map tempMap := make(map[string]*RegoData) @@ -435,7 +430,7 @@ func filterScanRules(e *Engine, policyPath string, scanRules []string) { e.regoDataMap = tempMap } -func filterSkipRules(e *Engine, policyPath string, skipRules []string) { +func (e *Engine) filterSkipRules(policyPath string, skipRules []string) { // remove rules to be skipped from the rego data map for _, ruleID := range skipRules { _, ok := e.regoDataMap[ruleID] diff --git a/pkg/policy/opa/engine_test.go b/pkg/policy/opa/engine_test.go index d7ceb6585..18056474d 100644 --- a/pkg/policy/opa/engine_test.go +++ b/pkg/policy/opa/engine_test.go @@ -9,7 +9,6 @@ func TestFilterRules(t *testing.T) { testPolicyPath := "test" type args struct { - e *Engine policyPath string scanRules []string skipRules []string @@ -19,68 +18,74 @@ func TestFilterRules(t *testing.T) { args args assert bool regoMapSize int + regoDataMap map[string]*RegoData }{ { - name: "no scan and skip rules", - args: args{ - e: &Engine{}, - }, + name: "no scan and skip rules", + args: args{}, + regoDataMap: nil, }, { name: "scan rules test", args: args{ - e: &Engine{ - regoDataMap: getTestRegoDataMap(10), - }, policyPath: testPolicyPath, scanRules: []string{"Rule.0", "Rule.1", "Rule.2", "Rule.3", "Rule.11"}, }, + regoDataMap: getTestRegoDataMap(10), assert: true, regoMapSize: 4, }, { name: "scan rules not found in path", args: args{ - e: &Engine{ - regoDataMap: getTestRegoDataMap(10), - }, policyPath: testPolicyPath, scanRules: []string{"Rule.11", "Rule.12", "Rule.13"}, }, + regoDataMap: getTestRegoDataMap(10), assert: true, regoMapSize: 0, }, { name: "skip rules test", args: args{ - e: &Engine{ - regoDataMap: getTestRegoDataMap(6), - }, policyPath: testPolicyPath, skipRules: []string{"Rule.1"}, }, + regoDataMap: getTestRegoDataMap(6), assert: true, regoMapSize: 5, }, { name: "skip rules not found in policy path", args: args{ - e: &Engine{ - regoDataMap: getTestRegoDataMap(20), - }, policyPath: testPolicyPath, skipRules: []string{"Rule.21", "Rule.22"}, }, + regoDataMap: getTestRegoDataMap(20), assert: true, regoMapSize: 20, }, + { + name: "both scan and skip rules supplied", + args: args{ + policyPath: testPolicyPath, + scanRules: []string{"Rule.10", "Rule.11", "Rule.12", "Rule.15", "Rule.31", "Rule.32", "Rule.40", "Rule.41", "Rule.42"}, + skipRules: []string{"Rule.31", "Rule.32", "Rule.38"}, + }, + regoDataMap: getTestRegoDataMap(50), + assert: true, + regoMapSize: 7, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - filterRules(tt.args.e, tt.args.policyPath, tt.args.scanRules, tt.args.skipRules) + e := &Engine{ + regoDataMap: tt.regoDataMap, + } + e.FilterRules(tt.args.policyPath, tt.args.scanRules, tt.args.skipRules) if tt.assert { - if len(tt.args.e.regoDataMap) != tt.regoMapSize { - t.Errorf("filterRules(): expected regoDataMap size = %d, got = %d", tt.regoMapSize, len(tt.args.e.regoDataMap)) + if len(e.regoDataMap) != tt.regoMapSize { + t.Errorf("filterRules(): expected regoDataMap size = %d, got = %d", tt.regoMapSize, len(e.regoDataMap)) } } }) diff --git a/pkg/runtime/executor.go b/pkg/runtime/executor.go index a50f67b3c..69d7d9881 100644 --- a/pkg/runtime/executor.go +++ b/pkg/runtime/executor.go @@ -17,27 +17,12 @@ package runtime import ( - "fmt" - "go.uber.org/zap" - "github.com/accurics/terrascan/pkg/config" iacProvider "github.com/accurics/terrascan/pkg/iac-providers" "github.com/accurics/terrascan/pkg/notifications" "github.com/accurics/terrascan/pkg/policy" opa "github.com/accurics/terrascan/pkg/policy/opa" - "github.com/pelletier/go-toml" -) - -const ( - rulesKey = "rules" - scanRulesKey = "scan-rules" - skipRulesKey = "skip-rules" -) - -var ( - errRuleNotString = fmt.Errorf("each scan and skip rule must be string") - errIncorrectValueForRules = fmt.Errorf("'scan-rules' and 'skip-rules' must be an array") ) // Executor object @@ -88,10 +73,9 @@ func (e *Executor) Init() error { } // read config file and update scan and skip rules - if err := e.initScanAndSkipRules(); err != nil { - if !(err == errIncorrectValueForRules || err == errRuleNotString) { - return err - } + if err := e.initRules(); err != nil { + zap.S().Error("error initialising scan and skip rules", zap.Error(err)) + return err } // create new IacProvider @@ -104,9 +88,10 @@ func (e *Executor) Init() error { // create new notifiers e.notifiers, err = notifications.NewNotifiers(e.configFile) if err != nil { - zap.S().Errorf("failed to create notifier(s). error: '%s'", err) + zap.S().Debug("failed to create notifier(s).", zap.Error(err)) // do not return an error if a key is not present in the config file if err != notifications.ErrTomlKeyNotPresent { + zap.S().Error("failed to create notifier(s).", zap.Error(err)) return err } } @@ -114,11 +99,17 @@ func (e *Executor) Init() error { // create a new policy engine based on IaC type zap.S().Debugf("using policy path %v", e.policyPath) for _, policyPath := range e.policyPath { - engine, err := opa.NewEngine(policyPath, e.scanRules, e.skipRules) + engine, err := opa.NewEngine() if err != nil { zap.S().Errorf("failed to create policy engine. error: '%s'", err) return err } + + // initialize the engine + if err := engine.Init(policyPath, e.scanRules, e.skipRules); err != nil { + zap.S().Errorf("%s", err) + return err + } e.policyEngine = append(e.policyEngine, engine) } @@ -168,68 +159,3 @@ func (e *Executor) Execute() (results Output, err error) { // successful return results, nil } - -// read the config file and update scan and skip rules -func (e *Executor) initScanAndSkipRules() error { - if e.configFile != "" { - configData, err := config.LoadConfig(e.configFile) - if err != nil { - zap.S().Error("error loading config file", zap.Error(err)) - return err - } - - if configData.Has(rulesKey) { - - data := (configData.Get(rulesKey)).(*toml.Tree) - - // read scan rules in the toml tree - if err := initRules(e, data, scanRulesKey); err != nil { - return err - } - - // read skip rules in the toml tree - if err := initRules(e, data, skipRulesKey); err != nil { - return err - } - } - } - return nil -} - -func initRules(e *Executor, tree *toml.Tree, key string) error { - rules, err := getRulesInTomlTree(tree, e.configFile, key) - if err != nil { - zap.S().Error("error reading config file", zap.Error(err)) - return err - } - if len(rules) > 0 { - if key == scanRulesKey { - e.scanRules = append(e.scanRules, rules...) - } else { - e.skipRules = append(e.skipRules, rules...) - } - } else { - zap.S().Debugf("key '%s' not found in the config file: %s", key, e.configFile) - } - return nil -} - -func getRulesInTomlTree(tree *toml.Tree, configFile, key string) ([]string, error) { - ruleSlice := make([]string, 0) - if tree.Has(key) { - rules, ok := (tree.Get(key)).([]interface{}) - if !ok { - zap.S().Errorf("key '%s' must be an array in the config file: %s", key, configFile) - return nil, errIncorrectValueForRules - } - for _, rule := range rules { - r, ok := rule.(string) - if !ok { - zap.S().Errorf("rules must be of type string for key: %s", key) - return nil, errRuleNotString - } - ruleSlice = append(ruleSlice, r) - } - } - return ruleSlice, nil -} diff --git a/pkg/runtime/executor_test.go b/pkg/runtime/executor_test.go index 97043be1a..c447854e4 100644 --- a/pkg/runtime/executor_test.go +++ b/pkg/runtime/executor_test.go @@ -28,7 +28,6 @@ import ( "github.com/accurics/terrascan/pkg/notifications" "github.com/accurics/terrascan/pkg/notifications/webhook" "github.com/accurics/terrascan/pkg/policy" - "github.com/pelletier/go-toml" ) var ( @@ -60,6 +59,9 @@ func (m MockPolicyEngine) Init(input string, scanRules, skipRules []string) erro return m.err } +func (m MockPolicyEngine) FilterRules(input string, scanRules, skipRules []string) { +} + func (m MockPolicyEngine) Configure() error { return m.err } @@ -265,174 +267,3 @@ func TestInit(t *testing.T) { }) } } - -func TestGetRulesInTomlTree(t *testing.T) { - // test data - fileLoadErr := "error while loading toml file" - emptyTomlTree, err := config.LoadConfig("testdata/empty.toml") - if err != nil { - t.Fatalf(fileLoadErr, err) - } - - configFileData, err := config.LoadConfig("testdata/scan-skip-rules.toml") - if err != nil { - t.Fatalf(fileLoadErr, err) - } - validRulesFormat := (configFileData.Get(rulesKey)).(*toml.Tree) - - configFileData, err = config.LoadConfig("testdata/invalid-scan-skip-rules.toml") - if err != nil { - t.Fatalf(fileLoadErr, err) - } - invalidRulesFormat := (configFileData.Get(rulesKey)).(*toml.Tree) - - type args struct { - tree *toml.Tree - configFile string - key string - } - tests := []struct { - name string - args args - want []string - wantErr bool - }{ - { - name: "empty toml file", - args: args{ - tree: emptyTomlTree, - configFile: "", - key: scanRulesKey, - }, - want: []string{}, - }, - { - name: "get scan rules - valid data", - args: args{ - tree: validRulesFormat, - configFile: "", - key: scanRulesKey, - }, - want: []string{"AWS.S3Bucket.DS.High.1043", "accurics.kubernetes.IAM.107"}, - }, - { - name: "get skip rules - valid data", - args: args{ - tree: validRulesFormat, - configFile: "", - key: skipRulesKey, - }, - want: []string{"AWS.S3Bucket.IAM.High.0370", "accurics.kubernetes.IAM.5", - "accurics.kubernetes.OPS.461", "accurics.kubernetes.IAM.109"}, - }, - { - name: "get scan rules - invalid data", - args: args{ - tree: invalidRulesFormat, - configFile: "", - key: scanRulesKey, - }, - wantErr: true, - }, - { - name: "get skip rules - invalid data", - args: args{ - tree: invalidRulesFormat, - configFile: "", - key: skipRulesKey, - }, - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := getRulesInTomlTree(tt.args.tree, tt.args.configFile, tt.args.key) - if (err != nil) != tt.wantErr { - t.Errorf("getRulesInTomlTree() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("getRulesInTomlTree() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestExecutorInitScanAndSkipRules(t *testing.T) { - assertionTestName := "valid config file with scan and skip rules" - - type fields struct { - configFile string - scanRules []string - skipRules []string - } - tests := []struct { - name string - fields fields - wantErr bool - }{ - { - name: "no config file", - fields: fields{}, - }, - { - name: "config file doesn't exist", - fields: fields{ - configFile: "testdata/test.toml", - }, - wantErr: true, - }, - { - name: "empty config file", - fields: fields{ - configFile: "testdata/empty.toml", - }, - }, - { - name: "config file with empty rules", - fields: fields{ - configFile: "testdata/webhook.toml", - }, - }, - { - name: assertionTestName, - fields: fields{ - configFile: "testdata/scan-skip-rules.toml", - scanRules: []string{"testRuleA", "testRuleB"}, - skipRules: []string{"testRuleC"}, - }, - }, - { - name: "valid config file with invalid scan rules", - fields: fields{ - configFile: "testdata/invalid-scan-skip-rules.toml", - }, - wantErr: true, - }, - { - name: "valid config file with invalid skip rules", - fields: fields{ - configFile: "testdata/invalid-skip-rules.toml", - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - e := &Executor{ - configFile: tt.fields.configFile, - scanRules: tt.fields.scanRules, - skipRules: tt.fields.skipRules, - } - if err := e.initScanAndSkipRules(); (err != nil) != tt.wantErr { - t.Errorf("Executor.initScanAndSkipRules() error = %v, wantErr %v", err, tt.wantErr) - } - if tt.name == assertionTestName { - if len(e.scanRules) != 4 && len(e.skipRules) != 5 { - t.Errorf("Expected scanRules: %d and skipRules: %d, got scanRules: %d and skipRules: %d", 4, 5, len(e.scanRules), len(e.skipRules)) - } - } - }) - } -} diff --git a/pkg/runtime/rules.go b/pkg/runtime/rules.go new file mode 100644 index 000000000..17773efc2 --- /dev/null +++ b/pkg/runtime/rules.go @@ -0,0 +1,30 @@ +package runtime + +import ( + "github.com/accurics/terrascan/pkg/config" + "go.uber.org/zap" +) + +// read the config file and update scan and skip rules +func (e *Executor) initRules() error { + if e.configFile == "" { + return nil + } + + configReader, err := config.NewTerrascanConfigReader(e.configFile) + if err != nil { + zap.S().Error("error loading config file", zap.Error(err)) + return err + } + + // append scan rules + if len(configReader.GetRules().ScanRules) > 0 { + e.scanRules = append(e.scanRules, configReader.GetRules().ScanRules...) + } + + // append skip rules + if len(configReader.GetRules().SkipRules) > 0 { + e.skipRules = append(e.skipRules, configReader.GetRules().SkipRules...) + } + return nil +} diff --git a/pkg/runtime/rules_test.go b/pkg/runtime/rules_test.go new file mode 100644 index 000000000..b042318ad --- /dev/null +++ b/pkg/runtime/rules_test.go @@ -0,0 +1,87 @@ +package runtime + +import ( + "testing" +) + +func TestExecutorInitRules(t *testing.T) { + type fields struct { + configFile string + scanRules []string + skipRules []string + } + tests := []struct { + name string + fields fields + wantErr bool + assert bool + lenScanRules int + lenSkipRules int + }{ + { + name: "no config file", + fields: fields{}, + }, + { + name: "config file doesn't exist", + fields: fields{ + configFile: "testdata/test.toml", + }, + wantErr: true, + }, + { + name: "empty config file", + fields: fields{ + configFile: "testdata/empty.toml", + }, + }, + { + name: "config file with empty rules", + fields: fields{ + configFile: "testdata/webhook.toml", + }, + }, + { + name: "valid config file with scan and skip rules", + fields: fields{ + configFile: "testdata/scan-skip-rules.toml", + scanRules: []string{"testRuleA", "testRuleB"}, + skipRules: []string{"testRuleC"}, + }, + assert: true, + lenScanRules: 4, + lenSkipRules: 5, + }, + { + name: "valid config file with invalid scan rules", + fields: fields{ + configFile: "testdata/invalid-scan-skip-rules.toml", + }, + wantErr: true, + }, + { + name: "valid config file with invalid skip rules", + fields: fields{ + configFile: "testdata/invalid-skip-rules.toml", + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := &Executor{ + configFile: tt.fields.configFile, + scanRules: tt.fields.scanRules, + skipRules: tt.fields.skipRules, + } + if err := e.initRules(); (err != nil) != tt.wantErr { + t.Errorf("Executor.initRules() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.assert { + if len(e.scanRules) != tt.lenScanRules && len(e.skipRules) != tt.lenSkipRules { + t.Errorf("Executor.initRules() expected scanRules: %d and skipRules: %d, got scanRules: %d and skipRules: %d", tt.lenScanRules, tt.lenSkipRules, len(e.scanRules), len(e.skipRules)) + } + } + }) + } +} diff --git a/pkg/runtime/testdata/invalid-notifier.toml b/pkg/runtime/testdata/invalid-notifier.toml index 74c795e09..bc9a0c9d1 100644 --- a/pkg/runtime/testdata/invalid-notifier.toml +++ b/pkg/runtime/testdata/invalid-notifier.toml @@ -3,4 +3,5 @@ # notifications configuration [notifications] [notifications.invalid] + type = "invalid" url = "https://httpbin.org/post" diff --git a/pkg/runtime/testdata/webhook.toml b/pkg/runtime/testdata/webhook.toml index 11c29a2bb..12e7a40d6 100644 --- a/pkg/runtime/testdata/webhook.toml +++ b/pkg/runtime/testdata/webhook.toml @@ -3,6 +3,8 @@ # notifications configuration [notifications] [notifications.webhook] + type = "webhook" + [notifications.webhook.config] url = "https://httpbin.org/post" # empty rules [rules] \ No newline at end of file From 78b68220e2b85d4bbe99ae2468b22a8b7c89a425 Mon Sep 17 00:00:00 2001 From: Pankaj Patil Date: Tue, 29 Dec 2020 17:31:48 +0530 Subject: [PATCH 7/7] fix code smells --- pkg/config/config-reader.go | 4 ++-- pkg/config/config-reader_test.go | 12 ++---------- pkg/config/global_test.go | 16 ++-------------- pkg/policy/opa/engine_test.go | 4 ++-- pkg/runtime/executor_test.go | 3 +++ 5 files changed, 11 insertions(+), 28 deletions(-) diff --git a/pkg/config/config-reader.go b/pkg/config/config-reader.go index f29ea2bfa..1c8cc5715 100644 --- a/pkg/config/config-reader.go +++ b/pkg/config/config-reader.go @@ -52,13 +52,13 @@ func NewTerrascanConfigReader(fileName string) (*TerrascanConfigReader, error) { // return error if file doesn't exist _, err := os.Stat(fileName) if err != nil { - zap.S().Debugf("config file: %s, doesn't exist", fileName) + zap.S().Error("config file: %s, doesn't exist", fileName) return configReader, ErrNotPresent } data, err := ioutil.ReadFile(fileName) if err != nil { - zap.S().Debugf("error loading config file", zap.Error(err)) + zap.S().Error("error loading config file", zap.Error(err)) return configReader, errTomlLoadConfig } diff --git a/pkg/config/config-reader_test.go b/pkg/config/config-reader_test.go index dcbe448c8..c0aeb674d 100644 --- a/pkg/config/config-reader_test.go +++ b/pkg/config/config-reader_test.go @@ -119,16 +119,8 @@ func TestNewTerrascanConfigReader(t *testing.T) { t.Errorf("NewTerrascanConfigReader() = got %v, want %v", got, tt.want) } if tt.assertGetters { - if !reflect.DeepEqual(got.GetPolicyConfig(), tt.Policy) { - t.Errorf("NewTerrascanConfigReader() = got config: %v, want config: %v", got.GetPolicyConfig(), tt.Policy) - } - - if !reflect.DeepEqual(got.GetNotifications(), tt.notifications) { - t.Errorf("NewTerrascanConfigReader() = got notifications: %v, want notifications: %v", got.GetNotifications(), tt.notifications) - } - - if !reflect.DeepEqual(got.GetRules(), tt.Rules) { - t.Errorf("NewTerrascanConfigReader() = got rules: %v, want rules: %v", got.GetRules(), tt.Rules) + if !reflect.DeepEqual(got.GetPolicyConfig(), tt.Policy) || !reflect.DeepEqual(got.GetNotifications(), tt.notifications) || !reflect.DeepEqual(got.GetRules(), tt.Rules) { + t.Errorf("NewTerrascanConfigReader() = got config: %v, notifications: %v, rules: %v want config: %v, notifications: %v, rules: %v", got.GetPolicyConfig(), got.GetNotifications(), got.GetRules(), tt.Policy, tt.notifications, tt.Rules) } } }) diff --git a/pkg/config/global_test.go b/pkg/config/global_test.go index 28b1cf107..23de3f711 100644 --- a/pkg/config/global_test.go +++ b/pkg/config/global_test.go @@ -74,20 +74,8 @@ func TestLoadGlobalConfig(t *testing.T) { t.Errorf("LoadGlobalConfig() error = %v, wantErr %v", err, tt.wantErr) } - if GetPolicyBasePath() != tt.policyBasePath { - t.Errorf("LoadGlobalConfig() error = got BasePath: %v, want BasePath: %v", tt.policyBasePath, Global.Policy.BasePath) - } - - if GetPolicyRepoPath() != tt.policyRepoPath { - t.Errorf("LoadGlobalConfig() error = got RepoPath: %v, want RepoPath: %v", tt.policyRepoPath, Global.Policy.RepoPath) - } - - if GetPolicyRepoURL() != tt.repoURL { - t.Errorf("LoadGlobalConfig() error = got RepoURL: %v, want RepoURL: %v", tt.repoURL, Global.Policy.RepoURL) - } - - if GetPolicyBranch() != tt.branchName { - t.Errorf("LoadGlobalConfig() error = got BranchName: %v, want BranchName: %v", tt.branchName, Global.Policy.Branch) + if GetPolicyBasePath() != tt.policyBasePath || GetPolicyRepoPath() != tt.policyRepoPath || GetPolicyRepoURL() != tt.repoURL || GetPolicyBranch() != tt.branchName { + t.Errorf("LoadGlobalConfig() error = got BasePath: %v, RepoPath: %v, RepoURL: %v, BranchName: %v, want BasePath: %v, RepoPath: %v, RepoURL: %v, BranchName: %v", GetPolicyBasePath(), GetPolicyRepoPath(), GetPolicyRepoURL(), GetPolicyBranch(), tt.policyBasePath, tt.policyRepoPath, tt.repoURL, tt.branchName) } }) } diff --git a/pkg/policy/opa/engine_test.go b/pkg/policy/opa/engine_test.go index 18056474d..0c097e7ce 100644 --- a/pkg/policy/opa/engine_test.go +++ b/pkg/policy/opa/engine_test.go @@ -29,7 +29,7 @@ func TestFilterRules(t *testing.T) { name: "scan rules test", args: args{ policyPath: testPolicyPath, - scanRules: []string{"Rule.0", "Rule.1", "Rule.2", "Rule.3", "Rule.11"}, + scanRules: []string{"Rule.0", "Rule.1", "Rule.2", "Rule.3", "Rule.10"}, }, regoDataMap: getTestRegoDataMap(10), assert: true, @@ -69,7 +69,7 @@ func TestFilterRules(t *testing.T) { name: "both scan and skip rules supplied", args: args{ policyPath: testPolicyPath, - scanRules: []string{"Rule.10", "Rule.11", "Rule.12", "Rule.15", "Rule.31", "Rule.32", "Rule.40", "Rule.41", "Rule.42"}, + scanRules: []string{"Rule.6", "Rule.7", "Rule.8", "Rule.15", "Rule.31", "Rule.32", "Rule.40", "Rule.41", "Rule.42"}, skipRules: []string{"Rule.31", "Rule.32", "Rule.38"}, }, regoDataMap: getTestRegoDataMap(50), diff --git a/pkg/runtime/executor_test.go b/pkg/runtime/executor_test.go index c447854e4..dcc82acad 100644 --- a/pkg/runtime/executor_test.go +++ b/pkg/runtime/executor_test.go @@ -60,6 +60,9 @@ func (m MockPolicyEngine) Init(input string, scanRules, skipRules []string) erro } func (m MockPolicyEngine) FilterRules(input string, scanRules, skipRules []string) { + /* + This method does nothing. Required to fullfil the Engine interface contract + */ } func (m MockPolicyEngine) Configure() error {