From 1ba644ef6b48377cb122e7c1b0487cbb913a8ee4 Mon Sep 17 00:00:00 2001 From: jennybuckley Date: Tue, 25 Sep 2018 11:21:00 -0700 Subject: [PATCH] Add e2e test for mutating webhooks affecting webhook-config objects --- test/e2e/apimachinery/webhook.go | 139 +++++++++++++++++++++++-------- test/images/webhook/BUILD | 1 + test/images/webhook/VERSION | 2 +- test/images/webhook/addlabel.go | 60 +++++++++++++ test/images/webhook/main.go | 5 ++ test/utils/image/manifest.go | 2 +- 6 files changed, 170 insertions(+), 39 deletions(-) create mode 100644 test/images/webhook/addlabel.go diff --git a/test/e2e/apimachinery/webhook.go b/test/e2e/apimachinery/webhook.go index ef942e18ad1..81dfd3af272 100644 --- a/test/e2e/apimachinery/webhook.go +++ b/test/e2e/apimachinery/webhook.go @@ -52,17 +52,18 @@ const ( roleBindingName = "webhook-auth-reader" // The webhook configuration names should not be reused between test instances. - crWebhookConfigName = "e2e-test-webhook-config-cr" - webhookConfigName = "e2e-test-webhook-config" - attachingPodWebhookConfigName = "e2e-test-webhook-config-attaching-pod" - mutatingWebhookConfigName = "e2e-test-mutating-webhook-config" - podMutatingWebhookConfigName = "e2e-test-mutating-webhook-pod" - crMutatingWebhookConfigName = "e2e-test-mutating-webhook-config-cr" - webhookFailClosedConfigName = "e2e-test-webhook-fail-closed" - webhookForWebhooksConfigName = "e2e-test-webhook-for-webhooks-config" - removableValidatingHookName = "e2e-test-should-be-removable-validating-webhook-config" - removableMutatingHookName = "e2e-test-should-be-removable-mutating-webhook-config" - crdWebhookConfigName = "e2e-test-webhook-config-crd" + crWebhookConfigName = "e2e-test-webhook-config-cr" + webhookConfigName = "e2e-test-webhook-config" + attachingPodWebhookConfigName = "e2e-test-webhook-config-attaching-pod" + mutatingWebhookConfigName = "e2e-test-mutating-webhook-config" + podMutatingWebhookConfigName = "e2e-test-mutating-webhook-pod" + crMutatingWebhookConfigName = "e2e-test-mutating-webhook-config-cr" + webhookFailClosedConfigName = "e2e-test-webhook-fail-closed" + validatingWebhookForWebhooksConfigName = "e2e-test-validating-webhook-for-webhooks-config" + mutatingWebhookForWebhooksConfigName = "e2e-test-mutating-webhook-for-webhooks-config" + dummyValidatingWebhookConfigName = "e2e-test-dummy-validating-webhook-config" + dummyMutatingWebhookConfigName = "e2e-test-dummy-mutating-webhook-config" + crdWebhookConfigName = "e2e-test-webhook-config-crd" skipNamespaceLabelKey = "skip-webhook-admission" skipNamespaceLabelValue = "yes" @@ -75,6 +76,8 @@ const ( failNamespaceLabelKey = "fail-closed-webhook" failNamespaceLabelValue = "yes" failNamespaceName = "fail-closed-namesapce" + addedLabelKey = "added-label" + addedLabelValue = "yes" ) var serverWebhookVersion = utilversion.MustParseSemantic("v1.8.0") @@ -154,10 +157,12 @@ var _ = SIGDescribe("AdmissionWebhook", func() { testMutatingPodWebhook(f) }) - It("Should not be able to prevent deleting validating-webhook-configurations or mutating-webhook-configurations", func() { - webhookCleanup := registerWebhookForWebhookConfigurations(f, context) - defer webhookCleanup() - testWebhookForWebhookConfigurations(f) + It("Should not be able to mutate or prevent deletion of webhook configuration objects", func() { + validatingWebhookCleanup := registerValidatingWebhookForWebhookConfigurations(f, context) + defer validatingWebhookCleanup() + mutatingWebhookCleanup := registerMutatingWebhookForWebhookConfigurations(f, context) + defer mutatingWebhookCleanup() + testWebhooksForWebhookConfigurations(f) }) It("Should mutate custom resource", func() { @@ -801,16 +806,18 @@ func testFailClosedWebhook(f *framework.Framework) { } } -func registerWebhookForWebhookConfigurations(f *framework.Framework, context *certContext) func() { +func registerValidatingWebhookForWebhookConfigurations(f *framework.Framework, context *certContext) func() { var err error client := f.ClientSet - By("Registering a webhook on ValidatingWebhookConfiguration and MutatingWebhookConfiguration objects, via the AdmissionRegistration API") + By("Registering a validating webhook on ValidatingWebhookConfiguration and MutatingWebhookConfiguration objects, via the AdmissionRegistration API") namespace := f.Namespace.Name - configName := webhookForWebhooksConfigName + configName := validatingWebhookForWebhooksConfigName failurePolicy := v1beta1.Fail - // This webhook will deny all requests to Delete admissionregistration objects + // This webhook denies all requests to Delete validating webhook configuration and + // mutating webhook configuration objects. It should never be called, however, because + // dynamic admission webhooks should not be called on requests involving webhook configuration objects. _, err = client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Create(&v1beta1.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: configName, @@ -841,7 +848,6 @@ func registerWebhookForWebhookConfigurations(f *framework.Framework, context *ce }, }, }) - framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) // The webhook configuration is honored in 10s. @@ -852,23 +858,76 @@ func registerWebhookForWebhookConfigurations(f *framework.Framework, context *ce } } -// This test assumes that the deletion-rejecting webhook defined in -// registerWebhookForWebhookConfigurations is in place. -func testWebhookForWebhookConfigurations(f *framework.Framework) { +func registerMutatingWebhookForWebhookConfigurations(f *framework.Framework, context *certContext) func() { var err error client := f.ClientSet - By("Creating a validating-webhook-configuration object") + By("Registering a mutating webhook on ValidatingWebhookConfiguration and MutatingWebhookConfiguration objects, via the AdmissionRegistration API") + + namespace := f.Namespace.Name + configName := mutatingWebhookForWebhooksConfigName + failurePolicy := v1beta1.Fail + + // This webhook adds a label to all requests create to validating webhook configuration and + // mutating webhook configuration objects. It should never be called, however, because + // dynamic admission webhooks should not be called on requests involving webhook configuration objects. + _, err = client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Create(&v1beta1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: configName, + }, + Webhooks: []v1beta1.Webhook{ + { + Name: "add-label-to-webhook-configurations.k8s.io", + Rules: []v1beta1.RuleWithOperations{{ + Operations: []v1beta1.OperationType{v1beta1.Create}, + Rule: v1beta1.Rule{ + APIGroups: []string{"admissionregistration.k8s.io"}, + APIVersions: []string{"*"}, + Resources: []string{ + "validatingwebhookconfigurations", + "mutatingwebhookconfigurations", + }, + }, + }}, + ClientConfig: v1beta1.WebhookClientConfig{ + Service: &v1beta1.ServiceReference{ + Namespace: namespace, + Name: serviceName, + Path: strPtr("/add-label"), + }, + CABundle: context.signingCert, + }, + FailurePolicy: &failurePolicy, + }, + }, + }) + framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) + + // The webhook configuration is honored in 10s. + time.Sleep(10 * time.Second) + return func() { + err := client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Delete(configName, nil) + framework.ExpectNoError(err, "deleting webhook config %s with namespace %s", configName, namespace) + } +} + +// This test assumes that the deletion-rejecting webhook defined in +// registerValidatingWebhookForWebhookConfigurations and the webhook-config-mutating +// webhook defined in registerMutatingWebhookForWebhookConfigurations already exist. +func testWebhooksForWebhookConfigurations(f *framework.Framework) { + var err error + client := f.ClientSet + By("Creating a dummy validating-webhook-configuration object") namespace := f.Namespace.Name failurePolicy := v1beta1.Ignore - _, err = client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Create(&v1beta1.ValidatingWebhookConfiguration{ + mutatedValidatingWebhookConfiguration, err := client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Create(&v1beta1.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ - Name: removableValidatingHookName, + Name: dummyValidatingWebhookConfigName, }, Webhooks: []v1beta1.Webhook{ { - Name: "should-be-removable-validating-webhook.k8s.io", + Name: "dummy-validating-webhook.k8s.io", Rules: []v1beta1.RuleWithOperations{{ Operations: []v1beta1.OperationType{v1beta1.Create}, // This will not match any real resources so this webhook should never be called. @@ -894,25 +953,28 @@ func testWebhookForWebhookConfigurations(f *framework.Framework) { }, }, }) - framework.ExpectNoError(err, "registering webhook config %s with namespace %s", removableValidatingHookName, namespace) + framework.ExpectNoError(err, "registering webhook config %s with namespace %s", dummyValidatingWebhookConfigName, namespace) + if mutatedValidatingWebhookConfiguration.ObjectMeta.Labels != nil && mutatedValidatingWebhookConfiguration.ObjectMeta.Labels[addedLabelKey] == addedLabelValue { + framework.Failf("expected %s not to be mutated by mutating webhooks but it was", dummyValidatingWebhookConfigName) + } // The webhook configuration is honored in 10s. time.Sleep(10 * time.Second) By("Deleting the validating-webhook-configuration, which should be possible to remove") - err = client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Delete(removableValidatingHookName, nil) - framework.ExpectNoError(err, "deleting webhook config %s with namespace %s", removableValidatingHookName, namespace) + err = client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Delete(dummyValidatingWebhookConfigName, nil) + framework.ExpectNoError(err, "deleting webhook config %s with namespace %s", dummyValidatingWebhookConfigName, namespace) - By("Creating a mutating-webhook-configuration object") + By("Creating a dummy mutating-webhook-configuration object") - _, err = client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Create(&v1beta1.MutatingWebhookConfiguration{ + mutatedMutatingWebhookConfiguration, err := client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Create(&v1beta1.MutatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ - Name: removableMutatingHookName, + Name: dummyMutatingWebhookConfigName, }, Webhooks: []v1beta1.Webhook{ { - Name: "should-be-removable-mutating-webhook.k8s.io", + Name: "dummy-mutating-webhook.k8s.io", Rules: []v1beta1.RuleWithOperations{{ Operations: []v1beta1.OperationType{v1beta1.Create}, // This will not match any real resources so this webhook should never be called. @@ -938,15 +1000,18 @@ func testWebhookForWebhookConfigurations(f *framework.Framework) { }, }, }) - framework.ExpectNoError(err, "registering webhook config %s with namespace %s", removableMutatingHookName, namespace) + framework.ExpectNoError(err, "registering webhook config %s with namespace %s", dummyMutatingWebhookConfigName, namespace) + if mutatedMutatingWebhookConfiguration.ObjectMeta.Labels != nil && mutatedMutatingWebhookConfiguration.ObjectMeta.Labels[addedLabelKey] == addedLabelValue { + framework.Failf("expected %s not to be mutated by mutating webhooks but it was", dummyMutatingWebhookConfigName) + } // The webhook configuration is honored in 10s. time.Sleep(10 * time.Second) By("Deleting the mutating-webhook-configuration, which should be possible to remove") - err = client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Delete(removableMutatingHookName, nil) - framework.ExpectNoError(err, "deleting webhook config %s with namespace %s", removableMutatingHookName, namespace) + err = client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Delete(dummyMutatingWebhookConfigName, nil) + framework.ExpectNoError(err, "deleting webhook config %s with namespace %s", dummyMutatingWebhookConfigName, namespace) } func createNamespace(f *framework.Framework, ns *v1.Namespace) error { diff --git a/test/images/webhook/BUILD b/test/images/webhook/BUILD index ac9bc7669f6..5d75f0e94dc 100644 --- a/test/images/webhook/BUILD +++ b/test/images/webhook/BUILD @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test") go_library( name = "go_default_library", srcs = [ + "addlabel.go", "alwaysdeny.go", "config.go", "configmap.go", diff --git a/test/images/webhook/VERSION b/test/images/webhook/VERSION index 1b67ceb7853..fe9e4faa6b2 100644 --- a/test/images/webhook/VERSION +++ b/test/images/webhook/VERSION @@ -1 +1 @@ -1.12v2 +1.13v1 diff --git a/test/images/webhook/addlabel.go b/test/images/webhook/addlabel.go new file mode 100644 index 00000000000..48ff86351cb --- /dev/null +++ b/test/images/webhook/addlabel.go @@ -0,0 +1,60 @@ +/* +Copyright 2018 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 main + +import ( + "encoding/json" + + "github.com/golang/glog" + "k8s.io/api/admission/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + addFirstLabelPatch string = `[ + { "op": "add", "path": "/metadata/labels", "value": {"added-label": "yes"}} + ]` + addAdditionalLabelPatch string = `[ + { "op": "add", "path": "/metadata/labels/added-label", "value": "yes" } + ]` +) + +// Add a label {"added-label": "yes"} to the object +func addLabel(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { + glog.V(2).Info("calling add-label") + obj := struct { + metav1.ObjectMeta + Data map[string]string + }{} + raw := ar.Request.Object.Raw + err := json.Unmarshal(raw, &obj) + if err != nil { + glog.Error(err) + return toAdmissionResponse(err) + } + + reviewResponse := v1beta1.AdmissionResponse{} + reviewResponse.Allowed = true + if len(obj.ObjectMeta.Labels) == 0 { + reviewResponse.Patch = []byte(addFirstLabelPatch) + } else { + reviewResponse.Patch = []byte(addAdditionalLabelPatch) + } + pt := v1beta1.PatchTypeJSONPatch + reviewResponse.PatchType = &pt + return &reviewResponse +} diff --git a/test/images/webhook/main.go b/test/images/webhook/main.go index aca37e46abf..37c0a1de8a9 100644 --- a/test/images/webhook/main.go +++ b/test/images/webhook/main.go @@ -95,6 +95,10 @@ func serveAlwaysDeny(w http.ResponseWriter, r *http.Request) { serve(w, r, alwaysDeny) } +func serveAddLabel(w http.ResponseWriter, r *http.Request) { + serve(w, r, addLabel) +} + func servePods(w http.ResponseWriter, r *http.Request) { serve(w, r, admitPods) } @@ -133,6 +137,7 @@ func main() { flag.Parse() http.HandleFunc("/always-deny", serveAlwaysDeny) + http.HandleFunc("/add-label", serveAddLabel) http.HandleFunc("/pods", servePods) http.HandleFunc("/pods/attach", serveAttachingPods) http.HandleFunc("/mutating-pods", serveMutatePods) diff --git a/test/utils/image/manifest.go b/test/utils/image/manifest.go index 8ca05a89ad4..cbc174b2265 100644 --- a/test/utils/image/manifest.go +++ b/test/utils/image/manifest.go @@ -82,7 +82,7 @@ var ( PrivateRegistry = registry.PrivateRegistry sampleRegistry = registry.SampleRegistry - AdmissionWebhook = ImageConfig{e2eRegistry, "webhook", "1.12v2"} + AdmissionWebhook = ImageConfig{e2eRegistry, "webhook", "1.13v1"} APIServer = ImageConfig{e2eRegistry, "sample-apiserver", "1.0"} AppArmorLoader = ImageConfig{e2eRegistry, "apparmor-loader", "1.0"} BusyBox = ImageConfig{dockerLibraryRegistry, "busybox", "1.29"}