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

fix: added validation for module local source dir #793

Merged
merged 5 commits into from
May 20, 2021

Conversation

Rchanger
Copy link
Contributor

#782
Fix: Added check for the module source local directory.

@codecov
Copy link

codecov bot commented May 19, 2021

Codecov Report

Merging #793 (e7c2d8f) into master (bea2473) will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #793      +/-   ##
==========================================
+ Coverage   74.75%   74.87%   +0.11%     
==========================================
  Files         111      111              
  Lines        3324     3339      +15     
==========================================
+ Hits         2485     2500      +15     
  Misses        654      654              
  Partials      185      185              
Impacted Files Coverage Δ
pkg/iac-providers/terraform/commons/load-dir.go 85.61% <100.00%> (+1.05%) ⬆️
pkg/utils/dir.go 81.81% <100.00%> (+15.15%) ⬆️

Copy link
Contributor

@kanchwala-yusuf kanchwala-yusuf left a comment

Choose a reason for hiding this comment

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

Have some minor comments, approving this PR

Comment on lines 322 to 324
var pathToModule string
var err error
var moduleDirDiags hcl.Diagnostics
Copy link
Contributor

Choose a reason for hiding this comment

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

These can go in a single var block, just makes the code tidier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make this change .

@@ -321,6 +321,7 @@ func (t TerraformDirectoryLoader) buildUnifiedConfig(rootMod *hclConfigs.Module,
// figure out path sub module directory, if it's remote then download it locally
var pathToModule string
var err error
var moduleDirDiags hcl.Diagnostics
if downloader.IsLocalSourceAddr(req.SourceAddr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering, if it is time to get rid of this if-else ladder and move to a switch case?

patilpankaj212
patilpankaj212 previously approved these changes May 19, 2021
@@ -344,6 +347,15 @@ func (t TerraformDirectoryLoader) buildUnifiedConfig(rootMod *hclConfigs.Module,
}
}

// verify whether the module source directory has any .tf config files
if !t.parser.IsConfigDir(pathToModule) {
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 modify this check because this will also modify the error message for non existing directories.

@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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@patilpankaj212 patilpankaj212 self-requested a review May 20, 2021 09:49
@kanchwala-yusuf kanchwala-yusuf merged commit 0c3c547 into tenable:master May 20, 2021
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.

Scan for terraform doesn't error out if a module definition refers to a directory with no tf files
3 participants