From 19168b3e0549db98b88ad49f5af347d5ebe65bb6 Mon Sep 17 00:00:00 2001 From: Kevin Delgado Date: Fri, 23 Apr 2021 02:03:15 +0000 Subject: [PATCH] Actually inject the scheme in StandaloneWebhook --- pkg/webhook/admission/admissiontest/doc.go | 18 ++++++++ pkg/webhook/admission/admissiontest/util.go | 50 +++++++++++++++++++++ pkg/webhook/admission/validator_test.go | 41 +++-------------- pkg/webhook/admission/webhook.go | 4 +- pkg/webhook/webhook_integration_test.go | 18 +++++--- 5 files changed, 88 insertions(+), 43 deletions(-) create mode 100644 pkg/webhook/admission/admissiontest/doc.go create mode 100644 pkg/webhook/admission/admissiontest/util.go diff --git a/pkg/webhook/admission/admissiontest/doc.go b/pkg/webhook/admission/admissiontest/doc.go new file mode 100644 index 0000000000..b4a7a42191 --- /dev/null +++ b/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 diff --git a/pkg/webhook/admission/admissiontest/util.go b/pkg/webhook/admission/admissiontest/util.go new file mode 100644 index 0000000000..420390db58 --- /dev/null +++ b/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 +} diff --git a/pkg/webhook/admission/validator_test.go b/pkg/webhook/admission/validator_test.go index 57aadec8cc..8ee4f0b898 100644 --- a/pkg/webhook/admission/validator_test.go +++ b/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) {} diff --git a/pkg/webhook/admission/webhook.go b/pkg/webhook/admission/webhook.go index f0db50be81..f19a2a23ef 100644 --- a/pkg/webhook/admission/webhook.go +++ b/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 } diff --git a/pkg/webhook/webhook_integration_test.go b/pkg/webhook/webhook_integration_test.go index 2ab7b4e24f..3e97cce2d8 100644 --- a/pkg/webhook/webhook_integration_test.go +++ b/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{})