Merge pull request #50546 from apelisse/plumb-openapi-validation

Automatic merge from submit-queue (batch tested with PRs 51039, 50512, 50546, 50965, 50467)

Kubectl: Plumb openapi validation (disabled by default)

**What this PR does / why we need it**: Creates a new flag '--openapi' and plumb in the validation code so that it can be used by default to validate objects against the openapi schema.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: partially https://github.com/kubernetes/kubectl/issues/49

**Special notes for your reviewer**:

This is not complete, the name of the variable must change for example.

**Release note**:
```release-note
Kubectl uses openapi for validation. If OpenAPI is not available on the server, it defaults back to the old Swagger.
```

pkg/kubectl/cmd/apply.go

@@ -187,7 +187,7 @@ func parsePruneResources(mapper meta.RESTMapper, gvks []string) ([]pruneResource
 }
 
 func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, options *ApplyOptions) error {
-	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
+	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "openapi-validation"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
 	if err != nil {
 		return err
 	}

pkg/kubectl/cmd/convert.go

@@ -128,7 +128,7 @@ func (o *ConvertOptions) Complete(f cmdutil.Factory, out io.Writer, cmd *cobra.C
 	// build the builder
 	o.builder = f.NewBuilder(!o.local)
 	if !o.local {
-		schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
+		schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "openapi-validation"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
 		if err != nil {
 			return err
 		}

pkg/kubectl/cmd/create.go

@@ -115,7 +115,7 @@ func RunCreate(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opt
 	if options.EditBeforeCreate {
 		return RunEditOnCreate(f, out, errOut, cmd, &options.FilenameOptions)
 	}
-	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
+	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "openapi-validation"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
 	if err != nil {
 		return err
 	}

pkg/kubectl/cmd/replace.go

@@ -92,7 +92,7 @@ func NewCmdReplace(f cmdutil.Factory, out io.Writer) *cobra.Command {
 }
 
 func RunReplace(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, options *resource.FilenameOptions) error {
-	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
+	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "openapi-validation"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
 	if err != nil {
 		return err
 	}
@@ -171,7 +171,7 @@ func RunReplace(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str
 }
 
 func forceReplace(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, shortOutput bool, options *resource.FilenameOptions) error {
-	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
+	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "openapi-validation"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
 	if err != nil {
 		return err
 	}

pkg/kubectl/cmd/rollingupdate.go

@@ -194,7 +194,7 @@ func RunRollingUpdate(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args
 	mapper, typer := f.Object()
 
 	if len(filename) != 0 {
-		schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
+		schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "openapi-validation"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
 		if err != nil {
 			return err
 		}

pkg/kubectl/cmd/testing/fake.go

@@ -411,7 +411,7 @@ func (f *FakeFactory) ResolveImage(name string) (string, error) {
 	return name, nil
 }
 
-func (f *FakeFactory) Validator(validate bool, cacheDir string) (validation.Schema, error) {
+func (f *FakeFactory) Validator(validate bool, openapi bool, cacheDir string) (validation.Schema, error) {
 	return f.tf.Validator, f.tf.Err
 }
 
@@ -699,7 +699,7 @@ func (f *fakeAPIFactory) AttachablePodForObject(object runtime.Object, timeout t
 	}
 }
 
-func (f *fakeAPIFactory) Validator(validate bool, cacheDir string) (validation.Schema, error) {
+func (f *fakeAPIFactory) Validator(validate bool, openapi bool, cacheDir string) (validation.Schema, error) {
 	return f.tf.Validator, f.tf.Err
 }
 

pkg/kubectl/cmd/util/BUILD

@@ -34,6 +34,7 @@ go_library(
         "//pkg/controller:go_default_library",
         "//pkg/kubectl:go_default_library",
         "//pkg/kubectl/cmd/util/openapi:go_default_library",
+        "//pkg/kubectl/cmd/util/openapi/validation:go_default_library",
         "//pkg/kubectl/plugins:go_default_library",
         "//pkg/kubectl/resource:go_default_library",
         "//pkg/kubectl/validation:go_default_library",

pkg/kubectl/cmd/util/editor/editoptions.go

@@ -229,7 +229,7 @@ func (o *EditOptions) Run() error {
 			glog.V(4).Infof("User edited:\n%s", string(edited))
 
 			// Apply validation
-			schema, err := o.f.Validator(o.EnableValidation, o.SchemaCacheDir)
+			schema, err := o.f.Validator(o.EnableValidation, o.UseOpenAPI, o.SchemaCacheDir)
 			if err != nil {
 				return preservedFile(err, file, o.ErrOut)
 			}

pkg/kubectl/cmd/util/factory.go

@@ -220,7 +220,7 @@ type ObjectMappingFactory interface {
 	AttachablePodForObject(object runtime.Object, timeout time.Duration) (*api.Pod, error)
 
 	// Returns a schema that can validate objects stored on disk.
-	Validator(validate bool, cacheDir string) (validation.Schema, error)
+	Validator(validate bool, openapi bool, cacheDir string) (validation.Schema, error)
 	// SwaggerSchema returns the schema declaration for the provided group version kind.
 	SwaggerSchema(schema.GroupVersionKind) (*swagger.ApiDeclaration, error)
 	// OpenAPISchema returns the schema openapi schema definiton

pkg/kubectl/cmd/util/factory_object_mapping.go

@@ -28,6 +28,7 @@ import (
 	"time"
 
 	swagger "github.com/emicklei/go-restful-swagger12"
+	"github.com/golang/glog"
 
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -47,6 +48,7 @@ import (
 	"k8s.io/kubernetes/pkg/controller"
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi"
+	openapivalidation "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi/validation"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/kubectl/validation"
 	"k8s.io/kubernetes/pkg/printers"
@@ -402,8 +404,20 @@ func (f *ring1Factory) AttachablePodForObject(object runtime.Object, timeout tim
 	return pod, err
 }
 
-func (f *ring1Factory) Validator(validate bool, cacheDir string) (validation.Schema, error) {
+func (f *ring1Factory) Validator(validate, openapi bool, cacheDir string) (validation.Schema, error) {
 	if validate {
+		if openapi {
+			resources, err := f.OpenAPISchema(cacheDir)
+			if err == nil {
+				return validation.ConjunctiveSchema{
+					openapivalidation.NewSchemaValidation(resources),
+					validation.NoDoubleKeySchema{},
+				}, nil
+			}
+
+			glog.Warningf("Failed to download OpenAPI (%v), falling back to swagger", err)
+		}
+
 		discovery, err := f.clientAccessFactory.DiscoveryClient()
 		if err != nil {
 			return nil, err

pkg/kubectl/cmd/util/helpers.go

@@ -395,16 +395,19 @@ func GetPodRunningTimeoutFlag(cmd *cobra.Command) (time.Duration, error) {
 func AddValidateFlags(cmd *cobra.Command) {
 	cmd.Flags().Bool("validate", true, "If true, use a schema to validate the input before sending it")
 	cmd.Flags().String("schema-cache-dir", fmt.Sprintf("~/%s/%s", clientcmd.RecommendedHomeDir, clientcmd.RecommendedSchemaName), fmt.Sprintf("If non-empty, load/store cached API schemas in this directory, default is '$HOME/%s/%s'", clientcmd.RecommendedHomeDir, clientcmd.RecommendedSchemaName))
+	cmd.Flags().Bool("openapi-validation", false, "If true, use openapi rather than swagger for validation.")
 	cmd.MarkFlagFilename("schema-cache-dir")
 }
 
 func AddValidateOptionFlags(cmd *cobra.Command, options *ValidateOptions) {
 	cmd.Flags().BoolVar(&options.EnableValidation, "validate", true, "If true, use a schema to validate the input before sending it")
 	cmd.Flags().StringVar(&options.SchemaCacheDir, "schema-cache-dir", fmt.Sprintf("~/%s/%s", clientcmd.RecommendedHomeDir, clientcmd.RecommendedSchemaName), fmt.Sprintf("If non-empty, load/store cached API schemas in this directory, default is '$HOME/%s/%s'", clientcmd.RecommendedHomeDir, clientcmd.RecommendedSchemaName))
+	cmd.Flags().BoolVar(&options.UseOpenAPI, "openapi-validation", false, "If true, use openapi rather than swagger for validation")
 	cmd.MarkFlagFilename("schema-cache-dir")
 }
 
 func AddOpenAPIFlags(cmd *cobra.Command) {
+	cmd.Flags().Bool("openapi-validation", false, "If true, use openapi rather than swagger for validation")
 	cmd.Flags().String("schema-cache-dir",
 		fmt.Sprintf("~/%s/%s", clientcmd.RecommendedHomeDir, clientcmd.RecommendedSchemaName),
 		fmt.Sprintf("If non-empty, load/store cached API schemas in this directory, default is '$HOME/%s/%s'",
@@ -448,6 +451,7 @@ func AddGeneratorFlags(cmd *cobra.Command, defaultGenerator string) {
 
 type ValidateOptions struct {
 	EnableValidation bool
+	UseOpenAPI       bool
 	SchemaCacheDir   string
 }
 

pkg/kubectl/cmd/util/openapi/validation/BUILD

@@ -17,6 +17,7 @@ go_library(
     ],
     tags = ["automanaged"],
     deps = [
+        "//pkg/api:go_default_library",
         "//pkg/api/util:go_default_library",
         "//pkg/kubectl/cmd/util/openapi:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
@@ -36,6 +37,7 @@ go_test(
     tags = ["automanaged"],
     deps = [
         ":go_default_library",
+        "//pkg/api/testapi:go_default_library",
         "//pkg/kubectl/cmd/util/openapi:go_default_library",
         "//pkg/kubectl/cmd/util/openapi/testing:go_default_library",
         "//vendor/github.com/onsi/ginkgo:go_default_library",

pkg/kubectl/cmd/util/openapi/validation/types.go

@@ -103,6 +103,9 @@ func (item *mapItem) VisitMap(schema *openapi.Map) {
 func (item *mapItem) VisitKind(schema *openapi.Kind) {
 	// Verify each sub-field.
 	for _, key := range item.sortedKeys() {
+		if item.Map[key] == nil {
+			continue
+		}
 		subItem, err := itemFactory(item.Path().FieldPath(key), item.Map[key])
 		if err != nil {
 			item.AddError(err)
@@ -118,7 +121,7 @@ func (item *mapItem) VisitKind(schema *openapi.Kind) {
 
 	// Verify that all required fields are present.
 	for _, required := range schema.RequiredFields {
-		if _, ok := item.Map[required]; !ok {
+		if v, ok := item.Map[required]; !ok || v == nil {
 			item.AddValidationError(MissingRequiredFieldError{Path: schema.GetPath().String(), Field: required})
 		}
 	}
@@ -139,7 +142,12 @@ func (item *arrayItem) VisitPrimitive(schema *openapi.Primitive) {
 
 func (item *arrayItem) VisitArray(schema *openapi.Array) {
 	for i, v := range item.Array {
-		subItem, err := itemFactory(item.Path().ArrayPath(i), v)
+		path := item.Path().ArrayPath(i)
+		if v == nil {
+			item.AddValidationError(InvalidObjectTypeError{Type: "nil", Path: path.String()})
+			continue
+		}
+		subItem, err := itemFactory(path, v)
 		if err != nil {
 			item.AddError(err)
 			continue

pkg/kubectl/cmd/util/openapi/validation/validation.go

@@ -19,11 +19,13 @@ package validation
 import (
 	"errors"
 	"fmt"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/kubernetes/pkg/api"
 	apiutil "k8s.io/kubernetes/pkg/api/util"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi"
 )
@@ -38,7 +40,7 @@ func NewSchemaValidation(resources openapi.Resources) *SchemaValidation {
 	}
 }
 
-func (v *SchemaValidation) Validate(data []byte) error {
+func (v *SchemaValidation) ValidateBytes(data []byte) error {
 	obj, err := parse(data)
 	if err != nil {
 		return err
@@ -49,21 +51,48 @@ func (v *SchemaValidation) Validate(data []byte) error {
 		return err
 	}
 
+	if strings.HasSuffix(gvk.Kind, "List") {
+		return utilerrors.NewAggregate(v.validateList(obj))
+	}
+
+	return utilerrors.NewAggregate(v.validateResource(obj, gvk))
+}
+
+func (v *SchemaValidation) validateList(object interface{}) []error {
+	fields := object.(map[string]interface{})
+	if fields == nil {
+		return []error{errors.New("invalid object to validate")}
+	}
+
+	errs := []error{}
+	for _, item := range fields["items"].([]interface{}) {
+		if gvk, err := getObjectKind(item); err != nil {
+			errs = append(errs, err)
+		} else {
+			errs = append(errs, v.validateResource(item, gvk)...)
+		}
+	}
+	return errs
+}
+
+func (v *SchemaValidation) validateResource(obj interface{}, gvk schema.GroupVersionKind) []error {
+	if !api.Registry.IsEnabledVersion(gvk.GroupVersion()) {
+		// if we don't have this in our scheme, just skip
+		// validation because its an object we don't recognize
+		return nil
+	}
+
 	resource := v.resources.LookupResource(gvk)
 	if resource == nil {
-		return fmt.Errorf("unknown object type %q", gvk)
+		return []error{fmt.Errorf("unknown object type %#v", gvk)}
 	}
 
 	rootValidation, err := itemFactory(openapi.NewPath(gvk.Kind), obj)
 	if err != nil {
-		return err
+		return []error{err}
 	}
 	resource.Accept(rootValidation)
-	errs := rootValidation.Errors()
+	return rootValidation.Errors()
-	if errs != nil {
-		return utilerrors.NewAggregate(errs)
-	}
-	return nil
 }
 
 func parse(data []byte) (interface{}, error) {
@@ -91,6 +120,7 @@ func getObjectKind(object interface{}) (schema.GroupVersionKind, error) {
 		return schema.GroupVersionKind{}, errors.New("apiVersion isn't string type")
 	}
 	version := apiutil.GetVersion(apiVersion.(string))
+	group := apiutil.GetGroup(apiVersion.(string))
 	kind := fields["kind"]
 	if kind == nil {
 		return schema.GroupVersionKind{}, errors.New("kind not set")
@@ -99,5 +129,5 @@ func getObjectKind(object interface{}) (schema.GroupVersionKind, error) {
 		return schema.GroupVersionKind{}, errors.New("kind isn't string type")
 	}
 
-	return schema.GroupVersionKind{Kind: kind.(string), Version: version}, nil
+	return schema.GroupVersionKind{Group: group, Version: version, Kind: kind.(string)}, nil
 }

pkg/kubectl/cmd/util/openapi/validation/validation_test.go

@@ -23,6 +23,8 @@ import (
 	. "github.com/onsi/gomega"
 
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	// This dependency is needed to register API types.
+	_ "k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi"
 	tst "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi/testing"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi/validation"
@@ -41,8 +43,30 @@ var _ = Describe("resource validation using OpenAPI Schema", func() {
 		Expect(validator).ToNot(BeNil())
 	})
 
+	It("finds Deployment in Schema and validates it", func() {
+		err := validator.ValidateBytes([]byte(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+metadata:
+  labels:
+    name: redis-master
+  name: name
+spec:
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        app: redis
+    spec:
+      containers:
+      - image: redis
+        name: redis
+`))
+		Expect(err).To(BeNil())
+	})
+
 	It("validates a valid pod", func() {
-		err := validator.Validate([]byte(`
+		err := validator.ValidateBytes([]byte(`
 apiVersion: v1
 kind: Pod
 metadata:
@@ -64,7 +88,7 @@ spec:
 	})
 
 	It("finds invalid command (string instead of []string) in Json Pod", func() {
-		err := validator.Validate([]byte(`
+		err := validator.ValidateBytes([]byte(`
 {
   "kind": "Pod",
   "apiVersion": "v1",
@@ -98,7 +122,7 @@ spec:
 	})
 
 	It("fails because hostPort is string instead of int", func() {
-		err := validator.Validate([]byte(`
+		err := validator.ValidateBytes([]byte(`
 {
   "kind": "Pod",
   "apiVersion": "v1",
@@ -150,7 +174,7 @@ spec:
 	})
 
 	It("fails because volume is not an array of object", func() {
-		err := validator.Validate([]byte(`
+		err := validator.ValidateBytes([]byte(`
 {
   "kind": "Pod",
   "apiVersion": "v1",
@@ -191,7 +215,7 @@ spec:
 	})
 
 	It("fails because some string lists have empty strings", func() {
-		err := validator.Validate([]byte(`
+		err := validator.ValidateBytes([]byte(`
 apiVersion: v1
 kind: Pod
 metadata:
@@ -209,14 +233,129 @@ spec:
 `))
 
 		Expect(err).To(Equal(utilerrors.NewAggregate([]error{
-			validation.InvalidObjectTypeError{
+			validation.ValidationError{
+				Path: "Pod.spec.containers[0].args",
+				Err: validation.InvalidObjectTypeError{
 					Path: "Pod.spec.containers[0].args[0]",
 					Type: "nil",
 				},
-			validation.InvalidObjectTypeError{
+			},
+			validation.ValidationError{
+				Path: "Pod.spec.containers[0].command",
+				Err: validation.InvalidObjectTypeError{
 					Path: "Pod.spec.containers[0].command[0]",
 					Type: "nil",
 				},
+			},
 		})))
 	})
+
+	It("fails if required fields are missing", func() {
+		err := validator.ValidateBytes([]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    name: redis-master
+  name: name
+spec:
+  containers:
+  - command: ["my", "command"]
+`))
+
+		Expect(err).To(Equal(utilerrors.NewAggregate([]error{
+			validation.ValidationError{
+				Path: "Pod.spec.containers[0]",
+				Err: validation.MissingRequiredFieldError{
+					Path:  "io.k8s.api.core.v1.Container",
+					Field: "name",
+				},
+			},
+			validation.ValidationError{
+				Path: "Pod.spec.containers[0]",
+				Err: validation.MissingRequiredFieldError{
+					Path:  "io.k8s.api.core.v1.Container",
+					Field: "image",
+				},
+			},
+		})))
+	})
+
+	It("fails if required fields are empty", func() {
+		err := validator.ValidateBytes([]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    name: redis-master
+  name: name
+spec:
+  containers:
+  - image:
+    name:
+`))
+
+		Expect(err).To(Equal(utilerrors.NewAggregate([]error{
+			validation.ValidationError{
+				Path: "Pod.spec.containers[0]",
+				Err: validation.MissingRequiredFieldError{
+					Path:  "io.k8s.api.core.v1.Container",
+					Field: "name",
+				},
+			},
+			validation.ValidationError{
+				Path: "Pod.spec.containers[0]",
+				Err: validation.MissingRequiredFieldError{
+					Path:  "io.k8s.api.core.v1.Container",
+					Field: "image",
+				},
+			},
+		})))
+	})
+
+	It("is fine with empty non-mandatory fields", func() {
+		err := validator.ValidateBytes([]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    name: redis-master
+  name: name
+spec:
+  containers:
+  - image: image
+    name: name
+    command:
+`))
+
+		Expect(err).To(BeNil())
+	})
+
+	It("can validate lists", func() {
+		err := validator.ValidateBytes([]byte(`
+apiVersion: v1
+kind: List
+items:
+  - apiVersion: v1
+    kind: Pod
+    metadata:
+      labels:
+        name: redis-master
+      name: name
+    spec:
+      containers:
+      - name: name
+`))
+
+		Expect(err).To(Equal(utilerrors.NewAggregate([]error{
+			validation.ValidationError{
+				Path: "Pod.spec.containers[0]",
+				Err: validation.MissingRequiredFieldError{
+					Path:  "io.k8s.api.core.v1.Container",
+					Field: "image",
+				},
+			},
+		})))
+	})
+
 })

test/integration/kubectl/kubectl_test.go

@@ -56,7 +56,7 @@ func TestKubectlValidation(t *testing.T) {
 	}
 	cmdConfig := clientcmd.NewNonInteractiveClientConfig(*cfg, "test", &overrides, nil)
 	factory := util.NewFactory(cmdConfig)
-	schema, err := factory.Validator(true, "")
+	schema, err := factory.Validator(true, true, "")
 	if err != nil {
 		t.Errorf("failed to get validator: %v", err)
 		return