Skip to content

Commit bfe6610

Browse files
damsiendamsien
committed
Add custom path option for webhooks
Review correction
1 parent 98d85d4 commit bfe6610

File tree

2 files changed

+207
-4
lines changed

2 files changed

+207
-4
lines changed

‎pkg/builder/webhook.go

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"errors"
2121
"net/http"
2222
"net/url"
23+
"regexp"
2324
"strings"
2425

2526
"github.com/go-logr/logr"
@@ -39,6 +40,7 @@ type WebhookBuilder struct {
3940
apiType runtime.Object
4041
customDefaulter admission.CustomDefaulter
4142
customValidator admission.CustomValidator
43+
customPath string
4244
gvk schema.GroupVersionKind
4345
mgr manager.Manager
4446
config *rest.Config
@@ -90,6 +92,12 @@ func (blder *WebhookBuilder) RecoverPanic(recoverPanic bool) *WebhookBuilder {
9092
return blder
9193
}
9294

95+
// WithCustomPath overrides the webhook's default path by the customPath
96+
func (blder *WebhookBuilder) WithCustomPath(customPath string) *WebhookBuilder {
97+
blder.customPath = customPath
98+
return blder
99+
}
100+
93101
// Complete builds the webhook.
94102
func (blder *WebhookBuilder) Complete() error {
95103
// Set the Config
@@ -140,8 +148,15 @@ func (blder *WebhookBuilder) registerWebhooks() error {
140148
}
141149

142150
// Register webhook(s) for type
143-
blder.registerDefaultingWebhook()
144-
blder.registerValidatingWebhook()
151+
err = blder.registerDefaultingWebhook()
152+
if err != nil {
153+
return err
154+
}
155+
156+
err = blder.registerValidatingWebhook()
157+
if err != nil {
158+
return err
159+
}
145160

146161
err = blder.registerConversionWebhook()
147162
if err != nil {
@@ -151,11 +166,18 @@ func (blder *WebhookBuilder) registerWebhooks() error {
151166
}
152167

153168
// registerDefaultingWebhook registers a defaulting webhook if necessary.
154-
func (blder *WebhookBuilder) registerDefaultingWebhook() {
169+
func (blder *WebhookBuilder) registerDefaultingWebhook() error {
155170
mwh := blder.getDefaultingWebhook()
156171
if mwh != nil {
157172
mwh.LogConstructor = blder.logConstructor
158173
path := generateMutatePath(blder.gvk)
174+
if blder.customPath != "" {
175+
generatedCustomPath, err := generateCustomPath(blder.customPath)
176+
if err != nil {
177+
return err
178+
}
179+
path = generatedCustomPath
180+
}
159181

160182
// Checking if the path is already registered.
161183
// If so, just skip it.
@@ -166,6 +188,8 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
166188
blder.mgr.GetWebhookServer().Register(path, mwh)
167189
}
168190
}
191+
192+
return nil
169193
}
170194

171195
func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
@@ -180,11 +204,18 @@ func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
180204
}
181205

182206
// registerValidatingWebhook registers a validating webhook if necessary.
183-
func (blder *WebhookBuilder) registerValidatingWebhook() {
207+
func (blder *WebhookBuilder) registerValidatingWebhook() error {
184208
vwh := blder.getValidatingWebhook()
185209
if vwh != nil {
186210
vwh.LogConstructor = blder.logConstructor
187211
path := generateValidatePath(blder.gvk)
212+
if blder.customPath != "" {
213+
generatedCustomPath, err := generateCustomPath(blder.customPath)
214+
if err != nil {
215+
return err
216+
}
217+
path = generatedCustomPath
218+
}
188219

189220
// Checking if the path is already registered.
190221
// If so, just skip it.
@@ -195,6 +226,8 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {
195226
blder.mgr.GetWebhookServer().Register(path, vwh)
196227
}
197228
}
229+
230+
return nil
198231
}
199232

200233
func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook {
@@ -251,3 +284,14 @@ func generateValidatePath(gvk schema.GroupVersionKind) string {
251284
return "/validate-" + strings.ReplaceAll(gvk.Group, ".", "-") + "-" +
252285
gvk.Version + "-" + strings.ToLower(gvk.Kind)
253286
}
287+
288+
const webhookPathStringValidation = `^((/[a-zA-Z0-9-_]+)+|/)$`
289+
290+
var validWebhookPathRegex = regexp.MustCompile(webhookPathStringValidation)
291+
292+
func generateCustomPath(customPath string) (string, error) {
293+
if !validWebhookPathRegex.MatchString(customPath) {
294+
return "", errors.New("customPath \"" + customPath + "\" does not match this regex: " + webhookPathStringValidation)
295+
}
296+
return customPath, nil
297+
}

‎pkg/builder/webhook_test.go

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,85 @@ func runTests(admissionReviewVersion string) {
153153
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
154154
})
155155

156+
It("should scaffold a custom defaulting webhook with a custom path", func() {
157+
By("creating a controller manager")
158+
m, err := manager.New(cfg, manager.Options{})
159+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
160+
161+
By("registering the type in the Scheme")
162+
builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()}
163+
builder.Register(&TestDefaulter{}, &TestDefaulterList{})
164+
err = builder.AddToScheme(m.GetScheme())
165+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
166+
167+
customPath := "/custom-defaulting-path"
168+
err = WebhookManagedBy(m).
169+
For(&TestDefaulter{}).
170+
WithDefaulter(&TestCustomDefaulter{}).
171+
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
172+
return admission.DefaultLogConstructor(testingLogger, req)
173+
}).
174+
WithCustomPath(customPath).
175+
Complete()
176+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
177+
svr := m.GetWebhookServer()
178+
ExpectWithOffset(1, svr).NotTo(BeNil())
179+
180+
reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `",
181+
"request":{
182+
"uid":"07e52e8d-4513-11e9-a716-42010a800270",
183+
"kind":{
184+
"group":"foo.test.org",
185+
"version":"v1",
186+
"kind":"TestDefaulter"
187+
},
188+
"resource":{
189+
"group":"foo.test.org",
190+
"version":"v1",
191+
"resource":"testdefaulter"
192+
},
193+
"namespace":"default",
194+
"name":"foo",
195+
"operation":"CREATE",
196+
"object":{
197+
"replica":1
198+
},
199+
"oldObject":null
200+
}
201+
}`)
202+
203+
ctx, cancel := context.WithCancel(context.Background())
204+
cancel()
205+
err = svr.Start(ctx)
206+
if err != nil && !os.IsNotExist(err) {
207+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
208+
}
209+
210+
By("sending a request to a mutating webhook path")
211+
path, err := generateCustomPath(customPath)
212+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
213+
req := httptest.NewRequest("POST", svcBaseAddr+path, reader)
214+
req.Header.Add("Content-Type", "application/json")
215+
w := httptest.NewRecorder()
216+
svr.WebhookMux().ServeHTTP(w, req)
217+
ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK))
218+
By("sanity checking the response contains reasonable fields")
219+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`))
220+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`))
221+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`))
222+
EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaulter"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
223+
224+
By("sending a request to a mutating webhook path that have been overrided by the custom path")
225+
path = generateMutatePath(testDefaulterGVK)
226+
_, err = reader.Seek(0, 0)
227+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
228+
req = httptest.NewRequest("POST", svcBaseAddr+path, reader)
229+
req.Header.Add("Content-Type", "application/json")
230+
w = httptest.NewRecorder()
231+
svr.WebhookMux().ServeHTTP(w, req)
232+
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
233+
})
234+
156235
It("should scaffold a custom defaulting webhook which recovers from panics", func() {
157236
By("creating a controller manager")
158237
m, err := manager.New(cfg, manager.Options{})
@@ -294,6 +373,86 @@ func runTests(admissionReviewVersion string) {
294373
EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
295374
})
296375

376+
It("should scaffold a custom validating webhook with a custom path", func() {
377+
By("creating a controller manager")
378+
m, err := manager.New(cfg, manager.Options{})
379+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
380+
381+
By("registering the type in the Scheme")
382+
builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()}
383+
builder.Register(&TestValidator{}, &TestValidatorList{})
384+
err = builder.AddToScheme(m.GetScheme())
385+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
386+
387+
customPath := "/custom-validating-path"
388+
err = WebhookManagedBy(m).
389+
For(&TestValidator{}).
390+
WithValidator(&TestCustomValidator{}).
391+
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
392+
return admission.DefaultLogConstructor(testingLogger, req)
393+
}).
394+
WithCustomPath(customPath).
395+
Complete()
396+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
397+
svr := m.GetWebhookServer()
398+
ExpectWithOffset(1, svr).NotTo(BeNil())
399+
400+
reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `",
401+
"request":{
402+
"uid":"07e52e8d-4513-11e9-a716-42010a800270",
403+
"kind":{
404+
"group":"foo.test.org",
405+
"version":"v1",
406+
"kind":"TestValidator"
407+
},
408+
"resource":{
409+
"group":"foo.test.org",
410+
"version":"v1",
411+
"resource":"testvalidator"
412+
},
413+
"namespace":"default",
414+
"name":"foo",
415+
"operation":"UPDATE",
416+
"object":{
417+
"replica":1
418+
},
419+
"oldObject":{
420+
"replica":2
421+
}
422+
}
423+
}`)
424+
425+
ctx, cancel := context.WithCancel(context.Background())
426+
cancel()
427+
err = svr.Start(ctx)
428+
if err != nil && !os.IsNotExist(err) {
429+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
430+
}
431+
432+
By("sending a request to a mutating webhook path that have been overrided by a custom path")
433+
path := generateValidatePath(testValidatorGVK)
434+
req := httptest.NewRequest("POST", svcBaseAddr+path, reader)
435+
req.Header.Add("Content-Type", "application/json")
436+
w := httptest.NewRecorder()
437+
svr.WebhookMux().ServeHTTP(w, req)
438+
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
439+
440+
By("sending a request to a validating webhook path")
441+
path, err = generateCustomPath(customPath)
442+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
443+
_, err = reader.Seek(0, 0)
444+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
445+
req = httptest.NewRequest("POST", svcBaseAddr+path, reader)
446+
req.Header.Add("Content-Type", "application/json")
447+
w = httptest.NewRecorder()
448+
svr.WebhookMux().ServeHTTP(w, req)
449+
ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK))
450+
By("sanity checking the response contains reasonable field")
451+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
452+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
453+
EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
454+
})
455+
297456
It("should scaffold a custom validating webhook which recovers from panics", func() {
298457
By("creating a controller manager")
299458
m, err := manager.New(cfg, manager.Options{})

0 commit comments

Comments
 (0)