diff --git a/cmd/check_test.go b/cmd/check_test.go index 0ecdc9a9..3a6b868e 100644 --- a/cmd/check_test.go +++ b/cmd/check_test.go @@ -16,8 +16,6 @@ import ( var checkFileContents = `--- meta: author: "go-ftw" - enabled: true - name: "mock-TestRunTests_Run.yaml" description: "Test file for go-ftw" tests: - # Standard GET request diff --git a/cmd/run_test.go b/cmd/run_test.go index cae1dcd6..2e22ffa3 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -23,8 +23,6 @@ import ( var testFileContentsTemplate = `--- meta: author: "go-ftw" - enabled: true - name: "mock-TestRunTests_Run.yaml" description: "Test file for go-ftw" tests: - # Standard GET request diff --git a/go.sum b/go.sum index abb508f2..f1545f15 100644 --- a/go.sum +++ b/go.sum @@ -84,8 +84,6 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= -github.com/rs/zerolog v1.32.0 h1:keLypqrlIjaFsbmJOBdB/qvyF8KEtCWHwobLp5l/mQ0= -github.com/rs/zerolog v1.32.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8= github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= diff --git a/runner/run.go b/runner/run.go index af443741..d18f23a1 100644 --- a/runner/run.go +++ b/runner/run.go @@ -86,7 +86,6 @@ func RunTest(runContext *TestRunContext, ftwTest *test.FTWTest) error { // if we received a particular test ID, skip until we find it if needToSkipTest(runContext.Include, runContext.Exclude, &testCase) { runContext.Stats.addResultToStats(Skipped, &testCase) - log.Trace().Msgf("\tskipping %s", testCase.IdString()) continue } test.ApplyPlatformOverrides(runContext.Config, &testCase) diff --git a/runner/run_test.go b/runner/run_test.go index 555940ea..8870844e 100644 --- a/runner/run_test.go +++ b/runner/run_test.go @@ -34,9 +34,6 @@ var testConfigMap = map[string]string{ testoverride: ignore: "920400-1": "This test result must be ignored" -`, - "TestDisabledRun": `--- -mode: 'cloud' `, "TestBrokenOverrideRun": `--- testoverride: @@ -96,7 +93,6 @@ testoverride: var destinationMap = map[string]string{ "TestBrokenOverrideRun": "http://example.com:1234", - "TestDisabledRun": "http://example.com:1234", } type runTestSuite struct { @@ -352,12 +348,6 @@ func (s *runTestSuite) TestBrokenPortOverrideRun() { s.LessOrEqual(0, res.Stats.TotalFailed(), "Oops, test run failed!") } -func (s *runTestSuite) TestDisabledRun() { - res, err := Run(s.cfg, s.ftwTests, RunnerConfig{}, s.out) - s.Require().NoError(err) - s.LessOrEqual(0, res.Stats.TotalFailed(), "Oops, test run failed!") -} - func (s *runTestSuite) TestLogsRun() { res, err := Run(s.cfg, s.ftwTests, RunnerConfig{}, s.out) s.Require().NoError(err) diff --git a/runner/testdata/TestBrokenOverrideRun.yaml b/runner/testdata/TestBrokenOverrideRun.yaml index e7da3d7a..a4699a6d 100644 --- a/runner/testdata/TestBrokenOverrideRun.yaml +++ b/runner/testdata/TestBrokenOverrideRun.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestBrokenOverrideRun.yaml" description: "Example Override Test" tests: - test_id: 1 diff --git a/runner/testdata/TestBrokenPortOverrideRun.yaml b/runner/testdata/TestBrokenPortOverrideRun.yaml index 484a8f1c..a4699a6d 100644 --- a/runner/testdata/TestBrokenPortOverrideRun.yaml +++ b/runner/testdata/TestBrokenPortOverrideRun.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestBrokenPortOverrideRun.yaml" description: "Example Override Test" tests: - test_id: 1 diff --git a/runner/testdata/TestCloudRun.yaml b/runner/testdata/TestCloudRun.yaml index 1a6c3fed..ef53d3d7 100644 --- a/runner/testdata/TestCloudRun.yaml +++ b/runner/testdata/TestCloudRun.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestCloudRun.yaml" description: "Example Test" tests: - test_id: 1 diff --git a/runner/testdata/TestDisabledRun.yaml b/runner/testdata/TestDisabledRun.yaml deleted file mode 100644 index bf1a03d2..00000000 --- a/runner/testdata/TestDisabledRun.yaml +++ /dev/null @@ -1,18 +0,0 @@ ---- -meta: - author: "tester" - enabled: false - name: "TestDisabledRun.yaml" - description: "we do not care, this test is disabled" -tests: - - test_id: 1 - description: "access real external site" - stages: - - input: - dest_addr: "{{ .TestAddr }}" - port: {{ .TestPort }} - headers: - User-Agent: "ModSecurity CRS 3 Tests" - Host: "{{ .TestAddr }}" - output: - status: 1234 diff --git a/runner/testdata/TestFailedTestsRun.yaml b/runner/testdata/TestFailedTestsRun.yaml index d1f44f44..1c33145a 100644 --- a/runner/testdata/TestFailedTestsRun.yaml +++ b/runner/testdata/TestFailedTestsRun.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestFailedTestsRun.yaml" description: "Example Test" tests: - test_id: 990 diff --git a/runner/testdata/TestIgnoredTestsRun.yaml b/runner/testdata/TestIgnoredTestsRun.yaml index f8248275..f182d47a 100644 --- a/runner/testdata/TestIgnoredTestsRun.yaml +++ b/runner/testdata/TestIgnoredTestsRun.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestIgnoredTestsRun.yaml" description: "Example Test" tests: - test_id: 1 diff --git a/runner/testdata/TestLogsRun.yaml b/runner/testdata/TestLogsRun.yaml index 94e077d5..00ceddd0 100644 --- a/runner/testdata/TestLogsRun.yaml +++ b/runner/testdata/TestLogsRun.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestLogsRun.yaml" description: "Example Test" tests: - test_id: 200 diff --git a/runner/testdata/TestOverrideRun.yaml b/runner/testdata/TestOverrideRun.yaml index 88ecf01a..d8fc3211 100644 --- a/runner/testdata/TestOverrideRun.yaml +++ b/runner/testdata/TestOverrideRun.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestOverrideRun.yaml" description: "Example Override Test" tests: - test_id: 1 diff --git a/runner/testdata/TestRetryOnce.yaml b/runner/testdata/TestRetryOnce.yaml index 0c69d532..74faa867 100644 --- a/runner/testdata/TestRetryOnce.yaml +++ b/runner/testdata/TestRetryOnce.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestRetryOnce.yaml" description: "Example Test" tests: - test_id: 1 diff --git a/runner/testdata/TestRunMultipleMatches.yaml b/runner/testdata/TestRunMultipleMatches.yaml index 710750f2..da93472d 100644 --- a/runner/testdata/TestRunMultipleMatches.yaml +++ b/runner/testdata/TestRunMultipleMatches.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestRunMultipleMatches.yaml" description: "Example Test with multiple expected outputs per single rule" tests: - test_id: 1 diff --git a/runner/testdata/TestRunTests_Run.yaml b/runner/testdata/TestRunTests_Run.yaml index 8582848a..f182d47a 100644 --- a/runner/testdata/TestRunTests_Run.yaml +++ b/runner/testdata/TestRunTests_Run.yaml @@ -1,8 +1,6 @@ --- meta: author: "tester" - enabled: true - name: "TestRunTests_Run.yaml" description: "Example Test" tests: - test_id: 1 diff --git a/test/files.go b/test/files.go index 26c78d03..57cf3fd5 100644 --- a/test/files.go +++ b/test/files.go @@ -30,16 +30,18 @@ func GetTestsFromFiles(globPattern string) ([]*FTWTest, error) { } for _, filePath := range testFiles { + fileName := path.Base(filePath) + log.Trace().Msgf("Loading %s", fileName) yamlString, err := readFileContents(filePath) if err != nil { return tests, err } - ftwTest, err := GetTestFromYaml(yamlString, path.Base(filePath)) + ftwTest, err := GetTestFromYaml(yamlString, fileName) if err != nil { - log.Error().Msgf("Problem detected in file %s:\n%s\n%s", + log.Warn().Msgf("Problem detected in file %s:\n%s\n%s", filePath, yaml.FormatError(err, true, true), DescribeYamlError(err)) - return tests, err + continue } tests = append(tests, ftwTest) diff --git a/test/files_test.go b/test/files_test.go index e0097561..e414da70 100644 --- a/test/files_test.go +++ b/test/files_test.go @@ -14,36 +14,33 @@ import ( var yamlTest = ` --- - meta: - author: "tester" - enabled: true - name: "911100.yaml" - description: "Description" - rule_id: 911100 - tests: - - test_id: 1 - stages: - - input: - autocomplete_headers: false - dest_addr: "127.0.0.1" - port: 80 - headers: - User-Agent: "ModSecurity CRS 3 Tests" - Host: "localhost" - output: - no_log_contains: "id \"911100\"" - - - test_id: 2 - stages: - - input: - dest_addr: "127.0.0.1" - port: 80 - method: "OPTIONS" - headers: - User-Agent: "ModSecurity CRS 3 Tests" - Host: "localhost" - output: - no_log_contains: "id \"911100\"" +meta: + author: "tester" + description: "Description" +rule_id: 911100 +tests: + - test_id: 1 + stages: + - input: + autocomplete_headers: false + dest_addr: "127.0.0.1" + port: 80 + headers: + User-Agent: "ModSecurity CRS 3 Tests" + Host: "localhost" + output: + no_log_contains: "id \"911100\"" + - test_id: 2 + stages: + - input: + dest_addr: "127.0.0.1" + port: 80 + method: "OPTIONS" + headers: + User-Agent: "ModSecurity CRS 3 Tests" + Host: "localhost" + output: + no_log_contains: "id \"911100\"" ` var wrongYamlTest = ` @@ -64,7 +61,7 @@ func (s *filesTestSuite) TestGetTestFromYAML() { for _, ft := range tests { s.Equal("tester", ft.Meta.Author) - s.Equal("911100.yaml", ft.Meta.Name) + s.Equal("Description", ft.Meta.Description) re := regexp.MustCompile("911100.*") diff --git a/test/testdata/TestCheckBenchmarkCheckFiles.yaml b/test/testdata/TestCheckBenchmarkCheckFiles.yaml index 18636ed5..392761d6 100644 --- a/test/testdata/TestCheckBenchmarkCheckFiles.yaml +++ b/test/testdata/TestCheckBenchmarkCheckFiles.yaml @@ -1,8 +1,6 @@ --- meta: author: "csanders-git, Franziska Bühler" - enabled: true - name: "920420.yaml" description: "Description" rule_id: 920420 tests: diff --git a/test/types.go b/test/types.go index f8d3d263..59ddd5d2 100644 --- a/test/types.go +++ b/test/types.go @@ -4,6 +4,7 @@ package test import ( + "fmt" "regexp" "strconv" @@ -125,34 +126,36 @@ func applyHeadersOverride(overrides *config.Overrides, input *Input) { } } -func postLoadTestFTWTest(ftwTest *FTWTest, fileName string) { +func postLoadTestFTWTest(ftwTest *FTWTest, fileName string) error { ftwTest.FileName = fileName - postLoadRuleId(ftwTest) + if err := postLoadRuleId(ftwTest); err != nil { + return err + } for index := 0; index < len(ftwTest.Tests); index++ { postLoadTest(ftwTest.RuleId, uint(index+1), &ftwTest.Tests[index]) } + return nil } -func postLoadRuleId(ftwTest *FTWTest) { +func postLoadRuleId(ftwTest *FTWTest) error { if ftwTest.RuleId > 0 { - return + return nil } if len(ftwTest.FileName) == 0 { - log.Fatal().Msg("The rule_id field is required for the top-level test structure") + return fmt.Errorf("the rule_id field is required for the top-level test structure") } else { ruleIdString := regexp.MustCompile(`\d+`).FindString(ftwTest.FileName) if len(ruleIdString) == 0 { - log.Fatal().Msg("Failed to fall back on filename to find rule ID of test. The rule_id field is required for the top-level test structure") - return + return fmt.Errorf("failed to fall back on filename to find rule ID of test. The rule_id field is required for the top-level test structure") } ruleId, err := strconv.ParseUint(ruleIdString, 10, 0) if err != nil { - log.Fatal().Msgf("failed to parse rule ID from filename '%s'", ftwTest.FileName) - return + return fmt.Errorf("failed to parse rule ID from filename '%s'", ftwTest.FileName) } ftwTest.RuleId = uint(ruleId) } + return nil } func postLoadTest(ruleId uint, testId uint, test *schema.Test) { test.RuleId = ruleId diff --git a/test/types_test.go b/test/types_test.go index 9ef013f8..82fbdda6 100644 --- a/test/types_test.go +++ b/test/types_test.go @@ -20,8 +20,6 @@ func TestTypesTestSuite(t *testing.T) { var autocompleteHeadersDefaultYaml = `--- meta: author: "tester" - enabled: true - name: "gotest-ftw.yaml" description: "Example Test" rule_id: 123456 tests: @@ -68,8 +66,6 @@ tests: var autocompleteHeadersFalseYaml = `--- meta: author: "tester" - enabled: true - name: "gotest-ftw.yaml" description: "Example Test" rule_id: 123456 tests: @@ -119,8 +115,6 @@ tests: var autocompleteHeadersTrueYaml = `--- meta: author: "tester" - enabled: true - name: "gotest-ftw.yaml" description: "Example Test" rule_id: 123456 tests: diff --git a/test/yaml.go b/test/yaml.go index b8fd890f..65c53939 100644 --- a/test/yaml.go +++ b/test/yaml.go @@ -9,49 +9,74 @@ import ( "github.com/goccy/go-yaml" ) +type errorMap struct { + matchers []string + explanation string +} + // GetTestFromYaml will get the tests to be processed from a YAML string. func GetTestFromYaml(testYaml []byte, fileName string) (ftwTest *FTWTest, err error) { ftwTest = &FTWTest{} err = yaml.Unmarshal(testYaml, ftwTest) if err != nil { - return &FTWTest{}, err + return nil, err } - postLoadTestFTWTest(ftwTest, fileName) + if err := postLoadTestFTWTest(ftwTest, fileName); err != nil { + return nil, err + } return ftwTest, nil } func DescribeYamlError(yamlError error) string { - matched, err := regexp.MatchString(`.*int was used where sequence is expected.*`, yamlError.Error()) - if err != nil { - return err.Error() - } - if matched { - return "\nTip: This might refer to a \"status\" line being '200', where it should be '[200]'.\n" + - "The default \"status\" is a list now.\n" + - "A simple example would be like this:\n\n" + - "status: 403\n" + - "needs to be changed to:\n\n" + - "status: 403\n\n" - } - matched, err = regexp.MatchString(`.*cannot unmarshal \[]interface {} into Go struct field FTWTest.Tests of type string.*`, yamlError.Error()) - if err != nil { - return err.Error() + errorMaps := []errorMap{ + { + matchers: []string{`.*int was used where sequence is expected.*`}, + explanation: "Tip: This might refer to a \"status\" line being '200', where it should be '[200]'.\n" + + "The default \"status\" is a list now.\n" + + "A simple example would be like this:\n\n" + + "status: 403\n" + + "needs to be changed to:\n\n" + + "status: 403", + }, + { + matchers: []string{`.*cannot unmarshal \[]interface {} into Go struct field FTWTest.Tests of type string.*`}, + explanation: "Tip: This might refer to \"data\" on the test being a list of strings instead of a proper YAML multiline.\n" + + "To fix this, convert this \"data\" string list to a multiline YAML and this will be fixed.\n" + + "A simple example would be like this:\n\n" + + "data:\n" + + " - 'Hello'\n" + + " - 'World'\n" + + "can be expressed as:\n\n" + + "data: |\n" + + " Hello\n" + + " World\n\n" + + "You can also remove single/double quotes from beggining and end of text, they are not needed. See https://yaml-multiline.info/ for additional help.", + }, + { + matchers: []string{ + "The rule_id field is required for the top-level test structure", + "Failed to fall back on filename to find rule ID of test. The rule_id field is required for the top-level test structure", + "failed to parse rule ID from filename ", + }, + explanation: "The `rule_id` field is missing from this file and the rule ID could not be determined otherwise.\n" + + "This might be a YAML file that is not a test.", + }, } - if matched { - return "\nTip: This might refer to \"data\" on the test being a list of strings instead of a proper YAML multiline.\n" + - "To fix this, convert this \"data\" string list to a multiline YAML and this will be fixed.\n" + - "A simple example would be like this:\n\n" + - "data:\n" + - " - 'Hello'\n" + - " - 'World'\n" + - "can be expressed as:\n\n" + - "data: |\n" + - " Hello\n" + - " World\n\n" + - "You can also remove single/double quotes from beggining and end of text, they are not needed. See https://yaml-multiline.info/ for additional help.\n" + errorMessage := yamlError.Error() + for _, em := range errorMaps { + for _, regex := range em.matchers { + matched, err := regexp.MatchString(regex, errorMessage) + if err != nil { + return err.Error() + } + if !matched { + continue + } + return "\n" + em.explanation + "\n\n" + } } - return "We do not have an extended explanation of this error." + return "\nWe do not have an extended explanation of this error\n\n" }