Skip to content
Permalink
Browse files

do not publish openapi for a schema containing bad types

minor rewords & refactor

run script: update misc

remove null from supported types

tmp
  • Loading branch information...
roycaihw authored and yue9944882 committed Jul 3, 2019
1 parent ae45744 commit b6e415a73bf4c454311c5480cd5aeddfd215ddf7
@@ -47,6 +47,7 @@ import (
var (
printerColumnDatatypes = sets.NewString("integer", "number", "string", "boolean", "date")
customResourceColumnDefinitionFormats = sets.NewString("int32", "int64", "float", "double", "byte", "date", "date-time", "password")
openapiV3Types = sets.NewString("string", "number", "integer", "boolean", "array", "object")
)

// ValidateCustomResourceDefinition statically validates
@@ -1157,3 +1158,74 @@ func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition,
return field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations").Key(v1beta1.KubeAPIApprovedAnnotation), newCRD.Annotations[v1beta1.KubeAPIApprovedAnnotation], reason)}
}
}

func SchemaHasInvalidTypes(s *apiextensions.JSONSchemaProps) bool {
if s == nil {
return false
}

if len(s.Type) > 0 && !openapiV3Types.Has(s.Type) {
return true
}

if s.Items != nil {
if s.Items != nil && SchemaHasInvalidTypes(s.Items.Schema) {
return true
}
for _, s := range s.Items.JSONSchemas {
if SchemaHasInvalidTypes(&s) {
return true
}
}
}
for _, s := range s.AllOf {
if SchemaHasInvalidTypes(&s) {
return true
}
}
for _, s := range s.AnyOf {
if SchemaHasInvalidTypes(&s) {
return true
}
}
for _, s := range s.OneOf {
if SchemaHasInvalidTypes(&s) {
return true
}
}
if SchemaHasInvalidTypes(s.Not) {
return true
}
for _, s := range s.Properties {
if SchemaHasInvalidTypes(&s) {
return true
}
}
if s.AdditionalProperties != nil {
if SchemaHasInvalidTypes(s.AdditionalProperties.Schema) {
return true
}
}
for _, s := range s.PatternProperties {
if SchemaHasInvalidTypes(&s) {
return true
}
}
if s.AdditionalItems != nil {
if SchemaHasInvalidTypes(s.AdditionalItems.Schema) {
return true
}
}
for _, s := range s.Definitions {
if SchemaHasInvalidTypes(&s) {
return true
}
}
for _, d := range s.Dependencies {
if SchemaHasInvalidTypes(d.Schema) {
return true
}
}

return false
}
@@ -14,6 +14,7 @@ go_library(
deps = [
"//staging/src/k8s.io/api/autoscaling/v1:go_default_library",
"//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
"//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation:go_default_library",
"//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema:go_default_library",
"//staging/src/k8s.io/apiextensions-apiserver/pkg/client/informers/internalversion/apiextensions/internalversion:go_default_library",
"//staging/src/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/internalversion:go_default_library",
@@ -22,10 +22,11 @@ import (
"strings"
"sync"

restful "github.com/emicklei/go-restful"
"github.com/emicklei/go-restful"
"github.com/go-openapi/spec"

v1 "k8s.io/api/autoscaling/v1"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation"
structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
@@ -68,11 +69,12 @@ func BuildSwagger(crd *apiextensions.CustomResourceDefinition, version string) (
if err != nil {
return nil, err
}

if s != nil && s.OpenAPIV3Schema != nil {
ss, err := structuralschema.NewStructural(s.OpenAPIV3Schema)
if err == nil && len(structuralschema.ValidateStructural(ss, nil)) == 0 {
// skip non-structural schemas
schema = ss.Unfold()
if !validation.SchemaHasInvalidTypes(s.OpenAPIV3Schema) {
if ss, err := structuralschema.NewStructural(s.OpenAPIV3Schema); err == nil {
schema = ss.Unfold()
}
}
}

@@ -540,3 +540,87 @@ func schemaDiff(a, b *spec.Schema) string {
}
return diff.StringDiff(string(as), string(bs))
}

func TestBuildSwagger(t *testing.T) {
tests := []struct {
name string
schema string
wantedSchema string
}{
{
"nil",
"",
`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
},
{
"with properties",
`{"type":"object","properties":{"spec":{"type":"object"},"status":{"type":"object"}}}`,
`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"spec":{"type":"object"},"status":{"type":"object"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
},
{
"with invalid-typed properties",
`{"type":"object","properties":{"spec":{"type":"bug"},"status":{"type":"object"}}}`,
`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var validation *apiextensions.CustomResourceValidation
if len(tt.schema) > 0 {
v1beta1Schema := &v1beta1.JSONSchemaProps{}
if err := json.Unmarshal([]byte(tt.schema), &v1beta1Schema); err != nil {
t.Fatal(err)
}
internalSchema := &apiextensions.JSONSchemaProps{}
v1beta1.Convert_v1beta1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(v1beta1Schema, internalSchema, nil)
validation = &apiextensions.CustomResourceValidation{
OpenAPIV3Schema: internalSchema,
}
}

// TODO: mostly copied from the test above. reuse code to cleanup
got, err := BuildSwagger(&apiextensions.CustomResourceDefinition{
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "bar.k8s.io",
Version: "v1",
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "foos",
Singular: "foo",
Kind: "Foo",
ListKind: "FooList",
},
Scope: apiextensions.NamespaceScoped,
Validation: validation,
},
}, "v1")
if err != nil {
t.Fatal(err)
}

var wantedSchema spec.Schema
if err := json.Unmarshal([]byte(tt.wantedSchema), &wantedSchema); err != nil {
t.Fatal(err)
}

gotSchema := got.Definitions["io.k8s.bar.v1.Foo"]
gotProperties := properties(gotSchema.Properties)
wantedProperties := properties(wantedSchema.Properties)
if !gotProperties.Equal(wantedProperties) {
t.Fatalf("unexpected properties, got: %s, expected: %s", gotProperties.List(), wantedProperties.List())
}

// wipe out TypeMeta/ObjectMeta content, with those many lines of descriptions. We trust that they match here.
for _, metaField := range []string{"kind", "apiVersion", "metadata"} {
if _, found := gotSchema.Properties["kind"]; found {
prop := gotSchema.Properties[metaField]
prop.Description = ""
gotSchema.Properties[metaField] = prop
}
}

if !reflect.DeepEqual(&wantedSchema, &gotSchema) {
t.Errorf("unexpected schema: %s\nwant = %#v\ngot = %#v", schemaDiff(&wantedSchema, &gotSchema), &wantedSchema, &gotSchema)
}
})
}
}

0 comments on commit b6e415a

Please sign in to comment.
You can’t perform that action at this time.