Merge pull request #1490 from kevindelgado/standalone-webhooks-fix-scheme

🐛 Actually inject the scheme in StandaloneWebhook

Author: k8s-ci-robot

pkg/webhook/admission/admissiontest/doc.go

@@ -0,0 +1,18 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package admissiontest contains fake webhooks for validating admission webhooks
+package admissiontest

pkg/webhook/admission/admissiontest/util.go

@@ -0,0 +1,50 @@
+package admissiontest
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+// FakeValidator provides fake validating webhook functionality for testing
+// It implements the admission.Validator interface and
+// rejects all requests with the same configured error
+// or passes if ErrorToReturn is nil.
+type FakeValidator struct {
+	// ErrorToReturn is the error for which the FakeValidator rejects all requests
+	ErrorToReturn error `json:"ErrorToReturn,omitempty"`
+	// GVKToReturn is the GroupVersionKind that the webhook operates on
+	GVKToReturn schema.GroupVersionKind
+}
+
+// ValidateCreate implements admission.Validator
+func (v *FakeValidator) ValidateCreate() error {
+	return v.ErrorToReturn
+}
+
+// ValidateUpdate implements admission.Validator
+func (v *FakeValidator) ValidateUpdate(old runtime.Object) error {
+	return v.ErrorToReturn
+}
+
+// ValidateDelete implements admission.Validator
+func (v *FakeValidator) ValidateDelete() error {
+	return v.ErrorToReturn
+}
+
+// GetObjectKind implements admission.Validator
+func (v *FakeValidator) GetObjectKind() schema.ObjectKind { return v }
+
+// DeepCopyObject implements admission.Validator
+func (v *FakeValidator) DeepCopyObject() runtime.Object {
+	return &FakeValidator{ErrorToReturn: v.ErrorToReturn, GVKToReturn: v.GVKToReturn}
+}
+
+// GroupVersionKind implements admission.Validator
+func (v *FakeValidator) GroupVersionKind() schema.GroupVersionKind {
+	return v.GVKToReturn
+}
+
+// SetGroupVersionKind implements admission.Validator
+func (v *FakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) {
+	v.GVKToReturn = gvk
+}

pkg/webhook/admission/validator_test.go

@@ -7,6 +7,7 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission/admissiontest"
 
 	admissionv1 "k8s.io/api/admission/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
@@ -16,13 +17,15 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 )
 
+var fakeValidatorVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "fakeValidator"}
+
 var _ = Describe("validatingHandler", func() {
 
 	decoder, _ := NewDecoder(scheme.Scheme)
 
 	Context("when dealing with successful results", func() {
 
-		f := &fakeValidator{ErrorToReturn: nil}
+		f := &admissiontest.FakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK}
 		handler := validatingHandler{validator: f, decoder: decoder}
 
 		It("should return 200 in response when create succeeds", func() {
@@ -85,7 +88,7 @@ var _ = Describe("validatingHandler", func() {
 				Code:    http.StatusUnprocessableEntity,
 			},
 		}
-		f := &fakeValidator{ErrorToReturn: expectedError}
+		f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK}
 		handler := validatingHandler{validator: f, decoder: decoder}
 
 		It("should propagate the Status from ValidateCreate's return value to the HTTP response", func() {
@@ -150,7 +153,7 @@ var _ = Describe("validatingHandler", func() {
 	Context("when dealing with non-status errors", func() {
 
 		expectedError := goerrors.New("some error")
-		f := &fakeValidator{ErrorToReturn: expectedError}
+		f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK}
 		handler := validatingHandler{validator: f, decoder: decoder}
 
 		It("should return 403 response when ValidateCreate with error message embedded", func() {
@@ -219,35 +222,3 @@ var _ = Describe("validatingHandler", func() {
 	PIt("should return 400 in response when delete fails on decode", func() {})
 
 })
-
-type fakeValidator struct {
-	ErrorToReturn error `json:"ErrorToReturn,omitempty"`
-}
-
-var _ Validator = &fakeValidator{}
-
-var fakeValidatorVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "fakeValidator"}
-
-func (v *fakeValidator) ValidateCreate() error {
-	return v.ErrorToReturn
-}
-
-func (v *fakeValidator) ValidateUpdate(old runtime.Object) error {
-	return v.ErrorToReturn
-}
-
-func (v *fakeValidator) ValidateDelete() error {
-	return v.ErrorToReturn
-}
-
-func (v *fakeValidator) GetObjectKind() schema.ObjectKind { return v }
-
-func (v *fakeValidator) DeepCopyObject() runtime.Object {
-	return &fakeValidator{ErrorToReturn: v.ErrorToReturn}
-}
-
-func (v *fakeValidator) GroupVersionKind() schema.GroupVersionKind {
-	return fakeValidatorVK
-}
-
-func (v *fakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) {}

pkg/webhook/admission/webhook.go

@@ -235,9 +235,7 @@ func StandaloneWebhook(hook *Webhook, opts StandaloneOptions) (http.Handler, err
 		opts.Scheme = scheme.Scheme
 	}
 
-	var err error
-	hook.decoder, err = NewDecoder(opts.Scheme)
-	if err != nil {
+	if err := hook.InjectScheme(opts.Scheme); err != nil {
 		return nil, err
 	}
 

pkg/webhook/webhook_integration_test.go

@@ -3,6 +3,7 @@ package webhook_test
 import (
 	"context"
 	"crypto/tls"
+	goerrors "errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -16,12 +17,14 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission/admissiontest"
 )
 
 var _ = Describe("Webhook", func() {
@@ -111,7 +114,8 @@ var _ = Describe("Webhook", func() {
 	Context("when running a standalone webhook", func() {
 		It("should reject create request for webhook that rejects all requests", func(done Done) {
 			ctx, cancel := context.WithCancel(context.Background())
-			// generate tls cfg
+
+			By("generating the TLS config")
 			certPath := filepath.Join(testenv.WebhookInstallOptions.LocalServingCertDir, "tls.crt")
 			keyPath := filepath.Join(testenv.WebhookInstallOptions.LocalServingCertDir, "tls.key")
 
@@ -126,18 +130,22 @@ var _ = Describe("Webhook", func() {
 				GetCertificate: certWatcher.GetCertificate,
 			}
 
-			// generate listener
+			By("generating the listener")
 			listener, err := tls.Listen("tcp",
 				net.JoinHostPort(testenv.WebhookInstallOptions.LocalServingHost,
 					strconv.Itoa(int(testenv.WebhookInstallOptions.LocalServingPort))), cfg)
 			Expect(err).NotTo(HaveOccurred())
 
-			// create and register the standalone webhook
-			hook, err := admission.StandaloneWebhook(&webhook.Admission{Handler: &rejectingValidator{}}, admission.StandaloneOptions{})
+			By("creating and registering the standalone webhook")
+			hook, err := admission.StandaloneWebhook(admission.ValidatingWebhookFor(
+				&admissiontest.FakeValidator{
+					ErrorToReturn: goerrors.New("Always denied"),
+					GVKToReturn:   schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
+				}), admission.StandaloneOptions{})
 			Expect(err).NotTo(HaveOccurred())
 			http.Handle("/failing", hook)
 
-			// run the http server
+			By("running the http server")
 			srv := &http.Server{}
 			go func() {
 				idleConnsClosed := make(chan struct{})