do not publish openapi for a schema containing bad types
minor rewords & refactor run script: update misc remove null from supported types
This commit is contained in:
		| @@ -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,75 @@ func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition, | ||||
| 		return field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations").Key(v1beta1.KubeAPIApprovedAnnotation), newCRD.Annotations[v1beta1.KubeAPIApprovedAnnotation], reason)} | ||||
| 	} | ||||
| } | ||||
|  | ||||
| // SchemaHasInvalidTypes returns true if it contains invalid offending openapi-v3 specification. | ||||
| 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,13 +69,14 @@ 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 | ||||
| 		if !validation.SchemaHasInvalidTypes(s.OpenAPIV3Schema) { | ||||
| 			if ss, err := structuralschema.NewStructural(s.OpenAPIV3Schema); err == nil { | ||||
| 				schema = ss.Unfold() | ||||
| 			} | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	// TODO(roycaihw): remove the WebService templating below. The following logic | ||||
| 	// comes from function registerResourceHandlers() in k8s.io/apiserver. | ||||
|   | ||||
| @@ -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) | ||||
| 			} | ||||
| 		}) | ||||
| 	} | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Haowei Cai
					Haowei Cai