From ae26fcc716d0f96d3d3446467bfeaa56e156a8a9 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 | 34 +++-- pkg/manager/internal.go | 7 + .../internal/integration/manager_test.go | 2 +- pkg/manager/manager.go | 5 + pkg/webhook/conversion/conversion.go | 134 +++++++++++++++++- pkg/webhook/conversion/conversion_test.go | 2 +- 6 files changed, 169 insertions(+), 15 deletions(-) diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 6f4726d274..278fbac834 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -19,6 +19,7 @@ package builder import ( "context" "errors" + "fmt" "net/http" "net/url" "regexp" @@ -45,6 +46,7 @@ type WebhookBuilder struct { customPath string customValidatorCustomPath string customDefaulterCustomPath string + converter []conversion.Converter gvk schema.GroupVersionKind mgr manager.Manager config *rest.Config @@ -86,6 +88,12 @@ func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator) return blder } +// WithConverters . +func (blder *WebhookBuilder) WithConverters(converter ...conversion.Converter) *WebhookBuilder { + blder.converter = converter + 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,18 +295,26 @@ 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 len(blder.converter) > 0 { + if err := blder.mgr.GetConverterRegistry().Register(blder.gvk, blder.converter...); err != nil { + return fmt.Errorf("failed to register converter for %s: %w", blder.gvk.Kind, 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 } - log.Info("Conversion webhook enabled", "GVK", blder.gvk) } + 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 a2c3e5324d..3c9dd53d71 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -35,6 +35,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" @@ -129,6 +130,8 @@ type controllerManager struct { // webhookServer if unset, and Add() it to controllerManager. webhookServerOnce sync.Once + conversionRegistry conversion.Registry + // leaderElectionID is the name of the resource that leader election // will use for holding the leader lock. leaderElectionID string @@ -279,6 +282,10 @@ func (cm *controllerManager) GetWebhookServer() webhook.Server { return cm.webhookServer } +func (cm *controllerManager) GetConverterRegistry() conversion.Registry { + return cm.conversionRegistry +} + 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..c4b4ceee0d 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(), conversion.NewRegistry(mgr.GetScheme())) 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 e0e94245e7..763482d501 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -34,6 +34,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" @@ -95,6 +96,9 @@ type Manager interface { // GetControllerOptions returns controller global configuration options. GetControllerOptions() config.Controller + + // GetConverterRegistry FIXME. + GetConverterRegistry() conversion.Registry } // Options are the arguments for creating a new Manager. @@ -445,6 +449,7 @@ func New(config *rest.Config, options Options) (Manager, error) { logger: options.Logger, elected: make(chan struct{}), webhookServer: options.WebhookServer, + conversionRegistry: conversion.NewRegistry(cluster.GetScheme()), 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..c3e712f8a7 100644 --- a/pkg/webhook/conversion/conversion.go +++ b/pkg/webhook/conversion/conversion.go @@ -34,6 +34,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/conversion" logf "sigs.k8s.io/controller-runtime/pkg/log" conversionmetrics "sigs.k8s.io/controller-runtime/pkg/webhook/conversion/metrics" @@ -43,14 +45,110 @@ var ( log = logf.Log.WithName("conversion-webhook") ) -func NewWebhookHandler(scheme *runtime.Scheme) http.Handler { - return &webhook{scheme: scheme, decoder: NewDecoder(scheme)} +type Registry struct { + scheme *runtime.Scheme + convertersByHubGK map[schema.GroupKind]convertersForHub +} + +func NewRegistry(scheme *runtime.Scheme) Registry { + return Registry{ + scheme: scheme, + convertersByHubGK: map[schema.GroupKind]convertersForHub{}, + } +} + +type convertersForHub struct { + hubGVK schema.GroupVersionKind + convertersBySpokeGVK map[schema.GroupVersionKind]Converter +} + +func (r Registry) Register(hubGVK schema.GroupVersionKind, converters ...Converter) error { + if _, ok := r.convertersByHubGK[hubGVK.GroupKind()]; ok { + return fmt.Errorf("converter already registered for %s", hubGVK.GroupKind()) + } + + // TODO: validate against schema that all converters have been registred for a type (similar to previous validation) + + r.convertersByHubGK[hubGVK.GroupKind()] = convertersForHub{ + hubGVK: hubGVK, + convertersBySpokeGVK: map[schema.GroupVersionKind]Converter{}, + } + for _, converter := range converters { + converterHubGVK, err := apiutil.GVKForObject(converter.GetHub(), r.scheme) + if err != nil { + return err + } + if hubGVK != converterHubGVK { + return fmt.Errorf("converter GVK does not match builder gvk: FIXME") + } + converterSpokeGVK, err := apiutil.GVKForObject(converter.GetSpoke(), r.scheme) + if err != nil { + return err + } + if hubGVK.GroupKind() != converterSpokeGVK.GroupKind() { + return fmt.Errorf("converter GVK does not match builder gvk: FIXME") + } + r.convertersByHubGK[hubGVK.GroupKind()].convertersBySpokeGVK[converterSpokeGVK] = converter + } + + return nil +} + +type Converter interface { + GetHub() client.Object + GetSpoke() client.Object + ConvertHubToSpoke(hub, spoke runtime.Object) error + ConvertSpokeToHub(hub, spoke runtime.Object) error +} + +func NewConverter[hubObject, spokeObject client.Object]( + hub hubObject, + spoke spokeObject, + convertHubToSpokeFunc func(src hubObject, dst spokeObject) error, + convertSpokeToHubFunc func(src spokeObject, dst hubObject) error, +) Converter { + return &converter[hubObject, spokeObject]{ + hub: hub, + spoke: spoke, + convertSpokeToHubFunc: convertSpokeToHubFunc, + convertHubToSpokeFunc: convertHubToSpokeFunc, + } +} + +var _ Converter = converter[client.Object, client.Object]{} + +type converter[hubObject, spokeObject client.Object] struct { + hub hubObject + spoke spokeObject + convertHubToSpokeFunc func(src hubObject, dst spokeObject) error + convertSpokeToHubFunc func(src spokeObject, dst hubObject) error +} + +func (c converter[hubObject, spokeObject]) GetHub() client.Object { + return c.hub +} + +func (c converter[hubObject, spokeObject]) GetSpoke() client.Object { + return c.spoke +} + +func (c converter[hubObject, spokeObject]) ConvertHubToSpoke(hub, spoke runtime.Object) error { + return c.convertHubToSpokeFunc(hub.(hubObject), spoke.(spokeObject)) +} + +func (c converter[hubObject, spokeObject]) ConvertSpokeToHub(hub, spoke runtime.Object) error { + return c.convertSpokeToHubFunc(spoke.(spokeObject), hub.(hubObject)) +} + +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 @@ -149,6 +247,34 @@ func (wh *webhook) convertObject(src, dst runtime.Object) error { return fmt.Errorf("conversion is not allowed between same type %T", src) } + if converters, ok := wh.registry.convertersByHubGK[srcGVK.GroupKind()]; ok { + srcIsHub := converters.hubGVK == srcGVK + dstIsHub := converters.hubGVK == dstGVK + _, srcIsConvertible := converters.convertersBySpokeGVK[srcGVK] + _, dstIsConvertible := converters.convertersBySpokeGVK[dstGVK] + + switch { + case srcIsHub && dstIsConvertible: + return converters.convertersBySpokeGVK[dstGVK].ConvertHubToSpoke(src, dst) + case dstIsHub && srcIsConvertible: + return converters.convertersBySpokeGVK[srcGVK].ConvertSpokeToHub(src, dst) + case srcIsConvertible && dstIsConvertible: + hubGVK := converters.hubGVK + hub, err := wh.scheme.New(hubGVK) + if err != nil { + return fmt.Errorf("failed to allocate an instance for gvk %v: %w", hubGVK, err) + } + if err := converters.convertersBySpokeGVK[srcGVK].ConvertSpokeToHub(src, hub); err != nil { + return fmt.Errorf("%T failed to convert to hub version %T : %w", src, hub, err) + } + if err := converters.convertersBySpokeGVK[dstGVK].ConvertHubToSpoke(hub, dst); err != nil { + return fmt.Errorf("%T failed to convert from hub version %T : %w", dst, hub, err) + } + default: + return fmt.Errorf("%T is not convertible to %T", src, dst) + } + } + srcIsHub, dstIsHub := isHub(src), isHub(dst) srcIsConvertible, dstIsConvertible := isConvertible(src), isConvertible(dst) diff --git a/pkg/webhook/conversion/conversion_test.go b/pkg/webhook/conversion/conversion_test.go index 489689bccb..b97e75957f 100644 --- a/pkg/webhook/conversion/conversion_test.go +++ b/pkg/webhook/conversion/conversion_test.go @@ -58,7 +58,7 @@ var _ = Describe("Conversion Webhook", func() { Expect(jobsv3.AddToScheme(scheme)).To(Succeed()) decoder = conversion.NewDecoder(scheme) - wh = conversion.NewWebhookHandler(scheme) + wh = conversion.NewWebhookHandler(scheme, conversion.NewRegistry(scheme)) }) doRequest := func(convReq *apix.ConversionReview) *apix.ConversionReview {