Skip to content

builder: correct uniqueItems and array types for non-body parameters #629

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.

Sign up for GitHub

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

Open
Lfan-ke wants to merge 1 commit into
base: master
Choose a base branch
from
+107 −33
Open

builder: correct uniqueItems and array types for non-body parameters #629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 16 additions & 3 deletions pkg/builder/openapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,9 +451,22 @@ func (o *openAPI) buildParameter(restParam common.Parameter, bodySample interfac
if openAPIType == "" {
return ret, fmt.Errorf("non-body Restful parameter type should be a simple type, but got : %v", restParam.DataType())
}
ret.Type = openAPIType
ret.Format = openAPIFormat
ret.UniqueItems = !restParam.AllowMultiple()
// uniqueItems is an array-only validation, so a scalar parameter must not
// carry it. A parameter that accepts multiple values is an array of the
// element type, serialized as repeated query values (collectionFormat: multi).
if restParam.AllowMultiple() {
ret.Type = "array"
ret.CollectionFormat = "multi"
ret.Items = &spec.Items{
SimpleSchema: spec.SimpleSchema{
Type: openAPIType,
Format: openAPIFormat,
},
}
} else {
ret.Type = openAPIType
ret.Format = openAPIFormat
}
return ret, nil
}

Expand Down
55 changes: 35 additions & 20 deletions pkg/builder/openapi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/stretchr/testify/assert"

openapi "k8s.io/kube-openapi/pkg/common"
"k8s.io/kube-openapi/pkg/common/restfuladapter"
"k8s.io/kube-openapi/pkg/util/jsontesting"
"k8s.io/kube-openapi/pkg/validation/spec"
)
Expand Down Expand Up @@ -292,12 +293,12 @@ func getTestCommonParameters() []spec.Parameter {
ret := make([]spec.Parameter, 2)
ret[0] = spec.Parameter{
Refable: spec.Refable{
Ref: spec.MustCreateRef("#/parameters/path-z6Ciiujn"),
Ref: spec.MustCreateRef("#/parameters/path-n7c4dXYK"),
},
}
ret[1] = spec.Parameter{
Refable: spec.Refable{
Ref: spec.MustCreateRef("#/parameters/pretty-nN7o5FEq"),
Ref: spec.MustCreateRef("#/parameters/pretty-Vg1Gut1p"),
},
}
return ret
Expand Down Expand Up @@ -328,12 +329,12 @@ func getAdditionalTestParameters() []spec.Parameter {
}
ret[1] = spec.Parameter{
Refable: spec.Refable{
Ref: spec.MustCreateRef("#/parameters/fparam-xCJg5kHS"),
Ref: spec.MustCreateRef("#/parameters/fparam-5cewTdsI"),
},
}
ret[2] = spec.Parameter{
Refable: spec.Refable{
Ref: spec.MustCreateRef("#/parameters/hparam-tx-jfxM1"),
Ref: spec.MustCreateRef("#/parameters/hparam-JCXdvvT5"),
},
}
return ret
Expand Down Expand Up @@ -408,6 +409,32 @@ func getTestOutputDefinition() spec.Schema {
}
}

func TestBuildParameterUniqueItemsAndArray(t *testing.T) {
assert := assert.New(t)
o := &openAPI{}

// A scalar parameter must not carry uniqueItems, which is an array-only validation.
scalar, err := o.buildParameter(&restfuladapter.ParamAdapter{
Param: restful.QueryParameter("limit", "maximum number of results").DataType("integer"),
}, nil)
assert.NoError(err)
assert.Equal("integer", scalar.Type)
assert.False(scalar.UniqueItems)
assert.Nil(scalar.Items)

// A parameter that accepts multiple values is an array of its element type.
multi, err := o.buildParameter(&restfuladapter.ParamAdapter{
Param: restful.QueryParameter("labelSelector", "label selectors").DataType("string").AllowMultiple(true),
}, nil)
assert.NoError(err)
assert.Equal("array", multi.Type)
assert.Equal("multi", multi.CollectionFormat)
assert.False(multi.UniqueItems)
if assert.NotNil(multi.Items) {
assert.Equal("string", multi.Items.Type)
}
}

