fix apiserver crash caused by nil pointer and ensure CRD schema

validator can be constructed during validation.
This commit is contained in:
carlory 2018-01-18 14:30:53 +08:00
parent b7100f1ee7
commit 8b8d522228
8 changed files with 48 additions and 46 deletions

View File

@ -11,8 +11,8 @@ go_library(
srcs = ["validation.go"],
importpath = "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation",
deps = [
"//vendor/github.com/go-openapi/spec:go_default_library",
"//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
"//vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/validation:go_default_library",
"//vendor/k8s.io/apiextensions-apiserver/pkg/features:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",

View File

@ -20,14 +20,13 @@ import (
"fmt"
"strings"
"github.com/go-openapi/spec"
genericvalidation "k8s.io/apimachinery/pkg/api/validation"
validationutil "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
)
@ -188,6 +187,12 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(customResourceValidation.OpenAPIV3Schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema)...)
}
// if validation passed otherwise, make sure we can actually construct a schema validator from this custom resource validation.
if len(allErrs) == 0 {
if _, err := apiservervalidation.NewSchemaValidator(customResourceValidation); err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("customResourceValidation"), "", fmt.Sprintf("error building validator: %v", err)))
}
}
return allErrs
}
@ -213,17 +218,6 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), ssv)...)
}
if schema.Ref != nil {
openapiRef, err := spec.NewRef(*schema.Ref)
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("ref"), *schema.Ref, err.Error()))
}
if !openapiRef.IsValidURI() {
allErrs = append(allErrs, field.Invalid(fldPath.Child("ref"), *schema.Ref, "ref does not point to a valid URI"))
}
}
if schema.AdditionalItems != nil {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalItems.Schema, fldPath.Child("additionalItems"), ssv)...)
}
@ -318,6 +312,10 @@ func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps
allErrs = append(allErrs, field.Forbidden(fldPath.Child("dependencies"), "dependencies is not supported"))
}
if schema.Ref != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("ref"), "ref is not supported"))
}
if schema.Type == "null" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("type"), "type cannot be set to null"))
}

View File

