Merge pull request #25172 from pwittrock/kubectl-apiversion

Automatic merge from submit-queue

Kubectl support for validating nested objects with different ApiGroups

## Pull Request Guidelines

1. Please read our [contributor guidelines](https://github.com/kubernetes/kubernetes/blob/master/CONTRIBUTING.md).
1. See our [developer guide](https://github.com/kubernetes/kubernetes/blob/master/docs/devel/development.md).
1. Follow the instructions for [labeling and writing a release note for this PR](https://github.com/kubernetes/kubernetes/blob/master/docs/devel/pull-requests.md#release-notes) in the block below.

```release-note
kubectl now supports validation of nested objects with different ApiGroups (e.g. objects in a List)
```


[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

#24089

hack/test-cmd.sh

@@ -1727,6 +1727,21 @@ __EOF__
   kubectl delete rs frontend "${kube_flags[@]}"
 
 
+  ######################
+  # Lists              #
+  ######################
+
+  kube::log::status "Testing kubectl(${version}:lists)"
+
+  ### Create a List with objects from multiple versions
+  # Command
+  kubectl create -f hack/testdata/list.yaml "${kube_flags[@]}"
+
+  ### Delete the List with objects from multiple versions
+  # Command
+  kubectl delete service/list-service-test deployment/list-deployment-test
+
+
   ######################
   # Multiple Resources #
   ######################

hack/testdata/list.yaml

@@ -0,0 +1,29 @@
+apiVersion: v1
+kind: List
+items:
+- apiVersion: v1
+  kind: Service
+  metadata:
+    name: list-service-test
+  spec:
+    ports:
+    - protocol: TCP
+      port: 80
+    selector:
+      app: list-deployment-test
+- apiVersion: extensions/v1beta1
+  kind: Deployment
+  metadata:
+    name: list-deployment-test
+    labels:
+      app: list-deployment-test
+  spec:
+    replicas: 1
+    template:
+      metadata:
+        labels:
+          app: list-deployment-test
+      spec:
+        containers:
+          - name: nginx
+            image: nginx

pkg/api/testing/fuzzer.go

@@ -413,6 +413,37 @@ func FuzzerFor(t *testing.T, version unversioned.GroupVersion, src rand.Source)
 				}
 			}
 		},
+		func(r *runtime.RawExtension, c fuzz.Continue) {
+			// Pick an arbitrary type and fuzz it
+			types := []runtime.Object{&api.Pod{}, &extensions.Deployment{}, &api.Service{}}
+			obj := types[c.Rand.Intn(len(types))]
+			c.Fuzz(obj)
+
+			// Find a codec for converting the object to raw bytes.  This is necessary for the
+			// api version and kind to be correctly set be serialization.
+			var codec runtime.Codec
+			switch obj.(type) {
+			case *api.Pod:
+				codec = testapi.Default.Codec()
+			case *extensions.Deployment:
+				codec = testapi.Extensions.Codec()
+			case *api.Service:
+				codec = testapi.Default.Codec()
+			default:
+				t.Errorf("Failed to find codec for object type: %T", obj)
+				return
+			}
+
+			// Convert the object to raw bytes
+			bytes, err := runtime.Encode(codec, obj)
+			if err != nil {
+				t.Errorf("Failed to encode object: %v", err)
+				return
+			}
+
+			// Set the bytes field on the RawExtension
+			r.Raw = bytes
+		},
 	)
 	return f
 }

pkg/api/validation/schema.go

@@ -26,6 +26,7 @@ import (
 	"github.com/emicklei/go-restful/swagger"
 	"github.com/golang/glog"
 	apiutil "k8s.io/kubernetes/pkg/api/util"
+	"k8s.io/kubernetes/pkg/runtime"
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 	"k8s.io/kubernetes/pkg/util/yaml"
 )
@@ -62,15 +63,17 @@ type NullSchema struct{}
 func (NullSchema) ValidateBytes(data []byte) error { return nil }
 
 type SwaggerSchema struct {
-	api swagger.ApiDeclaration
+	api      swagger.ApiDeclaration
+	delegate Schema // For delegating to other api groups
 }
 
