adds support to scan directory with all iac providers in cli mode #674
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #674 +/- ##
==========================================
+ Coverage 73.28% 73.85% +0.57%
==========================================
Files 110 110
Lines 3099 3175 +76
==========================================
+ Hits 2271 2345 +74
- Misses 650 652 +2
Partials 178 178
|
patilpankaj212
force-pushed
the
multiple-iac-provider-scans
branch
from
0425096 to
b5c0465
Compare
| resourceConfig := make(output.AllResourceConfigs) | ||
|
|
||
| // when dir path has value, only then it will 'all iac' scan | ||
| // when file path has value, we will go with the only iac provider in the list |
There was a problem hiding this comment.
Hey @patilpankaj212 , I wondering if we should keep the all iac scan behavior consistent between directory and file scanning?
| if e.iacType == "all" { | ||
| for _, ip := range iacProvider.SupportedIacProviders() { | ||
| // skip tfplan because it doesn't support directory scanning | ||
| if ip == "tfplan" { |
There was a problem hiding this comment.
Should this list also have kustomize iac type?
pkg/runtime/executor.go
Outdated
Comment on lines
169
to
193
| // channel for directory scan response | ||
| scanRespChan := make(chan dirScanResp) | ||
|
|
||
| // create results output from Iac provider[s] | ||
| for _, iacP := range e.iacProviders { | ||
| go func(ip iacProvider.IacProvider) { | ||
| rc, err := ip.LoadIacDir(e.dirPath) | ||
| scanRespChan <- dirScanResp{err, rc} | ||
| }(iacP) | ||
| } | ||
|
|
||
| for i := 0; i < len(e.iacProviders); i++ { | ||
| sr := <-scanRespChan | ||
| merr = multierror.Append(merr, sr.err) | ||
| // deduplication of resources | ||
| if len(resourceConfig) > 0 { | ||
| for key, r := range sr.rc { | ||
| updateResourceConfigs(key, r, resourceConfig) | ||
| } | ||
| } else { | ||
| for key := range sr.rc { | ||
| resourceConfig[key] = append(resourceConfig[key], sr.rc[key]...) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this piece of code is now eligible to be in it's own function. What do you think @patilpankaj212 ?
pkg/runtime/executor.go
Outdated
| return results, err | ||
|
|
||
| // for the iac providers that don't implement sub folder scanning | ||
| // return the error (existing behavior) |
There was a problem hiding this comment.
May be improve the comment here (existing behavior). Elaborate on the behavior rather than comment it as existing behavior
Because once this code is checked-in current behavior becomes the existing behavior.
pkg/runtime/executor.go
Outdated
Comment on lines
218
to
234
| // channel for engine evaluation result | ||
| evalResultChan := make(chan engineEvalResult) | ||
|
|
||
| for _, engine := range e.policyEngines { | ||
| output, err := engine.Evaluate(policy.EngineInput{InputData: &results.ResourceConfig}) | ||
| if err != nil { | ||
| return results, err | ||
| go func(eng policy.Engine) { | ||
| output, err := eng.Evaluate(policy.EngineInput{InputData: &results.ResourceConfig}) | ||
| evalResultChan <- engineEvalResult{err, output} | ||
| }(engine) | ||
| } | ||
|
|
||
| for i := 0; i < len(e.policyEngines); i++ { | ||
| evalR := <-evalResultChan | ||
| if evalR.err != nil { | ||
| return results, evalR.err | ||
| } | ||
| violations = violations.Add(output.AsViolationStore()) | ||
| violations = violations.Add(evalR.output.AsViolationStore()) | ||
| } |
There was a problem hiding this comment.
I believe, policy evaluation can also get it's own function.
pkg/runtime/executor.go
Outdated
Comment on lines
264
to
284
| func updateResourceConfigs(resourceType string, resources []output.ResourceConfig, allResources output.AllResourceConfigs) { | ||
| for _, res := range resources { | ||
| if !isConfigPresent(allResources[resourceType], res) { | ||
| allResources[resourceType] = append(allResources[resourceType], res) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // isConfigPresent checks whether a resource is already present in the list of configs or not | ||
| // the equality of a resource is based on name, source and config of the resource | ||
| func isConfigPresent(resources []output.ResourceConfig, resourceConfig output.ResourceConfig) bool { | ||
| for _, resource := range resources { | ||
| if resource.Name == resourceConfig.Name && resource.Source == resourceConfig.Source { | ||
| if reflect.DeepEqual(resource.Config, resourceConfig.Config) { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
There was a problem hiding this comment.
Can these functions become part of utils? I see wider applications of the deduplication logic.
patilpankaj212
force-pushed
the
multiple-iac-provider-scans
branch
from
bf4f874 to
c8bf725
Compare
kanchwala-yusuf
previously approved these changes
Apr 30, 2021
pkg/utils/resource.go
Outdated
| @@ -61,3 +62,25 @@ func GetResourceCount(resourceMapping map[string][]output.ResourceConfig) (count | |||
| } | |||
| return | |||
| } | |||
|
|
|||
| // UpdateResourceConfigs adds a resource of given type if it is not present in allResources | |||
| func UpdateResourceConfigs(resourceType string, resources []output.ResourceConfig, allResources output.AllResourceConfigs) { | |||
There was a problem hiding this comment.
Hey @patilpankaj212 ,
Does it make sense to have this function and other functions in this file to be moved to the output package itself. Come to think of it these are not utils in real sense but specific use cases related to the AllResourcesConfig in the output package.
|
Kudos, SonarCloud Quality Gate passed!
|
kanchwala-yusuf
approved these changes
Apr 30, 2021
devang-gaur
pushed a commit
to devang-gaur/terrascan
that referenced
this pull request
May 5, 2021
…nable#674) * initial changes * fix unit tests * run dir scan and engine evaluation concurrently * e2e test fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
scan_errors, which will include all the errors that occurred while the iac providers did the folder scan.scan_errorsis not supported with default human readable output, also it is not part of--config-only