Skip to content

Commit 9cf5a86

Browse files
damsiendamsien
committed
Add custom path option for webhooks
Review correction
1 parent 34beb4c commit 9cf5a86

File tree

2 files changed

+208
-4
lines changed

2 files changed

+208
-4
lines changed

‎pkg/builder/webhook.go

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"errors"
2121
"net/http"
2222
"net/url"
23+
"path"
24+
"regexp"
2325
"strings"
2426

2527
"github.com/go-logr/logr"
@@ -39,6 +41,7 @@ type WebhookBuilder struct {
3941
apiType runtime.Object
4042
customDefaulter admission.CustomDefaulter
4143
customValidator admission.CustomValidator
44+
customPath string
4245
gvk schema.GroupVersionKind
4346
mgr manager.Manager
4447
config *rest.Config
@@ -90,6 +93,12 @@ func (blder *WebhookBuilder) RecoverPanic(recoverPanic bool) *WebhookBuilder {
9093
return blder
9194
}
9295

96+
// WithCustomPath overrides the webhook's default path by the customPath
97+
func (blder *WebhookBuilder) WithCustomPath(customPath string) *WebhookBuilder {
98+
blder.customPath = customPath
99+
return blder
100+
}
101+
93102
// Complete builds the webhook.
94103
func (blder *WebhookBuilder) Complete() error {
95104
// Set the Config
@@ -140,8 +149,15 @@ func (blder *WebhookBuilder) registerWebhooks() error {
140149
}
141150

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

146162
err = blder.registerConversionWebhook()
147163
if err != nil {
@@ -151,11 +167,18 @@ func (blder *WebhookBuilder) registerWebhooks() error {
151167
}
152168

153169
// registerDefaultingWebhook registers a defaulting webhook if necessary.
154-
func (blder *WebhookBuilder) registerDefaultingWebhook() {
170+
func (blder *WebhookBuilder) registerDefaultingWebhook() error {
155171
mwh := blder.getDefaultingWebhook()
156172
if mwh != nil {
157173
mwh.LogConstructor = blder.logConstructor
158174
path := generateMutatePath(blder.gvk)
175+
if blder.customPath != "" {
176+
generatedCustomPath, err := generateCustomPath(blder.customPath)
177+
if err != nil {
178+
return err
179+
}
180+
path = generatedCustomPath
181+
}
159182

160183
// Checking if the path is already registered.
161184
// If so, just skip it.
@@ -166,6 +189,8 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
166189
blder.mgr.GetWebhookServer().Register(path, mwh)
167190
}
168191
}
192+
193+
return nil
169194
}
170195

171196
func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
@@ -180,11 +205,18 @@ func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
180205
}
181206

182207
// registerValidatingWebhook registers a validating webhook if necessary.
183-
func (blder *WebhookBuilder) registerValidatingWebhook() {
208+
func (blder *WebhookBuilder) registerValidatingWebhook() error {
184209
vwh := blder.getValidatingWebhook()
185210
if vwh != nil {
186211
vwh.LogConstructor = blder.logConstructor
187212
path := generateValidatePath(blder.gvk)
213+
if blder.customPath != "" {
214+
generatedCustomPath, err := generateCustomPath(blder.customPath)
215+
if err != nil {
216+
return err
217+
}
218+
path = generatedCustomPath
219+
}
188220

189221
// Checking if the path is already registered.
190222
// If so, just skip it.
@@ -195,6 +227,8 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {
195227
blder.mgr.GetWebhookServer().Register(path, vwh)
196228
}
197229
}
230+
231+
return nil
198232
}
199233

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

‎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)