From f30a14ddba9085e1892b7e7f80df1675a2f8299b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A8Owen=20Rumney?= Date: Fri, 23 Oct 2020 15:01:01 +0100 Subject: [PATCH] Add the final tests update the interface even further to get it cleaner --- README.md | 4 +++ models/result.go | 32 +++++++++++++++----- models/run.go | 61 ++++++++++++++++++++++++--------------- models/tool.go | 35 ++++++++++++---------- sarif/sarif.go | 30 ++++++++++++------- test/result_stage_test.go | 20 +++++-------- test/run_stage_test.go | 40 ++++++++++++------------- test/run_test.go | 2 +- test/sarif_stage_test.go | 8 ++--- 9 files changed, 135 insertions(+), 97 deletions(-) diff --git a/README.md b/README.md index eda4079..f750f3b 100644 --- a/README.md +++ b/README.md @@ -14,3 +14,7 @@ For more information about SARIF, you can visit the [Oasis Open](https://www.oas ## Usage +Add an import to `github.com/owenrumney/go-sarif/sarif` + +Creating a new Sarif report is done by passing the version, the only supported at the moment is `2.1.0` + diff --git a/models/result.go b/models/result.go index 2d293b2..9437a3f 100644 --- a/models/result.go +++ b/models/result.go @@ -32,18 +32,36 @@ type Location struct { Uri string `json:"uri"` } -func NewResult(level, message, ruleId string) *Result { +func newRuleResult(ruleId string) *Result { return &Result{ - Level: level, - Message: &TextBlock{ - Text: message, - }, RuleId: ruleId, } } -func (r *Result) AddLocation(location *PhysicalLocation) { - r.Locations = append(r.Locations, &ResultLocation{ +func (result *Result) WithLevel(level string) *Result { + result.Level = level + return result +} + +func (result *Result) WithMessage(message string) *Result { + result.Message = &TextBlock{ + Text: message, + } + return result +} + +func (result *Result) WithLocationDetails(path string, startLine, startColumn int) *Result { + location := &PhysicalLocation{ + ArtifactLocation: &ArtifactLocation{ + Uri: path, + }, + Region: &Region{ + StartLine: startLine, + StartColumn: startColumn, + }, + } + result.Locations = append(result.Locations, &ResultLocation{ PhysicalLocation: location, }) + return result } diff --git a/models/run.go b/models/run.go index e825069..16610b4 100644 --- a/models/run.go +++ b/models/run.go @@ -10,17 +10,11 @@ type LocationWrapper struct { Location *Location `json:"location,omitentry"` } -// NewRun will create a new Run -func NewRun(tool *Tool) *Run { - return &Run{ - Tool: tool, - } -} - -func (run *Run) GetOrCreateLocation(location *Location) (int, error) { +// AddArtifact returns the index of the newly added ArtifactLocation +func (run *Run) AddArtifact(location *Location) int { for i, l := range run.Artifacts { if l.Location.Uri == location.Uri { - return i, nil + return i } } run.Artifacts = append(run.Artifacts, &LocationWrapper{ @@ -28,23 +22,44 @@ func (run *Run) GetOrCreateLocation(location *Location) (int, error) { Uri: location.Uri, }, }) - return len(run.Artifacts) - 1, nil + return len(run.Artifacts) - 1 } -// AddResultDetails adds rules to the driver and artifact locations if they are missing. It adds the result to the result block as well -func (run *Run) AddResultDetails(rule *Rule, location *PhysicalLocation, result *Result) error { - ruleIndex, err := run.Tool.Driver.GetOrCreateRule(rule) - if err != nil { - return err - } - result.RuleIndex = ruleIndex - locationIndex, err := run.GetOrCreateLocation(&Location{Uri: location.ArtifactLocation.Uri}) - if err != nil { - return nil +func (run *Run) AddRule(ruleId string) *Rule { + for _, rule := range run.Tool.Driver.Rules { + if rule.Id == ruleId { + return rule + } } - location.ArtifactLocation.Index = locationIndex - result.AddLocation(location) + rule := newRule(ruleId) + run.Tool.Driver.Rules = append(run.Tool.Driver.Rules, rule) + return rule +} +func (run *Run) AddResult(ruleId string) *Result { + for _, result := range run.Results { + if result.RuleId == ruleId { + return result + } + } + result := newRuleResult(ruleId) run.Results = append(run.Results, result) - return nil + return result +} + +// AddResultDetails adds rules to the driver and artifact locations if they are missing. It adds the result to the result block as well +func (run *Run) AddResultDetails(rule *Rule, result *Result, location string) { + ruleIndex := run.Tool.Driver.getOrCreateRule(rule) + result.RuleIndex = ruleIndex + locationIndex := run.AddArtifact(&Location{Uri: location}) + updateResultLocationIndex(result, location, locationIndex) +} + +func updateResultLocationIndex(result *Result, location string, index int) { + for _, resultLocation := range result.Locations { + if resultLocation.PhysicalLocation.ArtifactLocation.Uri == location { + resultLocation.PhysicalLocation.ArtifactLocation.Index = index + break + } + } } diff --git a/models/tool.go b/models/tool.go index 21e40e2..f2fff35 100644 --- a/models/tool.go +++ b/models/tool.go @@ -17,32 +17,35 @@ type Rule struct { Properties map[string]string `json:"properties,omitempty"` } -func (driver *Driver) GetOrCreateRule(rule *Rule) (int, error) { +func (driver *Driver) getOrCreateRule(rule *Rule) int { for i, r := range driver.Rules { if r.Id == rule.Id { - return i, nil + return i } } driver.Rules = append(driver.Rules, rule) - return len(driver.Rules) - 1, nil + return len(driver.Rules) - 1 } -func NewRule(id, description, helpUri string, properties map[string]string) *Rule { +func newRule(ruleId string) *Rule { return &Rule{ - Id: id, - ShortDescription: &TextBlock{ - Text: description, - }, - HelpUri: helpUri, - Properties: properties, + Id: ruleId, } } -func NewTool(name, informationUri string) *Tool { - return &Tool{ - Driver: &Driver{ - Name: name, - InformationUri: informationUri, - }, +func (rule *Rule) WithDescription(description string) *Rule { + rule.ShortDescription = &TextBlock{ + Text: description, } + return rule +} + +func (rule *Rule) WithHelpUri(helpUrl string) *Rule { + rule.HelpUri = helpUrl + return rule +} + +func (rule *Rule) WithProperties(properties map[string]string) *Rule { + rule.Properties = properties + return rule } diff --git a/sarif/sarif.go b/sarif/sarif.go index 2d6fc65..95a0cb0 100644 --- a/sarif/sarif.go +++ b/sarif/sarif.go @@ -9,12 +9,12 @@ import ( "github.com/owenrumney/go-sarif/models" ) -type SarifVersion string +type Version string -const SarifVersion210 SarifVersion = "2.1.0" +const Version210 Version = "2.1.0" -var versions = map[SarifVersion]string{ - SarifVersion210: "http://json.schemastore.org/sarif-2.1.0-rtm.4", +var versions = map[Version]string{ + Version210: "http://json.schemastore.org/sarif-2.1.0-rtm.4", } type Report struct { @@ -23,7 +23,7 @@ type Report struct { Runs []*models.Run `json:"runs"` } -func New(version SarifVersion) (*Report, error) { +func New(version Version) (*Report, error) { schema, err := getVersionSchema(version) if err != nil { return nil, err @@ -35,7 +35,21 @@ func New(version SarifVersion) (*Report, error) { }, nil } -func getVersionSchema(version SarifVersion) (string, error) { +func (sarif *Report) AddRun(toolName, informationUri string) *models.Run { + tool := &models.Tool{ + Driver: &models.Driver{ + Name: toolName, + InformationUri: informationUri, + }, + } + run := &models.Run{ + Tool: tool, + } + sarif.Runs = append(sarif.Runs, run) + return run +} + +func getVersionSchema(version Version) (string, error) { for ver, schema := range versions { if ver == version { return schema, nil @@ -44,10 +58,6 @@ func getVersionSchema(version SarifVersion) (string, error) { return "", errors.New(fmt.Sprintf("version [%s] is not supported", version)) } -func (sarif *Report) AddRun(run *models.Run) { - sarif.Runs = append(sarif.Runs, run) -} - func (sarif *Report) Write(w io.Writer) error { marshal, err := json.Marshal(sarif) if err != nil { diff --git a/test/result_stage_test.go b/test/result_stage_test.go index f72fb93..2c651d1 100644 --- a/test/result_stage_test.go +++ b/test/result_stage_test.go @@ -22,7 +22,12 @@ func createNewResultTest(t *testing.T) (*resultTest, *resultTest, *resultTest) { } func (rt *resultTest) a_new_result() { - rt.result = models.NewResult("error", "there was an error", "test-rule") + rt.result = &models.Result{ + RuleId: "test-rule", + } + + rt.result.WithLevel("error"). + WithMessage("there was an error") } func (rt *resultTest) and() *resultTest { @@ -38,18 +43,7 @@ func (rt *resultTest) the_result_is_displayed_converted_a_string() { } func (rt *resultTest) the_result_has_a_location_added() *resultTest { - location := &models.PhysicalLocation{ - ArtifactLocation: &models.ArtifactLocation{ - Uri: "/tmp/code/location", - Index: 0, - }, - Region: &models.Region{ - StartColumn: 1, - StartLine: 1, - }, - } - - rt.result.AddLocation(location) + rt.result.WithLocationDetails("/tmp/code/location", 1, 1) return rt } diff --git a/test/run_stage_test.go b/test/run_stage_test.go index 10516f2..8a35d96 100644 --- a/test/run_stage_test.go +++ b/test/run_stage_test.go @@ -22,8 +22,14 @@ func createNewRunTest(t *testing.T) (*runTest, *runTest, *runTest) { } func (rt *runTest) a_new_run_is_created() { - tool := models.NewTool("tfsec", "https://tfsec.dev") - rt.run = models.NewRun(tool) + rt.run = &models.Run{ + Tool: &models.Tool{ + Driver: &models.Driver{ + Name: "tfsec", + InformationUri: "https://tfsec.dev", + }, + }, + } } func (rt *runTest) the_run_is_converted_to_a_string() { @@ -44,7 +50,7 @@ func (rt *runTest) an_artifact_is_added_to_the_run(locationUri string) *runTest Uri: locationUri, } - rt.run.GetOrCreateLocation(location) + rt.run.AddArtifact(location) return rt } @@ -58,11 +64,7 @@ func (rt *runTest) the_index_of_location_is(locationUri string, expectedIndex in Uri: locationUri, } - locationIndex, err := rt.run.GetOrCreateLocation(location) - if err != nil { - rt.t.Error(err) - } - + locationIndex := rt.run.AddArtifact(location) assert.Equal(rt.t, expectedIndex, locationIndex) return rt } @@ -70,20 +72,16 @@ func (rt *runTest) the_index_of_location_is(locationUri string, expectedIndex in func (rt *runTest) a_result_is_added_to_the_run() *runTest { resultLocation := "/tmp/result/code" - rule := models.NewRule("AWS001", "S3 Bucket has an ACL defined which allows public access.", "https://www.tfsec.dev/docs/aws/AWS001", nil) - - location := &models.PhysicalLocation{ - ArtifactLocation: &models.ArtifactLocation{ - Uri: resultLocation, - }, - Region: &models.Region{ - StartLine: 1, - StartColumn: 1, - }, - } + rule := rt.run.AddRule("AWS001"). + WithDescription("S3 Bucket has an ACL defined which allows public access."). + WithHelpUri("https://www.tfsec.dev/docs/aws/AWS001"). + WithProperties(map[string]string{"propertyName": "propertyValue"}) - result := models.NewResult("error", "Resource 'my_bucket' has an ACL which allows public access.", rule.Id) + result := rt.run.AddResult(rule.Id). + WithLevel("error"). + WithMessage("Resource 'my_bucket' has an ACL which allows public access."). + WithLocationDetails(resultLocation, 1, 1) - rt.run.AddResultDetails(rule, location, result) + rt.run.AddResultDetails(rule, result, resultLocation) return rt } diff --git a/test/run_test.go b/test/run_test.go index 5b29018..2bec1e4 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -42,7 +42,7 @@ func Test_getting_the_location_index_for_an_existing_run(t *testing.T) { func Test_create_a_run_with_a_result_added(t *testing.T) { given, when, then := createNewRunTest(t) - expected := `{"tool":{"driver":{"name":"tfsec","informationUri":"https://tfsec.dev","rules":[{"id":"AWS001","shortDescription":{"text":"S3 Bucket has an ACL defined which allows public access."},"helpUri":"https://www.tfsec.dev/docs/aws/AWS001"}]}},"artifacts":[{"location":{"uri":"/tmp/result/code"}}],"results":[{"level":"error","message":{"text":"Resource 'my_bucket' has an ACL which allows public access."},"ruleId":"AWS001","ruleIndex":0,"locations":[{"physicalLocation":{"artifactLocation":{"uri":"/tmp/result/code","index":0},"region":{"startLine":1,"startColumn":1}}}]}]}` + expected := `{"tool":{"driver":{"name":"tfsec","informationUri":"https://tfsec.dev","rules":[{"id":"AWS001","shortDescription":{"text":"S3 Bucket has an ACL defined which allows public access."},"helpUri":"https://www.tfsec.dev/docs/aws/AWS001","properties":{"propertyName":"propertyValue"}}]}},"artifacts":[{"location":{"uri":"/tmp/result/code"}}],"results":[{"level":"error","message":{"text":"Resource 'my_bucket' has an ACL which allows public access."},"ruleId":"AWS001","ruleIndex":0,"locations":[{"physicalLocation":{"artifactLocation":{"uri":"/tmp/result/code","index":0},"region":{"startLine":1,"startColumn":1}}}]}]}` given.a_new_run_is_created() when.a_result_is_added_to_the_run().and(). diff --git a/test/sarif_stage_test.go b/test/sarif_stage_test.go index fb291e8..47b228c 100644 --- a/test/sarif_stage_test.go +++ b/test/sarif_stage_test.go @@ -3,10 +3,8 @@ package test import ( "bytes" "github.com/stretchr/testify/assert" - "testing" - "github.com/owenrumney/go-sarif/models" "github.com/owenrumney/go-sarif/sarif" ) @@ -24,7 +22,7 @@ func createNewSarifTest(t *testing.T) (*sarifTest, *sarifTest, *sarifTest) { } func (st *sarifTest) a_new_sarif_report(version string) { - report, err := sarif.New(sarif.SarifVersion(version)) + report, err := sarif.New(sarif.Version(version)) if err != nil { panic(err) } @@ -49,8 +47,6 @@ func (st *sarifTest) and() *sarifTest { } func (st *sarifTest) a_driver_is_added() *sarifTest { - tool := models.NewTool("ESLint", "https://eslint.org") - run := models.NewRun(tool) - st.sarifReport.AddRun(run) + st.sarifReport.AddRun("ESLint", "https://eslint.org") return st }