@ -16,9 +16,6 @@ go_library(
],
importpath = "k8s.io/apiextensions-apiserver/pkg/apiserver",
deps = [
"//vendor/github.com/go-openapi/spec:go_default_library",
"//vendor/github.com/go-openapi/strfmt:go_default_library",
"//vendor/github.com/go-openapi/validate:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
"//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install:go_default_library",

View File

@ -24,9 +24,6 @@ import (
"sync/atomic"
"time"
openapispec "github.com/go-openapi/spec"
"github.com/go-openapi/strfmt"
"github.com/go-openapi/validate"
"github.com/golang/glog"
apiequality "k8s.io/apimachinery/pkg/api/equality"
@ -313,13 +310,13 @@ func (r *crdHandler) removeDeadStorage() {
}
// GetCustomResourceListerCollectionDeleter returns the ListerCollectionDeleter for
// the given uid, or nil if one does not exist.
func (r *crdHandler) GetCustomResourceListerCollectionDeleter(crd *apiextensions.CustomResourceDefinition) finalizer.ListerCollectionDeleter {
// the given uid, or nil if an error occurs.
func (r *crdHandler) GetCustomResourceListerCollectionDeleter(crd *apiextensions.CustomResourceDefinition) (finalizer.ListerCollectionDeleter, error) {
info, err := r.getOrCreateServingInfoFor(crd)
if err != nil {
utilruntime.HandleError(err)
return nil, err
}
return info.storage
return info.storage, nil
}
func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResourceDefinition) (*crdInfo, error) {
@ -354,15 +351,10 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource
}
creator := unstructuredCreator{}
// convert CRD schema to openapi schema
openapiSchema := &openapispec.Schema{}
if err := apiservervalidation.ConvertToOpenAPITypes(crd, openapiSchema); err != nil {
validator, err := apiservervalidation.NewSchemaValidator(crd.Spec.Validation)
if err != nil {
return nil, err
}
if err := openapispec.ExpandSchema(openapiSchema, nil, nil); err != nil {
return nil, err
}
validator := validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default)
storage := customresource.NewREST(
schema.GroupResource{Group: crd.Spec.Group, Resource: crd.Status.AcceptedNames.Plural},

View File

@ -12,6 +12,7 @@ go_library(
importpath = "k8s.io/apiextensions-apiserver/pkg/apiserver/validation",
deps = [
"//vendor/github.com/go-openapi/spec:go_default_library",
"//vendor/github.com/go-openapi/strfmt:go_default_library",
"//vendor/github.com/go-openapi/validate:go_default_library",
"//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
],

View File

@ -18,11 +18,24 @@ package validation
import (
"github.com/go-openapi/spec"
"github.com/go-openapi/strfmt"
"github.com/go-openapi/validate"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
)
// NewSchemaValidator creates an openapi schema validator for the given CRD validation.
func NewSchemaValidator(customResourceValidation *apiextensions.CustomResourceValidation) (*validate.SchemaValidator, error) {
// Convert CRD schema to openapi schema
openapiSchema := &spec.Schema{}
if customResourceValidation != nil {
if err := convertJSONSchemaProps(customResourceValidation.OpenAPIV3Schema, openapiSchema); err != nil {
return nil, err
}
}
return validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default), nil
}
// ValidateCustomResource validates the Custom Resource against the schema in the CustomResourceDefinition.
// CustomResource is a JSON data structure.
func ValidateCustomResource(customResource interface{}, validator *validate.SchemaValidator) error {
@ -33,16 +46,6 @@ func ValidateCustomResource(customResource interface{}, validator *validate.Sche
return nil
}
// ConvertToOpenAPITypes is used to convert internal types to go-openapi types.
func ConvertToOpenAPITypes(in *apiextensions.CustomResourceDefinition, out *spec.Schema) error {
if in.Spec.Validation != nil {
if err := convertJSONSchemaProps(in.Spec.Validation.OpenAPIV3Schema, out); err != nil {
return err
}
}
return nil
}
func convertJSONSchemaProps(in *apiextensions.JSONSchemaProps, out *spec.Schema) error {
if in == nil {
return nil

View File

@ -65,7 +65,7 @@ type ListerCollectionDeleter interface {
type CRClientGetter interface {
// GetCustomResourceListerCollectionDeleter gets the ListerCollectionDeleter for the given CRD
// UID.
GetCustomResourceListerCollectionDeleter(crd *apiextensions.CustomResourceDefinition) ListerCollectionDeleter
GetCustomResourceListerCollectionDeleter(crd *apiextensions.CustomResourceDefinition) (ListerCollectionDeleter, error)
}
// NewCRDFinalizer creates a new CRDFinalizer.
@ -155,9 +155,9 @@ func (c *CRDFinalizer) deleteInstances(crd *apiextensions.CustomResourceDefiniti
// Now we can start deleting items. While it would be ideal to use a REST API client, doing so
// could incorrectly delete a ThirdPartyResource with the same URL as the CustomResource, so we go
// directly to the storage instead. Since we control the storage, we know that delete collection works.
crClient := c.crClientGetter.GetCustomResourceListerCollectionDeleter(crd)
if crClient == nil {
err := fmt.Errorf("unable to find a custom resource client for %s.%s", crd.Status.AcceptedNames.Plural, crd.Spec.Group)
crClient, err := c.crClientGetter.GetCustomResourceListerCollectionDeleter(crd)
if err != nil {
err = fmt.Errorf("unable to find a custom resource client for %s.%s due to %v", crd.Status.AcceptedNames.Plural, crd.Spec.Group, err)
return apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Terminating,
Status: apiextensions.ConditionTrue,

View File

@ -403,16 +403,23 @@ func TestForbiddenFieldsInSchema(t *testing.T) {
t.Fatalf("unexpected non-error: uniqueItems cannot be set to true")
}
noxuDefinition.Spec.Validation.OpenAPIV3Schema.Ref = strPtr("#/definition/zeta")
noxuDefinition.Spec.Validation.OpenAPIV3Schema.Properties["zeta"] = apiextensionsv1beta1.JSONSchemaProps{
Type: "array",
UniqueItems: false,
}
_, err = testserver.CreateNewCustomResourceDefinition(noxuDefinition, apiExtensionClient, clientPool)
if err == nil {
t.Fatal("unexpected non-error: ref cannot be non-empty string")
}
noxuDefinition.Spec.Validation.OpenAPIV3Schema.Ref = nil
_, err = testserver.CreateNewCustomResourceDefinition(noxuDefinition, apiExtensionClient, clientPool)
if err != nil {
t.Fatal(err)
}
}
func float64Ptr(f float64) *float64 {
@ -422,3 +429,7 @@ func float64Ptr(f float64) *float64 {
func int64Ptr(f int64) *int64 {
return &f
}
func strPtr(str string) *string {
return &str
}