tfsec:chore - improve tests and code cleaning

This commit add some new asserts on successful parsing tfsec results
to verify that all fields of Vulnerability was filled.

Some code organization was also made, and the entities packages was
removed and the tfsec schema output was moved to tfsec package.

Updates #718

Signed-off-by: Matheus Alcantara <matheus.alcantara@zup.com.br>

internal/services/formatters/hcl/tfsec/formatter.go

@@ -16,19 +16,18 @@ package tfsec
 
 import (
 	"encoding/json"
-	"fmt"
+	"errors"
 	"strings"
 
 	"github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability"
 	"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
 	"github.com/ZupIT/horusec-devkit/pkg/enums/tools"
 	"github.com/ZupIT/horusec-devkit/pkg/utils/logger"
 
-	dockerEntities "github.com/ZupIT/horusec/internal/entities/docker"
+	"github.com/ZupIT/horusec/internal/entities/docker"
 	"github.com/ZupIT/horusec/internal/enums/images"
 	"github.com/ZupIT/horusec/internal/helpers/messages"
 	"github.com/ZupIT/horusec/internal/services/formatters"
-	"github.com/ZupIT/horusec/internal/services/formatters/hcl/tfsec/entities"
 	vulnhash "github.com/ZupIT/horusec/internal/utils/vuln_hash"
 )
 
@@ -64,8 +63,8 @@ func (f *Formatter) startTfSec(projectSubPath string) (string, error) {
 	return output, f.parseOutput(output)
 }
 
