Skip to content

Commit

Permalink
🐛 stop registering webhooks with same path when adding multiple contr…
Browse files Browse the repository at this point in the history
…ollers

If a kind has implemented Defaulter and(or) Validator interface(s),
multiple controllers for this kind should be able to be added to the
same controller manager.
  • Loading branch information
Mengqi Yu committed Jun 11, 2019
1 parent f99287c commit ccc5417
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 8 deletions.
29 changes: 21 additions & 8 deletions pkg/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package builder

import (
"fmt"
"net/http"
"net/url"
"strings"

"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -280,22 +282,33 @@ func (blder *Builder) doWebhook() error {
mwh := admission.DefaultingWebhookFor(defaulter)
if mwh != nil {
path := generateMutatePath(gvk)
log.Info("Registering a mutating webhook",
"GVK", gvk,
"path", path)

blder.mgr.GetWebhookServer().Register(path, mwh)
// Checking if the path is already registered.
// If so, just skip it.
h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}})
if p != path || h == nil {
log.Info("Registering a mutating webhook",
"GVK", gvk,
"path", path)
blder.mgr.GetWebhookServer().Register(path, mwh)
}
}
}

if validator, isValidator := blder.apiType.(admission.Validator); isValidator {
vwh := admission.ValidatingWebhookFor(validator)
if vwh != nil {
path := generateValidatePath(gvk)
log.Info("Registering a validating webhook",
"GVK", gvk,
"path", path)
blder.mgr.GetWebhookServer().Register(path, vwh)

// Checking if the path is already registered.
// If so, just skip it.
h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}})
if p != path || h == nil {
log.Info("Registering a validating webhook",
"GVK", gvk,
"path", path)
blder.mgr.GetWebhookServer().Register(path, vwh)
}
}
}

Expand Down
28 changes: 28 additions & 0 deletions pkg/builder/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,34 @@ var _ = Describe("application", func() {
Expect(w.Body).To(ContainSubstring(`"allowed":true`))
Expect(w.Body).To(ContainSubstring(`"code":200`))
})

It("should allow multiple controllers for the same kind", func() {
By("creating a controller manager")
m, err := manager.New(cfg, manager.Options{})
Expect(err).NotTo(HaveOccurred())

By("registering the type in the Scheme")
builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()}
builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{})
err = builder.AddToScheme(m.GetScheme())
Expect(err).NotTo(HaveOccurred())

By("creating the 1st controller")
ctrl1, err := ControllerManagedBy(m).
For(&TestDefaultValidator{}).
Owns(&appsv1.ReplicaSet{}).
Build(noop)
Expect(err).NotTo(HaveOccurred())
Expect(ctrl1).NotTo(BeNil())

By("creating the 2nd controller")
ctrl2, err := ControllerManagedBy(m).
For(&TestDefaultValidator{}).
Owns(&appsv1.ReplicaSet{}).
Build(noop)
Expect(err).NotTo(HaveOccurred())
Expect(ctrl2).NotTo(BeNil())
})
})

Describe("Start with SimpleController", func() {
Expand Down

0 comments on commit ccc5417

Please sign in to comment.