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

Add a separate OAS3NoRefSiblingsRule #576

Merged
merged 1 commit into from
Nov 19, 2024
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
1 change: 1 addition & 0 deletions functions/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func MapBuiltinFunctions() Functions {
funcs["oasTagDefined"] = openapi_functions.TagDefined{}
funcs["oasPathParam"] = openapi_functions.PathParameters{}
funcs["refSiblings"] = openapi_functions.NoRefSiblings{}
funcs["oasRefSiblings"] = openapi_functions.OASNoRefSiblings{}
funcs["typedEnum"] = openapi_functions.TypedEnum{}
funcs["duplicatedEnum"] = openapi_functions.DuplicatedEnum{}
funcs["noEvalDescription"] = openapi_functions.NoEvalInDescriptions{}
Expand Down
2 changes: 1 addition & 1 deletion functions/functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ import (

func TestMapBuiltinFunctions(t *testing.T) {
funcs := MapBuiltinFunctions()
assert.Len(t, funcs.GetAllFunctions(), 72)
assert.Len(t, funcs.GetAllFunctions(), 73)
}
67 changes: 67 additions & 0 deletions functions/openapi/oas_no_ref_siblings.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package openapi

import (
"strings"

"github.com/daveshanley/vacuum/model"
vacuumUtils "github.com/daveshanley/vacuum/utils"
"github.com/pb33f/libopenapi/utils"
"gopkg.in/yaml.v3"
)

// OASNoRefSiblings validates that no properties other than `description` and `summary` are added alongside a `$ref`.
// This rule helps ensure that only essential properties are attached to `$ref` nodes, preventing unnecessary and unused additions.
type OASNoRefSiblings struct {
}

// GetCategory returns the category of the OASNoRefSiblings rule.
func (nrs OASNoRefSiblings) GetCategory() string {
return model.FunctionCategoryOpenAPI
}

// GetSchema returns a model.RuleFunctionSchema defining the schema of the OASNoRefSiblings rule.
func (nrs OASNoRefSiblings) GetSchema() model.RuleFunctionSchema {
return model.RuleFunctionSchema{
Name: "oasRefSiblings",
}
}

func notAllowedKeys(node *yaml.Node) []string {
var notAllowedKeys []string

for i := 0; i < len(node.Content); i += 2 {
key := node.Content[i].Value
switch key {
case "$ref", "summary", "description":
continue
default:
notAllowedKeys = append(notAllowedKeys, key)
}
}
return notAllowedKeys
}

// RunRule will execute the OASNoRefSiblings rule, based on supplied context and a supplied []*yaml.Node slice.
func (nrs OASNoRefSiblings) RunRule(nodes []*yaml.Node, context model.RuleFunctionContext) []model.RuleFunctionResult {

if len(nodes) <= 0 {
return nil
}

var results []model.RuleFunctionResult
siblings := context.Index.GetReferencesWithSiblings()
for _, ref := range siblings {
notAllowedKeys := notAllowedKeys(ref.Node)
if len(notAllowedKeys) != 0 {
key, val := utils.FindKeyNode("$ref", ref.Node.Content)
results = append(results, model.RuleFunctionResult{
Message: "a `$ref` can only be placed next to `summary` and `description` but got:" + strings.Join(notAllowedKeys, " ,"),
StartNode: key,
EndNode: vacuumUtils.BuildEndNode(val),
Path: ref.Path,
Rule: context.Rule,
})
}
}
return results
}
264 changes: 264 additions & 0 deletions functions/openapi/oas_ref_siblings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
package openapi

import (
"testing"

"github.com/daveshanley/vacuum/model"
"github.com/pb33f/libopenapi/index"
"github.com/pb33f/libopenapi/utils"
"github.com/stretchr/testify/assert"
"gopkg.in/yaml.v3"
)

func TestOASNoRefSiblings_GetSchema(t *testing.T) {
def := OASNoRefSiblings{}
assert.Equal(t, "oasRefSiblings", def.GetSchema().Name)
}

func TestOASNoRefSiblings_RunRule(t *testing.T) {
def := OASNoRefSiblings{}
res := def.RunRule(nil, model.RuleFunctionContext{})
assert.Len(t, res, 0)
}

func TestOASNoRefSiblings_RunRule_Description(t *testing.T) {

yml := `paths:
/nice/{rice}:
requestBody:
content:
application/json:
schema:
description: this is a good place to do this
$ref: '#/components/schemas/Rice'
/hot/{dog}:
requestBody:
content:
application/json:
schema:
description: Still a good place to do this
$ref: '#/components/schemas/Dog'`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)
nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "oas3_no_ref_siblings", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

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

assert.Len(t, res, 0)
}

func TestOASNoRefSiblings_RunRule_Deprecated_Example(t *testing.T) {

yml := `paths:
/nice/{rice}:
requestBody:
content:
application/json:
schema:
description: the deprecated flag should not be here
deprecated: true
$ref: '#/components/schemas/Rice'
/hot/{dog}:
requestBody:
content:
application/json:
schema:
example: this should also not be here
$ref: '#/components/schemas/Dog'`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)
nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "oas3_no_ref_siblings", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

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

assert.Len(t, res, 2)
}

