Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ enable conversion webhook in webhook builder #504

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions examples/conversion/pkg/apis/addtoscheme_jobs_v1.go

This file was deleted.

25 changes: 0 additions & 25 deletions examples/conversion/pkg/apis/addtoscheme_jobs_v2.go

This file was deleted.

32 changes: 0 additions & 32 deletions examples/conversion/pkg/apis/apis.go

This file was deleted.

22 changes: 0 additions & 22 deletions examples/conversion/pkg/apis/jobs/v1/doc.go

This file was deleted.

22 changes: 0 additions & 22 deletions examples/conversion/pkg/apis/jobs/v2/doc.go

This file was deleted.

23 changes: 21 additions & 2 deletions pkg/builder/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
24 changes: 21 additions & 3 deletions pkg/builder/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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{}
Expand Down Expand Up @@ -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{}
Expand Down
2 changes: 0 additions & 2 deletions pkg/manager/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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")
}
Expand Down
48 changes: 31 additions & 17 deletions pkg/webhook/conversion/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we should log something in this case, since builtin types won't be sent to the webhoo; If it happens it usually means CRD conversion are configured by the CRD types doesn't implement Hub and Convertible interfaces.
Feel free to address this in a followup PR.

// 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 {
Expand Down
Loading