From 4f01483bc2c8f8fd52c0ee70113689118b4a21b2 Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Fri, 16 Aug 2024 12:51:38 -0400 Subject: [PATCH 1/3] - Renamed RuleSetExecution.SpecFileNam to SpecFilePath because it's actually set to the path, not just the filename - Include references from the root as well as the rolodex when checking unused components --- cmd/lint.go | 2 +- functions/openapi/unused_component.go | 16 ++- functions/openapi/unused_component_test.go | 151 ++++++--------------- motor/rule_applicator.go | 9 +- 4 files changed, 64 insertions(+), 114 deletions(-) diff --git a/cmd/lint.go b/cmd/lint.go index bf4c0594..69984cad 100644 --- a/cmd/lint.go +++ b/cmd/lint.go @@ -335,7 +335,7 @@ func lintFile(req utils.LintFileRequest) (int64, int, error) { result := motor.ApplyRulesToRuleSet(&motor.RuleSetExecution{ RuleSet: req.SelectedRS, Spec: specBytes, - SpecFileName: req.FileName, + SpecFilePath: req.FileName, CustomFunctions: req.Functions, Base: req.BaseFlag, AllowLookup: req.Remote, diff --git a/functions/openapi/unused_component.go b/functions/openapi/unused_component.go index e7008631..c8dab9b9 100644 --- a/functions/openapi/unused_component.go +++ b/functions/openapi/unused_component.go @@ -10,6 +10,7 @@ import ( "github.com/pb33f/libopenapi/index" "github.com/pb33f/libopenapi/utils" "gopkg.in/yaml.v3" + "maps" "strings" ) @@ -38,8 +39,12 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction var results []model.RuleFunctionResult - // extract all references, and every single component - allRefs := context.Index.GetAllReferences() + // extract all references, and every single component, recursively + allRefs := context.Document.GetRolodex().GetRootIndex().GetAllReferences() + for _, idx := range context.Document.GetRolodex().GetIndexes() { + refs := idx.GetAllReferences() + maps.Copy(allRefs, refs) + } schemas := context.Index.GetAllComponentSchemas() responses := context.Index.GetAllResponses() parameters := context.Index.GetAllParameters() @@ -49,7 +54,12 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction securitySchemes := context.Index.GetAllSecuritySchemes() links := context.Index.GetAllLinks() callbacks := context.Index.GetAllCallbacks() - mappedRefs := context.Index.GetMappedReferences() + + mappedRefs := context.Document.GetRolodex().GetRootIndex().GetMappedReferences() + for _, idx := range context.Document.GetRolodex().GetIndexes() { + refs := idx.GetMappedReferences() + maps.Copy(mappedRefs, refs) + } // extract securityRequirements from swagger. These are not mapped as they are not $refs // so, we need to map them as if they were. diff --git a/functions/openapi/unused_component_test.go b/functions/openapi/unused_component_test.go index 9df358f8..d308ce85 100644 --- a/functions/openapi/unused_component_test.go +++ b/functions/openapi/unused_component_test.go @@ -1,12 +1,15 @@ package openapi import ( + "fmt" "github.com/daveshanley/vacuum/model" + "github.com/pb33f/libopenapi" "github.com/pb33f/libopenapi/datamodel" "github.com/pb33f/libopenapi/index" "github.com/pb33f/libopenapi/utils" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" + "strings" "testing" ) @@ -23,7 +26,9 @@ func TestUnusedComponent_RunRule(t *testing.T) { func TestUnusedComponent_RunRule_Success(t *testing.T) { - yml := `paths: + yml := ` +openapi: 3.0.0 +paths: /naughty/{puppy}: parameters: - $ref: '#/components/parameters/Chewy' @@ -46,22 +51,7 @@ components: in: query name: chewy` - path := "$" - - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - - rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil) - ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) - - def := UnusedComponent{} - res := def.RunRule(nodes, ctx) - + res := setupUnusedComponentsTestContext(t, yml) assert.Len(t, res, 0) } @@ -85,24 +75,7 @@ paths: security: - sessionAuth: []` - path := "$" - - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - - rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil) - ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) - info, _ := datamodel.ExtractSpecInfo([]byte(yml)) - ctx.SpecInfo = info - ctx.Rule = &rule - - def := UnusedComponent{} - res := def.RunRule(nodes, ctx) + res := setupUnusedComponentsTestContext(t, yml) assert.Len(t, res, 0) } @@ -118,30 +91,15 @@ components: securitySchemes: SomeSecurity: description: A secure way to do things and stuff.` - path := "$" - - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - - rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil) - ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) - info, _ := datamodel.ExtractSpecInfo([]byte(yml)) - ctx.SpecInfo = info - - def := UnusedComponent{} - res := def.RunRule(nodes, ctx) - + res := setupUnusedComponentsTestContext(t, yml) assert.Len(t, res, 0) } func TestUnusedComponent_RunRule_Success_Fail_TwoMissing_Two_Undefined(t *testing.T) { - yml := `parameters: + yml := ` +openapi: 3.0.1 +parameters: Chewy: description: chewy in: query @@ -166,28 +124,15 @@ components: Kitty: $ref: '#/components/schemas/Puppy' ` - path := "$" - - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - - rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil) - ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) - - def := UnusedComponent{} - res := def.RunRule(nodes, ctx) - + res := setupUnusedComponentsTestContext(t, yml) assert.Len(t, res, 4) } func TestUnusedComponent_RunRule_Success_Fail_Four_Undefined(t *testing.T) { - yml := `paths: + yml := ` +openapi: 3.0.0 +paths: /naughty/{puppy}: parameters: - $ref: '#/components/parameters/Chewy' @@ -223,28 +168,15 @@ components: in: query name: chewy` - path := "$" - - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - - rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil) - ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) - - def := UnusedComponent{} - res := def.RunRule(nodes, ctx) - + res := setupUnusedComponentsTestContext(t, yml) assert.Len(t, res, 4) } func TestUnusedComponent_RunRule_Success_PolymorphicCheck(t *testing.T) { - yml := `paths: + yml := ` +openapi: 3.0.0 +paths: /naughty/{puppy}: get: responses: @@ -281,28 +213,15 @@ components: type: string description: bunny` - path := "$" - - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - - rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil) - ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) - - def := UnusedComponent{} - res := def.RunRule(nodes, ctx) - + res := setupUnusedComponentsTestContext(t, yml) assert.Len(t, res, 0) } func TestUnusedComponent_RunRule_Success_PolymorphicCheckAllOf(t *testing.T) { - yml := `paths: + yml := ` +openapi: 3.0.0 +paths: "/naughty/{puppy}": get: responses: @@ -340,21 +259,39 @@ components: category: type: String` + res := setupUnusedComponentsTestContext(t, yml) + assert.Len(t, res, 0) +} + +func setupUnusedComponentsTestContext(t *testing.T, yml string) []model.RuleFunctionResult { path := "$" var rootNode yaml.Node mErr := yaml.Unmarshal([]byte(yml), &rootNode) assert.NoError(t, mErr) + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + + if strings.Contains(yml, "swagger") { + _, _ = document.BuildV2Model() + } else { + _, _ = document.BuildV3Model() + } + nodes, _ := utils.FindNodes([]byte(yml), path) rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) config := index.CreateOpenAPIIndexConfig() ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + info, _ := datamodel.ExtractSpecInfo([]byte(yml)) + ctx.SpecInfo = info + ctx.Document = document def := UnusedComponent{} - res := def.RunRule(nodes, ctx) - assert.Len(t, res, 0) + return def.RunRule(nodes, ctx) } diff --git a/motor/rule_applicator.go b/motor/rule_applicator.go index 8ff2be1e..dd10ce51 100644 --- a/motor/rule_applicator.go +++ b/motor/rule_applicator.go @@ -54,7 +54,7 @@ type ruleContext struct { // of ApplyRulesToRuleSet to change, without a huge refactor. The ApplyRulesToRuleSet function only returns a single error also. type RuleSetExecution struct { RuleSet *rulesets.RuleSet // The RuleSet in which to apply - SpecFileName string // The name of the specification file, used to correctly label location + SpecFilePath string // The path of the specification file, used to correctly label location Spec []byte // The raw bytes of the OpenAPI specification. SpecInfo *datamodel.SpecInfo // Pre-parsed spec-info. CustomFunctions map[string]model.RuleFunction // custom functions loaded from plugin. @@ -109,13 +109,16 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult { // create new configurations indexConfig := index.CreateClosedAPIIndexConfig() + indexConfig.SpecFilePath = execution.SpecFilePath indexConfigUnresolved := index.CreateClosedAPIIndexConfig() + indexConfigUnresolved.SpecFilePath = execution.SpecFilePath // avoid building the index, we don't need it to run yet. indexConfig.AvoidBuildIndex = true //indexConfig.AvoidCircularReferenceCheck = true docConfig := datamodel.NewDocumentConfiguration() + docConfig.SpecFilePath = execution.SpecFilePath //docConfig.SkipCircularReferenceCheck = true if execution.IgnoreCircularArrayRef { @@ -883,7 +886,7 @@ func removeDuplicates(results *[]model.RuleFunctionResult, rse *RuleSetExecution origin := idx.FindNodeOrigin(result.StartNode) if origin != nil { if filepath.Base(origin.AbsoluteLocation) == "root.yaml" { - origin.AbsoluteLocation = rse.SpecFileName + origin.AbsoluteLocation = rse.SpecFilePath } result.Origin = origin } @@ -914,7 +917,7 @@ func removeDuplicates(results *[]model.RuleFunctionResult, rse *RuleSetExecution origin := idx.FindNodeOrigin(result.StartNode) if origin != nil { if filepath.Base(origin.AbsoluteLocation) == "root.yaml" { - origin.AbsoluteLocation = rse.SpecFileName + origin.AbsoluteLocation = rse.SpecFilePath } result.Origin = origin } From 440ff3853f6fa9975eb6b386cf602aec1597b604 Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Mon, 19 Aug 2024 11:46:00 -0400 Subject: [PATCH 2/3] Added GetAllReferences() and GetAllMappedReferences() methods to Rolodex --- functions/openapi/unused_component.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/functions/openapi/unused_component.go b/functions/openapi/unused_component.go index c8dab9b9..f816ed01 100644 --- a/functions/openapi/unused_component.go +++ b/functions/openapi/unused_component.go @@ -10,7 +10,6 @@ import ( "github.com/pb33f/libopenapi/index" "github.com/pb33f/libopenapi/utils" "gopkg.in/yaml.v3" - "maps" "strings" ) @@ -40,11 +39,7 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction var results []model.RuleFunctionResult // extract all references, and every single component, recursively - allRefs := context.Document.GetRolodex().GetRootIndex().GetAllReferences() - for _, idx := range context.Document.GetRolodex().GetIndexes() { - refs := idx.GetAllReferences() - maps.Copy(allRefs, refs) - } + allRefs := context.Document.GetRolodex().GetAllReferences() schemas := context.Index.GetAllComponentSchemas() responses := context.Index.GetAllResponses() parameters := context.Index.GetAllParameters() @@ -54,12 +49,7 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction securitySchemes := context.Index.GetAllSecuritySchemes() links := context.Index.GetAllLinks() callbacks := context.Index.GetAllCallbacks() - - mappedRefs := context.Document.GetRolodex().GetRootIndex().GetMappedReferences() - for _, idx := range context.Document.GetRolodex().GetIndexes() { - refs := idx.GetMappedReferences() - maps.Copy(mappedRefs, refs) - } + mappedRefs := context.Document.GetRolodex().GetAllMappedReferences() // extract securityRequirements from swagger. These are not mapped as they are not $refs // so, we need to map them as if they were. From 1ca4e4d208c72f2efff66a896c9f9f7ee58293c0 Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Fri, 11 Oct 2024 08:43:47 -0400 Subject: [PATCH 3/3] Undo renaming of public field --- cmd/lint.go | 2 +- motor/rule_applicator.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/lint.go b/cmd/lint.go index fcd06ca5..0edf7761 100644 --- a/cmd/lint.go +++ b/cmd/lint.go @@ -335,7 +335,7 @@ func lintFile(req utils.LintFileRequest) (int64, int, error) { result := motor.ApplyRulesToRuleSet(&motor.RuleSetExecution{ RuleSet: req.SelectedRS, Spec: specBytes, - SpecFilePath: req.FileName, + SpecFileName: req.FileName, CustomFunctions: req.Functions, Base: req.BaseFlag, AllowLookup: req.Remote, diff --git a/motor/rule_applicator.go b/motor/rule_applicator.go index dd10ce51..ca0af378 100644 --- a/motor/rule_applicator.go +++ b/motor/rule_applicator.go @@ -54,7 +54,7 @@ type ruleContext struct { // of ApplyRulesToRuleSet to change, without a huge refactor. The ApplyRulesToRuleSet function only returns a single error also. type RuleSetExecution struct { RuleSet *rulesets.RuleSet // The RuleSet in which to apply - SpecFilePath string // The path of the specification file, used to correctly label location + SpecFileName string // The path of the specification file, used to correctly label location Spec []byte // The raw bytes of the OpenAPI specification. SpecInfo *datamodel.SpecInfo // Pre-parsed spec-info. CustomFunctions map[string]model.RuleFunction // custom functions loaded from plugin. @@ -109,16 +109,16 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult { // create new configurations indexConfig := index.CreateClosedAPIIndexConfig() - indexConfig.SpecFilePath = execution.SpecFilePath + indexConfig.SpecFilePath = execution.SpecFileName indexConfigUnresolved := index.CreateClosedAPIIndexConfig() - indexConfigUnresolved.SpecFilePath = execution.SpecFilePath + indexConfigUnresolved.SpecFilePath = execution.SpecFileName // avoid building the index, we don't need it to run yet. indexConfig.AvoidBuildIndex = true //indexConfig.AvoidCircularReferenceCheck = true docConfig := datamodel.NewDocumentConfiguration() - docConfig.SpecFilePath = execution.SpecFilePath + docConfig.SpecFilePath = execution.SpecFileName //docConfig.SkipCircularReferenceCheck = true if execution.IgnoreCircularArrayRef { @@ -886,7 +886,7 @@ func removeDuplicates(results *[]model.RuleFunctionResult, rse *RuleSetExecution origin := idx.FindNodeOrigin(result.StartNode) if origin != nil { if filepath.Base(origin.AbsoluteLocation) == "root.yaml" { - origin.AbsoluteLocation = rse.SpecFilePath + origin.AbsoluteLocation = rse.SpecFileName } result.Origin = origin } @@ -917,7 +917,7 @@ func removeDuplicates(results *[]model.RuleFunctionResult, rse *RuleSetExecution origin := idx.FindNodeOrigin(result.StartNode) if origin != nil { if filepath.Base(origin.AbsoluteLocation) == "root.yaml" { - origin.AbsoluteLocation = rse.SpecFilePath + origin.AbsoluteLocation = rse.SpecFileName } result.Origin = origin }