Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Applied fix for #142 and some other tweaks #143

Merged
merged 2 commits into from
Oct 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- name: Set up Go 1.x
uses: actions/setup-go@v2
with:
go-version: ^1.16
go-version: ^1.18
id: go

- name: Checkout code
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/label-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ jobs:
- uses: jesusvasquez333/verify-pr-label-action@v1.4.0
with:
github-token: "${{ secrets.GITHUB_TOKEN }}"
valid-labels: "release/patch, release/minor, release/major"
valid-labels: "release/patch, release/minor, release/major, bugfix, feature"
pull-request-number: '${{ github.event.pull_request.number }}'
disable-reviews: true
2 changes: 1 addition & 1 deletion .github/workflows/tag.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: Tag

on:
push:
workflow_dispatch:
branches:
- main

Expand Down
27 changes: 24 additions & 3 deletions functions/openapi/unused_component.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,20 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction
links := context.Index.GetAllLinks()
callbacks := context.Index.GetAllCallbacks()

// create poly maps.
oneOfRefs := make(map[string]*index.Reference)
allOfRefs := make(map[string]*index.Reference)
anyOfRefs := make(map[string]*index.Reference)

// include all polymorphic references.
for _, ref := range context.Index.GetPolyAllOfReferences() {
allRefs[ref.Definition] = ref
allOfRefs[ref.Definition] = ref
}
for _, ref := range context.Index.GetPolyOneOfReferences() {
oneOfRefs[ref.Definition] = ref
}
for _, ref := range context.Index.GetPolyAnyOfReferences() {
anyOfRefs[ref.Definition] = ref
}

// if a component does not exist in allRefs, it was not referenced anywhere.
Expand All @@ -66,9 +78,18 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction
// find everything that was never referenced.
for _, resultMap := range mapsToSearch {
for key, ref := range resultMap {

// check everything!
if allRefs[key] == nil {
// nothing is using this!
notUsed[key] = ref
found := false
// check poly refs if the reference can't be found
if oneOfRefs[key] != nil || allOfRefs[key] != nil || anyOfRefs[key] != nil {
found = true
}
if !found {
// nothing is using this!
notUsed[key] = ref
}
}
}
}
Expand Down
57 changes: 57 additions & 0 deletions functions/openapi/unused_component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,60 @@ components:

assert.Len(t, res, 4)
}