-func NewSwaggerSchemaFromBytes(data []byte) (Schema, error) {
+func NewSwaggerSchemaFromBytes(data []byte, factory Schema) (Schema, error) {
 	schema := &SwaggerSchema{}
 	err := json.Unmarshal(data, &schema.api)
 	if err != nil {
 		return nil, err
 	}
+	schema.delegate = factory
 	return schema, nil
 }
 
@@ -78,11 +81,15 @@ func NewSwaggerSchemaFromBytes(data []byte) (Schema, error) {
 // It return nil if every item is ok.
 // Otherwise it return an error list contain errors of every item.
 func (s *SwaggerSchema) validateList(obj map[string]interface{}) []error {
-	allErrs := []error{}
 	items, exists := obj["items"]
 	if !exists {
-		return append(allErrs, fmt.Errorf("no items field in %#v", obj))
+		return []error{fmt.Errorf("no items field in %#v", obj)}
 	}
+	return s.validateItems(items)
+}
+
+func (s *SwaggerSchema) validateItems(items interface{}) []error {
+	allErrs := []error{}
 	itemList, ok := items.([]interface{})
 	if !ok {
 		return append(allErrs, fmt.Errorf("items isn't a slice"))
@@ -125,6 +132,7 @@ func (s *SwaggerSchema) validateList(obj map[string]interface{}) []error {
 			allErrs = append(allErrs, errs...)
 		}
 	}
+
 	return allErrs
 }
 
@@ -171,6 +179,25 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName stri
 	allErrs := []error{}
 	models := s.api.Models
 	model, ok := models.At(typeName)
+
+	// Verify the api version matches.  This is required for nested types with differing api versions because
+	// s.api only has schema for 1 api version (the parent object type's version).
+	// e.g. an extensions/v1beta1 Template embedding a /v1 Service requires the schema for the extensions/v1beta1
+	// api to delegate to the schema for the /v1 api.
+	// Only do this for !ok objects so that cross ApiVersion vendored types take precedence.
+	if !ok && s.delegate != nil {
+		fields, mapOk := obj.(map[string]interface{})
+		if !mapOk {
+			return append(allErrs, fmt.Errorf("field %s: expected object of type map[string]interface{}, but the actual type is %T", fieldName, obj))
+		}
+		if delegated, err := s.delegateIfDifferentApiVersion(runtime.Unstructured{Object: fields}); delegated {
+			if err != nil {
+				allErrs = append(allErrs, err)
+			}
+			return allErrs
+		}
+	}
+
 	if !ok {
 		return append(allErrs, TypeNotFoundError(typeName))
 	}
@@ -194,6 +221,17 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName stri
 	}
 	for key, value := range fields {
 		details, ok := properties.At(key)
+
+		// Special case for runtime.RawExtension and runtime.Objects because they always fail to validate
+		// This is because the actual values will be of some sub-type (e.g. Deployment) not the expected
+		// super-type (RawExtention)
+		if s.isGenericArray(details) {
+			errs := s.validateItems(value)
+			if len(errs) > 0 {
+				allErrs = append(allErrs, errs...)
+			}
+			continue
+		}
 		if !ok {
 			allErrs = append(allErrs, fmt.Errorf("found invalid field %s for %s", key, typeName))
 			continue
@@ -219,6 +257,42 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName stri
 	return allErrs
 }
 
+// delegateIfDifferentApiVersion delegates the validation of an object if its ApiGroup does not match the
+// current SwaggerSchema.
+// First return value is true if the validation was delegated (by a different ApiGroup SwaggerSchema)
+// Second return value is the result of the delegated validation if performed.
+func (s *SwaggerSchema) delegateIfDifferentApiVersion(obj runtime.Unstructured) (bool, error) {
+	// Never delegate objects in the same ApiVersion or we will get infinite recursion
+	if !s.isDifferentApiVersion(obj) {
+		return false, nil
+	}
+
+	// Convert the object back into bytes so that we can pass it to the ValidateBytes function
+	m, err := json.Marshal(obj.Object)
+	if err != nil {
+		return true, err
+	}
+
+	// Delegate validation of this object to the correct SwaggerSchema for its ApiGroup
+	return true, s.delegate.ValidateBytes(m)
+}
+
+// isDifferentApiVersion Returns true if obj lives in a different ApiVersion than the SwaggerSchema does.
+// The SwaggerSchema will not be able to process objects in different ApiVersions unless they are vendored.
+func (s *SwaggerSchema) isDifferentApiVersion(obj runtime.Unstructured) bool {
+	groupVersion := obj.GetAPIVersion()
+	return len(groupVersion) > 0 && s.api.ApiVersion != groupVersion
+}
+
+// isGenericArray Returns true if p is an array of generic Objects - either RawExtension or Object.
+func (s *SwaggerSchema) isGenericArray(p swagger.ModelProperty) bool {
+	return p.DataTypeFields.Type != nil &&
+		*p.DataTypeFields.Type == "array" &&
+		p.Items != nil &&
+		p.Items.Ref != nil &&
+		(*p.Items.Ref == "runtime.RawExtension" || *p.Items.Ref == "runtime.Object")
+}
+
 // This matches type name in the swagger spec, such as "v1.Binding".
 var versionRegexp = regexp.MustCompile(`^v.+\..*`)
 

pkg/api/validation/schema_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package validation
 
 import (
+	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"math/rand"
 	"strings"
@@ -25,7 +27,9 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/testapi"
 	apitesting "k8s.io/kubernetes/pkg/api/testing"
+	"k8s.io/kubernetes/pkg/apis/extensions"
 	"k8s.io/kubernetes/pkg/runtime"
+	k8syaml "k8s.io/kubernetes/pkg/util/yaml"
 
 	"github.com/ghodss/yaml"
 )
@@ -39,17 +43,86 @@ func readPod(filename string) ([]byte, error) {
 }
 
 func readSwaggerFile() ([]byte, error) {
-	// TODO this path is broken
-	pathToSwaggerSpec := "../../../api/swagger-spec/" + testapi.Default.GroupVersion().Version + ".json"
+	return readSwaggerApiFile(testapi.Default)
+}
+
+func readSwaggerApiFile(group testapi.TestGroup) ([]byte, error) {
+	// TODO: Figure out a better way of finding these files
+	var pathToSwaggerSpec string
+	if group.GroupVersion().Group == "" {
+		pathToSwaggerSpec = "../../../api/swagger-spec/" + group.GroupVersion().Version + ".json"
+	} else {
+		pathToSwaggerSpec = "../../../api/swagger-spec/" + group.GroupVersion().Group + "_" + group.GroupVersion().Version + ".json"
+	}
+
 	return ioutil.ReadFile(pathToSwaggerSpec)
 }
 
+// Mock delegating Schema.  Not a full fake impl.
+type Factory struct {
+	defaultSchema    Schema
+	extensionsSchema Schema
+}
+
+var _ Schema = &Factory{}
+
+// TODO: Consider using a mocking library instead or fully fleshing this out into a fake impl and putting it in some
+// generally available location
+func (f *Factory) ValidateBytes(data []byte) error {
+	var obj interface{}
+	out, err := k8syaml.ToJSON(data)
+	if err != nil {
+		return err
+	}
+	data = out
+	if err := json.Unmarshal(data, &obj); err != nil {
+		return err
+	}
+	fields, ok := obj.(map[string]interface{})
+	if !ok {
+		return fmt.Errorf("error in unmarshaling data %s", string(data))
+	}
+	// Note: This only supports the 2 api versions we expect from the test it is currently supporting.
+	groupVersion := fields["apiVersion"]
+	switch groupVersion {
+	case "v1":
+		return f.defaultSchema.ValidateBytes(data)
+	case "extensions/v1beta1":
+		return f.extensionsSchema.ValidateBytes(data)
+	default:
+		return fmt.Errorf("Unsupported API version %s", groupVersion)
+	}
+}
+
 func loadSchemaForTest() (Schema, error) {
 	data, err := readSwaggerFile()
 	if err != nil {
 		return nil, err
 	}
-	return NewSwaggerSchemaFromBytes(data)
+	return NewSwaggerSchemaFromBytes(data, nil)
+}
+
+func loadSchemaForTestWithFactory(group testapi.TestGroup, factory Schema) (Schema, error) {
+	data, err := readSwaggerApiFile(group)
+	if err != nil {
+		return nil, err
+	}
+	return NewSwaggerSchemaFromBytes(data, factory)
+}
+
+func NewFactory() (*Factory, error) {
+	f := &Factory{}
+	defaultSchema, err := loadSchemaForTestWithFactory(testapi.Default, f)
+	if err != nil {
+		return nil, err
+	}
+	f.defaultSchema = defaultSchema
+	extensionSchema, err := loadSchemaForTestWithFactory(testapi.Extensions, f)
+	if err != nil {
+		return nil, err
+	}
+	f.extensionsSchema = extensionSchema
+	return f, nil
 }
 
 func TestLoad(t *testing.T) {
@@ -91,6 +164,42 @@ func TestValidateOk(t *testing.T) {
 	}
 }
 
+func TestValidateDifferentApiVersions(t *testing.T) {
+	schema, err := loadSchemaForTest()
+	if err != nil {
+		t.Errorf("Failed to load: %v", err)
+	}
+
+	pod := &api.Pod{}
+	pod.APIVersion = "v1"
+	pod.Kind = "Pod"
+
+	deployment := &extensions.Deployment{}
+	deployment.APIVersion = "extensions/v1beta1"
+	deployment.Kind = "Deployment"
+
+	list := &api.List{}
+	list.APIVersion = "v1"
+	list.Kind = "List"
+	list.Items = []runtime.Object{pod, deployment}
+	bytes, err := json.Marshal(list)
+	if err != nil {
+		t.Error(err)
+	}
+	err = schema.ValidateBytes(bytes)
+	if err == nil {
+		t.Error(fmt.Errorf("Expected error when validating different api version and no delegate exists."))
+	}
+	f, err := NewFactory()
+	if err != nil {
+		t.Error(fmt.Errorf("Failed to create Schema factory %v.", err))
+	}
+	err = f.ValidateBytes(bytes)
+	if err != nil {
+		t.Error(fmt.Errorf("Failed to validate object with multiple ApiGroups: %v.", err))
+	}
+}
+
 func TestInvalid(t *testing.T) {
 	schema, err := loadSchemaForTest()
 	if err != nil {
@@ -171,7 +280,7 @@ func TestTypeOAny(t *testing.T) {
 	}
 	// Replace type: "any" in the spec by type: "object" and verify that the validation still passes.
 	newData := strings.Replace(string(data), `"type": "object"`, `"type": "any"`, -1)
-	schema, err := NewSwaggerSchemaFromBytes([]byte(newData))
+	schema, err := NewSwaggerSchemaFromBytes([]byte(newData), nil)
 	if err != nil {
 		t.Errorf("Failed to load: %v", err)
 	}

pkg/kubectl/cmd/util/factory.go

@@ -815,7 +815,7 @@ func writeSchemaFile(schemaData []byte, cacheDir, cacheFile, prefix, groupVersio
 	return nil
 }
 
-func getSchemaAndValidate(c schemaClient, data []byte, prefix, groupVersion, cacheDir string) (err error) {
+func getSchemaAndValidate(c schemaClient, data []byte, prefix, groupVersion, cacheDir string, delegate validation.Schema) (err error) {
 	var schemaData []byte
 	var firstSeen bool
 	fullDir, err := substituteUserHome(cacheDir)
@@ -836,7 +836,7 @@ func getSchemaAndValidate(c schemaClient, data []byte, prefix, groupVersion, cac
 			return err
 		}
 	}
-	schema, err := validation.NewSwaggerSchemaFromBytes(schemaData)
+	schema, err := validation.NewSwaggerSchemaFromBytes(schemaData, delegate)
 	if err != nil {
 		return err
 	}
@@ -849,7 +849,7 @@ func getSchemaAndValidate(c schemaClient, data []byte, prefix, groupVersion, cac
 		if err != nil {
 			return err
 		}
-		schema, err := validation.NewSwaggerSchemaFromBytes(schemaData)
+		schema, err := validation.NewSwaggerSchemaFromBytes(schemaData, delegate)
 		if err != nil {
 			return err
 		}
@@ -888,20 +888,20 @@ func (c *clientSwaggerSchema) ValidateBytes(data []byte) error {
 		if c.c.AutoscalingClient == nil {
 			return errors.New("unable to validate: no autoscaling client")
 		}
-		return getSchemaAndValidate(c.c.AutoscalingClient.RESTClient, data, "apis/", gvk.GroupVersion().String(), c.cacheDir)
+		return getSchemaAndValidate(c.c.AutoscalingClient.RESTClient, data, "apis/", gvk.GroupVersion().String(), c.cacheDir, c)
 	}
 	if gvk.Group == apps.GroupName {
 		if c.c.AppsClient == nil {
 			return errors.New("unable to validate: no autoscaling client")
 		}
-		return getSchemaAndValidate(c.c.AppsClient.RESTClient, data, "apis/", gvk.GroupVersion().String(), c.cacheDir)
+		return getSchemaAndValidate(c.c.AppsClient.RESTClient, data, "apis/", gvk.GroupVersion().String(), c.cacheDir, c)
 	}
 
 	if gvk.Group == batch.GroupName {
 		if c.c.BatchClient == nil {
 			return errors.New("unable to validate: no batch client")
 		}
-		return getSchemaAndValidate(c.c.BatchClient.RESTClient, data, "apis/", gvk.GroupVersion().String(), c.cacheDir)
+		return getSchemaAndValidate(c.c.BatchClient.RESTClient, data, "apis/", gvk.GroupVersion().String(), c.cacheDir, c)
 	}
 	if registered.IsThirdPartyAPIGroupVersion(gvk.GroupVersion()) {
 		// Don't attempt to validate third party objects
@@ -911,9 +911,9 @@ func (c *clientSwaggerSchema) ValidateBytes(data []byte) error {
 		if c.c.ExtensionsClient == nil {
 			return errors.New("unable to validate: no experimental client")
 		}
-		return getSchemaAndValidate(c.c.ExtensionsClient.RESTClient, data, "apis/", gvk.GroupVersion().String(), c.cacheDir)
+		return getSchemaAndValidate(c.c.ExtensionsClient.RESTClient, data, "apis/", gvk.GroupVersion().String(), c.cacheDir, c)
 	}
-	return getSchemaAndValidate(c.c.RESTClient, data, "api", gvk.GroupVersion().String(), c.cacheDir)
+	return getSchemaAndValidate(c.c.RESTClient, data, "api", gvk.GroupVersion().String(), c.cacheDir, c)
 }
 
 // DefaultClientConfig creates a clientcmd.ClientConfig with the following hierarchy:

pkg/kubectl/cmd/util/factory_test.go

@@ -198,7 +198,7 @@ func loadSchemaForTest() (validation.Schema, error) {
 	if err != nil {
 		return nil, err
 	}
-	return validation.NewSwaggerSchemaFromBytes(data)
+	return validation.NewSwaggerSchemaFromBytes(data, nil)
 }
 
 func TestRefetchSchemaWhenValidationFails(t *testing.T) {
@@ -250,7 +250,7 @@ func TestRefetchSchemaWhenValidationFails(t *testing.T) {
 	}
 
 	// Re-get request, should use HTTP and write
-	if getSchemaAndValidate(c, data, "foo", "bar", dir); err != nil {
+	if getSchemaAndValidate(c, data, "foo", "bar", dir, nil); err != nil {
 		t.Errorf("unexpected error validating: %v", err)
 	}
 	if requests["/swaggerapi/foo/bar"] != 1 {
@@ -295,7 +295,7 @@ func TestValidateCachesSchema(t *testing.T) {
 	}
 
 	// Initial request, should use HTTP and write
-	if getSchemaAndValidate(c, data, "foo", "bar", dir); err != nil {
+	if getSchemaAndValidate(c, data, "foo", "bar", dir, nil); err != nil {
 		t.Errorf("unexpected error validating: %v", err)
 	}
 	if _, err := os.Stat(path.Join(dir, "foo", "bar", schemaFileName)); err != nil {
@@ -306,7 +306,7 @@ func TestValidateCachesSchema(t *testing.T) {
 	}
 
 	// Same version and group, should skip HTTP
-	if getSchemaAndValidate(c, data, "foo", "bar", dir); err != nil {
+	if getSchemaAndValidate(c, data, "foo", "bar", dir, nil); err != nil {
 		t.Errorf("unexpected error validating: %v", err)
 	}
 	if requests["/swaggerapi/foo/bar"] != 2 {
@@ -314,7 +314,7 @@ func TestValidateCachesSchema(t *testing.T) {
 	}
 
 	// Different API group, should go to HTTP and write
-	if getSchemaAndValidate(c, data, "foo", "baz", dir); err != nil {
+	if getSchemaAndValidate(c, data, "foo", "baz", dir, nil); err != nil {
 		t.Errorf("unexpected error validating: %v", err)
 	}
 	if _, err := os.Stat(path.Join(dir, "foo", "baz", schemaFileName)); err != nil {
@@ -325,7 +325,7 @@ func TestValidateCachesSchema(t *testing.T) {
 	}
 
 	// Different version, should go to HTTP and write
-	if getSchemaAndValidate(c, data, "foo2", "bar", dir); err != nil {
+	if getSchemaAndValidate(c, data, "foo2", "bar", dir, nil); err != nil {
 		t.Errorf("unexpected error validating: %v", err)
 	}
 	if _, err := os.Stat(path.Join(dir, "foo2", "bar", schemaFileName)); err != nil {
@@ -336,7 +336,7 @@ func TestValidateCachesSchema(t *testing.T) {
 	}
 
 	// No cache dir, should go straight to HTTP and not write
-	if getSchemaAndValidate(c, data, "foo", "blah", ""); err != nil {
+	if getSchemaAndValidate(c, data, "foo", "blah", "", nil); err != nil {
 		t.Errorf("unexpected error validating: %v", err)
 	}
 	if requests["/swaggerapi/foo/blah"] != 1 {