Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement scan and skip rules #441

Merged
merged 7 commits into from
Jan 5, 2021

Conversation

patilpankaj212
Copy link
Contributor

  1. command line options to scan and skip specific rules
  2. config file based rule scanning and skipping
  3. unit tests

@codecov
Copy link

codecov bot commented Dec 23, 2020

Codecov Report

Merging #441 (78b6822) into master (c57bb06) will increase coverage by 0.72%.
The diff coverage is 91.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
+ Coverage   73.39%   74.11%   +0.72%     
==========================================
  Files          86       87       +1     
  Lines        1962     2013      +51     
==========================================
+ Hits         1440     1492      +52     
  Misses        388      388              
+ Partials      134      133       -1     
Impacted Files Coverage Δ
pkg/notifications/webhook/webhook.go 36.00% <63.63%> (+12.47%) ⬆️
pkg/config/config-reader.go 85.00% <85.00%> (ø)
pkg/config/global.go 92.30% <88.23%> (+14.80%) ⬆️
pkg/http-server/file-scan.go 71.87% <90.00%> (+4.01%) ⬆️
pkg/policy/opa/engine.go 64.17% <95.83%> (+3.73%) ⬆️
pkg/cli/run.go 91.17% <100.00%> (-0.13%) ⬇️
pkg/cli/scan.go 77.77% <100.00%> (+2.77%) ⬆️
pkg/http-server/remote-repo.go 62.96% <100.00%> (ø)
pkg/notifications/notifiers.go 92.10% <100.00%> (+7.89%) ⬆️
pkg/runtime/executor.go 90.00% <100.00%> (-4.00%) ⬇️
... and 3 more

2. define variable for error string in test func
pkg/http-server/file-scan.go Outdated Show resolved Hide resolved
@@ -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{})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be taking rules to scan and skip for the API server as well. This functionality applies to the scanning for API server as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented

pkg/policy/opa/engine.go Outdated Show resolved Hide resolved
pkg/policy/opa/engine.go Outdated Show resolved Hide resolved
filterRules(e, policyPath, scanRules, skipRules)

// update the rule count
e.stats.ruleCount = len(e.regoDataMap)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be it's a good idea to error out if the ruleCount is less than 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiple engines are created based on policy path slice, cannot error out.

}
if len(rules) > 0 {
if key == scanRulesKey {
e.scanRules = append(e.scanRules, rules...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the right place to append to scan rules, this function should just return the list of rules. Caller function should take responsibility of using the list of rules. In this way this function can become more generic and we would not require to pass the Executor object to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in the refactored code

pkg/policy/interface.go Outdated Show resolved Hide resolved
@@ -139,3 +168,68 @@ 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, initRules() as a function name should suffice, isn't it?

pkg/runtime/executor.go Outdated Show resolved Hide resolved
pkg/runtime/executor.go Outdated Show resolved Hide resolved
2. refactor the existing config reader code
3. update unit tests
4. incorporate PR review comments
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

pkg/config/global.go Show resolved Hide resolved
@williepaul williepaul merged commit b8a6849 into tenable:master Jan 5, 2021
@patilpankaj212 patilpankaj212 deleted the scan-and-skip-rules branch May 18, 2021 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants