CRD versioning with no-op converter

Author: mbohlool

pkg/master/controller/crdregistration/crdregistration_controller.go

@@ -77,9 +77,11 @@ func NewAutoRegistrationController(crdinformer crdinformers.CustomResourceDefini
 			cast := obj.(*apiextensions.CustomResourceDefinition)
 			c.enqueueCRD(cast)
 		},
-		UpdateFunc: func(_, obj interface{}) {
-			cast := obj.(*apiextensions.CustomResourceDefinition)
-			c.enqueueCRD(cast)
+		UpdateFunc: func(oldObj, newObj interface{}) {
+			// Enqueue both old and new object to make sure we remove and add appropriate API services.
+			// The working queue will resolve any duplicates and only changes will stay in the queue.
+			c.enqueueCRD(oldObj.(*apiextensions.CustomResourceDefinition))
+			c.enqueueCRD(newObj.(*apiextensions.CustomResourceDefinition))
 		},
 		DeleteFunc: func(obj interface{}) {
 			cast, ok := obj.(*apiextensions.CustomResourceDefinition)
@@ -120,8 +122,10 @@ func (c *crdRegistrationController) Run(threadiness int, stopCh <-chan struct{})
 		utilruntime.HandleError(err)
 	} else {
 		for _, crd := range crds {
-			if err := c.syncHandler(schema.GroupVersion{Group: crd.Spec.Group, Version: crd.Spec.Version}); err != nil {
-				utilruntime.HandleError(err)
+			for _, version := range crd.Spec.Versions {
+				if err := c.syncHandler(schema.GroupVersion{Group: crd.Spec.Group, Version: version.Name}); err != nil {
+					utilruntime.HandleError(err)
+				}
 			}
 		}
 	}
@@ -182,11 +186,12 @@ func (c *crdRegistrationController) processNextWorkItem() bool {
 }
 
 func (c *crdRegistrationController) enqueueCRD(crd *apiextensions.CustomResourceDefinition) {
-	c.queue.Add(schema.GroupVersion{Group: crd.Spec.Group, Version: crd.Spec.Version})
+	for _, version := range crd.Spec.Versions {
+		c.queue.Add(schema.GroupVersion{Group: crd.Spec.Group, Version: version.Name})
+	}
 }
 
 func (c *crdRegistrationController) handleVersionUpdate(groupVersion schema.GroupVersion) error {
-	found := false
 	apiServiceName := groupVersion.Version + "." + groupVersion.Group
 
 	// check all CRDs.  There shouldn't that many, but if we have problems later we can index them
@@ -195,26 +200,27 @@ func (c *crdRegistrationController) handleVersionUpdate(groupVersion schema.Grou
 		return err
 	}
 	for _, crd := range crds {
-		if crd.Spec.Version == groupVersion.Version && crd.Spec.Group == groupVersion.Group {
-			found = true
-			break
+		if crd.Spec.Group != groupVersion.Group {
+			continue
 		}
-	}
+		for _, version := range crd.Spec.Versions {
+			if version.Name != groupVersion.Version || !version.Served {
+				continue
+			}
 
-	if !found {
-		c.apiServiceRegistration.RemoveAPIServiceToSync(apiServiceName)
-		return nil
+			c.apiServiceRegistration.AddAPIServiceToSync(&apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: apiServiceName},
+				Spec: apiregistration.APIServiceSpec{
+					Group:                groupVersion.Group,
+					Version:              groupVersion.Version,
+					GroupPriorityMinimum: 1000, // CRDs should have relatively low priority
+					VersionPriority:      100,  // CRDs will be sorted by kube-like versions like any other APIService with the same VersionPriority
+				},
+			})
+			return nil
+		}
 	}
 
-	c.apiServiceRegistration.AddAPIServiceToSync(&apiregistration.APIService{
-		ObjectMeta: metav1.ObjectMeta{Name: apiServiceName},
-		Spec: apiregistration.APIServiceSpec{
-			Group:                groupVersion.Group,
-			Version:              groupVersion.Version,
-			GroupPriorityMinimum: 1000, // CRDs should have relatively low priority
-			VersionPriority:      100,  // CRDs should have relatively low priority
-		},
-	})
-
+	c.apiServiceRegistration.RemoveAPIServiceToSync(apiServiceName)
 	return nil
 }

pkg/master/controller/crdregistration/crdregistration_controller_test.go

@@ -42,8 +42,16 @@ func TestHandleVersionUpdate(t *testing.T) {
 			startingCRDs: []*apiextensions.CustomResourceDefinition{
 				{
 					Spec: apiextensions.CustomResourceDefinitionSpec{
-						Group:   "group.com",
-						Version: "v1",
+						Group: "group.com",
+						// Version field is deprecated and crd registration won't rely on it at all.
+						// defaulting route will fill up Versions field if user only provided version field.
+						Versions: []apiextensions.CustomResourceDefinitionVersion{
+							{
+								Name:    "v1",
+								Served:  true,
+								Storage: true,
+							},
+						},
 					},
 				},
 			},
@@ -66,8 +74,14 @@ func TestHandleVersionUpdate(t *testing.T) {
 			startingCRDs: []*apiextensions.CustomResourceDefinition{
 				{
 					Spec: apiextensions.CustomResourceDefinitionSpec{
-						Group:   "group.com",
-						Version: "v1",
+						Group: "group.com",
+						Versions: []apiextensions.CustomResourceDefinitionVersion{
+							{
+								Name:    "v1",
+								Served:  true,
+								Storage: true,
+							},
+						},
 					},
 				},
 			},
@@ -98,7 +112,6 @@ func TestHandleVersionUpdate(t *testing.T) {
 			t.Errorf("%s expected %v, got %v", test.name, test.expectedRemoved, registration.removed)
 		}
 	}
-
 }
 
 type fakeAPIServiceRegistration struct {

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go

@@ -42,6 +42,28 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
 			if len(obj.Names.ListKind) == 0 && len(obj.Names.Kind) > 0 {
 				obj.Names.ListKind = obj.Names.Kind + "List"
 			}
+			if len(obj.Versions) == 0 && len(obj.Version) != 0 {
+				obj.Versions = []apiextensions.CustomResourceDefinitionVersion{
+					{
+						Name:    obj.Version,
+						Served:  true,
+						Storage: true,
+					},
+				}
+			} else if len(obj.Versions) != 0 {
+				obj.Version = obj.Versions[0].Name
+			}
+		},
+		func(obj *apiextensions.CustomResourceDefinition, c fuzz.Continue) {
+			c.FuzzNoCustom(obj)
+
+			if len(obj.Status.StoredVersions) == 0 {
+				for _, v := range obj.Spec.Versions {
+					if v.Storage && !apiextensions.IsStoredVersion(obj, v.Name) {
+						obj.Status.StoredVersions = append(obj.Status.StoredVersions, v.Name)
+					}
+				}
+			}
 		},
 		func(obj *apiextensions.JSONSchemaProps, c fuzz.Continue) {
 			// we cannot use c.FuzzNoCustom because of the interface{} fields. So let's loop with reflection.

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go

@@ -0,0 +1,67 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package conversion
+
+import (
+	"fmt"
+
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+// NewCRDConverter returns a new CRD converter based on the conversion settings in crd object.
+func NewCRDConverter(crd *apiextensions.CustomResourceDefinition) (safe, unsafe runtime.ObjectConvertor) {
+	validVersions := map[schema.GroupVersion]bool{}
+	for _, version := range crd.Spec.Versions {
+		validVersions[schema.GroupVersion{Group: crd.Spec.Group, Version: version.Name}] = true
+	}
+
+	// The only converter right now is nopConverter. More converters will be returned based on the
+	// CRD object when they introduced.
+	unsafe = &nopConverter{
+		clusterScoped: crd.Spec.Scope == apiextensions.ClusterScoped,
+		validVersions: validVersions,
+	}
+	return &safeConverterWrapper{unsafe}, unsafe
+}
+
+// safeConverterWrapper is a wrapper over an unsafe object converter that makes copy of the input and then delegate to the unsafe converter.
+type safeConverterWrapper struct {
+	unsafe runtime.ObjectConvertor
+}
+
+var _ runtime.ObjectConvertor = &nopConverter{}
+
+// ConvertFieldLabel delegate the call to the unsafe converter.
+func (c *safeConverterWrapper) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+	return c.unsafe.ConvertFieldLabel(version, kind, label, value)
+}
+
+// Convert makes a copy of in object and then delegate the call to the unsafe converter.
+func (c *safeConverterWrapper) Convert(in, out, context interface{}) error {
+	inObject, ok := in.(runtime.Object)
+	if !ok {
+		return fmt.Errorf("input type %T in not valid for object conversion", in)
+	}
+	return c.unsafe.Convert(inObject.DeepCopyObject(), out, context)
+}
+
+// ConvertToVersion makes a copy of in object and then delegate the call to the unsafe converter.
+func (c *safeConverterWrapper) ConvertToVersion(in runtime.Object, target runtime.GroupVersioner) (runtime.Object, error) {
+	return c.unsafe.ConvertToVersion(in.DeepCopyObject(), target)
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/nop_converter.go

@@ -0,0 +1,100 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package conversion
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+// nopConverter is a converter that only sets the apiVersion fields, but does not real conversion. It supports fields selectors.
+type nopConverter struct {
+	clusterScoped bool
+	validVersions map[schema.GroupVersion]bool
+}
+
+var _ runtime.ObjectConvertor = &nopConverter{}
+
+func (c *nopConverter) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+	// We currently only support metadata.namespace and metadata.name.
+	switch {
+	case label == "metadata.name":
+		return label, value, nil
+	case !c.clusterScoped && label == "metadata.namespace":
+		return label, value, nil
+	default:
+		return "", "", fmt.Errorf("field label not supported: %s", label)
+	}
+}
+
+func (c *nopConverter) Convert(in, out, context interface{}) error {
+	unstructIn, ok := in.(*unstructured.Unstructured)
+	if !ok {
+		return fmt.Errorf("input type %T in not valid for unstructured conversion", in)
+	}
+
+	unstructOut, ok := out.(*unstructured.Unstructured)
+	if !ok {
+		return fmt.Errorf("output type %T in not valid for unstructured conversion", out)
+	}
+
+	outGVK := unstructOut.GroupVersionKind()
+	if !c.validVersions[outGVK.GroupVersion()] {
+		return fmt.Errorf("request to convert CRD from an invalid group/version: %s", outGVK.String())
+	}
+	inGVK := unstructIn.GroupVersionKind()
+	if !c.validVersions[inGVK.GroupVersion()] {
+		return fmt.Errorf("request to convert CRD to an invalid group/version: %s", inGVK.String())
+	}
+
+	unstructOut.SetUnstructuredContent(unstructIn.UnstructuredContent())
+	_, err := c.ConvertToVersion(unstructOut, outGVK.GroupVersion())
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func (c *nopConverter) convertToVersion(in runtime.Object, target runtime.GroupVersioner) error {
+	kind := in.GetObjectKind().GroupVersionKind()
+	gvk, ok := target.KindForGroupVersionKinds([]schema.GroupVersionKind{kind})
+	if !ok {
+		// TODO: should this be a typed error?
+		return fmt.Errorf("%v is unstructured and is not suitable for converting to %q", kind, target)
+	}
+	if !c.validVersions[gvk.GroupVersion()] {
+		return fmt.Errorf("request to convert CRD to an invalid group/version: %s", gvk.String())
+	}
+	in.GetObjectKind().SetGroupVersionKind(gvk)
+	return nil
+}
+
+// ConvertToVersion converts in object to the given gvk in place and returns the same `in` object.
+func (c *nopConverter) ConvertToVersion(in runtime.Object, target runtime.GroupVersioner) (runtime.Object, error) {
+	var err error
+	// Run the converter on the list items instead of list itself
+	if list, ok := in.(*unstructured.UnstructuredList); ok {
+		err = list.EachListItem(func(item runtime.Object) error {
+			return c.convertToVersion(item, target)
+		})
+	}
+	err = c.convertToVersion(in, target)
+	return in, err
+}