Skip to content

Commit a4c56b0

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1679 from k8s-infra-cherrypick-robot/cherry-pick-1676-to-release-0.10
✨ Allow webhooks to register custom validators/defaulter types
2 parents 9d7bcf7 + 4086785 commit a4c56b0

File tree

5 files changed

+461
-18
lines changed

5 files changed

+461
-18
lines changed

‎pkg/builder/webhook.go

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package builder
1818

1919
import (
20+
"errors"
2021
"net/http"
2122
"net/url"
2223
"strings"
@@ -32,10 +33,12 @@ import (
3233

3334
// WebhookBuilder builds a Webhook.
3435
type WebhookBuilder struct {
35-
apiType runtime.Object
36-
gvk schema.GroupVersionKind
37-
mgr manager.Manager
38-
config *rest.Config
36+
apiType runtime.Object
37+
withDefaulter admission.CustomDefaulter
38+
withValidator admission.CustomValidator
39+
gvk schema.GroupVersionKind
40+
mgr manager.Manager
41+
config *rest.Config
3942
}
4043

4144
// WebhookManagedBy allows inform its manager.Manager.
@@ -53,6 +56,18 @@ func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder {
5356
return blder
5457
}
5558

59+
// WithDefaulter takes a admission.WithDefaulter interface, a MutatingWebhook will be wired for this type.
60+
func (blder *WebhookBuilder) WithDefaulter(defaulter admission.CustomDefaulter) *WebhookBuilder {
61+
blder.withDefaulter = defaulter
62+
return blder
63+
}
64+
65+
// WithValidator takes a admission.WithValidator interface, a ValidatingWebhook will be wired for this type.
66+
func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator) *WebhookBuilder {
67+
blder.withValidator = validator
68+
return blder
69+
}
70+
5671
// Complete builds the webhook.
5772
func (blder *WebhookBuilder) Complete() error {
5873
// Set the Config
@@ -69,9 +84,13 @@ func (blder *WebhookBuilder) loadRestConfig() {
6984
}
7085

7186
func (blder *WebhookBuilder) registerWebhooks() error {
87+
typ, err := blder.getType()
88+
if err != nil {
89+
return err
90+
}
91+
7292
// Create webhook(s) for each type
73-
var err error
74-
blder.gvk, err = apiutil.GVKForObject(blder.apiType, blder.mgr.GetScheme())
93+
blder.gvk, err = apiutil.GVKForObject(typ, blder.mgr.GetScheme())
7594
if err != nil {
7695
return err
7796
}
@@ -88,12 +107,7 @@ func (blder *WebhookBuilder) registerWebhooks() error {
88107

89108
// registerDefaultingWebhook registers a defaulting webhook if th.
90109
func (blder *WebhookBuilder) registerDefaultingWebhook() {
91-
defaulter, isDefaulter := blder.apiType.(admission.Defaulter)
92-
if !isDefaulter {
93-
log.Info("skip registering a mutating webhook, admission.Defaulter interface is not implemented", "GVK", blder.gvk)
94-
return
95-
}
96-
mwh := admission.DefaultingWebhookFor(defaulter)
110+
mwh := blder.getDefaultingWebhook()
97111
if mwh != nil {
98112
path := generateMutatePath(blder.gvk)
99113

@@ -108,13 +122,21 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
108122
}
109123
}
110124

111-
func (blder *WebhookBuilder) registerValidatingWebhook() {
112-
validator, isValidator := blder.apiType.(admission.Validator)
113-
if !isValidator {
114-
log.Info("skip registering a validating webhook, admission.Validator interface is not implemented", "GVK", blder.gvk)
115-
return
125+
func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
126+
if defaulter := blder.withDefaulter; defaulter != nil {
127+
return admission.WithCustomDefaulter(blder.apiType, defaulter)
128+
}
129+
if defaulter, ok := blder.apiType.(admission.Defaulter); ok {
130+
return admission.DefaultingWebhookFor(defaulter)
116131
}
117-
vwh := admission.ValidatingWebhookFor(validator)
132+
log.Info(
133+
"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called",
134+
"GVK", blder.gvk)
135+
return nil
136+
}
137+
138+
func (blder *WebhookBuilder) registerValidatingWebhook() {
139+
vwh := blder.getValidatingWebhook()
118140
if vwh != nil {
119141
path := generateValidatePath(blder.gvk)
120142

@@ -129,6 +151,19 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {
129151
}
130152
}
131153

154+
func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook {
155+
if validator := blder.withValidator; validator != nil {
156+
return admission.WithCustomValidator(blder.apiType, validator)
157+
}
158+
if validator, ok := blder.apiType.(admission.Validator); ok {
159+
return admission.ValidatingWebhookFor(validator)
160+
}
161+
log.Info(
162+
"skip registering a validating webhook, object does not implement admission.Validator or WithValidator wasn't called",
163+
"GVK", blder.gvk)
164+
return nil
165+
}
166+
132167
func (blder *WebhookBuilder) registerConversionWebhook() error {
133168
ok, err := conversion.IsConvertible(blder.mgr.GetScheme(), blder.apiType)
134169
if err != nil {
@@ -145,6 +180,13 @@ func (blder *WebhookBuilder) registerConversionWebhook() error {
145180
return nil
146181
}
147182

183+
func (blder *WebhookBuilder) getType() (runtime.Object, error) {
184+
if blder.apiType != nil {
185+
return blder.apiType, nil
186+
}
187+
return nil, errors.New("For() must be called with a valid object")
188+
}
189+
148190
func (blder *WebhookBuilder) isAlreadyHandled(path string) bool {
149191
if blder.mgr.GetWebhookServer().WebhookMux == nil {
150192
return false

‎pkg/builder/webhook_test.go

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,81 @@ func runTests(admissionReviewVersion string) {
134134
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
135135
})
136136

137+
It("should scaffold a defaulting webhook with a custom defaulter", func() {
138+
By("creating a controller manager")
139+
m, err := manager.New(cfg, manager.Options{})
140+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
141+
142+
By("registering the type in the Scheme")
143+
builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()}
144+
builder.Register(&TestDefaulter{}, &TestDefaulterList{})
145+
err = builder.AddToScheme(m.GetScheme())
146+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
147+
148+
err = WebhookManagedBy(m).
149+
WithDefaulter(&TestCustomDefaulter{}).
150+
For(&TestDefaulter{}).
151+
Complete()
152+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
153+
svr := m.GetWebhookServer()
154+
ExpectWithOffset(1, svr).NotTo(BeNil())
155+
156+
reader := strings.NewReader(`{
157+
"kind":"AdmissionReview",
158+
"apiVersion":"admission.k8s.io/` + admissionReviewVersion + `",
159+
"request":{
160+
"uid":"07e52e8d-4513-11e9-a716-42010a800270",
161+
"kind":{
162+
"group":"",
163+
"version":"v1",
164+
"kind":"TestDefaulter"
165+
},
166+
"resource":{
167+
"group":"",
168+
"version":"v1",
169+
"resource":"testdefaulter"
170+
},
171+
"namespace":"default",
172+
"operation":"CREATE",
173+
"object":{
174+
"replica":1
175+
},
176+
"oldObject":null
177+
}
178+
}`)
179+
180+
ctx, cancel := context.WithCancel(context.Background())
181+
cancel()
182+
// TODO: we may want to improve it to make it be able to inject dependencies,
183+
// but not always try to load certs and return not found error.
184+
err = svr.Start(ctx)
185+
if err != nil && !os.IsNotExist(err) {
186+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
187+
}
188+
189+
By("sending a request to a mutating webhook path")
190+
path := generateMutatePath(testDefaulterGVK)
191+
req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
192+
req.Header.Add("Content-Type", "application/json")
193+
w := httptest.NewRecorder()
194+
svr.WebhookMux.ServeHTTP(w, req)
195+
ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK))
196+
By("sanity checking the response contains reasonable fields")
197+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`))
198+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`))
199+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`))
200+
201+
By("sending a request to a validating webhook path that doesn't exist")
202+
path = generateValidatePath(testDefaulterGVK)
203+
_, err = reader.Seek(0, 0)
204+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
205+
req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
206+
req.Header.Add("Content-Type", "application/json")
207+
w = httptest.NewRecorder()
208+
svr.WebhookMux.ServeHTTP(w, req)
209+
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
210+
})
211+
137212
It("should scaffold a validating webhook if the type implements the Validator interface", func() {
138213
By("creating a controller manager")
139214
m, err := manager.New(cfg, manager.Options{})
@@ -209,6 +284,82 @@ func runTests(admissionReviewVersion string) {
209284
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
210285
})
211286

287+
It("should scaffold a validating webhook with a custom validator", func() {
288+
By("creating a controller manager")
289+
m, err := manager.New(cfg, manager.Options{})
290+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
291+
292+
By("registering the type in the Scheme")
293+
builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()}
294+
builder.Register(&TestValidator{}, &TestValidatorList{})
295+
err = builder.AddToScheme(m.GetScheme())
296+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
297+
298+
err = WebhookManagedBy(m).
299+
WithValidator(&TestCustomValidator{}).
300+
For(&TestValidator{}).
301+
Complete()
302+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
303+
svr := m.GetWebhookServer()
304+
ExpectWithOffset(1, svr).NotTo(BeNil())
305+
306+
reader := strings.NewReader(`{
307+
"kind":"AdmissionReview",
308+
"apiVersion":"admission.k8s.io/` + admissionReviewVersion + `",
309+
"request":{
310+
"uid":"07e52e8d-4513-11e9-a716-42010a800270",
311+
"kind":{
312+
"group":"",
313+
"version":"v1",
314+
"kind":"TestValidator"
315+
},
316+
"resource":{
317+
"group":"",
318+
"version":"v1",
319+
"resource":"testvalidator"
320+
},
321+
"namespace":"default",
322+
"operation":"UPDATE",
323+
"object":{
324+
"replica":1
325+
},
326+
"oldObject":{
327+
"replica":2
328+
}
329+
}
330+
}`)
331+
332+
ctx, cancel := context.WithCancel(context.Background())
333+
cancel()
334+
// TODO: we may want to improve it to make it be able to inject dependencies,
335+
// but not always try to load certs and return not found error.
336+
err = svr.Start(ctx)
337+
if err != nil && !os.IsNotExist(err) {
338+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
339+
}
340+
341+
By("sending a request to a mutating webhook path that doesn't exist")
342+
path := generateMutatePath(testValidatorGVK)
343+
req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
344+
req.Header.Add("Content-Type", "application/json")
345+
w := httptest.NewRecorder()
346+
svr.WebhookMux.ServeHTTP(w, req)
347+
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
348+
349+
By("sending a request to a validating webhook path")
350+
path = generateValidatePath(testValidatorGVK)
351+
_, err = reader.Seek(0, 0)
352+
ExpectWithOffset(1, err).NotTo(HaveOccurred())
353+
req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
354+
req.Header.Add("Content-Type", "application/json")
355+
w = httptest.NewRecorder()
356+
svr.WebhookMux.ServeHTTP(w, req)
357+
ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK))
358+
By("sanity checking the response contains reasonable field")
359+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
360+
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
361+
})
362+
212363
It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() {
213364
By("creating a controller manager")
214365
m, err := manager.New(cfg, manager.Options{})
@@ -537,3 +688,51 @@ func (dv *TestDefaultValidator) ValidateDelete() error {
537688
}
538689
return nil
539690
}
691+
692+
// TestCustomDefaulter.
693+
694+
type TestCustomDefaulter struct{}
695+
696+
func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
697+
d := obj.(*TestDefaulter) //nolint:ifshort
698+
if d.Replica < 2 {
699+
d.Replica = 2
700+
}
701+
return nil
702+
}
703+
704+
var _ admission.CustomDefaulter = &TestCustomDefaulter{}
705+
706+
// TestCustomValidator.
707+
708+
type TestCustomValidator struct{}
709+
710+
func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error {
711+
v := obj.(*TestValidator) //nolint:ifshort
712+
if v.Replica < 0 {
713+
return errors.New("number of replica should be greater than or equal to 0")
714+
}
715+
return nil
716+
}
717+
718+
func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
719+
v := newObj.(*TestValidator)
720+
old := oldObj.(*TestValidator) //nolint:ifshort
721+
if v.Replica < 0 {
722+
return errors.New("number of replica should be greater than or equal to 0")
723+
}
724+
if v.Replica < old.Replica {
725+
return fmt.Errorf("new replica %v should not be fewer than old replica %v", v.Replica, old.Replica)
726+
}
727+
return nil
728+
}
729+
730+
func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
731+
v := obj.(*TestValidator) //nolint:ifshort
732+
if v.Replica > 0 {
733+
return errors.New("number of replica should be less than or equal to 0 to delete")
734+
}
735+
return nil
736+
}
737+
738+
var _ admission.CustomValidator = &TestCustomValidator{}

0 commit comments

Comments
 (0)