From 7fee22b7be57d2e5b83717a1329ea7aeac3aa561 Mon Sep 17 00:00:00 2001 From: jennybuckley Date: Wed, 11 Apr 2018 14:08:05 -0700 Subject: [PATCH] Fix flaky crd e2e tests --- test/e2e/apimachinery/webhook.go | 28 ++++++++++------------- test/images/webhook/BUILD | 1 + test/images/webhook/Makefile | 2 +- test/images/webhook/main.go | 39 ++++++++++++++++++++++++++++++++ test/utils/image/manifest.go | 2 +- 5 files changed, 54 insertions(+), 18 deletions(-) diff --git a/test/e2e/apimachinery/webhook.go b/test/e2e/apimachinery/webhook.go index 14d8a88619..5dbda0239c 100644 --- a/test/e2e/apimachinery/webhook.go +++ b/test/e2e/apimachinery/webhook.go @@ -73,8 +73,6 @@ const ( failNamespaceLabelKey = "fail-closed-webhook" failNamespaceLabelValue = "yes" failNamespaceName = "fail-closed-namesapce" - disallowedCrdLabelKey = "disallowed-crd" - disallowedCrdLabelValue = "yes" ) var serverWebhookVersion = utilversion.MustParseSemantic("v1.8.0") @@ -1139,13 +1137,18 @@ func registerValidatingWebhookForCRD(f *framework.Framework, context *certContex namespace := f.Namespace.Name configName := crdWebhookConfigName + + // This webhook will deny the creation of CustomResourceDefinitions which have the + // label "webhook-e2e-test":"webhook-disallow" + // NOTE: Because tests are run in parallel and in an unpredictable order, it is critical + // that no other test attempts to create CRD with that label. _, err := client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Create(&v1beta1.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: configName, }, Webhooks: []v1beta1.Webhook{ { - Name: "deny-crd.k8s.io", + Name: "deny-crd-with-unwanted-label.k8s.io", Rules: []v1beta1.RuleWithOperations{{ Operations: []v1beta1.OperationType{v1beta1.Create}, Rule: v1beta1.Rule{ @@ -1154,20 +1157,11 @@ func registerValidatingWebhookForCRD(f *framework.Framework, context *certContex Resources: []string{"customresourcedefinitions"}, }, }}, - NamespaceSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: disallowedCrdLabelKey, - Operator: metav1.LabelSelectorOpIn, - Values: []string{disallowedCrdLabelValue}, - }, - }, - }, ClientConfig: v1beta1.WebhookClientConfig{ Service: &v1beta1.ServiceReference{ Namespace: namespace, Name: serviceName, - Path: strPtr("/always-deny"), + Path: strPtr("/crd"), }, CABundle: context.signingCert, }, @@ -1209,8 +1203,10 @@ func testCRDDenyWebhook(f *framework.Framework) { } crd := &apiextensionsv1beta1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ - Name: testcrd.GetMetaName(), - Labels: map[string]string{disallowedCrdLabelKey: disallowedCrdLabelValue}, + Name: testcrd.GetMetaName(), + Labels: map[string]string{ + "webhook-e2e-test": "webhook-disallow", + }, }, Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{ Group: testcrd.ApiGroup, @@ -1228,7 +1224,7 @@ func testCRDDenyWebhook(f *framework.Framework) { // create CRD _, err = apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Create(crd) Expect(err).NotTo(BeNil()) - expectedErrMsg := "this webhook denies all requests" + expectedErrMsg := "the crd contains unwanted label" if !strings.Contains(err.Error(), expectedErrMsg) { framework.Failf("expect error contains %q, got %q", expectedErrMsg, err.Error()) } diff --git a/test/images/webhook/BUILD b/test/images/webhook/BUILD index 09c436d67d..21f24adbee 100644 --- a/test/images/webhook/BUILD +++ b/test/images/webhook/BUILD @@ -14,6 +14,7 @@ go_library( "//vendor/k8s.io/api/admission/v1beta1:go_default_library", "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", diff --git a/test/images/webhook/Makefile b/test/images/webhook/Makefile index 41da8c18aa..cf4009def1 100644 --- a/test/images/webhook/Makefile +++ b/test/images/webhook/Makefile @@ -13,7 +13,7 @@ # limitations under the License. IMAGE = gcr.io/kubernetes-e2e-test-images/k8s-sample-admission-webhook-amd64 -TAG = 1.10v1 +TAG = 1.10v2 build: CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -o webhook . diff --git a/test/images/webhook/main.go b/test/images/webhook/main.go index 7b546ce755..0b715cd60b 100644 --- a/test/images/webhook/main.go +++ b/test/images/webhook/main.go @@ -27,6 +27,7 @@ import ( "github.com/golang/glog" "k8s.io/api/admission/v1beta1" corev1 "k8s.io/api/core/v1" + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" // TODO: try this library to see if it generates correct json patch @@ -259,6 +260,37 @@ func admitCustomResource(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse return &reviewResponse } +// Deny all crds with the label "webhook-e2e-test":"webhook-disallow" +// This function expects all CRDs submitted to it to be apiextensions.k8s.io/v1beta1 +// TODO: When apiextensions.k8s.io/v1 is added we will need to update this function. +func admitCRD(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { + glog.V(2).Info("admitting crd") + crdResource := metav1.GroupVersionResource{Group: "apiextensions.k8s.io", Version: "v1beta1", Resource: "customresourcedefinitions"} + if ar.Request.Resource != crdResource { + err := fmt.Errorf("expect resource to be %s", crdResource) + glog.Error(err) + return toAdmissionResponse(err) + } + + raw := ar.Request.Object.Raw + crd := apiextensionsv1beta1.CustomResourceDefinition{} + deserializer := codecs.UniversalDeserializer() + if _, _, err := deserializer.Decode(raw, nil, &crd); err != nil { + glog.Error(err) + return toAdmissionResponse(err) + } + reviewResponse := v1beta1.AdmissionResponse{} + reviewResponse.Allowed = true + + if v, ok := crd.Labels["webhook-e2e-test"]; ok { + if v == "webhook-disallow" { + reviewResponse.Allowed = false + reviewResponse.Result = &metav1.Status{Message: "the crd contains unwanted label"} + } + } + return &reviewResponse +} + type admitFunc func(v1beta1.AdmissionReview) *v1beta1.AdmissionResponse func serve(w http.ResponseWriter, r *http.Request, admit admitFunc) { @@ -276,6 +308,7 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitFunc) { return } + glog.V(2).Info(fmt.Sprintf("handling request: %v", body)) var reviewResponse *v1beta1.AdmissionResponse ar := v1beta1.AdmissionReview{} deserializer := codecs.UniversalDeserializer() @@ -285,6 +318,7 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitFunc) { } else { reviewResponse = admit(ar) } + glog.V(2).Info(fmt.Sprintf("sending response: %v", reviewResponse)) response := v1beta1.AdmissionReview{} if reviewResponse != nil { @@ -332,6 +366,10 @@ func serveMutateCustomResource(w http.ResponseWriter, r *http.Request) { serve(w, r, mutateCustomResource) } +func serveCRD(w http.ResponseWriter, r *http.Request) { + serve(w, r, admitCRD) +} + func main() { var config Config config.addFlags() @@ -344,6 +382,7 @@ func main() { http.HandleFunc("/mutating-configmaps", serveMutateConfigmaps) http.HandleFunc("/custom-resource", serveCustomResource) http.HandleFunc("/mutating-custom-resource", serveMutateCustomResource) + http.HandleFunc("/crd", serveCRD) clientset := getClient() server := &http.Server{ Addr: ":443", diff --git a/test/utils/image/manifest.go b/test/utils/image/manifest.go index 348dd17266..be9ff8dc65 100644 --- a/test/utils/image/manifest.go +++ b/test/utils/image/manifest.go @@ -48,7 +48,7 @@ func (i *ImageConfig) SetVersion(version string) { } var ( - AdmissionWebhook = ImageConfig{e2eRegistry, "k8s-sample-admission-webhook", "1.10v1", true} + AdmissionWebhook = ImageConfig{e2eRegistry, "k8s-sample-admission-webhook", "1.10v2", true} APIServer = ImageConfig{e2eRegistry, "k8s-aggregator-sample-apiserver", "1.7v2", true} AppArmorLoader = ImageConfig{gcRegistry, "apparmor-loader", "0.1", false} BusyBox = ImageConfig{gcRegistry, "busybox", "1.24", false}