Skip to content

Commit

Permalink
✨ enable conversion webhook by default
Browse files Browse the repository at this point in the history
  • Loading branch information
droot committed May 14, 2019
1 parent d6324a4 commit 09c23f1
Show file tree
Hide file tree
Showing 8 changed files with 244 additions and 17 deletions.
4 changes: 2 additions & 2 deletions examples/conversion/pkg/apis/jobs/v1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ package v1

import (
"k8s.io/apimachinery/pkg/runtime/schema"
controllers "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/scheme"
)

var (
// SchemeGroupVersion is group version used to register these objects
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
Expand Down
4 changes: 2 additions & 2 deletions examples/conversion/pkg/apis/jobs/v2/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ package v2

import (
"k8s.io/apimachinery/pkg/runtime/schema"
controllers "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/scheme"
)

var (
// SchemeGroupVersion is group version used to register these objects
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
Expand Down
15 changes: 3 additions & 12 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,23 @@ require (
github.com/appscode/jsonpatch v0.0.0-20190108182946-7c0e3b262f30
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 // indirect
github.com/evanphx/json-patch v4.1.0+incompatible
github.com/fsnotify/fsnotify v1.4.7 // indirect
github.com/ghodss/yaml v1.0.0
github.com/go-logr/logr v0.1.0
github.com/go-logr/zapr v0.1.0
github.com/gogo/protobuf v1.1.1 // indirect
github.com/golang/groupcache v0.0.0-20180513044358-24b0969c4cb7 // indirect
github.com/golang/protobuf v1.2.0 // indirect
github.com/golangci/golangci-lint v1.15.0 // indirect
github.com/google/btree v1.0.0 // indirect
github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf // indirect
github.com/googleapis/gnostic v0.2.0 // indirect
github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
github.com/hashicorp/golang-lru v0.0.0-20180201235237-0fb14efe8c47 // indirect
github.com/hpcloud/tail v1.0.0 // indirect
github.com/imdario/mergo v0.3.6 // indirect
github.com/json-iterator/go v1.1.5 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742 // indirect
github.com/onsi/ginkgo v1.6.0
github.com/onsi/gomega v1.4.1
github.com/onsi/gomega v1.4.2
github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c // indirect
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
github.com/pkg/errors v0.8.1 // indirect
Expand All @@ -39,23 +36,17 @@ require (
go.uber.org/multierr v1.1.0 // indirect
go.uber.org/zap v1.9.1
golang.org/x/crypto v0.0.0-20180820150726-614d502a4dac // indirect
golang.org/x/net v0.0.0-20180821023952-922f4815f713 // indirect
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be // indirect
golang.org/x/sync v0.0.0-20190423024810-112230192c58 // indirect
golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc // indirect
golang.org/x/text v0.3.0 // indirect
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 // indirect
google.golang.org/appengine v1.1.0 // indirect
gopkg.in/fsnotify.v1 v1.4.7 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
gopkg.in/yaml.v2 v2.2.1 // indirect
k8s.io/api v0.0.0-20190222213804-5cb15d344471
k8s.io/apiextensions-apiserver v0.0.0-20190228180357-d002e88f6236
k8s.io/apimachinery v0.0.0-20190221213512-86fb29eff628
k8s.io/client-go v0.0.0-20190228174230-b40b2a5939e4
k8s.io/klog v0.1.0 // indirect
k8s.io/kube-openapi v0.0.0-20180731170545-e3762e86a74c // indirect
sigs.k8s.io/testing_frameworks v0.1.1
sigs.k8s.io/yaml v1.1.0 // indirect
sigs.k8s.io/yaml v1.1.0
)
151 changes: 151 additions & 0 deletions go.sum

Large diffs are not rendered by default.

11 changes: 10 additions & 1 deletion pkg/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -285,7 +286,15 @@ 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)
// TODO(droot): suppressing this error because we will get false positives
// for the multi-version types that are not managed by user-project.
// Figure out a safe approach to make it stricter.
return nil
}
return nil
}

func generateMutatePath(gvk schema.GroupVersionKind) string {
Expand Down
2 changes: 2 additions & 0 deletions pkg/manager/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/webhook/conversion/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,44 @@ 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 {
gvks, _, err := scheme.ObjectKinds(obj)
if err != nil {
return fmt.Errorf("error retriving object kinds for given object : %v", err)
}

hubFound := false
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) {
if hubFound {
return fmt.Errorf("multiple hub version defined for %T", obj)
}
hubFound = true
continue
}

if !isConvertible(instance) {
// non-hub needs to be convertible
return fmt.Errorf("%T needs to be convertible", instance)
}
}

if len(gvks) > 1 && !hubFound {
// multiple version discovered with no hub defined
return fmt.Errorf("%T has no hub defined", obj)
}

return nil
}

// isHub determines if passed-in object is a Hub or not.
func isHub(obj runtime.Object) bool {
_, yes := obj.(conversion.Hub)
Expand Down
36 changes: 36 additions & 0 deletions pkg/webhook/conversion/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 return error for non-convertible multi-version type", func() {
obj := &appsv1beta1.Deployment{
TypeMeta: metav1.TypeMeta{
Kind: "Deployment",
APIVersion: "apps/v1beta1",
},
}

err := CheckConvertibility(scheme, obj)
Expect(err).To(HaveOccurred())
})
})

0 comments on commit 09c23f1

Please sign in to comment.