func TestBuildOpenAPISpec(t *testing.T) {
config, container, assert := setUp(t, true)
expected := &spec.Swagger{
Expand All @@ -431,10 +458,7 @@ func TestBuildOpenAPISpec(t *testing.T) {
"builder.TestOutput": getTestOutputDefinition(),
},
Parameters: map[string]spec.Parameter{
"fparam-xCJg5kHS": {
CommonValidations: spec.CommonValidations{
UniqueItems: true,
},
"fparam-5cewTdsI": {
SimpleSchema: spec.SimpleSchema{
Type: "number",
},
Expand All @@ -444,10 +468,7 @@ func TestBuildOpenAPISpec(t *testing.T) {
Description: "a test form parameter",
},
},
"hparam-tx-jfxM1": {
CommonValidations: spec.CommonValidations{
UniqueItems: true,
},
"hparam-JCXdvvT5": {
SimpleSchema: spec.SimpleSchema{
Type: "integer",
},
Expand All @@ -457,10 +478,7 @@ func TestBuildOpenAPISpec(t *testing.T) {
Description: "a test head parameter",
},
},
"path-z6Ciiujn": {
CommonValidations: spec.CommonValidations{
UniqueItems: true,
},
"path-n7c4dXYK": {
SimpleSchema: spec.SimpleSchema{
Type: "string",
},
Expand All @@ -471,10 +489,7 @@ func TestBuildOpenAPISpec(t *testing.T) {
Required: true,
},
},
"pretty-nN7o5FEq": {
CommonValidations: spec.CommonValidations{
UniqueItems: true,
},
"pretty-Vg1Gut1p": {
SimpleSchema: spec.SimpleSchema{
Type: "string",
},
Expand Down
30 changes: 24 additions & 6 deletions pkg/builder3/openapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,12 +424,30 @@ func (o *openAPI) buildParameter(restParam common.Parameter) (ret *spec3.Paramet
return ret, fmt.Errorf("non-body Restful parameter type should be a simple type, but got : %v", restParam.DataType())
}

ret.Schema = &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{openAPIType},
Format: openAPIFormat,
UniqueItems: !restParam.AllowMultiple(),
},
// uniqueItems is an array-only validation, so a scalar parameter must not
// carry it. A parameter that accepts multiple values is an array of the
// element type (serialized with the default form style, explode=true).
if restParam.AllowMultiple() {
ret.Schema = &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"array"},
Items: &spec.SchemaOrArray{
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{openAPIType},
Format: openAPIFormat,
},
},
},
},
}
} else {
ret.Schema = &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{openAPIType},
Format: openAPIFormat,
},
}
}
return ret, nil
}
Expand Down
36 changes: 32 additions & 4 deletions pkg/builder3/openapi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/stretchr/testify/assert"

openapi "k8s.io/kube-openapi/pkg/common"
"k8s.io/kube-openapi/pkg/common/restfuladapter"
"k8s.io/kube-openapi/pkg/spec3"
"k8s.io/kube-openapi/pkg/util/jsontesting"
"k8s.io/kube-openapi/pkg/validation/spec"
Expand Down Expand Up @@ -288,8 +289,7 @@ func getTestCommonParameters() []*spec3.Parameter {
Required: true,
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"string"},
UniqueItems: true,
Type: []string{"string"},
},
},
},
Expand All @@ -301,8 +301,7 @@ func getTestCommonParameters() []*spec3.Parameter {
In: "query",
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"string"},
UniqueItems: true,
Type: []string{"string"},
},
},
},
Expand Down Expand Up @@ -427,6 +426,35 @@ func getTestOutputDefinition() *spec.Schema {
}
}

func TestBuildParameterUniqueItemsAndArray(t *testing.T) {
assert := assert.New(t)
o := &openAPI{}

// A scalar parameter must not carry uniqueItems, which is an array-only validation.
scalar, err := o.buildParameter(&restfuladapter.ParamAdapter{
Param: restful.QueryParameter("limit", "maximum number of results").DataType("integer"),
})
assert.NoError(err)
if assert.NotNil(scalar.Schema) {
assert.Equal(spec.StringOrArray{"integer"}, scalar.Schema.Type)
assert.False(scalar.Schema.UniqueItems)
assert.Nil(scalar.Schema.Items)
}

// A parameter that accepts multiple values is an array of its element type.
multi, err := o.buildParameter(&restfuladapter.ParamAdapter{
Param: restful.QueryParameter("labelSelector", "label selectors").DataType("string").AllowMultiple(true),
})
assert.NoError(err)
if assert.NotNil(multi.Schema) {
assert.Equal(spec.StringOrArray{"array"}, multi.Schema.Type)
assert.False(multi.Schema.UniqueItems)
if assert.NotNil(multi.Schema.Items) && assert.NotNil(multi.Schema.Items.Schema) {
assert.Equal(spec.StringOrArray{"string"}, multi.Schema.Items.Schema.Type)
}
}
}

func TestBuildOpenAPISpec(t *testing.T) {
config, container, assert := setUp(t, true)
expected := &spec3.OpenAPI{
Expand Down