func TestOASNoRefSiblings_RunRule_Components(t *testing.T) {

yml := `components:
schemas:
Beer:
description: perfect
$ref: '#/components/Yum'
Bottle:
type: string
Cake:
$ref: '#/components/Sugar'`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "oas3_no_ref_siblings", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

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

assert.Len(t, res, 0)

}

func TestOASNoRefSiblings_RunRule_Parameters(t *testing.T) {

yml := `parameters:
testParam:
$ref: '#/parameters/oldParam'
oldParam:
in: query
description: old
wrongParam:
description: I am allowed to be here
$ref: '#/parameters/oldParam'`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "oas3_no_ref_siblings", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

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

assert.Len(t, res, 0)

}

func TestOASNoRefSiblings_RunRule_Definitions(t *testing.T) {

yml := `definitions:
test:
$ref: '#/definitions/old'
old:
type: object
description: old
wrong:
description: I am allowed to be here
$ref: '#/definitions/oldParam'`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "oas3_no_ref_siblings", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

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

assert.Len(t, res, 0)

}

func TestOASNoRefSiblings_RunRule_Success(t *testing.T) {

yml := `paths:
/nice/{rice}:
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Rice'
/hot/{dog}:
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Dog'`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "oas3_no_ref_siblings", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

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

assert.Len(t, res, 0)

}

func TestOASNoRefSiblings_RunRule_Fail_Single(t *testing.T) {

yml := `paths:
/nice/{rice}:
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Rice'
/hot/{dog}:
requestBody:
content:
application/json:
schema:
type: integer
$ref: '#/components/schemas/Dog'`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "oas3_no_ref_siblings", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

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

assert.Len(t, res, 1)

}
4 changes: 3 additions & 1 deletion model/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ package model
import (
_ "embed"
"fmt"
"gopkg.in/yaml.v3"
"regexp"
"strings"

"gopkg.in/yaml.v3"
)

const (
Expand All @@ -15,6 +16,7 @@ const (
)

var OAS3_1Format = []string{OAS31}
var AllExceptOAS3_1 = []string{OAS2, OAS3}
var OAS3Format = []string{OAS3}
var OAS3AllFormat = []string{OAS3, OAS31}
var OAS2Format = []string{OAS2}
Expand Down
3 changes: 3 additions & 0 deletions rulesets/rule_fixes.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ const (
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!"

oas3noRefSiblingsFix string = "$ref values must not be placed next to sibling nodes, except `description` and `summary` nodes. " +
" This is wrong. remove all additional siblings!"

oas3UnusedComponentFix string = "Orphaned components are not used by anything. You might have plans to use them later, " +
"or they could be older schemas that never got cleaned up. A clean spec is a happy spec. Prune your orphaned components."

Expand Down
Loading
Loading