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

Support x-kubernetes-v2-spec extension #166

Merged
merged 1 commit into from
Jul 22, 2019
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
9 changes: 6 additions & 3 deletions pkg/builder/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ import (

const (
OpenAPIVersion = "2.0"
// TODO: Make this configurable.
extensionPrefix = "x-kubernetes-"
)

type openAPI struct {
Expand Down Expand Up @@ -154,6 +152,11 @@ func (o *openAPI) buildDefinitionRecursively(name string) error {
schema.Extensions[k] = v
}
}
if v, ok := item.Schema.Extensions[common.ExtensionV2Schema]; ok {
if v2Schema, isOpenAPISchema := v.(spec.Schema); isOpenAPISchema {
schema = v2Schema
}
}
o.swagger.Definitions[uniqueName] = schema
for _, v := range item.Dependencies {
if err := o.buildDefinitionRecursively(v); err != nil {
Expand Down Expand Up @@ -270,7 +273,7 @@ func (o *openAPI) buildOperations(route restful.Route, inPathCommonParamsMap map
},
}
for k, v := range route.Metadata {
if strings.HasPrefix(k, extensionPrefix) {
if strings.HasPrefix(k, common.ExtensionPrefix) {
if ret.Extensions == nil {
ret.Extensions = spec.Extensions{}
}
Expand Down
64 changes: 60 additions & 4 deletions pkg/builder/openapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,36 @@ type TestOutput struct {
Count int `json:"count,omitempty"`
}

type TestExtensionV2Schema struct{}

func (_ TestExtensionV2Schema) OpenAPIDefinition() *openapi.OpenAPIDefinition {
schema := spec.Schema{
VendorExtensible: spec.VendorExtensible{
Extensions: map[string]interface{}{
openapi.ExtensionV2Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"integer"},
},
},
},
},
}
schema.Description = "Test extension V2 spec conversion"
schema.Properties = map[string]spec.Schema{
"apple": {
SchemaProps: spec.SchemaProps{
Description: "Name of the output",
Type: []string{"string"},
Format: "",
},
},
}
return &openapi.OpenAPIDefinition{
Schema: schema,
Dependencies: []string{},
}
}

func (_ TestInput) OpenAPIDefinition() *openapi.OpenAPIDefinition {
schema := spec.Schema{}
schema.Description = "Test input"
Expand Down Expand Up @@ -182,12 +212,14 @@ func getConfig(fullMethods bool) (*openapi.Config, *restful.Container) {
},
GetDefinitions: func(_ openapi.ReferenceCallback) map[string]openapi.OpenAPIDefinition {
return map[string]openapi.OpenAPIDefinition{
"k8s.io/kube-openapi/pkg/builder.TestInput": *TestInput{}.OpenAPIDefinition(),
"k8s.io/kube-openapi/pkg/builder.TestOutput": *TestOutput{}.OpenAPIDefinition(),
"k8s.io/kube-openapi/pkg/builder.TestInput": *TestInput{}.OpenAPIDefinition(),
"k8s.io/kube-openapi/pkg/builder.TestOutput": *TestOutput{}.OpenAPIDefinition(),
"k8s.io/kube-openapi/pkg/builder.TestExtensionV2Schema": *TestExtensionV2Schema{}.OpenAPIDefinition(),
// Bazel changes the package name, this is ok for testing, but we need to fix it if it happened
// in the main code.
"k8s.io/kube-openapi/pkg/builder/go_default_test.TestInput": *TestInput{}.OpenAPIDefinition(),
"k8s.io/kube-openapi/pkg/builder/go_default_test.TestOutput": *TestOutput{}.OpenAPIDefinition(),
"k8s.io/kube-openapi/pkg/builder/go_default_test.TestInput": *TestInput{}.OpenAPIDefinition(),
"k8s.io/kube-openapi/pkg/builder/go_default_test.TestOutput": *TestOutput{}.OpenAPIDefinition(),
"k8s.io/kube-openapi/pkg/builder/go_default_test.TestExtensionV2Schema": *TestExtensionV2Schema{}.OpenAPIDefinition(),
}
},
GetDefinitionName: func(name string) (string, spec.Extensions) {
Expand Down Expand Up @@ -473,3 +505,27 @@ func TestBuildOpenAPIDefinitionsForResource(t *testing.T) {
}
assert.Equal(string(expected_json), string(actual_json))
}

func TestBuildOpenAPIDefinitionsForResourceWithExtensionV2Schema(t *testing.T) {
config, _, assert := setUp(t, true)
expected := &spec.Definitions{
"builder.TestExtensionV2Schema": spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"integer"},
},
},
}
swagger, err := BuildOpenAPIDefinitionsForResource(TestExtensionV2Schema{}, config)
if !assert.NoError(err) {
return
}
expected_json, err := json.Marshal(expected)
if !assert.NoError(err) {
return
}
actual_json, err := json.Marshal(swagger)
if !assert.NoError(err) {
return
}
assert.Equal(string(expected_json), string(actual_json))
}
18 changes: 18 additions & 0 deletions pkg/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ import (
"github.com/go-openapi/spec"
)

