From 8da12a65ee99c3723b2c1b818067d4d13297596a Mon Sep 17 00:00:00 2001 From: droot Date: Tue, 14 May 2019 12:39:35 -0700 Subject: [PATCH] :sparkles: enable conversion webhook by default --- .../conversion/pkg/apis/jobs/v1/register.go | 4 +- .../conversion/pkg/apis/jobs/v2/register.go | 4 +- pkg/builder/build.go | 7 +- pkg/manager/internal.go | 2 + pkg/webhook/conversion/conversion.go | 81 +++++++++++++++++++ pkg/webhook/conversion/conversion_test.go | 36 +++++++++ 6 files changed, 129 insertions(+), 5 deletions(-) diff --git a/examples/conversion/pkg/apis/jobs/v1/register.go b/examples/conversion/pkg/apis/jobs/v1/register.go index 3afa816eb6..8181537beb 100644 --- a/examples/conversion/pkg/apis/jobs/v1/register.go +++ b/examples/conversion/pkg/apis/jobs/v1/register.go @@ -25,7 +25,7 @@ package v1 import ( "k8s.io/apimachinery/pkg/runtime/schema" - controllers "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/scheme" ) var ( @@ -33,7 +33,7 @@ var ( SchemeGroupVersion = schema.GroupVersion{Group: "jobs.example.org", Version: "v1"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme - SchemeBuilder = &controllers.SchemeBuilder{GroupVersion: SchemeGroupVersion} + SchemeBuilder = &scheme.Builder{GroupVersion: SchemeGroupVersion} // AddToScheme is required by pkg/client/... AddToScheme = SchemeBuilder.AddToScheme diff --git a/examples/conversion/pkg/apis/jobs/v2/register.go b/examples/conversion/pkg/apis/jobs/v2/register.go index ce4a2d3ba5..70e26e15b0 100644 --- a/examples/conversion/pkg/apis/jobs/v2/register.go +++ b/examples/conversion/pkg/apis/jobs/v2/register.go @@ -25,7 +25,7 @@ package v2 import ( "k8s.io/apimachinery/pkg/runtime/schema" - controllers "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/scheme" ) var ( @@ -33,7 +33,7 @@ var ( SchemeGroupVersion = schema.GroupVersion{Group: "jobs.example.org", Version: "v2"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme - SchemeBuilder = &controllers.SchemeBuilder{GroupVersion: SchemeGroupVersion} + SchemeBuilder = &scheme.Builder{GroupVersion: SchemeGroupVersion} // AddToScheme is required by pkg/client/... AddToScheme = SchemeBuilder.AddToScheme diff --git a/pkg/builder/build.go b/pkg/builder/build.go index 7debf14743..f8692be072 100644 --- a/pkg/builder/build.go +++ b/pkg/builder/build.go @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "sigs.k8s.io/controller-runtime/pkg/webhook/conversion" ) // Supporting mocking out functions for testing @@ -285,7 +286,11 @@ func (blder *Builder) doWebhook() error { } } - return err + err = conversion.CheckConvertibility(blder.mgr.GetScheme(), blder.apiType) + if err != nil { + log.Error(err, "conversion check failed", "GVK", gvk) + } + return nil } func generateMutatePath(gvk schema.GroupVersionKind) string { diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 60138b904c..77f1d9cdd1 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -38,6 +38,7 @@ 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" ) var log = logf.RuntimeLog.WithName("manager") @@ -191,6 +192,7 @@ 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") } diff --git a/pkg/webhook/conversion/conversion.go b/pkg/webhook/conversion/conversion.go index 3bde7b1508..8ea8b0aee8 100644 --- a/pkg/webhook/conversion/conversion.go +++ b/pkg/webhook/conversion/conversion.go @@ -212,6 +212,87 @@ 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 +// 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 { + 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) + } + + 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) + } + + if isHub(instance) { + hubs = append(hubs, instance) + continue + } + + if !isConvertible(instance) { + nonSpokes = append(nonSpokes, instance) + continue + } + + spokes = append(spokes, instance) + } + + if len(gvks) == 1 { + return 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 + } + + 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 PartialImplementationError{ + hubs: hubs, + nonSpokes: nonSpokes, + spokes: spokes, + } +} + +// PartialImplementationError represents an error due to partial conversion +// implementation such as hub without spokes, multiple hubs or spokes without hub. +type PartialImplementationError struct { + gvk schema.GroupVersionKind + hubs []runtime.Object + nonSpokes []runtime.Object + spokes []runtime.Object +} + +func (e PartialImplementationError) Error() string { + if len(e.hubs) == 0 { + return fmt.Sprintf("no hub defined for gvk %s", e.gvk) + } + if len(e.hubs) > 1 { + return fmt.Sprintf("multiple(%d) hubs defined for group-kind '%s' ", + len(e.hubs), e.gvk.GroupKind()) + } + if len(e.nonSpokes) > 0 { + return fmt.Sprintf("%d inconvertible types detected for group-kind '%s'", + len(e.nonSpokes), e.gvk.GroupKind()) + } + return "" +} + // isHub determines if passed-in object is a Hub or not. func isHub(obj runtime.Object) bool { _, yes := obj.(conversion.Hub) diff --git a/pkg/webhook/conversion/conversion_test.go b/pkg/webhook/conversion/conversion_test.go index ba5b3d22da..8dccade594 100644 --- a/pkg/webhook/conversion/conversion_test.go +++ b/pkg/webhook/conversion/conversion_test.go @@ -200,3 +200,39 @@ var _ = Describe("Conversion Webhook", func() { }) }) + +var _ = Describe("Convertibility Check", func() { + + var scheme *runtime.Scheme + + BeforeEach(func() { + + scheme = kscheme.Scheme + Expect(jobsapis.AddToScheme(scheme)).To(Succeed()) + + }) + + It("should not return error for convertible types", func() { + obj := &jobsv2.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.example.org/v2", + }, + } + + err := CheckConvertibility(scheme, obj) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should not return error for a built-in multi-version type", func() { + obj := &appsv1beta1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "apps/v1beta1", + }, + } + + err := CheckConvertibility(scheme, obj) + Expect(err).NotTo(HaveOccurred()) + }) +})