✨ enable conversion webhook in webhook builder

Main changes:
 - Disable conversion webhook by default and enable only if a type
 implements convertible
 - Enhanced the test coverage to include spoke to spoke conversion

Author: droot

examples/conversion/pkg/apis/addtoscheme_jobs_v1.go

@@ -87,9 +87,9 @@ func (blder *WebhookBuilder) registerWebhooks() error {
 	blder.registerDefaultingWebhook()
 	blder.registerValidatingWebhook()
 
-	err = conversion.CheckConvertibility(blder.mgr.GetScheme(), blder.apiType)
+	err = blder.registerConversionWebhook()
 	if err != nil {
-		log.Error(err, "conversion check failed", "GVK", blder.gvk)
+		return err
 	}
 	return nil
 }
@@ -131,7 +131,26 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {
 	}
 }
 
+func (blder *WebhookBuilder) registerConversionWebhook() error {
+	ok, err := conversion.IsConvertible(blder.mgr.GetScheme(), blder.apiType)
+	if err != nil {
+		log.Error(err, "conversion check failed", "object", blder.apiType)
+		return err
+	}
+	if ok {
+		if !blder.isAlreadyHandled("/convert") {
+			blder.mgr.GetWebhookServer().Register("/convert", &conversion.Webhook{})
+		}
+		log.Info("conversion webhook enabled", "object", blder.apiType)
+	}
+
+	return nil
+}
+
 func (blder *WebhookBuilder) isAlreadyHandled(path string) bool {
+	if blder.mgr.GetWebhookServer().WebhookMux == nil {
+		return false
+	}
 	h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}})
 	if p == path && h != nil {
 		return true

examples/conversion/pkg/apis/addtoscheme_jobs_v2.go

@@ -281,13 +281,19 @@ type TestDefaulter struct {
 
 var testDefaulterGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestDefaulter"}
 
-func (*TestDefaulter) GetObjectKind() schema.ObjectKind { return nil }
+func (d *TestDefaulter) GetObjectKind() schema.ObjectKind { return d }
 func (d *TestDefaulter) DeepCopyObject() runtime.Object {
 	return &TestDefaulter{
 		Replica: d.Replica,
 	}
 }
 
+func (d *TestDefaulter) GroupVersionKind() schema.GroupVersionKind {
+	return testDefaulterGVK
+}
+
+func (d *TestDefaulter) SetGroupVersionKind(gvk schema.GroupVersionKind) {}
+
 var _ runtime.Object = &TestDefaulterList{}
 
 type TestDefaulterList struct{}
@@ -310,13 +316,19 @@ type TestValidator struct {
 
 var testValidatorGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestValidator"}
 
-func (*TestValidator) GetObjectKind() schema.ObjectKind { return nil }
+func (v *TestValidator) GetObjectKind() schema.ObjectKind { return v }
 func (v *TestValidator) DeepCopyObject() runtime.Object {
 	return &TestValidator{
 		Replica: v.Replica,
 	}
 }
 
+func (v *TestValidator) GroupVersionKind() schema.GroupVersionKind {
+	return testValidatorGVK
+}
+
+func (v *TestValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) {}
+
 var _ runtime.Object = &TestValidatorList{}
 
 type TestValidatorList struct{}
@@ -354,13 +366,19 @@ type TestDefaultValidator struct {
 
 var testDefaultValidatorGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestDefaultValidator"}
 
-func (*TestDefaultValidator) GetObjectKind() schema.ObjectKind { return nil }
+func (dv *TestDefaultValidator) GetObjectKind() schema.ObjectKind { return dv }
 func (dv *TestDefaultValidator) DeepCopyObject() runtime.Object {
 	return &TestDefaultValidator{
 		Replica: dv.Replica,
 	}
 }
 
+func (dv *TestDefaultValidator) GroupVersionKind() schema.GroupVersionKind {
+	return testDefaultValidatorGVK
+}
+
+func (dv *TestDefaultValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) {}
+
 var _ runtime.Object = &TestDefaultValidatorList{}
 
 type TestDefaultValidatorList struct{}

examples/conversion/pkg/apis/apis.go

@@ -38,7 +38,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/recorder"
 	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/conversion"
 )
 
 const (
@@ -218,7 +217,6 @@ func (cm *controllerManager) GetWebhookServer() *webhook.Server {
 			Port: cm.port,
 			Host: cm.host,
 		}
-		cm.webhookServer.Register("/convert", &conversion.Webhook{})
 		if err := cm.Add(cm.webhookServer); err != nil {
 			panic("unable to add webhookServer to the controller manager")
 		}

examples/conversion/pkg/apis/jobs/v1/doc.go

@@ -174,23 +174,24 @@ func (wh *Webhook) convertViaHub(src, dst conversion.Convertible) error {
 
 // getHub returns an instance of the Hub for passed-in object's group/kind.
 func (wh *Webhook) getHub(obj runtime.Object) (conversion.Hub, error) {
-	gvks, _, err := wh.scheme.ObjectKinds(obj)
-	if err != nil {
-		return nil, fmt.Errorf("error retriving object kinds for given object : %v", err)
+	gvks := objectGVKs(wh.scheme, obj)
+	if len(gvks) == 0 {
+		return nil, fmt.Errorf("error retrieving gvks for object : %v", obj)
 	}
 
 	var hub conversion.Hub
-	var isHub, hubFoundAlready bool
+	var hubFoundAlready bool
 	for _, gvk := range gvks {
 		instance, err := wh.scheme.New(gvk)
 		if err != nil {
 			return nil, fmt.Errorf("failed to allocate an instance for gvk %v %v", gvk, err)
 		}
-		if hub, isHub = instance.(conversion.Hub); isHub {
+		if val, isHub := instance.(conversion.Hub); isHub {
 			if hubFoundAlready {
 				return nil, fmt.Errorf("multiple hub version defined for %T", obj)
 			}
 			hubFoundAlready = true
+			hub = val
 		}
 	}
 	return hub, nil
@@ -216,21 +217,21 @@ func (wh *Webhook) allocateDstObject(apiVersion, kind string) (runtime.Object, e
 	return obj, nil
 }
 
-// CheckConvertibility determines if given type is convertible or not. For a type
+// IsConvertible determines if given type is convertible or not. For a type
 // to be convertible, the group-kind needs to have a Hub type defined and all
 // non-hub types must be able to convert to/from Hub.
-func CheckConvertibility(scheme *runtime.Scheme, obj runtime.Object) error {
+func IsConvertible(scheme *runtime.Scheme, obj runtime.Object) (bool, error) {
 	var hubs, spokes, nonSpokes []runtime.Object
 
-	gvks, _, err := scheme.ObjectKinds(obj)
-	if err != nil {
-		return fmt.Errorf("error retriving object kinds for given object : %v", err)
+	gvks := objectGVKs(scheme, obj)
+	if len(gvks) == 0 {
+		return false, fmt.Errorf("error retrieving gvks for object : %v", obj)
 	}
 
 	for _, gvk := range gvks {
 		instance, err := scheme.New(gvk)
 		if err != nil {
-			return fmt.Errorf("failed to allocate an instance for gvk %v %v", gvk, err)
+			return false, fmt.Errorf("failed to allocate an instance for gvk %v %v", gvk, err)
 		}
 
 		if isHub(instance) {
@@ -247,32 +248,45 @@ func CheckConvertibility(scheme *runtime.Scheme, obj runtime.Object) error {
 	}
 
 	if len(gvks) == 1 {
-		return nil // single version
+		return false, nil // single version
 	}
 
 	if len(hubs) == 0 && len(spokes) == 0 {
 		// multiple version detected with no conversion implementation. This is
 		// true for multi-version built-in types.
-		return nil
+		return false, nil
 	}
 
 	if len(hubs) == 1 && len(nonSpokes) == 0 { // convertible
 		spokeVersions := []string{}
 		for _, sp := range spokes {
 			spokeVersions = append(spokeVersions, sp.GetObjectKind().GroupVersionKind().String())
 		}
-		log.V(1).Info("conversion enabled for kind", "kind",
-			gvks[0].GroupKind(), "hub", hubs[0], "spokes", spokeVersions)
-		return nil
+		return true, nil
 	}
 
-	return PartialImplementationError{
+	return false, PartialImplementationError{
 		hubs:      hubs,
 		nonSpokes: nonSpokes,
 		spokes:    spokes,
 	}
 }
 
+// objectGVKs returns all (Group,Version,Kind) for the Group/Kind of given object.
+func objectGVKs(scheme *runtime.Scheme, obj runtime.Object) []schema.GroupVersionKind {
+	var gvks []schema.GroupVersionKind
+
+	objGVK := obj.GetObjectKind().GroupVersionKind()
+	knownTypes := scheme.AllKnownTypes()
+
+	for gvk := range knownTypes {
+		if objGVK.GroupKind() == gvk.GroupKind() {
+			gvks = append(gvks, gvk)
+		}
+	}
+	return gvks
+}
+
 // PartialImplementationError represents an error due to partial conversion
 // implementation such as hub without spokes, multiple hubs or spokes without hub.
 type PartialImplementationError struct {