Skip to content

Commit

Permalink
Added new ambiguous paths function and re-enabled schemas
Browse files Browse the repository at this point in the history
Schemas are now running as part of main ruleset. had to updated all the tests.
the schema will try the faster approach for OpenAPI, older swagger specs will run through the less validator (this needs populating through the app now also)

Signed-off-by: Dave Shanley <dave@quobix.com>
  • Loading branch information
daveshanley committed Jul 16, 2022
1 parent f4305ca commit 15be0ce
Show file tree
Hide file tree
Showing 16 changed files with 190 additions and 43 deletions.
1 change: 1 addition & 0 deletions functions/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func MapBuiltinFunctions() Functions {
funcs["oasPolymorphicOneOf"] = openapi_functions.PolymorphicOneOf{}
funcs["oasDocumentSchema"] = openapi_functions.OASSchema{}
funcs["oasAPIServers"] = openapi_functions.APIServers{}
funcs["noAmbiguousPaths"] = openapi_functions.AmbiguousPaths{}

})

Expand Down
2 changes: 1 addition & 1 deletion functions/functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ import (

func TestMapBuiltinFunctions(t *testing.T) {
funcs := MapBuiltinFunctions()
assert.Len(t, funcs.GetAllFunctions(), 38)
assert.Len(t, funcs.GetAllFunctions(), 39)
}
55 changes: 38 additions & 17 deletions functions/openapi/oas_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package openapi
import (
"fmt"
"github.com/daveshanley/vacuum/model"
"github.com/santhosh-tekuri/jsonschema/v5"
"github.com/xeipuuv/gojsonschema"
"gopkg.in/yaml.v3"
)
Expand Down Expand Up @@ -41,29 +42,49 @@ func (os OASSchema) RunRule(nodes []*yaml.Node, context model.RuleFunctionContex
return results
}

specBytes := *info.SpecJSONBytes
// Swagger specs are not supported with this schema checker (annoying, but you get what you pay for).
schema, err := jsonschema.CompileString("schema.json", info.APISchema)
if err != nil {

// create loader from original bytes.
doc := gojsonschema.NewStringLoader(string(specBytes))
// do the swagger thing.
swaggerSchema := gojsonschema.NewStringLoader(info.APISchema)
spec := gojsonschema.NewStringLoader(string(*info.SpecJSONBytes))
res, validateErr := gojsonschema.Validate(swaggerSchema, spec)

res, err := gojsonschema.Validate(info.APISchema, doc)
if validateErr != nil {
results = append(results, model.RuleFunctionResult{
Message: fmt.Sprintf("OpenAPI specification cannot be validated: %v", validateErr.Error()),
StartNode: nodes[0],
EndNode: nodes[0],
Path: "$",
Rule: context.Rule,
})
return results
}

if err != nil {
results = append(results, model.RuleFunctionResult{
Message: fmt.Sprintf("OpenAPI specification cannot be validated: %s", err.Error()),
StartNode: nodes[0],
EndNode: nodes[0],
Path: "$",
Rule: context.Rule,
})
return results
// if the spec is not valid, run through all the issues and return.
if !res.Valid() {
for _, resErr := range res.Errors() {
results = append(results, model.RuleFunctionResult{
Message: fmt.Sprintf("OpenAPI specification is invalid: %s", resErr.Description()),
StartNode: nodes[0],
EndNode: nodes[0],
Path: "$",
Rule: context.Rule,
})
}
return results
}
return nil
}

// if the spec is not valid, run through all the issues and return.
if !res.Valid() {
for _, err := range res.Errors() {
//validate using faster, more accurate resolver.
if validationError := schema.Validate(*info.SpecJSON); validationError != nil {
failure := validationError.(*jsonschema.ValidationError)
for _, fail := range failure.Causes {
results = append(results, model.RuleFunctionResult{
Message: fmt.Sprintf("OpenAPI specification is invalid: %s", err.Description()),
Message: fmt.Sprintf("OpenAPI specification is invalid: %s %v", fail.KeywordLocation,
fail.Message),
StartNode: nodes[0],
EndNode: nodes[0],
Path: "$",
Expand Down
53 changes: 53 additions & 0 deletions functions/openapi/oas_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,59 @@ paths:
assert.Len(t, res, 1)
}

func TestOAS3Schema_RunRule_Pass(t *testing.T) {

yml := `openapi: "3.0.0"
info:
contact:
name: Hi
url: https://quobix.com/vacuum
license:
name: MIT
termsOfService: https://quobix.com/vacuum
title: Quobix
version: "1.0"
paths:
/hi:
get:
responses:
"200":
description: OK`

path := "$"

specInfo, _ := model.ExtractSpecInfo([]byte(yml))

rule := buildOpenApiTestRuleAction(path, "oas3_schema", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
ctx.Index = model.NewSpecIndex(specInfo.RootNode)
ctx.SpecInfo = specInfo

def := OASSchema{}
res := def.RunRule([]*yaml.Node{specInfo.RootNode}, ctx)

assert.Len(t, res, 0)
}

func TestOAS3Schema_RunRule_Fail(t *testing.T) {

yml := `openapi: "3.0"`

path := "$"

specInfo, _ := model.ExtractSpecInfo([]byte(yml))

rule := buildOpenApiTestRuleAction(path, "oas3_schema", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
ctx.Index = model.NewSpecIndex(specInfo.RootNode)
ctx.SpecInfo = specInfo

def := OASSchema{}
res := def.RunRule([]*yaml.Node{specInfo.RootNode}, ctx)

assert.Len(t, res, 2)
}

func TestOAS2Schema_RunRule_Success(t *testing.T) {

yml := `swagger: '2.0'
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/json-iterator/go v1.1.12
github.com/mitchellh/mapstructure v1.5.0
github.com/pterm/pterm v0.12.42
github.com/santhosh-tekuri/jsonschema/v5 v5.0.0
github.com/spf13/cobra v1.5.0
github.com/stretchr/testify v1.8.0
github.com/vmware-labs/yaml-jsonpath v0.3.2
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ github.com/pterm/pterm v0.12.42/go.mod h1:hJgLlBafm45w/Hr0dKXxY//POD7CgowhePaG1s
github.com/rivo/uniseg v0.2.0 h1:S1pD9weZBuJdFmowNwbpi7BJ8TNftyUImj/0WQi72jY=
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/santhosh-tekuri/jsonschema/v5 v5.0.0 h1:TToq11gyfNlrMFZiYujSekIsPd9AmsA2Bj/iv+s4JHE=
github.com/santhosh-tekuri/jsonschema/v5 v5.0.0/go.mod h1:FKdcjfQW6rpZSnxxUvEA5H/cDPdvJ/SZJQLWWXWGrZ0=
github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ=
github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
Expand Down
3 changes: 1 addition & 2 deletions model/spec.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package model

import (
"github.com/xeipuuv/gojsonschema"
"gopkg.in/yaml.v3"
"time"
)
Expand All @@ -17,7 +16,7 @@ type SpecInfo struct {
SpecJSONBytes *[]byte `json:"-"` // original bytes converted to JSON
SpecJSON *map[string]interface{} `json:"-"` // standard JSON map of original bytes
Error error `json:"-"` // something go wrong?
APISchema gojsonschema.JSONLoader `json:"-"` // API Schema for supplied spec type (2 or 3)
APISchema string `json:"-"` // API Schema for supplied spec type (2 or 3)
Generated time.Time `json:"-"`
jsonParsingChannel chan bool
}
Expand Down
11 changes: 4 additions & 7 deletions model/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"errors"
"fmt"
"github.com/daveshanley/vacuum/utils"
"github.com/xeipuuv/gojsonschema"
"gopkg.in/yaml.v3"
"strings"
)
Expand All @@ -17,10 +16,10 @@ const (
OAS31 = "oas3_1"
)

//go:embed schemas/oas3-schema.yaml
//go:embed schemas/oas3-schema.json
var OpenAPI3SchemaData string

//go:embed schemas/swagger2-schema.yaml
//go:embed schemas/swagger2-schema.json
var OpenAPI2SchemaData string

var OAS3_1Format = []string{OAS31}
Expand Down Expand Up @@ -70,12 +69,10 @@ func ExtractSpecInfo(spec []byte) (*SpecInfo, error) {
// run in a separate thread, don't block.

if spec.SpecType == utils.OpenApi3 {
openAPI3JSON, _ := utils.ConvertYAMLtoJSON([]byte(OpenAPI3SchemaData))
spec.APISchema = gojsonschema.NewStringLoader(string(openAPI3JSON))
spec.APISchema = OpenAPI3SchemaData
}
if spec.SpecType == utils.OpenApi2 {
openAPI2JSON, _ := utils.ConvertYAMLtoJSON([]byte(OpenAPI2SchemaData))
spec.APISchema = gojsonschema.NewStringLoader(string(openAPI2JSON))
spec.APISchema = OpenAPI2SchemaData
}

if utils.IsYAML(string(bytes)) {
Expand Down
2 changes: 1 addition & 1 deletion parser/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ var openApiWat = `openapi: 3.2
info:
title: Test API, valid, but not quite valid
servers:
- url: http://quobix.com/api`
- url: https://quobix.com/vacuum`

func TestCheckSpecIsValidOpenAPI3_Error(t *testing.T) {

Expand Down
9 changes: 9 additions & 0 deletions plugin/plugin_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,12 @@ func TestLoadFunctions_Sample(t *testing.T) {
assert.Equal(t, 0, pm.LoadedFunctionCount())
}
}

func TestLoadFunctions_TestCompile(t *testing.T) {
pm, err := LoadFunctions("sample")
if runtime.GOOS != "windows" { // windows does not support this feature, at all.
assert.NotNil(t, pm)
assert.NoError(t, err)
assert.Equal(t, 0, pm.LoadedFunctionCount())
}
}
6 changes: 5 additions & 1 deletion rulesets/rule_fixes.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ const (
duplicatedEntryInEnumFix string = "Enums need to be unique, you can't duplicate them in the same definition. Please remove" +
" the duplicate value."

noRefSiblingsFix string = "$ref values must not be placed next to sibling nodes, There should only be a single node" +
noRefSiblingsFix string = "$ref values must not be placed next to sibling nodes, There should only be a single node " +
" when using $ref. A common mistake is adding 'description' next to a $ref. This is wrong. remove all siblings!"

oas3UnusedComponentFix string = "Orphaned components are not used by anything. You might have plans to use them later, " +
Expand All @@ -138,4 +138,8 @@ const (
unusedComponentFix string = "Unused components / definitions are generally the result of the OpenAPI contract being updated without " +
"considering references. Another reference could have been updated, or an operation changed that no longer references this component. " +
"Remove this component from the spec, or re-link to it from another component or operation to fix the problem."

ambiguousPathsFix string = "Paths must all resolve unambiguously, they can't be confused with one another (/{id}/ambiguous and /ambiguous/{id} " +
"are the same thing. Make sure every path and the variables used are unique and do conflict with one another. Check the ordering of variables " +
"and the naming of path segments."
)
24 changes: 22 additions & 2 deletions rulesets/ruleset_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ func GetOAS2SchemaRule() *model.Rule {
Description: "OpenAPI 2 specification is invalid",
Given: "$",
Resolved: false,
Recommended: true,
Recommended: false,
RuleCategory: model.RuleCategories[model.CategoryValidation],
Type: validation,
Severity: err,
Expand All @@ -833,7 +833,7 @@ func GetOAS3SchemaRule() *model.Rule {
Description: "OpenAPI 3 specification is invalid",
Given: "$",
Resolved: false,
Recommended: true,
Recommended: false,
RuleCategory: model.RuleCategories[model.CategoryValidation],
Type: validation,
Severity: err,
Expand Down Expand Up @@ -1129,3 +1129,23 @@ func GetOAS2ExamplesRule() *model.Rule {
HowToFix: oas3ExamplesFix,
}
}

// NoAmbiguousPaths will check for paths that are ambiguous with one another
func NoAmbiguousPaths() *model.Rule {
return &model.Rule{
Name: "No ambiguous paths, each path must resolve unambiguously",
Id: noAmbiguousPaths,
Formats: model.AllFormats,
Description: "Paths need to resolve unambiguously from one another",
Given: "$",
Resolved: true,
Recommended: true,
RuleCategory: model.RuleCategories[model.CategoryOperations],
Type: validation,
Severity: err,
Then: model.RuleAction{
Function: "ambiguousPaths",
},
HowToFix: ambiguousPathsFix,
}
}
4 changes: 3 additions & 1 deletion rulesets/rulesets.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const (
allPaths = "$.paths[*]"
allOperations = "[?(@.get || @.post || @.put || @.patch || @.delete || @.trace || @.options || @.head)]"

noAmbiguousPaths = "no-ambiguous-paths"
operationSuccessResponse = "operation-success-response"
operationOperationIdUnique = "operation-operationId-unique"
operationOperationId = "operation-operationId"
Expand Down Expand Up @@ -297,10 +298,11 @@ func generateDefaultOpenAPIRuleSet() *RuleSet {
rules[oas2OneOf] = GetOAS2PolymorphicOneOfRule()
rules[oas3ValidSchemaExample] = GetOAS3ExamplesRule()
rules[oas2ValidSchemaExample] = GetOAS2ExamplesRule()
rules[noAmbiguousPaths] = NoAmbiguousPaths()

// TODO: enable for a different ruleset.
//rules[oas2Schema] = GetOAS2SchemaRule()
//rules[oas3Schema] = GetOAS3SchemaRule()
rules[oas3Schema] = GetOAS3SchemaRule()

set := &RuleSet{
DocumentationURI: "https://quobix.com/vacuum/rulesets/all",
Expand Down
Loading

0 comments on commit 15be0ce

Please sign in to comment.