⚠ Remove deprecated Defaulter and Validator interfaces (#2877)

* remove deprecated Defaulter and Validator interfaces Signed-off-by: Troy Connor <troy0820@users.noreply.github.com> * remove log, add test for customValidator/customDefaulter Signed-off-by: Troy Connor <troy0820@users.noreply.github.com> * remove deprecated funcs, add test for customDefaulter Signed-off-by: Troy Connor <troy0820@users.noreply.github.com> * minimize the diff Signed-off-by: Troy Connor <troy0820@users.noreply.github.com> --------- Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>

Author: troy0820

pkg/builder/webhook.go

@@ -176,16 +176,6 @@ func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
 }
 return w
 }
-if defaulter, ok := blder.apiType.(admission.Defaulter); ok {
-w := admission.DefaultingWebhookFor(blder.mgr.GetScheme(), defaulter)
-if blder.recoverPanic != nil {
-w = w.WithRecoverPanic(*blder.recoverPanic)
-}
-return w
-}
-log.Info(
-"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called",
-"GVK", blder.gvk)
 return nil
 }
 
@@ -215,16 +205,6 @@ func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook {
 }
 return w
 }
-if validator, ok := blder.apiType.(admission.Validator); ok {
-w := admission.ValidatingWebhookFor(blder.mgr.GetScheme(), validator)
-if blder.recoverPanic != nil {
-w = w.WithRecoverPanic(*blder.recoverPanic)
-}
-return w
-}
-log.Info(
-"skip registering a validating webhook, object does not implement admission.Validator or WithValidator wasn't called",
-"GVK", blder.gvk)
 return nil
 }
 

pkg/builder/webhook_test.go

@@ -77,7 +77,7 @@ func runTests(admissionReviewVersion string) {
 close(stop)
 })
 