const (
// TODO: Make this configurable.
ExtensionPrefix = "x-kubernetes-"
ExtensionV2Schema = ExtensionPrefix + "v2-schema"
)

// OpenAPIDefinition describes single type. Normally these definitions are auto-generated using gen-openapi.
type OpenAPIDefinition struct {
Schema spec.Schema
Expand All @@ -43,6 +49,10 @@ type OpenAPIDefinitionGetter interface {
OpenAPIDefinition() *OpenAPIDefinition
}

type OpenAPIV3DefinitionGetter interface {
OpenAPIV3Definition() *OpenAPIDefinition
}

type PathHandler interface {
Handle(path string, handler http.Handler)
}
Expand Down Expand Up @@ -172,3 +182,11 @@ func EscapeJsonPointer(p string) string {
p = strings.Replace(p, "/", "~1", -1)
return p
}

func EmbedOpenAPIDefinitionIntoV2Extension(main OpenAPIDefinition, embedded OpenAPIDefinition) OpenAPIDefinition {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now, i cant think of a better name for this

if main.Schema.Extensions == nil {
main.Schema.Extensions = make(map[string]interface{})
}
main.Schema.Extensions[ExtensionV2Schema] = embedded.Schema
return main
}
46 changes: 42 additions & 4 deletions pkg/generators/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,16 @@ func methodReturnsValue(mt *types.Type, pkg, name string) bool {
return r.Name.Name == name && r.Name.Package == pkg
}

func hasOpenAPIV3DefinitionMethod(t *types.Type) bool {
for mn, mt := range t.Methods {
if mn != "OpenAPIV3Definition" {
continue
}
return methodReturnsValue(mt, openAPICommonPackagePath, "OpenAPIDefinition")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpenAPIDefinition or OpenAPIV3Definition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the name of struct OpenAPIDefinition under "pkg/common" package, instead of the function name

}
return false
}

func hasOpenAPIDefinitionMethod(t *types.Type) bool {
for mn, mt := range t.Methods {
if mn != "OpenAPIDefinition" {
Expand Down Expand Up @@ -304,9 +314,21 @@ func (g openAPITypeWriter) generateCall(t *types.Type) error {
case types.Struct:
args := argsFromType(t)
g.Do("\"$.$\": ", t.Name)
if hasOpenAPIDefinitionMethod(t) {

hasV2Definition := hasOpenAPIDefinitionMethod(t)
hasV2DefinitionTypeAndFormat := hasOpenAPIDefinitionMethods(t)
hasV3Definition := hasOpenAPIV3DefinitionMethod(t)

switch {
case hasV2DefinitionTypeAndFormat:
g.Do(nameTmpl+"(ref),\n", args)
case hasV2Definition && hasV3Definition:
g.Do("common.EmbedOpenAPIDefinitionIntoV2Extension($.type|raw${}.OpenAPIV3Definition(), $.type|raw${}.OpenAPIDefinition()),\n", args)
case hasV2Definition:
g.Do("$.type|raw${}.OpenAPIDefinition(),\n", args)
} else {
case hasV3Definition:
g.Do("$.type|raw${}.OpenAPIV3Definition(),\n", args)
default:
g.Do(nameTmpl+"(ref),\n", args)
}
}
Expand All @@ -317,14 +339,30 @@ func (g openAPITypeWriter) generate(t *types.Type) error {
// Only generate for struct type and ignore the rest
switch t.Kind {
case types.Struct:
if hasOpenAPIDefinitionMethod(t) {
hasV2Definition := hasOpenAPIDefinitionMethod(t)
hasV2DefinitionTypeAndFormat := hasOpenAPIDefinitionMethods(t)
hasV3Definition := hasOpenAPIV3DefinitionMethod(t)

if hasV2Definition || (hasV3Definition && !hasV2DefinitionTypeAndFormat) {
// already invoked directly
return nil
}

args := argsFromType(t)
g.Do("func "+nameTmpl+"(ref $.ReferenceCallback|raw$) $.OpenAPIDefinition|raw$ {\n", args)
if hasOpenAPIDefinitionMethods(t) {
switch {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no separate case for hasV3Definition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no separate case for hasV3Definition?

hasV3Definition case is short-circuited at L329, do you think about adding a case for hasV3Definition and commenting // this line will never reach for readability? the branches is getting more complex tho

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, we can leave it as it is now.

case hasV2DefinitionTypeAndFormat && hasV3Definition:
g.Do("return common.EmbedOpenAPIDefinitionIntoV2Extension($.type|raw${}.OpenAPIV3Definition(), $.OpenAPIDefinition|raw${\n"+
"Schema: spec.Schema{\n"+
"SchemaProps: spec.SchemaProps{\n", args)
g.generateDescription(t.CommentLines)
g.Do("Type:$.type|raw${}.OpenAPISchemaType(),\n"+
"Format:$.type|raw${}.OpenAPISchemaFormat(),\n"+
"},\n"+
"},\n"+
"})\n}\n\n", args)
return nil
case hasV2DefinitionTypeAndFormat:
g.Do("return $.OpenAPIDefinition|raw${\n"+
"Schema: spec.Schema{\n"+
"SchemaProps: spec.SchemaProps{\n", args)
Expand Down
120 changes: 120 additions & 0 deletions pkg/generators/openapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,79 @@ func (_ Blah) OpenAPIDefinition() openapi.OpenAPIDefinition {
assert.Equal(``, funcBuffer.String())
}

func TestCustomDefV3(t *testing.T) {
callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, `
package foo

import openapi "k8s.io/kube-openapi/pkg/common"

type Blah struct {
}

func (_ Blah) OpenAPIV3Definition() openapi.OpenAPIDefinition {
return openapi.OpenAPIDefinition{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"string"},
Format: "date-time",
},
},
}
}
`)
if callErr != nil {
t.Fatal(callErr)
}
if funcErr != nil {
t.Fatal(funcErr)
}
assert.Equal(`"base/foo.Blah": foo.Blah{}.OpenAPIV3Definition(),
`, callBuffer.String())
assert.Equal(``, funcBuffer.String())
}

func TestCustomDefV2AndV3(t *testing.T) {
callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, `
package foo

import openapi "k8s.io/kube-openapi/pkg/common"

type Blah struct {
}

func (_ Blah) OpenAPIV3Definition() openapi.OpenAPIDefinition {
return openapi.OpenAPIDefinition{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"string"},
Format: "date-time",
},
},
}
}

func (_ Blah) OpenAPIDefinition() openapi.OpenAPIDefinition {
return openapi.OpenAPIDefinition{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"string"},
Format: "date-time",
},
},
}
}
`)
if callErr != nil {
t.Fatal(callErr)
}
if funcErr != nil {
t.Fatal(funcErr)
}
assert.Equal(`"base/foo.Blah": common.EmbedOpenAPIDefinitionIntoV2Extension(foo.Blah{}.OpenAPIV3Definition(), foo.Blah{}.OpenAPIDefinition()),
`, callBuffer.String())
assert.Equal(``, funcBuffer.String())
}

func TestCustomDefs(t *testing.T) {
callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, `
package foo
Expand Down Expand Up @@ -437,6 +510,53 @@ Format:foo.Blah{}.OpenAPISchemaFormat(),
`, funcBuffer.String())
}

func TestCustomDefsV3(t *testing.T) {
callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, `
package foo

import openapi "k8s.io/kube-openapi/pkg/common"

// Blah is a custom type
type Blah struct {
}

func (_ Blah) OpenAPIV3Definition() openapi.OpenAPIDefinition {
return openapi.OpenAPIDefinition{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"string"},
Format: "date-time",
},
},
}
}

func (_ Blah) OpenAPISchemaType() []string { return []string{"string"} }
func (_ Blah) OpenAPISchemaFormat() string { return "date-time" }
`)
if callErr != nil {
t.Fatal(callErr)
}
if funcErr != nil {
t.Fatal(funcErr)
}
assert.Equal(`"base/foo.Blah": schema_base_foo_Blah(ref),
`, callBuffer.String())
assert.Equal(`func schema_base_foo_Blah(ref common.ReferenceCallback) common.OpenAPIDefinition {
return common.EmbedOpenAPIDefinitionIntoV2Extension(foo.Blah{}.OpenAPIV3Definition(), common.OpenAPIDefinition{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Description: "Blah is a custom type",
Type:foo.Blah{}.OpenAPISchemaType(),
Format:foo.Blah{}.OpenAPISchemaFormat(),
},
},
})
}

`, funcBuffer.String())
}

func TestPointer(t *testing.T) {
callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, `
package foo
Expand Down
8 changes: 6 additions & 2 deletions test/integration/builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

"github.com/emicklei/go-restful"
"github.com/go-openapi/spec"
"k8s.io/kube-openapi/pkg/builder"
builderv2 "k8s.io/kube-openapi/pkg/builder"
"k8s.io/kube-openapi/pkg/common"
"k8s.io/kube-openapi/pkg/util"
"k8s.io/kube-openapi/test/integration/pkg/generated"
Expand Down Expand Up @@ -57,7 +57,7 @@ func main() {
config := createOpenAPIBuilderConfig()
config.GetDefinitions = generated.GetOpenAPIDefinitions
// Build the Paths using a simple WebService for the final spec
swagger, serr := builder.BuildOpenAPISpec(createWebServices(), config)
swagger, serr := builderv2.BuildOpenAPISpec(createWebServices(), config)
if serr != nil {
log.Fatalf("ERROR: %s", serr.Error())
}
Expand Down Expand Up @@ -111,6 +111,10 @@ func createWebServices() []*restful.WebService {
w.Route(buildRouteForType(w, "listtype", "SetList"))
w.Route(buildRouteForType(w, "uniontype", "TopLevelUnion"))
w.Route(buildRouteForType(w, "uniontype", "InlinedUnion"))
w.Route(buildRouteForType(w, "custom", "Bal"))
w.Route(buildRouteForType(w, "custom", "Bak"))
w.Route(buildRouteForType(w, "custom", "Bac"))
w.Route(buildRouteForType(w, "custom", "Bah"))
return []*restful.WebService{w}
}

Expand Down
Loading