-func (f *Formatter) getDockerConfig(projectSubPath string) *dockerEntities.AnalysisData {
-	analysisData := &dockerEntities.AnalysisData{
+func (f *Formatter) getDockerConfig(projectSubPath string) *docker.AnalysisData {
+	analysisData := &docker.AnalysisData{
 		CMD:      f.AddWorkDirInCmd(CMD, projectSubPath, tools.TfSec),
 		Language: languages.HCL,
 	}
@@ -74,37 +73,32 @@ func (f *Formatter) getDockerConfig(projectSubPath string) *dockerEntities.Analy
 }
 
 func (f *Formatter) parseOutput(output string) error {
-	var vulnerabilities *entities.Vulnerabilities
+	var vulnerabilities *tfsecVulnerabilities
 
 	if err := json.Unmarshal([]byte(output), &vulnerabilities); err != nil {
 		if !strings.Contains(output, "panic") {
-			return fmt.Errorf("{HORUSEC_CLI} Error %s", output)
+			return errors.New(output)
 		}
-
 		return err
 	}
 
 	for index := range vulnerabilities.Results {
-		f.AddNewVulnerabilityIntoAnalysis(f.setVulnerabilityData(index, vulnerabilities.Results))
+		f.AddNewVulnerabilityIntoAnalysis(f.newVulnerability(&vulnerabilities.Results[index]))
 	}
 
 	return nil
 }
 
-func (f *Formatter) setVulnerabilityData(index int, results []entities.Result) *vulnerability.Vulnerability {
-	vuln := f.getDefaultVulnerabilityData()
-	vuln.Severity = results[index].GetSeverity()
-	vuln.Details = results[index].GetDetails()
-	vuln.Line = results[index].GetStartLine()
-	vuln.Code = f.GetCodeWithMaxCharacters(results[index].GetCode(), 0)
-	vuln.File = f.RemoveSrcFolderFromPath(results[index].GetFilename())
+func (f *Formatter) newVulnerability(result *tfsecResult) *vulnerability.Vulnerability {
+	vuln := &vulnerability.Vulnerability{
+		SecurityTool: tools.TfSec,
+		Language:     languages.HCL,
+		Severity:     result.getSeverity(),
+		Details:      result.getDetails(),
+		Line:         result.getStartLine(),
+		Code:         f.GetCodeWithMaxCharacters(result.getCode(), 0),
+		File:         f.RemoveSrcFolderFromPath(result.getFilename()),
+	}
 	vuln = vulnhash.Bind(vuln)
 	return f.SetCommitAuthor(vuln)
 }
-
-func (f *Formatter) getDefaultVulnerabilityData() *vulnerability.Vulnerability {
-	vulnerabilitySeverity := &vulnerability.Vulnerability{}
-	vulnerabilitySeverity.SecurityTool = tools.TfSec
-	vulnerabilitySeverity.Language = languages.HCL
-	return vulnerabilitySeverity
-}

internal/services/formatters/hcl/tfsec/formatter_test.go

@@ -18,83 +18,148 @@ import (
 	"errors"
 	"testing"
 
-	entitiesAnalysis "github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
+	"github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
+	"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
 	"github.com/ZupIT/horusec-devkit/pkg/enums/tools"
 	"github.com/stretchr/testify/assert"
 
-	cliConfig "github.com/ZupIT/horusec/config"
+	"github.com/ZupIT/horusec/config"
 	"github.com/ZupIT/horusec/internal/entities/toolsconfig"
-	"github.com/ZupIT/horusec/internal/entities/workdir"
 	"github.com/ZupIT/horusec/internal/services/formatters"
 	"github.com/ZupIT/horusec/internal/utils/testutil"
 )
 
-func TestStartHCLTfSec(t *testing.T) {
-	t.Run("should success execute container and process output", func(t *testing.T) {
+func TestTFSecParseOutput(t *testing.T) {
+	t.Run("should add 5 vulnerabilities on analysis with no errors", func(t *testing.T) {
 		dockerAPIControllerMock := testutil.NewDockerMock()
-		analysis := &entitiesAnalysis.Analysis{}
-		config := &cliConfig.Config{}
-		config.WorkDir = &workdir.WorkDir{}
-
-		output := `{"results":[{"rule_id":"AWS018","link":"https://github.com/liamg/tfsec/wiki/AWS018","location":{"filename":"/src/terraform/main.tf","start_line":2,"end_line":5},"description":"Resource 'aws_security_group_rule.my-rule' should include a description for auditing purposes.","severity":"ERROR"},{"rule_id":"AWS006","link":"https://github.com/liamg/tfsec/wiki/AWS006","location":{"filename":"/src/terraform/main.tf","start_line":4,"end_line":4},"description":"Resource 'aws_security_group_rule.my-rule' defines a fully open ingress security group rule.","severity":"WARNING"},{"rule_id":"AWS004","link":"https://github.com/liamg/tfsec/wiki/AWS004","location":{"filename":"/src/terraform/main.tf","start_line":9,"end_line":9},"description":"Resource 'aws_alb_listener.my-alb-listener' uses plain HTTP instead of HTTPS.","severity":"ERROR"},{"rule_id":"AWS003","link":"https://github.com/liamg/tfsec/wiki/AWS003","location":{"filename":"/src/terraform/main.tf","start_line":12,"end_line":14},"description":"Resource 'aws_db_security_group.my-group' uses EC2 Classic. Use a VPC instead.","severity":"ERROR"},{"rule_id":"AZU003","link":"https://github.com/liamg/tfsec/wiki/AZU003","location":{"filename":"/src/terraform/main.tf","start_line":18,"end_line":18},"description":"Resource 'azurerm_managed_disk.source' defines an unencrypted managed disk.","severity":"ERROR"}]}`
-
 		dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return(output, nil)
 
+		analysis := new(analysis.Analysis)
+		config := config.New()
+
 		service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
 		formatter := NewFormatter(service)
-
 		formatter.StartAnalysis("")
 
-		assert.NotEmpty(t, analysis)
 		assert.Len(t, analysis.AnalysisVulnerabilities, 5)
+		for _, v := range analysis.AnalysisVulnerabilities {
+			vuln := v.Vulnerability
+
+			assert.Equal(t, tools.TfSec, vuln.SecurityTool)
+			assert.Equal(t, languages.HCL, vuln.Language)
+			assert.NotEmpty(t, vuln.Details, "Expected not empty details")
+			assert.NotEmpty(t, vuln.Code, "Expected not empty code")
+			assert.NotEmpty(t, vuln.File, "Expected not emppty file name")
+			assert.NotEmpty(t, vuln.Line, "Expected not empty line")
+			assert.NotEmpty(t, vuln.Severity, "Expected not empty severity")
+
+		}
 	})
 
-	t.Run("should return error when invalid output", func(t *testing.T) {
+	t.Run("should add error on analysis when invalid output", func(t *testing.T) {
 		dockerAPIControllerMock := testutil.NewDockerMock()
-		analysis := &entitiesAnalysis.Analysis{}
-		config := &cliConfig.Config{}
-		config.WorkDir = &workdir.WorkDir{}
+		dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return("invalid", nil)
 
-		output := "!@#!@#"
+		analysis := new(analysis.Analysis)
 
-		dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return(output, nil)
+		config := config.New()
 
 		service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
 		formatter := NewFormatter(service)
+		formatter.StartAnalysis("")
 
-		assert.NotPanics(t, func() {
-			formatter.StartAnalysis("")
-		})
+		assert.True(t, analysis.HasErrors(), "Expected errors on analysis")
 	})
 
-	t.Run("should return error when executing container", func(t *testing.T) {
+	t.Run("should add error of executing container on analysis", func(t *testing.T) {
 		dockerAPIControllerMock := testutil.NewDockerMock()
-		analysis := &entitiesAnalysis.Analysis{}
-		config := &cliConfig.Config{}
-		config.WorkDir = &workdir.WorkDir{}
-
 		dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return("", errors.New("test"))
 
+		analysis := new(analysis.Analysis)
+
+		config := config.New()
 		service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
 		formatter := NewFormatter(service)
+		formatter.StartAnalysis("")
 
-		assert.NotPanics(t, func() {
-			formatter.StartAnalysis("")
-		})
+		assert.True(t, analysis.HasErrors(), "Expected errors on analysis")
 	})
 	t.Run("Should not execute tool because it's ignored", func(t *testing.T) {
-		analysis := &entitiesAnalysis.Analysis{}
+		analysis := new(analysis.Analysis)
+
 		dockerAPIControllerMock := testutil.NewDockerMock()
-		config := &cliConfig.Config{}
-		config.WorkDir = &workdir.WorkDir{}
+
+		config := config.New()
 		config.ToolsConfig = toolsconfig.ToolsConfig{
 			tools.TfSec: toolsconfig.Config{
 				IsToIgnore: true,
 			},
 		}
+
 		service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
 		formatter := NewFormatter(service)
-
 		formatter.StartAnalysis("")
 	})
 }
+
+const output = `
+{
+  "results": [
+    {
+      "rule_id": "AWS018",
+      "link": "https://github.com/liamg/tfsec/wiki/AWS018",
+      "location": {
+        "filename": "/src/terraform/main.tf",
+        "start_line": 2,
+        "end_line": 5
+      },
+      "description": "Resource 'aws_security_group_rule.my-rule' should include a description for auditing purposes.",
+      "severity": "ERROR"
+    },
+    {
+      "rule_id": "AWS006",
+      "link": "https://github.com/liamg/tfsec/wiki/AWS006",
+      "location": {
+        "filename": "/src/terraform/main.tf",
+        "start_line": 4,
+        "end_line": 4
+      },
+      "description": "Resource 'aws_security_group_rule.my-rule' defines a fully open ingress security group rule.",
+      "severity": "WARNING"
+    },
+    {
+      "rule_id": "AWS004",
+      "link": "https://github.com/liamg/tfsec/wiki/AWS004",
+      "location": {
+        "filename": "/src/terraform/main.tf",
+        "start_line": 9,
+        "end_line": 9
+      },
+      "description": "Resource 'aws_alb_listener.my-alb-listener' uses plain HTTP instead of HTTPS.",
+      "severity": "ERROR"
+    },
+    {
+      "rule_id": "AWS003",
+      "link": "https://github.com/liamg/tfsec/wiki/AWS003",
+      "location": {
+        "filename": "/src/terraform/main.tf",
+        "start_line": 12,
+        "end_line": 14
+      },
+      "description": "Resource 'aws_db_security_group.my-group' uses EC2 Classic. Use a VPC instead.",
+      "severity": "ERROR"
+    },
+    {
+      "rule_id": "AZU003",
+      "link": "https://github.com/liamg/tfsec/wiki/AZU003",
+      "location": {
+        "filename": "/src/terraform/main.tf",
+        "start_line": 18,
+        "end_line": 18
+      },
+      "description": "Resource 'azurerm_managed_disk.source' defines an unencrypted managed disk.",
+      "severity": "ERROR"
+    }
+  ]
+}
+`

internal/services/formatters/hcl/tfsec/entities/location.go → internal/services/formatters/hcl/tfsec/location.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package entities
+package tfsec
 
 type Location struct {
 	Filename  string `json:"filename"`

internal/services/formatters/hcl/tfsec/entities/result.go → internal/services/formatters/hcl/tfsec/result.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package entities
+package tfsec
 
 import (
 	"fmt"
@@ -21,35 +21,35 @@ import (
 	"github.com/ZupIT/horusec-devkit/pkg/enums/severities"
 )
 
-type Result struct {
+type tfsecResult struct {
 	RuleID      string   `json:"rule_id"`
 	Link        string   `json:"link"`
 	Location    Location `json:"location"`
 	Description string   `json:"description"`
 	Severity    string   `json:"severity"`
 }
 
-func (r *Result) GetDetails() string {
+func (r *tfsecResult) getDetails() string {
 	return r.RuleID + " -> [" + r.Description + "]"
 }
 
-func (r *Result) GetStartLine() string {
+func (r *tfsecResult) getStartLine() string {
 	return strconv.Itoa(r.Location.StartLine)
 }
 
-func (r *Result) GetCode() string {
+func (r *tfsecResult) getCode() string {
 	return fmt.Sprintf("code beetween line %d and %d.", r.Location.StartLine, r.Location.EndLine)
 }
 
-func (r *Result) GetFilename() string {
+func (r *tfsecResult) getFilename() string {
 	return r.Location.Filename
 }
 
-func (r *Result) GetSeverity() severities.Severity {
+func (r *tfsecResult) getSeverity() severities.Severity {
 	return r.mapSeverityValues()[r.Severity]
 }
 
-func (r *Result) mapSeverityValues() map[string]severities.Severity {
+func (r *tfsecResult) mapSeverityValues() map[string]severities.Severity {
 	return map[string]severities.Severity{
 		"ERROR":   severities.High,
 		"WARNING": severities.Medium,

internal/services/formatters/hcl/tfsec/entities/result_test.go → internal/services/formatters/hcl/tfsec/result_test.go

@@ -12,16 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package entities
+package tfsec
 
 import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func resultMock() *Result {
-	return &Result{
+func resultMock() *tfsecResult {
+	return &tfsecResult{
 		RuleID: "AWS123",
 		Link:   "test",
 		Location: Location{
@@ -36,24 +36,24 @@ func resultMock() *Result {
 
 func TestGetDetails(t *testing.T) {
 	t.Run("should success get result details", func(t *testing.T) {
-		assert.NotEmpty(t, resultMock().GetDetails())
+		assert.NotEmpty(t, resultMock().getDetails())
 	})
 }
 
 func TestGetStartLine(t *testing.T) {
 	t.Run("should success get start line", func(t *testing.T) {
-		assert.NotEmpty(t, resultMock().GetStartLine())
+		assert.NotEmpty(t, resultMock().getStartLine())
 	})
 }
 
 func TestGetCode(t *testing.T) {
 	t.Run("should success get code", func(t *testing.T) {
-		assert.NotEmpty(t, resultMock().GetCode())
+		assert.NotEmpty(t, resultMock().getCode())
 	})
 }
 
 func TestGetFilename(t *testing.T) {
 	t.Run("should success get filename", func(t *testing.T) {
-		assert.NotEmpty(t, resultMock().GetFilename())
+		assert.NotEmpty(t, resultMock().getFilename())
 	})
 }

internal/services/formatters/hcl/tfsec/entities/vulnerabilities.go → internal/services/formatters/hcl/tfsec/vulnerabilities.go

@@ -12,8 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package entities
+package tfsec
 
-type Vulnerabilities struct {
-	Results []Result `json:"results"`
+type tfsecVulnerabilities struct {
+	Results []tfsecResult `json:"results"`
 }