From 8846a37abe6d6c60fb0c648b3ea8f553242f7481 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Sat, 4 Oct 2025 16:12:00 +0200 Subject: [PATCH] Allow implementation of conversion outside of API packages --- pkg/builder/webhook.go | 39 +- pkg/manager/internal.go | 8 + .../internal/integration/manager_test.go | 2 +- pkg/manager/manager.go | 6 + pkg/webhook/conversion/conversion.go | 17 +- pkg/webhook/conversion/conversion_hubspoke.go | 173 ++++++ pkg/webhook/conversion/conversion_registry.go | 57 ++ pkg/webhook/conversion/conversion_test.go | 568 +++++++++++------- 8 files changed, 621 insertions(+), 249 deletions(-) create mode 100644 pkg/webhook/conversion/conversion_hubspoke.go create mode 100644 pkg/webhook/conversion/conversion_registry.go diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 6f4726d274..bb5b6deb56 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -45,6 +45,7 @@ type WebhookBuilder struct { customPath string customValidatorCustomPath string customDefaulterCustomPath string + converterConstructor func(*runtime.Scheme) (conversion.Converter, error) gvk schema.GroupVersionKind mgr manager.Manager config *rest.Config @@ -86,6 +87,13 @@ func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator) return blder } +// WithConverter takes a func that constructs a converter.Converter. +// The Converter will then be used by the conversion endpoint for the type passed into For(). +func (blder *WebhookBuilder) WithConverter(converterConstructor func(*runtime.Scheme) (conversion.Converter, error)) *WebhookBuilder { + blder.converterConstructor = converterConstructor + return blder +} + // WithLogConstructor overrides the webhook's LogConstructor. func (blder *WebhookBuilder) WithLogConstructor(logConstructor func(base logr.Logger, req *admission.Request) logr.Logger) *WebhookBuilder { blder.logConstructor = logConstructor @@ -287,17 +295,30 @@ func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook { } func (blder *WebhookBuilder) registerConversionWebhook() error { - ok, err := conversion.IsConvertible(blder.mgr.GetScheme(), blder.apiType) - if err != nil { - log.Error(err, "conversion check failed", "GVK", blder.gvk) - return err - } - if ok { - if !blder.isAlreadyHandled("/convert") { - blder.mgr.GetWebhookServer().Register("/convert", conversion.NewWebhookHandler(blder.mgr.GetScheme())) + if blder.converterConstructor != nil { + converter, err := blder.converterConstructor(blder.mgr.GetScheme()) + if err != nil { + return err } - log.Info("Conversion webhook enabled", "GVK", blder.gvk) + + if err := blder.mgr.GetConverterRegistry().RegisterConverter(blder.gvk.GroupKind(), converter); err != nil { + return err + } + } else { + ok, err := conversion.IsConvertible(blder.mgr.GetScheme(), blder.apiType) + if err != nil { + log.Error(err, "conversion check failed", "GVK", blder.gvk) + return err + } + if !ok { + return nil + } + } + + if !blder.isAlreadyHandled("/convert") { + blder.mgr.GetWebhookServer().Register("/convert", conversion.NewWebhookHandler(blder.mgr.GetScheme(), blder.mgr.GetConverterRegistry())) } + log.Info("Conversion webhook enabled", "GVK", blder.gvk) return nil } diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 4362022b8c..187d4f56c2 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -36,6 +36,7 @@ import ( "k8s.io/client-go/tools/leaderelection" "k8s.io/client-go/tools/leaderelection/resourcelock" "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/webhook/conversion" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" @@ -130,6 +131,9 @@ type controllerManager struct { // webhookServer if unset, and Add() it to controllerManager. webhookServerOnce sync.Once + // converterRegistry stores conversion.Converter for the conversion endpoint. + converterRegistry conversion.Registry + // leaderElectionID is the name of the resource that leader election // will use for holding the leader lock. leaderElectionID string @@ -284,6 +288,10 @@ func (cm *controllerManager) GetWebhookServer() webhook.Server { return cm.webhookServer } +func (cm *controllerManager) GetConverterRegistry() conversion.Registry { + return cm.converterRegistry +} + func (cm *controllerManager) GetLogger() logr.Logger { return cm.logger } diff --git a/pkg/manager/internal/integration/manager_test.go b/pkg/manager/internal/integration/manager_test.go index c83eead3c1..570c932abc 100644 --- a/pkg/manager/internal/integration/manager_test.go +++ b/pkg/manager/internal/integration/manager_test.go @@ -262,7 +262,7 @@ type ConversionWebhook struct { } func createConversionWebhook(mgr manager.Manager) *ConversionWebhook { - conversionHandler := conversion.NewWebhookHandler(mgr.GetScheme()) + conversionHandler := conversion.NewWebhookHandler(mgr.GetScheme(), mgr.GetConverterRegistry()) httpClient := http.Client{ // Setting a timeout to not get stuck when calling the readiness probe. Timeout: 5 * time.Second, diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 74983ddcea..af532ea741 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -36,6 +36,7 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/utils/ptr" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/webhook/conversion" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" @@ -97,6 +98,10 @@ type Manager interface { // GetControllerOptions returns controller global configuration options. GetControllerOptions() config.Controller + + // GetConverterRegistry returns the converter registry that is used to store conversion.Converter + // for the conversion endpoint. + GetConverterRegistry() conversion.Registry } // Options are the arguments for creating a new Manager. @@ -450,6 +455,7 @@ func New(config *rest.Config, options Options) (Manager, error) { logger: options.Logger, elected: make(chan struct{}), webhookServer: options.WebhookServer, + converterRegistry: conversion.NewRegistry(), leaderElectionID: options.LeaderElectionID, leaseDuration: *options.LeaseDuration, renewDeadline: *options.RenewDeadline, diff --git a/pkg/webhook/conversion/conversion.go b/pkg/webhook/conversion/conversion.go index a26fa348bb..3f98fb7ba7 100644 --- a/pkg/webhook/conversion/conversion.go +++ b/pkg/webhook/conversion/conversion.go @@ -43,14 +43,15 @@ var ( log = logf.Log.WithName("conversion-webhook") ) -func NewWebhookHandler(scheme *runtime.Scheme) http.Handler { - return &webhook{scheme: scheme, decoder: NewDecoder(scheme)} +func NewWebhookHandler(scheme *runtime.Scheme, registry Registry) http.Handler { + return &webhook{scheme: scheme, decoder: NewDecoder(scheme), registry: registry} } // webhook implements a CRD conversion webhook HTTP handler. type webhook struct { - scheme *runtime.Scheme - decoder *Decoder + scheme *runtime.Scheme + decoder *Decoder + registry Registry } // ensure Webhook implements http.Handler @@ -119,7 +120,7 @@ func (wh *webhook) handleConvertRequest(ctx context.Context, req *apix.Conversio if err != nil { return nil, err } - err = wh.convertObject(src, dst) + err = wh.convertObject(ctx, src, dst) if err != nil { return nil, err } @@ -137,7 +138,7 @@ func (wh *webhook) handleConvertRequest(ctx context.Context, req *apix.Conversio // convertObject will convert given a src object to dst object. // Note(droot): couldn't find a way to reduce the cyclomatic complexity under 10 // without compromising readability, so disabling gocyclo linter -func (wh *webhook) convertObject(src, dst runtime.Object) error { +func (wh *webhook) convertObject(ctx context.Context, src, dst runtime.Object) error { srcGVK := src.GetObjectKind().GroupVersionKind() dstGVK := dst.GetObjectKind().GroupVersionKind() @@ -149,6 +150,10 @@ func (wh *webhook) convertObject(src, dst runtime.Object) error { return fmt.Errorf("conversion is not allowed between same type %T", src) } + if converter, ok := wh.registry.GetConverter(srcGVK.GroupKind()); ok { + return converter.ConvertObject(ctx, src, dst) + } + srcIsHub, dstIsHub := isHub(src), isHub(dst) srcIsConvertible, dstIsConvertible := isConvertible(src), isConvertible(dst) diff --git a/pkg/webhook/conversion/conversion_hubspoke.go b/pkg/webhook/conversion/conversion_hubspoke.go new file mode 100644 index 0000000000..b33af92ff4 --- /dev/null +++ b/pkg/webhook/conversion/conversion_hubspoke.go @@ -0,0 +1,173 @@ +/* +Copyright 2025 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 ( + "context" + "fmt" + "slices" + "strings" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" +) + +func NewHubSpokeConverter[hubObject runtime.Object](hub hubObject, spokeConverter ...SpokeConverter[hubObject]) func(scheme *runtime.Scheme) (Converter, error) { + return func(scheme *runtime.Scheme) (Converter, error) { + hubGVK, err := apiutil.GVKForObject(hub, scheme) + if err != nil { + return nil, fmt.Errorf("failed to create hub spoke converter: failed to get GroupVersionKind for hub: %w", err) + } + allGVKs, err := objectGVKs(scheme, hub) + if err != nil { + return nil, fmt.Errorf("failed to create hub spoke converter for %s: %w", hubGVK.Kind, err) + } + spokeVersions := sets.New[string]() + for _, gvk := range allGVKs { + if gvk != hubGVK { + spokeVersions.Insert(gvk.Version) + } + } + + c := &hubSpokeConverter[hubObject]{ + scheme: scheme, + hubGVK: hubGVK, + spokeConverterByGVK: map[schema.GroupVersionKind]SpokeConverter[hubObject]{}, + } + + spokeConverterVersions := sets.New[string]() + for _, sc := range spokeConverter { + spokeGVK, err := apiutil.GVKForObject(sc.GetSpoke(), scheme) + if err != nil { + return nil, fmt.Errorf("failed to create hub spoke converter for %s: "+ + "failed to get GroupVersionKind for spoke converter: %w", + hubGVK.Kind, err) + } + if hubGVK.GroupKind() != spokeGVK.GroupKind() { + return nil, fmt.Errorf("failed to create hub spoke converter for %s: "+ + "spoke converter GroupKind %s does not match hub GroupKind %s", + hubGVK.Kind, spokeGVK.GroupKind(), hubGVK.GroupKind()) + } + + if _, ok := c.spokeConverterByGVK[spokeGVK]; ok { + return nil, fmt.Errorf("failed to create hub spoke converter for %s: "+ + "duplicate spoke converter for version %s", + hubGVK.Kind, spokeGVK.Version) + } + c.spokeConverterByGVK[spokeGVK] = sc + spokeConverterVersions.Insert(spokeGVK.Version) + } + + if !spokeConverterVersions.Equal(spokeVersions) { + return nil, fmt.Errorf("failed to create hub spoke converter for %s: "+ + "expected spoke converter for %s got spoke converter for %s", + hubGVK.Kind, sortAndJoin(spokeVersions), sortAndJoin(spokeConverterVersions)) + } + + return c, nil + } +} + +func sortAndJoin(set sets.Set[string]) string { + list := set.UnsortedList() + slices.Sort(list) + return strings.Join(list, ",") +} + +type hubSpokeConverter[hubObject runtime.Object] struct { + scheme *runtime.Scheme + hubGVK schema.GroupVersionKind + spokeConverterByGVK map[schema.GroupVersionKind]SpokeConverter[hubObject] +} + +func (c hubSpokeConverter[hubObject]) ConvertObject(ctx context.Context, src, dst runtime.Object) error { + srcGVK := src.GetObjectKind().GroupVersionKind() + dstGVK := dst.GetObjectKind().GroupVersionKind() + + if srcGVK.GroupKind() != dstGVK.GroupKind() { + return fmt.Errorf("src %T and dst %T does not belong to same API Group", src, dst) + } + + if srcGVK == dstGVK { + return fmt.Errorf("conversion is not allowed between same type %T", src) + } + + srcIsHub := c.hubGVK == srcGVK + dstIsHub := c.hubGVK == dstGVK + _, srcIsConvertible := c.spokeConverterByGVK[srcGVK] + _, dstIsConvertible := c.spokeConverterByGVK[dstGVK] + + switch { + case srcIsHub && dstIsConvertible: + return c.spokeConverterByGVK[dstGVK].ConvertHubToSpoke(ctx, src.(hubObject), dst) + case dstIsHub && srcIsConvertible: + return c.spokeConverterByGVK[srcGVK].ConvertSpokeToHub(ctx, src, dst.(hubObject)) + case srcIsConvertible && dstIsConvertible: + hub, err := c.scheme.New(c.hubGVK) + if err != nil { + return fmt.Errorf("failed to allocate an instance for GroupVersionKind %s: %w", c.hubGVK, err) + } + if err := c.spokeConverterByGVK[srcGVK].ConvertSpokeToHub(ctx, src, hub.(hubObject)); err != nil { + return fmt.Errorf("failed to convert spoke %s to hub %s : %w", srcGVK, c.hubGVK, err) + } + if err := c.spokeConverterByGVK[dstGVK].ConvertHubToSpoke(ctx, hub.(hubObject), dst); err != nil { + return fmt.Errorf("failed to convert hub %s to spoke %s : %w", c.hubGVK, dstGVK, err) + } + return nil + default: + return fmt.Errorf("failed to convert %s to %s: not convertible", srcGVK, dstGVK) + } +} + +type SpokeConverter[hubObject runtime.Object] interface { + GetSpoke() runtime.Object + ConvertHubToSpoke(ctx context.Context, hub hubObject, spoke runtime.Object) error + ConvertSpokeToHub(ctx context.Context, spoke runtime.Object, hub hubObject) error +} + +func NewSpokeConverter[hubObject, spokeObject client.Object]( + spoke spokeObject, + convertHubToSpokeFunc func(ctx context.Context, src hubObject, dst spokeObject) error, + convertSpokeToHubFunc func(ctx context.Context, src spokeObject, dst hubObject) error, +) SpokeConverter[hubObject] { + return &spokeConverter[hubObject, spokeObject]{ + spoke: spoke, + convertSpokeToHubFunc: convertSpokeToHubFunc, + convertHubToSpokeFunc: convertHubToSpokeFunc, + } +} + +type spokeConverter[hubObject, spokeObject runtime.Object] struct { + spoke spokeObject + convertHubToSpokeFunc func(ctx context.Context, src hubObject, dst spokeObject) error + convertSpokeToHubFunc func(ctx context.Context, src spokeObject, dst hubObject) error +} + +func (c spokeConverter[hubObject, spokeObject]) GetSpoke() runtime.Object { + return c.spoke +} + +func (c spokeConverter[hubObject, spokeObject]) ConvertHubToSpoke(ctx context.Context, hub hubObject, spoke runtime.Object) error { + return c.convertHubToSpokeFunc(ctx, hub, spoke.(spokeObject)) +} + +func (c spokeConverter[hubObject, spokeObject]) ConvertSpokeToHub(ctx context.Context, spoke runtime.Object, hub hubObject) error { + return c.convertSpokeToHubFunc(ctx, spoke.(spokeObject), hub) +} diff --git a/pkg/webhook/conversion/conversion_registry.go b/pkg/webhook/conversion/conversion_registry.go new file mode 100644 index 0000000000..6e68b5ffa6 --- /dev/null +++ b/pkg/webhook/conversion/conversion_registry.go @@ -0,0 +1,57 @@ +/* +Copyright 2025 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 ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +type Converter interface { + ConvertObject(ctx context.Context, src, dst runtime.Object) error +} + +type Registry interface { + RegisterConverter(gk schema.GroupKind, converter Converter) error + GetConverter(gk schema.GroupKind) (Converter, bool) +} + +type registry struct { + converterByGK map[schema.GroupKind]Converter +} + +func NewRegistry() Registry { + return registry{ + converterByGK: map[schema.GroupKind]Converter{}, + } +} +func (r registry) RegisterConverter(gk schema.GroupKind, converter Converter) error { + if _, ok := r.converterByGK[gk]; ok { + return fmt.Errorf("failed to register Converter for GroupKind %s: converter already registered", gk) + } + + r.converterByGK[gk] = converter + return nil +} + +func (r registry) GetConverter(gk schema.GroupKind) (Converter, bool) { + c, ok := r.converterByGK[gk] + return c, ok +} diff --git a/pkg/webhook/conversion/conversion_test.go b/pkg/webhook/conversion/conversion_test.go index 489689bccb..046ab44ced 100644 --- a/pkg/webhook/conversion/conversion_test.go +++ b/pkg/webhook/conversion/conversion_test.go @@ -18,6 +18,7 @@ package conversion_test import ( "bytes" + "context" "encoding/json" "io" "net/http" @@ -25,7 +26,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - + appsv1 "k8s.io/api/apps/v1" appsv1beta1 "k8s.io/api/apps/v1beta1" apix "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -39,285 +40,370 @@ import ( jobsv3 "sigs.k8s.io/controller-runtime/pkg/webhook/conversion/testdata/api/v3" ) -var _ = Describe("Conversion Webhook", func() { - - var respRecorder *httptest.ResponseRecorder - var decoder *conversion.Decoder - var scheme *runtime.Scheme - var wh http.Handler - - BeforeEach(func() { - respRecorder = &httptest.ResponseRecorder{ - Body: bytes.NewBuffer(nil), - } - - scheme = runtime.NewScheme() - Expect(kscheme.AddToScheme(scheme)).To(Succeed()) - Expect(jobsv1.AddToScheme(scheme)).To(Succeed()) - Expect(jobsv2.AddToScheme(scheme)).To(Succeed()) - Expect(jobsv3.AddToScheme(scheme)).To(Succeed()) - - decoder = conversion.NewDecoder(scheme) - wh = conversion.NewWebhookHandler(scheme) - }) - - doRequest := func(convReq *apix.ConversionReview) *apix.ConversionReview { - var payload bytes.Buffer +var _ = Describe("Conversion with Hub/ConvertTo/ConvertFrom methods", func() { + ConversionTest(false) +}) - Expect(json.NewEncoder(&payload).Encode(convReq)).Should(Succeed()) +var _ = Describe("Conversion with HubSpokeConverter", func() { + ConversionTest(true) +}) - convReview := &apix.ConversionReview{} - req := &http.Request{ - Body: io.NopCloser(bytes.NewReader(payload.Bytes())), +func ConversionTest(withHubSpokeConverter bool) { + Describe("Conversion Webhook", func() { + var respRecorder *httptest.ResponseRecorder + var decoder *conversion.Decoder + var scheme *runtime.Scheme + var wh http.Handler + + BeforeEach(func() { + respRecorder = &httptest.ResponseRecorder{ + Body: bytes.NewBuffer(nil), + } + + scheme = runtime.NewScheme() + Expect(kscheme.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv1.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv2.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv3.AddToScheme(scheme)).To(Succeed()) + + decoder = conversion.NewDecoder(scheme) + registry := conversion.NewRegistry() + + if withHubSpokeConverter { + converter, err := conversion.NewHubSpokeConverter(&jobsv2.ExternalJob{}, + conversion.NewSpokeConverter(&jobsv1.ExternalJob{}, convertHubToV1, convertV1ToHub), + conversion.NewSpokeConverter(&jobsv3.ExternalJob{}, convertHubToV3, convertV3ToHub), + )(scheme) + Expect(err).ToNot(HaveOccurred()) + Expect(registry.RegisterConverter(jobsv2.GroupVersion.WithKind("ExternalJob").GroupKind(), converter)).To(Succeed()) + } + + wh = conversion.NewWebhookHandler(scheme, registry) + }) + + doRequest := func(convReq *apix.ConversionReview) *apix.ConversionReview { + var payload bytes.Buffer + + Expect(json.NewEncoder(&payload).Encode(convReq)).Should(Succeed()) + + convReview := &apix.ConversionReview{} + req := &http.Request{ + Body: io.NopCloser(bytes.NewReader(payload.Bytes())), + } + wh.ServeHTTP(respRecorder, req) + Expect(json.NewDecoder(respRecorder.Result().Body).Decode(convReview)).To(Succeed()) + return convReview } - wh.ServeHTTP(respRecorder, req) - Expect(json.NewDecoder(respRecorder.Result().Body).Decode(convReview)).To(Succeed()) - return convReview - } - makeV1Obj := func() *jobsv1.ExternalJob { - return &jobsv1.ExternalJob{ - TypeMeta: metav1.TypeMeta{ - Kind: "ExternalJob", - APIVersion: "jobs.testprojects.kb.io/v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "obj-1", - }, - Spec: jobsv1.ExternalJobSpec{ - RunAt: "every 2 seconds", - }, + makeV1Obj := func() *jobsv1.ExternalJob { + return &jobsv1.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.testprojects.kb.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + Spec: jobsv1.ExternalJobSpec{ + RunAt: "every 2 seconds", + }, + } } - } - makeV2Obj := func() *jobsv2.ExternalJob { - return &jobsv2.ExternalJob{ - TypeMeta: metav1.TypeMeta{ - Kind: "ExternalJob", - APIVersion: "jobs.testprojects.kb.io/v2", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "obj-1", - }, - Spec: jobsv2.ExternalJobSpec{ - ScheduleAt: "every 2 seconds", - }, + makeV2Obj := func() *jobsv2.ExternalJob { + return &jobsv2.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.testprojects.kb.io/v2", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + Spec: jobsv2.ExternalJobSpec{ + ScheduleAt: "every 2 seconds", + }, + } } - } - It("should convert spoke to hub successfully", func() { + It("should convert spoke to hub successfully", func() { - v1Obj := makeV1Obj() + v1Obj := makeV1Obj() - expected := &jobsv2.ExternalJob{ - TypeMeta: metav1.TypeMeta{ - Kind: "ExternalJob", - APIVersion: "jobs.testprojects.kb.io/v2", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "obj-1", - }, - Spec: jobsv2.ExternalJobSpec{ - ScheduleAt: "every 2 seconds", - }, - } - - convReq := &apix.ConversionReview{ - TypeMeta: metav1.TypeMeta{}, - Request: &apix.ConversionRequest{ - DesiredAPIVersion: "jobs.testprojects.kb.io/v2", - Objects: []runtime.RawExtension{ - { - Object: v1Obj, + expected := &jobsv2.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.testprojects.kb.io/v2", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + Spec: jobsv2.ExternalJobSpec{ + ScheduleAt: "every 2 seconds", + }, + } + + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + DesiredAPIVersion: "jobs.testprojects.kb.io/v2", + Objects: []runtime.RawExtension{ + { + Object: v1Obj, + }, }, }, - }, - } + } - convReview := doRequest(convReq) + convReview := doRequest(convReq) - Expect(convReview.Response.ConvertedObjects).To(HaveLen(1)) - Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusSuccess)) - got, _, err := decoder.Decode(convReview.Response.ConvertedObjects[0].Raw) - Expect(err).NotTo(HaveOccurred()) - Expect(got).To(Equal(expected)) - }) + Expect(convReview.Response.ConvertedObjects).To(HaveLen(1)) + Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusSuccess)) + got, _, err := decoder.Decode(convReview.Response.ConvertedObjects[0].Raw) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(expected)) + }) - It("should convert hub to spoke successfully", func() { + It("should convert hub to spoke successfully", func() { - v2Obj := makeV2Obj() + v2Obj := makeV2Obj() - expected := &jobsv1.ExternalJob{ - TypeMeta: metav1.TypeMeta{ - Kind: "ExternalJob", - APIVersion: "jobs.testprojects.kb.io/v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "obj-1", - }, - Spec: jobsv1.ExternalJobSpec{ - RunAt: "every 2 seconds", - }, - } - - convReq := &apix.ConversionReview{ - TypeMeta: metav1.TypeMeta{}, - Request: &apix.ConversionRequest{ - DesiredAPIVersion: "jobs.testprojects.kb.io/v1", - Objects: []runtime.RawExtension{ - { - Object: v2Obj, + expected := &jobsv1.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.testprojects.kb.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + Spec: jobsv1.ExternalJobSpec{ + RunAt: "every 2 seconds", + }, + } + + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + DesiredAPIVersion: "jobs.testprojects.kb.io/v1", + Objects: []runtime.RawExtension{ + { + Object: v2Obj, + }, }, }, - }, - } + } - convReview := doRequest(convReq) + convReview := doRequest(convReq) - Expect(convReview.Response.ConvertedObjects).To(HaveLen(1)) - Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusSuccess)) - got, _, err := decoder.Decode(convReview.Response.ConvertedObjects[0].Raw) - Expect(err).NotTo(HaveOccurred()) - Expect(got).To(Equal(expected)) - }) + Expect(convReview.Response.ConvertedObjects).To(HaveLen(1)) + Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusSuccess)) + got, _, err := decoder.Decode(convReview.Response.ConvertedObjects[0].Raw) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(expected)) + }) - It("should convert spoke to spoke successfully", func() { + It("should convert spoke to spoke successfully", func() { - v1Obj := makeV1Obj() + v1Obj := makeV1Obj() - expected := &jobsv3.ExternalJob{ - TypeMeta: metav1.TypeMeta{ - Kind: "ExternalJob", - APIVersion: "jobs.testprojects.kb.io/v3", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "obj-1", - }, - Spec: jobsv3.ExternalJobSpec{ - DeferredAt: "every 2 seconds", - }, - } - - convReq := &apix.ConversionReview{ - TypeMeta: metav1.TypeMeta{}, - Request: &apix.ConversionRequest{ - DesiredAPIVersion: "jobs.testprojects.kb.io/v3", - Objects: []runtime.RawExtension{ - { - Object: v1Obj, + expected := &jobsv3.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.testprojects.kb.io/v3", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + Spec: jobsv3.ExternalJobSpec{ + DeferredAt: "every 2 seconds", + }, + } + + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + DesiredAPIVersion: "jobs.testprojects.kb.io/v3", + Objects: []runtime.RawExtension{ + { + Object: v1Obj, + }, }, }, - }, - } + } + + convReview := doRequest(convReq) + + Expect(convReview.Response.ConvertedObjects).To(HaveLen(1)) + Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusSuccess)) + got, _, err := decoder.Decode(convReview.Response.ConvertedObjects[0].Raw) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(expected)) + }) + + It("should return error when dest/src objects belong to different API groups", func() { + v1Obj := makeV1Obj() + + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + // request conversion for different group + DesiredAPIVersion: "jobss.example.org/v2", + Objects: []runtime.RawExtension{ + { + Object: v1Obj, + }, + }, + }, + } - convReview := doRequest(convReq) + convReview := doRequest(convReq) + Expect(convReview.Response.Result.Status).To(Equal("Failure")) + Expect(convReview.Response.ConvertedObjects).To(BeEmpty()) + }) - Expect(convReview.Response.ConvertedObjects).To(HaveLen(1)) - Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusSuccess)) - got, _, err := decoder.Decode(convReview.Response.ConvertedObjects[0].Raw) - Expect(err).NotTo(HaveOccurred()) - Expect(got).To(Equal(expected)) - }) + It("should return error when dest/src objects are of same type", func() { + + v1Obj := makeV1Obj() - It("should return error when dest/src objects belong to different API groups", func() { - v1Obj := makeV1Obj() - - convReq := &apix.ConversionReview{ - TypeMeta: metav1.TypeMeta{}, - Request: &apix.ConversionRequest{ - // request conversion for different group - DesiredAPIVersion: "jobss.example.org/v2", - Objects: []runtime.RawExtension{ - { - Object: v1Obj, + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + DesiredAPIVersion: "jobs.testprojects.kb.io/v1", + Objects: []runtime.RawExtension{ + { + Object: v1Obj, + }, }, }, - }, - } - - convReview := doRequest(convReq) - Expect(convReview.Response.Result.Status).To(Equal("Failure")) - Expect(convReview.Response.ConvertedObjects).To(BeEmpty()) - }) + } - It("should return error when dest/src objects are of same type", func() { + convReview := doRequest(convReq) + Expect(convReview.Response.Result.Status).To(Equal("Failure")) + Expect(convReview.Response.ConvertedObjects).To(BeEmpty()) + }) - v1Obj := makeV1Obj() + It("should return error when the API group does not have a hub defined", func() { - convReq := &apix.ConversionReview{ - TypeMeta: metav1.TypeMeta{}, - Request: &apix.ConversionRequest{ - DesiredAPIVersion: "jobs.testprojects.kb.io/v1", - Objects: []runtime.RawExtension{ - { - Object: v1Obj, + v1Obj := &appsv1beta1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "apps/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + } + + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + DesiredAPIVersion: "apps/v1", + Objects: []runtime.RawExtension{ + { + Object: v1Obj, + }, }, }, - }, - } + } + + convReview := doRequest(convReq) + Expect(convReview.Response.Result.Status).To(Equal("Failure")) + Expect(convReview.Response.ConvertedObjects).To(BeEmpty()) + }) + + It("should return error on panic in conversion", func() { + + v1Obj := makeV1Obj() + v1Obj.Spec.PanicInConversion = true + + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + DesiredAPIVersion: "jobs.testprojects.kb.io/v3", + Objects: []runtime.RawExtension{ + { + Object: v1Obj, + }, + }, + }, + } - convReview := doRequest(convReq) - Expect(convReview.Response.Result.Status).To(Equal("Failure")) - Expect(convReview.Response.ConvertedObjects).To(BeEmpty()) - }) + convReview := doRequest(convReq) - It("should return error when the API group does not have a hub defined", func() { + Expect(convReview.Response.ConvertedObjects).To(HaveLen(0)) + Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusFailure)) + Expect(convReview.Response.Result.Message).To(Equal("internal error occurred during conversion")) + }) + }) +} - v1Obj := &appsv1beta1.Deployment{ - TypeMeta: metav1.TypeMeta{ - Kind: "Deployment", - APIVersion: "apps/v1beta1", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "obj-1", - }, - } +var _ = Describe("NewHubSpokeConverter", func() { + var scheme *runtime.Scheme - convReq := &apix.ConversionReview{ - TypeMeta: metav1.TypeMeta{}, - Request: &apix.ConversionRequest{ - DesiredAPIVersion: "apps/v1", - Objects: []runtime.RawExtension{ - { - Object: v1Obj, - }, - }, - }, - } + BeforeEach(func() { + scheme = runtime.NewScheme() + Expect(jobsv1.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv2.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv3.AddToScheme(scheme)).To(Succeed()) + }) - convReview := doRequest(convReq) - Expect(convReview.Response.Result.Status).To(Equal("Failure")) - Expect(convReview.Response.ConvertedObjects).To(BeEmpty()) + It("should succeed if all converter are specified", func() { + _, err := conversion.NewHubSpokeConverter(&jobsv2.ExternalJob{}, + conversion.NewSpokeConverter(&jobsv1.ExternalJob{}, convertHubToV1, convertV1ToHub), + conversion.NewSpokeConverter(&jobsv3.ExternalJob{}, convertHubToV3, convertV3ToHub), + )(scheme) + Expect(err).ToNot(HaveOccurred()) }) - It("should return error on panic in conversion", func() { + It("should return error if hub is not registered in the scheme", func() { + _, err := conversion.NewHubSpokeConverter(&appsv1.Deployment{})(scheme) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("failed to create hub spoke converter: failed to get GroupVersionKind for hub: no kind is registered for the type v1.Deployment in scheme \"pkg/runtime/scheme.go:111\"")) + }) - v1Obj := makeV1Obj() - v1Obj.Spec.PanicInConversion = true + It("should return error if spoke is not registered in the scheme", func() { + _, err := conversion.NewHubSpokeConverter(&jobsv2.ExternalJob{}, + conversion.NewSpokeConverter(&appsv1.Deployment{}, + func(context.Context, *jobsv2.ExternalJob, *appsv1.Deployment) error { return nil }, + func(context.Context, *appsv1.Deployment, *jobsv2.ExternalJob) error { return nil }), + )(scheme) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("failed to create hub spoke converter for ExternalJob: failed to get GroupVersionKind for spoke converter: no kind is registered for the type v1.Deployment in scheme \"pkg/runtime/scheme.go:111\"")) + }) - convReq := &apix.ConversionReview{ - TypeMeta: metav1.TypeMeta{}, - Request: &apix.ConversionRequest{ - DesiredAPIVersion: "jobs.testprojects.kb.io/v3", - Objects: []runtime.RawExtension{ - { - Object: v1Obj, - }, - }, - }, - } + It("should return error if spoke does not have the same GroupKind as the hub", func() { + _ = kscheme.AddToScheme(scheme) + _, err := conversion.NewHubSpokeConverter(&jobsv2.ExternalJob{}, + conversion.NewSpokeConverter(&appsv1.Deployment{}, + func(context.Context, *jobsv2.ExternalJob, *appsv1.Deployment) error { return nil }, + func(context.Context, *appsv1.Deployment, *jobsv2.ExternalJob) error { return nil }), + )(scheme) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("failed to create hub spoke converter for ExternalJob: spoke converter GroupKind Deployment.apps does not match hub GroupKind ExternalJob.jobs.testprojects.kb.io")) + }) - convReview := doRequest(convReq) + It("should return error if same spoke is specified twice", func() { + _, err := conversion.NewHubSpokeConverter(&jobsv2.ExternalJob{}, + conversion.NewSpokeConverter(&jobsv1.ExternalJob{}, convertHubToV1, convertV1ToHub), + conversion.NewSpokeConverter(&jobsv3.ExternalJob{}, convertHubToV3, convertV3ToHub), // duplicate + conversion.NewSpokeConverter(&jobsv3.ExternalJob{}, convertHubToV3, convertV3ToHub), // duplicate + )(scheme) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("failed to create hub spoke converter for ExternalJob: duplicate spoke converter for version v3")) + }) - Expect(convReview.Response.ConvertedObjects).To(HaveLen(0)) - Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusFailure)) - Expect(convReview.Response.Result.Message).To(Equal("internal error occurred during conversion")) + It("should return error if a converter is missing", func() { + _, err := conversion.NewHubSpokeConverter(&jobsv2.ExternalJob{}, + conversion.NewSpokeConverter(&jobsv1.ExternalJob{}, convertHubToV1, convertV1ToHub), + // jobsv3.ExternalJob converter is missing + )(scheme) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("failed to create hub spoke converter for ExternalJob: expected spoke converter for v1,v3 got spoke converter for v1")) }) }) @@ -381,3 +467,19 @@ var _ = Describe("IsConvertible", func() { Expect(ok).ToNot(BeTrue()) }) }) + +func convertV1ToHub(_ context.Context, src *jobsv1.ExternalJob, dst *jobsv2.ExternalJob) error { + return src.ConvertTo(dst) +} + +func convertHubToV1(_ context.Context, src *jobsv2.ExternalJob, dst *jobsv1.ExternalJob) error { + return dst.ConvertFrom(src) +} + +func convertV3ToHub(_ context.Context, src *jobsv3.ExternalJob, dst *jobsv2.ExternalJob) error { + return src.ConvertTo(dst) +} + +func convertHubToV3(_ context.Context, src *jobsv2.ExternalJob, dst *jobsv3.ExternalJob) error { + return dst.ConvertFrom(src) +}