From d4958c0beec34a36af28d019358f686785bb459b Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 20 Aug 2019 14:30:01 -0700 Subject: [PATCH 1/3] Remove all skips from admission and CRD e2e tests that will be promoted to conformance --- test/e2e/apimachinery/crd_conversion_webhook.go | 6 ------ test/e2e/apimachinery/crd_publish_openapi.go | 8 +------- test/e2e/apimachinery/crd_watch.go | 2 -- test/e2e/apimachinery/custom_resource_definition.go | 3 --- test/e2e/apimachinery/webhook.go | 12 ------------ 5 files changed, 1 insertion(+), 30 deletions(-) diff --git a/test/e2e/apimachinery/crd_conversion_webhook.go b/test/e2e/apimachinery/crd_conversion_webhook.go index 551cb095c61..3af639c12d3 100644 --- a/test/e2e/apimachinery/crd_conversion_webhook.go +++ b/test/e2e/apimachinery/crd_conversion_webhook.go @@ -30,7 +30,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/intstr" - utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/client-go/dynamic" clientset "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/test/e2e/framework" @@ -54,8 +53,6 @@ const ( roleBindingCRDName = "crd-conversion-webhook-auth-reader" ) -var serverCRDConversionWebhookVersion = utilversion.MustParseSemantic("v1.13.0-alpha") - var apiVersions = []apiextensionsv1.CustomResourceDefinitionVersion{ { Name: "v1", @@ -129,9 +126,6 @@ var _ = SIGDescribe("CustomResourceConversionWebhook", func() { client = f.ClientSet namespaceName = f.Namespace.Name - // Make sure the relevant provider supports conversion webhook - framework.SkipUnlessServerVersionGTE(serverCRDConversionWebhookVersion, f.ClientSet.Discovery()) - ginkgo.By("Setting up server cert") context = setupServerCert(f.Namespace.Name, serviceCRDName) createAuthReaderRoleBindingForCRDConversion(f, f.Namespace.Name) diff --git a/test/e2e/apimachinery/crd_publish_openapi.go b/test/e2e/apimachinery/crd_publish_openapi.go index b285679db54..6dce7a38d59 100644 --- a/test/e2e/apimachinery/crd_publish_openapi.go +++ b/test/e2e/apimachinery/crd_publish_openapi.go @@ -35,7 +35,6 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" k8sclientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -47,17 +46,12 @@ import ( ) var ( - crdPublishOpenAPIVersion = utilversion.MustParseSemantic("v1.14.0") - metaPattern = `"kind":"%s","apiVersion":"%s/%s","metadata":{"name":"%s"}` + metaPattern = `"kind":"%s","apiVersion":"%s/%s","metadata":{"name":"%s"}` ) var _ = SIGDescribe("CustomResourcePublishOpenAPI", func() { f := framework.NewDefaultFramework("crd-publish-openapi") - ginkgo.BeforeEach(func() { - framework.SkipUnlessServerVersionGTE(crdPublishOpenAPIVersion, f.ClientSet.Discovery()) - }) - ginkgo.It("works for CRD with validation schema", func() { crd, err := setupCRD(f, schemaFoo, "foo", "v1") if err != nil { diff --git a/test/e2e/apimachinery/crd_watch.go b/test/e2e/apimachinery/crd_watch.go index 5162dd82553..be08b2a00b1 100644 --- a/test/e2e/apimachinery/crd_watch.go +++ b/test/e2e/apimachinery/crd_watch.go @@ -46,8 +46,6 @@ var _ = SIGDescribe("CustomResourceDefinition Watch", func() { */ ginkgo.It("watch on custom resource definition objects", func() { - framework.SkipUnlessServerVersionGTE(crdVersion, f.ClientSet.Discovery()) - const ( watchCRNameA = "name1" watchCRNameB = "name2" diff --git a/test/e2e/apimachinery/custom_resource_definition.go b/test/e2e/apimachinery/custom_resource_definition.go index 70f23c3d02e..f5b7344d623 100644 --- a/test/e2e/apimachinery/custom_resource_definition.go +++ b/test/e2e/apimachinery/custom_resource_definition.go @@ -29,15 +29,12 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/uuid" - utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/client-go/dynamic" "k8s.io/client-go/util/retry" "k8s.io/kubernetes/test/e2e/framework" e2elog "k8s.io/kubernetes/test/e2e/framework/log" ) -var crdVersion = utilversion.MustParseSemantic("v1.7.0") - var _ = SIGDescribe("CustomResourceDefinition resources", func() { f := framework.NewDefaultFramework("custom-resource-definition") diff --git a/test/e2e/apimachinery/webhook.go b/test/e2e/apimachinery/webhook.go index a6a43d745ef..2ee7f9afc54 100644 --- a/test/e2e/apimachinery/webhook.go +++ b/test/e2e/apimachinery/webhook.go @@ -36,7 +36,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/uuid" - utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/dynamic" clientset "k8s.io/client-go/kubernetes" @@ -77,8 +76,6 @@ const ( addedLabelValue = "yes" ) -var serverWebhookVersion = utilversion.MustParseSemantic("v1.8.0") - var _ = SIGDescribe("AdmissionWebhook", func() { var context *certContext f := framework.NewDefaultFramework("webhook") @@ -92,15 +89,6 @@ var _ = SIGDescribe("AdmissionWebhook", func() { client = f.ClientSet namespaceName = f.Namespace.Name - // Make sure the relevant provider supports admission webhook - framework.SkipUnlessServerVersionGTE(serverWebhookVersion, f.ClientSet.Discovery()) - framework.SkipUnlessProviderIs("gce", "gke", "local") - - _, err := f.ClientSet.AdmissionregistrationV1().ValidatingWebhookConfigurations().List(metav1.ListOptions{}) - if errors.IsNotFound(err) { - framework.Skipf("dynamic configuration of webhooks requires the admissionregistration.k8s.io group to be enabled") - } - // Make sure the namespace created for the test is labeled to be selected by the webhooks labelNamespace(f, f.Namespace.Name) From 363d1fec085930b29fb3059fec612cb6b4daaa53 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Thu, 22 Aug 2019 01:59:31 -0700 Subject: [PATCH 2/3] Switch to v1 for admission and CRDs for all api-machinery e2e tests --- .../test/integration/fixtures/resources.go | 32 ++++++++++++ test/e2e/apimachinery/BUILD | 1 - .../custom_resource_definition.go | 50 +++++++++---------- test/e2e/apimachinery/garbage_collector.go | 26 ++++++---- 4 files changed, 72 insertions(+), 37 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/resources.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/resources.go index 3f0c27acb25..0775971df58 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/resources.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/resources.go @@ -43,6 +43,38 @@ const ( noxuInstanceNum int64 = 9223372036854775807 ) +// NewRandomNameV1CustomResourceDefinition generates a CRD with random name to avoid name conflict in e2e tests +func NewRandomNameV1CustomResourceDefinition(scope apiextensionsv1.ResourceScope) *apiextensionsv1.CustomResourceDefinition { + // ensure the singular doesn't end in an s for now + gName := names.SimpleNameGenerator.GenerateName("foo") + "a" + return &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: gName + "s.mygroup.example.com"}, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: "mygroup.example.com", + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ + { + Name: "v1beta1", + Served: true, + Storage: true, + Schema: &apiextensionsv1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ + XPreserveUnknownFields: pointer.BoolPtr(true), + Type: "object", + }, + }, + }, + }, + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: gName + "s", + Singular: gName, + Kind: gName, + ListKind: gName + "List", + }, + Scope: scope, + }, + } +} + // NewRandomNameCustomResourceDefinition generates a CRD with random name to avoid name conflict in e2e tests func NewRandomNameCustomResourceDefinition(scope apiextensionsv1beta1.ResourceScope) *apiextensionsv1beta1.CustomResourceDefinition { // ensure the singular doesn't end in an s for now diff --git a/test/e2e/apimachinery/BUILD b/test/e2e/apimachinery/BUILD index 876cfcfc700..f16551e6f6b 100644 --- a/test/e2e/apimachinery/BUILD +++ b/test/e2e/apimachinery/BUILD @@ -42,7 +42,6 @@ go_library( "//staging/src/k8s.io/api/scheduling/v1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library", - "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/test/integration:go_default_library", diff --git a/test/e2e/apimachinery/custom_resource_definition.go b/test/e2e/apimachinery/custom_resource_definition.go index f5b7344d623..11a9cd2ea3e 100644 --- a/test/e2e/apimachinery/custom_resource_definition.go +++ b/test/e2e/apimachinery/custom_resource_definition.go @@ -19,7 +19,7 @@ package apimachinery import ( "github.com/onsi/ginkgo" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" "k8s.io/apiextensions-apiserver/test/integration/fixtures" "k8s.io/apimachinery/pkg/api/equality" @@ -52,14 +52,14 @@ var _ = SIGDescribe("CustomResourceDefinition resources", func() { apiExtensionClient, err := clientset.NewForConfig(config) framework.ExpectNoError(err, "initializing apiExtensionClient") - randomDefinition := fixtures.NewRandomNameCustomResourceDefinition(v1beta1.ClusterScoped) + randomDefinition := fixtures.NewRandomNameV1CustomResourceDefinition(v1.ClusterScoped) // Create CRD and waits for the resource to be recognized and available. - randomDefinition, err = fixtures.CreateNewCustomResourceDefinition(randomDefinition, apiExtensionClient, f.DynamicClient) + randomDefinition, err = fixtures.CreateNewV1CustomResourceDefinition(randomDefinition, apiExtensionClient, f.DynamicClient) framework.ExpectNoError(err, "creating CustomResourceDefinition") defer func() { - err = fixtures.DeleteCustomResourceDefinition(randomDefinition, apiExtensionClient) + err = fixtures.DeleteV1CustomResourceDefinition(randomDefinition, apiExtensionClient) framework.ExpectNoError(err, "deleting CustomResourceDefinition") }() }) @@ -80,30 +80,30 @@ var _ = SIGDescribe("CustomResourceDefinition resources", func() { testUUID := string(uuid.NewUUID()) // Create CRD and wait for the resource to be recognized and available. - crds := make([]*v1beta1.CustomResourceDefinition, testListSize) + crds := make([]*v1.CustomResourceDefinition, testListSize) for i := 0; i < testListSize; i++ { - crd := fixtures.NewRandomNameCustomResourceDefinition(v1beta1.ClusterScoped) + crd := fixtures.NewRandomNameV1CustomResourceDefinition(v1.ClusterScoped) crd.Labels = map[string]string{"e2e-list-test-uuid": testUUID} - crd, err = fixtures.CreateNewCustomResourceDefinition(crd, apiExtensionClient, f.DynamicClient) + crd, err = fixtures.CreateNewV1CustomResourceDefinition(crd, apiExtensionClient, f.DynamicClient) framework.ExpectNoError(err, "creating CustomResourceDefinition") crds[i] = crd } // Create a crd w/o the label to ensure the label selector matching works correctly - crd := fixtures.NewRandomNameCustomResourceDefinition(v1beta1.ClusterScoped) - crd, err = fixtures.CreateNewCustomResourceDefinition(crd, apiExtensionClient, f.DynamicClient) + crd := fixtures.NewRandomNameV1CustomResourceDefinition(v1.ClusterScoped) + crd, err = fixtures.CreateNewV1CustomResourceDefinition(crd, apiExtensionClient, f.DynamicClient) framework.ExpectNoError(err, "creating CustomResourceDefinition") defer func() { - err = fixtures.DeleteCustomResourceDefinition(crd, apiExtensionClient) + err = fixtures.DeleteV1CustomResourceDefinition(crd, apiExtensionClient) framework.ExpectNoError(err, "deleting CustomResourceDefinition") }() selectorListOpts := metav1.ListOptions{LabelSelector: "e2e-list-test-uuid=" + testUUID} - list, err := apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().List(selectorListOpts) + list, err := apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().List(selectorListOpts) framework.ExpectNoError(err, "listing CustomResourceDefinitions") framework.ExpectEqual(len(list.Items), testListSize) for _, actual := range list.Items { - var expected *v1beta1.CustomResourceDefinition + var expected *v1.CustomResourceDefinition for _, e := range crds { if e.Name == actual.Name && e.Namespace == actual.Namespace { expected = e @@ -119,7 +119,7 @@ var _ = SIGDescribe("CustomResourceDefinition resources", func() { // Use delete collection to remove the CRDs err = fixtures.DeleteCustomResourceDefinitions(selectorListOpts, apiExtensionClient) framework.ExpectNoError(err, "deleting CustomResourceDefinitions") - _, err = apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(crd.Name, metav1.GetOptions{}) + _, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(crd.Name, metav1.GetOptions{}) framework.ExpectNoError(err, "getting remaining CustomResourceDefinition") }) @@ -135,20 +135,20 @@ var _ = SIGDescribe("CustomResourceDefinition resources", func() { framework.ExpectNoError(err, "initializing apiExtensionClient") dynamicClient, err := dynamic.NewForConfig(config) framework.ExpectNoError(err, "initializing dynamic client") - gvr := v1beta1.SchemeGroupVersion.WithResource("customresourcedefinitions") + gvr := v1.SchemeGroupVersion.WithResource("customresourcedefinitions") resourceClient := dynamicClient.Resource(gvr) // Create CRD and waits for the resource to be recognized and available. - crd := fixtures.NewRandomNameCustomResourceDefinition(v1beta1.ClusterScoped) - crd, err = fixtures.CreateNewCustomResourceDefinition(crd, apiExtensionClient, f.DynamicClient) + crd := fixtures.NewRandomNameV1CustomResourceDefinition(v1.ClusterScoped) + crd, err = fixtures.CreateNewV1CustomResourceDefinition(crd, apiExtensionClient, f.DynamicClient) framework.ExpectNoError(err, "creating CustomResourceDefinition") defer func() { - err = fixtures.DeleteCustomResourceDefinition(crd, apiExtensionClient) + err = fixtures.DeleteV1CustomResourceDefinition(crd, apiExtensionClient) framework.ExpectNoError(err, "deleting CustomResourceDefinition") }() - var updated *v1beta1.CustomResourceDefinition - updateCondition := v1beta1.CustomResourceDefinitionCondition{Message: "updated"} + var updated *v1.CustomResourceDefinition + updateCondition := v1.CustomResourceDefinitionCondition{Message: "updated"} err = retry.RetryOnConflict(retry.DefaultRetry, func() error { // Use dynamic client to read the status sub-resource since typed client does not expose it. u, err := resourceClient.Get(crd.GetName(), metav1.GetOptions{}, "status") @@ -158,14 +158,14 @@ var _ = SIGDescribe("CustomResourceDefinition resources", func() { e2elog.Failf("Expected CustomResourceDefinition Spec to match status sub-resource Spec, but got:\n%s", diff.ObjectReflectDiff(status.Spec, crd.Spec)) } status.Status.Conditions = append(status.Status.Conditions, updateCondition) - updated, err = apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().UpdateStatus(status) + updated, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().UpdateStatus(status) return err }) framework.ExpectNoError(err, "updating CustomResourceDefinition status") expectCondition(updated.Status.Conditions, updateCondition) - patchCondition := v1beta1.CustomResourceDefinitionCondition{Message: "patched"} - patched, err := apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Patch( + patchCondition := v1.CustomResourceDefinitionCondition{Message: "patched"} + patched, err := apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Patch( crd.GetName(), types.JSONPatchType, []byte(`[{"op": "add", "path": "/status/conditions", "value": [{"message": "patched"}]}]`), @@ -177,14 +177,14 @@ var _ = SIGDescribe("CustomResourceDefinition resources", func() { }) }) -func unstructuredToCRD(obj *unstructured.Unstructured) *v1beta1.CustomResourceDefinition { - crd := new(v1beta1.CustomResourceDefinition) +func unstructuredToCRD(obj *unstructured.Unstructured) *v1.CustomResourceDefinition { + crd := new(v1.CustomResourceDefinition) err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, crd) framework.ExpectNoError(err, "converting unstructured to CustomResourceDefinition") return crd } -func expectCondition(conditions []v1beta1.CustomResourceDefinitionCondition, expected v1beta1.CustomResourceDefinitionCondition) { +func expectCondition(conditions []v1.CustomResourceDefinitionCondition, expected v1.CustomResourceDefinitionCondition) { for _, c := range conditions { if equality.Semantic.DeepEqual(c, expected) { return diff --git a/test/e2e/apimachinery/garbage_collector.go b/test/e2e/apimachinery/garbage_collector.go index 04d39c17afb..7d00009aa04 100644 --- a/test/e2e/apimachinery/garbage_collector.go +++ b/test/e2e/apimachinery/garbage_collector.go @@ -25,7 +25,7 @@ import ( batchv1 "k8s.io/api/batch/v1" batchv1beta1 "k8s.io/api/batch/v1beta1" "k8s.io/api/core/v1" - apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" apiextensionstestserver "k8s.io/apiextensions-apiserver/test/integration/fixtures" "k8s.io/apimachinery/pkg/api/errors" @@ -876,23 +876,25 @@ var _ = SIGDescribe("Garbage collector", func() { // Create a random custom resource definition and ensure it's available for // use. - definition := apiextensionstestserver.NewRandomNameCustomResourceDefinition(apiextensionsv1beta1.ClusterScoped) + definition := apiextensionstestserver.NewRandomNameV1CustomResourceDefinition(apiextensionsv1.ClusterScoped) defer func() { - err = apiextensionstestserver.DeleteCustomResourceDefinition(definition, apiExtensionClient) + err = apiextensionstestserver.DeleteV1CustomResourceDefinition(definition, apiExtensionClient) if err != nil && !errors.IsNotFound(err) { e2elog.Failf("failed to delete CustomResourceDefinition: %v", err) } }() - definition, err = apiextensionstestserver.CreateNewCustomResourceDefinition(definition, apiExtensionClient, f.DynamicClient) + definition, err = apiextensionstestserver.CreateNewV1CustomResourceDefinition(definition, apiExtensionClient, f.DynamicClient) if err != nil { e2elog.Failf("failed to create CustomResourceDefinition: %v", err) } + framework.ExpectEqual(len(definition.Spec.Versions), 1, "custom resource definition should have one version") + version := definition.Spec.Versions[0] // Get a client for the custom resource. - gvr := schema.GroupVersionResource{Group: definition.Spec.Group, Version: definition.Spec.Version, Resource: definition.Spec.Names.Plural} + gvr := schema.GroupVersionResource{Group: definition.Spec.Group, Version: version.Name, Resource: definition.Spec.Names.Plural} resourceClient := f.DynamicClient.Resource(gvr) - apiVersion := definition.Spec.Group + "/" + definition.Spec.Version + apiVersion := definition.Spec.Group + "/" + version.Name // Create a custom owner resource. ownerName := names.SimpleNameGenerator.GenerateName("owner") @@ -977,23 +979,25 @@ var _ = SIGDescribe("Garbage collector", func() { // Create a random custom resource definition and ensure it's available for // use. - definition := apiextensionstestserver.NewRandomNameCustomResourceDefinition(apiextensionsv1beta1.ClusterScoped) + definition := apiextensionstestserver.NewRandomNameV1CustomResourceDefinition(apiextensionsv1.ClusterScoped) defer func() { - err = apiextensionstestserver.DeleteCustomResourceDefinition(definition, apiExtensionClient) + err = apiextensionstestserver.DeleteV1CustomResourceDefinition(definition, apiExtensionClient) if err != nil && !errors.IsNotFound(err) { e2elog.Failf("failed to delete CustomResourceDefinition: %v", err) } }() - definition, err = apiextensionstestserver.CreateNewCustomResourceDefinition(definition, apiExtensionClient, f.DynamicClient) + definition, err = apiextensionstestserver.CreateNewV1CustomResourceDefinition(definition, apiExtensionClient, f.DynamicClient) if err != nil { e2elog.Failf("failed to create CustomResourceDefinition: %v", err) } + framework.ExpectEqual(len(definition.Spec.Versions), 1, "custom resource definition should have one version") + version := definition.Spec.Versions[0] // Get a client for the custom resource. - gvr := schema.GroupVersionResource{Group: definition.Spec.Group, Version: definition.Spec.Version, Resource: definition.Spec.Names.Plural} + gvr := schema.GroupVersionResource{Group: definition.Spec.Group, Version: version.Name, Resource: definition.Spec.Names.Plural} resourceClient := f.DynamicClient.Resource(gvr) - apiVersion := definition.Spec.Group + "/" + definition.Spec.Version + apiVersion := definition.Spec.Group + "/" + version.Name // Create a custom owner resource. ownerName := names.SimpleNameGenerator.GenerateName("owner") From a5bf6e6797575eaebe5de43aed34111c82c1d47c Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Thu, 22 Aug 2019 17:33:45 -0700 Subject: [PATCH 3/3] Replace time.Sleep with poll.wait in admission e2e tests --- test/e2e/apimachinery/webhook.go | 198 ++++++++++++++++++++++++++----- 1 file changed, 170 insertions(+), 28 deletions(-) diff --git a/test/e2e/apimachinery/webhook.go b/test/e2e/apimachinery/webhook.go index 2ee7f9afc54..46eb6269d9d 100644 --- a/test/e2e/apimachinery/webhook.go +++ b/test/e2e/apimachinery/webhook.go @@ -91,6 +91,7 @@ var _ = SIGDescribe("AdmissionWebhook", func() { // Make sure the namespace created for the test is labeled to be selected by the webhooks labelNamespace(f, f.Namespace.Name) + createWebhookConfigurationReadyNamespace(f) ginkgo.By("Setting up server cert") context = setupServerCert(namespaceName, serviceName) @@ -150,7 +151,7 @@ var _ = SIGDescribe("AdmissionWebhook", func() { defer validatingWebhookCleanup() mutatingWebhookCleanup := registerMutatingWebhookForWebhookConfigurations(f, f.UniqueName+"blocking", context, servicePort) defer mutatingWebhookCleanup() - testWebhooksForWebhookConfigurations(f, f.UniqueName, servicePort) + testWebhooksForWebhookConfigurations(f, f.UniqueName, context, servicePort) }) ginkgo.It("Should mutate custom resource", func() { @@ -683,12 +684,15 @@ func registerWebhook(f *framework.Framework, configName string, context *certCon // Server cannot talk to this webhook, so it always fails. // Because this webhook is configured fail-open, request should be admitted after the call fails. failOpenHook, + + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newValidatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(configName, nil) @@ -733,12 +737,14 @@ func registerWebhookForAttachingPod(f *framework.Framework, configName string, c MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newValidatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(configName, nil) @@ -758,12 +764,14 @@ func registerMutatingWebhookForConfigMap(f *framework.Framework, configName stri Webhooks: []admissionregistrationv1.MutatingWebhook{ newMutateConfigMapWebhookFixture(f, context, 1, servicePort), newMutateConfigMapWebhookFixture(f, context, 2, servicePort), + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newMutatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering mutating webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { client.AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(configName, nil) } } @@ -821,12 +829,14 @@ func registerMutatingWebhookForPod(f *framework.Framework, configName string, co MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newMutatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering mutating webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { client.AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(configName, nil) } } @@ -1067,12 +1077,14 @@ func registerFailClosedWebhook(f *framework.Framework, configName string, contex // Server cannot talk to this webhook, so it always fails. // Because this webhook is configured fail-closed, request should be rejected after the call fails. hook, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newValidatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { f.ClientSet.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(configName, nil) } @@ -1151,12 +1163,14 @@ func registerValidatingWebhookForWebhookConfigurations(f *framework.Framework, c MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newValidatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { err := client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(configName, nil) framework.ExpectNoError(err, "deleting webhook config %s with namespace %s", configName, namespace) @@ -1210,12 +1224,14 @@ func registerMutatingWebhookForWebhookConfigurations(f *framework.Framework, con MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newMutatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(configName, nil) framework.ExpectNoError(err, "deleting webhook config %s with namespace %s", configName, namespace) @@ -1225,7 +1241,7 @@ func registerMutatingWebhookForWebhookConfigurations(f *framework.Framework, con // 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, configName string, servicePort int32) { +func testWebhooksForWebhookConfigurations(f *framework.Framework, configName string, context *certContext, servicePort int32) { var err error client := f.ClientSet ginkgo.By("Creating a dummy validating-webhook-configuration object") @@ -1271,6 +1287,8 @@ func testWebhooksForWebhookConfigurations(f *framework.Framework, configName str MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newValidatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) @@ -1278,8 +1296,8 @@ func testWebhooksForWebhookConfigurations(f *framework.Framework, configName str e2elog.Failf("expected %s not to be mutated by mutating webhooks but it was", configName) } - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") ginkgo.By("Deleting the validating-webhook-configuration, which should be possible to remove") @@ -1325,6 +1343,8 @@ func testWebhooksForWebhookConfigurations(f *framework.Framework, configName str MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newMutatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering webhook config %s with namespace %s", configName, namespace) @@ -1332,8 +1352,8 @@ func testWebhooksForWebhookConfigurations(f *framework.Framework, configName str e2elog.Failf("expected %s not to be mutated by mutating webhooks but it was", configName) } - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") ginkgo.By("Deleting the mutating-webhook-configuration, which should be possible to remove") @@ -1542,12 +1562,14 @@ func registerWebhookForCustomResource(f *framework.Framework, configName string, MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newValidatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering custom resource webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(configName, nil) } @@ -1617,12 +1639,14 @@ func registerMutatingWebhookForCustomResource(f *framework.Framework, configName MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newMutatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering custom resource webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { client.AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(configName, nil) } } @@ -1819,12 +1843,14 @@ func registerValidatingWebhookForCRD(f *framework.Framework, configName string, MatchLabels: map[string]string{f.UniqueName: "true"}, }, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newValidatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering crd webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(configName, nil) } @@ -1943,12 +1969,14 @@ func registerSlowWebhook(f *framework.Framework, configName string, context *cer SideEffects: &sideEffectsNone, AdmissionReviewVersions: []string{"v1", "v1beta1"}, }, + // Register a webhook that can be probed by marker requests to detect when the configuration is ready. + newValidatingIsReadyWebhookFixture(f, context, servicePort), }, }) framework.ExpectNoError(err, "registering slow webhook config %s with namespace %s", configName, namespace) - // The webhook configuration is honored in 10s. - time.Sleep(10 * time.Second) + err = waitWebhookConfigurationReady(f) + framework.ExpectNoError(err, "waiting for webhook configuration to be ready") return func() { client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(configName, nil) @@ -2148,3 +2176,117 @@ func newMutateConfigMapWebhookFixture(f *framework.Framework, context *certConte }, } } + +// createWebhookConfigurationReadyNamespace creates a separate namespace for webhook configuration ready markers to +// prevent cross-talk with webhook configurations being tested. +func createWebhookConfigurationReadyNamespace(f *framework.Framework) { + ns, err := f.ClientSet.CoreV1().Namespaces().Create(&v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: f.Namespace.Name + "-markers", + Labels: map[string]string{f.UniqueName + "-markers": "true"}, + }, + }) + framework.ExpectNoError(err, "creating namespace for webhook configuration ready markers") + f.AddNamespacesToDelete(ns) +} + +// waitWebhookConfigurationReady sends "marker" requests until a webhook configuration is ready. +// A webhook created with newValidatingIsReadyWebhookFixture or newMutatingIsReadyWebhookFixture should first be added to +// the webhook configuration. +func waitWebhookConfigurationReady(f *framework.Framework) error { + cmClient := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name + "-markers") + return wait.PollImmediate(100*time.Millisecond, 30*time.Second, func() (bool, error) { + marker := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: string(uuid.NewUUID()), + Labels: map[string]string{ + f.UniqueName: "true", + }, + }, + } + _, err := cmClient.Create(marker) + if err != nil { + // The always-deny webhook does not provide a reason, so check for the error string we expect + if strings.Contains(err.Error(), "denied") { + return true, nil + } + return false, err + } + // best effort cleanup of markers that are no longer needed + _ = cmClient.Delete(marker.GetName(), nil) + framework.Logf("Waiting for webhook configuration to be ready...") + return false, nil + }) +} + +// newValidatingIsReadyWebhookFixture creates a validating webhook that can be added to a webhook configuration and then probed +// with "marker" requests via waitWebhookConfigurationReady to wait for a webhook configuration to be ready. +func newValidatingIsReadyWebhookFixture(f *framework.Framework, context *certContext, servicePort int32) admissionregistrationv1.ValidatingWebhook { + sideEffectsNone := admissionregistrationv1.SideEffectClassNone + return admissionregistrationv1.ValidatingWebhook{ + Name: "validating-is-webhook-configuration-ready.k8s.io", + Rules: []admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"configmaps"}, + }, + }}, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: f.Namespace.Name, + Name: serviceName, + Path: strPtr("/always-deny"), + Port: pointer.Int32Ptr(servicePort), + }, + CABundle: context.signingCert, + }, + SideEffects: &sideEffectsNone, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + // Scope the webhook to just the markers namespace + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{f.UniqueName + "-markers": "true"}, + }, + // appease createValidatingWebhookConfiguration isolation requirements + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{f.UniqueName: "true"}, + }, + } +} + +// newMutatingIsReadyWebhookFixture creates a mutating webhook that can be added to a webhook configuration and then probed +// with "marker" requests via waitWebhookConfigurationReady to wait for a webhook configuration to be ready. +func newMutatingIsReadyWebhookFixture(f *framework.Framework, context *certContext, servicePort int32) admissionregistrationv1.MutatingWebhook { + sideEffectsNone := admissionregistrationv1.SideEffectClassNone + return admissionregistrationv1.MutatingWebhook{ + Name: "mutating-is-webhook-configuration-ready.k8s.io", + Rules: []admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"configmaps"}, + }, + }}, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: f.Namespace.Name, + Name: serviceName, + Path: strPtr("/always-deny"), + Port: pointer.Int32Ptr(servicePort), + }, + CABundle: context.signingCert, + }, + SideEffects: &sideEffectsNone, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + // Scope the webhook to just the markers namespace + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{f.UniqueName + "-markers": "true"}, + }, + // appease createMutatingWebhookConfiguration isolation requirements + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{f.UniqueName: "true"}, + }, + } +}