diff --git a/cmd/build_results.go b/cmd/build_results.go index 5ee4da3e..952cba6e 100644 --- a/cmd/build_results.go +++ b/cmd/build_results.go @@ -6,14 +6,16 @@ import ( "github.com/daveshanley/vacuum/rulesets" "github.com/pterm/pterm" "os" + "time" ) func BuildResults( rulesetFlag string, specBytes []byte, customFunctions map[string]model.RuleFunction, - base string) (*model.RuleResultSet, *motor.RuleSetExecutionResult, error) { - return BuildResultsWithDocCheckSkip(rulesetFlag, specBytes, customFunctions, base, false) + base string, + timeout time.Duration) (*model.RuleResultSet, *motor.RuleSetExecutionResult, error) { + return BuildResultsWithDocCheckSkip(rulesetFlag, specBytes, customFunctions, base, false, timeout) } func BuildResultsWithDocCheckSkip( @@ -21,7 +23,8 @@ func BuildResultsWithDocCheckSkip( specBytes []byte, customFunctions map[string]model.RuleFunction, base string, - skipCheck bool) (*model.RuleResultSet, *motor.RuleSetExecutionResult, error) { + skipCheck bool, + timeout time.Duration) (*model.RuleResultSet, *motor.RuleSetExecutionResult, error) { // read spec and parse defaultRuleSets := rulesets.BuildDefaultRuleSets() @@ -52,6 +55,7 @@ func BuildResultsWithDocCheckSkip( Base: base, SkipDocumentCheck: skipCheck, AllowLookup: true, + Timeout: timeout, }) resultSet := model.NewRuleResultSet(ruleset.Results) diff --git a/cmd/build_results_test.go b/cmd/build_results_test.go index abb1caba..7fd2a827 100644 --- a/cmd/build_results_test.go +++ b/cmd/build_results_test.go @@ -6,11 +6,11 @@ import ( ) func TestBuildResults(t *testing.T) { - _, _, err := BuildResults("nuggets", nil, nil, "") + _, _, err := BuildResults("nuggets", nil, nil, "", 5) assert.Error(t, err) } func TestBuildResults_SkipCheck(t *testing.T) { - _, _, err := BuildResultsWithDocCheckSkip("nuggets", nil, nil, "", true) + _, _, err := BuildResultsWithDocCheckSkip("nuggets", nil, nil, "", true, 5) assert.Error(t, err) } diff --git a/cmd/dashboard.go b/cmd/dashboard.go index 01189663..4d3c960b 100644 --- a/cmd/dashboard.go +++ b/cmd/dashboard.go @@ -42,6 +42,7 @@ func GetDashboardCommand() *cobra.Command { } baseFlag, _ := cmd.Flags().GetString("base") skipCheckFlag, _ := cmd.Flags().GetBool("skip-check") + timeoutFlag, _ := cmd.Flags().GetInt("timeout") var err error vacuumReport, specBytes, _ := vacuum_report.BuildVacuumReportFromFile(args[0]) @@ -62,7 +63,8 @@ func GetDashboardCommand() *cobra.Command { customFunctions, _ := LoadCustomFunctions(functionsFlag) rulesetFlag, _ := cmd.Flags().GetString("ruleset") - resultSet, ruleset, err = BuildResultsWithDocCheckSkip(rulesetFlag, specBytes, customFunctions, baseFlag, skipCheckFlag) + resultSet, ruleset, err = BuildResultsWithDocCheckSkip(rulesetFlag, specBytes, customFunctions, + baseFlag, skipCheckFlag, time.Duration(timeoutFlag)*time.Second) if err != nil { pterm.Error.Printf("Failed to render dashboard: %v\n\n", err) return err diff --git a/cmd/html_report.go b/cmd/html_report.go index a4d09882..18dfc3c5 100644 --- a/cmd/html_report.go +++ b/cmd/html_report.go @@ -44,6 +44,7 @@ func GetHTMLReportCommand() *cobra.Command { noStyleFlag, _ := cmd.Flags().GetBool("no-style") baseFlag, _ := cmd.Flags().GetString("base") skipCheckFlag, _ := cmd.Flags().GetBool("skip-check") + timeoutFlag, _ := cmd.Flags().GetInt("timeout") // disable color and styling, for CI/CD use. // https://github.com/daveshanley/vacuum/issues/234 @@ -92,7 +93,8 @@ func GetHTMLReportCommand() *cobra.Command { customFunctions, _ := LoadCustomFunctions(functionsFlag) rulesetFlag, _ := cmd.Flags().GetString("ruleset") - resultSet, ruleset, err = BuildResultsWithDocCheckSkip(rulesetFlag, specBytes, customFunctions, baseFlag, skipCheckFlag) + resultSet, ruleset, err = BuildResultsWithDocCheckSkip(rulesetFlag, specBytes, customFunctions, + baseFlag, skipCheckFlag, time.Duration(timeoutFlag)*time.Second) if err != nil { pterm.Error.Printf("Failed to generate report: %v\n\n", err) return err diff --git a/cmd/lint.go b/cmd/lint.go index c3e0a339..09a177d0 100644 --- a/cmd/lint.go +++ b/cmd/lint.go @@ -53,6 +53,7 @@ func GetLintCommand() *cobra.Command { noBanner, _ := cmd.Flags().GetBool("no-banner") noMessage, _ := cmd.Flags().GetBool("no-message") allResults, _ := cmd.Flags().GetBool("all-results") + timeoutFlag, _ := cmd.Flags().GetInt("timeout") // disable color and styling, for CI/CD use. // https://github.com/daveshanley/vacuum/issues/234 @@ -177,6 +178,7 @@ func GetLintCommand() *cobra.Command { functions: customFunctions, lock: &printLock, logger: logger, + timeoutFlag: timeoutFlag, } fs, fp, err := lintFile(lfr) @@ -266,6 +268,7 @@ type lintFileRequest struct { errorsFlag bool totalFiles int fileIndex int + timeoutFlag int defaultRuleSets rulesets.RuleSets selectedRS *rulesets.RuleSet functions map[string]model.RuleFunction @@ -297,6 +300,7 @@ func lintFile(req lintFileRequest) (int64, int, error) { AllowLookup: req.remote, SkipDocumentCheck: req.skipCheckFlag, Logger: req.logger, + Timeout: time.Duration(req.timeoutFlag) * time.Second, }) results := result.Results diff --git a/cmd/root.go b/cmd/root.go index d0470684..9809b84f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -59,6 +59,7 @@ func GetRootCommand() *cobra.Command { rootCmd.PersistentFlags().BoolP("remote", "u", true, "Allow local files and remote (http) references to be looked up") rootCmd.PersistentFlags().BoolP("skip-check", "k", false, "Skip checking for a valid OpenAPI document, useful for linting fragments or non-OpenAPI documents") rootCmd.PersistentFlags().BoolP("debug", "w", false, "Turn on debug logging") + rootCmd.PersistentFlags().IntP("timeout", "g", 5, "Rule timeout in seconds, default is 5 seconds") regErr := rootCmd.RegisterFlagCompletionFunc("functions", cobra.FixedCompletions( []string{"so"}, cobra.ShellCompDirectiveFilterFileExt, diff --git a/cmd/spectral_report.go b/cmd/spectral_report.go index 67060580..2809ea8d 100644 --- a/cmd/spectral_report.go +++ b/cmd/spectral_report.go @@ -44,6 +44,7 @@ func GetSpectralReportCommand() *cobra.Command { noStyleFlag, _ := cmd.Flags().GetBool("no-style") baseFlag, _ := cmd.Flags().GetString("base") skipCheckFlag, _ := cmd.Flags().GetBool("skip-check") + timeoutFlag, _ := cmd.Flags().GetInt("timeout") // disable color and styling, for CI/CD use. // https://github.com/daveshanley/vacuum/issues/234 @@ -136,6 +137,7 @@ func GetSpectralReportCommand() *cobra.Command { SilenceLogs: true, Base: baseFlag, SkipDocumentCheck: skipCheckFlag, + Timeout: time.Duration(timeoutFlag) * time.Second, }) resultSet := model.NewRuleResultSet(ruleset.Results) diff --git a/cmd/vacuum_report.go b/cmd/vacuum_report.go index 41442e3c..c2de7b3a 100644 --- a/cmd/vacuum_report.go +++ b/cmd/vacuum_report.go @@ -44,6 +44,7 @@ func GetVacuumReportCommand() *cobra.Command { baseFlag, _ := cmd.Flags().GetString("base") junitFlag, _ := cmd.Flags().GetBool("junit") skipCheckFlag, _ := cmd.Flags().GetBool("skip-check") + timeoutFlag, _ := cmd.Flags().GetInt("timeout") // disable color and styling, for CI/CD use. // https://github.com/daveshanley/vacuum/issues/234 @@ -138,6 +139,7 @@ func GetVacuumReportCommand() *cobra.Command { SilenceLogs: true, Base: baseFlag, SkipDocumentCheck: skipCheckFlag, + Timeout: time.Duration(timeoutFlag) * time.Second, }) resultSet := model.NewRuleResultSet(ruleset.Results) diff --git a/functions/openapi/oas_schema.go b/functions/openapi/oas_schema.go index bfb88b05..9355d470 100644 --- a/functions/openapi/oas_schema.go +++ b/functions/openapi/oas_schema.go @@ -38,9 +38,6 @@ func (os OASSchema) RunRule(nodes []*yaml.Node, context model.RuleFunctionContex // grab the original bytes and the spec info from context. info := context.SpecInfo - // rule cannot proceed until JSON parsing is complete. Wait on channel to signal all clear. - <-info.GetJSONParsingChannel() - if info.SpecType == "" { // spec type is un-known, there is no point in running this rule. return results diff --git a/go.mod b/go.mod index d8022757..1ce715f1 100644 --- a/go.mod +++ b/go.mod @@ -11,8 +11,8 @@ require ( github.com/gizak/termui/v3 v3.1.0 github.com/json-iterator/go v1.1.12 github.com/mitchellh/mapstructure v1.5.0 - github.com/pb33f/libopenapi v0.14.2 - github.com/pb33f/libopenapi-validator v0.0.36 + github.com/pb33f/libopenapi v0.14.4 + github.com/pb33f/libopenapi-validator v0.0.37 github.com/pterm/pterm v0.12.72 github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 github.com/spf13/cobra v1.8.0 diff --git a/go.sum b/go.sum index 0baa945d..000ba4bd 100644 --- a/go.sum +++ b/go.sum @@ -150,10 +150,10 @@ github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1y github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= github.com/onsi/gomega v1.19.0 h1:4ieX6qQjPP/BfC3mpsAtIGGlxTWPeA3Inl/7DtXw1tw= github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= -github.com/pb33f/libopenapi v0.14.2 h1:FaEvmhCObhjE9olnvlYZSwgrkFuis5eviIpIqndTA1c= -github.com/pb33f/libopenapi v0.14.2/go.mod h1:m+4Pwri31UvcnZjuP8M7TlbR906DXJmMvYsbis234xg= -github.com/pb33f/libopenapi-validator v0.0.36 h1:mNCPMkxvdYljwUpLlFkSKzXXyxslNquh5JRWqrFCfEo= -github.com/pb33f/libopenapi-validator v0.0.36/go.mod h1:YaNEPkOg49iBOoj6WOyK68JmyomqTREIh0ZSgM4lqHk= +github.com/pb33f/libopenapi v0.14.4 h1:NbcYaBbG/6pnJM8lw4F6b5e54HandyKF452HUl4+9j4= +github.com/pb33f/libopenapi v0.14.4/go.mod h1:m+4Pwri31UvcnZjuP8M7TlbR906DXJmMvYsbis234xg= +github.com/pb33f/libopenapi-validator v0.0.37 h1:nD6c010yxaFs3hJC+I5esAXf0VE6QLV1iTGkpCXz7W4= +github.com/pb33f/libopenapi-validator v0.0.37/go.mod h1:nWO4jDe3dJwRskfEl2SAtV9LlCJ5ClCB+OBw2E7splo= github.com/pelletier/go-toml/v2 v2.1.0 h1:FnwAJ4oYMvbT/34k9zzHuZNrhlz48GB3/s6at6/MHO4= github.com/pelletier/go-toml/v2 v2.1.0/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= diff --git a/motor/rule_applicator.go b/motor/rule_applicator.go index db47fa8f..fccf5faa 100644 --- a/motor/rule_applicator.go +++ b/motor/rule_applicator.go @@ -4,6 +4,7 @@ package motor import ( + "context" "errors" "fmt" "io" @@ -12,6 +13,7 @@ import ( "os" "path/filepath" "sync" + "time" "github.com/daveshanley/vacuum/functions" "github.com/daveshanley/vacuum/model" @@ -26,20 +28,20 @@ import ( ) type ruleContext struct { - rule *model.Rule - specNode *yaml.Node - builtinFunctions functions.Functions - ruleResults *[]model.RuleFunctionResult - wg *sync.WaitGroup - errors *[]error - index *index.SpecIndex - specInfo *datamodel.SpecInfo - customFunctions map[string]model.RuleFunction - panicFunc func(p any) - silenceLogs bool - document libopenapi.Document - skipDocumentCheck bool - logger *slog.Logger + rule *model.Rule + specNode *yaml.Node + specNodeUnresolved *yaml.Node + builtinFunctions functions.Functions + ruleResults *[]model.RuleFunctionResult + errors *[]error + index *index.SpecIndex + specInfo *datamodel.SpecInfo + customFunctions map[string]model.RuleFunction + panicFunc func(p any) + silenceLogs bool + document libopenapi.Document + skipDocumentCheck bool + logger *slog.Logger } // RuleSetExecution is an instruction set for executing a ruleset. It's a convenience structure to allow the signature @@ -57,6 +59,7 @@ type RuleSetExecution struct { Document libopenapi.Document // a ready to render model. SkipDocumentCheck bool // Skip the document check, useful for fragments and non openapi specs. Logger *slog.Logger // A custom logger. + Timeout time.Duration // The timeout for each rule to run, prevents run-away rules, default is five seconds. } // RuleSetExecutionResult returns the results of running the ruleset against the supplied spec. @@ -426,39 +429,67 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult { var errs []error if execution.RuleSet != nil { + + totalRules := len(execution.RuleSet.Rules) + done := make(chan bool) for _, rule := range execution.RuleSet.Rules { - ruleSpec := specResolved - ruleIndex := indexResolved - info := specInfo - if !rule.Resolved { - info = specInfoUnresolved - ruleSpec = specUnresolved - ruleIndex = indexUnresolved - } - // this list of things is most likely going to grow a bit, so we use a nice clean message design. - ctx := ruleContext{ - rule: rule, - specNode: ruleSpec, - builtinFunctions: builtinFunctions, - ruleResults: &ruleResults, - wg: &ruleWaitGroup, - errors: &errs, - specInfo: info, - index: ruleIndex, - document: docResolved, - customFunctions: execution.CustomFunctions, - silenceLogs: execution.SilenceLogs, - skipDocumentCheck: execution.SkipDocumentCheck, - logger: docConfig.Logger, - } - if execution.PanicFunction != nil { - ctx.panicFunc = execution.PanicFunction - } - go runRule(ctx) + go func(rule *model.Rule, done chan bool) { + + ruleSpec := specResolved + ruleIndex := indexResolved + info := specInfo + if !rule.Resolved { + info = specInfoUnresolved + ruleSpec = specUnresolved + ruleIndex = indexUnresolved + } + + // this list of things is most likely going to grow a bit, so we use a nice clean message design. + ctx := ruleContext{ + rule: rule, + specNode: ruleSpec, + specNodeUnresolved: specUnresolved, + builtinFunctions: builtinFunctions, + ruleResults: &ruleResults, + errors: &errs, + specInfo: info, + index: ruleIndex, + document: docResolved, + customFunctions: execution.CustomFunctions, + silenceLogs: execution.SilenceLogs, + skipDocumentCheck: execution.SkipDocumentCheck, + logger: docConfig.Logger, + } + if execution.PanicFunction != nil { + ctx.panicFunc = execution.PanicFunction + } + + if execution.Timeout <= 0 { + execution.Timeout = time.Second * 5 // default + } + + timeoutCtx, ruleCancel := context.WithTimeout(context.Background(), execution.Timeout) + defer ruleCancel() + doneChan := make(chan bool) + go runRule(ctx, doneChan) + + select { + case <-timeoutCtx.Done(): + ctx.logger.Error("Rule timed out, skipping", "rule", rule.Id, "timeout", execution.Timeout) + break + case <-doneChan: + break + } + done <- true + }(rule, done) } - ruleWaitGroup.Wait() + completed := 0 + for completed < totalRules { + <-done + completed++ + } } ruleResults = *removeDuplicates(&ruleResults, execution, indexResolved) @@ -474,7 +505,7 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult { } } -func runRule(ctx ruleContext) { +func runRule(ctx ruleContext, doneChan chan bool) { if ctx.panicFunc != nil { defer func() { @@ -483,7 +514,6 @@ func runRule(ctx ruleContext) { } }() } - defer ctx.wg.Done() var givenPaths []string if x, ok := ctx.rule.Given.(string); ok { @@ -496,19 +526,62 @@ func runRule(ctx ruleContext) { if x, ok := ctx.rule.Given.([]interface{}); ok { for _, gpI := range x { - if gp, ok := gpI.(string); ok { + if gp, ko := gpI.(string); ko { givenPaths = append(givenPaths, gp) } } } - for _, givenPath := range givenPaths { + findNodes := func(node *yaml.Node, path string, errChan chan error, nodesChan chan []*yaml.Node) { + nodes, err := utils.FindNodesWithoutDeserializing(node, path) + if err != nil { + errChan <- err + } + nodesChan <- nodes + } - var nodes []*yaml.Node - var err error + var nodes []*yaml.Node + var err error + + for _, givenPath := range givenPaths { if givenPath != "$" { - nodes, err = utils.FindNodesWithoutDeserializing(ctx.specNode, givenPath) + + // create a timeout on this, if we can't get a result within 100ms, then + // try again, but with the unresolved spec. + lookupCtx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500) + defer cancel() + nodesChan := make(chan []*yaml.Node) + errChan := make(chan error) + + go findNodes(ctx.specNode, givenPath, errChan, nodesChan) + + select { + case nodes = <-nodesChan: + break + case err = <-errChan: + break + case <-lookupCtx.Done(): + ctx.logger.Warn("timeout looking for nodes, trying again with unresolved spec.", "path", givenPath) + + // ok, this timed out, let's try again with the unresolved spec. + lookupCtxFinal, finalCancel := context.WithTimeout(context.Background(), time.Millisecond*100) + defer finalCancel() + + go findNodes(ctx.specNodeUnresolved, givenPath, errChan, nodesChan) + + select { + case nodes = <-nodesChan: + break + case err = <-errChan: + break + case <-lookupCtxFinal.Done(): + err = fmt.Errorf("timed out looking for nodes using path '%s'", givenPath) + ctx.logger.Error("timeout looking for unresolved nodes, giving up.", "path", givenPath, "rule", + ctx.rule.Id) + } + } + } else { // if we're looking for the root, don't bother looking, we already have it. nodes = []*yaml.Node{ctx.specNode} @@ -543,6 +616,7 @@ func runRule(ctx ruleContext) { } } } + doneChan <- true } var lock sync.Mutex