-It("should scaffold a defaulting webhook if the type implements the Defaulter interface", func() {
+It("should scaffold a custom defaulting webhook if the type implements the CustomDefaulter interface", func() {
 By("creating a controller manager")
 m, err := manager.New(cfg, manager.Options{})
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -90,6 +90,7 @@ func runTests(admissionReviewVersion string) {
 
 err = WebhookManagedBy(m).
 For(&TestDefaulter{}).
+WithDefaulter(&TestCustomDefaulter{}).
 Complete()
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
 svr := m.GetWebhookServer()
@@ -147,7 +148,7 @@ func runTests(admissionReviewVersion string) {
 ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
 })
 
-It("should scaffold a defaulting webhook which recovers from panics", func() {
+It("should scaffold a custom defaulting webhook which recovers from panics", func() {
 By("creating a controller manager")
 m, err := manager.New(cfg, manager.Options{})
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -159,7 +160,9 @@ func runTests(admissionReviewVersion string) {
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
 
 err = WebhookManagedBy(m).
-For(&TestDefaulter{Panic: true}).
+For(&TestDefaulter{}).
+WithDefaulter(&TestCustomDefaulter{}).
+RecoverPanic(true).
 // RecoverPanic defaults to true.
 Complete()
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -285,7 +288,7 @@ func runTests(admissionReviewVersion string) {
 ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
 })
 
-It("should scaffold a validating webhook if the type implements the Validator interface", func() {
+It("should scaffold a custom validating webhook if the type implements the CustomValidator interface", func() {
 By("creating a controller manager")
 m, err := manager.New(cfg, manager.Options{})
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -298,6 +301,7 @@ func runTests(admissionReviewVersion string) {
 
 err = WebhookManagedBy(m).
 For(&TestValidator{}).
+WithValidator(&TestCustomValidator{}).
 Complete()
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
 svr := m.GetWebhookServer()
@@ -356,7 +360,7 @@ func runTests(admissionReviewVersion string) {
 ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
 })
 
-It("should scaffold a validating webhook which recovers from panics", func() {
+It("should scaffold a custom validating webhook which recovers from panics", func() {
 By("creating a controller manager")
 m, err := manager.New(cfg, manager.Options{})
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -368,7 +372,8 @@ func runTests(admissionReviewVersion string) {
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
 
 err = WebhookManagedBy(m).
-For(&TestValidator{Panic: true}).
+For(&TestValidator{}).
+WithValidator(&TestCustomValidator{}).
 RecoverPanic(true).
 Complete()
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -496,80 +501,7 @@ func runTests(admissionReviewVersion string) {
 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"`))
 })
 
-It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() {
-By("creating a controller manager")
-m, err := manager.New(cfg, manager.Options{})
-ExpectWithOffset(1, 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())
-ExpectWithOffset(1, err).NotTo(HaveOccurred())
-
-err = WebhookManagedBy(m).
-For(&TestDefaultValidator{}).
-Complete()
-ExpectWithOffset(1, err).NotTo(HaveOccurred())
-svr := m.GetWebhookServer()
-ExpectWithOffset(1, svr).NotTo(BeNil())
-
-reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `",
- "request":{
- "uid":"07e52e8d-4513-11e9-a716-42010a800270",
- "kind":{
- "group":"",
- "version":"v1",
- "kind":"TestDefaultValidator"
- },
- "resource":{
- "group":"",
- "version":"v1",
- "resource":"testdefaultvalidator"
- },
- "namespace":"default",
- "operation":"CREATE",
- "object":{
- "replica":1
- },
- "oldObject":null
- }
-}`)
-
-ctx, cancel := context.WithCancel(context.Background())
-cancel()
-err = svr.Start(ctx)
-if err != nil && !os.IsNotExist(err) {
-ExpectWithOffset(1, err).NotTo(HaveOccurred())
-}
-
-By("sending a request to a mutating webhook path")
-path := generateMutatePath(testDefaultValidatorGVK)
-req := httptest.NewRequest("POST", svcBaseAddr+path, reader)
-req.Header.Add("Content-Type", "application/json")
-w := httptest.NewRecorder()
-svr.WebhookMux().ServeHTTP(w, req)
-ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK))
-By("sanity checking the response contains reasonable field")
-ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`))
-ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`))
-ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`))
-
-By("sending a request to a validating webhook path")
-path = generateValidatePath(testDefaultValidatorGVK)
-_, err = reader.Seek(0, 0)
-ExpectWithOffset(1, err).NotTo(HaveOccurred())
-req = httptest.NewRequest("POST", svcBaseAddr+path, reader)
-req.Header.Add("Content-Type", "application/json")
-w = httptest.NewRecorder()
-svr.WebhookMux().ServeHTTP(w, req)
-ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK))
-By("sanity checking the response contains reasonable field")
-ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`))
-ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`))
-})
-
-It("should scaffold a validating webhook if the type implements the Validator interface to validate deletes", func() {
+It("should scaffold a custom validating webhook if the type implements the CustomValidator interface to validate deletes", func() {
 By("creating a controller manager")
 ctx, cancel := context.WithCancel(context.Background())
 
@@ -584,6 +516,7 @@ func runTests(admissionReviewVersion string) {
 
 err = WebhookManagedBy(m).
 For(&TestValidator{}).
+WithValidator(&TestCustomValidator{}).
 Complete()
 ExpectWithOffset(1, err).NotTo(HaveOccurred())
 svr := m.GetWebhookServer()
@@ -712,15 +645,6 @@ type TestDefaulterList struct{}
 func (*TestDefaulterList) GetObjectKind() schema.ObjectKind { return nil }
 func (*TestDefaulterList) DeepCopyObject() runtime.Object { return nil }
 
-func (d *TestDefaulter) Default() {
-if d.Panic {
-panic("fake panic test")
-}
-if d.Replica < 2 {
-d.Replica = 2
-}
-}
-
 // TestValidator.
 var _ runtime.Object = &TestValidator{}
 
@@ -753,43 +677,6 @@ type TestValidatorList struct{}
 func (*TestValidatorList) GetObjectKind() schema.ObjectKind { return nil }
 func (*TestValidatorList) DeepCopyObject() runtime.Object { return nil }
 
-var _ admission.Validator = &TestValidator{}
-
-func (v *TestValidator) ValidateCreate() (admission.Warnings, error) {
-if v.Panic {
-panic("fake panic test")
-}
-if v.Replica < 0 {
-return nil, errors.New("number of replica should be greater than or equal to 0")
-}
-return nil, nil
-}
-
-func (v *TestValidator) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-if v.Panic {
-panic("fake panic test")
-}
-if v.Replica < 0 {
-return nil, errors.New("number of replica should be greater than or equal to 0")
-}
-if oldObj, ok := old.(*TestValidator); !ok {
-return nil, fmt.Errorf("the old object is expected to be %T", oldObj)
-} else if v.Replica < oldObj.Replica {
-return nil, fmt.Errorf("new replica %v should not be fewer than old replica %v", v.Replica, oldObj.Replica)
-}
-return nil, nil
-}
-
-func (v *TestValidator) ValidateDelete() (admission.Warnings, error) {
-if v.Panic {
-panic("fake panic test")
-}
-if v.Replica > 0 {
-return nil, errors.New("number of replica should be less than or equal to 0 to delete")
-}
-return nil, nil
-}
-
 // TestDefaultValidator.
 var _ runtime.Object = &TestDefaultValidator{}
 
@@ -822,37 +709,7 @@ type TestDefaultValidatorList struct{}
 func (*TestDefaultValidatorList) GetObjectKind() schema.ObjectKind { return nil }
 func (*TestDefaultValidatorList) DeepCopyObject() runtime.Object { return nil }
 
-func (dv *TestDefaultValidator) Default() {
-if dv.Replica < 2 {
-dv.Replica = 2
-}
-}
-
-var _ admission.Validator = &TestDefaultValidator{}
-
-func (dv *TestDefaultValidator) ValidateCreate() (admission.Warnings, error) {
-if dv.Replica < 0 {
-return nil, errors.New("number of replica should be greater than or equal to 0")
-}
-return nil, nil
-}
-
-func (dv *TestDefaultValidator) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-if dv.Replica < 0 {
-return nil, errors.New("number of replica should be greater than or equal to 0")
-}
-return nil, nil
-}
-
-func (dv *TestDefaultValidator) ValidateDelete() (admission.Warnings, error) {
-if dv.Replica > 0 {
-return nil, errors.New("number of replica should be less than or equal to 0 to delete")
-}
-return nil, nil
-}
-
 // TestCustomDefaulter.
-
 type TestCustomDefaulter struct{}
 
 func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
@@ -866,6 +723,10 @@ func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) err
 }
 
 d := obj.(*TestDefaulter) //nolint:ifshort
+if d.Panic {
+panic("fake panic test")
+}
+
 if d.Replica < 2 {
 d.Replica = 2
 }
@@ -889,6 +750,9 @@ func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Obje
 }
 
 v := obj.(*TestValidator) //nolint:ifshort
+if v.Panic {
+panic("fake panic test")
+}
 if v.Replica < 0 {
 return nil, errors.New("number of replica should be greater than or equal to 0")
 }