func TestUnusedComponent_RunRule_Success_PolymorphicCheck(t *testing.T) {

yml := `paths:
/naughty/{puppy}:
get:
responses:
"200":
description: The naughty pup
content:
application/json:
schema:
oneOf:
- $ref: '#/components/schemas/Puppy'
"404":
description: The naughty kitty
content:
application/json:
schema:
anyOf:
- $ref: '#/components/schemas/Kitty'
"500":
description: The naughty bunny
content:
application/json:
schema:
allOf:
- $ref: '#/components/schemas/Bunny'
components:
schemas:
Puppy:
type: string
description: pup
Kitty:
type: string
description: kitty
Bunny:
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)
ctx.Index = index.NewSpecIndex(&rootNode)

def := UnusedComponent{}
res := def.RunRule(nodes, ctx)

assert.Len(t, res, 0)
}
6 changes: 6 additions & 0 deletions motor/rule_applicator.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/pb33f/libopenapi/index"
"github.com/pb33f/libopenapi/resolver"
"github.com/pb33f/libopenapi/utils"
"github.com/pterm/pterm"
"gopkg.in/yaml.v3"
"sync"
)
Expand Down Expand Up @@ -340,6 +341,11 @@ func buildResults(ctx ruleContext, ruleAction model.RuleAction, nodes []*yaml.No
SpecInfo: ctx.specInfo,
}

if ctx.specInfo.SpecFormat == "" && ctx.specInfo.Version == "" {
pterm.Warning.Printf("Specification version not detected, cannot apply rule `%s`\n", ctx.rule.Id)
return ctx.ruleResults
}

// validate the rule is configured correctly before running it.
res, errs := model.ValidateRuleFunctionContextAgainstSchema(ruleFunction, rfc)
if !res {
Expand Down
57 changes: 38 additions & 19 deletions motor/rule_applicator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,8 @@ func TestApplyRulesToRuleSet_CircularReferences(t *testing.T) {

func TestRuleSet_ContactProperties(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
contact:
name: pizza
email: monkey`
Expand All @@ -638,7 +639,8 @@ func TestRuleSet_ContactProperties(t *testing.T) {

func TestRuleSet_InfoContact(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
title: Terrible API Spec
description: No operations, no contact, useless.`

Expand All @@ -657,7 +659,8 @@ func TestRuleSet_InfoContact(t *testing.T) {

func TestRuleSet_InfoDescription(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
title: Terrible API Spec
contact:
name: rubbish
Expand All @@ -678,7 +681,8 @@ func TestRuleSet_InfoDescription(t *testing.T) {

func TestRuleSet_InfoLicense(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
title: Terrible API Spec
description: really crap
contact:
Expand All @@ -700,7 +704,8 @@ func TestRuleSet_InfoLicense(t *testing.T) {

func TestRuleSet_InfoLicenseUrl(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
title: Terrible API Spec
description: really crap
contact:
Expand All @@ -724,7 +729,8 @@ func TestRuleSet_InfoLicenseUrl(t *testing.T) {

func TestRuleSet_NoEvalInMarkdown(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
description: this has no eval('alert(1234') impact in vacuum, but JS tools might suffer.`

rules := make(map[string]*model.Rule)
Expand All @@ -742,7 +748,8 @@ func TestRuleSet_NoEvalInMarkdown(t *testing.T) {

func TestRuleSet_NoScriptInMarkdown(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
description: this has no impact in vacuum, <script>alert('XSS for you')</script>`

rules := make(map[string]*model.Rule)
Expand All @@ -761,7 +768,8 @@ func TestRuleSet_NoScriptInMarkdown(t *testing.T) {

func TestRuleSet_TagsAlphabetical(t *testing.T) {

yml := `tags:
yml := `openapi: 3.1.0
tags:
- name: zebra
- name: chicken
- name: puppy`
Expand All @@ -782,7 +790,8 @@ func TestRuleSet_TagsAlphabetical(t *testing.T) {

func TestRuleSet_TagsMissing(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
contact:
name: Duck
paths:
Expand Down Expand Up @@ -810,7 +819,8 @@ components:

func TestRuleSet_TagsNotArray(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
contact:
name: Duck
tags: none
Expand Down Expand Up @@ -839,7 +849,8 @@ components:

func TestRuleSet_TagsWrongType(t *testing.T) {

yml := `info:
yml := `openapi: 3.1.0
info:
contact:
name: Duck
tags:
Expand Down Expand Up @@ -869,7 +880,8 @@ components:

func TestRuleSet_OperationIdInvalidInUrl(t *testing.T) {

yml := `paths:
yml := `openapi: 3.1.0
paths:
/hi:
get:
operationId: nice rice
Expand All @@ -893,7 +905,8 @@ func TestRuleSet_OperationIdInvalidInUrl(t *testing.T) {

func TestRuleSetGetOperationTagsRule(t *testing.T) {

yml := `paths:
yml := `openapi: 3.1.0
paths:
/hi:
get:
tags:
Expand All @@ -918,7 +931,8 @@ func TestRuleSetGetOperationTagsRule(t *testing.T) {

func TestRuleSetGetOperationTagsMultipleRule(t *testing.T) {

yml := `paths:
yml := `openapi: 3.1.0
paths:
/hi:
get:
tags:
Expand Down Expand Up @@ -949,7 +963,8 @@ func TestRuleSetGetOperationTagsMultipleRule(t *testing.T) {

func TestRuleSetGetPathDeclarationsMustExist(t *testing.T) {

yml := `paths:
yml := `openapi: 3.1.0
paths:
/hi/{there}:
get:
operationId: a
Expand All @@ -974,7 +989,8 @@ func TestRuleSetGetPathDeclarationsMustExist(t *testing.T) {

func TestRuleSetNoPathTrailingSlashTest(t *testing.T) {

yml := `paths:
yml := `openapi: 3.1.0
paths:
/hi/{there}/:
get:
operationId: a
Expand All @@ -1000,7 +1016,8 @@ func TestRuleSetNoPathTrailingSlashTest(t *testing.T) {

func TestRuleSetNoPathQueryString(t *testing.T) {

yml := `paths:
yml := `openapi: 3.1.0
paths:
/hi/{there}?oh=yeah:
get:
operationId: a
Expand All @@ -1026,7 +1043,8 @@ func TestRuleSetNoPathQueryString(t *testing.T) {

func TestRuleTagDescriptionRequiredRule(t *testing.T) {

yml := `tags:
yml := `openapi: 3.1.0
tags:
- name: pizza
description: nice
- name: cinnamon
Expand Down Expand Up @@ -1245,7 +1263,8 @@ func TestRuleOAS3HostTrailingSlashRule(t *testing.T) {

func TestRuleOAS3HostTrailingSlashRule_Fail(t *testing.T) {

yml := `servers:
yml := `openapi: 3.1.0
servers:
- url: https://quobix.com/
- url: https://pb33f.io/
`
Expand Down
5 changes: 3 additions & 2 deletions motor/rule_tests/openapi_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ func Benchmark_DefaultOpenAPI(b *testing.B) {

func Test_Default_OpenAPIRuleSet_FireABunchOfIssues(t *testing.T) {

badDoc := `paths:
badDoc := `openapi: 3.0.3
paths:
/curry/{hurry}/{salsa}:
get:
tags:
Expand All @@ -71,7 +72,7 @@ func Test_Default_OpenAPIRuleSet_FireABunchOfIssues(t *testing.T) {
rules := rs.GenerateOpenAPIDefaultRuleSet()
lintExecution := motor.ApplyRulesToRuleSet(&motor.RuleSetExecution{RuleSet: rules, Spec: []byte(badDoc)})
assert.Len(t, lintExecution.Errors, 0)
assert.Len(t, lintExecution.Results, 29)
assert.Len(t, lintExecution.Results, 27)

for n := 0; n < len(lintExecution.Results); n++ {
assert.NotNil(t, lintExecution.Results[n].Path)
Expand Down
2 changes: 1 addition & 1 deletion rulesets/ruleset_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ func GetOAS2UnusedComponentRule() *model.Rule {
Name: "Check for unused definitions",
Id: oas2UnusedDefinition,
Formats: model.OAS2Format,
Description: "Check for unused components and bad references",
Description: "Check for unused definitions and bad references",
Given: "$",
Resolved: false,
Recommended: true,
Expand Down